Message-ID: <aIiXyEuPmWU00hFf@hovoldconsulting.com>
Date: Tue, 29 Jul 2025 11:43:36 +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 Sat, Jul 26, 2025 at 04:54:17PM +0200, Martin Blumenstingl wrote:
> On Fri, Jul 25, 2025 at 12:07 PM Johan Hovold <johan@...nel.org> wrote:

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

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.

> > > > 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 TTY layer will assert DTR/RTS on open() and deassert on close() as
long as HUPCL is set. If you implement tiocmset() you can use that to
toggle the lines as well.

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

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

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

That should not be needed. It looked like the vendor driver had some
variant of the usual request to control the modem lines. That should be
all that is needed.

Look at the vendor driver implementation of ch9344_tty_tiocmset() and
ch9344_port_dtr_rts().

Johan

Powered by blists - more mailing lists