Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver
@ 2019-09-23  2:24 Ji-Ze Hong (Peter Hong)
  2019-09-23  2:24 ` [PATCH V2 1/7] USB: serial: f81232: Extract LSR handler Ji-Ze Hong (Peter Hong)
                   ` (7 more replies)
  0 siblings, 8 replies; 24+ messages in thread
From: Ji-Ze Hong (Peter Hong) @ 2019-09-23  2:24 UTC (permalink / raw)
  To: johan
  Cc: gregkh, linux-usb, linux-kernel, peter_hong, Ji-Ze Hong (Peter Hong)

The Fintek F81532A/534A/535/536 is USB-to-2/4/8/12 serial ports device
and the serial port is default disabled when plugin computer.

The part number is a bit same with F81532/534, but F81534A series UART
core is enhanced from F81232, not F81532/534.  

The IC is contains devices as following:
	1. HUB (all devices is connected with this hub)
	2. GPIO/Control device. (enable serial port and control all GPIOs)
	3. serial port 1 to x (2/4/8/12)

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]

We'll try to do some code refacting before add F81534A series.

Ji-Ze Hong (Peter Hong) (7):
  USB: serial: f81232: Extract LSR handler
  USB: serial: f81232: Add tx_empty function
  USB: serial: f81232: Use devm_kzalloc
  USB: serial: f81232: Add F81534A support
  USB: serial: f81232: Set F81534A serial port with RS232 mode
  USB: serial: f81232: Add generator for F81534A
  USB: serial: f81232: Add gpiolib to GPIO device

 drivers/usb/serial/f81232.c | 604 ++++++++++++++++++++++++++++++++++--
 1 file changed, 570 insertions(+), 34 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH V2 1/7] USB: serial: f81232: Extract LSR handler
  2019-09-23  2:24 [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver Ji-Ze Hong (Peter Hong)
@ 2019-09-23  2:24 ` Ji-Ze Hong (Peter Hong)
  2019-10-23  9:23   ` Johan Hovold
  2019-09-23  2:24 ` [PATCH V2 2/7] USB: serial: f81232: Add tx_empty function Ji-Ze Hong (Peter Hong)
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Ji-Ze Hong (Peter Hong) @ 2019-09-23  2:24 UTC (permalink / raw)
  To: johan
  Cc: gregkh, linux-usb, linux-kernel, peter_hong, Ji-Ze Hong (Peter Hong)

Extract LSR handler to function.

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 43fa1f0716b7..c07d376c743d 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -322,10 +322,38 @@ static void f81232_read_int_callback(struct urb *urb)
 			__func__, retval);
 }
 
+static char f81232_handle_lsr(struct usb_serial_port *port, u8 lsr)
+{
+	struct f81232_private *priv = usb_get_serial_port_data(port);
+	char tty_flag = TTY_NORMAL;
+
+	if (!(lsr & UART_LSR_BRK_ERROR_BITS))
+		return tty_flag;
+
+	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);
+	}
+
+	return tty_flag;
+}
+
 static void f81232_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;
@@ -341,29 +369,8 @@ static void f81232_process_read_urb(struct urb *urb)
 	/* bulk-in data: [LSR(1Byte)+DATA(1Byte)][LSR(1Byte)+DATA(1Byte)]... */
 
 	for (i = 0; i < urb->actual_length; i += 2) {
-		tty_flag = TTY_NORMAL;
 		lsr = data[i];
-
-		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);
-			}
-		}
+		tty_flag = f81232_handle_lsr(port, lsr);
 
 		if (port->port.console && port->sysrq) {
 			if (usb_serial_handle_sysrq_char(port, data[i + 1]))
-- 
2.17.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH V2 2/7] USB: serial: f81232: Add tx_empty function
  2019-09-23  2:24 [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver Ji-Ze Hong (Peter Hong)
  2019-09-23  2:24 ` [PATCH V2 1/7] USB: serial: f81232: Extract LSR handler Ji-Ze Hong (Peter Hong)
@ 2019-09-23  2:24 ` Ji-Ze Hong (Peter Hong)
  2019-10-23  9:15   ` Johan Hovold
  2019-09-23  2:24 ` [PATCH V2 3/7] USB: serial: f81232: Use devm_kzalloc Ji-Ze Hong (Peter Hong)
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Ji-Ze Hong (Peter Hong) @ 2019-09-23  2:24 UTC (permalink / raw)
  To: johan
  Cc: gregkh, linux-usb, linux-kernel, peter_hong, Ji-Ze Hong (Peter Hong)

Add tx_empty() function for F81232.

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index c07d376c743d..b42b3738a768 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -685,6 +685,23 @@ 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;
+
+	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 & UART_LSR_TEMT) != UART_LSR_TEMT)
+		return false;
+
+	return true;
+}
+
 static int f81232_carrier_raised(struct usb_serial_port *port)
 {
 	u8 msr;
@@ -820,6 +837,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_process_read_urb,
 	.read_int_callback =	f81232_read_int_callback,
 	.port_probe =		f81232_port_probe,
-- 
2.17.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH V2 3/7] USB: serial: f81232: Use devm_kzalloc
  2019-09-23  2:24 [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver Ji-Ze Hong (Peter Hong)
  2019-09-23  2:24 ` [PATCH V2 1/7] USB: serial: f81232: Extract LSR handler Ji-Ze Hong (Peter Hong)
  2019-09-23  2:24 ` [PATCH V2 2/7] USB: serial: f81232: Add tx_empty function Ji-Ze Hong (Peter Hong)
@ 2019-09-23  2:24 ` Ji-Ze Hong (Peter Hong)
  2019-09-23  2:24 ` [PATCH V2 4/7] USB: serial: f81232: Add F81534A support Ji-Ze Hong (Peter Hong)
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Ji-Ze Hong (Peter Hong) @ 2019-09-23  2:24 UTC (permalink / raw)
  To: johan
  Cc: gregkh, linux-usb, linux-kernel, peter_hong, 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 b42b3738a768..e4db0aec9af0 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -756,7 +756,7 @@ static int f81232_port_probe(struct usb_serial_port *port)
 {
 	struct f81232_private *priv;
 
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(&port->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
@@ -772,16 +772,6 @@ static int f81232_port_probe(struct usb_serial_port *port)
 	return 0;
 }
 
-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];
@@ -841,7 +831,6 @@ static struct usb_serial_driver f81232_device = {
 	.process_read_urb =	f81232_process_read_urb,
 	.read_int_callback =	f81232_read_int_callback,
 	.port_probe =		f81232_port_probe,
-	.port_remove =		f81232_port_remove,
 	.suspend =		f81232_suspend,
 	.resume =		f81232_resume,
 };
-- 
2.17.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH V2 4/7] USB: serial: f81232: Add F81534A support
  2019-09-23  2:24 [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver Ji-Ze Hong (Peter Hong)
                   ` (2 preceding siblings ...)
  2019-09-23  2:24 ` [PATCH V2 3/7] USB: serial: f81232: Use devm_kzalloc Ji-Ze Hong (Peter Hong)
@ 2019-09-23  2:24 ` Ji-Ze Hong (Peter Hong)
  2019-10-23  9:59   ` Johan Hovold
  2019-09-23  2:24 ` [PATCH V2 5/7] USB: serial: f81232: Set F81534A serial port with RS232 mode Ji-Ze Hong (Peter Hong)
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Ji-Ze Hong (Peter Hong) @ 2019-09-23  2:24 UTC (permalink / raw)
  To: johan
  Cc: gregkh, linux-usb, linux-kernel, peter_hong, Ji-Ze Hong (Peter Hong)

The Fintek F81532A/534A/535/536 is USB-to-2/4/8/12 serial ports device
and the serial port is default disabled when plugin computer.

The IC is contains devices as following:
	1. HUB (all devices is connected with this hub)
	2. GPIO/Control device. (enable serial port and control GPIOs)
	3. serial port 1 to x (2/4/8/12)

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 | 131 ++++++++++++++++++++++++++++++++++--
 1 file changed, 127 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index e4db0aec9af0..36a17aedc2ae 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
@@ -21,11 +22,36 @@
 #include <linux/usb/serial.h>
 #include <linux/serial_reg.h>
 
+#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 */
+
 static const struct usb_device_id id_table[] = {
-	{ USB_DEVICE(0x1934, 0x0706) },
+	F81232_ID,
+	{ }					/* Terminating entry */
+};
+
+static const struct usb_device_id f81534a_id_table[] = {
+	F81534A_SERIES_ID,
+	{ }					/* Terminating entry */
+};
+
+static const struct usb_device_id all_serial_id_table[] = {
+	F81232_ID,
+	F81534A_SERIES_ID,
 	{ }					/* Terminating entry */
 };
-MODULE_DEVICE_TABLE(usb, id_table);
+MODULE_DEVICE_TABLE(usb, all_serial_id_table);
 
 /* Maximum baudrate for F81232 */
 #define F81232_MAX_BAUDRATE		1500000
@@ -35,6 +61,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)
@@ -61,6 +91,11 @@ 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))
+
 struct f81232_private {
 	struct mutex lock;
 	u8 modem_control;
@@ -383,6 +418,46 @@ 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;
+	unsigned char *data = urb->transfer_buffer;
+	char tty_flag;
+	unsigned int i;
+	u8 lsr;
+	u8 len;
+
+	if (urb->actual_length < 3) {
+		dev_err(&port->dev, "error actual_length: %d\n",
+				urb->actual_length);
+		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] */
+	lsr = data[len - 1];
+	tty_flag = f81232_handle_lsr(port, lsr);
+
+	if (port->port.console && port->sysrq) {
+		for (i = 1; i < urb->actual_length - 1; ++i)
+			if (!usb_serial_handle_sysrq_char(port, data[i]))
+				tty_insert_flip_char(&port->port, data[i],
+						tty_flag);
+	} else {
+		tty_insert_flip_string_fixed_flag(&port->port, &data[1],
+							tty_flag,
+							urb->actual_length - 2);
+	}
+
+	tty_flip_buffer_push(&port->port);
+}
+
 static void f81232_break_ctl(struct tty_struct *tty, int break_state)
 {
 	struct usb_serial_port *port = tty->driver_data;
@@ -666,6 +741,23 @@ static int f81232_open(struct tty_struct *tty, struct usb_serial_port *port)
 	return 0;
 }
 
