Re: [PATCH] usb-storage: block runtime suspend when the host is added

[Greg KH]

On Wed, Sep 21, 2016 at 01:07:55PM -0400, Alan Stern wrote:
> On Wed, 21 Sep 2016, Greg KH wrote:
> 
> > On Wed, Sep 21, 2016 at 11:25:37AM -0400, Alan Stern wrote:
> > > A warning message that was recently added to the PM core has alerted
> > > us to a potential problem in usb-storage.  Although the USB interface
> > > is in the runtime active state when the probe routine starts, until
> > > the runtime-PM usage count is incremented nothing forces it to remain
> > > active.  The increment doesn't happen until after scsi_add_host() is
> > > called, and that call causes the interface to go into runtime suspend.
> > > 
> > > Although this hasn't caused anything to go wrong for anyone, as far as
> > > I know, it still is a bug.
> > > 
> > > To prevent this from happening, move the existing call to
> > > usb_autopm_get_interface_no_resume() up from its current position to
> > > before scsi_add_host() is called.
> > > 
> > > Signed-off-by: Alan Stern 
> > > Reported-by: Carsten Mattner 
> > > Tested-by: Carsten Mattner 
> > > CC: 
> > 
> > It's a bit late for 4.8 for this, can we wait for 4.9-rc1?
> > 
> > This doesn't apply to my usb-next branch, so things have changed in this
> > area, did they solve the issue already?
> 
> Heh -- I _thought_ this issue seemed familiar.  :-)
> 
> Yes, it has already been fixed by commit a094760b9a77 ("usb: storage: 
> fix runtime pm issue in usb_stor_probe2") by Heiner Kallweit.  I even 
> Acked his patch, but I didn't remember it.
> 
> So Carsten, there's your answer.  The commit will be part of 4.9-rc1, 
> unless Greg shoehorns it into 4.8.
> 
> But if it doesn't get into 4.8 then it should be marked for
> 4.8.stable, to prevent those new warning messages from alarming people.

I'll take it for 4.8-stable when it hits Linus's tree.

thanks,

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

Re: [PATCH 2/2] musb: sunxi: Force session end on babble errors in host-mode

[Bin Liu]

Hi,

On Sun, Sep 18, 2016 at 06:50:18PM +0200, Hans de Goede wrote:
> The sunxi musb has a bug where sometimes it will generate a babble
> error on device disconnect instead of a disconnect irq. When this
> happens the musb-controller switches from host mode to device mode
> (it clears MUSB_DEVCTL_SESSION and sets MUSB_DEVCTL_BDEVICE) and
> gets stuck in this state.
> 
> Clearing this requires reporting Vbus low for 200 or more ms, but
> on some devices Vbus is simply always high (host-only mode, no Vbus
> control).
> 
> This commit calls sun4i_usb_phy_force_session_end() on babble errors
> in host-mode, fixing the musb controller being stuck in this state
> on systems without Vbus control; and also fixes the need to unplug
> the usb-b -> usb-a cable to get out of this state on systems with
> Vbus control.
> 
> Signed-off-by: Hans de Goede 
> ---
>  drivers/usb/musb/sunxi.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> index 1408245..5079d90 100644
> --- a/drivers/usb/musb/sunxi.c
> +++ b/drivers/usb/musb/sunxi.c
> @@ -192,8 +192,18 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void 
> *__hci)
>* normally babble never happens treat it as disconnect.
>*/
>   if ((musb->int_usb & MUSB_INTR_BABBLE) && is_host_active(musb)) {

musb_interrupt() handle BABBLE in host mode, and has a glue hook
musb_platform_recover() in musb_recover_from_babble().

Maybe you can use it?

> + struct sunxi_glue *glue =
> + dev_get_drvdata(musb->controller->parent);
> +
> + dev_warn(musb->controller->parent, "babble, treating as 
> disconnect\n");
> +
>   musb->int_usb &= ~MUSB_INTR_BABBLE;
>   musb->int_usb |= MUSB_INTR_DISCONNECT;
> + /*
> +  * Fix the musb controller sometimes getting stuck in
> +  * bdevice state after a babble error.
> +  */
> + sun4i_usb_phy_force_session_end(glue->phy);

As I commented in PATCH 1/2, can you somehow reuse
sun4i_usb_phy_set_mode() instead?

>   }
>  
>   if ((musb->int_usb & MUSB_INTR_RESET) && !is_host_active(musb)) {
> -- 
> 2.9.3

Regards,
-Bin.

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

Re: [PATCH 1/2] phy-sun4i-usb: Add sun4i_usb_phy_force_session_end() function

[Bin Liu]

Hi,

On Wed, Sep 21, 2016 at 11:05:33AM +0300, Hans de Goede wrote:
> Hi,
> 
> On 09/20/2016 07:45 AM, Kishon Vijay Abraham I wrote:
> >Hi,
> >
> >On Sunday 18 September 2016 10:20 PM, Hans de Goede wrote:
> >>The sunxi musb has a bug where sometimes it will generate a babble
> >>error on device disconnect instead of a disconnect irq. When this
> >>happens the musb-controller switches from host mode to device mode
> >>(it clears MUSB_DEVCTL_SESSION and sets MUSB_DEVCTL_BDEVICE) and
> >>gets stuck in this state.
> >>
> >>Clearing this requires reporting Vbus low for 200 or more ms, but
> >>on some devices Vbus is simply always high (host-only mode, no Vbus
> >>control). The phy-sun4i-usb code already has code to force a session
> >>end for devices without Vbus control.
> >>
> >>This commit adds a sun4i_usb_phy_force_session_end() function exporting
> >>this functionality to the sunxi-musb glue, so that it can force a session
> >>end to fixup the stuck state after a babble error.
> >>
> >>Signed-off-by: Hans de Goede 
> >>---
> >> drivers/phy/phy-sun4i-usb.c   | 11 +++
> >> include/linux/phy/phy-sun4i-usb.h |  7 +++
> >> 2 files changed, 18 insertions(+)
> >>
> >>diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> >>index 43c0d98..06f4e11a 100644
> >>--- a/drivers/phy/phy-sun4i-usb.c
> >>+++ b/drivers/phy/phy-sun4i-usb.c
> >>@@ -470,6 +470,17 @@ void sun4i_usb_phy_set_squelch_detect(struct phy 
> >>*_phy, bool enabled)
> >> }
> >> EXPORT_SYMBOL_GPL(sun4i_usb_phy_set_squelch_detect);
> >>
> >>+void sun4i_usb_phy_force_session_end(struct phy *_phy)
> >>+{
> >>+   struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
> >>+   struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
> >>+
> >>+   data->id_det = -1;
> >>+   data->force_session_end = true;
> >>+   queue_delayed_work(system_wq, >detect, 0);
> >>+}
> >>+EXPORT_SYMBOL_GPL(sun4i_usb_phy_force_session_end);
> >
> >Er.. one more export symbol :-(
> 
> Yes unfortunately we need one more to work around sunxi musb / phy bugs.

Instead, can you somehow reuse sun4i_usb_phy_set_mode()?

Regards,
-Bin.

> 
> >>+
> >> static const struct phy_ops sun4i_usb_phy_ops = {
> >>.init   = sun4i_usb_phy_init,
> >>.exit   = sun4i_usb_phy_exit,
> >>diff --git a/include/linux/phy/phy-sun4i-usb.h 
> >>b/include/linux/phy/phy-sun4i-usb.h
> >>index 50aed92..3bb773f 100644
> >>--- a/include/linux/phy/phy-sun4i-usb.h
> >>+++ b/include/linux/phy/phy-sun4i-usb.h
> >>@@ -23,4 +23,11 @@
> >>  */
> >> void sun4i_usb_phy_set_squelch_detect(struct phy *phy, bool enabled);
> >>
> >>+/**
> >>+ * sun4i_usb_force_session_end() - Force the current session to end
> >>+ *by reporting VBus low for 200+ ms
> >>+ * @phy: reference to a sun4i usb phy
> >>+ */
> >>+void sun4i_usb_phy_force_session_end(struct phy *phy);
> >
> >Should we include a static inline function if sun4i phy is not defined?
> 
> No, we're also not doing that for the already exported
> sun4i_usb_phy_set_squelch_detect()
> 
> And it is not necessary since the only caller is drivers/usb/musb/sunxi.c,
> and drivers/usb/musb/Kconfig has:
> 
> config USB_MUSB_SUNXI
> tristate "Allwinner (sunxi)"
> depends on PHY_SUN4I_USB
> 
> Regards,
> 
> Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: dwc2: add USBTrdTim to initial value

[Pengcheng Li]

After dwc2_core_reset，register is to the initial value, and the USBTrdTim
vale is 0x5. If hsotg->phyif = GUSBCFG_PHYIF8, after the dwc2_writel,the
value is 0xd.So we need to set the USBTrdTim to 0.

Signed-off-by: Pengcheng Li 
---
 drivers/usb/dwc2/gadget.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index af46adf..9e52e4f 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2531,7 +2531,7 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg 
*hsotg,
/* keep other bits untouched (so e.g. forced modes are not lost) */
usbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP |
-   GUSBCFG_HNPCAP);
+   GUSBCFG_HNPCAP | GUSBCFG_USBTRDTIM_MASK);
 
/* set the PLL on, remove the HNP/SRP and set the PHY */
val = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5;
@@ -3413,7 +3413,7 @@ static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)
/* keep other bits untouched (so e.g. forced modes are not lost) */
usbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP |
-   GUSBCFG_HNPCAP);
+   GUSBCFG_HNPCAP | GUSBCFG_USBTRDTIM_MASK);
 
/* set the PLL on, remove the HNP/SRP and set the PHY */
trdtim = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5;
-- 
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: [PATCH v2] musb: Export musb_root_disconnect for use in modules

[Bin Liu]

Hi,

On Sat, Sep 17, 2016 at 12:08:10PM +0200, Hans de Goede wrote:
> Export musb_root_disconnect for use in modules, so that musb glue
> code build as module can use it.
> 
> This fixes the buildbot errors for -next in arm64-allmodconfig
> and arm-allmodconfig.
> 
> Fixes: 7cba17ec9adc8cf ("musb: sunxi: Add support for platform_set_mode")
> Signed-off-by: Hans de Goede 

Applied. Thanks.

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

Re: [PATCH] usb: musb: fix error handling message in probe

[Bin Liu]

Hi,

On Sat, Sep 17, 2016 at 12:21:25AM +0200, Arnd Bergmann wrote:
> We print an error message when platform_device_register_full()
> fails, but the initialization of the argument has been removed,
> as shown in this warning:
> 
> drivers/usb/musb/da8xx.c: In function 'da8xx_probe':
> drivers/usb/musb/da8xx.c:521:3: error: 'ret' may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
> 
> This modifies the function to assign the return code before
> checking it, and does uses the same method in the check for
> usb_phy_generic_register() as well.
> 
> Fixes: 947c49afe41f ("usb: musb: da8xx: Remove mach code")
> Signed-off-by: Arnd Bergmann 

Applied. Thanks.
(Added 'da8xx:' prefix in the commit subject)

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

Re: [PATCH] usb-storage: block runtime suspend when the host is added

[Carsten Mattner]

On Wed, Sep 21, 2016 at 7:07 PM, Alan Stern  wrote:
> On Wed, 21 Sep 2016, Greg KH wrote:
>
> > On Wed, Sep 21, 2016 at 11:25:37AM -0400, Alan Stern wrote:
> > > A warning message that was recently added to the PM core has alerted
> > > us to a potential problem in usb-storage.  Although the USB interface
> > > is in the runtime active state when the probe routine starts, until
> > > the runtime-PM usage count is incremented nothing forces it to remain
> > > active.  The increment doesn't happen until after scsi_add_host() is
> > > called, and that call causes the interface to go into runtime suspend.
> > >
> > > Although this hasn't caused anything to go wrong for anyone, as far as
> > > I know, it still is a bug.
> > >
> > > To prevent this from happening, move the existing call to
> > > usb_autopm_get_interface_no_resume() up from its current position to
> > > before scsi_add_host() is called.
> > >
> > > Signed-off-by: Alan Stern 
> > > Reported-by: Carsten Mattner 
> > > Tested-by: Carsten Mattner 
> > > CC: 
> >
> > It's a bit late for 4.8 for this, can we wait for 4.9-rc1?
> >
> > This doesn't apply to my usb-next branch, so things have changed in this
> > area, did they solve the issue already?
>
> Heh -- I _thought_ this issue seemed familiar.  :-)
>
> Yes, it has already been fixed by commit a094760b9a77 ("usb: storage:
> fix runtime pm issue in usb_stor_probe2") by Heiner Kallweit.  I even
> Acked his patch, but I didn't remember it.
>
> So Carsten, there's your answer.  The commit will be part of 4.9-rc1,
> unless Greg shoehorns it into 4.8.

Looks reasonable to add to 4.8, but I'm not the maintainer,
so it's not my responsibility or call to make. Greg knows best.

> But if it doesn't get into 4.8 then it should be marked for
> 4.8.stable, to prevent those new warning messages from
> alarming people.

Thanks everyone, especially Alan!
--
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: 4.8-rc: runtime PM trying to activate child device host6 but parent (2-1.2:1.0) is not active

[Carsten Mattner]

On Wed, Sep 21, 2016 at 5:23 PM, Alan Stern  wrote:
> On Tue, 20 Sep 2016, Carsten Mattner wrote:
>
>> On Tue, Sep 20, 2016 at 9:49 PM, Alan Stern  
>> wrote:
>> > On Tue, 20 Sep 2016, Carsten Mattner wrote:
>> >
>> > > On Tue, Sep 20, 2016 at 5:41 PM, Alan Stern  
>> > > wrote:
>> > >
>> > > > I suspect the problem has been there all along, but it simply wasn't
>> > > > reported until commit 71723f95463d ("PM / runtime: print error when
>> > > > activating a child to unactive parent") was merged in 4.8-rc1.
>> > >
>> > > Is this just a false positive or a real error that had been silently
>> > > ignored all this time?
>> >
>> > It's a real error, albeit one that is quite unlikely to cause any real
>> > harm.  That's why nobody noticed it until the warning message was
>> > added.
>>
>> Just to be clear, your patch doesn't hide the error but merely silences
>> the safe-to-ignore condition after trying a little harder, right?
>
> I don't understand what you mean by "silences the safe-to-ignore
> condition" or "trying a little harder".  The patch fixes a real bug --
> it prevents the interface from going into runtime suspend at the wrong
> time.

I thought you meant it's a hardware/firmware fault and the fix works
around it, that's why I assumed it's a detect-and-ignore-try-again
scheme.
--
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] Few PM fixes for MUSB for v4.9 merge window

[Bin Liu]

Hi,

On Thu, Sep 15, 2016 at 09:29:12AM -0700, Tony Lindgren wrote:
> Hi,
> 
> I noticed few corner cases we're not handling properly with the
> musb hardware session bit based PM runtime heading for v4.9.
> 
> Probably the disconnect after unconfigure PM issue has always
> been there, but fixing it without the session bit based PM
> runtime would be a mess.
> 
> Regards,
> 
> Tony
> 
> Tony Lindgren (2):
>   usb: musb: Fix PM runtime for disconnect after unconfigure
>   usb: musb: Fix session based PM for first invalid VBUS
> 
>  drivers/usb/musb/musb_core.c   | 13 -
>  drivers/usb/musb/musb_core.h   |  1 +
>  drivers/usb/musb/musb_gadget.c |  3 +++
>  3 files changed, 12 insertions(+), 5 deletions(-)

Applied. Thanks.

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

Re: [Umap2][7/11][160a:3184] NULL pointer dereference

[Malcolm Priestley]

On 21/09/16 17:44, Greg KH wrote:

On Wed, Sep 21, 2016 at 06:34:03PM +0200, Oliver Neukum wrote:

On Thu, 2016-08-18 at 13:50 +0300, Binyamin Sharet wrote:

On 08/18/2016 01:31 PM, Oliver Neukum wrote:

On Wed, 2016-08-17 at 14:34 +0300, Binyamin Sharet wrote:

After connecting such a device, NULL pointer dereference in the

kernel.

You may need to connect another device or two after this one to

trigger

the oops.

Binyamin Sharet
Cisco, STARE-C

<< Attached:  160a_3184_dmesg_1.log >>
<< Attached:  160a_3184_dmesg_2.log >>

kernel: 4.8-rc2
result: reproduced

[..]



Attached.



Hi,

this fell through the cracks somehow. Does anybody know how
these devices should be to work with the driver?


Which device was this?  I'm slowly working on a patchset to move this
type of checking to the USB core to hopefully prevent this type of thing
from have to be added to each USB driver individually.


It's the VT6656.

The problem cannot be reproduced here on the Raspberry PI 2.

Looking at those logs it looks like the device reset during firmware 
downloading.


Is the PSU for PI up to job?

The device is power hungry, I am using a 2.4A @5v power supply.

Regards


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

Re: [PATCH v2 2/6] Documentation: dt-bindings: Add documentation for the Meson USB2 PHYs

[Kevin Hilman]

Rob Herring  writes:

> On Sun, Sep 11, 2016 at 03:41:07PM +0200, Martin Blumenstingl wrote:
>> Add the documentation for the bindings for the Meson8b and GXBB USB2
>> PHYs.
>> 
>> Signed-off-by: Martin Blumenstingl 
>> ---
>>  .../devicetree/bindings/phy/meson-usb2-phy.txt | 27 
>> ++
>>  1 file changed, 27 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/phy/meson-usb2-phy.txt
>> 
>> diff --git a/Documentation/devicetree/bindings/phy/meson-usb2-phy.txt 
>> b/Documentation/devicetree/bindings/phy/meson-usb2-phy.txt
>> new file mode 100644
>> index 000..9da5ea2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/meson-usb2-phy.txt
>> @@ -0,0 +1,27 @@
>> +* Amlogic USB2 PHY
>> +
>> +Required properties:
>> +- compatible:   Depending on the platform this should be one of:
>> +"amlogic,meson8b-usb2-phy"
>> +"amlogic,meson-gxbb-usb2-phy"
>> +- reg:  The base address and length of the registers
>> +- #phys-cells:  should be 0 (see phy-bindings.txt in this directory)
>> +- clocks:   phandle and clock identifier for the phy clocks
>> +- clock-names:  "usb_general" and "usb"
>> +
>> +Optional properties:
>> +- resets:   reference to the reset controller
>> +- phy-supply:   see phy-bindings.txt in this directory
>> +
>> +
>> +Example:
>> +
>> +usb0_phy: usb_phy@0 {
>
> usb-phy@0
>
> With that,
>
> Acked-by: Rob Herring 
>

Oops, I had already merged this one.

Martin, can you send a fixup patch for the underscores?

Thanks,

Kevin
--
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: XHCI: USB 2.0 device unresponsive on 3.0 controller

[Ben]

As an update on information! It happens with my mouse too, very seldom, but 
sometimes. There are not as many consecutive inputs from a user that can really 
get messed up on a USB 2.0 drives, but when I get home from work today I can 
try that and see if the file transfer stays intact. I will also bring home a 
few other keyboards and test them too. 

Regarding the logs:

After the keyboard stops working or receiving power (it seems) I manually 
disconnect and reconnect it.


> On Sep 21, 2016, at 08:19, Ben  wrote:
> 
> Good afternoon everyone,
> 
> My coolermaster Quickfire Rapid keyboard (ID: 2516:0004) has been acting up 
> since I installed arch linux for the first time last week. System overview in 
> attachment.
> 
> Bios is set for EHCI hand-off: Enabled; xHCI hand-off: Enabled (Originally 
> Diabled); xHCI: Auto (Tried smart auto and enabled; see Gigabyte specs for 
> expansion on info). On my EHCI ports the keyboard works without issue, so I'm 
> pointing towards the issue being in the xHCI driver.
> 
> Conditions for reproduce: Seemingly random sometimes. Pressing keyboard 
> modifiers such as ALT, CTL. or SHIFT will set it off around 75% of the time. 
> 100% reproducable with this keyboard and chipset. Others have had this 
> problem too using a variety of DEs and WMs.
> 
> Keylog in attachments for last recreation.
> 
> Symptoms: Keyboard will cut out when conditions are met. Sometimes, the last 
> key pressed is then stuck as the input until the device is removed. 
> Reconnecting will:
> 
>   Case 1: Work properly
>   Case 2: Cut out after a single letter
>   Case 3: Take the first input and repeat it indefinitely pending hard reset
> 
> I added "echo -n 'module xhci_hcd =p' > 
> /sys/kernel/debug/dynamic_debug/control" as root and ran dmesg. Output is in 
> attachment.
> 
> It seems as though the device disconnect and driver reinitialization are 
> successful, but the device reset and freeing device rings are not completing. 
> I could be wrong as I am still very new to kernel operation. I would 
> appreciate it if someone could take a look and assist with further debugging.
> 
> Thanks,
> 
> Ben
> 
> 
> 
> 
> 
> 
> 
> 
> 
--
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: xhci: Fix the patch inherit dma configuration from

[kbuild test robot]

Hi Sriram,

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.8-rc7 next-20160921]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]
[Suggest to use git(>=2.9.0) format-patch --base= (or --base=auto for 
convenience) to record what (public, well-known) commit your patch series was 
built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:
https://github.com/0day-ci/linux/commits/Sriram-Dash/usb-xhci-Fix-the-patch-inherit-dma-configuration-from/20160922-004329
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 
usb-testing
config: x86_64-randconfig-x012-201638 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/list.h:8:0,
from include/linux/pci.h:25,
from drivers/usb/host/xhci.c:23:
   drivers/usb/host/xhci.c: In function 'xhci_setup_msi':
>> drivers/usb/host/xhci.c:234:60: error: 'struct usb_bus' has no member named 
>> 'sysdev'
 struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.sysdev);
   ^
   include/linux/kernel.h:831:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^~~
