Re: [PATCH v3 4/6] usb: core: add power sequence handling for USB devices

[Peter Chen]

On Wed, Jul 27, 2016 at 09:25:11AM -0700, Joshua Clayton wrote:
> 
> 
> On 07/20/2016 02:40 AM, Peter Chen wrote:
> > Some hard-wired USB devices need to do power sequence to let the
> > device work normally, the typical power sequence like: enable USB
> > PHY clock, toggle reset pin, etc. But current Linux USB driver
> > lacks of such code to do it, it may cause some hard-wired USB devices
> > works abnormal or can't be recognized by controller at all.
> >
> > In this patch, it calls power sequence library APIs to finish
> > the power sequence events. At first, it calls pwrseq_alloc_generic
> > to create a generic power sequence instance, then it will do power
> > on sequence at hub's probe for all devices under this hub
> > (includes root hub).
> >
> > At hub_disconnect, it will do power off sequence which is at powered
> > on list.
> >
> > Signed-off-by: Peter Chen 
> > ---
> >  drivers/usb/core/Makefile |   1 +
> >  drivers/usb/core/hub.c|  12 --
> >  drivers/usb/core/hub.h|  12 ++
> >  drivers/usb/core/pwrseq.c | 100 
> > ++
> >  4 files changed, 122 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/usb/core/pwrseq.c
> >
> > diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> > index da36b78..5fd2f54 100644
> > --- a/drivers/usb/core/Makefile
> > +++ b/drivers/usb/core/Makefile
> > @@ -10,5 +10,6 @@ usbcore-y += port.o
> >  usbcore-$(CONFIG_OF)   += of.o
> >  usbcore-$(CONFIG_PCI)  += hcd-pci.o
> >  usbcore-$(CONFIG_ACPI) += usb-acpi.o
> > +usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o
> >  
> >  obj-$(CONFIG_USB)  += usbcore.o
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index bee1351..a346a8b 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -1700,6 +1700,7 @@ static void hub_disconnect(struct usb_interface *intf)
> > hub->error = 0;
> > hub_quiesce(hub, HUB_DISCONNECT);
> >  
> > +   hub_pwrseq_off(hub);
> > mutex_lock(_port_peer_mutex);
> >  
> > /* Avoid races with recursively_mark_NOTATTACHED() */
> > @@ -1733,6 +1734,7 @@ static int hub_probe(struct usb_interface *intf, 
> > const struct usb_device_id *id)
> > struct usb_endpoint_descriptor *endpoint;
> > struct usb_device *hdev;
> > struct usb_hub *hub;
> > +   int ret = -ENODEV;
> >  
> > desc = intf->cur_altsetting;
> > hdev = interface_to_usbdev(intf);
> > @@ -1839,6 +1841,7 @@ descriptor_error:
> > INIT_DELAYED_WORK(>leds, led_work);
> > INIT_DELAYED_WORK(>init_work, NULL);
> > INIT_WORK(>events, hub_event);
> > +   INIT_LIST_HEAD(>pwrseq_on_list);
> > usb_get_intf(intf);
> > usb_get_dev(hdev);
> >  
> > @@ -1852,11 +1855,14 @@ descriptor_error:
> > if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
> > hub->quirk_check_port_auto_suspend = 1;
> >  
> > -   if (hub_configure(hub, endpoint) >= 0)
> > -   return 0;
> > +   if (hub_configure(hub, endpoint) >= 0) {
> > +   ret = hub_pwrseq_on(hub);
> > +   if (!ret)
> > +   return 0;
> > +   }
> >  
> > hub_disconnect(intf);
> > -   return -ENODEV;
> > +   return ret;
> >  }
> >  
> >  static int
> > diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> > index 34c1a7e..9473f6f 100644
> > --- a/drivers/usb/core/hub.h
> > +++ b/drivers/usb/core/hub.h
> > @@ -78,6 +78,7 @@ struct usb_hub {
> > struct delayed_work init_work;
> > struct work_struct  events;
> > struct usb_port **ports;
> > +   struct list_headpwrseq_on_list; /* powered pwrseq node list */
> >  };
> >  
> >  /**
> > @@ -166,3 +167,14 @@ static inline int hub_port_debounce_be_stable(struct 
> > usb_hub *hub,
> >  {
> > return hub_port_debounce(hub, port1, false);
> >  }
> > +
> > +#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
> > +int hub_pwrseq_on(struct usb_hub *hub);
> > +void hub_pwrseq_off(struct usb_hub *hub);
> > +#else
> > +static inline int hub_pwrseq_on(struct usb_hub *hub)
> > +{
> > +   return 0;
> > +}
> > +static inline void hub_pwrseq_off(struct usb_hub *hub) {}
> > +#endif /* CONFIG_PWRSEQ_GENERIC */
> > diff --git a/drivers/usb/core/pwrseq.c b/drivers/usb/core/pwrseq.c
> > new file mode 100644
> > index 000..837fe66
> > --- /dev/null
> > +++ b/drivers/usb/core/pwrseq.c
> > @@ -0,0 +1,100 @@
> > +/*
> > + * pwrseq.cUSB device power sequence management
> > + *
> > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > + * Author: Peter Chen 
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2  of
> > + * the License as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * 

[PATCH] usb: hub: Fix unbalanced reference count and memory leak

[Viresh Kumar]

If the hub gets disconnected while the core is still activating it, this
can result in leaking memory of few USB structures.

This will happen if we have done a kref_get() from hub_activate() and
scheduled a delayed work item for HUB_INIT2/3. Now if hub_disconnect()
gets called before the delayed work expires, then we will cancel the
work from hub_quiesce(), but wouldn't do a kref_put(). And so the
unbalance.

kmemleak reports this as (with the commit e50293ef9775 backported to
3.10 kernel with other changes, though the same is true for mainline as
well):

unreferenced object 0xffc08af5b800 (size 1024):
  comm "khubd", pid 73, jiffies 4295051211 (age 6482.350s)
  hex dump (first 32 bytes):
30 68 f3 8c c0 ff ff ff 00 a0 b2 2e c0 ff ff ff  0h..
01 00 00 00 00 00 00 00 00 94 7d 40 c0 ff ff ff  ..}@
  backtrace:
[] create_object+0x148/0x2a0
[] kmemleak_alloc+0x80/0xbc
[] kmem_cache_alloc_trace+0x120/0x1ac
[] hub_probe+0x120/0xb84
[] usb_probe_interface+0x1ec/0x298
[] driver_probe_device+0x160/0x374
[] __device_attach+0x28/0x4c
[] bus_for_each_drv+0x78/0xac
[] device_attach+0x6c/0x9c
[] bus_probe_device+0x28/0xa0
[] device_add+0x324/0x604
[] usb_set_configuration+0x660/0x6cc
[] generic_probe+0x44/0x84
[] usb_probe_device+0x54/0x74
[] driver_probe_device+0x160/0x374
[] __device_attach+0x28/0x4c

Fix this by putting the reference in hub_quiesce() if we canceled a
pending work.

CC:  #4.4+
Fixes: e50293ef9775 ("USB: fix invalid memory access in hub_activate()")
Signed-off-by: Viresh Kumar 
---
Greg,

This is tested over 3.10 with backported patches only, sorry didn't had
a mainline setup to test this out. :(

 drivers/usb/core/hub.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bee13517676f..3173693fa8e3 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1315,7 +1315,8 @@ static void hub_quiesce(struct usb_hub *hub, enum 
hub_quiescing_type type)
struct usb_device *hdev = hub->hdev;
int i;
 
-   cancel_delayed_work_sync(>init_work);
+   if (cancel_delayed_work_sync(>init_work))
+   kref_put(>kref, hub_release);
 
/* hub_wq and related activity won't re-trigger */
hub->quiescing = 1;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] usb: serial: ftdi_sio Added 0a5c:6422 device ID for WICED USB UART dev board

[Greg KH]

On Wed, Jul 27, 2016 at 07:27:25PM +, Sheng-Hui  Chu wrote:
> 
> BCM20706V2_EVAL is a WICED dev board designed with FT2232H USB 2.0 UART/FIFO 
> IC.
> 
> To support BCM920706V2_EVAL dev board for WICED development on Linux.  Add 
> the VID(0a5c) and
> PID(6422) to ftdi_sio driver to allow loading ftdi_sio for this board.
> 
> Signed-off-by: Jeffrey Chu 

Your "From:" and this name do not match :(

> ---
>  drivers/usb/serial/ftdi_sio.c   | 1 +
>  drivers/usb/serial/ftdi_sio_ids.h | 6 
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 0082080..ef19af4 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -1008,6 +1008,7 @@ static const struct usb_device_id id_table_combined[] = 
> {
> { USB_DEVICE(ICPDAS_VID, ICPDAS_I7560U_PID) },
> { USB_DEVICE(ICPDAS_VID, ICPDAS_I7561U_PID) },
> { USB_DEVICE(ICPDAS_VID, ICPDAS_I7563U_PID) },
> +{ USB_DEVICE(WICED_USB_VID, WICED_USB20706V2_PID) },
> { }/* Terminating entry */
>  };

All of your leading whitespace was stripped out, making this impossible
to be applied :(

Care to try again?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Alan Stern]

On Wed, 27 Jul 2016, Tejun Heo wrote:

> Hello, Alan.
> 
> On Wed, Jul 27, 2016 at 02:54:51PM -0400, Alan Stern wrote:
> > > Hmm... I didn't know the whole USB stack could operate without
> > > allocating memory.  Does usb stack have mempools and stuff all the way
> > > through?
> > 
> > No -- the USB stack does need to allocate memory in order to operate.  
> > But it is careful to use GFP_NOIO or GFP_ATOMIC for allocations that
> > might be on the block-device path.
> 
> Hmm... That doesn't really make them dependable during memory reclaim.

True.  But it does mean that they can't cause a deadlock by waiting
indefinitely for some other memory to be paged out to the very device
they are on the access pathway for.

> What happens when those allocations fail?

The same thing that happens when any allocation fails -- the original
I/O request fails with -ENOMEM or the equivalent.  In the case of 
usb-storage, this is likely to trigger error recovery, which will need 
to allocate memory of its own...  A bad situation to get into.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 9/9] usb: gadget: f_hid: use alloc_ep_req()

