On Wed, Aug 09, 2017 at 02:18:06PM +0200, Sebastian Frei wrote:
Thanks for the patch. I have few comments below, and also make sure to
send the patch with a proper subject line, such as:
[PATCH v2] USB: serial: cp210x: fix partnum regression
> Make cp210x not abort if part number could not be read from device.
Please expand the commit message somewhat and mention your specific
device which now fails to probe. You should also mention the offending
commit that introduced the regression, preferably using a Fixes-tag:
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 f64e914a8985..d6afa823d6f0 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -1487,13 +1487,17 @@ static int cp210x_attach(struct usb_serial *serial)
> if (!priv)
> return -ENOMEM;
>
> + usb_set_serial_data(serial, priv);
> +
> 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 without GPIO support\n");
> + return 0;
Don't mention GPIO support in the error message, and instead of
returning early just continue with partnum set to a new constant:
#define CP210X_PARTNUM_UNKNOWN 255
> + }
>
> if (priv->partnum == CP210X_PARTNUM_CP2105) {
> result = cp2105_shared_gpio_init(serial);
> @@ -1504,10 +1508,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,
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
