[PATCH v6 0/7] usb: gadget: u_serial: console improvements

[PATCH v6 0/7] usb: gadget: u_serial: console improvements

[Michał Mirosław]

This series makes it possible to have more control over console using
USB serial gadget ports. This can be useful when you need more than
one USB console or are configuring multiple serial port function via
configfs.

The patches are against balbi/testing/next tree. You can also pull from:
   https://rere.qmqm.pl/git/linux  usb-console

Michał Mirosław (7):
  usb: gadget: u_serial: add missing port entry locking
  usb: gadget: u_serial: reimplement console support
  usb: gadget: u_serial: make OBEX port not a console
  usb: gadget: u_serial: allow more console gadget ports
  usb: gadget: u_serial: diagnose missed console messages
  usb: gadget: legacy/serial: allow dynamic removal
  usb: gadget: u_serial: use mutex for serialising open()s

 drivers/usb/gadget/function/f_acm.c    |  21 +
 drivers/usb/gadget/function/f_obex.c   |   2 +-
 drivers/usb/gadget/function/f_serial.c |  21 +
 drivers/usb/gadget/function/u_serial.c | 532 +++++++++++++------------
 drivers/usb/gadget/function/u_serial.h |   8 +
 drivers/usb/gadget/legacy/serial.c     |  49 ++-
 6 files changed, 370 insertions(+), 263 deletions(-)

-- 
2.20.1

[PATCH v6 1/7] usb: gadget: u_serial: add missing port entry locking

[Michał Mirosław]

gserial_alloc_line() misses locking (for a release barrier) while
resetting port entry on TTY allocation failure. Fix this.

Cc: stable@vger.kernel.org
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Tested-by: Ladislav Michl <ladis@linux-mips.org>

---
  v6: rebased on balbi/testing/next
  v5: no changes
  v4: no changes
  v3: cc-stable
  v2: no changes

---
 drivers/usb/gadget/function/u_serial.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index 65f634ec7fc2..bb1e2e1d0076 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1239,8 +1239,10 @@ int gserial_alloc_line(unsigned char *line_num)
 				__func__, port_num, PTR_ERR(tty_dev));
 
 		ret = PTR_ERR(tty_dev);
+		mutex_lock(&ports[port_num].lock);
 		port = ports[port_num].port;
 		ports[port_num].port = NULL;
+		mutex_unlock(&ports[port_num].lock);
 		gserial_free_port(port);
 		goto err;
 	}
-- 
2.20.1

[PATCH v6 2/7] usb: gadget: u_serial: reimplement console support

[Michał Mirosław]

Rewrite console support to fix a few shortcomings of the old code
preventing its use with multiple ports. This removes some duplicated
code and replaces a custom kthread with simpler workqueue item.

Only port ttyGS0 gets to be a console for now.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Tested-by: Ladislav Michl <ladis@linux-mips.org>

---
  v6: rebased on balbi/testing/next
  v5: no changes
  v4: cosmetic change to __gs_console_push()
  v3: no changes
  v2: no changes

---
 drivers/usb/gadget/function/u_serial.c | 351 ++++++++++++-------------
 1 file changed, 164 insertions(+), 187 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index bb1e2e1d0076..94f6999e8262 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -82,14 +82,12 @@
 #define GS_CONSOLE_BUF_SIZE	8192
 
 /* console info */
-struct gscons_info {
-	struct gs_port		*port;
-	struct task_struct	*console_thread;
-	struct kfifo		con_buf;
-	/* protect the buf and busy flag */
-	spinlock_t		con_lock;
-	int			req_busy;
-	struct usb_request	*console_req;
+struct gs_console {
+	struct console		console;
+	struct work_struct	work;
+	spinlock_t		lock;
+	struct usb_request	*req;
+	struct kfifo		buf;
 };
 
 /*
@@ -101,6 +99,9 @@ struct gs_port {
 	spinlock_t		port_lock;	/* guard port_* access */
 
 	struct gserial		*port_usb;
+#ifdef CONFIG_U_SERIAL_CONSOLE
+	struct gs_console	*console;
+#endif
 
 	bool			openclose;	/* open/close in progress */
 	u8			port_num;
@@ -889,36 +890,9 @@ static struct tty_driver *gs_tty_driver;
 
 #ifdef CONFIG_U_SERIAL_CONSOLE
 
-static struct gscons_info gscons_info;
-static struct console gserial_cons;
-
-static struct usb_request *gs_request_new(struct usb_ep *ep)
+static void gs_console_complete_out(struct usb_ep *ep, struct usb_request *req)
 {
-	struct usb_request *req = usb_ep_alloc_request(ep, GFP_ATOMIC);
-	if (!req)
-		return NULL;
-
-	req->buf = kmalloc(ep->maxpacket, GFP_ATOMIC);
-	if (!req->buf) {
-		usb_ep_free_request(ep, req);
-		return NULL;
-	}
-
-	return req;
-}
-
-static void gs_request_free(struct usb_request *req, struct usb_ep *ep)
-{
-	if (!req)
-		return;
-
-	kfree(req->buf);
-	usb_ep_free_request(ep, req);
-}
-
-static void gs_complete_out(struct usb_ep *ep, struct usb_request *req)
-{
-	struct gscons_info *info = &gscons_info;
+	struct gs_console *cons = req->context;
 
 	switch (req->status) {
 	default:
@@ -927,12 +901,12 @@ static void gs_complete_out(struct usb_ep *ep, struct usb_request *req)
 		/* fall through */
 	case 0:
 		/* normal completion */
-		spin_lock(&info->con_lock);
-		info->req_busy = 0;
-		spin_unlock(&info->con_lock);
-
-		wake_up_process(info->console_thread);
+		spin_lock(&cons->lock);
+		req->length = 0;
+		schedule_work(&cons->work);
+		spin_unlock(&cons->lock);
 		break;
+	case -ECONNRESET:
 	case -ESHUTDOWN:
 		/* disconnect */
 		pr_vdebug("%s: %s shutdown\n", __func__, ep->name);
@@ -940,190 +914,190 @@ static void gs_complete_out(struct usb_ep *ep, struct usb_request *req)
 	}
 }
 
-static int gs_console_connect(int port_num)
+static void __gs_console_push(struct gs_console *cons)
 {
-	struct gscons_info *info = &gscons_info;
-	struct gs_port *port;
+	struct usb_request *req = cons->req;
 	struct usb_ep *ep;
+	size_t size;
 
-	if (port_num != gserial_cons.index) {
-		pr_err("%s: port num [%d] is not support console\n",
-		       __func__, port_num);
-		return -ENXIO;
-	}
+	if (!req)
+		return;	/* disconnected */
 
-	port = ports[port_num].port;
-	ep = port->port_usb->in;
-	if (!info->console_req) {
-		info->console_req = gs_request_new(ep);
-		if (!info->console_req)
-			return -ENOMEM;
-		info->console_req->complete = gs_complete_out;
-	}
+	if (req->length)
+		return;	/* busy */
 
-	info->port = port;
-	spin_lock(&info->con_lock);
-	info->req_busy = 0;
-	spin_unlock(&info->con_lock);
-	pr_vdebug("port[%d] console connect!\n", port_num);
-	return 0;
-}
-
-static void gs_console_disconnect(struct usb_ep *ep)
-{
-	struct gscons_info *info = &gscons_info;
-	struct usb_request *req = info->console_req;
-
-	gs_request_free(req, ep);
-	info->console_req = NULL;
-}
-
-static int gs_console_thread(void *data)
-{
-	struct gscons_info *info = &gscons_info;
-	struct gs_port *port;
-	struct usb_request *req;
-	struct usb_ep *ep;
-	int xfer, ret, count, size;
+	ep = cons->console.data;
+	size = kfifo_out(&cons->buf, req->buf, ep->maxpacket);
+	if (!size)
+		return;
 
-	do {
-		port = info->port;
-		set_current_state(TASK_INTERRUPTIBLE);
-		if (!port || !port->port_usb
-		    || !port->port_usb->in || !info->console_req)
-			goto sched;
-
-		req = info->console_req;
-		ep = port->port_usb->in;
-
-		spin_lock_irq(&info->con_lock);
-		count = kfifo_len(&info->con_buf);
-		size = ep->maxpacket;
-
-		if (count > 0 && !info->req_busy) {
-			set_current_state(TASK_RUNNING);
-			if (count < size)
-				size = count;
-
-			xfer = kfifo_out(&info->con_buf, req->buf, size);
-			req->length = xfer;
-
-			spin_unlock(&info->con_lock);
-			ret = usb_ep_queue(ep, req, GFP_ATOMIC);
-			spin_lock(&info->con_lock);
-			if (ret < 0)
-				info->req_busy = 0;
-			else
-				info->req_busy = 1;
-
-			spin_unlock_irq(&info->con_lock);
-		} else {
-			spin_unlock_irq(&info->con_lock);
-sched:
-			if (kthread_should_stop()) {
-				set_current_state(TASK_RUNNING);
-				break;
-			}
-			schedule();
-		}
-	} while (1);
-
-	return 0;
+	req->length = size;
+	if (usb_ep_queue(ep, req, GFP_ATOMIC))
+		req->length = 0;
 }
 
-static int gs_console_setup(struct console *co, char *options)
+static void gs_console_work(struct work_struct *work)
 {
-	struct gscons_info *info = &gscons_info;
-	int status;
-
-	info->port = NULL;
-	info->console_req = NULL;
-	info->req_busy = 0;
-	spin_lock_init(&info->con_lock);
+	struct gs_console *cons = container_of(work, struct gs_console, work);
 
-	status = kfifo_alloc(&info->con_buf, GS_CONSOLE_BUF_SIZE, GFP_KERNEL);
-	if (status) {
-		pr_err("%s: allocate console buffer failed\n", __func__);
-		return status;
-	}
+	spin_lock_irq(&cons->lock);
 
-	info->console_thread = kthread_create(gs_console_thread,
-					      co, "gs_console");
-	if (IS_ERR(info->console_thread)) {
-		pr_err("%s: cannot create console thread\n", __func__);
-		kfifo_free(&info->con_buf);
-		return PTR_ERR(info->console_thread);
-	}
-	wake_up_process(info->console_thread);
+	__gs_console_push(cons);
 
-	return 0;
+	spin_unlock_irq(&cons->lock);
 }
 
 static void gs_console_write(struct console *co,
 			     const char *buf, unsigned count)
 {
-	struct gscons_info *info = &gscons_info;
+	struct gs_console *cons = container_of(co, struct gs_console, console);
 	unsigned long flags;
 
-	spin_lock_irqsave(&info->con_lock, flags);
-	kfifo_in(&info->con_buf, buf, count);
-	spin_unlock_irqrestore(&info->con_lock, flags);
+	spin_lock_irqsave(&cons->lock, flags);
 
-	wake_up_process(info->console_thread);
+	kfifo_in(&cons->buf, buf, count);
+
+	if (cons->req && !cons->req->length)
+		schedule_work(&cons->work);
+
+	spin_unlock_irqrestore(&cons->lock, flags);
 }
 
 static struct tty_driver *gs_console_device(struct console *co, int *index)
 {
-	struct tty_driver **p = (struct tty_driver **)co->data;
-
-	if (!*p)
-		return NULL;
-
 	*index = co->index;
-	return *p;
+	return gs_tty_driver;
 }
 
-static struct console gserial_cons = {
-	.name =		"ttyGS",
-	.write =	gs_console_write,
-	.device =	gs_console_device,
-	.setup =	gs_console_setup,
-	.flags =	CON_PRINTBUFFER,
-	.index =	-1,
-	.data =		&gs_tty_driver,
-};
-
-static void gserial_console_init(void)
+static int gs_console_connect(struct gs_port *port)
 {
-	register_console(&gserial_cons);
+	struct gs_console *cons = port->console;
+	struct usb_request *req;
+	struct usb_ep *ep;
+
+	if (!cons)
+		return 0;
+
+	ep = port->port_usb->in;
+	req = gs_alloc_req(ep, ep->maxpacket, GFP_ATOMIC);
+	if (!req)
+		return -ENOMEM;
+	req->complete = gs_console_complete_out;
+	req->context = cons;
+	req->length = 0;
+
+	spin_lock(&cons->lock);
+	cons->req = req;
+	cons->console.data = ep;
+	spin_unlock(&cons->lock);
+
+	pr_debug("ttyGS%d: console connected!\n", port->port_num);
+
+	schedule_work(&cons->work);
+
+	return 0;
+}
+
+static void gs_console_disconnect(struct gs_port *port)
+{
+	struct gs_console *cons = port->console;
+	struct usb_request *req;
+	struct usb_ep *ep;
+
+	if (!cons)
+		return;
+
+	spin_lock(&cons->lock);
+
+	req = cons->req;
+	ep = cons->console.data;
+	cons->req = NULL;
+
+	spin_unlock(&cons->lock);
+
+	if (!req)
+		return;
+
+	usb_ep_dequeue(ep, req);
+	gs_free_req(ep, req);
 }
 
-static void gserial_console_exit(void)
+static int gs_console_init(struct gs_port *port)
 {
-	struct gscons_info *info = &gscons_info;
+	struct gs_console *cons;
+	int err;
+
+	if (port->console)
+		return 0;
+
+	cons = kzalloc(sizeof(*port->console), GFP_KERNEL);
+	if (!cons)
+		return -ENOMEM;
+
+	strcpy(cons->console.name, "ttyGS");
+	cons->console.write = gs_console_write;
+	cons->console.device = gs_console_device;
+	cons->console.flags = CON_PRINTBUFFER;
+	cons->console.index = port->port_num;
+
+	INIT_WORK(&cons->work, gs_console_work);
+	spin_lock_init(&cons->lock);
+
+	err = kfifo_alloc(&cons->buf, GS_CONSOLE_BUF_SIZE, GFP_KERNEL);
+	if (err) {
+		pr_err("ttyGS%d: allocate console buffer failed\n", port->port_num);
+		kfree(cons);
+		return err;
+	}
+
+	port->console = cons;
+	register_console(&cons->console);
+
+	spin_lock_irq(&port->port_lock);
+	if (port->port_usb)
+		gs_console_connect(port);
+	spin_unlock_irq(&port->port_lock);
+
+	return 0;
+}
+
+static void gs_console_exit(struct gs_port *port)
+{
+	struct gs_console *cons = port->console;
+
+	if (!cons)
+		return;
+
+	unregister_console(&cons->console);
+
+	spin_lock_irq(&port->port_lock);
+	if (cons->req)
+		gs_console_disconnect(port);
+	spin_unlock_irq(&port->port_lock);
 
-	unregister_console(&gserial_cons);
-	if (!IS_ERR_OR_NULL(info->console_thread))
-		kthread_stop(info->console_thread);
-	kfifo_free(&info->con_buf);
+	cancel_work_sync(&cons->work);
+	kfifo_free(&cons->buf);
+	kfree(cons);
+	port->console = NULL;
 }
 
 #else
 
-static int gs_console_connect(int port_num)
+static int gs_console_connect(struct gs_port *port)
 {
 	return 0;
 }
 
-static void gs_console_disconnect(struct usb_ep *ep)
+static void gs_console_disconnect(struct gs_port *port)
 {
 }
 
-static void gserial_console_init(void)
+static int gs_console_init(struct gs_port *port)
 {
+	return -ENOSYS;
 }
 
-static void gserial_console_exit(void)
+static void gs_console_exit(struct gs_port *port)
 {
 }
 
@@ -1197,18 +1171,19 @@ void gserial_free_line(unsigned char port_num)
 		return;
 	}
 	port = ports[port_num].port;
