[PATCH 1/5] USB: serial: f81232: clear overrun flag

[PATCH 1/5] USB: serial: f81232: clear overrun flag

[Ji-Ze Hong (Peter Hong)]

The F81232 will report data and LSR with bulk like following format:
bulk-in data: [LSR(1Byte)+DATA(1Byte)][LSR(1Byte)+DATA(1Byte)]...

LSR will auto clear frame/parity/break error flag when reading by H/W,
but overrrun will only cleared when reading LSR. So this patch add a
worker to read LSR when OE.

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 96036f87b1de..46836041c50e 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -41,12 +41,14 @@ MODULE_DEVICE_TABLE(usb, id_table);
 #define FIFO_CONTROL_REGISTER		(0x02 + SERIAL_BASE_ADDRESS)
 #define LINE_CONTROL_REGISTER		(0x03 + SERIAL_BASE_ADDRESS)
 #define MODEM_CONTROL_REGISTER		(0x04 + SERIAL_BASE_ADDRESS)
+#define LINE_STATUS_REGISTER		(0x05 + SERIAL_BASE_ADDRESS)
 #define MODEM_STATUS_REGISTER		(0x06 + SERIAL_BASE_ADDRESS)
 
 struct f81232_private {
 	struct mutex lock;
 	u8 modem_control;
 	u8 modem_status;
+	struct work_struct lsr_work;
 	struct work_struct interrupt_work;
 	struct usb_serial_port *port;
 };
@@ -282,6 +284,7 @@ static void f81232_read_int_callback(struct urb *urb)
 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;
@@ -315,6 +318,7 @@ static void f81232_process_read_urb(struct urb *urb)
 
 			if (lsr & UART_LSR_OE) {
 				port->icount.overrun++;
+				schedule_work(&priv->lsr_work);
 				tty_insert_flip_char(&port->port, 0,
 						TTY_OVERRUN);
 			}
@@ -623,6 +627,21 @@ static void  f81232_interrupt_work(struct work_struct *work)
 	f81232_read_msr(priv->port);
 }
 
+static void f81232_lsr_worker(struct work_struct *work)
+{
+	struct f81232_private *priv;
+	struct usb_serial_port *port;
+	int status;
+	u8 tmp;
+
+	priv = container_of(work, struct f81232_private, lsr_work);
+	port = priv->port;
+
+	status = f81232_get_register(port, LINE_STATUS_REGISTER, &tmp);
+	if (status)
+		dev_warn(&port->dev, "read LSR failed: %d\n", status);
+}
+
 static int f81232_port_probe(struct usb_serial_port *port)
 {
 	struct f81232_private *priv;
@@ -633,6 +652,7 @@ static int f81232_port_probe(struct usb_serial_port *port)
 
 	mutex_init(&priv->lock);
 	INIT_WORK(&priv->interrupt_work,  f81232_interrupt_work);
+	INIT_WORK(&priv->lsr_work, f81232_lsr_worker);
 
 	usb_set_serial_port_data(port, priv);
 
-- 
2.7.4

[1/5] USB: serial: f81232: clear overrun flag

[Ji-Ze Hong (Peter Hong)]

The F81232 will report data and LSR with bulk like following format:
bulk-in data: [LSR(1Byte)+DATA(1Byte)][LSR(1Byte)+DATA(1Byte)]...

LSR will auto clear frame/parity/break error flag when reading by H/W,
but overrrun will only cleared when reading LSR. So this patch add a
worker to read LSR when OE.

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 96036f87b1de..46836041c50e 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -41,12 +41,14 @@ MODULE_DEVICE_TABLE(usb, id_table);
 #define FIFO_CONTROL_REGISTER		(0x02 + SERIAL_BASE_ADDRESS)
 #define LINE_CONTROL_REGISTER		(0x03 + SERIAL_BASE_ADDRESS)
 #define MODEM_CONTROL_REGISTER		(0x04 + SERIAL_BASE_ADDRESS)
+#define LINE_STATUS_REGISTER		(0x05 + SERIAL_BASE_ADDRESS)
 #define MODEM_STATUS_REGISTER		(0x06 + SERIAL_BASE_ADDRESS)
 
 struct f81232_private {
 	struct mutex lock;
 	u8 modem_control;
 	u8 modem_status;
+	struct work_struct lsr_work;
 	struct work_struct interrupt_work;
 	struct usb_serial_port *port;
 };
@@ -282,6 +284,7 @@ static void f81232_read_int_callback(struct urb *urb)
 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;
@@ -315,6 +318,7 @@ static void f81232_process_read_urb(struct urb *urb)
 
 			if (lsr & UART_LSR_OE) {
 				port->icount.overrun++;
+				schedule_work(&priv->lsr_work);
 				tty_insert_flip_char(&port->port, 0,
 						TTY_OVERRUN);
 			}
@@ -623,6 +627,21 @@ static void  f81232_interrupt_work(struct work_struct *work)
 	f81232_read_msr(priv->port);
 }
 
