On Tue, Sep 12, 2017 at 09:50:59AM +0200, Sebastian Frei wrote:
> When adding GPIO support for the cp2105, the mentioned commit by Martyn
> Welch introduced a query for the part number of the chip. Unfortunately the
> driver aborts loading when this query fails, so currently the driver can not
> be used with chips not supporting this query.
> I have a data cable for Siemens mobile phones (ID 10ab:10c5) where this is the
> case.
> With this patch the driver can be loaded even if the part number can not be
> queried.
I edited the above slightly before applying replacing "loading" with
"probing" and "loaded" with "bound" which are the terms we use for
the process of binding a driver to a new device.
Also try to keep your commit summaries (mail Subject) below 72 cols or
so.
> Fixes: cf5276ce7867 ("USB: serial: cp210x: Adding GPIO support for CP2105")
> Signed-off-by: Sebastian Frei <dr.nop@xxxxxxx>
>
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index 2d945c9f975c..a3e301b1eb73 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -352,6 +352,7 @@ static struct usb_serial_driver * const serial_drivers[] =
> {
This patch did not apply due to the above line wrapped. It was easy
enough to fix this time, but please make sure your mail setup does not
corrupt patches in the future.
> #define CP210X_PARTNUM_CP2104 0x04
> #define CP210X_PARTNUM_CP2105 0x05
> #define CP210X_PARTNUM_CP2108 0x08
> +#define CP210X_PARTNUM_UNKNOWN 0xFF
>
> /* CP210X_GET_COMM_STATUS returns these 0x13 bytes */
> struct cp210x_comm_status {
> @@ -1488,13 +1489,17 @@ static int cp210x_attach(struct usb_serial *serial)
> if (!priv)
> return -ENOMEM;
>
> + usb_set_serial_data(serial, priv);
> +
Moving usb_set_serial_data() is no longer needed with the changes you
did in v3 so I dropped this part of the change.
> result = cp210x_read_vendor_block(serial, REQTYPE_DEVICE_TO_HOST,
> CP210X_GET_PARTNUM, &priv->partnum,
> sizeof(priv->partnum));
> - if (result < 0)
> - goto err_free_priv;
>
> - usb_set_serial_data(serial, priv);
> + if (result < 0) {
> + dev_err(&serial->interface->dev,
> + "querying part number failed, continuing anyways\n");
And shortened this error message.
> + priv->partnum = CP210X_PARTNUM_UNKNOWN;
> + }
>
> if (priv->partnum == CP210X_PARTNUM_CP2105) {
> result = cp2105_shared_gpio_init(serial);
> @@ -1505,10 +1510,6 @@ static int cp210x_attach(struct usb_serial *serial)
> }
>
> return 0;
> -err_free_priv:
> - kfree(priv);
> -
> - return result;
> }
>
> static void cp210x_disconnect(struct usb_serial *serial)
>
Thanks for finding and fixing this regression.
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
