Re: [PATCH] uas: Add US_FL_IGNORE_RESIDUE for Initio Copropration INIC-3069

2017-07-26 Thread Oliver Neukum
Am Mittwoch, den 26.07.2017, 10:05 +0200 schrieb  Alan Swanson :
> Similar to commit d595259fbb7a7afed241b1afb2c4fe4b47de47fa for INIC-3169 in
> unusual_devs.h but INIC-3069 already present in unusual_uas.h. Both in same
> controller IC family.
> 
> Issue is that MakeMKV fails during key exchange with installed bluray drive
> with following error:
> 
> 002004: Error 'Scsi error - ILLEGAL REQUEST:COPY PROTECTION KEY EXCHANGE 
> FAILURE - KEY NOT ESTABLISHED'
> occurred while issuing SCSI command AD010..080002400 to device 'SG:dev_11:0'
> 
> Signed-off-by: Alan Swanson 
Acked-by: Oliver Neukum 
> ---
>  drivers/usb/storage/unusual_uas.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/storage/unusual_uas.h 
> b/drivers/usb/storage/unusual_uas.h
> index cbea9f32..cde11535 100644
> --- a/drivers/usb/storage/unusual_uas.h
> +++ b/drivers/usb/storage/unusual_uas.h
> @@ -124,9 +124,9 @@ UNUSUAL_DEV(0x0bc2, 0xab2a, 0x, 0x,
>  /* Reported-by: Benjamin Tissoires  */
>  UNUSUAL_DEV(0x13fd, 0x3940, 0x, 0x,
>   "Initio Corporation",
> - "",
> + "INIC-3069",
>   USB_SC_DEVICE, USB_PR_DEVICE, NULL,
> - US_FL_NO_ATA_1X),
> + US_FL_NO_ATA_1X | US_FL_IGNORE_RESIDUE),
>  
>  /* Reported-by: Tom Arild Naess  */
>  UNUSUAL_DEV(0x152d, 0x0539, 0x, 0x,

--
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] uas: Add US_FL_IGNORE_RESIDUE for Initio Copropration INIC-3069

2017-07-26 Thread Sergei Shtylyov

Hello!

On 7/26/2017 12:46 AM, Alan Swanson wrote:


Similar to commit d595259fbb7a7afed241b1afb2c4fe4b47de47fa for INIC-3169 in


   That's not how you cite a commit -- you should also specify the commit 
summary line enclosed in ("") (and 12-digit SHA1 would be enough).



unusual_devs.h but INIC-3069 already present in unusual_uas.h. Both in same
controller IC family.

Issue is that MakeMKV fails during key exchange with installed bluray drive
with following error:

002004: Error 'Scsi error - ILLEGAL REQUEST:COPY PROTECTION KEY EXCHANGE 
FAILURE - KEY NOT ESTABLISHED'
occurred while issuing SCSI command AD010..080002400 to device 'SG:dev_11:0'

Signed-off-by: Alan Swanson 

[...]

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


Re: v4.13-rc2: usb mouse stopped working?

2017-07-26 Thread Pavel Machek
Hi!

> > In v4.9:
> > 
> > [   87.064408] input: Logitech USB Optical Mouse as
> > /devices/pci:00/:00:1d.0/usb2/2-1/2-1.1/2-1.1.1/2-1.1.1.1/2-1.1.1.1:1.0/0003:046D:C05A.0005/input/input18
> > [   87.065951] hid-generic 0003:046D:C05A.0005: input,hidraw0: USB HID
> > v1.11 Mouse [Logitech USB Optical Mouse] on
> > usb-:00:1d.0-1.1.1.1/input0
> > pavel@duo:~$
> > 
> > v4.13-rc2:
> > 
> > [   87.886810] usb 2-1.1.1.1: new low-speed USB device number 10 using 
> > ehci-pci
> > [   88.019543] usb 2-1.1.1.1: New USB device found, idVendor=046d, 
> > idProduct=c05a
> > [   88.019561] usb 2-1.1.1.1: New USB device strings: Mfr=1, Product=2, 
> > SerialNumber=0
> > [   88.019572] usb 2-1.1.1.1: Product: USB Optical Mouse
> > [   88.019581] usb 2-1.1.1.1: Manufacturer: Logitech
> > [   88.027276] input: Logitech USB Optical Mouse as 
> > /devices/pci:00/:00:1d.0/usb2/2-1/2-1.1/2-1.1.1/2-1.1.1.1/2-1.1.1.1:1.0/0003:046D:C05A.0005/input/input18
> > [   88.027825] hid-generic 0003:046D:C05A.0005: input,hidraw1: USB HID
> > v1.11 Mouse [Logitech USB Optical Mouse] on
> > usb-:00:1d.0-1.1.1.1/input0
> 
> ... this is most likely caused by e399396a6b0, and fix for that is queued 
> in hid.git as cf601774c9f ("HID: usbhid: fix "always poll" quirk"); I'm 
> planning to send it to Linus' tomorrow, but if you can double-check that 
> it does fix your issue as well, that'd be appreciated.

I pulled newer git, and now it seems to work. Thanks! (Tested with
same mouse but different PC).
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


[PATCH V2] uas: Add US_FL_IGNORE_RESIDUE for Initio Corporation INIC-3069