+static void f81232_lsr_worker(struct work_struct *work)
+{
+	struct f81232_private *priv;
+	struct usb_serial_port *port;
+	int status;
+	u8 tmp;
+
+	priv = container_of(work, struct f81232_private, lsr_work);
+	port = priv->port;
+
+	status = f81232_get_register(port, LINE_STATUS_REGISTER, &tmp);
+	if (status)
+		dev_warn(&port->dev, "read LSR failed: %d\n", status);
+}
+
 static int f81232_port_probe(struct usb_serial_port *port)
 {
 	struct f81232_private *priv;
@@ -633,6 +652,7 @@ static int f81232_port_probe(struct usb_serial_port *port)
 
 	mutex_init(&priv->lock);
 	INIT_WORK(&priv->interrupt_work,  f81232_interrupt_work);
+	INIT_WORK(&priv->lsr_work, f81232_lsr_worker);
 
 	usb_set_serial_port_data(port, priv);
 

[PATCH 2/5] USB: serial: f81232: add high baud rate support

[Ji-Ze Hong (Peter Hong)]

The F81232 had 4 clocksource 1.846/18.46/14.77/24MHz and baud rates
can be up to 1.5Mbits with 24MHz.

F81232 Clock registers (106h)

Bit1-0:     Clock source selector
                    00: 1.846MHz.
                    01: 18.46MHz.
                    10: 24MHz.
                    11: 14.77MHz.

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 46836041c50e..bdd7f337cd5f 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -28,7 +28,8 @@ static const struct usb_device_id id_table[] = {
 MODULE_DEVICE_TABLE(usb, id_table);
 
 /* Maximum baudrate for F81232 */
-#define F81232_MAX_BAUDRATE		115200
+#define F81232_MAX_BAUDRATE		1500000
+#define F81232_DEF_BAUDRATE		9600
 
 /* USB Control EP parameter */
 #define F81232_REGISTER_REQUEST		0xa0
@@ -44,18 +45,42 @@ MODULE_DEVICE_TABLE(usb, id_table);
 #define LINE_STATUS_REGISTER		(0x05 + SERIAL_BASE_ADDRESS)
 #define MODEM_STATUS_REGISTER		(0x06 + SERIAL_BASE_ADDRESS)
 
+/*
+ * F81232 Clock registers (106h)
+ *
+ * Bit1-0:	Clock source selector
+ *			00: 1.846MHz.
+ *			01: 18.46MHz.
+ *			10: 24MHz.
+ *			11: 14.77MHz.
+ */
+#define F81232_CLK_REGISTER		0x106
+#define F81232_CLK_1_846_MHZ		0
+#define F81232_CLK_18_46_MHZ		BIT(0)
+#define F81232_CLK_24_MHZ		BIT(1)
+#define F81232_CLK_14_77_MHZ		(BIT(1) | BIT(0))
+#define F81232_CLK_MASK			GENMASK(1, 0)
+
 struct f81232_private {
 	struct mutex lock;
 	u8 modem_control;
 	u8 modem_status;
+	speed_t baud_base;
 	struct work_struct lsr_work;
 	struct work_struct interrupt_work;
 	struct usb_serial_port *port;
 };
 
-static int calc_baud_divisor(speed_t baudrate)
+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 };
+
+static int calc_baud_divisor(speed_t baudrate, speed_t clockrate)
 {
-	return DIV_ROUND_CLOSEST(F81232_MAX_BAUDRATE, baudrate);
+	if (!baudrate)
+		return 0;
+
+	return DIV_ROUND_CLOSEST(clockrate, baudrate);
 }
 
 static int f81232_get_register(struct usb_serial_port *port, u16 reg, u8 *val)
@@ -129,6 +154,21 @@ static int f81232_set_register(struct usb_serial_port *port, u16 reg, u8 val)
 	return status;
 }
 
+static int f81232_set_mask_register(struct usb_serial_port *port, u16 reg,
+					u8 mask, u8 val)
+{
+	int status;
+	u8 tmp;
+
+	status = f81232_get_register(port, reg, &tmp);
+	if (status)
+		return status;
+
+	tmp = (tmp & ~mask) | (val & mask);
+
+	return f81232_set_register(port, reg, tmp);
+}
+
 static void f81232_read_msr(struct usb_serial_port *port)
 {
 	int status;
@@ -346,13 +386,53 @@ static void f81232_break_ctl(struct tty_struct *tty, int break_state)
 	 */
 }
 
