2020-01-30 05:49:23

by Ji-Ze Hong (Peter Hong)

[permalink] [raw]
Subject: [PATCH V3 0/6] Add Fintek F81534A series usb-to-serial driver

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)
but this series patch not implement GPIO control.
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) (6):
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

drivers/usb/serial/f81232.c | 353 ++++++++++++++++++++++++++++++++----
1 file changed, 317 insertions(+), 36 deletions(-)

--
2.17.1


2020-01-30 05:49:23

by Ji-Ze Hong (Peter Hong)

[permalink] [raw]
Subject: [PATCH V3 1/6] USB: serial: f81232: Extract LSR handler

Extract LSR handler to function that can be re-used by
F81532A/534A/535/536.

Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
---
Changelog:
v3:
1. Change git comment for describe more information.

v2:
1: Separate LSR handler from original combined driver.

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

2020-01-30 05:49:29

by Ji-Ze Hong (Peter Hong)

[permalink] [raw]
Subject: [PATCH V3 6/6] USB: serial: f81232: Add generator for F81534A

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
write 0x8fff to GPIO device register F81534A_CTRL_CMD_ENABLE_PORT(116h)
to enable all available serial ports.

Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
---
Changelog:
v3:
1. Modify some define with prefix F81534A_CTRL_.
2. Use kmemdup() in f81534a_ctrl_set_register().
3. Not accpet with short transfers in f81534a_ctrl_set_register().
4. Add comment in f81534a_ctrl_enable_all_ports() to describe magic
constants.
5. Remove non-need usb_get_dev()/usb_put_dev().
6. Add F81534A_CTRL_ID in MODULE_DEVICE_TABLE().

v2:
1: Simplify the generator behavior.
2: Change multiply MODULE_DEVICE_TABLE() to 1 only.

drivers/usb/serial/f81232.c | 134 +++++++++++++++++++++++++++++++++++-
1 file changed, 133 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 21410a7f3a8b..0303f94b2521 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 f81232_id_table[] = {
F81232_ID,
{ } /* Terminating entry */
@@ -46,9 +49,15 @@ 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 combined_id_table[] = {
F81232_ID,
F81534A_SERIES_ID,
+ F81534A_CTRL_ID,
{ } /* Terminating entry */
};
MODULE_DEVICE_TABLE(usb, combined_id_table);
@@ -61,6 +70,7 @@ MODULE_DEVICE_TABLE(usb, combined_id_table);
#define F81232_REGISTER_REQUEST 0xa0
#define F81232_GET_REGISTER 0xc0
#define F81232_SET_REGISTER 0x40
+#define F81534A_ACCESS_REG_RETRY 2

#define SERIAL_BASE_ADDRESS 0x0120
#define RECEIVE_BUFFER_REGISTER (0x00 + SERIAL_BASE_ADDRESS)
@@ -92,6 +102,8 @@ MODULE_DEVICE_TABLE(usb, combined_id_table);
#define F81534A_TRIGGER_MULTIPLE_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 */
@@ -101,6 +113,9 @@ MODULE_DEVICE_TABLE(usb, combined_id_table);
#define F81534A_GPIO_MODE1_OUTPUT BIT(1)
#define F81534A_GPIO_MODE0_OUTPUT BIT(0)

+#define F81534A_CTRL_CMD_ENABLE_PORT 0x116
+
+
struct f81232_private {
struct mutex lock;
u8 modem_control;
@@ -848,6 +863,89 @@ 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 = kmemdup(val, size, GFP_KERNEL);
+ if (!tmp)
+ return -ENOMEM;
+
+ while (retry--) {
+ status = usb_control_msg(dev,
+ usb_sndctrlpipe(dev, 0),
+ F81232_REGISTER_REQUEST,
+ F81232_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, define as following:
+ * bit 15 : Reset behavior (when HUB got soft reset)
+ * 0: maintain all serial port enabled state.
+ * 1: disable all serial port.
+ * bit 0~11 : Serial port enable bit.
+ */
+ enable[0] = 0xff;
+ enable[1] = 0x8f;
+
+ status = f81534a_ctrl_set_register(dev, F81534A_CTRL_CMD_ENABLE_PORT,
+ sizeof(enable), enable);
+ if (status)
+ dev_warn(&dev->dev, "set ENABLE_PORT failed: %d\n", status);
+
+ return status;
+}
+
+static int f81534a_ctrl_probe(struct usb_interface *intf,
+ const struct usb_device_id *id)
+{
+
+ return f81534a_ctrl_enable_all_ports(intf);
+}
+
+static void f81534a_ctrl_disconnect(struct usb_interface *intf)
+{
+}
+
+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;
@@ -975,7 +1073,41 @@ static struct usb_serial_driver * const serial_drivers[] = {
NULL,
};

-module_usb_serial_driver(serial_drivers, combined_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,
+ combined_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 <[email protected]>");
--
2.17.1

2020-01-30 05:50:11

by Ji-Ze Hong (Peter Hong)

[permalink] [raw]
Subject: [PATCH V3 4/6] USB: serial: f81232: Add F81534A support

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) <[email protected]>
---
Changelog:
v3:
1. Add prefix f81232 to original id_table[].
2. Change all_serial_id_table[] to combined_id_table[].
3. Remove non-used defines.
4. Fix typos.
5. Change some error message more reable.
6. Change urb->actual_length to len in f81534a_process_read_urb().
7. Remove f81534a_port_probe() and add it to next patch.

v2:
1: Separate LSR handler to prior patch.
2: Use tty_insert_flip_string_fixed_char() when not a console.
3. Remove proxy function f81232_read_urb_proxy() and change it
to usb-serial subdriver.
4. Change some error message more reable.

drivers/usb/serial/f81232.c | 128 ++++++++++++++++++++++++++++++++++--
1 file changed, 122 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index e080d678b0c2..df197c723273 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 ([email protected])
* Copyright (C) 2012 Linux Foundation
@@ -21,11 +22,36 @@
#include <linux/usb/serial.h>
#include <linux/serial_reg.h>

-static const struct usb_device_id id_table[] = {
- { USB_DEVICE(0x1934, 0x0706) },
+#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 f81232_id_table[] = {
+ F81232_ID,
{ } /* Terminating entry */
};
-MODULE_DEVICE_TABLE(usb, id_table);
+
+static const struct usb_device_id f81534a_id_table[] = {
+ F81534A_SERIES_ID,
+ { } /* Terminating entry */
+};
+
+static const struct usb_device_id combined_id_table[] = {
+ F81232_ID,
+ F81534A_SERIES_ID,
+ { } /* Terminating entry */
+};
+MODULE_DEVICE_TABLE(usb, combined_id_table);

/* Maximum baudrate for F81232 */
#define F81232_MAX_BAUDRATE 1500000
@@ -61,6 +87,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_REG 0x107
+#define F81534A_TRIGGER_MASK GENMASK(3, 2)
+#define F81534A_TRIGGER_MULTIPLE_4X BIT(3)
+#define F81534A_FIFO_128BYTE (BIT(1) | BIT(0))
+
struct f81232_private {
struct mutex lock;
u8 modem_control;
@@ -383,6 +414,47 @@ 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, "short message received: %d\n",
+ urb->actual_length);
+ return;
+ }
+
+ len = data[0];
+ if (len != urb->actual_length) {
+ dev_err(&port->dev, "unexpected length: %d %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 < len - 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, len - 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 +738,24 @@ 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 mask;
+ u8 val;
+
+ val = F81534A_TRIGGER_MULTIPLE_4X | F81534A_FIFO_128BYTE;
+ mask = F81534A_TRIGGER_MASK | F81534A_FIFO_128BYTE;
+
+ status = f81232_set_mask_register(port, F81534A_MODE_REG, mask, val);
+ if (status) {
+ dev_err(&port->dev, "failed to set MODE_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);
@@ -810,7 +900,7 @@ static struct usb_serial_driver f81232_device = {
.owner = THIS_MODULE,
.name = "f81232",
},
- .id_table = id_table,
+ .id_table = f81232_id_table,
.num_ports = 1,
.bulk_in_size = 256,
.bulk_out_size = 256,
@@ -832,14 +922,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 = f81232_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, combined_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 <[email protected]>");
MODULE_AUTHOR("Peter Hong <[email protected]>");
MODULE_LICENSE("GPL v2");
--
2.17.1

2020-01-30 05:50:19

by Ji-Ze Hong (Peter Hong)

[permalink] [raw]
Subject: [PATCH V3 5/6] USB: serial: f81232: Set F81534A serial port with RS232 mode

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) <[email protected]>
---
Changelog:
v3:
1. Add f81534a_port_probe().

v2:
1: Add more describe in git comment message.
2: Remove non-used gpio_mode.

drivers/usb/serial/f81232.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index df197c723273..21410a7f3a8b 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -92,6 +92,15 @@ MODULE_DEVICE_TABLE(usb, combined_id_table);
#define F81534A_TRIGGER_MULTIPLE_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;
@@ -859,6 +868,19 @@ static int f81232_port_probe(struct usb_serial_port *port)
return 0;
}

+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);
+}
+
static int f81232_suspend(struct usb_serial *serial, pm_message_t message)
{
struct usb_serial_port *port = serial->port[0];
@@ -942,7 +964,7 @@ static struct usb_serial_driver f81534a_device = {
.tx_empty = f81232_tx_empty,
.process_read_urb = f81534a_process_read_urb,
.read_int_callback = f81232_read_int_callback,
- .port_probe = f81232_port_probe,
+ .port_probe = f81534a_port_probe,
.suspend = f81232_suspend,
.resume = f81232_resume,
};
--
2.17.1

2020-02-25 12:09:29

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH V3 4/6] USB: serial: f81232: Add F81534A support

On Thu, Jan 30, 2020 at 01:47:50PM +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) <[email protected]>

> +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, "short message received: %d\n",
> + urb->actual_length);
> + return;
> + }
> +
> + len = data[0];
> + if (len != urb->actual_length) {
> + dev_err(&port->dev, "unexpected length: %d %d\n", len,
> + urb->actual_length);
> + return;

I rephrased this as

dev_err(&port->dev, "malformed message received: %d (%d)\n",
urb->actual_length, len);

before applying.

Johan

2020-02-25 12:10:52

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH V3 6/6] USB: serial: f81232: Add generator for F81534A

On Thu, Jan 30, 2020 at 01:47:52PM +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
> write 0x8fff to GPIO device register F81534A_CTRL_CMD_ENABLE_PORT(116h)
> to enable all available serial ports.
>
> Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
> ---
> Changelog:
> v3:
> 1. Modify some define with prefix F81534A_CTRL_.
> 2. Use kmemdup() in f81534a_ctrl_set_register().
> 3. Not accpet with short transfers in f81534a_ctrl_set_register().
> 4. Add comment in f81534a_ctrl_enable_all_ports() to describe magic
> constants.
> 5. Remove non-need usb_get_dev()/usb_put_dev().
> 6. Add F81534A_CTRL_ID in MODULE_DEVICE_TABLE().
>
> v2:
> 1: Simplify the generator behavior.
> 2: Change multiply MODULE_DEVICE_TABLE() to 1 only.
>
> drivers/usb/serial/f81232.c | 134 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 133 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index 21410a7f3a8b..0303f94b2521 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 f81232_id_table[] = {
> F81232_ID,
> { } /* Terminating entry */
> @@ -46,9 +49,15 @@ 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 combined_id_table[] = {
> F81232_ID,
> F81534A_SERIES_ID,
> + F81534A_CTRL_ID,
> { } /* Terminating entry */
> };
> MODULE_DEVICE_TABLE(usb, combined_id_table);
> @@ -61,6 +70,7 @@ MODULE_DEVICE_TABLE(usb, combined_id_table);
> #define F81232_REGISTER_REQUEST 0xa0
> #define F81232_GET_REGISTER 0xc0
> #define F81232_SET_REGISTER 0x40
> +#define F81534A_ACCESS_REG_RETRY 2
>
> #define SERIAL_BASE_ADDRESS 0x0120
> #define RECEIVE_BUFFER_REGISTER (0x00 + SERIAL_BASE_ADDRESS)
> @@ -92,6 +102,8 @@ MODULE_DEVICE_TABLE(usb, combined_id_table);
> #define F81534A_TRIGGER_MULTIPLE_4X BIT(3)
> #define F81534A_FIFO_128BYTE (BIT(1) | BIT(0))
>
> +#define F81534A_MAX_PORT 12
> +

This define isn't used anymore.

> /* 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 */
> @@ -101,6 +113,9 @@ MODULE_DEVICE_TABLE(usb, combined_id_table);
> #define F81534A_GPIO_MODE1_OUTPUT BIT(1)
> #define F81534A_GPIO_MODE0_OUTPUT BIT(0)
>
> +#define F81534A_CTRL_CMD_ENABLE_PORT 0x116
> +
> +
> struct f81232_private {
> struct mutex lock;
> u8 modem_control;
> @@ -848,6 +863,89 @@ 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 = kmemdup(val, size, GFP_KERNEL);
> + if (!tmp)
> + return -ENOMEM;
> +
> + while (retry--) {
> + status = usb_control_msg(dev,
> + usb_sndctrlpipe(dev, 0),
> + F81232_REGISTER_REQUEST,
> + F81232_SET_REGISTER,
> + reg,
> + 0,
> + tmp,
> + size,
> + USB_CTRL_SET_TIMEOUT);
> + if (status != size) {
> + status = usb_translate_errors(status);

Please don't use usb_translate_errors() for non-errors (short
transfers). Handle it explicitly instead so that the intent is clear
here (e.g. to retry on short transfers).

> + 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, define as following:
> + * bit 15 : Reset behavior (when HUB got soft reset)
> + * 0: maintain all serial port enabled state.
> + * 1: disable all serial port.
> + * bit 0~11 : Serial port enable bit.
> + */
> + enable[0] = 0xff;
> + enable[1] = 0x8f;
> +
> + status = f81534a_ctrl_set_register(dev, F81534A_CTRL_CMD_ENABLE_PORT,
> + sizeof(enable), enable);
> + if (status)
> + dev_warn(&dev->dev, "set ENABLE_PORT failed: %d\n", status);

Use dev_err() here, and "failed to enable ports" is probably a better
wording.

> +
> + return status;
> +}
> +
> +static int f81534a_ctrl_probe(struct usb_interface *intf,
> + const struct usb_device_id *id)
> +{
> +
> + return f81534a_ctrl_enable_all_ports(intf);
> +}
> +
> +static void f81534a_ctrl_disconnect(struct usb_interface *intf)
> +{
> +}

Shouldn't you disable the ports when unbinding the control driver?

I've applied all the patches in the series up until this one so no need
to resend those.

Johan