2017-07-26 Thread Alan Swanson
Similar to commit d595259fbb7a ("usb-storage: Add ignore-residue quirk for
Initio INIC-3619") for INIC-3169 in unusual_devs.h but INIC-3069 already
present in unusual_uas.h. Both in same controller IC family.

Issue is that MakeMKV fails during key exchange with installed bluray drive
with following error:

002004: Error 'Scsi error - ILLEGAL REQUEST:COPY PROTECTION KEY EXCHANGE 
FAILURE - KEY NOT ESTABLISHED'
occurred while issuing SCSI command AD010..080002400 to device 'SG:dev_11:0'

Signed-off-by: Alan Swanson 
Acked-by: Oliver Neukum 
---
 drivers/usb/storage/unusual_uas.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Fix format of commit citation and the spelling of Corporation.

diff --git a/drivers/usb/storage/unusual_uas.h 
b/drivers/usb/storage/unusual_uas.h
index cbea9f32..cde11535 100644
--- a/drivers/usb/storage/unusual_uas.h
+++ b/drivers/usb/storage/unusual_uas.h
@@ -124,9 +124,9 @@ UNUSUAL_DEV(0x0bc2, 0xab2a, 0x, 0x,
 /* Reported-by: Benjamin Tissoires  */
 UNUSUAL_DEV(0x13fd, 0x3940, 0x, 0x,
"Initio Corporation",
-   "",
+   "INIC-3069",
USB_SC_DEVICE, USB_PR_DEVICE, NULL,
-   US_FL_NO_ATA_1X),
+   US_FL_NO_ATA_1X | US_FL_IGNORE_RESIDUE),
 
 /* Reported-by: Tom Arild Naess  */
 UNUSUAL_DEV(0x152d, 0x0539, 0x, 0x,
-- 
2.13.2

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


Re: [PATCH v3 3/3] power: wm831x_power: Support USB charger current limit management

2017-07-26 Thread Sebastian Reichel
Hi,

On Wed, Jul 26, 2017 at 11:05:25AM +0800, Baolin Wang wrote:
> On 25 July 2017 at 17:59, Sebastian Reichel
>  wrote:
> > On Tue, Jul 25, 2017 at 04:00:01PM +0800, Baolin Wang wrote:
> >> Integrate with the newly added USB charger interface to limit the current
> >> we draw from the USB input based on the input device configuration
> >> identified by the USB stack, allowing us to charge more quickly from high
> >> current inputs without drawing more current than specified from others.
> >>
> >> Signed-off-by: Mark Brown 
> >> Signed-off-by: Baolin Wang 
> >> ---
> >>  Documentation/devicetree/bindings/mfd/wm831x.txt |1 +
> >>  drivers/power/supply/wm831x_power.c  |   58 
> >> ++
> >>  2 files changed, 59 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mfd/wm831x.txt 
> >> b/Documentation/devicetree/bindings/mfd/wm831x.txt
> >> index 9f8b743..4e3bc07 100644
> >> --- a/Documentation/devicetree/bindings/mfd/wm831x.txt
> >> +++ b/Documentation/devicetree/bindings/mfd/wm831x.txt
> >> @@ -31,6 +31,7 @@ Required properties:
> >>  ../interrupt-controller/interrupts.txt
> >>
> >>  Optional sub-nodes:
> >> +  - usb-phy : Contains a phandle to the USB PHY.
> >>- regulators : Contains sub-nodes for each of the regulators supplied by
> >>  the device. The regulators are bound using their names listed below:
> >>
> >> diff --git a/drivers/power/supply/wm831x_power.c 
> >> b/drivers/power/supply/wm831x_power.c
> >> index 7082301..d3948ab 100644
> >> --- a/drivers/power/supply/wm831x_power.c
> >> +++ b/drivers/power/supply/wm831x_power.c
> >> @@ -13,6 +13,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>
> >>  #include 
> >>  #include 
> >> @@ -31,6 +32,8 @@ struct wm831x_power {
> >>   char usb_name[20];
> >>   char battery_name[20];
> >>   bool have_battery;
> >> + struct usb_phy *usb_phy;
> >> + struct notifier_block usb_notify;
> >>  };
> >>
> >>  static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
> >> @@ -125,6 +128,43 @@ static int wm831x_usb_get_prop(struct power_supply 
> >> *psy,
> >>   POWER_SUPPLY_PROP_VOLTAGE_NOW,
> >>  };
> >>
> >> +/* In milliamps */
> >> +static const unsigned int wm831x_usb_limits[] = {
> >> + 0,
> >> + 2,
> >> + 100,
> >> + 500,
> >> + 900,
> >> + 1500,
> >> + 1800,
> >> + 550,
> >> +};
> >> +
> >> +static int wm831x_usb_limit_change(struct notifier_block *nb,
> >> +unsigned long limit, void *data)
> >> +{
> >> + struct wm831x_power *wm831x_power = container_of(nb,
> >> +  struct wm831x_power,
> >> +  usb_notify);
> >> + unsigned int i, best;
> >> +
> >> + /* Find the highest supported limit */
> >> + best = 0;
> >> + for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
> >> + if (limit >= wm831x_usb_limits[i] &&
> >> + wm831x_usb_limits[best] < wm831x_usb_limits[i])
> >> + best = i;
> >> + }
> >> +
> >> + dev_dbg(wm831x_power->wm831x->dev,
> >> + "Limiting USB current to %umA", wm831x_usb_limits[best]);
> >> +
> >> + wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
> >> + WM831X_USB_ILIM_MASK, best);
> >> +
> >> + return 0;
> >> +}
> >> +
> >>  /*
> >>   *   Battery properties
> >>   */
> >> @@ -607,6 +647,19 @@ static int wm831x_power_probe(struct platform_device 
> >> *pdev)
> >>   }
> >>   }
> >>
> >> + power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev,
> >> +  "usb-phy", 0);
> >> + if (!IS_ERR(power->usb_phy)) {
> >> + power->usb_notify.notifier_call = wm831x_usb_limit_change;
> >> + ret = usb_register_notifier(power->usb_phy,
> >> + &power->usb_notify);
> >> + if (ret) {
> >> + dev_err(&pdev->dev, "Failed to register notifier: 
> >> %d\n",
> >> + ret);
> >> + goto err_bat_irq;
> >> + }
> >> + }
> >
> > No error handling for power->usb_phy? I think you should bail out
> > for all errors except for "not defined in DT". Especially I would
> > expect probe defer handling in case the power supply driver is
> > loaded before the phy driver.
> 
> Make sense. So I think I need to change like below:
> 
> power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
> if (!IS_ERR(power->usb_phy)) {
> power->usb_notify.notifier_call = wm831x_usb_limit_change;
> ret = usb_register_notifier(power->usb_phy, &power->usb_notify);
> if (ret) {
> 

Re: [PATCH] USB: hcd: Mark secondary HCD as dead if the primary one died

2017-07-26 Thread Alan Stern
On Tue, 25 Jul 2017, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki 
> 
> Make usb_hc_died() clear the HCD_FLAG_RH_RUNNING flag for the shared
> HCD and set HCD_FLAG_DEAD for it, in analogy with what is done for
> the primary one.
> 
> Among other thigs, this prevents check_root_hub_suspended() from
> returning -EBUSY for dead HCDs which helps to work around system
> suspend issues in some situations.
> 
> This actually fixes occasional suspend failures on one of my test
> machines.
> 
> Suggested-by: Alan Stern 
> Signed-off-by: Rafael J. Wysocki 
> ---
>  drivers/usb/core/hcd.c |2 ++
>  1 file changed, 2 insertions(+)
> 
> Index: linux-pm/drivers/usb/core/hcd.c
> ===
> --- linux-pm.orig/drivers/usb/core/hcd.c
> +++ linux-pm/drivers/usb/core/hcd.c
> @@ -2485,6 +2485,8 @@ void usb_hc_died (struct usb_hcd *hcd)
>   }
>   if (usb_hcd_is_primary_hcd(hcd) && hcd->shared_hcd) {
>   hcd = hcd->shared_hcd;
> + clear_bit(HCD_FLAG_RH_RUNNING, &hcd->flags);
> + set_bit(HCD_FLAG_DEAD, &hcd->flags);
>   if (hcd->rh_registered) {
>   clear_bit(HCD_FLAG_POLL_RH, &hcd->flags);

Acked-by: Alan Stern 

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


[PATCH v2 1/2] USB: musb: fix external abort on suspend

2017-07-26 Thread Johan Hovold
Make sure that the controller is runtime resumed when system suspending
to avoid an external abort when accessing the interrupt registers:

  Unhandled fault: external abort on non-linefetch (0x1008) at 0xd025840a
  ...
  [] (musb_default_readb) from [] 
(musb_disable_interrupts+0x84/0xa8)
  [] (musb_disable_interrupts) from [] 
(musb_suspend+0x38/0xb8)
  [] (musb_suspend) from [] (platform_pm_suspend+0x3c/0x64)

This is easily reproduced on a BBB by enabling the peripheral port only
(as the host port may enable the shared clock) and keeping it
disconnected so that the controller is runtime suspended. (Well, you
would also need to the not-yet-merged am33xx-suspend patches by Dave
Gerlach to be able to suspend the BBB.)

This is a regression that was introduced by commit 1c4d0b4e1806 ("usb:
musb: Remove pm_runtime_set_irq_safe") which allowed the parent glue
device to runtime suspend and thereby exposed a couple of older issues:

Register accesses without explicitly making sure the controller is
runtime resumed during suspend was first introduced by commit
c338412b5ded ("usb: musb: unconditionally save and restore the context
on suspend") in 3.14.

Commit a1fc1920 ("usb: musb: core: make sure musb is in RPM_ACTIVE on
resume") later started setting the RPM status to active during resume,
and this was also implicitly relying on the parent always being active.
Since commit 71723f95463d ("PM / runtime: print error when activating a
child to unactive parent") this now also results in the following
warning:

  musb-hdrc musb-hdrc.0: runtime PM trying to activate child device
musb-hdrc.0 but parent (47401400.usb) is not active

This patch has been verified on 4.13-rc2, 4.12 and 4.9 using a BBB
(the dsps glue would always be active also in 4.8).

Fixes: c338412b5ded ("usb: musb: unconditionally save and restore the context 
on suspend")
Fixes: a1fc1920 ("usb: musb: core: make sure musb is in RPM_ACTIVE on 
resume")
Fixes: 1c4d0b4e1806 ("usb: musb: Remove pm_runtime_set_irq_safe")
Cc: stable  # 4.8
Cc: Alan Stern 
Cc: Daniel Mack 
Cc: Dave Gerlach 
Cc: Rafael J. Wysocki 
Cc: Sebastian Andrzej Siewior 
Cc: Tony Lindgren 
Signed-off-by: Johan Hovold 
---
 drivers/usb/musb/musb_core.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 87cbd56cc761..b67692857daf 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2671,6 +2671,13 @@ static int musb_suspend(struct device *dev)
 {
struct musb *musb = dev_to_musb(dev);
unsigned long   flags;
+   int ret;
+
+   ret = pm_runtime_get_sync(dev);
+   if (ret < 0) {
+   pm_runtime_put_noidle(dev);
+   return ret;
+   }
 
musb_platform_disable(musb);
musb_disable_interrupts(musb);
@@ -2721,14 +2728,6 @@ static int musb_resume(struct device *dev)
if ((devctl & mask) != (musb->context.devctl & mask))
musb->port1_status = 0;
 
-   /*
-* The USB HUB code expects the device to be in RPM_ACTIVE once it came
-* out of suspend
-*/
-   pm_runtime_disable(dev);
-   pm_runtime_set_active(dev);
-   pm_runtime_enable(dev);
-
musb_start(musb);
 
spin_lock_irqsave(&musb->lock, flags);
@@ -2738,6 +2737,9 @@ static int musb_resume(struct device *dev)
error);
spin_unlock_irqrestore(&musb->lock, flags);
 
+   pm_runtime_mark_last_busy(dev);
+   pm_runtime_put_autosuspend(dev);
+
return 0;
 }
 
-- 
2.13.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 v2 0/2] USB: musb: fix external abort on suspend

2017-07-26 Thread Johan Hovold
This is a v2 of the fix for an external abort during suspend which I submitted
on Monday.

The commit message of patch 1/2 has only been slightly reworded, while
patch 2/2 is new.

Johan


Johan Hovold (2):
  USB: musb: fix external abort on suspend
  USB: musb: dsps: add explicit runtime resume at suspend

 drivers/usb/musb/musb_core.c | 18 ++
 drivers/usb/musb/musb_dsps.c | 13 +++--
 2 files changed, 21 insertions(+), 10 deletions(-)

-- 
2.13.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 v2 2/2] USB: musb: dsps: add explicit runtime resume at suspend

2017-07-26 Thread Johan Hovold
The musb_dsps driver is special in that the parent (glue) device's
driver is accessing registers mapped by the child. The clock is however
shared and is managed by the grandparent device.

Since commit 869c59782981 ("usb: musb: dsps: add support for suspend and
resume") the dsps driver has been accessing these registers as part of
suspend and resume.

The parent driver obviously cannot runtime resume the child during
system suspend and is currently relying on the fact that the child will
be RPM_ACTIVE throughout suspend. The suspend implementation also makes
sure to check that the child is indeed present (and hence the clock
enabled) before accessing the registers.

Let's add an explicit runtime resume of the glue device itself to enable the
clock before doing the register accesses in case these assumptions ever change
(i.e. if the child is left runtime suspended).

Note that the glue-timer cancellation is moved after the child-presence
check to keep error handling simple. This should be fine as the timer is
not setup until the controller is being registered and at that time
glue->musb and its driver data have already been initialised.

Cc: Alan Stern 
Cc: Daniel Mack 
Cc: Tony Lindgren 
Signed-off-by: Johan Hovold 
---
 drivers/usb/musb/musb_dsps.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index bc6a9be2ccc5..f6b526606ad1 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -1015,13 +1015,20 @@ static int dsps_suspend(struct device *dev)
const struct dsps_musb_wrapper *wrp = glue->wrp;
struct musb *musb = platform_get_drvdata(glue->musb);
void __iomem *mbase;
-
-   del_timer_sync(&glue->timer);
+   int ret;
 
if (!musb)
/* This can happen if the musb device is in -EPROBE_DEFER */
return 0;
 
+   ret = pm_runtime_get_sync(dev);
+   if (ret < 0) {
+   pm_runtime_put_noidle(dev);
+   return ret;
+   }
+
+   del_timer_sync(&glue->timer);
+
mbase = musb->ctrl_base;
glue->context.control = musb_readl(mbase, wrp->control);
glue->context.epintr = musb_readl(mbase, wrp->epintr_set);
@@ -1060,6 +1067,8 @@ static int dsps_resume(struct device *dev)
musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
dsps_mod_timer(glue, -1);
 
+   pm_runtime_put(dev);
+
return 0;
 }
 #endif
-- 
2.13.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: pixart optical mouse dont work

2017-07-26 Thread Greg KH
On Tue, Jul 25, 2017 at 04:37:00PM +0200, Matthias Holl wrote:
> dear kernel hackers,
> 
> with the new kernel 4.13 my optical mouse from pixart dont work
> but i can show it with lsusb
> 
> bus 007 device 003: id 093a:2510 pixart imaging,inc optical mouse
> 
> i changed nothing in my kernel .conf
> with kernel 4.12, 4.11...i had no problems
> i guess its something with the generic hid driver

Should be fixed now if you pull Linus's tree.  If not, please let the
HID maintainer know.

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: CPU lock-ups with 4.12.0+ kernels related to usb_storage

2017-07-26 Thread Christoph Hellwig
On Thu, Jul 13, 2017 at 01:00:29PM -0400, Alan Stern wrote:
> All right.  In the meantime, changing usb-storage won't hurt.
> Arthur, can you test the patch below?

Do you plan to send this fix to Greg for 4.13-rc?
--
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: fix deadlock involving host lock and scsi_done

2017-07-26 Thread Alan Stern
Christoph Hellwig says that since version 4.12, the kernel switched to
using blk-mq by default.  The old code used a softirq for handling
request completions, but blk-mq can handle completions in the caller's
context.  This may cause a problem for usb-storage, because it invokes
the ->scsi_done callback while holding the host lock, and the
completion routine sometimes tries to acquire the same lock (when
running the error handler, for example).

The consequence is that the existing code will sometimes deadlock upon
error completion of a SCSI command (with a lockdep warning).

This is easy enough to fix, since usb-storage doesn't really need to
hold the host lock while the callback runs.  It was simpler to write
it that way, but moving the call outside the locked region is pretty
easy and there's no downside.  That's what this patch does.

Signed-off-by: Alan Stern 
Reported-and-tested-by: Arthur Marsh 
CC: Christoph Hellwig 
CC: 

---


[as1835]


 drivers/usb/storage/usb.c |   18 --
 1 file changed, 12 insertions(+), 6 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
@@ -315,6 +315,7 @@ static int usb_stor_control_thread(void
 {
struct us_data *us = (struct us_data *)__us;
struct Scsi_Host *host = us_to_host(us);
+   struct scsi_cmnd *srb;
 
for (;;) {
usb_stor_dbg(us, "*** thread sleeping\n");
@@ -330,6 +331,7 @@ static int usb_stor_control_thread(void
scsi_lock(host);
 
/* When we are called with no command pending, we're done */
+   srb = us->srb;
if (us->srb == NULL) {
scsi_unlock(host);
mutex_unlock(&us->dev_mutex);
@@ -398,14 +400,11 @@ static int usb_stor_control_thread(void
/* lock access to the state */
scsi_lock(host);
 
-   /* indicate that the command is done */
-   if (us->srb->result != DID_ABORT << 16) {
-   usb_stor_dbg(us, "scsi cmd done, result=0x%x\n",
-us->srb->result);
-   us->srb->scsi_done(us->srb);
-   } else {
+   /* was the command aborted? */
+   if (us->srb->result == DID_ABORT << 16) {
 SkipForAbort:
usb_stor_dbg(us, "scsi command aborted\n");
+   srb = NULL; /* Don't call srb->scsi_done() */
}
 
/*
@@ -429,6 +428,13 @@ SkipForAbort:
 
/* unlock the device pointers */
mutex_unlock(&us->dev_mutex);
+
+   /* now that the locks are released, notify the SCSI core */
+   if (srb) {
+   usb_stor_dbg(us, "scsi cmd done, result=0x%x\n",
+   srb->result);
+   srb->scsi_done(srb);
+   }
} /* for (;;) */
 
/* Wait until we are told to stop */

--
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: CPU lock-ups with 4.12.0+ kernels related to usb_storage

2017-07-26 Thread Alan Stern
On Wed, 26 Jul 2017, Christoph Hellwig wrote:

> On Thu, Jul 13, 2017 at 01:00:29PM -0400, Alan Stern wrote:
> > All right.  In the meantime, changing usb-storage won't hurt.
> > Arthur, can you test the patch below?
> 
> Do you plan to send this fix to Greg for 4.13-rc?

I just submitted it.

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: hcd: Mark secondary HCD as dead if the primary one died

2017-07-26 Thread Rafael J. Wysocki
On Wednesday, July 26, 2017 10:21:54 AM Alan Stern wrote:
> On Tue, 25 Jul 2017, Rafael J. Wysocki wrote:
> 
> > From: Rafael J. Wysocki 
> > 
> > Make usb_hc_died() clear the HCD_FLAG_RH_RUNNING flag for the shared
> > HCD and set HCD_FLAG_DEAD for it, in analogy with what is done for
> > the primary one.
> > 
> > Among other thigs, this prevents check_root_hub_suspended() from
> > returning -EBUSY for dead HCDs which helps to work around system
> > suspend issues in some situations.
> > 
> > This actually fixes occasional suspend failures on one of my test
> > machines.
> > 
> > Suggested-by: Alan Stern 
> > Signed-off-by: Rafael J. Wysocki 
> > ---
> >  drivers/usb/core/hcd.c |2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > Index: linux-pm/drivers/usb/core/hcd.c
> > ===
> > --- linux-pm.orig/drivers/usb/core/hcd.c
> > +++ linux-pm/drivers/usb/core/hcd.c
> > @@ -2485,6 +2485,8 @@ void usb_hc_died (struct usb_hcd *hcd)
> > }
> > if (usb_hcd_is_primary_hcd(hcd) && hcd->shared_hcd) {
> > hcd = hcd->shared_hcd;
> > +   clear_bit(HCD_FLAG_RH_RUNNING, &hcd->flags);
> > +   set_bit(HCD_FLAG_DEAD, &hcd->flags);
> > if (hcd->rh_registered) {
> > clear_bit(HCD_FLAG_POLL_RH, &hcd->flags);
> 
> Acked-by: Alan Stern 
> 

Thanks!

I guess this should go in via USB, so Felipe & Greg, please apply or let me
know if you prefer me to handle it.

Thanks,
Rafael

--
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: USB disk speed regression WD Elements - with bisect result 22547c4cc4fe20698a6a85a55b8788859134b8e4

2017-07-26 Thread Alan Stern
[Added linux-usb mailing list to CC:]

Short description of bug: In 4.12 or later, when Zdenek's Western
Digital disk is attached to an EHCI controller, it ends up connecting
at full speed to the companion UHCI controller instead.  But when
commit 22547c4cc4fe ("usb: hub: Wait for connection to be reestablished
after port reset")  is reverted, the disk connects correctly at high
speed.

On Wed, 26 Jul 2017, Zdenek Kabelac wrote:

> Dne 25.7.2017 v 21:50 Alan Stern napsal(a):
> > On Tue, 25 Jul 2017, Zdenek Kabelac wrote:
> > 
> > 
> > 08:18 usb 2-2: USB disconnect, device number 2
> > 08:25 usb 2-2: new high-speed USB device number 3 using ehci-pci
> > 08:26 usb 2-2: new high-speed USB device number 4 using ehci-pci
> > 
> > If the drive were working entirely correctly, it wouldn't do that.
> > 
> > We could continue futzing around with hardware and driver tests for a
> > long time.  But there may be a shortcut: If you have a USB hub, you
> > could try attaching the drive through it.  It's entirely possible that
> > this will fix the problem.
> > 
> > If not, you'll have to start doing some very detailed tests.  As a
> > start, you can enable debugging for the usbcore and ehci_hcd drivers
> > immediately before the test:
> > 
> > echo 'module usbcore =p' >/sys/kernel/debug/dynamic_debug/control
> > echo 'module ehci_hcd =p' >/sys/kernel/debug/dynamic_debug/control
> > dmesg -C
> > 
> > Then after the test, see what shows up in the dmesg output.  And again,
> > we'll want to do a comparison.  In fact, 4.12 with and without the
> > commit you identified would make a better comparison than 4.12 vs. 4.8.
> > 
> 
> 
> Hi
> 
> 
> So here we go with traces - made with freshly compiled recent 4.13-rc2.
> OK trace is with revert patch applied.
> BAD trace is the one with it (essentially vaniala master).
> 
> Trace also has KOBJECT debugging enabled - I think difference is
> nicely visible between them - but I've no explanation for it.
> 
> Both traces start with cable detach followed with attachment.

Okay, I figured out the cause.

The USB stack assumes that if a high-speed device initialization fails
and the device is still connected, it means that the device can't run
properly at high speed and the computer needs to try again at full
speed.  See commit 90da096ee46b ("USB: force handover port to companion
when hub_port_connect_change fails").

In Zdenek's case, the device really does disconnect itself before the
second port reset.  If 22547c4cc4fe is present, it causes an extra
delay -- some 200 ms -- long enough for the device to reconnect again.  
So we appear to be in the situation described above, and the connection
is switched over to the companion controller.

If 22547c4cc4fe is not present, the kernel sees that the device is not
connected at the end of the second reset and gives up trying to
initialize the device.  When the device reconnects about 140 ms later,
the kernel treats it as a new connection and successfully negotiates a
high-speed link.

Zdenek, you check this explanation by commenting out these last two
lines at the end of hub_port_connect() in drivers/usb/core/hub.c:

if (hcd->driver->relinquish_port && !hub->hdev->parent)
hcd->driver->relinquish_port(hcd, port1);

That should prevent the connection from being handed over to the UHCI 
companion, allowing the device to operate at high speed.

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: drivers/usb/host/xhci-ring.c:1390 handle_cmd_completion

2017-07-26 Thread Jason A. Donenfeld
Hey Mathias,

While the oops has been fixed in the latest kernels, as of 4.12.3, I'm
still having issues where USB "stops working" after a while. If I
restart, then after a long while, it stops working again. If I simply
rmmod all the USB-related modules and modprobe xhci_pci, things work
again, but pretty soon after -- maybe 15ish minutes -- it's broken
again. If it's of any help, I'm often plugging and unplugging a USB
hub with quite a few devices on it. The message in my dmesg this time
isn't an BUG, like in the previous kernels, but rather this more
subdued set of messages:

[529153.583731] xhci_hcd :00:14.0: Timeout while waiting for setup
device command
[529159.215731] xhci_hcd :00:14.0: ERROR mismatched command completion event
[529159.215768] xhci_hcd :00:14.0: Timeout while waiting for setup
device command
[529159.423840] usb 2-2: device not accepting address 6, error -62
[529161.263641] xhci_hcd :00:14.0: Timeout while waiting for stop
endpoint command
[529166.895725] xhci_hcd :00:14.0: Command completion event does
not match command
[529166.895729] xhci_hcd :00:14.0: ERROR mismatched command
completion event
[529173.551626] xhci_hcd :00:14.0: ERROR mismatched command completion event
[529178.671242] xhci_hcd :00:14.0: xHCI host not responding to
stop endpoint command
.
[529178.671260] xhci_hcd :00:14.0: xHCI host controller not
responding, assume dead
[529178.671325] xhci_hcd :00:14.0: HC died; cleaning up
[529178.671369] xhci_hcd :00:14.0: Timeout waiting for reset
device command
[529178.671411] usb 1-2: USB disconnect, device number 16
[529178.671415] usb 1-2.2: USB disconnect, device number 17
[529178.671938] usb 1-2.3: USB disconnect, device number 18
[529178.671943] usb 1-2.3.4: USB disconnect, device number 20
[529178.673143] usb 1-2.4: USB disconnect, device number 19
[529178.673150] usb 1-2.4.1: USB disconnect, device number 21
[529178.756412] usb 1-2.4.2: USB disconnect, device number 22
[529178.830334] usb 1-2.4.3: USB disconnect, device number 23

Is this sufficient information? Or should I try to gather more somehow?

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


[PATCH v2 2/5] usb: xhci: Fix potential memory leak in xhci_disable_slot()

2017-07-26 Thread Lu Baolu
xhci_disable_slot() allows the invoker to pass a command pointer
as paramenter. Otherwise, it will allocate one. This will cause
memory leak when a command structure was allocated inside of this
function while queuing command trb fails. Another problem comes up
when the invoker passed a command pointer, but xhci_disable_slot()
frees it when it detects a dead host.

This patch fixes these two problems by removing the command parameter
from xhci_disable_slot().

Fixes: f9e609b82479 ("usb: xhci: Add helper function xhci_disable_slot().")
Cc: Guoqing Zhang 
Signed-off-by: Lu Baolu 
---
 drivers/usb/host/xhci-hub.c |  2 +-
 drivers/usb/host/xhci.c | 30 +-
 drivers/usb/host/xhci.h |  3 +--
 3 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 00721e8..c862d53 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -612,7 +612,7 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
xhci_dbg(xhci, "Disable all slots\n");
spin_unlock_irqrestore(&xhci->lock, *flags);
for (i = 1; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
-   retval = xhci_disable_slot(xhci, NULL, i);
+   retval = xhci_disable_slot(xhci, i);
if (retval)
xhci_err(xhci, "Failed to disable slot %d, %d. Enter 
test mode anyway\n",
 i, retval);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index e69073f..cb2461a 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3519,11 +3519,6 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct 
usb_device *udev)
struct xhci_virt_device *virt_dev;
struct xhci_slot_ctx *slot_ctx;
int i, ret;
-   struct xhci_command *command;
-
-   command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
-   if (!command)
-   return;
 
 #ifndef CONFIG_USB_DEFAULT_PERSIST
/*
@@ -3539,10 +3534,8 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct 
usb_device *udev)
/* If the host is halted due to driver unload, we still need to free the
 * device.
 */
-   if (ret <= 0 && ret != -ENODEV) {
-   kfree(command);
+   if (ret <= 0 && ret != -ENODEV)
return;
-   }
 
virt_dev = xhci->devs[udev->slot_id];
slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->out_ctx);
@@ -3554,22 +3547,21 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct 
usb_device *udev)
del_timer_sync(&virt_dev->eps[i].stop_cmd_timer);
}
 
-   xhci_disable_slot(xhci, command, udev->slot_id);
+   xhci_disable_slot(xhci, udev->slot_id);
/*
 * Event command completion handler will free any data structures
 * associated with the slot.  XXX Can free sleep?
 */
 }
 
-int xhci_disable_slot(struct xhci_hcd *xhci, struct xhci_command *command,
-   u32 slot_id)
+int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id)
 {
+   struct xhci_command *command;
unsigned long flags;
u32 state;
int ret = 0;
 
-   if (!command)
-   command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
+   command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
if (!command)
return -ENOMEM;
 
@@ -3588,7 +3580,7 @@ int xhci_disable_slot(struct xhci_hcd *xhci, struct 
xhci_command *command,
slot_id);
if (ret) {
spin_unlock_irqrestore(&xhci->lock, flags);
-   xhci_dbg(xhci, "FIXME: allocate a command ring segment\n");
+   kfree(command);
return ret;
}
xhci_ring_cmd_db(xhci);
@@ -3663,6 +3655,8 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device 
*udev)
return 0;
}
 
+   xhci_free_command(xhci, command);
+
if ((xhci->quirks & XHCI_EP_LIMIT_QUIRK)) {
spin_lock_irqsave(&xhci->lock, flags);
ret = xhci_reserve_host_control_ep_resources(xhci);
@@ -3698,18 +3692,12 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct 
usb_device *udev)
pm_runtime_get_noresume(hcd->self.controller);
 #endif
 
-
-   xhci_free_command(xhci, command);
/* Is this a LS or FS device under a HS hub? */
/* Hub or peripherial? */
return 1;
 
 disable_slot:
-   /* Disable slot, if we can do it without mem alloc */
-   kfree(command->completion);
-   command->completion = NULL;
-   command->status = 0;
-   return xhci_disable_slot(xhci, command, udev->slot_id);
+   return xhci_disable_slot(xhci, udev->slot_id);
 }
 
 /*
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index e3e9352..6325d58 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2003,8 +2003,7 @@ int xhci_

[PATCH v2 4/5] usb: xhci: Return error when host is dead in xhci_disable_slot()

2017-07-26 Thread Lu Baolu
xhci_disable_slot() is a helper for disabling a slot when a device
goes away or recovers from error situations. Currently, it returns
success when it sees a dead host. This is not the right way to go.
It should return error and let the invoker know that disable slot
command was failed due to a dead host.

Fixes: f9e609b82479 ("usb: xhci: Add helper function xhci_disable_slot().")
Cc: Guoqing Zhang 
Signed-off-by: Lu Baolu 
---
 drivers/usb/host/xhci.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2df601e..d6b728d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3568,10 +3568,9 @@ int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id)
state = readl(&xhci->op_regs->status);
if (state == 0x || (xhci->xhc_state & XHCI_STATE_DYING) ||
(xhci->xhc_state & XHCI_STATE_HALTED)) {
-   xhci_free_virt_device(xhci, slot_id);
spin_unlock_irqrestore(&xhci->lock, flags);
kfree(command);
-   return ret;
+   return -ENODEV;
}
 
ret = xhci_queue_slot_control(xhci, command, TRB_DISABLE_SLOT,
-- 
2.7.4

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


[PATCH v2 1/5] usb: xhci: Disable slot even virt-dev is null

2017-07-26 Thread Lu Baolu
xhci_disable_slot() is a helper for disabling a slot when a device
goes away or recovers from error situations. Currently, it checks
the corespoding virt-dev pointer and returns directly (w/o issuing
disable slot command) if it's null.

This is unnecessary and will cause problems in case where virt-dev
allocation fails and xhci_disable_slot() is called to roll back the
hardware state. Refer to the implementation of xhci_alloc_dev().

This patch removes lines to check virt-dev in xhci_disable_slot().

Fixes: f9e609b82479 ("usb: xhci: Add helper function xhci_disable_slot().")
Cc: Guoqing Zhang 
Signed-off-by: Lu Baolu 
---
 drivers/usb/host/xhci.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index b2ff1ff..e69073f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3567,11 +3567,7 @@ int xhci_disable_slot(struct xhci_hcd *xhci, struct 
xhci_command *command,
unsigned long flags;
u32 state;
int ret = 0;
-   struct xhci_virt_device *virt_dev;
 
-   virt_dev = xhci->devs[slot_id];
-   if (!virt_dev)
-   return -EINVAL;
if (!command)
command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
if (!command)
-- 
2.7.4

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


[PATCH v2 5/5] usb: xhci: Handle USB transaction error on address command

2017-07-26 Thread Lu Baolu
Xhci driver handles USB transaction errors on transfer events,
but transaction errors are possible on address device command
completion events as well.

The xHCI specification (section 4.6.5) says: A USB Transaction
Error Completion Code for an Address Device Command may be due
to a Stall response from a device. Software should issue a Disable
Slot Command for the Device Slot then an Enable Slot Command to
recover from this error.

This patch handles USB transaction errors on address command
completion events. The related discussion threads can be found
through below links.

http://marc.info/?l=linux-usb&m=149362010728921&w=2
http://marc.info/?l=linux-usb&m=149252752825755&w=2

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

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d6b728d..95780f8 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3822,6 +3822,11 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct 
usb_device *udev,
break;
case COMP_USB_TRANSACTION_ERROR:
dev_warn(&udev->dev, "Device not responding to setup %s.\n", 
act);
+
+   ret = xhci_disable_slot(xhci, udev->slot_id);
+   if (!ret)
+   xhci_alloc_dev(hcd, udev);
+
ret = -EPROTO;
break;
case COMP_INCOMPATIBLE_DEVICE_ERROR:
-- 
2.7.4

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


[PATCH v2 3/5] usb: xhci: Fix memory leak when xhci_disable_slot() returns error

2017-07-26 Thread Lu Baolu
If xhci_disable_slot() returns success, a disable slot command
trb was queued in the command ring. The command completion
handler will free the virtual device data structure associated
with the slot. On the other hand, when xhci_disable_slot()
returns error, the invokers should take the responsibilities to
free the slot related data structure. Otherwise, memory leakage
happens.

Signed-off-by: Lu Baolu 
---
 drivers/usb/host/xhci.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index cb2461a..2df601e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3547,11 +3547,9 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct 
usb_device *udev)
del_timer_sync(&virt_dev->eps[i].stop_cmd_timer);
}
 
-   xhci_disable_slot(xhci, udev->slot_id);
-   /*
-* Event command completion handler will free any data structures
-* associated with the slot.  XXX Can free sleep?
-*/
+   ret = xhci_disable_slot(xhci, udev->slot_id);
+   if (ret)
+   xhci_free_virt_device(xhci, udev->slot_id);
 }
 
 int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id)
@@ -3697,7 +3695,11 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct 
usb_device *udev)
return 1;
 
 disable_slot:
-   return xhci_disable_slot(xhci, udev->slot_id);
+   ret = xhci_disable_slot(xhci, udev->slot_id);
+   if (ret)
+   xhci_free_virt_device(xhci, udev->slot_id);
+
+   return 0;
 }
 
 /*
-- 
2.7.4

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


[PATCH v2 0/5] usb: xhci: Handle USB transaction error on address command

2017-07-26 Thread Lu Baolu
Xhci driver handles USB transaction errors on transfer events,
but transaction errors are possible on address device command
completion events as well.

The xHCI specification (section 4.6.5) says: A USB Transaction
Error Completion Code for an Address Device Command may be due
to a Stall response from a device. Software should issue a Disable
Slot Command for the Device Slot then an Enable Slot Command to
recover from this error.

The related discussion threads can be found through below links.

http://marc.info/?l=linux-usb&m=149362010728921&w=2
http://marc.info/?l=linux-usb&m=149252752825755&w=2

This patch set includes some fixes in xhci_disable_slot() as well
which will be used to handle USB transaction error on address
command.

---
Change log:

v1->v2:
 - include 4 fixes in xhci_disable_slot which will be used
   to handle USB transaction error on address command.

Lu Baolu (5):
  usb: xhci: Disable slot even virt-dev is null
  usb: xhci: Fix potential memory leak in xhci_disable_slot()
  usb: xhci: Fix memory leak when xhci_disable_slot() returns error
  usb: xhci: Return error when host is dead in xhci_disable_slot()
  usb: xhci: Handle USB transaction error on address command

 drivers/usb/host/xhci-hub.c |  2 +-
 drivers/usb/host/xhci.c | 52 ++---
 drivers/usb/host/xhci.h |  3 +--
 3 files changed, 23 insertions(+), 34 deletions(-)

-- 
2.7.4

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


Re: [PATCH v3 3/3] power: wm831x_power: Support USB charger current limit management

2017-07-26 Thread Baolin Wang
Hi,

On 26 July 2017 at 20:08, Sebastian Reichel
 wrote:
> Hi,
>
> On Wed, Jul 26, 2017 at 11:05:25AM +0800, Baolin Wang wrote:
>> On 25 July 2017 at 17:59, Sebastian Reichel
>>  wrote:
>> > On Tue, Jul 25, 2017 at 04:00:01PM +0800, Baolin Wang wrote:
>> >> Integrate with the newly added USB charger interface to limit the current
>> >> we draw from the USB input based on the input device configuration
>> >> identified by the USB stack, allowing us to charge more quickly from high
>> >> current inputs without drawing more current than specified from others.
>> >>
>> >> Signed-off-by: Mark Brown 
>> >> Signed-off-by: Baolin Wang 
>> >> ---
>> >>  Documentation/devicetree/bindings/mfd/wm831x.txt |1 +
>> >>  drivers/power/supply/wm831x_power.c  |   58 
>> >> ++
>> >>  2 files changed, 59 insertions(+)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/mfd/wm831x.txt 
>> >> b/Documentation/devicetree/bindings/mfd/wm831x.txt
>> >> index 9f8b743..4e3bc07 100644
>> >> --- a/Documentation/devicetree/bindings/mfd/wm831x.txt
>> >> +++ b/Documentation/devicetree/bindings/mfd/wm831x.txt
>> >> @@ -31,6 +31,7 @@ Required properties:
>> >>  ../interrupt-controller/interrupts.txt
>> >>
>> >>  Optional sub-nodes:
>> >> +  - usb-phy : Contains a phandle to the USB PHY.
>> >>- regulators : Contains sub-nodes for each of the regulators supplied 
>> >> by
>> >>  the device. The regulators are bound using their names listed below:
>> >>
>> >> diff --git a/drivers/power/supply/wm831x_power.c 
>> >> b/drivers/power/supply/wm831x_power.c
>> >> index 7082301..d3948ab 100644
>> >> --- a/drivers/power/supply/wm831x_power.c
>> >> +++ b/drivers/power/supply/wm831x_power.c
>> >> @@ -13,6 +13,7 @@
>> >>  #include 
>> >>  #include 
>> >>  #include 
>> >> +#include 
>> >>
>> >>  #include 
>> >>  #include 
>> >> @@ -31,6 +32,8 @@ struct wm831x_power {
>> >>   char usb_name[20];
>> >>   char battery_name[20];
>> >>   bool have_battery;
>> >> + struct usb_phy *usb_phy;
>> >> + struct notifier_block usb_notify;
>> >>  };
>> >>
>> >>  static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
>> >> @@ -125,6 +128,43 @@ static int wm831x_usb_get_prop(struct power_supply 
>> >> *psy,
>> >>   POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> >>  };
>> >>
>> >> +/* In milliamps */
>> >> +static const unsigned int wm831x_usb_limits[] = {
>> >> + 0,
>> >> + 2,
>> >> + 100,
>> >> + 500,
>> >> + 900,
>> >> + 1500,
>> >> + 1800,
>> >> + 550,
>> >> +};
>> >> +
>> >> +static int wm831x_usb_limit_change(struct notifier_block *nb,
>> >> +unsigned long limit, void *data)
>> >> +{
>> >> + struct wm831x_power *wm831x_power = container_of(nb,
>> >> +  struct 
>> >> wm831x_power,
>> >> +  usb_notify);
>> >> + unsigned int i, best;
>> >> +
>> >> + /* Find the highest supported limit */
>> >> + best = 0;
>> >> + for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
>> >> + if (limit >= wm831x_usb_limits[i] &&
>> >> + wm831x_usb_limits[best] < wm831x_usb_limits[i])
>> >> + best = i;
>> >> + }
>> >> +
>> >> + dev_dbg(wm831x_power->wm831x->dev,
>> >> + "Limiting USB current to %umA", wm831x_usb_limits[best]);
>> >> +
>> >> + wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
>> >> + WM831X_USB_ILIM_MASK, best);
>> >> +
>> >> + return 0;
>> >> +}
>> >> +
>> >>  /*
>> >>   *   Battery properties
>> >>   */
>> >> @@ -607,6 +647,19 @@ static int wm831x_power_probe(struct platform_device 
>> >> *pdev)
>> >>   }
>> >>   }
>> >>
>> >> + power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev,
>> >> +  "usb-phy", 0);
>> >> + if (!IS_ERR(power->usb_phy)) {
>> >> + power->usb_notify.notifier_call = wm831x_usb_limit_change;
>> >> + ret = usb_register_notifier(power->usb_phy,
>> >> + &power->usb_notify);
>> >> + if (ret) {
>> >> + dev_err(&pdev->dev, "Failed to register notifier: 
>> >> %d\n",
>> >> + ret);
>> >> + goto err_bat_irq;
>> >> + }
>> >> + }
>> >
>> > No error handling for power->usb_phy? I think you should bail out
>> > for all errors except for "not defined in DT". Especially I would
>> > expect probe defer handling in case the power supply driver is
>> > loaded before the phy driver.
>>
>> Make sense. So I think I need to change like below:
>>
>> power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
>> if (!IS_ERR(powe

[PATCH v4 0/3] Introduce USB charger support in USB phy

2017-07-26 Thread Baolin Wang
Currently the Linux kernel does not provide any standard integration of this
feature that integrates the USB subsystem with the system power regulation
provided by PMICs meaning that either vendors must add this in their kernels
or USB gadget devices based on Linux (such as mobile phones) may not behave
as they should. Thus provide a standard USB charger support in USB phy core
for doing this in kernel.

Now introduce one user with wm831x_power to support and test the usb charger.
In future we will also cnvert below power drivers:
drivers/power/supply/axp288_charger.c
drivers/power/supply/bq24190_charger.c
drivers/power/supply/charger-manager.c
drivers/power/supply/qcom_smbb.c

Changes since v3:
 - Bail out errors when failed to find usb phy for wm831x_power driver.
Changes since v2:
 - Add DT binding documentation for wm831x_power driver.
 - Change 'usb-phy' as one optional property for wm831x_power driver.
Changes since v1:
 - Fix building errors.

Baolin Wang (3):
  include: uapi: usb: Introduce USB charger type and state definition
  usb: phy: Add USB charger support
  power: wm831x_power: Support USB charger current limit management

 Documentation/devicetree/bindings/mfd/wm831x.txt |1 +
 drivers/power/supply/wm831x_power.c  |   72 ++
 drivers/usb/phy/phy.c|  272 ++
 include/linux/usb/phy.h  |   49 
 include/uapi/linux/usb/charger.h |   31 +++
 5 files changed, 425 insertions(+)
 create mode 100644 include/uapi/linux/usb/charger.h

-- 
1.7.9.5

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


[PATCH v4 3/3] power: wm831x_power: Support USB charger current limit management

2017-07-26 Thread Baolin Wang
Integrate with the newly added USB charger interface to limit the current
we draw from the USB input based on the input device configuration
identified by the USB stack, allowing us to charge more quickly from high
current inputs without drawing more current than specified from others.

Signed-off-by: Mark Brown 
Signed-off-by: Baolin Wang 
Acked-by: Lee Jones 
Acked-by: Charles Keepax 
---
 Documentation/devicetree/bindings/mfd/wm831x.txt |1 +
 drivers/power/supply/wm831x_power.c  |   72 ++
 2 files changed, 73 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/wm831x.txt 
b/Documentation/devicetree/bindings/mfd/wm831x.txt
index 9f8b743..4e3bc07 100644
--- a/Documentation/devicetree/bindings/mfd/wm831x.txt
+++ b/Documentation/devicetree/bindings/mfd/wm831x.txt
@@ -31,6 +31,7 @@ Required properties:
 ../interrupt-controller/interrupts.txt
 
 Optional sub-nodes:
+  - usb-phy : Contains a phandle to the USB PHY.
   - regulators : Contains sub-nodes for each of the regulators supplied by
 the device. The regulators are bound using their names listed below:
 
diff --git a/drivers/power/supply/wm831x_power.c 
b/drivers/power/supply/wm831x_power.c
index 7082301..dff6473 100644
--- a/drivers/power/supply/wm831x_power.c
+++ b/drivers/power/supply/wm831x_power.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -31,6 +32,8 @@ struct wm831x_power {
char usb_name[20];
char battery_name[20];
bool have_battery;
+   struct usb_phy *usb_phy;
+   struct notifier_block usb_notify;
 };
 
 static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
@@ -125,6 +128,43 @@ static int wm831x_usb_get_prop(struct power_supply *psy,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
 };
 
+/* In milliamps */
+static const unsigned int wm831x_usb_limits[] = {
+   0,
+   2,
+   100,
+   500,
+   900,
+   1500,
+   1800,
+   550,
+};
+
+static int wm831x_usb_limit_change(struct notifier_block *nb,
+  unsigned long limit, void *data)
+{
+   struct wm831x_power *wm831x_power = container_of(nb,
+struct wm831x_power,
+usb_notify);
+   unsigned int i, best;
+
+   /* Find the highest supported limit */
+   best = 0;
+   for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
+   if (limit >= wm831x_usb_limits[i] &&
+   wm831x_usb_limits[best] < wm831x_usb_limits[i])
+   best = i;
+   }
+
+   dev_dbg(wm831x_power->wm831x->dev,
+   "Limiting USB current to %umA", wm831x_usb_limits[best]);
+
+   wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
+   WM831X_USB_ILIM_MASK, best);
+
+   return 0;
+}
+
 /*
  * Battery properties
  */
