[PATCH] usb: ehci: Quiesce ASPEED EHCI before halting it on shutdown

[PATCH] usb: ehci: Quiesce ASPEED EHCI before halting it on shutdown

[Joe Wang]

On ASPEED EHCI controllers, clearing USBCMD.RS while both the periodic
and asynchronous schedules are enabled may leave the controller unable
to halt. If an asynchronous transaction is in progress when RS is
cleared, it may run past the end of the micro-frame. The controller then
clears its internal schedule buffer at the start of the next micro-frame
while it is already walking the periodic schedule, issues a DMA read
from address 0x0 and never sets USBSTS.HCHalted.

ehci_shutdown() calls ehci_silence_controller(), which halts the
controller without first turning off the schedules. For ASPEED
controllers, call ehci_quiesce() first to clear CMD_ASE/CMD_PSE and
wait for STS_ASS/STS_PSS to clear, as ehci_stop() and
ehci_bus_suspend() already do before halting. This is also the stop
sequence recommended by ASPEED.

Quiesce the controller after setting rh_state to EHCI_RH_STOPPING.
The asynchronous and periodic schedule-enable paths require
EHCI_RH_RUNNING, so this ordering prevents them from restarting the
schedules while the controller is being shut down.

Allow ehci_quiesce() to operate in EHCI_RH_STOPPING as well as
EHCI_RH_RUNNING. As documented for enum ehci_rh_state, both states
indicate that the controller may still be performing DMA, while
lower states indicate that it is not.

The controller-death path in ehci_irq() also sets EHCI_RH_STOPPING
and clears CMD_RUN, CMD_ASE and CMD_PSE. A quiesce call in that
state writes the command register with these bits still cleared
and polls the schedule status bits using the existing timeout
limits, without re-enabling the controller or its schedules.

Signed-off-by: Joe Wang <joe_wang@aspeedtech.com>
---
This issue was observed on a customer host platform using the AST2700
PCIe EHCI (ehci-pci), and the root cause was confirmed by the ASPEED
hardware team in RTL simulation. The stop sequence used here (clear
PSE/ASE, wait for PSS/ASS to clear, then clear RS) is their
recommended workaround and was confirmed to resolve the issue on that
platform. All ASPEED EHCI controllers share the same design, so the
workaround applies to every controller with is_aspeed set.

The quiesce is limited to ASPEED controllers to keep the change
minimal for other platforms.

Tested on AST2700 (ehci-platform): ehci_shutdown() and unbind/bind
(ehci_stop()) with a hub and USB mass storage under load, no
regressions.
---
 drivers/usb/host/ehci-hcd.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 3c46bb18c7f3..d984987c8691 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -274,7 +274,7 @@ static void ehci_quiesce (struct ehci_hcd *ehci)
 {
 	u32	temp;
 
-	if (ehci->rh_state != EHCI_RH_RUNNING)
+	if (ehci->rh_state < EHCI_RH_RUNNING)
 		return;
 
 	/* wait for any schedule enables/disables to take effect */
@@ -372,6 +372,14 @@ static void ehci_shutdown(struct usb_hcd *hcd)
 	ehci->enabled_hrtimer_events = 0;
 	spin_unlock_irq(&ehci->lock);
 
+	/*
+	 * ASPEED EHCI may never halt if RS is cleared while both schedules
+	 * are enabled, so turn them off first. rh_state is STOPPING at this
+	 * point, which keeps them from being re-enabled.
+	 */
+	if (ehci->is_aspeed)
+		ehci_quiesce(ehci);
+
 	ehci_silence_controller(ehci);
 
 	hrtimer_cancel(&ehci->hrtimer);

---
base-commit: 2f2f7eeef8b0a1a9b9ca998231419873ccae8d4e
change-id: 20261016-upstream_ehci_aspeed_shutdown-9ccffedb2a1c

Best regards,
-- 
Joe Wang <joe_wang@aspeedtech.com>

Re: [PATCH] usb: ehci: Quiesce ASPEED EHCI before halting it on shutdown

[Alan Stern]

On Fri, Oct 16, 2026 at 04:53:07PM +0800, Joe Wang wrote:
> On ASPEED EHCI controllers, clearing USBCMD.RS while both the periodic
> and asynchronous schedules are enabled may leave the controller unable
> to halt. If an asynchronous transaction is in progress when RS is
> cleared, it may run past the end of the micro-frame. The controller then
> clears its internal schedule buffer at the start of the next micro-frame
> while it is already walking the periodic schedule, issues a DMA read
> from address 0x0 and never sets USBSTS.HCHalted.
> 
> ehci_shutdown() calls ehci_silence_controller(), which halts the
> controller without first turning off the schedules. For ASPEED
> controllers, call ehci_quiesce() first to clear CMD_ASE/CMD_PSE and
> wait for STS_ASS/STS_PSS to clear, as ehci_stop() and
> ehci_bus_suspend() already do before halting. This is also the stop
> sequence recommended by ASPEED.
> 
> Quiesce the controller after setting rh_state to EHCI_RH_STOPPING.
> The asynchronous and periodic schedule-enable paths require
> EHCI_RH_RUNNING, so this ordering prevents them from restarting the
> schedules while the controller is being shut down.
> 
> Allow ehci_quiesce() to operate in EHCI_RH_STOPPING as well as
> EHCI_RH_RUNNING. As documented for enum ehci_rh_state, both states
> indicate that the controller may still be performing DMA, while
> lower states indicate that it is not.
> 
> The controller-death path in ehci_irq() also sets EHCI_RH_STOPPING
> and clears CMD_RUN, CMD_ASE and CMD_PSE. A quiesce call in that
> state writes the command register with these bits still cleared
> and polls the schedule status bits using the existing timeout
> limits, without re-enabling the controller or its schedules.
> 
> Signed-off-by: Joe Wang <joe_wang@aspeedtech.com>
> ---
> This issue was observed on a customer host platform using the AST2700
> PCIe EHCI (ehci-pci), and the root cause was confirmed by the ASPEED
> hardware team in RTL simulation. The stop sequence used here (clear
> PSE/ASE, wait for PSS/ASS to clear, then clear RS) is their
> recommended workaround and was confirmed to resolve the issue on that
> platform. All ASPEED EHCI controllers share the same design, so the
> workaround applies to every controller with is_aspeed set.
> 
> The quiesce is limited to ASPEED controllers to keep the change
> minimal for other platforms.
> 
> Tested on AST2700 (ehci-platform): ehci_shutdown() and unbind/bind
> (ehci_stop()) with a hub and USB mass storage under load, no
> regressions.

Under what circumstances does ehci_shutdown() run without ehci_stop() 
being called first?  Is controller death the only possibility?  If the 
controller is dead, will quiescing work?

Is there any real reason not to do this on all types of EHCI controller, 
not just ASPEED?

> ---
>  drivers/usb/host/ehci-hcd.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 3c46bb18c7f3..d984987c8691 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -274,7 +274,7 @@ static void ehci_quiesce (struct ehci_hcd *ehci)
>  {
>  	u32	temp;
>  
> -	if (ehci->rh_state != EHCI_RH_RUNNING)
> +	if (ehci->rh_state < EHCI_RH_RUNNING)
>  		return;
>  
>  	/* wait for any schedule enables/disables to take effect */
> @@ -372,6 +372,14 @@ static void ehci_shutdown(struct usb_hcd *hcd)
>  	ehci->enabled_hrtimer_events = 0;
>  	spin_unlock_irq(&ehci->lock);
>  
> +	/*
> +	 * ASPEED EHCI may never halt if RS is cleared while both schedules
> +	 * are enabled, so turn them off first. rh_state is STOPPING at this
> +	 * point, which keeps them from being re-enabled.
> +	 */
> +	if (ehci->is_aspeed)
> +		ehci_quiesce(ehci);

Don't you want to check whether the controller has already been 
quiesced?

Alan Stern

> +
>  	ehci_silence_controller(ehci);
>  
>  	hrtimer_cancel(&ehci->hrtimer);
> 
> ---
> base-commit: 2f2f7eeef8b0a1a9b9ca998231419873ccae8d4e
> change-id: 20261016-upstream_ehci_aspeed_shutdown-9ccffedb2a1c
> 
> Best regards,
> -- 
> Joe Wang <joe_wang@aspeedtech.com>
> 

RE: [PATCH] usb: ehci: Quiesce ASPEED EHCI before halting it on shutdown

[Joe Wang]

Hi Alan,

Thanks for the review.
> 
> Under what circumstances does ehci_shutdown() run without ehci_stop()
> being called first?  Is controller death the only possibility? 

No, it's the normal case: ehci_shutdown() runs from the .shutdown
callback during reboot, poweroff and kexec, where the HCD is not
removed, so ehci_stop() is never called and the controller is usually
still running with devices attached. The issue was seen on a host
using LinuxBoot (NERF) firmware, which boots the final OS with kexec.

Controller death is mentioned in the changelog only because relaxing
the check in ehci_quiesce() also affects that path.

> If the controller is dead, will quiescing work?

It won't help, but it's harmless. The death path has already cleared
CMD_RUN, CMD_ASE and CMD_PSE, so ehci_quiesce() writes those bits back
as zero and the handshakes either succeed at once or time out after
16 uframes. ehci_shutdown() already touches the registers through
ehci_halt() in that case anyway.

> Is there any real reason not to do this on all types of EHCI controller, not
> just ASPEED?

No EHCI technical reason that I know of. I limited it to ASPEED because
that's what the report and my testing cover. Since ehci_stop()
already quiesces unconditionally, doing it for all controllers should
be safe.

> Don't you want to check whether the controller has already been quiesced?

The quiesce has to come after rh_state is set to EHCI_RH_STOPPING;
otherwise a URB submission or hrtimer event could re-enable the
schedules through ehci_poll_ASS()/ehci_poll_PSS() before the halt.
That's why v1 relaxes the check in ehci_quiesce(). The side effect is
that a controller that was already suspended or halted gets quiesced
again, which is harmless but unnecessary.

I see two ways to avoid the extra ehci_quiesce() :

(a) Keep the relaxed check, and in ehci_shutdown() record under
   ehci->lock whether rh_state was EHCI_RH_RUNNING before setting
   EHCI_RH_STOPPING, quiescing only in that case. This is the
   smaller change.

(b) Same as (a), but leave ehci_quiesce() unchanged and have
   ehci_shutdown() call a separate helper for the schedule
   quiescing part. Other callers of ehci_quiesce() are then not
   affected at all.

Either can be combined with dropping the `is_aspeed` check. Which would
you prefer?

Thanks,
Joe

Re: [PATCH] usb: ehci: Quiesce ASPEED EHCI before halting it on shutdown

[Alan Stern]

On Tue, Oct 20, 2026 at 10:42:55AM +0000, Joe Wang wrote:
> Hi Alan,
> 
> Thanks for the review.
> > 
> > Under what circumstances does ehci_shutdown() run without ehci_stop()
> > being called first?  Is controller death the only possibility? 
> 
> No, it's the normal case: ehci_shutdown() runs from the .shutdown
> callback during reboot, poweroff and kexec, where the HCD is not
> removed, so ehci_stop() is never called and the controller is usually
> still running with devices attached. The issue was seen on a host
> using LinuxBoot (NERF) firmware, which boots the final OS with kexec.

Okay, so this will slow down a normal reboot or poweroff.  Hopefully not 
by very much, if everything is working properly.

> Controller death is mentioned in the changelog only because relaxing
> the check in ehci_quiesce() also affects that path.
> 
> > If the controller is dead, will quiescing work?
> 
> It won't help, but it's harmless. The death path has already cleared
> CMD_RUN, CMD_ASE and CMD_PSE, so ehci_quiesce() writes those bits back
> as zero and the handshakes either succeed at once or time out after
> 16 uframes. ehci_shutdown() already touches the registers through
> ehci_halt() in that case anyway.
> 
> > Is there any real reason not to do this on all types of EHCI controller, not
> > just ASPEED?
> 
> No EHCI technical reason that I know of. I limited it to ASPEED because
> that's what the report and my testing cover. Since ehci_stop()
> already quiesces unconditionally, doing it for all controllers should
> be safe.

I think it makes sense for the change to apply to all EHCI types.  It's 
not really a good idea to leave an independent DMA master running while 
the system is shutting down.

> > Don't you want to check whether the controller has already been quiesced?
> 
> The quiesce has to come after rh_state is set to EHCI_RH_STOPPING;
> otherwise a URB submission or hrtimer event could re-enable the
> schedules through ehci_poll_ASS()/ehci_poll_PSS() before the halt.
> That's why v1 relaxes the check in ehci_quiesce(). The side effect is
> that a controller that was already suspended or halted gets quiesced
> again, which is harmless but unnecessary.
> 
> I see two ways to avoid the extra ehci_quiesce() :
> 
> (a) Keep the relaxed check, and in ehci_shutdown() record under
>    ehci->lock whether rh_state was EHCI_RH_RUNNING before setting
>    EHCI_RH_STOPPING, quiescing only in that case. This is the
>    smaller change.
> 
> (b) Same as (a), but leave ehci_quiesce() unchanged and have
>    ehci_shutdown() call a separate helper for the schedule
>    quiescing part. Other callers of ehci_quiesce() are then not
>    affected at all.
> 
> Either can be combined with dropping the `is_aspeed` check. Which would
> you prefer?

(a) is more reasonable.

Alan Stern