| Message ID | 20210409155216.31867-3-johan@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d07082277f55cb395be00c813c62f3c956d1edb6 |
| Headers | show |
| Series | USB: serial: cp210x: provide gpio valid mask | expand |
On Fri, Apr 9, 2021 at 6:52 PM Johan Hovold <johan@kernel.org> wrote: > > Add a debug printk to dump the GPIO configuration stored in EEPROM > during probe. > > Signed-off-by: Johan Hovold <johan@kernel.org> > --- > drivers/usb/serial/cp210x.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c > index ceb3a656a075..ee595d1bea0a 100644 > --- a/drivers/usb/serial/cp210x.c > +++ b/drivers/usb/serial/cp210x.c > @@ -1543,10 +1543,16 @@ static int cp210x_gpio_init_valid_mask(struct gpio_chip *gc, > { > struct usb_serial *serial = gpiochip_get_data(gc); > struct cp210x_serial_private *priv = usb_get_serial_data(serial); > + struct device *dev = &serial->interface->dev; > unsigned long altfunc_mask = priv->gpio_altfunc; > > bitmap_complement(valid_mask, &altfunc_mask, ngpios); > > + if (bitmap_empty(valid_mask, ngpios)) > + dev_dbg(dev, "no pin configured for GPIO\n"); Shouldn't we drop the GPIO device completely in such a case? Bart, wouldn't it be a good idea for GPIO library to do something like this on driver's behalf? > + else > + dev_dbg(dev, "GPIO.%*pbl configured for GPIO\n", ngpios, > + valid_mask); A nit-pick: I would change GPIO -> pin in the second message in the first occurrence. > return 0; > } > > -- > 2.26.3 >
On Fri, Apr 09, 2021 at 07:22:26PM +0300, Andy Shevchenko wrote: > On Fri, Apr 9, 2021 at 6:52 PM Johan Hovold <johan@kernel.org> wrote: > > > > Add a debug printk to dump the GPIO configuration stored in EEPROM > > during probe. > > > > Signed-off-by: Johan Hovold <johan@kernel.org> > > --- > > drivers/usb/serial/cp210x.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c > > index ceb3a656a075..ee595d1bea0a 100644 > > --- a/drivers/usb/serial/cp210x.c > > +++ b/drivers/usb/serial/cp210x.c > > @@ -1543,10 +1543,16 @@ static int cp210x_gpio_init_valid_mask(struct gpio_chip *gc, > > { > > struct usb_serial *serial = gpiochip_get_data(gc); > > struct cp210x_serial_private *priv = usb_get_serial_data(serial); > > + struct device *dev = &serial->interface->dev; > > unsigned long altfunc_mask = priv->gpio_altfunc; > > > > bitmap_complement(valid_mask, &altfunc_mask, ngpios); > > > > + if (bitmap_empty(valid_mask, ngpios)) > > + dev_dbg(dev, "no pin configured for GPIO\n"); > > Shouldn't we drop the GPIO device completely in such a case? I considered it when we first added support for GPIOs to this driver but decided not to. The reason being that we want to to tell user-space that the device has gpio capability even if the GPIOs are currently muxed (in EEPROM) for other functionality. > Bart, wouldn't it be a good idea for GPIO library to do something like > this on driver's behalf? I'd say this is mostly useful for hotpluggable devices with EEPROM configuration and is probably best handled by the drivers. > > + else > > + dev_dbg(dev, "GPIO.%*pbl configured for GPIO\n", ngpios, > > + valid_mask); > > A nit-pick: > I would change GPIO -> pin in the second message in the first occurrence. "GPIO.n" are the actual pin names from the datasheet (cf. ftdi_sio which use "CBUSn" here). It's just a debug statement anyway. Johan
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index ceb3a656a075..ee595d1bea0a 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -1543,10 +1543,16 @@ static int cp210x_gpio_init_valid_mask(struct gpio_chip *gc, { struct usb_serial *serial = gpiochip_get_data(gc); struct cp210x_serial_private *priv = usb_get_serial_data(serial); + struct device *dev = &serial->interface->dev; unsigned long altfunc_mask = priv->gpio_altfunc; bitmap_complement(valid_mask, &altfunc_mask, ngpios); + if (bitmap_empty(valid_mask, ngpios)) + dev_dbg(dev, "no pin configured for GPIO\n"); + else + dev_dbg(dev, "GPIO.%*pbl configured for GPIO\n", ngpios, + valid_mask); return 0; }
Add a debug printk to dump the GPIO configuration stored in EEPROM during probe. Signed-off-by: Johan Hovold <johan@kernel.org> --- drivers/usb/serial/cp210x.c | 6 ++++++ 1 file changed, 6 insertions(+)