Re: [PATCH v4] USB: serial: cp210x: Implement GPIO support for CP2102N

From: Martyn Welch
Date: Fri Jul 20 2018 - 11:22:26 EST

Next message: Kevin Hilman: "Re: linux-next: Signed-off-by missing for commits in the amlogic tree"
Previous message: Rob Herring: "Re: [PATCH 3/3] arm64: dts: synaptics: add dtsi file for Synaptics AS370 SoC"
In reply to: Karoly Pados: "[PATCH v4] USB: serial: cp210x: Implement GPIO support for CP2102N"
Next in thread: Johan Hovold: "Re: [PATCH v4] USB: serial: cp210x: Implement GPIO support for CP2102N"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Fri, 2018-07-20 at 12:35 +0000, Karoly Pados wrote:
> This patch adds GPIO support for CP2102N devices.
>Â
> It introduces new generic code to support emulating separate
> input and outputs directions even though these devices
> only know output modes (open-drain and pushpull). Existing
> GPIO support for CP2105 has been migrated over to the new
> code structure.
>Â
> Only limitation is that for the QFN28 variant, only 4 out of
> 7 GPIOs are supported. This is because the config array
> locations of the last 3 pins are not documented, and reverse
> engineering revealed offsets that conflicted with other
> documented functions. Hence we'll play it safe instead
> until somebody clears this up further.
>Â
> Signed-off-by: Karoly Pados <pados@xxxxxxxx>

As Johan made some changes, I think his Signed-off-by should be here
asÂis in v3.

Other than that:

Acked-by: Martyn Welch <martyn.welch@xxxxxxxxxxxxxxx>

