Re: [PATCH 26/37] xhci: use the trb_to_noop() helper for command trbs

2017-01-23 Thread Mathias Nyman

On 20.01.2017 20:18, Felipe Balbi wrote:


Mathias Nyman  writes:


Remove duplicate code by using trb_to_noop() when
handling Aborted commads

Signed-off-by: Mathias Nyman 


isn't this just [1] 

https://marc.info/?i=20161229110109.26372-25-felipe.ba...@linux.intel.com



A simplified version of [1].
There are only two types of no-op trbs, command and transfer no-ops.
We always know to which one of these we want to turn the trb so just
give it as a parameter.

Solution [1] takes any trb and turns it into  no-op, it's a more generic
solution but it comes with the expense of code complexity, like the 20-case
switch statement to check trb type to narrow it down to two options.

I want xhci code to be as simple as possible.

-Mathias

--
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: [RESEND PATCH] usb: chipidea: delete an useless header include

2017-01-23 Thread Peter Chen
On Mon, Jan 23, 2017 at 03:05:45PM +0800, Jisheng Zhang wrote:
>  is for net phy drivers, we don't need it.
> 
> Signed-off-by: Jisheng Zhang 
> ---
>  drivers/usb/chipidea/core.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 3dbb4a21ab44..77078083e9fb 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -62,7 +62,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  

Applied, thanks.

-- 

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: [RESEND PATCH] usb: chipidea: usb2: delete the redundant setting default DMA mask code

2017-01-23 Thread Peter Chen
On Mon, Jan 23, 2017 at 03:09:23PM +0800, Jisheng Zhang wrote:
> Similar as commit 2b2fe36def08 ("usb: chipidea: imx: delete the
> redundant setting default DMA mask code"), the ci_hdrc_usb2 platform
> device is also created by device tree, the default DMA mask should be
> already set by of_dma_configure when the device are created. So delete
> the redundant code at driver.
> 
> Signed-off-by: Jisheng Zhang 
> ---
>  drivers/usb/chipidea/ci_hdrc_usb2.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c 
> b/drivers/usb/chipidea/ci_hdrc_usb2.c
> index 4456d2cf80ff..d162cc0bb8ce 100644
> --- a/drivers/usb/chipidea/ci_hdrc_usb2.c
> +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
> @@ -74,10 +74,6 @@ static int ci_hdrc_usb2_probe(struct platform_device *pdev)
>   }
>   }
>  
> - ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> - if (ret)
> - goto clk_err;
> -
>   ci_pdata->name = dev_name(dev);
>  
>   priv->ci_pdev = ci_hdrc_add_device(dev, pdev->resource,

Applied, thanks.

-- 

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: [PATCH 26/37] xhci: use the trb_to_noop() helper for command trbs

2017-01-23 Thread Mathias Nyman

On 23.01.2017 10:12, Mathias Nyman wrote:

On 20.01.2017 20:18, Felipe Balbi wrote:


Mathias Nyman  writes:


Remove duplicate code by using trb_to_noop() when
handling Aborted commads

Signed-off-by: Mathias Nyman 


isn't this just [1] 

https://marc.info/?i=20161229110109.26372-25-felipe.ba...@linux.intel.com



A simplified version of [1].
There are only two types of no-op trbs, command and transfer no-ops.
We always know to which one of these we want to turn the trb so just
give it as a parameter.

Solution [1] takes any trb and turns it into  no-op, it's a more generic
solution but it comes with the expense of code complexity, like the 20-case
switch statement to check trb type to narrow it down to two options.

I want xhci code to be as simple as possible.



I'm resending the series anyway to fix the extra empty lines and a typo,
want me to add a Suggested-by, or a "based on code by comment" to patch 25 and 
26?

-Mathias

--
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 26/37] xhci: use the trb_to_noop() helper for command trbs

2017-01-23 Thread Felipe Balbi

Hi,

Mathias Nyman  writes:
> On 20.01.2017 20:18, Felipe Balbi wrote:
>>
>> Mathias Nyman  writes:
>>
>>> Remove duplicate code by using trb_to_noop() when
>>> handling Aborted commads
>>>
>>> Signed-off-by: Mathias Nyman 
>>
>> isn't this just [1] 
>>
>> https://marc.info/?i=20161229110109.26372-25-felipe.ba...@linux.intel.com
>>
>
> A simplified version of [1].
> There are only two types of no-op trbs, command and transfer no-ops.
> We always know to which one of these we want to turn the trb so just
> give it as a parameter.

Right, the only difference is that you're passing NOOP type as argument
instead of figuring it out inside trb_to_noop(). I didn't do it like
that because I find it to be error prone. User has to remember if s/he
is currently dealing with transfer or command TRBs when the function
(totally not a critical path) can easily figure that out.

> Solution [1] takes any trb and turns it into  no-op, it's a more generic
> solution but it comes with the expense of code complexity, like the 20-case
> switch statement to check trb type to narrow it down to two options.

yeah, that 20-case switch statement is largely optimized by the
compiler, though. All cases are sequential integer values, so your
switch() gets optimized to something like:

if (type > X && type < Y)
noop = Z;
else if (type > W && type < V)
noop = T;
else
error();

Well, compiler could optimize this ever further if TRB types were held
in an enum instead of just several macros. Compiler would know for sure
which are allowed values for that argument... but let's not go into
that.

> I want xhci code to be as simple as possible.

a very honorable goal; common courtesy, however, would have you
commenting on the original patch and my explanation above could be given
to you. Then we would come to an agreement and I would, gladly, update
original patch.

In any case, it's just a patch ;-) No reason to stop your changes from
going upstream. Just, next time, please have the courtesy of discussing
changes in the list to avoid surprises ;-)

cheers

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH 26/37] xhci: use the trb_to_noop() helper for command trbs

2017-01-23 Thread Felipe Balbi

Hi,

Mathias Nyman  writes:
> On 23.01.2017 10:12, Mathias Nyman wrote:
>> On 20.01.2017 20:18, Felipe Balbi wrote:
>>>
>>> Mathias Nyman  writes:
>>>
 Remove duplicate code by using trb_to_noop() when
 handling Aborted commads

 Signed-off-by: Mathias Nyman 
>>>
>>> isn't this just [1] 
>>>
>>> https://marc.info/?i=20161229110109.26372-25-felipe.ba...@linux.intel.com
>>>
>>
>> A simplified version of [1].
>> There are only two types of no-op trbs, command and transfer no-ops.
>> We always know to which one of these we want to turn the trb so just
>> give it as a parameter.
>>
>> Solution [1] takes any trb and turns it into  no-op, it's a more generic
>> solution but it comes with the expense of code complexity, like the 20-case
>> switch statement to check trb type to narrow it down to two options.
>>
>> I want xhci code to be as simple as possible.
>>
>
> I'm resending the series anyway to fix the extra empty lines and a
> typo, want me to add a Suggested-by, or a "based on code by comment"
> to patch 25 and 26?

I'll leave the decision to you. Either way is fine.

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH 26/37] xhci: use the trb_to_noop() helper for command trbs

2017-01-23 Thread Mathias Nyman

On 23.01.2017 11:21, Felipe Balbi wrote:


Hi,

Mathias Nyman  writes:

On 20.01.2017 20:18, Felipe Balbi wrote:


Mathias Nyman  writes:


Remove duplicate code by using trb_to_noop() when
handling Aborted commads

Signed-off-by: Mathias Nyman 


isn't this just [1] 

https://marc.info/?i=20161229110109.26372-25-felipe.ba...@linux.intel.com



A simplified version of [1].
There are only two types of no-op trbs, command and transfer no-ops.
We always know to which one of these we want to turn the trb so just
give it as a parameter.


Right, the only difference is that you're passing NOOP type as argument
instead of figuring it out inside trb_to_noop(). I didn't do it like
that because I find it to be error prone. User has to remember if s/he
is currently dealing with transfer or command TRBs when the function
(totally not a critical path) can easily figure that out.


Solution [1] takes any trb and turns it into  no-op, it's a more generic
solution but it comes with the expense of code complexity, like the 20-case
switch statement to check trb type to narrow it down to two options.


yeah, that 20-case switch statement is largely optimized by the
compiler, though. All cases are sequential integer values, so your
switch() gets optimized to something like:

if (type > X && type < Y)
noop = Z;
else if (type > W && type < V)
noop = T;
else
error();

Well, compiler could optimize this ever further if TRB types were held
in an enum instead of just several macros. Compiler would know for sure
which are allowed values for that argument... but let's not go into
that.


I want xhci code to be as simple as possible.


a very honorable goal; common courtesy, however, would have you
commenting on the original patch and my explanation above could be given
to you. Then we would come to an agreement and I would, gladly, update
original patch.

In any case, it's just a patch ;-) No reason to stop your changes from
going upstream. Just, next time, please have the courtesy of discussing
changes in the list to avoid surprises ;-)



Sure, will remember that.
I wanted to get things forward to 4.11 before rc5 was out, and moving those
few lines under their own helper my way was faster. Didn't think it mattered.

-Mathias   



--
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 v3 3/3] usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in dsps

2017-01-23 Thread Alexandre Bailon
On 01/20/2017 09:17 PM, Bin Liu wrote:
> On Thu, Jan 19, 2017 at 11:06:59AM +0100, Alexandre Bailon wrote:
>> Despite the CPPI 4.1 is a generic DMA, it is tied to USB.
>> On the dsps, CPPI 4.1 interrupt's registers are in USBSS (the MUSB glue).
>> Currently, to enable / disable and clear interrupts, the CPPI 4.1 driver
>> maps and accesses to USBSS's register, which making CPPI 4.1 driver not
>> really generic.
>> Move the interrupt management to dsps driver.
>>
>> Signed-off-by: Alexandre Bailon 
>> ---
>>  drivers/dma/cppi41.c | 28 
>>  drivers/usb/musb/musb_dsps.c | 77 
>> ++--
>>  2 files changed, 82 insertions(+), 23 deletions(-)
> 
> This patch touches both dma and musb modules, I know it makes review
> easier, but how we get it merged? One maintainer ACK it and the other
> pick it up? Sorry for the dumb question, I am new as a maintainer...
> 
>>
>> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
>> index d5ba43a..4999e7d 100644
>> --- a/drivers/dma/cppi41.c
>> +++ b/drivers/dma/cppi41.c
> 
> [...]
> 
>> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
>> index 9f125e1..9dad3a6 100644
>> --- a/drivers/usb/musb/musb_dsps.c
>> +++ b/drivers/usb/musb/musb_dsps.c
>> @@ -121,6 +121,7 @@ struct dsps_glue {
>>  struct timer_list timer;/* otg_workaround timer */
>>  unsigned long last_timer;/* last timer data for each instance */
>>  bool sw_babble_enabled;
>> +void __iomem *usbss_base;
>>  
>>  struct dsps_context context;
>>  struct debugfs_regset32 regset;
>> @@ -145,6 +146,13 @@ static const struct debugfs_reg32 dsps_musb_regs[] = {
>>  { "mode",   0xe8 },
>>  };
>>  
>> +/* USBSS  / USB AM335x */
>> +#define USBSS_IRQ_STATUS0x28
>> +#define USBSS_IRQ_ENABLER   0x2c
>> +#define USBSS_IRQ_CLEARR0x30
>> +
>> +#define USBSS_IRQ_PD_COMP   (1 <<  2)
> 
> Please fix the double white spaces bwteen '<' and '2' this time.
> 
>> +
>>  /**
>>   * dsps_musb_enable - enable interrupts
>>   */
>> @@ -619,14 +627,72 @@ static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, 
>> u16 len, u8 *dst)
>>  }
>>  }
>>  
>> +#ifdef CONFIG_USB_TI_CPPI41_DMA
>> +static void dsps_dma_controller_callback(struct dma_controller *c)
>> +{
>> +struct musb *musb = c->musb;
>> +struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
>> +void __iomem *usbss_base = glue->usbss_base;
>> +u32 status;
>> +
>> +status = musb_readl(usbss_base, USBSS_IRQ_STATUS);
>> +if (status & USBSS_IRQ_PD_COMP)
>> +musb_writel(usbss_base, USBSS_IRQ_STATUS, USBSS_IRQ_PD_COMP);
>> +}
>> +
>> +static struct dma_controller *
>> +dsps_dma_controller_create(struct musb *musb, void __iomem *base)
>> +{
>> +struct dma_controller *controller;
>> +struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
>> +void __iomem *usbss_base = glue->usbss_base;
>> +
>> +controller = cppi41_dma_controller_create(musb, base);
>> +if (IS_ERR_OR_NULL(controller))
>> +return controller;
>> +
>> +musb_writel(usbss_base, USBSS_IRQ_ENABLER, USBSS_IRQ_PD_COMP);
>> +controller->dma_callback = dsps_dma_controller_callback;
>> +
>> +return controller;
>> +}
>> +
>> +static void dsps_dma_controller_destroy(struct dma_controller *c)
>> +{
>> +struct musb *musb = c->musb;
>> +struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
>> +void __iomem *usbss_base = glue->usbss_base;
>> +
>> +musb_writel(usbss_base, USBSS_IRQ_CLEARR, USBSS_IRQ_PD_COMP);
>> +cppi41_dma_controller_destroy(c);
>> +}
>> +
>> +static void dsps_dma_controller_suspend(struct dsps_glue *glue)
>> +{
>> +void __iomem *usbss_base = glue->usbss_base;
>> +
>> +musb_writel(usbss_base, USBSS_IRQ_CLEARR, USBSS_IRQ_PD_COMP);
>> +}
>> +
>> +static void dsps_dma_controller_resume(struct dsps_glue *glue)
>> +{
>> +void __iomem *usbss_base = glue->usbss_base;
>> +
>> +musb_writel(usbss_base, USBSS_IRQ_ENABLER, USBSS_IRQ_PD_COMP);
>> +}
> 
> The two functions above need to be wrapped in CONFIG_PM_SLEEP.
> 
>> +#else
>> +static void dsps_dma_controller_suspend(struct dsps_glue *glue) {}
>> +static void dsps_dma_controller_resume(struct dsps_glue *glue) {}
>> +#endif
>> +
>>  static struct musb_platform_ops dsps_ops = {
>>  .quirks = MUSB_DMA_CPPI41 | MUSB_INDEXED_EP,
>>  .init   = dsps_musb_init,
>>  .exit   = dsps_musb_exit,
>>  
>>  #ifdef CONFIG_USB_TI_CPPI41_DMA
>> -.dma_init   = cppi41_dma_controller_create,
>> -.dma_exit   = cppi41_dma_controller_destroy,
>> +.dma_init   = dsps_dma_controller_create,
>> +.dma_exit   = dsps_dma_controller_destroy,
>>  #endif
>>  .enable = dsps_musb_enable,
>>  .disable= dsps_musb_disable,
>> @@ -792,6 +858,9 @@ static int dsps_probe(struct platform_device *pdev)
>>  
>>  glue->dev = &pdev->dev;
>> 

[RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget

2017-01-23 Thread Petr Cvek
Hello,

It seems the USB product ID for g_webcam usb device gadget is incorrectly used 
from EEM gadget "Ethernet Emulation Model". So "webcam" device has a confusing 
description in lsusb:

1d6b:0102 Linux Foundation EEM Gadget

I would change it to 0x0106, which is a next unassigned value (according to 
http://www.linux-usb.org/usb.ids). Does this step require some official 
communication with the holder of VID (Linux Foundation)? Or all it takes is 
just to send patch to the kernel and to the usb.ids database?

I know it is only a cosmetic change on a legacy driver, but I assume it would 
be better to have some default value for configfs API than to borrow a PID from 
a whole different gadget.

Change in question would be something like this (I will send a proper patch 
after RFC):

Fixing USB Product ID for UVC gadget (used from Ethernet Emulation Model) and 
changing it to new one.

Signed-off-by: Petr Cvek 
---
 drivers/usb/gadget/legacy/webcam.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/legacy/webcam.c 
b/drivers/usb/gadget/legacy/webcam.c
index f9661cd627c8..988814f2e1c7 100644
--- a/drivers/usb/gadget/legacy/webcam.c
+++ b/drivers/usb/gadget/legacy/webcam.c
@@ -42,7 +42,7 @@ MODULE_PARM_DESC(trace, "Trace level bitmask");
  */
 
 #define WEBCAM_VENDOR_ID   0x1d6b  /* Linux Foundation */
-#define WEBCAM_PRODUCT_ID  0x0102  /* Webcam A/V gadget */
+#define WEBCAM_PRODUCT_ID  0x0106  /* UVC video gadget */
 #define WEBCAM_DEVICE_BCD  0x0010  /* 0.10 */
 
 static char webcam_vendor_label[] = "Linux Foundation";
-- 
2.11.0
--
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: gadget: uvc: webcam gadget USB PID is using value from a different gadget

2017-01-23 Thread Felipe Balbi

Hi,

Petr Cvek  writes:
> It seems the USB product ID for g_webcam usb device gadget is
> incorrectly used from EEM gadget "Ethernet Emulation Model". So
> "webcam" device has a confusing description in lsusb:
>
>   1d6b:0102 Linux Foundation EEM Gadget
>
> I would change it to 0x0106, which is a next unassigned value
> (according to http://www.linux-usb.org/usb.ids). Does this step
> require some official communication with the holder of VID (Linux
> Foundation)? Or all it takes is just to send patch to the kernel and
> to the usb.ids database?
>
> I know it is only a cosmetic change on a legacy driver, but I assume
> it would be better to have some default value for configfs API than to
> borrow a PID from a whole different gadget.
>
> Change in question would be something like this (I will send a proper
> patch after RFC):
>
> Fixing USB Product ID for UVC gadget (used from Ethernet Emulation
> Model) and changing it to new one.

Greg, do we still have an official Vendor ID? If so, any chance we can
allocate a unique default ID for the webcam gadget?

> Signed-off-by: Petr Cvek 
> ---
>  drivers/usb/gadget/legacy/webcam.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/legacy/webcam.c 
> b/drivers/usb/gadget/legacy/webcam.c
> index f9661cd627c8..988814f2e1c7 100644
> --- a/drivers/usb/gadget/legacy/webcam.c
> +++ b/drivers/usb/gadget/legacy/webcam.c
> @@ -42,7 +42,7 @@ MODULE_PARM_DESC(trace, "Trace level bitmask");
>   */
>  
>  #define WEBCAM_VENDOR_ID 0x1d6b  /* Linux Foundation */
> -#define WEBCAM_PRODUCT_ID0x0102  /* Webcam A/V gadget */
> +#define WEBCAM_PRODUCT_ID0x0106  /* UVC video gadget */
>  #define WEBCAM_DEVICE_BCD0x0010  /* 0.10 */
>  
>  static char webcam_vendor_label[] = "Linux Foundation";
> -- 
> 2.11.0
> --
> 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

-- 
balbi


signature.asc
Description: PGP signature


Re: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget

2017-01-23 Thread Greg Kroah-Hartman
On Mon, Jan 23, 2017 at 12:59:33PM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Petr Cvek  writes:
> > It seems the USB product ID for g_webcam usb device gadget is
> > incorrectly used from EEM gadget "Ethernet Emulation Model". So
> > "webcam" device has a confusing description in lsusb:
> >
> > 1d6b:0102 Linux Foundation EEM Gadget
> >
> > I would change it to 0x0106, which is a next unassigned value
> > (according to http://www.linux-usb.org/usb.ids). Does this step
> > require some official communication with the holder of VID (Linux
> > Foundation)? Or all it takes is just to send patch to the kernel and
> > to the usb.ids database?
> >
> > I know it is only a cosmetic change on a legacy driver, but I assume
> > it would be better to have some default value for configfs API than to
> > borrow a PID from a whole different gadget.
> >
> > Change in question would be something like this (I will send a proper
> > patch after RFC):
> >
> > Fixing USB Product ID for UVC gadget (used from Ethernet Emulation
> > Model) and changing it to new one.
> 
> Greg, do we still have an official Vendor ID?

Well, as official as we are allowed to get :)

> If so, any chance we can
> allocate a unique default ID for the webcam gadget?

Hm, I thought we wanted to get rid of the "legacy" gadget stuff.  Who is
using it these days?  It's not Android...

If it's really needed, then yes, you can use 0106, just let me know.
But do you really need a new id?  What's wrong with 0102?

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 2/8] usb: dwc3-omap: Fix missing break in dwc3_omap_set_mailbox()

2017-01-23 Thread Roger Quadros
We need to break from all cases if we want to treat
each one of them separately.

Fixes: d2728fb3e01f ("usb: dwc3: omap: Pass VBUS and ID events transparently")
Signed-off-by: Roger Quadros 
---
 drivers/usb/dwc3/dwc3-omap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index eb1b9cb..35b6351 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -250,6 +250,7 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
val = dwc3_omap_read_utmi_ctrl(omap);
val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG;
dwc3_omap_write_utmi_ctrl(omap, val);
+   break;
 
case OMAP_DWC3_VBUS_OFF:
val = dwc3_omap_read_utmi_ctrl(omap);
-- 
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


[PATCH 6/8] ARM: dts: dra7x-evm: Enable dual-role mode for USB1

2017-01-23 Thread Roger Quadros
USB1 port is micro-AB type and can function as peripheral
as well as host. Enable dual-role mode for USB1.

Signed-off-by: Roger Quadros 
---
 arch/arm/boot/dts/dra7-evm.dts  | 2 +-
 arch/arm/boot/dts/dra72-evm-common.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 132f2be..d64e72d 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -731,7 +731,7 @@
 };
 
 &usb1 {
-   dr_mode = "peripheral";
+   dr_mode = "otg";
pinctrl-names = "default";
pinctrl-0 = <&usb1_pins>;
 };
diff --git a/arch/arm/boot/dts/dra72-evm-common.dtsi 
b/arch/arm/boot/dts/dra72-evm-common.dtsi
index e50fbee..dc2c328 100644
--- a/arch/arm/boot/dts/dra72-evm-common.dtsi
+++ b/arch/arm/boot/dts/dra72-evm-common.dtsi
@@ -374,7 +374,7 @@
 };
 
 &usb1 {
-   dr_mode = "peripheral";
+   dr_mode = "otg";
 };
 
 &usb2 {
-- 
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


[PATCH 5/8] usb: dwc3: add dual-role support

2017-01-23 Thread Roger Quadros
If dr_mode is "otg" then support dual role mode of operation.

Get ID and VBUS information from the OTG controller
and put the controller in the appropriate state.

This is our dual-role state table.

ID  VBUSdual-role state
--  ---
0   x   A_HOST - Host controller active
1   0   B_IDLE - Both Host and Gadget controllers inactive
1   1   B_PERIPHERAL - Gadget controller active

Signed-off-by: Roger Quadros 
---
 drivers/usb/dwc3/core.c   | 583 --
 drivers/usb/dwc3/core.h   |  38 ++-
 drivers/usb/dwc3/gadget.c |  18 +-
 3 files changed, 615 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 369bab1..ca8b814 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -107,6 +108,7 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
reg = dwc3_readl(dwc->regs, DWC3_GCTL);
reg &= ~(DWC3_GCTL_PRTCAPDIR(DWC3_GCTL_PRTCAP_OTG));
reg |= DWC3_GCTL_PRTCAPDIR(mode);
+   dwc->current_mode = mode;
dwc3_writel(dwc->regs, DWC3_GCTL, reg);
 }
 
@@ -839,6 +841,495 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
return 0;
 }
 
