On Tue, Jul 19, 2022 at 04:04:53PM +0930, Jonathan Woithe wrote: > On Tue, Jul 19, 2022 at 07:53:42AM +0200, Johan Hovold wrote: > > I noticed one register write which was removed during 4.10 and we also > > added support for setting the LCR register which wasn't there before. > > > > Can you try the below patch which adds a register write that should > > match what the old driver did at 115200 baud? > > I applied this patch to 672c0c5. With the patch as supplied there was no > change: the first character was lost but all others were delivered as > expected. > > > If that doesn't help, can you try just ifdeffing out the LCR update? > > I changed the second of the added "#if -0" directives to "#if 1" (thus > removing the LCR update). This made things work: all characters were echoed > back to minicom as they were pressed. Perfect, thanks for confirming. > As a final test I changed the first of the added "#if 1" directives to > "#if 0", effectively removing the extra register write you added earlier. > The resulting kernel still worked. It appears that this extra register > write doesn't have any effect as far as the missing first character is > concerned. Thanks for checking. > On the basis of this, it seems the LCR update causes the loss of the first > character on version 0x27 hardware. Right, so it seems we have two issues here: First, the disabled rx timer, which we can fix. And second, the lost character after updating LCR, which we may be able to work around, for example, by disabling LCR control for older devices (i.e. pre-4.10 behaviour) or by not updating LCR unless the settings have changed (but that would still likely cause a lost character when we do). A third option may be to experiment with using the INIT command to set the line-speed and LCR, but that also has some side-effects like toggling the modem-control lines on termios changes, which I wanted to avoid. And we still need to set LCR on probe, which may trigger the lost character unless we do this using the INIT command and that turns out to work. Can you try the below patch and see if that still triggers the lost character after probe and line settings changes? Please also try to confirm that LCR can be updated this way at all. Note that using INIT like this seems to trigger a similar issue with my CH341A. Johan >From 1c937d58f83df00a7879ff82a2b980d11b3faf7b Mon Sep 17 00:00:00 2001 From: Johan Hovold <johan@xxxxxxxxxx> Date: Mon, 18 Jul 2022 10:21:41 +0200 Subject: [PATCH] USB: serial: ch341: fix disabled rx timer on older devices At least one older CH341 appears to have the RX timer enable bit inverted so that setting it disables the RX timer and prevents the FIFO from emptying until it is full. Only set the RX timer enable bit for devices with version newer than 0x27. Also try using the INIT command to update LCR to work around a separate issue with lost characters after LCR updates on older devices. Note that using INIT like this triggers a similar problem on newer CH341A. Reported-by: Jonathan Woithe <jwoithe@xxxxxxxxxx> Link: https://lore.kernel.org/r/Ys1iPTfiZRWj2gXs@xxxxxxxxxxxxxxxxxxx Not-Signed-off-yet-by: Johan Hovold <johan@xxxxxxxxxx> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx> --- drivers/usb/serial/ch341.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 2798fca71261..d6a4b2572c9c 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -97,7 +97,10 @@ struct ch341_private { u8 mcr; u8 msr; u8 lcr; + unsigned long quirks; + u8 version; + unsigned long break_end; }; @@ -250,8 +253,24 @@ static int ch341_set_baudrate_lcr(struct usb_device *dev, /* * CH341A buffers data until a full endpoint-size packet (32 bytes) * has been received unless bit 7 is set. + * + * At least one device with version 0x27 appears to have this bit + * inverted. + */ + if (priv->version > 0x27) + val |= BIT(7); + + /* + * At least one device with version 0x27 loses characters after + * updating LCR directly, try using INIT for that. + * + * Note that this appears to cause lost characters on newer CH341A. */ - val |= BIT(7); + if (priv->version <= 0x27) { + r = ch341_control_out(dev, CH341_REQ_SERIAL_INIT, + lcr << 8 | 0x9c, val); + return r; + } r = ch341_control_out(dev, CH341_REQ_WRITE_REG, CH341_REG_DIVISOR << 8 | CH341_REG_PRESCALER, @@ -308,7 +327,9 @@ static int ch341_configure(struct usb_device *dev, struct ch341_private *priv) r = ch341_control_in(dev, CH341_REQ_READ_VERSION, 0, 0, buffer, size); if (r) return r; - dev_dbg(&dev->dev, "Chip version: 0x%02x\n", buffer[0]); + + priv->version = buffer[0]; + dev_dbg(&dev->dev, "Chip version: 0x%02x\n", priv->version); r = ch341_control_out(dev, CH341_REQ_SERIAL_INIT, 0, 0); if (r < 0) -- 2.35.1