[Michal Nazarewicz]

On Tue, Jul 26 2016, Felipe F. Tonello wrote:
> Use gadget's framework allocation function instead of directly calling
> usb_ep_alloc_request().
>
> Signed-off-by: Felipe F. Tonello 

Acked-by: Michal Nazarewicz 

> ---
>  drivers/usb/gadget/function/f_hid.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_hid.c 
> b/drivers/usb/gadget/function/f_hid.c
> index a010496e4e05..89d2e9a5a04f 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -611,14 +611,10 @@ static int hidg_bind(struct usb_configuration *c, 
> struct usb_function *f)
>  
>   /* preallocate request and buffer */
>   status = -ENOMEM;
> - hidg->req = usb_ep_alloc_request(hidg->in_ep, GFP_KERNEL);
> + hidg->req = alloc_ep_req(hidg->in_ep, hidg->report_length);
>   if (!hidg->req)
>   goto fail;
>  
> - hidg->req->buf = kmalloc(hidg->report_length, GFP_KERNEL);
> - if (!hidg->req->buf)
> - goto fail;
> -
>   /* set descriptor dynamic values */
>   hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass;
>   hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol;
> -- 
> 2.9.0
>

-- 
Best regards
ミハウ “퓶퓲퓷퓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/9] usb: gadget: f_hid: use free_ep_req()

[Michal Nazarewicz]

On Tue, Jul 26 2016, Felipe F. Tonello wrote:
> We should always use free_ep_req() when allocating requests with
> alloc_ep_req().
>
> Signed-off-by: Felipe F. Tonello 

Acked-by: Michal Nazarewicz 

> ---
>  drivers/usb/gadget/function/f_hid.c | 10 +++---
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_hid.c 
> b/drivers/usb/gadget/function/f_hid.c
> index e82a7468252e..a010496e4e05 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -671,11 +671,8 @@ fail_free_descs:
>   usb_free_all_descriptors(f);
>  fail:
>   ERROR(f->config->cdev, "hidg_bind FAILED\n");
> - if (hidg->req != NULL) {
> - kfree(hidg->req->buf);
> - if (hidg->in_ep != NULL)
> - usb_ep_free_request(hidg->in_ep, hidg->req);
> - }
> + if (hidg->req != NULL)
> + free_ep_req(hidg->in_ep, hidg->req);
>  
>   return status;
>  }
> @@ -904,8 +901,7 @@ static void hidg_unbind(struct usb_configuration *c, 
> struct usb_function *f)
>  
>   /* disable/free request and end point */
>   usb_ep_disable(hidg->in_ep);
> - kfree(hidg->req->buf);
> - usb_ep_free_request(hidg->in_ep, hidg->req);
> + free_ep_req(hidg->in_ep, hidg->req);
>  
>   usb_free_all_descriptors(f);
>  }
> -- 
> 2.9.0
>

-- 
Best regards
ミハウ “퓶퓲퓷퓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/9] usb: gadget: remove useless parameter in alloc_ep_req()

[Michal Nazarewicz]

On Tue, Jul 26 2016, Felipe F. Tonello wrote:
> This parameter was not really necessary and gadget drivers would almost always

I personally like when commit messages can be read without subject, so
perhaps:

The default_length parameter of alloc_ep_req was not …

But that’s just me.

> create an inline function to pass the same value to len and default_len.
>
> So this patch also removes duplicate code from few drivers.
>
> Signed-off-by: Felipe F. Tonello 

Acked-by: Michal Nazarewicz 

> ---
>  drivers/usb/gadget/function/f_hid.c| 10 ++
>  drivers/usb/gadget/function/f_loopback.c   |  9 +
>  drivers/usb/gadget/function/f_midi.c   | 10 ++
>  drivers/usb/gadget/function/f_sourcesink.c | 11 ++-
>  drivers/usb/gadget/u_f.c   |  7 +++
>  drivers/usb/gadget/u_f.h   |  2 +-
>  6 files changed, 11 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_hid.c 
> b/drivers/usb/gadget/function/f_hid.c
> index 51980c50546d..e82a7468252e 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -362,12 +362,6 @@ static int f_hidg_open(struct inode *inode, struct file 
> *fd)
>  /*-*/
>  /*usb_function */
>  
> -static inline struct usb_request *hidg_alloc_ep_req(struct usb_ep *ep,
> - unsigned length)
> -{
> - return alloc_ep_req(ep, length, length);
> -}
> -
>  static void hidg_set_report_complete(struct usb_ep *ep, struct usb_request 
> *req)
>  {
>   struct f_hidg *hidg = (struct f_hidg *) req->context;
> @@ -549,8 +543,8 @@ static int hidg_set_alt(struct usb_function *f, unsigned 
> intf, unsigned alt)
>*/
>   for (i = 0; i < hidg->qlen && status == 0; i++) {
>   struct usb_request *req =
> - hidg_alloc_ep_req(hidg->out_ep,
> -   hidg->report_length);
> + alloc_ep_req(hidg->out_ep,
> + hidg->report_length);
>   if (req) {
>   req->complete = hidg_set_report_complete;
>   req->context  = hidg;
> diff --git a/drivers/usb/gadget/function/f_loopback.c 
> b/drivers/usb/gadget/function/f_loopback.c
> index 3a9f8f9c77bd..701ee0f11c33 100644
> --- a/drivers/usb/gadget/function/f_loopback.c
> +++ b/drivers/usb/gadget/function/f_loopback.c
> @@ -306,13 +306,6 @@ static void disable_loopback(struct f_loopback *loop)
>   VDBG(cdev, "%s disabled\n", loop->function.name);
>  }
>  
> -static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int len)
> -{
> - struct f_loopback   *loop = ep->driver_data;
> -
> - return alloc_ep_req(ep, len, loop->buflen);
> -}
> -
>  static int alloc_requests(struct usb_composite_dev *cdev,
> struct f_loopback *loop)
>  {
> @@ -333,7 +326,7 @@ static int alloc_requests(struct usb_composite_dev *cdev,
>   if (!in_req)
>   goto fail;
>  
> - out_req = lb_alloc_ep_req(loop->out_ep, 0);
> + out_req = alloc_ep_req(loop->out_ep, loop->buflen);
>   if (!out_req)
>   goto fail_in;
>  
> diff --git a/drivers/usb/gadget/function/f_midi.c 
> b/drivers/usb/gadget/function/f_midi.c
> index 3a47596afcab..abf26364b46f 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -208,12 +208,6 @@ static struct usb_gadget_strings *midi_strings[] = {
>   NULL,
>  };
>  
> -static inline struct usb_request *midi_alloc_ep_req(struct usb_ep *ep,
> - unsigned length)
> -{
> - return alloc_ep_req(ep, length, length);
> -}
> -
>  static const uint8_t f_midi_cin_length[] = {
>   0, 0, 2, 3, 3, 1, 2, 3, 3, 3, 3, 3, 2, 2, 3, 1
>  };
> @@ -365,7 +359,7 @@ static int f_midi_set_alt(struct usb_function *f, 
> unsigned intf, unsigned alt)
>   /* pre-allocate write usb requests to use on f_midi_transmit. */
>   while (kfifo_avail(>in_req_fifo)) {
>   struct usb_request *req =
> - midi_alloc_ep_req(midi->in_ep, midi->buflen);
> + alloc_ep_req(midi->in_ep, midi->buflen);
>  
>   if (req == NULL)
>   return -ENOMEM;
> @@ -379,7 +373,7 @@ static int f_midi_set_alt(struct usb_function *f, 
> unsigned intf, unsigned alt)
>   /* allocate a bunch of read buffers and queue them all at once. */
>   for (i = 0; i < midi->qlen && err == 0; i++) {
>   struct usb_request *req =
> - midi_alloc_ep_req(midi->out_ep, midi->buflen);

Re: [PATCH 2/9] usb: gadget: align buffer size when allocating for OUT endpoint

[Michal Nazarewicz]

On Tue, Jul 26 2016, Felipe F. Tonello wrote:
> Using usb_ep_align() makes sure that the buffer size for OUT endpoints is
> always aligned with wMaxPacketSize (512 usually). This makes sure
> that no buffer has the wrong size, which can cause nasty bugs.
>
> Signed-off-by: Felipe F. Tonello 
> ---
>  drivers/usb/gadget/u_f.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/gadget/u_f.c b/drivers/usb/gadget/u_f.c
> index 4bc7eea8bfc8..d1933b0b76c3 100644
> --- a/drivers/usb/gadget/u_f.c
> +++ b/drivers/usb/gadget/u_f.c
> @@ -12,6 +12,7 @@
>   */
>  
>  #include "u_f.h"
> +#include 
>  
>  struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int default_len)
>  {
> @@ -20,6 +21,8 @@ struct usb_request *alloc_ep_req(struct usb_ep *ep, int 
> len, int default_len)
>   req = usb_ep_alloc_request(ep, GFP_ATOMIC);
>   if (req) {
>   req->length = len ?: default_len;
> + if (usb_endpoint_dir_out(ep->desc))
> + req->length = usb_ep_align(ep, req->length);
>   req->buf = kmalloc(req->length, GFP_ATOMIC);
>   if (!req->buf) {
>   usb_ep_free_request(ep, req);

I’m a bit scared of this change.

Drivers which call alloc_ep_req and then ignore req->length using the
same length they passed to the function will silently drop data.

Drivers which do not ignore req->length may end up overwriting some
other buffer, e.g.:

some_buffer = kmalloc(length, GFP_KERNEL);
req = alloc_ep_req(ep, length, 0);
… later …
memcpy(some_buffer, req->buf, req->length);

-- 
Best regards
ミハウ “퓶퓲퓷퓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/9] usb: gadget: f_midi: defaults buflen sizes to 512

[Michal Nazarewicz]

On Tue, Jul 26 2016, Felipe F. Tonello wrote:
> 512 is the value used by wMaxPacketSize, as specified by the USB Spec. This
> makes sure this driver uses, by default, the most optimal value for IN and OUT
> endpoint requests.
>
> Signed-off-by: Felipe F. Tonello 

Acked-by: Michal Nazarewicz 

> ---
>  drivers/usb/gadget/function/f_midi.c | 2 +-
>  drivers/usb/gadget/legacy/gmidi.c| 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c 
> b/drivers/usb/gadget/function/f_midi.c
> index 39018dea7035..a7b50ac947f8 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -1122,7 +1122,7 @@ static struct usb_function_instance 
> *f_midi_alloc_inst(void)
>   opts->func_inst.free_func_inst = f_midi_free_inst;
>   opts->index = SNDRV_DEFAULT_IDX1;
>   opts->id = SNDRV_DEFAULT_STR1;
> - opts->buflen = 256;
> + opts->buflen = 512;
>   opts->qlen = 32;
>   opts->in_ports = 1;
>   opts->out_ports = 1;
> diff --git a/drivers/usb/gadget/legacy/gmidi.c 
> b/drivers/usb/gadget/legacy/gmidi.c
> index fc2ac150f5ff..0bf39c3ccdb1 100644
> --- a/drivers/usb/gadget/legacy/gmidi.c
> +++ b/drivers/usb/gadget/legacy/gmidi.c
> @@ -47,7 +47,7 @@ static char *id = SNDRV_DEFAULT_STR1;
>  module_param(id, charp, S_IRUGO);
>  MODULE_PARM_DESC(id, "ID string for the USB MIDI Gadget adapter.");
>  
> -static unsigned int buflen = 256;
> +static unsigned int buflen = 512;
>  module_param(buflen, uint, S_IRUGO);
>  MODULE_PARM_DESC(buflen, "MIDI buffer length");
>  
> -- 
> 2.9.0
>

-- 
Best regards
ミハウ “퓶퓲퓷퓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/9] usb: gadget: fix usb_ep_align_maybe endianness and new usb_ep_align

[Michal Nazarewicz]

On Tue, Jul 26 2016, Felipe F. Tonello wrote:
> USB spec specifies wMaxPacketSize to be little endian (as other properties),
> so when using this variable in the driver we should convert to the current
> CPU endianness if necessary.
>
> This patch also introduces usb_ep_align() which does always returns the
> aligned buffer size for an endpoint. This is useful to be used by USB requests
> allocator functions.
>
> Signed-off-by: Felipe F. Tonello 

Acked-by: Michal Nazarewicz 

> ---
>  include/linux/usb/gadget.h | 17 ++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index 612dbdfa388e..b8fa6901b881 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -418,8 +418,20 @@ static inline struct usb_gadget 
> *dev_to_usb_gadget(struct device *dev)
>   list_for_each_entry(tmp, &(gadget)->ep_list, ep_list)
>  
>  /**
> + * usb_ep_align - returns @len aligned to ep's maxpacketsize.
> + * @ep: the endpoint whose maxpacketsize is used to align @len
> + * @len: buffer size's length to align to @ep's maxpacketsize
> + *
> + * This helper is used to align buffer's size to an ep's maxpacketsize.
> + */
> +static inline size_t usb_ep_align(struct usb_ep *ep, size_t len)
> +{
> + return round_up(len, (size_t)le16_to_cpu(ep->desc->wMaxPacketSize));
> +}
> +
> +/**
>   * usb_ep_align_maybe - returns @len aligned to ep's maxpacketsize if gadget
> - *   requires quirk_ep_out_aligned_size, otherwise reguens len.
> + *   requires quirk_ep_out_aligned_size, otherwise returns len.
>   * @g: controller to check for quirk
>   * @ep: the endpoint whose maxpacketsize is used to align @len
>   * @len: buffer size's length to align to @ep's maxpacketsize
> @@ -430,8 +442,7 @@ static inline struct usb_gadget *dev_to_usb_gadget(struct 
> device *dev)
>  static inline size_t
>  usb_ep_align_maybe(struct usb_gadget *g, struct usb_ep *ep, size_t len)
>  {
> - return !g->quirk_ep_out_aligned_size ? len :
> - round_up(len, (size_t)ep->desc->wMaxPacketSize);
> + return !g->quirk_ep_out_aligned_size ? len : usb_ep_align(ep, len);

I’d drop the negation:

+   return g->quirk_ep_out_aligned_size ? usb_ep_align(ep, len) : len;

>  }
>  
>  /**
> -- 
> 2.9.0
>

-- 
Best regards
ミハウ “퓶퓲퓷퓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] usb: serial: ftdi_sio Added 0a5c:6422 device ID for WICED USB UART dev board

[Sheng-Hui Chu]

BCM20706V2_EVAL is a WICED dev board designed with FT2232H USB 2.0 UART/FIFO IC.

To support BCM920706V2_EVAL dev board for WICED development on Linux.  Add the 
VID(0a5c) and
PID(6422) to ftdi_sio driver to allow loading ftdi_sio for this board.

Signed-off-by: Jeffrey Chu 
---
 drivers/usb/serial/ftdi_sio.c   | 1 +
 drivers/usb/serial/ftdi_sio_ids.h | 6 
 2 files changed, 7 insertions(+)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 0082080..ef19af4 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1008,6 +1008,7 @@ static const struct usb_device_id id_table_combined[] = {
{ USB_DEVICE(ICPDAS_VID, ICPDAS_I7560U_PID) },
{ USB_DEVICE(ICPDAS_VID, ICPDAS_I7561U_PID) },
{ USB_DEVICE(ICPDAS_VID, ICPDAS_I7563U_PID) },
+{ USB_DEVICE(WICED_USB_VID, WICED_USB20706V2_PID) },
{ }/* Terminating entry */
 };

diff --git a/drivers/usb/serial/ftdi_sio_ids.h 
b/drivers/usb/serial/ftdi_sio_ids.h
index c5d6c1e..b29f280 100644
--- a/drivers/usb/serial/ftdi_sio_ids.h
+++ b/drivers/usb/serial/ftdi_sio_ids.h
@@ -1485,3 +1485,11 @@
 #define CHETCO_SEASMART_DISPLAY_PID0xA5AD /* SeaSmart NMEA2000 Display */
 #define CHETCO_SEASMART_LITE_PID0xA5AE /* SeaSmart Lite USB Adapter */
 #define CHETCO_SEASMART_ANALOG_PID0xA5AF /* SeaSmart Analog Adapter */
+
+/*
+ * WICED USB UART
+ */
+#define WICED_USB_VID0x0A5C
+#define WICED_USB20706V2_PID0x6422
--
2.1.4

This message and any attachments may contain Cypress (or its subsidiaries) 
confidential information. If it has been received in error, please advise the 
sender and immediately delete this message.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Tejun Heo]

Hello, Alan.

On Wed, Jul 27, 2016 at 02:54:51PM -0400, Alan Stern wrote:
> > Hmm... I didn't know the whole USB stack could operate without
> > allocating memory.  Does usb stack have mempools and stuff all the way
> > through?
> 
> No -- the USB stack does need to allocate memory in order to operate.  
> But it is careful to use GFP_NOIO or GFP_ATOMIC for allocations that
> might be on the block-device path.

Hmm... That doesn't really make them dependable during memory reclaim.
What happens when those allocations fail?

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 10/22] usb: serial: ti_usb_3410_5052: Change ti_write_byte function arguments

[Oliver Neukum]

On Wed, 2016-07-27 at 18:08 +0200, Mathieu OTHACEHE wrote:
> Hi,
> 
> > this makes me think something is wrong with the data structure.
> > We should have a be32 there, it seems to me.
> 
> You mean something like :
> 
> struct ti_write_data_bytes {
>   u8  bAddrType;
>   u8  bDataType;
>   u8  bDataCounter;
>   __be32  wBaseAddr;
>   u8  bData[0];
> } __packed;
> 
> and,
> 
> data->wBaseAddr = cpu_to_be32(addr) ?

Yes.

Regards
Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: serial: ftdi_sio: Added 0a5c:6422 device ID for WICED USB UART dev board

[Greg KH]

On Wed, Jul 27, 2016 at 11:54:23AM -0700, Greg KH wrote:
> On Wed, Jul 27, 2016 at 02:44:19PM -0400, Jeffrey Chu wrote:
> > Signed-off-by: Jeffrey Chu 
> 
> You should really put _some_ changelog comments in here, please?

Also, your From: address doesn't match your signed-off-by: address.  If
that's due to cypress.com 's email interface being horrid, just use the
correct "From:" syntax in the patch itself.
Documentation/SubmittingPatches describes how to do this.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Alan Stern]

On Wed, 27 Jul 2016, Tejun Heo wrote:

> Hello, Oliver.
> 
> On Wed, Jul 27, 2016 at 11:29:56AM +0200, Oliver Neukum wrote:
> > On Wed, 2016-07-27 at 14:50 +0530, Bhaktipriya Shridhar wrote:
> > > The workqueue "workqueue" has multiple workitems which may require
> > > ordering. Hence, a dedicated ordered workqueue has been used.
> > > Since the workqueue is not being used on a memory reclaim path,
> > > WQ_MEM_RECLAIM has not been set.
> > 
> > That is incorrect. The work queue is used by the HCD to handle
> > TDs, which are parts of basic IO. The HCD in turn is used by
> > usb-storage and uas, which are block drivers and those are obviously
> > used on the memory reclaim path.
> 
> Hmm... I didn't know the whole USB stack could operate without
> allocating memory.  Does usb stack have mempools and stuff all the way
> through?

No -- the USB stack does need to allocate memory in order to operate.  
But it is careful to use GFP_NOIO or GFP_ATOMIC for allocations that
might be on the block-device path.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: serial: ftdi_sio: Added 0a5c:6422 device ID for WICED USB UART dev board

[Greg KH]

On Wed, Jul 27, 2016 at 02:44:19PM -0400, Jeffrey Chu wrote:
> Signed-off-by: Jeffrey Chu 

You should really put _some_ changelog comments in here, please?

> ---
>  drivers/usb/serial/ftdi_sio.c | 1 +
>  drivers/usb/serial/ftdi_sio_ids.h | 8 
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 0082080..ef19af4 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -1008,6 +1008,7 @@ static const struct usb_device_id id_table_combined[] = 
> {
>  { USB_DEVICE(ICPDAS_VID, ICPDAS_I7560U_PID) },
>  { USB_DEVICE(ICPDAS_VID, ICPDAS_I7561U_PID) },
>  { USB_DEVICE(ICPDAS_VID, ICPDAS_I7563U_PID) },
> +{ USB_DEVICE(WICED_USB_VID, WICED_USB20706V2_PID) },
>  { }/* Terminating entry */
>  };
> 
> diff --git a/drivers/usb/serial/ftdi_sio_ids.h
> b/drivers/usb/serial/ftdi_sio_ids.h
> index c5d6c1e..b29f280 100644
> --- a/drivers/usb/serial/ftdi_sio_ids.h
> +++ b/drivers/usb/serial/ftdi_sio_ids.h
> @@ -1485,3 +1485,11 @@
>  #define CHETCO_SEASMART_DISPLAY_PID0xA5AD /* SeaSmart NMEA2000 Display */
>  #define CHETCO_SEASMART_LITE_PID0xA5AE /* SeaSmart Lite USB Adapter */
>  #define CHETCO_SEASMART_ANALOG_PID0xA5AF /* SeaSmart Analog Adapter */
> +
> +/*
> + * WICED USB UART
> + */
> +#define WICED_USB_VID0x0A5C
> +#define WICED_USB20706V2_PID0x6422
> +
> +

Why the two extra blank lines at the bottom of the file?

Also, your patch is corrupted, all tabs turned into spaces and can't be
applied.  Please fix it up and resend.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: serial: ftdi_sio: Added 0a5c:6422 device ID for WICED USB UART dev board

[Jeffrey Chu]

Signed-off-by: Jeffrey Chu 
---
 drivers/usb/serial/ftdi_sio.c | 1 +
 drivers/usb/serial/ftdi_sio_ids.h | 8 
 2 files changed, 9 insertions(+)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 0082080..ef19af4 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1008,6 +1008,7 @@ static const struct usb_device_id id_table_combined[] = {
 { USB_DEVICE(ICPDAS_VID, ICPDAS_I7560U_PID) },
 { USB_DEVICE(ICPDAS_VID, ICPDAS_I7561U_PID) },
 { USB_DEVICE(ICPDAS_VID, ICPDAS_I7563U_PID) },
+{ USB_DEVICE(WICED_USB_VID, WICED_USB20706V2_PID) },
 { }/* Terminating entry */
 };

diff --git a/drivers/usb/serial/ftdi_sio_ids.h
b/drivers/usb/serial/ftdi_sio_ids.h
index c5d6c1e..b29f280 100644
--- a/drivers/usb/serial/ftdi_sio_ids.h
+++ b/drivers/usb/serial/ftdi_sio_ids.h
@@ -1485,3 +1485,11 @@
 #define CHETCO_SEASMART_DISPLAY_PID0xA5AD /* SeaSmart NMEA2000 Display */
 #define CHETCO_SEASMART_LITE_PID0xA5AE /* SeaSmart Lite USB Adapter */
 #define CHETCO_SEASMART_ANALOG_PID0xA5AF /* SeaSmart Analog Adapter */
+
+/*
+ * WICED USB UART
+ */
+#define WICED_USB_VID0x0A5C
+#define WICED_USB20706V2_PID0x6422
+
+
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Tejun Heo]

