* [PATCH v5 1/6] usb: gadget: u_serial: add missing port entry locking
2019-07-22 15:26 [PATCH v5 0/6] usb: gadget: u_serial: console improvements Michał Mirosław
@ 2019-07-22 15:26 ` Michał Mirosław
2019-07-22 15:26 ` [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support Michał Mirosław
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Michał Mirosław @ 2019-07-22 15:26 UTC (permalink / raw)
To: linux-usb; +Cc: Felipe Balbi, Greg Kroah-Hartman, Ladislav Michl
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support
2019-07-22 15:26 [PATCH v5 0/6] usb: gadget: u_serial: console improvements Michał Mirosław
2019-07-22 15:26 ` [PATCH v5 1/6] usb: gadget: u_serial: add missing port entry locking Michał Mirosław
@ 2019-07-22 15:26 ` Michał Mirosław
2019-07-23 2:18 ` Baolin Wang
2019-08-09 12:05 ` Felipe Balbi
2019-07-22 15:26 ` [PATCH v5 3/6] usb: gadget: u_serial: make OBEX port not a console Michał Mirosław
` (3 subsequent siblings)
5 siblings, 2 replies; 13+ messages in thread
From: Michał Mirosław @ 2019-07-22 15:26 UTC (permalink / raw)
To: linux-usb; +Cc: Felipe Balbi, Greg Kroah-Hartman, Ladislav Michl
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support
2019-07-22 15:26 ` [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support Michał Mirosław
@ 2019-07-23 2:18 ` Baolin Wang
2019-07-23 8:15 ` Ladislav Michl
2019-07-23 12:06 ` Michał Mirosław
2019-08-09 12:05 ` Felipe Balbi
1 sibling, 2 replies; 13+ messages in thread
From: Baolin Wang @ 2019-07-23 2:18 UTC (permalink / raw)
To: Michał Mirosław
Cc: USB, Felipe Balbi, Greg Kroah-Hartman, Ladislav Michl
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support
2019-07-23 2:18 ` Baolin Wang
@ 2019-07-23 8:15 ` Ladislav Michl
2019-07-23 12:06 ` Michał Mirosław
1 sibling, 0 replies; 13+ messages in thread
From: Ladislav Michl @ 2019-07-23 8:15 UTC (permalink / raw)
To: Baolin Wang
Cc: Michał Mirosław, USB, Felipe Balbi, Greg Kroah-Hartman
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support
2019-07-23 2:18 ` Baolin Wang
2019-07-23 8:15 ` Ladislav Michl
@ 2019-07-23 12:06 ` Michał Mirosław
1 sibling, 0 replies; 13+ messages in thread
From: Michał Mirosław @ 2019-07-23 12:06 UTC (permalink / raw)
To: Baolin Wang; +Cc: USB, Felipe Balbi, Greg Kroah-Hartman, 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.
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support
2019-07-22 15:26 ` [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support Michał Mirosław
2019-07-23 2:18 ` Baolin Wang
@ 2019-08-09 12:05 ` Felipe Balbi
2019-08-10 8:11 ` Michał Mirosław
1 sibling, 1 reply; 13+ messages in thread
From: Felipe Balbi @ 2019-08-09 12:05 UTC (permalink / raw)
To: Michał Mirosław, linux-usb; +Cc: Greg Kroah-Hartman, Ladislav Michl
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support
2019-08-09 12:05 ` Felipe Balbi
@ 2019-08-10 8:11 ` Michał Mirosław
2019-09-11 10:05 ` Ladislav Michl
0 siblings, 1 reply; 13+ messages in thread
From: Michał Mirosław @ 2019-08-10 8:11 UTC (permalink / raw)
To: Felipe Balbi; +Cc: linux-usb, Greg Kroah-Hartman, Ladislav Michl
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support
2019-08-10 8:11 ` Michał Mirosław
@ 2019-09-11 10:05 ` Ladislav Michl
0 siblings, 0 replies; 13+ messages in thread
From: Ladislav Michl @ 2019-09-11 10:05 UTC (permalink / raw)
To: Felipe Balbi; +Cc: linux-usb, Michał Mirosław, Greg Kroah-Hartman
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 3/6] usb: gadget: u_serial: make OBEX port not a console
2019-07-22 15:26 [PATCH v5 0/6] usb: gadget: u_serial: console improvements Michał Mirosław
2019-07-22 15:26 ` [PATCH v5 1/6] usb: gadget: u_serial: add missing port entry locking Michał Mirosław
2019-07-22 15:26 ` [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support Michał Mirosław
@ 2019-07-22 15:26 ` Michał Mirosław
2019-07-22 15:26 ` [PATCH v5 4/6] usb: gadget: u_serial: allow more console gadget ports Michał Mirosław
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Michał Mirosław @ 2019-07-22 15:26 UTC (permalink / raw)
To: linux-usb; +Cc: Felipe Balbi, Greg Kroah-Hartman, Ladislav Michl
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v5 4/6] usb: gadget: u_serial: allow more console gadget ports
2019-07-22 15:26 [PATCH v5 0/6] usb: gadget: u_serial: console improvements Michał Mirosław
` (2 preceding siblings ...)
2019-07-22 15:26 ` [PATCH v5 3/6] usb: gadget: u_serial: make OBEX port not a console Michał Mirosław
@ 2019-07-22 15:26 ` Michał Mirosław
2019-07-22 15:26 ` [PATCH v5 5/6] usb: gadget: u_serial: diagnose missed console messages Michał Mirosław
2019-07-22 15:26 ` [PATCH v5 6/6] usb: gadget: legacy/serial: allow dynamic removal Michał Mirosław
5 siblings, 0 replies; 13+ messages in thread
From: Michał Mirosław @ 2019-07-22 15:26 UTC (permalink / raw)
To: linux-usb; +Cc: Felipe Balbi, Greg Kroah-Hartman, Ladislav Michl
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v5 5/6] usb: gadget: u_serial: diagnose missed console messages
2019-07-22 15:26 [PATCH v5 0/6] usb: gadget: u_serial: console improvements Michał Mirosław
` (3 preceding siblings ...)
2019-07-22 15:26 ` [PATCH v5 4/6] usb: gadget: u_serial: allow more console gadget ports Michał Mirosław
@ 2019-07-22 15:26 ` Michał Mirosław
2019-07-22 15:26 ` [PATCH v5 6/6] usb: gadget: legacy/serial: allow dynamic removal Michał Mirosław
5 siblings, 0 replies; 13+ messages in thread
From: Michał Mirosław @ 2019-07-22 15:26 UTC (permalink / raw)
To: linux-usb; +Cc: Felipe Balbi, Greg Kroah-Hartman, Ladislav Michl
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v5 6/6] usb: gadget: legacy/serial: allow dynamic removal
2019-07-22 15:26 [PATCH v5 0/6] usb: gadget: u_serial: console improvements Michał Mirosław
` (4 preceding siblings ...)
2019-07-22 15:26 ` [PATCH v5 5/6] usb: gadget: u_serial: diagnose missed console messages Michał Mirosław
@ 2019-07-22 15:26 ` Michał Mirosław
5 siblings, 0 replies; 13+ messages in thread
From: Michał Mirosław @ 2019-07-22 15:26 UTC (permalink / raw)
To: linux-usb; +Cc: Felipe Balbi, Greg Kroah-Hartman, Ladislav Michl
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
^ permalink raw reply related [flat|nested] 13+ messages in thread