diff mbox series

[1/2] USB: serial: ch341: fix lost character on LCR updates

Message ID 20220831081525.30557-2-johan@kernel.org (mailing list archive)
State Accepted
Commit 8e83622ae7ca481c76c8fd9579877f6abae64ca2
Headers show
Series USB: serial: ch341: fixes for older devices | expand

Commit Message

Johan Hovold Aug. 31, 2022, 8:15 a.m. UTC
Disable LCR updates for pre-0x30 devices which use a different (unknown)
protocol for line control and where the current register write causes
the next received character to be lost.

Note that updating LCR using the INIT command has no effect on these
devices either.

Reported-by: Jonathan Woithe <jwoithe@just42.net>
Link: https://lore.kernel.org/r/Ys1iPTfiZRWj2gXs@marvin.atrad.com.au
Fixes: 4e46c410e050 ("USB: serial: ch341: reinitialize chip on reconfiguration")
Fixes: 55fa15b5987d ("USB: serial: ch341: fix baud rate and line-control handling")
Cc: stable@vger.kernel.org      # 4.10
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/ch341.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Jiri Slaby Aug. 31, 2022, 8:36 a.m. UTC | #1
On 31. 08. 22, 10:15, Johan Hovold wrote:
> Disable LCR updates for pre-0x30 devices which use a different (unknown)
> protocol for line control and where the current register write causes
> the next received character to be lost.
> 
> Note that updating LCR using the INIT command has no effect on these
> devices either.
> 
> Reported-by: Jonathan Woithe <jwoithe@just42.net>
> Link: https://lore.kernel.org/r/Ys1iPTfiZRWj2gXs@marvin.atrad.com.au
> Fixes: 4e46c410e050 ("USB: serial: ch341: reinitialize chip on reconfiguration")
> Fixes: 55fa15b5987d ("USB: serial: ch341: fix baud rate and line-control handling")
> Cc: stable@vger.kernel.org      # 4.10
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>   drivers/usb/serial/ch341.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
> index 2798fca71261..2bcce172355b 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;

Could you move version above quirks? That would not create another 
7-byte padding in here. Actually it would not make ch341_private larger 
on 64bit at all, if I am looking correctly.

Other than that, looks good.

thanks,
Johan Hovold Aug. 31, 2022, 8:43 a.m. UTC | #2
On Wed, Aug 31, 2022 at 10:36:18AM +0200, Jiri Slaby wrote:
> On 31. 08. 22, 10:15, Johan Hovold wrote:
> > Disable LCR updates for pre-0x30 devices which use a different (unknown)
> > protocol for line control and where the current register write causes
> > the next received character to be lost.
> > 
> > Note that updating LCR using the INIT command has no effect on these
> > devices either.
> > 
> > Reported-by: Jonathan Woithe <jwoithe@just42.net>
> > Link: https://lore.kernel.org/r/Ys1iPTfiZRWj2gXs@marvin.atrad.com.au
> > Fixes: 4e46c410e050 ("USB: serial: ch341: reinitialize chip on reconfiguration")
> > Fixes: 55fa15b5987d ("USB: serial: ch341: fix baud rate and line-control handling")
> > Cc: stable@vger.kernel.org      # 4.10
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> >   drivers/usb/serial/ch341.c | 10 +++++++++-
> >   1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
> > index 2798fca71261..2bcce172355b 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;
> 
> Could you move version above quirks? That would not create another 
> 7-byte padding in here. Actually it would not make ch341_private larger 
> on 64bit at all, if I am looking correctly.

No, I added it after quirks on purpose as it isn't protected by the
spinlock and doesn't change during runtime like the shadow registers.

And I really don't care about saving 8 bytes on 64-bit. :)

Johan
diff mbox series

Patch

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 2798fca71261..2bcce172355b 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;
 };
 
@@ -265,6 +268,9 @@  static int ch341_set_baudrate_lcr(struct usb_device *dev,
 	 * (stop bits, parity and word length). Version 0x30 and above use
 	 * CH341_REG_LCR only and CH341_REG_LCR2 is always set to zero.
 	 */
+	if (priv->version < 0x30)
+		return 0;
+
 	r = ch341_control_out(dev, CH341_REQ_WRITE_REG,
 			      CH341_REG_LCR2 << 8 | CH341_REG_LCR, lcr);
 	if (r)
@@ -308,7 +314,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)