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@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);
+
        usbhsc_clk_put(priv);
        pm_runtime_disable(&pdev->dev);
 }

Thank you,
Claudiu