On Fri, Jul 25, 2025 at 12:07 PM Johan Hovold <johan@xxxxxxxxxx> wrote: > > On Tue, Jul 15, 2025 at 11:20:33PM +0200, Martin Blumenstingl wrote: > > > On Mon, May 12, 2025 at 12:03 PM Johan Hovold <johan@xxxxxxxxxx> wrote: > > > > The read urbs should be submitted at first open and stopped at last > > > close to avoid wasting resources when no one is using the device. > > > > > > I know we have a few drivers that do not do this currently, but it > > > shouldn't be that hard to get this right from the start. > > > If you're aware of an easy approach or you can recommend an existing > > driver that implements the desired behavior then please let me know. > > > > The speciality about ch348 is that all ports share the RX/TX URBs. > > My current idea is to implement this using a ref count (for the number > > of open ports) and mutex for locking. > > Just use a mutex and integer (not refcount) to count the number of open > ports. Submit the urbs on first open and stop them on last close. > > Not doing so, and instead submitting at attach(), means that the host > controller will be wasting power by polling the endpoints continuously > as long as the device is plugged in. Thanks, my code wasn't miles off of f81534.c but I'm following that more closely now. [...] > > I'm trying as you suggest: > > - submit the URB synchronously for port N > > - submit the URB synchronously for port N + 1 > > - ... > > > > This seems to work (using usb_bulk_msg). What doesn't work is > > submitting URBs in parallel (this is what the vendor driver prevents > > as well). > > No, the vendor driver tracks THRE per port > (ttyport[portnum].write_empty) and allows writing to more than one port > in parallel (e.g. releases the device write_lock while waiting for the > transfer to complete). > > I thought the problem was that you could not submit another urb for the > *same* port until the device buffer for that port had been emptied? > > This seems to be what the vendor driver is preventing. I managed to get it to work now without any unnecessary waiting. When I switched to just waiting for per-port THRE I accidentally re-used the same URB (along with its buffer) for all ports. This of course "corrupts" data, but it's my fault instead of the chip/firmware causing it. That's why I was referring to data corruption earlier. Thanks for your persistence and for making me look at my code again with a fresh mind. > > > You should implement dtr_rts() as well. > > > This will be the first time we need the "package type" information as > > CH348Q only supports CTS/RTS on the first four ports, for the last > > four the signals aren't routed outside the package. > > I need to see if I have other hardware with CTS/RTS pins to test this. > > Just connect a multimeter to the DTR and RTS pins and verify that they > are asserted on open and deasserted on close after issuing those control > requests (see ch9344_port_dtr_rts()). Do I need to set anything special in termios for this to work? The datasheet has a special note about DTR/TNOW (on page 8 for the CFG pin): > Unified configuration: During power-on, if the CFG pin is > at a high level or not connected, all DTRx/ TNOWx pins > are configured to function as TNOW. CFG pin is low, all > DTRx/ TNOWx pins are configured for DTR function. On my test board the CFG pin is HIGH. From how I understand you, RTS should at least change (even if DTR is in TNOW mode). No matter what I do: both pins are always LOW (right after modprobe, after opening the console, closing the console again, ...). I even set up the vendor driver to test this: it's the same situation there. If we need to make the DTR and RTS output something then the only way I know of right now is to switch them to GPIO mode (I have code for this but it's another ~300 lines patch on top of this). So I'd like to not implement .dtr_rts and drop all CRTSCTS related code for now. Best regards, Martin