+	gs_console_exit(port);
 	ports[port_num].port = NULL;
 	mutex_unlock(&ports[port_num].lock);
 
 	gserial_free_port(port);
 	tty_unregister_device(gs_tty_driver, port_num);
-	gserial_console_exit();
 }
 EXPORT_SYMBOL_GPL(gserial_free_line);
 
 int gserial_alloc_line(unsigned char *line_num)
 {
 	struct usb_cdc_line_coding	coding;
+	struct gs_port			*port;
 	struct device			*tty_dev;
 	int				ret;
 	int				port_num;
@@ -1231,23 +1206,24 @@ int gserial_alloc_line(unsigned char *line_num)
 
 	/* ... and sysfs class devices, so mdev/udev make /dev/ttyGS* */
 
-	tty_dev = tty_port_register_device(&ports[port_num].port->port,
+	port = ports[port_num].port;
+	tty_dev = tty_port_register_device(&port->port,
 			gs_tty_driver, port_num, NULL);
 	if (IS_ERR(tty_dev)) {
-		struct gs_port	*port;
 		pr_err("%s: failed to register tty for port %d, err %ld\n",
 				__func__, port_num, PTR_ERR(tty_dev));
 
 		ret = PTR_ERR(tty_dev);
 		mutex_lock(&ports[port_num].lock);
-		port = ports[port_num].port;
 		ports[port_num].port = NULL;
 		mutex_unlock(&ports[port_num].lock);
 		gserial_free_port(port);
 		goto err;
 	}
 	*line_num = port_num;
-	gserial_console_init();
+
+	if (!port_num)
+		gs_console_init(port);
 err:
 	return ret;
 }
@@ -1329,7 +1305,7 @@ int gserial_connect(struct gserial *gser, u8 port_num)
 			gser->disconnect(gser);
 	}
 
-	status = gs_console_connect(port_num);
+	status = gs_console_connect(port);
 	spin_unlock_irqrestore(&port->port_lock, flags);
 
 	return status;
@@ -1361,6 +1337,8 @@ 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);
 
+	gs_console_disconnect(port);
+
 	/* REVISIT as above: how best to track this? */
 	port->port_line_coding = gser->port_line_coding;
 
@@ -1388,7 +1366,6 @@ void gserial_disconnect(struct gserial *gser)
 	port->read_allocated = port->read_started =
 		port->write_allocated = port->write_started = 0;
 
-	gs_console_disconnect(gser->in);
 	spin_unlock_irqrestore(&port->port_lock, flags);
 }
 EXPORT_SYMBOL_GPL(gserial_disconnect);
-- 
2.20.1

[PATCH v6 3/7] usb: gadget: u_serial: make OBEX port not a console

[Michał Mirosław]

Prevent OBEX serial port from ever becoming a console. Console messages
will definitely break the protocol, and since you have to instantiate
the port making it explicitly for OBEX, there is no point in allowing
console to break it by mistake.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
  v6: rebased on balbi/testing/next
  v5: no changes
  v4: no changes
  v3: rename gserial_alloc_line_raw() -> gserial_alloc_line_no_console()
  v2: change of API
    + commit message massage

---
 drivers/usb/gadget/function/f_obex.c   |  2 +-
 drivers/usb/gadget/function/u_serial.c | 16 ++++++++++++----
 drivers/usb/gadget/function/u_serial.h |  1 +
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/function/f_obex.c b/drivers/usb/gadget/function/f_obex.c
index 55b7f57d2dc7..ab26d84ed95e 100644
--- a/drivers/usb/gadget/function/f_obex.c
+++ b/drivers/usb/gadget/function/f_obex.c
@@ -432,7 +432,7 @@ static struct usb_function_instance *obex_alloc_inst(void)
 		return ERR_PTR(-ENOMEM);
 
 	opts->func_inst.free_func_inst = obex_free_inst;
-	ret = gserial_alloc_line(&opts->port_num);
+	ret = gserial_alloc_line_no_console(&opts->port_num);
 	if (ret) {
 		kfree(opts);
 		return ERR_PTR(ret);
diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index 94f6999e8262..62280c23cde2 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1180,7 +1180,7 @@ void gserial_free_line(unsigned char port_num)
 }
 EXPORT_SYMBOL_GPL(gserial_free_line);
 
-int gserial_alloc_line(unsigned char *line_num)
+int gserial_alloc_line_no_console(unsigned char *line_num)
 {
 	struct usb_cdc_line_coding	coding;
 	struct gs_port			*port;
@@ -1221,12 +1221,20 @@ int gserial_alloc_line(unsigned char *line_num)
 		goto err;
 	}
 	*line_num = port_num;
