On Thu, Sep 15, 2016 at 04:57:34PM +0100, Aidan Thornton wrote:
> Changing the LCR register after initialization does not seem to be
> reliable on all chips (particularly not on CH341A). Restructure
> initialization and configuration to always reinit the chip on
> configuration changes instead and pass the LCR register value directly
> to the initialization command.
>
> Cleaned-up version of a patch by Grigori Goronzy
>
> Signed-off-by: Aidan Thornton <makosoft@xxxxxxxxx>
This series looks good to me, I just a couple of comments to this one.
> ---
> drivers/usb/serial/ch341.c | 38 ++++++++++++++++++++------------------
> 1 file changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
> index bdf525f..ce799d0 100644
> --- a/drivers/usb/serial/ch341.c
> +++ b/drivers/usb/serial/ch341.c
> @@ -132,10 +132,10 @@ static int ch341_control_in(struct usb_device *dev,
> return r;
> }
>
> -static int ch341_set_baudrate(struct usb_device *dev,
> - struct ch341_private *priv)
> +static int ch341_init_set_baudrate(struct usb_device *dev,
> + struct ch341_private *priv, unsigned ctrl)
> {
> - short a, b;
> + short a;
> int r;
> unsigned long factor;
> short divisor;
> @@ -155,11 +155,9 @@ static int ch341_set_baudrate(struct usb_device *dev,
>
> factor = 0x10000 - factor;
> a = (factor & 0xff00) | divisor;
> - b = factor & 0xff;
>
> - r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x1312, a);
> - if (!r)
> - r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x0f2c, b);
> + /* 0x9c is "enable SFR_UART Control register and timer" */
> + r = ch341_control_out(dev, CH341_REQ_SERIAL_INIT, 0x9c | (ctrl << 8), a | 0x80);
Please break this line or restructure the code to stay within 80 cols
(checkpatch.pl would have let you know).
> return r;
> }
> @@ -218,16 +216,12 @@ static int ch341_configure(struct usb_device *dev, struct ch341_private *priv)
> if (r < 0)
> goto out;
>
> - r = ch341_set_baudrate(dev, priv);
> - if (r < 0)
> - goto out;
> -
> /* expect two bytes 0x56 0x00 */
> r = ch341_control_in(dev, CH341_REQ_READ_REG, 0x2518, 0, buffer, size);
> if (r < 0)
> goto out;
>
> - r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x2518, 0x0050);
> + r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x2518, 0x00c3);
Why is this change needed? I see no write to this register in the
vendor driver.
> if (r < 0)
> goto out;
>
Thanks,
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
