Re: USB hot-plug not working (ASUS TP301UA-C4028T)

2016-10-14 Thread Pierre de Villemereuil 
Hi!

I've managed to build the patched kernel on OBS, install the package 
and boot! \o/

And I can confirm that the patch indeed fix the issue!

The kernel was booted without the usb.autosuspend=-1 option, of 
course.

Thank you guys!

Cheers,
Pierre.

Le jeudi 13 octobre 2016, 17:11:46 NZDT Alan Stern a écrit :
> On Fri, 14 Oct 2016, Pierre de Villemereuil wrote:
> > Hi!
> > 
> > I've branched the kernel package on the OpenSUSE Build Server, 
I'll
> > try to apply the patch there (this ought to be cleanest method).
> > 
> > Starting from the root of the kernel tarball, the patch should 
be
> > applied to drivers/pci/pci.c, am I right?
> 
> You just cd to the top directory of the kernel source, and do
> 
>   patch -p1  
> where "filename" is the contents of the email message containing 
the
> patch.  Or if you want, since the patch is so small, you can 
simply
> edit the file by hand to add in the new lines.
> 
> Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Problem with OHCI controller dying in 4.8

2016-10-14 Thread Benjamin Staton 
Alan,

I apologize, but git itself is still over my head, and I don't know
how to get bisect.  I download the kernel tar.xz files and tell
Debian's make-kpkg utility to compile them for me.

But for what it's worth, the patch from Bryan Paluch that changes
IO_WATCHDOG_DELAY in drivers/usb/host/ohci-hcd.c from 250 to 275 msecs
either fixed or masked the problem (no problem scanning with the patch
applied).  This would perhaps lend support to his theory about timing
changes having an unintended effect on the ohci drivers.

Quoting Mr. Paluch:
>> I suspect that the new timer wheel based timers in 4.8 have exposed a bug
>> in the ohci driver. Before the io watchdog timer was set to 250 with 20 msecs
>> of slack time. This is more than likely just a hacky work around [...]

Sure enough, I hacked, and it worked around.

Thanks,

Joe

On Fri, Oct 14, 2016 at 5:11 PM, Alan Stern  wrote:
> On Fri, 14 Oct 2016, Benjamin Staton wrote:
>
>> The Issue: Scanning (with xsane 0.999) while using kernel 4.8 (I
>> tested 4.8 rc1, rc5, rc7, rc8, and 4.8.1) fails when the scanner
>> becomes disconnected from the USB bus.  Every time.
>>
>> Scanning (with xsane 0.999) while using kernel 4.7.0 or earlier 4's
>> works without fail.
>>
>> My scanner is known to lsusb as "Bus 006 Device 002: ID 04a9:2206
>> Canon, Inc. CanoScan N650U/N656U" and to Xsane as "Canon CanoScan
>> N650U/N flatbed scanner [plustek:libusb:006:002]".
>>
>> Relevant lines from /var/log/kern.log at the time of the disconnect:
>> Oct 14 13:30:00 quiz kernel: [  167.746817] ohci-pci :00:12.0:
>> HcDoneHead not written back; disabled
>> Oct 14 13:30:00 quiz kernel: [  167.746829] ohci-pci :00:12.0: HC
>> died; cleaning up
>> Oct 14 13:30:00 quiz kernel: [  167.746916] usb 6-1: USB disconnect,
>> device number 2
>> Oct 14 13:30:00 quiz kernel: [  167.747618] usb 6-3: USB disconnect,
>> device number 3
>> Oct 14 13:30:00 quiz kernel: [  167.747624] usb 6-3.2: USB disconnect,
>> device number 4
>> Oct 14 13:30:00 quiz kernel: [  167.803283] usb 6-3.3: USB disconnect,
>> device number 5
>>
>> This is on an asrock 970M Pro3 motherboard w/AMD FX-8300 CPU and 32GB
>> RAM running debian testing/9.  Kernels built from kernel.org without
>> further patching.
>>
>> Apologies for posting to wrong place (bugzilla) and happy to provide more 
>> info.
>
> Can you run git bisect between 4.7 and 4.8-rc1 to find the particular
> commit that introduced this problem?  There haven't been any
> significant changes to the ohci-hcd driver during that period.
>
> Alan Stern
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re:

2016-10-14 Thread Benjamin Staton 
Brian,

Working - I appreciate this.  I applied this mini-patch to kernel
4.8.1 and confirm that after a few test scans, I now have no problem
with the USB Canon scanner.

Also seems to have cleared up a puzzling no-input problem I was having
with a cheap no-brand USB keyboard/video/mouse switch under 4.8.1.

Thanks,

Joe

On Fri, Oct 14, 2016 at 3:52 PM, Bryan Paluch  wrote:
> I've been experiencing similar issues with usb audio headsets. There is an
> issue for ubuntu 16.10 here
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1630063 .
>
> I was able to fix the issue by changing the IO_WATCHDOG_DELAY in ohci-hcd.c
> from 250 msec to 275 msec. I suspect that the new timer wheel based timers
> in 4.8 have exposed a bug in the ohci driver. Before the io watchdog timer
> was set to 250 with 20 msecs of slack time. This is more than likely just a
> hacky work around but I'm able to use my usb headphones now. Here's my diff
>
> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
> index 1700908..86612ac 100644
> --- a/drivers/usb/host/ohci-hcd.c
> +++ b/drivers/usb/host/ohci-hcd.c
> @@ -72,7 +72,7 @@
>  static const char hcd_name [] = "ohci_hcd";
>
>  #define STATECHANGE_DELAY msecs_to_jiffies(300)
> -#define IO_WATCHDOG_DELAY msecs_to_jiffies(250)
> +#define IO_WATCHDOG_DELAY msecs_to_jiffies(275)
>
>  #include "ohci.h"
>  #include "pci-quirks.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: Problem with OHCI controller dying in 4.8

2016-10-14 Thread Alan Stern 
On Fri, 14 Oct 2016, Benjamin Staton wrote:

> The Issue: Scanning (with xsane 0.999) while using kernel 4.8 (I
> tested 4.8 rc1, rc5, rc7, rc8, and 4.8.1) fails when the scanner
> becomes disconnected from the USB bus.  Every time.
> 
> Scanning (with xsane 0.999) while using kernel 4.7.0 or earlier 4's
> works without fail.
> 
> My scanner is known to lsusb as "Bus 006 Device 002: ID 04a9:2206
> Canon, Inc. CanoScan N650U/N656U" and to Xsane as "Canon CanoScan
> N650U/N flatbed scanner [plustek:libusb:006:002]".
> 
> Relevant lines from /var/log/kern.log at the time of the disconnect:
> Oct 14 13:30:00 quiz kernel: [  167.746817] ohci-pci :00:12.0:
> HcDoneHead not written back; disabled
> Oct 14 13:30:00 quiz kernel: [  167.746829] ohci-pci :00:12.0: HC
> died; cleaning up
> Oct 14 13:30:00 quiz kernel: [  167.746916] usb 6-1: USB disconnect,
> device number 2
> Oct 14 13:30:00 quiz kernel: [  167.747618] usb 6-3: USB disconnect,
> device number 3
> Oct 14 13:30:00 quiz kernel: [  167.747624] usb 6-3.2: USB disconnect,
> device number 4
> Oct 14 13:30:00 quiz kernel: [  167.803283] usb 6-3.3: USB disconnect,
> device number 5
> 
> This is on an asrock 970M Pro3 motherboard w/AMD FX-8300 CPU and 32GB
> RAM running debian testing/9.  Kernels built from kernel.org without
> further patching.
> 
> Apologies for posting to wrong place (bugzilla) and happy to provide more 
> info.

Can you run git bisect between 4.7 and 4.8-rc1 to find the particular 
commit that introduced this problem?  There haven't been any 
significant changes to the ohci-hcd driver during that period.

Alan Stern

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

[PATCH 50/57] [media] zr364xx: don't break long lines

