RE: [PATCH v3] usb: hub: try old enumeration scheme first for high speed devices
Hi greg: Sorry for the late response, I have applied the tags and resend, please receive it and check Thx Zengtao >-Original Message- >From: Greg KH [mailto:gre...@linuxfoundation.org] >Sent: Thursday, September 20, 2018 6:38 PM >To: Roger Quadros >Cc: Zengtao (B) ; st...@rowland.harvard.edu; >mathias.ny...@linux.intel.com; drink...@chromium.org; >felipe.ba...@linux.intel.com; dr...@endlessm.com; >ravisadin...@chromium.org; j...@perches.com; Jonathan Corbet >; Thomas Gleixner ; Ingo Molnar >; Rafael J. Wysocki ; >Marc Zyngier ; Kai-Heng Feng >; Thymo van Beers >; Frederic Weisbecker >; Konrad Rzeszutek Wilk ; >David Rientjes ; Mike Looijmans >; linux-...@vger.kernel.org; >linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org >Subject: Re: [PATCH v3] usb: hub: try old enumeration scheme first for >high speed devices > >On Mon, Aug 20, 2018 at 09:55:49AM +0300, Roger Quadros wrote: >> On 20/08/18 13:04, Zeng Tao wrote: >> > The new scheme is required just to support legacy low and full-speed >> > devices. For high speed devices, it will slower the enumeration speed. >> > So in this patch we try the "old" enumeration scheme first for high >> > speed devices, and this is what Windows does since Windows 8. >> > >> > Signed-off-by: Zeng Tao >> >> You need to add Alan's Ack here. I don't think you need to resend >though. >> >> Reviewed-by: Roger Quadros > >Please resend, I need his ack here :) > >thanks, > >greg k-h
RE: [RFT PATCH 1/2] xhci: Fix leaking USB3 shared_hcd at xhci removal
> Ensure that the shared_hcd pointer is valid when calling usb_put_hcd()
>
> The shared_hcd is removed and freed in xhci by first calling
> usb_remove_hcd(xhci-
> >shared_hcd), and later
> usb_put_hcd(xhci->shared_hcd)
>
> Afer commit fe190ed0d602 ("xhci: Do not halt the host until both HCD have
> disconnected their devices.") the shared_hcd was never properly put as
> xhci->shared_hcd was set to NULL before usb_put_hcd(xhci->shared_hcd)
> xhci->was
> called.
>
> shared_hcd (USB3) is removed before primary hcd (USB2).
> While removing the primary hcd we might need to handle xhci interrupts to
> cleanly
> remove last USB2 devices, therefore we need to set
> xhci->shared_hcd to NULL before removing the primary hcd to let xhci
> interrupt handler know shared_hcd is no longer available.
>
> xhci-plat.c, xhci-histb.c and xhci-mtk first create both their hcd's before
> adding them.
> so to keep the correct reverse removal order use a temporary shared_hcd
> variable
> for them.
> For more details see commit 4ac53087d6d4 ("usb: xhci: plat: Create both HCDs
> before adding them")
>
> Fixes: fe190ed0d602 ("xhci: Do not halt the host until both HCD have
> disconnected
> their devices.")
> Cc: Joel Stanley
> Cc: Chunfeng Yun
> Cc: Thierry Reding
> Cc: Jianguo Sun
> Cc:
> Reported-by: Jack Pham
> Signed-off-by: Mathias Nyman
> ---
> drivers/usb/host/xhci-histb.c | 6 --
> drivers/usb/host/xhci-mtk.c | 6 --
> drivers/usb/host/xhci-pci.c | 1 +
> drivers/usb/host/xhci-plat.c | 6 -- drivers/usb/host/xhci-tegra.c | 1 +
> drivers/usb/host/xhci.c | 2 --
> 6 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-histb.c b/drivers/usb/host/xhci-histb.c
> index
> 27f0016..3c4abb5 100644
> --- a/drivers/usb/host/xhci-histb.c
> +++ b/drivers/usb/host/xhci-histb.c
> @@ -325,14 +325,16 @@ static int xhci_histb_remove(struct platform_device
> *dev)
> struct xhci_hcd_histb *histb = platform_get_drvdata(dev);
> struct usb_hcd *hcd = histb->hcd;
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + struct usb_hcd *shared_hcd = xhci->shared_hcd;
>
> xhci->xhc_state |= XHCI_STATE_REMOVING;
>
> - usb_remove_hcd(xhci->shared_hcd);
> + usb_remove_hcd(shared_hcd);
> + xhci->shared_hcd = NULL;
> device_wakeup_disable(&dev->dev);
>
> usb_remove_hcd(hcd);
> - usb_put_hcd(xhci->shared_hcd);
> + usb_put_hcd(shared_hcd);
>
> xhci_histb_host_disable(histb);
> usb_put_hcd(hcd);
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c index
> 71d0d33..60987c7 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -590,12 +590,14 @@ static int xhci_mtk_remove(struct platform_device *dev)
> struct xhci_hcd_mtk *mtk = platform_get_drvdata(dev);
> struct usb_hcd *hcd = mtk->hcd;
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + struct usb_hcd *shared_hcd = xhci->shared_hcd;
>
> - usb_remove_hcd(xhci->shared_hcd);
> + usb_remove_hcd(shared_hcd);
> + xhci->shared_hcd = NULL;
> device_init_wakeup(&dev->dev, false);
>
> usb_remove_hcd(hcd);
> - usb_put_hcd(xhci->shared_hcd);
> + usb_put_hcd(shared_hcd);
> usb_put_hcd(hcd);
> xhci_mtk_sch_exit(mtk);
> xhci_mtk_clks_disable(mtk);
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index
> 51dd8e0..92fd6b6 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -356,6 +356,7 @@ static void xhci_pci_remove(struct pci_dev *dev)
> if (xhci->shared_hcd) {
> usb_remove_hcd(xhci->shared_hcd);
> usb_put_hcd(xhci->shared_hcd);
> + xhci->shared_hcd = NULL;
> }
>
> /* Workaround for spurious wakeups at shutdown with HSW */ diff --git
> a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index
> 94e9392..e5da8ce
> 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -359,14 +359,16 @@ static int xhci_plat_remove(struct platform_device *dev)
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> struct clk *clk = xhci->clk;
> struct clk *reg_clk = xhci->reg_clk;
> + struct usb_hcd *shared_hcd = xhci->shared_hcd;
>
> xhci->xhc_state |= XHCI_STATE_REMOVING;
>
> - usb_remove_hcd(xhci->shared_hcd);
> + usb_remove_hcd(shared_hcd);
> + xhci->shared_hcd = NULL;
> usb_phy_shutdown(hcd->usb_phy);
>
> usb_remove_hcd(hcd);
> - usb_put_hcd(xhci->shared_hcd);
> + usb_put_hcd(shared_hcd);
>
> clk_disable_unprepare(clk);
> clk_disable_unprepare(reg_clk);
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index
> 4b463e5..b1cce98 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -1240,6 +1240,7 @@ static int tegra_xusb_remove(struct platform_device
> *pdev)
>
> usb_remove_hcd(xhci->sha
RE: [RFT PATCH 2/2] xhci: handle port status events for removed USB3 hcd
> > Cc:
> > Reported-by: Peter Chen
> > Signed-off-by: Mathias Nyman
> > ---
> > drivers/usb/host/xhci-ring.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/usb/host/xhci-ring.c
> > b/drivers/usb/host/xhci-ring.c index f0a99aa..3d314b8 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -1552,6 +1552,13 @@ static void handle_port_status(struct xhci_hcd *xhci,
> > goto cleanup;
> > }
> >
> > + /* We might get interrupts after shared_hcd is removed */
> > + if (port->rhub == &xhci->usb3_rhub && xhci->shared_hcd == NULL) {
> > + xhci_dbg(xhci, "ignore port event for removed USB3 hcd\n");
> > + bogus_port_status = true;
> > + goto cleanup;
> > + }
> > +
> > hcd = port->rhub->hcd;
> > bus_state = &xhci->bus_state[hcd_index(hcd)];
> > hcd_portnum = port->hcd_portnum;
> >
>
> This probably only applies from 4.18 onwards, to test on older kernel try
> something
> like this instead:
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index
> 6996235..7925da9 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1606,7 +1606,11 @@ static void handle_port_status(struct xhci_hcd *xhci,
> hcd = xhci_to_hcd(xhci);
> if ((major_revision == 0x03) != (hcd->speed >= HCD_USB3))
> hcd = xhci->shared_hcd;
> -
> + if (!hcd) {
> + bogus_port_status = true;
> + goto cleanup;
> + }
> if (major_revision == 0) {
> xhci_warn(xhci, "Event for port %u not in "
> "Extended Capabilities, ignoring.\n",
>
>
> Jack, Peter, do these patches solve the remove issues you are seeing?
At my two USB3 platforms, only apply the 1st patch can fix my problem. Maybe
my USB3 port change interrupt occurs always before removing USB2 HCD.
Peter
[PATCH] usb: hub: try old enumeration scheme first for high speed devices
The new scheme is required just to support legacy low and full-speed
devices. For high speed devices, it will slower the enumeration speed.
So in this patch we try the "old" enumeration scheme first for high speed
devices, and this is what Windows does since Windows 8.
Signed-off-by: Zeng Tao
Acked-by: Alan Stern
Reviewed-by: Roger Quadros
---
Documentation/admin-guide/kernel-parameters.txt | 3 ++-
drivers/usb/core/hub.c | 4 +++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index 92eb1f4..151c527 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4610,7 +4610,8 @@
usbcore.old_scheme_first=
[USB] Start with the old device initialization
- scheme (default 0 = off).
+ scheme, applies only to low and full-speed devices
+(default 0 = off).
usbcore.usbfs_memory_mb=
[USB] Memory limit (in MB) for buffers allocated by
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 462ce49..fb4e6bb 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2660,11 +2660,13 @@ static bool use_new_scheme(struct usb_device *udev, int
retry,
{
int old_scheme_first_port =
port_dev->quirks & USB_PORT_QUIRK_OLD_SCHEME;
+ int quick_enumeration = (udev->speed == USB_SPEED_HIGH);
if (udev->speed >= USB_SPEED_SUPER)
return false;
- return USE_NEW_SCHEME(retry, old_scheme_first_port || old_scheme_first);
+ return USE_NEW_SCHEME(retry, old_scheme_first_port || old_scheme_first
+ || quick_enumeration);
}
/* Is a USB 3.0 port in the Inactive or Compliance Mode state?
--
2.7.4
[PATCH] usb: dwc2: disable power_down on rockchip devices
>From 04fbf78e4e569bf872f1ffcb0a6f9b89569dc913 Mon Sep 17 00:00:00 2001 From: Hal Emmerich Date: Thu, 19 Jul 2018 21:48:08 -0500 Subject: [PATCH] usb: dwc2: disable power_down on rockchip devices The bug would let the usb controller enter partial power down, which was formally known as hibernate, upon boot if nothing was plugged in to the port. Partial power down couldn't be exited properly, so any usb devices plugged in after boot would not be usable. Before the name change, params.hibernation was false by default, so _dwc2_hcd_suspend() would skip entering hibernation. With the rename, _dwc2_hcd_suspend() was changed to use params.power_down to decide whether or not to enter partial power down. Since params.power_down is non-zero by default, it needs to be set to 0 for rockchip devices to restore functionality. This bug was reported in the linux-usb thread: REGRESSION: usb: dwc2: USB device not seen after boot The commit that caused this regression is: 6d23ee9caa6790aea047f9aca7f3c03cb8d96eb6 Signed-off-by: Hal Emmerich --- drivers/usb/dwc2/params.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c index f03e41879224..492607adc506 100644 --- a/drivers/usb/dwc2/params.c +++ b/drivers/usb/dwc2/params.c @@ -82,6 +82,7 @@ static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg) p->host_perio_tx_fifo_size = 256; p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT; + p->power_down = 0; } static void dwc2_set_ltq_params(struct dwc2_hsotg *hsotg) -- 2.11.0
Re: [RFT PATCH 1/2] xhci: Fix leaking USB3 shared_hcd at xhci removal
On Thu, 2018-09-27 at 19:26 +0300, Mathias Nyman wrote:
> Ensure that the shared_hcd pointer is valid when calling usb_put_hcd()
>
> The shared_hcd is removed and freed in xhci by first calling
> usb_remove_hcd(xhci->shared_hcd), and later
> usb_put_hcd(xhci->shared_hcd)
>
> Afer commit fe190ed0d602 ("xhci: Do not halt the host until both HCD have
> disconnected their devices.") the shared_hcd was never properly put as
> xhci->shared_hcd was set to NULL before usb_put_hcd(xhci->shared_hcd) was
> called.
>
> shared_hcd (USB3) is removed before primary hcd (USB2).
> While removing the primary hcd we might need to handle xhci interrupts
> to cleanly remove last USB2 devices, therefore we need to set
> xhci->shared_hcd to NULL before removing the primary hcd to let xhci
> interrupt handler know shared_hcd is no longer available.
>
> xhci-plat.c, xhci-histb.c and xhci-mtk first create both their hcd's before
> adding them. so to keep the correct reverse removal order use a temporary
> shared_hcd variable for them.
> For more details see commit 4ac53087d6d4 ("usb: xhci: plat: Create both
> HCDs before adding them")
>
> Fixes: fe190ed0d602 ("xhci: Do not halt the host until both HCD have
> disconnected their devices.")
> Cc: Joel Stanley
> Cc: Chunfeng Yun
> Cc: Thierry Reding
> Cc: Jianguo Sun
> Cc:
> Reported-by: Jack Pham
> Signed-off-by: Mathias Nyman
> ---
> drivers/usb/host/xhci-histb.c | 6 --
> drivers/usb/host/xhci-mtk.c | 6 --
> drivers/usb/host/xhci-pci.c | 1 +
> drivers/usb/host/xhci-plat.c | 6 --
> drivers/usb/host/xhci-tegra.c | 1 +
> drivers/usb/host/xhci.c | 2 --
> 6 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-histb.c b/drivers/usb/host/xhci-histb.c
> index 27f0016..3c4abb5 100644
> --- a/drivers/usb/host/xhci-histb.c
> +++ b/drivers/usb/host/xhci-histb.c
> @@ -325,14 +325,16 @@ static int xhci_histb_remove(struct platform_device
> *dev)
> struct xhci_hcd_histb *histb = platform_get_drvdata(dev);
> struct usb_hcd *hcd = histb->hcd;
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + struct usb_hcd *shared_hcd = xhci->shared_hcd;
>
> xhci->xhc_state |= XHCI_STATE_REMOVING;
>
> - usb_remove_hcd(xhci->shared_hcd);
> + usb_remove_hcd(shared_hcd);
> + xhci->shared_hcd = NULL;
> device_wakeup_disable(&dev->dev);
>
> usb_remove_hcd(hcd);
> - usb_put_hcd(xhci->shared_hcd);
> + usb_put_hcd(shared_hcd);
>
> xhci_histb_host_disable(histb);
> usb_put_hcd(hcd);
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index 71d0d33..60987c7 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -590,12 +590,14 @@ static int xhci_mtk_remove(struct platform_device *dev)
> struct xhci_hcd_mtk *mtk = platform_get_drvdata(dev);
> struct usb_hcd *hcd = mtk->hcd;
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + struct usb_hcd *shared_hcd = xhci->shared_hcd;
>
> - usb_remove_hcd(xhci->shared_hcd);
> + usb_remove_hcd(shared_hcd);
> + xhci->shared_hcd = NULL;
> device_init_wakeup(&dev->dev, false);
>
> usb_remove_hcd(hcd);
> - usb_put_hcd(xhci->shared_hcd);
> + usb_put_hcd(shared_hcd);
> usb_put_hcd(hcd);
> xhci_mtk_sch_exit(mtk);
> xhci_mtk_clks_disable(mtk);
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 51dd8e0..92fd6b6 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -356,6 +356,7 @@ static void xhci_pci_remove(struct pci_dev *dev)
> if (xhci->shared_hcd) {
> usb_remove_hcd(xhci->shared_hcd);
> usb_put_hcd(xhci->shared_hcd);
> + xhci->shared_hcd = NULL;
> }
>
> /* Workaround for spurious wakeups at shutdown with HSW */
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 94e9392..e5da8ce 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -359,14 +359,16 @@ static int xhci_plat_remove(struct platform_device *dev)
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> struct clk *clk = xhci->clk;
> struct clk *reg_clk = xhci->reg_clk;
> + struct usb_hcd *shared_hcd = xhci->shared_hcd;
>
> xhci->xhc_state |= XHCI_STATE_REMOVING;
>
> - usb_remove_hcd(xhci->shared_hcd);
> + usb_remove_hcd(shared_hcd);
> + xhci->shared_hcd = NULL;
> usb_phy_shutdown(hcd->usb_phy);
>
> usb_remove_hcd(hcd);
> - usb_put_hcd(xhci->shared_hcd);
> + usb_put_hcd(shared_hcd);
>
> clk_disable_unprepare(clk);
> clk_disable_unprepare(reg_clk);
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index 4b463e5..b1cce98 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -1240,6 +1240,7 @@ static int tegra_xusb_remove(struct platform_device
> *pdev)
[PATCH] usb: usbtmc: check size before allocating buffer and remove duplicated allocation
From: Colin Ian King
Currently the allocation of a buffer is performed before a sanity check on
the required buffer size and if the buffer size is too large the error exit
return leaks the allocated buffer. Fix this by checking the size before
allocating.
Also, the same buffer is allocated again inside an if statement, causing
the first allocation to be leaked. Fix this by removing this second
redundant allocation.
Detected by CoverityScan, CID#1473697 ("Resource leak")
Fixes: 658f24f4523e ("usb: usbtmc: Add ioctl for generic requests on control")
Signed-off-by: Colin Ian King
---
drivers/usb/class/usbtmc.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 0fcb81a1399b..c01edff190d2 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -1895,18 +1895,14 @@ static int usbtmc_ioctl_request(struct
usbtmc_device_data *data,
if (res)
return -EFAULT;
+ if (request.req.wLength > USBTMC_BUFSIZE)
+ return -EMSGSIZE;
+
buffer = kmalloc(request.req.wLength, GFP_KERNEL);
if (!buffer)
return -ENOMEM;
- if (request.req.wLength > USBTMC_BUFSIZE)
- return -EMSGSIZE;
-
if (request.req.wLength) {
- buffer = kmalloc(request.req.wLength, GFP_KERNEL);
- if (!buffer)
- return -ENOMEM;
-
if ((request.req.bRequestType & USB_DIR_IN) == 0) {
/* Send control data to device */
res = copy_from_user(buffer, request.data,
--
2.17.1
Re: [RFT PATCH 2/2] xhci: handle port status events for removed USB3 hcd
On 27.09.2018 19:26, Mathias Nyman wrote:
At xhci removal the USB3 hcd (shared_hcd) is removed before the primary
USB2 hcd. Interrupts for port status changes may still occur for USB3
ports after the shared_hcd is freed, causing NULL pointer dereference.
Check if xhci->shared_hcd is still valid before handing USB3 port events
Cc:
Reported-by: Peter Chen
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-ring.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f0a99aa..3d314b8 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1552,6 +1552,13 @@ static void handle_port_status(struct xhci_hcd *xhci,
goto cleanup;
}
+ /* We might get interrupts after shared_hcd is removed */
+ if (port->rhub == &xhci->usb3_rhub && xhci->shared_hcd == NULL) {
+ xhci_dbg(xhci, "ignore port event for removed USB3 hcd\n");
+ bogus_port_status = true;
+ goto cleanup;
+ }
+
hcd = port->rhub->hcd;
bus_state = &xhci->bus_state[hcd_index(hcd)];
hcd_portnum = port->hcd_portnum;
This probably only applies from 4.18 onwards, to test on older kernel try
something
like this instead:
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 6996235..7925da9 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1606,7 +1606,11 @@ static void handle_port_status(struct xhci_hcd *xhci,
hcd = xhci_to_hcd(xhci);
if ((major_revision == 0x03) != (hcd->speed >= HCD_USB3))
hcd = xhci->shared_hcd;
-
+ if (!hcd) {
+ bogus_port_status = true;
+ goto cleanup;
+ }
if (major_revision == 0) {
xhci_warn(xhci, "Event for port %u not in "
"Extended Capabilities, ignoring.\n",
Jack, Peter, do these patches solve the remove issues you are seeing?
Thanks
-Mathias
[RFT PATCH 2/2] xhci: handle port status events for removed USB3 hcd
At xhci removal the USB3 hcd (shared_hcd) is removed before the primary
USB2 hcd. Interrupts for port status changes may still occur for USB3
ports after the shared_hcd is freed, causing NULL pointer dereference.
Check if xhci->shared_hcd is still valid before handing USB3 port events
Cc:
Reported-by: Peter Chen
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-ring.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f0a99aa..3d314b8 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1552,6 +1552,13 @@ static void handle_port_status(struct xhci_hcd *xhci,
goto cleanup;
}
+ /* We might get interrupts after shared_hcd is removed */
+ if (port->rhub == &xhci->usb3_rhub && xhci->shared_hcd == NULL) {
+ xhci_dbg(xhci, "ignore port event for removed USB3 hcd\n");
+ bogus_port_status = true;
+ goto cleanup;
+ }
+
hcd = port->rhub->hcd;
bus_state = &xhci->bus_state[hcd_index(hcd)];
hcd_portnum = port->hcd_portnum;
--
2.7.4
[RFT PATCH 1/2] xhci: Fix leaking USB3 shared_hcd at xhci removal
Ensure that the shared_hcd pointer is valid when calling usb_put_hcd()
The shared_hcd is removed and freed in xhci by first calling
usb_remove_hcd(xhci->shared_hcd), and later
usb_put_hcd(xhci->shared_hcd)
Afer commit fe190ed0d602 ("xhci: Do not halt the host until both HCD have
disconnected their devices.") the shared_hcd was never properly put as
xhci->shared_hcd was set to NULL before usb_put_hcd(xhci->shared_hcd) was
called.
shared_hcd (USB3) is removed before primary hcd (USB2).
While removing the primary hcd we might need to handle xhci interrupts
to cleanly remove last USB2 devices, therefore we need to set
xhci->shared_hcd to NULL before removing the primary hcd to let xhci
interrupt handler know shared_hcd is no longer available.
xhci-plat.c, xhci-histb.c and xhci-mtk first create both their hcd's before
adding them. so to keep the correct reverse removal order use a temporary
shared_hcd variable for them.
For more details see commit 4ac53087d6d4 ("usb: xhci: plat: Create both
HCDs before adding them")
Fixes: fe190ed0d602 ("xhci: Do not halt the host until both HCD have
disconnected their devices.")
Cc: Joel Stanley
Cc: Chunfeng Yun
Cc: Thierry Reding
Cc: Jianguo Sun
Cc:
Reported-by: Jack Pham
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-histb.c | 6 --
drivers/usb/host/xhci-mtk.c | 6 --
drivers/usb/host/xhci-pci.c | 1 +
drivers/usb/host/xhci-plat.c | 6 --
drivers/usb/host/xhci-tegra.c | 1 +
drivers/usb/host/xhci.c | 2 --
6 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-histb.c b/drivers/usb/host/xhci-histb.c
index 27f0016..3c4abb5 100644
--- a/drivers/usb/host/xhci-histb.c
+++ b/drivers/usb/host/xhci-histb.c
@@ -325,14 +325,16 @@ static int xhci_histb_remove(struct platform_device *dev)
struct xhci_hcd_histb *histb = platform_get_drvdata(dev);
struct usb_hcd *hcd = histb->hcd;
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ struct usb_hcd *shared_hcd = xhci->shared_hcd;
xhci->xhc_state |= XHCI_STATE_REMOVING;
- usb_remove_hcd(xhci->shared_hcd);
+ usb_remove_hcd(shared_hcd);
+ xhci->shared_hcd = NULL;
device_wakeup_disable(&dev->dev);
usb_remove_hcd(hcd);
- usb_put_hcd(xhci->shared_hcd);
+ usb_put_hcd(shared_hcd);
xhci_histb_host_disable(histb);
usb_put_hcd(hcd);
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 71d0d33..60987c7 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -590,12 +590,14 @@ static int xhci_mtk_remove(struct platform_device *dev)
struct xhci_hcd_mtk *mtk = platform_get_drvdata(dev);
struct usb_hcd *hcd = mtk->hcd;
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ struct usb_hcd *shared_hcd = xhci->shared_hcd;
- usb_remove_hcd(xhci->shared_hcd);
+ usb_remove_hcd(shared_hcd);
+ xhci->shared_hcd = NULL;
device_init_wakeup(&dev->dev, false);
usb_remove_hcd(hcd);
- usb_put_hcd(xhci->shared_hcd);
+ usb_put_hcd(shared_hcd);
usb_put_hcd(hcd);
xhci_mtk_sch_exit(mtk);
xhci_mtk_clks_disable(mtk);
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 51dd8e0..92fd6b6 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -356,6 +356,7 @@ static void xhci_pci_remove(struct pci_dev *dev)
if (xhci->shared_hcd) {
usb_remove_hcd(xhci->shared_hcd);
usb_put_hcd(xhci->shared_hcd);
+ xhci->shared_hcd = NULL;
}
/* Workaround for spurious wakeups at shutdown with HSW */
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 94e9392..e5da8ce 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -359,14 +359,16 @@ static int xhci_plat_remove(struct platform_device *dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci->clk;
struct clk *reg_clk = xhci->reg_clk;
+ struct usb_hcd *shared_hcd = xhci->shared_hcd;
xhci->xhc_state |= XHCI_STATE_REMOVING;
- usb_remove_hcd(xhci->shared_hcd);
+ usb_remove_hcd(shared_hcd);
+ xhci->shared_hcd = NULL;
usb_phy_shutdown(hcd->usb_phy);
usb_remove_hcd(hcd);
- usb_put_hcd(xhci->shared_hcd);
+ usb_put_hcd(shared_hcd);
clk_disable_unprepare(clk);
clk_disable_unprepare(reg_clk);
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 4b463e5..b1cce98 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -1240,6 +1240,7 @@ static int tegra_xusb_remove(struct platform_device *pdev)
usb_remove_hcd(xhci->shared_hcd);
usb_put_hcd(xhci->shared_hcd);
+ xhci->shared_hcd = NULL;
usb_remove_hcd(tegra->hcd);
usb_put_hcd(tegra->hcd);
diff --git a/drivers/usb/hos
[PATCH] dt-bindings: usb-xhci: Document r8a7744 support
Document r8a7744 xhci support. The driver will use the fallback compatible string "renesas,rcar-gen2-xhci", therefore no driver change is needed. Signed-off-by: Biju Das Reviewed-by: Chris Paterson --- This patch is tested against linux-next next-20180927 and usb-next --- Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt index fb564e7..fea8b15 100644 --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt @@ -8,6 +8,7 @@ Required properties: - "marvell,armada-375-xhci" for Armada 375 SoCs - "marvell,armada-380-xhci" for Armada 38x SoCs - "renesas,xhci-r8a7743" for r8a7743 SoC +- "renesas,xhci-r8a7744" for r8a7744 SoC - "renesas,xhci-r8a774a1" for r8a774a1 SoC - "renesas,xhci-r8a7790" for r8a7790 SoC - "renesas,xhci-r8a7791" for r8a7791 SoC -- 2.7.4
[PATCH] dt-bindings: usb: renesas_usbhs: Add support for r8a7744
Document support for RZ/G1N (R8A7744) SoC. Signed-off-by: Biju Das Reviewed-by: Chris Paterson --- This patch is tested against linux-next next-20180927 and usb-next --- Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt index 15fb3b3..a5dbdb5 100644 --- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt +++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt @@ -4,6 +4,7 @@ Required properties: - compatible: Must contain one or more of the following: - "renesas,usbhs-r8a7743" for r8a7743 (RZ/G1M) compatible device + - "renesas,usbhs-r8a7744" for r8a7744 (RZ/G1N) compatible device - "renesas,usbhs-r8a7745" for r8a7745 (RZ/G1E) compatible device - "renesas,usbhs-r8a774a1" for r8a774a1 (RZ/G2M) compatible device - "renesas,usbhs-r8a7790" for r8a7790 (R-Car H2) compatible device -- 2.7.4
Re: [PATCH] usb: dwc2: Fix HiKey regression caused by power_down feature
Hi John, On 9/25/2018 21:59, John Stultz wrote: > On Tue, Sep 25, 2018 at 3:04 AM, Artur Petrosyan > wrote: >> Just a clarification by this commit "[PATCH] usb: dwc2: Fix HiKey >> regression caused by power_down feature" >> https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3Dlinux-2Dusb-26m-3D152669095513248-26w-3D2&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=9hPBFKCJ_nBjJhGVrrlYOeOQjP_HlVzYqrC_D7niMJI&m=0lMkv7adFVwkzyaUzD6-pUG0iwg4fd6b1-aHQgbqvSI&s=m8SZvo3J_Za08sMbo-S9EkhoA06YnzEN-SRm-uTPnbg&e= >> >> the power_down is disabled setting "p->power_down = false;" in >> "dwc2_set_his_params" function. >> >> Could you please clarify that the testes done for those 3 patches were >> done enabling "p->power_down = true;" in "dwc2_set_his_params" function. > > So if I remove the "power_down = true" initialization, USB does not > seem to function. > > If I boot w/ the gadget port removed, the USB host ports do work, but > plugging in the gadget cable results in a bunch of: > dwc2 f72c.usb: Waiting for Host Mode, Mode=Peripheral > messages. > > If I boot w/ the gadget port plugged in, USB gadget mode doesn't seem > to function at all, and when I remove the gadget cable nothing > happens, it doesn't switch to host mode. > > thanks > -john > We would like to buy the HiKey board to perform testes. We found this HiKey LeMaker to have USB 2.0 ports https://www.ebay.com/itm/HiKey-LeMaker-version-2GB-Kirin-620-SoC-8-core-ARM-Cortex-A53-CPU-ARM-Mali-450/263958047308?hash=item3d75202a4c:g:aGsAAOSwOkxbqqot on ebay. Could you please confirm that it is the right board to test the issues you mention. Regards, Artur
Re: [PATCH 1/2] usb: typec: fusb302: Correct spelling mistake for toggling state
On Wed, Sep 26, 2018 at 04:23:51PM +0100, Adam Thomson wrote:
> There's a typo in the enum name of the 'OFF' state for toggling
> (TOGGLINE instead of TOGGLING). This commit resolves that trivial
> spelling inconsistency.
>
> Signed-off-by: Adam Thomson
Reviewed-by: Heikki Krogerus
> ---
> drivers/usb/typec/tcpm/fusb302.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/fusb302.c
> b/drivers/usb/typec/tcpm/fusb302.c
> index 6e9370a..fd851d8 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -42,7 +42,7 @@
> #define T_BC_LVL_DEBOUNCE_DELAY_MS 30
>
> enum toggling_mode {
> - TOGGLINE_MODE_OFF,
> + TOGGLING_MODE_OFF,
> TOGGLING_MODE_DRP,
> TOGGLING_MODE_SNK,
> TOGGLING_MODE_SRC,
> @@ -594,7 +594,7 @@ static int fusb302_set_toggling(struct fusb302_chip *chip,
> chip->intr_comp_chng = false;
> /* configure toggling mode: none/snk/src/drp */
> switch (mode) {
> - case TOGGLINE_MODE_OFF:
> + case TOGGLING_MODE_OFF:
> ret = fusb302_i2c_mask_write(chip, FUSB_REG_CONTROL2,
>FUSB_REG_CONTROL2_MODE_MASK,
>FUSB_REG_CONTROL2_MODE_NONE);
> @@ -626,7 +626,7 @@ static int fusb302_set_toggling(struct fusb302_chip *chip,
> break;
> }
>
> - if (mode == TOGGLINE_MODE_OFF) {
> + if (mode == TOGGLING_MODE_OFF) {
> /* mask TOGDONE interrupt */
> ret = fusb302_i2c_set_bits(chip, FUSB_REG_MASKA,
> FUSB_REG_MASKA_TOGDONE);
> @@ -702,7 +702,7 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum
> typec_cc_status cc)
> ret = -EINVAL;
> goto done;
> }
> - ret = fusb302_set_toggling(chip, TOGGLINE_MODE_OFF);
> + ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF);
> if (ret < 0) {
> fusb302_log(chip, "cannot stop toggling, ret=%d", ret);
> goto done;
> @@ -1292,7 +1292,7 @@ static int fusb302_handle_togdone_snk(struct
> fusb302_chip *chip,
> tcpm_cc_change(chip->tcpm_port);
> }
> /* turn off toggling */
> - ret = fusb302_set_toggling(chip, TOGGLINE_MODE_OFF);
> + ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF);
> if (ret < 0) {
> fusb302_log(chip,
> "cannot set toggling mode off, ret=%d", ret);
> @@ -1388,7 +1388,7 @@ static int fusb302_handle_togdone_src(struct
> fusb302_chip *chip,
> tcpm_cc_change(chip->tcpm_port);
> }
> /* turn off toggling */
> - ret = fusb302_set_toggling(chip, TOGGLINE_MODE_OFF);
> + ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF);
> if (ret < 0) {
> fusb302_log(chip,
> "cannot set toggling mode off, ret=%d", ret);
> --
> 1.9.1
--
heikki
Re: [PATCH 2/2] usb: typec: fusb302: Resolve fixed power role contract setup
On Wed, Sep 26, 2018 at 04:23:52PM +0100, Adam Thomson wrote:
> When the controller is configured for a fixed power role (Source
> only or Sink only), attach does not proceed within the TCPM state
> machine as there is no CC event generated by this driver to update
> the CC line status.
>
> To rectify this, when CC is configured as Source or Sink we now
> make use of the hardware's automatic fixed Source or Sink
> toggling mechanism, which detects attaches in the same way as for
> DRP toggling. In this way the result of toggling is handled in the
> same way by the 'fusb302_handle_togdone()' function, and CC events
> are generated as expected for TCPM allowing a contract to be
> established.
>
> Signed-off-by: Adam Thomson
Reviewed-by: Heikki Krogerus
> ---
> drivers/usb/typec/tcpm/fusb302.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/usb/typec/tcpm/fusb302.c
> b/drivers/usb/typec/tcpm/fusb302.c
> index fd851d8..43b64d9 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -679,6 +679,7 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum
> typec_cc_status cc)
> int ret = 0;
> bool pull_up, pull_down;
> u8 rd_mda;
> + enum toggling_mode mode;
>
> mutex_lock(&chip->lock);
> switch (cc) {
> @@ -764,6 +765,29 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum
> typec_cc_status cc)
> chip->intr_comp_chng = false;
> }
> fusb302_log(chip, "cc := %s", typec_cc_status_name[cc]);
> +
> + /* Enable detection for fixed SNK or SRC only roles */
> + switch (cc) {
> + case TYPEC_CC_RD:
> + mode = TOGGLING_MODE_SNK;
> + break;
> + case TYPEC_CC_RP_DEF:
> + case TYPEC_CC_RP_1_5:
> + case TYPEC_CC_RP_3_0:
> + mode = TOGGLING_MODE_SRC;
> + break;
> + default:
> + mode = TOGGLING_MODE_OFF;
> + break;
> + }
> +
> + if (mode != TOGGLING_MODE_OFF) {
> + ret = fusb302_set_toggling(chip, mode);
> + if (ret < 0)
> + fusb302_log(chip,
> + "cannot set fixed role toggling mode,
> ret=%d",
> + ret);
> + }
> done:
> mutex_unlock(&chip->lock);
>
> --
> 1.9.1
--
heikki
Re: [PATCH 2/5] usb: host: Add OHCI driver for Broadcom STB SoCs
On Thu, Sep 27, 2018 at 12:20 AM Al Cooper wrote: > > This driver enables USB OHCI on Broadcom ARM and MIPS STB SoCs. > The drivers depend on a matching "brcm,brcmstb-usb-phy" > Broadcom STB USB Phy driver. > > The standard platform driver can't be used because of differences > in PHY and Clock handling. The standard PHY handling in hcd.c will > do a phy_exit/phy_init on suspend/resume and this will end up > shutting down the PHYs to the point that the host controller > registers are no longer accessible and will cause suspend to crash. > The clocks specified in device tree for these drivers are not > available in mainline so instead of returning EPROBE_DEFER when > the specified clock is not found and eventually failing probe, > the clock pointer is set to NULL which disables all clock handling. Both of these sound like fairly minor differences, I wonder if the generic code could be adapted to have a special case for these rather than duplicating it. Arnd