-static void f81232_set_baudrate(struct usb_serial_port *port, speed_t baudrate)
+static int f81232_find_clk(speed_t baudrate)
+{
+	int idx;
+
+	for (idx = 0; idx < ARRAY_SIZE(baudrate_table); ++idx) {
+		if (baudrate <= baudrate_table[idx] &&
+				baudrate_table[idx] % baudrate == 0)
+			return idx;
+	}
+
+	return -EINVAL;
+}
+
+static void f81232_set_baudrate(struct tty_struct *tty,
+				struct usb_serial_port *port, speed_t baudrate,
+				speed_t old_baudrate)
 {
+	struct f81232_private *priv = usb_get_serial_port_data(port);
 	u8 lcr;
 	int divisor;
 	int status = 0;
+	int i;
+	int idx;
+	speed_t baud_list[] = {baudrate, old_baudrate, F81232_DEF_BAUDRATE};
+
+	for (i = 0; i < ARRAY_SIZE(baud_list); ++i) {
+		idx = f81232_find_clk(baud_list[i]);
+		if (idx >= 0) {
+			baudrate = baud_list[i];
+			tty_encode_baud_rate(tty, baudrate, baudrate);
+			break;
+		}
+	}
 
-	divisor = calc_baud_divisor(baudrate);
+	if (idx < 0)
+		return;
+
+	priv->baud_base = baudrate_table[idx];
+	divisor = calc_baud_divisor(baudrate, priv->baud_base);
+
+	status = f81232_set_mask_register(port, F81232_CLK_REGISTER,
+			F81232_CLK_MASK, clock_table[idx]);
+	if (status) {
+		dev_err(&port->dev, "%s failed to set CLK_REG: %d\n",
+			__func__, status);
+		return;
+	}
 
 	status = f81232_get_register(port, LINE_CONTROL_REGISTER,
 			 &lcr); /* get LCR */
@@ -442,6 +522,7 @@ static void f81232_set_termios(struct tty_struct *tty,
 	u8 new_lcr = 0;
 	int status = 0;
 	speed_t baudrate;
+	speed_t old_baud;
 
 	/* Don't change anything if nothing has changed */
 	if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
@@ -454,11 +535,12 @@ static void f81232_set_termios(struct tty_struct *tty,
 
 	baudrate = tty_get_baud_rate(tty);
 	if (baudrate > 0) {
-		if (baudrate > F81232_MAX_BAUDRATE) {
-			baudrate = F81232_MAX_BAUDRATE;
-			tty_encode_baud_rate(tty, baudrate, baudrate);
-		}
-		f81232_set_baudrate(port, baudrate);
+		if (old_termios)
+			old_baud = tty_termios_baud_rate(old_termios);
+		else
+			old_baud = F81232_DEF_BAUDRATE;
+
+		f81232_set_baudrate(tty, port, baudrate, old_baud);
 	}
 
 	if (C_PARENB(tty)) {
@@ -590,6 +672,7 @@ static int f81232_carrier_raised(struct usb_serial_port *port)
 static int f81232_get_serial_info(struct usb_serial_port *port,
 		unsigned long arg)
 {
+	struct f81232_private *priv = usb_get_serial_port_data(port);
 	struct serial_struct ser;
 
 	memset(&ser, 0, sizeof(ser));
@@ -597,7 +680,7 @@ static int f81232_get_serial_info(struct usb_serial_port *port,
 	ser.type = PORT_16550A;
 	ser.line = port->minor;
 	ser.port = port->port_number;
-	ser.baud_base = F81232_MAX_BAUDRATE;
+	ser.baud_base = priv->baud_base;
 
 	if (copy_to_user((void __user *)arg, &ser, sizeof(ser)))
 		return -EFAULT;
-- 
2.7.4

[2/5] USB: serial: f81232: add high baud rate support

[Ji-Ze Hong (Peter Hong)]

The F81232 had 4 clocksource 1.846/18.46/14.77/24MHz and baud rates
can be up to 1.5Mbits with 24MHz.

F81232 Clock registers (106h)

Bit1-0:     Clock source selector
                    00: 1.846MHz.
                    01: 18.46MHz.
                    10: 24MHz.
                    11: 14.77MHz.

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 46836041c50e..bdd7f337cd5f 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -28,7 +28,8 @@ static const struct usb_device_id id_table[] = {
 MODULE_DEVICE_TABLE(usb, id_table);
 
 /* Maximum baudrate for F81232 */
-#define F81232_MAX_BAUDRATE		115200
+#define F81232_MAX_BAUDRATE		1500000
+#define F81232_DEF_BAUDRATE		9600
 
 /* USB Control EP parameter */
 #define F81232_REGISTER_REQUEST		0xa0
@@ -44,18 +45,42 @@ MODULE_DEVICE_TABLE(usb, id_table);
 #define LINE_STATUS_REGISTER		(0x05 + SERIAL_BASE_ADDRESS)
 #define MODEM_STATUS_REGISTER		(0x06 + SERIAL_BASE_ADDRESS)
 
+/*
+ * F81232 Clock registers (106h)
+ *
+ * Bit1-0:	Clock source selector
+ *			00: 1.846MHz.
+ *			01: 18.46MHz.
+ *			10: 24MHz.
+ *			11: 14.77MHz.
+ */
+#define F81232_CLK_REGISTER		0x106
+#define F81232_CLK_1_846_MHZ		0
+#define F81232_CLK_18_46_MHZ		BIT(0)
+#define F81232_CLK_24_MHZ		BIT(1)
+#define F81232_CLK_14_77_MHZ		(BIT(1) | BIT(0))
+#define F81232_CLK_MASK			GENMASK(1, 0)
+
 struct f81232_private {
 	struct mutex lock;
 	u8 modem_control;
 	u8 modem_status;
+	speed_t baud_base;
 	struct work_struct lsr_work;
 	struct work_struct interrupt_work;
 	struct usb_serial_port *port;
 };
 
-static int calc_baud_divisor(speed_t baudrate)
+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 };
+
+static int calc_baud_divisor(speed_t baudrate, speed_t clockrate)
 {
-	return DIV_ROUND_CLOSEST(F81232_MAX_BAUDRATE, baudrate);
+	if (!baudrate)
+		return 0;
+
+	return DIV_ROUND_CLOSEST(clockrate, baudrate);
 }
 
 static int f81232_get_register(struct usb_serial_port *port, u16 reg, u8 *val)
@@ -129,6 +154,21 @@ static int f81232_set_register(struct usb_serial_port *port, u16 reg, u8 val)
 	return status;
 }
 
+static int f81232_set_mask_register(struct usb_serial_port *port, u16 reg,
+					u8 mask, u8 val)
+{
+	int status;
+	u8 tmp;
+
+	status = f81232_get_register(port, reg, &tmp);
+	if (status)
+		return status;
+
+	tmp = (tmp & ~mask) | (val & mask);
+
+	return f81232_set_register(port, reg, tmp);
+}
+
 static void f81232_read_msr(struct usb_serial_port *port)
 {
 	int status;
@@ -346,13 +386,53 @@ static void f81232_break_ctl(struct tty_struct *tty, int break_state)
 	 */
 }
 
-static void f81232_set_baudrate(struct usb_serial_port *port, speed_t baudrate)
+static int f81232_find_clk(speed_t baudrate)
+{
+	int idx;
+
+	for (idx = 0; idx < ARRAY_SIZE(baudrate_table); ++idx) {
+		if (baudrate <= baudrate_table[idx] &&
+				baudrate_table[idx] % baudrate == 0)
+			return idx;
+	}
+
+	return -EINVAL;
+}
+
+static void f81232_set_baudrate(struct tty_struct *tty,
+				struct usb_serial_port *port, speed_t baudrate,
+				speed_t old_baudrate)
 {
+	struct f81232_private *priv = usb_get_serial_port_data(port);
 	u8 lcr;
 	int divisor;
 	int status = 0;
+	int i;
+	int idx;
+	speed_t baud_list[] = {baudrate, old_baudrate, F81232_DEF_BAUDRATE};
+
+	for (i = 0; i < ARRAY_SIZE(baud_list); ++i) {
+		idx = f81232_find_clk(baud_list[i]);
+		if (idx >= 0) {
+			baudrate = baud_list[i];
+			tty_encode_baud_rate(tty, baudrate, baudrate);
+			break;
+		}
+	}
 
-	divisor = calc_baud_divisor(baudrate);
+	if (idx < 0)
+		return;
+
+	priv->baud_base = baudrate_table[idx];
+	divisor = calc_baud_divisor(baudrate, priv->baud_base);
+
+	status = f81232_set_mask_register(port, F81232_CLK_REGISTER,
+			F81232_CLK_MASK, clock_table[idx]);
+	if (status) {
+		dev_err(&port->dev, "%s failed to set CLK_REG: %d\n",
+			__func__, status);
+		return;
+	}
 
 	status = f81232_get_register(port, LINE_CONTROL_REGISTER,
 			 &lcr); /* get LCR */
@@ -442,6 +522,7 @@ static void f81232_set_termios(struct tty_struct *tty,
 	u8 new_lcr = 0;
 	int status = 0;
 	speed_t baudrate;
+	speed_t old_baud;
 
 	/* Don't change anything if nothing has changed */
 	if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
@@ -454,11 +535,12 @@ static void f81232_set_termios(struct tty_struct *tty,
 
 	baudrate = tty_get_baud_rate(tty);
 	if (baudrate > 0) {
-		if (baudrate > F81232_MAX_BAUDRATE) {
-			baudrate = F81232_MAX_BAUDRATE;
-			tty_encode_baud_rate(tty, baudrate, baudrate);
-		}
-		f81232_set_baudrate(port, baudrate);
+		if (old_termios)
+			old_baud = tty_termios_baud_rate(old_termios);
+		else
+			old_baud = F81232_DEF_BAUDRATE;
+
+		f81232_set_baudrate(tty, port, baudrate, old_baud);
 	}
 
 	if (C_PARENB(tty)) {
@@ -590,6 +672,7 @@ static int f81232_carrier_raised(struct usb_serial_port *port)
 static int f81232_get_serial_info(struct usb_serial_port *port,
 		unsigned long arg)
 {
+	struct f81232_private *priv = usb_get_serial_port_data(port);
 	struct serial_struct ser;
 
 	memset(&ser, 0, sizeof(ser));
@@ -597,7 +680,7 @@ static int f81232_get_serial_info(struct usb_serial_port *port,
 	ser.type = PORT_16550A;
 	ser.line = port->minor;
 	ser.port = port->port_number;
-	ser.baud_base = F81232_MAX_BAUDRATE;
+	ser.baud_base = priv->baud_base;
 
 	if (copy_to_user((void __user *)arg, &ser, sizeof(ser)))
 		return -EFAULT;

[PATCH 3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Ji-Ze Hong (Peter Hong)]

The F81232 can do remote wakeup via RX/RI pin with pulse.
This patch will use device_set_wakeup_enable to enable this
feature.

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index bdd7f337cd5f..dadf024ae494 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -742,6 +742,7 @@ static int f81232_port_probe(struct usb_serial_port *port)
 	port->port.drain_delay = 256;
 	priv->port = port;
 
+	device_set_wakeup_enable(&port->serial->dev->dev, true);
 	return 0;
 }
 
@@ -752,6 +753,7 @@ static int f81232_port_remove(struct usb_serial_port *port)
 	priv = usb_get_serial_port_data(port);
 	kfree(priv);
 
+	device_set_wakeup_enable(&port->serial->dev->dev, false);
 	return 0;
 }
 
-- 
2.7.4

[3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Ji-Ze Hong (Peter Hong)]

The F81232 can do remote wakeup via RX/RI pin with pulse.
This patch will use device_set_wakeup_enable to enable this
feature.

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index bdd7f337cd5f..dadf024ae494 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -742,6 +742,7 @@ static int f81232_port_probe(struct usb_serial_port *port)
 	port->port.drain_delay = 256;
 	priv->port = port;
 
+	device_set_wakeup_enable(&port->serial->dev->dev, true);
 	return 0;
 }
 
@@ -752,6 +753,7 @@ static int f81232_port_remove(struct usb_serial_port *port)
 	priv = usb_get_serial_port_data(port);
 	kfree(priv);
 
+	device_set_wakeup_enable(&port->serial->dev->dev, false);
 	return 0;
 }
 

[PATCH 4/5] USB: serial: f81232: implement break control

[Ji-Ze Hong (Peter Hong)]

Implement Fintek F81232 break on/off with LCR register.
It's the same with 16550A LCR register layout.

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index dadf024ae494..a054f69446fd 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -377,13 +377,18 @@ static void f81232_process_read_urb(struct urb *urb)
 
 static void f81232_break_ctl(struct tty_struct *tty, int break_state)
 {
-	/* FIXME - Stubbed out for now */
+	struct usb_serial_port *port = tty->driver_data;
+	struct f81232_private *priv = usb_get_serial_port_data(port);
+	int status;
 
-	/*
-	 * break_state = -1 to turn on break, and 0 to turn off break
-	 * see drivers/char/tty_io.c to see it used.
-	 * last_set_data_urb_value NEVER has the break bit set in it.
-	 */
+	mutex_lock(&priv->lock);
+
+	status = f81232_set_mask_register(port, LINE_CONTROL_REGISTER,
+						UART_LCR_SBC, !!break_state);
+	if (status)
+		dev_err(&port->dev, "set break failed: %d\n", status);
+
+	mutex_unlock(&priv->lock);
 }
 
 static int f81232_find_clk(speed_t baudrate)
-- 
2.7.4

[4/5] USB: serial: f81232: implement break control

[Ji-Ze Hong (Peter Hong)]

Implement Fintek F81232 break on/off with LCR register.
It's the same with 16550A LCR register layout.

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index dadf024ae494..a054f69446fd 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -377,13 +377,18 @@ static void f81232_process_read_urb(struct urb *urb)
 
 static void f81232_break_ctl(struct tty_struct *tty, int break_state)
 {
-	/* FIXME - Stubbed out for now */
+	struct usb_serial_port *port = tty->driver_data;
+	struct f81232_private *priv = usb_get_serial_port_data(port);
+	int status;
 
-	/*
-	 * break_state = -1 to turn on break, and 0 to turn off break
-	 * see drivers/char/tty_io.c to see it used.
-	 * last_set_data_urb_value NEVER has the break bit set in it.
-	 */
+	mutex_lock(&priv->lock);
+
+	status = f81232_set_mask_register(port, LINE_CONTROL_REGISTER,
+						UART_LCR_SBC, !!break_state);
+	if (status)
+		dev_err(&port->dev, "set break failed: %d\n", status);
+
+	mutex_unlock(&priv->lock);
 }
 
 static int f81232_find_clk(speed_t baudrate)

[PATCH 5/5] USB: serial: f81232: fix bulk_in/out size

[Ji-Ze Hong (Peter Hong)]

Fix Fintek F81232 bulk_in/out size to 64/16 according to the spec.
http://html.alldatasheet.com/html-pdf/406315/FINTEK/F81232/1762/8/F81232.html

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index a054f69446fd..f3ee537d643c 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
 	},
 	.id_table =		id_table,
 	.num_ports =		1,
-	.bulk_in_size =		256,
-	.bulk_out_size =	256,
+	.bulk_out_size =	16,
 	.open =			f81232_open,
 	.close =		f81232_close,
 	.dtr_rts =		f81232_dtr_rts,
-- 
2.7.4

[5/5] USB: serial: f81232: fix bulk_in/out size

[Ji-Ze Hong (Peter Hong)]

Fix Fintek F81232 bulk_in/out size to 64/16 according to the spec.
http://html.alldatasheet.com/html-pdf/406315/FINTEK/F81232/1762/8/F81232.html

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

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index a054f69446fd..f3ee537d643c 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
 	},
 	.id_table =		id_table,
 	.num_ports =		1,
-	.bulk_in_size =		256,
-	.bulk_out_size =	256,
+	.bulk_out_size =	16,
 	.open =			f81232_open,
 	.close =		f81232_close,
 	.dtr_rts =		f81232_dtr_rts,

Re: [PATCH 1/5] USB: serial: f81232: clear overrun flag

[Oliver Neukum]

Am Montag, den 22.01.2018, 15:58 +0800 schrieb  Ji-Ze Hong (Peter Hong)
:
> The F81232 will report data and LSR with bulk like following format:
> bulk-in data: [LSR(1Byte)+DATA(1Byte)][LSR(1Byte)+DATA(1Byte)]...
> 
> LSR will auto clear frame/parity/break error flag when reading by H/W,
> but overrrun will only cleared when reading LSR. So this patch add a
> worker to read LSR when OE.
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
[..] 
> +static void f81232_lsr_worker(struct work_struct *work)
> +{
> +	struct f81232_private *priv;
> +	struct usb_serial_port *port;
> +	int status;
> +	u8 tmp;
> +
> +	priv = container_of(work, struct f81232_private, lsr_work);
> +	port = priv->port;
> +
> +	status = f81232_get_register(port, LINE_STATUS_REGISTER, &tmp);
> +	if (status)
> +		dev_warn(&port->dev, "read LSR failed: %d\n", status);
> +}

Hi,

I am afraid this is incomplete. You are scheduling a work that does IO.
Hence you must cancel that work when the driver is unbound from the
interface. You must also not do IO like this while the system is suspending.

	Regards
		Oliver

[1/5] USB: serial: f81232: clear overrun flag

[Oliver Neukum]

Am Montag, den 22.01.2018, 15:58 +0800 schrieb  Ji-Ze Hong (Peter Hong)
:
> The F81232 will report data and LSR with bulk like following format:
> bulk-in data: [LSR(1Byte)+DATA(1Byte)][LSR(1Byte)+DATA(1Byte)]...
> 
> LSR will auto clear frame/parity/break error flag when reading by H/W,
> but overrrun will only cleared when reading LSR. So this patch add a
> worker to read LSR when OE.
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
[..] 
> +static void f81232_lsr_worker(struct work_struct *work)
> +{
> +	struct f81232_private *priv;
> +	struct usb_serial_port *port;
> +	int status;
> +	u8 tmp;
> +
> +	priv = container_of(work, struct f81232_private, lsr_work);
> +	port = priv->port;
> +
> +	status = f81232_get_register(port, LINE_STATUS_REGISTER, &tmp);
> +	if (status)
> +		dev_warn(&port->dev, "read LSR failed: %d\n", status);
> +}

Hi,

I am afraid this is incomplete. You are scheduling a work that does IO.
Hence you must cancel that work when the driver is unbound from the
interface. You must also not do IO like this while the system is suspending.

	Regards
		Oliver
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] USB: serial: f81232: add high baud rate support

[Andy Shevchenko]

On Mon, Jan 22, 2018 at 9:58 AM, Ji-Ze Hong (Peter Hong)
<hpeter@gmail.com> wrote:
> The F81232 had 4 clocksource 1.846/18.46/14.77/24MHz and baud rates
> can be up to 1.5Mbits with 24MHz.
>
> F81232 Clock registers (106h)
>
> Bit1-0:     Clock source selector
>                     00: 1.846MHz.
>                     01: 18.46MHz.
>                     10: 24MHz.
>                     11: 14.77MHz.

Hmm... Why not to provide a proper clk driver (based on table variant
of clk-divider) and use it here?


-- 
With Best Regards,
Andy Shevchenko

[2/5] USB: serial: f81232: add high baud rate support

[Andy Shevchenko]

On Mon, Jan 22, 2018 at 9:58 AM, Ji-Ze Hong (Peter Hong)
<hpeter@gmail.com> wrote:
> The F81232 had 4 clocksource 1.846/18.46/14.77/24MHz and baud rates
> can be up to 1.5Mbits with 24MHz.
>
> F81232 Clock registers (106h)
>
> Bit1-0:     Clock source selector
>                     00: 1.846MHz.
>                     01: 18.46MHz.
>                     10: 24MHz.
>                     11: 14.77MHz.

Hmm... Why not to provide a proper clk driver (based on table variant
of clk-divider) and use it here?

Re: [PATCH 1/5] USB: serial: f81232: clear overrun flag

[Ji-Ze Hong (Peter Hong)]

Hi Oliver,

Oliver Neukum 於 2018/1/22 下午 06:06 寫道:
>> +static void f81232_lsr_worker(struct work_struct *work)
>> +{
>> +	struct f81232_private *priv;
>> +	struct usb_serial_port *port;
>> +	int status;
>> +	u8 tmp;
>> +
>> +	priv = container_of(work, struct f81232_private, lsr_work);
>> +	port = priv->port;
>> +
>> +	status = f81232_get_register(port, LINE_STATUS_REGISTER, &tmp);
>> +	if (status)
>> +		dev_warn(&port->dev, "read LSR failed: %d\n", status);
>> +}
> 
> Hi,
> 
> I am afraid this is incomplete. You are scheduling a work that does IO.
> Hence you must cancel that work when the driver is unbound from the
> interface. You must also not do IO like this while the system is suspending.

We'll add cancel worker operation on close() and suspend() to prevent
from I/O operations in wrong time with next patch version.

Thanks
-- 
With Best Regards,
Peter Hong

[1/5] USB: serial: f81232: clear overrun flag

[Ji-Ze Hong (Peter Hong)]

Hi Oliver,

Oliver Neukum 於 2018/1/22 下午 06:06 寫道:
>> +static void f81232_lsr_worker(struct work_struct *work)
>> +{
>> +	struct f81232_private *priv;
>> +	struct usb_serial_port *port;
>> +	int status;
>> +	u8 tmp;
>> +
>> +	priv = container_of(work, struct f81232_private, lsr_work);
>> +	port = priv->port;
>> +
>> +	status = f81232_get_register(port, LINE_STATUS_REGISTER, &tmp);
>> +	if (status)
>> +		dev_warn(&port->dev, "read LSR failed: %d\n", status);
>> +}
> 
> Hi,
> 
> I am afraid this is incomplete. You are scheduling a work that does IO.
> Hence you must cancel that work when the driver is unbound from the
> interface. You must also not do IO like this while the system is suspending.

We'll add cancel worker operation on close() and suspend() to prevent
from I/O operations in wrong time with next patch version.

Thanks

Re: [PATCH 2/5] USB: serial: f81232: add high baud rate support

[Ji-Ze Hong (Peter Hong)]

Hi Andy,

Andy Shevchenko 於 2018/1/22 下午 10:55 寫道:
> On Mon, Jan 22, 2018 at 9:58 AM, Ji-Ze Hong (Peter Hong)
> <hpeter@gmail.com> wrote:
>> The F81232 had 4 clocksource 1.846/18.46/14.77/24MHz and baud rates
>> can be up to 1.5Mbits with 24MHz.
>>
>> F81232 Clock registers (106h)
>>
>> Bit1-0:     Clock source selector
>>                      00: 1.846MHz.
>>                      01: 18.46MHz.
>>                      10: 24MHz.
>>                      11: 14.77MHz.
> 
> Hmm... Why not to provide a proper clk driver (based on table variant
> of clk-divider) and use it here?
> 
> 

It seems too complex to use clock framework in this driver.
What do you think about this, Johan ?

Thanks
-- 
With Best Regards,
Peter Hong

[2/5] USB: serial: f81232: add high baud rate support

[Ji-Ze Hong (Peter Hong)]

Hi Andy,

Andy Shevchenko 於 2018/1/22 下午 10:55 寫道:
> On Mon, Jan 22, 2018 at 9:58 AM, Ji-Ze Hong (Peter Hong)
> <hpeter@gmail.com> wrote:
>> The F81232 had 4 clocksource 1.846/18.46/14.77/24MHz and baud rates
>> can be up to 1.5Mbits with 24MHz.
>>
>> F81232 Clock registers (106h)
>>
>> Bit1-0:     Clock source selector
>>                      00: 1.846MHz.
>>                      01: 18.46MHz.
>>                      10: 24MHz.
>>                      11: 14.77MHz.
> 
> Hmm... Why not to provide a proper clk driver (based on table variant
> of clk-divider) and use it here?
> 
> 

It seems too complex to use clock framework in this driver.
What do you think about this, Johan ?

Thanks

Re: [PATCH 2/5] USB: serial: f81232: add high baud rate support

[Johan Hovold]

On Tue, Jan 23, 2018 at 10:08:24AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Andy,
> 
> Andy Shevchenko 於 2018/1/22 下午 10:55 寫道:
> > On Mon, Jan 22, 2018 at 9:58 AM, Ji-Ze Hong (Peter Hong)
> > <hpeter@gmail.com> wrote:
> >> The F81232 had 4 clocksource 1.846/18.46/14.77/24MHz and baud rates
> >> can be up to 1.5Mbits with 24MHz.
> >>
> >> F81232 Clock registers (106h)
> >>
> >> Bit1-0:     Clock source selector
> >>                      00: 1.846MHz.
> >>                      01: 18.46MHz.
> >>                      10: 24MHz.
> >>                      11: 14.77MHz.
> > 
> > Hmm... Why not to provide a proper clk driver (based on table variant
> > of clk-divider) and use it here?
> 
> It seems too complex to use clock framework in this driver.
> What do you think about this, Johan ?

Yeah, you don't need to implement a clk driver for this. If anyone
thinks that would simplify things, I'd be happy to consider it as a
follow-on patch.

Thanks,
Johan

[2/5] USB: serial: f81232: add high baud rate support

[Johan Hovold]

On Tue, Jan 23, 2018 at 10:08:24AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Andy,
> 
> Andy Shevchenko 於 2018/1/22 下午 10:55 寫道:
> > On Mon, Jan 22, 2018 at 9:58 AM, Ji-Ze Hong (Peter Hong)
> > <hpeter@gmail.com> wrote:
> >> The F81232 had 4 clocksource 1.846/18.46/14.77/24MHz and baud rates
> >> can be up to 1.5Mbits with 24MHz.
> >>
> >> F81232 Clock registers (106h)
> >>
> >> Bit1-0:     Clock source selector
> >>                      00: 1.846MHz.
> >>                      01: 18.46MHz.
> >>                      10: 24MHz.
> >>                      11: 14.77MHz.
> > 
> > Hmm... Why not to provide a proper clk driver (based on table variant
> > of clk-divider) and use it here?
> 
> It seems too complex to use clock framework in this driver.
> What do you think about this, Johan ?

Yeah, you don't need to implement a clk driver for this. If anyone
thinks that would simplify things, I'd be happy to consider it as a
follow-on patch.

Thanks,
Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Johan Hovold]

On Mon, Jan 22, 2018 at 03:58:45PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> The F81232 can do remote wakeup via RX/RI pin with pulse.
> This patch will use device_set_wakeup_enable to enable this
> feature.

This is a policy decision that should be made by user space by setting
the power/wakeup attribute, and not something that something that
drivers should enable directly themselves.

Perhaps you really wanted to use device_set_wakeup_capable()? But then
you also need to honour the current setting in suspend() as well.

How have you tested this feature?

Johan

[3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Johan Hovold]

On Mon, Jan 22, 2018 at 03:58:45PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> The F81232 can do remote wakeup via RX/RI pin with pulse.
> This patch will use device_set_wakeup_enable to enable this
> feature.

This is a policy decision that should be made by user space by setting
the power/wakeup attribute, and not something that something that
drivers should enable directly themselves.

Perhaps you really wanted to use device_set_wakeup_capable()? But then
you also need to honour the current setting in suspend() as well.

How have you tested this feature?

Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] USB: serial: f81232: fix bulk_in/out size

[Johan Hovold]

On Mon, Jan 22, 2018 at 03:58:47PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Fix Fintek F81232 bulk_in/out size to 64/16 according to the spec.
> http://html.alldatasheet.com/html-pdf/406315/FINTEK/F81232/1762/8/F81232.html
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> ---
>  drivers/usb/serial/f81232.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index a054f69446fd..f3ee537d643c 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
>  	},
>  	.id_table =		id_table,
>  	.num_ports =		1,
> -	.bulk_in_size =		256,
> -	.bulk_out_size =	256,
> +	.bulk_out_size =	16,

These fields control the URB buffer sizes and defaults to the corresponding
endpoint max-packet size, which would be 16 for all endpoints according
to the datasheet above.

So it seems you should really be setting bulk_in_size to 64 here (and
possibly leave bulk_out_size unset) as that would appear to match your
device buffer sizes.

Johan

[5/5] USB: serial: f81232: fix bulk_in/out size

[Johan Hovold]

On Mon, Jan 22, 2018 at 03:58:47PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Fix Fintek F81232 bulk_in/out size to 64/16 according to the spec.
> http://html.alldatasheet.com/html-pdf/406315/FINTEK/F81232/1762/8/F81232.html
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> ---
>  drivers/usb/serial/f81232.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index a054f69446fd..f3ee537d643c 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
>  	},
>  	.id_table =		id_table,
>  	.num_ports =		1,
> -	.bulk_in_size =		256,
> -	.bulk_out_size =	256,
> +	.bulk_out_size =	16,

These fields control the URB buffer sizes and defaults to the corresponding
endpoint max-packet size, which would be 16 for all endpoints according
to the datasheet above.

So it seems you should really be setting bulk_in_size to 64 here (and
possibly leave bulk_out_size unset) as that would appear to match your
device buffer sizes.

Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Ji-Ze Hong (Peter Hong)]

Hi Johan,

Johan Hovold 於 2018/1/30 上午 11:57 寫道:
> On Mon, Jan 22, 2018 at 03:58:45PM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> The F81232 can do remote wakeup via RX/RI pin with pulse.
>> This patch will use device_set_wakeup_enable to enable this
>> feature.
> 
> This is a policy decision that should be made by user space by setting
> the power/wakeup attribute, and not something that something that
> drivers should enable directly themselves.
> 
> Perhaps you really wanted to use device_set_wakeup_capable()? But then
> you also need to honour the current setting in suspend() as well.
> 
> How have you tested this feature?
> 

Our USB-To-Serial support RI/ RX remote wakeup by Modem, Fax or
other peripherals and we had tested it by following procedure with
device_set_wakeup_enable() enabled:
     1. Using pm-suspend to S3
     2. Trigger a pulse to RI/RX to wake up system.

In our test, we can do remote wakeup only with
device_set_wakeup_enable() enabled.

