The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] USB: hub: call ACPI _PRR reset during port power-cycle on enumeration failure
@ 2026-03-26  1:17 Chia-Lin Kao (AceLan)
  2026-07-03  9:43 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 5+ messages in thread
From: Chia-Lin Kao (AceLan) @ 2026-03-26  1:17 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Mathias Nyman, Alan Stern, Kuen-Han Tsai,
	Ingo Molnar, Thomas Gleixner, Chia-Lin Kao (AceLan),
	Thorsten Blum, Kees Cook, Mika Westerberg, Heikki Krogerus,
	Chenyuan Yang
  Cc: linux-usb, linux-kernel

Some USB-connected devices (e.g. MT7925 Bluetooth on Dell laptops) expose
their hardware reset line via an ACPI Power Resource for Reset (_PRR)
rather than relying solely on VBUS cycling for recovery.  When the reset
GPIO gets stuck low the device stops responding on USB; a VBUS power-cycle
alone cannot recover it because the chip remains in reset regardless of
VBUS state.

Add usb_acpi_port_prr_reset() in usb-acpi.c that, given a hub device and
one-based port number, looks up the port's ACPI companion handle, evaluates
_PRR to obtain the power-resource reference, and then calls _RST on that
reference to toggle the reset line.  The function is a no-op if the port
has no ACPI handle or no _PRR method, so it is safe to call unconditionally
for every port.

Wire it into hub_port_connect() during the mid-retry VBUS power-cycle
(at (PORT_INIT_TRIES-1)/2 iterations), calling usb_acpi_port_prr_reset()
*after* VBUS goes off and *before* VBUS comes back on.  The ordering is
critical: on the tested hardware the ACPI _RST method (MBTR._RST) drives
BT_RST low for 200 ms then high again.  If _RST is called after VBUS is
already restored the GPIO pulse races with device enumeration starting on
the live bus; the device begins asserting USB signals while still held in
reset and enumeration fails.  Performing the reset while the port is
de-powered ensures the GPIO pulse completes fully before the device is
given power and time to initialise.

After VBUS is restored, add an msleep(100) conditional on _PRR._RST having
succeeded.  USB 2.0 spec §7.1.7.3 (Fig. 7-29) mandates a minimum of 100 ms
between VBUS power-on and the first reset signalling for power settling.
On root hubs, hub_power_on_good_delay() returns bPwrOn2PwrGood * 2 with
no minimum floor; on the tested xHCI root hub bPwrOn2PwrGood = 10, yielding
only 20 ms — well below the spec minimum.  (External hubs already enforce
a 100 ms minimum via hub_power_on_good_delay().)  When _PRR._RST has been
exercised the device must also complete its full power-on sequence (GPIO
de-assertion, internal oscillator start, firmware load) before USB
enumeration begins.  The 100 ms sleep enforces the spec minimum and gives
the device adequate settling time.

Tested on a Dell laptop with MT7925 Bluetooth (idVendor=0489,
idProduct=e139) whose BT_RST GPIO was stuck low.  With this fix the
device recovers autonomously at boot without requiring a G3
(mechanical power-off) cycle.  The relevant dmesg sequence:

  [    1.448491] usb 3-10: new high-speed USB device number 4 using xhci_hcd
  [    6.813942] usb 3-10: device descriptor read/64, error -110
  [   22.685978] usb 3-10: device descriptor read/64, error -110
  [   22.901715] usb 3-10: new high-speed USB device number 5 using xhci_hcd
  [   28.317963] usb 3-10: device descriptor read/64, error -110
  [   44.189949] usb 3-10: device descriptor read/64, error -110
  [   44.294065] usb usb3-port10: attempt power cycle
  [   44.872709] usb 3-10: new high-speed USB device number 6 using xhci_hcd
  [   44.888293] usb 3-10: New USB device found, idVendor=0489, idProduct=e139, bcdDevice= 1.00
  [   44.888318] usb 3-10: Manufacturer: MediaTek Inc.

Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
---
 drivers/usb/core/hub.c      | 14 ++++++++
 drivers/usb/core/usb-acpi.c | 68 +++++++++++++++++++++++++++++++++++++
 drivers/usb/core/usb.h      |  3 ++
 3 files changed, 85 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 24960ba9caa91..1740e96f73cc6 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5603,11 +5603,25 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
 
 		/* When halfway through our retry count, power-cycle the port */
 		if (i == (PORT_INIT_TRIES - 1) / 2) {
+			int prr_reset;
+
 			dev_info(&port_dev->dev, "attempt power cycle\n");
 			usb_hub_set_port_power(hdev, hub, port1, false);
 			msleep(2 * hub_power_on_good_delay(hub));
+			prr_reset = usb_acpi_port_prr_reset(hdev, port1);
 			usb_hub_set_port_power(hdev, hub, port1, true);
 			msleep(hub_power_on_good_delay(hub));
+			/*
+			 * USB 2.0 spec §7.1.7.3 requires at least 100 ms
+			 * between VBUS power-on and the first reset for power
+			 * settling.  hub_power_on_good_delay() on an xHCI root
+			 * hub returns bPwrOn2PwrGood * 2 with no minimum floor,
+			 * which can be as little as 20 ms.  When _PRR _RST was
+			 * also exercised the device must complete its power-on
+			 * sequence before enumeration; enforce the spec minimum.
+			 */
+			if (prr_reset == 0)
+				msleep(100);
 		}
 	}
 	if (hub->hdev->parent ||
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index 489dbdc96f94a..ee62e3fd8e3a1 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -142,6 +142,74 @@ int usb_acpi_set_power_state(struct usb_device *hdev, int index, bool enable)
 }
 EXPORT_SYMBOL_GPL(usb_acpi_set_power_state);
 
