Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for FREE WILi #244

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

Ytuf
Copy link

@Ytuf Ytuf commented Aug 10, 2024

Added support for the FREE WILi hardware development tool (https://freewili.com/) which uses FT1248 4-wire protocol on a FT232H chip. Tested with a FREE WILi, no problems I'm aware of.

Copy link
Contributor

@biot biot left a comment

Choose a reason for hiding this comment

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

Can you fix the indentation? It's all over the place, hard to read this code.

More information on coding style here:
https://github.com/sigrokproject/libsigrok/blob/master/HACKING

@wskellenger-intrepid
Copy link

wskellenger-intrepid commented Oct 1, 2024

@biot I've pushed the formatting updates in my fork and made a PR to @Ytuf's fork -- if he accepts my PR will those changes appear here? This is the first time I've tried to update someone else's PR. :)

Update: it's merged now, formatting is updated.

reformatted per Linux coding style
@biot
Copy link
Contributor

biot commented Oct 1, 2024

Ok, thanks for the reformatting. Please squash those commits though.

A few questions:

  • Is this an "official" effort from the FREE-WILi project? The docs there talk about a built-in logic analyzer, with a Pulseview screenshot. Is that running this driver?
  • What's the situation wrt FPGA gateware running on the device? Does it need a special gateware uploaded before this driver is able to talk to the device? It would help if you'd make a page on the wiki showing how to use it. @abraxa will hook you up with an account.
  • What's the deal with this single 31.25MHz samplerate? It looks like the driver doesn't tell the hardware about it, so is that some hardware limitation, or are there other samplerates it can use?
  • Is this a "streaming" LA, like the FX2 boards, or does it have a memory buffer it can use? Looks like streaming to me, in which case 31.25MHz seems to me wishful thinking. The FX2 boards at their top rate of 24MHz typically near-saturate a USB HS bus, where any other traffic on the bus kills the stream. So does this work? How long can it stream before it dies?

I'll do a thorough review once you've squashed your commits, but for now a couple of things I noticed:

  • The api.c:scan_device() function is a mess, and leaks allocations too.
  • Clean up some of the sr_dbg()/sr_spew() statements, some look like development debug stuff left in.

@wskellenger-intrepid
Copy link

Thank you @biot for your comments. Here is the feedback I can provide; from the technical perspective I will email @Ytuf to make sure he sees this.

  • Yes this is an "official" effort; the intention is that users could use FreeWili has a logic analyzer. Currently our only 'showcase' application for the FPGA that's onboard is the logic analyzer application.
  • I will ask @Ytuf to reply regarding you technical questions; he did all of the development for this feature. @msmoger-intrepid may also be able to answer some of your technical questions. Thanks for taking the time to have a look.
  • @Ytuf did mention to me that 31.25MHz is the absolute max and there are can be buffer overflow issues at that speed. This is a first attempt, your feedback and comments help to refine the solution further.
  • regarding squash; do most contributors usually submit a squashed solution, or is there an option to squash at the moment the contribution is merged? Reason I ask is if more than one person contributes to the same MR, squashing up until the point of commit would be difficult. However, I am not sure I anticipate anyone other than @Ytuf to be committing here.

@biot
Copy link
Contributor

biot commented Oct 1, 2024

I would strongly recommend going for other, lower samplerates. On the FX2 the 24MHz rate is super unreliable, and worse: it's different for everyone, and for some it never works at all. Shitty USB hub in between, other traffic on the bus, anything will kill it. I have a USB cable that consistently breaks the 24MHz stream, I assume some shielding issue. Save yourself the support headache.

New drivers always get squashed, since the initial devel commits aren't that relevant for sigrok. Typically we like a stub generated by the new-driver script from sigrok-util, and then a second commit filling out the stub. Check the repo for "Initial driver skeleton" commits.

@Ytuf
Copy link
Author

Ytuf commented Oct 1, 2024

Thanks for all the feedback @biot, and thank you for your help @wskellenger-intrepid!

What's the situation wrt FPGA gateware running on the device? Does it need a special gateware uploaded before this driver is able to talk to the device?

The FPGA communicates with the PC through an FTDI chip (the (FT232HQ-REEL)), it should be plug-and-play with no special preparation needed similar to the "FTDI-LA" logic analyzer.

What's the deal with this single 31.25MHz samplerate?

This is an initial hardware limitation as 31.25 MHz is simply the frequency the FPGA runs at. I am currently developing a more advanced version that should allow choosing between some samplerates. 8 MHz is my goal for the maximum samplerate.

Is this a "streaming" LA, like the FX2 boards, or does it have a memory buffer it can use?

The current released version has a 32 KB buffer built-in plus a 1 KB buffer on the FTDI chip. The version I'm working on now will have a 64 KB buffer. I am also dedicating 4 bits to RLE which gives these buffers a bit more wiggle room.

How long can it stream before it dies?

The current version never "dies", but it does occasionally overflow the buffer and therefore loses data. Honestly, it works a lot better than you'd expect. Once the hardware is adjusted to an 8 MHz sample rate this issue will no longer be present.

I'll take a look at the scan_devices() function and clean up the debug print statements.

Thanks again!

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

Successfully merging this pull request may close these issues.

4 participants