2016-10-14 Thread Mauro Carvalho Chehab 
Due to the 80-cols checkpatch warnings, several strings
were broken into multiple lines. This is not considered
a good practice anymore, as it makes harder to grep for
strings at the source code. So, join those continuation
lines.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/zr364xx/zr364xx.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/zr364xx/zr364xx.c 
b/drivers/media/usb/zr364xx/zr364xx.c
index cc128db85723..3950708cbb32 100644
--- a/drivers/media/usb/zr364xx/zr364xx.c
+++ b/drivers/media/usb/zr364xx/zr364xx.c
@@ -633,8 +633,7 @@ static int zr364xx_read_video_callback(struct 
zr364xx_camera *cam,
} else {
if (frm->cur_size + purb->actual_length > MAX_FRAME_SIZE) {
dev_info(&cam->udev->dev,
-"%s: buffer (%d bytes) too small to hold "
-"frame data. Discarding frame data.\n",
+"%s: buffer (%d bytes) too small to hold frame 
data. Discarding frame data.\n",
 __func__, MAX_FRAME_SIZE);
} else {
pdest += frm->cur_size;
@@ -1373,8 +1372,7 @@ static int zr364xx_board_init(struct zr364xx_camera *cam)
&cam->buffer.frame[i], i,
cam->buffer.frame[i].lpvbits);
if (cam->buffer.frame[i].lpvbits == NULL) {
-   printk(KERN_INFO KBUILD_MODNAME ": out of memory. "
-  "Using less frames\n");
+   printk(KERN_INFO KBUILD_MODNAME ": out of memory. Using 
less frames\n");
break;
}
}
-- 
2.7.4


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

Re: [PATCH] net: asix: Avoid looping when the device does not respond

2016-10-14 Thread David Miller 
From: Guenter Roeck 
Date: Thu, 13 Oct 2016 16:43:16 -0700

> Check answers from USB stack and avoid re-sending the request
> multiple times if the device does not respond.
> 
> This fixes the following problem, observed with a probably flaky adapter.
 ...
> Since the USB timeout is 5 seconds, and the operation is retried 30 times,
> this results in
 ...
> Signed-off-by: Guenter Roeck 

Applied, 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: Problem with musb dma packet

2016-10-14 Thread Bin Liu 
Hi,

On Thu, Oct 06, 2016 at 02:45:30PM +, Andrew Goodbody wrote:
> I am trying to investigate an issue on a TI Sitara CPU, AM3352 with
> the musb USB controller.
> 
> The scenario is that a device has been in use and working correctly.
> The device is an Android device and is presenting as an MTP device.

AM3352 musb is the host or device? Sounds like it is the host.

> That first session of use is finished and the port is reset but the
> device is not unplugged, so enumeration and configuration starts again

How this happened? How do you control the host reset the device but not
re-enumerate it?

> the next time it is needed. The transfers over EP0 using PIO all
> proceed as expected. The problem is the first bulk packet sent to EP1,
> which also happens to be the first dma packet. USBMON shows this
> packet as being sent. However a hardware analyser does not show this
> packet on the wire and of course there is no ACK either. Looking at
> the debug info from the musb driver I can see the dma being started
> and then the callback being called. MUSB_TXCSR_TXPKTRDY remains set as
> does MUSB_TXCSR_TXFIFONOTEMPTY so nothing else happens. The TXCSR
> register is 0x3403. The code is waiting for MUSB_TXCSR_TXPKTRDY to be
> cleared but that will not happen until an ACK is received. It just
> seems that the controller is not putting the packet onto the wire for
> some reason.

Sounds to me that the cppi teamdown before/during bus reset was not
complete, so the channels are not in the ready state for next transfers.

I don't have a better way to debug it, but you have to ensure the
teardown sequence does follow that in the normal device detach. If the
issue still happens, you would have to dump the cppi registers to see
why the channels are in the wrong state.

> 
> This is on 4.5.1.
> 
> I would welcome some pointers on how to proceed to debug this or else
> any information on possible applicable errata and workarounds for
> this.

It is more like software problem to me.

Good luck.

Regards,
-Bin.

> 
> Thanks,
> Andrew
--
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][PATCH 2/2] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220

2016-10-14 Thread John Youn 
On 10/13/2016 4:36 PM, John Stultz wrote:
> From: Chen Yu 
> 
> The Hi6220's usb controller is limited in that it does not
> automatically autonegotiate the usb speed. Thus it requires a
> quirk so that we can manually negotiate the best usb speed for
> the attached device.

Hi,

Could you expand more on this by explaining what exactly is the
limitation and the workaround?

[snip]

> +/*
> + * HPRT0_SPD_HIGH_SPEED: high speed
> + * HPRT0_SPD_FULL_SPEED: full speed
> + */
> +static void dwc2_change_bus_speed(struct usb_hcd *hcd, int speed)
> +{
> + struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> +
> + if (hsotg->core_params->speed == speed)
> + return;
> +
> + hsotg->core_params->speed = speed;
> + queue_work(hsotg->wq_otg, &hsotg->wf_otg);
> +}
> +
> +static int dwc2_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
> +{
> + struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> +
> + if (!hsotg->change_speed_quirk)
> + return 1;
> +
> + hsotg->device_count++;

Why do you need to track the device count?

> + dev_info(hsotg->dev, "Device count is %u after alloc dev\n",
> + hsotg->device_count);
> +
> + return 1;
> +}
> +
> +static void dwc2_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
> +{
> + struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> +
> + if (!hsotg->change_speed_quirk)
> + return;
> +
> + if (hsotg->device_count)
> + hsotg->device_count--;
> +
> + dev_info(hsotg->dev, "Device count is %u after free dev\n",
> + hsotg->device_count);
> +
> + if (hsotg->device_count == 1 && udev->parent &&
> + udev->parent->speed > USB_SPEED_UNKNOWN &&
> + udev->parent->speed < USB_SPEED_HIGH) {
> + dev_info(hsotg->dev, "Set speed to default high-speed\n");
> + dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
> + }
> +}
> +
> +static int dwc2_reset_device(struct usb_hcd *hcd, struct usb_device *udev)
> +{
> + struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> +
> + if (!hsotg->change_speed_quirk)
> + return 0;
> +
> + if (udev->speed == USB_SPEED_HIGH) {
> + dev_info(hsotg->dev, "Set speed to high-speed\n");
> + dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
> + } else if (udev->speed == USB_SPEED_FULL
> + || udev->speed == USB_SPEED_LOW) {
> + dev_info(hsotg->dev, "Set speed to full-speed\n");
> + dwc2_change_bus_speed(hcd, HPRT0_SPD_FULL_SPEED);
> + }

It seems you are reinitializing the core every time a device is reset
and the udev->speed does not match the core_param speed. But how is
the udev->speed being set correctly if the hw cannot negotiate the
speed in the first place?

Also should it be for every device? What about if a device gets
plugged in behind a hub? I don't think you want to execute this code
in that case.

This should only affect devices plugged into the root hub, correct?
And the hsotg controller only has one root hub port. It seems things
could be simplified a bit.

Regards,
John
--
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

[no subject]

2016-10-14 Thread Benjamin Staton 
The Issue: Scanning (with xsane 0.999) while using kernel 4.8 (I
tested 4.8 rc1, rc5, rc7, rc8, and 4.8.1) fails when the scanner
becomes disconnected from the USB bus.  Every time.

Scanning (with xsane 0.999) while using kernel 4.7.0 or earlier 4's
works without fail.

My scanner is known to lsusb as "Bus 006 Device 002: ID 04a9:2206
Canon, Inc. CanoScan N650U/N656U" and to Xsane as "Canon CanoScan
N650U/N flatbed scanner [plustek:libusb:006:002]".

Relevant lines from /var/log/kern.log at the time of the disconnect:
Oct 14 13:30:00 quiz kernel: [  167.746817] ohci-pci :00:12.0:
HcDoneHead not written back; disabled
Oct 14 13:30:00 quiz kernel: [  167.746829] ohci-pci :00:12.0: HC
died; cleaning up
Oct 14 13:30:00 quiz kernel: [  167.746916] usb 6-1: USB disconnect,
device number 2
Oct 14 13:30:00 quiz kernel: [  167.747618] usb 6-3: USB disconnect,
device number 3
Oct 14 13:30:00 quiz kernel: [  167.747624] usb 6-3.2: USB disconnect,
device number 4
Oct 14 13:30:00 quiz kernel: [  167.803283] usb 6-3.3: USB disconnect,
device number 5

This is on an asrock 970M Pro3 motherboard w/AMD FX-8300 CPU and 32GB
RAM running debian testing/9.  Kernels built from kernel.org without
further patching.

Apologies for posting to wrong place (bugzilla) and happy to provide more info.

Thanks,
Joe
--
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 0/2] Fixes for two more musb regressions

2016-10-14 Thread Bin Liu 
Hi,

On Fri, Sep 30, 2016 at 11:10:08AM -0700, Tony Lindgren wrote:
> Hi all,
> 
> Looks like we have two more regressions caused by my attempts
> to make musb PM simpler. As it's very close to the merge window
> opening, these are against current Linux next.
> 
> Once reviewed, tested and merged to the mainline kernel we can
> request these to be included also into v4.8.y kernel. Maybe Bin
> can tag these as cc stable v4.8 already when applying.
> 
> Regards,
> 
> Tony
> 
> Tony Lindgren (2):
>   usb: musb: Fix hardirq-safe hardirq-unsafe lock order error
>   usb: musb: Call pm_runtime from musb_gadget_queue

Applied.

Regards,
-Bin.

> 
>  drivers/usb/musb/musb_gadget.c | 4 
>  drivers/usb/musb/omap2430.c| 7 ++-
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> -- 
> 2.9.3
--
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][PATCH 2/2] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220

2016-10-14 Thread John Stultz 
On Fri, Oct 14, 2016 at 8:00 AM, Rob Herring  wrote:
> On Thu, Oct 13, 2016 at 6:29 PM, John Stultz  wrote:
>> From: Chen Yu 
>>
>> The Hi6220's usb controller is limited in that it does not
>> automatically autonegotiate the usb speed. Thus it requires a
>> quirk so that we can manually negotiate the best usb speed for
>> the attached device.
>>
>> Cc: Wei Xu 
>> Cc: Guodong Xu 
>> Cc: Amit Pundir 
>> Cc: Rob Herring 
>> Cc: Mark Rutland 
>> Cc: John Youn 
>> Cc: Douglas Anderson 
>> Cc: Greg Kroah-Hartman 
>> Cc: linux-usb@vger.kernel.org
>> Signed-off-by: Chen Yu 
>> Signed-off-by: John Stultz 
>> ---
>>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi |  1 +
>>  drivers/usb/dwc2/core.h   |  7 +++
>>  drivers/usb/dwc2/hcd.c| 75 
>> +++
>>  drivers/usb/dwc2/platform.c   |  3 ++
>>  4 files changed, 86 insertions(+)
>>
[snip]
>> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
>> index 8e1728b..21c328b 100644
>> --- a/drivers/usb/dwc2/platform.c
>> +++ b/drivers/usb/dwc2/platform.c
>> @@ -585,6 +585,9 @@ static int dwc2_driver_probe(struct platform_device *dev)
>> dev_dbg(&dev->dev, "mapped PA %08lx to VA %p\n",
>> (unsigned long)res->start, hsotg->regs);
>>
>> +   hsotg->change_speed_quirk = device_property_read_bool(&dev->dev,
>> +   "hi6220,change_speed_quirk");
>
> Can't this be determined from the hi6220's compatible string?

