Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1664513 > unrolled thread

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

Started byTal Shorer <tal.shorer@gmail.com>
First post2017-06-13 09:00 +0200
Last post2017-06-13 11:30 +0200
Articles 5 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2 0/8] Allow f_acm gadgets to notify the user about SetLineCoding requests Tal Shorer <tal.shorer@gmail.com> - 2017-06-13 09:00 +0200
    [PATCH v2 8/8] usb: gadget: u_serial: remove port_line_config from struct gserial Tal Shorer <tal.shorer@gmail.com> - 2017-06-13 09:00 +0200
    [PATCH v2 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding Tal Shorer <tal.shorer@gmail.com> - 2017-06-13 09:00 +0200
      Re: [PATCH v2 6/8] usb: gadget: f_acm: add an ioctl to get the  current line coding Greg KH <gregkh@linuxfoundation.org> - 2017-06-13 11:20 +0200
        Re: [PATCH v2 6/8] usb: gadget: f_acm: add an ioctl to get the  current line coding Tal Shorer <tal.shorer@gmail.com> - 2017-06-13 11:30 +0200

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

FromTal Shorer <tal.shorer@gmail.com>
Date2017-06-13 09:00 +0200
Subject[PATCH v2 0/8] Allow f_acm gadgets to notify the user about SetLineCoding requests
Message-ID<tRNXX-4BW-5@gated-at.bofh.it>
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.

Changes from v1:
- patch 5 was messed up, which made patch 6 also messed up. fixed both
  of these.

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

[toc] | [next] | [standalone]


#1664514 — [PATCH v2 8/8] usb: gadget: u_serial: remove port_line_config from struct gserial

FromTal Shorer <tal.shorer@gmail.com>
Date2017-06-13 09:00 +0200
Subject[PATCH v2 8/8] usb: gadget: u_serial: remove port_line_config from struct gserial
Message-ID<tRNXY-4BW-25@gated-at.bofh.it>
In reply to#1664513
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

[toc] | [prev] | [next] | [standalone]


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

FromTal Shorer <tal.shorer@gmail.com>
Date2017-06-13 09:00 +0200
Subject[PATCH v2 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding
Message-ID<tRNXY-4BW-29@gated-at.bofh.it>
In reply to#1664513
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  | 19 +++++++++++++++++++
 include/uapi/linux/usb/f_acm.h       | 12 ++++++++++++
 3 files changed, 32 insertions(+)
 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 188d314..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

[toc] | [prev] | [next] | [standalone]


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

FromGreg KH <gregkh@linuxfoundation.org>
Date2017-06-13 11:20 +0200
SubjectRe: [PATCH v2 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding
Message-ID<tRQ9r-68v-9@gated-at.bofh.it>
In reply to#1664515
On Tue, Jun 13, 2017 at 09:52:12AM +0300, Tal Shorer 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>
> ---
>  Documentation/ioctl/ioctl-number.txt |  1 +
>  drivers/usb/gadget/function/f_acm.c  | 19 +++++++++++++++++++
>  include/uapi/linux/usb/f_acm.h       | 12 ++++++++++++

Where is this ioctl being called?  On the tty device?  If so, which one?
The gadget driver's tty device node?  Or somewhere else?

confused at the different levels here, sorry.

greg k-h

[toc] | [prev] | [next] | [standalone]


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

FromTal Shorer <tal.shorer@gmail.com>
Date2017-06-13 11:30 +0200
SubjectRe: [PATCH v2 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding
Message-ID<tRQj7-6bD-1@gated-at.bofh.it>
In reply to#1664624
On Tue, Jun 13, 2017 at 12:19 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Jun 13, 2017 at 09:52:12AM +0300, Tal Shorer 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>
>> ---
>>  Documentation/ioctl/ioctl-number.txt |  1 +
>>  drivers/usb/gadget/function/f_acm.c  | 19 +++++++++++++++++++
>>  include/uapi/linux/usb/f_acm.h       | 12 ++++++++++++
>
> Where is this ioctl being called?  On the tty device?  If so, which one?
> The gadget driver's tty device node?  Or somewhere else?
On an acm ttyGS* fd, yes.
>
> confused at the different levels here, sorry.
>
> greg k-h

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web