Hello, Oliver.

On Wed, Jul 27, 2016 at 11:29:56AM +0200, Oliver Neukum wrote:
> On Wed, 2016-07-27 at 14:50 +0530, Bhaktipriya Shridhar wrote:
> > The workqueue "workqueue" has multiple workitems which may require
> > ordering. Hence, a dedicated ordered workqueue has been used.
> > Since the workqueue is not being used on a memory reclaim path,
> > WQ_MEM_RECLAIM has not been set.
> 
> That is incorrect. The work queue is used by the HCD to handle
> TDs, which are parts of basic IO. The HCD in turn is used by
> usb-storage and uas, which are block drivers and those are obviously
> used on the memory reclaim path.

Hmm... I didn't know the whole USB stack could operate without
allocating memory.  Does usb stack have mempools and stuff all the way
through?

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] usb: typec: Add USB Power Delivery sink port support

[Bin Gao]

On Wed, Jul 27, 2016 at 11:13:43AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Bin Gao  writes:
> > This patch implements a simple USB Power Delivery sink port state machine.
> > It assumes the hardware only handles PD packet transmitting and receiving
> > over the CC line of the USB Type-C connector. The state transition is
> > completely controlled by software. This patch only implement the sink port
> > function and it doesn't support source port and port swap yet.
> >
> > This patch depends on these two patches:
> > https://lkml.org/lkml/2016/6/29/349
> > https://lkml.org/lkml/2016/6/29/350
> >
> > Signed-off-by: Bin Gao 
> > Changes in v2:
> >  - Removed work queue so messages are directly handled in phy driver's 
> > interrupt context
> >  - used pr_debug instead of pr_info for message dump
> >  - Converted PD driver to tristate and typec driver is independent of it
> 
> this should be after the tearline (---) below. We don't want this in
> changelog ;-)
> 
> > +static void handle_source_cap(struct pd_sink_port *port, u8 msg_revision,
> > +   u8 nr_objs, u8 *buf)
> > +{
> > +   int i;
> > +   u32 *obj;
> > +   u8 type;
> > +   struct pd_source_cap *cap = port->source_caps;
> > +
> > +   /*
> > +* The PD spec revision included in SOURCE_CAPABILITY message is the
> > +* highest revision that the Source supports.
> > +*/
> > +   port->pd_rev = msg_revision;
> > +
> > +   /*
> > +* First we need to save all PDOs - they may be used in the future.
> > +* USB PD spec says we must use PDOs in the most recent
> > +* SOURCE_CAPABILITY message. Here we replace old PDOs with new ones.
> > +*/
> > +   port->nr_source_caps = 0;
> > +   for (i = 0; i < nr_objs; i++) {
> > +   obj = (u32 *)(buf + i * PD_OBJ_SIZE);
> > +   type = (*obj >> SOURCE_CAP_TYPE_BIT) & SOURCE_CAP_TYPE_MASK;
> > +   switch (type) {
> > +   case PS_TYPE_FIXED:
> > +   cap->ps_type = PS_TYPE_FIXED;
> > +   cap->fixed = *(struct pd_pdo_src_fixed *)obj;
> > +   break;
> > +   case PS_TYPE_VARIABLE:
> > +   cap->ps_type = PS_TYPE_VARIABLE;
> > +   cap->variable = *(struct pd_pdo_variable *)obj;
> > +   break;
> > +   case PS_TYPE_BATTERY:
> > +   cap->ps_type = PS_TYPE_BATTERY;
> > +   cap->battery = *(struct pd_pdo_battery *)obj;
> > +   break;
> > +   default: /* shouldn't come here */
> > +   pr_err("Invalid Source Capability type: %u.\n", type);
> > +   continue;
> > +   }
> > +   port->nr_source_caps++;
> > +   cap++;
> > +   }
> > +
> > +   if (port->nr_source_caps == 0) {
> > +   pr_err("There is no valid PDOs in SOURCE_CAPABILITY message\n");
> > +   return;
> > +   }
> > +
> > +   /* If a contract is not established, we need send a REQUEST message */
> > +   if (port->state == PD_SINK_STATE_WAIT_FOR_SRC_CAP) {
> 
> this is wrong. Read the fluxchart in figure 8-42. Source can decide to
> send another Source Capability before receiving our Good CRC and we need
> to work with that. This state check is, at a minimum, wrong. I'd
> actually just go ahead and remove it.
> 
> > +   if (!send_request(port))
> > +   port->state = PD_SINK_STATE_REQUEST_SENT;
> > +   }
> > +}
> 
> -- 
> balbi

I'll fix these in next revision. Thanks for your review.

-Bin
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] usb: typec: Add USB Power Delivery sink port support

[Bin Gao]

On Wed, Jul 27, 2016 at 11:21:13AM +0200, Oliver Neukum wrote:
> On Tue, 2016-07-26 at 11:37 -0700, Bin Gao wrote:
> > +#define MAKE_HEADER(port, header, msg, objs) \
> > +do { \
> > +   header->type = msg; \
> > +   header->data_role = PD_DATA_ROLE_UFP; \
> > +   header->revision = port->pd_rev; \
> > +   header->power_role = PD_POWER_ROLE_SINK; \
> > +   header->id = roll_msg_id(port); \
> > +   header->nr_objs = objs; \
> > +   header->extended = PD_MSG_NOT_EXTENDED; \
> > +} while (0)
> > +
> > +static struct pd_sink_port *sink_ports[MAX_NR_SINK_PORTS];
> > +static int nr_ports;
> > +
> > +BLOCKING_NOTIFIER_HEAD(pd_sink_notifier_list);
> > +
> > +static char *state_strings[] = {
> > +   "WAIT_FOR_SOURCE_CAPABILITY",
> > +   "REQUEST_SENT",
> > +   "ACCEPT_RECEIVED",
> > +   "POWER_SUPPLY_READY",
> > +};
> > +
> > +/* Control messages */
> > +static char *cmsg_strings[] = {
> > +   "GOODCRC",  /* 1 */
> > +   "GOTOMIN",  /* 2 */
> > +   "ACCEPT",   /* 3 */
> > +   "REJECT",   /* 4 */
> > +   "PING", /* 5 */
> > +   "PS_RDY",   /* 6 */
> > +   "GET_SRC_CAP",  /* 7 */
> > +   "GET_SINK_CAP", /* 8 */
> > +   "DR_SWAP",  /* 9 */
> > +   "PR_SWAP",  /* 10 */
> > +   "VCONN_SWAP",   /* 11 */
> > +   "WAIT", /* 12 */
> > +   "SOFT_RESET",   /* 13 */
> > +   "RESERVED", /* 14 */
> > +   "RESERVED", /* 15 */
> > +   "NOT_SUPPORTED",/* 16 */
> > +   "GET_SOURCE_CAP_EXTENDED",  /* 17 */
> > +   "GET_STATUS",   /* 18 */
> > +   "FR_SWAP",  /* 19 */
> > +   /* RESERVED 20 - 31 */
> > +};
> > +
> > +/* Data messages */
> > +static char *dmsg_strings[] = {
> > +   "SOURCE_CAP",   /* 1 */
> > +   "REQUEST",  /* 2 */
> > +   "BIST", /* 3 */
> > +   "SINK_CAP", /* 4 */
> > +   "BATTERY_STATUS",   /* 5 */
> > +   "ALERT",/* 6 */
> > +   "RESERVED", /* 7 */
> > +   "RESERVED", /* 8 */
> > +   "RESERVED", /* 9 */
> > +   "RESERVED", /* 10 */
> > +   "RESERVED", /* 11 */
> > +   "RESERVED", /* 12 */
> > +   "RESERVED", /* 13 */
> > +   "RESERVED", /* 14 */
> > +   "VENDOR_DEFINED",   /* 15 */
> > +   /* RESERVED 16 - 31 */
> > +};
> > +
> > +static char *state_to_string(enum pd_sink_state state)
> > +{
> > +   if (state < ARRAY_SIZE(state_strings))
> > +   return state_strings[state];
> > +   else
> > +   return NULL;
> > +}
> > +
> > +static char *msg_to_string(bool is_cmsg, u8 msg)
> > +{
> > +   int nr = is_cmsg ? ARRAY_SIZE(cmsg_strings) :
> > ARRAY_SIZE(dmsg_strings);
> > +
> > +   if (msg <= nr)
> > +   return is_cmsg ? cmsg_strings[msg - 1] :
> > dmsg_strings[msg - 1];
> > +   else
> > +   return "RESERVED";
> > +}
> > +
> > +static void print_message(int port, bool is_cmsg, u8 msg, bool recv)
> > +{
> > +   pr_debug("sink port %d: %s message %s %s\n", port,
> > +   is_cmsg ? "Control" : "Data",
> > +   msg_to_string(is_cmsg, msg),
> > +recv ? "received" : "sent)");
> > +}
> > +
> > +static inline bool fixed_ps_equal(struct sink_ps *p1,
> > +   struct sink_ps *p2)
> > +{
> > +   return p1->ps_type == p2->ps_type &&
> > +   p1->ps_fixed.voltage_fixed ==
> > p2->ps_fixed.voltage_fixed &&
> > +   p1->ps_fixed.current_default ==
> > p2->ps_fixed.current_default;
> > +}
> > +
> > +/* The message ID increments each time we send out a new message */
> > +static u8 roll_msg_id(struct pd_sink_port *port)
> > +{
> > +   u8 msg_id = port->msg_id;
> > +
> > +   if (msg_id == PD_MSG_ID_MAX)
> > +   msg_id = PD_MSG_ID_MIN;
> > +   else
> > +   msg_id++;
> > +
> > +   port->msg_id = msg_id;
> > +   return msg_id;
> > +}
> > +
> 
> These pieces of code are completely generic. They should be shared
> among drivers. Please move them into the include files.
> 
>   Regards
>   Oliver
> 
> 

They are only internally used by this driver (USB PD state machine driver).
And they are not used by drivers (typicall USB Type-C phy drivers) which
use APIs exposed by this driver. So I think it's not appropriate to move
to include files.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to 

Re: pwc over musb: 100% frame drop (lost) on high resolution stream

[Matwey V. Kornilov]

Hello,

I've just biseced commit, which introduced this issue

commit f551e13529833e052f75ec628a8af7b034af20f9
Author: Bin Liu 
Date:   Mon Apr 25 15:53:30 2016 -0500

Revert "usb: musb: musb_host: Enable HCD_BH flag to handle urb
return in bottom half"

I have not checked yet, if it was intentionnaly fixed.

2016-07-23 22:24 GMT+03:00 Matwey V. Kornilov :
> 2016-07-20 21:56 GMT+03:00 Matwey V. Kornilov :
>> 2016-07-20 18:06 GMT+03:00 Bin Liu :
>>> Hi,
>>>
>>> On Wed, Jul 20, 2016 at 05:44:56PM +0300, Matwey V. Kornilov wrote:
 2016-07-20 17:13 GMT+03:00 Bin Liu :
 > Hi,
 >
 > On Wed, Jul 20, 2016 at 09:09:42AM +0300, Matwey V. Kornilov wrote:
 >> 2016-07-20 0:34 GMT+03:00 Bin Liu :
 >> > Hi,
 >> >
 >> > On Wed, Jul 20, 2016 at 12:25:44AM +0300, Matwey V. Kornilov wrote:
 >> >> 2016-07-19 23:56 GMT+03:00 Bin Liu :
 >> >> > Hi,
 >> >> >
 >> >> > On Tue, Jul 19, 2016 at 11:21:17PM +0300, mat...@sai.msu.ru wrote:
 >> >> >> Hello,
 >> >> >>
 >> >> >> I have Philips SPC 900 camera (0471:0329) connected to my AM335x 
 >> >> >> based BeagleBoneBlack SBC.
 >> >> >> I am sure that both of them are fine and work properly.
 >> >> >> I am running Linux 4.6.4 (my kernel config is available at 
 >> >> >> https://clck.ru/A2kQs ) and I've just discovered, that there is 
 >> >> >> an issue with frame transfer when high resolution formats are 
 >> >> >> used.
 >> >> >>
 >> >> >> The issue is the following. I use simple v4l2 example tool (taken 
 >> >> >> from API docs), which source code is available at 
 >> >> >> http://pastebin.com/grcNXxfe
 >> >> >>
 >> >> >> When I use (see line 488) 640x480 frames
 >> >> >>
 >> >> >> fmt.fmt.pix.width   = 640;
 >> >> >> fmt.fmt.pix.height  = 480;
 >> >> >>
 >> >> >> then I get "select timeout" and don't get any frames.
 >> >> >>
 >> >> >> When I use 320x240 frames
 >> >> >>
 >> >> >> fmt.fmt.pix.width   = 320;
 >> >> >> fmt.fmt.pix.height  = 240;
 >> >> >>
 >> >> >> then about 60% frames are missed. An example outpout of ./a.out 
 >> >> >> -f is available at https://yadi.sk/d/aRka8xWPtSc4y
 >> >> >> It looks like there are pauses between bulks of frames (frame 
 >> >> >> counter and timestamp as returned from v4l2 API):
 >> >> >>
 >> >> >> 3 3705.142553
 >> >> >> 8 3705.342533
 >> >> >> 13 3705.542517
 >> >> >> 110 3708.776208
 >> >> >> 115 3708.976190
 >> >> >> 120 3709.176169
 >> >> >> 125 3709.376152
 >> >> >> 130 3709.576144
 >> >> >> 226 3712.807848
 >> >> >>
 >> >> >> When I use tiny 160x120 frames
 >> >> >>
 >> >> >> fmt.fmt.pix.width   = 160;
 >> >> >> fmt.fmt.pix.height  = 120;
 >> >> >>
 >> >> >> then more frames are received. See output example at 
 >> >> >> https://yadi.sk/d/DedBmH6ftSc9t
 >> >> >> That is why I thought that everything was fine in May when used 
 >> >> >> tiny xawtv window to check kernel OOPS presence (see 
 >> >> >> http://www.spinics.net/lists/linux-usb/msg141188.html for 
 >> >> >> reference)
 >> >> >>
 >> >> >> Even more. When I introduce USB hub between the host and the 
 >> >> >> webcam, I can not receive even any 320x240 frames.
 >> >> >>
 >> >> >> I've managed to use ftrace to see what is going on when no frames 
 >> >> >> are received.
 >> >> >> I've found that pwc_isoc_handler is called frequently as the 
 >> >> >> following:
 >> >> >>
 >> >> >>  0)   |  pwc_isoc_handler [pwc]() {
 >> >> >>  0)   |usb_submit_urb [usbcore]() {
 >> >> >>  0)   |  usb_submit_urb.part.3 [usbcore]() {
 >> >> >>  0)   |usb_hcd_submit_urb [usbcore]() {
 >> >> >>  0)   0.834 us|  usb_get_urb [usbcore]();
 >> >> >>  0)   |  musb_map_urb_for_dma [musb_hdrc]() {
 >> >> >>  0)   0.792 us|usb_hcd_map_urb_for_dma 
 >> >> >> [usbcore]();
 >> >> >>  0)   5.750 us|  }
 >> >> >>  0)   |  musb_urb_enqueue [musb_hdrc]() {
 >> >> >>  0)   0.750 us|_raw_spin_lock_irqsave();
 >> >> >>  0)   |usb_hcd_link_urb_to_ep [usbcore]() 
 >> >> >> {
 >> >> >>  0)   0.792 us|  _raw_spin_lock();
 >> >> >>  0)   0.791 us|  _raw_spin_unlock();
 >> >> >>  0) + 10.500 us   |}
 >> >> >>  0)   0.791 us|_raw_spin_unlock_irqrestore();
 >> >> >>  0) + 25.375 us   |  }
 >> >> >>  0) + 45.208 us   |}
 >> >> >>  0) + 51.042 us   |  }
 >> >> >>  0) + 

Re: [PATCH v3 4/6] usb: core: add power sequence handling for USB devices

[Joshua Clayton]

