-
Notifications
You must be signed in to change notification settings - Fork 588
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
valentyusb broken by litex change #710
Comments
Thanks @geertu for reporting. It seems Valenty USB is playing a bit with LiteX internals and i prefered updating it than adding a workaround in LiteX. The issue has been fixed with litex-hub/valentyusb@82235f9 and Linux-On-LiteX-Vexriscv is now also using this repository (litex-hub/linux-on-litex-vexriscv@c196ad1). I also did a couple of change to ValentyUSB to make it portable/generic: This allows for example to use the same code for the Fomu (iCE40) and OrangeCrab (ECP5) and still use IO Regs which was not possible before. So to fix your issue;
and rebuild :) |
Confirmed. |
Great, thanks for the feedback. |
Updating ValentyUSB seems like a good move here, thanks for patching it up @enjoy-digital The original code for hardware based USB-ACM, was definitely a bit of a hack with the CSR internals. |
@gregdavill: for now valentyUSB worked fine and has been really useful to enable USB-ACM! I just did small changes to fix the issue and simplify support on different hardware but this is still a temporary solution. I'll see in the future if we should try to have a simple USB-ACM code in LiteX directly or if we should rely on another one (Luna for example). |
My opinion is that LUNA is the future and ValentyUSB should be considered deprecated. |
ValentyUSB is still currently useful since easy to integrate in LiteX SoCs and USB-ACM has been working pretty well for now. In the future we'll probably integrate Luna, but I would also like to experiment with a simple USB-ACM core directly in LiteX. |
@enjoy-digital @mithro would you mind giving instructions on how to use the API of valentyUSB? I've read https://github.com/litex-hub/valentyusb/blob/hw_cdc_eptri/valentyusb/usbcore/cpu/cdc_eptri.py a little but I'm still quite new to the scene and don't understand a lot of concepts. My goal is to send bits from the (So far I've been trying with the I've put a little code in the From reading the soc.uart.source.data
soc.uart.sink.data
soc.uart._rxtx.r
soc.uart._rxtx.w
soc.uart.tx_fifo
soc.uart.rx_fifo
soc.uart.phy.out_buffer_rd
soc.uart.phy.out_buffer.get_port(write_capable=True, clock_domain="usb_12") .we .dat_w As you can see I'm also confused what goes into the fomu and what goes out. The In the code it says Help and instructions are greatly appreciated. Thanks for the hard work on litex/valentyusb/fomu. |
Strange, I thought I encountered this issue a while back, and that @enjoy-digital had fixed it up. See #984 . But I can now reproduce the error you're seeing.
|
@tcal-x: Can you share the command to reproduce the issue? |
Sorry for the omission!
|
(I realize that does not make much sense as a design; you'd want to expose the Wishbone bus over a debug interface so that you can connect to it from the host and twiddle CSRs.) |
Was this regarding my problem? |
Hi @aMOPel , sorry I was not clear. I was talking about my build, which just removed the CPU but didn't add a debug interface. I don't have enough background to answer your other questions. |
@tcal-x I see, thank you. |
@aMOPel , I have successfully built the Luna ACM loopback example for Fomu. If you are willing to use Amaranth-HDL (formerly nMigen) instead of Migen, that might be a good starting point. You would hook into this area: https://github.com/greatscottgadgets/luna/blob/main/examples/usb/acm_serial.py#L29-L34 If you decide to go that way, I can provide some help. |
@tcal-x you've been in this community for a while, do you think there is gonna be help from someone with experience with valentyusb? I mean, mithro said it's deprecated, but litex is still using it even a year after he said that. That, however, is probably only the case because other things have priority, I suppose. My current alternative is following this repos approach For Luna + amaranth, I'll have to get into new things again. They appear to be better documented than valentyusb/migen and in active development and my biggest struggle so far, has been finding resources that explain things to a beginner (thoroughly). So I guess it's at least worth a look. So I might take you up on that offer, thanks. |
@aMOPel: ValentyUSB has been contributed by @xobs, @mithro and emulates the CSR interface of the LiteX UART and indeed requires a CPU to operate. We are using it on some boards since it is convenient and we haven't spend the time to explore the Luna/Amaranth alternative for this. For a proper USB ACM core with TX/RX streaming interface, Luna/Amaranth is probably better as suggested by @tcal-x. I also think @gregdavill is using this on the Butterstick with LiteX. |
@aMOPel I'm no expert, my interest in USB is as a user -- I just want a tty/uart that works and hopefully takes as few resources as possible! I've encountered these other USB implementations while poking around; you might find them useful or at least educational:
|
You shouldn't necessarily need a CPU to make use of the CDC_ACM wrapper for ValentyUSB. Basically it should enumerate correctly without a CPU, and then you just use the two FIFOs, Using it in this manner is a bit of a hack from the original use, so you'll likely have to get in and cut out pieces from I have also used LUNA for this purpose on the ButterStick, however in that case we have a ULPI HS USB PHY, so the code/wrapper I've got for that isn't a direct drop in for your project. But for reference this is the code: https://github.com/gregdavill/luna-usb-serial-acm |
Thank you all for answering.
That looks awesome. I was already looking into no2muacm, thinking about how to integrate it in migen, but apparently it has already been done. Thanks a lot. @gregdavill that was in fact exactly what I was trying to do with # valentyusb/usbcore/cpu/cdc_eptri.py
# the way I understand it:
self._rxtx # (CSR Interface)
# ^
# |
tx_fifo.sink.data.eq(self._rxtx.r),
# ^
# |
tx_fifo.source.connect(self.source)
# ^
# |
self.source
# |
# v
self.source.connect(phy.source),
# in phy is the actual usb lnterface
# self.source is in the middle between tx_fifo and phy.source
# the tx_fifo seems to be used between self.source and the CSR Also when it comes to debugging, why it doesn't work, I'm still at a loss. # litex_boards/targets/kosagi_fomu.py
src = soc.uart.source
char = 0x77
soc.comb += [
src.data.eq(char),
] and variations of it. |
Commit 470b687
broke the build on OrangeCrab, using https://github.com/gregdavill/valentyusb/tree/hw_cdc_eptri
Traceback (most recent call last):
File "./make.py", line 556, in
main()
File "./make.py", line 495, in main
soc = SoCLinux(board.soc_cls, **soc_kwargs)
File ".../OrangeCrab/linux-on-litex-vexriscv.bad/soc_linux.py", line 284, in SoCLinux
return _SoCLinux(**kwargs)
File ".../OrangeCrab/linux-on-litex-vexriscv.bad/soc_linux.py", line 107, in init
soc_cls.init(self,
File ".../litex/litex-boards/litex_boards/targets/orangecrab.py", line 160, in init
SoCCore.init(self, platform, sys_clk_freq,
File ".../litex/litex/litex/soc/integration/soc_core.py", line 184, in init
self.add_uart(name=uart_name, baudrate=uart_baudrate, fifo_depth=uart_fifo_depth)
File ".../litex/litex/litex/soc/integration/soc.py", line 1115, in add_uart
self.submodules.uart = cdc_eptri.CDCUsb(usb_iobuf)
File "valentyusb/valentyusb/usbcore/cpu/cdc_eptri.py", line 103, in init
self.submodules.phy = phy = ClockDomainsRenamer("usb_12")(CDCUsbPHY(iobuf, debug=debug, vid=vid, pid=pid, product=product, manufacturer=manufacturer))
File "valentyusb/valentyusb/usbcore/cpu/cdc_eptri.py", line 462, in init
If(usb.out_ev_pending.w,
File ".../litex/migen/migen/fhdl/module.py", line 136, in getattr
raise AttributeError("'"+self.class.name+"' object has no attribute '"+name+"'")
AttributeError: 'CSRStatus' object has no attribute 'w'
Anyone with a clue? Thanks!
Originally posted by @geertu in #701 (comment)
The text was updated successfully, but these errors were encountered: