Unbreak libsigrok fx2lafw on FreeBSD #6

smortex · 2018-01-24T08:20:18Z

These patches where sent to the sigrok-devel@ mailing-list but I got no feedback. Since sending patches over a mailing list is a good way to forget about them, and because pull-requests to this repository seems to be merged, please allow me to submit it again 😉. Thanks!

After updating sigrok and pulseview on FreeBSD, I could not use my fx2lafw device anymore. After investigating the issue, I found out the culprit: when usb_get_port_path() is called while the device is open, the function fails, but since the return value is never tested, the uninitialized memory that was supposed to receive the path is copied and considered valid, and execution continues.

As a consequence, when attempting to compare the device path and uninitialized memory later on, no match is found.

Here are two commits, the first one ensure the device is closed before calling usb_get_port_path() (I could only test with the fx2lafw driver, but some drivers had the wrong order and where changed too). The second patch test the return value of usb_get_port_path() so that errors are not ignored anymore.

The usb_get_port_path() function opens the passed device on FreeBSD, which fails if the device has already been open.

This function can fail. If so, do not ignore the failure.

uwehermann · 2018-01-28T17:05:04Z

Hi, thanks for your patch (and sorry for the delay)! The code looks good so far (will also test in a bit, but others have reported the fix works already, so it should be fine).

Maybe the first patch can be done a bit differently, though -- instead of forcing the driver authors to know/remember to use the correct order, could usb_get_port_path() simply check if the device is already open and simply close it before opening (or not re-open it twice)? If that's possible, all drivers (current and future ones) would work without any changes required and without things driver writers need to remember, which would be great.

I haven't actually looked into whether that's easily possible, just a random idea.

smortex · 2018-01-29T17:00:49Z

I do not have much knowledge about the way to do things with USB, after a quick look at the FreeBSD code of libusb, I found that the older API (libusb-0.1) specifically checked for an already opened device (maybe this is not done in newer versions on purpose).

I do not see something in the public API to check if the device has already been open. I asked for insights on IRC, maybe we will have valuable feedback from some FreeBSD developer 😜

hselasky · 2018-01-29T18:54:14Z

Hi, It is possible that FreeBSD can pull out this information while enumerating the devices. Then it does not matter when it is called.
--HPS

smortex · 2018-01-29T19:09:28Z

@hselasky that would be awesome I think! In the meantime, do you have a short-term recommendation / best practice recommendation for this particular issue?

hselasky · 2018-01-29T19:10:27Z

I think it would be best to have the libusb device handle open, before querying this information. I don't see how this can hurt.

uwehermann · 2018-02-18T23:52:56Z

OK, so I've finally tried the patches on a FreeBSD 11.1 install on an amd64 laptop. I can confirm that without the patches you cannot get any samples from an fx2lafw device. After the patches the logs look different, but I'm still unable to get any samples.

Test: sigrok-cli -d fx2lafw -c samplerate=1mhz --samples 10 -l 5

I've also tried running sigrok-cli via sudo to be sure that it's not permissions related, but maybe that's not enough and/or I'm missing some other required steps on FreeBSD?

I'm seeing "Waiting for device to reset", "Unable to get version info: LIBUSB_ERROR_NO_DEVICE" and "Device failed to renumerate" here. Can anybody else reproduce this and/or suggest any fixes for this?

Thanks!

smortex · 2018-02-19T09:20:53Z

@uwehermann on FreeBSD, usbconfig will list the USB devices accessible to the current user (I guess it corresponds basically what lsusb reports on GNU/Linux). Running this command before / after pluging-in your device you can check if the user will be a able to communicate with it. In order to use my device as a user, I added a /usr/local/etc/devd/sigrok.conf with (I am a member of the wheel group, use usbconfig dump_device_desc to see devices vendor & product ID):

notify 110 {
	match "system"     "USB";
	match "subsystem"  "DEVICE";
	match "type"       "ATTACH";
	match "vendor"     "0x04b4";
	match "product"    "0x8613";
	action "/usr/bin/chgrp wheel /dev/$ugen";
	action "/bin/chmod g+rw /dev/$ugen";
};

When running your command, I get:

λ sigrok-cli -d fx2lafw -c samplerate=1mhz --samples 10 -l 5
sr: [00:00.000000] log: libsigrok loglevel set to 5.
sr: [00:00.000020] backend: libsigrok 0.5.0/4:0:0 (rt: 0.5.0/4:0:0).
sr: [00:00.000048] backend: Libs: glib 2.50.3 (rt: 2.50.3/5003:3), libzip 1.3.2, libserialport 0.1.1/1:0:1 (rt: 0.1.1/1:0:1), libusb-1.0 , libftdi 1.4, librevisa 0.0.20130412.
sr: [00:00.000057] backend: Host: amd64-portbld-freebsd11.1, little-endian.
sr: [00:00.000062] backend: SCPI backends: TCP, RPC, serial, VISA, USBTMC.
sr: [00:00.000085] backend: Sanity-checking all drivers.
sr: [00:00.000094] backend: Sanity-checking all input modules.
sr: [00:00.000099] backend: Sanity-checking all output modules.
sr: [00:00.000103] backend: Sanity-checking all transform modules.
srd: libsigrokdecode loglevel set to 5.
sr: [00:00.000688] ezusb: uploading firmware to device on 0.10
sr: [00:00.000748] ezusb: setting CPU reset mode on...
sr: [00:00.001021] resource: Attempt to open '/home/romain/.local/share/sigrok-firmware/fx2lafw-cypress-fx2.fw' failed: No such file or directory
sr: [00:00.001044] resource: Opened '/usr/local/share/sigrok-firmware/fx2lafw-cypress-fx2.fw'.
sr: [00:00.001114] ezusb: Uploading firmware 'fx2lafw-cypress-fx2.fw'.
sr: [00:00.001669] ezusb: Uploaded 4096 bytes.
sr: [00:00.002290] ezusb: Uploaded 4024 bytes.
sr: [00:00.002296] ezusb: Firmware upload done.
sr: [00:00.002299] ezusb: setting CPU reset mode off...
sr: [00:00.002432] hwdriver: Scan of 'fx2lafw' found 1 devices.
sr: [00:00.002469] fx2lafw: Waiting for device to reset.
sr: [00:00.410977] fx2lafw: Waited 408ms.
sr: [00:00.518225] fx2lafw: Waited 515ms.
sr: [00:00.620721] fx2lafw: Waited 618ms.
sr: [00:00.727974] fx2lafw: Waited 725ms.
sr: [00:00.835224] fx2lafw: Waited 832ms.
sr: [00:00.936349] fx2lafw: Waited 933ms.
sr: [00:01.036940] fx2lafw: Waited 1034ms.
sr: [00:01.138538] fx2lafw: Waited 1136ms.
sr: [00:01.244808] fx2lafw: Waited 1242ms.
sr: [00:01.346975] fx2lafw: Waited 1344ms.
sr: [00:01.454225] fx2lafw: Waited 1451ms.
sr: [00:01.559039] fx2lafw: Waited 1556ms.
sr: [00:01.666280] fx2lafw: Waited 1663ms.
sr: [00:01.768303] fx2lafw: Waited 1765ms.
sr: [00:01.875536] fx2lafw: Waited 1873ms.
sr: [00:01.982971] fx2lafw: Waited 1980ms.
sr: [00:02.090229] fx2lafw: Waited 2087ms.
sr: [00:02.090834] fx2lafw: Opened device on 0.10 (logical) / usb/0-1.5.2 (physical), interface 0, firmware 1.3.
sr: [00:02.090851] fx2lafw: Detected REVID=4, it's a Cypress CY7C68013 (FX2).
sr: [00:02.090861] fx2lafw: Device came back after 2087ms.
sr: [00:02.090936] hwdriver: sr_config_set(): key 30000 (samplerate) sdi 0x80401e480 cg NULL -> uint64 1000000
sr: [00:02.090956] hwdriver: sr_config_set(): key 50001 (limit_samples) sdi 0x80401e480 cg NULL -> uint64 10
sr: [00:02.091000] session: Using thread-default main context.
sr: [00:02.091007] session: Starting.
sr: [00:02.091041] fx2lafw: submitting transfer: 0
sr: [00:02.091182] fx2lafw: submitting transfer: 1
sr: [00:02.091391] fx2lafw: submitting transfer: 2
sr: [00:02.091397] fx2lafw: submitting transfer: 3
sr: [00:02.091400] fx2lafw: submitting transfer: 4
sr: [00:02.091404] fx2lafw: submitting transfer: 5
sr: [00:02.091407] fx2lafw: submitting transfer: 6
sr: [00:02.091411] fx2lafw: submitting transfer: 7
sr: [00:02.091414] fx2lafw: submitting transfer: 8
sr: [00:02.091417] fx2lafw: submitting transfer: 9
sr: [00:02.091428] fx2lafw: submitting transfer: 10
sr: [00:02.091432] fx2lafw: submitting transfer: 11
sr: [00:02.091436] fx2lafw: submitting transfer: 12
sr: [00:02.091439] fx2lafw: submitting transfer: 13
sr: [00:02.091442] fx2lafw: submitting transfer: 14
sr: [00:02.091446] fx2lafw: submitting transfer: 15
sr: [00:02.091449] fx2lafw: submitting transfer: 16
sr: [00:02.091452] fx2lafw: submitting transfer: 17
sr: [00:02.091455] fx2lafw: submitting transfer: 18
sr: [00:02.091459] fx2lafw: submitting transfer: 19
sr: [00:02.091466] fx2lafw: submitting transfer: 20
sr: [00:02.091470] fx2lafw: submitting transfer: 21
sr: [00:02.091473] fx2lafw: submitting transfer: 22
sr: [00:02.091476] fx2lafw: submitting transfer: 23
sr: [00:02.091480] fx2lafw: submitting transfer: 24
sr: [00:02.091483] fx2lafw: submitting transfer: 25
sr: [00:02.091486] fx2lafw: submitting transfer: 26
sr: [00:02.091490] fx2lafw: submitting transfer: 27
sr: [00:02.091493] fx2lafw: submitting transfer: 28
sr: [00:02.091496] fx2lafw: submitting transfer: 29
sr: [00:02.091501] fx2lafw: submitting transfer: 30
sr: [00:02.091505] fx2lafw: submitting transfer: 31
sr: [00:02.091508] std: fx2lafw: Starting acquisition.
sr: [00:02.091512] std: fx2lafw: Sending SR_DF_HEADER packet.
sr: [00:02.091518] session: Running transform module 'nop'.
sr: [00:02.091522] transform/nop: Received packet of type 10000, passing on unmodified.
sr: [00:02.091526] session: bus: Received SR_DF_HEADER packet.
cli: Received SR_DF_HEADER.
sr: [00:02.091586] hwdriver: sr_config_get(): key 30000 (samplerate) sdi 0x80401e480 cg NULL -> uint64 1000000
sr: [00:02.091596] fx2lafw: GPIF delay = 47, clocksource = 48MHz.
sr: [00:02.097260] fx2lafw: receive_transfer(): status LIBUSB_SUCCESS received 10240 bytes.
sr: [00:02.097275] session: Running transform module 'nop'.
sr: [00:02.097279] transform/nop: Received packet of type 10004, passing on unmodified.
sr: [00:02.097282] session: bus: Received SR_DF_LOGIC packet (20 bytes, unitsize = 2).
cli: Received SR_DF_LOGIC (20 bytes, unitsize = 2).
sr: [00:02.097298] hwdriver: sr_config_get(): key 30000 (samplerate) sdi 0x80401e480 cg NULL -> uint64 1000000
libsigrok 0.5.0
Acquisition with 16/16 channels at 1 MHz
sr: [00:02.097350] std: fx2lafw: Sending SR_DF_END packet.
sr: [00:02.097356] session: Running transform module 'nop'.
sr: [00:02.097359] transform/nop: Received packet of type 10001, passing on unmodified.
sr: [00:02.097363] session: bus: Received SR_DF_END packet.
D0:11111111 11
D1:11111111 11
D2:11111111 11
D3:00000000 00
D4:11111111 11
D5:11111111 11
D6:00000000 00
D7:11111111 11
D8:11111111 11
D9:11111111 11
D10:00000000 00
D11:11111111 11
D12:11111111 11
D13:11111111 11
D14:11111111 11
D15:00000000 00
cli: Received SR_DF_END.
sr: [00:02.097390] usb: usb_source_finalize
sr: [00:02.097405] session: Stopped.
sr: [00:02.097432] fx2lafw: fx2lafw: Closing device on 0.10 (logical) / usb/0-1.5.2 (physical) interface 0.

uwehermann · 2018-02-21T10:49:17Z

The two patches are merged upstream now (with one additional fix in the new hantek-4032l driver), and mentioning the upstream Bugzilla bug #1109 (which is now marked as fixed). Thanks a lot!

I haven't yet been able to test this myself, still getting "Device failed to renumerate" for now, but I'll look into that and/or try installing FreeBSD on another box to see if it's a hardware problem or such.

I'm closing this issue as fixed, but if anyone wants to make an additional patch to move the fix into the common usb_get_port_path() function in usb.c, please go ahead. That would make any potential new drivers work automatically, without authors having to remember to use the correct ordering of the function calls.

smortex · 2018-02-21T12:05:24Z

Thanks @uwehermann !

FYI, I bought a fx2lafw device to @hselasky who should receive it in a few weeks… This will help if we need further tweaking as he is the main author of FreeBSD's USB stack 👍

hselasky · 2018-03-23T09:51:39Z

Hi,

I've now investigated this issue and made the following patch for libsigrok:
libsigrok.txt

In addition there is a patch for FreeBSD's libusb:
https://svnweb.freebsd.org/changeset/base/331419

--HPS

hselasky · 2018-03-23T09:54:07Z

Please note that under FreeBSD the same USB device can only be opened one time. The current libsigrok code tried to open the same USB device handle multiple times, due to the port path patch, and this won't work.

