On Fri, Apr 15, 2016 at 11:14:08PM +0200, Grigori Goronzy 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.
> 
> v2: get rid of unused variable, improve error handling.
> 
> Tested-by: Ryan Barber <r...@skyscraper.nu>
> Signed-off-by: Grigori Goronzy <g...@chown.ath.cx>
> ---
>  drivers/usb/serial/ch341.c | 47 
> ++++++++++++++++++++++++----------------------
>  1 file changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
> index 95c8a40..6181616 100644
> --- a/drivers/usb/serial/ch341.c
> +++ b/drivers/usb/serial/ch341.c
> @@ -61,6 +61,8 @@
>   * the Net/FreeBSD uchcom.c driver by Takanori Watanabe.  Domo arigato.
>   */
>  
> +#define CH341_SERIAL_INIT      0xA1
> +#define CH341_VERSION          0x5F

Use the CH341_REQ prefix keep the entries sorted.

>  #define CH341_MODEM_CTRL       0xA4
>  #define CH341_REQ_WRITE_REG    0x9A
>  #define CH341_REQ_READ_REG     0x95
> @@ -129,10 +131,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;
> @@ -152,11 +154,8 @@ static int ch341_set_baudrate(struct usb_device *dev,
>  
>       factor = 0x10000 - factor;
>       a = (factor & 0xff00) | divisor;
> -     b = factor & 0xff;
>  
> -     r = ch341_control_out(dev, 0x9a, 0x1312, a);
> -     if (!r)
> -             r = ch341_control_out(dev, 0x9a, 0x0f2c, b);
> +     r = ch341_control_out(dev, CH341_SERIAL_INIT, 0x9c | (ctrl << 8), a | 
> 0x80);
>  
>       return r;
>  }
> @@ -177,7 +176,7 @@ static int ch341_get_status(struct usb_device *dev, 
> struct ch341_private *priv)
>       if (!buffer)
>               return -ENOMEM;
>  
> -     r = ch341_control_in(dev, 0x95, 0x0706, 0, buffer, size);
> +     r = ch341_control_in(dev, CH341_REQ_READ_REG, 0x0706, 0, buffer, size);

This looks like an unrelated change.

>       if (r < 0)
>               goto out;
>  
> @@ -207,24 +206,20 @@ static int ch341_configure(struct usb_device *dev, 
> struct ch341_private *priv)
>               return -ENOMEM;
>  
>       /* expect two bytes 0x27 0x00 */
> -     r = ch341_control_in(dev, 0x5f, 0, 0, buffer, size);
> +     r = ch341_control_in(dev, CH341_VERSION, 0, 0, buffer, size);

Ditto.

>       if (r < 0)
>               goto out;
>  
> -     r = ch341_control_out(dev, 0xa1, 0, 0);
> -     if (r < 0)
> -             goto out;
> -
> -     r = ch341_set_baudrate(dev, priv);
> +     r = ch341_control_out(dev, CH341_SERIAL_INIT, 0, 0);
>       if (r < 0)
>               goto out;
>  
>       /* expect two bytes 0x56 0x00 */
> -     r = ch341_control_in(dev, 0x95, 0x2518, 0, buffer, size);
> +     r = ch341_control_in(dev, CH341_REQ_READ_REG, 0x2518, 0, buffer, size);

So does this.

Please fix up all the magic constants in a preparatory patch, which
should make it easier to see what's really going on here.

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to