Re: [PATCH] xhci:Fix NULL pointer in xhci debugfs

[Mathias Nyman]

On 19.01.2018 04:13, Zhengjun Xing wrote:

Commit dde634057da7 ("xhci: Fix use-after-free in xhci debugfs") causes a
null pointer dereference while fixing xhci-debugfs usage of ring pointers
that were freed during hibernate.

The fix passed addresses to ring pointers instead, but forgot to do this
change for the xhci_ring_trb_show function.

The address of the ring pointer passed to xhci-debugfs was of a temporary
ring pointer "new_ring" instead of the actual ring "ring" pointer. The
temporary new_ring pointer will be set to NULL later causing the NULL
pointer dereference.

This issue was seen when reading xhci related files in debugfs:

cat /sys/kernel/debug/usb/xhci/*/devices/*/ep*/trbs

[  184.604861] BUG: unable to handle kernel NULL pointer dereference at (null)
[  184.613776] IP: xhci_ring_trb_show+0x3a/0x890
[  184.618733] PGD 264193067 P4D 264193067 PUD 263238067 PMD 0
[  184.625184] Oops:  [#1] SMP
[  184.726410] RIP: 0010:xhci_ring_trb_show+0x3a/0x890
[  184.731944] RSP: 0018:ba8243c0fd90 EFLAGS: 00010246
[  184.737880] RAX:  RBX:  RCX: 000295d6
[  184.746020] RDX: 000295d5 RSI: 0001 RDI: 971a6418d400
[  184.754121] RBP:  R08:  R09: 
[  184.76] R10: 971a64c98a80 R11: 971a62a00e40 R12: 971a62a85500
[  184.770325] R13: 0002 R14: 971a6418d400 R15: 971a6418d400
[  184.778448] FS:  7fe725a79700() GS:971a6ec0() 
knlGS:
[  184.787644] CS:  0010 DS:  ES:  CR0: 80050033
[  184.794168] CR2:  CR3: 00025f365005 CR4: 003606f0
[  184.802318] Call Trace:
[  184.805094]  ? seq_read+0x281/0x3b0
[  184.809068]  seq_read+0xeb/0x3b0
[  184.812735]  full_proxy_read+0x4d/0x70
[  184.817007]  __vfs_read+0x23/0x120
[  184.820870]  vfs_read+0x91/0x130
[  184.824538]  SyS_read+0x42/0x90
[  184.828106]  entry_SYSCALL_64_fastpath+0x1a/0x7d

Fixes: dde634057da7 ("xhci: Fix use-after-free in xhci debugfs")
Signed-off-by: Zhengjun Xing 
---


Thanks, adding  to queue

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

RE: [RFC V2 PATCH 3/3] usb: chipidea: usbmisc: small clean up

[Peter Chen]

 
> 
> Signed-off-by: Michael Trimarchi 
> ---
> Changes v1->v2:
> - None
> ---
>  drivers/usb/chipidea/usbmisc_imx.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
> b/drivers/usb/chipidea/usbmisc_imx.c
> index 1f47ef6..34ad5bf 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -315,13 +315,12 @@ static int usbmisc_imx6q_set_wakeup
>   val = readl(usbmisc->base + data->index * 4);
>   if (enabled) {
>   val |= wakeup_setting;
> - writel(val, usbmisc->base + data->index * 4);
>   } else {
>   if (val & MX6_BM_WAKEUP_INTR)
>   pr_debug("wakeup int at ci_hdrc.%d\n", data->index);
>   val &= ~wakeup_setting;
> - writel(val, usbmisc->base + data->index * 4);
>   }
> + writel(val, usbmisc->base + data->index * 4);
>   spin_unlock_irqrestore(>lock, flags);
> 
>   return ret;
> --

Applied, thanks.

Peter
--
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 TYPEC: RT1711H Type-C Chip Driver

[李書帆]

Hi Jun,

  For now, RT1711H is not fully compatible with TCPCI. So the existing tcpci.c 
may not work for it.

Best Regards,
*
Shu-Fan Lee
Richtek Technology Corporation
TEL: +886-3-5526789 #2359
FAX: +886-3-5526612
*

-Original Message-
From: Jun Li [mailto:jun...@nxp.com]
Sent: Friday, January 19, 2018 11:10 AM
To: ShuFanLee; heikki.kroge...@linux.intel.com
Cc: cy_huang(黃啟原); shufan_lee(李書帆); linux-ker...@vger.kernel.org; 
linux-usb@vger.kernel.org; Guenter Roeck
Subject: RE: [PATCH] USB TYPEC: RT1711H Type-C Chip Driver

Hi
> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of ShuFanLee
> Sent: Wednesday, January 10, 2018 2:59 PM
> To: heikki.kroge...@linux.intel.com
> Cc: cy_hu...@richtek.com; shufan_...@richtek.com; linux-
> ker...@vger.kernel.org; linux-usb@vger.kernel.org
> Subject: [PATCH] USB TYPEC: RT1711H Type-C Chip Driver
>
> From: ShuFanLee 
>
> Richtek RT1711H Type-C chip driver that works with Type-C Port
> Controller Manager to provide USB PD and USB Type-C functionalities.

A general question, is this Rt1711h type-c chip compatible with TCPCI 
(Universal Serial Bus Type-C Port Controller Interface Specification)?
looks like it has the same register map and has some extension, can the 
existing ./drivers/staging/typec/tcpic.c basically work for you?

+Guenter

Li Jun

>
> Signed-off-by: ShuFanLee 
> ---
>  .../devicetree/bindings/usb/richtek,rt1711h.txt|   38 +
>  arch/arm64/boot/dts/hisilicon/rt1711h.dtsi |   11 +
>  drivers/usb/typec/Kconfig  |2 +
>  drivers/usb/typec/Makefile |1 +
>  drivers/usb/typec/rt1711h/Kconfig  |7 +
>  drivers/usb/typec/rt1711h/Makefile |2 +
>  drivers/usb/typec/rt1711h/rt1711h.c| 2241 
> 
>  drivers/usb/typec/rt1711h/rt1711h.h|  300 +++
>  8 files changed, 2602 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/usb/richtek,rt1711h.txt
>  create mode 100644 arch/arm64/boot/dts/hisilicon/rt1711h.dtsi
>  create mode 100644 drivers/usb/typec/rt1711h/Kconfig  create mode
> 100644 drivers/usb/typec/rt1711h/Makefile
>  create mode 100644 drivers/usb/typec/rt1711h/rt1711h.c
>  create mode 100644 drivers/usb/typec/rt1711h/rt1711h.h
>
* Email Confidentiality Notice 

The information contained in this e-mail message (including any attachments) 
may be confidential, proprietary, privileged, or otherwise exempt from 
disclosure under applicable laws. It is intended to be conveyed only to the 
designated recipient(s). Any use, dissemination, distribution, printing, 
retaining or copying of this e-mail (including its attachments) by unintended 
recipient(s) is strictly prohibited and may be unlawful. If you are not an 
intended recipient of this e-mail, or believe that you have received this 
e-mail in error, please notify the sender immediately (by replying to this 
e-mail), delete any and all copies of this e-mail (including any attachments) 
from your system, and do not disclose the content of this e-mail to any other 
person. Thank you!

RE: [PATCH] USB TYPEC: RT1711H Type-C Chip Driver

[Jun Li]

Hi
> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of ShuFanLee
> Sent: Wednesday, January 10, 2018 2:59 PM
> To: heikki.kroge...@linux.intel.com
> Cc: cy_hu...@richtek.com; shufan_...@richtek.com; linux-
> ker...@vger.kernel.org; linux-usb@vger.kernel.org
> Subject: [PATCH] USB TYPEC: RT1711H Type-C Chip Driver
> 
> From: ShuFanLee 
> 
> Richtek RT1711H Type-C chip driver that works with
> Type-C Port Controller Manager to provide USB PD and
> USB Type-C functionalities.

A general question, is this Rt1711h type-c chip compatible with TCPCI
(Universal Serial Bus Type-C Port Controller Interface Specification)?
looks like it has the same register map and has some extension, can
the existing ./drivers/staging/typec/tcpic.c basically work for you?

+Guenter

Li Jun 

> 
> Signed-off-by: ShuFanLee 
> ---
>  .../devicetree/bindings/usb/richtek,rt1711h.txt|   38 +
>  arch/arm64/boot/dts/hisilicon/rt1711h.dtsi |   11 +
>  drivers/usb/typec/Kconfig  |2 +
>  drivers/usb/typec/Makefile |1 +
>  drivers/usb/typec/rt1711h/Kconfig  |7 +
>  drivers/usb/typec/rt1711h/Makefile |2 +
>  drivers/usb/typec/rt1711h/rt1711h.c| 2241 
> 
>  drivers/usb/typec/rt1711h/rt1711h.h|  300 +++
>  8 files changed, 2602 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/usb/richtek,rt1711h.txt
>  create mode 100644 arch/arm64/boot/dts/hisilicon/rt1711h.dtsi
>  create mode 100644 drivers/usb/typec/rt1711h/Kconfig
>  create mode 100644 drivers/usb/typec/rt1711h/Makefile
>  create mode 100644 drivers/usb/typec/rt1711h/rt1711h.c
>  create mode 100644 drivers/usb/typec/rt1711h/rt1711h.h
> 

[PATCH] xhci:Fix NULL pointer in xhci debugfs

[Zhengjun Xing]

Commit dde634057da7 ("xhci: Fix use-after-free in xhci debugfs") causes a
null pointer dereference while fixing xhci-debugfs usage of ring pointers
that were freed during hibernate.

The fix passed addresses to ring pointers instead, but forgot to do this
change for the xhci_ring_trb_show function.

The address of the ring pointer passed to xhci-debugfs was of a temporary
ring pointer "new_ring" instead of the actual ring "ring" pointer. The
temporary new_ring pointer will be set to NULL later causing the NULL
pointer dereference.

This issue was seen when reading xhci related files in debugfs:

cat /sys/kernel/debug/usb/xhci/*/devices/*/ep*/trbs

[  184.604861] BUG: unable to handle kernel NULL pointer dereference at (null)
[  184.613776] IP: xhci_ring_trb_show+0x3a/0x890
[  184.618733] PGD 264193067 P4D 264193067 PUD 263238067 PMD 0
[  184.625184] Oops:  [#1] SMP
[  184.726410] RIP: 0010:xhci_ring_trb_show+0x3a/0x890
[  184.731944] RSP: 0018:ba8243c0fd90 EFLAGS: 00010246
[  184.737880] RAX:  RBX:  RCX: 000295d6
[  184.746020] RDX: 000295d5 RSI: 0001 RDI: 971a6418d400
[  184.754121] RBP:  R08:  R09: 
[  184.76] R10: 971a64c98a80 R11: 971a62a00e40 R12: 971a62a85500
[  184.770325] R13: 0002 R14: 971a6418d400 R15: 971a6418d400
[  184.778448] FS:  7fe725a79700() GS:971a6ec0() 
knlGS:
[  184.787644] CS:  0010 DS:  ES:  CR0: 80050033
[  184.794168] CR2:  CR3: 00025f365005 CR4: 003606f0
[  184.802318] Call Trace:
[  184.805094]  ? seq_read+0x281/0x3b0
[  184.809068]  seq_read+0xeb/0x3b0
[  184.812735]  full_proxy_read+0x4d/0x70
[  184.817007]  __vfs_read+0x23/0x120
[  184.820870]  vfs_read+0x91/0x130
[  184.824538]  SyS_read+0x42/0x90
[  184.828106]  entry_SYSCALL_64_fastpath+0x1a/0x7d

Fixes: dde634057da7 ("xhci: Fix use-after-free in xhci debugfs")
Signed-off-by: Zhengjun Xing 
---
 drivers/usb/host/xhci-debugfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index e26e685d8a57..5851052d4668 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -211,7 +211,7 @@ static void xhci_ring_dump_segment(struct seq_file *s,
 static int xhci_ring_trb_show(struct seq_file *s, void *unused)
 {
int i;
-   struct xhci_ring*ring = s->private;
+   struct xhci_ring*ring = *(struct xhci_ring **)s->private;
struct xhci_segment *seg = ring->first_seg;
 
for (i = 0; i < ring->num_segs; i++) {
@@ -387,7 +387,7 @@ void xhci_debugfs_create_endpoint(struct xhci_hcd *xhci,
 
snprintf(epriv->name, sizeof(epriv->name), "ep%02d", ep_index);
epriv->root = xhci_debugfs_create_ring_dir(xhci,
-  >eps[ep_index].new_ring,
+  >eps[ep_index].ring,
   epriv->name,
   spriv->root);
spriv->eps[ep_index] = epriv;
-- 
2.11.0

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

[PATCH] usbip: vhci_hcd: update 'status' file header and format

[Shuah Khan]

Commit 2f2d0088eb93
("usbip: prevent vhci_hcd driver from leaking a socket pointer address")
in the /sys/devices/platform/vhci_hcd/status.

Fix the header and field alignment to reflect the changes and make it
easier to read.

Signed-off-by: Shuah Khan 
---
 drivers/usb/usbip/vhci_sysfs.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index 091f76b7196d..a9de15cab2ec 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -17,10 +17,10 @@
 
 /*
  * output example:
- * hub port sta spd dev   sockfdlocal_busid
- * hs   004 000   3 1-2.3
+ * hub port sta spd dev   sockfd local_busid
+ * hs   004 000   03 1-2.3
  * 
- * ss  0008 004 000   4 2-3.4
+ * ss  0008 004 000   04 2-3.4
  * 
  *
  * Output includes socket fd instead of socket pointer address to avoid
@@ -44,13 +44,13 @@ static void port_show_vhci(char **out, int hub, int port, 
struct vhci_device *vd
if (vdev->ud.status == VDEV_ST_USED) {
*out += sprintf(*out, "%03u %08x ",
  vdev->speed, vdev->devid);
-   *out += sprintf(*out, "%u %s",
+   *out += sprintf(*out, "%06u %s",
  vdev->ud.sockfd,
  dev_name(>udev->dev));
 
} else {
*out += sprintf(*out, "000  ");
-   *out += sprintf(*out, " 0-0");
+   *out += sprintf(*out, "00 0-0");
}
 
*out += sprintf(*out, "\n");
@@ -148,7 +148,7 @@ static ssize_t status_show(struct device *dev,
int pdev_nr;
 
out += sprintf(out,
-  "hub port sta spd dev  socket   
local_busid\n");
+  "hub port sta spd dev  sockfd local_busid\n");
 
pdev_nr = status_name_to_id(attr->attr.name);
if (pdev_nr < 0)
-- 
2.14.1

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

[PATCH] usb: gadget: compress return logic into one line

[Gustavo A. R. Silva]

Simplify return logic and avoid unnecessary variable assignment.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/usb/gadget/legacy/ncm.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/legacy/ncm.c b/drivers/usb/gadget/legacy/ncm.c
index fcee1ee0..8465f08 100644
--- a/drivers/usb/gadget/legacy/ncm.c
+++ b/drivers/usb/gadget/legacy/ncm.c
@@ -102,10 +102,8 @@ static int ncm_do_config(struct usb_configuration *c)
}
 
f_ncm = usb_get_function(f_ncm_inst);
-   if (IS_ERR(f_ncm)) {
-   status = PTR_ERR(f_ncm);
-   return status;
-   }
+   if (IS_ERR(f_ncm))
+   return PTR_ERR(f_ncm);
 
status = usb_add_function(c, f_ncm);
if (status < 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 v4] usb: ehci-omap: don't complain on -EPROBE_DEFER when no PHY found

[Ladislav Michl]

Don't complain on -EPROBE_DEFER when no PHY found, the driver
probe will be retried later.

Signed-off-by: Ladislav Michl 
Acked-by: Tony Lindgren 
Acked-by: Roger Quadros 
Acked-by: Alan Stern 
---
 Changes:
 - v2: None, just added Tony's ack
 - v3: Fixed commit message typo spotted by Jaejoong Kim, added Roger's ack
 - v4: Do not break printk string, Alan's ack. I hope he forgives me
   changing error message a bit. IMHO shorter with the same meaning.
   (this patch gained unbelievable lines_changed/versions ratio ;-))

 drivers/usb/host/ehci-omap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 854b146a457d..8d8bafc70c1f 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -167,7 +167,8 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
continue;
 
ret = PTR_ERR(phy);
-   dev_err(dev, "Can't get PHY device for port %d: %d\n",
+   if (ret != -EPROBE_DEFER)
+   dev_err(dev, "Can't get PHY for port %d: %d\n",
i, ret);
goto err_phy;
}
-- 
2.15.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dts-bindings: usb-uhci: Document newly added properties

[Joel Stanley]

On Mon, Jan 15, 2018 at 6:06 PM, Benjamin Herrenschmidt
 wrote:
> Commit 4642d34a439f80e16af0d56ed6258a33abae257a
> "usb/uhci: Add support for Aspeed BMC SoCs" added
> support for a new #ports property, along with
> Aspeed specific compatible strings. This adds
> documentation for them.
>
> Signed-off-by: Benjamin Herrenschmidt 

Acked-by: Joel Stanley 


> ---
>  Documentation/devicetree/bindings/usb/usb-uhci.txt | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/usb-uhci.txt 
> b/Documentation/devicetree/bindings/usb/usb-uhci.txt
> index 298133416c97..c1153eb38a9a 100644
> --- a/Documentation/devicetree/bindings/usb/usb-uhci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-uhci.txt
> @@ -3,8 +3,14 @@ Generic Platform UHCI Controller
>
>  Required properties:
>  - compatible : "generic-uhci" (deprecated: "platform-uhci")
> +  supports additional Aspeed specific workarounds by also
> +  including the "aspeed,ast2400-uhci" or "aspeed,ast2500-uhci"
> +  string.
>  - reg : Should contain 1 register ranges(address and length)
>  - interrupts : UHCI controller interrupt
> +- #ports: (optional) 1 cell, number of ports (overrides
> +  auto-detection which is needed among others on some Aspeed
> +  parts).
>
>  Example:
>
>
>
--
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] usb: ehci-omap: don't complain on -EPROBE_DEFER when no PHY found

[Alan Stern]

On Thu, 18 Jan 2018, Ladislav Michl wrote:

> On Thu, Jan 18, 2018 at 10:18:41AM -0500, Alan Stern wrote:
> > On Thu, 18 Jan 2018, Ladislav Michl wrote:
> > 
> > > Don't complain on -EPROBE_DEFER when no PHY found, the driver
> > > probe will be retried later.
> > > 
> > > Signed-off-by: Ladislav Michl 
> > > Acked-by: Tony Lindgren 
> > > Acked-by: Roger Quadros 
> > > ---
> > >  Changes:
> > >  - v2: None, just added Tony's ack
> > >  - v3: Fixed commit message typo spotted by Jaejoong Kim, added Roger's 
> > > ack
> > 
> > Although there's nothing wrong with making this change, people don't 
> > like to see character strings broken up just to avoid long lines.  Can 
> > you redo the patch without breaking the error message string?
> 
> Sure I could, but I broke that long line intentionaly. Line lenght rule is
> normally broken for string just to allow people grepping for error message in
> source files and I guess everyone sane would grep for "Can't get PHY device 
> for
> port" which is on single line. So this way we:
> 1) respect 80 chars limit
> 2) have error message grepable.
> 
> Do you still insist on making this one long line? (I have no problem sending
> v4, just wanted to give some rationale before I eventually do so)

I don't have hard, set-in-stone views on this.  And I have broken quite 
a few printk strings in my time -- but that was before people reached a 
consensus on whether it should be done.

Even with the rationale, it's quite likely that somebody will complain
if we merge a patch that splits the string.  And it's good practice to
make sure that checkpatch.pl doesn't complain, in any case.

So yes, please do resubmit the patch with one long line.  When you do,
you may add

Acked-by: Alan Stern 

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

[RFC V2 PATCH 3/3] usb: chipidea: usbmisc: small clean up

[Michael Trimarchi]

The register write can be done outside the if and else condition

Signed-off-by: Michael Trimarchi 
---
Changes v1->v2:
- None
---
 drivers/usb/chipidea/usbmisc_imx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c
index 1f47ef6..34ad5bf 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -315,13 +315,12 @@ static int usbmisc_imx6q_set_wakeup
val = readl(usbmisc->base + data->index * 4);
if (enabled) {
val |= wakeup_setting;
-   writel(val, usbmisc->base + data->index * 4);
} else {
if (val & MX6_BM_WAKEUP_INTR)
pr_debug("wakeup int at ci_hdrc.%d\n", data->index);
val &= ~wakeup_setting;
-   writel(val, usbmisc->base + data->index * 4);
}
+   writel(val, usbmisc->base + data->index * 4);
spin_unlock_irqrestore(>lock, flags);
 
return ret;
-- 
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

[RFC V2 PATCH 2/3] usb: chipidea: usbmisc: evdo can be set e/o reset

[Michael Trimarchi]

evdo bit can be set or reset. We can not trust evdo bit
status after bootloader stage

Signed-off-by: Michael Trimarchi 
---
Changes v1->v2:
- None
---
 drivers/usb/chipidea/usbmisc_imx.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c
index a52f5a8..1f47ef6 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -151,14 +151,18 @@ static int usbmisc_imx25_post(struct imx_usbmisc_data 
*data)
if (data->index)
return 0;
 
-   if (data->evdo) {
-   spin_lock_irqsave(>lock, flags);
-   reg = usbmisc->base + MX25_USB_PHY_CTRL_OFFSET;
-   val = readl(reg);
-   writel(val | MX25_BM_EXTERNAL_VBUS_DIVIDER, reg);
-   spin_unlock_irqrestore(>lock, flags);
-   usleep_range(5000, 1); /* needed to stabilize voltage */
-   }
+   spin_lock_irqsave(>lock, flags);
+   reg = usbmisc->base + MX25_USB_PHY_CTRL_OFFSET;
+   val = readl(reg);
+
+   if (data->evdo)
+   val |= MX25_BM_EXTERNAL_VBUS_DIVIDER;
+   else
+   val &= ~MX25_BM_EXTERNAL_VBUS_DIVIDER;
+
+   writel(val, reg);
+   spin_unlock_irqrestore(>lock, flags);
+   usleep_range(5000, 1); /* needed to stabilize voltage */
 
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

[RFC V2 PATCH 1/3] usb: chipidea: usbmisc: evdo is only specific to OTG port

[Michael Trimarchi]

The USB_PHY_CTRL_FUNC is used specific for OTG port as described
in user manual. EVDO need to be set only for index 0 that
correspond to OTG port

Signed-off-by: Michael Trimarchi 
---
Changes v1->v2
- %s/corrispond/correspond/
---
 drivers/usb/chipidea/usbmisc_imx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c
index 8cdf0af1..a52f5a8 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -148,6 +148,9 @@ static int usbmisc_imx25_post(struct imx_usbmisc_data *data)
if (data->index > 2)
return -EINVAL;
 
+   if (data->index)
+   return 0;
+
if (data->evdo) {
spin_lock_irqsave(>lock, flags);
reg = usbmisc->base + MX25_USB_PHY_CTRL_OFFSET;
-- 
2.7.4

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

RE: [PATCH v2] r8152: disable RX aggregation on Dell TB16 dock

[Mario.Limonciello]

> -Original Message-
> From: Kai Heng Feng [mailto:kai.heng.f...@canonical.com]
> Sent: Thursday, January 18, 2018 10:57 AM
> To: David Miller 
> Cc: Hayes Wang ; gre...@linuxfoundation.org; linux-
> u...@vger.kernel.org; net...@vger.kernel.org; linux-ker...@vger.kernel.org;
> Limonciello, Mario ; nic_s...@realtek.com
> Subject: Re: [PATCH v2] r8152: disable RX aggregation on Dell TB16 dock
> 
> 
> 
> > On 18 Jan 2018, at 10:50 PM, David Miller  wrote:
> >
> > From: Hayes Wang 
> > Date: Thu, 18 Jan 2018 03:04:08 +
> >
> >> [...]
>  r8153 on Dell TB15/16 dock corrupts rx packets.
> 
>  This change is suggested by Realtek. They guess that the XHCI
>  controller doesn't have enough buffer, and their guesswork is correct,
>  once the RX aggregation gets disabled, the issue is gone.
> 
>  ASMedia is currently working on a real sulotion for this issue.
> 
>  Dell and ODM confirm the bcdDevice and iSerialNumber is unique for TB16.
> 
>  Note that TB15 has different bcdDevice and iSerialNumber, which are
>  not unique values. If you still have TB15, please contact Dell to
>  replace it with TB16.
> >>
> >> Excuse me. I don't understand why this patch is for specific USB nic 
> >> rather than
> xHCI.
> >> It seems to make the specific USB nic working and the other ones keeping 
> >> error.
> >
> > Well, are we sure that the device being in the TB16 dock doesn't
> > contribute to the issue as well?

Previous version of this patch checked the parent device to ensure it was in 
TB16.
I believe there was negative feedback to that approach, which prompted the 
discussion
to check bcdDevice and iSerialNumber with all vendors involved.

If it's still desirable to analyze parentage tree, I suppose bcdDevice, 
iSerialNumber
and parent's USB device VID/PID can be analyzed  all at the same time.

> 
> This is what vendors concluded for now. The very same NIC on WD15 doesn’t
> have the issue.

And just so it's extra clear to everyone on this list - WD15 has different 
bcdDevice
iSerialNumber, and doesn't connect to ASMedia host controller.

Re: [PATCH v6 00/99] XArray version 6

[Matthew Wilcox]

On Thu, Jan 18, 2018 at 05:56:12PM +0100, David Sterba wrote:
> On Thu, Jan 18, 2018 at 08:48:43AM -0800, Matthew Wilcox wrote:
> > Thank you!  I shall attempt to debug.  Was this with a btrfs root
> > filesystem?  I'm most suspicious of those patches right now, since they've
> > received next to no testing.  I'm going to put together a smaller patchset
> > which just does the page cache conversion and nothing else in the hope
> > that we can get that merged this year.
> 
> No, the root is ext3 and there was no btrfs filesytem mounted at the
> time.

Found it; I was missing a prerequisite patch.  New (smaller) patch series
coming soon.
--
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] usb: ehci-omap: don't complain on -EPROBE_DEFER when no PHY found

[Ladislav Michl]

On Thu, Jan 18, 2018 at 10:18:41AM -0500, Alan Stern wrote:
> On Thu, 18 Jan 2018, Ladislav Michl wrote:
> 
> > Don't complain on -EPROBE_DEFER when no PHY found, the driver
> > probe will be retried later.
> > 
> > Signed-off-by: Ladislav Michl 
> > Acked-by: Tony Lindgren 
> > Acked-by: Roger Quadros 
> > ---
> >  Changes:
> >  - v2: None, just added Tony's ack
> >  - v3: Fixed commit message typo spotted by Jaejoong Kim, added Roger's ack
> 
> Although there's nothing wrong with making this change, people don't 
> like to see character strings broken up just to avoid long lines.  Can 
> you redo the patch without breaking the error message string?

Sure I could, but I broke that long line intentionaly. Line lenght rule is
normally broken for string just to allow people grepping for error message in
source files and I guess everyone sane would grep for "Can't get PHY device for
port" which is on single line. So this way we:
1) respect 80 chars limit
2) have error message grepable.

Do you still insist on making this one long line? (I have no problem sending
v4, just wanted to give some rationale before I eventually do so)

Best regards,
ladis

> >  drivers/usb/host/ehci-omap.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> > index 854b146a457d..fd6170962a1a 100644
> > --- a/drivers/usb/host/ehci-omap.c
> > +++ b/drivers/usb/host/ehci-omap.c
> > @@ -167,8 +167,9 @@ static int ehci_hcd_omap_probe(struct platform_device 
> > *pdev)
> > continue;
> >  
> > ret = PTR_ERR(phy);
> > -   dev_err(dev, "Can't get PHY device for port %d: %d\n",
> > -   i, ret);
> > +   if (ret != -EPROBE_DEFER)
> > +   dev_err(dev, "Can't get PHY device for port "
> > +   "%d: %d\n", i, ret);
> > goto err_phy;
> > }
--
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 v6 00/99] XArray version 6

[David Sterba]

On Thu, Jan 18, 2018 at 08:48:43AM -0800, Matthew Wilcox wrote:
> Thank you!  I shall attempt to debug.  Was this with a btrfs root
> filesystem?  I'm most suspicious of those patches right now, since they've
> received next to no testing.  I'm going to put together a smaller patchset
> which just does the page cache conversion and nothing else in the hope
> that we can get that merged this year.

No, the root is ext3 and there was no btrfs filesytem mounted at the
time.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] r8152: disable RX aggregation on Dell TB16 dock