+/**
+ * usb_acpi_port_prr_reset - issue an ACPI _PRR reset on a hub port
+ * @hdev: USB device belonging to the usb hub
+ * @port1: port number (one-based)
+ *
+ * Some devices expose their hardware reset line via an ACPI Power Resource for
+ * Reset (_PRR).  When such a device fails to enumerate (e.g. because the reset
+ * GPIO is stuck low), the USB power-cycle alone is not enough; the firmware
+ * reset path must also be exercised.
+ *
+ * This function evaluates _PRR on the port's ACPI companion to obtain the
+ * power-resource reference and then calls _RST on that resource to toggle the
+ * reset line.  It is intended to be called alongside the mid-retry VBUS
+ * power-cycle already performed by hub_port_connect().
+ *
+ * Returns 0 on success, -ENODEV if the port has no ACPI handle or no _PRR
+ * method, or a negative error code on failure.
+ */
+int usb_acpi_port_prr_reset(struct usb_device *hdev, int port1)
+{
+	acpi_handle port_handle;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *pkg, *ref;
+	acpi_status status;
+	int ret = 0;
+
+	port_handle = usb_get_hub_port_acpi_handle(hdev, port1);
+	if (!port_handle)
+		return -ENODEV;
+
+	if (!acpi_has_method(port_handle, "_PRR"))
+		return -ENODEV;
+
+	status = acpi_evaluate_object(port_handle, "_PRR", NULL, &buffer);
+	if (ACPI_FAILURE(status)) {
+		dev_dbg(&hdev->dev, "port%d: _PRR evaluation failed: %s\n",
+			port1, acpi_format_exception(status));
+		return -ENODEV;
+	}
+
+	pkg = buffer.pointer;
+	if (!pkg || pkg->type != ACPI_TYPE_PACKAGE || pkg->package.count < 1) {
+		dev_dbg(&hdev->dev, "port%d: _PRR returned unexpected object\n",
+			port1);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ref = &pkg->package.elements[0];
+	if (ref->type != ACPI_TYPE_LOCAL_REFERENCE || !ref->reference.handle) {
+		dev_dbg(&hdev->dev, "port%d: _PRR element is not a reference\n",
+			port1);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	status = acpi_evaluate_object(ref->reference.handle, "_RST", NULL, NULL);
+	if (ACPI_FAILURE(status)) {
+		dev_dbg(&hdev->dev, "port%d: _RST evaluation failed: %s\n",
+			port1, acpi_format_exception(status));
+		ret = -EIO;
+	}
+
+out:
+	kfree(buffer.pointer);
+	return ret;
+}
+
 /**
  * usb_acpi_add_usb4_devlink - add device link to USB4 Host Interface for tunneled USB3 devices
  *
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index a9b37aeb515be..4d3dc3bd881b2 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -211,7 +211,10 @@ extern int usb_acpi_register(void);
 extern void usb_acpi_unregister(void);
 extern acpi_handle usb_get_hub_port_acpi_handle(struct usb_device *hdev,
 	int port1);
+extern int usb_acpi_port_prr_reset(struct usb_device *hdev, int port1);
 #else
 static inline int usb_acpi_register(void) { return 0; };
 static inline void usb_acpi_unregister(void) { };
+static inline int usb_acpi_port_prr_reset(struct usb_device *hdev,
+					  int port1) { return -ENODEV; }
 #endif
-- 
2.53.0


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

* Re: [PATCH] USB: hub: call ACPI _PRR reset during port power-cycle on enumeration failure
  2026-03-26  1:17 [PATCH] USB: hub: call ACPI _PRR reset during port power-cycle on enumeration failure Chia-Lin Kao (AceLan)
@ 2026-07-03  9:43 ` Greg Kroah-Hartman
  2026-07-06  5:04   ` AceLan Kao
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2026-07-03  9:43 UTC (permalink / raw)
  To: Chia-Lin Kao (AceLan)
  Cc: Mathias Nyman, Alan Stern, Kuen-Han Tsai, Ingo Molnar,
	Thomas Gleixner, Thorsten Blum, Kees Cook, Mika Westerberg,
	Heikki Krogerus, Chenyuan Yang, linux-usb, linux-kernel

On Thu, Mar 26, 2026 at 09:17:08AM +0800, Chia-Lin Kao (AceLan) wrote:
> Some USB-connected devices (e.g. MT7925 Bluetooth on Dell laptops) expose
> their hardware reset line via an ACPI Power Resource for Reset (_PRR)
> rather than relying solely on VBUS cycling for recovery.  When the reset
> GPIO gets stuck low the device stops responding on USB; a VBUS power-cycle
> alone cannot recover it because the chip remains in reset regardless of
> VBUS state.
> 
> Add usb_acpi_port_prr_reset() in usb-acpi.c that, given a hub device and
> one-based port number, looks up the port's ACPI companion handle, evaluates
> _PRR to obtain the power-resource reference, and then calls _RST on that
> reference to toggle the reset line.  The function is a no-op if the port
> has no ACPI handle or no _PRR method, so it is safe to call unconditionally
> for every port.
> 
> Wire it into hub_port_connect() during the mid-retry VBUS power-cycle
> (at (PORT_INIT_TRIES-1)/2 iterations), calling usb_acpi_port_prr_reset()
> *after* VBUS goes off and *before* VBUS comes back on.  The ordering is
> critical: on the tested hardware the ACPI _RST method (MBTR._RST) drives
> BT_RST low for 200 ms then high again.  If _RST is called after VBUS is
> already restored the GPIO pulse races with device enumeration starting on
> the live bus; the device begins asserting USB signals while still held in
> reset and enumeration fails.  Performing the reset while the port is
> de-powered ensures the GPIO pulse completes fully before the device is
> given power and time to initialise.
> 
> After VBUS is restored, add an msleep(100) conditional on _PRR._RST having
> succeeded.  USB 2.0 spec §7.1.7.3 (Fig. 7-29) mandates a minimum of 100 ms
> between VBUS power-on and the first reset signalling for power settling.
> On root hubs, hub_power_on_good_delay() returns bPwrOn2PwrGood * 2 with
> no minimum floor; on the tested xHCI root hub bPwrOn2PwrGood = 10, yielding
> only 20 ms — well below the spec minimum.  (External hubs already enforce
> a 100 ms minimum via hub_power_on_good_delay().)  When _PRR._RST has been
> exercised the device must also complete its full power-on sequence (GPIO
> de-assertion, internal oscillator start, firmware load) before USB
> enumeration begins.  The 100 ms sleep enforces the spec minimum and gives
> the device adequate settling time.
> 
> Tested on a Dell laptop with MT7925 Bluetooth (idVendor=0489,
> idProduct=e139) whose BT_RST GPIO was stuck low.  With this fix the
> device recovers autonomously at boot without requiring a G3
> (mechanical power-off) cycle.  The relevant dmesg sequence:
> 
>   [    1.448491] usb 3-10: new high-speed USB device number 4 using xhci_hcd
>   [    6.813942] usb 3-10: device descriptor read/64, error -110
>   [   22.685978] usb 3-10: device descriptor read/64, error -110
>   [   22.901715] usb 3-10: new high-speed USB device number 5 using xhci_hcd
>   [   28.317963] usb 3-10: device descriptor read/64, error -110
>   [   44.189949] usb 3-10: device descriptor read/64, error -110
>   [   44.294065] usb usb3-port10: attempt power cycle
>   [   44.872709] usb 3-10: new high-speed USB device number 6 using xhci_hcd
>   [   44.888293] usb 3-10: New USB device found, idVendor=0489, idProduct=e139, bcdDevice= 1.00
>   [   44.888318] usb 3-10: Manufacturer: MediaTek Inc.
> 
> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> ---
>  drivers/usb/core/hub.c      | 14 ++++++++
>  drivers/usb/core/usb-acpi.c | 68 +++++++++++++++++++++++++++++++++++++
>  drivers/usb/core/usb.h      |  3 ++
>  3 files changed, 85 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 24960ba9caa91..1740e96f73cc6 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -5603,11 +5603,25 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
>  
>  		/* When halfway through our retry count, power-cycle the port */
>  		if (i == (PORT_INIT_TRIES - 1) / 2) {
> +			int prr_reset;
> +
>  			dev_info(&port_dev->dev, "attempt power cycle\n");
>  			usb_hub_set_port_power(hdev, hub, port1, false);
>  			msleep(2 * hub_power_on_good_delay(hub));
> +			prr_reset = usb_acpi_port_prr_reset(hdev, port1);
>  			usb_hub_set_port_power(hdev, hub, port1, true);
>  			msleep(hub_power_on_good_delay(hub));
> +			/*
> +			 * USB 2.0 spec §7.1.7.3 requires at least 100 ms
> +			 * between VBUS power-on and the first reset for power
> +			 * settling.  hub_power_on_good_delay() on an xHCI root
> +			 * hub returns bPwrOn2PwrGood * 2 with no minimum floor,
> +			 * which can be as little as 20 ms.  When _PRR _RST was
> +			 * also exercised the device must complete its power-on
> +			 * sequence before enumeration; enforce the spec minimum.
> +			 */
> +			if (prr_reset == 0)
> +				msleep(100);
>  		}
>  	}
>  	if (hub->hdev->parent ||
> diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
> index 489dbdc96f94a..ee62e3fd8e3a1 100644
> --- a/drivers/usb/core/usb-acpi.c
> +++ b/drivers/usb/core/usb-acpi.c
> @@ -142,6 +142,74 @@ int usb_acpi_set_power_state(struct usb_device *hdev, int index, bool enable)
>  }
>  EXPORT_SYMBOL_GPL(usb_acpi_set_power_state);
>  
> +/**
> + * usb_acpi_port_prr_reset - issue an ACPI _PRR reset on a hub port
> + * @hdev: USB device belonging to the usb hub
> + * @port1: port number (one-based)
> + *
> + * Some devices expose their hardware reset line via an ACPI Power Resource for
> + * Reset (_PRR).  When such a device fails to enumerate (e.g. because the reset
> + * GPIO is stuck low), the USB power-cycle alone is not enough; the firmware
> + * reset path must also be exercised.
> + *
> + * This function evaluates _PRR on the port's ACPI companion to obtain the
> + * power-resource reference and then calls _RST on that resource to toggle the
> + * reset line.  It is intended to be called alongside the mid-retry VBUS
> + * power-cycle already performed by hub_port_connect().
> + *
> + * Returns 0 on success, -ENODEV if the port has no ACPI handle or no _PRR
> + * method, or a negative error code on failure.
> + */
> +int usb_acpi_port_prr_reset(struct usb_device *hdev, int port1)
> +{
> +	acpi_handle port_handle;
> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *pkg, *ref;
> +	acpi_status status;
> +	int ret = 0;
> +
> +	port_handle = usb_get_hub_port_acpi_handle(hdev, port1);
> +	if (!port_handle)
> +		return -ENODEV;
> +
> +	if (!acpi_has_method(port_handle, "_PRR"))
> +		return -ENODEV;
> +
> +	status = acpi_evaluate_object(port_handle, "_PRR", NULL, &buffer);
> +	if (ACPI_FAILURE(status)) {
> +		dev_dbg(&hdev->dev, "port%d: _PRR evaluation failed: %s\n",
> +			port1, acpi_format_exception(status));
> +		return -ENODEV;
> +	}
> +
> +	pkg = buffer.pointer;
> +	if (!pkg || pkg->type != ACPI_TYPE_PACKAGE || pkg->package.count < 1) {
> +		dev_dbg(&hdev->dev, "port%d: _PRR returned unexpected object\n",
> +			port1);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ref = &pkg->package.elements[0];
> +	if (ref->type != ACPI_TYPE_LOCAL_REFERENCE || !ref->reference.handle) {
> +		dev_dbg(&hdev->dev, "port%d: _PRR element is not a reference\n",
> +			port1);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	status = acpi_evaluate_object(ref->reference.handle, "_RST", NULL, NULL);
> +	if (ACPI_FAILURE(status)) {
> +		dev_dbg(&hdev->dev, "port%d: _RST evaluation failed: %s\n",
> +			port1, acpi_format_exception(status));
> +		ret = -EIO;
> +	}
> +
> +out:
> +	kfree(buffer.pointer);
> +	return ret;
> +}
> +
>  /**
>   * usb_acpi_add_usb4_devlink - add device link to USB4 Host Interface for tunneled USB3 devices
>   *
> diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
> index a9b37aeb515be..4d3dc3bd881b2 100644
> --- a/drivers/usb/core/usb.h
> +++ b/drivers/usb/core/usb.h
> @@ -211,7 +211,10 @@ extern int usb_acpi_register(void);
>  extern void usb_acpi_unregister(void);
>  extern acpi_handle usb_get_hub_port_acpi_handle(struct usb_device *hdev,
>  	int port1);
> +extern int usb_acpi_port_prr_reset(struct usb_device *hdev, int port1);
>  #else
>  static inline int usb_acpi_register(void) { return 0; };
>  static inline void usb_acpi_unregister(void) { };
> +static inline int usb_acpi_port_prr_reset(struct usb_device *hdev,
> +					  int port1) { return -ENODEV; }
>  #endif
> -- 
> 2.53.0
> 
> 

Please see the review comments at:
	https://sashiko.dev/#/patchset/20260326011708.1128840-1-acelan.kao@canonical.com

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

* Re: [PATCH] USB: hub: call ACPI _PRR reset during port power-cycle on enumeration failure
  2026-07-03  9:43 ` Greg Kroah-Hartman
@ 2026-07-06  5:04   ` AceLan Kao
  2026-07-06  6:37     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 5+ messages in thread
From: AceLan Kao @ 2026-07-06  5:04 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Mathias Nyman, Alan Stern, Kuen-Han Tsai, Ingo Molnar,
	Thomas Gleixner, Thorsten Blum, Kees Cook, Mika Westerberg,
	Heikki Krogerus, Chenyuan Yang, linux-usb, linux-kernel

Greg Kroah-Hartman <gregkh@linuxfoundation.org> 於 2026年7月3日週五 下午5:43寫道:
>
> On Thu, Mar 26, 2026 at 09:17:08AM +0800, Chia-Lin Kao (AceLan) wrote:
> > Some USB-connected devices (e.g. MT7925 Bluetooth on Dell laptops) expose
> > their hardware reset line via an ACPI Power Resource for Reset (_PRR)
> > rather than relying solely on VBUS cycling for recovery.  When the reset
> > GPIO gets stuck low the device stops responding on USB; a VBUS power-cycle
> > alone cannot recover it because the chip remains in reset regardless of
> > VBUS state.
> >
> > Add usb_acpi_port_prr_reset() in usb-acpi.c that, given a hub device and
> > one-based port number, looks up the port's ACPI companion handle, evaluates
> > _PRR to obtain the power-resource reference, and then calls _RST on that
> > reference to toggle the reset line.  The function is a no-op if the port
> > has no ACPI handle or no _PRR method, so it is safe to call unconditionally
> > for every port.
> >
> > Wire it into hub_port_connect() during the mid-retry VBUS power-cycle
> > (at (PORT_INIT_TRIES-1)/2 iterations), calling usb_acpi_port_prr_reset()
> > *after* VBUS goes off and *before* VBUS comes back on.  The ordering is
> > critical: on the tested hardware the ACPI _RST method (MBTR._RST) drives
> > BT_RST low for 200 ms then high again.  If _RST is called after VBUS is
> > already restored the GPIO pulse races with device enumeration starting on
> > the live bus; the device begins asserting USB signals while still held in
> > reset and enumeration fails.  Performing the reset while the port is
> > de-powered ensures the GPIO pulse completes fully before the device is
> > given power and time to initialise.
> >
> > After VBUS is restored, add an msleep(100) conditional on _PRR._RST having
> > succeeded.  USB 2.0 spec §7.1.7.3 (Fig. 7-29) mandates a minimum of 100 ms
> > between VBUS power-on and the first reset signalling for power settling.
> > On root hubs, hub_power_on_good_delay() returns bPwrOn2PwrGood * 2 with
> > no minimum floor; on the tested xHCI root hub bPwrOn2PwrGood = 10, yielding
> > only 20 ms — well below the spec minimum.  (External hubs already enforce
> > a 100 ms minimum via hub_power_on_good_delay().)  When _PRR._RST has been
> > exercised the device must also complete its full power-on sequence (GPIO
> > de-assertion, internal oscillator start, firmware load) before USB
> > enumeration begins.  The 100 ms sleep enforces the spec minimum and gives
> > the device adequate settling time.
> >
> > Tested on a Dell laptop with MT7925 Bluetooth (idVendor=0489,
> > idProduct=e139) whose BT_RST GPIO was stuck low.  With this fix the
> > device recovers autonomously at boot without requiring a G3
> > (mechanical power-off) cycle.  The relevant dmesg sequence:
> >
> >   [    1.448491] usb 3-10: new high-speed USB device number 4 using xhci_hcd
> >   [    6.813942] usb 3-10: device descriptor read/64, error -110
> >   [   22.685978] usb 3-10: device descriptor read/64, error -110
> >   [   22.901715] usb 3-10: new high-speed USB device number 5 using xhci_hcd
> >   [   28.317963] usb 3-10: device descriptor read/64, error -110
> >   [   44.189949] usb 3-10: device descriptor read/64, error -110
> >   [   44.294065] usb usb3-port10: attempt power cycle
> >   [   44.872709] usb 3-10: new high-speed USB device number 6 using xhci_hcd
> >   [   44.888293] usb 3-10: New USB device found, idVendor=0489, idProduct=e139, bcdDevice= 1.00
> >   [   44.888318] usb 3-10: Manufacturer: MediaTek Inc.
> >
> > Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> > ---
> >  drivers/usb/core/hub.c      | 14 ++++++++
> >  drivers/usb/core/usb-acpi.c | 68 +++++++++++++++++++++++++++++++++++++
> >  drivers/usb/core/usb.h      |  3 ++
> >  3 files changed, 85 insertions(+)
> >
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 24960ba9caa91..1740e96f73cc6 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -5603,11 +5603,25 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
> >
> >               /* When halfway through our retry count, power-cycle the port */
> >               if (i == (PORT_INIT_TRIES - 1) / 2) {
> > +                     int prr_reset;
> > +
> >                       dev_info(&port_dev->dev, "attempt power cycle\n");
> >                       usb_hub_set_port_power(hdev, hub, port1, false);
> >                       msleep(2 * hub_power_on_good_delay(hub));
> > +                     prr_reset = usb_acpi_port_prr_reset(hdev, port1);
> >                       usb_hub_set_port_power(hdev, hub, port1, true);
> >                       msleep(hub_power_on_good_delay(hub));
> > +                     /*
> > +                      * USB 2.0 spec §7.1.7.3 requires at least 100 ms
> > +                      * between VBUS power-on and the first reset for power
> > +                      * settling.  hub_power_on_good_delay() on an xHCI root
> > +                      * hub returns bPwrOn2PwrGood * 2 with no minimum floor,
> > +                      * which can be as little as 20 ms.  When _PRR _RST was
> > +                      * also exercised the device must complete its power-on
> > +                      * sequence before enumeration; enforce the spec minimum.
> > +                      */
> > +                     if (prr_reset == 0)
> > +                             msleep(100);
> >               }
> >       }
> >       if (hub->hdev->parent ||
> > diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
> > index 489dbdc96f94a..ee62e3fd8e3a1 100644
> > --- a/drivers/usb/core/usb-acpi.c
> > +++ b/drivers/usb/core/usb-acpi.c
> > @@ -142,6 +142,74 @@ int usb_acpi_set_power_state(struct usb_device *hdev, int index, bool enable)
> >  }
> >  EXPORT_SYMBOL_GPL(usb_acpi_set_power_state);
> >
> > +/**
> > + * usb_acpi_port_prr_reset - issue an ACPI _PRR reset on a hub port
> > + * @hdev: USB device belonging to the usb hub
> > + * @port1: port number (one-based)
> > + *
> > + * Some devices expose their hardware reset line via an ACPI Power Resource for
> > + * Reset (_PRR).  When such a device fails to enumerate (e.g. because the reset
> > + * GPIO is stuck low), the USB power-cycle alone is not enough; the firmware
> > + * reset path must also be exercised.
> > + *
> > + * This function evaluates _PRR on the port's ACPI companion to obtain the
> > + * power-resource reference and then calls _RST on that resource to toggle the
> > + * reset line.  It is intended to be called alongside the mid-retry VBUS
> > + * power-cycle already performed by hub_port_connect().
> > + *
> > + * Returns 0 on success, -ENODEV if the port has no ACPI handle or no _PRR
> > + * method, or a negative error code on failure.
> > + */
> > +int usb_acpi_port_prr_reset(struct usb_device *hdev, int port1)
> > +{
> > +     acpi_handle port_handle;
> > +     struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> > +     union acpi_object *pkg, *ref;
> > +     acpi_status status;
> > +     int ret = 0;
> > +
> > +     port_handle = usb_get_hub_port_acpi_handle(hdev, port1);
> > +     if (!port_handle)
> > +             return -ENODEV;
> > +
> > +     if (!acpi_has_method(port_handle, "_PRR"))
> > +             return -ENODEV;
> > +
> > +     status = acpi_evaluate_object(port_handle, "_PRR", NULL, &buffer);
> > +     if (ACPI_FAILURE(status)) {
> > +             dev_dbg(&hdev->dev, "port%d: _PRR evaluation failed: %s\n",
> > +                     port1, acpi_format_exception(status));
> > +             return -ENODEV;
> > +     }
> > +
> > +     pkg = buffer.pointer;
> > +     if (!pkg || pkg->type != ACPI_TYPE_PACKAGE || pkg->package.count < 1) {
> > +             dev_dbg(&hdev->dev, "port%d: _PRR returned unexpected object\n",
> > +                     port1);
> > +             ret = -EINVAL;
> > +             goto out;
> > +     }
> > +
> > +     ref = &pkg->package.elements[0];
> > +     if (ref->type != ACPI_TYPE_LOCAL_REFERENCE || !ref->reference.handle) {
> > +             dev_dbg(&hdev->dev, "port%d: _PRR element is not a reference\n",
> > +                     port1);
> > +             ret = -EINVAL;
> > +             goto out;
> > +     }
> > +
> > +     status = acpi_evaluate_object(ref->reference.handle, "_RST", NULL, NULL);
> > +     if (ACPI_FAILURE(status)) {
> > +             dev_dbg(&hdev->dev, "port%d: _RST evaluation failed: %s\n",
> > +                     port1, acpi_format_exception(status));
> > +             ret = -EIO;
> > +     }
> > +
> > +out:
> > +     kfree(buffer.pointer);
> > +     return ret;
> > +}
> > +
> >  /**
> >   * usb_acpi_add_usb4_devlink - add device link to USB4 Host Interface for tunneled USB3 devices
> >   *
> > diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
> > index a9b37aeb515be..4d3dc3bd881b2 100644
> > --- a/drivers/usb/core/usb.h
> > +++ b/drivers/usb/core/usb.h
> > @@ -211,7 +211,10 @@ extern int usb_acpi_register(void);
> >  extern void usb_acpi_unregister(void);
> >  extern acpi_handle usb_get_hub_port_acpi_handle(struct usb_device *hdev,
> >       int port1);
> > +extern int usb_acpi_port_prr_reset(struct usb_device *hdev, int port1);
> >  #else
> >  static inline int usb_acpi_register(void) { return 0; };
> >  static inline void usb_acpi_unregister(void) { };
> > +static inline int usb_acpi_port_prr_reset(struct usb_device *hdev,
> > +                                       int port1) { return -ENODEV; }
> >  #endif
> > --
> > 2.53.0
> >
> >
>
> Please see the review comments at:
>         https://sashiko.dev/#/patchset/20260326011708.1128840-1-acelan.kao@canonical.com

Thanks Greg.

Went through the sashiko review. The single finding claims
usb_acpi_port_prr_reset() should iterate every element of the _PRR
package and call _RST on each. That's based on a misreading of _PRR.

Per ACPI 6.4 §7.3.26 (_PRR, Power Resource for Reset):

  "This object evaluates to a single reference to a power resource. ...
   Return Value: A single element Package containing a Reference to the
   power reset resource."

_PRR is a fixed single-element package, not a variable-length list.
ACPICA encodes this in acpredef.h as ACPI_PTYPE1_FIXED with 1 reference
(as opposed to _PR0/_PR1/_PR2/_PR3, which are ACPI_PTYPE1_VAR reference
lists -- I suspect that's the source of the confusion). The "reset must
affect all ACPI-described devices that reference it" wording in the _RST
description refers to the devices sharing that one power resource, not to
multiple resources in the package.

The two existing in-tree _PRR/_RST users take elements[0] and call _RST
on it exactly as this patch does:

  - drivers/bluetooth/btintel.c: btintel_acpi_reset_method()
  - drivers/net/.../iwlwifi/pcie/gen1_2/trans.c: iwl_trans_pcie_call_reset()

Both are in fact stricter -- they reject package.count != 1, whereas I
accept count >= 1 and use the first element. Happy to tighten my check
from "< 1" to "!= 1" to match that precedent and make the single-element
invariant explicit, but there's no functional issue here.

Thanks,
AceLan

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

* Re: [PATCH] USB: hub: call ACPI _PRR reset during port power-cycle on enumeration failure
  2026-07-06  5:04   ` AceLan Kao
@ 2026-07-06  6:37     ` Greg Kroah-Hartman
  2026-07-06  8:26       ` AceLan Kao
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2026-07-06  6:37 UTC (permalink / raw)
  To: AceLan Kao
  Cc: Mathias Nyman, Alan Stern, Kuen-Han Tsai, Ingo Molnar,
	Thomas Gleixner, Thorsten Blum, Kees Cook, Mika Westerberg,
	Heikki Krogerus, Chenyuan Yang, linux-usb, linux-kernel

On Mon, Jul 06, 2026 at 01:04:19PM +0800, AceLan Kao wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> 於 2026年7月3日週五 下午5:43寫道:
> >
> > On Thu, Mar 26, 2026 at 09:17:08AM +0800, Chia-Lin Kao (AceLan) wrote:
> > > Some USB-connected devices (e.g. MT7925 Bluetooth on Dell laptops) expose
> > > their hardware reset line via an ACPI Power Resource for Reset (_PRR)
> > > rather than relying solely on VBUS cycling for recovery.  When the reset
> > > GPIO gets stuck low the device stops responding on USB; a VBUS power-cycle
> > > alone cannot recover it because the chip remains in reset regardless of
> > > VBUS state.
> > >
> > > Add usb_acpi_port_prr_reset() in usb-acpi.c that, given a hub device and
> > > one-based port number, looks up the port's ACPI companion handle, evaluates
> > > _PRR to obtain the power-resource reference, and then calls _RST on that
> > > reference to toggle the reset line.  The function is a no-op if the port
> > > has no ACPI handle or no _PRR method, so it is safe to call unconditionally
> > > for every port.
> > >
> > > Wire it into hub_port_connect() during the mid-retry VBUS power-cycle
> > > (at (PORT_INIT_TRIES-1)/2 iterations), calling usb_acpi_port_prr_reset()
> > > *after* VBUS goes off and *before* VBUS comes back on.  The ordering is
> > > critical: on the tested hardware the ACPI _RST method (MBTR._RST) drives
> > > BT_RST low for 200 ms then high again.  If _RST is called after VBUS is
> > > already restored the GPIO pulse races with device enumeration starting on
> > > the live bus; the device begins asserting USB signals while still held in
> > > reset and enumeration fails.  Performing the reset while the port is
> > > de-powered ensures the GPIO pulse completes fully before the device is
> > > given power and time to initialise.
> > >
> > > After VBUS is restored, add an msleep(100) conditional on _PRR._RST having
> > > succeeded.  USB 2.0 spec §7.1.7.3 (Fig. 7-29) mandates a minimum of 100 ms
> > > between VBUS power-on and the first reset signalling for power settling.
> > > On root hubs, hub_power_on_good_delay() returns bPwrOn2PwrGood * 2 with
> > > no minimum floor; on the tested xHCI root hub bPwrOn2PwrGood = 10, yielding
> > > only 20 ms — well below the spec minimum.  (External hubs already enforce
> > > a 100 ms minimum via hub_power_on_good_delay().)  When _PRR._RST has been
> > > exercised the device must also complete its full power-on sequence (GPIO
> > > de-assertion, internal oscillator start, firmware load) before USB
> > > enumeration begins.  The 100 ms sleep enforces the spec minimum and gives
> > > the device adequate settling time.
> > >
> > > Tested on a Dell laptop with MT7925 Bluetooth (idVendor=0489,
> > > idProduct=e139) whose BT_RST GPIO was stuck low.  With this fix the
> > > device recovers autonomously at boot without requiring a G3
> > > (mechanical power-off) cycle.  The relevant dmesg sequence:
> > >
> > >   [    1.448491] usb 3-10: new high-speed USB device number 4 using xhci_hcd
> > >   [    6.813942] usb 3-10: device descriptor read/64, error -110
> > >   [   22.685978] usb 3-10: device descriptor read/64, error -110
> > >   [   22.901715] usb 3-10: new high-speed USB device number 5 using xhci_hcd
> > >   [   28.317963] usb 3-10: device descriptor read/64, error -110
> > >   [   44.189949] usb 3-10: device descriptor read/64, error -110
> > >   [   44.294065] usb usb3-port10: attempt power cycle
> > >   [   44.872709] usb 3-10: new high-speed USB device number 6 using xhci_hcd
> > >   [   44.888293] usb 3-10: New USB device found, idVendor=0489, idProduct=e139, bcdDevice= 1.00
> > >   [   44.888318] usb 3-10: Manufacturer: MediaTek Inc.
> > >
> > > Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> > > ---
> > >  drivers/usb/core/hub.c      | 14 ++++++++
> > >  drivers/usb/core/usb-acpi.c | 68 +++++++++++++++++++++++++++++++++++++
> > >  drivers/usb/core/usb.h      |  3 ++
> > >  3 files changed, 85 insertions(+)
> > >
> > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > > index 24960ba9caa91..1740e96f73cc6 100644
> > > --- a/drivers/usb/core/hub.c
> > > +++ b/drivers/usb/core/hub.c
> > > @@ -5603,11 +5603,25 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
> > >
> > >               /* When halfway through our retry count, power-cycle the port */
> > >               if (i == (PORT_INIT_TRIES - 1) / 2) {
> > > +                     int prr_reset;
> > > +
> > >                       dev_info(&port_dev->dev, "attempt power cycle\n");
> > >                       usb_hub_set_port_power(hdev, hub, port1, false);
> > >                       msleep(2 * hub_power_on_good_delay(hub));
> > > +                     prr_reset = usb_acpi_port_prr_reset(hdev, port1);
> > >                       usb_hub_set_port_power(hdev, hub, port1, true);
> > >                       msleep(hub_power_on_good_delay(hub));
> > > +                     /*
> > > +                      * USB 2.0 spec §7.1.7.3 requires at least 100 ms
> > > +                      * between VBUS power-on and the first reset for power
> > > +                      * settling.  hub_power_on_good_delay() on an xHCI root
> > > +                      * hub returns bPwrOn2PwrGood * 2 with no minimum floor,
> > > +                      * which can be as little as 20 ms.  When _PRR _RST was
> > > +                      * also exercised the device must complete its power-on
> > > +                      * sequence before enumeration; enforce the spec minimum.
> > > +                      */
> > > +                     if (prr_reset == 0)
> > > +                             msleep(100);
> > >               }
> > >       }
> > >       if (hub->hdev->parent ||
> > > diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
> > > index 489dbdc96f94a..ee62e3fd8e3a1 100644
> > > --- a/drivers/usb/core/usb-acpi.c
> > > +++ b/drivers/usb/core/usb-acpi.c
> > > @@ -142,6 +142,74 @@ int usb_acpi_set_power_state(struct usb_device *hdev, int index, bool enable)
> > >  }
> > >  EXPORT_SYMBOL_GPL(usb_acpi_set_power_state);
> > >
> > > +/**
> > > + * usb_acpi_port_prr_reset - issue an ACPI _PRR reset on a hub port
> > > + * @hdev: USB device belonging to the usb hub
> > > + * @port1: port number (one-based)
> > > + *
> > > + * Some devices expose their hardware reset line via an ACPI Power Resource for
> > > + * Reset (_PRR).  When such a device fails to enumerate (e.g. because the reset
> > > + * GPIO is stuck low), the USB power-cycle alone is not enough; the firmware
> > > + * reset path must also be exercised.
> > > + *
> > > + * This function evaluates _PRR on the port's ACPI companion to obtain the
> > > + * power-resource reference and then calls _RST on that resource to toggle the
> > > + * reset line.  It is intended to be called alongside the mid-retry VBUS
> > > + * power-cycle already performed by hub_port_connect().
> > > + *
> > > + * Returns 0 on success, -ENODEV if the port has no ACPI handle or no _PRR
> > > + * method, or a negative error code on failure.
> > > + */
> > > +int usb_acpi_port_prr_reset(struct usb_device *hdev, int port1)
> > > +{
> > > +     acpi_handle port_handle;
> > > +     struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> > > +     union acpi_object *pkg, *ref;
> > > +     acpi_status status;
> > > +     int ret = 0;
> > > +
> > > +     port_handle = usb_get_hub_port_acpi_handle(hdev, port1);
> > > +     if (!port_handle)
> > > +             return -ENODEV;
> > > +
> > > +     if (!acpi_has_method(port_handle, "_PRR"))
> > > +             return -ENODEV;
> > > +
> > > +     status = acpi_evaluate_object(port_handle, "_PRR", NULL, &buffer);
> > > +     if (ACPI_FAILURE(status)) {
> > > +             dev_dbg(&hdev->dev, "port%d: _PRR evaluation failed: %s\n",
> > > +                     port1, acpi_format_exception(status));
> > > +             return -ENODEV;
> > > +     }
> > > +
> > > +     pkg = buffer.pointer;
> > > +     if (!pkg || pkg->type != ACPI_TYPE_PACKAGE || pkg->package.count < 1) {
> > > +             dev_dbg(&hdev->dev, "port%d: _PRR returned unexpected object\n",
> > > +                     port1);
> > > +             ret = -EINVAL;
> > > +             goto out;
> > > +     }
> > > +
> > > +     ref = &pkg->package.elements[0];
> > > +     if (ref->type != ACPI_TYPE_LOCAL_REFERENCE || !ref->reference.handle) {
> > > +             dev_dbg(&hdev->dev, "port%d: _PRR element is not a reference\n",
> > > +                     port1);
> > > +             ret = -EINVAL;
> > > +             goto out;
> > > +     }
> > > +
> > > +     status = acpi_evaluate_object(ref->reference.handle, "_RST", NULL, NULL);
> > > +     if (ACPI_FAILURE(status)) {
> > > +             dev_dbg(&hdev->dev, "port%d: _RST evaluation failed: %s\n",
> > > +                     port1, acpi_format_exception(status));
> > > +             ret = -EIO;
> > > +     }
> > > +
> > > +out:
> > > +     kfree(buffer.pointer);
> > > +     return ret;
> > > +}
> > > +
> > >  /**
> > >   * usb_acpi_add_usb4_devlink - add device link to USB4 Host Interface for tunneled USB3 devices
> > >   *
> > > diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
> > > index a9b37aeb515be..4d3dc3bd881b2 100644
> > > --- a/drivers/usb/core/usb.h
> > > +++ b/drivers/usb/core/usb.h
> > > @@ -211,7 +211,10 @@ extern int usb_acpi_register(void);
> > >  extern void usb_acpi_unregister(void);
> > >  extern acpi_handle usb_get_hub_port_acpi_handle(struct usb_device *hdev,
> > >       int port1);
> > > +extern int usb_acpi_port_prr_reset(struct usb_device *hdev, int port1);
> > >  #else
> > >  static inline int usb_acpi_register(void) { return 0; };
> > >  static inline void usb_acpi_unregister(void) { };
> > > +static inline int usb_acpi_port_prr_reset(struct usb_device *hdev,
> > > +                                       int port1) { return -ENODEV; }
> > >  #endif
> > > --
> > > 2.53.0
> > >
> > >
> >
> > Please see the review comments at:
> >         https://sashiko.dev/#/patchset/20260326011708.1128840-1-acelan.kao@canonical.com
> 
> Thanks Greg.
> 
> Went through the sashiko review. The single finding claims
> usb_acpi_port_prr_reset() should iterate every element of the _PRR
> package and call _RST on each. That's based on a misreading of _PRR.
> 
> Per ACPI 6.4 §7.3.26 (_PRR, Power Resource for Reset):
> 
>   "This object evaluates to a single reference to a power resource. ...
>    Return Value: A single element Package containing a Reference to the
>    power reset resource."
> 
> _PRR is a fixed single-element package, not a variable-length list.
> ACPICA encodes this in acpredef.h as ACPI_PTYPE1_FIXED with 1 reference
> (as opposed to _PR0/_PR1/_PR2/_PR3, which are ACPI_PTYPE1_VAR reference
> lists -- I suspect that's the source of the confusion). The "reset must
> affect all ACPI-described devices that reference it" wording in the _RST
> description refers to the devices sharing that one power resource, not to
> multiple resources in the package.
> 
> The two existing in-tree _PRR/_RST users take elements[0] and call _RST
> on it exactly as this patch does:
> 
>   - drivers/bluetooth/btintel.c: btintel_acpi_reset_method()
>   - drivers/net/.../iwlwifi/pcie/gen1_2/trans.c: iwl_trans_pcie_call_reset()
> 
> Both are in fact stricter -- they reject package.count != 1, whereas I
> accept count >= 1 and use the first element. Happy to tighten my check
> from "< 1" to "!= 1" to match that precedent and make the single-element
> invariant explicit, but there's no functional issue here.

This is going to require a review from the ACPI maintainer that this is
the correct thing to be doing, no matter which way it ends up.  So
please submit a new version and cc: those developers (and tighten up
your cc: list as well to only those that are relevant.)

thanks,

greg k-h

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

* Re: [PATCH] USB: hub: call ACPI _PRR reset during port power-cycle on enumeration failure
  2026-07-06  6:37     ` Greg Kroah-Hartman
@ 2026-07-06  8:26       ` AceLan Kao
  0 siblings, 0 replies; 5+ messages in thread
From: AceLan Kao @ 2026-07-06  8:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Mathias Nyman, Alan Stern, Kuen-Han Tsai, Ingo Molnar,
	Thomas Gleixner, Thorsten Blum, Kees Cook, Mika Westerberg,
	Heikki Krogerus, Chenyuan Yang, linux-usb, linux-kernel

Greg Kroah-Hartman <gregkh@linuxfoundation.org> 於 2026年7月6日週一 下午2:38寫道:
>
> On Mon, Jul 06, 2026 at 01:04:19PM +0800, AceLan Kao wrote:
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> 於 2026年7月3日週五 下午5:43寫道:
> > >
> > > On Thu, Mar 26, 2026 at 09:17:08AM +0800, Chia-Lin Kao (AceLan) wrote:
> > > > Some USB-connected devices (e.g. MT7925 Bluetooth on Dell laptops) expose
> > > > their hardware reset line via an ACPI Power Resource for Reset (_PRR)
> > > > rather than relying solely on VBUS cycling for recovery.  When the reset
> > > > GPIO gets stuck low the device stops responding on USB; a VBUS power-cycle
> > > > alone cannot recover it because the chip remains in reset regardless of
> > > > VBUS state.
> > > >
> > > > Add usb_acpi_port_prr_reset() in usb-acpi.c that, given a hub device and
> > > > one-based port number, looks up the port's ACPI companion handle, evaluates
> > > > _PRR to obtain the power-resource reference, and then calls _RST on that
> > > > reference to toggle the reset line.  The function is a no-op if the port
> > > > has no ACPI handle or no _PRR method, so it is safe to call unconditionally
> > > > for every port.
> > > >
> > > > Wire it into hub_port_connect() during the mid-retry VBUS power-cycle
> > > > (at (PORT_INIT_TRIES-1)/2 iterations), calling usb_acpi_port_prr_reset()
> > > > *after* VBUS goes off and *before* VBUS comes back on.  The ordering is
> > > > critical: on the tested hardware the ACPI _RST method (MBTR._RST) drives
> > > > BT_RST low for 200 ms then high again.  If _RST is called after VBUS is
> > > > already restored the GPIO pulse races with device enumeration starting on
> > > > the live bus; the device begins asserting USB signals while still held in
> > > > reset and enumeration fails.  Performing the reset while the port is
> > > > de-powered ensures the GPIO pulse completes fully before the device is
> > > > given power and time to initialise.
> > > >
> > > > After VBUS is restored, add an msleep(100) conditional on _PRR._RST having
> > > > succeeded.  USB 2.0 spec §7.1.7.3 (Fig. 7-29) mandates a minimum of 100 ms
> > > > between VBUS power-on and the first reset signalling for power settling.
> > > > On root hubs, hub_power_on_good_delay() returns bPwrOn2PwrGood * 2 with
> > > > no minimum floor; on the tested xHCI root hub bPwrOn2PwrGood = 10, yielding
> > > > only 20 ms — well below the spec minimum.  (External hubs already enforce
> > > > a 100 ms minimum via hub_power_on_good_delay().)  When _PRR._RST has been
> > > > exercised the device must also complete its full power-on sequence (GPIO
> > > > de-assertion, internal oscillator start, firmware load) before USB
> > > > enumeration begins.  The 100 ms sleep enforces the spec minimum and gives
> > > > the device adequate settling time.
> > > >
> > > > Tested on a Dell laptop with MT7925 Bluetooth (idVendor=0489,
> > > > idProduct=e139) whose BT_RST GPIO was stuck low.  With this fix the
> > > > device recovers autonomously at boot without requiring a G3
> > > > (mechanical power-off) cycle.  The relevant dmesg sequence:
> > > >
> > > >   [    1.448491] usb 3-10: new high-speed USB device number 4 using xhci_hcd
> > > >   [    6.813942] usb 3-10: device descriptor read/64, error -110
> > > >   [   22.685978] usb 3-10: device descriptor read/64, error -110
> > > >   [   22.901715] usb 3-10: new high-speed USB device number 5 using xhci_hcd
> > > >   [   28.317963] usb 3-10: device descriptor read/64, error -110
> > > >   [   44.189949] usb 3-10: device descriptor read/64, error -110
> > > >   [   44.294065] usb usb3-port10: attempt power cycle
> > > >   [   44.872709] usb 3-10: new high-speed USB device number 6 using xhci_hcd
> > > >   [   44.888293] usb 3-10: New USB device found, idVendor=0489, idProduct=e139, bcdDevice= 1.00
> > > >   [   44.888318] usb 3-10: Manufacturer: MediaTek Inc.
> > > >
> > > > Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> > > > ---
> > > >  drivers/usb/core/hub.c      | 14 ++++++++
> > > >  drivers/usb/core/usb-acpi.c | 68 +++++++++++++++++++++++++++++++++++++
> > > >  drivers/usb/core/usb.h      |  3 ++
> > > >  3 files changed, 85 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > > > index 24960ba9caa91..1740e96f73cc6 100644
> > > > --- a/drivers/usb/core/hub.c
> > > > +++ b/drivers/usb/core/hub.c
> > > > @@ -5603,11 +5603,25 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
> > > >
> > > >               /* When halfway through our retry count, power-cycle the port */
> > > >               if (i == (PORT_INIT_TRIES - 1) / 2) {
> > > > +                     int prr_reset;
> > > > +
> > > >                       dev_info(&port_dev->dev, "attempt power cycle\n");
> > > >                       usb_hub_set_port_power(hdev, hub, port1, false);
> > > >                       msleep(2 * hub_power_on_good_delay(hub));
> > > > +                     prr_reset = usb_acpi_port_prr_reset(hdev, port1);
> > > >                       usb_hub_set_port_power(hdev, hub, port1, true);
> > > >                       msleep(hub_power_on_good_delay(hub));
> > > > +                     /*
> > > > +                      * USB 2.0 spec §7.1.7.3 requires at least 100 ms
> > > > +                      * between VBUS power-on and the first reset for power
> > > > +                      * settling.  hub_power_on_good_delay() on an xHCI root
> > > > +                      * hub returns bPwrOn2PwrGood * 2 with no minimum floor,
> > > > +                      * which can be as little as 20 ms.  When _PRR _RST was
> > > > +                      * also exercised the device must complete its power-on
> > > > +                      * sequence before enumeration; enforce the spec minimum.
> > > > +                      */
> > > > +                     if (prr_reset == 0)
> > > > +                             msleep(100);
> > > >               }
> > > >       }
> > > >       if (hub->hdev->parent ||
> > > > diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
> > > > index 489dbdc96f94a..ee62e3fd8e3a1 100644
> > > > --- a/drivers/usb/core/usb-acpi.c
> > > > +++ b/drivers/usb/core/usb-acpi.c
> > > > @@ -142,6 +142,74 @@ int usb_acpi_set_power_state(struct usb_device *hdev, int index, bool enable)
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(usb_acpi_set_power_state);
> > > >
> > > > +/**
> > > > + * usb_acpi_port_prr_reset - issue an ACPI _PRR reset on a hub port
> > > > + * @hdev: USB device belonging to the usb hub
> > > > + * @port1: port number (one-based)
> > > > + *
> > > > + * Some devices expose their hardware reset line via an ACPI Power Resource for
> > > > + * Reset (_PRR).  When such a device fails to enumerate (e.g. because the reset
> > > > + * GPIO is stuck low), the USB power-cycle alone is not enough; the firmware
> > > > + * reset path must also be exercised.
> > > > + *
> > > > + * This function evaluates _PRR on the port's ACPI companion to obtain the
> > > > + * power-resource reference and then calls _RST on that resource to toggle the
> > > > + * reset line.  It is intended to be called alongside the mid-retry VBUS
> > > > + * power-cycle already performed by hub_port_connect().
> > > > + *
> > > > + * Returns 0 on success, -ENODEV if the port has no ACPI handle or no _PRR
> > > > + * method, or a negative error code on failure.
> > > > + */
> > > > +int usb_acpi_port_prr_reset(struct usb_device *hdev, int port1)
> > > > +{
> > > > +     acpi_handle port_handle;
> > > > +     struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> > > > +     union acpi_object *pkg, *ref;
> > > > +     acpi_status status;
> > > > +     int ret = 0;
> > > > +
> > > > +     port_handle = usb_get_hub_port_acpi_handle(hdev, port1);
> > > > +     if (!port_handle)
> > > > +             return -ENODEV;
> > > > +
> > > > +     if (!acpi_has_method(port_handle, "_PRR"))
> > > > +             return -ENODEV;
> > > > +
> > > > +     status = acpi_evaluate_object(port_handle, "_PRR", NULL, &buffer);
> > > > +     if (ACPI_FAILURE(status)) {
> > > > +             dev_dbg(&hdev->dev, "port%d: _PRR evaluation failed: %s\n",
> > > > +                     port1, acpi_format_exception(status));
> > > > +             return -ENODEV;
> > > > +     }
> > > > +
> > > > +     pkg = buffer.pointer;
> > > > +     if (!pkg || pkg->type != ACPI_TYPE_PACKAGE || pkg->package.count < 1) {
> > > > +             dev_dbg(&hdev->dev, "port%d: _PRR returned unexpected object\n",
> > > > +                     port1);
> > > > +             ret = -EINVAL;
> > > > +             goto out;
> > > > +     }
> > > > +
> > > > +     ref = &pkg->package.elements[0];
> > > > +     if (ref->type != ACPI_TYPE_LOCAL_REFERENCE || !ref->reference.handle) {
> > > > +             dev_dbg(&hdev->dev, "port%d: _PRR element is not a reference\n",
> > > > +                     port1);
> > > > +             ret = -EINVAL;
> > > > +             goto out;
> > > > +     }
> > > > +
> > > > +     status = acpi_evaluate_object(ref->reference.handle, "_RST", NULL, NULL);
> > > > +     if (ACPI_FAILURE(status)) {
> > > > +             dev_dbg(&hdev->dev, "port%d: _RST evaluation failed: %s\n",
> > > > +                     port1, acpi_format_exception(status));
> > > > +             ret = -EIO;
> > > > +     }
> > > > +
> > > > +out:
> > > > +     kfree(buffer.pointer);
> > > > +     return ret;
> > > > +}
> > > > +
> > > >  /**
> > > >   * usb_acpi_add_usb4_devlink - add device link to USB4 Host Interface for tunneled USB3 devices
> > > >   *
> > > > diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
> > > > index a9b37aeb515be..4d3dc3bd881b2 100644
> > > > --- a/drivers/usb/core/usb.h
> > > > +++ b/drivers/usb/core/usb.h
> > > > @@ -211,7 +211,10 @@ extern int usb_acpi_register(void);
> > > >  extern void usb_acpi_unregister(void);
> > > >  extern acpi_handle usb_get_hub_port_acpi_handle(struct usb_device *hdev,
> > > >       int port1);
> > > > +extern int usb_acpi_port_prr_reset(struct usb_device *hdev, int port1);
> > > >  #else
> > > >  static inline int usb_acpi_register(void) { return 0; };
> > > >  static inline void usb_acpi_unregister(void) { };
> > > > +static inline int usb_acpi_port_prr_reset(struct usb_device *hdev,
> > > > +                                       int port1) { return -ENODEV; }
> > > >  #endif
> > > > --
> > > > 2.53.0
> > > >
> > > >
> > >
> > > Please see the review comments at:
> > >         https://sashiko.dev/#/patchset/20260326011708.1128840-1-acelan.kao@canonical.com
> >
> > Thanks Greg.
> >
> > Went through the sashiko review. The single finding claims
> > usb_acpi_port_prr_reset() should iterate every element of the _PRR
> > package and call _RST on each. That's based on a misreading of _PRR.
> >
> > Per ACPI 6.4 §7.3.26 (_PRR, Power Resource for Reset):
> >
> >   "This object evaluates to a single reference to a power resource. ...
> >    Return Value: A single element Package containing a Reference to the
> >    power reset resource."
> >
> > _PRR is a fixed single-element package, not a variable-length list.
> > ACPICA encodes this in acpredef.h as ACPI_PTYPE1_FIXED with 1 reference
> > (as opposed to _PR0/_PR1/_PR2/_PR3, which are ACPI_PTYPE1_VAR reference
> > lists -- I suspect that's the source of the confusion). The "reset must
> > affect all ACPI-described devices that reference it" wording in the _RST
> > description refers to the devices sharing that one power resource, not to
> > multiple resources in the package.
> >
> > The two existing in-tree _PRR/_RST users take elements[0] and call _RST
> > on it exactly as this patch does:
> >
> >   - drivers/bluetooth/btintel.c: btintel_acpi_reset_method()
> >   - drivers/net/.../iwlwifi/pcie/gen1_2/trans.c: iwl_trans_pcie_call_reset()
> >
> > Both are in fact stricter -- they reject package.count != 1, whereas I
> > accept count >= 1 and use the first element. Happy to tighten my check
> > from "< 1" to "!= 1" to match that precedent and make the single-element
> > invariant explicit, but there's no functional issue here.
>
> This is going to require a review from the ACPI maintainer that this is
> the correct thing to be doing, no matter which way it ends up.  So
> please submit a new version and cc: those developers (and tighten up
> your cc: list as well to only those that are relevant.)
v2 is submitted -
https://lore.kernel.org/lkml/20260706080117.3754550-1-acelan.kao@canonical.com/

>
> thanks,
>
> greg k-h

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

end of thread, other threads:[~2026-07-06  8:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-26  1:17 [PATCH] USB: hub: call ACPI _PRR reset during port power-cycle on enumeration failure Chia-Lin Kao (AceLan)
2026-07-03  9:43 ` Greg Kroah-Hartman
2026-07-06  5:04   ` AceLan Kao
2026-07-06  6:37     ` Greg Kroah-Hartman
2026-07-06  8:26       ` AceLan Kao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox