Skip to content

sipeed-slogic-analyzer: Add Support for Sipeed SLogic Series, including SLogic16U3 5Gbps#262

Closed
taorye wants to merge 0 commit intosigrokproject:masterfrom
sipeed:master
Closed

sipeed-slogic-analyzer: Add Support for Sipeed SLogic Series, including SLogic16U3 5Gbps#262
taorye wants to merge 0 commit intosigrokproject:masterfrom
sipeed:master

Conversation

@taorye
Copy link

@taorye taorye commented Apr 9, 2025

Building upon the foundation established in #212, this PR delivers:

  • Legacy Hardware Maintenance (Slogic Combo 8)

    • support stable 8ch@40MHz sampling on Windows as well as Linux
  • New Hardware Support (Slogic16U3: USB3.2 Gen1 5Gbps)

    • Initial Slogic16U3 driver implementation

    • 200MHz/16ch mode based on max 430MiB/s throughput

@taorye
Copy link
Author

taorye commented Apr 9, 2025

We are preparing to send a batch of patches to the sigrok mailing list. This PR serves as a record for tracking purposes. After the patches are merged upstream, we will proactively close this PR and notify everyone. Of course, before merging, everyone is welcome to pull and test these changes. We will promptly respond to any questions or issues raised.

@axel-h
Copy link

axel-h commented Apr 9, 2025

Thanks. May I ask when will SlogicBasic16U3 be available?

@Zepan
Copy link

Zepan commented Apr 9, 2025

It will open preorder in the last week of April, and start ship before June.
And maybe we change name to SLogic16U3? it is shorter to spell ^_^

@taorye taorye changed the title sipeed-slogic-analyzer: Add Support for Sipeed Slogic Series, including SlogicBasic16U3 5Gbps sipeed-slogic-analyzer: Add Support for Sipeed Slogic Series, including Slogic16U3 5Gbps Apr 10, 2025
@taorye taorye changed the title sipeed-slogic-analyzer: Add Support for Sipeed Slogic Series, including Slogic16U3 5Gbps sipeed-slogic-analyzer: Add Support for Sipeed SLogic Series, including SLogic16U3 5Gbps Apr 10, 2025
@taorye
Copy link
Author

taorye commented Apr 11, 2025

  • Email submission confirmed: Message #59171619
  • Next steps: Awaiting maintainer feedback

Copy link
Contributor

@StefanBruens StefanBruens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commits should be cleaned up and squashed as appropriate.

There is a lot of code cleanup necessary.

The code for SLogic 8 and SLogic16U3 should be in separate commits.

Comment on lines +255 to +305
do {
struct libusb_transfer *transfer = libusb_alloc_transfer(0);
if (!transfer) {
sr_err("Failed to allocate libusb transfer!");
return SR_ERR_IO;
}
do {
devc->per_transfer_nbytes = (devc->per_transfer_nbytes + (2*16*1024-1)) & ~(2*16*1024-1);
devc->per_transfer_duration = devc->per_transfer_nbytes * SR_KHZ(1) * 8 / (devc->cur_samplerate * devc->cur_samplechannel);
sr_dbg("Plan to receive %u bytes per %ums...", devc->per_transfer_nbytes, devc->per_transfer_duration);

uint8_t *dev_buf = g_malloc(devc->per_transfer_nbytes);
if (!dev_buf) {
sr_dbg("Failed to allocate memory: %u bytes! Half .", devc->per_transfer_nbytes);
devc->per_transfer_nbytes >>= 1;
continue;
}

libusb_fill_bulk_transfer(transfer, usb->devhdl, devc->model->ep_in,
dev_buf, devc->per_transfer_nbytes, NULL,
NULL, 0);

ret = libusb_submit_transfer(transfer);
if (ret) {
g_free(transfer->buffer);
if (ret == LIBUSB_ERROR_NO_MEM) {
sr_dbg("Failed to submit transfer: %s!", libusb_error_name(ret));
devc->per_transfer_nbytes >>= 1;
continue;
} else {
sr_err("Failed to submit transfer: %s!", libusb_error_name(ret));
libusb_free_transfer(transfer);
return SR_ERR_IO;
}
} else {
ret = libusb_cancel_transfer(transfer);
if (ret) {
sr_dbg("Failed to cancel transfer: %s!", libusb_error_name(ret));
}
libusb_handle_events_timeout_completed(drvc->sr_ctx->libusb_ctx, &(struct timeval){3, 0}, NULL);
g_free(transfer->buffer);

devc->per_transfer_nbytes >>= 1;
devc->per_transfer_duration = devc->per_transfer_nbytes / (devc->cur_samplerate / SR_KHZ(1) * devc->cur_samplechannel / 8);
break;
}
} while (devc->per_transfer_nbytes > 32*1024); // 32kiB > 125ms * 1MHZ * 2ch
libusb_free_transfer(transfer);
sr_info("Nice plan! :) => %u bytes per %ums.", devc->per_transfer_nbytes, devc->per_transfer_duration);
} while (0);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should go into a separate function, with a name telling what this code block is doing (i.e. probe the maximum transfer size).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, I will refactor them soon.
THANK YOU for your advise!