On 07/20/2016 02:40 AM, Peter Chen wrote:
> Some hard-wired USB devices need to do power sequence to let the
> device work normally, the typical power sequence like: enable USB
> PHY clock, toggle reset pin, etc. But current Linux USB driver
> lacks of such code to do it, it may cause some hard-wired USB devices
> works abnormal or can't be recognized by controller at all.
>
> In this patch, it calls power sequence library APIs to finish
> the power sequence events. At first, it calls pwrseq_alloc_generic
> to create a generic power sequence instance, then it will do power
> on sequence at hub's probe for all devices under this hub
> (includes root hub).
>
> At hub_disconnect, it will do power off sequence which is at powered
> on list.
>
> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/core/Makefile |   1 +
>  drivers/usb/core/hub.c|  12 --
>  drivers/usb/core/hub.h|  12 ++
>  drivers/usb/core/pwrseq.c | 100 
> ++
>  4 files changed, 122 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/usb/core/pwrseq.c
>
> diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> index da36b78..5fd2f54 100644
> --- a/drivers/usb/core/Makefile
> +++ b/drivers/usb/core/Makefile
> @@ -10,5 +10,6 @@ usbcore-y += port.o
>  usbcore-$(CONFIG_OF) += of.o
>  usbcore-$(CONFIG_PCI)+= hcd-pci.o
>  usbcore-$(CONFIG_ACPI)   += usb-acpi.o
> +usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o
>  
>  obj-$(CONFIG_USB)+= usbcore.o
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index bee1351..a346a8b 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1700,6 +1700,7 @@ static void hub_disconnect(struct usb_interface *intf)
>   hub->error = 0;
>   hub_quiesce(hub, HUB_DISCONNECT);
>  
> + hub_pwrseq_off(hub);
>   mutex_lock(_port_peer_mutex);
>  
>   /* Avoid races with recursively_mark_NOTATTACHED() */
> @@ -1733,6 +1734,7 @@ static int hub_probe(struct usb_interface *intf, const 
> struct usb_device_id *id)
>   struct usb_endpoint_descriptor *endpoint;
>   struct usb_device *hdev;
>   struct usb_hub *hub;
> + int ret = -ENODEV;
>  
>   desc = intf->cur_altsetting;
>   hdev = interface_to_usbdev(intf);
> @@ -1839,6 +1841,7 @@ descriptor_error:
>   INIT_DELAYED_WORK(>leds, led_work);
>   INIT_DELAYED_WORK(>init_work, NULL);
>   INIT_WORK(>events, hub_event);
> + INIT_LIST_HEAD(>pwrseq_on_list);
>   usb_get_intf(intf);
>   usb_get_dev(hdev);
>  
> @@ -1852,11 +1855,14 @@ descriptor_error:
>   if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
>   hub->quirk_check_port_auto_suspend = 1;
>  
> - if (hub_configure(hub, endpoint) >= 0)
> - return 0;
> + if (hub_configure(hub, endpoint) >= 0) {
> + ret = hub_pwrseq_on(hub);
> + if (!ret)
> + return 0;
> + }
>  
>   hub_disconnect(intf);
> - return -ENODEV;
> + return ret;
>  }
>  
>  static int
> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> index 34c1a7e..9473f6f 100644
> --- a/drivers/usb/core/hub.h
> +++ b/drivers/usb/core/hub.h
> @@ -78,6 +78,7 @@ struct usb_hub {
>   struct delayed_work init_work;
>   struct work_struct  events;
>   struct usb_port **ports;
> + struct list_headpwrseq_on_list; /* powered pwrseq node list */
>  };
>  
>  /**
> @@ -166,3 +167,14 @@ static inline int hub_port_debounce_be_stable(struct 
> usb_hub *hub,
>  {
>   return hub_port_debounce(hub, port1, false);
>  }
> +
> +#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
> +int hub_pwrseq_on(struct usb_hub *hub);
> +void hub_pwrseq_off(struct usb_hub *hub);
> +#else
> +static inline int hub_pwrseq_on(struct usb_hub *hub)
> +{
> + return 0;
> +}
> +static inline void hub_pwrseq_off(struct usb_hub *hub) {}
> +#endif /* CONFIG_PWRSEQ_GENERIC */
> diff --git a/drivers/usb/core/pwrseq.c b/drivers/usb/core/pwrseq.c
> new file mode 100644
> index 000..837fe66
> --- /dev/null
> +++ b/drivers/usb/core/pwrseq.c
> @@ -0,0 +1,100 @@
> +/*
> + * pwrseq.c  USB device power sequence management
> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Author: Peter Chen 
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2  of
> + * the License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + 

Re: [PATCH v2 10/22] usb: serial: ti_usb_3410_5052: Change ti_write_byte function arguments

[Mathieu OTHACEHE]

Hi,

> this makes me think something is wrong with the data structure.
> We should have a be32 there, it seems to me.

You mean something like :

struct ti_write_data_bytes {
u8  bAddrType;
u8  bDataType;
u8  bDataCounter;
__be32  wBaseAddr;
u8  bData[0];
} __packed;

and,

data->wBaseAddr = cpu_to_be32(addr) ?

Thanks,

Mathieu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Patch for pl2302 usb/serial module.

[Greg KH]

On Wed, Jul 27, 2016 at 07:46:01AM -0400, Serge Bromow wrote:
> Please Consider the Environment before Printing the E-Mail. IMPORTANT
> NOTICE: This message is intended only for the use of the individual or
> entity to which it is addressed. The message may contain information that is
> privileged, confidential and exempt from disclosure under applicable law. If
> the reader of this message is not the intended recipient, or the employee or
> agent responsible for delivering the message to the intended recipient, you
> are notified that any dissemination, distribution or copying of this
> communication is strictly prohibited. If you have received this
> communication in error, please notify DineAmix Inc. immediately by email at
> postmas...@dineamix.ca. Thank you.

We really can't take patches, or even respond to, emails with footers
like this, as it's obviously not compatible with our development model.
Can you remove this, add a signed-off-by: line (as per the
Documentation/SubmittingPatches file), and make the patch the proper
"depth" (i.e. be able to be applied from the root of the kernel tree)
and resend it?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 03/22] usb: serial: ti_usb_3410_5052: Use kzalloc instead of kmalloc

[Mathieu OTHACEHE]

> in that case, where is the initialisation to 0 you avoid and hence
> can remove from the code?

Hi,

In v1, kzalloc was useful to avoid wFlags initialisation to 0 :

https://lkml.org/lkml/2016/5/12/139

In v2 wFlags initialisation has be removed so this patch has no purpose
anymore, my bad.

Thanks,

Mathieu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Patch for pl2302 usb/serial module.

[Oliver Neukum]

On Wed, 2016-07-27 at 07:46 -0400, Serge Bromow wrote:
> In summary I am using a new HP product (display pole) that works with 
> the pl2303 module but is not recognized as the device product ID is
> not 
> in the module table. To correct this I have attached 2 patches that I 
> would like applied to the pl2303 module if possible.
> 
> As this is my first patch submission I may have omitted some 
> information. Please let me know if you require more information.

Hi,

thank you for making this patch. It is technically correct.
The submission format, however, is missing a few things. The
canonical way of submitting a patch is described in the kernel itself
in its Documentation directory in the file SubmittingPatches

HTH
Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch for pl2302 usb/serial module.

[Serge Bromow]

I was referred to your group from "helpd...@kernel.org".


In summary I am using a new HP product (display pole) that works with 
the pl2303 module but is not recognized as the device product ID is not 
in the module table. To correct this I have attached 2 patches that I 
would like applied to the pl2303 module if possible.


As this is my first patch submission I may have omitted some 
information. Please let me know if you require more information.


Regards,

Serge Bromow

--
Signature

Serge Bromow

DineAmix Inc.   (613) 
260-8629 888 411-6636 Ottawa, Canada.
Please Consider the Environment before Printing the E-Mail. IMPORTANT 
NOTICE: This message is intended only for the use of the individual or 
entity to which it is addressed. The message may contain information 
that is privileged, confidential and exempt from disclosure under 
applicable law. If the reader of this message is not the intended 
recipient, or the employee or agent responsible for delivering the 
message to the intended recipient, you are notified that any 
dissemination, distribution or copying of this communication is strictly 
prohibited. If you have received this communication in error, please 
notify DineAmix Inc. immediately by email at postmas...@dineamix.ca. 
Thank you.


--- pl2303.c	2016-07-27 05:54:57.676741858 -0400
+++ pl2303.change.c	2016-07-27 05:54:10.249145021 -0400
@@ -86,6 +86,7 @@
 	{ USB_DEVICE(HP_VENDOR_ID, HP_LD960_PRODUCT_ID) },
 	{ USB_DEVICE(HP_VENDOR_ID, HP_LCM220_PRODUCT_ID) },
 	{ USB_DEVICE(HP_VENDOR_ID, HP_LCM960_PRODUCT_ID) },
+	{ USB_DEVICE(HP_VENDOR_ID, HP_LX220_PRODUCT_ID) },
 	{ USB_DEVICE(CRESSI_VENDOR_ID, CRESSI_EDY_PRODUCT_ID) },
 	{ USB_DEVICE(ZEAGLE_VENDOR_ID, ZEAGLE_N2ITION3_PRODUCT_ID) },
 	{ USB_DEVICE(SONY_VENDOR_ID, SONY_QN3USB_PRODUCT_ID) },
--- pl2303.h	2016-07-27 05:55:08.888646534 -0400
+++ pl2303.change.h	2016-07-27 05:54:26.089010381 -0400
@@ -124,6 +124,7 @@
 #define HP_LCM220_PRODUCT_ID	0x3139
 #define HP_LCM960_PRODUCT_ID	0x3239
 #define HP_LD220_PRODUCT_ID	0x3524
+#define HP_LX220_PRODUCT_ID	0x4349
 
 /* Cressi Edy (diving computer) PC interface */
 #define CRESSI_VENDOR_ID	0x04b8

Re: [PATCH v2 03/22] usb: serial: ti_usb_3410_5052: Use kzalloc instead of kmalloc

[Sergei Shtylyov]

On 7/26/2016 8:59 PM, Mathieu OTHACEHE wrote:


Use kzalloc instead of kmalloc to avoid field initialisation to 0.


   Avoid? kzalloc() does initialize to 0. :-)


Signed-off-by: Mathieu OTHACEHE 


MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: gadget: remove redundant self assignment

[Peter Chen]

On Mon, Jul 25, 2016 at 10:57:36PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> The assignment ret = ret is redundant and can be removed.
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/usb/gadget/udc/core.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index ff8685e..48cf1a3 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -107,10 +107,8 @@ int usb_ep_enable(struct usb_ep *ep)
>   goto out;
>  
>   ret = ep->ops->enable(ep, ep->desc);
> - if (ret) {
> - ret = ret;
> + if (ret)
>   goto out;
> - }
>  
>   ep->enabled = true;
>  

Reviewed-by: Peter Chen 

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Oliver Neukum]

On Wed, 2016-07-27 at 14:50 +0530, Bhaktipriya Shridhar wrote:
> The workqueue "workqueue" has multiple workitems which may require
> ordering. Hence, a dedicated ordered workqueue has been used.
> Since the workqueue is not being used on a memory reclaim path,
> WQ_MEM_RECLAIM has not been set.

That is incorrect. The work queue is used by the HCD to handle
TDs, which are parts of basic IO. The HCD in turn is used by
usb-storage and uas, which are block drivers and those are obviously
used on the memory reclaim path.

HTH
Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] usb: typec: Add USB Power Delivery sink port support

[Oliver Neukum]

On Tue, 2016-07-26 at 11:37 -0700, Bin Gao wrote:
> +#define MAKE_HEADER(port, header, msg, objs) \
> +do { \
> +   header->type = msg; \
> +   header->data_role = PD_DATA_ROLE_UFP; \
> +   header->revision = port->pd_rev; \
> +   header->power_role = PD_POWER_ROLE_SINK; \
> +   header->id = roll_msg_id(port); \
> +   header->nr_objs = objs; \
> +   header->extended = PD_MSG_NOT_EXTENDED; \
> +} while (0)
> +
> +static struct pd_sink_port *sink_ports[MAX_NR_SINK_PORTS];
> +static int nr_ports;
> +
> +BLOCKING_NOTIFIER_HEAD(pd_sink_notifier_list);
> +
> +static char *state_strings[] = {
> +   "WAIT_FOR_SOURCE_CAPABILITY",
> +   "REQUEST_SENT",
> +   "ACCEPT_RECEIVED",
> +   "POWER_SUPPLY_READY",
> +};
> +
> +/* Control messages */
> +static char *cmsg_strings[] = {
> +   "GOODCRC",  /* 1 */
> +   "GOTOMIN",  /* 2 */
> +   "ACCEPT",   /* 3 */
> +   "REJECT",   /* 4 */
> +   "PING", /* 5 */
> +   "PS_RDY",   /* 6 */
> +   "GET_SRC_CAP",  /* 7 */
> +   "GET_SINK_CAP", /* 8 */
> +   "DR_SWAP",  /* 9 */
> +   "PR_SWAP",  /* 10 */
> +   "VCONN_SWAP",   /* 11 */
> +   "WAIT", /* 12 */
> +   "SOFT_RESET",   /* 13 */
> +   "RESERVED", /* 14 */
> +   "RESERVED", /* 15 */
> +   "NOT_SUPPORTED",/* 16 */
> +   "GET_SOURCE_CAP_EXTENDED",  /* 17 */
> +   "GET_STATUS",   /* 18 */
> +   "FR_SWAP",  /* 19 */
> +   /* RESERVED 20 - 31 */
> +};
> +
> +/* Data messages */
> +static char *dmsg_strings[] = {
> +   "SOURCE_CAP",   /* 1 */
> +   "REQUEST",  /* 2 */
> +   "BIST", /* 3 */
> +   "SINK_CAP", /* 4 */
> +   "BATTERY_STATUS",   /* 5 */
> +   "ALERT",/* 6 */
> +   "RESERVED", /* 7 */
> +   "RESERVED", /* 8 */
> +   "RESERVED", /* 9 */
> +   "RESERVED", /* 10 */
> +   "RESERVED", /* 11 */
> +   "RESERVED", /* 12 */
> +   "RESERVED", /* 13 */
> +   "RESERVED", /* 14 */
> +   "VENDOR_DEFINED",   /* 15 */
> +   /* RESERVED 16 - 31 */
> +};
> +
> +static char *state_to_string(enum pd_sink_state state)
> +{
> +   if (state < ARRAY_SIZE(state_strings))
> +   return state_strings[state];
> +   else
> +   return NULL;
> +}
> +
> +static char *msg_to_string(bool is_cmsg, u8 msg)
> +{
> +   int nr = is_cmsg ? ARRAY_SIZE(cmsg_strings) :
> ARRAY_SIZE(dmsg_strings);
> +
> +   if (msg <= nr)
> +   return is_cmsg ? cmsg_strings[msg - 1] :
> dmsg_strings[msg - 1];
> +   else
> +   return "RESERVED";
> +}
> +
> +static void print_message(int port, bool is_cmsg, u8 msg, bool recv)
> +{
> +   pr_debug("sink port %d: %s message %s %s\n", port,
> +   is_cmsg ? "Control" : "Data",
> +   msg_to_string(is_cmsg, msg),
> +recv ? "received" : "sent)");
> +}
> +
> +static inline bool fixed_ps_equal(struct sink_ps *p1,
> +   struct sink_ps *p2)
> +{
> +   return p1->ps_type == p2->ps_type &&
> +   p1->ps_fixed.voltage_fixed ==
> p2->ps_fixed.voltage_fixed &&
> +   p1->ps_fixed.current_default ==
> p2->ps_fixed.current_default;
> +}
> +
> +/* The message ID increments each time we send out a new message */
> +static u8 roll_msg_id(struct pd_sink_port *port)
> +{
> +   u8 msg_id = port->msg_id;
> +
> +   if (msg_id == PD_MSG_ID_MAX)
> +   msg_id = PD_MSG_ID_MIN;
> +   else
> +   msg_id++;
> +
> +   port->msg_id = msg_id;
> +   return msg_id;
> +}
> +

These pieces of code are completely generic. They should be shared
among drivers. Please move them into the include files.

Regards
Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

[Bhaktipriya Shridhar]

alloc_ordered_workqueue replaces the deprecated
create_singlethread_workqueue.

The workqueue "workqueue" has multiple workitems which may require
ordering. Hence, a dedicated ordered workqueue has been used.
Since the workqueue is not being used on a memory reclaim path,
WQ_MEM_RECLAIM has not been set.

Can the workitems run concurrently? Is the ordering among work items
necessary?

Thanks.

Signed-off-by: Bhaktipriya Shridhar 
---
 drivers/usb/host/u132-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/u132-hcd.c b/drivers/usb/host/u132-hcd.c
index 43d5293..cbde189 100644
--- a/drivers/usb/host/u132-hcd.c
+++ b/drivers/usb/host/u132-hcd.c
@@ -3206,7 +3206,7 @@ static int __init u132_hcd_init(void)
if (usb_disabled())
return -ENODEV;
printk(KERN_INFO "driver %s\n", hcd_name);
-   workqueue = create_singlethread_workqueue("u132");
+   workqueue = alloc_ordered_workqueue("u132", 0);
retval = platform_driver_register(_platform_driver);
return retval;
 }
--
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] whci: Remove deprecated create_singlethread_workqueue

[Bhaktipriya Shridhar]

alloc_ordered_workqueue replaces the deprecated
create_singlethread_workqueue.

The workqueue "workqueue" has multiple workitems which may require
ordering. Hence, a dedicated ordered workqueue has been used.
Since the workqueue is not being used on a memory reclaim path,
WQ_MEM_RECLAIM has not been set.

Can the workitems run concurrently? Is the ordering among work items necessary?

Thanks.

Signed-off-by: Bhaktipriya Shridhar 
---
 drivers/usb/host/whci/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/whci/init.c b/drivers/usb/host/whci/init.c
index e363723..ad8eb57 100644
--- a/drivers/usb/host/whci/init.c
+++ b/drivers/usb/host/whci/init.c
@@ -65,7 +65,7 @@ int whc_init(struct whc *whc)
init_waitqueue_head(>cmd_wq);
init_waitqueue_head(>async_list_wq);
init_waitqueue_head(>periodic_list_wq);
-   whc->workqueue = 
create_singlethread_workqueue(dev_name(>umc->dev));
+   whc->workqueue = alloc_ordered_workqueue(dev_name(>umc->dev), 0);
if (whc->workqueue == NULL) {
ret = -ENOMEM;
goto error;
--
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] usb: typec: add PD sink port support for Intel Whiskey Cove PMIC Typc-C PHY driver

[Felipe Balbi]

(before anything: please have a look at the patches I wrote on top of
this. They might help you somewhat

https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/log/?h=bxt-typec-pd-fixes)

Bin Gao  writes:
> From: Chandra Sekhar Anagani 
>
> This adds PD sink port support for the USB Type-C PHY on Intel WhiskeyCove
> PMIC which is available on some of the Intel Broxton SoC based platforms.
>
> This patch depends on these two patches:
> https://lkml.org/lkml/2016/6/29/349
> https://lkml.org/lkml/2016/6/29/350
>
> Signed-off-by: Chandra Sekhar Anagani 
> Signed-off-by: Pranav Tipnis 
> Signed-off-by: Bin Gao 
> Changes in v2:
>  - Added PD support for cold boot case

like before, move it after tearline. Also, what does this mean?

