RE: [PATCH 0/2] usb: host: Drop resume calls on {e,o}hci_platform_suspend()

[Biju Das]

Hi Phillipe,

Thanks for the feedback.

> -----Original Message-----
> From: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> Sent: 08 December 2025 08:34
> Subject: Re: [PATCH 0/2] usb: host: Drop resume calls on {e,o}hci_platform_suspend()
> 
> On Mo, 2025-12-08 at 07:50 +0000, Biju Das wrote:
> > Hi Alan Stern,
> >
> > > -----Original Message-----
> > > From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> > > Sent: 07 December 2025 16:36
> > > Subject: Re: [PATCH 0/2] usb: host: Drop resume calls on
> > > {e,o}hci_platform_suspend()
> > >
> > > On Sun, Dec 07, 2025 at 12:47:25PM +0000, Biju wrote:
> > > > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > >
> > > > As per the suspend_devices_and_enter() [1], if .suspend() fails,
> > > > it invoke the .resume() callback.
> > >
> > > Quite wrong.  If .suspend() fails, the core assumes the device is
> > > still at full power.  It does not try to resume the device.
> >
> > But now in ehci/ohci suspend() is calling ehci_resume without checking
> > the status of reset_deassert that can lead to synchronous abort and reboot is the only way to
> recover.
> >
> > For the reset_assert failure cases in suspend(),
> >
> > Case 1) Exclusive reset assert, reset register bit set to assert, but status bit is not set, so we
> get timeout error
> > 	  The following access to ehci registers can lead to synchronous abort.
> 
> Let the reset controller driver set the reset register bit back to deasserted state when returning a
> timeout error after waiting for the status bit to change.

OK, Will do.

Currently the driver set register bit back to asserted state when returning a
timeout error for deassert() [1].


[1] https://elixir.bootlin.com/linux/v6.18-rc7/source/drivers/clk/renesas/rzg2l-cpg.c#L1672

> 
> > Case 2) Array reset assert, reset register bit is set to deassert, but we are not checking the
> status bit
> >         and if the device not transitioned to deassert state, then
> > that can lead to synchronous abort
> 
> The status of array resets can not be checked with the current API.

OK.

> 
> > I guess we should explicirtly call reset_control_deassert(priv->rsts)
> > to make sure the device is in deasserted state before calling ehci_resume().
> >
> > I may be wrong. Please correct me if I am wrong.
> 
> The reset controller driver should leave the reset in a deasserted state when returning an error from
> reset_control_assert().

Agreed.

Cheers,
Biju