Message-ID: <aJCoRFe-RFW1MuDk@hovoldconsulting.com>
Date: Mon, 4 Aug 2025 14:32:04 +0200
From: Johan Hovold <johan@...nel.org>
To: Martin Blumenstingl <martin.blumenstingl@...glemail.com>
Cc: Corentin Labbe <clabbe@...libre.com>, gregkh@...uxfoundation.org,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
	david@...t.cz
Subject: Re: [PATCH v8 1/2] usb: serial: add support for CH348

On Tue, Jul 29, 2025 at 10:45:20PM +0200, Martin Blumenstingl wrote:
> On Tue, Jul 29, 2025 at 11:43 AM Johan Hovold <johan@...nel.org> wrote:
> > On Sat, Jul 26, 2025 at 04:54:17PM +0200, Martin Blumenstingl wrote:

> > > 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.
> >
> > Glad to hear you got it working. Did you confirm that you really need to
> > wait for THRE before submitting the next URB too? I don't see why the
> > vendor driver would be doing this otherwise, but perhaps it only affects
> > older, broken firmware, or something.

> I'm using Corentin's test script [0] for sending data and by
> connecting RX6 to TX7 and TX6 to RX7.

May be better to use a second different device (rather than loopback)
for testing so that you can separate any tx issues from rx issues.

> For a 1024 byte buffer:
> [ 3029.068311] ch348 ttyUSB6: submitted 509 bytes for urb 0
> [ 3029.068827] ch348 ttyUSB6: submitted 509 bytes for urb 1
> [ 3029.069363] ch348 ttyUSB7: submitted 5 bytes for urb 0
> [ 3029.069902] ch348 ttyUSB7: UART_IIR_THRI - unknown byte: 0x00
> [ 3029.215272] ch348 ttyUSB6: UART_IIR_THRI - unknown byte: 0x00
> [ 3029.215908] ch348 ttyUSB6: submitted 6 bytes for urb 0
> [ 3029.233628] ch348 ttyUSB6: UART_IIR_THRI - unknown byte: 0x00
> -> data is received without corruption
> 
> With a 2048 byte buffer the general flow seems fine:
> [ 3031.073101] ch348 ttyUSB6: submitted 509 bytes for urb 0
> [ 3031.073777] ch348 ttyUSB6: submitted 509 bytes for urb 1
> [ 3031.220068] ch348 ttyUSB6: UART_IIR_THRI - unknown byte: 0x00
> [ 3031.220697] ch348 ttyUSB6: submitted 509 bytes for urb 0
> [ 3031.221342] ch348 ttyUSB6: submitted 509 bytes for urb 1
> [ 3031.512113] ch348 ttyUSB6: UART_IIR_THRI - unknown byte: 0x00
> [ 3031.512795] ch348 ttyUSB6: submitted 12 bytes for urb 0
> [ 3031.513359] ch348 ttyUSB7: submitted 5 bytes for urb 0
> [ 3031.513859] ch348 ttyUSB7: UART_IIR_THRI - unknown byte: 0x00
> [ 3031.530476] ch348 ttyUSB6: UART_IIR_THRI - unknown byte: 0x00
> However, the receiving end sees different data (at around byte 513-518
> in my tests) than we wanted to send.
> 
> My general flow is:
> - check if we have received THRE - if not: don't transmit more data on this port
> - submit up to two URBs with up to 512 - 3 (CH348_TX_HDRSIZE) bytes to
> not exceed the HW TX FIFO size of 1024 bytes (page 1 in the datasheet)
> if the kfifo has enough data

If you're going to wait for the device fifo to clear completely you can
just use a single urb with larger (1k) buffer too.

> If you want me to test something else then please let me know and I'll try it.
> Otherwise I'll not dig much deeper, given the fact that I don't know
> how the firmware works (e.g. in which order they send the status to
> the host and what kind of state they hold internally) and we can still
> optimize this later.

Yeah, as long as you are certain that the generic implementation does
not work and that you indeed need to track THRE per port.

> [...]
> > > 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.
> >
> > Got a link to the datasheet? Not sure what they refer to as TNOW.

> Sure, try the direct link [1] - and if it doesn't work try [2].
> It doesn't document any registers, so it's a high-level datasheet -
> nor a programmers handbook.

Ok, so TNOW is used for RS485 to signal that the device is transmitting.
 
> > > 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.
> >
> > I don't think the console code will assert DTR/RTS, you need to open the
> > port as a regular tty.

Yes, even if the device is configured in hardware for TNOW mode (instead
of DTR function) you should still be able to control RTS (at least as
long as the device is not configured for automatic hardware flow control).

Johan

Powered by blists - more mailing lists