+static int dwc3_drd_start_host(struct dwc3 *dwc, int on);
+static int dwc3_drd_start_gadget(struct dwc3 *dwc, int on);
+
+/* dwc->lock must be held */
+static void dwc3_drd_statemachine(struct dwc3 *dwc, int id, int vbus)
+{
+   enum usb_otg_state new_state;
+   int protocol;
+
+   if (id == dwc->otg_fsm.id && vbus == dwc->otg_fsm.b_sess_vld)
+   return;
+
+   dwc->otg_fsm.id = id;
+   dwc->otg_fsm.b_sess_vld = vbus;
+
+   if (!id) {
+   new_state = OTG_STATE_A_HOST;
+   } else{
+   if (vbus)
+   new_state = OTG_STATE_B_PERIPHERAL;
+   else
+   new_state = OTG_STATE_B_IDLE;
+   }
+
+   if (dwc->otg.state == new_state)
+   return;
+
+   protocol = dwc->otg_fsm.protocol;
+   switch (new_state) {
+   case OTG_STATE_B_IDLE:
+   if (protocol == PROTO_GADGET)
+   dwc3_drd_start_gadget(dwc, 0);
+   else if (protocol == PROTO_HOST)
+   dwc3_drd_start_host(dwc, 0);
+   dwc->otg_fsm.protocol = PROTO_UNDEF;
+   break;
+   case OTG_STATE_B_PERIPHERAL:
+   if (protocol == PROTO_HOST)
+   dwc3_drd_start_host(dwc, 0);
+
+   if (protocol != PROTO_GADGET) {
+   dwc->otg_fsm.protocol = PROTO_GADGET;
+   dwc3_drd_start_gadget(dwc, 1);
+   }
+   break;
+   case OTG_STATE_A_HOST:
+   if (protocol == PROTO_GADGET)
+   dwc3_drd_start_gadget(dwc, 0);
+
+   if (protocol != PROTO_HOST) {
+   dwc->otg_fsm.protocol = PROTO_HOST;
+   dwc3_drd_start_host(dwc, 1);
+   }
+   break;
+   default:
+   dev_err(dwc->dev, "drd: invalid usb-drd state: %s\n",
+   usb_otg_state_string(new_state));
+   return;
+   }
+
+   dwc->otg.state = new_state;
+}
+
+/* dwc->lock must be held */
+static void dwc3_otg_fsm_sync(struct dwc3 *dwc)
+{
+   u32 reg, osts;
+   int id, vbus;
+
+   /*
+* calling dwc3_otg_fsm_sync() during resume breaks host
+* if adapter was removed during suspend as xhci driver
+* is not prepared to see hcd removal before xhci_resume.
+*/
+   if (dwc->otg_prevent_sync)
+   return;
+
+   do {
+   reg = dwc3_readl(dwc->regs, DWC3_OSTS);
+
+   id = !!(reg & DWC3_OSTS_CONIDSTS);
+   vbus = !!(reg & DWC3_OSTS_BSESVLD);
+
+   dwc3_drd_statemachine(dwc, id, vbus);
+   osts = dwc3_readl(dwc->regs, DWC3_OSTS);
+   /*
+* OTG status might have changed and if we don't re-check
+* here we will loose events as OTG events have been
+* temporarily disabled.
+*/
+   /* FIXME: there is still a small time window where we can
+* miss an OTG event. i.e. from here till where we enable the
+* OEVTEN in OTG IRQ thread handler. Try another solution
+* where OEVTEN can be enabled but otg irq in GIC is disabled.
+* so any OTG changes can re-trigger the OTG IRQ after current
+* ISR ends.
+*/
+   } while (osts != reg);
+}
+
+static void dwc3_otg_mask_irq(struct dwc3 *dwc)
+{
+   dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
+   dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
+}
+
+static void dwc3_otg_unmask_irq(struct dwc3 *dwc)
+{
+   dwc3_wri

[PATCH 3/8] usb: dwc3: use BIT() macro where possible

2017-01-23 Thread Roger Quadros
To avoid checkpatch warnings with new patches let's
start using the BIT() macro wherever possible.

Signed-off-by: Roger Quadros 
---
 drivers/usb/dwc3/core.h  | 192 +--
 drivers/usb/dwc3/dwc3-omap.c |  48 +--
 drivers/usb/dwc3/gadget.h|  20 ++---
 3 files changed, 130 insertions(+), 130 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 14b7602..d514dca 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -65,7 +65,7 @@
 #define DWC3_DEVICE_EVENT_OVERFLOW 11
 
 #define DWC3_GEVNTCOUNT_MASK   0xfffc
-#define DWC3_GEVNTCOUNT_EHB(1 << 31)
+#define DWC3_GEVNTCOUNT_EHBBIT(31)
 #define DWC3_GSNPSID_MASK  0x
 #define DWC3_GSNPSREV_MASK 0x
 
@@ -175,11 +175,11 @@
 /* Global RX Threshold Configuration Register */
 #define DWC3_GRXTHRCFG_MAXRXBURSTSIZE(n) (((n) & 0x1f) << 19)
 #define DWC3_GRXTHRCFG_RXPKTCNT(n) (((n) & 0xf) << 24)
-#define DWC3_GRXTHRCFG_PKTCNTSEL (1 << 29)
+#define DWC3_GRXTHRCFG_PKTCNTSEL BIT(29)
 
 /* Global Configuration Register */
 #define DWC3_GCTL_PWRDNSCALE(n)((n) << 19)
-#define DWC3_GCTL_U2RSTECN (1 << 16)
+#define DWC3_GCTL_U2RSTECN BIT(16)
 #define DWC3_GCTL_RAMCLKSEL(x) (((x) & DWC3_GCTL_CLK_MASK) << 6)
 #define DWC3_GCTL_CLK_BUS  (0)
 #define DWC3_GCTL_CLK_PIPE (1)
@@ -192,24 +192,24 @@
 #define DWC3_GCTL_PRTCAP_DEVICE2
 #define DWC3_GCTL_PRTCAP_OTG   3
 
-#define DWC3_GCTL_CORESOFTRESET(1 << 11)
-#define DWC3_GCTL_SOFITPSYNC   (1 << 10)
+#define DWC3_GCTL_CORESOFTRESETBIT(11)
+#define DWC3_GCTL_SOFITPSYNC   BIT(10)
 #define DWC3_GCTL_SCALEDOWN(n) ((n) << 4)
 #define DWC3_GCTL_SCALEDOWN_MASK   DWC3_GCTL_SCALEDOWN(3)
-#define DWC3_GCTL_DISSCRAMBLE  (1 << 3)
-#define DWC3_GCTL_U2EXIT_LFPS  (1 << 2)
-#define DWC3_GCTL_GBLHIBERNATIONEN (1 << 1)
-#define DWC3_GCTL_DSBLCLKGTNG  (1 << 0)
+#define DWC3_GCTL_DISSCRAMBLE  BIT(3)
+#define DWC3_GCTL_U2EXIT_LFPS  BIT(2)
+#define DWC3_GCTL_GBLHIBERNATIONEN BIT(1)
+#define DWC3_GCTL_DSBLCLKGTNG  BIT(0)
 
 /* Global User Control 1 Register */
-#define DWC3_GUCTL1_DEV_L1_EXIT_BY_HW  (1 << 24)
+#define DWC3_GUCTL1_DEV_L1_EXIT_BY_HW  BIT(24)
 
 /* Global USB2 PHY Configuration Register */
-#define DWC3_GUSB2PHYCFG_PHYSOFTRST(1 << 31)
-#define DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS (1 << 30)
-#define DWC3_GUSB2PHYCFG_SUSPHY(1 << 6)
-#define DWC3_GUSB2PHYCFG_ULPI_UTMI (1 << 4)
-#define DWC3_GUSB2PHYCFG_ENBLSLPM  (1 << 8)
+#define DWC3_GUSB2PHYCFG_PHYSOFTRSTBIT(31)
+#define DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS BIT(30)
+#define DWC3_GUSB2PHYCFG_SUSPHYBIT(6)
+#define DWC3_GUSB2PHYCFG_ULPI_UTMI BIT(4)
+#define DWC3_GUSB2PHYCFG_ENBLSLPM  BIT(8)
 #define DWC3_GUSB2PHYCFG_PHYIF(n)  (n << 3)
 #define DWC3_GUSB2PHYCFG_PHYIF_MASKDWC3_GUSB2PHYCFG_PHYIF(1)
 #define DWC3_GUSB2PHYCFG_USBTRDTIM(n)  (n << 10)
@@ -220,25 +220,25 @@
 #define UTMI_PHYIF_8_BIT   0
 
 /* Global USB2 PHY Vendor Control Register */
-#define DWC3_GUSB2PHYACC_NEWREGREQ (1 << 25)
-#define DWC3_GUSB2PHYACC_BUSY  (1 << 23)
-#define DWC3_GUSB2PHYACC_WRITE (1 << 22)
+#define DWC3_GUSB2PHYACC_NEWREGREQ BIT(25)
+#define DWC3_GUSB2PHYACC_BUSY  BIT(23)
+#define DWC3_GUSB2PHYACC_WRITE BIT(22)
 #define DWC3_GUSB2PHYACC_ADDR(n)   (n << 16)
 #define DWC3_GUSB2PHYACC_EXTEND_ADDR(n)(n << 8)
 #define DWC3_GUSB2PHYACC_DATA(n)   (n & 0xff)
 
 /* Global USB3 PIPE Control Register */
-#define DWC3_GUSB3PIPECTL_PHYSOFTRST   (1 << 31)
-#define DWC3_GUSB3PIPECTL_U2SSINP3OK   (1 << 29)
-#define DWC3_GUSB3PIPECTL_DISRXDETINP3 (1 << 28)
-#define DWC3_GUSB3PIPECTL_REQP1P2P3(1 << 24)
+#define DWC3_GUSB3PIPECTL_PHYSOFTRST   BIT(31)
+#define DWC3_GUSB3PIPECTL_U2SSINP3OK   BIT(29)
+#define DWC3_GUSB3PIPECTL_DISRXDETINP3 BIT(28)
+#define DWC3_GUSB3PIPECTL_REQP1P2P3BIT(24)
 #define DWC3_GUSB3PIPECTL_DEP1P2P3(n)  ((n) << 19)
 #define DWC3_GUSB3PIPECTL_DEP1P2P3_MASKDWC3_GUSB3PIPECTL_DEP1P2P3(7)
 #define DWC3_GUSB3PIPECTL_DEP1P2P3_EN  DWC3_GUSB3PIPECTL_DEP1P2P3(1)
-#define DWC3_GUSB3PIPECTL_DEPOCHANGE   (1 << 18)
-#define DWC3_GUSB3PIPECTL_SUSPHY   (1 << 17)
-#define DWC3_GUSB3PIPECTL_LFPSFILT (1 << 9)
-#define DWC3_GUSB3PIPECTL_RX_DETOPOLL  (1 << 8)
+#define DWC3_GUSB3PIPECTL_DEPOCHANGE   BIT(18)
+#define DWC3_GUSB3PIPECTL_SUSPHY   BIT(17)
+#define DWC3_GUSB3PIPECTL_LFPSFILT BIT(9)
+#define DWC3_GUSB3PIPECTL_RX_DETOPOLL  BIT(8)
 #define DWC3_GUSB3PIPECTL_TX_DEEPH_MASKDWC3_GUSB3PIPECTL_TX_DEEPH(3)
 #define DWC3_GUSB3PIPECTL_TX_DEEPH(n)  ((n) << 1)
 
@@ -247,7 +247,7 @@
 #define DWC3_GTXFIFOSIZ_TXFSTADDR(n)   ((n) & 0x)
 
 /* Global Event Size Registers */
-#define DWC3_GEVNTSIZ_INTMASK  (1 << 31)
+#define DWC3_GEVNTSIZ_INTMASK  BIT(31)
 #define DWC3

[PATCH 0/8] usb: dwc3: add dual-role support

2017-01-23 Thread Roger Quadros
Hi,

We rely on the OTG controller block to provide us with
VBUS and ID line status via an interrupt.

This is then used to switch the controller between host, peripheral
and idle roles based on the following table.

ID  VBUSdual-role state
--  ---
0   x   A_HOST - Host controller active
1   0   B_IDLE - Both Host and Gadget controllers inactive
1   1   B_PERIPHERAL - Gadget controller active

Couple of things to clarify:
- There is a small window where we can potentially miss an
event related to OTG. I've added a comment in the code where this
could happen. How can we prevent this? Is it better to just leave
the OTG events unmasked (but keep otg_irq on ARM GIC disabled)
so that any new events can be captured by the OTG event register
and interrupt re-triggered if it has not been serviced by the
previous interrupt.
- I'm running the entire dual-role state change logic inside
the threaded interrupt handler with dwc->lock (spinlock) held
but IRQs enabled. OTG events are very rare i.e. manual intervention
so I don't see this as a problem. Just wanted to double check.
- Some SoC's (e.g. Qualcomm MSM) do not have the OTG controller block
but do have both host and peripheral controllers and so can operate
in dual role mode. Current series does not address this case.
We can get dual-role to work with such SoCs if core.c can get
information about ID and VBUS somehow (private interface
from parent or directly read extcon?).

cheers,
-roger

Roger Quadros (8):
  usb: otg-fsm: Prevent build warning "VDBG" redefined
  usb: dwc3-omap: Fix missing break in dwc3_omap_set_mailbox()
  usb: dwc3: use BIT() macro where possible
  usb: dwc3: core.h: add some register definitions
  usb: dwc3: add dual-role support
  ARM: dts: dra7x-evm: Enable dual-role mode for USB1
  ARM: dts: am43xx: Enable dual-role mode for USB1
  ARM: dts: am57xx-idk: Enable dual-role mode for USB2

 arch/arm/boot/dts/am437x-gp-evm.dts  |   2 +-
 arch/arm/boot/dts/am437x-sk-evm.dts  |   2 +-
 arch/arm/boot/dts/am43x-epos-evm.dts |   2 +-
 arch/arm/boot/dts/am57xx-idk-common.dtsi |   2 +-
 arch/arm/boot/dts/dra7-evm.dts   |   2 +-
 arch/arm/boot/dts/dra72-evm-common.dtsi  |   2 +-
 drivers/usb/common/usb-otg-fsm.c |   7 +
 drivers/usb/dwc3/core.c  | 583 ++-
 drivers/usb/dwc3/core.h  | 312 -
 drivers/usb/dwc3/dwc3-omap.c |  49 +--
 drivers/usb/dwc3/gadget.c|  18 +-
 drivers/usb/dwc3/gadget.h|  20 +-
 drivers/usb/phy/phy-fsl-usb.c|   7 +
 include/linux/usb/otg-fsm.h  |  15 -
 14 files changed, 848 insertions(+), 175 deletions(-)

-- 
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: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget

2017-01-23 Thread Greg Kroah-Hartman
On Mon, Jan 23, 2017 at 12:20:03PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Jan 23, 2017 at 12:59:33PM +0200, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > Petr Cvek  writes:
> > > It seems the USB product ID for g_webcam usb device gadget is
> > > incorrectly used from EEM gadget "Ethernet Emulation Model". So
> > > "webcam" device has a confusing description in lsusb:
> > >
> > >   1d6b:0102 Linux Foundation EEM Gadget
> > >
> > > I would change it to 0x0106, which is a next unassigned value
> > > (according to http://www.linux-usb.org/usb.ids). Does this step
> > > require some official communication with the holder of VID (Linux
> > > Foundation)? Or all it takes is just to send patch to the kernel and
> > > to the usb.ids database?
> > >
> > > I know it is only a cosmetic change on a legacy driver, but I assume
> > > it would be better to have some default value for configfs API than to
> > > borrow a PID from a whole different gadget.
> > >
> > > Change in question would be something like this (I will send a proper
> > > patch after RFC):
> > >
> > > Fixing USB Product ID for UVC gadget (used from Ethernet Emulation
> > > Model) and changing it to new one.
> > 
> > Greg, do we still have an official Vendor ID?
> 
> Well, as official as we are allowed to get :)
> 
> > If so, any chance we can
> > allocate a unique default ID for the webcam gadget?
> 
> Hm, I thought we wanted to get rid of the "legacy" gadget stuff.  Who is
> using it these days?  It's not Android...
> 
> If it's really needed, then yes, you can use 0106, just let me know.
> But do you really need a new id?  What's wrong with 0102?

Oops, no, 0106 is already reserved.  0107 is the next "free" number.

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 1/8] usb: otg-fsm: Prevent build warning "VDBG" redefined

2017-01-23 Thread Roger Quadros
If usb/otg-fsm.h and usb/composite.h are included together
then it results in the build warning [1].

Prevent that by defining VDBG locally.

Also get rid of MPC_LOC which doesn't seem to be used
by anyone.

[1] - warning fixed by this patch:

In file included from drivers/usb/dwc3/core.h:33,
   from drivers/usb/dwc3/ep0.c:33:
   include/linux/usb/otg-fsm.h:30:1: warning: "VDBG" redefined
   In file included from drivers/usb/dwc3/ep0.c:31:
   include/linux/usb/composite.h:615:1: warning: this is the location
   of the previous definition

Signed-off-by: Roger Quadros 
Reviewed-by: Jun Li 
Acked-by: Peter Chen 
---
 drivers/usb/common/usb-otg-fsm.c |  7 +++
 drivers/usb/phy/phy-fsl-usb.c|  7 +++
 include/linux/usb/otg-fsm.h  | 15 ---
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 2f537bb..b8fe31e 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -31,6 +31,13 @@
 #include 
 #include 
 
+#ifdef VERBOSE
+#define VDBG(fmt, args...) pr_debug("[%s]  " fmt, \
+__func__, ## args)
+#else
+#define VDBG(stuff...) do {} while (0)
+#endif
+
 /* Change USB protocol when there is a protocol change */
 static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
 {
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 94eb292..a8784ec 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -44,6 +44,13 @@
 
 #include "phy-fsl-usb.h"
 
+#ifdef VERBOSE
+#define VDBG(fmt, args...) pr_debug("[%s]  " fmt, \
+__func__, ## args)
+#else
+#define VDBG(stuff...) do {} while (0)
+#endif
+
 #define DRIVER_VERSION "Rev. 1.55"
 #define DRIVER_AUTHOR "Jerry Huang/Li Yang"
 #define DRIVER_DESC "Freescale USB OTG Transceiver Driver"
diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index 7a03505..a0a8f87 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -21,21 +21,6 @@
 #include 
 #include 
 
-#undef VERBOSE
-
-#ifdef VERBOSE
-#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
-__func__, ## args)
-#else
-#define VDBG(stuff...) do {} while (0)
-#endif
-
-#ifdef VERBOSE
-#define MPC_LOC printk("Current Location [%s]:[%d]\n", __FILE__, __LINE__)
-#else
-#define MPC_LOC do {} while (0)
-#endif
-
 #define PROTO_UNDEF(0)
 #define PROTO_HOST (1)
 #define PROTO_GADGET   (2)
-- 
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


[PATCH 7/8] ARM: dts: am43xx: Enable dual-role mode for USB1

2017-01-23 Thread Roger Quadros
USB1 port is micro-AB type and can function as peripheral
as well as host. Enable dual-role mode for USB1.

Signed-off-by: Roger Quadros 
---
 arch/arm/boot/dts/am437x-gp-evm.dts  | 2 +-
 arch/arm/boot/dts/am437x-sk-evm.dts  | 2 +-
 arch/arm/boot/dts/am43x-epos-evm.dts | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts 
b/arch/arm/boot/dts/am437x-gp-evm.dts
index 957840c..74728cc 100644
--- a/arch/arm/boot/dts/am437x-gp-evm.dts
+++ b/arch/arm/boot/dts/am437x-gp-evm.dts
@@ -772,7 +772,7 @@
 };
 
 &usb1 {
-   dr_mode = "peripheral";
+   dr_mode = "otg";
status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts 
b/arch/arm/boot/dts/am437x-sk-evm.dts
index 319d942..cfffce8 100644
--- a/arch/arm/boot/dts/am437x-sk-evm.dts
+++ b/arch/arm/boot/dts/am437x-sk-evm.dts
@@ -596,7 +596,7 @@
 };
 
 &usb1 {
-   dr_mode = "peripheral";
+   dr_mode = "otg";
status = "okay";
pinctrl-names = "default";
pinctrl-0 = <&usb1_pins>;
diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts 
b/arch/arm/boot/dts/am43x-epos-evm.dts
index 9d35c3f..8a48d9b 100644
--- a/arch/arm/boot/dts/am43x-epos-evm.dts
+++ b/arch/arm/boot/dts/am43x-epos-evm.dts
@@ -669,7 +669,7 @@
 };
 
 &usb1 {
-   dr_mode = "peripheral";
+   dr_mode = "otg";
status = "okay";
 };
 
-- 
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


[PATCH 4/8] usb: dwc3: core.h: add some register definitions

2017-01-23 Thread Roger Quadros
Add OTG and GHWPARAMS6 register definitions

Signed-off-by: Roger Quadros 
---
 drivers/usb/dwc3/core.h | 82 +
 1 file changed, 82 insertions(+)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index d514dca..fc82d2e 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -204,6 +204,15 @@
 /* Global User Control 1 Register */
 #define DWC3_GUCTL1_DEV_L1_EXIT_BY_HW  BIT(24)
 
+/* Global Status Register */
+#define DWC3_GSTS_OTG_IP   BIT(10)
+#define DWC3_GSTS_BC_IPBIT(9)
+#define DWC3_GSTS_ADP_IP   BIT(8)
+#define DWC3_GSTS_HOST_IP  BIT(7)
+#define DWC3_GSTS_DEVICE_IPBIT(6)
+#define DWC3_GSTS_CSR_TIMEOUT  BIT(5)
+#define DWC3_GSTS_BUS_ERR_ADDR_VLD BIT(4)
+
 /* Global USB2 PHY Configuration Register */
 #define DWC3_GUSB2PHYCFG_PHYSOFTRSTBIT(31)
 #define DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS BIT(30)
@@ -288,6 +297,11 @@
 #define DWC3_MAX_HIBER_SCRATCHBUFS 15
 
 /* Global HWPARAMS6 Register */
+#define DWC3_GHWPARAMS6_BCSUPPORT  BIT(14)
+#define DWC3_GHWPARAMS6_OTG3SUPPORTBIT(13)
+#define DWC3_GHWPARAMS6_ADPSUPPORT BIT(12)
+#define DWC3_GHWPARAMS6_HNPSUPPORT BIT(11)
+#define DWC3_GHWPARAMS6_SRPSUPPORT BIT(10)
 #define DWC3_GHWPARAMS6_EN_FPGABIT(7)
 
 /* Global HWPARAMS7 Register */
@@ -469,6 +483,74 @@
 #define DWC3_DEV_IMOD_INTERVAL_SHIFT   0
 #define DWC3_DEV_IMOD_INTERVAL_MASK(0x << 0)
 
+/* OTG Configuration Register */
+#define DWC3_OCFG_DISPWRCUTTOFFBIT(5)
+#define DWC3_OCFG_HIBDISMASK   BIT(4)
+#define DWC3_OCFG_SFTRSTMASK   BIT(3)
+#define DWC3_OCFG_OTGVERSION   BIT(2)
+#define DWC3_OCFG_HNPCAP   BIT(1)
+#define DWC3_OCFG_SRPCAP   BIT(0)
+
+/* OTG CTL Register */
+#define DWC3_OCTL_OTG3GOERRBIT(7)
+#define DWC3_OCTL_PERIMODE BIT(6)
+#define DWC3_OCTL_PRTPWRCTLBIT(5)
+#define DWC3_OCTL_HNPREQ   BIT(4)
+#define DWC3_OCTL_SESREQ   BIT(3)
+#define DWC3_OCTL_TERMSELIDPULSE   BIT(2)
+#define DWC3_OCTL_DEVSETHNPEN  BIT(1)
+#define DWC3_OCTL_HSTSETHNPEN  BIT(0)
+
+/* OTG Event Register */
+#define DWC3_OEVT_DEVICEMODE   BIT(31)
+#define DWC3_OEVT_XHCIRUNSTPSETBIT(27)
+#define DWC3_OEVT_DEVRUNSTPSET BIT(26)
+#define DWC3_OEVT_HIBENTRY BIT(25)
+#define DWC3_OEVT_CONIDSTSCHNG BIT(24)
+#define DWC3_OEVT_HRRCONFNOTIF BIT(23)
+#define DWC3_OEVT_HRRINITNOTIF BIT(22)
+#define DWC3_OEVT_ADEVIDLE BIT(21)
+#define DWC3_OEVT_ADEVBHOSTEND BIT(20)
+#define DWC3_OEVT_ADEVHOST BIT(19)
+#define DWC3_OEVT_ADEVHNPCHNG  BIT(18)
+#define DWC3_OEVT_ADEVSRPDET   BIT(17)
+#define DWC3_OEVT_ADEVSESSENDDET   BIT(16)
+#define DWC3_OEVT_BDEVBHOSTEND BIT(11)
+#define DWC3_OEVT_BDEVHNPCHNG  BIT(10)
+#define DWC3_OEVT_BDEVSESSVLDDET   BIT(9)
+#define DWC3_OEVT_BDEVVBUSCHNG BIT(8)
+#define DWC3_OEVT_BSESSVLD BIT(3)
+#define DWC3_OEVT_HSTNEGSTSBIT(2)
+#define DWC3_OEVT_SESREQSTSBIT(1)
+#define DWC3_OEVT_ERRORBIT(0)
+
+/* OTG Event Enable Register */
+#define DWC3_OEVTEN_XHCIRUNSTPSETENBIT(27)
+#define DWC3_OEVTEN_DEVRUNSTPSETEN BIT(26)
+#define DWC3_OEVTEN_HIBENTRYEN BIT(25)
+#define DWC3_OEVTEN_CONIDSTSCHNGEN BIT(24)
+#define DWC3_OEVTEN_HRRCONFNOTIFEN BIT(23)
+#define DWC3_OEVTEN_HRRINITNOTIFEN BIT(22)
+#define DWC3_OEVTEN_ADEVIDLEEN BIT(21)
+#define DWC3_OEVTEN_ADEVBHOSTENDEN BIT(20)
+#define DWC3_OEVTEN_ADEVHOSTEN BIT(19)
+#define DWC3_OEVTEN_ADEVHNPCHNGEN  BIT(18)
+#define DWC3_OEVTEN_ADEVSRPDETEN   BIT(17)
+#define DWC3_OEVTEN_ADEVSESSENDDETEN   BIT(16)
+#define DWC3_OEVTEN_BDEVHOSTENDEN  BIT(11)
+#define DWC3_OEVTEN_BDEVHNPCHNGEN  BIT(10)
+#define DWC3_OEVTEN_BDEVSESSVLDDETEN   BIT(9)
+#define DWC3_OEVTEN_BDEVVBUSCHNGE  BIT(8)
+
+/* OTG Status Register */
+#define DWC3_OSTS_DEVRUNSTPBIT(13)
+#define DWC3_OSTS_XHCIRUNSTP   BIT(12)
+#define DWC3_OSTS_PERIPHERALSTATE  BIT(4)
+#define DWC3_OSTS_XHCIPRTPOWER BIT(3)
+#define DWC3_OSTS_BSESVLD  BIT(2)
+#define DWC3_OSTS_VBUSVLD  BIT(1)
+#define DWC3_OSTS_CONIDSTS BIT(0)
+
 /* Structures */
 
 struct dwc3_trb;
-- 
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


[PATCH 8/8] ARM: dts: am57xx-idk: Enable dual-role mode for USB2

2017-01-23 Thread Roger Quadros
USB port is micro-AB type and can function as peripheral
as well as host. Enable dual-role mode for USB2.

Signed-off-by: Roger Quadros 
---
 arch/arm/boot/dts/am57xx-idk-common.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/am57xx-idk-common.dtsi 
b/arch/arm/boot/dts/am57xx-idk-common.dtsi
index 814a720..14bdb24 100644
--- a/arch/arm/boot/dts/am57xx-idk-common.dtsi
+++ b/arch/arm/boot/dts/am57xx-idk-common.dtsi
@@ -376,7 +376,7 @@
 };
 
 &usb2 {
-   dr_mode = "peripheral";
+   dr_mode = "otg";
 };
 
 &mmc2 {
-- 
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: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget

2017-01-23 Thread Greg KH
On Mon, Jan 23, 2017 at 11:30:01AM +0100, Petr Cvek wrote:
> Hello,
> 
> It seems the USB product ID for g_webcam usb device gadget is incorrectly 
> used from EEM gadget "Ethernet Emulation Model". So "webcam" device has a 
> confusing description in lsusb:
> 
>   1d6b:0102 Linux Foundation EEM Gadget
> 
> I would change it to 0x0106, which is a next unassigned value
> (according to http://www.linux-usb.org/usb.ids). Does this step
> require some official communication with the holder of VID (Linux
> Foundation)? Or all it takes is just to send patch to the kernel and
> to the usb.ids database?

Note, usb.ids doesn't seem to have picked up the last few changes I
asked to reserve there, here's the latest reserved id list from the LF
that I am in charge of:

# Linux Foundation USB id list.
1d6b  Linux Foundation
0001  1.1 root hub
0002  2.0 root hub
0003  3.0 root hub
0100  PTP Gadget
0101  Audio Gadget
0102  EEM Gadget
0103  NCM (Ethernet) Gadget
0104  Multifunction Composite Gadget
0105  FunctionFS Gadget
0106  Composite Gadget: ACM + Mass Storage
0200  Qemu Audio Device
0246  BlueZ Host Stack
0247  BlueZ for Android

> I know it is only a cosmetic change on a legacy driver, but I assume
> it would be better to have some default value for configfs API than to
> borrow a PID from a whole different gadget.

For class devices, they really don't need a new id, we should just use
only one of them as it doesn't matter :)

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: gadget: uvc: webcam gadget USB PID is using value from a different gadget

2017-01-23 Thread Felipe Balbi

Hi,

Greg KH  writes:
> On Mon, Jan 23, 2017 at 11:30:01AM +0100, Petr Cvek wrote:
>> Hello,
>> 
>> It seems the USB product ID for g_webcam usb device gadget is incorrectly 
>> used from EEM gadget "Ethernet Emulation Model". So "webcam" device has a 
>> confusing description in lsusb:
>> 
>>  1d6b:0102 Linux Foundation EEM Gadget
>> 
>> I would change it to 0x0106, which is a next unassigned value
>> (according to http://www.linux-usb.org/usb.ids). Does this step
>> require some official communication with the holder of VID (Linux
>> Foundation)? Or all it takes is just to send patch to the kernel and
>> to the usb.ids database?
>
> Note, usb.ids doesn't seem to have picked up the last few changes I
> asked to reserve there, here's the latest reserved id list from the LF
> that I am in charge of:
>
> # Linux Foundation USB id list.
> 1d6b  Linux Foundation
>   0001  1.1 root hub
>   0002  2.0 root hub
>   0003  3.0 root hub
>   0100  PTP Gadget
>   0101  Audio Gadget
>   0102  EEM Gadget
>   0103  NCM (Ethernet) Gadget
>   0104  Multifunction Composite Gadget
>   0105  FunctionFS Gadget
>   0106  Composite Gadget: ACM + Mass Storage
>   0200  Qemu Audio Device
>   0246  BlueZ Host Stack
>   0247  BlueZ for Android
>
>> I know it is only a cosmetic change on a legacy driver, but I assume
>> it would be better to have some default value for configfs API than to
>> borrow a PID from a whole different gadget.
>
> For class devices, they really don't need a new id, we should just use
> only one of them as it doesn't matter :)

fine by me. Just lsusb will look funky ;-)

-- 
balbi


signature.asc
Description: PGP signature


Re: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget

2017-01-23 Thread Greg KH
On Mon, Jan 23, 2017 at 01:27:59PM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Greg KH  writes:
> > On Mon, Jan 23, 2017 at 11:30:01AM +0100, Petr Cvek wrote:
> >> Hello,
> >> 
> >> It seems the USB product ID for g_webcam usb device gadget is incorrectly 
> >> used from EEM gadget "Ethernet Emulation Model". So "webcam" device has a 
> >> confusing description in lsusb:
> >> 
> >>1d6b:0102 Linux Foundation EEM Gadget
> >> 
> >> I would change it to 0x0106, which is a next unassigned value
> >> (according to http://www.linux-usb.org/usb.ids). Does this step
> >> require some official communication with the holder of VID (Linux
> >> Foundation)? Or all it takes is just to send patch to the kernel and
> >> to the usb.ids database?
> >
> > Note, usb.ids doesn't seem to have picked up the last few changes I
> > asked to reserve there, here's the latest reserved id list from the LF
> > that I am in charge of:
> >
> > # Linux Foundation USB id list.
> > 1d6b  Linux Foundation
> > 0001  1.1 root hub
> > 0002  2.0 root hub
> > 0003  3.0 root hub
> > 0100  PTP Gadget
> > 0101  Audio Gadget
> > 0102  EEM Gadget
> > 0103  NCM (Ethernet) Gadget
> > 0104  Multifunction Composite Gadget
> > 0105  FunctionFS Gadget
> > 0106  Composite Gadget: ACM + Mass Storage
> > 0200  Qemu Audio Device
> > 0246  BlueZ Host Stack
> > 0247  BlueZ for Android
> >
> >> I know it is only a cosmetic change on a legacy driver, but I assume
> >> it would be better to have some default value for configfs API than to
> >> borrow a PID from a whole different gadget.
> >
> > For class devices, they really don't need a new id, we should just use
> > only one of them as it doesn't matter :)
> 
> fine by me. Just lsusb will look funky ;-)

Heh, true, but I thought lsusb would use a string if the device provided
it.  Haven't looked at that portion of the code in a very long time...
--
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/4] usb: gadget: f_hid: fix: Move IN request allocation to set_alt()

2017-01-23 Thread Felipe Balbi
Krzysztof Opasiak  writes:

> Since commit ba1582f22231 ("usb: gadget: f_hid: use alloc_ep_req()")
> we cannot allocate any requests in bind() as we check if we should
> align request buffer based on endpoint descriptor which is assigned
> in set_alt().
>
> Allocating request in bind() function causes a NULL pointer
> dereference.
>
> This commit moves allocation of IN request from bind() to set_alt()
> to prevent this issue.
>
> Fixes: ba1582f22231 ("usb: gadget: f_hid: use alloc_ep_req()")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Krzysztof Opasiak 

please rebase on testing/next:

checking file drivers/usb/gadget/function/f_hid.c
Hunk #1 succeeded at 338 (offset 54 lines).
Hunk #2 succeeded at 348 (offset 54 lines).
Hunk #3 succeeded at 363 (offset 54 lines).
Hunk #4 succeeded at 376 (offset 54 lines).
Hunk #5 succeeded at 611 (offset 54 lines).
Hunk #6 succeeded at 648 (offset 54 lines).
Hunk #7 FAILED at 611.
Hunk #8 succeeded at 686 (offset 54 lines).
Hunk #9 succeeded at 765 (offset 54 lines).
Hunk #10 succeeded at 802 (offset 65 lines).
Hunk #11 succeeded at 1068 (offset 65 lines).
1 out of 11 hunks FAILED

previous 3 patches applied fine, though.

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH 0/8] usb: dwc3: add dual-role support