[Kai Heng Feng]

> On 18 Jan 2018, at 10:50 PM, David Miller  wrote:
> 
> From: Hayes Wang 
> Date: Thu, 18 Jan 2018 03:04:08 +
> 
>> [...]
 r8153 on Dell TB15/16 dock corrupts rx packets.
 
 This change is suggested by Realtek. They guess that the XHCI
 controller doesn't have enough buffer, and their guesswork is correct,
 once the RX aggregation gets disabled, the issue is gone.
 
 ASMedia is currently working on a real sulotion for this issue.
 
 Dell and ODM confirm the bcdDevice and iSerialNumber is unique for TB16.
 
 Note that TB15 has different bcdDevice and iSerialNumber, which are
 not unique values. If you still have TB15, please contact Dell to
 replace it with TB16.
>> 
>> Excuse me. I don't understand why this patch is for specific USB nic rather 
>> than xHCI.
>> It seems to make the specific USB nic working and the other ones keeping 
>> error.
> 
> Well, are we sure that the device being in the TB16 dock doesn't
> contribute to the issue as well?

This is what vendors concluded for now. The very same NIC on WD15 doesn’t
have the issue.

> 
> If the problem only shows up with XHCI and this dock, then this patch
> was the appropriate way to deal with the problem for now.

--
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: core: power on PHYs before initializing core

[Enric Balletbo Serra]

2018-01-17 22:46 GMT+01:00 Brian Norris :
> On Fri, Jan 12, 2018 at 12:00:16PM +0800, William Wu wrote:
>> The dwc3_core_init() gets the PHYs and initializes the PHYs with
>> the usb_phy_init() and phy_init() functions before initializing
>> core, and power on the PHYs after core initialization is done.
>>
>> However, some platforms (e.g. Rockchip RK3399 DWC3 with Type-C
>> USB3 PHY), it needs to do some special operation while power on
>> the Type-C PHY before initializing DWC3 core. It's because that
>> the RK3399 Type-C PHY requires to hold the DWC3 controller in
>> reset state to keep the PIPE power state in P2 while configuring
>> the Type-C PHY, otherwise, it may cause waiting for the PIPE ready
>> timeout. In this case, if we power on the PHYs after the DWC3 core
>> initialization is done, the core will be reset to uninitialized
>> state after power on the PHYs.
>>
>> Fix this by powering on the PHYs before initializing core. And
>> because the GUID register may also be reset in this case, so we
>> need to configure the GUID register after powering on the PHYs.
>>
>> Signed-off-by: William Wu 
>
> This kinda should be part of your series:
>
> [PATCH 0/3] Reset USB3 controller before initializing Type-C PHY on rk3399
>