> ---
>  drivers/usb/typec/typec_wcove.c | 309 
> 
>  1 file changed, 285 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/usb/typec/typec_wcove.c b/drivers/usb/typec/typec_wcove.c
> index c7c2d28..000d6ae 100644
> --- a/drivers/usb/typec/typec_wcove.c
> +++ b/drivers/usb/typec/typec_wcove.c
> @@ -3,6 +3,7 @@
>   *
>   * Copyright (C) 2016 Intel Corporation
>   * Author: Heikki Krogerus 
> + * Author: Chandra Sekhar Anagani 
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
> @@ -10,9 +11,11 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -25,6 +28,7 @@
>  #define USBC_CONTROL30x7003
>  #define USBC_CC1_CTRL0x7004
>  #define USBC_CC2_CTRL0x7005
> +#define USBC_CC_SEL  0x7006
>  #define USBC_STATUS1 0x7007
>  #define USBC_STATUS2 0x7008
>  #define USBC_STATUS3 0x7009
> @@ -32,7 +36,16 @@
>  #define USBC_IRQ20x7016
>  #define USBC_IRQMASK10x7017
>  #define USBC_IRQMASK20x7018
> -
> +#define USBC_PD_CFG1 0x7019
> +#define USBC_PD_CFG2 0x701a
> +#define USBC_PD_CFG3 0x701b
> +#define USBC_PD_STATUS   0x701c
> +#define USBC_RX_STATUS   0x701d
> +#define USBC_RX_INFO 0x701e
> +#define USBC_TX_CMD  0x701f
> +#define USBC_TX_INFO 0x7020
> +#define USBC_RX_DATA_START   0x7028
> +#define USBC_TX_DATA_START   0x7047
>  /* Register bits */
>  
>  #define USBC_CONTROL1_MODE_DRP(r)((r & ~0x7) | 4)
> @@ -44,7 +57,9 @@
>  #define USBC_CONTROL3_PD_DIS BIT(1)
>  
>  #define USBC_CC_CTRL_VCONN_ENBIT(1)
> +#define USBC_CC_CTRL_TX_EN   BIT(2)
>  
> +#define USBC_CC_SEL_CCSEL(BIT(0) | BIT(1))
>  #define USBC_STATUS1_DET_ONGOING BIT(6)
>  #define USBC_STATUS1_RSLT(r) (r & 0xf)
>  #define USBC_RSLT_NOTHING0
> @@ -79,11 +94,44 @@
>USBC_IRQ2_RX_HR | USBC_IRQ2_RX_CR | \
>USBC_IRQ2_TX_SUCCESS | USBC_IRQ2_TX_FAIL)
>  
> +#define USBC_PD_CFG1_ID_FILL BIT(7)
> +
> +#define USBC_PD_CFG2_SOP_RX  BIT(0)
> +
> +#define USBC_PD_CFG3_SR_SOP2 (BIT(7) | BIT(6))
> +#define USBC_PD_CFG3_SR_SOP1 (BIT(5) | BIT(4))
> +#define USBC_PD_CFG3_SR_SOP0 (BIT(3) | BIT(2))
> +#define USBC_PD_CFG3_DATAROLEBIT(1)
> +#define USBC_PD_CFG3_PWRROLE BIT(0)
> +
> +#define USBC_TX_CMD_TXBUF_RDYBIT(0)
> +#define USBC_TX_CMD_TX_START BIT(1)
> +#define USBC_TX_CMD_TXBUF_CMD(r) ((r >> 5) & 0x7)
> +
> +#define USBC_TX_INFO_TX_SOP  (BIT(0) | BIT(1) | BIT(2))
> +#define USBC_TX_INFO_TX_RETRIES  (BIT(3) | BIT(4) | BIT(5))
> +
> +#define USBC_RX_STATUS_RX_DATA   BIT(7)
> +#define USBC_RX_STATUS_RX_OVERRUNBIT(6)
> +#define USBC_RX_STATUS_RX_CLEAR  BIT(0)
> +
> +#define USBC_PD_STATUS_RX_RSLT(r)((r >> 3) & 0x7)
> +#define USBC_PD_STATUS_TX_RSLT(r)(r & 0x7)
> +
> +#define USBC_RX_INFO_RXBYTES(r)  ((r >> 3) & 0x1f)
> +#define USBC_RX_INFO_RX_SOP(r)   (r & 0x7)
> +
> +#define USBC_PD_RX_BUF_LEN   30
> +#define USBC_PD_TX_BUF_LEN   30
> +#define USBC_PD_SEND_HR  (3 << 5)
> +
>  struct wcove_typec {
> + int pd_port_num;
>   struct mutex lock; /* device lock */
>   struct device *dev;
>   struct regmap *regmap;
>   struct typec_port *port;
> + struct pd_sink_port pd_port;
>   struct typec_capability cap;
>   struct typec_connection con;
>   struct typec_partner partner;
> @@ -106,6 +154,50 @@ enum wcove_typec_role {
>   WCOVE_ROLE_DEVICE,
>  };
>  
> +static struct sink_ps profiles[] = {
> +
> + {
> + .ps_type = PS_TYPE_FIXED,
> + 

Re: [PATCH v2 10/22] usb: serial: ti_usb_3410_5052: Change ti_write_byte function arguments

[Oliver Neukum]

On Tue, 2016-07-26 at 19:59 +0200, Mathieu OTHACEHE wrote:
> @@ -1626,7 +1624,7 @@ static int ti_write_byte(struct usb_serial_port
> *port,
> data->bAddrType = TI_RW_DATA_ADDR_XDATA;
> data->bDataType = TI_RW_DATA_BYTE;
> data->bDataCounter = 1;
> -   data->wBaseAddrHi = cpu_to_be16(addr>>16);
> +   data->wBaseAddrHi = cpu_to_be16(addr >> 16);
> data->wBaseAddrLo = cpu_to_be16(addr);
> data->bData[0] = mask;
> data->bData[1] = byte;
> -- 

Hi,

this makes me think something is wrong with the data structure.
We should have a be32 there, it seems to me.

Regards
Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] usb: typec: Add USB Power Delivery sink port support

[Felipe Balbi]

Hi,

Bin Gao  writes:
> This patch implements a simple USB Power Delivery sink port state machine.
> It assumes the hardware only handles PD packet transmitting and receiving
> over the CC line of the USB Type-C connector. The state transition is
> completely controlled by software. This patch only implement the sink port
> function and it doesn't support source port and port swap yet.
>
> This patch depends on these two patches:
> https://lkml.org/lkml/2016/6/29/349
> https://lkml.org/lkml/2016/6/29/350
>
> Signed-off-by: Bin Gao 
> Changes in v2:
>  - Removed work queue so messages are directly handled in phy driver's 
> interrupt context
>  - used pr_debug instead of pr_info for message dump
>  - Converted PD driver to tristate and typec driver is independent of it

this should be after the tearline (---) below. We don't want this in
changelog ;-)

> +static void handle_source_cap(struct pd_sink_port *port, u8 msg_revision,
> + u8 nr_objs, u8 *buf)
> +{
> + int i;
> + u32 *obj;
> + u8 type;
> + struct pd_source_cap *cap = port->source_caps;
> +
> + /*
> +  * The PD spec revision included in SOURCE_CAPABILITY message is the
> +  * highest revision that the Source supports.
> +  */
> + port->pd_rev = msg_revision;
> +
> + /*
> +  * First we need to save all PDOs - they may be used in the future.
> +  * USB PD spec says we must use PDOs in the most recent
> +  * SOURCE_CAPABILITY message. Here we replace old PDOs with new ones.
> +  */
> + port->nr_source_caps = 0;
> + for (i = 0; i < nr_objs; i++) {
> + obj = (u32 *)(buf + i * PD_OBJ_SIZE);
> + type = (*obj >> SOURCE_CAP_TYPE_BIT) & SOURCE_CAP_TYPE_MASK;
> + switch (type) {
> + case PS_TYPE_FIXED:
> + cap->ps_type = PS_TYPE_FIXED;
> + cap->fixed = *(struct pd_pdo_src_fixed *)obj;
> + break;
> + case PS_TYPE_VARIABLE:
> + cap->ps_type = PS_TYPE_VARIABLE;
> + cap->variable = *(struct pd_pdo_variable *)obj;
> + break;
> + case PS_TYPE_BATTERY:
> + cap->ps_type = PS_TYPE_BATTERY;
> + cap->battery = *(struct pd_pdo_battery *)obj;
> + break;
> + default: /* shouldn't come here */
> + pr_err("Invalid Source Capability type: %u.\n", type);
> + continue;
> + }
> + port->nr_source_caps++;
> + cap++;
> + }
> +
> + if (port->nr_source_caps == 0) {
> + pr_err("There is no valid PDOs in SOURCE_CAPABILITY message\n");
> + return;
> + }
> +
> + /* If a contract is not established, we need send a REQUEST message */
> + if (port->state == PD_SINK_STATE_WAIT_FOR_SRC_CAP) {

this is wrong. Read the fluxchart in figure 8-42. Source can decide to
send another Source Capability before receiving our Good CRC and we need
to work with that. This state check is, at a minimum, wrong. I'd
actually just go ahead and remove it.

> + if (!send_request(port))
> + port->state = PD_SINK_STATE_REQUEST_SENT;
> + }
> +}

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2 03/22] usb: serial: ti_usb_3410_5052: Use kzalloc instead of kmalloc

[Oliver Neukum]

On Tue, 2016-07-26 at 19:59 +0200, Mathieu OTHACEHE wrote:
> Use kzalloc instead of kmalloc to avoid field initialisation to 0.
> 
> Signed-off-by: Mathieu OTHACEHE 
> ---
>  drivers/usb/serial/ti_usb_3410_5052.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/ti_usb_3410_5052.c 
> b/drivers/usb/serial/ti_usb_3410_5052.c
> index 50a74b3..b694d69 100644
> --- a/drivers/usb/serial/ti_usb_3410_5052.c
> +++ b/drivers/usb/serial/ti_usb_3410_5052.c
> @@ -969,7 +969,7 @@ static void ti_set_termios(struct tty_struct *tty,
>   if (tport == NULL)
>   return;
>  
> - config = kmalloc(sizeof(*config), GFP_KERNEL);
> + config = kzalloc(sizeof(*config), GFP_KERNEL);
>   if (!config)
>   return;
>  

Hi,

in that case, where is the initialisation to 0 you avoid and hence
can remove from the code?

Regards
Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html