-
-	if (!port_num)
-		gs_console_init(port);
 err:
 	return ret;
 }
+EXPORT_SYMBOL_GPL(gserial_alloc_line_no_console);
+
+int gserial_alloc_line(unsigned char *line_num)
+{
+	int ret = gserial_alloc_line_no_console(line_num);
+
+	if (!ret && !*line_num)
+		gs_console_init(ports[*line_num].port);
+
+	return ret;
+}
 EXPORT_SYMBOL_GPL(gserial_alloc_line);
 
 /**
diff --git a/drivers/usb/gadget/function/u_serial.h b/drivers/usb/gadget/function/u_serial.h
index 9acaac1cbb75..8b472b0c8cb4 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -54,6 +54,7 @@ struct usb_request *gs_alloc_req(struct usb_ep *ep, unsigned len, gfp_t flags);
 void gs_free_req(struct usb_ep *, struct usb_request *req);
 
 /* management of individual TTY ports */
+int gserial_alloc_line_no_console(unsigned char *port_line);
 int gserial_alloc_line(unsigned char *port_line);
 void gserial_free_line(unsigned char port_line);
 
-- 
2.20.1

[PATCH v6 4/7] usb: gadget: u_serial: allow more console gadget ports

[Michał Mirosław]

Allow configuring more than one console using USB serial or ACM gadget.

By default, only first (ttyGS0) is a console, but this may be changed
using function's new "console" attribute.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

---
  v6: rebased on balbi/testing/next
  v5: fixed locking in gserial_get_console()
  v4: fixed locking in gserial_set_console()
  v3: no changes
  v2: no changes

---
 drivers/usb/gadget/function/f_acm.c    | 21 +++++++++++
 drivers/usb/gadget/function/f_serial.c | 21 +++++++++++
 drivers/usb/gadget/function/u_serial.c | 48 ++++++++++++++++++++++++++
 drivers/usb/gadget/function/u_serial.h |  7 ++++
 4 files changed, 97 insertions(+)

diff --git a/drivers/usb/gadget/function/f_acm.c b/drivers/usb/gadget/function/f_acm.c
index 9fc98de83624..7c152c28b26c 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -771,6 +771,24 @@ static struct configfs_item_operations acm_item_ops = {
 	.release                = acm_attr_release,
 };
 
+#ifdef CONFIG_U_SERIAL_CONSOLE
+
+static ssize_t f_acm_console_store(struct config_item *item,
+		const char *page, size_t count)
+{
+	return gserial_set_console(to_f_serial_opts(item)->port_num,
+				   page, count);
+}
+
+static ssize_t f_acm_console_show(struct config_item *item, char *page)
+{
+	return gserial_get_console(to_f_serial_opts(item)->port_num, page);
+}
+
+CONFIGFS_ATTR(f_acm_, console);
+
+#endif /* CONFIG_U_SERIAL_CONSOLE */
+
 static ssize_t f_acm_port_num_show(struct config_item *item, char *page)
 {
 	return sprintf(page, "%u\n", to_f_serial_opts(item)->port_num);
@@ -779,6 +797,9 @@ static ssize_t f_acm_port_num_show(struct config_item *item, char *page)
 CONFIGFS_ATTR_RO(f_acm_, port_num);
 
 static struct configfs_attribute *acm_attrs[] = {
+#ifdef CONFIG_U_SERIAL_CONSOLE
+	&f_acm_attr_console,
+#endif
 	&f_acm_attr_port_num,
 	NULL,
 };
diff --git a/drivers/usb/gadget/function/f_serial.c b/drivers/usb/gadget/function/f_serial.c
index c860f30a0ea2..1406255d0865 100644
--- a/drivers/usb/gadget/function/f_serial.c
+++ b/drivers/usb/gadget/function/f_serial.c
@@ -266,6 +266,24 @@ static struct configfs_item_operations serial_item_ops = {
 	.release	= serial_attr_release,
 };
 
+#ifdef CONFIG_U_SERIAL_CONSOLE
+
+static ssize_t f_serial_console_store(struct config_item *item,
+		const char *page, size_t count)
+{
+	return gserial_set_console(to_f_serial_opts(item)->port_num,
+				   page, count);
+}
+
+static ssize_t f_serial_console_show(struct config_item *item, char *page)
+{
+	return gserial_get_console(to_f_serial_opts(item)->port_num, page);
+}
+
+CONFIGFS_ATTR(f_serial_, console);
+
+#endif /* CONFIG_U_SERIAL_CONSOLE */
+
 static ssize_t f_serial_port_num_show(struct config_item *item, char *page)
 {
 	return sprintf(page, "%u\n", to_f_serial_opts(item)->port_num);
@@ -274,6 +292,9 @@ static ssize_t f_serial_port_num_show(struct config_item *item, char *page)
 CONFIGFS_ATTR_RO(f_serial_, port_num);
 
 static struct configfs_attribute *acm_attrs[] = {
+#ifdef CONFIG_U_SERIAL_CONSOLE
+	&f_serial_attr_console,
+#endif
 	&f_serial_attr_port_num,
 	NULL,
 };
diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index 62280c23cde2..0da00546006f 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1081,6 +1081,54 @@ static void gs_console_exit(struct gs_port *port)
 	port->console = NULL;
 }
 
+ssize_t gserial_set_console(unsigned char port_num, const char *page, size_t count)
+{
+	struct gs_port *port;
+	bool enable;
+	int ret;
+
+	ret = strtobool(page, &enable);
+	if (ret)
+		return ret;
+
+	mutex_lock(&ports[port_num].lock);
+	port = ports[port_num].port;
+
+	if (WARN_ON(port == NULL)) {
+		ret = -ENXIO;
+		goto out;
+	}
+
+	if (enable)
+		ret = gs_console_init(port);
+	else
+		gs_console_exit(port);
+out:
+	mutex_unlock(&ports[port_num].lock);
+
+	return ret < 0 ? ret : count;
+}
+EXPORT_SYMBOL_GPL(gserial_set_console);
+
+ssize_t gserial_get_console(unsigned char port_num, char *page)
+{
+	struct gs_port *port;
+	ssize_t ret;
+
+	mutex_lock(&ports[port_num].lock);
+	port = ports[port_num].port;
+
+	if (WARN_ON(port == NULL))
+		ret = -ENXIO;
+	else
+		ret = sprintf(page, "%u\n", !!port->console);
+
+	mutex_unlock(&ports[port_num].lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(gserial_get_console);
+
 #else
 
 static int gs_console_connect(struct gs_port *port)
diff --git a/drivers/usb/gadget/function/u_serial.h b/drivers/usb/gadget/function/u_serial.h
index 8b472b0c8cb4..e5b08ab8cf7a 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -58,6 +58,13 @@ int gserial_alloc_line_no_console(unsigned char *port_line);
 int gserial_alloc_line(unsigned char *port_line);
 void gserial_free_line(unsigned char port_line);
 
+#ifdef CONFIG_U_SERIAL_CONSOLE
+
+ssize_t gserial_set_console(unsigned char port_num, const char *page, size_t count);
+ssize_t gserial_get_console(unsigned char port_num, char *page);
+
+#endif /* CONFIG_U_SERIAL_CONSOLE */
+
 /* connect/disconnect is handled by individual functions */
 int gserial_connect(struct gserial *, u8 port_num);
 void gserial_disconnect(struct gserial *);
-- 
2.20.1

[PATCH v6 6/7] usb: gadget: legacy/serial: allow dynamic removal

[Michał Mirosław]

Legacy serial USB gadget is still useful as an early console,
before userspace is up. Later it could be replaced with proper
configfs-configured composite gadget - that use case is enabled
by this patch.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

---
  v6: rebased on balbi/testing/next
  v5: no changes
  v4: initial revision, new in the patchset

---
 drivers/usb/gadget/legacy/serial.c | 49 +++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/legacy/serial.c b/drivers/usb/gadget/legacy/serial.c
index de30d7628eef..da44f89f5e73 100644
--- a/drivers/usb/gadget/legacy/serial.c
+++ b/drivers/usb/gadget/legacy/serial.c
@@ -97,6 +97,36 @@ static unsigned n_ports = 1;
 module_param(n_ports, uint, 0);
 MODULE_PARM_DESC(n_ports, "number of ports to create, default=1");
 
+static bool enable = true;
+
+static int switch_gserial_enable(bool do_enable);
+
+static int enable_set(const char *s, const struct kernel_param *kp)
+{
+	bool do_enable;
+	int ret;
+
+	if (!s)	/* called for no-arg enable == default */
+		return 0;
+
+	ret = strtobool(s, &do_enable);
+	if (ret || enable == do_enable)
+		return ret;
+
+	ret = switch_gserial_enable(do_enable);
+	if (!ret)
+		enable = do_enable;
+
+	return ret;
+}
+
+static const struct kernel_param_ops enable_ops = {
+	.set = enable_set,
+	.get = param_get_bool,
+};
+
+module_param_cb(enable, &enable_ops, &enable, 0644);
+
 /*-------------------------------------------------------------------------*/
 
 static struct usb_configuration serial_config_driver = {
@@ -240,6 +270,19 @@ static struct usb_composite_driver gserial_driver = {
 	.unbind		= gs_unbind,
 };
 
+static int switch_gserial_enable(bool do_enable)
+{
+	if (!serial_config_driver.label)
+		/* init() was not called, yet */
+		return 0;
+
+	if (do_enable)
+		return usb_composite_probe(&gserial_driver);
+
+	usb_composite_unregister(&gserial_driver);
+	return 0;
+}
+
 static int __init init(void)
 {
 	/* We *could* export two configs; that'd be much cleaner...
@@ -266,12 +309,16 @@ static int __init init(void)
 	}
 	strings_dev[STRING_DESCRIPTION_IDX].s = serial_config_driver.label;
 
+	if (!enable)
+		return 0;
+
 	return usb_composite_probe(&gserial_driver);
 }
 module_init(init);
 
 static void __exit cleanup(void)
 {
-	usb_composite_unregister(&gserial_driver);
+	if (enable)
+		usb_composite_unregister(&gserial_driver);
 }
 module_exit(cleanup);
-- 
2.20.1

[PATCH v6 5/7] usb: gadget: u_serial: diagnose missed console messages

[Michał Mirosław]

Insert markers in console stream marking places where data
is missing. This makes the hole in the data stand out clearly
instead of glueing together unrelated messages.

Example output as seen from USB host side:

[    0.064078] pinctrl core: registered pin 16 (UART3_RTS_N PC0) on 70000868.pinmux
[    0.064130] pinctrl
[missed 114987 bytes]
[    4.302299] udevd[134]: starting version 3.2.5
[    4.306845] random: udevd: uninitialized urandom read (16 bytes read)

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
  v6: rebased on balbi/testing/next
  v5: no changes
  v4: no changes
  v3: added example output
    + lowercase "missed"
  v2: commit message massage

---
 drivers/usb/gadget/function/u_serial.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index 0da00546006f..a248ed0fd5d2 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -88,6 +88,7 @@ struct gs_console {
 	spinlock_t		lock;
 	struct usb_request	*req;
 	struct kfifo		buf;
+	size_t			missed;
 };
 
 /*
@@ -931,6 +932,15 @@ static void __gs_console_push(struct gs_console *cons)
 	if (!size)
 		return;
 
+	if (cons->missed && ep->maxpacket >= 64) {
+		char buf[64];
+		size_t len;
+
+		len = sprintf(buf, "\n[missed %zu bytes]\n", cons->missed);
+		kfifo_in(&cons->buf, buf, len);
+		cons->missed = 0;
+	}
+
 	req->length = size;
 	if (usb_ep_queue(ep, req, GFP_ATOMIC))
 		req->length = 0;
@@ -952,10 +962,13 @@ static void gs_console_write(struct console *co,
 {
 	struct gs_console *cons = container_of(co, struct gs_console, console);
 	unsigned long flags;
+	size_t n;
 
 	spin_lock_irqsave(&cons->lock, flags);
 
-	kfifo_in(&cons->buf, buf, count);
+	n = kfifo_in(&cons->buf, buf, count);
+	if (n < count)
+		cons->missed += count - n;
 
 	if (cons->req && !cons->req->length)
 		schedule_work(&cons->work);
-- 
2.20.1

[PATCH v6 7/7] usb: gadget: u_serial: use mutex for serialising open()s

[Michał Mirosław]

Remove home-made waiting mechanism from gs_open() and rely on
portmaster's mutex to do the job.

Note: This releases thread waiting on close() when another thread
open()s simultaneously.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

---
  v6: initial revision, new in the patchset

---
 drivers/usb/gadget/function/u_serial.c | 112 ++++++++-----------------
 1 file changed, 37 insertions(+), 75 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index a248ed0fd5d2..f986e5c55974 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -104,7 +104,6 @@ struct gs_port {
 	struct gs_console	*console;
 #endif
 
-	bool			openclose;	/* open/close in progress */
 	u8			port_num;
 
 	struct list_head	read_pool;
@@ -588,82 +587,45 @@ static int gs_open(struct tty_struct *tty, struct file *file)
 {
 	int		port_num = tty->index;
 	struct gs_port	*port;
-	int		status;
+	int		status = 0;
 
-	do {
-		mutex_lock(&ports[port_num].lock);
-		port = ports[port_num].port;
-		if (!port)
-			status = -ENODEV;
-		else {
-			spin_lock_irq(&port->port_lock);
+	mutex_lock(&ports[port_num].lock);
+	port = ports[port_num].port;
+	if (!port) {
+		status = -ENODEV;
+		goto out;
+	}
 
-			/* already open?  Great. */
-			if (port->port.count) {
-				status = 0;
-				port->port.count++;
-
-			/* currently opening/closing? wait ... */
-			} else if (port->openclose) {
-				status = -EBUSY;
-
-			/* ... else we do the work */
-			} else {
-				status = -EAGAIN;
-				port->openclose = true;
-			}
-			spin_unlock_irq(&port->port_lock);
-		}
-		mutex_unlock(&ports[port_num].lock);
-
-		switch (status) {
-		default:
-			/* fully handled */
-			return status;
-		case -EAGAIN:
-			/* must do the work */
-			break;
-		case -EBUSY:
-			/* wait for EAGAIN task to finish */
-			msleep(1);
-			/* REVISIT could have a waitchannel here, if
-			 * concurrent open performance is important
-			 */
-			break;
-		}
-	} while (status != -EAGAIN);
-
-	/* Do the "real open" */
 	spin_lock_irq(&port->port_lock);
 
 	/* allocate circular buffer on first open */
 	if (!kfifo_initialized(&port->port_write_buf)) {
 
 		spin_unlock_irq(&port->port_lock);
+
+		/*
+		 * portmaster's mutex still protects from simultaneous open(),
+		 * and close() can't happen, yet.
+		 */
+
 		status = kfifo_alloc(&port->port_write_buf,
 				     WRITE_BUF_SIZE, GFP_KERNEL);
-		spin_lock_irq(&port->port_lock);
-
 		if (status) {
 			pr_debug("gs_open: ttyGS%d (%p,%p) no buffer\n",
-				port->port_num, tty, file);
-			port->openclose = false;
-			goto exit_unlock_port;
+				 port_num, tty, file);
+			goto out;
 		}
+
+		spin_lock_irq(&port->port_lock);
 	}
 
-	/* REVISIT if REMOVED (ports[].port NULL), abort the open
-	 * to let rmmod work faster (but this way isn't wrong).
-	 */
-
-	/* REVISIT maybe wait for "carrier detect" */
+	/* already open?  Great. */
+	if (port->port.count++)
+		goto exit_unlock_port;
 
 	tty->driver_data = port;
 	port->port.tty = tty;
 
-	port->port.count = 1;
-	port->openclose = false;
-
 	/* if connected, start the I/O stream */
 	if (port->port_usb) {
 		struct gserial	*gser = port->port_usb;
@@ -677,20 +639,21 @@ static int gs_open(struct tty_struct *tty, struct file *file)
 
 	pr_debug("gs_open: ttyGS%d (%p,%p)\n", port->port_num, tty, file);
 
-	status = 0;
-
 exit_unlock_port:
 	spin_unlock_irq(&port->port_lock);
+out:
+	mutex_unlock(&ports[port_num].lock);
 	return status;
 }
 
-static int gs_writes_finished(struct gs_port *p)
+static int gs_close_flush_done(struct gs_port *p)
 {
 	int cond;
 
-	/* return true on disconnect or empty buffer */
+	/* return true on disconnect or empty buffer or if raced with open() */
 	spin_lock_irq(&p->port_lock);
-	cond = (p->port_usb == NULL) || !kfifo_len(&p->port_write_buf);
+	cond = p->port_usb == NULL || !kfifo_len(&p->port_write_buf) ||
+		p->port.count > 1;
 	spin_unlock_irq(&p->port_lock);
 
 	return cond;
@@ -704,6 +667,7 @@ static void gs_close(struct tty_struct *tty, struct file *file)
 	spin_lock_irq(&port->port_lock);
 
 	if (port->port.count != 1) {
+raced_with_open:
 		if (port->port.count == 0)
 			WARN_ON(1);
 		else
@@ -713,12 +677,6 @@ static void gs_close(struct tty_struct *tty, struct file *file)
 
 	pr_debug("gs_close: ttyGS%d (%p,%p) ...\n", port->port_num, tty, file);
 
-	/* mark port as closing but in use; we can drop port lock
-	 * and sleep if necessary
-	 */
-	port->openclose = true;
-	port->port.count = 0;
-
 	gser = port->port_usb;
 	if (gser && gser->disconnect)
 		gser->disconnect(gser);
@@ -729,9 +687,13 @@ static void gs_close(struct tty_struct *tty, struct file *file)
 	if (kfifo_len(&port->port_write_buf) > 0 && gser) {
 		spin_unlock_irq(&port->port_lock);
 		wait_event_interruptible_timeout(port->drain_wait,
-					gs_writes_finished(port),
+					gs_close_flush_done(port),
 					GS_CLOSE_TIMEOUT * HZ);
 		spin_lock_irq(&port->port_lock);
+
+		if (port->port.count != 1)
+			goto raced_with_open;
+
 		gser = port->port_usb;
 	}
 
@@ -744,10 +706,9 @@ static void gs_close(struct tty_struct *tty, struct file *file)
 	else
 		kfifo_reset(&port->port_write_buf);
 
+	port->port.count = 0;
 	port->port.tty = NULL;
 
-	port->openclose = false;
-
 	pr_debug("gs_close: ttyGS%d (%p,%p) done!\n",
 			port->port_num, tty, file);
 
@@ -1207,8 +1168,9 @@ static int gs_closed(struct gs_port *port)
 	int cond;
 
 	spin_lock_irq(&port->port_lock);
-	cond = (port->port.count == 0) && !port->openclose;
+	cond = port->port.count == 0;
 	spin_unlock_irq(&port->port_lock);
+
 	return cond;
 }
 
@@ -1413,7 +1375,7 @@ void gserial_disconnect(struct gserial *gser)
 
 	port->port_usb = NULL;
 	gser->ioport = NULL;
-	if (port->port.count > 0 || port->openclose) {
+	if (port->port.count > 0) {
 		wake_up_interruptible(&port->drain_wait);
 		if (port->port.tty)
 			tty_hangup(port->port.tty);
@@ -1426,7 +1388,7 @@ void gserial_disconnect(struct gserial *gser)
 
 	/* finally, free any unused/unusable I/O buffers */
 	spin_lock_irqsave(&port->port_lock, flags);
-	if (port->port.count == 0 && !port->openclose)
+	if (port->port.count == 0)
 		kfifo_free(&port->port_write_buf);
 	gs_free_requests(gser->out, &port->read_pool, NULL);
 	gs_free_requests(gser->out, &port->read_queue, NULL);
-- 
2.20.1