Should we add device_set_wakeup_capable() & device_set_wakeup_enable()
like following link??
https://elixir.free-electrons.com/linux/latest/source/drivers/media/rc/mceusb.c#L1476

Thanks
-- 
With Best Regards,
Peter Hong

[3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Ji-Ze Hong (Peter Hong)]

Hi Johan,

Johan Hovold 於 2018/1/30 上午 11:57 寫道:
> On Mon, Jan 22, 2018 at 03:58:45PM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> The F81232 can do remote wakeup via RX/RI pin with pulse.
>> This patch will use device_set_wakeup_enable to enable this
>> feature.
> 
> This is a policy decision that should be made by user space by setting
> the power/wakeup attribute, and not something that something that
> drivers should enable directly themselves.
> 
> Perhaps you really wanted to use device_set_wakeup_capable()? But then
> you also need to honour the current setting in suspend() as well.
> 
> How have you tested this feature?
> 

Our USB-To-Serial support RI/ RX remote wakeup by Modem, Fax or
other peripherals and we had tested it by following procedure with
device_set_wakeup_enable() enabled:
     1. Using pm-suspend to S3
     2. Trigger a pulse to RI/RX to wake up system.

In our test, we can do remote wakeup only with
device_set_wakeup_enable() enabled.

Should we add device_set_wakeup_capable() & device_set_wakeup_enable()
like following link??
https://elixir.free-electrons.com/linux/latest/source/drivers/media/rc/mceusb.c#L1476

