Re: [PATCH] usb: gadget: Add the console support for usb-to-serial port

[Andy Shevchenko]

On Mon, Nov 16, 2015 at 9:05 AM, Baolin Wang <baolin.wang@xxxxxxxxxx> wrote:
> It dose not work when we want to use the usb-to-serial port based
> on one usb gadget as a console. Thus this patch adds the console
> initialization to support this request.


> @@ -79,6 +80,16 @@
>   */
>  #define QUEUE_SIZE             16
>  #define WRITE_BUF_SIZE         8192            /* TX only */
> +#define GS_BUFFER_SIZE         (4096)

Redundant parens

> +#define GS_CONSOLE_BUF_SIZE    (2 * GS_BUFFER_SIZE)
> +
> +struct gscons_info {
> +       struct gs_port          *port;
> +       struct tty_driver       *tty_driver;
> +       struct work_struct      work;
> +       int                     buf_tail;
> +       char                    buf[GS_CONSOLE_BUF_SIZE];

Can't be malloced once?


> +static struct usb_request *gs_request_new(struct usb_ep *ep, int buffer_size)
> +{
> +       struct usb_request *req = usb_ep_alloc_request(ep, GFP_ATOMIC);
> +
> +       if (!req)

For sake of readability it's better to have assignment explicitly before 'if'.

> +               return NULL;
> +
> +       /* now allocate buffers for the requests */
> +       req->buf = kmalloc(buffer_size, 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) {

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)
> +{
> +       if (req->status != 0 && req->status != -ECONNRESET)
> +               return;

Something missed here. Currently it's no-op.

> +}
> +
> +static struct console gserial_cons;
> +static int gs_console_connect(void)
> +{
> +       struct gscons_info *info = gserial_cons.data;
> +       int port_num = gserial_cons.index;
> +       struct usb_request *req;
> +       struct gs_port *port;
> +       struct usb_ep *ep;
> +
> +       if (port_num >= MAX_U_SERIAL_PORTS || port_num < 0) {
> +               pr_err("%s: port num [%d] exceeds the range.\n",
> +                      __func__, port_num);
> +               return -ENXIO;
> +       }
> +
> +       port = ports[port_num].port;
> +       if (!port) {
> +               pr_err("%s: serial line [%d] not allocated.\n",
> +                      __func__, port_num);
> +               return -ENODEV;
> +       }
> +
> +       if (!port->port_usb) {
> +               pr_err("%s: no port usb.\n", __func__);

Starting from here could it be dev_err and so on?

> +               return -ENODEV;
> +       }
> +
> +       ep = port->port_usb->in;
> +       if (!ep) {
> +               pr_err("%s: no usb endpoint.\n", __func__);
> +               return -ENXIO;
> +       }
> +
> +       req = port->console_req;
> +       if (!req) {
> +               req = gs_request_new(ep, GS_BUFFER_SIZE);
> +               if (!req) {
> +                       pr_err("%s: request fail.\n", __func__);
> +                       return -ENOMEM;
> +               }
> +               req->complete = gs_complete_out;
> +       }
> +
> +       info->port = port;
> +
> +       pr_debug("%s: port[%d] console connect!\n", __func__, port_num);

Dynamic debug will add function name if asked.

> +       return 0;
> +}
> +
> +static void gs_console_work(struct work_struct *work)
> +{
> +       struct gscons_info *info = container_of(work, struct gscons_info, work);
> +       struct gs_port *port = info->port;
> +       struct usb_request *req;
> +       struct usb_ep *ep;
> +       int xfer, ret, count;
> +       char *p;
> +
> +       if (!port || !port->port_usb)
> +               return;
> +
> +       req = port->console_req;
> +       ep = port->port_usb->in;
> +       if (!req || !ep)
> +               return;
> +
> +       spin_lock_irq(&port->port_lock);
> +       count = info->buf_tail;
> +       p = info->buf;
> +
> +       while (count > 0 && !port->write_busy) {

> +               if (count > GS_BUFFER_SIZE)
> +                       xfer = GS_BUFFER_SIZE;
> +               else
> +                       xfer = count;

xfer = min_t(â, count, GS_BUFFER_SIZE);

> +
> +               memcpy(req->buf, p, xfer);
> +               req->length = xfer;
> +
> +               port->write_busy = true;
> +               spin_unlock(&port->port_lock);
> +               ret = usb_ep_queue(ep, req, GFP_ATOMIC);
> +               spin_lock(&port->port_lock);
> +               port->write_busy = false;
> +               if (ret < 0)
> +                       break;
> +
> +               p += xfer;
> +               count -= xfer;
> +       }
> +
> +       info->buf_tail -= count;
> +       spin_unlock_irq(&port->port_lock);
> +}
> +
> +static int gs_console_setup(struct console *co, char *options)
> +{
> +       struct gscons_info *gscons_info;
> +
> +       gscons_info = kzalloc(sizeof(struct gscons_info), GFP_KERNEL);
> +       if (!gscons_info)
> +               return -ENOMEM;
> +
> +       gscons_info->port = NULL;
> +       gscons_info->tty_driver = gs_tty_driver;
> +       INIT_WORK(&gscons_info->work, gs_console_work);
> +       gscons_info->buf_tail = 0;
> +       co->data = gscons_info;
> +
> +       return 0;
> +}
> +
> +static void gs_console_write(struct console *co,
> +                            const char *buf, unsigned count)
> +{
> +       struct gscons_info *info = co->data;
> +       int avail, xfer;
> +       char *p;
> +
> +       avail = GS_CONSOLE_BUF_SIZE - info->buf_tail;

> +       if (count > avail)
> +               xfer = avail;
> +       else
> +               xfer = count;

Ditto.

> +
> +       p = &info->buf[info->buf_tail];
> +       memcpy(p, buf, xfer);
> +       info->buf_tail += xfer;
> +
> +       schedule_work(&info->work);
> +}
> +
> +static struct tty_driver *gs_console_device(struct console *co, int *index)
> +{
> +       struct gscons_info *info = co->data;
> +
> +       *index = co->index;
> +       return info->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,
> +};
> +
> +static void gserial_console_init(void)
> +{
> +       register_console(&gserial_cons);
> +}
> +
> +static void gserial_console_exit(void)
> +{
> +       struct gscons_info *info = gserial_cons.data;
> +       struct gs_port *port = info->port;
> +       struct usb_request *req;
> +       struct usb_ep *ep;
> +
> +       if (port && port->port_usb) {
> +               req = port->console_req;
> +               ep = port->port_usb->in;
> +               gs_request_free(req, ep);
> +       }
> +
> +       kfree(info);
> +       unregister_console(&gserial_cons);
> +}
> +
> +#else
> +
> +static int gs_console_connect(void)
> +{
> +       return 0;
> +}
> +
> +static void gserial_console_init(void)
> +{
> +}
> +
> +static void gserial_console_exit(void)
> +{
> +}
> +
> +#endif
> +
>  /**
>   * gserial_connect - notify TTY I/O glue that USB link is active
>   * @gser: the function, set up with endpoints and descriptors
> @@ -1219,6 +1453,7 @@ int gserial_connect(struct gserial *gser, u8 port_num)
>                         gser->disconnect(gser);
>         }
>
> +       status = gs_console_connect();
>         spin_unlock_irqrestore(&port->port_lock, flags);
>
>         return status;
> @@ -1320,6 +1555,8 @@ static int userial_init(void)
>                 goto fail;
>         }
>
> +       gserial_console_init();
> +
>         pr_debug("%s: registered %d ttyGS* device%s\n", __func__,
>                         MAX_U_SERIAL_PORTS,
>                         (MAX_U_SERIAL_PORTS == 1) ? "" : "s");
> @@ -1334,6 +1571,7 @@ module_init(userial_init);
>
>  static void userial_cleanup(void)
>  {
> +       gserial_console_exit();
>         tty_unregister_driver(gs_tty_driver);
>         put_tty_driver(gs_tty_driver);
>         gs_tty_driver = NULL;
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/