>> drivers/usb/host/xhci.c:234:26: note: in expansion of macro 'to_pci_dev'
 struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.sysdev);
 ^~
   drivers/usb/host/xhci.c: In function 'xhci_free_irq':
   drivers/usb/host/xhci.c:260:59: error: 'struct usb_bus' has no member named 
'sysdev'
 struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.sysdev);
  ^
   include/linux/kernel.h:831:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^~~
   drivers/usb/host/xhci.c:260:25: note: in expansion of macro 'to_pci_dev'
 struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.sysdev);
^~
   drivers/usb/host/xhci.c: In function 'xhci_setup_msix':
   drivers/usb/host/xhci.c:283:45: error: 'struct usb_bus' has no member named 
'sysdev'
 struct pci_dev *pdev = to_pci_dev(hcd->self.sysdev);
^
   include/linux/kernel.h:831:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^~~
   drivers/usb/host/xhci.c:283:25: note: in expansion of macro 'to_pci_dev'
 struct pci_dev *pdev = to_pci_dev(hcd->self.sysdev);
^~
   drivers/usb/host/xhci.c: In function 'xhci_cleanup_msix':
   drivers/usb/host/xhci.c:338:45: error: 'struct usb_bus' has no member named 
'sysdev'
 struct pci_dev *pdev = to_pci_dev(hcd->self.sysdev);
^
   include/linux/kernel.h:831:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^~~
   drivers/usb/host/xhci.c:338:25: note: in expansion of macro 'to_pci_dev'
 struct pci_dev *pdev = to_pci_dev(hcd->self.sysdev);
^~
   drivers/usb/host/xhci.c: In function 'xhci_try_enable_msi':
   drivers/usb/host/xhci.c:377:43: error: 'struct usb_bus' has no member named 
'sysdev'
 pdev = to_pci_dev(xhci_to_hcd(xhci)->self.sysdev);
  ^
   include/linux/kernel.h:831:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^~~
   drivers/usb/host/xhci.c:377:9: note: in expansion of macro 'to_pci_dev'
 pdev = to_pci_dev(xhci_to_hcd(xhci)->self.sysdev);
^~
   drivers/usb/host/xhci.c: In function 'xhci_shutdown':
   drivers/usb/host/xhci.c:746:46: error: 'struct usb_bus' has no member named 
'sysdev'
  usb_disable_xhci_ports(to_pci_dev(hcd->self.sysdev));
 ^
   include/linux/kernel.h:831:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^~~
   drivers/usb/host/xhci.c:746:26: note: in expansion of macro 'to_pci_dev'
  usb_disable_xhci_ports(to_pci_dev(hcd->self.sysdev));
 ^~
   drivers/usb/host/xhci.c:763:43: error: 'struc

[peter.chen-usb:peter-usb-dev 45/57] drivers/usb/Kconfig:39:error: recursive dependency detected!

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git 
peter-usb-dev
head:   559e1f160d721bd3e1c2311096d463176c67877f
commit: 60b2ae6dc9900dc67801db287fa7fc757922d5aa [45/57] usb: chipidea: msm: 
Add reset controller for PHY POR bit
config: avr32-atngw100_defconfig (attached as .config)
compiler: avr-gcc (GCC) 4.9.2
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 60b2ae6dc9900dc67801db287fa7fc757922d5aa
# save the attached .config to linux build tree
make.cross ARCH=avr32 

Note: the peter.chen-usb/peter-usb-dev HEAD 
559e1f160d721bd3e1c2311096d463176c67877f builds fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

   kernel/time/Kconfig:155:warning: range is invalid
>> drivers/usb/Kconfig:39:error: recursive dependency detected!
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/usb/Kconfig:39:  symbol USB is selected by MOUSE_APPLETOUCH
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/input/mouse/Kconfig:187: symbol MOUSE_APPLETOUCH depends on INPUT
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/input/Kconfig:8: symbol INPUT is selected by VT
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/tty/Kconfig:12:  symbol VT is selected by FB_STI
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/video/fbdev/Kconfig:674: symbol FB_STI depends on FB
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/video/fbdev/Kconfig:5:   symbol FB is selected by 
DRM_KMS_FB_HELPER
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/gpu/drm/Kconfig:42:  symbol DRM_KMS_FB_HELPER is selected by 
DRM_KMS_CMA_HELPER
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/gpu/drm/Kconfig:98:  symbol DRM_KMS_CMA_HELPER is selected by DRM_IMX
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/gpu/drm/imx/Kconfig:1:   symbol DRM_IMX depends on IMX_IPUV3_CORE
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/gpu/ipu-v3/Kconfig:1:symbol IMX_IPUV3_CORE depends on 
RESET_CONTROLLER
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/reset/Kconfig:4: symbol RESET_CONTROLLER is selected by 
USB_CHIPIDEA
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/usb/chipidea/Kconfig:1:  symbol USB_CHIPIDEA depends on 
USB_EHCI_HCD
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/usb/host/Kconfig:84: symbol USB_EHCI_HCD depends on USB
--
   kernel/time/Kconfig:155:warning: range is invalid
>> drivers/usb/Kconfig:39:error: recursive dependency detected!
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/usb/Kconfig:39:  symbol USB is selected by MOUSE_APPLETOUCH
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/input/mouse/Kconfig:187: symbol MOUSE_APPLETOUCH depends on INPUT
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/input/Kconfig:8: symbol INPUT is selected by VT
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/tty/Kconfig:12:  symbol VT is selected by FB_STI
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/video/fbdev/Kconfig:674: symbol FB_STI depends on FB
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/video/fbdev/Kconfig:5:   symbol FB is selected by 
DRM_KMS_FB_HELPER
   For a resolution refer to 

Re: [PATCH] usb-storage: block runtime suspend when the host is added

[Alan Stern]

On Wed, 21 Sep 2016, Greg KH wrote:

> On Wed, Sep 21, 2016 at 11:25:37AM -0400, Alan Stern wrote:
> > A warning message that was recently added to the PM core has alerted
> > us to a potential problem in usb-storage.  Although the USB interface
> > is in the runtime active state when the probe routine starts, until
> > the runtime-PM usage count is incremented nothing forces it to remain
> > active.  The increment doesn't happen until after scsi_add_host() is
> > called, and that call causes the interface to go into runtime suspend.
> > 
> > Although this hasn't caused anything to go wrong for anyone, as far as
> > I know, it still is a bug.
> > 
> > To prevent this from happening, move the existing call to
> > usb_autopm_get_interface_no_resume() up from its current position to
> > before scsi_add_host() is called.
> > 
> > Signed-off-by: Alan Stern 
> > Reported-by: Carsten Mattner 
> > Tested-by: Carsten Mattner 
> > CC: 
> 
> It's a bit late for 4.8 for this, can we wait for 4.9-rc1?
> 
> This doesn't apply to my usb-next branch, so things have changed in this
> area, did they solve the issue already?

Heh -- I _thought_ this issue seemed familiar.  :-)

