valentyusb broken by litex change

geertu commented 3 years ago

Commit 470b6873ca2cf539be0699c387067b873485cb31 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 https://github.com/enjoy-digital/litex/issues/701#issuecomment-734391506

enjoy-digital commented 3 years ago

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 https://github.com/litex-hub/valentyusb/commit/82235f990ce134e49c14f4babcc9b5613b1b2044 and Linux-On-LiteX-Vexriscv is now also using this repository (https://github.com/litex-hub/linux-on-litex-vexriscv/commit/c196ad1a3b7bace55bbe8beba39bd73bf68d4648).

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; In your linux-on-litex-vexriscv directory:

rm -rf valentyusb
git pull

and rebuild :)

geertu commented 3 years ago

Confirmed. Thanks a lot!

enjoy-digital commented 3 years ago

Great, thanks for the feedback.

gregdavill commented 3 years ago

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.

enjoy-digital commented 3 years ago

@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).

mithro commented 3 years ago

My opinion is that LUNA is the future and ValentyUSB should be considered deprecated.

enjoy-digital commented 3 years ago

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.

aMOPel commented 2 years ago

@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 fomu pvt with migen code to /dev/ttyACM, using the litex build from the litex_boards repo for fomu.

(So far I've been trying with the serv cpu because it fits. But I don't actually need a CPU for the design, however trying to build with None gave me csr collision errors, which I didn't know how to resolve. Not sure if using a CPU conflicts with my goal here.)

I've put a little code in the main() of https://github.com/litex-hub/litex-boards/blob/master/litex_boards/targets/kosagi_fomu.py#L166 to try and see what could work, but to no avail so far.

From reading the cdc_eptri.py code I gathered it's probably one of these, I need to send the bits to:

    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 out_buffer seemed like the best choice, because it's a buffer / Memory class, but since it's hidden in the phy submodule it's probably not the inteded way to use it.

In the code it says _rxtx is the uart interface, but since its a CSR instance, it's probably meant for use with the wishbone bus/ wishbone-tool cli? Also a leading _ usually means private class member from what I know.

Help and instructions are greatly appreciated.

Thanks for the hard work on litex/valentyusb/fomu.

tcal-x commented 2 years ago

(... But I don't actually need a CPU for the design, however trying to build with None gave me csr collision errors, which I didn't know how to resolve. )

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.

ERROR:SoCBusHandler:Region overlap between psram and csr:
ERROR:SoCBusHandler:Origin: 0x00000000, Size: 0x00020000, Mode: RW, Cached: True Linker: False
ERROR:SoCBusHandler:Origin: 0x00000000, Size: 0x00010000, Mode: RW, Cached: False Linker: False

enjoy-digital commented 2 years ago

@tcal-x: Can you share the command to reproduce the issue?

tcal-x commented 2 years ago

@tcal-x: Can you share the command to reproduce the issue?

Sorry for the omission!

./kosagi_fomy.py --cpu-type=None --build

tcal-x commented 2 years ago

(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.)

aMOPel commented 2 years ago

@tcal-x

(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? Because I don't want to twiddle CSRs from the host, but just read data at /dev/ttyACM the fomu sends there. Is a CPU required for this? And in any case what endpoint, exposed by litex/valentyusb, can I use from migen code to send the data?

tcal-x commented 2 years ago

Was this regarding my problem? Because I don't want to twiddle CSRs from the host, but just read data at /dev/ttyACM the fomu sends there. Is a CPU required for this? And in any case what endpoint, exposed by litex/valentyusb can I use from migen code to send the data?

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.

aMOPel commented 2 years ago

@tcal-x I see, thank you.

tcal-x commented 2 years ago

@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.

aMOPel commented 2 years ago

@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 https://github.com/stef/nb-fomu-hw and use tinyfpga. Which seems to be even older/less maintained than valentyusb. But at least there is a working example.

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.

enjoy-digital commented 2 years ago

@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.

tcal-x commented 2 years ago

@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:

https://github.com/no2fpga/no2migen/tree/master/examples (Fomu example ready to go)
https://github.com/ulixxe/usb_cdc (found from this discussion: https://github.com/BrunoLevy/learn-fpga/issues/21)

gregdavill commented 2 years ago

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, source/sink to read/write data back and forth from the computer.

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 cdc_eptri.py yourself.

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

aMOPel commented 2 years ago

Thank you all for answering.

@tcal-x

https://github.com/no2fpga/no2migen/tree/master/examples (Fomu example ready to go)

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 cdc_eptri.py. However I didn't quite get it to work. For one thing I don't understand the intricacies of that code, like how the connection chain works in detail.

# 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. I was trying things like this:

# litex_boards/targets/kosagi_fomu.py
    src = soc.uart.source

    char = 0x77

    soc.comb += [
        src.data.eq(char),
    ]

and variations of it. /dev/ttyACM0 was created, but remained empty. The build was with --cpu-type=serv as I mentioned above, because --cpu-type=None created CSR collisions.

enjoy-digital / litex

valentyusb broken by litex change #710