Kingst la2016 driver fixup #131
Conversation
…e meaningful tokens
Fix the address of registers within the fpga which are used for contolling input thresholds, sampling, triggering and bulk transfers. These addresses were derived from observations of the usb bus and of the internal la2016 spi bus. See wiki for captures of spi bus activity.
There are, in fact, two pwm outputs which must both be configured to control the dac output which offsets input signals. See the last two pages of the schematic (on wiki) for details.
The OEM software communicates with device IC U10 to perform challenge-response authentication, ensuring the device is genuine. We have no knowledge of the obfuscation or encryption method used. Additionally it is a rolling code, so the response changes even with same challenge. In any case, we can just ignore this IC U10, it does not hinder sigrok support. Removing this code from the initialisation routine. See schematic in wiki for more info on U10.
A magic number within the device eeprom identifies the device type and hardware revision. Using this number, the correct fpga bitstream is selected and loaded.
…ing long captures
The Kingst LA2016 and LA1016 use the same hardware, just the coding of authentication IC U10 is different. U10 limits the LA1016 to using an FPGA bitstream that samples at a maximum of 100MHz. Other than that, the devices operate identically and the driver just needs to select the correct bitstream and limit the maximum sample rate appropriately. This commit adds support for the existing two hardware revisions of LA1016, which require different bitstreams.
|
@Planet911 I can't thank you enough for this contribution! For the first time my Kingst LA2016 is working flawlessly with sigrok after getting the device a few months back! I wasted so much time trying various methods and failing, but your branch works without any modification. To test this, I've:
Thanks again so much! |
|
@resonantworks That's great, thanks for the positive feedback, hopefully it will help get this pulled into mainline. |
|
@Planet911 is it possible for someone to kick off a PR build of pulseview for osx that includes these changes? I just picked up a LA1016 and I would like to give this PR a try . I tried building from source but am not familiar with this project and kept running into issues. |
|
I would also really appreciate a windows build to try it out since that's what I use at work. I've tried following the MXE build instructions, but the patches are out of date and I couldn't see which version of MXE is being used. @uwehermann |
|
@stevenspmd @matthew-brown-bluefruit |
Glad it not just me ;) |
|
Managed to build it via MXE, but it unfortunately fails to run properly on Windows. Started PulseView, clicked Run, nothing happens, clicked Stop, hangs, disconnected USB, UI responsive again but no capture. This happens with the both the KingST driver and the WinUSB patched the Zadig supplied with the PulseView installer. Attached is the log from the PulseView debug build console window. Here's how I got the MXE script to work on Ubuntu 20.04 (WSL):
sudo apt-get install git-core gcc g++ make autoconf autoconf-archive automake libtool pkg-config libglib2.0-dev libglibmm-2.4-dev libzip-dev libusb-1.0-0-dev libftdi1-dev libieee1284-3-dev libvisa-dev nettle-dev libavahi-client-dev libhidapi-dev check doxygen python-numpy python-dev python-gi-dev python-setuptools swig default-jdk asciidoctor
cd ~ # Can put this somewhere else, but you'll need to use your path in the MXE patching below..
git clone git://sigrok.org/sigrok-util
cd sigrok-util
git remote add Planet911 [email protected]:Planet911/sigrok-util.git
git fetch Planet911
git merge Planet911/sigrok-fwextract-kingst-la2016-fixup-extraction
cd ~ # Needs to be in $HOME for this to work
git clone https://github.com/mxe/mxe.git mxe-git
cd mxe-git
git checkout build-2020-06-06
patch -p1 < ../sigrok-util/cross-compile/mingw/mxe_fixes.patch
sudo apt install autoconf automake autopoint bash bison bzip2 flex g++ g++-multilib gettext git gperf intltool libc6-dev-i386 libgdk-pixbuf2.0-dev libltdl-dev libssl-dev libtool-bin libxml-parser-perl lzip make openssl p7zip-full patch perl python ruby sed unzip wget xz-utils
make MXE_TARGETS=x86_64-w64-mingw32.static.posix MXE_PLUGIN_DIRS=plugins/examples/qt5-freeze gcc glib libzip libusb1 libftdi1 hidapi glibmm qtbase qtimageformats qtsvg qttranslations boost check gendef libieee1284 qtbase_CONFIGURE_OPTS='-no-sql-mysql'
...
TARGET="x86_64"
PARALLEL="-j 8" # Set to your number of cores
DEBUG=1
...
# libsigrok
#$GIT_CLONE $REPO_BASE/libsigrok
git clone [email protected]:sigrokproject/libsigrok.git
cd libsigrok
# Patch in KingST driver fix: Planet911:kingst-la2016-driver-fixup
git remote add Planet911 [email protected]:Planet911/libsigrok.git
git fetch Planet911
git merge --no-edit Planet911/kingst-la2016-driver-fixup
./autogen.sh
...
cd ~/sigrok-util/cross-compile/mingw
./sigrok-cross-mingw
cp build_debug_64/pulseview/contrib/pulseview-0.5.0-git-*-installer.exe /mnt/c/tempHere's the built Windows installer (max attachment size 25M - github madness - unzip each one, then use 7zip to decompress the split archive - ugh!): |
|
@resonantworks I tried your build (from the linked 7z archive) and I see the same thing here. The capture runs correctly but then bulk upload of capture data on EP6IN fails to start. Wireshark shows the kingst driver is writing the upload parameters correctly (over the control endpoint), it is just the EP6IN bulk which stalls. Wireshark doesn't show any bulk in request at that point. To me this smells like a USB endpoint issue, something needed to kickstart the EP6IN transfer on Windows. |
|
@Planet911 Thanks for the update. I've got a 'Zaleae Logic 16' in the office, so I'll see what happens in PulseView on Windows with that later this week when go in. |
|
@Planet911 I got the 'Zaleae Logic 16' to work with the build by using Zadig 2.5 to replace the driver with 'libusb-win32' (fails on 'WinUSB'), but can't seem to change the driver for the KingST LA2016 as it always resets back to 'WinUSB' after running PulseView. |
|
@resonantworks Using your excellent 'quick guide' above, I've built the Windows version of PulseView and (with this PR applied) my Kingst LA is now working fine in Win10. It's using the Kingst Windows driver but of course it's just a wrapper around the generic winusb driver. The only difference I can see from your build process is that I did a release build (DEBUG=0 in the sigrok-cross-mingw script). Build script attached, although changes are just as you describe above. Edit: One minor thing, I also had to 'apt-get install nsis' as it wasn't included above. Edit2: Agggh, nope, not fixed, the issue seems to still be there intermittently. Hmm. |
|
@Planet911, that's brilliant news! I'd love to see this get mainlined since it's such good hardware for the money (if/when the maintainers come out of their caves one day). |
|
@resonantworks, someone from the sigrok mailing list has a working win64 build of pulseview with this PR's kingst support. It seems that their build of libusb-1.0.a works where mine doesn't, for some as yet unknown reason (dependencies or build environment, to be investigated). Anyway, here is the 64bit build of libusb-1.0.a which gives me a working PulseView on win64. Put it here: |
|
Thanks for the updated @Planet911 ;). Doesn't surprise me at all due to the MXE stuff. I'll try take a look later when I get a free moment. |
| /* usb vendor class control requests to the cypress FX2 microcontroller */ | ||
| #define CMD_EEPROM 0xa2 /* ctrl_in reads, ctrl_out writes */ | ||
| #define CMD_FPGA_INIT 0x50 /* used before and after FPGA bitstream loading */ | ||
| #define CMD_FPGA_SPI 0x20 /* access registers in the FPGA over SPI bus, ctrl_in reads, ctrl_out writes */ | ||
| #define CMD_FPGA_ENABLE 0x10 | ||
| #define CMD_BULK_RESET 0x38 /* flush FX2 usb endpoint 6 IN fifos */ | ||
| #define CMD_BULK_START 0x30 /* begin transfer of capture data via usb endpoint 6 IN */ | ||
| #define CMD_KAUTH 0x60 /* communicate with authentication ic U10, not used */ |
There was a problem hiding this comment.
I think these defines should be sorted by increasing hex values. I also think one space after '#define' instead of a tab is enough. While I have no strong opinion on the latter one, I think the sorting would be good to have because it makes future changes a lot easier.
| #define REG_RUN 0x00 /* read capture status, write capture start */ | ||
| #define REG_PWM_EN 0x02 /* user pwm channels on/off */ | ||
| #define REG_CAPT_MODE 0x03 /* set to 0x00 for capture to sdram, 0x01 bypass sdram for streaming */ | ||
| #define REG_BULK 0x08 /* write start address and number of bytes for capture data bulk upload */ | ||
| #define REG_SAMPLING 0x10 /* write capture config, read capture data location in sdram */ | ||
| #define REG_TRIGGER 0x20 /* write level and edge trigger config */ | ||
| #define REG_THRESHOLD 0x68 /* write two pwm configs to control input threshold dac */ | ||
| #define REG_PWM1 0x70 /* write config for user pwm1 */ | ||
| #define REG_PWM2 0x78 /* write config for user pwm2 */ |
There was a problem hiding this comment.
Same as above. I think one space after '#define' will do.
|
Disclaimer:I don't have this hardware and know nothing about it. Still, I want to support sigrok with code review. |
|
Awesome. I have the LA1016. Is there an osx (or even a linux) build available? |
|
@wsakernel Thank you for approving, I'm still around and happy to tidy up the code. Please could you also pull the associated firmware extraction script changes I made, which are required to support these driver changes? See sigrok-util PR here: |
|
Thanks for the pointer. Done now. |
|
BTW do I get it right that the old code worked because of the old firmware? And this new code requires the new firmware? If so, is it possible to check for the old firmware and suggest an update to the user if it is found? |
|
@wsakernel The manufacturer firmware hasn't changed functionality AFAIK, it's just that with my driver changes there are now four different hardware versions supported, each of which need different FPGA firmware. The update to the extraction code will now extract all four of those FPGA firmwares (it was just extracting one). Also the user had to change the firmware filename to match that expected by the old driver, fixed that too. |
|
I wondered about firmware compatibility because when you remove the CTRL_* defines and replace them with the REG_* defines, there are some different hex values for the names, e.g. for BULK (from 0x10 to 0x08), SAMPLING, TRIGGER, THRESHOLD. |
|
@wsakernel Yes, I wondered how it ever worked too. It never has for me. I know of one person on the sigrok devel ML who had it working at least partially on sigrok-cli. AFAIK nobody had it working on PulseView. I bought the LA2016 because it was marked as supported by sigrok but I ended up doing quite a bit of work to get it functional. |
|
Thank you all very much! It is always great to see users contribute but since I don't have the hardware in question myself, it's always hard to judge whether the changes are an improvement or not. That's why I appreciate when more people chime in than just the commit author as that allows me to better judge the situation. |
|
By the way - the Windows and Linux container builds including this change will be available in roughly 24 hours while the OSX builds unfortunately need to be checked by the jenkins admin before they'll be updated again. My apologies. |
|
I have just tested the Linux nightly build of PulseView and the Kingst LA1016 and LA2016 are both working, which is great. However, neither of them work with the Windows nightly build. This looks like the libusb issue I mentioned above where I could only make a working Win build of PV using libusb built by someone else. I was hoping it was just my build process but it seems the sigrok jenkins build is the same. I don't know what the issue is and debugging is a PITA due to the cross compilation build process :-( |
|
Thanks for letting me know, I wonder if it's related to sigrokproject/sigrok-util#9 - I'll see if I can provide you a build with this change to let you test it. |
|
Hmm, no, I think the problem is something deeper. |
|
OK, I found the problem. It's related to part of this sigrok mxe patch. Specifically, the lines which enable libusb RAW_IO policy: |
|
I have a LA2016 arriving in a couple of days. Any news on the SigRok PV Win10 compatibility with LA2016? I just want to know if it is worth using time on installing SigRok for this device now or if I should wait. Thanks for your great work! |
|
I need to give this a test drive too. I have the LA1016 and osx.
…On Wed, Sep 29, 2021 at 5:20 AM NRollo ***@***.***> wrote:
I have a LA2016 arriving in a couple of days. Any news on the SigRok PV
Win10 compatibility with LA2016? I just want to know if it is worth using
time on installing SigRok for this device now or if I should wait.
Thanks for you great work!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#131 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADX5B2MAWQTNNOJFMLEDFMTUELK6HANCNFSM42M3W4QA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
@NRollo No progress yet. If your super-keen you could compile your own PV for Win and disable the libusb RAW_IO feature. As to a proper solution, RAW_IO will need to remain enabled (it has been in sigrok since 2016 or so) and so changes to the capture read buffer management in the LA2016 libsigrok driver is probably the way forward (ask libusb for 512 bytes at a time). |
|
@Planet911 Thanks for the prompt answer! |
|
@abraxa @wsakernel I have tracked down the issue with the windows build and created a new PR to fix it. It's a tiny PR so hopefully we can get this pulled relatively quickly, please! |
|
@Planet911 Great work!! |
|
I merged @Planet911 's PR and updated nightlies should be available in about 9 hours. |
|
@abraxa I've developed open firmware for the fx2 mcu in this analyser. Can you create a new sigrok repository, perhaps named 'sigrok-firmware-kingst-la2016'? I'll then submit a PR where we can track testing and code review. |
|
@abraxa I've developed open firmware for the fx2 mcu in this analyser. The firmware has a similar project structure to sigrok-firmware-fx2lafw but it doesn't really belong in any existing repository. How should I proceed to have this reviewed for inclusion in sigrok? Thanks. |
|
@Planet911 From my experience with this project, such things (new repo) are best sorted out via IRC. Try to reach abraxa there. Or maybe open a new ticket here at libsigrok. |
|
@resonantworks @stevenspmd @NRollo |
The sigrok driver for the Kingst LA2016 is currently broken for many use cases, it doesn't work at all for me in PulseView.
This pr fixes a number of driver issues, mainly the fpga spi register addressing, and also adds support for older LA2016 devices.
It also adds support for LA1016 devices (old and new revisions) which have the same hardware as LA2016 devices but use different fpga bitstreams and are limited to 100MHz sampling.
This has all been tested on LA2016 and LA1016 devices, on linux host but I don't envisage any portability issues.