avg-I · 2018-06-06T16:04:25Z

@hselasky, has the FreeBSD port / package been fixed?

smortex · 2018-06-06T17:09:22Z

@avg-I regarding the port itself, there was no release of libsigrok and the fix was not added as a local patch, to if you are using one of the concerned devices, it will probably not work out of the box with the FreeBSD packages.

The change @hselasky did improves how things work at the OS level, but AFAICR, the fix of this PR is needed in all cases. Maybe it's worth adding a patch to the ports tree. @avg-I if you have problems with a driver, can you confirm that this patch fixes the problem?

avg-I · 2018-06-06T17:55:54Z

@smortex, this is what I have with a vanilla FreeBSD package:

sigrok-cli --driver fx2lafw --show
Driver functions:
    Logic analyzer
Scan options:
    conn
fx2lafw:conn=2.2 - Saleae Logic with 8 channels: D0 D1 D2 D3 D4 D5 D6 D7
sr: fx2lafw: Unable to open device.
Failed to open device.

If I clone this repo and manually build libsigrok, then everything is okay for me:

LD_PRELOAD=libsigrok/.libs/libsigrok.so.4.0.0 sigrok-cli --driver fx2lafw --show
Driver functions:
    Logic analyzer
Scan options:
    conn
fx2lafw:conn=2.2 - Saleae Logic with 8 channels: D0 D1 D2 D3 D4 D5 D6 D7
Channel groups:
    Logic: channels D0 D1 D2 D3 D4 D5 D6 D7
Supported configuration options across all channel groups:
    continuous: on, off
    limit_samples: 0 (current)
    conn: 2.2 (current)
    samplerate - supported samplerates:
      20 kHz
...
      24 MHz
    Supported triggers: 0 1 r f e 
    captureratio: 0 (current)

I haven't tried libsigrok.txt from @hselasky

smortex · 2018-06-06T18:39:46Z

@avg-I I opened a PR with the local diff of my package:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=228789

NOTE: Once the device is connected and has entered the command mode (0x51), every following byte will be interpreted as a bit mask for switching relays. There is no way to leave the command mode! IMPORTANT: When reconnecting to the device (e.g. with sigrok-cli or SmuView) the connect will fail and the "query device"-command (0x50) will be interpreted as a bit mask and will turn on relays sigrokproject#1-sigrokproject#4, sigrokproject#6 and sigrokproject#8. You have to power cycle the device to be able to reconnect!

smortex added 2 commits January 24, 2018 09:09

Ensure device is closed before usb_get_port_path()

6b7f074

The usb_get_port_path() function opens the passed device on FreeBSD, which fails if the device has already been open.

Check usb_get_port_path() return value

b985dfd

This function can fail. If so, do not ignore the failure.

uwehermann closed this Feb 21, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unbreak libsigrok fx2lafw on FreeBSD #6

Unbreak libsigrok fx2lafw on FreeBSD #6

smortex commented Jan 24, 2018

uwehermann commented Jan 28, 2018

smortex commented Jan 29, 2018

hselasky commented Jan 29, 2018

smortex commented Jan 29, 2018

hselasky commented Jan 29, 2018

uwehermann commented Feb 18, 2018

smortex commented Feb 19, 2018

uwehermann commented Feb 21, 2018

smortex commented Feb 21, 2018

hselasky commented Mar 23, 2018

hselasky commented Mar 23, 2018

avg-I commented Jun 6, 2018

smortex commented Jun 6, 2018

avg-I commented Jun 6, 2018

smortex commented Jun 6, 2018

Unbreak libsigrok fx2lafw on FreeBSD #6

Unbreak libsigrok fx2lafw on FreeBSD #6

Conversation

smortex commented Jan 24, 2018

uwehermann commented Jan 28, 2018

smortex commented Jan 29, 2018

hselasky commented Jan 29, 2018

smortex commented Jan 29, 2018

hselasky commented Jan 29, 2018

uwehermann commented Feb 18, 2018

smortex commented Feb 19, 2018

uwehermann commented Feb 21, 2018

smortex commented Feb 21, 2018

hselasky commented Mar 23, 2018

hselasky commented Mar 23, 2018

avg-I commented Jun 6, 2018

smortex commented Jun 6, 2018

avg-I commented Jun 6, 2018

smortex commented Jun 6, 2018