+static int f81534a_open(struct tty_struct *tty, struct usb_serial_port *port)
+{
+	int status;
+	u8 val;
+
+	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;
+	}
+
+	return f81232_open(tty, port);
+}
+
 static void f81232_close(struct usb_serial_port *port)
 {
 	struct f81232_private *port_priv = usb_get_serial_port_data(port);
@@ -772,6 +864,11 @@ static int f81232_port_probe(struct usb_serial_port *port)
 	return 0;
 }
 
+static int f81534a_port_probe(struct usb_serial_port *port)
+{
+	return f81232_port_probe(port);
+}
+
 static int f81232_suspend(struct usb_serial *serial, pm_message_t message)
 {
 	struct usb_serial_port *port = serial->port[0];
@@ -835,14 +932,40 @@ static struct usb_serial_driver f81232_device = {
 	.resume =		f81232_resume,
 };
 
+static struct usb_serial_driver f81534a_device = {
+	.driver = {
+		.owner =	THIS_MODULE,
+		.name =		"f81534a",
+	},
+	.id_table =		f81534a_id_table,
+	.num_ports =		1,
+	.open =			f81534a_open,
+	.close =		f81232_close,
+	.dtr_rts =		f81232_dtr_rts,
+	.carrier_raised =	f81232_carrier_raised,
+	.get_serial =		f81232_get_serial_info,
+	.break_ctl =		f81232_break_ctl,
+	.set_termios =		f81232_set_termios,
+	.tiocmget =		f81232_tiocmget,
+	.tiocmset =		f81232_tiocmset,
+	.tiocmiwait =		usb_serial_generic_tiocmiwait,
+	.tx_empty =		f81232_tx_empty,
+	.process_read_urb =	f81534a_process_read_urb,
+	.read_int_callback =	f81232_read_int_callback,
+	.port_probe =		f81534a_port_probe,
+	.suspend =		f81232_suspend,
+	.resume =		f81232_resume,
+};
+
 static struct usb_serial_driver * const serial_drivers[] = {
 	&f81232_device,
+	&f81534a_device,
 	NULL,
 };
 
-module_usb_serial_driver(serial_drivers, id_table);
+module_usb_serial_driver(serial_drivers, all_serial_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.17.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH V2 5/7] USB: serial: f81232: Set F81534A serial port with RS232 mode
  2019-09-23  2:24 [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver Ji-Ze Hong (Peter Hong)
                   ` (3 preceding siblings ...)
  2019-09-23  2:24 ` [PATCH V2 4/7] USB: serial: f81232: Add F81534A support Ji-Ze Hong (Peter Hong)
@ 2019-09-23  2:24 ` Ji-Ze Hong (Peter Hong)
  2019-10-23 11:53   ` Johan Hovold
  2019-09-23  2:24 ` [PATCH V2 6/7] USB: serial: f81232: Add generator for F81534A Ji-Ze Hong (Peter Hong)
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Ji-Ze Hong (Peter Hong) @ 2019-09-23  2:24 UTC (permalink / raw)
  To: johan
  Cc: gregkh, linux-usb, linux-kernel, peter_hong, Ji-Ze Hong (Peter Hong)

The Fintek F81532A/534A/535/536 is USB-to-2/4/8/12 serial ports device
and the serial ports are default disabled. Each port contains max 3 pins
GPIO and the 3 pins are default pull high with input mode.

When the serial port had activated (running probe()), we'll transform the
3 pins from GPIO function publicly to control Tranceiver privately use.
We'll default set to 0/0/1 for control transceiver to RS232 mode.

Otherwise, If the serial port is not active, the 3 pins is in GPIO mode
and controlled by global GPIO device with VID/PID: 2c42/16f8.

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 36a17aedc2ae..01cb5a5ea1d2 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -96,6 +96,15 @@ MODULE_DEVICE_TABLE(usb, all_serial_id_table);
 #define F81534A_TRIGGER_MULTPILE_4X	BIT(3)
 #define F81534A_FIFO_128BYTE		(BIT(1) | BIT(0))
 
+/* 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;
@@ -866,6 +875,14 @@ static int f81232_port_probe(struct usb_serial_port *port)
 
 static int f81534a_port_probe(struct usb_serial_port *port)
 {
+	int status;
+
+	/* tri-state with pull-high, default RS232 Mode */
+	status = f81232_set_register(port, F81534A_GPIO_REG,
+					F81534A_GPIO_MODE2_DIR);
+	if (status)
+		return status;
+
 	return f81232_port_probe(port);
 }
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH V2 6/7] USB: serial: f81232: Add generator for F81534A
  2019-09-23  2:24 [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver Ji-Ze Hong (Peter Hong)
                   ` (4 preceding siblings ...)
  2019-09-23  2:24 ` [PATCH V2 5/7] USB: serial: f81232: Set F81534A serial port with RS232 mode Ji-Ze Hong (Peter Hong)
@ 2019-09-23  2:24 ` Ji-Ze Hong (Peter Hong)
  2019-10-23 12:05   ` Johan Hovold
  2019-10-23 12:25   ` Johan Hovold
  2019-09-23  2:24 ` [PATCH V2 7/7] USB: serial: f81232: Add gpiolib to GPIO device Ji-Ze Hong (Peter Hong)
  2019-10-23  9:21 ` [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver Johan Hovold
  7 siblings, 2 replies; 24+ messages in thread
From: Ji-Ze Hong (Peter Hong) @ 2019-09-23  2:24 UTC (permalink / raw)
  To: johan
  Cc: gregkh, linux-usb, linux-kernel, peter_hong, 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 and we
need write 0x8fff to GPIO device register F81534A_CMD_ENABLE_PORT(116h)
to enable all available serial ports.

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 01cb5a5ea1d2..82cc1e6cff62 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -36,6 +36,9 @@
 	{ 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 */
@@ -46,6 +49,11 @@ static const struct usb_device_id f81534a_id_table[] = {
 	{ }					/* 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,
@@ -105,6 +113,8 @@ MODULE_DEVICE_TABLE(usb, all_serial_id_table);
 #define F81534A_GPIO_MODE1_OUTPUT	BIT(1)
 #define F81534A_GPIO_MODE0_OUTPUT	BIT(0)
 
+#define F81534A_CMD_ENABLE_PORT		0x116
+
 struct f81232_private {
 	struct mutex lock;
 	u8 modem_control;
@@ -853,6 +863,95 @@ static void f81232_lsr_worker(struct work_struct *work)
 		dev_warn(&port->dev, "read LSR failed: %d\n", 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_enable_all_ports(struct usb_interface *intf)
+{
+	struct usb_device *dev = interface_to_usbdev(intf);
+	unsigned char enable[2];
+	int status;
+
+	/* enable all available serial ports */
+	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;
+}
+
+static int f81534a_ctrl_probe(struct usb_interface *intf,
+				const struct usb_device_id *id)
+{
+	struct usb_device *dev = interface_to_usbdev(intf);
+	int status;
+
+	status = f81534a_ctrl_enable_all_ports(intf);
+	if (status)
+		return status;
+
+	dev = usb_get_dev(dev);
+	return 0;
+}
+
+static void f81534a_ctrl_disconnect(struct usb_interface *intf)
+{
+	struct usb_device *dev = interface_to_usbdev(intf);
+
+	usb_put_dev(dev);
+}
+
+static int f81534a_ctrl_resume(struct usb_interface *intf)
+{
+	return f81534a_ctrl_enable_all_ports(intf);
+}
+
 static int f81232_port_probe(struct usb_serial_port *port)
 {
 	struct f81232_private *priv;
@@ -980,7 +1079,41 @@ static struct usb_serial_driver * const serial_drivers[] = {
 	NULL,
 };
 
-module_usb_serial_driver(serial_drivers, all_serial_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,
+	.resume =	f81534a_ctrl_resume,
+};
+
+static int __init f81232_init(void)
+{
+	int status;
+
+	status = usb_register_driver(&f81534a_ctrl_driver, THIS_MODULE,
+			KBUILD_MODNAME);
+	if (status)
+		return status;
+
+	status = usb_serial_register_drivers(serial_drivers, KBUILD_MODNAME,
+			all_serial_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.17.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH V2 7/7] USB: serial: f81232: Add gpiolib to GPIO device
  2019-09-23  2:24 [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver Ji-Ze Hong (Peter Hong)
                   ` (5 preceding siblings ...)
  2019-09-23  2:24 ` [PATCH V2 6/7] USB: serial: f81232: Add generator for F81534A Ji-Ze Hong (Peter Hong)
@ 2019-09-23  2:24 ` Ji-Ze Hong (Peter Hong)
  2019-10-23 12:22   ` Johan Hovold
  2019-10-23  9:21 ` [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver Johan Hovold
  7 siblings, 1 reply; 24+ messages in thread
From: Ji-Ze Hong (Peter Hong) @ 2019-09-23  2:24 UTC (permalink / raw)
  To: johan
  Cc: gregkh, linux-usb, linux-kernel, peter_hong, Ji-Ze Hong (Peter Hong)

The Fintek F81534A series contains 3 GPIOs per UART and The max GPIOs
is 12x3 = 36 GPIOs and this patch will implements GPIO device as a
gpiochip to control all GPIO pins even transforms to transceiver pins.

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 82cc1e6cff62..dc9b28738b80 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>
@@ -104,6 +105,8 @@ MODULE_DEVICE_TABLE(usb, all_serial_id_table);
 #define F81534A_TRIGGER_MULTPILE_4X	BIT(3)
 #define F81534A_FIFO_128BYTE		(BIT(1) | BIT(0))
 
+#define F81534A_MAX_PORT		12
+
 /* 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 */
@@ -115,6 +118,13 @@ MODULE_DEVICE_TABLE(usb, all_serial_id_table);
 
 #define F81534A_CMD_ENABLE_PORT		0x116
 
+/*
+ * 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;
@@ -126,6 +136,12 @@ struct f81232_private {
 	struct usb_serial_port *port;
 };
 
+struct f81534a_ctrl_private {
+	struct usb_interface *intf;
+	struct gpio_chip chip;
+	struct mutex lock;
+};
+
 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 };
@@ -863,6 +879,50 @@ 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)
 {
@@ -908,6 +968,182 @@ static int f81534a_ctrl_set_register(struct usb_device *dev, u16 reg, u16 size,
 	return status;
 }
 
+#ifdef CONFIG_GPIOLIB
+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;
+}
+
+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;
+	int reg;
+
+	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
+	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
+	reg = F81534A_CTRL_GPIO_REG + set;
+
+	mutex_lock(&priv->lock);
+
+	status = f81534a_ctrl_get_register(dev, reg, 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;
+	int reg;
+	u8 mask;
+
+	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
+	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
+	mask = F81534A_GPIO_MODE0_DIR << idx;
+	reg = F81534A_CTRL_GPIO_REG + set;
+
+	mutex_lock(&priv->lock);
+	status = f81534a_ctrl_set_mask_register(dev, reg, mask, mask);
+	mutex_unlock(&priv->lock);
+
+	return status;
+}
+
+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;
+	int reg;
+	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);
+	reg = F81534A_CTRL_GPIO_REG + set;
+	data = val ? BIT(idx) : 0;
+
+	mutex_lock(&priv->lock);
+	status = f81534a_ctrl_set_mask_register(dev, reg, mask, data);
+	mutex_unlock(&priv->lock);
+
+	return status;
+}
+
+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;
+	int reg;
+	u8 mask;
+
+	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
+	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
+	mask = F81534A_GPIO_MODE0_DIR << idx;
+	reg = F81534A_CTRL_GPIO_REG + set;
+
+	mutex_lock(&priv->lock);
+
+	status = f81534a_ctrl_get_register(dev, reg, 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";
+	priv->chip.can_sleep = true;
+	/* 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, "failed to register gpiochip: %d\n",
+				status);
+		return status;
+	}
+
+	return 0;
+}
+#else
+static int f81534a_prepare_gpio(struct usb_interface *intf)
+{
+	return 0;
+}
+#endif
+
 static int f81534a_ctrl_enable_all_ports(struct usb_interface *intf)
 {
 	struct usb_device *dev = interface_to_usbdev(intf);
@@ -930,8 +1166,21 @@ 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;
 	int status;
 
+	priv = devm_kzalloc(&intf->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	mutex_init(&priv->lock);
+	usb_set_intfdata(intf, priv);
+	priv->intf = intf;
+
+	status = f81534a_prepare_gpio(intf);
+	if (status)
+		return status;
+
 	status = f81534a_ctrl_enable_all_ports(intf);
 	if (status)
 		return status;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 2/7] USB: serial: f81232: Add tx_empty function
  2019-09-23  2:24 ` [PATCH V2 2/7] USB: serial: f81232: Add tx_empty function Ji-Ze Hong (Peter Hong)
@ 2019-10-23  9:15   ` Johan Hovold
  0 siblings, 0 replies; 24+ messages in thread
From: Johan Hovold @ 2019-10-23  9:15 UTC (permalink / raw)
  To: Ji-Ze Hong (Peter Hong)
  Cc: johan, gregkh, linux-usb, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)

On Mon, Sep 23, 2019 at 10:24:44AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Add tx_empty() function for F81232.
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> ---
>  drivers/usb/serial/f81232.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index c07d376c743d..b42b3738a768 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -685,6 +685,23 @@ 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;
> +
> +	status = f81232_get_register(port, LINE_STATUS_REGISTER, &tmp);
> +	if (status) {
> +		dev_err(&port->dev, "get LSR status failed: %d\n", status);
> +		return false;

You need to return true here on errors like the other drivers do
(consider a disconnected device where you end up with -ENODEV here).

> +	}
> +
> +	if ((tmp & UART_LSR_TEMT) != UART_LSR_TEMT)
> +		return false;
> +
> +	return true;
> +}

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver
  2019-09-23  2:24 [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver Ji-Ze Hong (Peter Hong)
                   ` (6 preceding siblings ...)
  2019-09-23  2:24 ` [PATCH V2 7/7] USB: serial: f81232: Add gpiolib to GPIO device Ji-Ze Hong (Peter Hong)
@ 2019-10-23  9:21 ` Johan Hovold
  7 siblings, 0 replies; 24+ messages in thread
From: Johan Hovold @ 2019-10-23  9:21 UTC (permalink / raw)
  To: Ji-Ze Hong (Peter Hong)
  Cc: johan, gregkh, linux-usb, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)

On Mon, Sep 23, 2019 at 10:24:42AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> The Fintek F81532A/534A/535/536 is USB-to-2/4/8/12 serial ports device
> and the serial port is default disabled when plugin computer.
> 
> The part number is a bit same with F81532/534, but F81534A series UART
> core is enhanced from F81232, not F81532/534.  
> 
> The IC is contains devices as following:
> 	1. HUB (all devices is connected with this hub)
> 	2. GPIO/Control device. (enable serial port and control all GPIOs)
> 	3. serial port 1 to x (2/4/8/12)
> 
> 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]
> 
> We'll try to do some code refacting before add F81534A series.

Make sure to always provide a changelog when sending an updated series
(either here for the entire series or below the --- line in the
individual patches).

What has changed since v1?

> Ji-Ze Hong (Peter Hong) (7):
>   USB: serial: f81232: Extract LSR handler
>   USB: serial: f81232: Add tx_empty function
>   USB: serial: f81232: Use devm_kzalloc
>   USB: serial: f81232: Add F81534A support
>   USB: serial: f81232: Set F81534A serial port with RS232 mode
>   USB: serial: f81232: Add generator for F81534A
>   USB: serial: f81232: Add gpiolib to GPIO device
> 
>  drivers/usb/serial/f81232.c | 604 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 570 insertions(+), 34 deletions(-)

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 1/7] USB: serial: f81232: Extract LSR handler
  2019-09-23  2:24 ` [PATCH V2 1/7] USB: serial: f81232: Extract LSR handler Ji-Ze Hong (Peter Hong)
@ 2019-10-23  9:23   ` Johan Hovold
  0 siblings, 0 replies; 24+ messages in thread
From: Johan Hovold @ 2019-10-23  9:23 UTC (permalink / raw)
  To: Ji-Ze Hong (Peter Hong)
  Cc: johan, gregkh, linux-usb, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)

On Mon, Sep 23, 2019 at 10:24:43AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Extract LSR handler to function.

Try to always provide a motivation for your changes, don't just say what
you do (e.g. mention reusing this for the new device types if that's
what you plan on doing).

> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 4/7] USB: serial: f81232: Add F81534A support
  2019-09-23  2:24 ` [PATCH V2 4/7] USB: serial: f81232: Add F81534A support Ji-Ze Hong (Peter Hong)
@ 2019-10-23  9:59   ` Johan Hovold
  0 siblings, 0 replies; 24+ messages in thread
From: Johan Hovold @ 2019-10-23  9:59 UTC (permalink / raw)
  To: Ji-Ze Hong (Peter Hong)
  Cc: johan, gregkh, linux-usb, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)

On Mon, Sep 23, 2019 at 10:24:46AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> The Fintek F81532A/534A/535/536 is USB-to-2/4/8/12 serial ports device
> and the serial port is default disabled when plugin computer.
> 
> The IC is contains devices as following:
> 	1. HUB (all devices is connected with this hub)
> 	2. GPIO/Control device. (enable serial port and control GPIOs)
> 	3. serial port 1 to x (2/4/8/12)
> 
> 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 | 131 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 127 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index e4db0aec9af0..36a17aedc2ae 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
> @@ -21,11 +22,36 @@
>  #include <linux/usb/serial.h>
>  #include <linux/serial_reg.h>
>  
> +#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 */
> +
>  static const struct usb_device_id id_table[] = {

Add a prefix here as well?

> -	{ USB_DEVICE(0x1934, 0x0706) },
> +	F81232_ID,
> +	{ }					/* Terminating entry */
> +};
> +
> +static const struct usb_device_id f81534a_id_table[] = {
> +	F81534A_SERIES_ID,
> +	{ }					/* Terminating entry */
> +};
> +
> +static const struct usb_device_id all_serial_id_table[] = {

combined_id_table would be a name more in line with the rest of the
drivers, if you end up using this one for the MODULE_DEVICE_TABLE.

> +	F81232_ID,
> +	F81534A_SERIES_ID,
>  	{ }					/* Terminating entry */
>  };
> -MODULE_DEVICE_TABLE(usb, id_table);
> +MODULE_DEVICE_TABLE(usb, all_serial_id_table);
>  
>  /* Maximum baudrate for F81232 */
>  #define F81232_MAX_BAUDRATE		1500000
> @@ -35,6 +61,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

This patch doesn't use any of these, and looks like you can just use the
F81232 defines directly anyway.

>  #define SERIAL_BASE_ADDRESS		0x0120
>  #define RECEIVE_BUFFER_REGISTER		(0x00 + SERIAL_BASE_ADDRESS)
> @@ -61,6 +91,11 @@ 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)

MULTPILE typo?

> +#define F81534A_FIFO_128BYTE		(BIT(1) | BIT(0))
> +
>  struct f81232_private {
>  	struct mutex lock;
>  	u8 modem_control;
> @@ -383,6 +418,46 @@ 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;
> +	unsigned char *data = urb->transfer_buffer;
> +	char tty_flag;
> +	unsigned int i;
> +	u8 lsr;
> +	u8 len;
> +
> +	if (urb->actual_length < 3) {
> +		dev_err(&port->dev, "error actual_length: %d\n",

Rephrase as "short message received" or similar.

> +				urb->actual_length);
> +		return;
> +	}
> +
> +	len = data[0];
> +	if (len != urb->actual_length) {
> +		dev_err(&port->dev, "len(%d) != urb->actual_length(%d)\n", len,
> +				urb->actual_length);

Avoid c-expressions in error messages, rephrase this in English (e.g.
unexpected length or similar).

> +		return;
> +	}
> +
> +	/* bulk-in data: [LEN][Data.....][LSR] */
> +	lsr = data[len - 1];
> +	tty_flag = f81232_handle_lsr(port, lsr);
> +
> +	if (port->port.console && port->sysrq) {
> +		for (i = 1; i < urb->actual_length - 1; ++i)

Use len here?

And please add brackets here since the body is a multi-line statement.

> +			if (!usb_serial_handle_sysrq_char(port, data[i]))

Maybe also here.

> +				tty_insert_flip_char(&port->port, data[i],
> +						tty_flag);
> +	} else {
> +		tty_insert_flip_string_fixed_flag(&port->port, &data[1],
> +							tty_flag,
> +							urb->actual_length - 2);

len

> +	}
> +
> +	tty_flip_buffer_push(&port->port);
> +}
> +
>  static void f81232_break_ctl(struct tty_struct *tty, int break_state)
>  {
>  	struct usb_serial_port *port = tty->driver_data;
> @@ -666,6 +741,23 @@ static int f81232_open(struct tty_struct *tty, struct usb_serial_port *port)
>  	return 0;
>  }
>  
> +static int f81534a_open(struct tty_struct *tty, struct usb_serial_port *port)
> +{
> +	int status;
> +	u8 val;
> +
> +	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);

Add also a mask temporary if that can avoid the line break?

> +	if (status) {
> +		dev_err(&port->dev, "failed to set MODE_CONF_REG: %d\n",
> +				status);
> +		return status;
> +	}
> +
> +	return f81232_open(tty, port);
> +}
> +
>  static void f81232_close(struct usb_serial_port *port)
>  {
>  	struct f81232_private *port_priv = usb_get_serial_port_data(port);
> @@ -772,6 +864,11 @@ static int f81232_port_probe(struct usb_serial_port *port)
>  	return 0;
>  }
>  
> +static int f81534a_port_probe(struct usb_serial_port *port)
> +{
> +	return f81232_port_probe(port);
> +}

Maybe wait with adding this one until you need it and use
f18232_port_probe below instead.

> +
>  static int f81232_suspend(struct usb_serial *serial, pm_message_t message)
>  {
>  	struct usb_serial_port *port = serial->port[0];
> @@ -835,14 +932,40 @@ static struct usb_serial_driver f81232_device = {
>  	.resume =		f81232_resume,
>  };
>  
> +static struct usb_serial_driver f81534a_device = {
> +	.driver = {
> +		.owner =	THIS_MODULE,
> +		.name =		"f81534a",
> +	},
> +	.id_table =		f81534a_id_table,
> +	.num_ports =		1,
> +	.open =			f81534a_open,
> +	.close =		f81232_close,
> +	.dtr_rts =		f81232_dtr_rts,
> +	.carrier_raised =	f81232_carrier_raised,
> +	.get_serial =		f81232_get_serial_info,
> +	.break_ctl =		f81232_break_ctl,
> +	.set_termios =		f81232_set_termios,
> +	.tiocmget =		f81232_tiocmget,
> +	.tiocmset =		f81232_tiocmset,
> +	.tiocmiwait =		usb_serial_generic_tiocmiwait,
> +	.tx_empty =		f81232_tx_empty,
> +	.process_read_urb =	f81534a_process_read_urb,
> +	.read_int_callback =	f81232_read_int_callback,
> +	.port_probe =		f81534a_port_probe,
> +	.suspend =		f81232_suspend,
> +	.resume =		f81232_resume,
> +};
> +
>  static struct usb_serial_driver * const serial_drivers[] = {
>  	&f81232_device,
> +	&f81534a_device,
>  	NULL,
>  };
>  
> -module_usb_serial_driver(serial_drivers, id_table);
> +module_usb_serial_driver(serial_drivers, all_serial_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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 5/7] USB: serial: f81232: Set F81534A serial port with RS232 mode
  2019-09-23  2:24 ` [PATCH V2 5/7] USB: serial: f81232: Set F81534A serial port with RS232 mode Ji-Ze Hong (Peter Hong)
@ 2019-10-23 11:53   ` Johan Hovold
  2019-10-24  8:52     ` Ji-Ze Hong (Peter Hong)
  0 siblings, 1 reply; 24+ messages in thread
From: Johan Hovold @ 2019-10-23 11:53 UTC (permalink / raw)
  To: Ji-Ze Hong (Peter Hong)
  Cc: johan, gregkh, linux-usb, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)

On Mon, Sep 23, 2019 at 10:24:47AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> The Fintek F81532A/534A/535/536 is USB-to-2/4/8/12 serial ports device
> and the serial ports are default disabled. Each port contains max 3 pins
> GPIO and the 3 pins are default pull high with input mode.
> 
> When the serial port had activated (running probe()), we'll transform the
> 3 pins from GPIO function publicly to control Tranceiver privately use.

I'm not sure I understand what you're saying here.

> We'll default set to 0/0/1 for control transceiver to RS232 mode.
> 
> Otherwise, If the serial port is not active, the 3 pins is in GPIO mode
> and controlled by global GPIO device with VID/PID: 2c42/16f8.

Does this mean that you can control the three GPIOs either through the
serial device or through the gpio-control device (which are two separate
USB devices)?

> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> ---
>  drivers/usb/serial/f81232.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index 36a17aedc2ae..01cb5a5ea1d2 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -96,6 +96,15 @@ MODULE_DEVICE_TABLE(usb, all_serial_id_table);
>  #define F81534A_TRIGGER_MULTPILE_4X	BIT(3)
>  #define F81534A_FIFO_128BYTE		(BIT(1) | BIT(0))
>  
> +/* 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;
> @@ -866,6 +875,14 @@ static int f81232_port_probe(struct usb_serial_port *port)
>  
>  static int f81534a_port_probe(struct usb_serial_port *port)
>  {
> +	int status;
> +
> +	/* tri-state with pull-high, default RS232 Mode */
> +	status = f81232_set_register(port, F81534A_GPIO_REG,
> +					F81534A_GPIO_MODE2_DIR);
> +	if (status)
> +		return status;

Ok, so you reset the tranceiver config on every probe.

Are the three GPIOs always connected to one particular tranceiver, or
are they truly general purpose?

In the latter case, it doesn't seem like a good idea to drive pins 0
and 1 low here as you have know idea what they're used for.

> +
>  	return f81232_port_probe(port);
>  }

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 6/7] USB: serial: f81232: Add generator for F81534A
  2019-09-23  2:24 ` [PATCH V2 6/7] USB: serial: f81232: Add generator for F81534A Ji-Ze Hong (Peter Hong)
@ 2019-10-23 12:05   ` Johan Hovold
  2019-10-23 12:25   ` Johan Hovold
  1 sibling, 0 replies; 24+ messages in thread
From: Johan Hovold @ 2019-10-23 12:05 UTC (permalink / raw)
  To: Ji-Ze Hong (Peter Hong)
  Cc: johan, gregkh, linux-usb, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)

On Mon, Sep 23, 2019 at 10:24:48AM +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 and we
> need write 0x8fff to GPIO device register F81534A_CMD_ENABLE_PORT(116h)
> to enable all available serial ports.
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> ---
>  drivers/usb/serial/f81232.c | 135 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 134 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index 01cb5a5ea1d2..82cc1e6cff62 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -36,6 +36,9 @@
>  	{ 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 */
> @@ -46,6 +49,11 @@ static const struct usb_device_id f81534a_id_table[] = {
>  	{ }					/* 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,
> @@ -105,6 +113,8 @@ MODULE_DEVICE_TABLE(usb, all_serial_id_table);
>  #define F81534A_GPIO_MODE1_OUTPUT	BIT(1)
>  #define F81534A_GPIO_MODE0_OUTPUT	BIT(0)
>  
> +#define F81534A_CMD_ENABLE_PORT		0x116

Maybe use a distinct prefix for requests to the control device (e.g.
F81534A_CTRL_).

> +
>  struct f81232_private {
>  	struct mutex lock;
>  	u8 modem_control;
> @@ -853,6 +863,95 @@ static void f81232_lsr_worker(struct work_struct *work)
>  		dev_warn(&port->dev, "read LSR failed: %d\n", 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);

Use kmemdup().

> +
> +	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;

This will retry on short transfers, is that what you intended?

> +
> +			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_enable_all_ports(struct usb_interface *intf)
> +{
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	unsigned char enable[2];
> +	int status;
> +
> +	/* enable all available serial ports */
> +	enable[0] = 0xff;
> +	enable[1] = 0x8f;

What are these magic constants?

> +
> +	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;
> +}
> +
> +static int f81534a_ctrl_probe(struct usb_interface *intf,
> +				const struct usb_device_id *id)
> +{
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	int status;
> +
> +	status = f81534a_ctrl_enable_all_ports(intf);
> +	if (status)
> +		return status;
> +
> +	dev = usb_get_dev(dev);

This isn't strictly needed.

> +	return 0;
> +}

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 7/7] USB: serial: f81232: Add gpiolib to GPIO device
  2019-09-23  2:24 ` [PATCH V2 7/7] USB: serial: f81232: Add gpiolib to GPIO device Ji-Ze Hong (Peter Hong)
@ 2019-10-23 12:22   ` Johan Hovold
  2019-10-30  2:00     ` Ji-Ze Hong (Peter Hong)
  0 siblings, 1 reply; 24+ messages in thread
From: Johan Hovold @ 2019-10-23 12:22 UTC (permalink / raw)
  To: Ji-Ze Hong (Peter Hong)
  Cc: johan, gregkh, linux-usb, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)

On Mon, Sep 23, 2019 at 10:24:49AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> The Fintek F81534A series contains 3 GPIOs per UART and The max GPIOs
> is 12x3 = 36 GPIOs and this patch will implements GPIO device as a
> gpiochip to control all GPIO pins even transforms to transceiver pins.

Depending to your answer to my question whether these pins are truly
general purpose or not, this may not be the right interface.

> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> ---
>  drivers/usb/serial/f81232.c | 249 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 249 insertions(+)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index 82cc1e6cff62..dc9b28738b80 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>
> @@ -104,6 +105,8 @@ MODULE_DEVICE_TABLE(usb, all_serial_id_table);
>  #define F81534A_TRIGGER_MULTPILE_4X	BIT(3)
>  #define F81534A_FIFO_128BYTE		(BIT(1) | BIT(0))
>  
> +#define F81534A_MAX_PORT		12
> +
>  /* 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 */
> @@ -115,6 +118,13 @@ MODULE_DEVICE_TABLE(usb, all_serial_id_table);
>  
>  #define F81534A_CMD_ENABLE_PORT		0x116
>  
> +/*
> + * 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;
> @@ -126,6 +136,12 @@ struct f81232_private {
>  	struct usb_serial_port *port;
>  };
>  
> +struct f81534a_ctrl_private {
> +	struct usb_interface *intf;
> +	struct gpio_chip chip;
> +	struct mutex lock;
> +};
> +
>  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 };
> @@ -863,6 +879,50 @@ 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)

Looks like this one needs to go under CONFIG_GPIOLIB as well.

> +{
> +	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)
>  {
> @@ -908,6 +968,182 @@ static int f81534a_ctrl_set_register(struct usb_device *dev, u16 reg, u16 size,
>  	return status;
>  }
>  
> +#ifdef CONFIG_GPIOLIB
> +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;
> +}
> +
> +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;
> +	int reg;
> +
> +	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
> +	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
> +	reg = F81534A_CTRL_GPIO_REG + set;
> +
> +	mutex_lock(&priv->lock);
> +
> +	status = f81534a_ctrl_get_register(dev, reg, 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;
> +	int reg;
> +	u8 mask;
> +
> +	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
> +	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
> +	mask = F81534A_GPIO_MODE0_DIR << idx;
> +	reg = F81534A_CTRL_GPIO_REG + set;
> +
> +	mutex_lock(&priv->lock);
> +	status = f81534a_ctrl_set_mask_register(dev, reg, mask, mask);
> +	mutex_unlock(&priv->lock);
> +
> +	return status;
> +}
> +
> +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;
> +	int reg;
> +	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);
> +	reg = F81534A_CTRL_GPIO_REG + set;
> +	data = val ? BIT(idx) : 0;
> +
> +	mutex_lock(&priv->lock);
> +	status = f81534a_ctrl_set_mask_register(dev, reg, mask, data);
> +	mutex_unlock(&priv->lock);
> +
> +	return status;
> +}
> +
> +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;
> +	int reg;
> +	u8 mask;
> +
> +	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
> +	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
> +	mask = F81534A_GPIO_MODE0_DIR << idx;
> +	reg = F81534A_CTRL_GPIO_REG + set;
> +
> +	mutex_lock(&priv->lock);
> +
> +	status = f81534a_ctrl_get_register(dev, reg, 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)

Could you rename this f81534a_gpiochip_register or similar?

> +{
> +	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";
> +	priv->chip.can_sleep = true;
> +	/* M0(SD)/M1/M2 */
> +	priv->chip.ngpio = F81534A_CTRL_GPIO_MAX_PIN * F81534A_MAX_PORT;
> +	priv->chip.base = -1;
> +
> +	priv->intf = intf;

Maybe store the parent struct usb_device instead since that's what you
end up using?

> +	mutex_init(&priv->lock);

Already initialised in probe() below. Same with priv->intf by the way.

> +
> +	status = devm_gpiochip_add_data(&intf->dev, &priv->chip, priv);
> +	if (status) {
> +		dev_err(&intf->dev, "failed to register gpiochip: %d\n",
> +				status);
> +		return status;
> +	}

Have you tried disconnecting the device with gpios requested? This used
to break gpiolib, but was fixed. Just want to make sure it hasn't
regressed.

> +
> +	return 0;
> +}
> +#else
> +static int f81534a_prepare_gpio(struct usb_interface *intf)
> +{
> +	return 0;
> +}
> +#endif
> +
>  static int f81534a_ctrl_enable_all_ports(struct usb_interface *intf)
>  {
>  	struct usb_device *dev = interface_to_usbdev(intf);
> @@ -930,8 +1166,21 @@ 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;
>  	int status;
>  
> +	priv = devm_kzalloc(&intf->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	mutex_init(&priv->lock);
> +	usb_set_intfdata(intf, priv);
> +	priv->intf = intf;
> +
> +	status = f81534a_prepare_gpio(intf);
> +	if (status)
> +		return status;
> +
>  	status = f81534a_ctrl_enable_all_ports(intf);
>  	if (status)
>  		return status;

Overall this all look much better now.

Thanks,
Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 6/7] USB: serial: f81232: Add generator for F81534A
  2019-09-23  2:24 ` [PATCH V2 6/7] USB: serial: f81232: Add generator for F81534A Ji-Ze Hong (Peter Hong)
  2019-10-23 12:05   ` Johan Hovold
@ 2019-10-23 12:25   ` Johan Hovold
  1 sibling, 0 replies; 24+ messages in thread
From: Johan Hovold @ 2019-10-23 12:25 UTC (permalink / raw)
  To: Ji-Ze Hong (Peter Hong)
  Cc: johan, gregkh, linux-usb, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)

On Mon, Sep 23, 2019 at 10:24:48AM +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 and we
> need write 0x8fff to GPIO device register F81534A_CMD_ENABLE_PORT(116h)
> to enable all available serial ports.
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> ---
>  drivers/usb/serial/f81232.c | 135 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 134 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index 01cb5a5ea1d2..82cc1e6cff62 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -36,6 +36,9 @@
>  	{ 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 */
> @@ -46,6 +49,11 @@ static const struct usb_device_id f81534a_id_table[] = {
>  	{ }					/* 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,

Forgot to mention: don't you want F81534A_CTRL_ID in the combined
(MODULE_DEVICE_TABLE) as well?

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 5/7] USB: serial: f81232: Set F81534A serial port with RS232 mode
  2019-10-23 11:53   ` Johan Hovold
@ 2019-10-24  8:52     ` Ji-Ze Hong (Peter Hong)
  2020-01-08 14:35       ` Johan Hovold
  0 siblings, 1 reply; 24+ messages in thread
From: Ji-Ze Hong (Peter Hong) @ 2019-10-24  8:52 UTC (permalink / raw)
  To: Johan Hovold
  Cc: gregkh, linux-usb, linux-kernel, peter_hong, Ji-Ze Hong (Peter Hong)

Hi Johan,

Johan Hovold 於 2019/10/23 下午 07:53 寫道:
> On Mon, Sep 23, 2019 at 10:24:47AM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> The Fintek F81532A/534A/535/536 is USB-to-2/4/8/12 serial ports device
>> and the serial ports are default disabled. Each port contains max 3 pins
>> GPIO and the 3 pins are default pull high with input mode.
>>
>> When the serial port had activated (running probe()), we'll transform the
>> 3 pins from GPIO function publicly to control Tranceiver privately use.
> 
> I'm not sure I understand what you're saying here.
> 
>> We'll default set to 0/0/1 for control transceiver to RS232 mode.
>>
>> Otherwise, If the serial port is not active, the 3 pins is in GPIO mode
>> and controlled by global GPIO device with VID/PID: 2c42/16f8.
> 
> Does this mean that you can control the three GPIOs either through the
> serial device or through the gpio-control device (which are two separate
> USB devices)?
> 

Yes, when 1 F81534A connect to Host, it'll report device as following.
	virtual HUB
		GPIO Device.
		serial port 1
		...
		serial port n

The link are F81534A pin-out:
	https://imgur.com/a/AZHqQ1N

So we can control F81534A series all GPIO pins via GPIO Device.
Serial ports are also control MODE0_x,  MODE1_x,  MODE2_x
(e.g. UART1 MODE0_1,  MODE1_1,  MODE2_1), but when Serial ports
is h/w disabled (DTR pull low), the mode pin will change to GPIO pin.


> Ok, so you reset the tranceiver config on every probe.
> 
> Are the three GPIOs always connected to one particular tranceiver, or
> are they truly general purpose?
> 
> In the latter case, it doesn't seem like a good idea to drive pins 0
> and 1 low here as you have know idea what they're used for.

If we want to change the mode pin to GPIO pin, it need do h/w disable.
It the serial ports are activated, the 3 pin will be mode pin and set
default 0/0/1 to RS232 mode due to this driver not implement RS422/485
currently.

Thanks
-- 
With Best Regards,
Peter Hong

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 7/7] USB: serial: f81232: Add gpiolib to GPIO device
  2019-10-23 12:22   ` Johan Hovold
@ 2019-10-30  2:00     ` Ji-Ze Hong (Peter Hong)
  2020-01-08 14:46       ` Johan Hovold
  0 siblings, 1 reply; 24+ messages in thread
From: Ji-Ze Hong (Peter Hong) @ 2019-10-30  2:00 UTC (permalink / raw)
  To: Johan Hovold
  Cc: gregkh, linux-usb, linux-kernel, peter_hong, Ji-Ze Hong (Peter Hong)

Hi Johan,

Johan Hovold 於 2019/10/23 下午 08:22 寫道:
> On Mon, Sep 23, 2019 at 10:24:49AM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> The Fintek F81534A series contains 3 GPIOs per UART and The max GPIOs
>> is 12x3 = 36 GPIOs and this patch will implements GPIO device as a
>> gpiochip to control all GPIO pins even transforms to transceiver pins.
> 
> Depending to your answer to my question whether these pins are truly
> general purpose or not, this may not be the right interface.

Our F81534A series contains F81532A/534A/535/536. For the following link
of F81534A pin-out:
	https://imgur.com/a/AZHqQ1N

We had 2 type about GPIO pins, MODEx_y & GPIOxx. All MODEx_y & GPIOxx
are GPIOs and can be controlled by GPIO device, but they had some
difference about usage.
	MODEx_y:
		1. 3 pins(x: 0/1/2) can be access by UART port y.
		2. Used to control UART's transceiver normally, but it
		   also can be configure as GPIO when UART disabled by
		   H/W (DTR strap to GND).
	GPIOxx:
		1. Access only by GPIO device.

The series patch only support RS233 mode for all serial port, So we'll
direct set all MODEx_y to (0/0/1) for our demo board for default. If
user really want to use the pin, we had provide the gpiolib with GPIO
device, but we'll recommend user to use GPIOxy first.

Is any suggest about this ? Could I maintain this for this series patch?

>> +
>> +	status = devm_gpiochip_add_data(&intf->dev, &priv->chip, priv);
>> +	if (status) {
>> +		dev_err(&intf->dev, "failed to register gpiochip: %d\n",
>> +				status);
>> +		return status;
>> +	}
> 
> Have you tried disconnecting the device with gpios requested? This used
> to break gpiolib, but was fixed. Just want to make sure it hasn't
> regressed.

I had try export GPIOs and detach the F81534A in kernel 5.0.0, it seems
no problem. Is any link about this issue for me to do more test ?

Thanks

-- 
With Best Regards,
Peter Hong

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 5/7] USB: serial: f81232: Set F81534A serial port with RS232 mode
  2019-10-24  8:52     ` Ji-Ze Hong (Peter Hong)
@ 2020-01-08 14:35       ` Johan Hovold
       [not found]         ` <3c79f786-de34-550e-3964-d7fb334f6d56@gmail.com>
  0 siblings, 1 reply; 24+ messages in thread
From: Johan Hovold @ 2020-01-08 14:35 UTC (permalink / raw)
  To: Ji-Ze Hong (Peter Hong)
  Cc: Johan Hovold, gregkh, linux-usb, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)

Hi Peter,

Sorry about the late reply. Had a change to look into this again today.

On Thu, Oct 24, 2019 at 04:52:01PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Johan,
> 
> Johan Hovold 於 2019/10/23 下午 07:53 寫道:
> > On Mon, Sep 23, 2019 at 10:24:47AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> >> The Fintek F81532A/534A/535/536 is USB-to-2/4/8/12 serial ports device
> >> and the serial ports are default disabled. Each port contains max 3 pins
> >> GPIO and the 3 pins are default pull high with input mode.
> >>
> >> When the serial port had activated (running probe()), we'll transform the
> >> 3 pins from GPIO function publicly to control Tranceiver privately use.
> > 
> > I'm not sure I understand what you're saying here.
> > 
> >> We'll default set to 0/0/1 for control transceiver to RS232 mode.
> >>
> >> Otherwise, If the serial port is not active, the 3 pins is in GPIO mode
> >> and controlled by global GPIO device with VID/PID: 2c42/16f8.
> > 
> > Does this mean that you can control the three GPIOs either through the
> > serial device or through the gpio-control device (which are two separate
> > USB devices)?
> 
> Yes, when 1 F81534A connect to Host, it'll report device as following.
> 	virtual HUB
> 		GPIO Device.
> 		serial port 1
> 		...
> 		serial port n

Could you post lsusb -v output for this with a couple of UARTs enabled?

> The link are F81534A pin-out:
> 	https://imgur.com/a/AZHqQ1N

Do you have a datasheet for the device?

I think I'm starting to get an idea of how this work, but I really don't
like having to spend this much time on detective work just to understand
how the hw works.

> So we can control F81534A series all GPIO pins via GPIO Device.
> Serial ports are also control MODE0_x,  MODE1_x,  MODE2_x
> (e.g. UART1 MODE0_1,  MODE1_1,  MODE2_1), but when Serial ports
> is h/w disabled (DTR pull low), the mode pin will change to GPIO pin.

So you tie a ports DTR pin, even though it's normally an output, and use
that at boot to determine whether the UART should be enabled or not?

And the GPIO device can only control a pin if the corresponding port is
disabled?

Can you read back the enable state of each port?

> > Ok, so you reset the tranceiver config on every probe.
> > 
> > Are the three GPIOs always connected to one particular tranceiver, or
> > are they truly general purpose?
> > 
> > In the latter case, it doesn't seem like a good idea to drive pins 0
> > and 1 low here as you have know idea what they're used for.
> 
> If we want to change the mode pin to GPIO pin, it need do h/w disable.
> It the serial ports are activated, the 3 pin will be mode pin and set
> default 0/0/1 to RS232 mode due to this driver not implement RS422/485
> currently.

What about devices using a different tranceiver? Should the state of the
mode pins ultimately be tied to VID/PID (can your customers change
VID/PID)?

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 7/7] USB: serial: f81232: Add gpiolib to GPIO device
  2019-10-30  2:00     ` Ji-Ze Hong (Peter Hong)
@ 2020-01-08 14:46       ` Johan Hovold
  2020-01-09  2:43         ` Ji-Ze Hong (Peter Hong)
  0 siblings, 1 reply; 24+ messages in thread
From: Johan Hovold @ 2020-01-08 14:46 UTC (permalink / raw)
  To: Ji-Ze Hong (Peter Hong)
  Cc: Johan Hovold, gregkh, linux-usb, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)

On Wed, Oct 30, 2019 at 10:00:12AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Johan,
> 
> Johan Hovold 於 2019/10/23 下午 08:22 寫道:
> > On Mon, Sep 23, 2019 at 10:24:49AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> >> The Fintek F81534A series contains 3 GPIOs per UART and The max GPIOs
> >> is 12x3 = 36 GPIOs and this patch will implements GPIO device as a
> >> gpiochip to control all GPIO pins even transforms to transceiver pins.
> > 
> > Depending to your answer to my question whether these pins are truly
> > general purpose or not, this may not be the right interface.
> 
> Our F81534A series contains F81532A/534A/535/536. For the following link
> of F81534A pin-out:
> 	https://imgur.com/a/AZHqQ1N
> 
> We had 2 type about GPIO pins, MODEx_y & GPIOxx. All MODEx_y & GPIOxx
> are GPIOs and can be controlled by GPIO device, but they had some
> difference about usage.
> 	MODEx_y:
> 		1. 3 pins(x: 0/1/2) can be access by UART port y.
> 		2. Used to control UART's transceiver normally, but it
> 		   also can be configure as GPIO when UART disabled by
> 		   H/W (DTR strap to GND).
> 	GPIOxx:
> 		1. Access only by GPIO device.
> 
> The series patch only support RS233 mode for all serial port, So we'll
> direct set all MODEx_y to (0/0/1) for our demo board for default. If
> user really want to use the pin, we had provide the gpiolib with GPIO
> device, but we'll recommend user to use GPIOxy first.

Do you mean that you'd need to register a separate gpio chip per port in
order to expose an interface for changing the MODEx_y pins for an
enabled UART? Or can you do that through the "global" gpio device?

> Is any suggest about this ? Could I maintain this for this series patch?

I understood from your other mail that the gpio device would not be able
to control the pins of an enabled port. In either case, I think you need
to refuse a request for a pin that's already in use by the corresponding
port.

Also is there a way to determine the number of available pins by
detecting the chip/package type? I'm assuming not all 36 pins are always
accessible?

> >> +	status = devm_gpiochip_add_data(&intf->dev, &priv->chip, priv);
> >> +	if (status) {
> >> +		dev_err(&intf->dev, "failed to register gpiochip: %d\n",
> >> +				status);
> >> +		return status;
> >> +	}
> > 
> > Have you tried disconnecting the device with gpios requested? This used
> > to break gpiolib, but was fixed. Just want to make sure it hasn't
> > regressed.
> 
> I had try export GPIOs and detach the F81534A in kernel 5.0.0, it seems
> no problem. Is any link about this issue for me to do more test ?

Then it's hopefully fine. Thanks for verifying.

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 7/7] USB: serial: f81232: Add gpiolib to GPIO device
  2020-01-08 14:46       ` Johan Hovold
@ 2020-01-09  2:43         ` Ji-Ze Hong (Peter Hong)
  2020-01-13 15:24           ` Johan Hovold
  0 siblings, 1 reply; 24+ messages in thread
From: Ji-Ze Hong (Peter Hong) @ 2020-01-09  2:43 UTC (permalink / raw)
  To: Johan Hovold
  Cc: gregkh, linux-usb, linux-kernel, peter_hong, Ji-Ze Hong (Peter Hong)

Hi Johan,

Johan Hovold 於 2020/1/8 下午 10:46 寫道:
> On Wed, Oct 30, 2019 at 10:00:12AM +0800, Ji-Ze Hong (Peter Hong) wrote:

>> We had 2 type about GPIO pins, MODEx_y & GPIOxx. All MODEx_y & GPIOxx
>> are GPIOs and can be controlled by GPIO device, but they had some
>> difference about usage.
>> 	MODEx_y:
>> 		1. 3 pins(x: 0/1/2) can be access by UART port y.
>> 		2. Used to control UART's transceiver normally, but it
>> 		   also can be configure as GPIO when UART disabled by
>> 		   H/W (DTR strap to GND).
>> 	GPIOxx:
>> 		1. Access only by GPIO device.
>>
>> The series patch only support RS233 mode for all serial port, So we'll
>> direct set all MODEx_y to (0/0/1) for our demo board for default. If
>> user really want to use the pin, we had provide the gpiolib with GPIO
>> device, but we'll recommend user to use GPIOxy first.
> 
> Do you mean that you'd need to register a separate gpio chip per port in
> order to expose an interface for changing the MODEx_y pins for an
> enabled UART? Or can you do that through the "global" gpio device?

No, I still implement the gpiolib in GPIO Device. Sorry for bad
describe.

>> Is any suggest about this ? Could I maintain this for this series patch?
> 
> I understood from your other mail that the gpio device would not be able
> to control the pins of an enabled port. In either case, I think you need
> to refuse a request for a pin that's already in use by the corresponding
> port.

OK, I'll change the code as previous mail as following:

I can read the UART enable state from GPIO Device, so I can do when the
GPIO is associated with UART enabled, change it as output only otherwise
can be set to input/output.

> Also is there a way to determine the number of available pins by
> detecting the chip/package type? I'm assuming not all 36 pins are always
> accessible?

Yes, we had register to get package type, I'll add UART enable & package
type to determinate final GPIO pin out.

-- 
With Best Regards,
Peter Hong

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 5/7] USB: serial: f81232: Set F81534A serial port with RS232 mode
       [not found]         ` <3c79f786-de34-550e-3964-d7fb334f6d56@gmail.com>
@ 2020-01-13 15:17           ` Johan Hovold
  2020-01-14  1:08             ` Ji-Ze Hong (Peter Hong)
  0 siblings, 1 reply; 24+ messages in thread
From: Johan Hovold @ 2020-01-13 15:17 UTC (permalink / raw)
  To: Ji-Ze Hong (Peter Hong)
  Cc: Johan Hovold, gregkh, linux-usb, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)

On Thu, Jan 09, 2020 at 10:37:09AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Johan,
> 
> Johan Hovold 於 2020/1/8 下午 10:35 寫道:
> >> Yes, when 1 F81534A connect to Host, it'll report device as following.
> >> 	virtual HUB
> >> 		GPIO Device.
> >> 		serial port 1
> >> 		...
> >> 		serial port n
> > 
> > Could you post lsusb -v output for this with a couple of UARTs enabled?
> 
> The following lsusb log is F81536 informations
> 2c42:1608 => HUB
> 2c42:16f8 => GPIO device
> 2c42:163x => UART (need driver enable)

> *after insmod driver and wait for complete

> Bus 001 Device 053: ID 2c42:1636
> Bus 001 Device 044: ID 2c42:16f8
> Bus 001 Device 043: ID 2c42:1608

Can you post verbose ("lsusb -v") output for the three device types for
completeness?

> >> The link are F81534A pin-out:
> >> 	https://imgur.com/a/AZHqQ1N
> > 
> > Do you have a datasheet for the device?
> > 
> > I think I'm starting to get an idea of how this work, but I really don't
> > like having to spend this much time on detective work just to understand
> > how the hw works.
> 
> The following link is F81536 spec:
> https://drive.google.com/drive/folders/1oA8DvpevFXoTLCDfPZHzaBWKr32ch5xc?usp=sharing

Thanks!

> >> So we can control F81534A series all GPIO pins via GPIO Device.
> >> Serial ports are also control MODE0_x,  MODE1_x,  MODE2_x
> >> (e.g. UART1 MODE0_1,  MODE1_1,  MODE2_1), but when Serial ports
> >> is h/w disabled (DTR pull low), the mode pin will change to GPIO pin.
> > 
> > So you tie a ports DTR pin, even though it's normally an output, and use
> > that at boot to determine whether the UART should be enabled or not?
> > 
> > And the GPIO device can only control a pin if the corresponding port is
> > disabled?
> > 
> > Can you read back the enable state of each port?
> 
> DTR pin of the F81534A series are strap pin on power on, when IC detect
> it pulled to low, the UART device can't enable and DTR change to input
> mode.

Can you read back the state of the DTR pin when pulled low? So that you
can determine which UARTs have been disabled in hardware?

> I can read the UART enable state from GPIO Device, so I can do when the
> GPIO is associated with UART enabled, change it as output only otherwise
> can be set to input/output. Is this OK ??

I'm leaning more towards not exporting this as a gpio device at all.
It's clear from the datasheet (and your implementation) that these pins
are supposed to be used with your own tranceivers. You can start with
only supporting RS232, and then we can discuss an interface for
switching modes later.

> > What about devices using a different tranceiver? Should the state of the
> > mode pins ultimately be tied to VID/PID (can your customers change
> > VID/PID)?
> > 
> 
> Our device VID/PID is changeable, but we assume don't change it.

Ok, then I guess we must assume that the MODE pins are only connected to
your tranceivers and hardcoding the various configurations is fine.

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 7/7] USB: serial: f81232: Add gpiolib to GPIO device
  2020-01-09  2:43         ` Ji-Ze Hong (Peter Hong)
@ 2020-01-13 15:24           ` Johan Hovold
  0 siblings, 0 replies; 24+ messages in thread
From: Johan Hovold @ 2020-01-13 15:24 UTC (permalink / raw)
  To: Ji-Ze Hong (Peter Hong)
  Cc: Johan Hovold, gregkh, linux-usb, linux-kernel, peter_hong,
	Ji-Ze Hong (Peter Hong)

On Thu, Jan 09, 2020 at 10:43:48AM +0800, Ji-Ze Hong (Peter Hong) wrote:

> Johan Hovold 於 2020/1/8 下午 10:46 寫道:

> > I understood from your other mail that the gpio device would not be able
> > to control the pins of an enabled port. In either case, I think you need
> > to refuse a request for a pin that's already in use by the corresponding
> > port.
> 
> OK, I'll change the code as previous mail as following:
> 
> I can read the UART enable state from GPIO Device, so I can do when the
> GPIO is associated with UART enabled, change it as output only otherwise
> can be set to input/output.
> 
> > Also is there a way to determine the number of available pins by
> > detecting the chip/package type? I'm assuming not all 36 pins are always
> > accessible?
> 
> Yes, we had register to get package type, I'll add UART enable & package
> type to determinate final GPIO pin out.

I suggest you start without any gpiochip, just add a simple control
driver which enables each UART (only the ones available in the package
and which have not been hardware disabled perhaps).

We don't want a user to be able to change the tranceiver mode behind the
serial driver's back so to speak.

Exposing GPIO pins (not MODE pins) in packages which have those enabled
should be fine, but you can add that later.

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH V2 5/7] USB: serial: f81232: Set F81534A serial port with RS232 mode
  2020-01-13 15:17           ` Johan Hovold
@ 2020-01-14  1:08             ` Ji-Ze Hong (Peter Hong)
  0 siblings, 0 replies; 24+ messages in thread
From: Ji-Ze Hong (Peter Hong) @ 2020-01-14  1:08 UTC (permalink / raw)
  To: Johan Hovold
  Cc: gregkh, linux-usb, linux-kernel, peter_hong, Ji-Ze Hong (Peter Hong)

Hi Johan,

Johan Hovold 於 2020/1/13 下午 11:17 寫道:
> On Thu, Jan 09, 2020 at 10:37:09AM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> Bus 001 Device 053: ID 2c42:1636
>> Bus 001 Device 044: ID 2c42:16f8
>> Bus 001 Device 043: ID 2c42:1608
> 
> Can you post verbose ("lsusb -v") output for the three device types for
> completeness?

Bus 001 Device 007: ID 2c42:1608
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            9 Hub
   bDeviceSubClass         0 Unused
   bDeviceProtocol         1 Single TT
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x1608
   bcdDevice           80.ff
   iManufacturer           0
   iProduct                1 FINTEK_USB2.0 HUB
   iSerial                 0
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           25
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xe0
       Self Powered
       Remote Wakeup
     MaxPower              500mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           1
       bInterfaceClass         9 Hub
       bInterfaceSubClass      0 Unused
       bInterfaceProtocol      0 Full speed (or root) hub
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0002  1x 2 bytes
         bInterval              12
Hub Descriptor:
   bLength              10
   bDescriptorType      41
   nNbrPorts            13
   wHubCharacteristic 0x00e9
     Per-port power switching
     Per-port overcurrent protection
     TT think time 32 FS bits
     Port indicators
   bPwrOn2PwrGood        8 * 2 milli seconds
   bHubContrCurrent    100 milli Ampere
   DeviceRemovable    0x03 0xc0
   PortPwrCtrlMask    0xff 0xa2
  Hub Port Status:
    Port 1: 0000.0503 highspeed power enable connect
    Port 2: 0000.0503 highspeed power enable connect
    Port 3: 0000.0503 highspeed power enable connect
    Port 4: 0000.0503 highspeed power enable connect
    Port 5: 0000.0503 highspeed power enable connect
    Port 6: 0000.0503 highspeed power enable connect
    Port 7: 0000.0503 highspeed power enable connect
    Port 8: 0000.0503 highspeed power enable connect
    Port 9: 0000.0503 highspeed power enable connect
    Port 10: 0000.0503 highspeed power enable connect
    Port 11: 0000.0503 highspeed power enable connect
    Port 12: 0000.0503 highspeed power enable connect
    Port 13: 0000.0503 highspeed power enable connect
Device Qualifier (for other device speed):
   bLength                10
   bDescriptorType         6
   bcdUSB               2.00
   bDeviceClass            9 Hub
   bDeviceSubClass         0 Unused
   bDeviceProtocol         0 Full speed (or root) hub
   bMaxPacketSize0        64
   bNumConfigurations      1
Device Status:     0x0001
   Self Powered

Bus 001 Device 008: ID 2c42:16f8
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x16f8
   bcdDevice           80.00
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3 88635600168801
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           18
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           0
       bInterfaceClass         0 (Defined at Interface level)
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              0
Device Status:     0x0000
   (Bus Powered)

Bus 001 Device 016: ID 2c42:1636
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x1636
   bcdDevice           88.00
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           39
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           3
       bInterfaceClass         0 (Defined at Interface level)
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0010  1x 16 bytes
         bInterval              10
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x82  EP 2 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0080  1x 128 bytes
         bInterval               0
Device Status:     0x0002
   (Bus Powered)
   Remote Wakeup Enabled

Bus 001 Device 015: ID 2c42:1636
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x1636
   bcdDevice           87.00
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           39
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           3
       bInterfaceClass         0 (Defined at Interface level)
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0010  1x 16 bytes
         bInterval              10
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x82  EP 2 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0080  1x 128 bytes
         bInterval               0
Device Status:     0x0002
   (Bus Powered)
   Remote Wakeup Enabled

Bus 001 Device 014: ID 2c42:1636
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x1636
   bcdDevice           86.00
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           39
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           3
       bInterfaceClass         0 (Defined at Interface level)
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0010  1x 16 bytes
         bInterval              10
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x82  EP 2 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0080  1x 128 bytes
         bInterval               0
Device Status:     0x0002
   (Bus Powered)
   Remote Wakeup Enabled

Bus 001 Device 013: ID 2c42:1636
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x1636
   bcdDevice           85.00
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           39
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           3
       bInterfaceClass         0 (Defined at Interface level)
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0010  1x 16 bytes
         bInterval              10
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x82  EP 2 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0080  1x 128 bytes
         bInterval               0
Device Status:     0x0002
   (Bus Powered)
   Remote Wakeup Enabled

Bus 001 Device 012: ID 2c42:1636
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x1636
   bcdDevice           84.00
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           39
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           3
       bInterfaceClass         0 (Defined at Interface level)
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0010  1x 16 bytes
         bInterval              10
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x82  EP 2 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0080  1x 128 bytes
         bInterval               0
Device Status:     0x0002
   (Bus Powered)
   Remote Wakeup Enabled

Bus 001 Device 011: ID 2c42:1636
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x1636
   bcdDevice           83.00
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           39
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           3
       bInterfaceClass         0 (Defined at Interface level)
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0010  1x 16 bytes
         bInterval              10
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x82  EP 2 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0080  1x 128 bytes
         bInterval               0
Device Status:     0x0002
   (Bus Powered)
   Remote Wakeup Enabled

Bus 001 Device 010: ID 2c42:1636
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x1636
   bcdDevice           82.00
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           39
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           3
       bInterfaceClass         0 (Defined at Interface level)
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0010  1x 16 bytes
         bInterval              10
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x82  EP 2 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0080  1x 128 bytes
         bInterval               0
Device Status:     0x0002
   (Bus Powered)
   Remote Wakeup Enabled

Bus 001 Device 009: ID 2c42:1636
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x1636
   bcdDevice           81.00
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           39
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           3
       bInterfaceClass         0 (Defined at Interface level)
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0010  1x 16 bytes
         bInterval              10
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x82  EP 2 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0080  1x 128 bytes
         bInterval               0
Device Status:     0x0002
   (Bus Powered)
   Remote Wakeup Enabled

Bus 001 Device 020: ID 2c42:1636
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x1636
   bcdDevice           8c.00
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           39
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           3
       bInterfaceClass         0 (Defined at Interface level)
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0010  1x 16 bytes
         bInterval              10
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x82  EP 2 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0080  1x 128 bytes
         bInterval               0
Device Status:     0x0002
   (Bus Powered)
   Remote Wakeup Enabled

Bus 001 Device 019: ID 2c42:1636
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x1636
   bcdDevice           8b.00
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           39
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           3
       bInterfaceClass         0 (Defined at Interface level)
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0010  1x 16 bytes
         bInterval              10
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x82  EP 2 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0080  1x 128 bytes
         bInterval               0
Device Status:     0x0002
   (Bus Powered)
   Remote Wakeup Enabled

Bus 001 Device 018: ID 2c42:1636
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x1636
   bcdDevice           8a.00
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           39
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           3
       bInterfaceClass         0 (Defined at Interface level)
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0010  1x 16 bytes
         bInterval              10
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x82  EP 2 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0080  1x 128 bytes
         bInterval               0
Device Status:     0x0002
   (Bus Powered)
   Remote Wakeup Enabled

Bus 001 Device 017: ID 2c42:1636
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        64
   idVendor           0x2c42
   idProduct          0x1636
   bcdDevice           89.00
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           39
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower              100mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           3
       bInterfaceClass         0 (Defined at Interface level)
       bInterfaceSubClass      0
       bInterfaceProtocol      0
       iInterface              0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0010  1x 16 bytes
         bInterval              10
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x82  EP 2 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0200  1x 512 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0080  1x 128 bytes
         bInterval               0
Device Status:     0x0002
   (Bus Powered)
   Remote Wakeup Enabled

>>> Can you read back the enable state of each port?
>>
>> DTR pin of the F81534A series are strap pin on power on, when IC detect
>> it pulled to low, the UART device can't enable and DTR change to input
>> mode.
> 
> Can you read back the state of the DTR pin when pulled low? So that you
> can determine which UARTs have been disabled in hardware?

If the DTR pin pulled low, the UART device always can't be enabled.
We can read the port en/disabled infomation in GPIO device only.

> 
> I'm leaning more towards not exporting this as a gpio device at all.
> It's clear from the datasheet (and your implementation) that these pins
> are supposed to be used with your own tranceivers. You can start with
> only supporting RS232, and then we can discuss an interface for
> switching modes later.

OK, I'll send patch v3 only with UART & device enable function after
clarify all issues.

>>>
>>
>> Our device VID/PID is changeable, but we assume don't change it.
> 
> Ok, then I guess we must assume that the MODE pins are only connected to
> your tranceivers and hardcoding the various configurations is fine.

Agree

-- 
With Best Regards,
Peter Hong

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, back to index

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-23  2:24 [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver Ji-Ze Hong (Peter Hong)
2019-09-23  2:24 ` [PATCH V2 1/7] USB: serial: f81232: Extract LSR handler Ji-Ze Hong (Peter Hong)
2019-10-23  9:23   ` Johan Hovold
2019-09-23  2:24 ` [PATCH V2 2/7] USB: serial: f81232: Add tx_empty function Ji-Ze Hong (Peter Hong)
2019-10-23  9:15   ` Johan Hovold
2019-09-23  2:24 ` [PATCH V2 3/7] USB: serial: f81232: Use devm_kzalloc Ji-Ze Hong (Peter Hong)
2019-09-23  2:24 ` [PATCH V2 4/7] USB: serial: f81232: Add F81534A support Ji-Ze Hong (Peter Hong)
2019-10-23  9:59   ` Johan Hovold
2019-09-23  2:24 ` [PATCH V2 5/7] USB: serial: f81232: Set F81534A serial port with RS232 mode Ji-Ze Hong (Peter Hong)
2019-10-23 11:53   ` Johan Hovold
2019-10-24  8:52     ` Ji-Ze Hong (Peter Hong)
2020-01-08 14:35       ` Johan Hovold
     [not found]         ` <3c79f786-de34-550e-3964-d7fb334f6d56@gmail.com>
2020-01-13 15:17           ` Johan Hovold
2020-01-14  1:08             ` Ji-Ze Hong (Peter Hong)
2019-09-23  2:24 ` [PATCH V2 6/7] USB: serial: f81232: Add generator for F81534A Ji-Ze Hong (Peter Hong)
2019-10-23 12:05   ` Johan Hovold
2019-10-23 12:25   ` Johan Hovold
2019-09-23  2:24 ` [PATCH V2 7/7] USB: serial: f81232: Add gpiolib to GPIO device Ji-Ze Hong (Peter Hong)
2019-10-23 12:22   ` Johan Hovold
2019-10-30  2:00     ` Ji-Ze Hong (Peter Hong)
2020-01-08 14:46       ` Johan Hovold
2020-01-09  2:43         ` Ji-Ze Hong (Peter Hong)
2020-01-13 15:24           ` Johan Hovold
2019-10-23  9:21 ` [PATCH V2 0/7] Add Fintek F81534A series usb-to-serial driver Johan Hovold

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git