+1

> or at least mentioned there, because the series there doesn't quite
> right otherwise, no?
>
> Anyway, I think this patch looks OK. I don't immediately see good
> reasons for delaying the PHY init until later, and I do see reasons why
> it could be useful earlier:
>
> Reviewed-by: Brian Norris 

Tested on Samsung Chromebook Plus with current mainline

Tested-by: Enric Balletbo i Serra 

>
>> ---
>>  drivers/usb/dwc3/core.c | 46 ++
>>  1 file changed, 22 insertions(+), 24 deletions(-)
--
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 v6 00/99] XArray version 6

[Matthew Wilcox]

On Thu, Jan 18, 2018 at 05:07:50PM +0100, David Sterba wrote:
> On Wed, Jan 17, 2018 at 12:20:24PM -0800, Matthew Wilcox wrote:
> > From: Matthew Wilcox 
> > 
> > This version of the XArray has no known bugs.
> 
> I've booted this patchset on 2 boxes, both had random problems during
> boot. On one I was not able to diagnose what went wrong. On the other
> one the system booted up to userspace and failed to set up networking.
> Serial console worked and the network service complained about wrong
> format of /usr/share/wicked/schema/team.xml . That's supposed to be a
> text file, though hexdump showed me lots of zeros. Trimmed output:
> 
>   00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||
> *
> (similar output here)
> *
> 0a10  00 00 00 00 00 00 00 00  11 03 00 00 00 00 00 00  ||
> 0a20  20 8b 7f 01 00 00 00 00  a0 84 7d 01 00 00 00 00  | .}.|
> 0a30  00 00 00 00 00 00 00 00  10 89 7f 01 00 00 00 00  ||
> 0a40  a0 84 7d 01 00 00 00 00  00 00 00 00 00 00 00 00  |..}.|
> 0a50  80 8a 7f 01 00 00 00 00  e0 cf 7d 01 00 00 00 00  |..}.|
> 0a60  00 00 00 00 00 00 00 00  60 8a 7f 01 00 00 00 00  |`...|
> 0a70  a0 84 7d 01 00 00 00 00  00 00 00 00 00 00 00 00  |..}.|
> 0a80  30 89 7f 01 00 00 00 00  a0 84 7d 01 00 00 00 00  |0.}.|
> 0a90  00 00 00 00 00 00 00 00  60 f2 7f 01 00 00 00 00  |`...|
> 0aa0  40 fd 7e 01 00 00 00 00  00 00 00 00 00 00 00 00  |@.~.|
> 0ab0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||
> *
> 1000  3e 0a 20 20 3c 2f 6d 65  74 68 6f 64 3e 0a 3c 2f  |>.  . 1010  73 65 72 76 69 63 65 3e  0a   |service>.|
> 
> There's something at the end of the file that does look like a xml fragment.
> The file size is 4121. This looks to me like exactly the first page of the 
> file
> was not read correctly.
> 
> The xml file is supposed to be read-only during startup, so there was no write
> in flight. 'rpm -Vv' reported only this file corrupted. Booting to other
> kernels was fine, network up, and the file was ok again. So the
> corruption happened only in memory, which leads me to conclusion that
> there is an unknown bug in your patchset.

Thank you!  I shall attempt to debug.  Was this with a btrfs root
filesystem?  I'm most suspicious of those patches right now, since they've
received next to no testing.  I'm going to put together a smaller patchset
which just does the page cache conversion and nothing else in the hope
that we can get that merged this year.
--
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 v6 00/99] XArray version 6

[David Sterba]

On Wed, Jan 17, 2018 at 12:20:24PM -0800, Matthew Wilcox wrote:
> From: Matthew Wilcox 
> 
> This version of the XArray has no known bugs.

I've booted this patchset on 2 boxes, both had random problems during
boot. On one I was not able to diagnose what went wrong. On the other
one the system booted up to userspace and failed to set up networking.
Serial console worked and the network service complained about wrong
format of /usr/share/wicked/schema/team.xml . That's supposed to be a
text file, though hexdump showed me lots of zeros. Trimmed output:

  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||
*
(similar output here)
*
0a10  00 00 00 00 00 00 00 00  11 03 00 00 00 00 00 00  ||
0a20  20 8b 7f 01 00 00 00 00  a0 84 7d 01 00 00 00 00  | .}.|
0a30  00 00 00 00 00 00 00 00  10 89 7f 01 00 00 00 00  ||
0a40  a0 84 7d 01 00 00 00 00  00 00 00 00 00 00 00 00  |..}.|
0a50  80 8a 7f 01 00 00 00 00  e0 cf 7d 01 00 00 00 00  |..}.|
0a60  00 00 00 00 00 00 00 00  60 8a 7f 01 00 00 00 00  |`...|
0a70  a0 84 7d 01 00 00 00 00  00 00 00 00 00 00 00 00  |..}.|
0a80  30 89 7f 01 00 00 00 00  a0 84 7d 01 00 00 00 00  |0.}.|
0a90  00 00 00 00 00 00 00 00  60 f2 7f 01 00 00 00 00  |`...|
0aa0  40 fd 7e 01 00 00 00 00  00 00 00 00 00 00 00 00  |@.~.|
0ab0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||
*
1000  3e 0a 20 20 3c 2f 6d 65  74 68 6f 64 3e 0a 3c 2f  |>.  ..|

There's something at the end of the file that does look like a xml fragment.
The file size is 4121. This looks to me like exactly the first page of the file
was not read correctly.

The xml file is supposed to be read-only during startup, so there was no write
in flight. 'rpm -Vv' reported only this file corrupted. Booting to other
kernels was fine, network up, and the file was ok again. So the
corruption happened only in memory, which leads me to conclusion that
there is an unknown bug in your patchset.
--
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: Not enough bandwidth with Magewell XI100DUSB-HDMI

[Mathias Nyman]

On 17.01.2018 18:46, Michael Tretter wrote:

Hello,

I am using a Magewell XI100DUSB-HDMI frame grabber which by itself works
fine. However, I get a "Not enough bandwidth for new device state."
error for any other USB device that is plugged after the frame grabber.
This error is caused by the xHC sending a Bandwidth Error for
the Configure Endpoint command during xhci_check_bandwidth(). I
tested it with a USB-storage device (high speed and SuperSpeed) and a
USB 3.0 hub, but there is no difference. If I plug the frame grabber
after the other device, both work fine.

The error only occurs, if Pulseaudio opened the device, which selects
an alternate interface for the AudioStreaming interface. This alternate
interface uses an isochronous endpoint with an interval of 1 ms and 192
BytesPerInterval. This should not reserve all bandwidth of the root
hub. I implemented the Get Port Bandwidth command to check the
available bandwidth of the xHC and it reports 80 % for HS ports, 90 %
for SS ports, and 0 % on the port used by the frame grabber. LGTM.

I am currently testing on 4.15-rc8, but I can reproduce the same issue
with 4.12, too.

This is the output of /sys/kernel/debug/usb/devices for the frame
grabber with the isochronous endpoint selected:

T:  Bus=02 Lev=01 Prnt=01 Port=03 Cnt=01 Dev#=  2 Spd=5000
MxCh= 0 D:  Ver= 3.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS= 9 #Cfgs=  1
P:  Vendor=2935 ProdID=0001 Rev= 0.01
S:  Manufacturer=Magewell
S:  Product=XI100DUSB-HDMI
S:  SerialNumber=A201170218035
C:* #Ifs= 5 Cfg#= 1 Atr=80 MxPwr=800mA
A:  FirstIf#= 0 IfCount= 2 Cls=0e(video) Sub=03 Prot=00
A:  FirstIf#= 2 IfCount= 2 Cls=01(audio) Sub=01 Prot=00
I:* If#= 0 Alt= 0 #EPs= 1 Cls=0e(video) Sub=01 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=03(Int.) MxPS=  16 Ivl=125us
I:* If#= 1 Alt= 0 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=83(I) Atr=02(Bulk) MxPS=1024 Ivl=0ms
I:* If#= 2 Alt= 0 #EPs= 0 Cls=01(audio) Sub=01 Prot=00 
Driver=snd-usb-audio
I:  If#= 3 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 
Driver=snd-usb-audio
I:* If#= 3 Alt= 1 #EPs= 1 Cls=01(audio) Sub=02 Prot=00 
Driver=snd-usb-audio
E:  Ad=85(I) Atr=05(Isoc) MxPS= 192 Ivl=1ms
I:* If#= 4 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
E:  Ad=86(I) Atr=03(Int.) MxPS=  64 Ivl=64ms

I don't understand, why the xHC sends the Bandwidth Error. I already
looked at the information about the xhci in the debugfs and the event
tracing, but I did not find anything relevant. Is there anything else
that I can look for and test

Odd, there shouldn't be any bandwidth error in this case.

There was one other case where bogus bandwidth errors for isoc transfers were
reported. The reporter saw some correlation between the Bandwidth issues and
LPM/MEL (Link power management/Max exit latency) settings.

It could be worth disabling LPM, or just prevent changing the MEL to check if 
you see the same correlation.

I might be able to get one of those devices.
Do you have more detail (a testscript) on how to trigger the issue,
and details about the xHCI host controller you are using.

If you also could you send me the xhci debug and trace outputs just in case.

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

[BUG] usbfs "did not claim interface before use"

[Matthew Wilcox]

My laptop just died horribly and I'm picking through the pieces.
I'm running Debian testing, and it was kernel 4.13.0-1-amd64 #1 Debian
4.13.13-1 although now I've rebooted to 4.14.0-2-amd64 #1 SMP Debian
4.14.7-1

It seems that the original cause of the problem was usbfs going mad and
spraying this message all over my log files:

Jan 18 09:50:24 bobo kernel: [2212381.201087] usb 3-2: usbfs: process 13608 
(events) did not claim interface 0 before use
Jan 18 09:50:24 bobo kernel: [2212381.201091] usb 3-2: usbfs: process 13608 
(events) did not claim interface 0 before use
Jan 18 09:50:24 bobo kernel: [2212381.201096] usb 3-2: usbfs: process 13608 
(events) did not claim interface 0 before use
Jan 18 09:50:24 bobo kernel: [2212381.201101] usb 3-2: usbfs: process 13608 
(events) did not claim interface 0 before use

(I have 28153585 lines in kern.log and 28149224 of them contain 'usbfs').
Can this warning be ratelimited, please?

Also, it is not coding style compliant as you cannot grep for "did not
claim interface" due to the line split.

(after 280 million lines of this crap, dmcrypt failed an order-2 memory
allocation, and things went seriously downhill after that)
--
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] usb: ehci-omap: don't complain on -EPROBE_DEFER when no PHY found

[Alan Stern]

On Thu, 18 Jan 2018, Ladislav Michl wrote:

> Don't complain on -EPROBE_DEFER when no PHY found, the driver
> probe will be retried later.
> 
> Signed-off-by: Ladislav Michl 
> Acked-by: Tony Lindgren 
> Acked-by: Roger Quadros 
> ---
>  Changes:
>  - v2: None, just added Tony's ack
>  - v3: Fixed commit message typo spotted by Jaejoong Kim, added Roger's ack

Although there's nothing wrong with making this change, people don't 
like to see character strings broken up just to avoid long lines.  Can 
you redo the patch without breaking the error message string?

Alan Stern

>  drivers/usb/host/ehci-omap.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index 854b146a457d..fd6170962a1a 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -167,8 +167,9 @@ static int ehci_hcd_omap_probe(struct platform_device 
> *pdev)
>   continue;
>  
>   ret = PTR_ERR(phy);
> - dev_err(dev, "Can't get PHY device for port %d: %d\n",
> - i, ret);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "Can't get PHY device for port "
> + "%d: %d\n", i, ret);
>   goto err_phy;
>   }

--
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 v1 1/1] usb: xhci: do not create and register shared_hcd when USB3.0 is disabled

[Mathias Nyman]

On 18.01.2018 09:27, Tung Vuong Nguyen wrote:

On Tue, Jan 16, 2018 at 9:50 PM, Mathias Nyman
 wrote:


Hi, Sorry about the delay


On 04.01.2018 07:17, Thang Q. Nguyen wrote:


Hi,

On Sat, Dec 16, 2017 at 10:45 AM, Thang Q. Nguyen  wrote:


From: Tung Nguyen 

Currently, hcd->shared_hcd always creates and registers to the usb-core.
If, for some reasons, USB3 downstream port is disabled, no roothub port for
USB3.0 is found. This causes kernel to display an error:
hub 2-0:1.0: config failed, hub doesn't have any ports! (err -19)
This patch checks, creates and registers shared_hcd if USB3.0 downstream
port is available.

Signed-off-by: Tung Nguyen 
Signed-off-by: Thang Q. Nguyen 
---
   drivers/usb/host/xhci-mem.c  |  2 +-
   drivers/usb/host/xhci-plat.c | 26 +++--
   drivers/usb/host/xhci.c  | 54 

   3 files changed, 54 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 554a8a5..157d1e7 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1067,7 +1067,7 @@ static u32 xhci_find_real_port_number(struct xhci_hcd 
*xhci,
  struct usb_device *top_dev;
  struct usb_hcd *hcd;

-   if (udev->speed >= USB_SPEED_SUPER)
+   if (udev->speed >= USB_SPEED_SUPER && xhci->shared_hcd)
  hcd = xhci->shared_hcd;
  else
  hcd = xhci->main_hcd;
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 6f03830..e812e3d 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -253,12 +253,6 @@ static int xhci_plat_probe(struct platform_device *pdev)

  xhci->clk = clk;
  xhci->main_hcd = hcd;
-   xhci->shared_hcd = __usb_create_hcd(driver, sysdev, >dev,
-   dev_name(>dev), hcd);
-   if (!xhci->shared_hcd) {
-   ret = -ENOMEM;
-   goto disable_clk;
-   }

  if (device_property_read_bool(sysdev, "usb2-lpm-disable"))
  xhci->quirks |= XHCI_HW_LPM_DISABLE;
@@ -290,12 +284,20 @@ static int xhci_plat_probe(struct platform_device *pdev)
  if (ret)
  goto disable_usb_phy;

-   if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
-   xhci->shared_hcd->can_do_streams = 1;
-
-   ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
-   if (ret)
-   goto dealloc_usb2_hcd;
+   if (xhci->num_usb3_ports > 0) {
+   xhci->shared_hcd = __usb_create_hcd(driver, sysdev, >dev,
+   dev_name(>dev), hcd);
+   if (!xhci->shared_hcd) {
+   ret = -ENOMEM;
+   goto disable_clk;
+   }
+   if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
+   xhci->shared_hcd->can_do_streams = 1;
+
+   ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
+   if (ret)
+   goto dealloc_usb2_hcd;
+   }

  device_enable_async_suspend(>dev);
  pm_runtime_put_noidle(>dev);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 05104bd..4824bf6 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -417,12 +417,14 @@ static void compliance_mode_recovery(struct timer_list *t)
  i + 1);
  xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
  "Attempting compliance mode 
recovery");
-   hcd = xhci->shared_hcd;
+   if (xhci->shared_hcd) {
+   hcd = xhci->shared_hcd;

-   if (hcd->state == HC_STATE_SUSPENDED)
-   usb_hcd_resume_root_hub(hcd);
+   if (hcd->state == HC_STATE_SUSPENDED)
+   usb_hcd_resume_root_hub(hcd);

-   usb_hcd_poll_rh_status(hcd);
+   usb_hcd_poll_rh_status(hcd);
+   }
  }
  }

@@ -611,6 +613,18 @@ int xhci_run(struct usb_hcd *hcd)
  if (ret)
  xhci_free_command(xhci, command);
  }