@@ -607,6 +647,33 @@ static int wm831x_power_probe(struct platform_device *pdev)
}
}
 
+   power->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
+   ret = PTR_ERR_OR_ZERO(power->usb_phy);
+
+   switch (ret) {
+   case 0:
+   power->usb_notify.notifier_call = wm831x_usb_limit_change;
+   ret = usb_register_notifier(power->usb_phy, &power->usb_notify);
+   if (ret) {
+   dev_err(&pdev->dev, "Failed to register notifier: %d\n",
+   ret);
+   goto err_bat_irq;
+   }
+   break;
+   case -EINVAL:
+   case -ENODEV:
+   /* ignore missing usb-phy, it's optional */
+   power->usb_phy = NULL;
+   ret = 0;
+   break;
+   default:
+   dev_err(&pdev->dev, "Failed to find USB phy: %d\n", ret);
+   /* fall-through */
+   case -EPROBE_DEFER:
+   goto err_bat_irq;
+   break;
+   }
+
return ret;
 
 err_bat_irq:
@@ -637,6 +704,11 @@ static int wm831x_power_remove(struct platform_device 
*pdev)
struct wm831x *wm831x = wm831x_power->wm831x;
int irq, i;
 
+   if (wm831x_power->usb_phy) {
+   usb_unregister_notifier(wm831x_power->usb_phy,
+   &wm831x_power->usb_notify);
+   }
+
for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
irq = wm831x_irq(wm831x, 
 platform_get_irq_byname(pdev,
-- 
1.7.9.5

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


[PATCH v4 2/3] usb: phy: Add USB charger support

2017-07-26 Thread Baolin Wang
This patch introduces the usb charger support based on usb phy that
makes an enhancement to a power driver. The basic conception of the
usb charger is that, when one usb charger is added or removed by
reporting from the extcon device state change, the usb charger will
report to power user to set the current limitation.

Power user can register a notifiee on the usb phy by issuing
usb_register_notifier() to get notified by charger status changes
or charger current changes.

we can notify what current to be drawn to power user according to
different charger type, and now we have 2 methods to get charger type.
One is get charger type from extcon subsystem, which also means the
charger state changes. Another is we can get the charger type from
USB controller detecting or PMIC detecting, and the charger state
changes should be told by issuing usb_phy_set_charger_state().

Signed-off-by: Baolin Wang 
---
 drivers/usb/phy/phy.c   |  272 +++
 include/linux/usb/phy.h |   49 +
 2 files changed, 321 insertions(+)

diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index 032f5af..2dc48bb 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -18,6 +18,18 @@
 
 #include 
 
+/* Default current range by charger type. */
+#define DEFAULT_SDP_CUR_MIN2
+#define DEFAULT_SDP_CUR_MAX500
+#define DEFAULT_SDP_CUR_MIN_SS 150
+#define DEFAULT_SDP_CUR_MAX_SS 900
+#define DEFAULT_DCP_CUR_MIN500
+#define DEFAULT_DCP_CUR_MAX5000
+#define DEFAULT_CDP_CUR_MIN1500
+#define DEFAULT_CDP_CUR_MAX5000
+#define DEFAULT_ACA_CUR_MIN1500
+#define DEFAULT_ACA_CUR_MAX5000
+
 static LIST_HEAD(phy_list);
 static LIST_HEAD(phy_bind_list);
 static DEFINE_SPINLOCK(phy_lock);
@@ -77,6 +89,221 @@ static struct usb_phy *__of_usb_find_phy(struct device_node 
*node)
return ERR_PTR(-EPROBE_DEFER);
 }
 
+static void usb_phy_set_default_current(struct usb_phy *usb_phy)
+{
+   usb_phy->chg_cur.sdp_min = DEFAULT_SDP_CUR_MIN;
+   usb_phy->chg_cur.sdp_max = DEFAULT_SDP_CUR_MAX;
+   usb_phy->chg_cur.dcp_min = DEFAULT_DCP_CUR_MIN;
+   usb_phy->chg_cur.dcp_max = DEFAULT_DCP_CUR_MAX;
+   usb_phy->chg_cur.cdp_min = DEFAULT_CDP_CUR_MIN;
+   usb_phy->chg_cur.cdp_max = DEFAULT_CDP_CUR_MAX;
+   usb_phy->chg_cur.aca_min = DEFAULT_ACA_CUR_MIN;
+   usb_phy->chg_cur.aca_max = DEFAULT_ACA_CUR_MAX;
+}
+
+/**
+ * usb_phy_notify_charger_work - notify the USB charger state
+ * @work - the charger work to notify the USB charger state
+ *
+ * This work can be issued when USB charger state has been changed or
+ * USB charger current has been changed, then we can notify the current
+ * what can be drawn to power user and the charger state to userspace.
+ *
+ * If we get the charger type from extcon subsystem, we can notify the
+ * charger state to power user automatically by usb_phy_get_charger_type()
+ * issuing from extcon subsystem.
+ *
+ * If we get the charger type from ->charger_detect() instead of extcon
+ * subsystem, the usb phy driver should issue usb_phy_set_charger_state()
+ * to set charger state when the charger state has been changed.
+ */
+static void usb_phy_notify_charger_work(struct work_struct *work)
+{
+   struct usb_phy *usb_phy = container_of(work, struct usb_phy, chg_work);
+   char uchger_state[50] = { 0 };
+   char *envp[] = { uchger_state, NULL };
+   unsigned int min, max;
+
+   switch (usb_phy->chg_state) {
+   case USB_CHARGER_PRESENT:
+   usb_phy_get_charger_current(usb_phy, &min, &max);
+
+   atomic_notifier_call_chain(&usb_phy->notifier, max, usb_phy);
+   snprintf(uchger_state, ARRAY_SIZE(uchger_state),
+"USB_CHARGER_STATE=%s", "USB_CHARGER_PRESENT");
+   break;
+   case USB_CHARGER_ABSENT:
+   usb_phy_set_default_current(usb_phy);
+
+   atomic_notifier_call_chain(&usb_phy->notifier, 0, usb_phy);
+   snprintf(uchger_state, ARRAY_SIZE(uchger_state),
+"USB_CHARGER_STATE=%s", "USB_CHARGER_ABSENT");
+   break;
+   default:
+   dev_warn(usb_phy->dev, "Unknown USB charger state: %d\n",
+usb_phy->chg_state);
+   return;
+   }
+
+   kobject_uevent_env(&usb_phy->dev->kobj, KOBJ_CHANGE, envp);
+}
+
+static void __usb_phy_get_charger_type(struct usb_phy *usb_phy)
+{
+   if (extcon_get_state(usb_phy->edev, EXTCON_CHG_USB_SDP) > 0) {
+   usb_phy->chg_type = SDP_TYPE;
+   usb_phy->chg_state = USB_CHARGER_PRESENT;
+   } else if (extcon_get_state(usb_phy->edev, EXTCON_CHG_USB_CDP) > 0) {
+   usb_phy->chg_type = CDP_TYPE;
+   usb_phy->chg_state = USB_CHARGER_PRESENT;
+   } else if (extcon_get_state(usb_phy->edev, EXTCON_CHG_USB_DCP) > 0) {
+   usb_phy->chg_type = DCP_TYPE;
+   usb_phy->chg_state = USB_CHARGER_PRESENT;
+

[PATCH v4 1/3] include: uapi: usb: Introduce USB charger type and state definition

2017-07-26 Thread Baolin Wang
Introducing USB charger type and state definition can help
to support USB charging which will be added in USB phy core.

Signed-off-by: Baolin Wang 
---
 include/uapi/linux/usb/charger.h |   31 +++
 1 file changed, 31 insertions(+)
 create mode 100644 include/uapi/linux/usb/charger.h

diff --git a/include/uapi/linux/usb/charger.h b/include/uapi/linux/usb/charger.h
new file mode 100644
index 000..5f72af3
--- /dev/null
+++ b/include/uapi/linux/usb/charger.h
@@ -0,0 +1,31 @@
+/*
+ * This file defines the USB charger type and state that are needed for
+ * USB device APIs.
+ */
+
+#ifndef _UAPI__LINUX_USB_CHARGER_H
+#define _UAPI__LINUX_USB_CHARGER_H
+
+/*
+ * USB charger type:
+ * SDP (Standard Downstream Port)
+ * DCP (Dedicated Charging Port)
+ * CDP (Charging Downstream Port)
+ * ACA (Accessory Charger Adapters)
+ */
+enum usb_charger_type {
+   UNKNOWN_TYPE,
+   SDP_TYPE,
+   DCP_TYPE,
+   CDP_TYPE,
+   ACA_TYPE,
+};
+
+/* USB charger state */
+enum usb_charger_state {
+   USB_CHARGER_DEFAULT,
+   USB_CHARGER_PRESENT,
+   USB_CHARGER_ABSENT,
+};
+
+#endif /* _UAPI__LINUX_USB_CHARGER_H */
-- 
1.7.9.5

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