> ---
>Â
> Patch changelog:
> v1: Initial implementation.
> v2: Incorporate Johan's feedback, most importantly generalize
>ÂÂÂÂÂnew code and unify with CP2105.
> v3 (by Johan): Apply stylistic and minor bug fixes.
> v4: Corrected commit message. No code changes.
>Â
>Â
>ÂÂdrivers/usb/serial/cp210x.c | 245 ++++++++++++++++++++++++++++++--
> ----
>ÂÂ1 file changed, 209 insertions(+), 36 deletions(-)
>Â
> diff --git a/drivers/usb/serial/cp210x.c
> b/drivers/usb/serial/cp210x.c
> index 4a118eb13590..b9bc80700be7 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -224,9 +224,10 @@ MODULE_DEVICE_TABLE(usb, id_table);
>ÂÂstruct cp210x_serial_private {
>ÂÂ#ifdef CONFIG_GPIOLIB
>ÂÂ struct gpio_chip gc;
> - u8 config;
> - u8 gpio_mode;
>ÂÂ bool gpio_registered;
> + u8 gpio_pushpull;
> + u8 gpio_altfunc;
> + u8 gpio_input;
>ÂÂ#endif
>ÂÂ u8 partnum;
>ÂÂ speed_t max_speed;
> @@ -343,6 +344,7 @@ static struct usb_serial_driver * const
> serial_drivers[] = {
>ÂÂ#define CONTROL_WRITE_RTS 0x0200
>ÂÂ
>ÂÂ/* CP210X_VENDOR_SPECIFIC values */
> +#define CP210X_READ_2NCONFIG 0x000E
>ÂÂ#define CP210X_READ_LATCH 0x00C2
>ÂÂ#define CP210X_GET_PARTNUM 0x370B
>ÂÂ#define CP210X_GET_PORTCONFIG 0x370C
> @@ -452,6 +454,12 @@ struct cp210x_config {
>ÂÂ#define CP2105_GPIO1_RXLED_MODE BIT(1)
>ÂÂ#define CP2105_GPIO1_RS485_MODE BIT(2)
>ÂÂ
> +/* CP2102N configuration array indices */
> +#define CP210X_2NCONFIG_CONFIG_VERSION_IDX 2
> +#define CP210X_2NCONFIG_GPIO_MODE_IDX 581
> +#define CP210X_2NCONFIG_GPIO_RSTLATCH_IDX 587
> +#define CP210X_2NCONFIG_GPIO_CONTROL_IDX 600
> +
>ÂÂ/* CP210X_VENDOR_SPECIFIC, CP210X_WRITE_LATCH call writes these 0x2
> bytes. */
>ÂÂstruct cp210x_gpio_write {
>ÂÂ u8 mask;
> @@ -1313,17 +1321,8 @@ static int cp210x_gpio_request(struct
> gpio_chip *gc, unsigned int offset)
>ÂÂ struct usb_serial *serial = gpiochip_get_data(gc);
>ÂÂ struct cp210x_serial_private *priv =
> usb_get_serial_data(serial);
>ÂÂ
> - switch (offset) {
> - case 0:
> - if (priv->config & CP2105_GPIO0_TXLED_MODE)
> - return -ENODEV;
> - break;
> - case 1:
> - if (priv->config & (CP2105_GPIO1_RXLED_MODE |
> - ÂÂÂÂCP2105_GPIO1_RS485_MODE))
> - return -ENODEV;
> - break;
> - }
> + if (priv->gpio_altfunc & BIT(offset))
> + return -ENODEV;
>ÂÂ
>ÂÂ return 0;
>ÂÂ}
> @@ -1331,10 +1330,15 @@ static int cp210x_gpio_request(struct
> gpio_chip *gc, unsigned int offset)
>ÂÂ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;
>ÂÂ int result;
>ÂÂ u8 buf;
>ÂÂ
> - result = cp210x_read_vendor_block(serial,
> REQTYPE_INTERFACE_TO_HOST,
> + if (priv->partnum == CP210X_PARTNUM_CP2105)
> + req_type = REQTYPE_INTERFACE_TO_HOST;
> +
> + result = cp210x_read_vendor_block(serial, req_type,
>ÂÂ ÂÂCP210X_READ_LATCH, &buf,
> sizeof(buf));
>ÂÂ if (result < 0)
>ÂÂ return result;
> @@ -1345,7 +1349,9 @@ static int cp210x_gpio_get(struct gpio_chip
> *gc, unsigned int gpio)
>ÂÂstatic void cp210x_gpio_set(struct gpio_chip *gc, unsigned int gpio,
> int value)
>ÂÂ{
>ÂÂ struct usb_serial *serial = gpiochip_get_data(gc);
> + struct cp210x_serial_private *priv =
> usb_get_serial_data(serial);
>ÂÂ struct cp210x_gpio_write buf;
> + int result;
>ÂÂ
>ÂÂ if (value == 1)
>ÂÂ buf.state = BIT(gpio);
> @@ -1354,25 +1360,68 @@ static void cp210x_gpio_set(struct gpio_chip
> *gc, unsigned int gpio, int value)
>ÂÂ
>ÂÂ buf.mask = BIT(gpio);
>ÂÂ
> - cp210x_write_vendor_block(serial, REQTYPE_HOST_TO_INTERFACE,
> - ÂÂCP210X_WRITE_LATCH, &buf,
> sizeof(buf));
> + if (priv->partnum == CP210X_PARTNUM_CP2105) {
> + result = cp210x_write_vendor_block(serial,
> + ÂÂÂREQTYPE_HOST_TO_I
> NTERFACE,
> + ÂÂÂCP210X_WRITE_LATC
> H, &buf,
> + ÂÂÂsizeof(buf));
> + } else {
> + u16 wIndex = buf.state << 8 | buf.mask;
> +
> + result = usb_control_msg(serial->dev,
> + Âusb_sndctrlpipe(serial-
> >dev, 0),
> + ÂCP210X_VENDOR_SPECIFIC,
> + ÂREQTYPE_HOST_TO_DEVICE,
> + ÂCP210X_WRITE_LATCH,
> + ÂwIndex,
> + ÂNULL, 0,
> USB_CTRL_SET_TIMEOUT);
> + }
> +
> + if (result < 0) {
> + dev_err(&serial->interface->dev, "failed to set GPIO
> value: %d\n",
> + result);
> + }
>ÂÂ}
>ÂÂ
>ÂÂstatic int cp210x_gpio_direction_get(struct gpio_chip *gc, unsigned
> int gpio)
>ÂÂ{
> - /* Hardware does not support an input mode */
> - return 0;
> + struct usb_serial *serial = gpiochip_get_data(gc);
> + struct cp210x_serial_private *priv =
> usb_get_serial_data(serial);
> +
> + return priv->gpio_input & BIT(gpio);
>ÂÂ}
>ÂÂ
>ÂÂstatic int cp210x_gpio_direction_input(struct gpio_chip *gc,
> unsigned int gpio)
>ÂÂ{
> - /* Hardware does not support an input mode */
> - return -ENOTSUPP;
> + struct usb_serial *serial = gpiochip_get_data(gc);
> + struct cp210x_serial_private *priv =
> usb_get_serial_data(serial);
> +
> + if (priv->partnum == CP210X_PARTNUM_CP2105) {
> + /* hardware does not support an input mode */
> + return -ENOTSUPP;
> + }
> +
> + /* push-pull pins cannot be changed to be inputs */
> + if (priv->gpio_pushpull & BIT(gpio))
> + return -EINVAL;
> +
> + /* make sure to release pin if it is being driven low */
> + cp210x_gpio_set(gc, gpio, 1);
> +
> + priv->gpio_input |= BIT(gpio);
> +
> + return 0;
>ÂÂ}
>ÂÂ
>ÂÂstatic int cp210x_gpio_direction_output(struct gpio_chip *gc,
> unsigned int gpio,
>ÂÂ int value)
>ÂÂ{
> + struct usb_serial *serial = gpiochip_get_data(gc);
> + struct cp210x_serial_private *priv =
> usb_get_serial_data(serial);
> +
> + priv->gpio_input &= ~BIT(gpio);
> + cp210x_gpio_set(gc, gpio, value);
> +
>ÂÂ return 0;
>ÂÂ}
>ÂÂ
> @@ -1385,11 +1434,11 @@ static int cp210x_gpio_set_config(struct
> gpio_chip *gc, unsigned int gpio,
>ÂÂ
>ÂÂ /* Succeed only if in correct mode (this can't be set at
> runtime) */
>ÂÂ if ((param == PIN_CONFIG_DRIVE_PUSH_PULL) &&
> - ÂÂÂÂ(priv->gpio_mode & BIT(gpio)))
> + ÂÂÂÂ(priv->gpio_pushpull & BIT(gpio)))
>ÂÂ return 0;
>ÂÂ
>ÂÂ if ((param == PIN_CONFIG_DRIVE_OPEN_DRAIN) &&
> - ÂÂÂÂ!(priv->gpio_mode & BIT(gpio)))
> + ÂÂÂÂ!(priv->gpio_pushpull & BIT(gpio)))
>ÂÂ return 0;
>ÂÂ
>ÂÂ return -ENOTSUPP;
> @@ -1402,12 +1451,13 @@ static int cp210x_gpio_set_config(struct
> gpio_chip *gc, unsigned int gpio,
>ÂÂÂ* this driver that provide GPIO do so in a way that does not impact
> other
>ÂÂÂ* signals and are thus expected to have very different
> initialisation.
>ÂÂÂ*/
> -static int cp2105_shared_gpio_init(struct usb_serial *serial)
> +static int cp2105_gpioconf_init(struct usb_serial *serial)
>ÂÂ{
>ÂÂ struct cp210x_serial_private *priv =
> usb_get_serial_data(serial);
>ÂÂ struct cp210x_pin_mode mode;
>ÂÂ struct cp210x_config config;
>ÂÂ u8 intf_num = cp210x_interface_num(serial);
> + u8 iface_config;
>ÂÂ int result;
>ÂÂ
>ÂÂ result = cp210x_read_vendor_block(serial,
> REQTYPE_DEVICE_TO_HOST,
> @@ -1424,20 +1474,26 @@ static int cp2105_shared_gpio_init(struct
> usb_serial *serial)
>ÂÂ
>ÂÂ /*ÂÂ2 banks of GPIO - One for the pins taken from each
> serial port */
>ÂÂ if (intf_num == 0) {
> - if (mode.eci == CP210X_PIN_MODE_MODEM)
> + if (mode.eci == CP210X_PIN_MODE_MODEM) {
> + /* mark all GPIOs of this interface as
> reserved */
> + priv->gpio_altfunc = 0xff;
>ÂÂ return 0;
> + }
>ÂÂ
> - priv->config = config.eci_cfg;
> - priv->gpio_mode =
> (u8)((le16_to_cpu(config.gpio_mode) &
> + iface_config = config.eci_cfg;
> + priv->gpio_pushpull =
> (u8)((le16_to_cpu(config.gpio_mode) &
>ÂÂ CP210X_ECI_GPIO_MODE
> _MASK) >>
>ÂÂ CP210X_ECI_GPIO_MODE
> _OFFSET);
>ÂÂ priv->gc.ngpio = 2;
>ÂÂ } else if (intf_num == 1) {
> - if (mode.sci == CP210X_PIN_MODE_MODEM)
> + if (mode.sci == CP210X_PIN_MODE_MODEM) {
> + /* mark all GPIOs of this interface as
> reserved */
> + priv->gpio_altfunc = 0xff;
>ÂÂ return 0;
> + }
>ÂÂ
> - priv->config = config.sci_cfg;
> - priv->gpio_mode =
> (u8)((le16_to_cpu(config.gpio_mode) &
> + iface_config = config.sci_cfg;
> + priv->gpio_pushpull =
> (u8)((le16_to_cpu(config.gpio_mode) &
>ÂÂ CP210X_SCI_GPIO_MODE
> _MASK) >>
>ÂÂ CP210X_SCI_GPIO_MODE
> _OFFSET);
>ÂÂ priv->gc.ngpio = 3;
> @@ -1445,6 +1501,125 @@ static int cp2105_shared_gpio_init(struct
> usb_serial *serial)
>ÂÂ return -ENODEV;
>ÂÂ }
>ÂÂ
> + /* mark all pins which are not in GPIO mode */
> + if (iface_config & CP2105_GPIO0_TXLED_MODE) /* GPIO 0
> */
> + priv->gpio_altfunc |= BIT(0);
> + if (iface_config & (CP2105_GPIO1_RXLED_MODE | /* GPIO
> 1 */
> + CP2105_GPIO1_RS485_MODE))
> + priv->gpio_altfunc |= BIT(1);
> +
> + /* driver implementation for CP2105 only supports outputs */
> + priv->gpio_input = 0;
> +
> + return 0;
> +}
> +
> +static int cp2102n_gpioconf_init(struct usb_serial *serial)
> +{
> + struct cp210x_serial_private *priv =
> usb_get_serial_data(serial);
> + const u16 config_size = 0x02a6;
> + u8 gpio_rst_latch;
> + u8 config_version;
> + u8 gpio_pushpull;
> + u8 *config_buf;
> + u8 gpio_latch;
> + u8 gpio_ctrl;
> + int result;
> + u8 i;
> +
> + /*
> + Â* Retrieve device configuration from the device.
> + Â* The array received contains all customization settings
> done at the
> + Â* factory/manufacturer. Format of the array is documented
> at the
> + Â* time of writing at:
> + Â* https://www.silabs.com/community/interface/knowledge-base
> .entry.html/2017/03/31/cp2102n_setconfig-xsfa
> + Â*/
> + config_buf = kmalloc(config_size, GFP_KERNEL);
> + if (!config_buf)
> + return -ENOMEM;
> +
> + result = cp210x_read_vendor_block(serial,
> + ÂÂREQTYPE_DEVICE_TO_HOST,
> + ÂÂCP210X_READ_2NCONFIG,
> + ÂÂconfig_buf,
> + ÂÂconfig_size);
> + if (result < 0) {
> + kfree(config_buf);
> + return result;
> + }
> +
> + config_version =
> config_buf[CP210X_2NCONFIG_CONFIG_VERSION_IDX];
> + gpio_pushpull = config_buf[CP210X_2NCONFIG_GPIO_MODE_IDX];
> + gpio_ctrl = config_buf[CP210X_2NCONFIG_GPIO_CONTROL_IDX];
> + gpio_rst_latch =
> config_buf[CP210X_2NCONFIG_GPIO_RSTLATCH_IDX];
> +
> + kfree(config_buf);
> +
> + /* Make sure this is a config format we understand. */
> + if (config_version != 0x01)
> + return -ENOTSUPP;
> +
> + /*
> + Â* We only support 4 GPIOs even on the QFN28 package,
> because
> + Â* config locations of GPIOs 4-6 determined using reverse
> + Â* engineering revealed conflicting offsets with other
> + Â* documented functions. So we'll just play it safe for now.
> + Â*/
> + priv->gc.ngpio = 4;
> +
> + /*
> + Â* Get default pin states after reset. Needed so we can
> determine
> + Â* the direction of an open-drain pin.
> + Â*/
> + gpio_latch = (gpio_rst_latch >> 3) & 0x0f;
> +
> + /* 0 indicates open-drain mode, 1 is push-pull */
> + priv->gpio_pushpull = (gpio_pushpull >> 3) & 0x0f;
> +
> + /* 0 indicates GPIO mode, 1 is alternate function */
> + priv->gpio_altfunc = (gpio_ctrl >> 2) & 0x0f;
> +
> + /*
> + Â* The CP2102N does not strictly has input and output pin
> modes,
> + Â* it only knows open-drain and push-pull modes which is set
> at
> + Â* factory. An open-drain pin can function both as an
> + Â* input or an output. We emulate input mode for open-drain
> pins
> + Â* by making sure they are not driven low, and we do not
> allow
> + Â* push-pull pins to be set as an input.
> + Â*/
> + for (i = 0; i < priv->gc.ngpio; ++i) {
> + /*
> + Â* Set direction to "input" iff pin is open-drain
> and reset
> + Â* value is 1.
> + Â*/
> + if (!(priv->gpio_pushpull & BIT(i)) && (gpio_latch &
> BIT(i)))
> + priv->gpio_input |= BIT(i);
> + }
> +
> + return 0;
> +}
> +
> +static int cp210x_gpio_init(struct usb_serial *serial)
> +{
> + struct cp210x_serial_private *priv =
> usb_get_serial_data(serial);
> + int result;
> +
> + switch (priv->partnum) {
> + case CP210X_PARTNUM_CP2105:
> + result = cp2105_gpioconf_init(serial);
> + break;
> + case CP210X_PARTNUM_CP2102N_QFN28:
> + case CP210X_PARTNUM_CP2102N_QFN24:
> + case CP210X_PARTNUM_CP2102N_QFN20:
> + result = cp2102n_gpioconf_init(serial);
> + break;
> + default:
> + return 0;
> + }
> +
> + if (result < 0)
> + return result;
> +
>ÂÂ priv->gc.label = "cp210x";
>ÂÂ priv->gc.request = cp210x_gpio_request;
>ÂÂ priv->gc.get_direction = cp210x_gpio_direction_get;
> @@ -1477,7 +1652,7 @@ static void cp210x_gpio_remove(struct
> usb_serial *serial)
>ÂÂ
>ÂÂ#else
>ÂÂ
> -static int cp2105_shared_gpio_init(struct usb_serial *serial)
> +static int cp210x_gpio_init(struct usb_serial *serial)
>ÂÂ{
>ÂÂ return 0;
>ÂÂ}
> @@ -1588,12 +1763,10 @@ static int cp210x_attach(struct usb_serial
> *serial)
>ÂÂ
>ÂÂ cp210x_init_max_speed(serial);
>ÂÂ
> - if (priv->partnum == CP210X_PARTNUM_CP2105) {
> - result = cp2105_shared_gpio_init(serial);
> - if (result < 0) {
> - dev_err(&serial->interface->dev,
> - "GPIO initialisation failed,
> continuing without GPIO support\n");
> - }
> + result = cp210x_gpio_init(serial);
> + if (result < 0) {
> + dev_err(&serial->interface->dev, "GPIO
> initialisation failed: %d\n",
> + result);
>ÂÂ }
>ÂÂ
>ÂÂ return 0;

Next message: Kevin Hilman: "Re: linux-next: Signed-off-by missing for commits in the amlogic tree"
Previous message: Rob Herring: "Re: [PATCH 3/3] arm64: dts: synaptics: add dtsi file for Synaptics AS370 SoC"
In reply to: Karoly Pados: "[PATCH v4] USB: serial: cp210x: Implement GPIO support for CP2102N"
Next in thread: Johan Hovold: "Re: [PATCH v4] USB: serial: cp210x: Implement GPIO support for CP2102N"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]