[PATCH 0/8] Allow f_acm gadgets to notify the user about SetLineCoding requests

[PATCH 0/8] Allow f_acm gadgets to notify the user about SetLineCoding requests

[Tal Shorer]

I'm currently working on a project where I'd like to have an omap board
running linux be a usb-to-uart converter (using f_acm), and I've ran
into an issue: there's no way for the application to know if the host
has issued a SetLineCoding requests (after which parity/baudrate should
be changed to match the host's request).

This series adds the support necessary to achieve that:
- Allowing tty drivers to supply a poll() function to notify the user of
        driver-specific events.
- Propagating poll() and ioctl() from u_serial to the next layer (f_acm)
        in this case.
- Let the user read the current line coding set by the host (via an
        ioctl() call).
- Notify the user when there's a pending SetLineCoding request they
        haven't read yet

The last patch also removes up the port_line_config field from
struct gserial. It made no sense to have there (and had a REVISIT
comment at every turn), it was never used and it was initialized with
invalid values.

Tal Shorer (8):
  tty: add a poll() callback in struct tty_operations
  usb: gadget: u_serial: propagate poll() to the next layer
  usb: gadget: f_acm: validate set_line_coding requests
  usb: gadget: u_serial: propagate ioctl() to the next layer
  usb: gadget: f_acm: initialize port_line_coding when creating an
    instance
  usb: gadget: f_acm: add an ioctl to get the current line coding
  usb: gadget: f_acm: notify the user on SetLineCoding
  usb: gadget: u_serial: remove port_line_config from struct gserial

 Documentation/ioctl/ioctl-number.txt   |  1 +
 drivers/tty/n_tty.c                    |  2 ++
 drivers/usb/gadget/function/f_acm.c    | 66 +++++++++++++++++++++++++++++-----
 drivers/usb/gadget/function/u_serial.c | 53 ++++++++++++++++-----------
 drivers/usb/gadget/function/u_serial.h |  7 ++--
 include/linux/tty_driver.h             |  3 ++
 include/uapi/linux/usb/f_acm.h         | 12 +++++++
 7 files changed, 113 insertions(+), 31 deletions(-)
 create mode 100644 include/uapi/linux/usb/f_acm.h

--
2.7.4

[PATCH 1/8] tty: add a poll() callback in struct tty_operations

[Tal Shorer]

If a tty driver wants to notify the user of some exceptional event,
such as a usb cdc acm device set_line_coding event, it needs a way to
modify the mask returned by poll() and possible also add wait queues.
In order to do that, we allow the driver to supply a poll() callback
of its own, which will be called in n_tty_poll().

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 drivers/tty/n_tty.c        | 2 ++
 include/linux/tty_driver.h | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index bdf0e6e..7af8c29 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2394,6 +2394,8 @@ static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
 			tty_chars_in_buffer(tty) < WAKEUP_CHARS &&
 			tty_write_room(tty) > 0)
 		mask |= POLLOUT | POLLWRNORM;
+	if (tty->ops->poll)
+		mask |= tty->ops->poll(tty, file, wait);
 	return mask;
 }
 
diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index b742b5e..630ef03 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -243,6 +243,7 @@
 #include <linux/list.h>
 #include <linux/cdev.h>
 #include <linux/termios.h>
+#include <linux/poll.h>
 
 struct tty_struct;
 struct tty_driver;
@@ -285,6 +286,8 @@ struct tty_operations {
 	int (*set_termiox)(struct tty_struct *tty, struct termiox *tnew);
 	int (*get_icount)(struct tty_struct *tty,
 				struct serial_icounter_struct *icount);
+	unsigned int (*poll)(struct tty_struct *tty, struct file *file,
+				poll_table *wait);
 #ifdef CONFIG_CONSOLE_POLL
 	int (*poll_init)(struct tty_driver *driver, int line, char *options);
 	int (*poll_get_char)(struct tty_driver *driver, int line);
-- 
2.7.4

[PATCH 2/8] usb: gadget: u_serial: propagate poll() to the next layer

[Tal Shorer]

In order for a serial function to add flags to the poll() mask of their
tty files, propagate the poll() callback to the next layer so it can
return a mask if it sees fit to do so.

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 drivers/usb/gadget/function/u_serial.c | 16 ++++++++++++++++
 drivers/usb/gadget/function/u_serial.h |  3 +++
 2 files changed, 19 insertions(+)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index 9b0805f..d466f58 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1025,6 +1025,21 @@ static int gs_break_ctl(struct tty_struct *tty, int duration)
 	return status;
 }
 
+static unsigned int gs_poll(struct tty_struct *tty, struct file *file,
+	poll_table *wait)
+{
+	struct gs_port *port = tty->driver_data;
+	struct gserial *gser;
+	unsigned int mask = 0;
+
+	spin_lock_irq(&port->port_lock);
+	gser = port->port_usb;
+	if (gser && gser->poll)
+		mask |= gser->poll(gser, file, wait);
+	spin_unlock_irq(&port->port_lock);
+	return mask;
+}
+
 static const struct tty_operations gs_tty_ops = {
 	.open =			gs_open,
 	.close =		gs_close,
@@ -1035,6 +1050,7 @@ static const struct tty_operations gs_tty_ops = {
 	.chars_in_buffer =	gs_chars_in_buffer,
 	.unthrottle =		gs_unthrottle,
 	.break_ctl =		gs_break_ctl,
+	.poll =			gs_poll,
 };
 
 /*-------------------------------------------------------------------------*/
diff --git a/drivers/usb/gadget/function/u_serial.h b/drivers/usb/gadget/function/u_serial.h
index c20210c..ce00840 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -12,6 +12,7 @@
 #ifndef __U_SERIAL_H
 #define __U_SERIAL_H
 
+#include <linux/poll.h>
 #include <linux/usb/composite.h>
 #include <linux/usb/cdc.h>
 
@@ -50,6 +51,8 @@ struct gserial {
 	void (*connect)(struct gserial *p);
 	void (*disconnect)(struct gserial *p);
 	int (*send_break)(struct gserial *p, int duration);
+	unsigned int (*poll)(struct gserial *p, struct file *file,
+				poll_table *wait);
 };
 
 /* utilities to allocate/free request and buffer */
-- 
2.7.4

[PATCH 3/8] usb: gadget: f_acm: validate set_line_coding requests

[Tal Shorer]

We shouldn't accept malformed set_line_coding requests.
All values were taken from table 17 (section 6.3.11) of the cdc1.2 spec
available at http://www.usb.org/developers/docs/devclass_docs/
The table is in the file PTSN120.pdf.

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 drivers/usb/gadget/function/f_acm.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/function/f_acm.c b/drivers/usb/gadget/function/f_acm.c
index 5e3828d..e023313 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -326,13 +326,22 @@ static void acm_complete_set_line_coding(struct usb_ep *ep,
 		struct usb_cdc_line_coding	*value = req->buf;
 
 		/* REVISIT:  we currently just remember this data.
-		 * If we change that, (a) validate it first, then
-		 * (b) update whatever hardware needs updating,
-		 * (c) worry about locking.  This is information on
-		 * the order of 9600-8-N-1 ... most of which means
-		 * nothing unless we control a real RS232 line.
-		 */
-		acm->port_line_coding = *value;
+		* If we change that,
+		* (a) update whatever hardware needs updating,
+		* (b) worry about locking.  This is information on
+		* the order of 9600-8-N-1 ... most of which means
+		* nothing unless we control a real RS232 line.
+		*/
+		dev_dbg(&cdev->gadget->dev,
+			"acm ttyGS%d set_line_coding: %d %d %d %d\n",
+			acm->port_num, le32_to_cpu(value->dwDTERate),
+			value->bCharFormat, value->bParityType,
+			value->bDataBits);
+		if (value->bCharFormat > 2 || value->bParityType > 4 ||
+				value->bDataBits < 5 || value->bDataBits > 8)
+			usb_ep_set_halt(ep);
+		else
+			acm->port_line_coding = *value;
 	}
 }
 
-- 
2.7.4

[PATCH 4/8] usb: gadget: u_serial: propagate ioctl() to the next layer

[Tal Shorer]

In order for a serial function to implement its own ioctl() calls,
propagate the ioctl() callback to the next layer so it can handle it if
it sees fit to do so.

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 drivers/usb/gadget/function/u_serial.c | 15 +++++++++++++++
 drivers/usb/gadget/function/u_serial.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index d466f58..8d9abf1 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1040,6 +1040,20 @@ static unsigned int gs_poll(struct tty_struct *tty, struct file *file,
 	return mask;
 }
 