2017-01-23 Thread Chanwoo Choi
Hi Roger,

On 2017년 01월 23일 20:19, Roger Quadros wrote:
> - Some SoC's (e.g. Qualcomm MSM) do not have the OTG controller block
> but do have both host and peripheral controllers and so can operate
> in dual role mode. Current series does not address this case.
> We can get dual-role to work with such SoCs if core.c can get
> information about ID and VBUS somehow (private interface
> from parent or directly read extcon?).

Also, Exynos SoC doesn't include the physical otg block
to detect the type of external connector(HOST or Peripheral).

So, our team member and me are preparing the patches
to identify the type of external connector with extcon
This approach is not dependent on any specific SoC.

Maybe, we will send the RFC patches within this month.

-- 
Best Regards,
Chanwoo Choi
S/W R&D Center
Samsung Electronics
--
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/28] usb: dwc2: Remove unused otg_ver parameter

2017-01-23 Thread Felipe Balbi
John Youn  writes:

> The otg_ver parameter only controls the SRP pulsing method and defaults
> to the 1.3 behavior. It is unused and can be removed.
>
> Signed-off-by: John Youn 

this patch fails to apply:

checking file drivers/usb/dwc2/core.c
checking file drivers/usb/dwc2/core.h
Hunk #2 succeeded at 467 (offset 6 lines).
Hunk #3 succeeded at 1180 (offset 9 lines).
checking file drivers/usb/dwc2/hcd.c
Hunk #1 succeeded at 2258 (offset -2 lines).
checking file drivers/usb/dwc2/params.c
Hunk #3 succeeded at 106 (offset 7 lines).
Hunk #4 succeeded at 136 (offset 7 lines).
Hunk #5 succeeded at 166 (offset 7 lines).
Hunk #6 succeeded at 196 (offset 7 lines).
Hunk #7 succeeded at 970 with fuzz 1 (offset -2 lines).
Hunk #8 FAILED at 1132.
1 out of 8 hunks FAILED

please rebase on testing/next

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH 2/2] usb: gadget: uac2: add req_number as parameter

2017-01-23 Thread Felipe Balbi

Hi,

Peter Chen  writes:
> On Tue, Jan 17, 2017 at 05:57:50PM +0800, Peter Chen wrote:
>> On Tue, Jan 17, 2017 at 11:23:55AM +0200, Felipe Balbi wrote:
>> > 
>> > Hi,
>> > 
>> > Peter Chen  writes:
>> > > On Mon, Jan 16, 2017 at 12:40:06PM +0200, Felipe Balbi wrote:
>> > >> 
>> > >> Hi,
>> > >> 
>> > >> Peter Chen  writes:
>> > >> > There are only two requests for uac2, it may not be enough at high
>> > >> > loading system which usb interrupt handler can't be serviced on
>> > >> > time, then the data will be lost since it is isoc transfer for audio.
>> > >> >
>> > >> > In this patch, we introduce a parameter for the number for usb 
>> > >> > request,
>> > >> > and the user can override it if current number for request is not 
>> > >> > enough
>> > >> > for his/her use case.
>> > >> >
>> > >> > Besides, update this parameter for legacy audio gadget and 
>> > >> > documentation.
>> > >> 
>> > >> I would rather just drop pre-allocation of requests. Every time Audio
>> > >> wants transmit data you allocate and queue a request there and free()
>> > >> the request on completion.
>> > >> 
>> > >> All of these functions already have a ton of parameters :-s
>> > >> 
>> > >
>> > > At high loading system, how can we make sure the interrupt can be
>> > > serviced in one isoc time slice? We ran out this problem at one
>> > > customer project, and even at default mainline, the aplay will
>> > > meet underrun using UAC2.
>> > 
>> > well, if you don't have any limits to how deep your queue can be, that
>> > should cover it, no? Another way to look at this is that 2 requests
>> > really _is_ too little, and that can be calculated. How much data do you
>> > want to send per interval and what is the interval? Currently, uac2 is
>> > using 1024-byte requests with bInterval of 4, which means an interval of
>> > 1ms (2^(4-1) * 125 us = 1000 us).
>> > 
>> > I can see how we would miss an interval if our CPU is hogged by another
>> > task and our interrupt handler can't run for over 1ms. Maybe, as a fix,
>> > we should just increase USB_XFERS to a value that would work. Doubling
>> > it will make us allocate exactly one PAGE (4096) for rbuf. What are you
>> > passing as req_number on this customer project?
>> > 
>> > -- 
>> > balbi
>> 
>> Set req_number as 4 can let aplay work well using mainline kernel.
>> For the customer project, the number is 20 since they don't care
>> the memory. The system latency and memory requirement are different
>> per projects, why not let user choose it?
>> 
>
> Any more comments, Felipe? If you don't like extra parameter, I can
> have a patch to change USB_XFERS as 4.

I've applied with the extra parameters. You're right, if we just change
USB_XFERS to 4 we're probably gonna have that parameter being changed
every few months or so.

-- 
balbi


signature.asc
Description: PGP signature


Re: [BISSECTED]: BUG: usb: dwc3: usb ports not working anymore on odroid-XU4

2017-01-23 Thread Felipe Balbi

Hi,

Richard Genoud  writes:
> On 19/01/2017 09:03, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Richard Genoud  writes:
>>> Hi,
>>> Since commit c499ff71ff2a2 ("usb: dwc3: core: re-factor init and exit 
>>> paths")
>>> (merged in 4.8), the usb ports on odroid-XU4 don't work anymore.
>>>
>>> [ Actually, it's commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all 
>>> cores"), cf below ]
>>>
>>> Inserting an usb key (USB2.0) on the USB3.0 port result in:
>>> [   64.488264] xhci-hcd xhci-hcd.2.auto: Port resume took longer than 2 
>>> msec, port status = 0xc400fe3
>>> [   74.568156] xhci-hcd xhci-hcd.2.auto: xHCI host not responding to stop 
>>> endpoint command.
>>> [   74.574806] xhci-hcd xhci-hcd.2.auto: Assuming host is dying, halting 
>>> host.
>>> [   74.601970] xhci-hcd xhci-hcd.2.auto: HC died; cleaning up
>>> [   74.606276] usb 3-1: USB disconnect, device number 2
>>> [   74.613565] usb 4-1: USB disconnect, device number 2
>>> [   74.621208] usb usb3-port1: couldn't allocate usb_device
>>> NB: it's not related to USB2.0 devices, I get the same result with an 
>>> USB3.0 device (SATA to USB3 for instance).
>>> NB2: it doesn't happen on an odriod-XU3 board, that doesn't have the 
>>> realtek RTL8153 chip.
>>>
>>> I instrumented what was read/written in the registers before and after this 
>>> patch, and I found that the culprit is:
>>> if (dwc->revision > DWC3_REVISION_194A)
>>> reg |= DWC3_GUSB3PIPECTL_SUSPHY;
>>> Before commit c499ff71ff2a2 ("usb: dwc3: core: re-factor init and exit 
>>> paths"), dwc3_phy_setup() was
>>> done early in dwc3_probe() and thus, dwc->revision wasn't set yet (==0)
>>> After this commit, dwc3_phy_setup() is done in dwc3_core_init(), after 
>>> setting dwc->revision (it's done in dwc3_core_is_valid()).
>>>
>>> (The dwc3->revision on odroid-XU4 is 5533200a.)
>>>
>>> If I comment out the 2 lines:
>>> if (dwc->revision > DWC3_REVISION_194A)
>>> reg |= DWC3_GUSB3PIPECTL_SUSPHY;
>>> The usb key is recognized right away:
>>> [   38.008158] usb 3-1.2: new high-speed USB device number 3 using xhci-hcd
>>> [   38.138924] usb 3-1.2: New USB device found, idVendor=abcd, 
>>> idProduct=1234
>>> [   38.144399] usb 3-1.2: New USB device strings: Mfr=1, Product=2, 
>>> SerialNumber=3
>>>
>>> I took a look at the history behind those 2 lines, and they've been 
>>> introduced by:
>>> commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores") in 3.19.
>>> In 3.19, the dwc->revision dwc3_phy_setup() was set in dwc3_core_init().
>>> And, booting a 3.19 kernel on XU4 gives the same error (whereas booting a 
>>> 3.18 kernel is ok)
>>>
>>> [ So, I should have started this email with:
>>> Since commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores"),
>>> the usb ports on odroid-XU4 don't work anymore. ]
>>>
>>> The funny thing is that commit 45bb7de213d8("usb: dwc3: setup phys 
>>> earlier") (merged in 4.2)
>>> moved dwc3_phy_setup() in dwc3_probe(), way before dwc->revision was set, 
>>> acting like a revert
>>> on commit 2164a476205c("usb: dwc3: set SUSPHY bit for all cores")
>>> That's why on 4.2, inserting an USB key on odroid-XU4 worked again,
>>>
>>> So, to make a resume:
>>> xxx-> 3.18: usb ok
>>> 3.19->4.1: usb error (due to commit 2164a476205ccc ("usb: dwc3: set SUSPHY 
>>> bit for all cores"))
>>> 4.2->4.7: usb ok (due to commit 45bb7de213d8("usb: dwc3: setup phys 
>>> earlier"))
>>> 4.8->now: usb error (due to commit c499ff71ff2a2 ("usb: dwc3: core: 
>>> re-factor init and exit paths"))
>>>
>>>
>>> any idea on this ?
>> 
>> Thanks for bisecting. This could be caused by a similar case as
>> described at [1]. Can you check if adding quirk
>> "snps,dis_u3_susphy_quirk" helps ?
>> 
>> [1] 
>> https://marc.info/?i=2b3535c5ece8b5419e3ecbe300772909021b3fd...@us01wembx2.internal.synopsys.com
>> 
> Thanks for your quick answer !
>
> yes, adding this patch:
>
> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu4.dts 
> b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
> index 2faf88627a48..c76e76bc9ade 100644
> --- a/arch/arm/boot/dts/exynos5422-odroidxu4.dts
> +++ b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
> @@ -43,6 +43,9 @@
>   status = "okay";
>  };
>  
> +&usbdrd_dwc3_0 {
> + snps,dis_u3_susphy_quirk;
> +};
>  &usbdrd_dwc3_1 {
>   dr_mode = "host";
>  };
>
> removes the error "Port resume took longer than 2 msec, port
> status = 0xc400fe3" and the USB devices are detected.

okay great. Temporarily, you can probably push a patch doing that to
your DTS. At least you're gonna have things working until we understand
what's going on with DWC3.

-- 
balbi


signature.asc
Description: PGP signature


Re: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget

2017-01-23 Thread Petr Cvek
Dne 23.1.2017 v 12:32 Greg KH napsal(a):
 I know it is only a cosmetic change on a legacy driver, but I assume
 it would be better to have some default value for configfs API than to
 borrow a PID from a whole different gadget.
>>>
>>> For class devices, they really don't need a new id, we should just use
>>> only one of them as it doesn't matter :)
>>

So using 0106 "Composite gadget: ACM + Mass Storage" or 0104 "Multifunction 
Composite Gadget" should be fine? (my actual setup is not multifunction though).

I'm actually trying to catch bug, when using configfs access webcam does not 
work or kernel oopses (and webcam does not work) :-D.

>> fine by me. Just lsusb will look funky ;-)
> 
> Heh, true, but I thought lsusb would use a string if the device provided
> it.  Haven't looked at that portion of the code in a very long time...
> 

My lsusb shows separate strings (using usbutils from slackware64-current):

Bus 004 Device 003: ID 1d6b:0102 Linux Foundation EEM Gadget
...
  idVendor   0x1d6b Linux Foundation
  idProduct  0x0102 EEM Gadget
  bcdDevice4.07
  iManufacturer   1 Linux Foundation
  iProduct2 Webcam gadget
...


Best regards,
Petr

--
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 2/4] usb: dwc2: host: Correct snpsid checking for GDFIFOCFG

2017-01-23 Thread Felipe Balbi
John Youn  writes:
> From: Sevak Arakelyan 
>
> GDFIFOCFG is available from IP version 2.91a. Fix the code to reflect
> this.
>
> Signed-off-by: Sevak Arakelyan 
> Signed-off-by: John Youn 

doesn't apply :-( please rebase

-- 
balbi


signature.asc
Description: PGP signature


Re: [BISSECTED]: BUG: usb: dwc3: usb ports not working anymore on odroid-XU4

2017-01-23 Thread Richard Genoud
2017-01-23 12:45 GMT+01:00 Felipe Balbi :
>
> Hi,
>
> Richard Genoud  writes:
>> On 19/01/2017 09:03, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> Richard Genoud  writes:
 Hi,
 Since commit c499ff71ff2a2 ("usb: dwc3: core: re-factor init and exit 
 paths")
 (merged in 4.8), the usb ports on odroid-XU4 don't work anymore.

 [ Actually, it's commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all 
 cores"), cf below ]

 Inserting an usb key (USB2.0) on the USB3.0 port result in:
 [   64.488264] xhci-hcd xhci-hcd.2.auto: Port resume took longer than 
 2 msec, port status = 0xc400fe3
 [   74.568156] xhci-hcd xhci-hcd.2.auto: xHCI host not responding to stop 
 endpoint command.
 [   74.574806] xhci-hcd xhci-hcd.2.auto: Assuming host is dying, halting 
 host.
 [   74.601970] xhci-hcd xhci-hcd.2.auto: HC died; cleaning up
 [   74.606276] usb 3-1: USB disconnect, device number 2
 [   74.613565] usb 4-1: USB disconnect, device number 2
 [   74.621208] usb usb3-port1: couldn't allocate usb_device
 NB: it's not related to USB2.0 devices, I get the same result with an 
 USB3.0 device (SATA to USB3 for instance).
 NB2: it doesn't happen on an odriod-XU3 board, that doesn't have the 
 realtek RTL8153 chip.

 I instrumented what was read/written in the registers before and after 
 this patch, and I found that the culprit is:
 if (dwc->revision > DWC3_REVISION_194A)
 reg |= DWC3_GUSB3PIPECTL_SUSPHY;
 Before commit c499ff71ff2a2 ("usb: dwc3: core: re-factor init and exit 
 paths"), dwc3_phy_setup() was
 done early in dwc3_probe() and thus, dwc->revision wasn't set yet (==0)
 After this commit, dwc3_phy_setup() is done in dwc3_core_init(), after 
 setting dwc->revision (it's done in dwc3_core_is_valid()).

 (The dwc3->revision on odroid-XU4 is 5533200a.)

 If I comment out the 2 lines:
 if (dwc->revision > DWC3_REVISION_194A)
 reg |= DWC3_GUSB3PIPECTL_SUSPHY;
 The usb key is recognized right away:
 [   38.008158] usb 3-1.2: new high-speed USB device number 3 using xhci-hcd
 [   38.138924] usb 3-1.2: New USB device found, idVendor=abcd, 
 idProduct=1234
 [   38.144399] usb 3-1.2: New USB device strings: Mfr=1, Product=2, 
 SerialNumber=3

 I took a look at the history behind those 2 lines, and they've been 
 introduced by:
 commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores") in 3.19.
 In 3.19, the dwc->revision dwc3_phy_setup() was set in dwc3_core_init().
 And, booting a 3.19 kernel on XU4 gives the same error (whereas booting a 
 3.18 kernel is ok)

 [ So, I should have started this email with:
 Since commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores"),
 the usb ports on odroid-XU4 don't work anymore. ]

 The funny thing is that commit 45bb7de213d8("usb: dwc3: setup phys 
 earlier") (merged in 4.2)
 moved dwc3_phy_setup() in dwc3_probe(), way before dwc->revision was set, 
 acting like a revert
 on commit 2164a476205c("usb: dwc3: set SUSPHY bit for all cores")
 That's why on 4.2, inserting an USB key on odroid-XU4 worked again,

 So, to make a resume:
 xxx-> 3.18: usb ok
 3.19->4.1: usb error (due to commit 2164a476205ccc ("usb: dwc3: set SUSPHY 
 bit for all cores"))
 4.2->4.7: usb ok (due to commit 45bb7de213d8("usb: dwc3: setup phys 
 earlier"))
 4.8->now: usb error (due to commit c499ff71ff2a2 ("usb: dwc3: core: 
 re-factor init and exit paths"))


 any idea on this ?
>>>
>>> Thanks for bisecting. This could be caused by a similar case as
>>> described at [1]. Can you check if adding quirk
>>> "snps,dis_u3_susphy_quirk" helps ?
>>>
>>> [1] 
>>> https://marc.info/?i=2b3535c5ece8b5419e3ecbe300772909021b3fd...@us01wembx2.internal.synopsys.com
>>>
>> Thanks for your quick answer !
>>
>> yes, adding this patch:
>>
>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu4.dts 
>> b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
>> index 2faf88627a48..c76e76bc9ade 100644
>> --- a/arch/arm/boot/dts/exynos5422-odroidxu4.dts
>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
>> @@ -43,6 +43,9 @@
>>   status = "okay";
>>  };
>>
>> +&usbdrd_dwc3_0 {
>> + snps,dis_u3_susphy_quirk;
>> +};
>>  &usbdrd_dwc3_1 {
>>   dr_mode = "host";
>>  };
>>
>> removes the error "Port resume took longer than 2 msec, port
>> status = 0xc400fe3" and the USB devices are detected.
>
> okay great. Temporarily, you can probably push a patch doing that to
> your DTS. At least you're gonna have things working until we understand
> what's going on with DWC3.
Ok.

Thanks !

Richard.
--
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: dwc2: fix "iomem 0x00000000" message by setting iomem parameters in usb_hcd

2017-01-23 Thread Felipe Balbi

Hi,

John Youn  writes:
> @@ -1229,7 +1229,8 @@ static inline void dwc2_hcd_connect(struct 
> dwc2_hsotg *hsotg) {}
>  static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool 
> force) {}
>  static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
>  static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
> -static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
> +static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
> + struct resource *res)
>  { return 0; }
>  static inline int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg)
>  { return 0; }
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 911c3b36..2cfbd10e 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -4964,7 +4964,7 @@ static void dwc2_hcd_release(struct dwc2_hsotg 
> *hsotg)
>   * USB bus with the core and calls the hc_driver->start() function. It 
> returns
>   * a negative error on failure.
>   */
> -int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
> +int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq, struct resource 
> *res)
>  {
>   struct usb_hcd *hcd;
>   struct dwc2_host_chan *channel;
> @@ -5021,6 +5021,9 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
>
>   hcd->has_tt = 1;
>
> + hcd->rsrc_start = res->start;
> + hcd->rsrc_len = resource_size(res);
> +
>   ((struct wrapper_priv_data *) &hcd->hcd_priv)->hsotg = hsotg;
>   hsotg->priv = hcd;
>
> diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
> index 1ed5fa2b..2305b5fb 100644
> --- a/drivers/usb/dwc2/hcd.h
> +++ b/drivers/usb/dwc2/hcd.h
> @@ -521,7 +521,8 @@ static inline u8 dwc2_hcd_is_pipe_out(struct 
> dwc2_hcd_pipe_info *pipe)
>   return !dwc2_hcd_is_pipe_in(pipe);
>  }
>
> -extern int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq);
> +extern int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
> +  struct resource *res);
>  extern void dwc2_hcd_remove(struct dwc2_hsotg *hsotg);
>
>  /* Transaction Execution Functions */
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index 4fc8c603..5ddc8860 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -445,7 +445,7 @@ static int dwc2_driver_probe(struct platform_device 
> *dev)
>   }
>
>   if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) {
> - retval = dwc2_hcd_init(hsotg, hsotg->irq);
> + retval = dwc2_hcd_init(hsotg, hsotg->irq, res);

 This is a good idea, but there's more work that can come out of this, if
 you're interested:

 i) devm_ioremap_resource() could be called by the generic layer
 ii) devm_request_irq() could be move to the generic layer
 iii) dwc2_hcd_init() could also be called generically as long as dr_mode
  is set properly.
 iv) dwc2_debugfs_init() could be called generically as well

 IOW, dwc2_driver_probe() could be as minimal as:

 static int dwc2_driver_probe(struct platform_device *dev)
 {
struct dwc2_hsotg *hsotg;
struct resource *res;
int retval;

hsotg = devm_kzalloc(&dev->dev, sizeof(*hsotg), GFP_KERNEL);
if (!hsotg)
return -ENOMEM;

hsotg->dev = &dev->dev;

if (!dev->dev.dma_mask)
dev->dev.dma_mask = &dev->dev.coherent_dma_mask;

retval = dma_set_coherent_mask(&dev->dev, DMA_BIT_MASK(32));
if (retval)
return retval;

hsotg->res = platform_get_resource(dev, IORESOURCE_MEM, 0);
hsotg->irq = platform_get_irq(dev, 0);

retval = dwc2_get_dr_mode(hsotg);
if (retval)
return retval;

retval = dwc2_get_hwparams(hsotg);
if (retval)
return retval;

platform_set_drvdata(dev, hsotg);

retval = dwc2_core_init(hsotg);
if (retval)
return retval;

return 0;
 }

 dwc2_core_init() needs to implemented, of course, but it could hide all
 details about what to do with IRQs and resources and what not. Assuming
 you can properly initialize clocks, it could even handle clocks
 generically for you.

>>> I see what you mean. I'm just a little confused about the term "generic" 
>>> here:
>>> For me something generic has more than one user. Here we seem to speak just
>>> about factoring out some code from the probe function, or?
>>
>> :-) by generic, I mean moving some of that code to
>> drivers/usb/dwc2/core.c so it can be used by both PCI and non-PCI
>> systems.
>>
>>> Your proposal for reducing the probe function would mean to reorder some 
>>> calls
>>> like the ones to dwc2_

Re: usb: gadget: Kernel panic (NULL pointer dereference) when using fsl_udc2_core on i.MX31 PDK

2017-01-23 Thread Felipe Balbi

Hi,

Magnus Lilja  writes:
> Hi
>
> I tried the fsl_udc_core gadget driver on the i.MX31 PDK board and got a 
> kernel panic (NULL pointer dereference) when connecting the USB cable. I 
> had the g_serial module loaded as well.
>
> The NULL pointer panic comes from gadget/udc/core.c 
> usb_gadget_giveback_request() which calls req->complete() and in some 
> cases req->complete is NULL.
>
> Commit 304f7e5e1d08 ("usb: gadget: Refactor request completion") changed 
> fsl_udc2_core.c (and several other files) and in fsl_udc2_core.c a check 
> that req->complete is non-NULL was removed:
>
> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
> @@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
>  ep->stopped = 1;
>
>  spin_unlock(&ep->udc->lock);
> -   /* complete() is from gadget layer,
> -* eg fsg->bulk_in_complete() */
> -   if (req->req.complete)
> -   req->req.complete(&ep->ep, &req->req);
> +
> +   usb_gadget_giveback_request(&ep->ep, &req->req);
>
>  spin_lock(&ep->udc->lock);
>  ep->stopped = stopped;
>
> If I re-introduce the check (either in fsl_udc_core.c or core.c) at 
> least USB gadget operation using g_serial seems to work just fine.
>
> I don't know the logic in detail to understand whether this is a proper 
> fix or if there is some other more problem with the fls_udc_core driver. 
> Does anyone have input in this matter?
>
> I can produce a proper patch that fixes this problem by re-introducing 
> the check (in either fsl_udc_core.c or core.c) if that is a proper 
> solution and I can also assist in testing other fixes to the problem.

->complete() is supposed to be mandatory. Which gadget do you have that
->doesn't set ->complete() to a valid function pointer?

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH] usb: dwc3: ep0: Fix the possible missed request for handling delay STATUS phase

2017-01-23 Thread Felipe Balbi

Hi,

Alan Stern  writes:
> On Mon, 16 Jan 2017, Felipe Balbi wrote:
>
>> > The gadget driver never calls usb_ep_queue in order to receive the next
>> > SETUP packet; the UDC driver takes care of SETUP handling
>> > automatically.
>> 
>> yeah, that's another thing I'd like to change. Currently, we have no
>> means to either try to implement device-initiated LPM without adding a
>> ton of hacks to UDC drivers. If we require upper layers (composite.c,
>> most of the time) to usb_ep_queue() separate requests for all 3 phases
>> of a ctrl transfer, we can actually rely on the fact that a new SETUP
>> phase hasn't been queued yet to trigger U3 entry.
>
> I haven't given any thought to LPM.

okay

> However, requiring gadget drivers to request SETUP packets seems rather
> questionable.  It flies against the USB spec, which requires

right, maybe SETUP is a bit of an overkill. DATA and STATUS, however,
should be doable.

> peripherals to accept SETUP packets at any time -- a device is not
> allowed to NAK or STALL a SETUP packet (see 8.4.6.4 in the USB-2 spec).  
> In fact, the hardware in UDCs probably isn't capable of doing it.
>
> This means that to do what you want, the UDC driver would have to
> accept SETUP packets at any time, and store the most recent packet
> contents.  Then, when the gadget driver submits a request, the UDC
> driver would give it this stored data.  It would also have to detect

that's right, I missed that part.

> and prevent a nasty race where the gadget driver tries to queue a
> request on ep0 that is a response to an old SETUP, one that has already
> been overwritten.  I'm not even sure preventing this race would be
> possible in your scheme.
>
> The advantage to invoking the gadget driver's setup callback directly
> from the UDC driver's interrupt handler is that the gadget driver will
> know immediately when an old SETUP has become stale.  (That's what
> ep0_req_tag is for in f_mass_storage.)  It also provides a concurrency
> guarantee, because the driver does not re-enable UDC SETUP interrupts 
> until the handler is finished.
>
>> Another detail that this helps is that PM (overall) becomes simpler as,
>> most likely, we won't need to mess with transfer cancellation, for
>> example.
>
> System PM on a gadget is always troublesome.  Even if the USB 
> connection is a wakeup source, it may not be possible to guarantee that 
> the gadget can wake up quickly enough to handle an incoming packet.

that's true.

>> > You are suggesting that status stage requests should not be queued 
>> > automatically by UDC drivers but instead queued explicitly by gadget 
>> > drivers.  This would mean changing every UDC driver and every gadget 
>> > driver.
>> 
>> yes, a bit of work but has been done before. One example that comes to
>> mind is when I added ->udc_start() and ->udc_stop(). It's totally
>> doable. We can, for instance, add a temporary
>> "wants_explicit_ctrl_phases"  flag to struct usb_gadget which, if set,
>> will tell composite.c (or whatever) that the UDC wants explicitly queued
>> ctrl phases.
>
> The term used in the USB spec is "stage", not "phase".  "Phase" refers
> to the packets making up a single transaction: token, data, and
> handshake.
>
> Also, data stages are already explicit.  So your temporary flag might 
> better be called "wants_explicit_status_stages".

I stand corrected ;-)

>> Then add support for that to each UDC and set the flag. Once all are
>> converted, add one extra patch to remove the flag and the legacy
>> code. This has, of course, the draw back of increasing complexity until
>> everything is converted over; but if it's all done in a single series, I
>> can't see any problems with that.
>> 
>> > Also, it won't fix the race that Baolin Wang found.  The setup routine
>> 
>> well, it will help... see below.
>> 
>> > is always called in interrupt context, so it can't sleep.  Doing
>> > anything non-trivial will require a separate task, and it's possible
>> > that this task will try to enqueue the data-stage or status-stage
>> > request before the UDC driver is ready to handle it (for example, 
>> > before or shortly after the setup routine returns).
>> >
>> > To work properly, the UDC driver must be able to accept a request for 
>> > ep0 any time after it invokes the setup callback -- either before the 
>> > callback returns or after.
>> 
>> Right, all UDCs are *already* required to support this case anyway
>> because of USB_GADGET_DELAYED_STATUS. There was a bug in DWC3, sure, but
>> it was already required to support this case.
>> 
>> By removing USB_GADGET_DELAYED_STATUS altogether and making phases more
>> explict, we enforce this requirement and it'll be much easier to test
>> for it IMO.
>
> Okay, I can see the point of requiring explicit status requests.  
> Implementing it will be a little tricky, because right now some status 
> requests already are explicit (those for length-0 OUT transfers) while 
> others are implicit.

exactly. And that's source o

Re: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?

2017-01-23 Thread Felipe Balbi

Hi,

Vincent Pelletier  writes:
> On Mon, 16 Jan 2017 14:48:31 +, Vincent Pelletier
>  wrote:
>> 3) I declared 4 endpoints (2 IN, 2 OUT). And I went one level deeper
>>down the rabbit hole: now enumeration fails with this message on
>>host:
>
> Aaand... I dug this part of the hole myself. In the library part of my
> python module, and I was looking at the test program. Shame on me.
>
> Next tests: After a bit of patching f_fs so it accepts creating 30
> endpoints, more endpoints seem to have issues above the 7th (ignoring
> direction).
>
> 1IN bandwidth: 0 B/s (0.20s)
> 1OUTbandwidth: 30230053 B/s (0.21s)
> 2IN bandwidth: 29541962 B/s (0.22s)
> 2OUTbandwidth: 29995671 B/s (0.22s)
> 3IN bandwidth: 28787881 B/s (0.22s)
> 3OUTbandwidth: 30428794 B/s (0.22s)
> 4IN bandwidth: 29768777 B/s (0.22s)
> 4OUTbandwidth: 30377747 B/s (0.22s)
> 5IN bandwidth: 29521290 B/s (0.22s)
> 5OUTbandwidth: 30133012 B/s (0.22s)
> 6IN bandwidth: 29662245 B/s (0.22s)
> 6OUTbandwidth: 30335271 B/s (0.22s)
> 7IN bandwidth: 29467489 B/s (0.22s)
> 7OUTbandwidth: 30387586 B/s (0.22s)
> 8IN bandwidth: 0 B/s (0.20s)
> 8OUTbandwidth: 0 B/s (0.20s)
> 9IN bandwidth: 0 B/s (0.20s)
> 9OUTbandwidth: 30174377 B/s (0.22s)
> 10INbandwidth: 0 B/s (0.20s)
> 10OUT   bandwidth: 30389703 B/s (0.22s)
> 11INbandwidth: 0 B/s (0.20s)
> 11OUT   bandwidth: 29947720 B/s (0.21s)
> 12INbandwidth: 0 B/s (0.20s)
> 12OUT   bandwidth: 29386298 B/s (0.21s)
> 13INbandwidth: 0 B/s (0.20s)
> 13OUT   bandwidth: 30097119 B/s (0.22s)
> 14INbandwidth: 0 B/s (0.20s)
> 14OUT   bandwidth: 30099782 B/s (0.22s)
> 15INbandwidth: 0 B/s (0.20s)
> 15OUT   bandwidth: 31122053 B/s (0.21s)
>
> Would this pattern ring a bell ?

it could be that we're ran out of IN endpoints. There's a maximum number
to how many IN endpoints we can have enabled at one time and, currently,
dwc3 is not enforcing that in any way (I'll get that sorted out for
v4.12, v4.11 is already too late)

> I did the stall test again (on all endpoints and on 8IN + 9IN), and the
> halted endpoints behave as expected.
> I also verified the right data reaches/comes from the right endpoint
> (for those which work), without error.
> I did not check with my protocol analyser yet.
>
> I attach the patch against f_fs.c as an RFC.
>
> Regards,
> -- 
> Vincent Pelletier
> From fc77bb86dfbf088ce4a63510867210c2f2d8b0af Mon Sep 17 00:00:00 2001
> Message-Id: 
> 
> From: Vincent Pelletier 
> Date: Tue, 17 Jan 2017 12:57:39 +
> Subject: usb: gadget: f_fs: Accept up to 30 endpoints.
>
> It is allowed by the USB specification to enabled same-address, opposite-
> direction endpoints simultaneously, which means 30 non-zero endpoints
> are allowed. So double eps_addrmap length to 30.
> The original code only accepted 14 descriptors, because the first
> eps_addrmap entry is unused (it is a placeholder for endpoint zero).
> So increase eps_addrmap length by one to 31.
>
> Signed-off-by: Vincent Pelletier 
> ---
>  drivers/usb/gadget/function/f_fs.c | 2 +-
>  drivers/usb/gadget/function/u_fs.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c 
> b/drivers/usb/gadget/function/f_fs.c
> index 5c91a6f4613b..206184b561b7 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -2101,7 +2101,7 @@ static int __ffs_data_do_entity(enum ffs_entity_type 
> type,
>   case FFS_ENDPOINT:
>   d = (void *)desc;
>   helper->eps_count++;
> - if (helper->eps_count >= 15)
> + if (helper->eps_count >= 31)
>   return -EINVAL;
>   /* Check if descriptors for any speed were already parsed */
>   if (!helper->ffs->eps_count && !helper->ffs->interfaces_count)
> diff --git a/drivers/usb/gadget/function/u_fs.h 
> b/drivers/usb/gadget/function/u_fs.h
> index 60139854e0b1..8eaf473d1ac0 100644
> --- a/drivers/usb/gadget/function/u_fs.h
> +++ b/drivers/usb/gadget/function/u_fs.h
> @@ -247,7 +247,7 @@ struct ffs_data {
>  
>   unsigneduser_flags;
>  
> - u8  eps_addrmap[15];
> + u8  eps_addrmap[31];

please send this as a proper patch using git send-email. Also, please
define a macro for this constant. If we ever need to change it again,
it'll change in one place.

thanks

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH v2] usb: dwc3: handle DWC_USB3_NUM == DWC_USB3_NUM_IN_EPS

2017-01-23 Thread Felipe Balbi

Hi,

Bryan O'Donoghue  writes:
> - DWC_USB3_NUM indicates the number of Device mode single directional
>   endpoints, including OUT and IN endpoint 0.
>
> - DWC_USB3_NUM_IN_EPS indicates the maximum number of Device mode IN
>   endpoints active at any time, including control endpoint 0.
>
> It's possible to configure RTL such that DWC_USB3_NUM_EPS is equal to
> DWC_USB3_NUM_IN_EPS.
>
> dwc3-core calculates the number of OUT endpoints as DWC_USB3_NUM minus
> DWC_USB3_NUM_IN_EPS. If RTL has been configured with DWC_USB3_NUM_IN_EPS
> equal to DWC_USB3_NUM then dwc3-core will calculate the number of OUT
> endpoints as zero.
>
> For example a from dwc3_core_num_eps() shows:
> [1.565000]  /usb0@f01d: found 8 IN and 0 OUT endpoints
>
> This patch works around this case by detecting when DWC_USB3_NUM_EPS is
> equal to DWC3_USB3_NUM_IN_EPS and over-rides the calculation of the number
> of OUT and IN endpoints to make dwc->num_in_eps equal to half
> DWC_USB3_NUM_EPS.
>
> If DWC_USB3_NUM_EPS is equal to DWC3_USB3_NUM_IN_EPS and the endpoint count
> is an odd number then dwc->num_out_eps will be assigned the extra endpoint.

sorry, now that I spent some more time with this. Isn't something like
below solving all problems?

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 369bab16a824..68c9c84b7216 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -397,8 +397,7 @@ static void dwc3_core_num_eps(struct dwc3 *dwc)
 {
struct dwc3_hwparams*parms = &dwc->hwparams;
 
-   dwc->num_in_eps = DWC3_NUM_IN_EPS(parms);
-   dwc->num_out_eps = DWC3_NUM_EPS(parms) - dwc->num_in_eps;
+   dwc->num_eps = DWC3_NUM_EPS(parms);
 }
 
 static void dwc3_cache_hwparams(struct dwc3 *dwc)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 0a664d8eba3f..8c187df0aa42 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2001,23 +2002,7 @@ static int dwc3_gadget_init_hw_endpoints(struct dwc3 
*dwc,
 
 static int dwc3_gadget_init_endpoints(struct dwc3 *dwc)
 {
-   int ret;
-
-   INIT_LIST_HEAD(&dwc->gadget.ep_list);
-
-   ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_out_eps, 0);
-   if (ret < 0) {
-   dev_err(dwc->dev, "failed to initialize OUT endpoints\n");
-   return ret;
-   }
-
-   ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_in_eps, 1);
-   if (ret < 0) {
-   dev_err(dwc->dev, "failed to initialize IN endpoints\n");
-   return ret;
-   }
-
-   return 0;
+   return dwc3_gadget_init_hw_endpoints(dwc, dwc->num_eps);
 }
 
 static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)

(clearly this won't compile... It's just to illustrate)

The HW actually already tells us total number of endpoints and according
to John, they can all behave in either direction. Can you test it out
and finish it up as a proper patch?

I'll make sure to fix up the "maximum number of IN endpoints enabled at
one time" for v4.12.

thanks

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH 36/37] xhci: simplify how we store TDs in urb private data

2017-01-23 Thread Mathias Nyman

On 23.01.2017 13:57, David Laight wrote:

From: Mathias Nyman

Sent: 20 January 2017 14:47



Instead of storing a zero length array of td pointers, and then
allocate memory both for the td pointer array and the td's, just
use a zero length array of actual td's in urb private data.


This reminds me of an old patch that got reverted because things
broke because the lifetimes/accesses of the data wasn't the obvious one.



Can you recall more details about that case? I'd hate to revert this
later.

I looked at the history how we ended up with this type of allocation for
urb priv. I couldn't find any reverts or reasons why it's the way it is.
  


That might have been a different structure.

FWIW it is a shame that things like USB3 ethernet can't transmit and
receive without all these intermediate and dynamically allocated
structures.


Well, there's one less allocation after this patch :)

-Mathias
--
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 02/37] xhci: rename EP_HALT_PENDING to EP_STOP_CMD_PENDING

2017-01-23 Thread Mathias Nyman
We don't want to confuse halted and stalled endpoint states with
a flag indicating we are waiting for a stop endpoint command to
finish or timeout

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 6 +++---
 drivers/usb/host/xhci.c  | 6 +++---
 drivers/usb/host/xhci.h  | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 46df89e..213cb02 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -410,7 +410,7 @@ void xhci_ring_ep_doorbell(struct xhci_hcd *xhci,
 * pointer command pending because the device can choose to start any
 * stream once the endpoint is on the HW schedule.
 */
-   if ((ep_state & EP_HALT_PENDING) || (ep_state & SET_DEQ_PENDING) ||
+   if ((ep_state & EP_STOP_CMD_PENDING) || (ep_state & SET_DEQ_PENDING) ||
(ep_state & EP_HALTED))
return;
writel(DB_VALUE(ep_index, stream_id), db_addr);
@@ -626,7 +626,7 @@ static void td_to_noop(struct xhci_hcd *xhci, struct 
xhci_ring *ep_ring,
 static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd *xhci,
struct xhci_virt_ep *ep)
 {
-   ep->ep_state &= ~EP_HALT_PENDING;
+   ep->ep_state &= ~EP_STOP_CMD_PENDING;
/* Can't del_timer_sync in interrupt, so we attempt to cancel.  If the
 * timer is running on another CPU, we don't decrement stop_cmds_pending
 * (since we didn't successfully stop the watchdog timer).
@@ -914,7 +914,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
 
ep->stop_cmds_pending--;
 
-   if (ep->stop_cmds_pending || !(ep->ep_state & EP_HALT_PENDING)) {
+   if (ep->stop_cmds_pending || !(ep->ep_state & EP_STOP_CMD_PENDING)) {
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
"Stop EP timer ran, but no command pending, "
"exiting.");
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 9a0ec11..fcb3fa4 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1563,13 +1563,13 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb 
*urb, int status)
/* Queue a stop endpoint command, but only if this is
 * the first cancellation to be handled.
 */
-   if (!(ep->ep_state & EP_HALT_PENDING)) {
+   if (!(ep->ep_state & EP_STOP_CMD_PENDING)) {
command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
if (!command) {
ret = -ENOMEM;
goto done;
}
-   ep->ep_state |= EP_HALT_PENDING;
+   ep->ep_state |= EP_STOP_CMD_PENDING;
ep->stop_cmds_pending++;
ep->stop_cmd_timer.expires = jiffies +
XHCI_STOP_EP_CMD_TIMEOUT * HZ;
@@ -3610,7 +3610,7 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device 
*udev)
 
/* Stop any wayward timer functions (which may grab the lock) */
for (i = 0; i < 31; ++i) {
-   virt_dev->eps[i].ep_state &= ~EP_HALT_PENDING;
+   virt_dev->eps[i].ep_state &= ~EP_STOP_CMD_PENDING;
del_timer_sync(&virt_dev->eps[i].stop_cmd_timer);
}
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2d7b637..198f403 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -912,7 +912,7 @@ struct xhci_virt_ep {
unsigned intep_state;
 #define SET_DEQ_PENDING(1 << 0)
 #define EP_HALTED  (1 << 1)/* For stall handling */
-#define EP_HALT_PENDING(1 << 2)/* For URB cancellation 
*/
+#define EP_STOP_CMD_PENDING(1 << 2)/* For URB cancellation */
 /* Transitioning the endpoint to using streams, don't enqueue URBs */
 #define EP_GETTING_STREAMS (1 << 3)
 #define EP_HAS_STREAMS (1 << 4)
-- 
1.9.1

--
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 05/37] usb: xhci: remove unnecessary second abort try

2017-01-23 Thread Mathias Nyman
From: Lu Baolu 

The second try was a workaround for (what we thought was) command
ring failing to stop in the first place. But this turns out to be
due to the race that we have fixed(see "xhci: Fix race related to
abort operation"). With that fix, it is time to remove the second
try.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 18 +-
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 8ccd24e..bcc0894 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -362,19 +362,11 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, 
unsigned long flags)
ret = xhci_handshake(&xhci->op_regs->cmd_ring,
CMD_RING_RUNNING, 0, 5 * 1000 * 1000);
if (ret < 0) {
-   /* we are about to kill xhci, give it one more chance */
-   xhci_write_64(xhci, temp_64 | CMD_RING_ABORT,
- &xhci->op_regs->cmd_ring);
-   udelay(1000);
-   ret = xhci_handshake(&xhci->op_regs->cmd_ring,
-CMD_RING_RUNNING, 0, 3 * 1000 * 1000);
-   if (ret < 0) {
-   xhci_err(xhci, "Stopped the command ring failed, "
-"maybe the host is dead\n");
-   xhci->xhc_state |= XHCI_STATE_DYING;
-   xhci_halt(xhci);
-   return -ESHUTDOWN;
-   }
+   xhci_err(xhci,
+"Stop command ring failed, maybe the host is dead\n");
+   xhci->xhc_state |= XHCI_STATE_DYING;
+   xhci_halt(xhci);
+   return -ESHUTDOWN;
}
/*
 * Writing the CMD_RING_ABORT bit should cause a cmd completion event,
-- 
1.9.1

--
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 01/37] xhci: simplify if statement to make it more readable

2017-01-23 Thread Mathias Nyman
No functional change, De Morgan !(A && B) = (!A || !B)

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index e32029a..46df89e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -913,7 +913,8 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
spin_lock_irqsave(&xhci->lock, flags);
 
ep->stop_cmds_pending--;
-   if (!(ep->stop_cmds_pending == 0 && (ep->ep_state & EP_HALT_PENDING))) {
+
+   if (ep->stop_cmds_pending || !(ep->ep_state & EP_HALT_PENDING)) {
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
"Stop EP timer ran, but no command pending, "
"exiting.");
-- 
1.9.1

--
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 00/37] xhci features for usb-next 4.11

2017-01-23 Thread Mathias Nyman
Hi Greg

version 2.
A lot of xhci cleanups, refactoring and changes for 4.11
Including major tracing rework by Felipe

Changes since v1:

 Removed extra empty lines in patch 25/37
 Added "based on" entry to patch 25/37 and 26/37 commit messages
 fixed typo in patch 32/37 commit message

-Mathias


Alexander Stein (1):
  xhci: Put warning message on a single line

Baolin Wang (1):
  usb: host: xhci: Remove unused 'addr_64' variable in xhci_hcd
structure

Felipe Balbi (20):
  usb: xhci: add quirk flag for broken PED bits
  usb: host: xhci-plat: enable BROKEN_PED quirk if platform requested
  usb: host: xhci: change pre-increments to post-increments
  usb: host: xhci: print HCIVERSION on debug
  usb: host: xhci: rename completion codes to match spec
  usb: host: xhci: simplify irq handler return
  usb: host: xhci: remove unneded semicolon
  usb: host: xhci: use slightly better list helpers
  usb: host: xhci: reorder variable definitions
  usb: host: xhci: introduce xhci_td_cleanup()
  usb: host: xhci: remove bogus __releases()/__acquires() annotation
  usb: host: xhci: check for a valid ring when unmapping bounce buffer
  usb: host: xhci: unconditionally call xhci_unmap_td_bounce_buffer()
  usb: host: xhci: convert to list_for_each_entry_safe()
  usb: host: xhci: combine event TRB completion debugging messages
  usb: host: xhci: make a generic TRB tracer
  usb: host: xhci: add urb_enqueue/dequeue/giveback tracers
  usb: host: xhci: convert several if() to a single switch statement
  usb: host: xhci: remove newline from tracer
  usb: host: xhci: add xhci_virt_device tracer

Lu Baolu (5):
  usb: xhci: remove unnecessary second abort try
  usb: xhci: remove unnecessary assignment
  usb: xhci: avoid unnecessary calculation
  usb: xhci: use list_is_singular for cmd_list
  usb: xhci: remove unnecessary return in xhci_pci_setup()

Mathias Nyman (10):
  xhci: simplify if statement to make it more readable
  xhci: rename EP_HALT_PENDING to EP_STOP_CMD_PENDING
  xhci: detect stop endpoint race using pending timer instead of
counter.
  xhci: remove unnecessary check for pending timer
  xhci: Introduce helper to turn one TRB into a no-op
  xhci: use the trb_to_noop() helper for command trbs
  xhci: rename size variable to num_tds
  xhci: Rename variables related to transfer descritpors
  xhci: simplify how we store TDs in urb private data
  xhci: refactor xhci_urb_enqueue

 Documentation/devicetree/bindings/usb/usb-xhci.txt |   1 +
 drivers/usb/host/xhci-dbg.c|  22 +-
 drivers/usb/host/xhci-ext-caps.h   |   2 +-
 drivers/usb/host/xhci-hub.c|  14 +-
 drivers/usb/host/xhci-mem.c|  30 +-
 drivers/usb/host/xhci-pci.c|   6 +-
 drivers/usb/host/xhci-plat.c   |   3 +
 drivers/usb/host/xhci-ring.c   | 463 +-
 drivers/usb/host/xhci-trace.h  | 184 ++-
 drivers/usb/host/xhci.c| 212 -
 drivers/usb/host/xhci.h| 528 ++---
 11 files changed, 982 insertions(+), 483 deletions(-)

-- 
1.9.1

--
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 04/37] xhci: remove unnecessary check for pending timer

2017-01-23 Thread Mathias Nyman
Checking if the command timeout timer is pending when queueing the
first command to the command ring is not really useful, remove it.

Suggested-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 2ce132b..8ccd24e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3815,8 +3815,7 @@ static int queue_command(struct xhci_hcd *xhci, struct 
xhci_command *cmd,
list_add_tail(&cmd->cmd_list, &xhci->cmd_list);
 
/* if there are no other commands queued we start the timeout timer */
-   if (xhci->cmd_list.next == &cmd->cmd_list &&
-   !delayed_work_pending(&xhci->cmd_timer)) {
+   if (xhci->cmd_list.next == &cmd->cmd_list) {
xhci->current_cmd = cmd;
xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT);
}
-- 
1.9.1

--
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 03/37] xhci: detect stop endpoint race using pending timer instead of counter.

2017-01-23 Thread Mathias Nyman
A counter was used to find out if the stop endpoint completion raced with
the stop endpoint timeout timer. This was needed in case the stop ep
completion failed to delete the timer as it was running on anoter cpu.

The EP_STOP_CMD_PENDING flag was not enough as a new stop endpoint command
may be queued between the command completion and timeout function, which
would set the flag back.

Instead of the separate counter that was used we can detect the race by
checking both the STOP_EP_PENDING flag and timer_pending in the timeout
function.

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 27 +++
 drivers/usb/host/xhci.c  |  1 -
 drivers/usb/host/xhci.h  |  1 -
 3 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 213cb02..2ce132b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -627,12 +627,8 @@ static void xhci_stop_watchdog_timer_in_irq(struct 
xhci_hcd *xhci,
struct xhci_virt_ep *ep)
 {
ep->ep_state &= ~EP_STOP_CMD_PENDING;
-   /* Can't del_timer_sync in interrupt, so we attempt to cancel.  If the
-* timer is running on another CPU, we don't decrement stop_cmds_pending
-* (since we didn't successfully stop the watchdog timer).
-*/
-   if (del_timer(&ep->stop_cmd_timer))
-   ep->stop_cmds_pending--;
+   /* Can't del_timer_sync in interrupt */
+   del_timer(&ep->stop_cmd_timer);
 }
 
 /*
@@ -895,10 +891,8 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci,
  * simple flag to say whether there is a pending stop endpoint command for a
  * particular endpoint.
  *
- * Instead we use a combination of that flag and a counter for the number of
- * pending stop endpoint commands.  If the timer is the tail end of the last
- * stop endpoint command, and the endpoint's command is still pending, we 
assume
- * the host is dying.
+ * Instead we use a combination of that flag and checking if a new timer is
+ * pending.
  */
 void xhci_stop_endpoint_command_watchdog(unsigned long arg)
 {
@@ -912,13 +906,11 @@ void xhci_stop_endpoint_command_watchdog(unsigned long 
arg)
 
spin_lock_irqsave(&xhci->lock, flags);
 
-   ep->stop_cmds_pending--;
-
-   if (ep->stop_cmds_pending || !(ep->ep_state & EP_STOP_CMD_PENDING)) {
-   xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
-   "Stop EP timer ran, but no command pending, "
-   "exiting.");
+   /* bail out if cmd completed but raced with stop ep watchdog timer.*/
+   if (!(ep->ep_state & EP_STOP_CMD_PENDING) ||
+   timer_pending(&ep->stop_cmd_timer)) {
spin_unlock_irqrestore(&xhci->lock, flags);
+   xhci_dbg(xhci, "Stop EP timer raced with cmd completion, exit");
return;
}
 
@@ -927,7 +919,10 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
/* Oops, HC is dead or dying or at least not responding to the stop
 * endpoint command.
 */
+
xhci->xhc_state |= XHCI_STATE_DYING;
+   ep->ep_state &= ~EP_STOP_CMD_PENDING;
+
/* Disable interrupts from the host controller and start halting it */
xhci_quiesce(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index fcb3fa4..fb7a6dc 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1570,7 +1570,6 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb 
*urb, int status)
goto done;
}
ep->ep_state |= EP_STOP_CMD_PENDING;
-   ep->stop_cmds_pending++;
ep->stop_cmd_timer.expires = jiffies +
XHCI_STOP_EP_CMD_TIMEOUT * HZ;
add_timer(&ep->stop_cmd_timer);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 198f403..cdf8c03 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -924,7 +924,6 @@ struct xhci_virt_ep {
unsigned intstopped_stream;
/* Watchdog timer for stop endpoint command to cancel URBs */
struct timer_list   stop_cmd_timer;
-   int stop_cmds_pending;
struct xhci_hcd *xhci;
/* Dequeue pointer and dequeue segment for a submitted Set TR Dequeue
 * command.  We'll need to update the ring's dequeue segment and dequeue
-- 
1.9.1

--
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 07/37] xhci: Put warning message on a single line

2017-01-23 Thread Mathias Nyman
From: Alexander Stein 

This allows someone to grep for the complete warning message as in;
xhci-hcd xhci-hcd.0.auto: USB core suspending device not in U0/U1/U2.

Signed-off-by: Alexander Stein 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0ef1690..2d154e2 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -990,8 +990,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 
wValue,
temp = readl(port_array[wIndex]);
if ((temp & PORT_PE) == 0 || (temp & PORT_RESET)
|| (temp & PORT_PLS_MASK) >= XDEV_U3) {
-   xhci_warn(xhci, "USB core suspending device "
- "not in U0/U1/U2.\n");
+   xhci_warn(xhci, "USB core suspending device not 
in U0/U1/U2.\n");
goto error;
}
 
-- 
1.9.1

--
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 11/37] usb: xhci: avoid unnecessary calculation

2017-01-23 Thread Mathias Nyman
From: Lu Baolu 

No need to calculate remainder and length_field, if there is
no data phase of a control transfer.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 61b5fea..2374a13 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3237,7 +3237,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t 
mem_flags,
struct usb_ctrlrequest *setup;
struct xhci_generic_trb *start_trb;
int start_cycle;
-   u32 field, length_field, remainder;
+   u32 field;
struct urb_priv *urb_priv;
struct xhci_td *td;
 
@@ -3310,16 +3310,16 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t 
mem_flags,
else
field = TRB_TYPE(TRB_DATA);
 
-   remainder = xhci_td_remainder(xhci, 0,
-  urb->transfer_buffer_length,
-  urb->transfer_buffer_length,
-  urb, 1);
-
-   length_field = TRB_LEN(urb->transfer_buffer_length) |
-   TRB_TD_SIZE(remainder) |
-   TRB_INTR_TARGET(0);
-
if (urb->transfer_buffer_length > 0) {
+   u32 length_field, remainder;
+
+   remainder = xhci_td_remainder(xhci, 0,
+   urb->transfer_buffer_length,
+   urb->transfer_buffer_length,
+   urb, 1);
+   length_field = TRB_LEN(urb->transfer_buffer_length) |
+   TRB_TD_SIZE(remainder) |
+   TRB_INTR_TARGET(0);
if (setup->bRequestType & USB_DIR_IN)
field |= TRB_DIR_IN;
queue_trb(xhci, ep_ring, true,
-- 
1.9.1

--
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 12/37] usb: xhci: use list_is_singular for cmd_list

2017-01-23 Thread Mathias Nyman
From: Lu Baolu 

Use list_is_singular() to check if cmd_list has only one entry.

[use list_empty() in queue command instead -Mathias]
Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 2374a13..b11f479 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1425,7 +1425,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
}
 
/* restart timer if this wasn't the last command */
-   if (cmd->cmd_list.next != &xhci->cmd_list) {
+   if (!list_is_singular(&xhci->cmd_list)) {
xhci->current_cmd = list_entry(cmd->cmd_list.next,
   struct xhci_command, cmd_list);
xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT);
@@ -3802,14 +3802,15 @@ static int queue_command(struct xhci_hcd *xhci, struct 
xhci_command *cmd,
}
 
cmd->command_trb = xhci->cmd_ring->enqueue;
-   list_add_tail(&cmd->cmd_list, &xhci->cmd_list);
 
/* if there are no other commands queued we start the timeout timer */
-   if (xhci->cmd_list.next == &cmd->cmd_list) {
+   if (list_empty(&xhci->cmd_list)) {
xhci->current_cmd = cmd;
xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT);
}
 
+   list_add_tail(&cmd->cmd_list, &xhci->cmd_list);
+
queue_trb(xhci, xhci->cmd_ring, false, field1, field2, field3,
field4 | xhci->cmd_ring->cycle_state);
return 0;
-- 
1.9.1

--
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 06/37] usb: host: xhci: Remove unused 'addr_64' variable in xhci_hcd structure

2017-01-23 Thread Mathias Nyman
From: Baolin Wang 

Since the 'addr_64' variable as legacy is unused now, then remove it from
xhci_hcd structure.

Signed-off-by: Baolin Wang 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index cdf8c03..5bf9df2 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1548,7 +1548,6 @@ struct xhci_hcd {
u8  max_ports;
u8  isoc_threshold;
int event_ring_max;
-   int addr_64;
/* 4KB min, 128MB max */
int page_size;
/* Valid values are 12 to 20, inclusive */
-- 
1.9.1

--
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 08/37] usb: xhci: add quirk flag for broken PED bits

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

Some devices from Texas Instruments [1] suffer from
a silicon bug where Port Enabled/Disabled bit
should not be used to silence an erroneous device.

The bug is so that if port is disabled with PED
bit, an IRQ for device removal (or attachment)
will never fire.

Just for the sake of completeness, the actual
problem lies with SNPS USB IP and this affects
all known versions up to 3.00a. A separate
patch will be added to dwc3 to enabled this
quirk flag if version is <= 3.00a.

[1] - AM572x Silicon Errata http://www.ti.com/lit/er/sprz429j/sprz429j.pdf
Section i896— USB xHCI Port Disable Feature Does Not Work

Signed-off-by: Felipe Balbi 
Signed-off-by: Roger Quadros 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c | 6 ++
 drivers/usb/host/xhci.h | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 2d154e2..8b906c3 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -458,6 +458,12 @@ static void xhci_disable_port(struct usb_hcd *hcd, struct 
xhci_hcd *xhci,
return;
}
 
+   if (xhci->quirks & XHCI_BROKEN_PORT_PED) {
+   xhci_dbg(xhci,
+"Broken Port Enabled/Disabled, ignoring port disable 
request.\n");
+   return;
+   }
+
/* Write 1 to disable the port */
writel(port_status | PORT_PE, addr);
port_status = readl(addr);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 5bf9df2..b8474a2 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1648,6 +1648,9 @@ struct xhci_hcd {
 #define XHCI_SSIC_PORT_UNUSED  (1 << 22)
 #define XHCI_NO_64BIT_SUPPORT  (1 << 23)
 #define XHCI_MISSING_CAS   (1 << 24)
+/* For controller with a broken Port Disable implementation */
+#define XHCI_BROKEN_PORT_PED   (1 << 25)
+
unsigned intnum_active_eps;
unsigned intlimit_active_eps;
/* There are two roothubs to keep track of bus suspend info for */
-- 
1.9.1

--
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 15/37] usb: host: xhci: print HCIVERSION on debug

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

When calling xhci_dbg_regs() we actually _do_ want to know XHCI's
version. This might help figure out why certain problems only happen
in some cases.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-dbg.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index a3b67f3..363d125 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -37,10 +37,8 @@ void xhci_dbg_regs(struct xhci_hcd *xhci)
&xhci->cap_regs->hc_capbase, temp);
xhci_dbg(xhci, "//   CAPLENGTH: 0x%x\n",
(unsigned int) HC_LENGTH(temp));
-#if 0
xhci_dbg(xhci, "//   HCIVERSION: 0x%x\n",
(unsigned int) HC_VERSION(temp));
-#endif
 
xhci_dbg(xhci, "// xHCI operational registers at %p:\n", xhci->op_regs);
 
-- 
1.9.1

--
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 13/37] usb: xhci: remove unnecessary return in xhci_pci_setup()

2017-01-23 Thread Mathias Nyman
From: Lu Baolu 

Remove the unnecessary return line in xhci_pci_setup().

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-pci.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 954abfd..fc99f51 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -242,11 +242,7 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
xhci_dbg(xhci, "Got SBRN %u\n", (unsigned int) xhci->sbrn);
 
/* Find any debug ports */
-   retval = xhci_pci_reinit(xhci, pdev);
-   if (!retval)
-   return retval;
-
-   return retval;
+   return xhci_pci_reinit(xhci, pdev);
 }
 
 /*
-- 
1.9.1

--
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 16/37] usb: host: xhci: rename completion codes to match spec

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

Cleanup only. This patch is a mechaninal rename to make sure our macros
for TRB completion codes match what the specification uses to refer to
such errors. The idea behind this is that it makes it far easier to grep
the specification and match it with implementation.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c  |   3 +-
 drivers/usb/host/xhci-ring.c | 124 +--
 drivers/usb/host/xhci.c  |  48 -
 drivers/usb/host/xhci.h  | 106 +---
 4 files changed, 124 insertions(+), 157 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 8b906c3..50d086b 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -418,7 +418,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
/* Wait for last stop endpoint command to finish */
wait_for_completion(cmd->completion);
 
-   if (cmd->status == COMP_CMD_ABORT || cmd->status == COMP_CMD_STOP) {
+   if (cmd->status == COMP_COMMAND_ABORTED ||
+   cmd->status == COMP_STOPPED) {
xhci_warn(xhci, "Timeout while waiting for stop endpoint 
command\n");
ret = -ETIME;
}
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b11f479..f71f4dd 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -304,10 +304,10 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd 
*xhci,
/* Turn all aborted commands in list to no-ops, then restart */
list_for_each_entry(i_cmd, &xhci->cmd_list, cmd_list) {
 
-   if (i_cmd->status != COMP_CMD_ABORT)
+   if (i_cmd->status != COMP_COMMAND_ABORTED)
continue;
 
-   i_cmd->status = COMP_CMD_STOP;
+   i_cmd->status = COMP_STOPPED;
 
xhci_dbg(xhci, "Turn aborted command %p to no-op\n",
 i_cmd->command_trb);
@@ -1038,10 +1038,10 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd 
*xhci, int slot_id,
unsigned int slot_state;
 
switch (cmd_comp_code) {
-   case COMP_TRB_ERR:
+   case COMP_TRB_ERROR:
xhci_warn(xhci, "WARN Set TR Deq Ptr cmd invalid 
because of stream ID configuration\n");
break;
-   case COMP_CTX_STATE:
+   case COMP_CONTEXT_STATE_ERROR:
xhci_warn(xhci, "WARN Set TR Deq Ptr cmd failed due to 
incorrect slot or ep state.\n");
ep_state = GET_EP_CTX_STATE(ep_ctx);
slot_state = le32_to_cpu(slot_ctx->dev_state);
@@ -1050,7 +1050,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd 
*xhci, int slot_id,
"Slot state = %u, EP state = %u",
slot_state, ep_state);
break;
-   case COMP_EBADSLT:
+   case COMP_SLOT_NOT_ENABLED_ERROR:
xhci_warn(xhci, "WARN Set TR Deq Ptr cmd failed because 
slot %u was not enabled.\n",
slot_id);
break;
@@ -1247,7 +1247,7 @@ void xhci_cleanup_command_queue(struct xhci_hcd *xhci)
 {
struct xhci_command *cur_cmd, *tmp_cmd;
list_for_each_entry_safe(cur_cmd, tmp_cmd, &xhci->cmd_list, cmd_list)
-   xhci_complete_del_and_free_cmd(cur_cmd, COMP_CMD_ABORT);
+   xhci_complete_del_and_free_cmd(cur_cmd, COMP_COMMAND_ABORTED);
 }
 
 void xhci_handle_command_timeout(struct work_struct *work)
@@ -1270,7 +1270,7 @@ void xhci_handle_command_timeout(struct work_struct *work)
return;
}
/* mark this command to be cancelled */
-   xhci->current_cmd->status = COMP_CMD_ABORT;
+   xhci->current_cmd->status = COMP_COMMAND_ABORTED;
 
/* Make sure command ring is running before aborting it */
hw_ring_state = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
@@ -1344,7 +1344,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status));
 
/* If CMD ring stopped we own the trbs between enqueue and dequeue */
-   if (cmd_comp_code == COMP_CMD_STOP) {
+   if (cmd_comp_code == COMP_STOPPED) {
complete_all(&xhci->cmd_ring_stop_completion);
return;
}
@@ -1361,9 +1361,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
 * The command ring is stopped now, but the xHC will issue a Command
 * Ring Stopped event which will cause us to restart it.
 */
-   if (cmd_comp_code == COMP_CMD_ABORT) {
+   if (cmd_comp_code == COMP_COMMAND_ABORTED) {
xhci->cmd_ring_state = CMD_RING_STATE_ST

[PATCH v2 25/37] xhci: Introduce helper to turn one TRB into a no-op

2017-01-23 Thread Mathias Nyman
Useful for turning both transfer and command trbs
into no-ops.

Based on earlier code by Felipe Balbi 

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 29 +
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 8b78eee..bd00ada 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -129,6 +129,21 @@ static void inc_td_cnt(struct urb *urb)
urb_priv->td_cnt++;
 }
 
+static void trb_to_noop(union xhci_trb *trb, u32 noop_type)
+{
+   if (trb_is_link(trb)) {
+   /* unchain chained link TRBs */
+   trb->link.control &= cpu_to_le32(~TRB_CHAIN);
+   } else {
+   trb->generic.field[0] = 0;
+   trb->generic.field[1] = 0;
+   trb->generic.field[2] = 0;
+   /* Preserve only the cycle bit of this TRB */
+   trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE);
+   trb->generic.field[3] |= cpu_to_le32(TRB_TYPE(noop_type));
+   }
+}
+
 /* Updates trb to point to the next TRB in the ring, and updates seg if the 
next
  * TRB is in a new segment.  This does not skip over link TRBs, and it does not
  * effect the ring dequeue or enqueue pointers.
@@ -592,18 +607,8 @@ static void td_to_noop(struct xhci_hcd *xhci, struct 
xhci_ring *ep_ring,
union xhci_trb *trb = td->first_trb;
 
while (1) {
-   if (trb_is_link(trb)) {
-   /* unchain chained link TRBs */
-   trb->link.control &= cpu_to_le32(~TRB_CHAIN);
-   } else {
-   trb->generic.field[0] = 0;
-   trb->generic.field[1] = 0;
-   trb->generic.field[2] = 0;
-   /* Preserve only the cycle bit of this TRB */
-   trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE);
-   trb->generic.field[3] |= cpu_to_le32(
-   TRB_TYPE(TRB_TR_NOOP));
-   }
+   trb_to_noop(trb, TRB_TR_NOOP);
+
/* flip cycle if asked to */
if (flip_cycle && trb != td->first_trb && trb != td->last_trb)
trb->generic.field[3] ^= cpu_to_le32(TRB_CYCLE);
-- 
1.9.1

--
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 18/37] usb: host: xhci: remove unneded semicolon

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

it does no good, let's remove it.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ext-caps.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
index e0244fb..28deea5 100644
--- a/drivers/usb/host/xhci-ext-caps.h
+++ b/drivers/usb/host/xhci-ext-caps.h
@@ -117,7 +117,7 @@ static inline int xhci_find_next_ext_cap(void __iomem 
*base, u32 start, int id)
offset = XHCI_HCC_EXT_CAPS(val) << 2;
if (!offset)
return 0;
-   };
+   }
do {
val = readl(base + offset);
if (val == ~0)
-- 
1.9.1

--
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 37/37] xhci: refactor xhci_urb_enqueue

2017-01-23 Thread Mathias Nyman
Use switch instead of several if statements

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 93 -
 1 file changed, 37 insertions(+), 56 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index dde5c2d..6d6c460 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1334,7 +1334,7 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb 
*urb, gfp_t mem_flags)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
unsigned long flags;
int ret = 0;
-   unsigned int slot_id, ep_index;
+   unsigned int slot_id, ep_index, ep_state;
struct urb_priv *urb_priv;
int num_tds;
 
@@ -1348,8 +1348,7 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb 
*urb, gfp_t mem_flags)
if (!HCD_HW_ACCESSIBLE(hcd)) {
if (!in_interrupt())
xhci_dbg(xhci, "urb submitted during PCI suspend\n");
-   ret = -ESHUTDOWN;
-   goto exit;
+   return -ESHUTDOWN;
}
 
if (usb_endpoint_xfer_isoc(&urb->ep->desc))
@@ -1386,69 +1385,51 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb 
*urb, gfp_t mem_flags)
return ret;
}
}
+   }
 
-   /* We have a spinlock and interrupts disabled, so we must pass
-* atomic context to this function, which may allocate memory.
-*/
-   spin_lock_irqsave(&xhci->lock, flags);
-   if (xhci->xhc_state & XHCI_STATE_DYING)
-   goto dying;
+   spin_lock_irqsave(&xhci->lock, flags);
+
+   if (xhci->xhc_state & XHCI_STATE_DYING) {
+   xhci_dbg(xhci, "Ep 0x%x: URB %p submitted for non-responsive 
xHCI host.\n",
+urb->ep->desc.bEndpointAddress, urb);
+   ret = -ESHUTDOWN;
+   goto free_priv;
+   }
+
+   switch (usb_endpoint_type(&urb->ep->desc)) {
+
+   case USB_ENDPOINT_XFER_CONTROL:
ret = xhci_queue_ctrl_tx(xhci, GFP_ATOMIC, urb,
-   slot_id, ep_index);
-   if (ret)
-   goto free_priv;
-   spin_unlock_irqrestore(&xhci->lock, flags);
-   } else if (usb_endpoint_xfer_bulk(&urb->ep->desc)) {
-   spin_lock_irqsave(&xhci->lock, flags);
-   if (xhci->xhc_state & XHCI_STATE_DYING)
-   goto dying;
-   if (xhci->devs[slot_id]->eps[ep_index].ep_state &
-   EP_GETTING_STREAMS) {
-   xhci_warn(xhci, "WARN: Can't enqueue URB while bulk ep "
-   "is transitioning to using streams.\n");
-   ret = -EINVAL;
-   } else if (xhci->devs[slot_id]->eps[ep_index].ep_state &
-   EP_GETTING_NO_STREAMS) {
-   xhci_warn(xhci, "WARN: Can't enqueue URB while bulk ep "
-   "is transitioning to "
-   "not having streams.\n");
+slot_id, ep_index);
+   break;
+   case USB_ENDPOINT_XFER_BULK:
+   ep_state = xhci->devs[slot_id]->eps[ep_index].ep_state;
+   if (ep_state & (EP_GETTING_STREAMS | EP_GETTING_NO_STREAMS)) {
+   xhci_warn(xhci, "WARN: Can't enqueue URB, ep in streams 
transition state %x\n",
+ ep_state);
ret = -EINVAL;
-   } else {
-   ret = xhci_queue_bulk_tx(xhci, GFP_ATOMIC, urb,
-   slot_id, ep_index);
+   break;
}
-   if (ret)
-   goto free_priv;
-   spin_unlock_irqrestore(&xhci->lock, flags);
-   } else if (usb_endpoint_xfer_int(&urb->ep->desc)) {
-   spin_lock_irqsave(&xhci->lock, flags);
-   if (xhci->xhc_state & XHCI_STATE_DYING)
-   goto dying;
+   ret = xhci_queue_bulk_tx(xhci, GFP_ATOMIC, urb,
+slot_id, ep_index);
+   break;
+
+
+   case USB_ENDPOINT_XFER_INT:
ret = xhci_queue_intr_tx(xhci, GFP_ATOMIC, urb,
slot_id, ep_index);
-   if (ret)
-   goto free_priv;
-   spin_unlock_irqrestore(&xhci->lock, flags);
-   } else {
-   spin_lock_irqsave(&xhci->lock, flags);
-   if (xhci->xhc_state & XHCI_STATE_DYING)
-   goto dying;
+   break;
+
+   case USB_ENDPOINT_XFER_ISOC:
ret = xhci_queue_isoc_tx_prepare(xhci, GFP_ATOMIC, urb,
slot_id, ep_index);
-   if (ret)
-   

[PATCH v2 30/37] usb: host: xhci: add urb_enqueue/dequeue/giveback tracers

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

These three new tracers will help us tie TRBs into URBs by *also*
looking into URB lifetime.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c  |  1 +
 drivers/usb/host/xhci-trace.h | 70 +++
 drivers/usb/host/xhci.c   |  5 
 3 files changed, 76 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 4316273..e0a49cb 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -642,6 +642,7 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
usb_hcd_unlink_urb_from_ep(hcd, urb);
spin_unlock(&xhci->lock);
usb_hcd_giveback_urb(hcd, urb, status);
+   trace_xhci_urb_giveback(urb);
spin_lock(&xhci->lock);
 }
 
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index d01524b..4bad0d6 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -158,6 +158,76 @@
TP_ARGS(ring, trb)
 );
 
+DECLARE_EVENT_CLASS(xhci_log_urb,
+   TP_PROTO(struct urb *urb),
+   TP_ARGS(urb),
+   TP_STRUCT__entry(
+   __field(void *, urb)
+   __field(unsigned int, pipe)
+   __field(unsigned int, stream)
+   __field(int, status)
+   __field(unsigned int, flags)
+   __field(int, num_mapped_sgs)
+   __field(int, num_sgs)
+   __field(int, length)
+   __field(int, actual)
+   __field(int, epnum)
+   __field(int, dir_in)
+   __field(int, type)
+   ),
+   TP_fast_assign(
+   __entry->urb = urb;
+   __entry->pipe = urb->pipe;
+   __entry->stream = urb->stream_id;
+   __entry->status = urb->status;
+   __entry->flags = urb->transfer_flags;
+   __entry->num_mapped_sgs = urb->num_mapped_sgs;
+   __entry->num_sgs = urb->num_sgs;
+   __entry->length = urb->transfer_buffer_length;
+   __entry->actual = urb->actual_length;
+   __entry->epnum = usb_endpoint_num(&urb->ep->desc);
+   __entry->dir_in = usb_endpoint_dir_in(&urb->ep->desc);
+   __entry->type = usb_endpoint_type(&urb->ep->desc);
+   ),
+   TP_printk("ep%d%s-%s: urb %p pipe %u length %d/%d sgs %d/%d stream %d 
flags %08x",
+   __entry->epnum, __entry->dir_in ? "in" : "out",
+   ({ char *s;
+   switch (__entry->type) {
+   case USB_ENDPOINT_XFER_INT:
+   s = "intr";
+   break;
+   case USB_ENDPOINT_XFER_CONTROL:
+   s = "control";
+   break;
+   case USB_ENDPOINT_XFER_BULK:
+   s = "bulk";
+   break;
+   case USB_ENDPOINT_XFER_ISOC:
+   s = "isoc";
+   break;
+   default:
+   s = "UNKNOWN";
+   } s; }), __entry->urb, __entry->pipe, __entry->actual,
+   __entry->length, __entry->num_mapped_sgs,
+   __entry->num_sgs, __entry->stream, __entry->flags
+   )
+);
+
+DEFINE_EVENT(xhci_log_urb, xhci_urb_enqueue,
+   TP_PROTO(struct urb *urb),
+   TP_ARGS(urb)
+);
+
+DEFINE_EVENT(xhci_log_urb, xhci_urb_giveback,
+   TP_PROTO(struct urb *urb),
+   TP_ARGS(urb)
+);
+
+DEFINE_EVENT(xhci_log_urb, xhci_urb_dequeue,
+   TP_PROTO(struct urb *urb),
+   TP_ARGS(urb)
+);
+
 #endif /* __XHCI_TRACE_H */
 
 /* this part must be outside header guard */
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5d6b5a2..958c92b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1383,6 +1383,8 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb 
*urb, gfp_t mem_flags)
urb_priv->td_cnt = 0;
urb->hcpriv = urb_priv;
 
+   trace_xhci_urb_enqueue(urb);
+
if (usb_endpoint_xfer_control(&urb->ep->desc)) {
/* Check to see if the max packet size for the default control
 * endpoint changed during FS device enumeration
@@ -1509,6 +1511,9 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb 
*urb, int status)
 
xhci = hcd_to_xhci(hcd);
spin_lock_irqsave(&xhci->lock, flags);
+
+   trace_xhci_urb_dequeue(urb);
+
/* Make sure the URB hasn't completed or been unlinked already */
ret = usb_hcd_check_unlink_urb(hcd, urb, status);
if (ret || !urb->hcpriv)
-- 
1.9.1

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

[PATCH v2 09/37] usb: host: xhci-plat: enable BROKEN_PED quirk if platform requested

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

In case 'quirk-broken-port-ped' property is passed in via device property,
we should enable the corresponding BROKEN_PED quirk flag for XHCI core.

[rog...@ti.com] Updated code from platform data to device property
and added DT binding.

Signed-off-by: Felipe Balbi 
Signed-off-by: Roger Quadros 
Signed-off-by: Mathias Nyman 
---
 Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
 drivers/usb/host/xhci-plat.c   | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt 
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index 0b7d857..2d80b60 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -27,6 +27,7 @@ Required properties:
 Optional properties:
   - clocks: reference to a clock
   - usb3-lpm-capable: determines if platform is USB3 LPM capable
+  - quirk-broken-port-ped: set if the controller has broken port disable 
mechanism
 
 Example:
usb@f0931000 {
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index f96caeb..e1939de 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -232,6 +232,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (device_property_read_bool(&pdev->dev, "usb3-lpm-capable"))
xhci->quirks |= XHCI_LPM_SUPPORT;
 
+   if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
+   xhci->quirks |= XHCI_BROKEN_PORT_PED;
+
hcd->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
if (IS_ERR(hcd->usb_phy)) {
ret = PTR_ERR(hcd->usb_phy);
-- 
1.9.1

--
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 10/37] usb: xhci: remove unnecessary assignment

2017-01-23 Thread Mathias Nyman
From: Lu Baolu 

Drop an unnecessary assignment in prepare_transfer().

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index bcc0894..61b5fea 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2844,8 +2844,6 @@ static int prepare_transfer(struct xhci_hcd *xhci,
td->start_seg = ep_ring->enq_seg;
td->first_trb = ep_ring->enqueue;
 
-   urb_priv->td[td_index] = td;
-
return 0;
 }
 
-- 
1.9.1

--
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 14/37] usb: host: xhci: change pre-increments to post-increments

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

This is a cleanup patch only, no functional changes. The idea is just to
make sure for loops look the same all over the driver.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-dbg.c | 20 ++--
 drivers/usb/host/xhci-mem.c |  8 
 drivers/usb/host/xhci.c | 14 +++---
 3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index 74c42f7..a3b67f3 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -177,7 +177,7 @@ static void xhci_print_ports(struct xhci_hcd *xhci)
ports = HCS_MAX_PORTS(xhci->hcs_params1);
addr = &xhci->op_regs->port_status_base;
for (i = 0; i < ports; i++) {
-   for (j = 0; j < NUM_PORT_REGS; ++j) {
+   for (j = 0; j < NUM_PORT_REGS; j++) {
xhci_dbg(xhci, "%p port %s reg = 0x%x\n",
addr, names[j],
(unsigned int) readl(addr));
@@ -240,7 +240,7 @@ void xhci_print_run_regs(struct xhci_hcd *xhci)
xhci_dbg(xhci, "  %p: Microframe index = 0x%x\n",
&xhci->run_regs->microframe_index,
(unsigned int) temp);
-   for (i = 0; i < 7; ++i) {
+   for (i = 0; i < 7; i++) {
temp = readl(&xhci->run_regs->rsvd[i]);
if (temp != XHCI_INIT_VALUE)
xhci_dbg(xhci, "  WARN: %p: Rsvd[%i] = 0x%x\n",
@@ -259,7 +259,7 @@ void xhci_print_registers(struct xhci_hcd *xhci)
 void xhci_print_trb_offsets(struct xhci_hcd *xhci, union xhci_trb *trb)
 {
int i;
-   for (i = 0; i < 4; ++i)
+   for (i = 0; i < 4; i++)
xhci_dbg(xhci, "Offset 0x%x = 0x%x\n",
i*4, trb->generic.field[i]);
 }
@@ -332,7 +332,7 @@ void xhci_debug_segment(struct xhci_hcd *xhci, struct 
xhci_segment *seg)
u64 addr = seg->dma;
union xhci_trb *trb = seg->trbs;
 
-   for (i = 0; i < TRBS_PER_SEGMENT; ++i) {
+   for (i = 0; i < TRBS_PER_SEGMENT; i++) {
trb = &seg->trbs[i];
xhci_dbg(xhci, "@%016llx %08x %08x %08x %08x\n", addr,
 lower_32_bits(le64_to_cpu(trb->link.segment_ptr)),
@@ -413,7 +413,7 @@ void xhci_dbg_erst(struct xhci_hcd *xhci, struct xhci_erst 
*erst)
int i;
struct xhci_erst_entry *entry;
 
-   for (i = 0; i < erst->num_entries; ++i) {
+   for (i = 0; i < erst->num_entries; i++) {
entry = &erst->entries[i];
xhci_dbg(xhci, "@%016llx %08x %08x %08x %08x\n",
 addr,
@@ -440,7 +440,7 @@ void xhci_dbg_cmd_ptrs(struct xhci_hcd *xhci)
 static void dbg_rsvd64(struct xhci_hcd *xhci, u64 *ctx, dma_addr_t dma)
 {
int i;
-   for (i = 0; i < 4; ++i) {
+   for (i = 0; i < 4; i++) {
xhci_dbg(xhci, "@%p (virt) @%08llx "
 "(dma) %#08llx - rsvd64[%d]\n",
 &ctx[4 + i], (unsigned long long)dma,
@@ -496,7 +496,7 @@ static void xhci_dbg_slot_ctx(struct xhci_hcd *xhci, struct 
xhci_container_ctx *
&slot_ctx->dev_state,
(unsigned long long)dma, slot_ctx->dev_state);
dma += field_size;
-   for (i = 0; i < 4; ++i) {
+   for (i = 0; i < 4; i++) {
xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - rsvd[%d]\n",
&slot_ctx->reserved[i], (unsigned long long)dma,
slot_ctx->reserved[i], i);
@@ -519,7 +519,7 @@ static void xhci_dbg_ep_ctx(struct xhci_hcd *xhci,
 
if (last_ep < 31)
last_ep_ctx = last_ep + 1;
-   for (i = 0; i < last_ep_ctx; ++i) {
+   for (i = 0; i < last_ep_ctx; i++) {
unsigned int epaddr = xhci_get_endpoint_address(i);
struct xhci_ep_ctx *ep_ctx = xhci_get_ep_ctx(xhci, ctx, i);
dma_addr_t dma = ctx->dma +
@@ -544,7 +544,7 @@ static void xhci_dbg_ep_ctx(struct xhci_hcd *xhci,
&ep_ctx->tx_info,
(unsigned long long)dma, ep_ctx->tx_info);
dma += field_size;
-   for (j = 0; j < 3; ++j) {
+   for (j = 0; j < 3; j++) {
xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - 
rsvd[%d]\n",
&ep_ctx->reserved[j],
(unsigned long long)dma,
@@ -583,7 +583,7 @@ void xhci_dbg_ctx(struct xhci_hcd *xhci,
 &ctrl_ctx->add_flags, (unsigned long long)dma,
 ctrl_ctx->add_flags);
dma += field_size;
-   for (i = 0; i < 6; ++i) {
+   for (i = 0; i < 6; i++) {
xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - 
rsvd2[%d]\n",

[PATCH v2 17/37] usb: host: xhci: simplify irq handler return

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

Instead of having several return points, let's use a local variable and
a single place to return. This makes the code slightly easier to read.

[set ret = IRQ_HANDLED in default working case  -Mathias]
Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 32 +---
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f71f4dd..efc4657 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2605,27 +2605,28 @@ static int xhci_handle_event(struct xhci_hcd *xhci)
 irqreturn_t xhci_irq(struct usb_hcd *hcd)
 {
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
-   u32 status;
-   u64 temp_64;
union xhci_trb *event_ring_deq;
+   irqreturn_t ret = IRQ_NONE;
dma_addr_t deq;
+   u64 temp_64;
+   u32 status;
 
spin_lock(&xhci->lock);
/* Check if the xHC generated the interrupt, or the irq is shared */
status = readl(&xhci->op_regs->status);
-   if (status == 0x)
-   goto hw_died;
-
-   if (!(status & STS_EINT)) {
-   spin_unlock(&xhci->lock);
-   return IRQ_NONE;
+   if (status == 0x) {
+   ret = IRQ_HANDLED;
+   goto out;
}
+
+   if (!(status & STS_EINT))
+   goto out;
+
if (status & STS_FATAL) {
xhci_warn(xhci, "WARNING: Host System Error\n");
xhci_halt(xhci);
-hw_died:
-   spin_unlock(&xhci->lock);
-   return IRQ_HANDLED;
+   ret = IRQ_HANDLED;
+   goto out;
}
 
/*
@@ -2656,9 +2657,8 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd)
temp_64 = xhci_read_64(xhci, &xhci->ir_set->erst_dequeue);
xhci_write_64(xhci, temp_64 | ERST_EHB,
&xhci->ir_set->erst_dequeue);
-   spin_unlock(&xhci->lock);
-
-   return IRQ_HANDLED;
+   ret = IRQ_HANDLED;
+   goto out;
}
 
event_ring_deq = xhci->event_ring->dequeue;
@@ -2683,10 +2683,12 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd)
/* Clear the event handler busy flag (RW1C); event ring is empty. */
temp_64 |= ERST_EHB;
xhci_write_64(xhci, temp_64, &xhci->ir_set->erst_dequeue);
+   ret = IRQ_HANDLED;
 
+out:
spin_unlock(&xhci->lock);
 
-   return IRQ_HANDLED;
+   return ret;
 }
 
 irqreturn_t xhci_msi_irq(int irq, void *hcd)
-- 
1.9.1

--
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 24/37] usb: host: xhci: unconditionally call xhci_unmap_td_bounce_buffer()

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

xhci_unmap_td_bounce_buffer() already checks for a valid td->bounce_seg
and bails out early if that's invalid. There's no need to check for this
twice.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 0de9966..8b78eee 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -793,8 +793,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, 
int slot_id,
 * just overwrite it (because the URB has been unlinked).
 */
ep_ring = xhci_urb_to_transfer_ring(xhci, cur_td->urb);
-   if (ep_ring && cur_td->bounce_seg)
-   xhci_unmap_td_bounce_buffer(xhci, ep_ring, cur_td);
+   xhci_unmap_td_bounce_buffer(xhci, ep_ring, cur_td);
inc_td_cnt(cur_td->urb);
if (last_td_in_urb(cur_td))
xhci_giveback_urb_in_irq(xhci, cur_td, 0);
@@ -820,8 +819,7 @@ static void xhci_kill_ring_urbs(struct xhci_hcd *xhci, 
struct xhci_ring *ring)
if (!list_empty(&cur_td->cancelled_td_list))
list_del_init(&cur_td->cancelled_td_list);
 
-   if (cur_td->bounce_seg)
-   xhci_unmap_td_bounce_buffer(xhci, ring, cur_td);
+   xhci_unmap_td_bounce_buffer(xhci, ring, cur_td);
 
inc_td_cnt(cur_td->urb);
if (last_td_in_urb(cur_td))
@@ -1833,8 +1831,7 @@ static int xhci_td_cleanup(struct xhci_hcd *xhci, struct 
xhci_td *td,
urb_priv = urb->hcpriv;
 
/* if a bounce buffer was used to align this td then unmap it */
-   if (td->bounce_seg)
-   xhci_unmap_td_bounce_buffer(xhci, ep_ring, td);
+   xhci_unmap_td_bounce_buffer(xhci, ep_ring, td);
 
/* Do one last check of the actual transfer length.
 * If the host controller said we transferred more data than the buffer
-- 
1.9.1

--
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 20/37] usb: host: xhci: reorder variable definitions

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

no functional changes. Simple cleanup to make sure variables are ordered
in a 'reverse christmas tree' fashion. While at that, also remove an
obsolete comment which doesn't apply anymore.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 4e7a6c2..627518d 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1822,22 +1822,18 @@ int xhci_is_vendor_info_code(struct xhci_hcd *xhci, 
unsigned int trb_comp_code)
return 0;
 }
 
-/*
- * Finish the td processing, remove the td from td list;
- * Return 1 if the urb can be given back.
- */
 static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
union xhci_trb *ep_trb, struct xhci_transfer_event *event,
struct xhci_virt_ep *ep, int *status, bool skip)
 {
struct xhci_virt_device *xdev;
-   struct xhci_ring *ep_ring;
-   unsigned int slot_id;
-   int ep_index;
-   struct urb *urb = NULL;
struct xhci_ep_ctx *ep_ctx;
+   struct xhci_ring *ep_ring;
struct urb_priv *urb_priv;
+   struct urb *urb = NULL;
+   unsigned int slot_id;
u32 trb_comp_code;
+   int ep_index;
 
slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
xdev = xhci->devs[slot_id];
-- 
1.9.1

--
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 28/37] usb: host: xhci: combine event TRB completion debugging messages

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

If we just provide a helper to convert completion code to string, we can
combine all debugging messages into a single print.

[keep the old debug messages, for warn and grep -Mathias]
Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.h | 80 +
 1 file changed, 80 insertions(+)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index aa63e38..ebdd920 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1097,6 +1097,86 @@ struct xhci_transfer_event {
 #define COMP_SECONDARY_BANDWIDTH_ERROR 35
 #define COMP_SPLIT_TRANSACTION_ERROR   36
 
+static inline const char *xhci_trb_comp_code_string(u8 status)
+{
+   switch (status) {
+   case COMP_INVALID:
+   return "Invalid";
+   case COMP_SUCCESS:
+   return "Success";
+   case COMP_DATA_BUFFER_ERROR:
+   return "Data Buffer Error";
+   case COMP_BABBLE_DETECTED_ERROR:
+   return "Babble Detected";
+   case COMP_USB_TRANSACTION_ERROR:
+   return "USB Transaction Error";
+   case COMP_TRB_ERROR:
+   return "TRB Error";
+   case COMP_STALL_ERROR:
+   return "Stall Error";
+   case COMP_RESOURCE_ERROR:
+   return "Resource Error";
+   case COMP_BANDWIDTH_ERROR:
+   return "Bandwidth Error";
+   case COMP_NO_SLOTS_AVAILABLE_ERROR:
+   return "No Slots Available Error";
+   case COMP_INVALID_STREAM_TYPE_ERROR:
+   return "Invalid Stream Type Error";
+   case COMP_SLOT_NOT_ENABLED_ERROR:
+   return "Slot Not Enabled Error";
+   case COMP_ENDPOINT_NOT_ENABLED_ERROR:
+   return "Endpoint Not Enabled Error";
+   case COMP_SHORT_PACKET:
+   return "Short Packet";
+   case COMP_RING_UNDERRUN:
+   return "Ring Underrun";
+   case COMP_RING_OVERRUN:
+   return "Ring Overrun";
+   case COMP_VF_EVENT_RING_FULL_ERROR:
+   return "VF Event Ring Full Error";
+   case COMP_PARAMETER_ERROR:
+   return "Parameter Error";
+   case COMP_BANDWIDTH_OVERRUN_ERROR:
+   return "Bandwidth Overrun Error";
+   case COMP_CONTEXT_STATE_ERROR:
+   return "Context State Error";
+   case COMP_NO_PING_RESPONSE_ERROR:
+   return "No Ping Response Error";
+   case COMP_EVENT_RING_FULL_ERROR:
+   return "Event Ring Full Error";
+   case COMP_INCOMPATIBLE_DEVICE_ERROR:
+   return "Incompatible Device Error";
+   case COMP_MISSED_SERVICE_ERROR:
+   return "Missed Service Error";
+   case COMP_COMMAND_RING_STOPPED:
+   return "Command Ring Stopped";
+   case COMP_COMMAND_ABORTED:
+   return "Command Aborted";
+   case COMP_STOPPED:
+   return "Stopped";
+   case COMP_STOPPED_LENGTH_INVALID:
+   return "Stopped - Length Invalid";
+   case COMP_STOPPED_SHORT_PACKET:
+   return "Stopped - Short Packet";
+   case COMP_MAX_EXIT_LATENCY_TOO_LARGE_ERROR:
+   return "Max Exit Latency Too Large Error";
+   case COMP_ISOCH_BUFFER_OVERRUN:
+   return "Isoch Buffer Overrun";
+   case COMP_EVENT_LOST_ERROR:
+   return "Event Lost Error";
+   case COMP_UNDEFINED_ERROR:
+   return "Undefined Error";
+   case COMP_INVALID_STREAM_ID_ERROR:
+   return "Invalid Stream ID Error";
+   case COMP_SECONDARY_BANDWIDTH_ERROR:
+   return "Secondary Bandwidth Error";
+   case COMP_SPLIT_TRANSACTION_ERROR:
+   return "Split Transaction Error";
+   default:
+   return "Unknown!!";
+   }
+}
+
 struct xhci_link_trb {
/* 64-bit segment pointer*/
__le64 segment_ptr;
-- 
1.9.1

--
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 32/37] usb: host: xhci: remove newline from tracer

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

If we add that newline, the output will look like the following:

 kworker/2:1-42[002]    169.811435: xhci_address_ctx:
ctx_64=0, ctx_type=2, ctx_dma=@153fbd000, ctx_va=@880153fbd000

We would rather have that in a single line.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-trace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 4bad0d6..08bed2f 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -103,7 +103,7 @@
((HCC_64BYTE_CONTEXT(xhci->hcc_params) + 1) * 32) *
((ctx->type == XHCI_CTX_TYPE_INPUT) + ep_num + 1));
),
-   TP_printk("\nctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p",
+   TP_printk("ctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p",
__entry->ctx_64, __entry->ctx_type,
(unsigned long long) __entry->ctx_dma, __entry->ctx_va
)
-- 
1.9.1

--
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 29/37] usb: host: xhci: make a generic TRB tracer

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

instead of having a tracer that can only trace command completions,
let's promote this tracer so it can trace and decode any TRB.

With that, it will be easier to extrapolate the lifetime of any TRB
which might help debugging certain issues.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c  |  14 +-
 drivers/usb/host/xhci-trace.h |  55 ---
 drivers/usb/host/xhci.h   | 329 ++
 3 files changed, 375 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 059825b..4316273 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1319,6 +1319,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
 
cmd_dma = le64_to_cpu(event->cmd_trb);
cmd_trb = xhci->cmd_ring->dequeue;
+
+   trace_xhci_handle_command(xhci->cmd_ring, &cmd_trb->generic);
+
cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
cmd_trb);
/*
@@ -1335,8 +1338,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
 
cancel_delayed_work(&xhci->cmd_timer);
 
-   trace_xhci_cmd_completion(cmd_trb, (struct xhci_generic_trb *) event);
-
cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status));
 
/* If CMD ring stopped we own the trbs between enqueue and dequeue */
@@ -2479,6 +2480,10 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 
ep_trb = &ep_seg->trbs[(ep_trb_dma - ep_seg->dma) /
sizeof(*ep_trb)];
+
+   trace_xhci_handle_transfer(ep_ring,
+   (struct xhci_generic_trb *) ep_trb);
+
/*
 * No-op TRB should not trigger interrupts.
 * If ep_trb is a no-op TRB, it means the
@@ -2545,6 +2550,8 @@ static int xhci_handle_event(struct xhci_hcd *xhci)
xhci->event_ring->cycle_state)
return 0;
 
+   trace_xhci_handle_event(xhci->event_ring, &event->generic);
+
/*
 * Barrier between reading the TRB_CYCLE (valid) flag above and any
 * speculative reads of the event's flags/data below.
@@ -2714,6 +2721,9 @@ static void queue_trb(struct xhci_hcd *xhci, struct 
xhci_ring *ring,
trb->field[1] = cpu_to_le32(field2);
trb->field[2] = cpu_to_le32(field3);
trb->field[3] = cpu_to_le32(field4);
+
+   trace_xhci_queue_trb(ring, trb);
+
inc_enq(xhci, ring, more_trbs_coming);
 }
 
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 59c0565..d01524b 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -115,34 +115,47 @@
TP_ARGS(xhci, ctx, ep_num)
 );
 
-DECLARE_EVENT_CLASS(xhci_log_event,
-   TP_PROTO(void *trb_va, struct xhci_generic_trb *ev),
-   TP_ARGS(trb_va, ev),
+DECLARE_EVENT_CLASS(xhci_log_trb,
+   TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
+   TP_ARGS(ring, trb),
TP_STRUCT__entry(
-   __field(void *, va)
-   __field(u64, dma)
-   __field(u32, status)
-   __field(u32, flags)
-   __dynamic_array(u8, trb, sizeof(struct xhci_generic_trb))
+   __field(u32, type)
+   __field(u32, field0)
+   __field(u32, field1)
+   __field(u32, field2)
+   __field(u32, field3)
),
TP_fast_assign(
-   __entry->va = trb_va;
-   __entry->dma = ((u64)le32_to_cpu(ev->field[1])) << 32 |
-   le32_to_cpu(ev->field[0]);
-   __entry->status = le32_to_cpu(ev->field[2]);
-   __entry->flags = le32_to_cpu(ev->field[3]);
-   memcpy(__get_dynamic_array(trb), trb_va,
-   sizeof(struct xhci_generic_trb));
+   __entry->type = ring->type;
+   __entry->field0 = le32_to_cpu(trb->field[0]);
+   __entry->field1 = le32_to_cpu(trb->field[1]);
+   __entry->field2 = le32_to_cpu(trb->field[2]);
+   __entry->field3 = le32_to_cpu(trb->field[3]);
),
-   TP_printk("\ntrb_dma=@%llx, trb_va=@%p, status=%08x, flags=%08x",
-   (unsigned long long) __entry->dma, __entry->va,
-   __entry->status, __entry->flags
+   TP_printk("%s: %s", xhci_ring_type_string(__entry->type),
+   xhci_decode_trb(__entry->field0, __entry->field1,
+   __entry->field2, __entry->field3)
)
 );
 
-DEFINE_EVENT(xhci_log_event, xhci_cmd_completion,
-   TP_PROTO(void *trb_va, struct xhci_generic_trb *ev),
-   TP_ARGS(trb_va, ev)
+DEFINE_EVENT(xhci_log_trb, xhci_handle_event,
+   TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
+   TP_ARGS(ring, trb)
+);

[PATCH v2 19/37] usb: host: xhci: use slightly better list helpers

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

Replace list_entry() with list_first_entry() and list_for_each() with
list_for_each_entry(). This makes the code slightly more readable.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index efc4657..4e7a6c2 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -689,7 +689,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, 
int slot_id,
unsigned int ep_index;
struct xhci_ring *ep_ring;
struct xhci_virt_ep *ep;
-   struct list_head *entry;
struct xhci_td *cur_td = NULL;
struct xhci_td *last_unlinked_td;
 
@@ -706,6 +705,8 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, 
int slot_id,
memset(&deq_state, 0, sizeof(deq_state));
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3]));
ep = &xhci->devs[slot_id]->eps[ep_index];
+   last_unlinked_td = list_last_entry(&ep->cancelled_td_list,
+   struct xhci_td, cancelled_td_list);
 
if (list_empty(&ep->cancelled_td_list)) {
xhci_stop_watchdog_timer_in_irq(xhci, ep);
@@ -719,8 +720,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, 
int slot_id,
 * it.  We're also in the event handler, so we can't get re-interrupted
 * if another Stop Endpoint command completes
 */
-   list_for_each(entry, &ep->cancelled_td_list) {
-   cur_td = list_entry(entry, struct xhci_td, cancelled_td_list);
+   list_for_each_entry(cur_td, &ep->cancelled_td_list, cancelled_td_list) {
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
"Removing canceled TD starting at 0x%llx 
(dma).",
(unsigned long long)xhci_trb_virt_to_dma(
@@ -762,7 +762,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, 
int slot_id,
 */
list_del_init(&cur_td->td_list);
}
-   last_unlinked_td = cur_td;
+
xhci_stop_watchdog_timer_in_irq(xhci, ep);
 
/* If necessary, queue a Set Transfer Ring Dequeue Pointer command */
@@ -784,7 +784,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, 
int slot_id,
 * So stop when we've completed the URB for the last TD we unlinked.
 */
do {
-   cur_td = list_entry(ep->cancelled_td_list.next,
+   cur_td = list_first_entry(&ep->cancelled_td_list,
struct xhci_td, cancelled_td_list);
list_del_init(&cur_td->cancelled_td_list);
 
@@ -1335,7 +1335,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
return;
}
 
-   cmd = list_entry(xhci->cmd_list.next, struct xhci_command, cmd_list);
+   cmd = list_first_entry(&xhci->cmd_list, struct xhci_command, cmd_list);
 
cancel_delayed_work(&xhci->cmd_timer);
 
@@ -1426,8 +1426,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
 
/* restart timer if this wasn't the last command */
if (!list_is_singular(&xhci->cmd_list)) {
-   xhci->current_cmd = list_entry(cmd->cmd_list.next,
-  struct xhci_command, cmd_list);
+   xhci->current_cmd = list_first_entry(&cmd->cmd_list,
+   struct xhci_command, cmd_list);
xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT);
} else if (xhci->current_cmd == cmd) {
xhci->current_cmd = NULL;
@@ -2421,7 +2421,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
goto cleanup;
}
 
-   td = list_entry(ep_ring->td_list.next, struct xhci_td, td_list);
+   td = list_first_entry(&ep_ring->td_list, struct xhci_td,
+ td_list);
if (ep->skip)
td_num--;
 
-- 
1.9.1

--
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 26/37] xhci: use the trb_to_noop() helper for command trbs

2017-01-23 Thread Mathias Nyman
Remove duplicate code by using trb_to_noop() when
handling Aborted commads

Based on earlier code by Felipe Balbi 

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index bd00ada..92715b9 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -314,7 +314,6 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd 
*xhci,
 struct xhci_command *cur_cmd)
 {
struct xhci_command *i_cmd;
-   u32 cycle_state;
 
/* Turn all aborted commands in list to no-ops, then restart */
list_for_each_entry(i_cmd, &xhci->cmd_list, cmd_list) {
@@ -326,15 +325,8 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd 
*xhci,
 
xhci_dbg(xhci, "Turn aborted command %p to no-op\n",
 i_cmd->command_trb);
-   /* get cycle state from the original cmd trb */
-   cycle_state = le32_to_cpu(
-   i_cmd->command_trb->generic.field[3]) & TRB_CYCLE;
-   /* modify the command trb to no-op command */
-   i_cmd->command_trb->generic.field[0] = 0;
-   i_cmd->command_trb->generic.field[1] = 0;
-   i_cmd->command_trb->generic.field[2] = 0;
-   i_cmd->command_trb->generic.field[3] = cpu_to_le32(
-   TRB_TYPE(TRB_CMD_NOOP) | cycle_state);
+
+   trb_to_noop(i_cmd->command_trb, TRB_CMD_NOOP);
 
/*
 * caller waiting for completion is called when command
-- 
1.9.1

--
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 23/37] usb: host: xhci: check for a valid ring when unmapping bounce buffer

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

This way we can remove checks for valid ring from call sites of
xhci_unmap_td_bounce_buffer()

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 9084afb..0de9966 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -655,7 +655,7 @@ static void xhci_unmap_td_bounce_buffer(struct xhci_hcd 
*xhci,
struct xhci_segment *seg = td->bounce_seg;
struct urb *urb = td->urb;
 
-   if (!seg || !urb)
+   if (!ring || !seg || !urb)
return;
 
if (usb_urb_dir_out(urb)) {
-- 
1.9.1

--
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 22/37] usb: host: xhci: remove bogus __releases()/__acquires() annotation

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

handle_tx_event() is not releasing xhci->lock nor reacquiring it, remove
the bogus annotation.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 31518dd..9084afb 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2223,8 +2223,6 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, 
struct xhci_td *td,
  */
 static int handle_tx_event(struct xhci_hcd *xhci,
struct xhci_transfer_event *event)
-   __releases(&xhci->lock)
-   __acquires(&xhci->lock)
 {
struct xhci_virt_device *xdev;
struct xhci_virt_ep *ep;
-- 
1.9.1

--
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 33/37] usb: host: xhci: add xhci_virt_device tracer

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

Let's start tracing at least part of an xhci_virt_device lifetime. We
might want to extend this tracepoint class later, but for now it already
exposes quite a bit of valuable information.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c   |  2 ++
 drivers/usb/host/xhci-mem.c   |  7 ++
 drivers/usb/host/xhci-trace.h | 57 +++
 drivers/usb/host/xhci.c   |  1 +
 4 files changed, 67 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 50d086b..3bddeaa 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -389,6 +389,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
if (!virt_dev)
return -ENODEV;
 
+   trace_xhci_stop_device(virt_dev);
+
cmd = xhci_alloc_command(xhci, false, true, GFP_NOIO);
if (!cmd) {
xhci_dbg(xhci, "Couldn't allocate command structure.\n");
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 0019094..e452492 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -936,6 +936,9 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int 
slot_id)
return;
 
dev = xhci->devs[slot_id];
+
+   trace_xhci_free_virt_device(dev);
+
xhci->dcbaa->dev_context_ptrs[slot_id] = 0;
if (!dev)
return;
@@ -1075,6 +1078,8 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int 
slot_id,
 &xhci->dcbaa->dev_context_ptrs[slot_id],
 le64_to_cpu(xhci->dcbaa->dev_context_ptrs[slot_id]));
 
+   trace_xhci_alloc_virt_device(dev);
+
return 1;
 fail:
xhci_free_virt_device(xhci, slot_id);
@@ -1249,6 +1254,8 @@ int xhci_setup_addressable_virt_dev(struct xhci_hcd 
*xhci, struct usb_device *ud
ep0_ctx->deq = cpu_to_le64(dev->eps[0].ring->first_seg->dma |
   dev->eps[0].ring->cycle_state);
 
+   trace_xhci_setup_addressable_virt_device(dev);
+
/* Steps 7 and 8 were done in xhci_alloc_virt_device() */
 
return 0;
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 08bed2f..1ac2cdf 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -158,6 +158,63 @@
TP_ARGS(ring, trb)
 );
 
+DECLARE_EVENT_CLASS(xhci_log_virt_dev,
+   TP_PROTO(struct xhci_virt_device *vdev),
+   TP_ARGS(vdev),
+   TP_STRUCT__entry(
+   __field(void *, vdev)
+   __field(unsigned long long, out_ctx)
+   __field(unsigned long long, in_ctx)
+   __field(int, devnum)
+   __field(int, state)
+   __field(int, speed)
+   __field(u8, portnum)
+   __field(u8, level)
+   __field(int, slot_id)
+   ),
+   TP_fast_assign(
+   __entry->vdev = vdev;
+   __entry->in_ctx = (unsigned long long) vdev->in_ctx->dma;
+   __entry->out_ctx = (unsigned long long) vdev->out_ctx->dma;
+   __entry->devnum = vdev->udev->devnum;
+   __entry->state = vdev->udev->state;
+   __entry->speed = vdev->udev->speed;
+   __entry->portnum = vdev->udev->portnum;
+   __entry->level = vdev->udev->level;
+   __entry->slot_id = vdev->udev->slot_id;
+   ),
+   TP_printk("vdev %p ctx %llx | %llx num %d state %d speed %d port %d 
level %d slot %d",
+   __entry->vdev, __entry->in_ctx, __entry->out_ctx,
+   __entry->devnum, __entry->state, __entry->speed,
+   __entry->portnum, __entry->level, __entry->slot_id
+   )
+);
+
+DEFINE_EVENT(xhci_log_virt_dev, xhci_alloc_virt_device,
+   TP_PROTO(struct xhci_virt_device *vdev),
+   TP_ARGS(vdev)
+);
+
+DEFINE_EVENT(xhci_log_virt_dev, xhci_free_virt_device,
+   TP_PROTO(struct xhci_virt_device *vdev),
+   TP_ARGS(vdev)
+);
+
+DEFINE_EVENT(xhci_log_virt_dev, xhci_setup_device,
+   TP_PROTO(struct xhci_virt_device *vdev),
+   TP_ARGS(vdev)
+);
+
+DEFINE_EVENT(xhci_log_virt_dev, xhci_setup_addressable_virt_device,
+   TP_PROTO(struct xhci_virt_device *vdev),
+   TP_ARGS(vdev)
+);
+
+DEFINE_EVENT(xhci_log_virt_dev, xhci_stop_device,
+   TP_PROTO(struct xhci_virt_device *vdev),
+   TP_ARGS(vdev)
+);
+
 DECLARE_EVENT_CLASS(xhci_log_urb,
TP_PROTO(struct urb *urb),
TP_ARGS(urb),
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 958c92b..4968e9a 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3848,6 +3848,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct 
usb_device *udev,
le32_to_cpu(slot_ctx->dev_info) >> 27);
 
spin_lock_irqsave(&xhci->lock, flags);
+   trace_xhci_setup_device(virt_dev);
ret = xhci

[PATCH v2 21/37] usb: host: xhci: introduce xhci_td_cleanup()

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

By extracting xhci_td_cleanup() from finish_td(), code before clearer
and easier to follow.

There are no functional changes with this patch. It's merely a cleanup.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 92 
 1 file changed, 50 insertions(+), 42 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 627518d..31518dd 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1822,6 +1822,55 @@ int xhci_is_vendor_info_code(struct xhci_hcd *xhci, 
unsigned int trb_comp_code)
return 0;
 }
 
+static int xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td,
+   struct xhci_ring *ep_ring, int *status)
+{
+   struct urb_priv *urb_priv;
+   struct urb *urb = NULL;
+
+   /* Clean up the endpoint's TD list */
+   urb = td->urb;
+   urb_priv = urb->hcpriv;
+
+   /* if a bounce buffer was used to align this td then unmap it */
+   if (td->bounce_seg)
+   xhci_unmap_td_bounce_buffer(xhci, ep_ring, td);
+
+   /* Do one last check of the actual transfer length.
+* If the host controller said we transferred more data than the buffer
+* length, urb->actual_length will be a very big number (since it's
+* unsigned).  Play it safe and say we didn't transfer anything.
+*/
+   if (urb->actual_length > urb->transfer_buffer_length) {
+   xhci_warn(xhci, "URB req %u and actual %u transfer length 
mismatch\n",
+ urb->transfer_buffer_length, urb->actual_length);
+   urb->actual_length = 0;
+   *status = 0;
+   }
+   list_del_init(&td->td_list);
+   /* Was this TD slated to be cancelled but completed anyway? */
+   if (!list_empty(&td->cancelled_td_list))
+   list_del_init(&td->cancelled_td_list);
+
+   inc_td_cnt(urb);
+   /* Giveback the urb when all the tds are completed */
+   if (last_td_in_urb(td)) {
+   if ((urb->actual_length != urb->transfer_buffer_length &&
+(urb->transfer_flags & URB_SHORT_NOT_OK)) ||
+   (*status != 0 && !usb_endpoint_xfer_isoc(&urb->ep->desc)))
+   xhci_dbg(xhci, "Giveback URB %p, len = %d, expected = 
%d, status = %d\n",
+urb, urb->actual_length,
+urb->transfer_buffer_length, *status);
+
+   /* set isoc urb status to 0 just as EHCI, UHCI, and OHCI */
+   if (usb_pipetype(urb->pipe) == PIPE_ISOCHRONOUS)
+   *status = 0;
+   xhci_giveback_urb_in_irq(xhci, td, *status);
+   }
+
+   return 0;
+}
+
 static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td,
union xhci_trb *ep_trb, struct xhci_transfer_event *event,
struct xhci_virt_ep *ep, int *status, bool skip)
@@ -1829,8 +1878,6 @@ static int finish_td(struct xhci_hcd *xhci, struct 
xhci_td *td,
struct xhci_virt_device *xdev;
struct xhci_ep_ctx *ep_ctx;
struct xhci_ring *ep_ring;
-   struct urb_priv *urb_priv;
-   struct urb *urb = NULL;
unsigned int slot_id;
u32 trb_comp_code;
int ep_index;
@@ -1873,46 +1920,7 @@ static int finish_td(struct xhci_hcd *xhci, struct 
xhci_td *td,
}
 
 td_cleanup:
-   /* Clean up the endpoint's TD list */
-   urb = td->urb;
-   urb_priv = urb->hcpriv;
-
-   /* if a bounce buffer was used to align this td then unmap it */
-   if (td->bounce_seg)
-   xhci_unmap_td_bounce_buffer(xhci, ep_ring, td);
-
-   /* Do one last check of the actual transfer length.
-* If the host controller said we transferred more data than the buffer
-* length, urb->actual_length will be a very big number (since it's
-* unsigned).  Play it safe and say we didn't transfer anything.
-*/
-   if (urb->actual_length > urb->transfer_buffer_length) {
-   xhci_warn(xhci, "URB req %u and actual %u transfer length 
mismatch\n",
- urb->transfer_buffer_length, urb->actual_length);
-   urb->actual_length = 0;
-   *status = 0;
-   }
-   list_del_init(&td->td_list);
-   /* Was this TD slated to be cancelled but completed anyway? */
-   if (!list_empty(&td->cancelled_td_list))
-   list_del_init(&td->cancelled_td_list);
-
-   inc_td_cnt(urb);
-   /* Giveback the urb when all the tds are completed */
-   if (last_td_in_urb(td)) {
-   if ((urb->actual_length != urb->transfer_buffer_length &&
-(urb->transfer_flags & URB_SHORT_NOT_OK)) ||
-   (*status != 0 && !usb_endpoint_xfer_isoc(&urb->ep->desc)))
-   xhci_dbg(xhci, "Giveback URB %p, len = %d, expected = 
%d, status = %d\n",
-

[PATCH v2 36/37] xhci: simplify how we store TDs in urb private data

2017-01-23 Thread Mathias Nyman
Instead of storing a zero length array of td pointers, and then
allocate memory both for the td pointer array and the td's, just
use a zero length array of actual td's in urb private data.

old:

struct urb_priv {
   struct xhci_td *td[0]
}

new:

struct urb_priv {
struct xhci_td td[0]
}

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mem.c  |  5 +
 drivers/usb/host/xhci-ring.c | 20 ++--
 drivers/usb/host/xhci.c  | 24 ++--
 drivers/usb/host/xhci.h  |  2 +-
 4 files changed, 18 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index e452492..ba1853f4 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1817,10 +1817,7 @@ struct xhci_command *xhci_alloc_command(struct xhci_hcd 
*xhci,
 
 void xhci_urb_free_priv(struct urb_priv *urb_priv)
 {
-   if (urb_priv) {
-   kfree(urb_priv->td[0]);
-   kfree(urb_priv);
-   }
+   kfree(urb_priv);
 }
 
 void xhci_free_command(struct xhci_hcd *xhci,
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7a70406..d9936c7 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2838,7 +2838,7 @@ static int prepare_transfer(struct xhci_hcd *xhci,
return ret;
 
urb_priv = urb->hcpriv;
-   td = urb_priv->td[td_index];
+   td = &urb_priv->td[td_index];
 
INIT_LIST_HEAD(&td->td_list);
INIT_LIST_HEAD(&td->cancelled_td_list);
@@ -3134,7 +3134,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t 
mem_flags,
if (urb->transfer_flags & URB_ZERO_PACKET && urb_priv->num_tds > 1)
need_zero_pkt = true;
 
-   td = urb_priv->td[0];
+   td = &urb_priv->td[0];
 
/*
 * Don't give the first TRB to the hardware (by toggling the cycle bit)
@@ -3227,7 +3227,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t 
mem_flags,
ret = prepare_transfer(xhci, xhci->devs[slot_id],
   ep_index, urb->stream_id,
   1, urb, 1, mem_flags);
-   urb_priv->td[1]->last_trb = ring->enqueue;
+   urb_priv->td[1].last_trb = ring->enqueue;
field = TRB_TYPE(TRB_NORMAL) | ring->cycle_state | TRB_IOC;
queue_trb(xhci, ring, 0, 0, 0, TRB_INTR_TARGET(0), field);
}
@@ -3279,7 +3279,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t 
mem_flags,
return ret;
 
urb_priv = urb->hcpriv;
-   td = urb_priv->td[0];
+   td = &urb_priv->td[0];
 
/*
 * Don't give the first TRB to the hardware (by toggling the cycle bit)
@@ -3567,7 +3567,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, 
gfp_t mem_flags,
return ret;
goto cleanup;
}
-   td = urb_priv->td[i];
+   td = &urb_priv->td[i];
 
/* use SIA as default, if frame id is used overwrite it */
sia_frame_id = TRB_SIA;
@@ -3674,20 +3674,20 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, 
gfp_t mem_flags,
/* Clean up a partially enqueued isoc transfer. */
 
for (i--; i >= 0; i--)
-   list_del_init(&urb_priv->td[i]->td_list);
+   list_del_init(&urb_priv->td[i].td_list);
 
/* Use the first TD as a temporary variable to turn the TDs we've queued
 * into No-ops with a software-owned cycle bit. That way the hardware
 * won't accidentally start executing bogus TDs when we partially
 * overwrite them.  td->first_trb and td->start_seg are already set.
 */
-   urb_priv->td[0]->last_trb = ep_ring->enqueue;
+   urb_priv->td[0].last_trb = ep_ring->enqueue;
/* Every TRB except the first & last will have its cycle bit flipped. */
-   td_to_noop(xhci, ep_ring, urb_priv->td[0], true);
+   td_to_noop(xhci, ep_ring, &urb_priv->td[0], true);
 
/* Reset the ring enqueue back to the first TRB and its cycle bit. */
-   ep_ring->enqueue = urb_priv->td[0]->first_trb;
-   ep_ring->enq_seg = urb_priv->td[0]->start_seg;
+   ep_ring->enqueue = urb_priv->td[0].first_trb;
+   ep_ring->enq_seg = urb_priv->td[0].start_seg;
ep_ring->cycle_state = start_cycle;
ep_ring->num_trbs_free = ep_ring->num_trbs_free_temp;
usb_hcd_unlink_urb_from_ep(bus_to_hcd(urb->dev->bus), urb);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index bee6272..dde5c2d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1332,12 +1332,11 @@ static int xhci_check_maxpacket(struct xhci_hcd *xhci, 
unsigned int slot_id,
 int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
 {
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
-   struct xhci_td *buffer;
unsigned long fl

[PATCH v2 35/37] xhci: Rename variables related to transfer descritpors

2017-01-23 Thread Mathias Nyman
urb_priv structure has a count on how many TDs the
URB contains, and how many of those TD's we have handled.

rename:
length -> num_tds
td_cnt -> num_tds_done

No functional changes

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 10 +-
 drivers/usb/host/xhci.c  | 14 +++---
 drivers/usb/host/xhci.h  |  4 ++--
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index e0a49cb..7a70406 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -119,14 +119,14 @@ static bool last_td_in_urb(struct xhci_td *td)
 {
struct urb_priv *urb_priv = td->urb->hcpriv;
 
-   return urb_priv->td_cnt == urb_priv->length;
+   return urb_priv->num_tds_done == urb_priv->num_tds;
 }
 
 static void inc_td_cnt(struct urb *urb)
 {
struct urb_priv *urb_priv = urb->hcpriv;
 
-   urb_priv->td_cnt++;
+   urb_priv->num_tds_done++;
 }
 
 static void trb_to_noop(union xhci_trb *trb, u32 noop_type)
@@ -2055,7 +2055,7 @@ static int process_isoc_td(struct xhci_hcd *xhci, struct 
xhci_td *td,
ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event->buffer));
trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len));
urb_priv = td->urb->hcpriv;
-   idx = urb_priv->td_cnt;
+   idx = urb_priv->num_tds_done;
frame = &td->urb->iso_frame_desc[idx];
requested = frame->length;
remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
@@ -2134,7 +2134,7 @@ static int skip_isoc_td(struct xhci_hcd *xhci, struct 
xhci_td *td,
 
ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event->buffer));
urb_priv = td->urb->hcpriv;
-   idx = urb_priv->td_cnt;
+   idx = urb_priv->num_tds_done;
frame = &td->urb->iso_frame_desc[idx];
 
/* The transfer is partly done. */
@@ -3131,7 +3131,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t 
mem_flags,
urb_priv = urb->hcpriv;
 
/* Deal with URB_ZERO_PACKET - need one more td/trb */
-   if (urb->transfer_flags & URB_ZERO_PACKET && urb_priv->length > 1)
+   if (urb->transfer_flags & URB_ZERO_PACKET && urb_priv->num_tds > 1)
need_zero_pkt = true;
 
td = urb_priv->td[0];
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 40b1486..bee6272 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1379,8 +1379,8 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb 
*urb, gfp_t mem_flags)
buffer++;
}
 
-   urb_priv->length = num_tds;
-   urb_priv->td_cnt = 0;
+   urb_priv->num_tds = num_tds;
+   urb_priv->num_tds_done = 0;
urb->hcpriv = urb_priv;
 
trace_xhci_urb_enqueue(urb);
@@ -1523,8 +1523,8 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb 
*urb, int status)
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
"HW died, freeing TD.");
urb_priv = urb->hcpriv;
-   for (i = urb_priv->td_cnt;
-i < urb_priv->length && xhci->devs[urb->dev->slot_id];
+   for (i = urb_priv->num_tds_done;
+i < urb_priv->num_tds && xhci->devs[urb->dev->slot_id];
 i++) {
td = urb_priv->td[i];
if (!list_empty(&td->td_list))
@@ -1549,8 +1549,8 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb 
*urb, int status)
}
 
urb_priv = urb->hcpriv;
-   i = urb_priv->td_cnt;
-   if (i < urb_priv->length)
+   i = urb_priv->num_tds_done;
+   if (i < urb_priv->num_tds)
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
"Cancel URB %p, dev %s, ep 0x%x, "
"starting at offset 0x%llx",
@@ -1560,7 +1560,7 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb 
*urb, int status)
urb_priv->td[i]->start_seg,
urb_priv->td[i]->first_trb));
 
-   for (; i < urb_priv->length; i++) {
+   for (; i < urb_priv->num_tds; i++) {
td = urb_priv->td[i];
list_add_tail(&td->cancelled_td_list, &ep->cancelled_td_list);
}
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 9193a42..dab2719 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1608,8 +1608,8 @@ struct xhci_scratchpad {
 };
 
 struct urb_priv {
-   int length;
-   int td_cnt;
+   int num_tds;
+   int num_tds_done;
struct  xhci_td *td[0];
 };
 
-- 
1.9.1

--
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 36/37] xhci: simplify how we store TDs in urb private data

2017-01-23 Thread David Laight
From: Mathias Nyman
> Sent: 20 January 2017 14:47

> Instead of storing a zero length array of td pointers, and then
> allocate memory both for the td pointer array and the td's, just
> use a zero length array of actual td's in urb private data.

This reminds me of an old patch that got reverted because things
broke because the lifetimes/accesses of the data wasn't the obvious one.

That might have been a different structure.

FWIW it is a shame that things like USB3 ethernet can't transmit and
receive without all these intermediate and dynamically allocated
structures.

David

--
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 31/37] usb: host: xhci: convert several if() to a single switch statement

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

when getting endpoint type, a switch statement looks
better than a series of if () branches. There are no
functional changes with this patch, cleanup only.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mem.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 0640e762..0019094 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1414,14 +1414,16 @@ static u32 xhci_get_endpoint_type(struct 
usb_host_endpoint *ep)
 
in = usb_endpoint_dir_in(&ep->desc);
 
-   if (usb_endpoint_xfer_control(&ep->desc))
+   switch (usb_endpoint_type(&ep->desc)) {
+   case USB_ENDPOINT_XFER_CONTROL:
return CTRL_EP;
-   if (usb_endpoint_xfer_bulk(&ep->desc))
+   case USB_ENDPOINT_XFER_BULK:
return in ? BULK_IN_EP : BULK_OUT_EP;
-   if (usb_endpoint_xfer_isoc(&ep->desc))
+   case USB_ENDPOINT_XFER_ISOC:
return in ? ISOC_IN_EP : ISOC_OUT_EP;
-   if (usb_endpoint_xfer_int(&ep->desc))
+   case USB_ENDPOINT_XFER_INT:
return in ? INT_IN_EP : INT_OUT_EP;
+   }
return 0;
 }
 
-- 
1.9.1

--
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 34/37] xhci: rename size variable to num_tds

2017-01-23 Thread Mathias Nyman
No functinal changes.
num_tds describes the number of transfer descriptor better than "size"

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4968e9a..40b1486 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1337,7 +1337,7 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb 
*urb, gfp_t mem_flags)
int ret = 0;
unsigned int slot_id, ep_index;
struct urb_priv *urb_priv;
-   int size, i;
+   int num_tds, i;
 
if (!urb || xhci_check_args(hcd, urb->dev, urb->ep,
true, true, __func__) <= 0)
@@ -1354,32 +1354,32 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb 
*urb, gfp_t mem_flags)
}
 
if (usb_endpoint_xfer_isoc(&urb->ep->desc))
-   size = urb->number_of_packets;
+   num_tds = urb->number_of_packets;
else if (usb_endpoint_is_bulk_out(&urb->ep->desc) &&
urb->transfer_buffer_length > 0 &&
urb->transfer_flags & URB_ZERO_PACKET &&
!(urb->transfer_buffer_length % usb_endpoint_maxp(&urb->ep->desc)))
-   size = 2;
+   num_tds = 2;
else
-   size = 1;
+   num_tds = 1;
 
urb_priv = kzalloc(sizeof(struct urb_priv) +
- size * sizeof(struct xhci_td *), mem_flags);
+  num_tds * sizeof(struct xhci_td *), mem_flags);
if (!urb_priv)
return -ENOMEM;
 
-   buffer = kzalloc(size * sizeof(struct xhci_td), mem_flags);
+   buffer = kzalloc(num_tds * sizeof(struct xhci_td), mem_flags);
if (!buffer) {
kfree(urb_priv);
return -ENOMEM;
}
 
-   for (i = 0; i < size; i++) {
+   for (i = 0; i < num_tds; i++) {
urb_priv->td[i] = buffer;
buffer++;
}
 
-   urb_priv->length = size;
+   urb_priv->length = num_tds;
urb_priv->td_cnt = 0;
urb->hcpriv = urb_priv;
 
-- 
1.9.1

--
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 27/37] usb: host: xhci: convert to list_for_each_entry_safe()

2017-01-23 Thread Mathias Nyman
From: Felipe Balbi 

instead of using while(!list_empty()) followed by list_first_entry(), we
can actually use list_for_each_entry_safe().

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 92715b9..059825b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -808,11 +808,11 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd 
*xhci, int slot_id,
 static void xhci_kill_ring_urbs(struct xhci_hcd *xhci, struct xhci_ring *ring)
 {
struct xhci_td *cur_td;
+   struct xhci_td *tmp;
 
-   while (!list_empty(&ring->td_list)) {
-   cur_td = list_first_entry(&ring->td_list,
-   struct xhci_td, td_list);
+   list_for_each_entry_safe(cur_td, tmp, &ring->td_list, td_list) {
list_del_init(&cur_td->td_list);
+
if (!list_empty(&cur_td->cancelled_td_list))
list_del_init(&cur_td->cancelled_td_list);
 
@@ -828,6 +828,7 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci,
int slot_id, int ep_index)
 {
struct xhci_td *cur_td;
+   struct xhci_td *tmp;
struct xhci_virt_ep *ep;
struct xhci_ring *ring;
 
@@ -853,12 +854,12 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci,
slot_id, ep_index);
xhci_kill_ring_urbs(xhci, ring);
}
-   while (!list_empty(&ep->cancelled_td_list)) {
-   cur_td = list_first_entry(&ep->cancelled_td_list,
-   struct xhci_td, cancelled_td_list);
-   list_del_init(&cur_td->cancelled_td_list);
 
+   list_for_each_entry_safe(cur_td, tmp, &ep->cancelled_td_list,
+   cancelled_td_list) {
+   list_del_init(&cur_td->cancelled_td_list);
inc_td_cnt(cur_td->urb);
+
if (last_td_in_urb(cur_td))
xhci_giveback_urb_in_irq(xhci, cur_td, -ESHUTDOWN);
}
-- 
1.9.1

--
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 32/37] usb: host: xhci: remove newline from tracer

2017-01-23 Thread Felipe Balbi

Hi,

David Laight  writes:
> From: Mathias Nyman
>> Sent: 20 January 2017 14:47
>> From: Felipe Balbi 
>> 
>> If we add that newline, the output will like like the following:
>> 
>>  kworker/2:1-42[002]    169.811435: xhci_address_ctx:
>> ctx_64=0, ctx_type=2, ctx_dma=@153fbd000, ctx_va=@880153fbd000
>> 
>> We would rather have that in a single line.
> ...
>> diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
>> index 4bad0d6..08bed2f 100644
>> --- a/drivers/usb/host/xhci-trace.h
>> +++ b/drivers/usb/host/xhci-trace.h
>> @@ -103,7 +103,7 @@
>>  ((HCC_64BYTE_CONTEXT(xhci->hcc_params) + 1) * 32) *
>>  ((ctx->type == XHCI_CTX_TYPE_INPUT) + ep_num + 1));
>>  ),
>> -TP_printk("\nctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p",
>> +TP_printk("ctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p",
>>  __entry->ctx_64, __entry->ctx_type,
>>  (unsigned long long) __entry->ctx_dma, __entry->ctx_va
>
> I suspect the '\n' needs to go at the end of the previous trace?
> Attempts to generate single lines from multiple printk() calls
> are doomed to give unreadable output.
>
> As are attempts to generate readable multi-line output.
> (Ever had multiple cpus splat stack traces at the same time??)

TP_printk() adds \n to the end of every single tracepoint. Tracepoints
shouldn't have \n ever ;-)

-- 
balbi


signature.asc
Description: PGP signature


RE: [PATCH 32/37] usb: host: xhci: remove newline from tracer

2017-01-23 Thread David Laight
From: Mathias Nyman
> Sent: 20 January 2017 14:47
> From: Felipe Balbi 
> 
> If we add that newline, the output will like like the following:
> 
>  kworker/2:1-42[002]    169.811435: xhci_address_ctx:
> ctx_64=0, ctx_type=2, ctx_dma=@153fbd000, ctx_va=@880153fbd000
> 
> We would rather have that in a single line.
...
> diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
> index 4bad0d6..08bed2f 100644
> --- a/drivers/usb/host/xhci-trace.h
> +++ b/drivers/usb/host/xhci-trace.h
> @@ -103,7 +103,7 @@
>   ((HCC_64BYTE_CONTEXT(xhci->hcc_params) + 1) * 32) *
>   ((ctx->type == XHCI_CTX_TYPE_INPUT) + ep_num + 1));
>   ),
> - TP_printk("\nctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p",
> + TP_printk("ctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p",
>   __entry->ctx_64, __entry->ctx_type,
>   (unsigned long long) __entry->ctx_dma, __entry->ctx_va

I suspect the '\n' needs to go at the end of the previous trace?
Attempts to generate single lines from multiple printk() calls
are doomed to give unreadable output.

As are attempts to generate readable multi-line output.
(Ever had multiple cpus splat stack traces at the same time??)

David
--
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 v2 1/5] UDC: Split the driver into amd (pci) and Synopsys core driver

2017-01-23 Thread Raviteja Garimella
Hi Florian,

On Fri, Jan 20, 2017 at 12:58 AM, Florian Fainelli  wrote:
> On 01/19/2017 02:44 AM, Raviteja Garimella wrote:
>> Hi,
>>
>> On Thu, Jan 19, 2017 at 12:15 AM, Florian Fainelli  
>> wrote:
>>> On 01/17/2017 12:05 AM, Raviteja Garimella wrote:
 This patch splits the amd5536udc driver into two -- one that does
 pci device registration and the other file that does the rest of
 the driver tasks like the gadget/ep ops etc for Synopsys UDC.

 This way of splitting helps in exporting core driver symbols which
 can be used by any other platform/pci driver that is written for
 the same Synopsys USB device controller.

 The current patch also includes a change in the Kconfig and Makefile.
 A new config option USB_SNP_CORE will be selected automatically when
 any one of the platform or pci driver for the same UDC is selected.

 Signed-off-by: Raviteja Garimella 
>>>
>>> Although the changes you have done make sense and it is most certainly a
>>> good idea to split udc core from bus specific glue logic, it is really
>>> hard to review the changes per-se because of the file rename, could that
>>> happen at a later time?
>>
>> If you start looking at this specific patch from the header file 
>> (amd5536udc.h),
>> the additions in there comprise of:
>> - 9 function declarations
>> - some module parameter variable declarations that's moved out from the older
>>   common file amd5536udc.c
>> - 2 #includes that are needed by all files.
>
> Well, I don't really question the changes themselves, rather how this is
> presented as a patch series to reviewers.
>
> What I would do, to help introduce both the rename, and the splitting of
> core vs. bus-glue specific changes is:
>
> - have an initial patch which extracts the core functionality of the
> driver and the PCI bus glue logic into adm5536udc_pci.c and left
> adm5536udc.c intact (that would be a small delta to review)
>
> - have a second patch that performs the file rename from adm5536udc.c
> into snps_udc_core.c and updates adm5536udc_pci.c eventually as a result
> of that, then again, a reviewer can ignore the rename part (don't format
> to generate your patches with git format-patch -M in that case) and just
> focus on the conversion part for adm5536udc_pci.c
>

Just waited for any more comments coming in. I will submit the next version
as PATCH like the way you suggested.

>>
>> So, basically what's done for this split is that:
>> 1. the static keyword is removed from those 9 functions in the new file
>> snps_udc_core.c and are exported.
>> 2. The module parameters declarations (since they are used in both core
>> and pci file) are moved to the header file now.
>
> These should really be part of the commit messages for each commit doing
> the changes, this is meant to help a reviewer understand what you are
> doing, and to some degree, will help him/her make an educated decision
> as to what part of the code the focus should be put on.
>

Will do.

Thanks,
Ravi

> Thanks
> --
> Florian
--
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: dwc3: handle DWC_USB3_NUM == DWC_USB3_NUM_IN_EPS

2017-01-23 Thread Bryan O'Donoghue
On 23/01/17 12:08, Felipe Balbi wrote:
> 
> Hi,
> 
> Bryan O'Donoghue  writes:
>> - DWC_USB3_NUM indicates the number of Device mode single directional
>>   endpoints, including OUT and IN endpoint 0.
>>
>> - DWC_USB3_NUM_IN_EPS indicates the maximum number of Device mode IN
>>   endpoints active at any time, including control endpoint 0.
>>
>> It's possible to configure RTL such that DWC_USB3_NUM_EPS is equal to
>> DWC_USB3_NUM_IN_EPS.
>>
>> dwc3-core calculates the number of OUT endpoints as DWC_USB3_NUM minus
>> DWC_USB3_NUM_IN_EPS. If RTL has been configured with DWC_USB3_NUM_IN_EPS
>> equal to DWC_USB3_NUM then dwc3-core will calculate the number of OUT
>> endpoints as zero.
>>
>> For example a from dwc3_core_num_eps() shows:
>> [1.565000]  /usb0@f01d: found 8 IN and 0 OUT endpoints
>>
>> This patch works around this case by detecting when DWC_USB3_NUM_EPS is
>> equal to DWC3_USB3_NUM_IN_EPS and over-rides the calculation of the number
>> of OUT and IN endpoints to make dwc->num_in_eps equal to half
>> DWC_USB3_NUM_EPS.
>>
>> If DWC_USB3_NUM_EPS is equal to DWC3_USB3_NUM_IN_EPS and the endpoint count
>> is an odd number then dwc->num_out_eps will be assigned the extra endpoint.
> 
> sorry, now that I spent some more time with this. Isn't something like
> below solving all problems?
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 369bab16a824..68c9c84b7216 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -397,8 +397,7 @@ static void dwc3_core_num_eps(struct dwc3 *dwc)
>  {
>   struct dwc3_hwparams*parms = &dwc->hwparams;
>  
> - dwc->num_in_eps = DWC3_NUM_IN_EPS(parms);
> - dwc->num_out_eps = DWC3_NUM_EPS(parms) - dwc->num_in_eps;
> + dwc->num_eps = DWC3_NUM_EPS(parms);
>  }
>  
>  static void dwc3_cache_hwparams(struct dwc3 *dwc)
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 0a664d8eba3f..8c187df0aa42 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2001,23 +2002,7 @@ static int dwc3_gadget_init_hw_endpoints(struct dwc3 
> *dwc,
>  
>  static int dwc3_gadget_init_endpoints(struct dwc3 *dwc)
>  {
> - int ret;
> -
> - INIT_LIST_HEAD(&dwc->gadget.ep_list);
> -
> - ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_out_eps, 0);
> - if (ret < 0) {
> - dev_err(dwc->dev, "failed to initialize OUT endpoints\n");
> - return ret;
> - }
> -
> - ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_in_eps, 1);
> - if (ret < 0) {
> - dev_err(dwc->dev, "failed to initialize IN endpoints\n");
> - return ret;
> - }
> -
> - return 0;
> + return dwc3_gadget_init_hw_endpoints(dwc, dwc->num_eps);

Well I hadn't considered that level of change myself but, it should work.

>  
>  static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
> 
> (clearly this won't compile... It's just to illustrate)
> 
> The HW actually already tells us total number of endpoints and according
> to John, they can all behave in either direction. Can you test it out
> and finish it up as a proper patch?

Sure no problem.

> 
> I'll make sure to fix up the "maximum number of IN endpoints enabled at
> one time" for v4.12.
> 
> thanks
> 

--
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 v3 1/3] usb: musb: dma: Add a DMA completion platform callback

2017-01-23 Thread Alexandre Bailon
On 01/20/2017 09:00 PM, Bin Liu wrote:
> On Thu, Jan 19, 2017 at 11:06:57AM +0100, Alexandre Bailon wrote:
>> Currently, the CPPI 4.1 driver is not completely generic and
>> only work on dsps. This is because of IRQ management.
>> Add a callback to dma_controller that could be invoked on DMA completion
>> to acknodlege the IRQ.
>>
>> Signed-off-by: Alexandre Bailon 
>> ---
>>  drivers/usb/musb/musb_cppi41.c | 7 +--
>>  drivers/usb/musb/musb_dma.h| 4 
>>  2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
>> index 1636385..f7d3d27 100644
>> --- a/drivers/usb/musb/musb_cppi41.c
>> +++ b/drivers/usb/musb/musb_cppi41.c
>> @@ -217,6 +217,10 @@ static void cppi41_dma_callback(void *private_data)
>>  int is_hs = 0;
>>  bool empty;
>>  
>> +controller = cppi41_channel->controller;
>> +if (controller->controller.dma_callback)
>> +controller->controller.dma_callback(&controller->controller);
>> +
>>  spin_lock_irqsave(&musb->lock, flags);
>>  
>>  dmaengine_tx_status(cppi41_channel->dc, cppi41_channel->cookie,
>> @@ -249,8 +253,6 @@ static void cppi41_dma_callback(void *private_data)
>>   * We spin on HS (no longer than than 25us and setup a timer on
>>   * FS to check for the bit and complete the transfer.
>>   */
>> -controller = cppi41_channel->controller;
>> -
>>  if (is_host_active(musb)) {
>>  if (musb->port1_status & USB_PORT_STAT_HIGH_SPEED)
>>  is_hs = 1;
>> @@ -695,6 +697,7 @@ cppi41_dma_controller_create(struct musb *musb, void 
>> __iomem *base)
>>  controller->controller.channel_program = cppi41_dma_channel_program;
>>  controller->controller.channel_abort = cppi41_dma_channel_abort;
>>  controller->controller.is_compatible = cppi41_is_compatible;
>> +controller->controller.musb = musb;
>>  
>>  ret = cppi41_dma_controller_start(controller);
>>  if (ret)
>> diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
>> index 46357e1..8bea0cd 100644
>> --- a/drivers/usb/musb/musb_dma.h
>> +++ b/drivers/usb/musb/musb_dma.h
>> @@ -181,10 +181,13 @@ dma_channel_status(struct dma_channel *c)
>>   * @channel_release: call this to release a DMA channel
>>   * @channel_abort: call this to abort a pending DMA transaction,
>>   *  returning it to FREE (but allocated) state
>> + * @platform_dma_callback: invoked on DMA completion, useful to run platform
>> + *  code such IRQ acknowledgment.
>>   *
>>   * Controllers manage dma channels.
>>   */
>>  struct dma_controller {
>> +struct musb *musb;
> 
> Since musb is added here, can we clean up the corresponding one in
> struct cppi41_dma_controller and struct cppi?
Yes, and also for the one in tusb_omap_dma.
> 
> Regards,
> -Bin.
> 
Regards,
Alexandre
--
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 6/6] dt-bindings: mt8173-mtu3: add reference clock

2017-01-23 Thread Rob Herring
On Sat, Jan 21, 2017 at 7:49 PM, Chunfeng Yun  wrote:
> Hi,
>
> On Sat, 2017-01-21 at 14:11 -0600, Rob Herring wrote:
>> On Wed, Jan 18, 2017 at 02:08:27PM +0800, Chunfeng Yun wrote:
>> > add a reference clock for compatibility
>>
>> Why? This block suddenly has 2 clocks instead of 1?
> In fact, there is a reference clock which comes from 26M oscillator
> directly. I ignore it because it is a fixed-clock in DTS, and always
> turned on for mt8173. But later, I find that I made a mistake before
> when I bring up it on a new platform whose reference clock comes from
> PLL, and need control it. So here add it, no matter it is a fixed-clock
> or not.

Add this explanation to the changelog.

Rob
--
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: [RESEND PATCH 6/6] dt-bindings: phy-mt65xx-usb: add support for mt2712 platform

2017-01-23 Thread Rob Herring
On Sat, Jan 21, 2017 at 8:50 PM, Chunfeng Yun  wrote:
> On Sat, 2017-01-21 at 14:08 -0600, Rob Herring wrote:
>> On Wed, Jan 18, 2017 at 02:00:14PM +0800, Chunfeng Yun wrote:
>> > add a new compatible string for "mt2712", and a new reference clock
>> > for SuperSpeed analog phy;
>> >
>> > Signed-off-by: Chunfeng Yun 
>> > ---
>> >  .../devicetree/bindings/phy/phy-mt65xx-usb.txt |   81 
>> > +---
>> >  1 file changed, 70 insertions(+), 11 deletions(-)

[...]

>> > @@ -31,21 +37,27 @@ Example:
>> >  u3phy: usb-phy@1129 {
>> > compatible = "mediatek,mt8173-u3phy";
>> > reg = <0 0x1129 0 0x800>;
>> > -   clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
>> > -   clock-names = "u3phya_ref";
>> > +   clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>, <&clk26m>;
>> > +   clock-names = "u2ref_clk", "u3ref_clk";
>> > #address-cells = <2>;
>> > #size-cells = <2>;
>> > ranges;
>> > status = "okay";
>> >
>> > -   phy_port0: port@11290800 {
>> > -   reg = <0 0x11290800 0 0x800>;
>> > +   u2port0: port@11290800 {
>>
>> port is for OF graph. This should be usb-phy@... instead.
> Is there any problems if u2port0's name@addr is the same as its parent's
> (u3phy)?  as following:
> u3phy: usb-phy@1129 {
> compatible = ...;
> // no reg property
> clocks = ...;
> u2port0: usb-phy@1129 {
> reg = ...;
> }

No problem, that is fine.

Rob
--
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] HID: get rid of HID_QUIRK_NO_INIT_REPORTS

2017-01-23 Thread Jiri Kosina
On Thu, 5 Jan 2017, Benjamin Tissoires wrote:

> I don't know what to do about hiddev too. I don't know if we have actual 
> users besides some debugging tools. And hidraw is much better than 
> hiddev, so ideally, I'd like to remove it some way.

First, I'd love to get rid of HID_QUIRK_NO_INIT_REPORTS; I am 
whole-heartedly convinced that initializing reports was a big mistake to 
start with, but changing history is hard :)

Regarding hiddev -- there actually are 'legitimate' non-debug users; for 
example NUT (UPS management software) has been using it last time I 
looked. I am pretty sure there are more.

So we'd rather be careful not to break hiddev by default.

I'll think about your RFC a little bit more.

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
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: gadget: uvc: webcam gadget USB PID is using value from a different gadget

2017-01-23 Thread Greg KH
On Mon, Jan 23, 2017 at 12:47:31PM +0100, Petr Cvek wrote:
> Dne 23.1.2017 v 12:32 Greg KH napsal(a):
>  I know it is only a cosmetic change on a legacy driver, but I assume
>  it would be better to have some default value for configfs API than to
>  borrow a PID from a whole different gadget.
> >>>
> >>> For class devices, they really don't need a new id, we should just use
> >>> only one of them as it doesn't matter :)
> >>
> 
> So using 0106 "Composite gadget: ACM + Mass Storage" or 0104
> "Multifunction Composite Gadget" should be fine? (my actual setup is
> not multifunction though).
> 
> I'm actually trying to catch bug, when using configfs access webcam
> does not work or kernel oopses (and webcam does not work) :-D.

The product id should never be an issue for a bug, if so, then something
a lot worse is going on.

> >> fine by me. Just lsusb will look funky ;-)
> > 
> > Heh, true, but I thought lsusb would use a string if the device provided
> > it.  Haven't looked at that portion of the code in a very long time...
> > 
> 
> My lsusb shows separate strings (using usbutils from slackware64-current):
> 
> Bus 004 Device 003: ID 1d6b:0102 Linux Foundation EEM Gadget
> ...
>   idVendor   0x1d6b Linux Foundation
>   idProduct  0x0102 EEM Gadget
>   bcdDevice4.07
>   iManufacturer   1 Linux Foundation
>   iProduct2 Webcam gadget
> ...

Ah, I guess it doesn't, but who knows how old that version of usbutils
is, considering the last release I did was well over a year ago.  I
should do a new one one of these days...

Anyway, I'd like to not assign a product id to a chunk of code that is
going to be eventually deleted.  Felipe, what's the plan for the
"legacy" gadget code.  Is it ever going away?

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: [PATCHv15 3/3] usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY

2017-01-23 Thread Felipe Balbi

Hi,

Heikki Krogerus  writes:
> +static int wcove_typec_probe(struct platform_device *pdev)
> +{

[snip]

> + wcove->cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;

we have a slight problem here that affects users of this particular
driver. Well, more specifically, it affects Intel Joule.

Because of the way ASL was written and the way Intel's DRD mux works, we
don't have a state which means "don't route USB signals to either Host
or Peripheral". Because of that, when we plug the type-c cable either
XHCI or dwc3 will have noise coming into the IP.

If default mode ends up being peripheral we have two possible problems
here:

i) device-to-device cable assembly

this won't be a big problem because we will just negotiate who's
Sink and who's Source then change mux on one side.

ii) lack of disconnect IRQ on dwc3

Because of how ASL was written, when we unplug the cable, mux's
VBUS_VALID bit won't be cleared which means dwc3 won't generate
disconnect interrupt. This means that upon cable reconnect (!!)
we will run ->disconnect() callback. The result is that dwc3
will never runtime suspend.

If default mode ends up being host we're possibly going to end up with a
host-to-host cable assembly. Now this can cause 2 issues:

i) port config error

host-to-host is not a supported cable assembly. While we see
errors on dmesg, eventually type-c negotiation will happen and
nothing will actually break.

ii) DbC can start on the other end of the cable

Now this was really surprising to me. When testing this on Intel
Joule and plugging Intel Joule straight to my PC's roothub port,
I noticed Joule ended up being host and my Desktop () became
a peripheral enumerated by Joule. I can only assume this is DbC
somehow being started.

The only way to have Joule become a peripheral is to connect it
through an external hub to my PC. Odd, ain't it?

I'm not sure how to solve this problem apart from modifying BIOS :-( Any
ideas?

-- 
balbi


signature.asc
Description: PGP signature


Re: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?

2017-01-23 Thread Vincent Pelletier
Hello,

On Mon, 23 Jan 2017 14:00:40 +0200, Felipe Balbi
 wrote:
> it could be that we're ran out of IN endpoints. There's a maximum number
> to how many IN endpoints we can have enabled at one time and, currently,
> dwc3 is not enforcing that in any way (I'll get that sorted out for
> v4.12, v4.11 is already too late)

8OUT failing also caught my eye, which is the only OUT endpoint
failing. And it happens when ep_addr & 0x7 == 0. Could something
(hardware ?) be confused and handle this as a kind of EP0 ?

Then, there would be 3 patterns to my uneducated eye:
- 15IN and several downward would fail because of the dwc3 active IN
  endpoint limit you mention
- 8IN and 8OUT would fail for some aliasing-with-EP0 reason
- 1IN and 9IN may fail for a related reason (depending on where the IN
  endpoint limit exactly stands - 8 or 9 I guess - and whether EP0
  counts towards the limit)

BTW, I also checked what my protocol analyser says in this 30-endpoints
version, but the result is quite boring:
- all (failing) IN endpoints NAK
- the one failing OUT endpoint NAKs the first attempt, then follows the
  PING protocol (MAK'ing all PINGs until the 0.2s timeout I set in the
  host test app).
Addresses are properly transmitted (ie, it's not on bus level that EP8
would get aliased). I still could not find the time to rebuild my
machine's kernel on your xhci-cleanup branch, so I'm still on 4.8.11
from a bit outdated Debian sid package).

I also noticed your commit about dwc3 having problems when clearing
halt of an already non-halted endpoint (and reciprocally), and I do
clear halts in the device test program. I tried commenting that out in
my program and merging your testing/next branch in my working copy,
without any improvement.

Regards,
-- 
Vincent Pelletier


pgpvG5cslVx5L.pgp
Description: Signature digitale OpenPGP


Re: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget

2017-01-23 Thread Felipe Balbi

Hi,

Greg KH  writes:
>> >> fine by me. Just lsusb will look funky ;-)
>> > 
>> > Heh, true, but I thought lsusb would use a string if the device provided
>> > it.  Haven't looked at that portion of the code in a very long time...
>> > 
>> 
>> My lsusb shows separate strings (using usbutils from slackware64-current):
>> 
>> Bus 004 Device 003: ID 1d6b:0102 Linux Foundation EEM Gadget
>> ...
>>   idVendor   0x1d6b Linux Foundation
>>   idProduct  0x0102 EEM Gadget
>>   bcdDevice4.07
>>   iManufacturer   1 Linux Foundation
>>   iProduct2 Webcam gadget
>> ...
>
> Ah, I guess it doesn't, but who knows how old that version of usbutils
> is, considering the last release I did was well over a year ago.  I
> should do a new one one of these days...
>
> Anyway, I'd like to not assign a product id to a chunk of code that is
> going to be eventually deleted.  Felipe, what's the plan for the
> "legacy" gadget code.  Is it ever going away?

Well, I wasn't really planning on deleting them just stopped accepting
any new one. I wanted to avoid angry mobs complaining about not having a
g_mass_storage.ko anymore.

Personally, I don't feel strongly about the legacy gadget
drivers. They're not really needed anymore as everything they do can be
done with configfs already. Perhaps we could schedule their removal for
v5.0?

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH v3 2/3] dmaengine: cppi41: Add support of DA8xx to CPPI 4.1

2017-01-23 Thread Alexandre Bailon
On 01/19/2017 07:15 PM, Sergei Shtylyov wrote:
> On 01/19/2017 02:13 PM, Alexandre Bailon wrote:
>
> > The DA8xx has a CPPI 4.1 DMA controller.
> > This is add the glue layer required to make it work on DA8xx.
> >
> > Signed-off-by: Alexandre Bailon 
> > ---
> >  drivers/dma/Kconfig  |  6 +++---
> >  drivers/dma/cppi41.c | 24 
> >  2 files changed, 27 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> > index 0d6a96e..03ae2a1 100644
> > --- a/drivers/dma/Kconfig
> > +++ b/drivers/dma/Kconfig
> > @@ -514,12 +514,12 @@ config TIMB_DMA
> >Enable support for the Timberdale FPGA DMA engine.
> >
> >  config TI_CPPI41
> > -tristate "AM33xx CPPI41 DMA support"
> > -depends on ARCH_OMAP
> > +tristate "CPPI41 DMA support"
>
>Grr... CPPI 4.1, please.
>
> > +depends on (ARCH_OMAP || ARCH_DAVINCI_DA8XX)
> >  select DMA_ENGINE
> >  help
> >The Communications Port Programming Interface (CPPI) 4.1 DMA
> > engine
> > -  is currently used by the USB driver on AM335x platforms.
> > +  is currently used by the USB driver on AM335x and DA8xx platforms.
> >
> >  config TI_DMA_CROSSBAR
> >  bool
> > diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
> > index 9767b97..f6f2d84 100644
> > --- a/drivers/dma/cppi41.c
> > +++ b/drivers/dma/cppi41.c
> [...]
> > @@ -951,8 +965,18 @@ static const struct cppi_glue_infos
> > am335x_usb_infos = {
> >  .qmgr_num_pend = 5,
> >  };
> >
> > +static const struct cppi_glue_infos da8xx_usb_infos = {
> > +.isr = cppi41_irq,
>
>Isn't the ISR the same for all glues now?
True. I should remove isr callback.
>
> [...]
>
> 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: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?

2017-01-23 Thread Felipe Balbi

Hi,

Vincent Pelletier  writes:
> On Mon, 23 Jan 2017 14:00:40 +0200, Felipe Balbi
>  wrote:
>> it could be that we're ran out of IN endpoints. There's a maximum number
>> to how many IN endpoints we can have enabled at one time and, currently,
>> dwc3 is not enforcing that in any way (I'll get that sorted out for
>> v4.12, v4.11 is already too late)
>
> 8OUT failing also caught my eye, which is the only OUT endpoint
> failing. And it happens when ep_addr & 0x7 == 0. Could something
> (hardware ?) be confused and handle this as a kind of EP0 ?

hmmm, what does the following show?

# mkdir -p /d
# mount -t debugfs none /d
# grep HWPARAMS3 /d/*dwc3*/regdump

> Then, there would be 3 patterns to my uneducated eye:
> - 15IN and several downward would fail because of the dwc3 active IN
>   endpoint limit you mention
> - 8IN and 8OUT would fail for some aliasing-with-EP0 reason
> - 1IN and 9IN may fail for a related reason (depending on where the IN
>   endpoint limit exactly stands - 8 or 9 I guess - and whether EP0
>   counts towards the limit)

well, the IN limit can be lower than what HW reports depending on how
your TX FIFO space is setup.

> BTW, I also checked what my protocol analyser says in this 30-endpoints
> version, but the result is quite boring:
> - all (failing) IN endpoints NAK
> - the one failing OUT endpoint NAKs the first attempt, then follows the
>   PING protocol (MAK'ing all PINGs until the 0.2s timeout I set in the
>   host test app).
> Addresses are properly transmitted (ie, it's not on bus level that EP8
> would get aliased). I still could not find the time to rebuild my
> machine's kernel on your xhci-cleanup branch, so I'm still on 4.8.11
> from a bit outdated Debian sid package).
>
> I also noticed your commit about dwc3 having problems when clearing
> halt of an already non-halted endpoint (and reciprocally), and I do
> clear halts in the device test program. I tried commenting that out in
> my program and merging your testing/next branch in my working copy,
> without any improvement.

thorough testing, thanks :-)

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH 36/37] xhci: simplify how we store TDs in urb private data

2017-01-23 Thread Mathias Nyman

On 23.01.2017 14:15, Mathias Nyman wrote:

On 23.01.2017 13:57, David Laight wrote:

From: Mathias Nyman

Sent: 20 January 2017 14:47



Instead of storing a zero length array of td pointers, and then
allocate memory both for the td pointer array and the td's, just
use a zero length array of actual td's in urb private data.


This reminds me of an old patch that got reverted because things
broke because the lifetimes/accesses of the data wasn't the obvious one.



Can you recall more details about that case? I'd hate to revert this
later.



Thanks for the (off list) info.

You submitted a similar patch in 2013, but it caused a regression and was not 
applied:

https://www.spinics.net/lists/linux-usb/msg99748.html

Reason it regressed back then was because we handled stalled endpoints a bit 
differently.
We saved a pointer to the current TD of a stalled endpoint, then gave back the
URB, and freed urb_priv, but _not_ the TD that urb_priv->td[x] pointed to.

This TD was later used in the endpoint reset callback to figure out where we 
should
set the dequeue pointer of that ring.

Handling stalled endpoints has changed since then. We don't store the TD 
pointer in
STALL cases, and we free the urb_priv completely when giving back the URB

see:

commit 8e71a322fdb127814bcba423a512914ca5bc6cf5
USB: xhci: Reset a halted endpoint immediately when we encounter a stall.

So now it should work

-Mathias


--
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: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?

2017-01-23 Thread Vincent Pelletier
On Mon, 23 Jan 2017 16:30:57 +0200, Felipe Balbi
 wrote:
> hmmm, what does the following show?
> 
> # mkdir -p /d
> # mount -t debugfs none /d
> # grep HWPARAMS3 /d/*dwc3*/regdump

# grep HWPARAMS3 /sys/kernel/debug/*dwc3*/regdump
GHWPARAMS3 = 0x10420089

Regards,
-- 
Vincent Pelletier


pgp9ZJv9GNzfE.pgp
Description: Signature digitale OpenPGP


usb: gadget: f_fs: Accept up to 30 endpoints.

2017-01-23 Thread Vincent Pelletier
It is allowed by the USB specification to enabled same-address, opposite-
direction endpoints simultaneously, which means 30 non-zero endpoints
are allowed. So double eps_addrmap length to 30.
The original code only accepted 14 descriptors out of a likely intended 15
(as there are 15 endpoint addresses, ignoring direction), because the first
eps_addrmap entry is unused (it is a placeholder for endpoint zero). So
increase eps_addrmap length by one to 31.

Signed-off-by: Vincent Pelletier 
---
 drivers/usb/gadget/function/f_fs.c | 2 +-
 drivers/usb/gadget/function/u_fs.h | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 5c91a6f4613b..c4364c87 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -2101,7 +2101,7 @@ static int __ffs_data_do_entity(enum ffs_entity_type type,
case FFS_ENDPOINT:
d = (void *)desc;
helper->eps_count++;
-   if (helper->eps_count >= 15)
+   if (helper->eps_count >= FFS_MAX_EPS_COUNT)
return -EINVAL;
/* Check if descriptors for any speed were already parsed */
if (!helper->ffs->eps_count && !helper->ffs->interfaces_count)
diff --git a/drivers/usb/gadget/function/u_fs.h 
b/drivers/usb/gadget/function/u_fs.h
index 60139854e0b1..4b6969451cdc 100644
--- a/drivers/usb/gadget/function/u_fs.h
+++ b/drivers/usb/gadget/function/u_fs.h
@@ -247,7 +247,8 @@ struct ffs_data {
 
unsigneduser_flags;
 
-   u8  eps_addrmap[15];
+#define FFS_MAX_EPS_COUNT 31
+   u8  eps_addrmap[FFS_MAX_EPS_COUNT];
 
unsigned short  strings_count;
unsigned short  interfaces_count;
-- 
2.11.0

--
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: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?

2017-01-23 Thread Vincent Pelletier
Changes since try 1:
- impove commit message a bit
- #declare array size to avoid duplication

--
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 linux-next v2 1/1] usb: gadget: udc: atmel: Update endpoint allocation scheme

2017-01-23 Thread cristian.birsan
From: Cristian Birsan 

Update atmel udc driver with a new enpoint allocation scheme. The data
sheet requires that all endpoints are allocated in order.

Signed-off-by: Cristian Birsan 
---
 drivers/usb/gadget/udc/Kconfig  |  14 ++
 drivers/usb/gadget/udc/atmel_usba_udc.c | 236 +++-
 drivers/usb/gadget/udc/atmel_usba_udc.h |  10 +-
 3 files changed, 227 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index 658b8da..4b69f28 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -60,6 +60,20 @@ config USB_ATMEL_USBA
  USBA is the integrated high-speed USB Device controller on
  the AT32AP700x, some AT91SAM9 and AT91CAP9 processors from Atmel.
 
+ The fifo_mode parameter is used to select endpoint allocation mode.
+ fifo_mode = 0 is used to let the driver autoconfigure the endpoints.
+ In this case 2 banks are allocated for isochronous endpoints and
+ only one bank is allocated for the rest of the endpoints.
+
+ fifo_mode = 1 is a generic maximum fifo size (1024 bytes) 
configuration
+ allowing the usage of ep1 - ep6
+
+ fifo_mode = 2 is a generic performance maximum fifo size (1024 bytes)
+ configuration allowing the usage of ep1 - ep3
+
+ fifo_mode = 3 is a balanced performance configuration allowing the
+ the usage of ep1 - ep8
+
 config USB_BCM63XX_UDC
tristate "Broadcom BCM63xx Peripheral Controller"
depends on BCM63XX
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 12c7687..11bbce2 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -318,6 +319,91 @@ static inline void usba_cleanup_debugfs(struct usba_udc 
*udc)
 }
 #endif
 
+static ushort fifo_mode;
+
+/* "modprobe ... fifo_mode=1" etc */
+module_param(fifo_mode, ushort, 0x0);
+MODULE_PARM_DESC(fifo_mode, "Endpoint configuration mode");
+
+/* mode 0 - uses autoconfig */
+
+/* mode 1 - fits in 8KB, generic max fifo configuration */
+static struct usba_fifo_cfg mode_1_cfg[] = {
+{ .hw_ep_num = 0, .fifo_size = 64, .nr_banks = 1, },
+{ .hw_ep_num = 1, .fifo_size = 1024,   .nr_banks = 2, },
+{ .hw_ep_num = 2, .fifo_size = 1024,   .nr_banks = 1, },
+{ .hw_ep_num = 3, .fifo_size = 1024,   .nr_banks = 1, },
+{ .hw_ep_num = 4, .fifo_size = 1024,   .nr_banks = 1, },
+{ .hw_ep_num = 5, .fifo_size = 1024,   .nr_banks = 1, },
+{ .hw_ep_num = 6, .fifo_size = 1024,   .nr_banks = 1, },
+};
+
+/* mode 2 - fits in 8KB, performance max fifo configuration */
+static struct usba_fifo_cfg mode_2_cfg[] = {
+{ .hw_ep_num = 0, .fifo_size = 64, .nr_banks = 1, },
+{ .hw_ep_num = 1, .fifo_size = 1024,   .nr_banks = 3, },
+{ .hw_ep_num = 2, .fifo_size = 1024,   .nr_banks = 2, },
+{ .hw_ep_num = 3, .fifo_size = 1024,   .nr_banks = 2, },
+};
+
+/* mode 3 - fits in 8KB, mixed fifo configuration */
+static struct usba_fifo_cfg mode_3_cfg[] = {
+{ .hw_ep_num = 0, .fifo_size = 64, .nr_banks = 1, },
+{ .hw_ep_num = 1, .fifo_size = 1024,   .nr_banks = 2, },
+{ .hw_ep_num = 2, .fifo_size = 512,.nr_banks = 2, },
+{ .hw_ep_num = 3, .fifo_size = 512,.nr_banks = 2, },
+{ .hw_ep_num = 4, .fifo_size = 512,.nr_banks = 2, },
+{ .hw_ep_num = 5, .fifo_size = 512,.nr_banks = 2, },
+{ .hw_ep_num = 6, .fifo_size = 512,.nr_banks = 2, },
+};
+
+/* mode 4 - fits in 8KB, custom fifo configuration */
+static struct usba_fifo_cfg mode_4_cfg[] = {
+{ .hw_ep_num = 0, .fifo_size = 64, .nr_banks = 1, },
+{ .hw_ep_num = 1, .fifo_size = 512,.nr_banks = 2, },
+{ .hw_ep_num = 2, .fifo_size = 512,.nr_banks = 2, },
+{ .hw_ep_num = 3, .fifo_size = 8,  .nr_banks = 2, },
+{ .hw_ep_num = 4, .fifo_size = 512,.nr_banks = 2, },
+{ .hw_ep_num = 5, .fifo_size = 512,.nr_banks = 2, },
+{ .hw_ep_num = 6, .fifo_size = 16, .nr_banks = 2, },
+{ .hw_ep_num = 7, .fifo_size = 8,  .nr_banks = 2, },
+{ .hw_ep_num = 8, .fifo_size = 8,  .nr_banks = 2, },
+};
+/* Add additional configurations here */
+
+int usba_config_fifo_table(struct usba_udc *udc)
+{
+   int n;
+
+   switch (fifo_mode) {
+   default:
+   fifo_mode = 0;
+   case 0:
+   udc->fifo_cfg = NULL;
+   n = 0;
+   break;
+   case 1:
+   udc->fifo_cfg = mode_1_cfg;
+   n = ARRAY_SIZE(mode_1_cfg);
+   break;
+   case 2:
+   udc->fifo_cfg = mode_2_cfg;
+   n = ARRAY_SIZE(mode_2_cfg);
+   break;
+   case 3:
+   udc->fifo_cfg = mode_3_cfg;
+   n = ARRAY_SIZE(mode_3_cfg);
+   break;
+   case 4:
+   udc->fifo_cfg = mode_4_cfg;
+   n = ARRAY_SIZE(mode_4_cfg);
+   break;
+ 

Re: [PATCH v5] USB: Add uPD78F0730 USB to Serial Adaptor Driver

2017-01-23 Thread Johan Hovold
On Sun, Jan 22, 2017 at 12:32:16AM +0300, Maksim Salau wrote:
> The adaptor can be found on development boards for 78k, RL78 and V850
> microcontrollers produced by Renesas Electronics Corporation.
> 
> This is not a full-featured USB to serial converter, however it allows
> basic communication and simple control which is enough for programming of
> on-board flash and debugging through a debug monitor.
> 
> uPD78F0730 is a USB-enabled microcontroller with USB-to-UART conversion
> implemented in firmware.
> 
> This chip is also present in some debugging adaptors which use it for
> USB-to-SPI conversion as well. The present driver doesn't cover SPI,
> only USB-to-UART conversion is supported.
> 
> Signed-off-by: Maksim Salau 
> ---
> Changes in v5:
>   Fixed a typo in assignment of opcode of the SET_DTR_RTS request
> 
> Changes in v4:
>   Addressed comments from Johan

You made some further changes than what I suggested but forgot to
document those. Often better to explicitly list the changes made rather
than refer to review comments this way.

This already looks really good; just a few minor things noted below.

>  drivers/usb/serial/Kconfig  |   9 +
>  drivers/usb/serial/Makefile |   1 +
>  drivers/usb/serial/upd78f0730.c | 458 
> 
>  3 files changed, 468 insertions(+)
>  create mode 100644 drivers/usb/serial/upd78f0730.c

> +static int upd78f0730_send_ctl(struct usb_serial_port *port,
> + void *data, int size)
> +{
> + struct device *dev = &port->dev;
> + struct usb_device *usbdev = port->serial->dev;
> + void *buf;
> + int res;
> +
> + if (!size || !data)
> + return -EINVAL;
> +
> + buf = kmemdup(data, size, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + res = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x00,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> + 0x, 0x, buf, size, USB_CTRL_SET_TIMEOUT);
> +
> + kfree(buf);
> +
> + if (res < 0)
> + return res;
> +
> + if (res != size) {
> + dev_err(dev, "%s - send failed: opcode=%02x, size=%d, res=%d\n",
> + __func__, *(u8 *)data, size, res);

You still want an error message in case res < 0 (as in v3), but you can
return that errno instead of -EIO then.

You can drop the __func__ prefixes you added to error messages in v5
throughout, better to spell out what went wrong (e.g. "failed to send
control request %02x: %d\n", *(u8 *)data, res).

> + /* The maximum expected length of a transfer is 6 bytes */
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int upd78f0730_attach(struct usb_serial *serial)
> +{
> + const char num_ports = serial->num_ports;
> +
> + if (serial->num_bulk_in < num_ports ||
> + serial->num_bulk_out < num_ports) {
> + dev_err(&serial->interface->dev, "%s - missing endpoints\n",
> + __func__);
> + return -ENODEV;
> + }
> +
> + return 0;
> +}

This is new in v5, and should have been mentioned in the changelog above.

Note that this is not strictly necessary, though. Some drivers
dereferenced pointers without the appropriate sanity checks, but this
driver and the generic callbacks it uses, does not suffer from such
problems.

I suggest you just drop this check (i.e. the attach callback).

> +static int upd78f0730_tiocmget(struct tty_struct *tty)
> +{
> + struct device *dev = tty->dev;
> + struct upd78f0730_port_private *private;
> + struct usb_serial_port *port = tty->driver_data;
> + int signals;
> + int res;
> +
> + private = usb_get_serial_port_data(port);
> +
> + mutex_lock(&private->lock);
> + signals = private->line_signals;
> + mutex_unlock(&private->lock);
> +
> + res = ((signals & UPD78F0730_DTR) ? TIOCM_DTR : 0) |
> + ((signals & UPD78F0730_RTS) ? TIOCM_RTS : 0);
> +
> + dev_dbg(dev, "%s - res = %x\n", __func__, res);

Note that __func__ is fine for concise debug messages like this one
(that users generally won't see).

But please drop it from all ERR and WARN messages.

> +
> + return res;
> +}

Thanks,
Johan
--
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: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?

2017-01-23 Thread Felipe Balbi

Hi,

Vincent Pelletier  writes:
> On Mon, 23 Jan 2017 16:30:57 +0200, Felipe Balbi
>  wrote:
>> hmmm, what does the following show?
>> 
>> # mkdir -p /d
>> # mount -t debugfs none /d
>> # grep HWPARAMS3 /d/*dwc3*/regdump
>
> # grep HWPARAMS3 /sys/kernel/debug/*dwc3*/regdump
> GHWPARAMS3 = 0x10420089

odd, this board was configured with support for all 32 endpoints. You
should not be running out of FIFO space and you can have all 16 IN
endpoints enabled at the same time. This all shouldn't matter for you.

Unless total TX FIFO size isn't large enough for all 16 endpoints, which
I kinda doubt.

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCHv15 2/3] usb: USB Type-C connector class

2017-01-23 Thread Felipe Balbi

Hi,

Heikki Krogerus  writes:
> +static void typec_report_identity(struct device *dev)
> +{
> + sysfs_notify(&dev->kobj, "identity", "id_header");
> + sysfs_notify(&dev->kobj, "identity", "cert_stat");
> + sysfs_notify(&dev->kobj, "identity", "product");

if you sysfs_notify() all three everytime this might cause issues for
userspace pollers. What will happen is that you're gonna change
e.g. id_header and threads polling id_header, cert_stat and product will
be notified of what's supposed to be new data. Maybe this should know
what it's notifying and only notify what actually changed. Seems like
just passing one extra char * argument is enough:

static void typec_report_identity(struct device *dev, const char *prop)
{
sysfs_notify(&dev->kobj, "identity", prop);
}

> +static ssize_t power_role_show(struct device *dev,
> +struct device_attribute *attr, char *buf)
> +{
> + struct typec_port *port = to_typec_port(dev);
> +
> + if (port->cap->type == TYPEC_PORT_DRP)
> + return sprintf(buf, "%s\n", port->pwr_role == TYPEC_SOURCE ?
> +"[source] sink" : "source [sink]");
> +
> + return sprintf(buf, "[%s]\n", typec_roles[port->pwr_role]);
> +}
> +static DEVICE_ATTR_RW(power_role);
> +
> +static const char * const typec_pwr_opmodes[] = {
> + [TYPEC_PWR_MODE_USB]= "default",
> + [TYPEC_PWR_MODE_1_5A]   = "1.5A",
> + [TYPEC_PWR_MODE_3_0A]   = "3.0A",
> + [TYPEC_PWR_MODE_PD] = "usb_power_delivery",

do you need this to have underscores? How about "USB Power Delivery",
instead? Userland could use the string untreated as part of GUI.

> +static ssize_t vconn_source_store(struct device *dev,
> +   struct device_attribute *attr,
> +   const char *buf, size_t size)
> +{
> + struct typec_port *port = to_typec_port(dev);
> + enum typec_role role;
> + int ret;
> +
> + if (!port->cap->pd_revision) {
> + dev_dbg(dev, "VCONN swap depends on USB Power Delivery\n");
> + return -EOPNOTSUPP;
> + }
> +
> + if (!port->cap->vconn_set) {
> + dev_dbg(dev, "VCONN swapping not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + if (sysfs_streq(buf, "1"))
> + role = TYPEC_SOURCE;
> + else if (sysfs_streq(buf, "0"))
> + role = TYPEC_SINK;
> + else
> + return -EINVAL;

seems to me this could be stringified (both store and show) to "source"
and "sink".

> +
> + ret = port->cap->vconn_set(port->cap, role);
> + if (ret)
> + return ret;
> +
> + return size;
> +}
> +
> +static ssize_t vconn_source_show(struct device *dev,
> +  struct device_attribute *attr, char *buf)
> +{
> + struct typec_port *port = to_typec_port(dev);
> +
> + return sprintf(buf, "%d\n", port->vconn_role == TYPEC_SOURCE ? 1 : 0);
> +}
> +static DEVICE_ATTR_RW(vconn_source);
> +
> +static ssize_t supported_accessory_modes_show(struct device *dev,
> +   struct device_attribute *attr,
> +   char *buf)
> +{
> + struct typec_port *port = to_typec_port(dev);
> + ssize_t ret = 0;
> + int i;
> +
> + if (!port->cap->accessory)
> + return 0;
> +
> + for (i = 0; port->cap->accessory[i]; i++)
> + ret += sprintf(buf + ret, "%s ",
> +typec_accessory_modes[port->cap->accessory[i]]);

possible buffer overflow here?

-- 
balbi


signature.asc
Description: PGP signature


[PATCH linux-next v2 0/1] usb: gadget: udc: atmel: Update endpoint allocation scheme

2017-01-23 Thread cristian.birsan
From: Cristian Birsan 

Hi,

This patch updates the usb endpoint allocation scheme for atmel usba
driver to make sure all endpoints are allocated in order. This requirement
comes from the datasheet of the controller.

The allocation scheme is decided by fifo_mode parameter. For fifo_mode = 0
the driver tries to autoconfigure the endpoints fifo size. All other modes
contain fixed configurations optimized for different purposes. The idea is
somehow similar with the approach used on musb driver.

Please let me know if you have any comments or suggestions.

Changes since v1:
- Minor reworks based on received fedback


Kind regards,
Cristian

Cristian Birsan (1):
  usb: gadget: udc: atmel: Update endpoint allocation scheme

 drivers/usb/gadget/udc/Kconfig  |  14 ++
 drivers/usb/gadget/udc/atmel_usba_udc.c | 236 +++-
 drivers/usb/gadget/udc/atmel_usba_udc.h |  10 +-
 3 files changed, 227 insertions(+), 33 deletions(-)

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


  1   2   >