[PATCH V1 0/6] USB: serial: f81232: Add F81534A support

[PATCH V1 0/6] USB: serial: f81232: Add F81534A support

[Ji-Ze Hong (Peter Hong)]

This series patches will add Fintek F81532A/534A/535/536 support and
refactoring some source code.

The Fintek F81532A/534A/535/536 is USB-to-2/4/8/12 serial ports device.
It cotains a HUB, a GPIO device and 2/4/8/12 serial ports. The F81534A
series will default enable only HUB & GPIO device when plugged and disable
UARTs as default. We need control GPIO device to enable serial port with
special sequence.

The most serial port features of F81534A series is same with F81232.
That's the difference with following:
	1. More RX FIFO and cache. (128byte FIFO + max to 128bytes*4 cache)
	2. up to 3MBits baudrate.
	3. 3x GPIOs per port to control transceiver.
	4. UART devices need enabled by GPIO device register.

Ji-Ze Hong (Peter Hong) (6):
  USB: serial: f81232: Add F81534A support
  USB: serial: f81232: Force F81534A with RS232 mode
  USB: serial: f81232: Add generator for F81534A
  USB: serial: f81232: Add tx_empty function
  USB: serial: f81232: Use devm_kzalloc
  USB: serial: f81232: Add gpiolib to GPIO device

 drivers/usb/serial/f81232.c | 760 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 741 insertions(+), 19 deletions(-)

-- 
2.7.4

[PATCH V1 1/6] USB: serial: f81232: Add F81534A support

[Ji-Ze Hong (Peter Hong)]

The Fintek F81532A/534A/535/536 is USB-to-2/4/8/12 serial ports device.
It's most same with F81232, the UART device is difference as follow:
	1. TX/RX bulk size is 128/512bytes
	2. RX bulk layout change:
		F81232: [LSR(1Byte)+DATA(1Byte)][LSR(1Byte)+DATA(1Byte)]...
		F81534A:[LEN][Data.....][LSR]

Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
---
 drivers/usb/serial/f81232.c | 153 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 144 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 43fa1f0716b7..84efcc66aa56 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
  * Fintek F81232 USB to serial adaptor driver
+ * Fintek F81532A/534A/535/536 USB to 2/4/8/12 serial adaptor driver
  *
  * Copyright (C) 2012 Greg Kroah-Hartman (gregkh@linuxfoundation.org)
  * Copyright (C) 2012 Linux Foundation
@@ -22,7 +23,20 @@
 #include <linux/serial_reg.h>
 
 static const struct usb_device_id id_table[] = {
+	/* F81232 */
 	{ USB_DEVICE(0x1934, 0x0706) },
+
+	/* F81532A/534A/535/536 */
+	{ USB_DEVICE(0x2c42, 0x1602) },		/* In-Box 2 port UART device */
+	{ USB_DEVICE(0x2c42, 0x1604) },		/* In-Box 4 port UART device */
+	{ USB_DEVICE(0x2c42, 0x1605) },		/* In-Box 8 port UART device */
+	{ USB_DEVICE(0x2c42, 0x1606) },		/* In-Box 12 port UART device */
+	{ USB_DEVICE(0x2c42, 0x1608) },		/* Non-Flash type */
+
+	{ USB_DEVICE(0x2c42, 0x1632) },		/* 2 port UART device */
+	{ USB_DEVICE(0x2c42, 0x1634) },		/* 4 port UART device */
+	{ USB_DEVICE(0x2c42, 0x1635) },		/* 8 port UART device */
+	{ USB_DEVICE(0x2c42, 0x1636) },		/* 12 port UART device */
 	{ }					/* Terminating entry */
 };
 MODULE_DEVICE_TABLE(usb, id_table);
@@ -61,15 +75,25 @@ MODULE_DEVICE_TABLE(usb, id_table);
 #define F81232_CLK_14_77_MHZ		(BIT(1) | BIT(0))
 #define F81232_CLK_MASK			GENMASK(1, 0)
 
+#define F81534A_MODE_CONF_REG		0x107
+#define F81534A_TRIGGER_MASK		GENMASK(3, 2)
+#define F81534A_TRIGGER_MULTPILE_4X	BIT(3)
+#define F81534A_FIFO_128BYTE		(BIT(1) | BIT(0))
+
+#define F81232_F81232_TYPE		1
+#define F81232_F81534A_TYPE		2
+
 struct f81232_private {
 	struct mutex lock;
 	u8 modem_control;
 	u8 modem_status;
 	u8 shadow_lcr;
+	u8 device_type;
 	speed_t baud_base;
 	struct work_struct lsr_work;
 	struct work_struct interrupt_work;
 	struct usb_serial_port *port;
+	void (*process_read_urb)(struct urb *urb);
 };
 
 static u32 const baudrate_table[] = { 115200, 921600, 1152000, 1500000 };
@@ -376,6 +400,78 @@ static void f81232_process_read_urb(struct urb *urb)
 	tty_flip_buffer_push(&port->port);
 }
 