Ah. Good suggestion! I'm moving the quirk field to the core_params.
Should avoid any dts or binding changes.

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

[RESEND PATCH] usb: dwc2: Add msleep for host-only

2016-10-14 Thread John Youn 
From: Heiko Stuebner 

Although a host-only controller should not have any associated delay,
some rockchip SOC platforms will not show the correct host-values of
registers until after a delay.

So add a 50 ms sleep when in host-only mode.

Signed-off-by: John Youn 
Signed-off-by: Heiko Stuebner 
---

Resend as the original did not seem to make it to the list.

Hi Felipe,

Can you queue this in your fixes tree when you get a chance. It fixes
a regression that got into 4.9-rc1.

Thanks,
John

 drivers/usb/dwc2/core.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index fa9b26b..4c0fa0b 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -463,9 +463,18 @@ static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)
  */
 void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg)
 {
+   bool ret;
+
switch (hsotg->dr_mode) {
case USB_DR_MODE_HOST:
-   dwc2_force_mode(hsotg, true);
+   ret = dwc2_force_mode(hsotg, true);
+   /*
+* NOTE: This is required for some rockchip soc based
+* platforms on their host-only dwc2.
+*/
+   if (!ret)
+   msleep(50);
+
break;
case USB_DR_MODE_PERIPHERAL:
dwc2_force_mode(hsotg, false);
-- 
2.10.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][PATCH 2/2] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220

2016-10-14 Thread Rob Herring 
On Thu, Oct 13, 2016 at 6:29 PM, John Stultz  wrote:
> From: Chen Yu 
>
> The Hi6220's usb controller is limited in that it does not
> automatically autonegotiate the usb speed. Thus it requires a
> quirk so that we can manually negotiate the best usb speed for
> the attached device.
>
> Cc: Wei Xu 
> Cc: Guodong Xu 
> Cc: Amit Pundir 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Cc: John Youn 
> Cc: Douglas Anderson 
> Cc: Greg Kroah-Hartman 
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Chen Yu 
> Signed-off-by: John Stultz 
> ---
>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi |  1 +
>  drivers/usb/dwc2/core.h   |  7 +++
>  drivers/usb/dwc2/hcd.c| 75 
> +++
>  drivers/usb/dwc2/platform.c   |  3 ++
>  4 files changed, 86 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi 
> b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> index 17839db..2c8f435 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> @@ -752,6 +752,7 @@
> g-np-tx-fifo-size = <128>;
> g-tx-fifo-size = <128 128 128 128 128 128>;
> interrupts = <0 77 0x4>;
> +   hi6220,change_speed_quirk;
> };
>
> mailbox: mailbox@f751 {
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 2a21a04..24fea73 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -823,6 +823,10 @@ struct dwc2_hregs_backup {
>   * @frame_list_sz:  Frame list size
>   * @desc_gen_cache: Kmem cache for generic descriptors
>   * @desc_hsisoc_cache:  Kmem cache for hs isochronous descriptors
> + * @change_speed_quirk: Change speed configuration to DWC2_SPEED_PARAM_FULL
> + *  while full&low speed device connect. And change speed
> + *  back to DWC2_SPEED_PARAM_HIGH while device is gone.
> + * @device_count:   Number of devices connect to root hub
>   *
>   * These are for peripheral mode:
>   *
> @@ -951,6 +955,9 @@ struct dwc2_hsotg {
> struct kmem_cache *desc_gen_cache;
> struct kmem_cache *desc_hsisoc_cache;
>
> +   int change_speed_quirk;
> +   unsigned int device_count;
> +
>  #ifdef DEBUG
> u32 frrem_samples;
> u64 frrem_accum;
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index b374e60..663033e 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -4868,6 +4868,75 @@ static void _dwc2_hcd_clear_tt_buffer_complete(struct 
> usb_hcd *hcd,
> spin_unlock_irqrestore(&hsotg->lock, flags);
>  }
>
> +/*
> + * HPRT0_SPD_HIGH_SPEED: high speed
> + * HPRT0_SPD_FULL_SPEED: full speed
> + */
> +static void dwc2_change_bus_speed(struct usb_hcd *hcd, int speed)
> +{
> +   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> +
> +   if (hsotg->core_params->speed == speed)
> +   return;
> +
> +   hsotg->core_params->speed = speed;
> +   queue_work(hsotg->wq_otg, &hsotg->wf_otg);
> +}
> +
> +static int dwc2_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
> +{
> +   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> +
> +   if (!hsotg->change_speed_quirk)
> +   return 1;
> +
> +   hsotg->device_count++;
> +   dev_info(hsotg->dev, "Device count is %u after alloc dev\n",
> +   hsotg->device_count);
> +
> +   return 1;
> +}
> +
> +static void dwc2_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
> +{
> +   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> +
> +   if (!hsotg->change_speed_quirk)
> +   return;
> +
> +   if (hsotg->device_count)
> +   hsotg->device_count--;
> +
> +   dev_info(hsotg->dev, "Device count is %u after free dev\n",
> +   hsotg->device_count);
> +
> +   if (hsotg->device_count == 1 && udev->parent &&
> +   udev->parent->speed > USB_SPEED_UNKNOWN &&
> +   udev->parent->speed < USB_SPEED_HIGH) {
> +   dev_info(hsotg->dev, "Set speed to default high-speed\n");
> +   dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
> +   }
> +}
> +
> +static int dwc2_reset_device(struct usb_hcd *hcd, struct usb_device *udev)
> +{
> +   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> +
> +   if (!hsotg->change_speed_quirk)
> +   return 0;
> +
> +   if (udev->speed == USB_SPEED_HIGH) {
> +   dev_info(hsotg->dev, "Set speed to high-speed\n");
> +   dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
> +   } else if (udev->speed == USB_SPEED_FULL
> +   || udev->speed == USB_SPEED_LOW) {
> +   dev_info(hsotg->dev, "Set speed to full-speed\n");
> +   dwc2_change_bus_speed(hcd, HPRT0_SPD_FULL_SPEED);
> +   }
> +
> +   return

Re: [PATCH v2] usb: dwc3: gadget: Wait for end transfer complete before free irq

2016-10-14 Thread Alan Stern 
On Fri, 14 Oct 2016, Felipe Balbi wrote:

> argh, we have nested spinlocks :-( Well, we shouldn't call
> usb_ep_disable() with locks held AFAICR. So the following should be
> enough:
> 
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 919d7d1b611c..2e9359c58eb9 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -732,8 +732,10 @@ static void reset_config(struct usb_composite_dev *cdev)
> DBG(cdev, "reset config\n");
>  
> list_for_each_entry(f, &cdev->config->functions, list) {
> +   spin_unlock_irq(&cdev->lock);
> if (f->disable)
> f->disable(f);
> +   spin_lock_irq(&cdev->lock);
>  
> bitmap_zero(f->endpoints, 32);
> }
> 
> Alan, do you remember if we have a requirement for not holding locks
> when calling usb_ep_disable()? I can't find Documentation about it, but
> history shows me that usb_ep_disable() was called without locks and IRQs
> enabled. Do you think we should update documentation about this?

I don't think there is any requirement for interrupts to be enabled 
when usb_ep_disable() runs.  At least, a quick check shows that both 
net2280 and dummy-hcd use spin_lock_irqsave() rather than spin_lock() 
in their disable routines.

Holding locks is a different story.  It should be okay for a gadget 
driver to hold one of its own locks when disabling an endpoint (which 
means that the gadget's disable routine shouldn't wait for a concurrent 
giveback to finish), but we might want to avoid holding a lock in the 
composite core.  Although even that might be okay -- I can't think of 
any reason why a udc driver would need to call back into the composite 
core while disabling an endpoint.  It should be a pretty self-contained 
operation.

Alan Stern

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

Re: All USB devices stop working. Their reconnect not helps

2016-10-14 Thread Greg KH 
On Fri, Oct 14, 2016 at 06:53:29PM +0500, Михаил Гаврилов wrote:
> [   16.685607] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [   16.893766] usb 2-5.4: Device not responding to setup address.
> [   17.098457] usb 2-5.4: device not accepting address 6, error -71
> [   17.162458] usb 1-9.1.6: new full-speed USB device number 8 using xhci_hcd
> [   17.242470] usb 1-9.1.6: New USB device found, idVendor=046d, 
> idProduct=c52b
> [   17.242476] usb 1-9.1.6: New USB device strings: Mfr=1, Product=2, 
> SerialNumber=0
> [   17.242479] usb 1-9.1.6: Product: USB Receiver
> [   17.242482] usb 1-9.1.6: Manufacturer: Logitech


Hm, not good.



> [   30.678097] usb 2-5.4: Device not responding to setup address.
> [   30.882771] usb 2-5.4: device not accepting address 7, error -71
> [   30.950961] usb 1-9.1.2: reset high-speed USB device number 5 using 
> xhci_hcd
> [   31.096011] option 1-9.1.2:1.0: GSM modem (1-port) converter detected
> [   31.096269] usb 1-9.1.2: GSM modem (1-port) converter now attached to 
> ttyUSB0
> [   31.096304] option 1-9.1.2:1.1: GSM modem (1-port) converter detected
> [   31.096474] usb 1-9.1.2: GSM modem (1-port) converter now attached to 
> ttyUSB1

Also odd...

> [   86.385506] usb 2-5.4: Device not responding to setup address.
> [   86.590238] usb 2-5.4: device not accepting address 12, error -71
> [   86.590798] usb 2-5-port4: unable to enumerate USB device
> [  101.516693] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  101.722125] usb 2-5.4: Device not responding to setup address.
> [  101.926702] usb 2-5.4: device not accepting address 16, error -71
> [  115.338361] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  115.546467] usb 2-5.4: Device not responding to setup address.
> [  115.751170] usb 2-5.4: device not accepting address 17, error -71
> [  129.185004] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  129.397080] usb 2-5.4: Device not responding to setup address.
> [  129.601697] usb 2-5.4: device not accepting address 18, error -71
> [  143.024578] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  143.231669] usb 2-5.4: Device not responding to setup address.
> [  143.436203] usb 2-5.4: device not accepting address 19, error -71
> [  143.436814] usb 2-5-port4: unable to enumerate USB device
> [  157.882154] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  158.090299] usb 2-5.4: Device not responding to setup address.
> [  158.294896] usb 2-5.4: device not accepting address 22, error -71
> [  171.710696] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  171.916643] usb 2-5.4: Device not responding to setup address.
> [  172.121397] usb 2-5.4: device not accepting address 23, error -71
> [  185.542558] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  185.751236] usb 2-5.4: Device not responding to setup address.
> [  185.955910] usb 2-5.4: device not accepting address 24, error -71
> [  199.376149] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  199.585824] usb 2-5.4: Device not responding to setup address.
> [  199.790405] usb 2-5.4: device not accepting address 25, error -71
> [  199.790982] usb 2-5-port4: unable to enumerate USB device
> [  215.279864] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  215.484730] usb 2-5.4: Device not responding to setup address.
> [  215.689305] usb 2-5.4: device not accepting address 30, error -71
> [  229.092273] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  229.303070] usb 2-5.4: Device not responding to setup address.
> [  229.507787] usb 2-5.4: device not accepting address 31, error -71
> [  243.006432] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  243.217684] usb 2-5.4: Device not responding to setup address.
> [  243.422307] usb 2-5.4: device not accepting address 32, error -71
> [  256.887746] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  257.100277] usb 2-5.4: Device not responding to setup address.
> [  257.304830] usb 2-5.4: device not accepting address 33, error -71
> [  257.305437] usb 2-5-port4: unable to enumerate USB device
> [  272.222579] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  272.430894] usb 2-5.4: Device not responding to setup address.
> [  272.635589] usb 2-5.4: device not accepting address 37, error -71
> [  286.017843] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  286.225508] usb 2-5.4: Device not responding to setup address.
> [  286.430078] usb 2-5.4: device not accepting address 38, error -71
> [  299.838156] xhci_hcd :00:14.0: Timeout while waiting for setup device 
> command
> [  300.044133] usb 2-5.4: Device not responding to setup address.
> [  300.2486

Re: [PATCH v8 0/8] power: add power sequence library

2016-10-14 Thread Rafael J. Wysocki 
On Friday, October 14, 2016 10:59:47 AM Peter Chen wrote:
> Hi all,
> 
> This is a follow-up for my last power sequence framework patch set [1].
> According to Rob Herring and Ulf Hansson's comments[2]. The kinds of
> power sequence instances will be added at postcore_initcall, the match
> criteria is compatible string first, if the compatible string is not
> matched between dts and library, it will try to use generic power sequence.
>
> The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> if only one power sequence instance is needed, for more power sequences
> are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub 
> driver).
> 
> In future, if there are special power sequence requirements, the special
> power sequence library can be created.
> 
> This patch set is tested on i.mx6 sabresx evk using a dts change, I use
> two hot-plug devices to simulate this use case, the related binding
> change is updated at patch [1/6], The udoo board changes were tested
> using my last power sequence patch set.[3]
> 
> Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
> need to power on itself before it can be found by ULPI bus.
> 
> [1] http://www.spinics.net/lists/linux-usb/msg142755.html
> [2] http://www.spinics.net/lists/linux-usb/msg143106.html
> [3] http://www.spinics.net/lists/linux-usb/msg142815.html
> 
> Changes for v8:
> - Allocate one extra pwrseq instance if pwrseq_get has succeed, it can avoid
>   preallocate instances problem which the number of instance is decided at
>   compile time, thanks for Heiko Stuebner's suggestion [Patch 2/8]
> - Delete pwrseq_compatible_sample.c which is the demo purpose to show 
> compatible
>   match method. [Patch 2/8]
> - Add Maciej S. Szmigiero's tested-by. [Patch 7/8]
> 
> Changes for v7:
> - Create kinds of power sequence instance at postcore_initcall, and match
>   the instance with node using compatible string, the beneit of this is
>   the host driver doesn't need to consider which pwrseq instance needs
>   to be used, and pwrseq core will match it, however, it eats some memories
>   if less power sequence instances are used. [Patch 2/8]
> - Add pwrseq_compatible_sample.c to test match pwrseq using device_id. [Patch 
> 2/8]
> - Fix the comments Vaibhav Hiremath adds for error path for clock and do not
>   use device_node for parameters at pwrseq_on. [Patch 2/8]
> - Simplify the caller to use power sequence, follows Alan's commnets [Patch 
> 4/8]
> - Tested three pwrseq instances together using both specific compatible 
> string and
>   generic libraries.
> 
> Changes for v6:
> - Add Matthias Kaehlcke's Reviewed-by and Tested-by. (patch [2/6])
> - Change chipidea core of_node assignment for coming user. (patch [5/6])
> - Applies Joshua Clayton's three dts changes for two boards,
>   the USB device's reg has only #address-cells, but without #size-cells.
> 
> Changes for v5:
> - Delete pwrseq_register/pwrseq_unregister, which is useless currently
> - Fix the linker error when the pwrseq user is compiled as module
> 
> Changes for v4:
> - Create the patch on next-20160722 
> - Fix the of_node is not NULL after chipidea driver is unbinded [Patch 5/6]
> - Using more friendly wait method for reset gpio [Patch 2/6]
> - Support multiple input clocks [Patch 2/6]
> - Add Rob Herring's ack for DT changes
> - Add Joshua Clayton's Tested-by
> 
> Changes for v3:
> - Delete "power-sequence" property at binding-doc, and change related code
>   at both library and user code.
> - Change binding-doc example node name with Rob's comments
> - of_get_named_gpio_flags only gets the gpio, but without setting gpio flags,
>   add additional code request gpio with proper gpio flags
> - Add Philipp Zabel's Ack and MAINTAINER's entry
> 
> Changes for v2:
> - Delete "pwrseq" prefix and clock-names for properties at dt binding
> - Should use structure not but its pointer for kzalloc
> - Since chipidea core has no of_node, let core's of_node equals glue
>   layer's at core's probe
> 
> Joshua Clayton (2):
>   ARM: dts: imx6qdl: Enable usb node children with 
>   ARM: dts: imx6q-evi: Fix onboard hub reset line
> 
> Peter Chen (6):
>   binding-doc: power: pwrseq-generic: add binding doc for generic power
> sequence library
>   power: add power sequence library
>   binding-doc: usb: usb-device: add optional properties for power
> sequence
>   usb: core: add power sequence handling for USB devices
>   usb: chipidea: let chipidea core device of_node equal's glue layer
> device of_node
>   ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property
> 
>  .../bindings/power/pwrseq/pwrseq-generic.txt   |  48 ++
>  .../devicetree/bindings/usb/usb-device.txt |  10 +-
>  MAINTAINERS|   9 +
>  arch/arm/boot/dts/imx6q-evi.dts|  25 +--
>  arch/arm/boot/dts/imx6qdl-udoo.dtsi|  26 ++-
>  arch/arm/boot/dts/imx6qdl.dtsi |   6 +
>  drivers

