[PATCH v5 0/6] usb: gadget: u_serial: console improvements

[PATCH v5 0/6] 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 usb-next tree. You can also pull from:
   https://rere.qmqm.pl/git/linux  usb-console


Michał Mirosław (6):
  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

 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 | 420 ++++++++++++++-----------
 drivers/usb/gadget/function/u_serial.h |   8 +
 drivers/usb/gadget/legacy/serial.c     |  49 ++-
 6 files changed, 333 insertions(+), 188 deletions(-)

-- 
2.20.1

[PATCH v5 1/6] 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>

---
  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 v5 2/6] 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>

---
  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 v5 3/6] 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>

---
  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 v5 5/6] 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>

---
  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 v5 4/6] 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>

---
  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 v5 6/6] 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>

---
  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

Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support

[Baolin Wang]

Hi Michal,

On Mon, 22 Jul 2019 at 23:26, Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
>
> 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.

Could you elaborate on why changing kthread to a workqueue? The
purpose of using kthread here is considering that the kthead has a
better scheduler response than pooled kworker.

>
> 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>
>
> ---
>   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
>


-- 
Baolin Wang
Best Regards

Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support

[Ladislav Michl]

On Tue, Jul 23, 2019 at 10:18:15AM +0800, Baolin Wang wrote:
> Hi Michal,
> 
> On Mon, 22 Jul 2019 at 23:26, Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> >
> > 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.
> 
> Could you elaborate on why changing kthread to a workqueue? The
> purpose of using kthread here is considering that the kthead has a
> better scheduler response than pooled kworker.

...which is not that relevant there. Current kthead implementation
is buggy, see this series for what needs to be done to fix it:
https://marc.info/?l=linux-usb&m=156305214227371&w=2
and as Michał's fix is superior to fixing kthread I'm voting for his
solution. Only one of my patches is still needed and I'll resend
once this fix hits -next.

> >
> > 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>
> >
> > ---
> >   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
> >
> 
> 
> -- 
> Baolin Wang
> Best Regards

Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support

[Michał Mirosław]

On Tue, Jul 23, 2019 at 10:18:15AM +0800, Baolin Wang wrote:
> Hi Michal,
> 
> On Mon, 22 Jul 2019 at 23:26, Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> >
> > 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.
> Could you elaborate on why changing kthread to a workqueue? The
> purpose of using kthread here is considering that the kthead has a
> better scheduler response than pooled kworker.

Mainly locking problems and single-instance design. The kthread looked
like it's reimplementing workqueue mechanics. If the scheduling latency
turns out important, the workqueue used can be changed to dedicated one
or one with higher priority.

Best Regards,
Michał Mirosław

Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support

[Felipe Balbi]

Hi,

Michał Mirosław <mirq-linux@rere.qmqm.pl> writes:
> 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>

checking file drivers/usb/gadget/function/u_serial.c
Hunk #7 FAILED at 1206.
Hunk #8 succeeded at 1302 (offset -2 lines).
Hunk #9 succeeded at 1334 (offset -2 lines).
Hunk #10 succeeded at 1363 (offset -2 lines).
1 out of 10 hunks FAILED

Could you rebase on my testing/next?

-- 
balbi

Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support

[Michał Mirosław]

On Fri, Aug 09, 2019 at 03:05:55PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Michał Mirosław <mirq-linux@rere.qmqm.pl> writes:
> > 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>
> 
> checking file drivers/usb/gadget/function/u_serial.c
> Hunk #7 FAILED at 1206.
> Hunk #8 succeeded at 1302 (offset -2 lines).
> Hunk #9 succeeded at 1334 (offset -2 lines).
> Hunk #10 succeeded at 1363 (offset -2 lines).
> 1 out of 10 hunks FAILED
> 
> Could you rebase on my testing/next?

Sure. Should be ready in a few minutes.

Best Regards,
Michał Mirosław

Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support

[Ladislav Michl]

Hi Felipe,

On Sat, Aug 10, 2019 at 10:11:05AM +0200, Michał Mirosław wrote:
> On Fri, Aug 09, 2019 at 03:05:55PM +0300, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > Michał Mirosław <mirq-linux@rere.qmqm.pl> writes:
> > > 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>
> > 
> > checking file drivers/usb/gadget/function/u_serial.c
> > Hunk #7 FAILED at 1206.
> > Hunk #8 succeeded at 1302 (offset -2 lines).
> > Hunk #9 succeeded at 1334 (offset -2 lines).
> > Hunk #10 succeeded at 1363 (offset -2 lines).
> > 1 out of 10 hunks FAILED
> > 
> > Could you rebase on my testing/next?

I do not see this patch in -next and there were no more comments from you.
Is there anything else you need to review patchset?

Thanks,
	ladis

> Sure. Should be ready in a few minutes.
> 
> Best Regards,
> Michał Mirosław