Add Sipeed SLogic Analyzer Driver Support#275
Add Sipeed SLogic Analyzer Driver Support#275taorye wants to merge 22 commits intosigrokproject:masterfrom
Conversation
|
This pull request is the third submission (see previous: #212, #262) for integrating support for the Sipeed SLogic series logic analyzers into libsigrok. The prior two PRs were interrupted before merging; this version has been fully refactored and thoroughly validated. The driver has been thoroughly tested and confirmed to work correctly on Windows, Linux, and macOS platforms! We sincerely invite everyone to pull and test this branch! Your feedback and suggestions are highly appreciated—we are committed to responding actively and working together to further improve this integration. |
|
I'm not qualified to give a real OK for this code, but in reading it through it appears well designed and easy to understand. |
|
Hi! @PetteriAimonen Thanks for looking at it. I'm actually surprised how short and easy the driver is. I've received SLogic Combo8 yesterday, I plan to have a look at the driver over the weekend. |
|
@Krakonos I manage to compile libsigrok with @taorye's pull request. Both PulseView and sigrok-cli can successfully detect the Slogic Combo8. However, there seems to be a limitation with the sampling rate. Using the default rate of 40M consistently results in a timeout error. I haven’t encountered this issue with the PulseView version provided on Sipeed’s website. |
|
@weilking Interesting. I've been using the SLogic Combo 8 for a bit here to work on a project and didn't find any issues so far. Just to be sure, i retested at 40MHz and can sample for pretty long runs without any issues (tested up to 1G at 40MHz). However, I'm running linux. I wonder if this relates to the issues discussed in PRs #242 and #235 ? |
|
@weilking Just noticed the timeout is over a million seconds timeout. That is suspicious. Just to check, are you building a 64bit version of sigrok? |
|
Ok, I'm now able to reproduce something. Maybe similar. I took some doing: ... And the last 5 lines basically repeat forever until I cancel manually the transfer (using pulseview). I had to set up buffer size 2 and sample rate to 160MHz. Enough samples need to be taken. It work up to 1Msamples and a bit, 10Msamples reliably fails on my regular USB hub. However, it works fine plugged directly into the root hub. A classic. Now, I think it's fine the transfer fails due to not enough bandwidth, however I think it should do so more gracefully. @taorye Can you check this out? While I don't see this as a show stopper, since a lot of analyzers fail due to not enough bandwidth. However, what is weird (at least in my case above) is that the code still tries to read more data, it just doesn't receive any and loops forever. |
|
Libusb has had performance issues on Windows for a long time now. At least at some point, sigrok was using a patched libusb for Windows builds. It would be worth it to compare the libusb-1.0.dll versions between the working and not-working build. The Windows properties window should show the version information. |
|
@PetteriAimonen I see. Do you mean the RAW_IO patch? After looking into a chain of rejected patches, it seems this summer one was finally merged: libusb/libusb#1512 , merged in commit: This would be a nice addition if somebody wanted to contribute to the windows support. |
Libsigrok was built with x86_64. |
|
I received my Slogic16U3 today and have built this version of libsigrok to use with it. I primarilly run on Linux Mint, but also have access to Fedora and Windows 11. I have been noticing on my inital tests that running the device at higher sample rates and higher numbers of samples is causing data to get stretched, seems to only be a problem at 800 MHz and with multiple channels enabled. If I drop down to 1 channel from 2 it gets more reliable.
Please let me know any ways I can help this project! |
|
@tykosa I wonder if you tested with the last two fixed pushed by the author? Reading those I think they might be related. |
|
Aside from a lot of GCC warnings (signed vs. unsigned comparisons, Patch diff --git a/src/hardware/sipeed-slogic-analyzer/api.c b/src/hardware/sipeed-slogic-analyzer/api.c
index 90224259f..a39681b43 100644
--- a/src/hardware/sipeed-slogic-analyzer/api.c
+++ b/src/hardware/sipeed-slogic-analyzer/api.c
@@ -903,6 +903,8 @@ static int slogic16U3_remote_test_mode(const struct sr_dev_inst *sdi, uint32_t m
sr_dbg("Succeed to configure test_mode.");
}
}
+
+ return SR_OK;
}
static int slogic16U3_remote_reset(const struct sr_dev_inst *sdi) { |
|
The one big remaining issue is that you can enable more channels that specified in buffer_size and have garbage in Patch diff --git a/src/hardware/sipeed-slogic-analyzer/api.c b/src/hardware/sipeed-slogic-analyzer/api.c
index 737a53a17..798a0025d 100644
--- a/src/hardware/sipeed-slogic-analyzer/api.c
+++ b/src/hardware/sipeed-slogic-analyzer/api.c
@@ -465,14 +465,10 @@ static int config_set(uint32_t key, GVariant *data,
for (GSList *l = devc->digital_group->channels; l;
l = l->next) {
struct sr_channel *ch = l->data;
- if (ch->type ==
- SR_CHANNEL_LOGIC) { /* Might as well do this now, these
+ if (ch->type ==
+ SR_CHANNEL_LOGIC) { /* Might as well do this now, these
are static. */
- sr_dev_channel_enable(
- ch, (ch->index >=
- devc->cur_samplechannel) ?
- FALSE :
- TRUE);
+ ch->enabled = ch->index >= devc->cur_samplechannel ? FALSE : TRUE;
} else {
sr_warn("devc->digital_group->channels[%u] is not Logic?",
ch->index);
@@ -517,6 +513,42 @@ static int config_set(uint32_t key, GVariant *data,
return ret;
}
+int config_channel_set(const struct sr_dev_inst *sdi, struct sr_channel *ch, unsigned int changes) {
+ struct dev_context *devc = sdi ? (sdi->priv) : NULL;
+ if(!devc || !devc->model || !devc->model->samplechannel_table || !devc->model->limit_samplerate_table){
+ return SR_ERR;
+ }
+
+ if(changes != SR_CHANNEL_SET_ENABLED){
+ return SR_OK;
+ }
+
+ uint64_t new_samplechannel = devc->model->samplechannel_table[0];
+ for (GSList *l = devc->digital_group->channels; l;l = l->next) {
+ struct sr_channel *ch = l->data;
+ if(!ch->enabled || ch->index < new_samplechannel){
+ continue;
+ }
+ for(unsigned int i = 0; i < devc->model->samplerate_table_size; i++){
+ if(devc->model->samplechannel_table[i] > ch->index){
+ new_samplechannel = devc->model->samplechannel_table[i];
+ break;
+ }
+ }
+ }
+
+ if(new_samplechannel > devc->cur_samplechannel){
+ devc->cur_samplechannel = new_samplechannel;
+ devc->limit_samplerate = devc->model->limit_samplerate_table[
+ std_i32_idx(g_variant_new_int32(devc->cur_samplechannel),
+ devc->model->samplechannel_table, devc->model->samplechannel_table_size)
+ ];
+ if (devc->cur_samplerate > devc->limit_samplerate)
+ devc->cur_samplerate = devc->limit_samplerate;
+ }
+ return SR_OK;
+}
+
static int config_list(uint32_t key, GVariant **data,
const struct sr_dev_inst *sdi,
const struct sr_channel_group *cg)
@@ -572,6 +604,7 @@ static struct sr_dev_driver sipeed_slogic_analyzer_driver_info = {
.scan = scan,
.dev_list = std_dev_list,
.dev_clear = std_dev_clear,
+ .config_channel_set = config_channel_set,
.config_get = config_get,
.config_set = config_set,
.config_list = config_list,So, now if you have |
|
It should be working perfectly fine with the following PulseView changes ( sigrokproject/pulseview#111 )
you can grab commits here (should be equal to comments in this PR, but prefer git version anyway): |
sipeed-slogic-analyzer: Add support for SlogicCombo8 feat: use pattern to control active channels feat: capture data and regroup channels feat: now support max 160Msps(2ch) continous and multi channel sipeed-slogic-analyzer: Add support for SlogicBasic16U3 sipeed-slogic-analyzer: feat: use SR_CONF_BUFFERSIZE to configurate the channel sipeed-slogic-analyzer: feat: at least 2 transfers are pending in parallel, and evaluate transfer latency, also fix cancel_transfer must be handled sipeed-slogic-analyzer: feat: attempt to use function pointers to maintain compatibility with Lite 8 sipeed-slogic-analyzer: fix: slogic lite 8 use ep(0x81) sipeed-slogic-analyzer: fix: stop while has no transfers submitted sipeed-slogic-analyzer: fix: skip sending real stop command for slogic lite 8 sipeed-slogic-analyzer: fix: libusb_handle_events at right time, use `%%` and start from 125ms sipeed-slogic-analyzer: fix: transfers not consumed from the end, so just use NUM_MAX_TRANSFERS instead of num_transfers_used sipeed-slogic-analyzer: chore: add udev rules for slogic sipeed-slogic-analyzer: feat: support sample rate and channel configuration sipeed-slogic-analyzer: feat: separate sr_session_send(sdi, &packet) from receive_transfer handler to ensure usb work continuously sipeed-slogic-analyzer: feat: switch back to 5% tolerance and use a continuous count to determine real timeout sipeed-slogic-analyzer: refactor: simplify and reuse data packet format sipeed-slogic-analyzer: perf: use GThread for raw_data_handle to not distrub handle_events sipeed-slogic-analyzer: fix: aux length starts from bit9 and try to split control write in 4 bytes sipeed-slogic-analyzer: feat: add samplerate configuration support sipeed-slogic-analyzer: fix: select updated samplerate base sipeed-slogic-analyzer: fix: use 4MiB size transfers default to clear previously generated EP data sipeed-slogic-analyzer: refactor: consolidate data submission logic by using `slogic_submit_raw_data` Replace multiple data submission implementations with the common `slogic_submit_raw_data` function to maintain consistency and reduce code duplication. sipeed-slogic-analyzer: Prepare for upstream submission * Implement USB streaming thread isolation - Decouples raw data processing from libusb transfers callback - Enables stable 200MHz sampling (16ch) on SlogicBasic16U3 (max bandwidth: up to 430 MiB/s on test) * Windows-specific enhancements - Resolves USB scheduling bottleneck - Support stable 40MHz sampling (8ch) on SlogicCombo8 - Verified on Win10/Win11 platforms (VMs included) Co-authored-by: Shenzhen Sipeed Team <support@sipeed.com> Co-authored-by: taorye <hongtao@sipeed.com, taorye@outlook.com> sipeed-slogic-analyzer: fix: standardize product name to "SLogic16U3" sipeed-slogic-analyzer: refactor: lift per_transfer_nbytes enumeration into a separate function fix: use libusb_handle_events after to free transfer that submitted in `train_bulk_in_transfer` feat: support to set vref feat: support adjust vref fix: handle libusb_event in a dedicated thread after opening usb device, and set LIBUSB_TRANSFER_FREE_BUFFER. Ensure all transfers can be freed. feat: support test on patten selected tmp: stop sampling after all transfers are freed fix: update samplerates and voltage threshold calculations; handle acquisition abort correctly fix: implement soft trigger logic and manage trigger state in acquisition
…receive_transfer function
|
Thank you for your contribution! Your patch has been merged into our fork, and we truly appreciate the time and effort you put into improving the project. Great work! |
When operating at the full bandwidth of 3.2 Gbps, data loss may occur due to certain USB SuperSpeed host drivers failing to retrieve data in a timely manner, resulting in insufficient effective bandwidth. Therefore:
|
|
Regarding bandwidths and data loss, the real constraint is probably the amount of buffer space available and host latency spikes. Even if the host could transfer e.g. 3 Gbps from a disk drive just fine, it may be unable to do the same with SLogic. I think SLogic has about 64 kB of buffer memory? Therefore to operate at 3.2 Gbps, the host must start new transfers within 160 µs. If the host has larger latency peaks, for example 1 ms, the available bandwidth drops to 0.5 Gbps. It's unfortunate that the available bandwidth drops very quicky for even small amounts of extra latency. |
…timeout calculation in data transfer
fix: add a timeout mechanism that stops acquisition if no data is received for several consecutive transfers
The infinite loop bug has been fixed in this PR. Thanks for testing! |
|
Hi All, thanks for all the work done in this PR (and especially to @weilking and @taorye). I'll do a second pass sometime during the holidays. In the meantime, I wonder if you have the protocol documented somewhere? It's all in the code, but it is somewhat hard to extract the exact protocol and impossible to fully check if the code implements it well. |
|
|
||
| static const struct slogic_model support_models[] = { | ||
| { | ||
| .name = "Sogic Combo 8", |
There was a problem hiding this comment.
@taorye Thank's for your work getting the support upstreamed.
I think there is a small typo here. I don't know whether this warrants a separate commit though.
| .name = "Sogic Combo 8", | |
| .name = "SLogic Combo 8", |
|
Hi! I've done more testing on the PR and found it seems to fix the main issues. I have a couple of reservations:
I don't see an issue with these, but if this is expected and handled properly, I suggest those to be dropped to dbg level.
Overall, I don't see any more issues. I will reiterate it would be nice to have more documentation besides the code itself on how the communication protocol works if possible. Even if it's couple of comments in the driver. |
Ports upstream compilation fixes and forked PKGBUILD









This pull request adds support for the Sipeed SLogic series logic analyzers (SLogic Combo8, SLogic16U3) to libsigrok. Key changes include:
This contribution enables libsigrok to work with Sipeed SLogic logic analyzers, broadening hardware compatibility.