Comment on lines +292 to +294
sr_dbg("Failed to cancel transfer: %s!", libusb_error_name(ret));
}
libusb_handle_events_timeout_completed(drvc->sr_ctx->libusb_ctx, &(struct timeval){3, 0}, NULL);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad indentation.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should remember to format them first then.
Thanks!

Comment on lines +107 to +121
static inline void clear_ep(const struct sr_dev_inst *sdi) {
struct dev_context *devc = sdi->priv;
struct sr_usb_dev_inst *usb = sdi->conn;
uint8_t ep = devc->model->ep_in;

size_t tmp_size = 4 * 1024 * 1024;
uint8_t *tmp = malloc(tmp_size);
int actual_length = 0;
do {
libusb_bulk_transfer(usb->devhdl, ep,
tmp, tmp_size, &actual_length, 100);
} while (actual_length);
free(tmp);
sr_dbg("Cleared EP: 0x%02x", ep);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be moved to the api.c file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

#define TRANSFERS_DURATION_TOLERANCE 0.05f

enum {
PATTERN_MODE_NOMAL,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'normal'

Comment on lines +148 to +154
if (devc->num_transfers_used) {
for (size_t i = 0; i < NUM_MAX_TRANSFERS; ++i) {
struct libusb_transfer *transfer = devc->transfers[i];
if (transfer) {
libusb_cancel_transfer(transfer);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch leaks all uncompleted transfers.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, if (devc->raw_data_queue == NULL) is not enough to ensure transfers to be freed.
I'll add some restrictive conditions to guarantee cleanup.

Comment on lines +118 to +121
#define GEN_PATTERN(P) [P] = #P
GEN_PATTERN(PATTERN_MODE_NOMAL),
GEN_PATTERN(PATTERN_MODE_TEST_MAX_SPEED),
#undef GEN_PATTERN
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define GEN_PATTERN(P) [P] = #P
GEN_PATTERN(PATTERN_MODE_NOMAL),
GEN_PATTERN(PATTERN_MODE_TEST_MAX_SPEED),
#undef GEN_PATTERN
[PATTERN_MODE_NORMAL] = "Normal",
[PATTERN_MODE_TEST_MAX_SPEED] = "Test_Max_Speed",

{
size_t retry = 0;
memset(cmd_aux, 0, sizeof(cmd_aux));
*(uint32_t*)(cmd_aux) = 0x00000001;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not portable.


slogic_usb_control_write(sdi, SLOGIC16U3_CONTROL_OUT_REQ_REG_WRITE, SLOGIC16U3_R32_CTRL, 0x0000, ARRAY_AND_SIZE(cmd_derst), 500);

{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split into separate function.

}


{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split into separate function.

if (!data && len) {
sr_warn("%s: Nothing to write although len(%u)>0!", __func__, len);
len = 0;
} else if (len & 0x3) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, this has to be a hard error, you will do an out-of-bounds read (or even write below).

@dovandung
Copy link

Please add Trigger function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants