linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] xhci fixes for usb-linus
@ 2022-08-25 15:08 Mathias Nyman
  2022-08-25 15:08 ` [PATCH 1/3] xhci: Fix null pointer dereference in remove if xHC has only one roothub Mathias Nyman
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Mathias Nyman @ 2022-08-25 15:08 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

Hi Greg

A few xhci fixes for usb-linus.

USB3 devices aren't visible immediately after xHC reset, so don't
stop polling the roothub and suspend too early after xHC reset.

Also Revert the port poweroff patch due to regression,
and fix a null pointer issue for xHC hosts with just one roothub.

-Mathias

Mathias Nyman (3):
  xhci: Fix null pointer dereference in remove if xHC has only one
    roothub
  xhci: Add grace period after xHC start to prevent premature runtime
    suspend.
  Revert "xhci: turn off port power in shutdown"

 drivers/usb/host/xhci-hub.c  | 13 ++++++++++++-
 drivers/usb/host/xhci-plat.c | 11 ++++++++---
 drivers/usb/host/xhci.c      | 19 +++++--------------
 drivers/usb/host/xhci.h      |  4 +---
 4 files changed, 26 insertions(+), 21 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] xhci: Fix null pointer dereference in remove if xHC has only one roothub
  2022-08-25 15:08 [PATCH 0/3] xhci fixes for usb-linus Mathias Nyman
@ 2022-08-25 15:08 ` Mathias Nyman
  2022-08-25 15:08 ` [PATCH 2/3] xhci: Add grace period after xHC start to prevent premature runtime suspend Mathias Nyman
  2022-08-25 15:08 ` [PATCH 3/3] Revert "xhci: turn off port power in shutdown" Mathias Nyman
  2 siblings, 0 replies; 4+ messages in thread
From: Mathias Nyman @ 2022-08-25 15:08 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman, Alexey Sheplyakov

The remove path in xhci platform driver tries to remove and put both main
and shared hcds even if only a main hcd exists (one roothub)

This causes a null pointer dereference in reboot for those controllers.

Check that the shared_hcd exists before trying to remove it.

Fixes: e0fe986972f5 ("usb: host: xhci-plat: prepare operation w/o shared hcd")
Reported-by: Alexey Sheplyakov <asheplyakov@basealt.ru>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-plat.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 044855818cb1..a8641b6536ee 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -398,12 +398,17 @@ static int xhci_plat_remove(struct platform_device *dev)
 	pm_runtime_get_sync(&dev->dev);
 	xhci->xhc_state |= XHCI_STATE_REMOVING;
 
-	usb_remove_hcd(shared_hcd);
-	xhci->shared_hcd = NULL;
+	if (shared_hcd) {
+		usb_remove_hcd(shared_hcd);
+		xhci->shared_hcd = NULL;
+	}
+
 	usb_phy_shutdown(hcd->usb_phy);
 
 	usb_remove_hcd(hcd);
-	usb_put_hcd(shared_hcd);
+
+	if (shared_hcd)
+		usb_put_hcd(shared_hcd);
 
 	clk_disable_unprepare(clk);
 	clk_disable_unprepare(reg_clk);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] xhci: Add grace period after xHC start to prevent premature runtime suspend.
  2022-08-25 15:08 [PATCH 0/3] xhci fixes for usb-linus Mathias Nyman
  2022-08-25 15:08 ` [PATCH 1/3] xhci: Fix null pointer dereference in remove if xHC has only one roothub Mathias Nyman
@ 2022-08-25 15:08 ` Mathias Nyman
  2022-08-25 15:08 ` [PATCH 3/3] Revert "xhci: turn off port power in shutdown" Mathias Nyman
  2 siblings, 0 replies; 4+ messages in thread
From: Mathias Nyman @ 2022-08-25 15:08 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman, stable

After xHC controller is started, either in probe or resume, it can take
a while before any of the connected usb devices are visible to the roothub
due to link training.

It's possible xhci driver loads, sees no acivity and suspends the host
before the USB device is visible.

In one testcase with a hotplugged xHC controller the host finally detected
the connected USB device and generated a wake 500ms after host initial
start.

If hosts didn't suspend the device duringe training it probablty wouldn't
take up to 500ms to detect it, but looking at specs reveal USB3 link
training has a couple long timeout values, such as 120ms
RxDetectQuietTimeout, and 360ms PollingLFPSTimeout.

So Add a 500ms grace period that keeps polling the roothub for 500ms after
start, preventing runtime suspend until USB devices are detected.

Cc: stable@vger.kernel.org
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c | 11 +++++++++++
 drivers/usb/host/xhci.c     |  4 +++-
 drivers/usb/host/xhci.h     |  2 +-
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0fdc014c9401..b30298986a69 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1648,6 +1648,17 @@ int xhci_hub_status_data(struct usb_hcd *hcd, char *buf)
 
 	status = bus_state->resuming_ports;
 
+	/*
+	 * SS devices are only visible to roothub after link training completes.
+	 * Keep polling roothubs for a grace period after xHC start
+	 */
+	if (xhci->run_graceperiod) {
+		if (time_before(jiffies, xhci->run_graceperiod))
+			status = 1;
+		else
+			xhci->run_graceperiod = 0;
+	}
+
 	mask = PORT_CSC | PORT_PEC | PORT_OCC | PORT_PLC | PORT_WRC | PORT_CEC;
 
 	/* For each port, did anything change?  If so, set that bit in buf. */
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 65858f607437..1afd32beec99 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -151,9 +151,11 @@ int xhci_start(struct xhci_hcd *xhci)
 		xhci_err(xhci, "Host took too long to start, "
 				"waited %u microseconds.\n",
 				XHCI_MAX_HALT_USEC);
-	if (!ret)
+	if (!ret) {
 		/* clear state flags. Including dying, halted or removing */
 		xhci->xhc_state = 0;
+		xhci->run_graceperiod = jiffies + msecs_to_jiffies(500);
+	}
 
 	return ret;
 }
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 1960b47acfb2..df6f2ebaff18 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1826,7 +1826,7 @@ struct xhci_hcd {
 
 	/* Host controller watchdog timer structures */
 	unsigned int		xhc_state;
-
+	unsigned long		run_graceperiod;
 	u32			command;
 	struct s3_save		s3;
 /* Host controller is dying - not responding to commands. "I'm not dead yet!"
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] Revert "xhci: turn off port power in shutdown"
  2022-08-25 15:08 [PATCH 0/3] xhci fixes for usb-linus Mathias Nyman
  2022-08-25 15:08 ` [PATCH 1/3] xhci: Fix null pointer dereference in remove if xHC has only one roothub Mathias Nyman
  2022-08-25 15:08 ` [PATCH 2/3] xhci: Add grace period after xHC start to prevent premature runtime suspend Mathias Nyman
@ 2022-08-25 15:08 ` Mathias Nyman
  2 siblings, 0 replies; 4+ messages in thread
From: Mathias Nyman @ 2022-08-25 15:08 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Mathias Nyman

This reverts commit 83810f84ecf11dfc5a9414a8b762c3501b328185.

Turning off port power in shutdown did cause issues such as a laptop not
proprly powering off, and some specific usb devies failing to enumerate the
subsequent boot after a warm reset.

So revert this.

Fixes: 83810f84ecf1 ("xhci: turn off port power in shutdown")
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c |  2 +-
 drivers/usb/host/xhci.c     | 15 ++-------------
 drivers/usb/host/xhci.h     |  2 --
 3 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index b30298986a69..4619d5e89d5b 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -652,7 +652,7 @@ struct xhci_hub *xhci_get_rhub(struct usb_hcd *hcd)
  * It will release and re-aquire the lock while calling ACPI
  * method.
  */
-void xhci_set_port_power(struct xhci_hcd *xhci, struct usb_hcd *hcd,
+static void xhci_set_port_power(struct xhci_hcd *xhci, struct usb_hcd *hcd,
 				u16 index, bool on, unsigned long *flags)
 	__must_hold(&xhci->lock)
 {
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 1afd32beec99..38649284ff88 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -793,8 +793,6 @@ static void xhci_stop(struct usb_hcd *hcd)
 void xhci_shutdown(struct usb_hcd *hcd)
 {
 	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
-	unsigned long flags;
-	int i;
 
 	if (xhci->quirks & XHCI_SPURIOUS_REBOOT)
 		usb_disable_xhci_ports(to_pci_dev(hcd->self.sysdev));
@@ -810,21 +808,12 @@ void xhci_shutdown(struct usb_hcd *hcd)
 		del_timer_sync(&xhci->shared_hcd->rh_timer);
 	}
 
-	spin_lock_irqsave(&xhci->lock, flags);
+	spin_lock_irq(&xhci->lock);
 	xhci_halt(xhci);
-
-	/* Power off USB2 ports*/
-	for (i = 0; i < xhci->usb2_rhub.num_ports; i++)
-		xhci_set_port_power(xhci, xhci->main_hcd, i, false, &flags);
-
-	/* Power off USB3 ports*/
-	for (i = 0; i < xhci->usb3_rhub.num_ports; i++)
-		xhci_set_port_power(xhci, xhci->shared_hcd, i, false, &flags);
-
 	/* Workaround for spurious wakeups at shutdown with HSW */
 	if (xhci->quirks & XHCI_SPURIOUS_WAKEUP)
 		xhci_reset(xhci, XHCI_RESET_SHORT_USEC);
-	spin_unlock_irqrestore(&xhci->lock, flags);
+	spin_unlock_irq(&xhci->lock);
 
 	xhci_cleanup_msix(xhci);
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index df6f2ebaff18..7caa0db5e826 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2196,8 +2196,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, u16 wIndex,
 int xhci_hub_status_data(struct usb_hcd *hcd, char *buf);
 int xhci_find_raw_port_number(struct usb_hcd *hcd, int port1);
 struct xhci_hub *xhci_get_rhub(struct usb_hcd *hcd);
-void xhci_set_port_power(struct xhci_hcd *xhci, struct usb_hcd *hcd, u16 index,
-			 bool on, unsigned long *flags);
 
 void xhci_hc_died(struct xhci_hcd *xhci);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-08-25 15:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-25 15:08 [PATCH 0/3] xhci fixes for usb-linus Mathias Nyman
2022-08-25 15:08 ` [PATCH 1/3] xhci: Fix null pointer dereference in remove if xHC has only one roothub Mathias Nyman
2022-08-25 15:08 ` [PATCH 2/3] xhci: Add grace period after xHC start to prevent premature runtime suspend Mathias Nyman
2022-08-25 15:08 ` [PATCH 3/3] Revert "xhci: turn off port power in shutdown" Mathias Nyman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).