Thanks

Re: [PATCH 5/5] USB: serial: f81232: fix bulk_in/out size

[Ji-Ze Hong (Peter Hong)]

Hi Johan,

Johan Hovold 於 2018/1/30 下午 12:11 寫道:
> On Mon, Jan 22, 2018 at 03:58:47PM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
>> index a054f69446fd..f3ee537d643c 100644
>> --- a/drivers/usb/serial/f81232.c
>> +++ b/drivers/usb/serial/f81232.c
>> @@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
>>   	},
>>   	.id_table =		id_table,
>>   	.num_ports =		1,
>> -	.bulk_in_size =		256,
>> -	.bulk_out_size =	256,
>> +	.bulk_out_size =	16,
> 
> So it seems you should really be setting bulk_in_size to 64 here (and
> possibly leave bulk_out_size unset) as that would appear to match your
> device buffer sizes.

Yes, we want to set the bulk_in_size as 64. The public datasheet has
some error with bulk in/out, the correct size is 64.

We had test the bulk_out_size set the same with internal TX FIFO will
make the best performance in tests, but it's ok to set 64. In my opinion
, I'll prefer to set 16.

The following information is the F81232 dump by lsusb:

Bus 002 Device 007: ID 1934:0706 Feature Integration Technology Inc. 
(Fintek)
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               1.10
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        16
   idVendor           0x1934 Feature Integration Technology Inc. (Fintek)
   idProduct          0x0706
   bcdDevice            0.01
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3 88635600168801
   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     0x0040  1x 64 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     0x0040  1x 64 bytes
         bInterval               0
