Re: [PATCH v13] USB: serial: cp210x: add support for GPIOs on CP2108

From: Johan Hovold
Date: Wed Jun 16 2021 - 11:45:53 EST


On Thu, Jun 10, 2021 at 03:28:44PM +0200, Johan Hovold wrote:
> From: Pho Tran <pho.tran@xxxxxxxxxx>
>
> Similar to some other CP210x device types, CP2108 has a number of GPIO
> pins that can be exposed through gpiolib.
>
> CP2108 has four serial interfaces but only one set of GPIO pins, which
> is modelled as a single gpio chip and registered as a child of the first
> interface.
>
> CP2108 has 16 GPIOs so the width of the state variables needs to be
> extended to 16 bits and this is also reflected in the control requests.
>
> Like CP2104, CP2108 have GPIO pins with configurable alternate
> functions and pins unavailable for GPIO use are determined and reported
> to gpiolib at probe.
>
> Signed-off-by: Pho Tran <pho.tran@xxxxxxxxxx>
> Co-developed-by: Tung Pham <tung.pham@xxxxxxxxxx>
> Signed-off-by: Tung Pham <tung.pham@xxxxxxxxxx>
> [ johan: rewrite gpio get() and set(); misc cleanups; amend commit
> message ]
> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> ---
>
> Tested using CP2108 and CP2102N.
>
> Changes in v13
> - rewrite cp210x_gpio_get() using a shared 16 bit mask and
> le16_to_cpus()
> - rewrite cp210x_gpio_set() using shared 16-bit mask and state
> variables
> - drop pointless no-op shift and mask operations during initialisation
> - reorder defines
> - reword some comments
> - fix some style issues
> - amend commit message
>
> v12 can be found here:
> - https://lore.kernel.org/r/20210426091244.19994-1-tupham@xxxxxxxxxx
>
>
> drivers/usb/serial/cp210x.c | 189 ++++++++++++++++++++++++++++++++----
> 1 file changed, 170 insertions(+), 19 deletions(-)

> @@ -1414,52 +1468,84 @@ static int cp210x_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> {
> struct usb_serial *serial = gpiochip_get_data(gc);
> struct cp210x_serial_private *priv = usb_get_serial_data(serial);
> - u8 req_type = REQTYPE_DEVICE_TO_HOST;
> + u8 req_type;
> + u16 mask;
> int result;
> - u8 buf;
> -
> - if (priv->partnum == CP210X_PARTNUM_CP2105)
> - req_type = REQTYPE_INTERFACE_TO_HOST;
> + int len;
>
> result = usb_autopm_get_interface(serial->interface);
> if (result)
> return result;
>
> - result = cp210x_read_vendor_block(serial, req_type,
> - CP210X_READ_LATCH, &buf, sizeof(buf));
> + switch (priv->partnum) {
> + case CP210X_PARTNUM_CP2105:
> + req_type = REQTYPE_INTERFACE_TO_HOST;
> + len = 1;
> + break;
> + case CP210X_PARTNUM_CP2108:
> + req_type = REQTYPE_INTERFACE_TO_HOST;
> + len = 2;
> + break;
> + default:
> + req_type = REQTYPE_DEVICE_TO_HOST;
> + len = 1;
> + break;
> + }
> +
> + mask = 0;
> + result = cp210x_read_vendor_block(serial, req_type, CP210X_READ_LATCH,
> + &mask, len);
> +
> usb_autopm_put_interface(serial->interface);
> +
> if (result < 0)
> return result;
>
> - return !!(buf & BIT(gpio));
> + le16_to_cpus((__le16 *)&mask);

Now applied after removing the (__le16 *) cast here.

> +
> + return !!(mask & BIT(gpio));
> }

Johan