+   /*
+* Execute xhci_start() in case xhci->shared_hcd is not registered.
+* If the xhci->shared_hcd doesn't exist, no one triggers to start
+* the xhci which should be done before exitting run function
+*/
+   if (!xhci->shared_hcd) {
+   if (xhci_start(xhci)) {



This probably won't work as primary hcd was added before shared_hcd was created.
usb_add_hcd(hcd) calls xhci_run() before xhci->shared_hcd exists, so this will
cause the xHC to start before the shared_hcd is created or setup.

-Mathias


Hi Mathias
I 

Re: [PATCH v2] r8152: disable RX aggregation on Dell TB16 dock

[David Miller]

From: Hayes Wang 
Date: Thu, 18 Jan 2018 03:04:08 +

> [...]
>> > r8153 on Dell TB15/16 dock corrupts rx packets.
>> >
>> > This change is suggested by Realtek. They guess that the XHCI
>> > controller doesn't have enough buffer, and their guesswork is correct,
>> > once the RX aggregation gets disabled, the issue is gone.
>> >
>> > ASMedia is currently working on a real sulotion for this issue.
>> >
>> > Dell and ODM confirm the bcdDevice and iSerialNumber is unique for TB16.
>> >
>> > Note that TB15 has different bcdDevice and iSerialNumber, which are
>> > not unique values. If you still have TB15, please contact Dell to
>> > replace it with TB16.
> 
> Excuse me. I don't understand why this patch is for specific USB nic rather 
> than xHCI.
> It seems to make the specific USB nic working and the other ones keeping 
> error.

Well, are we sure that the device being in the TB16 dock doesn't
contribute to the issue as well?

If the problem only shows up with XHCI and this dock, then this patch
was the appropriate way to deal with the problem for now.
--
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 v6 85/99] btrfs: Remove unused spinlock

[David Sterba]

On Wed, Jan 17, 2018 at 12:21:49PM -0800, Matthew Wilcox wrote:
> From: Matthew Wilcox 
> 
> The reada_lock in struct btrfs_device was only initialised, and not
> actually used.  That's good because there's another lock also called
> reada_lock in the btrfs_fs_info that was quite heavily used.  Remove
> this one.
> 
> Signed-off-by: Matthew Wilcox 

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

RE: [PATCH] USB TYPEC: RT1711H Type-C Chip Driver

[李書帆]

Dear Gerg,

  Many thanks to your comment.
  I've checked all of them and here are some questions need your help.

> +Example:
> +rt1711h@4e {
> +status = "ok";
> +compatible = "richtek,typec_rt1711h";
> +reg = <0x4e>;
> +rt,intr_gpio = < 0 0x0>;
> +rt,name = "rt1711h";
> +rt,port_type = <2>; /* 0: DFP, 1: UFP, 2: DRP */
> +rt,def_role = <0>; /* 0: SNK, 1: SRC, -1: TYPEC_NO_PREFERRED_ROLE */
> +};

dts stuff needs to always be in a separate file so the DT maintainers can 
review/ack it.  Split this patch up into smaller pieces please.

Ok, I'll split it into two patches, one for source code and once for dts 
related files.


> diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> index bb3138a..e3aaf3c 100644
> --- a/drivers/usb/typec/Makefile
> +++ b/drivers/usb/typec/Makefile
> @@ -2,6 +2,7 @@
>  obj-$(CONFIG_TYPEC)+= typec.o
>  obj-$(CONFIG_TYPEC_TCPM)+= tcpm.o
>  obj-y+= fusb302/
> +obj-$(CONFIG_TYPEC_RT1711H)+= rt1711h/

Why do you need a whole directory for one file?

Is the suggestion to move rt1711h.c to the same directory level as tcpm? i.e. 
drivers/usb/typec/rt1711h.c


> +

This last #include should not be needed.  If it does, you are doing something 
really wrong...

Ok, this #include will be removed


> +
> +#include "rt1711h.h"

Why a .h file for a single .c file?

Is the suggestion to move all content in rt1711h.h into rt1711h.c?


> +
> +#define RT1711H_DRV_VERSION"1.0.3"

When code is in the kernel tree, versions mean nothing, you will note that no 
other USB driver has them, right?  Please remove.

Ok, this will be removed.


kernel types are u16, u32, u8, and the like, not uint16_t, those are for 
userspace code only.
Yeah, other drivers do it, but you shouldn't :)

Ok, I'll use u16, u32 and u8 instead of uint16_t, uint32_t and uint8_t


> +/* IRQ */
> +struct kthread_worker irq_worker;
> +struct kthread_work irq_work;
> +struct task_struct *irq_worker_task;

3 things for an irq handler?  That feels wrong.

> +atomic_t poll_count;

Like I said before, why is this an atomic?

I'll use threaded_irq instead, the above things will be removed.


> +/* I2C */
> +atomic_t i2c_busy;
> +atomic_t pm_suspend;

Why are these atomic?  You know that doesn't mean they do not need locking, 
right?

For my understanding, a single operation on atomic_t doesn't need lock, like a 
single atomic_set.
But two consecutive operations doesn't guarantee anything. Like atomic_set 
followed by an atomic_read.
This part is referenced from fusb302 used to make sure I2C is idle before 
system suspends.
It only needs to guarantee a single read/write on these variable is atomic 
operation, so atomic is used.


> +};
> +
> +/*
> + * Logging & debugging
> + */
> +
> +#ifdef CONFIG_DEBUG_FS
> +
> +static int rt1711h_reg_block_read(struct rt1711h_chip *chip, uint8_t reg,
> +int len, uint8_t *data);
> +static int rt1711h_reg_block_write(struct rt1711h_chip *chip, uint8_t reg,
> +int len, const uint8_t *data);
> +
> +struct reg_desc {
> +uint8_t addr;
> +uint8_t size;
> +};
> +#define DECL_REG(_addr, _size) {.addr = _addr, .size = _size}
> +
> +static struct reg_desc rt1711h_reg_desc[] = {
> +DECL_REG(RT1711H_REG_VID, 2),
> +DECL_REG(RT1711H_REG_PID, 2),
> +DECL_REG(RT1711H_REG_DID, 2),
> +DECL_REG(RT1711H_REG_TYPEC_REV, 2),
> +DECL_REG(RT1711H_REG_PD_REV, 2),
> +DECL_REG(RT1711H_REG_PDIF_REV, 2),
> +DECL_REG(RT1711H_REG_ALERT, 2),
> +DECL_REG(RT1711H_REG_ALERT_MASK, 2),
> +DECL_REG(RT1711H_REG_POWER_STATUS_MASK, 1),
> +DECL_REG(RT1711H_REG_FAULT_STATUS_MASK, 1),
> +DECL_REG(RT1711H_REG_TCPC_CTRL, 1),
> +DECL_REG(RT1711H_REG_ROLE_CTRL, 1),
> +DECL_REG(RT1711H_REG_FAULT_CTRL, 1),
> +DECL_REG(RT1711H_REG_POWER_CTRL, 1),
> +DECL_REG(RT1711H_REG_CC_STATUS, 1),
> +DECL_REG(RT1711H_REG_POWER_STATUS, 1),
> +DECL_REG(RT1711H_REG_FAULT_STATUS, 1),
> +DECL_REG(RT1711H_REG_COMMAND, 1),
> +DECL_REG(RT1711H_REG_MSG_HDR_INFO, 1),
> +DECL_REG(RT1711H_REG_RX_DETECT, 1),
> +DECL_REG(RT1711H_REG_RX_BYTE_CNT, 1),
> +DECL_REG(RT1711H_REG_RX_BUF_FRAME_TYPE, 1),
> +DECL_REG(RT1711H_REG_RX_HDR, 2),
> +DECL_REG(RT1711H_REG_RX_DATA, 1),
> +DECL_REG(RT1711H_REG_TRANSMIT, 1),
> +DECL_REG(RT1711H_REG_TX_BYTE_CNT, 1),
> +DECL_REG(RT1711H_REG_TX_HDR, 2),
> +DECL_REG(RT1711H_REG_TX_DATA, 1),
> +DECL_REG(RT1711H_REG_CLK_CTRL2, 1),
> +DECL_REG(RT1711H_REG_CLK_CTRL3, 1),
> +DECL_REG(RT1711H_REG_BMC_CTRL, 1),
> +DECL_REG(RT1711H_REG_BMCIO_RXDZSEL, 1),
> 

Re: [PATCH v2] usb: dwc3: core: Power-off core/PHYs on system_suspend in host mode

[Roger Quadros]

On 18/01/18 13:24, Manu Gautam wrote:
> Commit 689bf72c6e0d ("usb: dwc3: Don't reinitialize core during
> host bus-suspend/resume") updated suspend/resume routines to not
> power_off and reinit PHYs/core for host mode.
> It broke platforms that rely on DWC3 core to power_off PHYs to
> enter low power state on system suspend.
> 
> Perform dwc3_core_exit/init only during host mode system_suspend/
> resume to addresses power regression from above mentioned patch
> and also allow USB session to stay connected across
> runtime_suspend/resume in host mode. While at it also replace
> existing checks for HOST only dr_mode with current_dr_role to
> have similar core driver behavior for both Host-only and DRD+Host
> configurations.
> 
> Fixes: 689bf72c6e0d ("usb: dwc3: Don't reinitialize core during host 
> bus-suspend/resume")
> Signed-off-by: Manu Gautam 

Tested to work on dra7x platform.

Reviewed-by: Roger Quadros 

-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] usb: dwc3: core: Power-off core/PHYs on system_suspend in host mode

[Manu Gautam]

Commit 689bf72c6e0d ("usb: dwc3: Don't reinitialize core during
host bus-suspend/resume") updated suspend/resume routines to not
power_off and reinit PHYs/core for host mode.
It broke platforms that rely on DWC3 core to power_off PHYs to
enter low power state on system suspend.

Perform dwc3_core_exit/init only during host mode system_suspend/
resume to addresses power regression from above mentioned patch
and also allow USB session to stay connected across
runtime_suspend/resume in host mode. While at it also replace
existing checks for HOST only dr_mode with current_dr_role to
have similar core driver behavior for both Host-only and DRD+Host
configurations.

Fixes: 689bf72c6e0d ("usb: dwc3: Don't reinitialize core during host 
bus-suspend/resume")
Signed-off-by: Manu Gautam 
---

Changes since v1:
  - Incorporated Roger's review comments to replace dr_mode checks
with current_dr_role.
  - Cleanup current_dr_role assigment by moving it to dwc3_set_prtcap.

 drivers/usb/dwc3/core.c | 36 ++--
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index dabfa16..32d9ae1 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -111,6 +111,8 @@ static void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
reg &= ~(DWC3_GCTL_PRTCAPDIR(DWC3_GCTL_PRTCAP_OTG));
reg |= DWC3_GCTL_PRTCAPDIR(mode);
dwc3_writel(dwc->regs, DWC3_GCTL, reg);
+
+   dwc->current_dr_role = mode;
 }
 
 static void __dwc3_set_mode(struct work_struct *work)
@@ -144,8 +146,6 @@ static void __dwc3_set_mode(struct work_struct *work)
 
dwc3_set_prtcap(dwc, dwc->desired_dr_role);
 
-   dwc->current_dr_role = dwc->desired_dr_role;
-
spin_unlock_irqrestore(>lock, flags);
 
switch (dwc->desired_dr_role) {
@@ -229,7 +229,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
 * XHCI driver will reset the host block. If dwc3 was configured for
 * host-only mode, then we can return early.
 */
-   if (dwc->dr_mode == USB_DR_MODE_HOST)
+   if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST)
return 0;
 
reg = dwc3_readl(dwc->regs, DWC3_DCTL);
@@ -926,7 +926,6 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 
switch (dwc->dr_mode) {
case USB_DR_MODE_PERIPHERAL:
-   dwc->current_dr_role = DWC3_GCTL_PRTCAP_DEVICE;
dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
 
if (dwc->usb2_phy)
@@ -942,7 +941,6 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
}
break;
case USB_DR_MODE_HOST:
-   dwc->current_dr_role = DWC3_GCTL_PRTCAP_HOST;
dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
 
if (dwc->usb2_phy)
@@ -1290,7 +1288,7 @@ static int dwc3_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_PM
-static int dwc3_suspend_common(struct dwc3 *dwc)
+static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
 {
unsigned long   flags;
 
@@ -1302,6 +1300,10 @@ static int dwc3_suspend_common(struct dwc3 *dwc)
dwc3_core_exit(dwc);
break;
case DWC3_GCTL_PRTCAP_HOST:
+   /* do nothing during host runtime_suspend */
+   if (!PMSG_IS_AUTO(msg))
+   dwc3_core_exit(dwc);
+   break;
default:
/* do nothing */
break;
@@ -1310,7 +1312,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc)
return 0;
 }
 
-static int dwc3_resume_common(struct dwc3 *dwc)
+static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
 {
unsigned long   flags;
int ret;
@@ -1326,6 +1328,13 @@ static int dwc3_resume_common(struct dwc3 *dwc)
spin_unlock_irqrestore(>lock, flags);
break;
case DWC3_GCTL_PRTCAP_HOST:
+   /* nothing to do on host runtime_resume */
+   if (!PMSG_IS_AUTO(msg)) {
+   ret = dwc3_core_init(dwc);
+   if (ret)
+   return ret;
+   }
+   break;
default:
/* do nothing */
break;
@@ -1337,12 +1346,11 @@ static int dwc3_resume_common(struct dwc3 *dwc)
 static int dwc3_runtime_checks(struct dwc3 *dwc)
 {
switch (dwc->current_dr_role) {
-   case USB_DR_MODE_PERIPHERAL:
-   case USB_DR_MODE_OTG:
+   case DWC3_GCTL_PRTCAP_DEVICE:
if (dwc->connected)
return -EBUSY;
break;
-   case USB_DR_MODE_HOST:
+   case DWC3_GCTL_PRTCAP_HOST:
default:
/* do nothing */
break;
@@ -1359,7 +1367,7 @@ static int dwc3_runtime_suspend(struct device *dev)
if (dwc3_runtime_checks(dwc))
return -EBUSY;
 
-   ret = 

Re: [PATCH] usb: dwc3: core: Power-off core/PHYs on system_suspend in host mode

[Manu Gautam]

Hi Roger,


On 1/17/2018 4:16 PM, Roger Quadros wrote:
> Manu,
>
> On 16/01/18 08:26, Manu Gautam wrote:
>
[snip]
> After these changes applied, I could get suspend/resume working properly on 
> dra7x-evm
> for host, device and dual-role cases.
>  

Thanks Roger. I will incorporate these changes as well.

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
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] CDC-ACM: apply quirk for card reader

[Oliver Neukum]

This devices drops random bytes from messages if you talk to it
too fast.

Signed-off-by: 
CC: sta...@vger.kernel.org
---
 drivers/usb/class/cdc-acm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 8e0636c963a7..349aac4472bd 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1752,6 +1752,9 @@ static const struct usb_device_id acm_ids[] = {
{ USB_DEVICE(0x0ace, 0x1611), /* ZyDAS 56K USB MODEM - new version */
.driver_info = SINGLE_RX_URB, /* firmware bug */
},
+   { USB_DEVICE(0x11ca, 0x0201), /* VeriFone Mx870 Gadget Serial */
+   .driver_info = SINGLE_RX_URB,
+   },
{ USB_DEVICE(0x22b8, 0x7000), /* Motorola Q Phone */
.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
},
-- 
2.13.6

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

Questions about "usb usb8-port1: cannot disable (err = -32)"

[Yoshihiro Shimoda]

Hi,

My environment (R-Car H3) outputs the following error when I disconnected
a usb 3.0 SSD device from the xhci host controller.

[  267.755777] xhci-hcd ee00.usb: Cannot set link state.
[  267.761188] usb usb8-port1: cannot disable (err = -32)
[  267.772166] usb 8-1: USB disconnect, device number 2

But, the environment can detect connection again.

I investigate this issue and found the following commit is related:
  37be6676 usb: hub: Fix auto-remount of safely removed or ejected USB-3 devices

The xhci-hub.c has the following code:
if ((temp & PORT_PE) == 0 ||
(link_state > USB_SS_PORT_LS_U3)) {
xhci_warn(xhci, "Cannot set link state.\n");
goto error;
}

And, the code above will be called by hub_set_port_link_state(..., 
USB_SS_PORT_LS_U3)
in hub_port_disable():
/*
 * USB-3 does not have a similar link state as USB-2 that will avoid negotiating
 * a connection with a plugged-in cable but will signal the host when the cable
 * is unplugged. Disable remote wake and set link state to U3 for USB-3 devices
 */
static int hub_port_disable(struct usb_hub *hub, int port1, int set_state)
{
struct usb_port *port_dev = hub->ports[port1 - 1];
struct usb_device *hdev = hub->hdev;
int ret = 0;

if (!hub->error) {
if (hub_is_superspeed(hub->hdev)) {
hub_usb3_port_prepare_disable(hub, port_dev);
ret = hub_set_port_link_state(hub, port_dev->portnum,
  USB_SS_PORT_LS_U3);

The hub_port_disable() will be called by port_event():
   /*
 * Warm reset a USB3 protocol port if it's in
 * SS.Inactive state.
 */
if (hub_port_warm_reset_required(hub, port1, portstatus)) {
dev_dbg(_dev->dev, "do warm reset\n");
if (!udev || !(portstatus & USB_PORT_STAT_CONNECTION)
|| udev->state == USB_STATE_NOTATTACHED) {
if (hub_port_reset(hub, port1, NULL,
HUB_BH_RESET_TIME, true) < 0)
hub_port_disable(hub, port1, 1);

However, according to Figure 35 in xhci spec, the PED will be set to 0
after the usb3 root hub enters "Error" or "Disconnected" state.
So, IIUC, a usb driver should not call hub_set_port_link_state() in such a case.

Question 1:
 - Is my understanding correct?

Question 2:
 - How do I resolve the issue? (I don't have any idea for now...)

Best regards,
Yoshihiro Shimoda

--
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 v3] usb: ehci-omap: don't complain on -EPROBE_DEFER when no PHY found

[Ladislav Michl]

Don't complain on -EPROBE_DEFER when no PHY found, the driver
probe will be retried later.

Signed-off-by: Ladislav Michl 
Acked-by: Tony Lindgren 
Acked-by: Roger Quadros 
---
 Changes:
 - v2: None, just added Tony's ack
 - v3: Fixed commit message typo spotted by Jaejoong Kim, added Roger's ack

 drivers/usb/host/ehci-omap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 854b146a457d..fd6170962a1a 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -167,8 +167,9 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
continue;
 
ret = PTR_ERR(phy);
-   dev_err(dev, "Can't get PHY device for port %d: %d\n",
-   i, ret);
+   if (ret != -EPROBE_DEFER)
+   dev_err(dev, "Can't get PHY device for port "
+   "%d: %d\n", i, ret);
goto err_phy;
}
 
-- 
2.15.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