+static void f81534a_process_read_urb(struct urb *urb)
+{
+	struct usb_serial_port *port = urb->context;
+	struct f81232_private *priv = usb_get_serial_port_data(port);
+	unsigned char *data = urb->transfer_buffer;
+	char tty_flag;
+	unsigned int i;
+	u8 lsr;
+	u8 len;
+
+	if (urb->status) {
+		dev_err(&port->dev, "urb->status: %d\n", urb->status);
+		return;
+	}
+
+	if (!urb->actual_length) {
+		dev_err(&port->dev, "urb->actual_length == 0\n");
+		return;
+	}
+
+	len = data[0];
+	if (len != urb->actual_length) {
+		dev_err(&port->dev, "len(%d) != urb->actual_length(%d)\n", len,
+				urb->actual_length);
+		return;
+	}
+
+	/* bulk-in data: [LEN][Data.....][LSR] */
+	tty_flag = TTY_NORMAL;
+
+	lsr = data[len - 1];
+	if (lsr & UART_LSR_BRK_ERROR_BITS) {
+		if (lsr & UART_LSR_BI) {
+			tty_flag = TTY_BREAK;
+			port->icount.brk++;
+			usb_serial_handle_break(port);
+		} else if (lsr & UART_LSR_PE) {
+			tty_flag = TTY_PARITY;
+			port->icount.parity++;
+		} else if (lsr & UART_LSR_FE) {
+			tty_flag = TTY_FRAME;
+			port->icount.frame++;
+		}
+
+		if (lsr & UART_LSR_OE) {
+			port->icount.overrun++;
+			schedule_work(&priv->lsr_work);
+			tty_insert_flip_char(&port->port, 0, TTY_OVERRUN);
+		}
+	}
+
+	for (i = 1; i < urb->actual_length - 1; i++) {
+		if (port->port.console && port->sysrq) {
+			if (usb_serial_handle_sysrq_char(port, data[i]))
+				continue;
+		}
+
+		tty_insert_flip_char(&port->port, data[i], tty_flag);
+	}
+
+	tty_flip_buffer_push(&port->port);
+}
+
+static void f81232_read_urb_proxy(struct urb *urb)
+{
+	struct usb_serial_port *port = urb->context;
+	struct f81232_private *priv = usb_get_serial_port_data(port);
+
+	if (priv->process_read_urb)
+		priv->process_read_urb(urb);
+}
+
 static void f81232_break_ctl(struct tty_struct *tty, int break_state)
 {
 	struct usb_serial_port *port = tty->driver_data;
@@ -487,13 +583,28 @@ static void f81232_set_baudrate(struct tty_struct *tty,
 
 static int f81232_port_enable(struct usb_serial_port *port)
 {
+	struct f81232_private *port_priv = usb_get_serial_port_data(port);
 	u8 val;
 	int status;
 
-	/* fifo on, trigger8, clear TX/RX*/
-	val = UART_FCR_TRIGGER_8 | UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
-			UART_FCR_CLEAR_XMIT;
+	if (port_priv->device_type != F81232_F81232_TYPE) {
+		val = F81534A_TRIGGER_MULTPILE_4X | F81534A_FIFO_128BYTE;
+		status = f81232_set_mask_register(port, F81534A_MODE_CONF_REG,
+				F81534A_TRIGGER_MASK | F81534A_FIFO_128BYTE,
+				val);
+		if (status) {
+			dev_err(&port->dev, "failed to set MODE_CONF_REG: %d\n",
+					status);
+			return status;
+		}
+
+		val = UART_FCR_TRIGGER_14;
+	} else {
+		val = UART_FCR_TRIGGER_8;
+	}
 
+	/* fifo on, clear TX/RX */
+	val |= UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT;
 	status = f81232_set_register(port, FIFO_CONTROL_REGISTER, val);
 	if (status) {
 		dev_err(&port->dev, "%s failed to set FCR: %d\n",
@@ -631,6 +742,16 @@ static int f81232_tiocmset(struct tty_struct *tty,
 	return f81232_set_mctrl(port, set, clear);
 }
 
+static void f81232_detect_device_type(struct usb_serial_port *port)
+{
+	struct f81232_private *port_priv = usb_get_serial_port_data(port);
+
+	if (port->serial->dev->descriptor.idProduct == 0x0706)
+		port_priv->device_type = F81232_F81232_TYPE;
+	else
+		port_priv->device_type = F81232_F81534A_TYPE;
+}
+
 static int f81232_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
 	int result;
@@ -731,6 +852,7 @@ static void f81232_lsr_worker(struct work_struct *work)
 static int f81232_port_probe(struct usb_serial_port *port)
 {
 	struct f81232_private *priv;
+	int status = 0;
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -741,11 +863,26 @@ static int f81232_port_probe(struct usb_serial_port *port)
 	INIT_WORK(&priv->lsr_work, f81232_lsr_worker);
 
 	usb_set_serial_port_data(port, priv);
+	f81232_detect_device_type(port);
 
 	port->port.drain_delay = 256;
 	priv->port = port;
 
-	return 0;
+	switch (priv->device_type) {
+	case F81232_F81534A_TYPE:
+		priv->process_read_urb = f81534a_process_read_urb;
+		break;
+
+	case F81232_F81232_TYPE:
+		priv->process_read_urb = f81232_process_read_urb;
+		break;
+
+	default:
+		status = -ENODEV;
+		break;
+	}
+
+	return status;
 }
 
 static int f81232_port_remove(struct usb_serial_port *port)
@@ -797,12 +934,10 @@ static int f81232_resume(struct usb_serial *serial)
 static struct usb_serial_driver f81232_device = {
 	.driver = {
 		.owner =	THIS_MODULE,
-		.name =		"f81232",
+		.name =		"f81232/f81534a",
 	},
 	.id_table =		id_table,
 	.num_ports =		1,
-	.bulk_in_size =		256,
-	.bulk_out_size =	256,
 	.open =			f81232_open,
 	.close =		f81232_close,
 	.dtr_rts =		f81232_dtr_rts,
@@ -813,7 +948,7 @@ static struct usb_serial_driver f81232_device = {
 	.tiocmget =		f81232_tiocmget,
 	.tiocmset =		f81232_tiocmset,
 	.tiocmiwait =		usb_serial_generic_tiocmiwait,
-	.process_read_urb =	f81232_process_read_urb,
+	.process_read_urb =	f81232_read_urb_proxy,
 	.read_int_callback =	f81232_read_int_callback,
 	.port_probe =		f81232_port_probe,
 	.port_remove =		f81232_port_remove,
@@ -828,7 +963,7 @@ static struct usb_serial_driver * const serial_drivers[] = {
 
 module_usb_serial_driver(serial_drivers, id_table);
 
-MODULE_DESCRIPTION("Fintek F81232 USB to serial adaptor driver");
+MODULE_DESCRIPTION("Fintek F81232/532A/534A/535/536 USB to serial driver");
 MODULE_AUTHOR("Greg Kroah-Hartman <gregkh@linuxfoundation.org>");
 MODULE_AUTHOR("Peter Hong <peter_hong@fintek.com.tw>");
 MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH V1 1/6] USB: serial: f81232: Add F81534A support

[Johan Hovold]

On Thu, Jun 06, 2019 at 10:54:11AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> The Fintek F81532A/534A/535/536 is USB-to-2/4/8/12 serial ports device.
> It's most same with F81232, the UART device is difference as follow:
> 	1. TX/RX bulk size is 128/512bytes
> 	2. RX bulk layout change:
> 		F81232: [LSR(1Byte)+DATA(1Byte)][LSR(1Byte)+DATA(1Byte)]...
> 		F81534A:[LEN][Data.....][LSR]
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> ---
>  drivers/usb/serial/f81232.c | 153 +++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 144 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index 43fa1f0716b7..84efcc66aa56 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
>   * Fintek F81232 USB to serial adaptor driver
> + * Fintek F81532A/534A/535/536 USB to 2/4/8/12 serial adaptor driver
>   *
>   * Copyright (C) 2012 Greg Kroah-Hartman (gregkh@linuxfoundation.org)
>   * Copyright (C) 2012 Linux Foundation
> @@ -22,7 +23,20 @@
>  #include <linux/serial_reg.h>
>  
>  static const struct usb_device_id id_table[] = {
> +	/* F81232 */
>  	{ USB_DEVICE(0x1934, 0x0706) },
> +
> +	/* F81532A/534A/535/536 */
> +	{ USB_DEVICE(0x2c42, 0x1602) },		/* In-Box 2 port UART device */
> +	{ USB_DEVICE(0x2c42, 0x1604) },		/* In-Box 4 port UART device */
> +	{ USB_DEVICE(0x2c42, 0x1605) },		/* In-Box 8 port UART device */
> +	{ USB_DEVICE(0x2c42, 0x1606) },		/* In-Box 12 port UART device */
> +	{ USB_DEVICE(0x2c42, 0x1608) },		/* Non-Flash type */
> +
> +	{ USB_DEVICE(0x2c42, 0x1632) },		/* 2 port UART device */
> +	{ USB_DEVICE(0x2c42, 0x1634) },		/* 4 port UART device */
> +	{ USB_DEVICE(0x2c42, 0x1635) },		/* 8 port UART device */
> +	{ USB_DEVICE(0x2c42, 0x1636) },		/* 12 port UART device */
>  	{ }					/* Terminating entry */
>  };
>  MODULE_DEVICE_TABLE(usb, id_table);
> @@ -61,15 +75,25 @@ MODULE_DEVICE_TABLE(usb, id_table);
>  #define F81232_CLK_14_77_MHZ		(BIT(1) | BIT(0))
>  #define F81232_CLK_MASK			GENMASK(1, 0)
>  
> +#define F81534A_MODE_CONF_REG		0x107
> +#define F81534A_TRIGGER_MASK		GENMASK(3, 2)
> +#define F81534A_TRIGGER_MULTPILE_4X	BIT(3)
> +#define F81534A_FIFO_128BYTE		(BIT(1) | BIT(0))
> +
> +#define F81232_F81232_TYPE		1
> +#define F81232_F81534A_TYPE		2
> +
>  struct f81232_private {
>  	struct mutex lock;
>  	u8 modem_control;
>  	u8 modem_status;
>  	u8 shadow_lcr;
> +	u8 device_type;
>  	speed_t baud_base;
>  	struct work_struct lsr_work;
>  	struct work_struct interrupt_work;
>  	struct usb_serial_port *port;
> +	void (*process_read_urb)(struct urb *urb);
>  };
>  
>  static u32 const baudrate_table[] = { 115200, 921600, 1152000, 1500000 };
> @@ -376,6 +400,78 @@ static void f81232_process_read_urb(struct urb *urb)
>  	tty_flip_buffer_push(&port->port);
>  }
>  
> +static void f81534a_process_read_urb(struct urb *urb)
> +{
> +	struct usb_serial_port *port = urb->context;
> +	struct f81232_private *priv = usb_get_serial_port_data(port);
> +	unsigned char *data = urb->transfer_buffer;
> +	char tty_flag;
> +	unsigned int i;
> +	u8 lsr;
> +	u8 len;
> +
> +	if (urb->status) {
> +		dev_err(&port->dev, "urb->status: %d\n", urb->status);

Please use proper error messages in English (not C) here and throughout.

But this one isn't needed since it should have been checked by the
completion handler.

> +		return;
> +	}
> +
> +	if (!urb->actual_length) {
> +		dev_err(&port->dev, "urb->actual_length == 0\n");
> +		return;
> +	}
> +
> +	len = data[0];
> +	if (len != urb->actual_length) {
> +		dev_err(&port->dev, "len(%d) != urb->actual_length(%d)\n", len,
> +				urb->actual_length);
> +		return;
> +	}
> +
> +	/* bulk-in data: [LEN][Data.....][LSR] */
> +	tty_flag = TTY_NORMAL;
> +
> +	lsr = data[len - 1];

What if len == 1?

> +	if (lsr & UART_LSR_BRK_ERROR_BITS) {
> +		if (lsr & UART_LSR_BI) {
> +			tty_flag = TTY_BREAK;
> +			port->icount.brk++;
> +			usb_serial_handle_break(port);
> +		} else if (lsr & UART_LSR_PE) {
> +			tty_flag = TTY_PARITY;
> +			port->icount.parity++;
> +		} else if (lsr & UART_LSR_FE) {
> +			tty_flag = TTY_FRAME;
> +			port->icount.frame++;
> +		}
> +
> +		if (lsr & UART_LSR_OE) {
> +			port->icount.overrun++;
> +			schedule_work(&priv->lsr_work);
> +			tty_insert_flip_char(&port->port, 0, TTY_OVERRUN);
> +		}
> +	}

Isn't this something mostly shared with f81232r? Refactor?

> +
> +	for (i = 1; i < urb->actual_length - 1; i++) {
> +		if (port->port.console && port->sysrq) {
> +			if (usb_serial_handle_sysrq_char(port, data[i]))
> +				continue;
> +		}
> +
> +		tty_insert_flip_char(&port->port, data[i], tty_flag);

Use tty_insert_flip_string_fixed_char() when not a console.

> +	}
> +
> +	tty_flip_buffer_push(&port->port);
> +}
> +
> +static void f81232_read_urb_proxy(struct urb *urb)
> +{
> +	struct usb_serial_port *port = urb->context;
> +	struct f81232_private *priv = usb_get_serial_port_data(port);
> +
> +	if (priv->process_read_urb)
> +		priv->process_read_urb(urb);
> +}

No, please add a proper usb-serial subdriver (to this file) rather
than reimplement this type abstraction yourself.

> +
>  static void f81232_break_ctl(struct tty_struct *tty, int break_state)
>  {
>  	struct usb_serial_port *port = tty->driver_data;
> @@ -487,13 +583,28 @@ static void f81232_set_baudrate(struct tty_struct *tty,
>  
>  static int f81232_port_enable(struct usb_serial_port *port)
>  {
> +	struct f81232_private *port_priv = usb_get_serial_port_data(port);
>  	u8 val;
>  	int status;
>  
> -	/* fifo on, trigger8, clear TX/RX*/
> -	val = UART_FCR_TRIGGER_8 | UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
> -			UART_FCR_CLEAR_XMIT;
> +	if (port_priv->device_type != F81232_F81232_TYPE) {
> +		val = F81534A_TRIGGER_MULTPILE_4X | F81534A_FIFO_128BYTE;
> +		status = f81232_set_mask_register(port, F81534A_MODE_CONF_REG,
> +				F81534A_TRIGGER_MASK | F81534A_FIFO_128BYTE,
> +				val);
> +		if (status) {
> +			dev_err(&port->dev, "failed to set MODE_CONF_REG: %d\n",
> +					status);
> +			return status;
> +		}
> +
> +		val = UART_FCR_TRIGGER_14;
> +	} else {
> +		val = UART_FCR_TRIGGER_8;
> +	}
>  
> +	/* fifo on, clear TX/RX */
> +	val |= UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT;
>  	status = f81232_set_register(port, FIFO_CONTROL_REGISTER, val);
>  	if (status) {
>  		dev_err(&port->dev, "%s failed to set FCR: %d\n",
> @@ -631,6 +742,16 @@ static int f81232_tiocmset(struct tty_struct *tty,
>  	return f81232_set_mctrl(port, set, clear);
>  }
>  
> +static void f81232_detect_device_type(struct usb_serial_port *port)
> +{
> +	struct f81232_private *port_priv = usb_get_serial_port_data(port);
> +
> +	if (port->serial->dev->descriptor.idProduct == 0x0706)

Missing le16_to_cpu()

Customers cannot set their own device ids?

Not needed with subdriver anyway.

> +		port_priv->device_type = F81232_F81232_TYPE;
> +	else
> +		port_priv->device_type = F81232_F81534A_TYPE;
> +}
> +
>  static int f81232_open(struct tty_struct *tty, struct usb_serial_port *port)
>  {
>  	int result;
> @@ -731,6 +852,7 @@ static void f81232_lsr_worker(struct work_struct *work)
>  static int f81232_port_probe(struct usb_serial_port *port)
>  {
>  	struct f81232_private *priv;
> +	int status = 0;
>  
>  	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
> @@ -741,11 +863,26 @@ static int f81232_port_probe(struct usb_serial_port *port)
>  	INIT_WORK(&priv->lsr_work, f81232_lsr_worker);
>  
>  	usb_set_serial_port_data(port, priv);
> +	f81232_detect_device_type(port);
>  
>  	port->port.drain_delay = 256;
>  	priv->port = port;
>  
> -	return 0;
> +	switch (priv->device_type) {
> +	case F81232_F81534A_TYPE:
> +		priv->process_read_urb = f81534a_process_read_urb;
> +		break;
> +
> +	case F81232_F81232_TYPE:
> +		priv->process_read_urb = f81232_process_read_urb;
> +		break;
> +
> +	default:
> +		status = -ENODEV;
> +		break;
> +	}
> +
> +	return status;
>  }
>  
>  static int f81232_port_remove(struct usb_serial_port *port)
> @@ -797,12 +934,10 @@ static int f81232_resume(struct usb_serial *serial)
>  static struct usb_serial_driver f81232_device = {
>  	.driver = {
>  		.owner =	THIS_MODULE,
> -		.name =		"f81232",
> +		.name =		"f81232/f81534a",
>  	},
>  	.id_table =		id_table,
>  	.num_ports =		1,
> -	.bulk_in_size =		256,
> -	.bulk_out_size =	256,

Why change this?

>  	.open =			f81232_open,
>  	.close =		f81232_close,
>  	.dtr_rts =		f81232_dtr_rts,
> @@ -813,7 +948,7 @@ static struct usb_serial_driver f81232_device = {
>  	.tiocmget =		f81232_tiocmget,
>  	.tiocmset =		f81232_tiocmset,
>  	.tiocmiwait =		usb_serial_generic_tiocmiwait,
> -	.process_read_urb =	f81232_process_read_urb,
> +	.process_read_urb =	f81232_read_urb_proxy,
>  	.read_int_callback =	f81232_read_int_callback,
>  	.port_probe =		f81232_port_probe,
>  	.port_remove =		f81232_port_remove,
> @@ -828,7 +963,7 @@ static struct usb_serial_driver * const serial_drivers[] = {
>  
>  module_usb_serial_driver(serial_drivers, id_table);
>  
> -MODULE_DESCRIPTION("Fintek F81232 USB to serial adaptor driver");
> +MODULE_DESCRIPTION("Fintek F81232/532A/534A/535/536 USB to serial driver");
>  MODULE_AUTHOR("Greg Kroah-Hartman <gregkh@linuxfoundation.org>");
>  MODULE_AUTHOR("Peter Hong <peter_hong@fintek.com.tw>");
>  MODULE_LICENSE("GPL v2");

Johan

[PATCH V1 2/6] USB: serial: f81232: Force F81534A with RS232 mode

[Ji-Ze Hong (Peter Hong)]

Force F81534A series UARTs with RS232 mode in port_probe().

Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
---
 drivers/usb/serial/f81232.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 84efcc66aa56..75dfc0b9ef30 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -83,12 +83,22 @@ MODULE_DEVICE_TABLE(usb, id_table);
 #define F81232_F81232_TYPE		1
 #define F81232_F81534A_TYPE		2
 
+/* Serial port self GPIO control, 2bytes [control&output data][input data] */
+#define F81534A_GPIO_REG		0x10e
+#define F81534A_GPIO_MODE2_DIR		BIT(6) /* 1: input, 0: output */
+#define F81534A_GPIO_MODE1_DIR		BIT(5)
+#define F81534A_GPIO_MODE0_DIR		BIT(4)
+#define F81534A_GPIO_MODE2_OUTPUT	BIT(2)
+#define F81534A_GPIO_MODE1_OUTPUT	BIT(1)
+#define F81534A_GPIO_MODE0_OUTPUT	BIT(0)
+
 struct f81232_private {
 	struct mutex lock;
 	u8 modem_control;
 	u8 modem_status;
 	u8 shadow_lcr;
 	u8 device_type;
+	u8 gpio_mode;
 	speed_t baud_base;
 	struct work_struct lsr_work;
 	struct work_struct interrupt_work;
@@ -871,6 +881,11 @@ static int f81232_port_probe(struct usb_serial_port *port)
 	switch (priv->device_type) {
 	case F81232_F81534A_TYPE:
 		priv->process_read_urb = f81534a_process_read_urb;
+		priv->gpio_mode = F81534A_GPIO_MODE2_DIR;
+
+		/* tri-state with pull-high, default RS232 Mode */
+		status = f81232_set_register(port, F81534A_GPIO_REG,
+					priv->gpio_mode);
 		break;
 
 	case F81232_F81232_TYPE:
-- 
2.7.4

Re: [PATCH V1 2/6] USB: serial: f81232: Force F81534A with RS232 mode

[Johan Hovold]

On Thu, Jun 06, 2019 at 10:54:12AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Force F81534A series UARTs with RS232 mode in port_probe().

Please expand on why you need this here.

> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> ---
>  drivers/usb/serial/f81232.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index 84efcc66aa56..75dfc0b9ef30 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -83,12 +83,22 @@ MODULE_DEVICE_TABLE(usb, id_table);
>  #define F81232_F81232_TYPE		1
>  #define F81232_F81534A_TYPE		2
>  
> +/* Serial port self GPIO control, 2bytes [control&output data][input data] */
> +#define F81534A_GPIO_REG		0x10e
> +#define F81534A_GPIO_MODE2_DIR		BIT(6) /* 1: input, 0: output */
> +#define F81534A_GPIO_MODE1_DIR		BIT(5)
> +#define F81534A_GPIO_MODE0_DIR		BIT(4)
> +#define F81534A_GPIO_MODE2_OUTPUT	BIT(2)
> +#define F81534A_GPIO_MODE1_OUTPUT	BIT(1)
> +#define F81534A_GPIO_MODE0_OUTPUT	BIT(0)
> +
>  struct f81232_private {
>  	struct mutex lock;
>  	u8 modem_control;
>  	u8 modem_status;
>  	u8 shadow_lcr;
>  	u8 device_type;
> +	u8 gpio_mode;

Why store the mode? Are you going to use it later?

>  	speed_t baud_base;
>  	struct work_struct lsr_work;
>  	struct work_struct interrupt_work;
> @@ -871,6 +881,11 @@ static int f81232_port_probe(struct usb_serial_port *port)
>  	switch (priv->device_type) {
>  	case F81232_F81534A_TYPE:
>  		priv->process_read_urb = f81534a_process_read_urb;
> +		priv->gpio_mode = F81534A_GPIO_MODE2_DIR;
> +
> +		/* tri-state with pull-high, default RS232 Mode */
> +		status = f81232_set_register(port, F81534A_GPIO_REG,
> +					priv->gpio_mode);
>  		break;
>  
>  	case F81232_F81232_TYPE:

Johan

[PATCH V1 3/6] USB: serial: f81232: Add generator for F81534A

[Ji-Ze Hong (Peter Hong)]

The Fintek F81534A series is contains 1 HUB / 1 GPIO device / n UARTs,
but the UART is default disable and need enabled by GPIO device(2c42/16F8).
When F81534A plug to host, we can only see 1 HUB & 1 GPIO device, add
GPIO device USB interface to device_list and trigger generate worker,
f81534a_generate_worker to run f81534a_ctrl_generate_ports().

The operation in f81534a_ctrl_generate_ports() as following:
	1: Write 0x8fff to F81534A_CMD_ENABLE_PORT register for enable all
	   UART device.

	2: Read port existence & current status from F81534A_CMD_PORT_STATUS
	   register. the higher 16bit will indicate the UART existence. If the
	   UART is existence, we'll check it GPIO mode as long as not default
	   value (default is all input mode).

	3: 1 GPIO device will check with max 15s and check next GPIO device when
	   timeout. (F81534A_CTRL_RETRY * F81534A_CTRL_TIMER)

Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
---
 drivers/usb/serial/f81232.c | 356 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 355 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 75dfc0b9ef30..e9470fb0d691 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -41,6 +41,12 @@ static const struct usb_device_id id_table[] = {
 };
 MODULE_DEVICE_TABLE(usb, id_table);
 
+static const struct usb_device_id f81534a_ctrl_id_table[] = {
+	{ USB_DEVICE(0x2c42, 0x16f8) },		/* Global control device */
+	{ }					/* Terminating entry */
+};
+MODULE_DEVICE_TABLE(usb, f81534a_ctrl_id_table);
+
 /* Maximum baudrate for F81232 */
 #define F81232_MAX_BAUDRATE		1500000
 #define F81232_DEF_BAUDRATE		9600
@@ -49,6 +55,10 @@ MODULE_DEVICE_TABLE(usb, id_table);
 #define F81232_REGISTER_REQUEST		0xa0
 #define F81232_GET_REGISTER		0xc0
 #define F81232_SET_REGISTER		0x40
+#define F81534A_REGISTER_REQUEST	F81232_REGISTER_REQUEST
+#define F81534A_GET_REGISTER		F81232_GET_REGISTER
+#define F81534A_SET_REGISTER		F81232_SET_REGISTER
+#define F81534A_ACCESS_REG_RETRY	2
 
 #define SERIAL_BASE_ADDRESS		0x0120
 #define RECEIVE_BUFFER_REGISTER		(0x00 + SERIAL_BASE_ADDRESS)
@@ -83,6 +93,10 @@ MODULE_DEVICE_TABLE(usb, id_table);
 #define F81232_F81232_TYPE		1
 #define F81232_F81534A_TYPE		2
 
+#define F81534A_MAX_PORT		12
+#define F81534A_CTRL_TIMER		1000
+#define F81534A_CTRL_RETRY		15
+
 /* Serial port self GPIO control, 2bytes [control&output data][input data] */
 #define F81534A_GPIO_REG		0x10e
 #define F81534A_GPIO_MODE2_DIR		BIT(6) /* 1: input, 0: output */
@@ -92,6 +106,16 @@ MODULE_DEVICE_TABLE(usb, id_table);
 #define F81534A_GPIO_MODE1_OUTPUT	BIT(1)
 #define F81534A_GPIO_MODE0_OUTPUT	BIT(0)
 
+#define F81534A_CMD_ENABLE_PORT		0x116
+#define F81534A_CMD_PORT_STATUS		0x117
+
+/*
+ * Control device global GPIO control,
+ * 2bytes [control&output data][input data]
+ */
+#define F81534A_CTRL_GPIO_REG		0x1601
+#define F81534A_CTRL_GPIO_MAX_PIN	3
+
 struct f81232_private {
 	struct mutex lock;
 	u8 modem_control;
@@ -106,10 +130,27 @@ struct f81232_private {
 	void (*process_read_urb)(struct urb *urb);
 };
 
+struct f81534a_ctrl_private {
+	struct usb_interface *intf;
+	struct mutex lock;
+	int device_idx;
+};
+
+struct f81534a_device {
+	struct list_head list;
+	struct usb_interface *intf;
+	int check_index;
+	int check_retry;
+};
+
 static u32 const baudrate_table[] = { 115200, 921600, 1152000, 1500000 };
 static u8 const clock_table[] = { F81232_CLK_1_846_MHZ, F81232_CLK_14_77_MHZ,
 				F81232_CLK_18_46_MHZ, F81232_CLK_24_MHZ };
 
+struct delayed_work f81534a_generate_worker;
+static DEFINE_MUTEX(device_mutex);
+static LIST_HEAD(device_list);
+
 static int calc_baud_divisor(speed_t baudrate, speed_t clockrate)
 {
 	if (!baudrate)
@@ -859,6 +900,281 @@ static void f81232_lsr_worker(struct work_struct *work)
 		dev_warn(&port->dev, "read LSR failed: %d\n", status);
 }
 
+static int f81534a_ctrl_get_register(struct usb_device *dev, u16 reg, u16 size,
+					void *val)
+{
+	int retry = F81534A_ACCESS_REG_RETRY;
+	int status;
+	u8 *tmp;
+
+	tmp = kmalloc(size, GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	while (retry--) {
+		status = usb_control_msg(dev,
+					usb_rcvctrlpipe(dev, 0),
+					F81534A_REGISTER_REQUEST,
+					F81534A_GET_REGISTER,
+					reg,
+					0,
+					tmp,
+					size,
+					USB_CTRL_GET_TIMEOUT);
+		if (status != size) {
+			status = usb_translate_errors(status);
+			if (status == -EIO)
+				continue;
+
+			status = -EIO;
+		} else {
+			status = 0;
+			memcpy(val, tmp, size);
+		}
+
+		break;
+	}
+
+	if (status) {
+		dev_err(&dev->dev, "get reg: %x, failed status: %d\n", reg,
+				status);
+	}
+
+	kfree(tmp);
+	return status;
+}
+
+static int f81534a_ctrl_set_register(struct usb_device *dev, u16 reg, u16 size,
+					void *val)
+{
+	int retry = F81534A_ACCESS_REG_RETRY;
+	int status;
+	u8 *tmp;
+
+	tmp = kmalloc(size, GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	memcpy(tmp, val, size);
+
+	while (retry--) {
+		status = usb_control_msg(dev,
+					usb_sndctrlpipe(dev, 0),
+					F81534A_REGISTER_REQUEST,
+					F81534A_SET_REGISTER,
+					reg,
+					0,
+					tmp,
+					size,
+					USB_CTRL_SET_TIMEOUT);
+		if (status != size) {
+			status = usb_translate_errors(status);
+			if (status == -EIO)
+				continue;
+
+			status = -EIO;
+		} else {
+			status = 0;
+		}
+
+		break;
+	}
+
+	if (status) {
+		dev_err(&dev->dev, "set ctrl reg: %x, failed status: %d\n", reg,
+				status);
+	}
+
+	kfree(tmp);
+	return status;
+}
+
+static int f81534a_ctrl_generate_ports(struct usb_interface *intf,
+					struct f81534a_device *device)
+{
+	struct usb_device *dev = interface_to_usbdev(intf);
+	uint32_t port_status;
+	u8 enable[2];
+	u8 tmp;
+	u8 mask;
+	int status;
+
+	/* enable all ports */
+	mask = F81534A_GPIO_MODE2_DIR | F81534A_GPIO_MODE1_DIR |
+			F81534A_GPIO_MODE0_DIR;
+	enable[0] = 0xff;
+	enable[1] = 0x8f;
+
+	status = f81534a_ctrl_set_register(dev, F81534A_CMD_ENABLE_PORT,
+			sizeof(enable), enable);
+	if (status) {
+		dev_warn(&dev->dev, "set CMD_ENABLE_PORT failed: %d\n", status);
+		return status;
+	}
+
+	/* get port state */
+	status = f81534a_ctrl_get_register(dev,
+			F81534A_CMD_PORT_STATUS, sizeof(port_status),
+			&port_status);
+	if (status) {
+		dev_warn(&dev->dev, "get CMD_PORT_STATUS failed: %d\n", status);
+		return status;
+	}
+
+	port_status >>= 16;
+
+	for (; device->check_index < F81534A_MAX_PORT; ++device->check_index) {
+		/* check port is exist, skip when not exist */
+		if (!(port_status & BIT(device->check_index)))
+			continue;
+
+		/*
+		 * All gpio for a port is default to input mode. It'll change
+		 * to RS232 mode after f81232_port_probe()/f81534a_port_init()
+		 * (2 output 0 & 1 input with pull high).
+		 */
+		status = f81534a_ctrl_get_register(dev,
+					F81534A_CTRL_GPIO_REG +
+					device->check_index, sizeof(tmp), &tmp);
+		if (status) {
+			dev_warn(&dev->dev, "get CTRL_GPIO_REG failed: %d\n",
+					status);
+			return status;
+		}
+
+		/* Check port had inited by f81232_port_probe() */
+		if ((tmp & mask) == mask)
+			break;
+	}
+
+	if (device->check_index < F81534A_MAX_PORT)
+		return -EAGAIN;
+
+	return 0;
+}
+
+static void f81534a_ctrl_generate_worker(struct work_struct *work)
+{
+	struct f81534a_device *device;
+	int status;
+
+	mutex_lock(&device_mutex);
+	list_for_each_entry(device, &device_list, list) {
+		if (device->check_index >= F81534A_MAX_PORT)
+			continue;
+
+		if (device->check_retry >= F81534A_CTRL_RETRY)
+			continue;
+
+		device->check_retry++;
+
+		status = f81534a_ctrl_generate_ports(device->intf, device);
+		if (status == -EAGAIN) {
+			dev_dbg(&device->intf->dev, "delayed generating: %d\n",
+					device->check_retry);
+
+			schedule_delayed_work(&f81534a_generate_worker,
+					msecs_to_jiffies(F81534A_CTRL_TIMER));
+			break;
+		} else if (!status) {
+			/* make this device generated */
+			device->check_index = F81534A_MAX_PORT;
+
+			dev_dbg(&device->intf->dev, "generated complete\n");
+		} else {
+			/* skip this device to generate */
+			device->check_index = F81534A_MAX_PORT;
+
+			dev_err(&device->intf->dev,
+					"error: %d, do next device generate\n",
+					status);
+		}
+	}
+
+	mutex_unlock(&device_mutex);
+}
+
+static int f81534a_ctrl_probe(struct usb_interface *intf,
+				const struct usb_device_id *id)
+{
+	struct usb_device *dev = interface_to_usbdev(intf);
+	struct f81534a_ctrl_private *priv;
+	struct f81534a_device *device;
+
+	priv = devm_kzalloc(&intf->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	device = devm_kzalloc(&intf->dev, sizeof(*device), GFP_KERNEL);
+	if (!device)
+		return -ENOMEM;
+
+	mutex_init(&priv->lock);
+	usb_set_intfdata(intf, priv);
+
+	INIT_LIST_HEAD(&device->list);
+	device->intf = intf;
+
+	mutex_lock(&device_mutex);
+	list_add_tail(&device->list, &device_list);
+	mutex_unlock(&device_mutex);
+
+	dev = usb_get_dev(dev);
+	schedule_delayed_work(&f81534a_generate_worker,
+				msecs_to_jiffies(F81534A_CTRL_TIMER));
+
+	return 0;
+}
+
+static void f81534a_ctrl_disconnect(struct usb_interface *intf)
+{
+	struct f81534a_ctrl_private *priv;
+	struct f81534a_device *device;
+	struct usb_device *dev;
+
+	mutex_lock(&device_mutex);
+
+	list_for_each_entry(device, &device_list, list) {
+		if (device->intf == intf) {
+			list_del(&device->list);
+
+			priv = usb_get_intfdata(intf);
+			dev = interface_to_usbdev(intf);
+			usb_put_dev(dev);
+			break;
+		}
+	}
+
+	mutex_unlock(&device_mutex);
+}
+
+static int f81534a_ctrl_suspend(struct usb_interface *intf,
+					pm_message_t message)
+{
+	struct f81534a_device *device;
+
+	flush_delayed_work(&f81534a_generate_worker);
+
+	mutex_lock(&device_mutex);
+
+	list_for_each_entry(device, &device_list, list) {
+		device->check_index = 0;
+		device->check_retry = 0;
+	}
+
+	mutex_unlock(&device_mutex);
+
+	return 0;
+}
+
+static int f81534a_ctrl_resume(struct usb_interface *intf)
+{
+	schedule_delayed_work(&f81534a_generate_worker,
+				msecs_to_jiffies(F81534A_CTRL_TIMER));
+
+	return 0;
+}
+
 static int f81232_port_probe(struct usb_serial_port *port)
 {
 	struct f81232_private *priv;
@@ -976,7 +1292,45 @@ static struct usb_serial_driver * const serial_drivers[] = {
 	NULL,
 };
 
-module_usb_serial_driver(serial_drivers, id_table);
+static struct usb_driver f81534a_ctrl_driver = {
+	.name =		"f81534a_ctrl",
+	.id_table =	f81534a_ctrl_id_table,
+	.probe =	f81534a_ctrl_probe,
+	.disconnect =	f81534a_ctrl_disconnect,
+	.suspend =	f81534a_ctrl_suspend,
+	.resume =	f81534a_ctrl_resume,
+};
+
+static int __init f81232_init(void)
+{
+	int status;
+
+	INIT_DELAYED_WORK(&f81534a_generate_worker,
+			f81534a_ctrl_generate_worker);
+
+	status = usb_register_driver(&f81534a_ctrl_driver, THIS_MODULE,
+			KBUILD_MODNAME);
+	if (status)
+		return status;
+
+	status = usb_serial_register_drivers(serial_drivers, KBUILD_MODNAME,
+			id_table);
+	if (status) {
+		usb_deregister(&f81534a_ctrl_driver);
+		return status;
+	}
+
+	return 0;
+}
+
+static void __exit f81232_exit(void)
+{
+	usb_serial_deregister_drivers(serial_drivers);
+	usb_deregister(&f81534a_ctrl_driver);
+}
+
+module_init(f81232_init);
+module_exit(f81232_exit);
 
 MODULE_DESCRIPTION("Fintek F81232/532A/534A/535/536 USB to serial driver");
 MODULE_AUTHOR("Greg Kroah-Hartman <gregkh@linuxfoundation.org>");
-- 
2.7.4

Re: [PATCH V1 3/6] USB: serial: f81232: Add generator for F81534A

[Johan Hovold]

On Thu, Jun 06, 2019 at 10:54:13AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> The Fintek F81534A series is contains 1 HUB / 1 GPIO device / n UARTs,
> but the UART is default disable and need enabled by GPIO device(2c42/16F8).
> When F81534A plug to host, we can only see 1 HUB & 1 GPIO device, add
> GPIO device USB interface to device_list and trigger generate worker,
> f81534a_generate_worker to run f81534a_ctrl_generate_ports().
> 
> The operation in f81534a_ctrl_generate_ports() as following:
> 	1: Write 0x8fff to F81534A_CMD_ENABLE_PORT register for enable all
> 	   UART device.
> 
> 	2: Read port existence & current status from F81534A_CMD_PORT_STATUS
> 	   register. the higher 16bit will indicate the UART existence. If the
> 	   UART is existence, we'll check it GPIO mode as long as not default
> 	   value (default is all input mode).
> 
> 	3: 1 GPIO device will check with max 15s and check next GPIO device when
> 	   timeout. (F81534A_CTRL_RETRY * F81534A_CTRL_TIMER)
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>

This is all looks crazy... Please better describe how the device works,
and you want to implement support.

> ---
>  drivers/usb/serial/f81232.c | 356 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 355 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index 75dfc0b9ef30..e9470fb0d691 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -41,6 +41,12 @@ static const struct usb_device_id id_table[] = {
>  };
>  MODULE_DEVICE_TABLE(usb, id_table);
>  
> +static const struct usb_device_id f81534a_ctrl_id_table[] = {
> +	{ USB_DEVICE(0x2c42, 0x16f8) },		/* Global control device */
> +	{ }					/* Terminating entry */
> +};
> +MODULE_DEVICE_TABLE(usb, f81534a_ctrl_id_table);

You can only have one MODULE_DEVICE_TABLE()...

> +
>  /* Maximum baudrate for F81232 */
>  #define F81232_MAX_BAUDRATE		1500000
>  #define F81232_DEF_BAUDRATE		9600
> @@ -49,6 +55,10 @@ MODULE_DEVICE_TABLE(usb, id_table);
>  #define F81232_REGISTER_REQUEST		0xa0
>  #define F81232_GET_REGISTER		0xc0
>  #define F81232_SET_REGISTER		0x40
> +#define F81534A_REGISTER_REQUEST	F81232_REGISTER_REQUEST
> +#define F81534A_GET_REGISTER		F81232_GET_REGISTER
> +#define F81534A_SET_REGISTER		F81232_SET_REGISTER
> +#define F81534A_ACCESS_REG_RETRY	2
>  
>  #define SERIAL_BASE_ADDRESS		0x0120
>  #define RECEIVE_BUFFER_REGISTER		(0x00 + SERIAL_BASE_ADDRESS)
> @@ -83,6 +93,10 @@ MODULE_DEVICE_TABLE(usb, id_table);
>  #define F81232_F81232_TYPE		1
>  #define F81232_F81534A_TYPE		2
>  
> +#define F81534A_MAX_PORT		12
> +#define F81534A_CTRL_TIMER		1000
> +#define F81534A_CTRL_RETRY		15
> +
>  /* Serial port self GPIO control, 2bytes [control&output data][input data] */
>  #define F81534A_GPIO_REG		0x10e
>  #define F81534A_GPIO_MODE2_DIR		BIT(6) /* 1: input, 0: output */
> @@ -92,6 +106,16 @@ MODULE_DEVICE_TABLE(usb, id_table);
>  #define F81534A_GPIO_MODE1_OUTPUT	BIT(1)
>  #define F81534A_GPIO_MODE0_OUTPUT	BIT(0)
>  
> +#define F81534A_CMD_ENABLE_PORT		0x116
> +#define F81534A_CMD_PORT_STATUS		0x117
> +
> +/*
> + * Control device global GPIO control,
> + * 2bytes [control&output data][input data]
> + */
> +#define F81534A_CTRL_GPIO_REG		0x1601
> +#define F81534A_CTRL_GPIO_MAX_PIN	3
> +
>  struct f81232_private {
>  	struct mutex lock;
>  	u8 modem_control;
> @@ -106,10 +130,27 @@ struct f81232_private {
>  	void (*process_read_urb)(struct urb *urb);
>  };
>  
> +struct f81534a_ctrl_private {
> +	struct usb_interface *intf;
> +	struct mutex lock;
> +	int device_idx;
> +};
> +
> +struct f81534a_device {
> +	struct list_head list;
> +	struct usb_interface *intf;
> +	int check_index;
> +	int check_retry;
> +};
> +
>  static u32 const baudrate_table[] = { 115200, 921600, 1152000, 1500000 };
>  static u8 const clock_table[] = { F81232_CLK_1_846_MHZ, F81232_CLK_14_77_MHZ,
>  				F81232_CLK_18_46_MHZ, F81232_CLK_24_MHZ };
>  
> +struct delayed_work f81534a_generate_worker;
> +static DEFINE_MUTEX(device_mutex);
> +static LIST_HEAD(device_list);
> +
>  static int calc_baud_divisor(speed_t baudrate, speed_t clockrate)
>  {
>  	if (!baudrate)
> @@ -859,6 +900,281 @@ static void f81232_lsr_worker(struct work_struct *work)
>  		dev_warn(&port->dev, "read LSR failed: %d\n", status);
>  }
>  
> +static int f81534a_ctrl_get_register(struct usb_device *dev, u16 reg, u16 size,
> +					void *val)
> +{
> +	int retry = F81534A_ACCESS_REG_RETRY;
> +	int status;
> +	u8 *tmp;
> +
> +	tmp = kmalloc(size, GFP_KERNEL);
> +	if (!tmp)
> +		return -ENOMEM;
> +
> +	while (retry--) {
> +		status = usb_control_msg(dev,
> +					usb_rcvctrlpipe(dev, 0),
> +					F81534A_REGISTER_REQUEST,
> +					F81534A_GET_REGISTER,
> +					reg,
> +					0,
> +					tmp,
> +					size,
> +					USB_CTRL_GET_TIMEOUT);
> +		if (status != size) {
> +			status = usb_translate_errors(status);
> +			if (status == -EIO)
> +				continue;
> +
> +			status = -EIO;
> +		} else {
> +			status = 0;
> +			memcpy(val, tmp, size);
> +		}
> +
> +		break;
> +	}
> +
> +	if (status) {
> +		dev_err(&dev->dev, "get reg: %x, failed status: %d\n", reg,
> +				status);
> +	}
> +
> +	kfree(tmp);
> +	return status;
> +}
> +
> +static int f81534a_ctrl_set_register(struct usb_device *dev, u16 reg, u16 size,
> +					void *val)
> +{
> +	int retry = F81534A_ACCESS_REG_RETRY;
> +	int status;
> +	u8 *tmp;
> +
> +	tmp = kmalloc(size, GFP_KERNEL);
> +	if (!tmp)
> +		return -ENOMEM;
> +
> +	memcpy(tmp, val, size);
> +
> +	while (retry--) {
> +		status = usb_control_msg(dev,
> +					usb_sndctrlpipe(dev, 0),
> +					F81534A_REGISTER_REQUEST,
> +					F81534A_SET_REGISTER,
> +					reg,
> +					0,
> +					tmp,
> +					size,
> +					USB_CTRL_SET_TIMEOUT);
> +		if (status != size) {
> +			status = usb_translate_errors(status);
> +			if (status == -EIO)
> +				continue;
> +
> +			status = -EIO;
> +		} else {
> +			status = 0;
> +		}
> +
> +		break;
> +	}
> +
> +	if (status) {
> +		dev_err(&dev->dev, "set ctrl reg: %x, failed status: %d\n", reg,
> +				status);
> +	}
> +
> +	kfree(tmp);
> +	return status;
> +}
> +
> +static int f81534a_ctrl_generate_ports(struct usb_interface *intf,
> +					struct f81534a_device *device)
> +{
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	uint32_t port_status;
> +	u8 enable[2];
> +	u8 tmp;
> +	u8 mask;
> +	int status;
> +
> +	/* enable all ports */
> +	mask = F81534A_GPIO_MODE2_DIR | F81534A_GPIO_MODE1_DIR |
> +			F81534A_GPIO_MODE0_DIR;
> +	enable[0] = 0xff;
> +	enable[1] = 0x8f;
> +
> +	status = f81534a_ctrl_set_register(dev, F81534A_CMD_ENABLE_PORT,
> +			sizeof(enable), enable);
> +	if (status) {
> +		dev_warn(&dev->dev, "set CMD_ENABLE_PORT failed: %d\n", status);
> +		return status;
> +	}
> +
> +	/* get port state */
> +	status = f81534a_ctrl_get_register(dev,
> +			F81534A_CMD_PORT_STATUS, sizeof(port_status),
> +			&port_status);
> +	if (status) {
> +		dev_warn(&dev->dev, "get CMD_PORT_STATUS failed: %d\n", status);
> +		return status;
> +	}
> +
> +	port_status >>= 16;
> +
> +	for (; device->check_index < F81534A_MAX_PORT; ++device->check_index) {
> +		/* check port is exist, skip when not exist */
> +		if (!(port_status & BIT(device->check_index)))
> +			continue;
> +
> +		/*
> +		 * All gpio for a port is default to input mode. It'll change
> +		 * to RS232 mode after f81232_port_probe()/f81534a_port_init()
> +		 * (2 output 0 & 1 input with pull high).
> +		 */
> +		status = f81534a_ctrl_get_register(dev,
> +					F81534A_CTRL_GPIO_REG +
> +					device->check_index, sizeof(tmp), &tmp);
> +		if (status) {
> +			dev_warn(&dev->dev, "get CTRL_GPIO_REG failed: %d\n",
> +					status);
> +			return status;
> +		}
> +
> +		/* Check port had inited by f81232_port_probe() */
> +		if ((tmp & mask) == mask)
> +			break;
> +	}
> +
> +	if (device->check_index < F81534A_MAX_PORT)
> +		return -EAGAIN;
> +
> +	return 0;
> +}
> +
> +static void f81534a_ctrl_generate_worker(struct work_struct *work)
> +{
> +	struct f81534a_device *device;
> +	int status;
> +
> +	mutex_lock(&device_mutex);
> +	list_for_each_entry(device, &device_list, list) {
> +		if (device->check_index >= F81534A_MAX_PORT)
> +			continue;
> +
> +		if (device->check_retry >= F81534A_CTRL_RETRY)
> +			continue;
> +
> +		device->check_retry++;
> +
> +		status = f81534a_ctrl_generate_ports(device->intf, device);
> +		if (status == -EAGAIN) {
> +			dev_dbg(&device->intf->dev, "delayed generating: %d\n",
> +					device->check_retry);
> +
> +			schedule_delayed_work(&f81534a_generate_worker,
> +					msecs_to_jiffies(F81534A_CTRL_TIMER));
> +			break;
> +		} else if (!status) {
> +			/* make this device generated */
> +			device->check_index = F81534A_MAX_PORT;
> +
> +			dev_dbg(&device->intf->dev, "generated complete\n");
> +		} else {
> +			/* skip this device to generate */
> +			device->check_index = F81534A_MAX_PORT;
> +
> +			dev_err(&device->intf->dev,
> +					"error: %d, do next device generate\n",
> +					status);
> +		}
> +	}
> +
> +	mutex_unlock(&device_mutex);
> +}
> +
> +static int f81534a_ctrl_probe(struct usb_interface *intf,
> +				const struct usb_device_id *id)
> +{
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	struct f81534a_ctrl_private *priv;
> +	struct f81534a_device *device;
> +
> +	priv = devm_kzalloc(&intf->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	device = devm_kzalloc(&intf->dev, sizeof(*device), GFP_KERNEL);
> +	if (!device)
> +		return -ENOMEM;
> +
> +	mutex_init(&priv->lock);
> +	usb_set_intfdata(intf, priv);
> +
> +	INIT_LIST_HEAD(&device->list);
> +	device->intf = intf;
> +
> +	mutex_lock(&device_mutex);
> +	list_add_tail(&device->list, &device_list);
> +	mutex_unlock(&device_mutex);
> +
> +	dev = usb_get_dev(dev);
> +	schedule_delayed_work(&f81534a_generate_worker,
> +				msecs_to_jiffies(F81534A_CTRL_TIMER));
> +
> +	return 0;
> +}
> +
> +static void f81534a_ctrl_disconnect(struct usb_interface *intf)
> +{
> +	struct f81534a_ctrl_private *priv;
> +	struct f81534a_device *device;
> +	struct usb_device *dev;
> +
> +	mutex_lock(&device_mutex);
> +
> +	list_for_each_entry(device, &device_list, list) {
> +		if (device->intf == intf) {
> +			list_del(&device->list);
> +
> +			priv = usb_get_intfdata(intf);
> +			dev = interface_to_usbdev(intf);
> +			usb_put_dev(dev);
> +			break;
> +		}
> +	}
> +
> +	mutex_unlock(&device_mutex);
> +}
> +
> +static int f81534a_ctrl_suspend(struct usb_interface *intf,
> +					pm_message_t message)
> +{
> +	struct f81534a_device *device;
> +
> +	flush_delayed_work(&f81534a_generate_worker);
> +
> +	mutex_lock(&device_mutex);
> +
> +	list_for_each_entry(device, &device_list, list) {
> +		device->check_index = 0;
> +		device->check_retry = 0;
> +	}
> +
> +	mutex_unlock(&device_mutex);
> +
> +	return 0;
> +}
> +
> +static int f81534a_ctrl_resume(struct usb_interface *intf)
> +{
> +	schedule_delayed_work(&f81534a_generate_worker,
> +				msecs_to_jiffies(F81534A_CTRL_TIMER));
> +
> +	return 0;
> +}
> +
>  static int f81232_port_probe(struct usb_serial_port *port)
>  {
>  	struct f81232_private *priv;
> @@ -976,7 +1292,45 @@ static struct usb_serial_driver * const serial_drivers[] = {
>  	NULL,
>  };
>  
> -module_usb_serial_driver(serial_drivers, id_table);
> +static struct usb_driver f81534a_ctrl_driver = {
> +	.name =		"f81534a_ctrl",
> +	.id_table =	f81534a_ctrl_id_table,
> +	.probe =	f81534a_ctrl_probe,
> +	.disconnect =	f81534a_ctrl_disconnect,
> +	.suspend =	f81534a_ctrl_suspend,
> +	.resume =	f81534a_ctrl_resume,
> +};
> +
> +static int __init f81232_init(void)
> +{
> +	int status;
> +
> +	INIT_DELAYED_WORK(&f81534a_generate_worker,
> +			f81534a_ctrl_generate_worker);
> +
> +	status = usb_register_driver(&f81534a_ctrl_driver, THIS_MODULE,
> +			KBUILD_MODNAME);
> +	if (status)
> +		return status;
> +
> +	status = usb_serial_register_drivers(serial_drivers, KBUILD_MODNAME,
> +			id_table);
> +	if (status) {
> +		usb_deregister(&f81534a_ctrl_driver);
> +		return status;
> +	}
> +
> +	return 0;
> +}
> +
> +static void __exit f81232_exit(void)
> +{
> +	usb_serial_deregister_drivers(serial_drivers);
> +	usb_deregister(&f81534a_ctrl_driver);
> +}
> +
> +module_init(f81232_init);
> +module_exit(f81232_exit);
>  
>  MODULE_DESCRIPTION("Fintek F81232/532A/534A/535/536 USB to serial driver");
>  MODULE_AUTHOR("Greg Kroah-Hartman <gregkh@linuxfoundation.org>");

Johan

Re: [PATCH V1 3/6] USB: serial: f81232: Add generator for F81534A

[Ji-Ze Hong (Peter Hong)]

Hi Johan,

Johan Hovold 於 2019/8/28 下午 11:02 寫道:
> On Thu, Jun 06, 2019 at 10:54:13AM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> The Fintek F81534A series is contains 1 HUB / 1 GPIO device / n UARTs,
>> but the UART is default disable and need enabled by GPIO device(2c42/16F8).
>> When F81534A plug to host, we can only see 1 HUB & 1 GPIO device, add
>> GPIO device USB interface to device_list and trigger generate worker,
>> f81534a_generate_worker to run f81534a_ctrl_generate_ports().
>>
>> The operation in f81534a_ctrl_generate_ports() as following:
>> 	1: Write 0x8fff to F81534A_CMD_ENABLE_PORT register for enable all
>> 	   UART device.
>>
>> 	2: Read port existence & current status from F81534A_CMD_PORT_STATUS
>> 	   register. the higher 16bit will indicate the UART existence. If the
>> 	   UART is existence, we'll check it GPIO mode as long as not default
>> 	   value (default is all input mode).
>>
>> 	3: 1 GPIO device will check with max 15s and check next GPIO device when
>> 	   timeout. (F81534A_CTRL_RETRY * F81534A_CTRL_TIMER)
>>
>> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> 
> This is all looks crazy... Please better describe how the device works,
> and you want to implement support.

I'll try to refactor more simply for first add into kernel.

>> ---
>>   drivers/usb/serial/f81232.c | 356 +++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 355 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
>> index 75dfc0b9ef30..e9470fb0d691 100644
>> --- a/drivers/usb/serial/f81232.c
>> +++ b/drivers/usb/serial/f81232.c
>> @@ -41,6 +41,12 @@ static const struct usb_device_id id_table[] = {
>>   };
>>   MODULE_DEVICE_TABLE(usb, id_table);
>>   
>> +static const struct usb_device_id f81534a_ctrl_id_table[] = {
>> +	{ USB_DEVICE(0x2c42, 0x16f8) },		/* Global control device */
>> +	{ }					/* Terminating entry */
>> +};
>> +MODULE_DEVICE_TABLE(usb, f81534a_ctrl_id_table);
> 
> You can only have one MODULE_DEVICE_TABLE()...

I had a question about this. In this file, we'll need support 3 sets of
id f81232(1)/f81534a(9)/f81534a_ctrl(1). So I will refactor the code
about id section to the below due to the id table will use more than
once:

=======================================================================
#define F81232_ID		\
	{ USB_DEVICE(0x1934, 0x0706) }	/* 1 port UART device */

#define F81534A_SERIES_ID	\
	{ USB_DEVICE(0x2c42, 0x1602) },	/* In-Box 2 port UART device */	\
	{ USB_DEVICE(0x2c42, 0x1604) },	/* In-Box 4 port UART device */	\
	{ USB_DEVICE(0x2c42, 0x1605) },	/* In-Box 8 port UART device */	\
	{ USB_DEVICE(0x2c42, 0x1606) },	/* In-Box 12 port UART device */ \
	{ USB_DEVICE(0x2c42, 0x1608) },	/* Non-Flash type */ \
	{ USB_DEVICE(0x2c42, 0x1632) },	/* 2 port UART device */ \
	{ USB_DEVICE(0x2c42, 0x1634) },	/* 4 port UART device */ \
	{ USB_DEVICE(0x2c42, 0x1635) },	/* 8 port UART device */ \
	{ USB_DEVICE(0x2c42, 0x1636) }	/* 12 port UART device */

#define F81534A_CTRL_ID		\
	{ USB_DEVICE(0x2c42, 0x16f8) }	/* Global control device */

static const struct usb_device_id id_table[] = {
	F81232_ID,
	{ }					/* Terminating entry */
};

static const struct usb_device_id f81534a_id_table[] = {
	F81534A_SERIES_ID,
	{ }					/* Terminating entry */
};

static const struct usb_device_id f81534a_ctrl_id_table[] = {
	F81534A_CTRL_ID,
	{ }					/* Terminating entry */
};

static const struct usb_device_id all_serial_id_table[] = {
	F81232_ID,
	F81534A_SERIES_ID,
	{ }					/* Terminating entry */
};
MODULE_DEVICE_TABLE(usb, all_serial_id_table);
=======================================================================

but the checkpatch.pl give me the warning below:
ERROR: Macros with complex values should be enclosed in parentheses
#42: FILE: f81232.c:28:
+#define F81534A_SERIES_ID      \
+       { USB_DEVICE(0x2c42, 0x1602) }, /* In-Box 2 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1604) }, /* In-Box 4 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1605) }, /* In-Box 8 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1606) }, /* In-Box 12 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1608) }, /* Non-Flash type */ \
+       { USB_DEVICE(0x2c42, 0x1632) }, /* 2 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1634) }, /* 4 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1635) }, /* 8 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1636) }  /* 12 port UART device */

Is any suggestion ?

Thanks
-- 
With Best Regards,
Peter Hong

Re: [PATCH V1 3/6] USB: serial: f81232: Add generator for F81534A

[Johan Hovold]

On Mon, Sep 02, 2019 at 10:59:17AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Johan,
> 
> Johan Hovold 於 2019/8/28 下午 11:02 寫道:
> > On Thu, Jun 06, 2019 at 10:54:13AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> >> The Fintek F81534A series is contains 1 HUB / 1 GPIO device / n UARTs,
> >> but the UART is default disable and need enabled by GPIO device(2c42/16F8).
> >> When F81534A plug to host, we can only see 1 HUB & 1 GPIO device, add
> >> GPIO device USB interface to device_list and trigger generate worker,
> >> f81534a_generate_worker to run f81534a_ctrl_generate_ports().
> >>
> >> The operation in f81534a_ctrl_generate_ports() as following:
> >> 	1: Write 0x8fff to F81534A_CMD_ENABLE_PORT register for enable all
> >> 	   UART device.
> >>
> >> 	2: Read port existence & current status from F81534A_CMD_PORT_STATUS
> >> 	   register. the higher 16bit will indicate the UART existence. If the
> >> 	   UART is existence, we'll check it GPIO mode as long as not default
> >> 	   value (default is all input mode).
> >>
> >> 	3: 1 GPIO device will check with max 15s and check next GPIO device when
> >> 	   timeout. (F81534A_CTRL_RETRY * F81534A_CTRL_TIMER)
> >>
> >> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> > 
> > This is all looks crazy... Please better describe how the device works,
> > and you want to implement support.
> 
> I'll try to refactor more simply for first add into kernel.
> 
> >> ---
> >>   drivers/usb/serial/f81232.c | 356 +++++++++++++++++++++++++++++++++++++++++++-
> >>   1 file changed, 355 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> >> index 75dfc0b9ef30..e9470fb0d691 100644
> >> --- a/drivers/usb/serial/f81232.c
> >> +++ b/drivers/usb/serial/f81232.c
> >> @@ -41,6 +41,12 @@ static const struct usb_device_id id_table[] = {
> >>   };
> >>   MODULE_DEVICE_TABLE(usb, id_table);
> >>   
> >> +static const struct usb_device_id f81534a_ctrl_id_table[] = {
> >> +	{ USB_DEVICE(0x2c42, 0x16f8) },		/* Global control device */
> >> +	{ }					/* Terminating entry */
> >> +};
> >> +MODULE_DEVICE_TABLE(usb, f81534a_ctrl_id_table);
> > 
> > You can only have one MODULE_DEVICE_TABLE()...
> 
> I had a question about this. In this file, we'll need support 3 sets of
> id f81232(1)/f81534a(9)/f81534a_ctrl(1). So I will refactor the code
> about id section to the below due to the id table will use more than
> once:
> 
> =======================================================================
> #define F81232_ID		\
> 	{ USB_DEVICE(0x1934, 0x0706) }	/* 1 port UART device */
> 
> #define F81534A_SERIES_ID	\
> 	{ USB_DEVICE(0x2c42, 0x1602) },	/* In-Box 2 port UART device */	\
> 	{ USB_DEVICE(0x2c42, 0x1604) },	/* In-Box 4 port UART device */	\
> 	{ USB_DEVICE(0x2c42, 0x1605) },	/* In-Box 8 port UART device */	\
> 	{ USB_DEVICE(0x2c42, 0x1606) },	/* In-Box 12 port UART device */ \
> 	{ USB_DEVICE(0x2c42, 0x1608) },	/* Non-Flash type */ \
> 	{ USB_DEVICE(0x2c42, 0x1632) },	/* 2 port UART device */ \
> 	{ USB_DEVICE(0x2c42, 0x1634) },	/* 4 port UART device */ \
> 	{ USB_DEVICE(0x2c42, 0x1635) },	/* 8 port UART device */ \
> 	{ USB_DEVICE(0x2c42, 0x1636) }	/* 12 port UART device */
> 
> #define F81534A_CTRL_ID		\
> 	{ USB_DEVICE(0x2c42, 0x16f8) }	/* Global control device */
> 
> static const struct usb_device_id id_table[] = {
> 	F81232_ID,
> 	{ }					/* Terminating entry */
> };
> 
> static const struct usb_device_id f81534a_id_table[] = {
> 	F81534A_SERIES_ID,
> 	{ }					/* Terminating entry */
> };
> 
> static const struct usb_device_id f81534a_ctrl_id_table[] = {
> 	F81534A_CTRL_ID,
> 	{ }					/* Terminating entry */
> };
> 
> static const struct usb_device_id all_serial_id_table[] = {
> 	F81232_ID,
> 	F81534A_SERIES_ID,
> 	{ }					/* Terminating entry */
> };
> MODULE_DEVICE_TABLE(usb, all_serial_id_table);
> =======================================================================
> 
> but the checkpatch.pl give me the warning below:
> ERROR: Macros with complex values should be enclosed in parentheses
> #42: FILE: f81232.c:28:
> +#define F81534A_SERIES_ID      \
> +       { USB_DEVICE(0x2c42, 0x1602) }, /* In-Box 2 port UART device */ \
> +       { USB_DEVICE(0x2c42, 0x1604) }, /* In-Box 4 port UART device */ \
> +       { USB_DEVICE(0x2c42, 0x1605) }, /* In-Box 8 port UART device */ \
> +       { USB_DEVICE(0x2c42, 0x1606) }, /* In-Box 12 port UART device */ \
> +       { USB_DEVICE(0x2c42, 0x1608) }, /* Non-Flash type */ \
> +       { USB_DEVICE(0x2c42, 0x1632) }, /* 2 port UART device */ \
> +       { USB_DEVICE(0x2c42, 0x1634) }, /* 4 port UART device */ \
> +       { USB_DEVICE(0x2c42, 0x1635) }, /* 8 port UART device */ \
> +       { USB_DEVICE(0x2c42, 0x1636) }  /* 12 port UART device */
> 
> Is any suggestion ?

Just ignore checkpatch.pl, that's often the right answer. We already
have something similar to the above in usb-serial-simple.c.

Unless you can come up with some better way to deal with this.

Johan

[PATCH V1 4/6] USB: serial: f81232: Add tx_empty function

[Ji-Ze Hong (Peter Hong)]

Add tx_empty() function for F81232 & F81534A series.

Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
---
 drivers/usb/serial/f81232.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index e9470fb0d691..7d1ec8f9d168 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -850,6 +850,24 @@ static void f81232_dtr_rts(struct usb_serial_port *port, int on)
 		f81232_set_mctrl(port, 0, TIOCM_DTR | TIOCM_RTS);
 }
 
+static bool f81232_tx_empty(struct usb_serial_port *port)
+{
+	int status;
+	u8 tmp;
+	u8 both_empty = UART_LSR_TEMT | UART_LSR_THRE;
+
+	status = f81232_get_register(port, LINE_STATUS_REGISTER, &tmp);
+	if (status) {
+		dev_err(&port->dev, "get LSR status failed: %d\n", status);
+		return false;
+	}
+
+	if ((tmp & both_empty) != both_empty)
+		return false;
+
+	return true;
+}
+
 static int f81232_carrier_raised(struct usb_serial_port *port)
 {
 	u8 msr;
@@ -1279,6 +1297,7 @@ static struct usb_serial_driver f81232_device = {
 	.tiocmget =		f81232_tiocmget,
 	.tiocmset =		f81232_tiocmset,
 	.tiocmiwait =		usb_serial_generic_tiocmiwait,
+	.tx_empty =		f81232_tx_empty,
 	.process_read_urb =	f81232_read_urb_proxy,
 	.read_int_callback =	f81232_read_int_callback,
 	.port_probe =		f81232_port_probe,
-- 
2.7.4

Re: [PATCH V1 4/6] USB: serial: f81232: Add tx_empty function

[Johan Hovold]

On Thu, Jun 06, 2019 at 10:54:14AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Add tx_empty() function for F81232 & F81534A series.
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> ---
>  drivers/usb/serial/f81232.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index e9470fb0d691..7d1ec8f9d168 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -850,6 +850,24 @@ static void f81232_dtr_rts(struct usb_serial_port *port, int on)
>  		f81232_set_mctrl(port, 0, TIOCM_DTR | TIOCM_RTS);
>  }
>  
> +static bool f81232_tx_empty(struct usb_serial_port *port)
> +{
> +	int status;
> +	u8 tmp;
> +	u8 both_empty = UART_LSR_TEMT | UART_LSR_THRE;

Doesn't TEMT being set imply that THRE is set? So you only need to check
the former?

> +
> +	status = f81232_get_register(port, LINE_STATUS_REGISTER, &tmp);
> +	if (status) {
> +		dev_err(&port->dev, "get LSR status failed: %d\n", status);
> +		return false;
> +	}
> +
> +	if ((tmp & both_empty) != both_empty)
> +		return false;
> +
> +	return true;
> +}
> +
>  static int f81232_carrier_raised(struct usb_serial_port *port)
>  {
>  	u8 msr;
> @@ -1279,6 +1297,7 @@ static struct usb_serial_driver f81232_device = {
>  	.tiocmget =		f81232_tiocmget,
>  	.tiocmset =		f81232_tiocmset,
>  	.tiocmiwait =		usb_serial_generic_tiocmiwait,
> +	.tx_empty =		f81232_tx_empty,
>  	.process_read_urb =	f81232_read_urb_proxy,
>  	.read_int_callback =	f81232_read_int_callback,
>  	.port_probe =		f81232_port_probe,

Johan

[PATCH V1 5/6] USB: serial: f81232: Use devm_kzalloc

[Ji-Ze Hong (Peter Hong)]

Use devm_kzalloc() to replace kzalloc().

Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
---
 drivers/usb/serial/f81232.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 7d1ec8f9d168..708d85c7d822 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -1198,7 +1198,7 @@ static int f81232_port_probe(struct usb_serial_port *port)
 	struct f81232_private *priv;
 	int status = 0;
 
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(&port->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
@@ -1234,16 +1234,6 @@ static int f81232_port_probe(struct usb_serial_port *port)
 	return status;
 }
 
-static int f81232_port_remove(struct usb_serial_port *port)
-{
-	struct f81232_private *priv;
-
-	priv = usb_get_serial_port_data(port);
-	kfree(priv);
-
-	return 0;
-}
-
 static int f81232_suspend(struct usb_serial *serial, pm_message_t message)
 {
 	struct usb_serial_port *port = serial->port[0];
@@ -1301,7 +1291,6 @@ static struct usb_serial_driver f81232_device = {
 	.process_read_urb =	f81232_read_urb_proxy,
 	.read_int_callback =	f81232_read_int_callback,
 	.port_probe =		f81232_port_probe,
-	.port_remove =		f81232_port_remove,
 	.suspend =		f81232_suspend,
 	.resume =		f81232_resume,
 };
-- 
2.7.4

[PATCH V1 6/6] USB: serial: f81232: Add gpiolib to GPIO device

[Ji-Ze Hong (Peter Hong)]

The Fintek F81534A series contains 3 GPIOs per UART and The max GPIOs
is 12x3 = 36 GPIOs.

Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
---
 drivers/usb/serial/f81232.c | 210 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 210 insertions(+)

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 708d85c7d822..a53240bc164a 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -18,6 +18,7 @@
 #include <linux/moduleparam.h>
 #include <linux/mutex.h>
 #include <linux/uaccess.h>
+#include <linux/gpio.h>
 #include <linux/usb.h>
 #include <linux/usb/serial.h>
 #include <linux/serial_reg.h>
@@ -132,6 +133,7 @@ struct f81232_private {
 
 struct f81534a_ctrl_private {
 	struct usb_interface *intf;
+	struct gpio_chip chip;
 	struct mutex lock;
 	int device_idx;
 };
@@ -1007,6 +1009,204 @@ static int f81534a_ctrl_set_register(struct usb_device *dev, u16 reg, u16 size,
 	return status;
 }
 
+static int f81534a_ctrl_set_mask_register(struct usb_device *dev, u16 reg,
+		u8 mask, u8 val)
+{
+	int status;
+	u8 tmp;
+
+	status = f81534a_ctrl_get_register(dev, reg, 1, &tmp);
+	if (status)
+		return status;
+
+
+	tmp = (tmp & ~mask) | (val & mask);
+
+	status = f81534a_ctrl_set_register(dev, reg, 1, &tmp);
+	if (status)
+		return status;
+
+	return 0;
+}
+
+#ifdef CONFIG_GPIOLIB
+static int f81534a_gpio_get(struct gpio_chip *chip, unsigned int gpio_num)
+{
+	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
+	struct usb_interface *intf = priv->intf;
+	struct usb_device *dev = interface_to_usbdev(intf);
+	int status;
+	u8 tmp[2];
+	int set;
+	int idx;
+
+	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
+	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
+
+	status = mutex_lock_interruptible(&priv->lock);
+	if (status)
+		return -EINTR;
+
+	status = f81534a_ctrl_get_register(dev, F81534A_CTRL_GPIO_REG + set,
+							sizeof(tmp), tmp);
+	if (status) {
+		mutex_unlock(&priv->lock);
+		return status;
+	}
+
+	mutex_unlock(&priv->lock);
+
+	return !!(tmp[1] & BIT(idx));
+}
+
+static int f81534a_gpio_direction_in(struct gpio_chip *chip,
+					unsigned int gpio_num)
+{
+	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
+	struct usb_interface *intf = priv->intf;
+	struct usb_device *dev = interface_to_usbdev(intf);
+	int status;
+	int set;
+	int idx;
+	u8 mask;
+
+	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
+	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
+	mask = F81534A_GPIO_MODE0_DIR << idx;
+
+	status = mutex_lock_interruptible(&priv->lock);
+	if (status)
+		return -EINTR;
+
+	status = f81534a_ctrl_set_mask_register(dev, F81534A_CTRL_GPIO_REG +
+				set, mask, mask);
+	if (status) {
+		mutex_unlock(&priv->lock);
+		return status;
+	}
+
+	mutex_unlock(&priv->lock);
+
+	return 0;
+}
+
+static int f81534a_gpio_direction_out(struct gpio_chip *chip,
+				     unsigned int gpio_num, int val)
+{
+	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
+	struct usb_interface *intf = priv->intf;
+	struct usb_device *dev = interface_to_usbdev(intf);
+	int status;
+	int set;
+	int idx;
+	u8 mask;
+	u8 data;
+
+	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
+	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
+	mask = (F81534A_GPIO_MODE0_DIR << idx) | BIT(idx);
+	data = val ? BIT(idx) : 0;
+
+	status = mutex_lock_interruptible(&priv->lock);
+	if (status)
+		return -EINTR;
+
+	status = f81534a_ctrl_set_mask_register(dev, F81534A_CTRL_GPIO_REG +
+				set, mask, data);
+	if (status) {
+		mutex_unlock(&priv->lock);
+		return status;
+	}
+
+	mutex_unlock(&priv->lock);
+
+	return 0;
+}
+
+static void f81534a_gpio_set(struct gpio_chip *chip, unsigned int gpio_num,
+				int val)
+{
+	f81534a_gpio_direction_out(chip, gpio_num, val);
+}
+
+static int f81534a_gpio_get_direction(struct gpio_chip *chip,
+				unsigned int gpio_num)
+{
+	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
+	struct usb_interface *intf = priv->intf;
+	struct usb_device *dev = interface_to_usbdev(intf);
+	int status;
+	u8 tmp[2];
+	int set;
+	int idx;
+	u8 mask;
+
+	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
+	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
+	mask = F81534A_GPIO_MODE0_DIR << idx;
+
+	status = mutex_lock_interruptible(&priv->lock);
+	if (status)
+		return -EINTR;
+
+	status = f81534a_ctrl_get_register(dev, F81534A_CTRL_GPIO_REG + set,
+							sizeof(tmp), tmp);
+	if (status) {
+		mutex_unlock(&priv->lock);
+		return status;
+	}
+
+	mutex_unlock(&priv->lock);
+
+	if (tmp[0] & mask)
+		return GPIOF_DIR_IN;
+
+	return GPIOF_DIR_OUT;
+}
+
+static int f81534a_prepare_gpio(struct usb_interface *intf)
+{
+	struct f81534a_ctrl_private *priv = usb_get_intfdata(intf);
+	int status;
+
+	priv->chip.parent = &intf->dev;
+	priv->chip.owner = THIS_MODULE;
+	priv->chip.get_direction = f81534a_gpio_get_direction,
+	priv->chip.get = f81534a_gpio_get;
+	priv->chip.direction_input = f81534a_gpio_direction_in;
+	priv->chip.set = f81534a_gpio_set;
+	priv->chip.direction_output = f81534a_gpio_direction_out;
+	priv->chip.label = "f81534a";
+	/* M0(SD)/M1/M2 */
+	priv->chip.ngpio = F81534A_CTRL_GPIO_MAX_PIN * F81534A_MAX_PORT;
+	priv->chip.base = -1;
+
+	priv->intf = intf;
+	mutex_init(&priv->lock);
+
+	status = devm_gpiochip_add_data(&intf->dev, &priv->chip, priv);
+	if (status) {
+		dev_err(&intf->dev, "%s: failed: %d\n", __func__, status);
+		return status;
+	}
+
+	return 0;
+}
+#else
+static int f81534a_prepare_gpio(struct usb_interface *intf)
+{
+	dev_warn(&intf->dev, "CONFIG_GPIOLIB is not enabled\n");
+	dev_warn(&intf->dev, "The GPIOLIB interface will not register\n");
+
+	return 0;
+}
+#endif
+
+static int f81534a_release_gpio(struct usb_interface *intf)
+{
+	return 0;
+}
+
 static int f81534a_ctrl_generate_ports(struct usb_interface *intf,
 					struct f81534a_device *device)
 {
@@ -1118,6 +1318,7 @@ static int f81534a_ctrl_probe(struct usb_interface *intf,
 	struct usb_device *dev = interface_to_usbdev(intf);
 	struct f81534a_ctrl_private *priv;
 	struct f81534a_device *device;
+	int status;
 
 	priv = devm_kzalloc(&intf->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -1130,6 +1331,10 @@ static int f81534a_ctrl_probe(struct usb_interface *intf,
 	mutex_init(&priv->lock);
 	usb_set_intfdata(intf, priv);
 
+	status = f81534a_prepare_gpio(intf);
+	if (status)
+		return status;
+
 	INIT_LIST_HEAD(&device->list);
 	device->intf = intf;
 
@@ -1158,6 +1363,11 @@ static void f81534a_ctrl_disconnect(struct usb_interface *intf)
 
 			priv = usb_get_intfdata(intf);
 			dev = interface_to_usbdev(intf);
+
+			mutex_lock(&priv->lock);
+			f81534a_release_gpio(intf);
+			mutex_unlock(&priv->lock);
+
 			usb_put_dev(dev);
 			break;
 		}
-- 
2.7.4

Re: [PATCH V1 6/6] USB: serial: f81232: Add gpiolib to GPIO device

[Johan Hovold]

On Thu, Jun 06, 2019 at 10:54:16AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> The Fintek F81534A series contains 3 GPIOs per UART and The max GPIOs
> is 12x3 = 36 GPIOs.

How does this relate to the GPIOs used for transceiver setup? Are these
really general purpose?

Side note: Please explain the relationship to f81534 which you already
have a driver for. Is f81534a all that different that it belongs with
f81232? Confusing...

> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> ---
>  drivers/usb/serial/f81232.c | 210 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 210 insertions(+)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index 708d85c7d822..a53240bc164a 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -18,6 +18,7 @@
>  #include <linux/moduleparam.h>
>  #include <linux/mutex.h>
>  #include <linux/uaccess.h>
> +#include <linux/gpio.h>
>  #include <linux/usb.h>
>  #include <linux/usb/serial.h>
>  #include <linux/serial_reg.h>
> @@ -132,6 +133,7 @@ struct f81232_private {
>  
>  struct f81534a_ctrl_private {
>  	struct usb_interface *intf;
> +	struct gpio_chip chip;
>  	struct mutex lock;
>  	int device_idx;
>  };
> @@ -1007,6 +1009,204 @@ static int f81534a_ctrl_set_register(struct usb_device *dev, u16 reg, u16 size,
>  	return status;
>  }
>  
> +static int f81534a_ctrl_set_mask_register(struct usb_device *dev, u16 reg,
> +		u8 mask, u8 val)
> +{
> +	int status;
> +	u8 tmp;
> +
> +	status = f81534a_ctrl_get_register(dev, reg, 1, &tmp);
> +	if (status)
> +		return status;
> +
> +
> +	tmp = (tmp & ~mask) | (val & mask);
> +
> +	status = f81534a_ctrl_set_register(dev, reg, 1, &tmp);
> +	if (status)
> +		return status;
> +
> +	return 0;
> +}

You'll get a warning about this one being unused with !GPIOLIB.

> +#ifdef CONFIG_GPIOLIB
> +static int f81534a_gpio_get(struct gpio_chip *chip, unsigned int gpio_num)
> +{
> +	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
> +	struct usb_interface *intf = priv->intf;
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	int status;
> +	u8 tmp[2];
> +	int set;
> +	int idx;
> +
> +	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
> +	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
> +
> +	status = mutex_lock_interruptible(&priv->lock);

Why _interruptible?

> +	if (status)
> +		return -EINTR;
> +
> +	status = f81534a_ctrl_get_register(dev, F81534A_CTRL_GPIO_REG + set,
> +							sizeof(tmp), tmp);
> +	if (status) {
> +		mutex_unlock(&priv->lock);
> +		return status;
> +	}
> +
> +	mutex_unlock(&priv->lock);
> +
> +	return !!(tmp[1] & BIT(idx));
> +}
> +
> +static int f81534a_gpio_direction_in(struct gpio_chip *chip,
> +					unsigned int gpio_num)
> +{
> +	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
> +	struct usb_interface *intf = priv->intf;
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	int status;
> +	int set;
> +	int idx;
> +	u8 mask;
> +
> +	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
> +	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
> +	mask = F81534A_GPIO_MODE0_DIR << idx;
> +
> +	status = mutex_lock_interruptible(&priv->lock);
> +	if (status)
> +		return -EINTR;
> +
> +	status = f81534a_ctrl_set_mask_register(dev, F81534A_CTRL_GPIO_REG +
> +				set, mask, mask);
> +	if (status) {
> +		mutex_unlock(&priv->lock);
> +		return status;

Just return status below instead.

> +	}
> +
> +	mutex_unlock(&priv->lock);
> +
> +	return 0;
> +}
> +
> +static int f81534a_gpio_direction_out(struct gpio_chip *chip,
> +				     unsigned int gpio_num, int val)
> +{
> +	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
> +	struct usb_interface *intf = priv->intf;
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	int status;
> +	int set;
> +	int idx;
> +	u8 mask;
> +	u8 data;
> +
> +	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
> +	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
> +	mask = (F81534A_GPIO_MODE0_DIR << idx) | BIT(idx);
> +	data = val ? BIT(idx) : 0;
> +
> +	status = mutex_lock_interruptible(&priv->lock);
> +	if (status)
> +		return -EINTR;
> +
> +	status = f81534a_ctrl_set_mask_register(dev, F81534A_CTRL_GPIO_REG +
> +				set, mask, data);

Please keep set on the same line as the reg define, but why not
calculate a reg temporary above instead?

> +	if (status) {
> +		mutex_unlock(&priv->lock);
> +		return status;

As above.

> +	}
> +
> +	mutex_unlock(&priv->lock);
> +
> +	return 0;
> +}
> +
> +static void f81534a_gpio_set(struct gpio_chip *chip, unsigned int gpio_num,
> +				int val)
> +{
> +	f81534a_gpio_direction_out(chip, gpio_num, val);
> +}
> +
> +static int f81534a_gpio_get_direction(struct gpio_chip *chip,
> +				unsigned int gpio_num)
> +{
> +	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
> +	struct usb_interface *intf = priv->intf;
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	int status;
> +	u8 tmp[2];
> +	int set;
> +	int idx;
> +	u8 mask;
> +
> +	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
> +	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
> +	mask = F81534A_GPIO_MODE0_DIR << idx;
> +
> +	status = mutex_lock_interruptible(&priv->lock);
> +	if (status)
> +		return -EINTR;
> +
> +	status = f81534a_ctrl_get_register(dev, F81534A_CTRL_GPIO_REG + set,
> +							sizeof(tmp), tmp);
> +	if (status) {
> +		mutex_unlock(&priv->lock);
> +		return status;
> +	}
> +
> +	mutex_unlock(&priv->lock);
> +
> +	if (tmp[0] & mask)
> +		return GPIOF_DIR_IN;
> +
> +	return GPIOF_DIR_OUT;
> +}
> +
> +static int f81534a_prepare_gpio(struct usb_interface *intf)
> +{
> +	struct f81534a_ctrl_private *priv = usb_get_intfdata(intf);
> +	int status;
> +
> +	priv->chip.parent = &intf->dev;
> +	priv->chip.owner = THIS_MODULE;
> +	priv->chip.get_direction = f81534a_gpio_get_direction,
> +	priv->chip.get = f81534a_gpio_get;
> +	priv->chip.direction_input = f81534a_gpio_direction_in;
> +	priv->chip.set = f81534a_gpio_set;
> +	priv->chip.direction_output = f81534a_gpio_direction_out;
> +	priv->chip.label = "f81534a";
> +	/* M0(SD)/M1/M2 */
> +	priv->chip.ngpio = F81534A_CTRL_GPIO_MAX_PIN * F81534A_MAX_PORT;

It looks like you should have one gpiochip per port.

> +	priv->chip.base = -1;

You need to set the can_sleep flag.

> +
> +	priv->intf = intf;
> +	mutex_init(&priv->lock);
> +
> +	status = devm_gpiochip_add_data(&intf->dev, &priv->chip, priv);
> +	if (status) {
> +		dev_err(&intf->dev, "%s: failed: %d\n", __func__, status);

No need for __func__. Spell what went wrong (e.g. "failed to register
gpiochip: %d\n").

> +		return status;
> +	}
> +
> +	return 0;
> +}
> +#else
> +static int f81534a_prepare_gpio(struct usb_interface *intf)
> +{
> +	dev_warn(&intf->dev, "CONFIG_GPIOLIB is not enabled\n");
> +	dev_warn(&intf->dev, "The GPIOLIB interface will not register\n");

Please remove this.

> +
> +	return 0;
> +}
> +#endif
> +
> +static int f81534a_release_gpio(struct usb_interface *intf)
> +{
> +	return 0;
> +}

Remove.

> +
>  static int f81534a_ctrl_generate_ports(struct usb_interface *intf,
>  					struct f81534a_device *device)
>  {
> @@ -1118,6 +1318,7 @@ static int f81534a_ctrl_probe(struct usb_interface *intf,
>  	struct usb_device *dev = interface_to_usbdev(intf);
>  	struct f81534a_ctrl_private *priv;
>  	struct f81534a_device *device;
> +	int status;
>  
>  	priv = devm_kzalloc(&intf->dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
> @@ -1130,6 +1331,10 @@ static int f81534a_ctrl_probe(struct usb_interface *intf,
>  	mutex_init(&priv->lock);
>  	usb_set_intfdata(intf, priv);
>  
> +	status = f81534a_prepare_gpio(intf);
> +	if (status)
> +		return status;
> +
>  	INIT_LIST_HEAD(&device->list);
>  	device->intf = intf;
>  
> @@ -1158,6 +1363,11 @@ static void f81534a_ctrl_disconnect(struct usb_interface *intf)
>  
>  			priv = usb_get_intfdata(intf);
>  			dev = interface_to_usbdev(intf);
> +
> +			mutex_lock(&priv->lock);
> +			f81534a_release_gpio(intf);
> +			mutex_unlock(&priv->lock);
> +
>  			usb_put_dev(dev);
>  			break;
>  		}

Johan