+static int gs_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
+{
+	struct gs_port *port = tty->driver_data;
+	struct gserial *gser;
+	int ret = -ENOIOCTLCMD;
+
+	spin_lock_irq(&port->port_lock);
+	gser = port->port_usb;
+	if (gser && gser->ioctl)
+		ret = gser->ioctl(gser, cmd, arg);
+	spin_unlock_irq(&port->port_lock);
+	return ret;
+}
+
 static const struct tty_operations gs_tty_ops = {
 	.open =			gs_open,
 	.close =		gs_close,
@@ -1051,6 +1065,7 @@ static const struct tty_operations gs_tty_ops = {
 	.unthrottle =		gs_unthrottle,
 	.break_ctl =		gs_break_ctl,
 	.poll =			gs_poll,
+	.ioctl =		gs_ioctl,
 };
 
 /*-------------------------------------------------------------------------*/
diff --git a/drivers/usb/gadget/function/u_serial.h b/drivers/usb/gadget/function/u_serial.h
index ce00840..8d0901e 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -53,6 +53,7 @@ struct gserial {
 	int (*send_break)(struct gserial *p, int duration);
 	unsigned int (*poll)(struct gserial *p, struct file *file,
 				poll_table *wait);
+	int (*ioctl)(struct gserial *p, unsigned int cmd, unsigned long arg);
 };
 
 /* utilities to allocate/free request and buffer */
-- 
2.7.4

[PATCH 5/8] usb: gadget: f_acm: initialize port_line_coding when creating an instance

[Tal Shorer]

Initialize acm->port_line_coding with something that makes sense so
that we can return a valid line coding if the host requests
GetLineCoding before requesting SetLineCoding

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 drivers/usb/gadget/function/f_acm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/gadget/function/f_acm.c b/drivers/usb/gadget/function/f_acm.c
index e023313..b7a1466 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -763,6 +763,12 @@ static struct usb_function *acm_alloc_func(struct usb_function_instance *fi)
 	acm->port.func.unbind = acm_unbind;
 	acm->port.func.free_func = acm_free_func;
 
+	/* initialize port_line_coding with something that makes sense */
+	coding.dwDTERate = cpu_to_le32(9600);
+	coding.bCharFormat = USB_CDC_1_STOP_BITS;
+	coding.bParityType = USB_CDC_NO_PARITY;
+	coding.bDataBits = 8;
+
 	return &acm->port.func;
 }
 
-- 
2.7.4

[PATCH 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Tal Shorer]

The user can issue USB_F_GET_LINE_CODING to get the current line coding
as set by the host (or the default if unset yet).

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 Documentation/ioctl/ioctl-number.txt |  1 +
 drivers/usb/gadget/function/f_acm.c  | 27 +++++++++++++++++++++++----
 include/uapi/linux/usb/f_acm.h       | 12 ++++++++++++
 3 files changed, 36 insertions(+), 4 deletions(-)
 create mode 100644 include/uapi/linux/usb/f_acm.h

diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 1e9fcb4..3d70680 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -329,6 +329,7 @@ Code  Seq#(hex)	Include File		Comments
 0xCA	80-8F	uapi/scsi/cxlflash_ioctl.h
 0xCB	00-1F	CBM serial IEC bus	in development:
 					<mailto:michael.klein@puffin.lb.shuttle.de>
+0xCD	10-1F	linux/usb/f_acm.h
 0xCD	01	linux/reiserfs_fs.h
 0xCF	02	fs/cifs/ioctl.c
 0xDB	00-0F	drivers/char/mwave/mwavepub.h
diff --git a/drivers/usb/gadget/function/f_acm.c b/drivers/usb/gadget/function/f_acm.c
index b7a1466..5feea7c 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/err.h>
+#include <uapi/linux/usb/f_acm.h>
 
 #include "u_serial.h"
 
@@ -611,6 +612,23 @@ static int acm_send_break(struct gserial *port, int duration)
 	return acm_notify_serial_state(acm);
 }
 
+static int acm_ioctl(struct gserial *port, unsigned int cmd, unsigned long arg)
+{
+	struct f_acm	*acm = port_to_acm(port);
+	int 		ret = -ENOIOCTLCMD;
+
+	switch (cmd) {
+	case USB_F_ACM_GET_LINE_CODING:
+		if (copy_to_user((__user void *)arg, &acm->port_line_coding,
+				sizeof(acm->port_line_coding)))
+			ret = -EFAULT;
+		else
+			ret = 0;
+		break;
+	}
+	return ret;
+}
+
 /*-------------------------------------------------------------------------*/
 
 /* ACM function driver setup/binding */
