RE: [PATCH] usb: renesas_usbhs: Fix power-off ordering on unbind

[Biju Das]

Hi Claudiu,

Thanks for testing.

> -----Original Message-----
> From: Claudiu Beznea <claudiu.beznea@xxxxxxxxx>
> Sent: 16 June 2026 09:58
> Subject: Re: [PATCH] usb: renesas_usbhs: Fix power-off ordering on unbind
> 
> Hi, Biju,
> 
> On 6/15/26 20:39, Biju wrote:
> > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> >
> > Move the usbhsc_power_ctrl() call to before usbhs_*_hardware_exit(),
> > so that usbhs_*_hardware_exit() sets priv->phy to NULL only after
> > usbhsc_power_ctrl() has executed, which controls the PHY power.
> >
> > Fixes: eb9ac779830b ("usb: renesas_usbhs: Fix synchronous external
> > abort on unbind")
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> >   drivers/usb/renesas_usbhs/common.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/renesas_usbhs/common.c
> > b/drivers/usb/renesas_usbhs/common.c
> > index 8c93bde4b816..614b724a0e52 100644
> > --- a/drivers/usb/renesas_usbhs/common.c
> > +++ b/drivers/usb/renesas_usbhs/common.c
> > @@ -813,6 +813,10 @@ static void usbhs_remove(struct platform_device
> > *pdev)
> >
> >   	flush_delayed_work(&priv->notify_hotplug_work);
> >
> > +	/* power off */
> > +	if (!usbhs_get_dparam(priv, runtime_pwctrl))
> > +		usbhsc_power_ctrl(priv, 0);
> > +
> 
> Moving this back here will lead to the issue described in commit eb9ac779830b
> ("usb: renesas_usbhs: Fix synchronous external abort on unbind") being reproducible again. I've checked
> it on RZ/G2L.
> 
> Instead, the below diff fixes both the USB PHY regulator WARN_ON() stack trace and still keeps away the
> crash fixed by eb9ac779830b ("usb: renesas_usbhs: Fix synchronous external abort on unbind").
> 
> diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c
> index 8c93bde4b816..843468d42786 100644
> --- a/drivers/usb/renesas_usbhs/common.c
> +++ b/drivers/usb/renesas_usbhs/common.c
> @@ -813,7 +813,6 @@ static void usbhs_remove(struct platform_device *pdev)
> 
>          flush_delayed_work(&priv->notify_hotplug_work);
> 
> -       usbhs_platform_call(priv, hardware_exit, pdev);
>          reset_control_assert(priv->rsts);
> 
>          /*
> @@ -832,6 +831,8 @@ static void usbhs_remove(struct platform_device *pdev)
>          if (!usbhs_get_dparam(priv, runtime_pwctrl))
>                  usbhsc_power_ctrl(priv, 0);
> 
> +       usbhs_platform_call(priv, hardware_exit, pdev);
> +
>          usbhsc_clk_put(priv);
>          pm_runtime_disable(&pdev->dev);
>   }
> 
> Along with it, the reset_control_assert() could also be moved as follows to avoid accessing registers IPs
> with the reset line asserted. The following was also tested on RZ/G2L:
> 
> diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c
> index 8c93bde4b816..51d3035f82be 100644
> --- a/drivers/usb/renesas_usbhs/common.c
> +++ b/drivers/usb/renesas_usbhs/common.c
> @@ -813,9 +813,6 @@ static void usbhs_remove(struct platform_device *pdev)
> 
>          flush_delayed_work(&priv->notify_hotplug_work);
> 
> -       usbhs_platform_call(priv, hardware_exit, pdev);
> -       reset_control_assert(priv->rsts);
> -
>          /*
>           * Explicitly free the IRQ to ensure the interrupt handler is
>           * disabled and synchronized before freeing resources.
> @@ -832,6 +829,9 @@ static void usbhs_remove(struct platform_device *pdev)
>          if (!usbhs_get_dparam(priv, runtime_pwctrl))
>                  usbhsc_power_ctrl(priv, 0);
> 
> +       usbhs_platform_call(priv, hardware_exit, pdev);
> +       reset_control_assert(priv->rsts);
> +

OK will move the code like this in next version.

Cheers,
Biju