On Fri, Nov 01, 2013 at 12:49:22PM +0100, Frank Schäfer wrote:
> Some PL2303 chips are reported to lose bytes if the serial settings are set to
> the same values as before.
> At least HXD chips are affected, HX chips seem to be ok.
> The current code tries to avoid this by calling tty_termios_hw_change(), but
> this check isn't sufficient because different requested baud rates can result
> in identic encoded baud rates.
> The solution is to save and compare the encoded settings instead.
We're gonna need something along these lines, but this is just a hack
that not even correct.
> Signed-off-by: Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxx
> ---
> drivers/usb/serial/pl2303.c | 22 ++++++++++++++--------
> 1 Datei geändert, 14 Zeilen hinzugefügt(+), 8 Zeilen entfernt(-)
>
> diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> index bedf8e4..27a2691 100644
> --- a/drivers/usb/serial/pl2303.c
> +++ b/drivers/usb/serial/pl2303.c
> @@ -155,6 +155,7 @@ struct pl2303_private {
> spinlock_t lock;
> u8 line_control;
> u8 line_status;
> + u8 line_settings_enc[7]; /* baud rate, data bits, stop bits, parity */
> };
>
> static int pl2303_vendor_read(__u16 value, __u16 index,
> @@ -500,14 +501,6 @@ static void pl2303_set_termios(struct tty_struct *tty,
> int i;
> u8 control;
>
> - /*
> - * The PL2303 is reported to lose bytes if you change serial settings
> - * even to the same values as before. Thus we actually need to filter
> - * in this specific case.
> - */
> - if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
> - return;
> -
>
> buf = kzalloc(7, GFP_KERNEL);
> if (!buf) {
> dev_err(&port->dev, "%s - out of memory.\n", __func__);
Here another call to get the line settings is made. Have you verified
that that isn't related to the lost bytes?
> @@ -591,10 +584,23 @@ static void pl2303_set_termios(struct tty_struct *tty,
> dev_dbg(&port->dev, "parity = none\n");
> }
>
> + /*
> + * Some PL2303 chips are reported to lose bytes if you change the
> + * serial settings even to the same values as before.
> + * At least HXD chips are affected, HX chips seem to be ok.
> + * Thus we actually need to filter in this specific case.
> + * NOTE: a tty_termios_hw_change() check isn't sufficient, because
> + * different baud rates can result in identic encoded values.
> + */
> + if (!memcmp(priv->line_settings_enc, buf, 7))
> + return;
> +
Here you're returning if there no baud rate change, which means that
flow control is never handled a bit further down in set_termios.
> i = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
> SET_LINE_REQUEST, SET_LINE_REQUEST_TYPE,
> 0, 0, buf, 7, 100);
> dev_dbg(&port->dev, "0x21:0x20:0:0 %d\n", i);
> + if (i == 7)
> + memcpy(buf, priv->line_settings_enc, 7);
>
> /* change control lines if we are switching to or from B0 */
> spin_lock_irqsave(&priv->lock, flags);
> --
> 1.7.10.4
Again, this too much of a hack. There is a long-standing bug that needs
to be addressed. Let's think some more of how best to handle that (and
get to the bottom with exactly what requests are causing it, etc).
The important thing is to fix the regression. We can fix this bug during
the 3.13-cycle if the fix is not too invasive.
Sorry,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