@@ -749,6 +767,7 @@ static struct usb_function *acm_alloc_func(struct usb_function_instance *fi)
 	acm->port.connect = acm_connect;
 	acm->port.disconnect = acm_disconnect;
 	acm->port.send_break = acm_send_break;
+	acm->port.ioctl = acm_ioctl;
 
 	acm->port.func.name = "acm";
 	acm->port.func.strings = acm_strings;
@@ -764,10 +783,10 @@ static struct usb_function *acm_alloc_func(struct usb_function_instance *fi)
 	acm->port.func.free_func = acm_free_func;
 
 	/* initialize port_line_coding with something that makes sense */
-	coding.dwDTERate = cpu_to_le32(9600);
-	coding.bCharFormat = USB_CDC_1_STOP_BITS;
-	coding.bParityType = USB_CDC_NO_PARITY;
-	coding.bDataBits = 8;
+	acm->port_line_coding.dwDTERate = cpu_to_le32(9600);
+	acm->port_line_coding.bCharFormat = USB_CDC_1_STOP_BITS;
+	acm->port_line_coding.bParityType = USB_CDC_NO_PARITY;
+	acm->port_line_coding.bDataBits = 8;
 
 	return &acm->port.func;
 }
diff --git a/include/uapi/linux/usb/f_acm.h b/include/uapi/linux/usb/f_acm.h
new file mode 100644
index 0000000..51f96f0
--- /dev/null
+++ b/include/uapi/linux/usb/f_acm.h
@@ -0,0 +1,12 @@
+/* f_acm.h -- Header file for USB CDC-ACM gadget function */
+
+#ifndef __UAPI_LINUX_USB_F_ACM_H
+#define __UAPI_LINUX_USB_F_ACM_H
+
+#include <linux/usb/cdc.h>
+#include <linux/ioctl.h>
+
+/* The 0xCD code is also used by reiserfs. we use 0x10-0x1F range */
+#define USB_F_ACM_GET_LINE_CODING _IOR(0xCD, 0x10, struct usb_cdc_line_coding)
+
+#endif /* __UAPI_LINUX_USB_F_ACM_H */
-- 
2.7.4

[PATCH 7/8] usb: gadget: f_acm: notify the user on SetLineCoding

[Tal Shorer]

Notify the user with a POLLPRI event when the host issues a
SetLineCoding request so that they can act upon it, for example by
configuring the line coding on a real serial port.

The event is cleared when the user reads the line coding using
USB_F_ACM_GET_LINE_CODING ioctl()

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 drivers/usb/gadget/function/f_acm.c | 38 ++++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/gadget/function/f_acm.c b/drivers/usb/gadget/function/f_acm.c
index 5feea7c..0983999 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -58,6 +58,9 @@ struct f_acm {
 	struct usb_request		*notify_req;
 
 	struct usb_cdc_line_coding	port_line_coding;	/* 8-N-1 etc */
+	/* we have a SetLineCoding request that the user haven't read yet */
+	bool set_line_coding_pending;
+	wait_queue_head_t set_line_coding_waitq;
 
 	/* SetControlLineState request -- CDC 1.1 section 6.2.14 (INPUT) */
 	u16				port_handshake_bits;
@@ -326,23 +329,19 @@ static void acm_complete_set_line_coding(struct usb_ep *ep,
 	} else {
 		struct usb_cdc_line_coding	*value = req->buf;
 
-		/* REVISIT:  we currently just remember this data.
-		* If we change that,
-		* (a) update whatever hardware needs updating,
-		* (b) worry about locking.  This is information on
-		* the order of 9600-8-N-1 ... most of which means
-		* nothing unless we control a real RS232 line.
-		*/
 		dev_dbg(&cdev->gadget->dev,
 			"acm ttyGS%d set_line_coding: %d %d %d %d\n",
 			acm->port_num, le32_to_cpu(value->dwDTERate),
 			value->bCharFormat, value->bParityType,
 			value->bDataBits);
 		if (value->bCharFormat > 2 || value->bParityType > 4 ||
-				value->bDataBits < 5 || value->bDataBits > 8)
+				value->bDataBits < 5 || value->bDataBits > 8) {
 			usb_ep_set_halt(ep);
-		else
+		} else {
 			acm->port_line_coding = *value;
+			acm->set_line_coding_pending = true;
+			wake_up_interruptible(&acm->set_line_coding_waitq);
+		}
 	}
 }
 
@@ -598,6 +597,19 @@ static void acm_disconnect(struct gserial *port)
 	acm_notify_serial_state(acm);
 }
 
+static unsigned int acm_poll(struct gserial *port, struct file *file,
+	poll_table *wait)
+{
+	unsigned int mask = 0;
+	struct f_acm *acm = port_to_acm(port);
+
+	poll_wait(file, &acm->set_line_coding_waitq, wait);
+	if (acm->set_line_coding_pending)
+		mask |= POLLPRI;
+	return mask;
+}
+
+
 static int acm_send_break(struct gserial *port, int duration)
 {
 	struct f_acm		*acm = port_to_acm(port);
@@ -620,10 +632,12 @@ static int acm_ioctl(struct gserial *port, unsigned int cmd, unsigned long arg)
 	switch (cmd) {
 	case USB_F_ACM_GET_LINE_CODING:
 		if (copy_to_user((__user void *)arg, &acm->port_line_coding,
-				sizeof(acm->port_line_coding)))
+				sizeof(acm->port_line_coding))) {
 			ret = -EFAULT;
-		else
+		} else {
 			ret = 0;
+			acm->set_line_coding_pending = false;
+		}
 		break;
 	}
 	return ret;
@@ -763,11 +777,13 @@ static struct usb_function *acm_alloc_func(struct usb_function_instance *fi)
 		return ERR_PTR(-ENOMEM);
 
 	spin_lock_init(&acm->lock);
+	init_waitqueue_head(&acm->set_line_coding_waitq);
 
 	acm->port.connect = acm_connect;
 	acm->port.disconnect = acm_disconnect;
 	acm->port.send_break = acm_send_break;
 	acm->port.ioctl = acm_ioctl;
+	acm->port.poll = acm_poll;
 
 	acm->port.func.name = "acm";
 	acm->port.func.strings = acm_strings;
-- 
2.7.4

[PATCH 8/8] usb: gadget: u_serial: remove port_line_config from struct gserial

[Tal Shorer]