Re: [PATCH 2/3] usb: dwc3: gadget: wait for End Transfer to complete

2016-10-14 Thread Felipe Balbi 

Hi,

Felipe Balbi  writes:
> Instead of just delaying for 100us, we should
> actually wait for End Transfer Command Complete
> interrupt before moving on. Note that this should
> only be done if we're dealing with one of the core
> revisions that actually require the interrupt before
> moving on.
>
> Reported-by: Baolin Wang 
> Signed-off-by: Felipe Balbi 

I've updated this one in order to fix the comment we had about delaying
100us. No further changes were made, only the comment. Here it is:

8<
From f3fa94f3171709f787a30e3c5ce69a668960b66e Mon Sep 17 00:00:00 2001
From: Felipe Balbi 
Date: Thu, 13 Oct 2016 14:09:47 +0300
Subject: [PATCH v2] usb: dwc3: gadget: wait for End Transfer to complete

Instead of just delaying for 100us, we should
actually wait for End Transfer Command Complete
interrupt before moving on. Note that this should
only be done if we're dealing with one of the core
revisions that actually require the interrupt before
moving on.

Reported-by: Baolin Wang 
Signed-off-by: Felipe Balbi 
---

. Changes since v1:
- Fix comment

 drivers/usb/dwc3/core.h   | 10 ++-
 drivers/usb/dwc3/gadget.c | 71 +++
 2 files changed, 50 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index e878366ead00..2f6f7c4bc8d4 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -504,6 +505,7 @@ struct dwc3_event_buffer {
  * @endpoint: usb endpoint
  * @pending_list: list of pending requests for this endpoint
  * @started_list: list of started requests on this endpoint
+ * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer complete
  * @lock: spinlock for endpoint request queue traversal
  * @regs: pointer to first endpoint register
  * @trb_pool: array of transaction buffers
@@ -529,6 +531,8 @@ struct dwc3_ep {
struct list_headpending_list;
struct list_headstarted_list;
 
+   wait_queue_head_t   wait_end_transfer;
+
spinlock_t  lock;
void __iomem*regs;
 
@@ -545,7 +549,8 @@ struct dwc3_ep {
 #define DWC3_EP_BUSY   (1 << 4)
 #define DWC3_EP_PENDING_REQUEST(1 << 5)
 #define DWC3_EP_MISSED_ISOC(1 << 6)
-
+#define DWC3_EP_END_TRANSFER_PENDING (1 << 7)
+#define DWC3_EP_KICK_POST_END_TRANSFER (1 << 8)
/* This last one is specific to EP0 */
 #define DWC3_EP0_DIR_IN(1 << 31)
 
@@ -1047,6 +1052,9 @@ struct dwc3_event_depevt {
 #define DEPEVT_TRANSFER_BUS_EXPIRY 2
 
u32 parameters:16;
+
+/* For Command Complete Events */
+#define DEPEVT_PARAMETER_CMD(n) (((n) & (0xf << 8)) >> 8)
 } __packed;
 
 /**
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 3ba05b12e49a..05a5b54a001b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -598,6 +598,8 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
reg |= DWC3_DALEPENA_EP(dep->number);
dwc3_writel(dwc->regs, DWC3_DALEPENA, reg);
 
+   init_waitqueue_head(&dep->wait_end_transfer);
+
if (usb_endpoint_xfer_control(desc))
return 0;
 
@@ -1151,6 +1153,11 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, 
struct dwc3_request *req)
if (!dwc3_calc_trbs_left(dep))
return 0;
 
+   if (dep->flags & DWC3_EP_END_TRANSFER_PENDING) {
+   dep->flags |= DWC3_EP_KICK_POST_END_TRANSFER;
+   return 0;
+   }
+
ret = __dwc3_gadget_kick_transfer(dep, 0);
if (ret && ret != -EBUSY)
dwc3_trace(trace_dwc3_gadget,
@@ -2156,6 +2163,7 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
 {
struct dwc3_ep  *dep;
u8  epnum = event->endpoint_number;
+   u8  cmd;
 
dep = dwc->eps[epnum];
 
@@ -2200,8 +2208,15 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
return;
}
break;
-   case DWC3_DEPEVT_RXTXFIFOEVT:
case DWC3_DEPEVT_EPCMDCMPLT:
+   cmd = DEPEVT_PARAMETER_CMD(event->parameters);
+
+   if (cmd == DWC3_DEPCMD_ENDTRANSFER) {
+   dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
+   wake_up(&dep->wait_end_transfer);
+   }
+   break;
+   case DWC3_DEPEVT_RXTXFIFOEVT:
break;
}
 }
@@ -2246,6 +2261,7 @@ static void dwc3_reset_gadget(struct dwc3 *dwc)
 }
 
 static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum, bool force)
+__releases(&dwc->lock) __acquires(&dwc->lock)
 {
struct dwc3_ep *dep;
struct dwc3_gadget_ep_cmd_params params;
@@ -2254,36 +2270,20 @@ static void dwc3_stop

[PATCH 2/3] usb: dwc3: gadget: wait for End Transfer to complete

2016-10-14 Thread Felipe Balbi 
Instead of just delaying for 100us, we should
actually wait for End Transfer Command Complete
interrupt before moving on. Note that this should
only be done if we're dealing with one of the core
revisions that actually require the interrupt before
moving on.

Reported-by: Baolin Wang 
Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/core.h   | 10 +-
 drivers/usb/dwc3/gadget.c | 36 
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index e878366ead00..2f6f7c4bc8d4 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -504,6 +505,7 @@ struct dwc3_event_buffer {
  * @endpoint: usb endpoint
  * @pending_list: list of pending requests for this endpoint
  * @started_list: list of started requests on this endpoint
+ * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer complete
  * @lock: spinlock for endpoint request queue traversal
  * @regs: pointer to first endpoint register
  * @trb_pool: array of transaction buffers
@@ -529,6 +531,8 @@ struct dwc3_ep {
struct list_headpending_list;
struct list_headstarted_list;
 
+   wait_queue_head_t   wait_end_transfer;
+
spinlock_t  lock;
void __iomem*regs;
 
@@ -545,7 +549,8 @@ struct dwc3_ep {
 #define DWC3_EP_BUSY   (1 << 4)
 #define DWC3_EP_PENDING_REQUEST(1 << 5)
 #define DWC3_EP_MISSED_ISOC(1 << 6)
-
+#define DWC3_EP_END_TRANSFER_PENDING (1 << 7)
+#define DWC3_EP_KICK_POST_END_TRANSFER (1 << 8)
/* This last one is specific to EP0 */
 #define DWC3_EP0_DIR_IN(1 << 31)
 
@@ -1047,6 +1052,9 @@ struct dwc3_event_depevt {
 #define DEPEVT_TRANSFER_BUS_EXPIRY 2
 
u32 parameters:16;
+
+/* For Command Complete Events */
+#define DEPEVT_PARAMETER_CMD(n) (((n) & (0xf << 8)) >> 8)
 } __packed;
 
 /**
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 3ba05b12e49a..8e3f9062a7d9 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -598,6 +598,8 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
reg |= DWC3_DALEPENA_EP(dep->number);
dwc3_writel(dwc->regs, DWC3_DALEPENA, reg);
 
+   init_waitqueue_head(&dep->wait_end_transfer);
+
if (usb_endpoint_xfer_control(desc))
return 0;
 
@@ -1151,6 +1153,11 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, 
struct dwc3_request *req)
if (!dwc3_calc_trbs_left(dep))
return 0;
 
+   if (dep->flags & DWC3_EP_END_TRANSFER_PENDING) {
+   dep->flags |= DWC3_EP_KICK_POST_END_TRANSFER;
+   return 0;
+   }
+
ret = __dwc3_gadget_kick_transfer(dep, 0);
if (ret && ret != -EBUSY)
dwc3_trace(trace_dwc3_gadget,
@@ -2156,6 +2163,7 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
 {
struct dwc3_ep  *dep;
u8  epnum = event->endpoint_number;
+   u8  cmd;
 
dep = dwc->eps[epnum];
 
@@ -2200,8 +2208,15 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
return;
}
break;
-   case DWC3_DEPEVT_RXTXFIFOEVT:
case DWC3_DEPEVT_EPCMDCMPLT:
+   cmd = DEPEVT_PARAMETER_CMD(event->parameters);
+
+   if (cmd == DWC3_DEPCMD_ENDTRANSFER) {
+   dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
+   wake_up(&dep->wait_end_transfer);
+   }
+   break;
+   case DWC3_DEPEVT_RXTXFIFOEVT:
break;
}
 }
@@ -2246,6 +2261,7 @@ static void dwc3_reset_gadget(struct dwc3 *dwc)
 }
 
 static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum, bool force)
+__releases(&dwc->lock) __acquires(&dwc->lock)
 {
struct dwc3_ep *dep;
struct dwc3_gadget_ep_cmd_params params;
@@ -2254,7 +2270,8 @@ static void dwc3_stop_active_transfer(struct dwc3 *dwc, 
u32 epnum, bool force)
 
dep = dwc->eps[epnum];
 
-   if (!dep->resource_index)
+   if ((dep->flags & DWC3_EP_END_TRANSFER_PENDING) ||
+   !dep->resource_index)
return;
 
/*
@@ -2295,11 +2312,22 @@ static void dwc3_stop_active_transfer(struct dwc3 *dwc, 
u32 epnum, bool force)
memset(¶ms, 0, sizeof(params));
ret = dwc3_send_gadget_ep_cmd(dep, cmd, ¶ms);
WARN_ON_ONCE(ret);
+
+   if (dwc3_is_usb31(dwc) || dwc->revision < DWC3_REVISION_310A) {
+   dep->flags |= DWC3_EP_END_TRANSFER_PENDING;
+   wait_event_lock_irq(dep->wait_end_transfer,
+   !(dep->flags & DWC3_EP_END_TRANSFER_PENDING),
+   dwc->lock);
+   }
+
dep

[PATCH 3/3] usb: dwc3: gadget: purge dwc3_stop_active_transfers()

2016-10-14 Thread Felipe Balbi 
That function is unnecessarily called from
dwc3_gadget_reset_interrupt(). Gadget drivers (and
thus, functions) are required to dequeue all pending
requests when they get notified about a USB Bus
Reset.

Trying to make sure there are no pending requests
only serves the purpose of working around possibly
bad gadgets.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/gadget.c | 20 
 1 file changed, 20 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 8e3f9062a7d9..bcbb456e1129 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2330,24 +2330,6 @@ __releases(&dwc->lock) __acquires(&dwc->lock)
}
 }
 
-static void dwc3_stop_active_transfers(struct dwc3 *dwc)
-{
-   u32 epnum;
-
-   for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
-   struct dwc3_ep *dep;
-
-   dep = dwc->eps[epnum];
-   if (!dep)
-   continue;
-
-   if (!(dep->flags & DWC3_EP_ENABLED))
-   continue;
-
-   dwc3_remove_requests(dwc, dep);
-   }
-}
-
 static void dwc3_clear_stall_all_ep(struct dwc3 *dwc)
 {
u32 epnum;
@@ -2433,8 +2415,6 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc)
reg &= ~DWC3_DCTL_TSTCTRL_MASK;
dwc3_writel(dwc->regs, DWC3_DCTL, reg);
dwc->test_mode = false;
-
-   dwc3_stop_active_transfers(dwc);
dwc3_clear_stall_all_ep(dwc);
 
/* Reset device address to zero */
-- 
2.10.0.440.g21f862b

--
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/3] usb: gadget: composite: unlock before calling f->disable()

2016-10-14 Thread Felipe Balbi 
f->disable() will call usb_ep_disable() which should
be called with IRQs enabled and locks released.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/gadget/composite.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 919d7d1b611c..2e9359c58eb9 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -732,8 +732,10 @@ static void reset_config(struct usb_composite_dev *cdev)
DBG(cdev, "reset config\n");
 
list_for_each_entry(f, &cdev->config->functions, list) {
+   spin_unlock_irq(&cdev->lock);
if (f->disable)
f->disable(f);
+   spin_lock_irq(&cdev->lock);
 
bitmap_zero(f->endpoints, 32);
}
-- 
2.10.0.440.g21f862b

--
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: gadget: Wait for end transfer complete before free irq

2016-10-14 Thread Felipe Balbi 

Hi,

Baolin Wang  writes:
>> Baolin Wang  writes:
>>> I see what the problem is. Databook tells us we *must* set CMDIOC 
>>> when
>>> issuing EndTransfer command and we should always wait for Command
>>> Complete IRQ. However, we took a shortcut and just delayed for 100us
>>> after issuing End Transfer.
>>
>> Yes, but 100us delay is not enough in some scenarios, like changing
>> function with configfs frequently, we will met problems.
>
> heh, 100us *is* enough. However we still get an IRQ because we 
> requested
> for it. If you want to fix this, then you need to find a way to get 
> rid
> of that 100us udelay() and add a proper wait_for_completion() to delay
> execution until command complete IRQ fires up.

 I haven't tested this yet, but it shows the idea (I think we might 
 still
 have a race with ep_queue if we're still waiting for End Transfer, but
>>>
>>> Yes, maybe we need check DWC3_EP_END_TRANSFER_PENDING flag when
>>> queuing one request.
>>
>> Yeah, I'll add that check later. I still need to make sure we would
>> still try to kick any transfers that might have been queued while
>> waiting for End Transfer Command Complete IRQ.
>
> OK. But I still worried if there are other races in some places which

 There are no other places where this needs to be sorted out.

> is not easy to find out by testing. No introducing race condition
> would be one better solution, anyway I would like to test the patch
> you send out firstly.

 Right, but your patch was working around another problem, rather then
 fixing it.

 Here's another version which should make sure everything remains working
 as it should.

 8<
 From 1f922a902a21d5cee32082be18bbfbf1d46137e4 Mon Sep 17 00:00:00 2001
 From: Felipe Balbi 
 Date: Thu, 13 Oct 2016 14:09:47 +0300
 Subject: [PATCH] usb: dwc3: gadget: wait for End Transfer to complete

 Instead of just delaying for 100us, we should
 actually wait for End Transfer Command Complete
 interrupt before moving on. Note that this should
 only be done if we're dealing with one of the core
 revisions that actually require the interrupt before
 moving on.

 Reported-by: Baolin Wang 
 Signed-off-by: Felipe Balbi 
 ---
  drivers/usb/dwc3/core.h   | 10 +-
  drivers/usb/dwc3/gadget.c | 31 ---
  2 files changed, 37 insertions(+), 4 deletions(-)

 diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
 index e878366ead00..cf495d932252 100644
 --- a/drivers/usb/dwc3/core.h
 +++ b/drivers/usb/dwc3/core.h
 @@ -26,6 +26,7 @@
  #include 
  #include 
  #include 
 +#include 

  #include 
  #include 
 @@ -504,6 +505,7 @@ struct dwc3_event_buffer {
   * @endpoint: usb endpoint
   * @pending_list: list of pending requests for this endpoint
   * @started_list: list of started requests on this endpoint
 + * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer 
 complete
   * @lock: spinlock for endpoint request queue traversal
   * @regs: pointer to first endpoint register
   * @trb_pool: array of transaction buffers
 @@ -529,6 +531,8 @@ struct dwc3_ep {
 struct list_headpending_list;
 struct list_headstarted_list;

 +   wait_queue_head_t   wait_end_transfer;
 +
 spinlock_t  lock;
 void __iomem*regs;

 @@ -545,7 +549,8 @@ struct dwc3_ep {
  #define DWC3_EP_BUSY   (1 << 4)
  #define DWC3_EP_PENDING_REQUEST(1 << 5)
  #define DWC3_EP_MISSED_ISOC(1 << 6)
 -
 +#define DWC3_EP_END_TRANSFER_PENDING (1 << 7)
 +#define DWC3_EP_KICK_POST_END_TRANSFER (1 << 8)
 /* This last one is specific to EP0 */
  #define DWC3_EP0_DIR_IN(1 << 31)

 @@ -1047,6 +1052,9 @@ struct dwc3_event_depevt {
  #define DEPEVT_TRANSFER_BUS_EXPIRY 2

 u32 parameters:16;
 +
 +/* For Command Complete Events */
 +#define DEPEVT_PARAMETER_CMD(n) (((n) & (0xf << 24)) >> 24)
  } __packed;

  /**
 diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
 index 3ba05b12e49a..a3f81b5ba901 100644
 --- a/drivers/usb/dwc3/gadget.c
 +++ b/drivers/usb/dwc3/gadget.c
 @@ -598,6 +598,8 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
 reg |= DWC3_DALEPENA_EP(dep->number);
 dwc3_writel(dwc->regs, DWC3_DALEPENA, reg);

 +   init_waitqueue_head(&dep->wait_end_transfer);
 +

[PATCH v4] usb: dwc3: Wait for control tranfer completed when stopping gadget

2016-10-14 Thread Baolin Wang 
When we change the USB function with configfs dynamically, we possibly met this
situation: one core is doing the control transfer, another core is trying to
unregister the USB gadget from userspace, we must wait for completing this
control tranfer, or it will hang the controller to set the DEVCTRLHLT flag.

Signed-off-by: Baolin Wang 
---
Changes since v3:
 - Remove the patch 1 which will cause start gadget failed.
 - Remove try_again and refactor the code.

Changes since v2:
 - Move disconnect checking into dwc3_send_gadget_ep_cmd().
 - Rename completion name and issue complete() at one place.
 - Move completion initialization into dwc3_gadget_init().

Changes since v1:
 - Split into 2 separate ptaches.
 - Choose complete mechanism instead of polling.
---
 drivers/usb/dwc3/core.h   |2 ++
 drivers/usb/dwc3/ep0.c|2 ++
 drivers/usb/dwc3/gadget.c |   28 +++-
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index b2317e7..23765a1 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -745,6 +745,7 @@ struct dwc3_scratchpad_array {
  * @ep0_usb_req: dummy req used while handling STD USB requests
  * @ep0_bounce_addr: dma address of ep0_bounce
  * @scratch_addr: dma address of scratchbuf
+ * @ep0_in_setup: one control transfer is completed and enter setup phase
  * @lock: for synchronizing
  * @dev: pointer to our struct device
  * @xhci: pointer to our xHCI child
@@ -843,6 +844,7 @@ struct dwc3 {
dma_addr_t  ep0_bounce_addr;
dma_addr_t  scratch_addr;
struct dwc3_request ep0_usb_req;
+   struct completion   ep0_in_setup;
 
/* device lock */
spinlock_t  lock;
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index fe79d77..06c167a 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -311,6 +311,8 @@ void dwc3_ep0_out_start(struct dwc3 *dwc)
ret = dwc3_ep0_start_trans(dwc, 0, dwc->ctrl_req_addr, 8,
DWC3_TRBCTL_CONTROL_SETUP, false);
WARN_ON(ret < 0);
+
+   complete(&dwc->ep0_in_setup);
 }
 
 static struct dwc3_ep *dwc3_wIndex_to_dep(struct dwc3 *dwc, __le16 wIndex_le)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 1783406..3722c90 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1434,6 +1434,13 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int 
is_on, int suspend)
if (pm_runtime_suspended(dwc->dev))
return 0;
 
+   /*
+* Per databook, when we want to stop the gadget, if a control transfer
+* is still in process, complete it and get the core into setup phase.
+*/
+   if (!is_on && dwc->ep0state != EP0_SETUP_PHASE)
+   return -EBUSY;
+
reg = dwc3_readl(dwc->regs, DWC3_DCTL);
if (is_on) {
if (dwc->revision <= DWC3_REVISION_187A) {
@@ -1480,11 +1487,28 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int 
is_on)
 {
struct dwc3 *dwc = gadget_to_dwc(g);
unsigned long   flags;
-   int ret;
+   int ret = 0;
 
is_on = !!is_on;
 
spin_lock_irqsave(&dwc->lock, flags);
+   if (!is_on && !pm_runtime_suspended(dwc->dev) &&
+   dwc->ep0state != EP0_SETUP_PHASE) {
+   reinit_completion(&dwc->ep0_in_setup);
+   ret = -EBUSY;
+   }
+   spin_unlock_irqrestore(&dwc->lock, flags);
+
+   if (ret == -EBUSY) {
+   ret = wait_for_completion_timeout(&dwc->ep0_in_setup,
+ msecs_to_jiffies(500));
+   if (ret == 0) {
+   dev_err(dwc->dev, "timeout to wait for setup phase.\n");
+   return -ETIMEDOUT;
+   }
+   }
+
+   spin_lock_irqsave(&dwc->lock, flags);
ret = dwc3_gadget_run_stop(dwc, is_on, false);
spin_unlock_irqrestore(&dwc->lock, flags);
 
@@ -2911,6 +2935,8 @@ int dwc3_gadget_init(struct dwc3 *dwc)
goto err4;
}
 
+   init_completion(&dwc->ep0_in_setup);
+
dwc->gadget.ops = &dwc3_gadget_ops;
dwc->gadget.speed   = USB_SPEED_UNKNOWN;
dwc->gadget.sg_supported= true;
-- 
1.7.9.5

--
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: gadget: Wait for end transfer complete before free irq

2016-10-14 Thread Baolin Wang 
Hi Felipe,

On 14 October 2016 at 15:46, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>> I see what the problem is. Databook tells us we *must* set CMDIOC 
>> when
>> issuing EndTransfer command and we should always wait for Command
>> Complete IRQ. However, we took a shortcut and just delayed for 100us
>> after issuing End Transfer.
>
> Yes, but 100us delay is not enough in some scenarios, like changing
> function with configfs frequently, we will met problems.

 heh, 100us *is* enough. However we still get an IRQ because we 
 requested
 for it. If you want to fix this, then you need to find a way to get rid
 of that 100us udelay() and add a proper wait_for_completion() to delay
 execution until command complete IRQ fires up.
>>>
>>> I haven't tested this yet, but it shows the idea (I think we might still
>>> have a race with ep_queue if we're still waiting for End Transfer, but
>>
>> Yes, maybe we need check DWC3_EP_END_TRANSFER_PENDING flag when
>> queuing one request.
>
> Yeah, I'll add that check later. I still need to make sure we would
> still try to kick any transfers that might have been queued while
> waiting for End Transfer Command Complete IRQ.

 OK. But I still worried if there are other races in some places which
>>>
>>> There are no other places where this needs to be sorted out.
>>>
 is not easy to find out by testing. No introducing race condition
 would be one better solution, anyway I would like to test the patch
 you send out firstly.
>>>
>>> Right, but your patch was working around another problem, rather then
>>> fixing it.
>>>
>>> Here's another version which should make sure everything remains working
>>> as it should.
>>>
>>> 8<
>>> From 1f922a902a21d5cee32082be18bbfbf1d46137e4 Mon Sep 17 00:00:00 2001
>>> From: Felipe Balbi 
>>> Date: Thu, 13 Oct 2016 14:09:47 +0300
>>> Subject: [PATCH] usb: dwc3: gadget: wait for End Transfer to complete
>>>
>>> Instead of just delaying for 100us, we should
>>> actually wait for End Transfer Command Complete
>>> interrupt before moving on. Note that this should
>>> only be done if we're dealing with one of the core
>>> revisions that actually require the interrupt before
>>> moving on.
>>>
>>> Reported-by: Baolin Wang 
>>> Signed-off-by: Felipe Balbi 
>>> ---
>>>  drivers/usb/dwc3/core.h   | 10 +-
>>>  drivers/usb/dwc3/gadget.c | 31 ---
>>>  2 files changed, 37 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>>> index e878366ead00..cf495d932252 100644
>>> --- a/drivers/usb/dwc3/core.h
>>> +++ b/drivers/usb/dwc3/core.h
>>> @@ -26,6 +26,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>
>>>  #include 
>>>  #include 
>>> @@ -504,6 +505,7 @@ struct dwc3_event_buffer {
>>>   * @endpoint: usb endpoint
>>>   * @pending_list: list of pending requests for this endpoint
>>>   * @started_list: list of started requests on this endpoint
>>> + * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer 
>>> complete
>>>   * @lock: spinlock for endpoint request queue traversal
>>>   * @regs: pointer to first endpoint register
>>>   * @trb_pool: array of transaction buffers
>>> @@ -529,6 +531,8 @@ struct dwc3_ep {
>>> struct list_headpending_list;
>>> struct list_headstarted_list;
>>>
>>> +   wait_queue_head_t   wait_end_transfer;
>>> +
>>> spinlock_t  lock;
>>> void __iomem*regs;
>>>
>>> @@ -545,7 +549,8 @@ struct dwc3_ep {
>>>  #define DWC3_EP_BUSY   (1 << 4)
>>>  #define DWC3_EP_PENDING_REQUEST(1 << 5)
>>>  #define DWC3_EP_MISSED_ISOC(1 << 6)
>>> -
>>> +#define DWC3_EP_END_TRANSFER_PENDING (1 << 7)
>>> +#define DWC3_EP_KICK_POST_END_TRANSFER (1 << 8)
>>> /* This last one is specific to EP0 */
>>>  #define DWC3_EP0_DIR_IN(1 << 31)
>>>
>>> @@ -1047,6 +1052,9 @@ struct dwc3_event_depevt {
>>>  #define DEPEVT_TRANSFER_BUS_EXPIRY 2
>>>
>>> u32 parameters:16;
>>> +
>>> +/* For Command Complete Events */
>>> +#define DEPEVT_PARAMETER_CMD(n) (((n) & (0xf << 24)) >> 24)
>>>  } __packed;
>>>
>>>  /**
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 3ba05b12e49a..a3f81b5ba901 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -598,6 +598,8 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
>>> reg |= DWC3_DALEPENA_EP(dep->number);
>>> dwc3_writel(dwc->regs, DWC3_DALEPENA, reg);
>>>
>>> +   init_waitqueue_head(&dep->wait_end_transfer);
>>> +
>>> if (usb_endpoint_xfer_control(desc))
>>> return 0;
>>>
>>> @@ -1151,6 +

[PATCH V11 1/1] usb:serial: Add Fintek F81532/534 driver

2016-10-14 Thread Ji-Ze Hong (Peter Hong) 
This driver is for Fintek F81532/F81534 USB to Serial Ports IC.

F81532 spec:
https://drive.google.com/file/d/0B8vRwwYO7aMFOTRRMmhWQVNvajQ/view?usp=
sharing

F81534 spec:
https://drive.google.com/file/d/0B8vRwwYO7aMFV29pQWJqbVBNc00/view?usp=
sharing

Features:
1. F81532 is 1-to-2 & F81534 is 1-to-4 serial ports IC
2. Support Baudrate from B50 to B115200.

Reviewed-by: Johan Hovold 
Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
Changelog:
V11
1. Reduce F81534_MAX_BUS_RETRY from 2000 to 20. We are only using
   internal SPI bus to read flash when attach() & calc_num_ports()
   and never read flash when the F81532/534 in full loading, so we
   can reduce retry count.
2. Modify attach() & calc_num_ports() read default value only when
   can't find the custom setting.
3. Change tx_empty protect method from spinlock to set_bit()/
   clear_bit()/test_and_clear_bit().
4. Move calculate tty_idx[i] from port_probe() to attach().
5. Add f81534_tx_empty()

V10
1. Change the submit/kill timming for read URBs, submit when first
   serial port open and kill when final port close.
2. Remove all source code about controlling GPIOs.
3. Change the f81534_tiocmget() from reading shadow MSR with delay
   20ms to read MSR register directly.
4. Using tty_port_initialized() to check port opened/closed.
5. Add sanity check for variables.

v9
1. Remove lots of code to make more generic for F81532/534. e.g.,
   high baud rate support, RS485/422 mode switch, most of GPIO
   control and internal storage write functional.
2. Change f81534_tiocmget() MSR delay from schedule_timeout_killable()
   to wait_for_completion_killable_timeout(). This IC will delayed
   receiving MSR change when doing loop-back test e.g., BurnInTest.
   We'll reset completion "msr_done" in f81534_update_mctrl(). If we
   changed MCR, the next f81534_tiocmget() will delay for 20ms or
   continue with new MSR arrived.
3. Fix for non-zero buffer allocated in f81534_setup_ports(). It'll
   make device malfunctional with incorrect tx length for other ports.

v8
1. Remove driver mode GPIOLIB & RS485 control support, the driver will
   only load GPIO/UART Mode when driver attach() & port_probe().
2. Add more documents for 3 generation IC with f81534_calc_num_ports().
3. Simplify the GPIO register structure "f81534_pin_control".
4. Change all counter type from int to size_t.
5. Change some failed message with failed: "status code" and remove all
   exclamation mark in messages.
6. Change all save blocks to block0 due to the driver is only used 1
   block (block0) to save data.
7. Change read MSR in open() instead of port_probe().
8. use GFP_ATOMIC kmalloc mode in write().
9. Maintain old style with 1 read URBs and 4 write URBs like mxuports.c
   I had tested with submit 4 read URBs, but it'll make some port freeze
   when doing BurnInTest Port test.

v7
1. Make all gpiolib function with #ifdef CONFIG_GPIOLIB marco block.
   Due to F81532/534 could run without gpiolib, we implements
   f81534_prepare_gpio()/f81534_release_gpio() always success without
   CONFIG_GPIOLIB.
2. Fix crash when receiving MSR change on driver load/unload. It's cause
   by f81534_process_read_urb() get read URB callback data, but port
   private data is not init complete or released. We solve with 2
   modifications.

   1. add null pointer check with f81534_process_read_urb(). We'll skip
  this report when port_priv = NULL.
   2. when "one" port f81534_port_remove() is called, kill the port-0
  read URB before kfree port_priv.

v6
1. Re-implement the write()/resume() function. Due to this device cant be
   suitable with generic write(), we'll do the submit write URB when
   write()/received tx empty/set_termios()/resume()
2. Logic/Phy Port mapping rewrite in f81534_port_probe() &
   f81534_phy_to_logic_port().
3. Introduced "Port Hide" function. Some customer use F81532 reference
   design for HW layout, but really use F81534 IC. We'll check
   F81534_PORT_CONF_DISABLE_PORT flag with in uart mode field to do
   port hide with port not used. It can be avoid end-user to use not
   layouted port.
4. The 4x3 output-only open-drain pins for F81532/534 is designed for
   control outer devices (with our EVB for examples, the 4 sets of pins
   are designed to control transceiver mode). So we decide to implement
   with gpiolib interface.
5. Add device vendor id with 0x2c42

v5
1. Change f81534_port_disable/enable() from H/W mode to S/W mode
   It'll skip all rx data when port is not opened.
2. Some function modifier add with static (Thanks for Paul Bolle)
3. It's will direct return when count=0 in f81534_write() to
   reduce spin_lock usage.

v4
1. clearify f81534_process_read_urb() with
   f81534_p

Re: [PATCH v2] usb: dwc3: gadget: Wait for end transfer complete before free irq

2016-10-14 Thread Felipe Balbi 

Hi,

Baolin Wang  writes:
> I see what the problem is. Databook tells us we *must* set CMDIOC when
> issuing EndTransfer command and we should always wait for Command
> Complete IRQ. However, we took a shortcut and just delayed for 100us
> after issuing End Transfer.

 Yes, but 100us delay is not enough in some scenarios, like changing
 function with configfs frequently, we will met problems.
>>>
>>> heh, 100us *is* enough. However we still get an IRQ because we requested
>>> for it. If you want to fix this, then you need to find a way to get rid
>>> of that 100us udelay() and add a proper wait_for_completion() to delay
>>> execution until command complete IRQ fires up.
>>
>> I haven't tested this yet, but it shows the idea (I think we might still
>> have a race with ep_queue if we're still waiting for End Transfer, but
>
> Yes, maybe we need check DWC3_EP_END_TRANSFER_PENDING flag when
> queuing one request.

 Yeah, I'll add that check later. I still need to make sure we would
 still try to kick any transfers that might have been queued while
 waiting for End Transfer Command Complete IRQ.
>>>
>>> OK. But I still worried if there are other races in some places which
>>
>> There are no other places where this needs to be sorted out.
>>
>>> is not easy to find out by testing. No introducing race condition
>>> would be one better solution, anyway I would like to test the patch
>>> you send out firstly.
>>
>> Right, but your patch was working around another problem, rather then
>> fixing it.
>>
>> Here's another version which should make sure everything remains working
>> as it should.
>>
>> 8<
>> From 1f922a902a21d5cee32082be18bbfbf1d46137e4 Mon Sep 17 00:00:00 2001
>> From: Felipe Balbi 
>> Date: Thu, 13 Oct 2016 14:09:47 +0300
>> Subject: [PATCH] usb: dwc3: gadget: wait for End Transfer to complete
>>
>> Instead of just delaying for 100us, we should
>> actually wait for End Transfer Command Complete
>> interrupt before moving on. Note that this should
>> only be done if we're dealing with one of the core
>> revisions that actually require the interrupt before
>> moving on.
>>
>> Reported-by: Baolin Wang 
>> Signed-off-by: Felipe Balbi 
>> ---
>>  drivers/usb/dwc3/core.h   | 10 +-
>>  drivers/usb/dwc3/gadget.c | 31 ---
>>  2 files changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index e878366ead00..cf495d932252 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -26,6 +26,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  #include 
>>  #include 
>> @@ -504,6 +505,7 @@ struct dwc3_event_buffer {
>>   * @endpoint: usb endpoint
>>   * @pending_list: list of pending requests for this endpoint
>>   * @started_list: list of started requests on this endpoint
>> + * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer 
>> complete
>>   * @lock: spinlock for endpoint request queue traversal
>>   * @regs: pointer to first endpoint register
>>   * @trb_pool: array of transaction buffers
>> @@ -529,6 +531,8 @@ struct dwc3_ep {
>> struct list_headpending_list;
>> struct list_headstarted_list;
>>
>> +   wait_queue_head_t   wait_end_transfer;
>> +
>> spinlock_t  lock;
>> void __iomem*regs;
>>
>> @@ -545,7 +549,8 @@ struct dwc3_ep {
>>  #define DWC3_EP_BUSY   (1 << 4)
>>  #define DWC3_EP_PENDING_REQUEST(1 << 5)
>>  #define DWC3_EP_MISSED_ISOC(1 << 6)
>> -
>> +#define DWC3_EP_END_TRANSFER_PENDING (1 << 7)
>> +#define DWC3_EP_KICK_POST_END_TRANSFER (1 << 8)
>> /* This last one is specific to EP0 */
>>  #define DWC3_EP0_DIR_IN(1 << 31)
>>
>> @@ -1047,6 +1052,9 @@ struct dwc3_event_depevt {
>>  #define DEPEVT_TRANSFER_BUS_EXPIRY 2
>>
>> u32 parameters:16;
>> +
>> +/* For Command Complete Events */
>> +#define DEPEVT_PARAMETER_CMD(n) (((n) & (0xf << 24)) >> 24)
>>  } __packed;
>>
>>  /**
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 3ba05b12e49a..a3f81b5ba901 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -598,6 +598,8 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
>> reg |= DWC3_DALEPENA_EP(dep->number);
>> dwc3_writel(dwc->regs, DWC3_DALEPENA, reg);
>>
>> +   init_waitqueue_head(&dep->wait_end_transfer);
>> +
>> if (usb_endpoint_xfer_control(desc))
>> return 0;
>>
>> @@ -1151,6 +1153,11 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep 
>> *dep, struct dwc3_request *req)
>> if (!dwc3_calc_trbs_left(dep))
>> return 0;
>>
>> +   if (dep->flags & DWC3_EP_END_T

Re: [PATCH v2] usb: dwc3: gadget: Wait for end transfer complete before free irq

2016-10-14 Thread Baolin Wang 
Hi Felipe,

On 13 October 2016 at 21:34, Felipe Balbi  wrote:
>
>
> Hi,
>
> Baolin Wang  writes:
>>> Baolin Wang  writes:
>> Baolin Wang  writes:
>> I'm thinking this is a bug in configfs interface of Gadget API, not
>> dwc3. The only reason for this to happen would be if we get to
>> ->udc_stop() with endpoints still enabled.
>>
>> Can you check if that's the case? i.e. can you check if any endpoints
>> are still enabled when we get here?
>
> No, it is not any endpoints are still enabled. Like I said in commit
> message, we will start end transfer command when disable the endpoint,
> if the end transfer command complete event comes after we have freed
> the gadget irq, it will trigger the interrupt line all the time to
> crash the system.

 I see what the problem is. Databook tells us we *must* set CMDIOC when
 issuing EndTransfer command and we should always wait for Command
 Complete IRQ. However, we took a shortcut and just delayed for 100us
 after issuing End Transfer.
>>>
>>> Yes, but 100us delay is not enough in some scenarios, like changing
>>> function with configfs frequently, we will met problems.
>>
>> heh, 100us *is* enough. However we still get an IRQ because we requested
>> for it. If you want to fix this, then you need to find a way to get rid
>> of that 100us udelay() and add a proper wait_for_completion() to delay
>> execution until command complete IRQ fires up.
>
> I haven't tested this yet, but it shows the idea (I think we might still
> have a race with ep_queue if we're still waiting for End Transfer, but

 Yes, maybe we need check DWC3_EP_END_TRANSFER_PENDING flag when
 queuing one request.
>>>
>>> Yeah, I'll add that check later. I still need to make sure we would
>>> still try to kick any transfers that might have been queued while
>>> waiting for End Transfer Command Complete IRQ.
>>
>> OK. But I still worried if there are other races in some places which
>
> There are no other places where this needs to be sorted out.
>
>> is not easy to find out by testing. No introducing race condition
>> would be one better solution, anyway I would like to test the patch
>> you send out firstly.
>
> Right, but your patch was working around another problem, rather then
> fixing it.
>
> Here's another version which should make sure everything remains working
> as it should.
>
> 8<
> From 1f922a902a21d5cee32082be18bbfbf1d46137e4 Mon Sep 17 00:00:00 2001
> From: Felipe Balbi 
> Date: Thu, 13 Oct 2016 14:09:47 +0300
> Subject: [PATCH] usb: dwc3: gadget: wait for End Transfer to complete
>
> Instead of just delaying for 100us, we should
> actually wait for End Transfer Command Complete
> interrupt before moving on. Note that this should
> only be done if we're dealing with one of the core
> revisions that actually require the interrupt before
> moving on.
>
> Reported-by: Baolin Wang 
> Signed-off-by: Felipe Balbi 
> ---
>  drivers/usb/dwc3/core.h   | 10 +-
>  drivers/usb/dwc3/gadget.c | 31 ---
>  2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index e878366ead00..cf495d932252 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -504,6 +505,7 @@ struct dwc3_event_buffer {
>   * @endpoint: usb endpoint
>   * @pending_list: list of pending requests for this endpoint
>   * @started_list: list of started requests on this endpoint
> + * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer complete
>   * @lock: spinlock for endpoint request queue traversal
>   * @regs: pointer to first endpoint register
>   * @trb_pool: array of transaction buffers
> @@ -529,6 +531,8 @@ struct dwc3_ep {
> struct list_headpending_list;
> struct list_headstarted_list;
>
> +   wait_queue_head_t   wait_end_transfer;
> +
> spinlock_t  lock;
> void __iomem*regs;
>
> @@ -545,7 +549,8 @@ struct dwc3_ep {
>  #define DWC3_EP_BUSY   (1 << 4)
>  #define DWC3_EP_PENDING_REQUEST(1 << 5)
>  #define DWC3_EP_MISSED_ISOC(1 << 6)
> -
> +#define DWC3_EP_END_TRANSFER_PENDING (1 << 7)
> +#define DWC3_EP_KICK_POST_END_TRANSFER (1 << 8)
> /* This last one is specific to EP0 */
>  #define DWC3_EP0_DIR_IN(1 << 31)
>
> @@ -1047,6 +1052,9 @@ struct dwc3_event_depevt {
>  #define DEPEVT_TRANSFER_BUS_EXPIRY 2
>
> u32 parameters:16;
> +
> +/* For Command Complete Events */
> +#define DEPEVT_PARAMETER_CMD(n) (((n) & (0xf << 24)) >> 24)
>  } __packed;
>
>  /**
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/