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

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

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

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@bp.renesas.com>
---
 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);
+
 	usbhs_platform_call(priv, hardware_exit, pdev);
 	reset_control_assert(priv->rsts);
 
@@ -828,10 +832,6 @@ static void usbhs_remove(struct platform_device *pdev)
 	usbhs_fifo_remove(priv);
 	usbhs_pipe_remove(priv);
 
-	/* power off */
-	if (!usbhs_get_dparam(priv, runtime_pwctrl))
-		usbhsc_power_ctrl(priv, 0);
-
 	usbhsc_clk_put(priv);
 	pm_runtime_disable(&pdev->dev);
 }
-- 
2.43.0

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

[Claudiu Beznea]

Hi, Biju,

On 6/15/26 20:39, Biju wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> 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@bp.renesas.com>
> ---
>   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);
+
         usbhsc_clk_put(priv);
         pm_runtime_disable(&pdev->dev);
  }

Thank you,
Claudiu

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@tuxon.dev>
> 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@bp.renesas.com>
> >
> > 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@bp.renesas.com>
> > ---
> >   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