GetLineCoding and SetLineCoding are a cdc-acm thing. It doesn't make
sense to have that in the generic u_serial layer. Moreso, f_acm has its
own port_line_coding in its own struct and it uses that, while the one
in struct gserial is set once upon initialization and then never used.
Also, the initialized never-used values were invalid, with bDataBits
and bCharFormat having each other's value.

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 drivers/usb/gadget/function/u_serial.c | 22 ++--------------------
 drivers/usb/gadget/function/u_serial.h |  3 ---
 2 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index 8d9abf1..654d4a6 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -129,9 +129,6 @@ struct gs_port {
 	wait_queue_head_t	drain_wait;	/* wait while writes drain */
 	bool                    write_busy;
 	wait_queue_head_t	close_wait;
-
-	/* REVISIT this state ... */
-	struct usb_cdc_line_coding port_line_coding;	/* 8-N-1 etc */
 };
 
 static struct portmaster {
@@ -1314,7 +1311,7 @@ static void gserial_console_exit(void)
 #endif
 
 static int
-gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
+gs_port_alloc(unsigned port_num)
 {
 	struct gs_port	*port;
 	int		ret = 0;
@@ -1343,7 +1340,6 @@ gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
 	INIT_LIST_HEAD(&port->write_pool);
 
 	port->port_num = port_num;
-	port->port_line_coding = *coding;
 
 	ports[port_num].port = port;
 out:
@@ -1392,18 +1388,12 @@ EXPORT_SYMBOL_GPL(gserial_free_line);
 
 int gserial_alloc_line(unsigned char *line_num)
 {
-	struct usb_cdc_line_coding	coding;
 	struct device			*tty_dev;
 	int				ret;
 	int				port_num;
 
-	coding.dwDTERate = cpu_to_le32(9600);
-	coding.bCharFormat = 8;
-	coding.bParityType = USB_CDC_NO_PARITY;
-	coding.bDataBits = USB_CDC_1_STOP_BITS;
-
 	for (port_num = 0; port_num < MAX_U_SERIAL_PORTS; port_num++) {
-		ret = gs_port_alloc(port_num, &coding);
+		ret = gs_port_alloc(port_num);
 		if (ret == -EBUSY)
 			continue;
 		if (ret)
@@ -1491,11 +1481,6 @@ int gserial_connect(struct gserial *gser, u8 port_num)
 	gser->ioport = port;
 	port->port_usb = gser;
 
-	/* REVISIT unclear how best to handle this state...
-	 * we don't really couple it with the Linux TTY.
-	 */
-	gser->port_line_coding = port->port_line_coding;
-
 	/* REVISIT if waiting on "carrier detect", signal. */
 
 	/* if it's already open, start I/O ... and notify the serial
@@ -1543,9 +1528,6 @@ void gserial_disconnect(struct gserial *gser)
 	/* tell the TTY glue not to do I/O here any more */
 	spin_lock_irqsave(&port->port_lock, flags);
 
-	/* REVISIT as above: how best to track this? */
-	port->port_line_coding = gser->port_line_coding;
-
 	port->port_usb = NULL;
 	gser->ioport = NULL;
 	if (port->port.count > 0 || port->openclose) {
diff --git a/drivers/usb/gadget/function/u_serial.h b/drivers/usb/gadget/function/u_serial.h
index 8d0901e..0549efe 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -44,9 +44,6 @@ struct gserial {
 	struct usb_ep			*in;
 	struct usb_ep			*out;
 
-	/* REVISIT avoid this CDC-ACM support harder ... */
-	struct usb_cdc_line_coding port_line_coding;	/* 9600-8-N-1 etc */
-
 	/* notification callbacks */
 	void (*connect)(struct gserial *p);
 	void (*disconnect)(struct gserial *p);
-- 
2.7.4

Re: [PATCH 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Tal Shorer]

On Mon, Jun 12, 2017 at 8:26 PM, Tal Shorer <tal.shorer@gmail.com> wrote:
> The user can issue USB_F_GET_LINE_CODING to get the current line coding
> as set by the host (or the default if unset yet).
>
> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
> ---

> @@ -764,10 +783,10 @@ static struct usb_function *acm_alloc_func(struct usb_function_instance *fi)
>         acm->port.func.free_func = acm_free_func;
>
>         /* initialize port_line_coding with something that makes sense */
> -       coding.dwDTERate = cpu_to_le32(9600);
> -       coding.bCharFormat = USB_CDC_1_STOP_BITS;
> -       coding.bParityType = USB_CDC_NO_PARITY;
> -       coding.bDataBits = 8;
> +       acm->port_line_coding.dwDTERate = cpu_to_le32(9600);
> +       acm->port_line_coding.bCharFormat = USB_CDC_1_STOP_BITS;
> +       acm->port_line_coding.bParityType = USB_CDC_NO_PARITY;
> +       acm->port_line_coding.bDataBits = 8;
>
>         return &acm->port.func;
>  }
This hunk was messed up somehow and will not apply. I can resend a v2 if necessary, but the correct patch is as follows:

From: Tal Shorer <tal.shorer@gmail.com>
Subject: [PATCH 6/8] usb: gadget: f_acm: add an ioctl to get the current line
 coding

The user can issue USB_F_GET_LINE_CODING to get the current line coding
as set by the host (or the default if unset yet).

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 Documentation/ioctl/ioctl-number.txt |  1 +
 drivers/usb/gadget/function/f_acm.c  | 27 +++++++++++++++++++++++----
 include/uapi/linux/usb/f_acm.h       | 12 ++++++++++++
 3 files changed, 36 insertions(+), 4 deletions(-)
 create mode 100644 include/uapi/linux/usb/f_acm.h

diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 1e9fcb4..3d70680 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -329,6 +329,7 @@ Code  Seq#(hex)	Include File		Comments
 0xCA	80-8F	uapi/scsi/cxlflash_ioctl.h
 0xCB	00-1F	CBM serial IEC bus	in development:
 					<mailto:michael.klein@puffin.lb.shuttle.de>
+0xCD	10-1F	linux/usb/f_acm.h
 0xCD	01	linux/reiserfs_fs.h
 0xCF	02	fs/cifs/ioctl.c
 0xDB	00-0F	drivers/char/mwave/mwavepub.h
diff --git a/drivers/usb/gadget/function/f_acm.c b/drivers/usb/gadget/function/f_acm.c
index b7a1466..5feea7c 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/err.h>
+#include <uapi/linux/usb/f_acm.h>

 #include "u_serial.h"

@@ -611,6 +612,23 @@ static int acm_send_break(struct gserial *port, int duration)
 	return acm_notify_serial_state(acm);
 }

+static int acm_ioctl(struct gserial *port, unsigned int cmd, unsigned long arg)
+{
+	struct f_acm	*acm = port_to_acm(port);
+	int 		ret = -ENOIOCTLCMD;
+
+	switch (cmd) {
+	case USB_F_ACM_GET_LINE_CODING:
+		if (copy_to_user((__user void *)arg, &acm->port_line_coding,
+				sizeof(acm->port_line_coding)))
+			ret = -EFAULT;
+		else
+			ret = 0;
+		break;
+	}
+	return ret;
+}
+
 /*-------------------------------------------------------------------------*/

 /* ACM function driver setup/binding */
@@ -749,6 +767,7 @@ static struct usb_function *acm_alloc_func(struct usb_function_instance *fi)
 	acm->port.connect = acm_connect;
 	acm->port.disconnect = acm_disconnect;
 	acm->port.send_break = acm_send_break;
+	acm->port.ioctl = acm_ioctl;

 	acm->port.func.name = "acm";
 	acm->port.func.strings = acm_strings;
@@ -763,6 +763,12 @@ static struct usb_function *acm_alloc_func(struct usb_function_instance *fi)
 	acm->port.func.unbind = acm_unbind;
 	acm->port.func.free_func = acm_free_func;

+	/* initialize port_line_coding with something that makes sense */
+	coding.dwDTERate = cpu_to_le32(9600);
+	coding.bCharFormat = USB_CDC_1_STOP_BITS;
+	coding.bParityType = USB_CDC_NO_PARITY;
+	coding.bDataBits = 8;
+
 	return &acm->port.func;
 }

diff --git a/include/uapi/linux/usb/f_acm.h b/include/uapi/linux/usb/f_acm.h
new file mode 100644
index 0000000..51f96f0
--- /dev/null
+++ b/include/uapi/linux/usb/f_acm.h
@@ -0,0 +1,12 @@
+/* f_acm.h -- Header file for USB CDC-ACM gadget function */
+
+#ifndef __UAPI_LINUX_USB_F_ACM_H
+#define __UAPI_LINUX_USB_F_ACM_H
+
+#include <linux/usb/cdc.h>
+#include <linux/ioctl.h>
+
+/* The 0xCD code is also used by reiserfs. we use 0x10-0x1F range */
+#define USB_F_ACM_GET_LINE_CODING _IOR(0xCD, 0x10, struct usb_cdc_line_coding)
+
+#endif /* __UAPI_LINUX_USB_F_ACM_H */
--
2.7.4

Re: [PATCH 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Tal Shorer]

On Mon, Jun 12, 2017 at 9:02 PM, Tal Shorer <tal.shorer@gmail.com> wrote:
> On Mon, Jun 12, 2017 at 8:26 PM, Tal Shorer <tal.shorer@gmail.com> wrote:
>> The user can issue USB_F_GET_LINE_CODING to get the current line coding
>> as set by the host (or the default if unset yet).
>>
>> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
>> ---
>
>> @@ -764,10 +783,10 @@ static struct usb_function *acm_alloc_func(struct usb_function_instance *fi)
>>         acm->port.func.free_func = acm_free_func;
>>
>>         /* initialize port_line_coding with something that makes sense */
>> -       coding.dwDTERate = cpu_to_le32(9600);
>> -       coding.bCharFormat = USB_CDC_1_STOP_BITS;
>> -       coding.bParityType = USB_CDC_NO_PARITY;
>> -       coding.bDataBits = 8;
>> +       acm->port_line_coding.dwDTERate = cpu_to_le32(9600);
>> +       acm->port_line_coding.bCharFormat = USB_CDC_1_STOP_BITS;
>> +       acm->port_line_coding.bParityType = USB_CDC_NO_PARITY;
>> +       acm->port_line_coding.bDataBits = 8;
>>
>>         return &acm->port.func;
>>  }
> This hunk was messed up somehow and will not apply. I can resend a v2 if necessary, but the correct patch is as follows:
Actually, it shouldn't even be in this patch. I somehow meshed 5 and 6 together.
The correct PATCH 6/8 is as follows:

>From d7d56ceb8020f10623a04b4634f93e4cc678454d Mon Sep 17 00:00:00 2001
From: Tal Shorer <tal.shorer@gmail.com>
Date: Mon, 12 Jun 2017 19:40:56 +0300
Subject: [PATCH 6/8] usb: gadget: f_acm: add an ioctl to get the current line
 coding

The user can issue USB_F_GET_LINE_CODING to get the current line coding
as set by the host (or the default if unset yet).

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 Documentation/ioctl/ioctl-number.txt |  1 +
 drivers/usb/gadget/function/f_acm.c  | 27 +++++++++++++++++++++++----
 include/uapi/linux/usb/f_acm.h       | 12 ++++++++++++
 3 files changed, 36 insertions(+), 4 deletions(-)
 create mode 100644 include/uapi/linux/usb/f_acm.h

diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 1e9fcb4..3d70680 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -329,6 +329,7 @@ Code  Seq#(hex)	Include File		Comments
 0xCA	80-8F	uapi/scsi/cxlflash_ioctl.h
 0xCB	00-1F	CBM serial IEC bus	in development:
 					<mailto:michael.klein@puffin.lb.shuttle.de>
+0xCD	10-1F	linux/usb/f_acm.h
 0xCD	01	linux/reiserfs_fs.h
 0xCF	02	fs/cifs/ioctl.c
 0xDB	00-0F	drivers/char/mwave/mwavepub.h
diff --git a/drivers/usb/gadget/function/f_acm.c b/drivers/usb/gadget/function/f_acm.c
index b7a1466..5feea7c 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/err.h>
+#include <uapi/linux/usb/f_acm.h>

 #include "u_serial.h"

@@ -611,6 +612,23 @@ static int acm_send_break(struct gserial *port, int duration)
 	return acm_notify_serial_state(acm);
 }

+static int acm_ioctl(struct gserial *port, unsigned int cmd, unsigned long arg)
+{
+	struct f_acm	*acm = port_to_acm(port);
+	int 		ret = -ENOIOCTLCMD;
+
+	switch (cmd) {
+	case USB_F_ACM_GET_LINE_CODING:
+		if (copy_to_user((__user void *)arg, &acm->port_line_coding,
+				sizeof(acm->port_line_coding)))
+			ret = -EFAULT;
+		else
+			ret = 0;
+		break;
+	}
+	return ret;
+}
+
 /*-------------------------------------------------------------------------*/

 /* ACM function driver setup/binding */
@@ -749,6 +767,7 @@ static struct usb_function *acm_alloc_func(struct usb_function_instance *fi)
 	acm->port.connect = acm_connect;
 	acm->port.disconnect = acm_disconnect;
 	acm->port.send_break = acm_send_break;
+	acm->port.ioctl = acm_ioctl;

 	acm->port.func.name = "acm";
 	acm->port.func.strings = acm_strings;
diff --git a/include/uapi/linux/usb/f_acm.h b/include/uapi/linux/usb/f_acm.h
new file mode 100644
index 0000000..51f96f0
--- /dev/null
+++ b/include/uapi/linux/usb/f_acm.h
@@ -0,0 +1,12 @@
+/* f_acm.h -- Header file for USB CDC-ACM gadget function */
+
+#ifndef __UAPI_LINUX_USB_F_ACM_H
+#define __UAPI_LINUX_USB_F_ACM_H
+
+#include <linux/usb/cdc.h>
+#include <linux/ioctl.h>
+
+/* The 0xCD code is also used by reiserfs. we use 0x10-0x1F range */
+#define USB_F_ACM_GET_LINE_CODING _IOR(0xCD, 0x10, struct usb_cdc_line_coding)
+
+#endif /* __UAPI_LINUX_USB_F_ACM_H */
--
2.7.4

Re: [PATCH 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Alan Cox]

On Mon, 12 Jun 2017 20:26:13 +0300
Tal Shorer <tal.shorer@gmail.com> wrote:

> The user can issue USB_F_GET_LINE_CODING to get the current line coding
> as set by the host (or the default if unset yet).

No this is not how to do it. We don't want weirdass ioctls for each
different tty device type.

There are two ways this can work. The first is actually done by plenty of
real physical hardware and that is to simply update the termios of the
logical channel to reflect the settings negotiated by the link layer
below (in your course USB ACM).

If that isn't sufficient then implement an ioctl that could work for *ALL*
tty devices - for example return a termios structure indicating the
relevant values on top of the current tty termios settings not some USB
ACM magic object.

Alan