Yes, it has already been fixed by commit a094760b9a77 ("usb: storage: 
fix runtime pm issue in usb_stor_probe2") by Heiner Kallweit.  I even 
Acked his patch, but I didn't remember it.

So Carsten, there's your answer.  The commit will be part of 4.9-rc1, 
unless Greg shoehorns it into 4.8.

But if it doesn't get into 4.8 then it should be marked for
4.8.stable, to prevent those new warning messages from alarming people.

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: [Umap2][7/11][160a:3184] NULL pointer dereference

[Greg KH]

On Wed, Sep 21, 2016 at 06:34:03PM +0200, Oliver Neukum wrote:
> On Thu, 2016-08-18 at 13:50 +0300, Binyamin Sharet wrote:
> > On 08/18/2016 01:31 PM, Oliver Neukum wrote:
> > > On Wed, 2016-08-17 at 14:34 +0300, Binyamin Sharet wrote:
> > >>> After connecting such a device, NULL pointer dereference in the
> > >> kernel.
> > >>> You may need to connect another device or two after this one to
> > >> trigger
> > >>> the oops.
> > >>>
> > >>> Binyamin Sharet
> > >>> Cisco, STARE-C
> > >>>
> > >>> << Attached:  160a_3184_dmesg_1.log >>
> > >>> << Attached:  160a_3184_dmesg_2.log >>
> > >> kernel: 4.8-rc2
> > >> result: reproduced
> [..]
> > >
> > Attached.
> > 
> 
> Hi,
> 
> this fell through the cracks somehow. Does anybody know how
> these devices should be to work with the driver?

Which device was this?  I'm slowly working on a patchset to move this
type of checking to the USB core to hopefully prevent this type of thing
from have to be added to each USB driver individually.

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: crash by cdc_acm driver in kernels 4.8-rc1/5

[Wim Osterholt]

On Wed, Sep 21, 2016 at 02:21:17PM +0200, Oliver Neukum wrote:
> in sysfs.

Google pointed me to /sys/bus/usb/drivers/usb/*
where I find all kinds of 'bConfigurationValue'.
Now is the problem to find which one you could mean.

Under /sys/bus/usb/drivers/usb/7-1  I find
manufacturer  which reads 'Conexant'  and
bConfigurationValue  which reads '1'


Regards, Wim.


--
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: [Umap2][7/11][160a:3184] NULL pointer dereference

[Oliver Neukum]

On Thu, 2016-08-18 at 13:50 +0300, Binyamin Sharet wrote:
> On 08/18/2016 01:31 PM, Oliver Neukum wrote:
> > On Wed, 2016-08-17 at 14:34 +0300, Binyamin Sharet wrote:
> >>> After connecting such a device, NULL pointer dereference in the
> >> kernel.
> >>> You may need to connect another device or two after this one to
> >> trigger
> >>> the oops.
> >>>
> >>> Binyamin Sharet
> >>> Cisco, STARE-C
> >>>
> >>> << Attached:  160a_3184_dmesg_1.log >>
> >>> << Attached:  160a_3184_dmesg_2.log >>
> >> kernel: 4.8-rc2
> >> result: reproduced
[..]
> >
> Attached.
> 

Hi,

this fell through the cracks somehow. Does anybody know how
these devices should be to work with the driver?

Regards
Oliver


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

Re: [PATCH] usb-storage: block runtime suspend when the host is added

[Greg KH]

On Wed, Sep 21, 2016 at 11:25:37AM -0400, Alan Stern wrote:
> A warning message that was recently added to the PM core has alerted
> us to a potential problem in usb-storage.  Although the USB interface
> is in the runtime active state when the probe routine starts, until
> the runtime-PM usage count is incremented nothing forces it to remain
> active.  The increment doesn't happen until after scsi_add_host() is
> called, and that call causes the interface to go into runtime suspend.
> 
> Although this hasn't caused anything to go wrong for anyone, as far as
> I know, it still is a bug.
> 
> To prevent this from happening, move the existing call to
> usb_autopm_get_interface_no_resume() up from its current position to
> before scsi_add_host() is called.
> 
> Signed-off-by: Alan Stern 
> Reported-by: Carsten Mattner 
> Tested-by: Carsten Mattner 
> CC: 

It's a bit late for 4.8 for this, can we wait for 4.9-rc1?

This doesn't apply to my usb-next branch, so things have changed in this
area, did they solve the issue already?

thanks,

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

Re: [PATCH] usb: misc: legousbtower: Fix NULL pointer deference

[Greg KH]

On Mon, Sep 19, 2016 at 07:09:51PM +0100, James wrote:
> From: Greg Kroah-Hartman 
> 
> This patch fixes a NULL pointer dereference caused by a race codition in the
> probe function of the legousbtower driver. It re-structures the probe
> function to only register the interface after successfully reading the
> board's firmware ID.
> 
> The probe function does not deregister the usb interface after an error
> receiving the devices firmware ID. The device file registered
> (/dev/usb/legousbtower%d) may be read/written globally before the probe
> function returns. When tower_delete is called in the probe function
> (after an r/w has been initiated), core dev structures are deleted while
> the file operation functions are still running. If the 0 address is mappable
> on the machine, this vulnerability can be used to create a Local Priviege
> Escalation exploit via a write-what-where condition by remapping
> dev->interrupt_out_buffer in tower_write. A forged USB device and local
> program execution would be required for LPE. The USB
> device would have to delay the control message in tower_probe and accept
> the control urb in tower_open whilst guest code initiated a write to the
> device file as tower_delete is called from the error in tower_probe.
> 
> This bug has existed since 2003. Patch tested by emulated device.
> 
> Reported-by: James Patrick-Evans 
> Tested-by: James Patrick-Evans 
> Signed-off-by: James Patrick-Evans 
> Signed-off-by: Greg Kroah-Hartman 
> ---
> drivers/usb/misc/legousbtower.c | 35 +--
> 1 file changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
> index 7771be3..4dd531a 100644
> --- a/drivers/usb/misc/legousbtower.c
> +++ b/drivers/usb/misc/legousbtower.c
> @@ -898,24 +898,6 @@ static int tower_probe (struct usb_interface *interface, 
> const struct usb_device
>dev->interrupt_in_interval = interrupt_in_interval ? 
> interrupt_in_interval : dev->interrupt_in_endpoint->bInterval;
>dev->interrupt_out_interval = interrupt_out_interval ? 
> interrupt_out_interval : dev->interrupt_out_endpoint->bInterval;
> 
> -   /* we can register the device now, as it is ready */
> -   usb_set_intfdata (interface, dev);
> -
> -   retval = usb_register_dev (interface, _class);
> -
> -   if (retval) {
> -   /* something prevented us from registering this driver */
> -   dev_err(idev, "Not able to get a minor for this device.\n");
> -   usb_set_intfdata (interface, NULL);
> -   goto error;
> -   }
> -   dev->minor = interface->minor;
> -
> -   /* let the user know what node this device is now attached to */
> -   dev_info(>dev, "LEGO USB Tower #%d now attached to major "
> -"%d minor %d\n", (dev->minor - LEGO_USB_TOWER_MINOR_BASE),
> -USB_MAJOR, dev->minor);
> -
>/* get the firmware version and log it */
>result = usb_control_msg (udev,
>  usb_rcvctrlpipe(udev, 0),
> @@ -936,6 +918,23 @@ static int tower_probe (struct usb_interface *interface, 
> const struct usb_device
> get_version_reply.minor,
> le16_to_cpu(get_version_reply.build_no));
> 
> +   /* we can register the device now, as it is ready */
> +   usb_set_intfdata (interface, dev);
> +
> +   retval = usb_register_dev (interface, _class);
> +
> +   if (retval) {
> +   /* something prevented us from registering this driver */
> +   dev_err(idev, "Not able to get a minor for this device.\n");
> +   usb_set_intfdata (interface, NULL);
> +   goto error;
> +   }
> +   dev->minor = interface->minor;
> +
> +   /* let the user know what node this device is now attached to */
> +   dev_info(>dev, "LEGO USB Tower #%d now attached to major "
> +"%d minor %d\n", (dev->minor - LEGO_USB_TOWER_MINOR_BASE),
> +USB_MAJOR, dev->minor);
> 
> exit:
>return retval;
> --

Ugh, all of the tabs got turned into spaces, next time be more careful
with your email client :(

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: crash by cdc_acm driver in kernels 4.8-rc1/5

[Wim Osterholt]

On Wed, Sep 21, 2016 at 02:21:17PM +0200, Oliver Neukum wrote:
> On Tue, 2016-09-20 at 17:45 +0200, Wim Osterholt wrote:
> 
> Anyway, which of its configurations is used?
> Please look up the bConfigurationValue for your device
> in sysfs.

And what might that be?
'locate sysfs' gives one hit at /etc/init.d/sysfs
When I say 'sysfs stop' it stops udev.
When I say 'sysfs start' it says nothing.
Again 'sysfs start' says sysfs already started.
That doesn't have changed anything.

There's a /sys/fs/ext4/* with nothing that you seem to mean.

There's a /proc/sys/fs with nothing that you seem to mean.

There's one mention of acm in /proc/tty/drivers and nowhere I see anything
that might be of any interest somehow.


Regards, Wim.


- w...@djo.tudelft.nl -

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

[PATCH] USB: bcma: drop Northstar PHY 2.0 initialization code

[Rafał Miłecki]

From: Rafał Miłecki 

This driver should initialize controller only, PHY initialization should
be handled by separated PHY driver. We already have phy-bcm-ns-usb2 in
place so let it makes its duty.

Signed-off-by: Rafał Miłecki 
---
 drivers/usb/host/bcma-hcd.c | 80 ++---
 1 file changed, 25 insertions(+), 55 deletions(-)

diff --git a/drivers/usb/host/bcma-hcd.c b/drivers/usb/host/bcma-hcd.c
index e0761d9..5f425c8 100644
--- a/drivers/usb/host/bcma-hcd.c
+++ b/drivers/usb/host/bcma-hcd.c
@@ -239,44 +239,10 @@ static int bcma_hcd_usb20_old_arm_init(struct 
bcma_hcd_device *usb_dev)
return 0;
 }
 
-static void bcma_hcd_init_chip_arm_phy(struct bcma_device *dev)
-{
-   struct bcma_device *arm_core;
-   void __iomem *dmu;
-
-   arm_core = bcma_find_core(dev->bus, BCMA_CORE_ARMCA9);
-   if (!arm_core) {
-   dev_err(>dev, "can not find ARM Cortex A9 ihost core\n");
-   return;
-   }
-
-   dmu = ioremap_nocache(arm_core->addr_s[0], 0x1000);
-   if (!dmu) {
-   dev_err(>dev, "can not map ARM Cortex A9 ihost core\n");
-   return;
-   }
-
-   /* Unlock DMU PLL settings */
-   iowrite32(0xea68, dmu + 0x180);
-
-   /* Write USB 2.0 PLL control setting */
-   iowrite32(0x00dd10c3, dmu + 0x164);
-
-   /* Lock DMU PLL settings */
-   iowrite32(0x, dmu + 0x180);
-
-   iounmap(dmu);
-}
-
-static void bcma_hcd_init_chip_arm_hc(struct bcma_device *dev)
+static void bcma_hcd_usb20_ns_init_hc(struct bcma_device *dev)
 {
u32 val;
 
-   /*
-* Delay after PHY initialized to ensure HC is ready to be configured
-*/
-   usleep_range(1000, 2000);
-
/* Set packet buffer OUT threshold */
val = bcma_read32(dev, 0x94);
val &= 0x;
@@ -287,20 +253,33 @@ static void bcma_hcd_init_chip_arm_hc(struct bcma_device 
*dev)
val = bcma_read32(dev, 0x9c);
val |= 1;
bcma_write32(dev, 0x9c, val);
+
+   /*
+* Broadcom initializes PHY and then waits to ensure HC is ready to be
+* configured. In our case the order is reversed. We just initialized
+* controller and we let HCD initialize PHY, so let's wait (sleep) now.
+*/
+   usleep_range(1000, 2000);
 }
 
-static void bcma_hcd_init_chip_arm(struct bcma_device *dev)
+/**
+ * bcma_hcd_usb20_ns_init - Initialize Northstar USB 2.0 controller
+ */
+static int bcma_hcd_usb20_ns_init(struct bcma_hcd_device *bcma_hcd)
 {
-   bcma_core_enable(dev, 0);
+   struct bcma_device *core = bcma_hcd->core;
+   struct bcma_chipinfo *ci = >bus->chipinfo;
+   struct device *dev = >dev;
 
-   if (dev->bus->chipinfo.id == BCMA_CHIP_ID_BCM4707 ||
-   dev->bus->chipinfo.id == BCMA_CHIP_ID_BCM53018) {
-   if (dev->bus->chipinfo.pkg == BCMA_PKG_ID_BCM4707 ||
-   dev->bus->chipinfo.pkg == BCMA_PKG_ID_BCM4708)
-   bcma_hcd_init_chip_arm_phy(dev);
+   bcma_core_enable(core, 0);
 
-   bcma_hcd_init_chip_arm_hc(dev);
-   }
+   if (ci->id == BCMA_CHIP_ID_BCM4707 ||
+   ci->id == BCMA_CHIP_ID_BCM53018)
+   bcma_hcd_usb20_ns_init_hc(core);
+
+   of_platform_default_populate(dev->of_node, NULL, dev);
+
+   return 0;
 }
 
 static void bcma_hci_platform_power_gpio(struct bcma_device *dev, bool val)
@@ -373,16 +352,7 @@ static int bcma_hcd_usb20_init(struct bcma_hcd_device 
*usb_dev)
if (dma_set_mask_and_coherent(dev->dma_dev, DMA_BIT_MASK(32)))
return -EOPNOTSUPP;
 
-   switch (dev->id.id) {
-   case BCMA_CORE_NS_USB20:
-   bcma_hcd_init_chip_arm(dev);
-   break;
-   case BCMA_CORE_USB20_HOST:
-   bcma_hcd_init_chip_mips(dev);
-   break;
-   default:
-   return -ENODEV;
-   }
+   bcma_hcd_init_chip_mips(dev);
 
/* In AI chips EHCI is addrspace 0, OHCI is 1 */
ohci_addr = dev->addr_s[0];
@@ -451,7 +421,7 @@ static int bcma_hcd_probe(struct bcma_device *core)
err = -ENOTSUPP;
break;
case BCMA_CORE_NS_USB20:
-   err = bcma_hcd_usb20_init(usb_dev);
+   err = bcma_hcd_usb20_ns_init(usb_dev);
break;
case BCMA_CORE_NS_USB30:
err = bcma_hcd_usb30_init(usb_dev);
-- 
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

[PATCH] usb-storage: block runtime suspend when the host is added

[Alan Stern]

A warning message that was recently added to the PM core has alerted
us to a potential problem in usb-storage.  Although the USB interface
is in the runtime active state when the probe routine starts, until
the runtime-PM usage count is incremented nothing forces it to remain
active.  The increment doesn't happen until after scsi_add_host() is
called, and that call causes the interface to go into runtime suspend.

Although this hasn't caused anything to go wrong for anyone, as far as
I know, it still is a bug.

To prevent this from happening, move the existing call to
usb_autopm_get_interface_no_resume() up from its current position to
before scsi_add_host() is called.

Signed-off-by: Alan Stern 
Reported-by: Carsten Mattner 
Tested-by: Carsten Mattner 
CC: 

---


[as1813]


 drivers/usb/storage/usb.c |9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Index: usb-4.x/drivers/usb/storage/usb.c
===
--- usb-4.x.orig/drivers/usb/storage/usb.c
+++ usb-4.x/drivers/usb/storage/usb.c
@@ -1070,17 +1070,18 @@ int usb_stor_probe2(struct us_data *us)
result = usb_stor_acquire_resources(us);
if (result)
goto BadDevice;
+
+   usb_autopm_get_interface_no_resume(us->pusb_intf);
snprintf(us->scsi_name, sizeof(us->scsi_name), "usb-storage %s",
dev_name(>pusb_intf->dev));
result = scsi_add_host(us_to_host(us), dev);
if (result) {
dev_warn(dev,
"Unable to add the scsi host\n");
-   goto BadDevice;
+   goto AddHostFailed;
}
 
/* Submit the delayed_work for SCSI-device scanning */
-   usb_autopm_get_interface_no_resume(us->pusb_intf);
set_bit(US_FLIDX_SCAN_PENDING, >dflags);
 
if (delay_use > 0)
@@ -1090,7 +1091,9 @@ int usb_stor_probe2(struct us_data *us)
return 0;
 
/* We come here if there are any problems */
-BadDevice:
+ AddHostFailed:
+   usb_autopm_put_interface_no_suspend(us->pusb_intf);
+ BadDevice:
usb_stor_dbg(us, "storage_probe() failed\n");
release_everything(us);
return result;

--
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: 4.8-rc: runtime PM trying to activate child device host6 but parent (2-1.2:1.0) is not active

[Alan Stern]

On Tue, 20 Sep 2016, Carsten Mattner wrote:

> On Tue, Sep 20, 2016 at 9:49 PM, Alan Stern  wrote:
> > On Tue, 20 Sep 2016, Carsten Mattner wrote:
> >
> > > On Tue, Sep 20, 2016 at 5:41 PM, Alan Stern  
> > > wrote:
> > >
> > > > I suspect the problem has been there all along, but it simply wasn't
> > > > reported until commit 71723f95463d ("PM / runtime: print error when
> > > > activating a child to unactive parent") was merged in 4.8-rc1.
> > >
> > > Is this just a false positive or a real error that had been silently
> > > ignored all this time?
> >
> > It's a real error, albeit one that is quite unlikely to cause any real
> > harm.  That's why nobody noticed it until the warning message was
> > added.
> 
> Just to be clear, your patch doesn't hide the error but merely silences
> the safe-to-ignore condition after trying a little harder, right?

I don't understand what you mean by "silences the safe-to-ignore
condition" or "trying a little harder".  The patch fixes a real bug --
it prevents the interface from going into runtime suspend at the wrong
time.

Alan Stern

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

RE: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev

[Sriram Dash]

>From: Arnd Bergmann [mailto:a...@arndb.de]
>On Wednesday, September 21, 2016 11:06:47 AM CEST Sriram Dash wrote:
>>
>> Hello Arnd,
>>
>> We tried this patch on NXP platforms (ls2085 and ls1043) which use
>> dwc3 controller without any glue layer. On first go, this did not
>> work. But after minimal reworks mention snippet below, we are able to
>> verify that the USB was working OK.
>>
>>  drivers/usb/host/xhci-mem.c | 12 ++--
>>  drivers/usb/host/xhci.c | 20 ++--
>>
>> -   struct device *dev = xhci_to_hcd(xhci)->self.controller;
>> +   struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
>>
>> We believe the patch needs little modification to work or there might
>> be chances we may have missed something. Any idea?
>
>
>I had not tried the patch, it was just sent for clarification what I meant, so 
>I'm glad
>you got it working with just minimal changes.
>
>Unfortunately, I can't tell from your lines above what exactly you changed, 
>can you
>send that again as a proper patch?
>

Sure.

==
>From 8b0dea1513e9e73a11dcfa802ddc71cca11d66f8 Mon Sep 17 00:00:00 2001
From: Sriram Dash 
Date: Wed, 21 Sep 2016 11:39:30 +0530
Subject: [PATCH] usb: xhci: Fix the patch inherit dma configuration from
 parent dev

Fixes the patch https://patchwork.kernel.org/patch/9319527/
("usb: dwc3: host: inherit dma configuration from parent dev").

Signed-off-by: Sriram Dash 
---
 drivers/usb/host/xhci-mem.c | 12 ++--
 drivers/usb/host/xhci.c | 20 ++--
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6afe323..79608df 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -586,7 +586,7 @@ static void xhci_free_stream_ctx(struct xhci_hcd *xhci,
unsigned int num_stream_ctxs,
struct xhci_stream_ctx *stream_ctx, dma_addr_t dma)
 {
-   struct device *dev = xhci_to_hcd(xhci)->self.controller;
+   struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs;
 
if (size > MEDIUM_STREAM_ARRAY_SIZE)
@@ -614,7 +614,7 @@ static struct xhci_stream_ctx *xhci_alloc_stream_ctx(struct 
xhci_hcd *xhci,
unsigned int num_stream_ctxs, dma_addr_t *dma,
gfp_t mem_flags)
 {
-   struct device *dev = xhci_to_hcd(xhci)->self.controller;
+   struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs;
 
if (size > MEDIUM_STREAM_ARRAY_SIZE)
@@ -1644,7 +1644,7 @@ void xhci_slot_copy(struct xhci_hcd *xhci,
 static int scratchpad_alloc(struct xhci_hcd *xhci, gfp_t flags)
 {
int i;
-   struct device *dev = xhci_to_hcd(xhci)->self.controller;
+   struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
int num_sp = HCS_MAX_SCRATCHPAD(xhci->hcs_params2);
 
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
@@ -1716,7 +1716,7 @@ static void scratchpad_free(struct xhci_hcd *xhci)
 {
int num_sp;
int i;
-   struct device *dev = xhci_to_hcd(xhci)->self.controller;
+   struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
 
if (!xhci->scratchpad)
return;
@@ -1792,7 +1792,7 @@ void xhci_free_command(struct xhci_hcd *xhci,
 
 void xhci_mem_cleanup(struct xhci_hcd *xhci)
 {
-   struct device   *dev = xhci_to_hcd(xhci)->self.controller;
+   struct device   *dev = xhci_to_hcd(xhci)->self.sysdev;
int size;
int i, j, num_ports;
 
@@ -2334,7 +2334,7 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, 
gfp_t flags)
 int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 {
dma_addr_t  dma;
-   struct device   *dev = xhci_to_hcd(xhci)->self.controller;
+   struct device   *dev = xhci_to_hcd(xhci)->self.sysdev;
unsigned intval, val2;
u64 val_64;
struct xhci_segment *seg;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 01d96c9..9a1ff09 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -231,7 +231,7 @@ static int xhci_free_msi(struct xhci_hcd *xhci)
 static int xhci_setup_msi(struct xhci_hcd *xhci)
 {
int ret;
-   struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+   struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.sysdev);
 
ret = pci_enable_msi(pdev);
if (ret) {
@@ -257,7 +257,7 @@ static int xhci_setup_msi(struct xhci_hcd *xhci)
  */
 static void xhci_free_irq(struct xhci_hcd *xhci)
 {
-   struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
+   struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.sysdev);
int ret;
 
/* return if using legacy interrupt */
@@ -280,7 +280,7 @@ static int 

Re: VL805 USB 3.0 does not see connected devices (only on x86_64) (x86 is ok)

[c400]

02:00.0 USB controller: VIA Technologies, Inc. VL805 USB 3.0 Host
Controller (rev 01) (prog-if 30 [XHCI])
Subsystem: VIA Technologies, Inc. VL805 USB 3.0 Host Controller
Flags: bus master, fast devsel, latency 0, IRQ 41
Memory at fbb0 (64-bit, non-prefetchable) [size=4K]
Capabilities: [80] Power Management version 3
Capabilities: [90] MSI: Enable+ Count=1/4 Maskable- 64bit+
Capabilities: [c4] Express Endpoint, MSI 00
Kernel driver in use: xhci_hcd
Kernel modules: xhci_pci
--
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: g_webcam Isoch high bandwidth transfer

[Bin Liu]

On Wed, Sep 21, 2016 at 08:27:02AM -0500, Bin Liu wrote:
> On Wed, Sep 21, 2016 at 11:01:21AM +0300, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > Bin Liu  writes:
> > > Hi,
> > >
> > > I am trying to check Isoch high bandwidth transfer with g_webcam.ko in
> > >  high-speed connection.
> > >
> > > First I hacked webcam.c as follows to enable 640x480@30fps mode.
> > >
> > > diff --git a/drivers/usb/gadget/legacy/webcam.c 
> > > b/drivers/usb/gadget/legacy/webcam.c
> > > index 72c976b..9eb315f 100644
> > > --- a/drivers/usb/gadget/legacy/webcam.c
> > > +++ b/drivers/usb/gadget/legacy/webcam.c
> > > @@ -191,15 +191,15 @@ static const struct UVC_FRAME_UNCOMPRESSED(3) 
> > > uvc_frame_yuv_360p = {
> > > .bFrameIndex= 1,
> > > .bmCapabilities = 0,
> > > .wWidth = cpu_to_le16(640),
> > > -   .wHeight= cpu_to_le16(360),
> > > +   .wHeight= cpu_to_le16(480),
> > > .dwMinBitRate   = cpu_to_le32(18432000),
> > > .dwMaxBitRate   = cpu_to_le32(55296000),
> > > -   .dwMaxVideoFrameBufferSize  = cpu_to_le32(460800),
> > > -   .dwDefaultFrameInterval = cpu_to_le32(66),
> > > +   .dwMaxVideoFrameBufferSize  = cpu_to_le32(614400),
> > > +   .dwDefaultFrameInterval = cpu_to_le32(33),
> > > .bFrameIntervalType = 3,
> > > -   .dwFrameInterval[0] = cpu_to_le32(66),
> > > -   .dwFrameInterval[1] = cpu_to_le32(100),
> > > -   .dwFrameInterval[2] = cpu_to_le32(500),
> > > +   .dwFrameInterval[0] = cpu_to_le32(33),
> > > +   .dwFrameInterval[1] = cpu_to_le32(66),
> > > +   .dwFrameInterval[2] = cpu_to_le32(100),
> > >  };
> > >
> > > then loaded g_webcam.ko as
> > >
> > > # modprobe g_webcam streaming_maxpacket=3072
> > >
> > > The endpoint descriptor showing on the host is
> > >
> > >   Endpoint Descriptor:
> > > bLength 7
> > > bDescriptorType 5
> > > bEndpointAddress 0x8d  EP 13 IN
> > > bmAttributes5
> > >   Transfer TypeIsochronous
> > >   Synch Type   Asynchronous
> > >   Usage Type   Data
> > > wMaxPacketSize 0x1400  3x 1024 bytes
> > > bInterval   1
> > >
> > > However the usb bus trace shows only one transaction with 1024-bytes 
> > > packet in
> > > every SOF. The host only sends one IN packet in every SOF, I am expecting 
> > > 2~3
> > > 1024-bytes transactions, since this would be required to transfer 
> > > 640x480@30fps
> > > YUV frames in high-speed.
> > >
> > > DId I miss anything in the setup?
> > 
> > MUSB or DWC3? This looks like a UDC bug to me. Can you show a screenshot
> 
> Happened on both MUSB and DWC3.

Indeed, it is MUSB gadget driver problem.

> 
> > of your bus analyzer? When host sends IN token, are you replying with
> 
> The trace screenshot on DWC3 is attached.
> 
> > DATA0, DATA1 or DATA2?
> 
> Good hint! It is DATA0!
> 
> > 
> > -- 
> > balbi
> 
> Regards,
> -Bin.


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

Re: VL805 USB 3.0 does not see connected devices (only on x86_64) (x86 is ok)

[Alan Stern]

On Wed, 21 Sep 2016, c400 wrote:

> >Maybe you can put the quirk in /etc/modprobe.d so it will be used
> >automatically.  Add a line to /etc/modprobe.d/local.conf saying:
> 
> >options xhci_hcd quirks=0x00800090
> 
> i`ve done and it works

Congratulations!

Can you post the "lspci -v -s 2:00" output?  Maybe the
XHCI_NO_64BIT_SUPPORT quirk flag should be set for all controllers of
this sort.

Alan Stern

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

Re: [PATCH] mmc: rtsx_usb_sdmmc: Handle runtime PM while changing led

[Alan Stern]

On Wed, 21 Sep 2016, Ulf Hansson wrote:

> > > My concern is also 2s autosuspend timeout which is set for the usb
> > > device. Somehow I feel we need to be able "share" more information
> > > between a parent-child relationship, in this case between the sdmmc
> > > device and the usb device.
> >
> > I agree, but it's not clear how this should be done.  One easy solution
> > would be to turn off USB autosuspend and do all the runtime-PM
> > management in the sdmmc and memstick drivers.
> 
> Hmm, this could be a very good option. In the end the sdmmc/memstick
> drivers knows best about which autosuspend timeout to use.

I tend to agree, and so does Oliver.

> > > An observation I made, is when the sdmmc device gets runtime resumed
> > > (pm_runtime_get_sync()), the parent device (the usb device) may also
> > > become runtime resumed (unless it's already). In this sequence, but
> > > *only* when actually runtime resuming the usb device, the runtime PM
> > > core decides to update the last busy mark for the usb device. Should
> > > it really do that?
> >
> > Yes, that's deliberate.  The whole idea of autosuspend is to prevent
> > the device from being runtime-suspended too soon after it was used, and
> > the PM core considers runtime-resume to be a form of usage.
> 
> I understand it's deliberate, but I was more question whether we
> actually should have the runtime PM core to behaves like this.
> 
> I don't think its behaviour is consistent, as in such case all calls
> to __pm_runtime_resume() should trigger the runtime PM core to update
> the last busy mark.

Not a bad idea...

> Don't get me wrong, I am *not* suggesting we should do that change, as
> it would mean the last busy mark would be updated way too often.

The updates aren't very expensive.  Just one atomic write.  It probably
takes less time than acquiring the runtime-PM spinlock.

> Instead, perhaps it's better to leave the responsibility of updating
> the last busy mark to the runtime PM users solely.

Maybe, but I think doing it once in the core, like this, can remove the
need for a lot of function calls in drivers.

> > > If we assume that the usb device shouldn't be used with a timeout less
> > > than 2s, then I think we have two options:
> > >
> > > *) As the mmc polling timeout is 1s, there is really no point in
> > > trying to runtime suspend the usb device, it may just be left runtime
> > > resumed all the time. Wasting power, of course!
> >
> > Or we can decrease the USB autosuspend delay to 100 ms.
> 
> Yes, something like that makes sense to me.
> 
> Unless we decide to turn off autosuspend completely for the usb host
> as you suggested above. Then it would really become clear that the
> sdmmc/memstick drivers gets the responsible for the autosuspend, which
> certainly makes most sense.

Yes.

> > > **) Add an interface to allow dynamically changes of the mmc polling
> > > timeout to better suit the current user.
> >
> > Note that the block layer does its own polling for removable media, and
> > it already has a sysfs interface to control the polling interval (or
> > disable it entirely).  But I don't know how the MMC stack interacts
> > with the block layer.
> >
> > One awkward point is that the sdmmc and memstick drivers each do their
> > own polling.  This is a waste.  You can see it in the usbmon trace;
> > every second there are two query-response interactions.  Even if
> > there's no good way to reduce the number, we should at least try to
> > synchronize the polls so that the device doesn't need to be resumed
> > twice every second.
> 
> Yes, you are right. I just haven't been able to prioritize doing that
> change for MMC. Another thing added on my mmc TODO list. :-)

To tell the truth, I'm not sure how you would synchronize the polling
activities in the sdmmc and memstick drivers.  Move most of it into the
upper MFD driver?

One point worth mentioning is that if you already know an SDMMC card is
present then there's no reason to poll for a MemoryStick card, and vice
versa.

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: xHCI problem? [was Re: Erratic USB device behavior and device loss]

[Alan Stern]

On Wed, 21 Sep 2016, Ritesh Raj Sarraf wrote:

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA512
> 
> Hi Alan,
> 
> On Tue, 2016-09-20 at 10:16 -0400, Alan Stern wrote:
> > This is a lot better.  No more I/O errors.
> > 
> > We still have irregular suspends and resumes, but that's to be 
> > expected.  More worrying are the spontaneous disconnects.  They don't 
> > seem to be related to the suspend/resume activity.
> > 
> > You can disable suspend for this device entirely by doing:
> > 
> > echo on >/sys/bus/usb/devices/2-4/power/control
> > 
> > I'm afraid that this won't prevent the device from disconnecting
> > itself, though.  This appears to be some sort of hardware bug that
> > can't be fixed in software.
> 
> I'm not sure what you were referring to when you said "No more I/O errors".

I was referring to the attempts at I/O while the device was suspended.  
They didn't occur in your most recent test.

> But I still got these errors today, with all patches applied.
> 
> Sep 21 14:58:11 learner kernel: usb 2-4: new high-speed USB device number 98
> using xhci_hcd
> Sep 21 14:58:18 learner kernel: usb 2-4: new high-speed USB device number 102
> using xhci_hcd
> Sep 21 14:58:24 learner kernel: usb 2-4: new high-speed USB device number 106
> using xhci_hcd
> Sep 21 14:58:31 learner kernel: usb 2-4: new high-speed USB device number 114
> using xhci_hcd
> Sep 21 14:58:41 learner kernel: usb 2-4: new high-speed USB device number 12
> using xhci_hcd
> Sep 21 14:58:41 learner kernel: usb 2-4: device descriptor read/64, error -71
> Sep 21 14:58:41 learner kernel: usb 2-4: device descriptor read/64, error -71
> Sep 21 14:58:41 learner kernel: usb 2-4: new high-speed USB device number 13
> using xhci_hcd

These are a completely different kind of error.  They occurred during a 
reset, which followed one of those spontaneous disconnects.  Probably 
the cause of the disconnect is also the cause of these errors.

Alan Stern

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

Re: [PATCH] mmc: rtsx_usb_sdmmc: Handle runtime PM while changing led

[Alan Stern]

On Wed, 21 Sep 2016, Oliver Neukum wrote:

> On Tue, 2016-09-20 at 10:12 -0400, Alan Stern wrote:
> > On Tue, 20 Sep 2016, Oliver Neukum wrote:
> 
> > That shouldn't be an issue in this case, at least, not with the current 
> > code.  The sdmmc and memstick drivers block autosuspend if media is 
> > present.
> 
> Good.
> > 
> > > > Which means that autosuspend matters only when a card isn't present, 
> > > > and the host is polled every second or so to see whether a card has 
> > > > been inserted.
> > > > 
> > > > Under those circumstances you probably don't want to use
> > > > autosuspend.  
> > > > That is, resuming before each poll and suspending afterward may use 
> > > > less energy than staying at full power all the time.
> > > 
> > > Is that based on concrete figures about power consumption?
> > 
> > No.
> 
> Well, I have no idea how to improve this much without hideous
> overengineering.
> 
> > > And it seems to me that we need a way to indicate that the heuristics
> > > should not be used, but a device immediately suspended. The timer
> > > is sensible only if the next wakeup is unknown.
> > 
> > The driver can always turn off autosuspend if it wants to.
> 
> Yes, but this is not the point. A heuristic with a timeout makes
> sense only if the uses are unpredictable. If you know with a high
> degree of probability when the next activity comes, you ought to either
> suspend now or not all until the next activity.
> 
> Likewise the heuristic is appropriate for leaf nodes. You get nothing
> from a delay on inner nodes.

Almost true, but not quite.  When an inner node has more than one leaf
beneath it, enabling an autosuspend delay for the inner node can make 
sense -- particularly if the leaf activities are uncorrelated.

> Any storage (generic sense) device
> is an inner node. It should suspend immediately after the block
> device which is the leaf node.

Yes.  In this case, however, the USB device has two platform devices 
beneath it: one for SDMMC and one for MemoryStick cards.

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: XHCI: USB 2.0 device unresponsive on 3.0 controller

[Greg KH]

On Wed, Sep 21, 2016 at 08:19:54AM -0500, Ben wrote:
> Good afternoon everyone,
> 
> My coolermaster Quickfire Rapid keyboard (ID: 2516:0004) has been acting up 
> since I installed arch linux for the first time last week. System overview in 
> attachment.
> 
> Bios is set for EHCI hand-off: Enabled; xHCI hand-off: Enabled (Originally 
> Diabled); xHCI: Auto (Tried smart auto and enabled; see Gigabyte specs for 
> expansion on info). On my EHCI ports the keyboard works without issue, so I'm 
> pointing towards the issue being in the xHCI driver.
> 
> Conditions for reproduce: Seemingly random sometimes. Pressing keyboard 
> modifiers such as ALT, CTL. or SHIFT will set it off around 75% of the time. 
> 100% reproducable with this keyboard and chipset. Others have had this 
> problem too using a variety of DEs and WMs.

Does it happen for any other type of device plugged in that is not a
keyboard?

And it looks from your log like you disconnected the devices and then
plugged them back in?  Did you do that manually, or did that happen on
its own?

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: g_webcam Isoch high bandwidth transfer

[Bin Liu]

On Wed, Sep 21, 2016 at 11:01:21AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Bin Liu  writes:
> > Hi,
> >
> > I am trying to check Isoch high bandwidth transfer with g_webcam.ko in
> >  high-speed connection.
> >
> > First I hacked webcam.c as follows to enable 640x480@30fps mode.
> >
> > diff --git a/drivers/usb/gadget/legacy/webcam.c 
> > b/drivers/usb/gadget/legacy/webcam.c
> > index 72c976b..9eb315f 100644
> > --- a/drivers/usb/gadget/legacy/webcam.c
> > +++ b/drivers/usb/gadget/legacy/webcam.c
> > @@ -191,15 +191,15 @@ static const struct UVC_FRAME_UNCOMPRESSED(3) 
> > uvc_frame_yuv_360p = {
> > .bFrameIndex= 1,
> > .bmCapabilities = 0,
> > .wWidth = cpu_to_le16(640),
> > -   .wHeight= cpu_to_le16(360),
> > +   .wHeight= cpu_to_le16(480),
> > .dwMinBitRate   = cpu_to_le32(18432000),
> > .dwMaxBitRate   = cpu_to_le32(55296000),
> > -   .dwMaxVideoFrameBufferSize  = cpu_to_le32(460800),
> > -   .dwDefaultFrameInterval = cpu_to_le32(66),
> > +   .dwMaxVideoFrameBufferSize  = cpu_to_le32(614400),
> > +   .dwDefaultFrameInterval = cpu_to_le32(33),
> > .bFrameIntervalType = 3,
> > -   .dwFrameInterval[0] = cpu_to_le32(66),
> > -   .dwFrameInterval[1] = cpu_to_le32(100),
> > -   .dwFrameInterval[2] = cpu_to_le32(500),
> > +   .dwFrameInterval[0] = cpu_to_le32(33),
> > +   .dwFrameInterval[1] = cpu_to_le32(66),
> > +   .dwFrameInterval[2] = cpu_to_le32(100),
> >  };
> >
> > then loaded g_webcam.ko as
> >
> > # modprobe g_webcam streaming_maxpacket=3072
> >
> > The endpoint descriptor showing on the host is
> >
> >   Endpoint Descriptor:
> > bLength 7
> > bDescriptorType 5
> > bEndpointAddress 0x8d  EP 13 IN
> > bmAttributes5
> >   Transfer TypeIsochronous
> >   Synch Type   Asynchronous
> >   Usage Type   Data
> > wMaxPacketSize 0x1400  3x 1024 bytes
> > bInterval   1
> >
> > However the usb bus trace shows only one transaction with 1024-bytes packet 
> > in
> > every SOF. The host only sends one IN packet in every SOF, I am expecting 
> > 2~3
> > 1024-bytes transactions, since this would be required to transfer 
> > 640x480@30fps
> > YUV frames in high-speed.
> >
> > DId I miss anything in the setup?
> 
> MUSB or DWC3? This looks like a UDC bug to me. Can you show a screenshot

Happened on both MUSB and DWC3.

> of your bus analyzer? When host sends IN token, are you replying with

The trace screenshot on DWC3 is attached.

> DATA0, DATA1 or DATA2?

Good hint! It is DATA0!

> 
> -- 
> balbi

Regards,
-Bin.

Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev

[Arnd Bergmann]

On Wednesday, September 21, 2016 11:43:59 AM CEST Sriram Dash wrote:
> >From: Arnd Bergmann [mailto:a...@arndb.de]
> >On Wednesday, September 21, 2016 11:06:47 AM CEST Sriram Dash wrote:
> 
> ==
> From 8b0dea1513e9e73a11dcfa802ddc71cca11d66f8 Mon Sep 17 00:00:00 2001
> From: Sriram Dash 
> Date: Wed, 21 Sep 2016 11:39:30 +0530
> Subject: [PATCH] usb: xhci: Fix the patch inherit dma configuration from
>  parent dev
> 
> Fixes the patch https://patchwork.kernel.org/patch/9319527/
> ("usb: dwc3: host: inherit dma configuration from parent dev").
> 
> Signed-off-by: Sriram Dash 
> ---
>  drivers/usb/host/xhci-mem.c | 12 ++--
>  drivers/usb/host/xhci.c | 20 ++--
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 6afe323..79608df 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c

All the changes you did to this file seem fine, I completely missed that part.

> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 01d96c9..9a1ff09 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -231,7 +231,7 @@ static int xhci_free_msi(struct xhci_hcd *xhci)
>  static int xhci_setup_msi(struct xhci_hcd *xhci)
>  {
>   int ret;
> - struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> + struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.sysdev);
>  
>   ret = pci_enable_msi(pdev);
>   if (ret) {

This one is interesting as I stumbled over some code comment mentioning
that for dwc3-pci, we don't support MSI. I think with this change,
we /can/ actually support MSI, but this could be a separate patch
and we'd have to test it on dwc3-pci hardware. Same for most of
this file.

> @@ -745,7 +745,7 @@ void xhci_shutdown(struct usb_hcd *hcd)
>   struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>  
>   if (xhci->quirks & XHCI_SPURIOUS_REBOOT)
> - usb_disable_xhci_ports(to_pci_dev(hcd->self.controller));
> + usb_disable_xhci_ports(to_pci_dev(hcd->self.sysdev));
>  
>   spin_lock_irq(>lock);
>   xhci_halt(xhci);

This seems obviously correct, but I don't yet see why it currently
works. We probably don't call this function on dwc3.

>  #ifdef CONFIG_PM
> @@ -3605,7 +3605,7 @@ void xhci_free_dev(struct usb_hcd *hcd, struct 
> usb_device *udev)
>* if no devices remain.
>*/
>   if (xhci->quirks & XHCI_RESET_ON_RESUME)
> - pm_runtime_put_noidle(hcd->self.controller);
> + pm_runtime_put_noidle(hcd->self.sysdev);
>  #endif
>  
>   ret = xhci_check_args(hcd, udev, NULL, 0, true, __func__);

I suspect this one is wrong, based on what Felipe explained earlier:
the power management should propagate down from the child to the
parent device.

Someone who understands this better than I do should look at it.

> @@ -3745,7 +3745,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct 
> usb_device *udev)
>* suspend if there is a device attached.
>*/
>   if (xhci->quirks & XHCI_RESET_ON_RESUME)
> - pm_runtime_get_noresume(hcd->self.controller);
> + pm_runtime_get_noresume(hcd->self.sysdev);
>  #endif
>  
>  

Same here.

> @@ -4834,7 +4834,7 @@ int xhci_get_frame(struct usb_hcd *hcd)
>  int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
>  {
>   struct xhci_hcd *xhci;
> - struct device   *dev = hcd->self.controller;
> + struct device   *dev = hcd->self.sysdev;
>   int retval;


This one calls

get_quirks(dev, xhci);

not sure whether this should be called with self.controller or
self.sysdev, we should audit every one of the callers here to
be sure:

drivers/usb/host/xhci-mtk.c:return xhci_gen_setup(hcd, xhci_mtk_quirks);
drivers/usb/host/xhci-pci.c:retval = xhci_gen_setup(hcd, xhci_pci_quirks);
drivers/usb/host/xhci-plat.c:   return xhci_gen_setup(hcd, xhci_plat_quirks);
drivers/usb/host/xhci-rcar.c:* xhci_gen_setup().
drivers/usb/host/xhci-tegra.c:  return xhci_gen_setup(hcd, tegra_xhci_quirks);

Arnd
--
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: crash by cdc_acm driver in kernels 4.8-rc1/5

[Oliver Neukum]

On Tue, 2016-09-20 at 17:45 +0200, Wim Osterholt wrote:
> On Tue, Sep 20, 2016 at 03:05:14PM +0200, Oliver Neukum wrote:
> > 
> > I cannot replicate it. Could you please provide "lsusb -v"?
> > 
> > Regards
> > Oliver
> 
> It concerns these type of modems:
> http://www.ebay.nl/itm/191933738340
> http://www.ebay.nl/itm/121590899044

OK. These devices are unusual in having two outputs.
I've ordered a device, but it will take weeks to ship.
It is definitely valuable for testing.

Anyway, which of its configurations is used?
Please look up the bConfigurationValue for your device
in sysfs.

Regards
Oliver


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

Re: xHCI problem? [was Re: Erratic USB device behavior and device loss]

[Ritesh Raj Sarraf]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

Hello Ulf,

On Wed, 2016-09-21 at 13:17 +0200, Ulf Hansson wrote:
> 
> I am pretty sure the memstick driver causes additional access to the
> usb device without first calling pm_runtime_get_sync(). To eliminate
> those cases from causing the issues, could you try disable the
> memstick driver all-together?

I'm assuming you are referring to the rtsx_usb_ms driver ?

What is the oddest thing right now, is that none of the rtsx modules are
reported loaded.

rrs@learner:~$ lsmod | grep -i rts
2016-09-21 / 17:07:08 ♒♒♒  ☹  => 1  

where as the module was built for the kernel, and does load when asked manually.

rrs@learner:~$ less /boot/config-4.8.0-rc7alxb+ 
2016-09-21 / 17:07:54 ♒♒♒  ☺  
rrs@learner:~$ find /lib/modules/4.8.0-rc7alxb+/ | grep rtsx
/lib/modules/4.8.0-rc7alxb+/kernel/drivers/mmc/host/rtsx_usb_sdmmc.ko
/lib/modules/4.8.0-rc7alxb+/kernel/drivers/mmc/host/rtsx_pci_sdmmc.ko
/lib/modules/4.8.0-rc7alxb+/kernel/drivers/mfd/rtsx_usb.ko
/lib/modules/4.8.0-rc7alxb+/kernel/drivers/mfd/rtsx_pci.ko
/lib/modules/4.8.0-rc7alxb+/kernel/drivers/memstick/host/rtsx_pci_ms.ko
/lib/modules/4.8.0-rc7alxb+/kernel/drivers/memstick/host/rtsx_usb_ms.ko
2016-09-21 / 17:08:09 ♒♒♒  ☺  

rrs@learner:~$ sudo modprobe rtsx-usb-sdmmc
2016-09-21 / 17:08:46 ♒♒♒  ☺  

rrs@learner:~$ dmesg | tail -n 5
[ 6870.017311] usb 2-4: Device not responding to setup address.
[ 6870.223471] usb 2-4: device not accepting address 19, error -71
[ 6870.223536] usb usb2-port4: unable to enumerate USB device
[ 7958.474543] [drm:intel_pipe_update_end [i915]] *ERROR* Atomic update failure
on pipe A (start=166356 end=166357) time 3 us, min 1073, max 1079, scanline
start 1088, end 1080
[ 9814.785241] usbcore: registered new interface driver rtsx_usb
2016-09-21 / 17:10:07 ♒♒♒  ☺  



- -- 
Ritesh Raj Sarraf
RESEARCHUT - http://www.researchut.com
"Necessity is the mother of invention."
-BEGIN PGP SIGNATURE-

iQIcBAEBCgAGBQJX4nIMAAoJEKY6WKPy4XVpg4gP/Rkp5Wje2vTUHwnrJZzAKd65
V1VWkTyCfpbCTw4nAyMbzDevyoXJf3P+ktrcyQonvoZr/dsd/LOmjSXcjNJaFX0z
Vj+1lGOZeN1mr6vKMbh2y188oU4RyHkeCfg7SNdo1VuhVST/m2jOecCznXuEtpiS
TI66lmre0SNRusKHRQNDtaP4hFW0KDBmtXvc8pnNOL5781qua0pF12VIP5SsqriP
3d/DcqlVR0Lqh7X7wdt5Knp9ilSvsrCgGbimBarRUYnrOrhklJH9UwKXcHWVypzt
M0XvjO7R6JrBUoM6s8EA6gKmNxwlsIrjKFUFlTT6HPLb52fjpYkbYqCRUmqyIJZB
t3uA0hNKcqLav+Fg7ugT6ePAPeVkANDnbrPE+g69KOtM/CEJHbHkLxqznb/0lpMU
+SAp/Jz1CmDLt8M3s8gS9iCUWVrWy1oyDpMQsYIrTYOG6oOEOoTYEf/g5D6PBtY0
r1tD5bU/cZJV61YKer2xRDNu1YbAYkvX3XskFD7DFsnZpCyBXKGZ7gWpWety9kJJ
iBiqvn5Rk7jvL6EhIb/TQ867QLmhQCzZumPClFM4z7b8G2E440vykw5D5sKv91+H
qu9Abcxe0R0T9pxCFRz+//DYlxGvDDlUSyNBkrv6aGS1rNeDY/e4FWbNuEz9UoGn
LNiunOtwzBndajfEFETk
=tEAz
-END PGP SIGNATURE-

--
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: dwc3: host: inherit dma configuration from parent dev

[Sriram Dash]

>From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-ow...@vger.kernel.org]
>On Wednesday, September 7, 2016 1:24:07 PM CEST Felipe Balbi wrote:
>>
>> Hi,
>>
>> Arnd Bergmann  writes:
>>
>> [...]
>>
>> > Regarding the DMA configuration that you mention in
>> > ci_hdrc_add_device(), I think we should replace
>> >
>> > pdev->dev.dma_mask = dev->dma_mask;
>> > pdev->dev.dma_parms = dev->dma_parms;
>> > dma_set_coherent_mask(>dev, dev->coherent_dma_mask);
>> >
>> > with of_dma_configure(), which has the chance to configure more than
>> > just those three, as the dma API might look into different aspects:
>> >
>> > - iommu specific configuration
>> > - cache coherency information
>> > - bus type
>> > - dma offset
>> > - dma_map_ops pointer
>> >
>> > We try to handle everything in of_dma_configure() at configuration
>> > time, and that would be the place to add anything else that we might
>> > need in the future.
>>
>> There are a couple problems with this:
>>
>> 1) won't work for PCI-based systems.
>>
>> DWC3 is used in production PCI-based HW and also in Synopsys HAPS DX
>> platform (FPGA that appears like a PCI card to host PC)
>
>Right, I was specifically talking about the code in chipidea here, which I 
>think is
>never used on the PCI bus, and how the current code is broken. We can probably 
>do
>better than of_dma_configure() (see below), but it would be an improvement.
>
>> 2) not very robust solution
>>
>> of_dma_configure() will hardcode 32-bit DMA dmask for xhci-plat
>> because that's not created by DT. The only reason why this works at
>> all is because of the default 32-bit dma mask thing :-) So, how is it
>> any different than copying 32-bit dma mask from parent?
>
>The idea here is that you pass in the parent of_node along with the child 
>device
>pointer, so it would behave exactly like the parent already does.
>The difference is that it also handles all the other attributes besides the 
>mask.
>
>However, to summarize the discussion so far, I agree that
>of_dma_configure() is not the solution to these problems, and I think we can do
>much better:
>
>Splitting the usb_bus->controller field into the Linux-internal device (used 
>for the
>sysfs hierarchy, for printks and for power management) and a new pointer (used 
>for
>DMA, DT enumeration and phy lookup) probably covers all that we really need.
>
>I've prototyped it below, with the dwc3, xhci and chipidea changes together 
>with
>the core changes. I've surely made mistakes there and don't expect it to work 
>out
>of the box, but this should give an idea of how I think this can all be solved 
>in the
>least invasive way.
>

Hello Arnd,

We tried this patch on NXP platforms (ls2085 and ls1043) which use dwc3 
controller without any glue layer. On first go, this did not work. But after
minimal reworks mention snippet below, we are able to verify that the USB
was working OK.

 drivers/usb/host/xhci-mem.c | 12 ++--
 drivers/usb/host/xhci.c | 20 ++--

-   struct device *dev = xhci_to_hcd(xhci)->self.controller;
+   struct device *dev = xhci_to_hcd(xhci)->self.sysdev;

We believe the patch needs little modification to work or there might be chances
we may have missed something. Any idea?

Regards,
Sriram

>I noticed that the gadget interface already has a way to handle the DMA 
>allocation
>by device, so I added that in as well.
>
>Signed-off-by: Arnd Bergmann 
>
> drivers/usb/chipidea/core.c|  4 
> drivers/usb/chipidea/host.c|  3 ++-
> drivers/usb/chipidea/udc.c |  8 
> drivers/usb/core/buffer.c  | 12 ++--
> drivers/usb/core/hcd.c | 48 
> +--
>-
> drivers/usb/core/usb.c | 16 
> drivers/usb/dwc3/core.c| 28 +++-
> drivers/usb/dwc3/core.h|  1 +
> drivers/usb/dwc3/dwc3-exynos.c | 10 --
> drivers/usb/dwc3/dwc3-st.c |  1 -
> drivers/usb/dwc3/ep0.c |  8 
> drivers/usb/dwc3/gadget.c  | 34 +-
> drivers/usb/dwc3/host.c| 13 -
> drivers/usb/host/ehci-fsl.c|  4 ++--
> drivers/usb/host/xhci-plat.c   | 32 +---
> include/linux/usb.h|  1 +
> include/linux/usb/hcd.h|  3 +++
>
>diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index
>69426e644d17..dff69837b349 100644
>--- a/drivers/usb/chipidea/core.c
>+++ b/drivers/usb/chipidea/core.c
>@@ -833,10 +833,6 @@ struct platform_device *ci_hdrc_add_device(struct device
>*dev,
>   }
>
>   pdev->dev.parent = dev;
>-  pdev->dev.dma_mask = dev->dma_mask;
>-  pdev->dev.dma_parms = dev->dma_parms;
>-  dma_set_coherent_mask(>dev, dev->coherent_dma_mask);
>-
>   ret = platform_device_add_resources(pdev, res, nres);
>   if (ret)
>   goto err;
>diff --git a/drivers/usb/chipidea/host.c 

Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev

[Arnd Bergmann]

On Wednesday, September 21, 2016 11:06:47 AM CEST Sriram Dash wrote:
> 
> Hello Arnd,
> 
> We tried this patch on NXP platforms (ls2085 and ls1043) which use dwc3 
> controller without any glue layer. On first go, this did not work. But after
> minimal reworks mention snippet below, we are able to verify that the USB
> was working OK.
> 
>  drivers/usb/host/xhci-mem.c | 12 ++--
>  drivers/usb/host/xhci.c | 20 ++--
> 
> -   struct device *dev = xhci_to_hcd(xhci)->self.controller;
> +   struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
> 
> We believe the patch needs little modification to work or there might be 
> chances
> we may have missed something. Any idea?


I had not tried the patch, it was just sent for clarification what I
meant, so I'm glad you got it working with just minimal changes.

Unfortunately, I can't tell from your lines above what exactly you
changed, can you send that again as a proper patch?

I think I also had some minimal changes that I did myself in order
to fix a build regression I introduced.

Arnd
--
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: xHCI problem? [was Re: Erratic USB device behavior and device loss]

[Ulf Hansson]

On 21 September 2016 at 13:10, Ritesh Raj Sarraf  wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA512
>
> Hi Alan,
>
> On Tue, 2016-09-20 at 10:16 -0400, Alan Stern wrote:
>> This is a lot better.  No more I/O errors.
>>
>> We still have irregular suspends and resumes, but that's to be
>> expected.  More worrying are the spontaneous disconnects.  They don't
>> seem to be related to the suspend/resume activity.
>>
>> You can disable suspend for this device entirely by doing:
>>
>> echo on >/sys/bus/usb/devices/2-4/power/control
>>
>> I'm afraid that this won't prevent the device from disconnecting
>> itself, though.  This appears to be some sort of hardware bug that
>> can't be fixed in software.
>
> I'm not sure what you were referring to when you said "No more I/O errors".
> But I still got these errors today, with all patches applied.
>
> Sep 21 14:58:11 learner kernel: usb 2-4: new high-speed USB device number 98
> using xhci_hcd
> Sep 21 14:58:18 learner kernel: usb 2-4: new high-speed USB device number 102
> using xhci_hcd
> Sep 21 14:58:24 learner kernel: usb 2-4: new high-speed USB device number 106
> using xhci_hcd
> Sep 21 14:58:31 learner kernel: usb 2-4: new high-speed USB device number 114
> using xhci_hcd
> Sep 21 14:58:41 learner kernel: usb 2-4: new high-speed USB device number 12
> using xhci_hcd
> Sep 21 14:58:41 learner kernel: usb 2-4: device descriptor read/64, error -71
> Sep 21 14:58:41 learner kernel: usb 2-4: device descriptor read/64, error -71
> Sep 21 14:58:41 learner kernel: usb 2-4: new high-speed USB device number 13
> using xhci_hcd
> Sep 21 14:58:41 learner kernel: usb 2-4: device descriptor read/64, error -71
> Sep 21 14:58:41 learner kernel: usb 2-4: device descriptor read/64, error -71
> Sep 21 14:58:42 learner kernel: usb 2-4: new high-speed USB device number 14
> using xhci_hcd
> Sep 21 14:58:42 learner kernel: usb 2-4: Device not responding to setup 
> address.
> Sep 21 14:58:42 learner kernel: usb 2-4: Device not responding to setup 
> address.
> Sep 21 14:58:42 learner kernel: usb 2-4: device not accepting address 14, 
> error
> - -71
> Sep 21 14:58:42 learner kernel: usb 2-4: new high-speed USB device number 15
> using xhci_hcd
> Sep 21 14:58:42 learner kernel: usb 2-4: Device not responding to setup 
> address.
> Sep 21 14:58:42 learner kernel: usb 2-4: Device not responding to setup 
> address.
> Sep 21 14:58:43 learner kernel: usb 2-4: device not accepting address 15, 
> error
> - -71
> Sep 21 14:58:43 learner kernel: usb usb2-port4: unable to enumerate USB device
> Sep 21 16:19:39 learner kernel: ahci :00:1f.2: port does not support 
> device
> sleep
> Sep 21 16:19:39 learner kernel: NMI watchdog: enabled on all CPUs, permanently
> consumes one hw-
> Sep 21 16:19:39 learner kernel: EXT4-fs (dm-0): re-mounted. Opts:
> errors=remount-ro,data=ordere
> Sep 21 16:19:39 learner kernel: EXT4-fs (sda6): re-mounted. Opts:
> data=ordered,commit=0
> Sep 21 16:19:39 learner kernel: EXT4-fs (dm-3): re-mounted. Opts:
> errors=remount-ro,data=writeb
> Sep 21 16:19:39 learner kernel: usb 2-4: new high-speed USB device number 16
> using xhci_hcd
> Sep 21 16:19:39 learner kernel: usb 2-4: device descriptor read/64, error -71
> Sep 21 16:19:40 learner kernel: usb 2-4: device descriptor read/64, error -71
> Sep 21 16:19:40 learner kernel: usb 2-4: new high-speed USB device number 17
> using xhci_hcd
> Sep 21 16:19:40 learner kernel: usb 2-4: device descriptor read/64, error -71
> Sep 21 16:19:40 learner kernel: usb 2-4: device descriptor read/64, error -71
> Sep 21 16:19:40 learner kernel: usb 2-4: new high-speed USB device number 18
> using xhci_hcd
> Sep 21 16:19:40 learner kernel: usb 2-4: Device not responding to setup 
> address.
> Sep 21 16:19:41 learner kernel: usb 2-4: Device not responding to setup 
> address.
> Sep 21 16:19:41 learner kernel: usb 2-4: device not accepting address 18, 
> error
> - -71
> Sep 21 16:19:41 learner kernel: usb 2-4: new high-speed USB device number 19
> using xhci_hcd
>

I am pretty sure the memstick driver causes additional access to the
usb device without first calling pm_runtime_get_sync(). To eliminate
those cases from causing the issues, could you try disable the
memstick driver all-together?

Kind regards
Uffe
--
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: xHCI problem? [was Re: Erratic USB device behavior and device loss]

[Ritesh Raj Sarraf]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

Hi Alan,

On Tue, 2016-09-20 at 10:16 -0400, Alan Stern wrote:
> This is a lot better.  No more I/O errors.
> 
> We still have irregular suspends and resumes, but that's to be 
> expected.  More worrying are the spontaneous disconnects.  They don't 
> seem to be related to the suspend/resume activity.
> 
> You can disable suspend for this device entirely by doing:
> 
> echo on >/sys/bus/usb/devices/2-4/power/control
> 
> I'm afraid that this won't prevent the device from disconnecting
> itself, though.  This appears to be some sort of hardware bug that
> can't be fixed in software.

I'm not sure what you were referring to when you said "No more I/O errors".
But I still got these errors today, with all patches applied.

Sep 21 14:58:11 learner kernel: usb 2-4: new high-speed USB device number 98
using xhci_hcd
Sep 21 14:58:18 learner kernel: usb 2-4: new high-speed USB device number 102
using xhci_hcd
Sep 21 14:58:24 learner kernel: usb 2-4: new high-speed USB device number 106
using xhci_hcd
Sep 21 14:58:31 learner kernel: usb 2-4: new high-speed USB device number 114
using xhci_hcd
Sep 21 14:58:41 learner kernel: usb 2-4: new high-speed USB device number 12
using xhci_hcd
Sep 21 14:58:41 learner kernel: usb 2-4: device descriptor read/64, error -71
Sep 21 14:58:41 learner kernel: usb 2-4: device descriptor read/64, error -71
Sep 21 14:58:41 learner kernel: usb 2-4: new high-speed USB device number 13
using xhci_hcd
Sep 21 14:58:41 learner kernel: usb 2-4: device descriptor read/64, error -71
Sep 21 14:58:41 learner kernel: usb 2-4: device descriptor read/64, error -71
Sep 21 14:58:42 learner kernel: usb 2-4: new high-speed USB device number 14
using xhci_hcd
Sep 21 14:58:42 learner kernel: usb 2-4: Device not responding to setup address.
Sep 21 14:58:42 learner kernel: usb 2-4: Device not responding to setup address.
Sep 21 14:58:42 learner kernel: usb 2-4: device not accepting address 14, error
- -71
Sep 21 14:58:42 learner kernel: usb 2-4: new high-speed USB device number 15
using xhci_hcd
Sep 21 14:58:42 learner kernel: usb 2-4: Device not responding to setup address.
Sep 21 14:58:42 learner kernel: usb 2-4: Device not responding to setup address.
Sep 21 14:58:43 learner kernel: usb 2-4: device not accepting address 15, error
- -71
Sep 21 14:58:43 learner kernel: usb usb2-port4: unable to enumerate USB device
Sep 21 16:19:39 learner kernel: ahci :00:1f.2: port does not support device
sleep
Sep 21 16:19:39 learner kernel: NMI watchdog: enabled on all CPUs, permanently
consumes one hw-
Sep 21 16:19:39 learner kernel: EXT4-fs (dm-0): re-mounted. Opts:
errors=remount-ro,data=ordere
Sep 21 16:19:39 learner kernel: EXT4-fs (sda6): re-mounted. Opts:
data=ordered,commit=0
Sep 21 16:19:39 learner kernel: EXT4-fs (dm-3): re-mounted. Opts:
errors=remount-ro,data=writeb
Sep 21 16:19:39 learner kernel: usb 2-4: new high-speed USB device number 16
using xhci_hcd
Sep 21 16:19:39 learner kernel: usb 2-4: device descriptor read/64, error -71
Sep 21 16:19:40 learner kernel: usb 2-4: device descriptor read/64, error -71
Sep 21 16:19:40 learner kernel: usb 2-4: new high-speed USB device number 17
using xhci_hcd
Sep 21 16:19:40 learner kernel: usb 2-4: device descriptor read/64, error -71
Sep 21 16:19:40 learner kernel: usb 2-4: device descriptor read/64, error -71
Sep 21 16:19:40 learner kernel: usb 2-4: new high-speed USB device number 18
using xhci_hcd
Sep 21 16:19:40 learner kernel: usb 2-4: Device not responding to setup address.
Sep 21 16:19:41 learner kernel: usb 2-4: Device not responding to setup address.
Sep 21 16:19:41 learner kernel: usb 2-4: device not accepting address 18, error
- -71
Sep 21 16:19:41 learner kernel: usb 2-4: new high-speed USB device number 19
using xhci_hcd


- -- 
Ritesh Raj Sarraf
RESEARCHUT - http://www.researchut.com
"Necessity is the mother of invention."
-BEGIN PGP SIGNATURE-

iQIcBAEBCgAGBQJX4mqUAAoJEKY6WKPy4XVpEHMP/RhcDQXxt3LTOpGhJizyqZ4z
7Sm1tcBe/4NKP80nUpiI0geQYHYfRTR93hGKFayp48ULstn8xJ8T3ItZIS0WmZDK
TJcdxXzrkWMGNAGQFjNd9Lk1C7h1IIuo2D5xDuhrpHGMc5y4UVmpPixQRwEnbzG9
zX+PabvummAmlzT1+cRyO10uwpGFzsJ3SDkokjkxZ/aViL+vBU58/qiXIFH1D1hX
KTY8ABZjh4Hnkw07EcQh0xKztEbE/v2wJWPSx4RCPbsRdO5vdKUtOtWB7+1WVAY3
noSrvNWjj0Ntnm0+t4XIid1fDmNumK0EcYe8fDb/GqAuYDTqjcIZ5ANCaVSM/joq
suY7KTXVe44Pol1Bb89lERR49QAkxyKJViNc0bNSkp0+F4u4cDW9o0q6s0X6xw5b
LdAQHQek92IRNmT7v4gYO9bUKUBurqgHuUdi3iYlylbvs8UAzHmOL3nrFBz2GIcG
KQvqmvENy31VIlIMx+k3SipyedG77LIAmxX8bG7Xlu8lSZz3sPkMz7RJYeW0QwQ6
lC2cWiF2cn5K/0eTQPW3MX5H9m5qlq0QGaDrf8kGX6XpRKR3Qsu98L+R+AAmViQ9
kd2eBFzL4JdNVhXgNWrNk5mr0R0D9RB/58YWize3sASPg75zCFQCNOoFTPNZGN1q
edPs8uwkN5O2cy+0ur8n
=uZvc
-END PGP SIGNATURE-

--
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] mmc: rtsx_usb_sdmmc: Handle runtime PM while changing led

[Ulf Hansson]

On 20 September 2016 at 16:09, Alan Stern  wrote:
>
> On Tue, 20 Sep 2016, Ulf Hansson wrote:
>
> > >> I am wondering what you think would be a good autosuspend timeout in
> > >> this case? It seems to me that the only thing the rtsx driver really
> > >> care about is to tell the parent device that it needs to be runtime
> > >> resumed during a certain timeframe, more or less it would like to
> > >> inherit the parents settings.
> > >>
> > >> Other mmc hosts, not being usb-mmc devices, are using an autosuspend
> > >> timeout of ~50-100ms but that doesn't seem like good value here,
> > >> right?
> > >
> > > Well, if you decide to let the device go into runtime suspend between
> > > polls then there's no reason to use autosuspend at all.  Once a poll
> > > has ended, you know there won't be any more activity until the next
> > > poll.
> >
> > We could change that, as currently the approach in the mmc core isn't
> > that sophisticated. I even think this has been discussed earlier for
> > the very similar reasons regards polling card detect mode.
> >
> > I guess the main reason to why we yet have changed this, is because
> > mmc host drivers are using an autosuspend timeout of ~50-100 ms, so in
> > the end it haven't been such a big deal.
>
> No, it isn't.  Although at a polling interval of 1 second, it means
> reducing the low-power time by as much as 10%.

Yes, I agree we shouldn't neglect its impact - especially in this usb use case.

I will put it on my TODO list for mmc PM things.

>
> > > On the other hand, if you decide to keep the device at full power all
> > > the time during polling, then any autosuspend timeout larger than 1000
> > > ms would do what you want.
> > >
> > > Mostly I'm concerned about how this will interact with the USB runtime
> > > PM.  The thing is, suspending the sdmmc device doesn't save any energy,
> > > whereas suspending the USB device does.
> >
> > Yes, I agree.
> >
> > My concern is also 2s autosuspend timeout which is set for the usb
> > device. Somehow I feel we need to be able "share" more information
> > between a parent-child relationship, in this case between the sdmmc
> > device and the usb device.
>
> I agree, but it's not clear how this should be done.  One easy solution
> would be to turn off USB autosuspend and do all the runtime-PM
> management in the sdmmc and memstick drivers.

Hmm, this could be a very good option. In the end the sdmmc/memstick
drivers knows best about which autosuspend timeout to use.

>
> > An observation I made, is when the sdmmc device gets runtime resumed
> > (pm_runtime_get_sync()), the parent device (the usb device) may also
> > become runtime resumed (unless it's already). In this sequence, but
> > *only* when actually runtime resuming the usb device, the runtime PM
> > core decides to update the last busy mark for the usb device. Should
> > it really do that?
>
> Yes, that's deliberate.  The whole idea of autosuspend is to prevent
> the device from being runtime-suspended too soon after it was used, and
> the PM core considers runtime-resume to be a form of usage.

I understand it's deliberate, but I was more question whether we
actually should have the runtime PM core to behaves like this.

I don't think its behaviour is consistent, as in such case all calls
to __pm_runtime_resume() should trigger the runtime PM core to update
the last busy mark.

Don't get me wrong, I am *not* suggesting we should do that change, as
it would mean the last busy mark would be updated way too often.
Instead, perhaps it's better to leave the responsibility of updating
the last busy mark to the runtime PM users solely.

>
> > Moreover, I am curious about the 2s usb timeout. Why isn't that chosen
> > to something like ~100ms instead? Is there is a long latency to
> > runtime resume the usb device or because we fear to wear out the HW,
> > which may be powered on/off too frequently?
>
> When I first implemented runtime PM for the USB stack, I had to choose
> a autosuspend timeout.  Not having any basis for such a choice, and
> figuring that a suspend-resume cycle takes around 100 ms, I decided
> that 2 seconds would be a reasonable value.  But it's just a default;
> drivers and userspace can change it whenever they want.

Okay, I see. Thanks for sharing that information.

>
> > If we assume that the usb device shouldn't be used with a timeout less
> > than 2s, then I think we have two options:
> >
> > *) As the mmc polling timeout is 1s, there is really no point in
> > trying to runtime suspend the usb device, it may just be left runtime
> > resumed all the time. Wasting power, of course!
>
> Or we can decrease the USB autosuspend delay to 100 ms.

Yes, something like that makes sense to me.

Unless we decide to turn off autosuspend completely for the usb host
as you suggested above. Then it would really become clear that the
sdmmc/memstick drivers gets the responsible for the autosuspend, which
certainly makes most sense.

>
> > 

Re: [PATCH] mmc: rtsx_usb_sdmmc: Handle runtime PM while changing led

[Oliver Neukum]

On Tue, 2016-09-20 at 10:12 -0400, Alan Stern wrote:
> On Tue, 20 Sep 2016, Oliver Neukum wrote:

> That shouldn't be an issue in this case, at least, not with the current 
> code.  The sdmmc and memstick drivers block autosuspend if media is 
> present.

Good.
> 
> > > Which means that autosuspend matters only when a card isn't present, 
> > > and the host is polled every second or so to see whether a card has 
> > > been inserted.
> > > 
> > > Under those circumstances you probably don't want to use
> > > autosuspend.  
> > > That is, resuming before each poll and suspending afterward may use 
> > > less energy than staying at full power all the time.
> > 
> > Is that based on concrete figures about power consumption?
> 
> No.

Well, I have no idea how to improve this much without hideous
overengineering.

> > And it seems to me that we need a way to indicate that the heuristics
> > should not be used, but a device immediately suspended. The timer
> > is sensible only if the next wakeup is unknown.
> 
> The driver can always turn off autosuspend if it wants to.

Yes, but this is not the point. A heuristic with a timeout makes
sense only if the uses are unpredictable. If you know with a high
degree of probability when the next activity comes, you ought to either
suspend now or not all until the next activity.

Likewise the heuristic is appropriate for leaf nodes. You get nothing
from a delay on inner nodes. Any storage (generic sense) device
is an inner node. It should suspend immediately after the block
device which is the leaf node.

Regards
Oliver



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

[PATCH v5 1/2] usb: chipidea: imx: configure imx for ULPI phy

[Fabien Lahoudere]

In order to use ULPI phy with usb host 2 and 3, we need to configure
controller register to enable ULPI features.

Each USB controller have different behaviour, so in order to avoid to have
several "swicth(data->index)" and lock/unlock, we prefer to get the index
switch and then test for features if they exist for this index.
This patch also remove useless test of reg and val. Those two values cannot
be NULL.

Signed-off-by: Fabien Lahoudere 
---
 drivers/usb/chipidea/ci_hdrc_imx.c |  5 +++
 drivers/usb/chipidea/ci_hdrc_imx.h |  1 +
 drivers/usb/chipidea/usbmisc_imx.c | 75 +++---
 3 files changed, 67 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
b/drivers/usb/chipidea/ci_hdrc_imx.c
index 0991794..96c0e33 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "ci.h"
@@ -146,6 +147,10 @@ static struct imx_usbmisc_data 
*usbmisc_get_init_data(struct device *dev)
if (of_find_property(np, "external-vbus-divider", NULL))
data->evdo = 1;
 
+
+   if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI)
+   data->ulpi = 1;
+
return data;
 }
 
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h 
b/drivers/usb/chipidea/ci_hdrc_imx.h
index 409aa5ca8..d666c9f 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -19,6 +19,7 @@ struct imx_usbmisc_data {
unsigned int disable_oc:1; /* over current detect disabled */
unsigned int oc_polarity:1; /* over current polarity if oc enabled */
unsigned int evdo:1; /* set external vbus divider option */
+   unsigned int ulpi:1; /* connected to an ULPI phy */
 };
 
 int imx_usbmisc_init(struct imx_usbmisc_data *);
diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c
index 20d02a5..11f51bd 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -46,11 +46,20 @@
 
 #define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08
 #define MX53_USB_OTG_PHY_CTRL_1_OFFSET 0x0c
+#define MX53_USB_CTRL_1_OFFSET 0x10
+#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK (0x11 << 2)
+#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI BIT(2)
+#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK (0x11 << 6)
+#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6)
 #define MX53_USB_UH2_CTRL_OFFSET   0x14
 #define MX53_USB_UH3_CTRL_OFFSET   0x18
 #define MX53_BM_OVER_CUR_DIS_H1BIT(5)
 #define MX53_BM_OVER_CUR_DIS_OTG   BIT(8)
 #define MX53_BM_OVER_CUR_DIS_UHx   BIT(30)
+#define MX53_USB_CTRL_1_UH2_ULPI_ENBIT(26)
+#define MX53_USB_CTRL_1_UH3_ULPI_ENBIT(27)
+#define MX53_USB_UHx_CTRL_WAKE_UP_EN   BIT(7)
+#define MX53_USB_UHx_CTRL_ULPI_INT_EN  BIT(8)
 #define MX53_USB_PHYCTRL1_PLLDIV_MASK  0x3
 #define MX53_USB_PLL_DIV_24_MHZ0x01
 
@@ -199,31 +208,69 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data 
*data)
val |= MX53_USB_PLL_DIV_24_MHZ;
writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET);
 
-   if (data->disable_oc) {
-   spin_lock_irqsave(>lock, flags);
-   switch (data->index) {
-   case 0:
+   spin_lock_irqsave(>lock, flags);
+
+   switch (data->index) {
+   case 0:
+   if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG;
-   break;
-   case 1:
+   writel(val, reg);
+   }
+   break;
+   case 1:
+   if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1;
-   break;
-   case 2:
+   writel(val, reg);
+   }
+   break;
+   case 2:
+   if (data->ulpi) {
+   /* set USBH2 into ULPI-mode. */
+   reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
+   val = readl(reg) | MX53_USB_CTRL_1_UH2_ULPI_EN;
+   /* select ULPI clock */
+   val &= ~MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK;
+   val |= MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI;
+   writel(val, reg);
+   /* Set interrupt wake up enable */
+   reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
+   val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN
+   | MX53_USB_UHx_CTRL_ULPI_INT_EN;
+   writel(val, reg);
+   }
+   if (data->disable_oc) {
reg = usbmisc->base + 

[PATCH v5 2/2] usb: chipidea: imx: Add binding to disable USB 60Mhz clock

[Fabien Lahoudere]

This binding allow to disable the internal 60Mhz clock for USB host2 or
host3.

Signed-off-by: Fabien Lahoudere 
---
 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt |  1 +
 drivers/usb/chipidea/ci_hdrc_imx.c |  2 ++
 drivers/usb/chipidea/ci_hdrc_imx.h |  1 +
 drivers/usb/chipidea/usbmisc_imx.c | 13 +
 4 files changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt 
b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
index 0e03344..f83da66 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -84,6 +84,7 @@ i.mx specific properties
 - over-current-active-high: over current signal polarity is high active,
   typically over current signal polarity is low active.
 - external-vbus-divider: enables off-chip resistor divider for Vbus
+- disable-int60ck: disable internal 60MHz clock for usb host2 or host3 on imx53
 
 Example:
 
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
b/drivers/usb/chipidea/ci_hdrc_imx.c
index 96c0e33..89a9d98 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -147,6 +147,8 @@ static struct imx_usbmisc_data 
*usbmisc_get_init_data(struct device *dev)
if (of_find_property(np, "external-vbus-divider", NULL))
data->evdo = 1;
 
+   if (of_find_property(np, "disable-int60ck", NULL))
+   data->disable_int60ck = 1;
 
if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI)
data->ulpi = 1;
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h 
b/drivers/usb/chipidea/ci_hdrc_imx.h
index d666c9f..43bafae 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -20,6 +20,7 @@ struct imx_usbmisc_data {
unsigned int oc_polarity:1; /* over current polarity if oc enabled */
unsigned int evdo:1; /* set external vbus divider option */
unsigned int ulpi:1; /* connected to an ULPI phy */
+   unsigned int disable_int60ck:1; /* disable 60 MHZ clock */
 };
 
 int imx_usbmisc_init(struct imx_usbmisc_data *);
diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c
index 11f51bd..a781f87 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -53,6 +53,9 @@
 #define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6)
 #define MX53_USB_UH2_CTRL_OFFSET   0x14
 #define MX53_USB_UH3_CTRL_OFFSET   0x18
+#define MX53_USB_CLKONOFF_CTRL_OFFSET  0x24
+#define MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF BIT(21)
+#define MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF BIT(22)
 #define MX53_BM_OVER_CUR_DIS_H1BIT(5)
 #define MX53_BM_OVER_CUR_DIS_OTG   BIT(8)
 #define MX53_BM_OVER_CUR_DIS_UHx   BIT(30)
@@ -240,6 +243,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data 
*data)
| MX53_USB_UHx_CTRL_ULPI_INT_EN;
writel(val, reg);
}
+   if (data->disable_int60ck) {
+   reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET;
+   val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF;
+   writel(val, reg);
+   }
if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
@@ -261,6 +269,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data 
*data)
| MX53_USB_UHx_CTRL_ULPI_INT_EN;
writel(val, reg);
}
+   if (data->disable_int60ck) {
+   reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET;
+   val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF;
+   writel(val, reg);
+   }
if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
-- 
2.1.4

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

[PATCH v5 0/2] usb: chipidea: imx: Add USB configuration for imx53

[Fabien Lahoudere]

Changes in V2:
- Patches sent to early with bad contents
Changes in V3:
- Change subject
- Split "configure imx for ULPI phy" for disable-oc code
Changes in V4:
- Fix "Change switch order" commit message
- Indent switch/case (set case on the same column as switch)
- Remove useless test in "Change switch order"
Changes in V5:
- Squash "Change switch order" and "configure imx for ULPI phy"
- Add device tree binding documentation

Fabien Lahoudere (2):
  usb: chipidea: imx: configure imx for ULPI phy
  usb: chipidea: imx: Add binding to disable USB 60Mhz clock

 .../devicetree/bindings/usb/ci-hdrc-usb2.txt   |  1 +
 drivers/usb/chipidea/ci_hdrc_imx.c |  7 ++
 drivers/usb/chipidea/ci_hdrc_imx.h |  2 +
 drivers/usb/chipidea/usbmisc_imx.c | 88 ++
 4 files changed, 84 insertions(+), 14 deletions(-)

-- 
2.1.4

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

Re: [PATCH 1/2] phy-sun4i-usb: Add sun4i_usb_phy_force_session_end() function

[Hans de Goede]

Hi,

On 09/20/2016 07:45 AM, Kishon Vijay Abraham I wrote:

Hi,

On Sunday 18 September 2016 10:20 PM, Hans de Goede wrote:

The sunxi musb has a bug where sometimes it will generate a babble
error on device disconnect instead of a disconnect irq. When this
happens the musb-controller switches from host mode to device mode
(it clears MUSB_DEVCTL_SESSION and sets MUSB_DEVCTL_BDEVICE) and
gets stuck in this state.

Clearing this requires reporting Vbus low for 200 or more ms, but
on some devices Vbus is simply always high (host-only mode, no Vbus
control). The phy-sun4i-usb code already has code to force a session
end for devices without Vbus control.

This commit adds a sun4i_usb_phy_force_session_end() function exporting
this functionality to the sunxi-musb glue, so that it can force a session
end to fixup the stuck state after a babble error.

Signed-off-by: Hans de Goede 
---
 drivers/phy/phy-sun4i-usb.c   | 11 +++
 include/linux/phy/phy-sun4i-usb.h |  7 +++
 2 files changed, 18 insertions(+)

diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 43c0d98..06f4e11a 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -470,6 +470,17 @@ void sun4i_usb_phy_set_squelch_detect(struct phy *_phy, 
bool enabled)
 }
 EXPORT_SYMBOL_GPL(sun4i_usb_phy_set_squelch_detect);

+void sun4i_usb_phy_force_session_end(struct phy *_phy)
+{
+   struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+   struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+
+   data->id_det = -1;
+   data->force_session_end = true;
+   queue_delayed_work(system_wq, >detect, 0);
+}
+EXPORT_SYMBOL_GPL(sun4i_usb_phy_force_session_end);


Er.. one more export symbol :-(


Yes unfortunately we need one more to work around sunxi musb / phy bugs.


+
 static const struct phy_ops sun4i_usb_phy_ops = {
.init   = sun4i_usb_phy_init,
.exit   = sun4i_usb_phy_exit,
diff --git a/include/linux/phy/phy-sun4i-usb.h 
b/include/linux/phy/phy-sun4i-usb.h
index 50aed92..3bb773f 100644
--- a/include/linux/phy/phy-sun4i-usb.h
+++ b/include/linux/phy/phy-sun4i-usb.h
@@ -23,4 +23,11 @@
  */
 void sun4i_usb_phy_set_squelch_detect(struct phy *phy, bool enabled);

+/**
+ * sun4i_usb_force_session_end() - Force the current session to end
+ *by reporting VBus low for 200+ ms
+ * @phy: reference to a sun4i usb phy
+ */
+void sun4i_usb_phy_force_session_end(struct phy *phy);


Should we include a static inline function if sun4i phy is not defined?


No, we're also not doing that for the already exported
sun4i_usb_phy_set_squelch_detect()

And it is not necessary since the only caller is drivers/usb/musb/sunxi.c,
and drivers/usb/musb/Kconfig has:

config USB_MUSB_SUNXI
tristate "Allwinner (sunxi)"
depends on PHY_SUN4I_USB

Regards,

Hans
--
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: g_webcam Isoch high bandwidth transfer

[Felipe Balbi]

Hi,

Bin Liu  writes:
> Hi,
>
> I am trying to check Isoch high bandwidth transfer with g_webcam.ko in
>  high-speed connection.
>
> First I hacked webcam.c as follows to enable 640x480@30fps mode.
>
> diff --git a/drivers/usb/gadget/legacy/webcam.c 
> b/drivers/usb/gadget/legacy/webcam.c
> index 72c976b..9eb315f 100644
> --- a/drivers/usb/gadget/legacy/webcam.c
> +++ b/drivers/usb/gadget/legacy/webcam.c
> @@ -191,15 +191,15 @@ static const struct UVC_FRAME_UNCOMPRESSED(3) 
> uvc_frame_yuv_360p = {
> .bFrameIndex= 1,
> .bmCapabilities = 0,
> .wWidth = cpu_to_le16(640),
> -   .wHeight= cpu_to_le16(360),
> +   .wHeight= cpu_to_le16(480),
> .dwMinBitRate   = cpu_to_le32(18432000),
> .dwMaxBitRate   = cpu_to_le32(55296000),
> -   .dwMaxVideoFrameBufferSize  = cpu_to_le32(460800),
> -   .dwDefaultFrameInterval = cpu_to_le32(66),
> +   .dwMaxVideoFrameBufferSize  = cpu_to_le32(614400),
> +   .dwDefaultFrameInterval = cpu_to_le32(33),
> .bFrameIntervalType = 3,
> -   .dwFrameInterval[0] = cpu_to_le32(66),
> -   .dwFrameInterval[1] = cpu_to_le32(100),
> -   .dwFrameInterval[2] = cpu_to_le32(500),
> +   .dwFrameInterval[0] = cpu_to_le32(33),
> +   .dwFrameInterval[1] = cpu_to_le32(66),
> +   .dwFrameInterval[2] = cpu_to_le32(100),
>  };
>
> then loaded g_webcam.ko as
>
> # modprobe g_webcam streaming_maxpacket=3072
>
> The endpoint descriptor showing on the host is
>
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x8d  EP 13 IN
> bmAttributes5
>   Transfer TypeIsochronous
>   Synch Type   Asynchronous
>   Usage Type   Data
> wMaxPacketSize 0x1400  3x 1024 bytes
> bInterval   1
>
> However the usb bus trace shows only one transaction with 1024-bytes packet in
> every SOF. The host only sends one IN packet in every SOF, I am expecting 2~3
> 1024-bytes transactions, since this would be required to transfer 
> 640x480@30fps
> YUV frames in high-speed.
>
> DId I miss anything in the setup?

MUSB or DWC3? This looks like a UDC bug to me. Can you show a screenshot
of your bus analyzer? When host sends IN token, are you replying with
DATA0, DATA1 or DATA2?

-- 
balbi


signature.asc
Description: PGP signature

Re: UAS and f_tcm -- is anyone using it?

[Felipe Balbi]

Hi,

John Youn  writes:
 Just from the code it seems there will be some fundamental issues with
 it, such as the value of maxpacket size and some alt-interface
 stuff. At least when used with DWC3.
>>>
>>> such as?
>>
>> I'll see if I can write up the exact issues later. I have to go back
>> to my notes to remind myself.
>>>
>>> Ok, coming back to this uas gadget stuff.
>>>
>>> I've finally had a chance to go back to my notes. Here are some of the
>>> concrete issues that I found, that I was able to workaround in some
>>> way.
>>>
>>> * EP's for UAS alt-interface cannot be configured correctly because
>>>   the config_ep_for_speed() in composite.c does not take into account
>>>   alt-interfaces. This results in incorrectly configured EP in the
>>>   controller (no streaming enabled, wrong direction, etc).
>> 
>> cool, that's a bug in composite.c which needs to be fixed.
>> 
>>> * START_XFER command needs to enable streams
>> 
>> bug in dwc3 which needs to be fixed.
>> 
>>> * XFER_NOT_READY event needs to be enabled for streams
>> 
>> bug in dwc3 which needs to be fixed :-) In any case, can you point me to
>> section in documention which states Streams *requires* XFER_NOT_READY?
>
> It should be the same as used for BULK. Maybe it's not needed with
> your recent enhancements? Not sure. I have to rebase and test.

okay

>>> * For OUT EPs, the TCM/target framework sets receive buffers size as
>>>   512 bytes. This cannot work in SUPERSPEED as you must always provide
>>>   at least MPS-sized buffers. This causes all writes to fail. I'm not
>> 
>> that's a good point. TCM gadget should be checking for ep out aligned
>> quirk flag.
>
> Is this an existing quirk? It's not just about alignment, but the size
> of the rx buffer, which it should know based on the MPS for the speed.

Yeah, it's part of struct usb_gadget:

 * @quirk_ep_out_aligned_size: epout requires buffer size to be aligned to
 *  MaxPacketSize.


>>>   sure how to properly fix this as it should be fixed at the function
>>>   driver level, and this size comes from the target framework. I put a
>>>   workaround at the controller-level.
>>>
>>> After addressing these issues, UAS read/write works to some extent.
>>> But it is still unstable in ways that point to issues in the target
>>> framework, things to do with locking/race conditions there.
>> 
>> Alright, those are all bugs which need to be fixed. Certainly we don't
>> need an entire new gadget just because you've found some bugs, right?
>
> Sure. I was just curious if there was any interest in it since we have
> an existing driver we could port.

I see.

>> We'd be much better off fixing these problems because then more folks
>> would benefit from them.
>
> Agree if it was working and being used in the first place.

Well, if you found bugs with it, you used it :-) We have so many gadgets
you can't expect us to always constantly test them all, there's just not
enough time for _me_ to test all gadgets. I have to rely on the fact
that hundreds are involved with the gadget framework and assume some of
these gadgets will get tested eventually.

> But anyways I'll focus on TCM.

thanks

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v4 2/3] usb: chipidea: imx: configure imx for ULPI phy

[Fabien Lahoudere]

Hi,

On 21/09/16 09:12, Peter Chen wrote:

On Mon, Sep 19, 2016 at 01:45:39PM +0200, Fabien Lahoudere wrote:

In order to use ULPI phy with usb host 2 and 3, we need to configure
controller register to enable ULPI features.

Signed-off-by: Fabien Lahoudere 
---
 drivers/usb/chipidea/ci_hdrc_imx.c |  5 +
 drivers/usb/chipidea/ci_hdrc_imx.h |  1 +
 drivers/usb/chipidea/usbmisc_imx.c | 37 +
 3 files changed, 43 insertions(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
b/drivers/usb/chipidea/ci_hdrc_imx.c
index 0991794..96c0e33 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 

 #include "ci.h"
@@ -146,6 +147,10 @@ static struct imx_usbmisc_data 
*usbmisc_get_init_data(struct device *dev)
if (of_find_property(np, "external-vbus-divider", NULL))
data->evdo = 1;

+
+   if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI)
+   data->ulpi = 1;
+
return data;
 }

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h 
b/drivers/usb/chipidea/ci_hdrc_imx.h
index 409aa5ca8..d666c9f 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -19,6 +19,7 @@ struct imx_usbmisc_data {
unsigned int disable_oc:1; /* over current detect disabled */
unsigned int oc_polarity:1; /* over current polarity if oc enabled */
unsigned int evdo:1; /* set external vbus divider option */
+   unsigned int ulpi:1; /* connected to an ULPI phy */
 };

 int imx_usbmisc_init(struct imx_usbmisc_data *);
diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c
index 9549821..11f51bd 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -46,11 +46,20 @@

 #define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08
 #define MX53_USB_OTG_PHY_CTRL_1_OFFSET 0x0c
+#define MX53_USB_CTRL_1_OFFSET 0x10
+#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK (0x11 << 2)
+#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI BIT(2)
+#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK (0x11 << 6)
+#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6)
 #define MX53_USB_UH2_CTRL_OFFSET   0x14
 #define MX53_USB_UH3_CTRL_OFFSET   0x18
 #define MX53_BM_OVER_CUR_DIS_H1BIT(5)
 #define MX53_BM_OVER_CUR_DIS_OTG   BIT(8)
 #define MX53_BM_OVER_CUR_DIS_UHx   BIT(30)
+#define MX53_USB_CTRL_1_UH2_ULPI_ENBIT(26)
+#define MX53_USB_CTRL_1_UH3_ULPI_ENBIT(27)
+#define MX53_USB_UHx_CTRL_WAKE_UP_EN   BIT(7)
+#define MX53_USB_UHx_CTRL_ULPI_INT_EN  BIT(8)
 #define MX53_USB_PHYCTRL1_PLLDIV_MASK  0x3
 #define MX53_USB_PLL_DIV_24_MHZ0x01

@@ -217,6 +226,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data 
*data)
}
break;
case 2:
+   if (data->ulpi) {
+   /* set USBH2 into ULPI-mode. */
+   reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
+   val = readl(reg) | MX53_USB_CTRL_1_UH2_ULPI_EN;
+   /* select ULPI clock */
+   val &= ~MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK;
+   val |= MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI;
+   writel(val, reg);
+   /* Set interrupt wake up enable */
+   reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
+   val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN
+   | MX53_USB_UHx_CTRL_ULPI_INT_EN;
+   writel(val, reg);
+   }


Ok, it seems your 1st patch is just let code structure change be easy
in this one, and I may give the wrong comments that you had improved
disable oc code before.

If you agree with me, would you squash these two into one, and send
again. (delete "reg && val" is necessary).


ok I will squash. =-D



Peter

if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
@@ -224,6 +247,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data 
*data)
}
break;
case 3:
+   if (data->ulpi) {
+   /* set USBH3 into ULPI-mode. */
+   reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
+   val = readl(reg) | MX53_USB_CTRL_1_UH3_ULPI_EN;
+   /* select ULPI clock */
+   val &= ~MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK;
+   val |= MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI;
+   writel(val, reg);
+   /* Set interrupt wake up enable */
+   reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
+   val = readl(reg) | 

Re: [PATCH v4 1/3] usb: chipidea: imx: Change switch order

[Fabien Lahoudere]

Hi,

On 21/09/16 08:57, Peter Chen wrote:

On Mon, Sep 19, 2016 at 01:45:38PM +0200, Fabien Lahoudere wrote:

Each USB controller have different behaviour, so in order to avoid to have
several "swicth(data->index)" and lock/unlock, we prefer to get the index
switch and then test for features if they exist for this index.
This patch also remove useless test of reg and val. Those two values cannot
be NULL.


Sorry, I can't see the benefits of this change.
Considering certain controller doesn't have oc feature, with current
code it only needs one comparison (flag disable_oc), but with your
changes, it needs two comparisons.


The benefits are visible with next patches only but you ask me to split 
it, so I do.


Thanks

Fabien



Peter


Signed-off-by: Fabien Lahoudere 
---
 drivers/usb/chipidea/usbmisc_imx.c | 38 --
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c
index 20d02a5..9549821 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -199,31 +199,41 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data 
*data)
val |= MX53_USB_PLL_DIV_24_MHZ;
writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET);

-   if (data->disable_oc) {
-   spin_lock_irqsave(>lock, flags);
-   switch (data->index) {
-   case 0:
+   spin_lock_irqsave(>lock, flags);
+
+   switch (data->index) {
+   case 0:
+   if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG;
-   break;
-   case 1:
+   writel(val, reg);
+   }
+   break;
+   case 1:
+   if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1;
-   break;
-   case 2:
+   writel(val, reg);
+   }
+   break;
+   case 2:
+   if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
-   break;
-   case 3:
+   writel(val, reg);
+   }
+   break;
+   case 3:
+   if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
-   break;
-   }
-   if (reg && val)
writel(val, reg);
-   spin_unlock_irqrestore(>lock, flags);
+   }
+   break;
}

+   spin_unlock_irqrestore(>lock, flags);
+
return 0;
 }

--
2.1.4




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

Re: [PATCH v4 3/3] usb: chipidea: imx: Add binding to disable USB 60Mhz clock

[Peter Chen]

On Mon, Sep 19, 2016 at 01:45:40PM +0200, Fabien Lahoudere wrote:
> This binding allow to disable the internal 60Mhz clock for USB host2 and
> host3.
> 
> Signed-off-by: Fabien Lahoudere 
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c |  2 ++
>  drivers/usb/chipidea/ci_hdrc_imx.h |  1 +
>  drivers/usb/chipidea/usbmisc_imx.c | 13 +
>  3 files changed, 16 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
> b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 96c0e33..89a9d98 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -147,6 +147,8 @@ static struct imx_usbmisc_data 
> *usbmisc_get_init_data(struct device *dev)
>   if (of_find_property(np, "external-vbus-divider", NULL))
>   data->evdo = 1;
>  
> + if (of_find_property(np, "disable-int60ck", NULL))
> + data->disable_int60ck = 1;
>  
>   if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI)
>   data->ulpi = 1;
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h 
> b/drivers/usb/chipidea/ci_hdrc_imx.h
> index d666c9f..43bafae 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.h
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
> @@ -20,6 +20,7 @@ struct imx_usbmisc_data {
>   unsigned int oc_polarity:1; /* over current polarity if oc enabled */
>   unsigned int evdo:1; /* set external vbus divider option */
>   unsigned int ulpi:1; /* connected to an ULPI phy */
> + unsigned int disable_int60ck:1; /* disable 60 MHZ clock */
>  };
>  
>  int imx_usbmisc_init(struct imx_usbmisc_data *);
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
> b/drivers/usb/chipidea/usbmisc_imx.c
> index 11f51bd..a781f87 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -53,6 +53,9 @@
>  #define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6)
>  #define MX53_USB_UH2_CTRL_OFFSET 0x14
>  #define MX53_USB_UH3_CTRL_OFFSET 0x18
> +#define MX53_USB_CLKONOFF_CTRL_OFFSET0x24
> +#define MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF BIT(21)
> +#define MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF BIT(22)
>  #define MX53_BM_OVER_CUR_DIS_H1  BIT(5)
>  #define MX53_BM_OVER_CUR_DIS_OTG BIT(8)
>  #define MX53_BM_OVER_CUR_DIS_UHx BIT(30)
> @@ -240,6 +243,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data 
> *data)
>   | MX53_USB_UHx_CTRL_ULPI_INT_EN;
>   writel(val, reg);
>   }
> + if (data->disable_int60ck) {
> + reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET;
> + val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF;
> + writel(val, reg);
> + }
>   if (data->disable_oc) {
>   reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
>   val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
> @@ -261,6 +269,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data 
> *data)
>   | MX53_USB_UHx_CTRL_ULPI_INT_EN;
>   writel(val, reg);
>   }
> + if (data->disable_int60ck) {
> + reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET;
> + val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF;
> + writel(val, reg);
> + }
>   if (data->disable_oc) {
>   reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
>   val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
> -- 
> 2.1.4
> 

Like I commented before, you need to have binding-doc update
(ocumentation/devicetree/bindings/usb/ci-hdrc-usb2.txt)
Besides, try to run ./scripts/get_maintainer.pl to get the
necessary maintainers and reviewers. You may get some
useful tips for reading Documentation/SubmittingPatches

-- 

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 v4 2/3] usb: chipidea: imx: configure imx for ULPI phy

[Peter Chen]

On Mon, Sep 19, 2016 at 01:45:39PM +0200, Fabien Lahoudere wrote:
> In order to use ULPI phy with usb host 2 and 3, we need to configure
> controller register to enable ULPI features.
> 
> Signed-off-by: Fabien Lahoudere 
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c |  5 +
>  drivers/usb/chipidea/ci_hdrc_imx.h |  1 +
>  drivers/usb/chipidea/usbmisc_imx.c | 37 +
>  3 files changed, 43 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
> b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 0991794..96c0e33 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "ci.h"
> @@ -146,6 +147,10 @@ static struct imx_usbmisc_data 
> *usbmisc_get_init_data(struct device *dev)
>   if (of_find_property(np, "external-vbus-divider", NULL))
>   data->evdo = 1;
>  
> +
> + if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI)
> + data->ulpi = 1;
> +
>   return data;
>  }
>  
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h 
> b/drivers/usb/chipidea/ci_hdrc_imx.h
> index 409aa5ca8..d666c9f 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.h
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
> @@ -19,6 +19,7 @@ struct imx_usbmisc_data {
>   unsigned int disable_oc:1; /* over current detect disabled */
>   unsigned int oc_polarity:1; /* over current polarity if oc enabled */
>   unsigned int evdo:1; /* set external vbus divider option */
> + unsigned int ulpi:1; /* connected to an ULPI phy */
>  };
>  
>  int imx_usbmisc_init(struct imx_usbmisc_data *);
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
> b/drivers/usb/chipidea/usbmisc_imx.c
> index 9549821..11f51bd 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -46,11 +46,20 @@
>  
>  #define MX53_USB_OTG_PHY_CTRL_0_OFFSET   0x08
>  #define MX53_USB_OTG_PHY_CTRL_1_OFFSET   0x0c
> +#define MX53_USB_CTRL_1_OFFSET   0x10
> +#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK (0x11 << 2)
> +#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI BIT(2)
> +#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK (0x11 << 6)
> +#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6)
>  #define MX53_USB_UH2_CTRL_OFFSET 0x14
>  #define MX53_USB_UH3_CTRL_OFFSET 0x18
>  #define MX53_BM_OVER_CUR_DIS_H1  BIT(5)
>  #define MX53_BM_OVER_CUR_DIS_OTG BIT(8)
>  #define MX53_BM_OVER_CUR_DIS_UHx BIT(30)
> +#define MX53_USB_CTRL_1_UH2_ULPI_EN  BIT(26)
> +#define MX53_USB_CTRL_1_UH3_ULPI_EN  BIT(27)
> +#define MX53_USB_UHx_CTRL_WAKE_UP_EN BIT(7)
> +#define MX53_USB_UHx_CTRL_ULPI_INT_ENBIT(8)
>  #define MX53_USB_PHYCTRL1_PLLDIV_MASK0x3
>  #define MX53_USB_PLL_DIV_24_MHZ  0x01
>  
> @@ -217,6 +226,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data 
> *data)
>   }
>   break;
>   case 2:
> + if (data->ulpi) {
> + /* set USBH2 into ULPI-mode. */
> + reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
> + val = readl(reg) | MX53_USB_CTRL_1_UH2_ULPI_EN;
> + /* select ULPI clock */
> + val &= ~MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK;
> + val |= MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI;
> + writel(val, reg);
> + /* Set interrupt wake up enable */
> + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
> + val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN
> + | MX53_USB_UHx_CTRL_ULPI_INT_EN;
> + writel(val, reg);
> + }

Ok, it seems your 1st patch is just let code structure change be easy
in this one, and I may give the wrong comments that you had improved
disable oc code before.

If you agree with me, would you squash these two into one, and send
again. (delete "reg && val" is necessary).

Peter
>   if (data->disable_oc) {
>   reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
>   val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
> @@ -224,6 +247,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data 
> *data)
>   }
>   break;
>   case 3:
> + if (data->ulpi) {
> + /* set USBH3 into ULPI-mode. */
> + reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
> + val = readl(reg) | MX53_USB_CTRL_1_UH3_ULPI_EN;
> + /* select ULPI clock */
> + val &= ~MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK;
> + val |= MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI;
> + writel(val, reg);
> + /* Set interrupt wake up enable */
> + reg = 

Re: [PATCH v4 1/3] usb: chipidea: imx: Change switch order

[Peter Chen]

On Mon, Sep 19, 2016 at 01:45:38PM +0200, Fabien Lahoudere wrote:
> Each USB controller have different behaviour, so in order to avoid to have
> several "swicth(data->index)" and lock/unlock, we prefer to get the index
> switch and then test for features if they exist for this index.
> This patch also remove useless test of reg and val. Those two values cannot
> be NULL.

Sorry, I can't see the benefits of this change.
Considering certain controller doesn't have oc feature, with current
code it only needs one comparison (flag disable_oc), but with your
changes, it needs two comparisons. 

Peter
> 
> Signed-off-by: Fabien Lahoudere 
> ---
>  drivers/usb/chipidea/usbmisc_imx.c | 38 
> --
>  1 file changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
> b/drivers/usb/chipidea/usbmisc_imx.c
> index 20d02a5..9549821 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -199,31 +199,41 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data 
> *data)
>   val |= MX53_USB_PLL_DIV_24_MHZ;
>   writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET);
>  
> - if (data->disable_oc) {
> - spin_lock_irqsave(>lock, flags);
> - switch (data->index) {
> - case 0:
> + spin_lock_irqsave(>lock, flags);
> +
> + switch (data->index) {
> + case 0:
> + if (data->disable_oc) {
>   reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
>   val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG;
> - break;
> - case 1:
> + writel(val, reg);
> + }
> + break;
> + case 1:
> + if (data->disable_oc) {
>   reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
>   val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1;
> - break;
> - case 2:
> + writel(val, reg);
> + }
> + break;
> + case 2:
> + if (data->disable_oc) {
>   reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
>   val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
> - break;
> - case 3:
> + writel(val, reg);
> + }
> + break;
> + case 3:
> + if (data->disable_oc) {
>   reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
>   val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
> - break;
> - }
> - if (reg && val)
>   writel(val, reg);
> - spin_unlock_irqrestore(>lock, flags);
> + }
> + break;
>   }
>  
> + spin_unlock_irqrestore(>lock, flags);
> +
>   return 0;
>  }
>  
> -- 
> 2.1.4
> 

-- 

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 2/2] usb: chipidea: otg: fix reading otgsc register

[Peter Chen]

On Mon, Sep 19, 2016 at 01:44:36PM +0200, Sascha Hauer wrote:
> When switching from gadget to host the driver waits for VBUS

It should be switching from host to gadget

> to disappear before doing the actual switch. This waiting is
> done by letting hw_wait_reg() poll the OTGSC register. This is
> buggy. hw_wait_reg() directly reads the register, but for
> reading the OTGSC register hw_read_otgsc() must be used since
> this function eventually patches the VBUS status read from extcon
> into the raw register value.
> To fix this inline the hw_wait_reg() code into its only user and
> use the correct function to read the OTGSC register. Since
> hw_wait_reg() is unused now remove it.
> 

Stephen Boyd has already posted a similar fix at below:
http://www.spinics.net/lists/linux-usb/msg146304.html

For patch 1, if there is a separate API to wait vbus to be low,
it seems not so necessary.

Peter
> Signed-off-by: Sascha Hauer 
> ---
>  drivers/usb/chipidea/ci.h   |  3 ---
>  drivers/usb/chipidea/core.c | 32 
>  drivers/usb/chipidea/otg.c  | 16 
>  3 files changed, 12 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index cd41455..05bc4d6 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -428,9 +428,6 @@ int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
>  
>  u8 hw_port_test_get(struct ci_hdrc *ci);
>  
> -int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
> - u32 value, unsigned int timeout_ms);
> -
>  void ci_platform_configure(struct ci_hdrc *ci);
>  
>  int dbg_create_files(struct ci_hdrc *ci);
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 69426e6..01390e0 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -516,38 +516,6 @@ int hw_device_reset(struct ci_hdrc *ci)
>   return 0;
>  }
>  
> -/**
> - * hw_wait_reg: wait the register value
> - *
> - * Sometimes, it needs to wait register value before going on.
> - * Eg, when switch to device mode, the vbus value should be lower
> - * than OTGSC_BSV before connects to host.
> - *
> - * @ci: the controller
> - * @reg: register index
> - * @mask: mast bit
> - * @value: the bit value to wait
> - * @timeout_ms: timeout in millisecond
> - *
> - * This function returns an error code if timeout
> - */
> -int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
> - u32 value, unsigned int timeout_ms)
> -{
> - unsigned long elapse = jiffies + msecs_to_jiffies(timeout_ms);
> -
> - while (hw_read(ci, reg, mask) != value) {
> - if (time_after(jiffies, elapse)) {
> - dev_err(ci->dev, "timeout waiting for %08x in %d\n",
> - mask, reg);
> - return -ETIMEDOUT;
> - }
> - msleep(20);
> - }
> -
> - return 0;
> -}
> -
>  static irqreturn_t ci_irq(int irq, void *data)
>  {
>   struct ci_hdrc *ci = data;
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 91989be..d1c4cdf 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -104,7 +104,6 @@ void ci_handle_vbus_change(struct ci_hdrc *ci)
>   usb_gadget_vbus_disconnect(>gadget);
>  }
>  
> -#define CI_VBUS_STABLE_TIMEOUT_MS 5000
>  static void ci_handle_id_switch(struct ci_hdrc *ci)
>  {
>   enum ci_role role = ci_otg_role(ci);
> @@ -117,10 +116,19 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
>  
>   ci_role_stop(ci);
>  
> - if (role == CI_ROLE_GADGET)
> + if (role == CI_ROLE_GADGET) {
> + unsigned long elapse = jiffies + msecs_to_jiffies(5000);
> +
>   /* wait vbus lower than OTGSC_BSV */
> - hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
> - CI_VBUS_STABLE_TIMEOUT_MS);
> + while (hw_read_otgsc(ci, OTGSC_BSV)) {
> + if (time_after(jiffies, elapse)) {
> + dev_err(ci->dev,
> + "timeout waiting for VBUS disappear\n");
> + break;
> + }
> + msleep(20);
> + }
> + }
>  
>   ci_role_start(ci, role);
>  }
> -- 
> 2.8.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

-- 

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 0/4] usb: early: add support for early printk through USB3 debug port

[Lu Baolu]

Hi,

On 09/20/2016 10:06 PM, Mathias Nyman wrote:
> On 29.08.2016 08:26, Lu Baolu wrote:
>> xHCI debug capability (DbC) is an optional but standalone
>> functionality provided by an xHCI host controller. With DbC
>> hardware initialized, the system will present a debug device
>> through the USB3 debug port (normally the first USB3 port).
>> The debug device is fully compliant with the USB framework
>> and provides the equivalent of a very high performance (USB3)
>> full-duplex serial link between the debug host and target.
>> The DbC functionality is independent of xHCI host. There
>> isn't any precondition from xHCI host side for DbC to work.
>>
>> This patch set adds support for early printk functionality
>> through a USB3 debug port by 1) initializing and enabling
>> the DbC hardware during early boot; 2) registering a boot
>> console to the system so that early printk messages can go
>> through the USB3 debug port. It also includes some lines
>> of changes in usb_debug driver so that it can be bound when
>> a USB3 debug device is enumerated.
>>
>> This is the resend version. Original patch set was submitted
>> several months ago. This resend version addresses the review
>> comment here [1].
>>
>> [1] https://lkml.org/lkml/2016/2/16/444
>>
>
> So other than making sure memory is freed,
> (and maybe cleanup the duplicate code) I don't really have any objections.
>

Thanks for your time.

I will refresh my patch according to your review comments.

Thinking that the merge window will be closed soon. I won't send
out the refreshed patch until the next rc1 is out. I'm willing to hear
more comments during this time.

> As you state in PATCH 1/4, this could be useful in the specific case of
> kernel debugging when machine crashes very early before the console code is
> initialized. For normal operation it is not recommended.
>
> And as it's not recommended or used for normal operations it shouldn't do any
> harm either
>
> -Mathias
>

Best regards,
Lu Baolu
--
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 1/4] usb: dbc: early driver for xhci debug capability

[Lu Baolu]

Hi,

On 09/20/2016 09:54 PM, Mathias Nyman wrote:
> On 29.08.2016 08:26, Lu Baolu wrote:
>> xHCI debug capability (DbC) is an optional but standalone
>> functionality provided by an xHCI host controller. Software
>> learns this capability by walking through the extended
>> capability list of the host. xHCI specification describes
>> DbC in section 7.6.
>>
>> This patch introduces the code to probe and initialize the
>> debug capability hardware during early boot. With hardware
>> initialized, the debug target (system on which this code is
>> running) will present a debug device through the debug port
>> (normally the first USB3 port). The debug device is fully
>> compliant with the USB framework and provides the equivalent
>> of a very high performance (USB3) full-duplex serial link
>> between the debug host and target. The DbC functionality is
>> independent of xHCI host. There isn't any precondition from
>> xHCI host side for DbC to work.
>>
>> This patch also includes bulk out and bulk in interfaces.
>> These interfaces could be used to implement early printk
>> bootconsole or hook to various system debuggers.
>>
>> This code is designed to be only used for kernel debugging
>> when machine crashes very early before the console code is
>> initialized. For normal operation it is not recommended.
>>
>> Cc: Mathias Nyman 
>> Signed-off-by: Lu Baolu 
>> ---
>
> Some comments inline

Thank you for reviewing my patch.

>
>> +
>> +#ifdef XDBC_TRACE
>> +#definexdbc_tracetrace_printk
>> +#else
>> +static inline void xdbc_trace(const char *fmt, ...) { }
>> +#endif /* XDBC_TRACE */
>
> I guess there's probably no better way to enable/disable debug messages for 
> this
> driver this early, and ehci-dbgp does the same.
>
> So I guess It's ok, but it still looks weird
>
>> +
>> +static void xdbc_early_delay(unsigned long count)
>> +{
>> +u8 val;
>> +
>> +val = inb(0x80);
>> +while (count-- > 0)
>> +outb(val, 0x80);
>> +}
>> +
>> +static void xdbc_runtime_delay(unsigned long count)
>> +{
>> +udelay(count);
>> +}
>> +
>
> Glad to see the addition of this runtime_delay in addition
> to the hack of reading port 0x80 for a 1us delay.
>
> And that the early_delay is only used for as long as it must.
>
>
>> +static int xdbc_handle_external_reset(void)
>> +{
>> +u32 ctrl;
>> +int ret;
>> +
>> +writel(0, _reg->control);
>> +ret = handshake(_reg->control, CTRL_DCE, 0, 10, 10);
>> +if (ret)
>> +return ret;
>> +
>> +xdbc_mem_init();
>> +mmiowb();
>> +
>> +ctrl = readl(_reg->control);
>> +writel(ctrl | CTRL_DCE | CTRL_PED, _reg->control);
>> +ret = handshake(_reg->control,
>> +CTRL_DCE, CTRL_DCE, 10, 10);
>> +if (ret)
>> +return ret;
>> +
>> +if (xdbc.vendor == PCI_VENDOR_ID_INTEL)
>> +xdbc_do_reset_debug_port(xdbc.port_number, 1);
>> +
>> +/* wait for port connection */
>> +ret = handshake(_reg->portsc,
>> +PORTSC_CCS, PORTSC_CCS, 500, 10);
>> +if (ret)
>> +return ret;
>> +
>> +/* wait for debug device to be configured */
>> +ret = handshake(_reg->control,
>> +CTRL_DCR, CTRL_DCR, 500, 10);
>> +if (ret)
>> +return ret;
>> +
>> +xdbc.flags |= XDBC_FLAGS_INITIALIZED | XDBC_FLAGS_CONFIGURED;
>> +
>> +xdbc_bulk_transfer(NULL, XDBC_MAX_PACKET, true);
>> +
>> +return 0;
>> +}
>> +
> ...
>
>> +static int __init xdbc_early_start(void)
>> +{
>> +u32 ctrl, status;
>> +int ret;
>> +
>> +ctrl = readl(_reg->control);
>> +writel(ctrl | CTRL_DCE | CTRL_PED, _reg->control);
>> +ret = handshake(_reg->control,
>> +CTRL_DCE, CTRL_DCE, 10, 100);
>> +if (ret) {
>> +pr_notice("falied to initialize hardware\n");
>> +return ret;
>> +}
>> +
>> +/* reset port to avoid bus hang */
>> +if (xdbc.vendor == PCI_VENDOR_ID_INTEL)
>> +xdbc_reset_debug_port();
>> +
>> +/* wait for port connection */
>> +ret = handshake(_reg->portsc,
>> +PORTSC_CCS, PORTSC_CCS, 500, 100);
>> +if (ret) {
>> +pr_notice("waiting for connection timed out\n");
>> +return ret;
>> +}
>> +
>> +/* wait for debug device to be configured */
>> +ret = handshake(_reg->control,
>> +CTRL_DCR, CTRL_DCR, 500, 100);
>> +if (ret) {
>> +pr_notice("waiting for device configuration timed out\n");
>> +return ret;
>> +}
>> +
>> +/* port should have a valid port# */
>> +status = readl(_reg->status);
>> +if (!DCST_DPN(status)) {
>> +pr_notice("invalid root hub port number\n");
>> +return -ENODEV;
>> +}
>> +
>> +xdbc.port_number = DCST_DPN(status);
>> +
>> +pr_notice("DbC is running now, control 0x%08x port ID %d\n",
>> +  readl(_reg->control), xdbc.port_number);
>> +
>> +return 0;
>> +}
>
>
> xdbc_early_start() and