Device Status:     0x0000
   (Bus Powered)

Thanks
-- 
With Best Regards,
Peter Hong

[5/5] USB: serial: f81232: fix bulk_in/out size

[Ji-Ze Hong (Peter Hong)]

Hi Johan,

Johan Hovold 於 2018/1/30 下午 12:11 寫道:
> On Mon, Jan 22, 2018 at 03:58:47PM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
>> index a054f69446fd..f3ee537d643c 100644
>> --- a/drivers/usb/serial/f81232.c
>> +++ b/drivers/usb/serial/f81232.c
>> @@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
>>   	},
>>   	.id_table =		id_table,
>>   	.num_ports =		1,
>> -	.bulk_in_size =		256,
>> -	.bulk_out_size =	256,
>> +	.bulk_out_size =	16,
> 
> So it seems you should really be setting bulk_in_size to 64 here (and
> possibly leave bulk_out_size unset) as that would appear to match your
> device buffer sizes.

Yes, we want to set the bulk_in_size as 64. The public datasheet has
some error with bulk in/out, the correct size is 64.

We had test the bulk_out_size set the same with internal TX FIFO will
make the best performance in tests, but it's ok to set 64. In my opinion
, I'll prefer to set 16.

The following information is the F81232 dump by lsusb:

Bus 002 Device 007: ID 1934:0706 Feature Integration Technology Inc. 
(Fintek)
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               1.10
   bDeviceClass            0 (Defined at Interface level)
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0        16
   idVendor           0x1934 Feature Integration Technology Inc. (Fintek)
   idProduct          0x0706
   bcdDevice            0.01
   iManufacturer           1 FINTEK
   iProduct                2 USB TO UART BRIDGE
   iSerial                 3 88635600168801
   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     0x0040  1x 64 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     0x0040  1x 64 bytes
         bInterval               0
Device Status:     0x0000
   (Bus Powered)

Thanks

Re: [PATCH 3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Johan Hovold]

On Thu, Feb 01, 2018 at 11:13:01AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Johan,
> 
> Johan Hovold 於 2018/1/30 上午 11:57 寫道:
> > On Mon, Jan 22, 2018 at 03:58:45PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> >> The F81232 can do remote wakeup via RX/RI pin with pulse.
> >> This patch will use device_set_wakeup_enable to enable this
> >> feature.
> > 
> > This is a policy decision that should be made by user space by setting
> > the power/wakeup attribute, and not something that something that
> > drivers should enable directly themselves.
> > 
> > Perhaps you really wanted to use device_set_wakeup_capable()? But then
> > you also need to honour the current setting in suspend() as well.
> > 
> > How have you tested this feature?
> > 
> 
> Our USB-To-Serial support RI/ RX remote wakeup by Modem, Fax or
> other peripherals and we had tested it by following procedure with
> device_set_wakeup_enable() enabled:
>      1. Using pm-suspend to S3
>      2. Trigger a pulse to RI/RX to wake up system.
> 
> In our test, we can do remote wakeup only with
> device_set_wakeup_enable() enabled.

Yeah, but you need to enable it though sysfs. Not every device should be
able to wake the system up. That's a decision left for user space.

> Should we add device_set_wakeup_capable() & device_set_wakeup_enable()
> like following link??
> https://elixir.free-electrons.com/linux/latest/source/drivers/media/rc/mceusb.c#L1476

No, your driver should not call device_set_wakeup_enable() itself. Just
set the wakeup capable flag in probe. And if you can disable the wake up
feature, this needs to be done at suspend depending on what user space
has requested.

Johan

[3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Johan Hovold]

On Thu, Feb 01, 2018 at 11:13:01AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Johan,
> 
> Johan Hovold 於 2018/1/30 上午 11:57 寫道:
> > On Mon, Jan 22, 2018 at 03:58:45PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> >> The F81232 can do remote wakeup via RX/RI pin with pulse.
> >> This patch will use device_set_wakeup_enable to enable this
> >> feature.
> > 
> > This is a policy decision that should be made by user space by setting
> > the power/wakeup attribute, and not something that something that
> > drivers should enable directly themselves.
> > 
> > Perhaps you really wanted to use device_set_wakeup_capable()? But then
> > you also need to honour the current setting in suspend() as well.
> > 
> > How have you tested this feature?
> > 
> 
> Our USB-To-Serial support RI/ RX remote wakeup by Modem, Fax or
> other peripherals and we had tested it by following procedure with
> device_set_wakeup_enable() enabled:
>      1. Using pm-suspend to S3
>      2. Trigger a pulse to RI/RX to wake up system.
> 
> In our test, we can do remote wakeup only with
> device_set_wakeup_enable() enabled.

Yeah, but you need to enable it though sysfs. Not every device should be
able to wake the system up. That's a decision left for user space.

> Should we add device_set_wakeup_capable() & device_set_wakeup_enable()
> like following link??
> https://elixir.free-electrons.com/linux/latest/source/drivers/media/rc/mceusb.c#L1476

No, your driver should not call device_set_wakeup_enable() itself. Just
set the wakeup capable flag in probe. And if you can disable the wake up
feature, this needs to be done at suspend depending on what user space
has requested.

Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] USB: serial: f81232: fix bulk_in/out size

[Johan Hovold]

On Thu, Feb 01, 2018 at 01:50:55PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Johan,
> 
> Johan Hovold 於 2018/1/30 下午 12:11 寫道:
> > On Mon, Jan 22, 2018 at 03:58:47PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> >> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> >> index a054f69446fd..f3ee537d643c 100644
> >> --- a/drivers/usb/serial/f81232.c
> >> +++ b/drivers/usb/serial/f81232.c
> >> @@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
> >>   	},
> >>   	.id_table =		id_table,
> >>   	.num_ports =		1,
> >> -	.bulk_in_size =		256,
> >> -	.bulk_out_size =	256,
> >> +	.bulk_out_size =	16,
> > 
> > So it seems you should really be setting bulk_in_size to 64 here (and
> > possibly leave bulk_out_size unset) as that would appear to match your
> > device buffer sizes.
> 
> Yes, we want to set the bulk_in_size as 64. The public datasheet has
> some error with bulk in/out, the correct size is 64.
> 
> We had test the bulk_out_size set the same with internal TX FIFO will
> make the best performance in tests, but it's ok to set 64. In my opinion
> , I'll prefer to set 16.

Having larger URB buffers than the endpoint size is typically more
efficient, but sometimes there are hardware issues that needs to be
worked around.

Johan

[5/5] USB: serial: f81232: fix bulk_in/out size

[Johan Hovold]

On Thu, Feb 01, 2018 at 01:50:55PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Johan,
> 
> Johan Hovold 於 2018/1/30 下午 12:11 寫道:
> > On Mon, Jan 22, 2018 at 03:58:47PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> >> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> >> index a054f69446fd..f3ee537d643c 100644
> >> --- a/drivers/usb/serial/f81232.c
> >> +++ b/drivers/usb/serial/f81232.c
> >> @@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
> >>   	},
> >>   	.id_table =		id_table,
> >>   	.num_ports =		1,
> >> -	.bulk_in_size =		256,
> >> -	.bulk_out_size =	256,
> >> +	.bulk_out_size =	16,
> > 
> > So it seems you should really be setting bulk_in_size to 64 here (and
> > possibly leave bulk_out_size unset) as that would appear to match your
> > device buffer sizes.
> 
> Yes, we want to set the bulk_in_size as 64. The public datasheet has
> some error with bulk in/out, the correct size is 64.
> 
> We had test the bulk_out_size set the same with internal TX FIFO will
> make the best performance in tests, but it's ok to set 64. In my opinion
> , I'll prefer to set 16.

Having larger URB buffers than the endpoint size is typically more
efficient, but sometimes there are hardware issues that needs to be
worked around.

Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Ji-Ze Hong (Peter Hong)]

Hi Johan,

Johan Hovold 於 2018/2/4 上午 09:46 寫道:
> On Thu, Feb 01, 2018 at 11:13:01AM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> Our USB-To-Serial support RI/ RX remote wakeup by Modem, Fax or
>> other peripherals and we had tested it by following procedure with
>> device_set_wakeup_enable() enabled:
>>       1. Using pm-suspend to S3
>>       2. Trigger a pulse to RI/RX to wake up system.
>>
>> In our test, we can do remote wakeup only with
>> device_set_wakeup_enable() enabled.
> 
> Yeah, but you need to enable it though sysfs. Not every device should be
> able to wake the system up. That's a decision left for user space.
> 
>> Should we add device_set_wakeup_capable() & device_set_wakeup_enable()
>> like following link??
>> https://elixir.free-electrons.com/linux/latest/source/drivers/media/rc/mceusb.c#L1476
> 
> No, your driver should not call device_set_wakeup_enable() itself. Just
> set the wakeup capable flag in probe. And if you can disable the wake up
> feature, this needs to be done at suspend depending on what user space
> has requested.

We'll change to device_set_wakeup_capable() and send the v2 patch when
test OK.

Thanks
-- 
With Best Regards,
Peter Hong

[3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Ji-Ze Hong (Peter Hong)]

Hi Johan,

Johan Hovold 於 2018/2/4 上午 09:46 寫道:
> On Thu, Feb 01, 2018 at 11:13:01AM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> Our USB-To-Serial support RI/ RX remote wakeup by Modem, Fax or
>> other peripherals and we had tested it by following procedure with
>> device_set_wakeup_enable() enabled:
>>       1. Using pm-suspend to S3
>>       2. Trigger a pulse to RI/RX to wake up system.
>>
>> In our test, we can do remote wakeup only with
>> device_set_wakeup_enable() enabled.
> 
> Yeah, but you need to enable it though sysfs. Not every device should be
> able to wake the system up. That's a decision left for user space.
> 
>> Should we add device_set_wakeup_capable() & device_set_wakeup_enable()
>> like following link??
>> https://elixir.free-electrons.com/linux/latest/source/drivers/media/rc/mceusb.c#L1476
> 
> No, your driver should not call device_set_wakeup_enable() itself. Just
> set the wakeup capable flag in probe. And if you can disable the wake up
> feature, this needs to be done at suspend depending on what user space
> has requested.

We'll change to device_set_wakeup_capable() and send the v2 patch when
test OK.

Thanks