[PATCH] usb: renesas_usbhs: Fix synchronous external abort on unbind

Claudiu posted 1 patch 3 months, 2 weeks ago
There is a newer version of this series
drivers/usb/renesas_usbhs/common.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
[PATCH] usb: renesas_usbhs: Fix synchronous external abort on unbind
Posted by Claudiu 3 months, 2 weeks ago
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

A synchronous external abort occurs on the Renesas RZ/G3S SoC if unbind is
executed after the configuration sequence described above:

modprobe usb_f_ecm
modprobe libcomposite
modprobe configfs
cd /sys/kernel/config/usb_gadget
mkdir -p g1
cd g1
echo "0x1d6b" > idVendor
echo "0x0104" > idProduct
mkdir -p strings/0x409
echo "0123456789" > strings/0x409/serialnumber
echo "Renesas." > strings/0x409/manufacturer
echo "Ethernet Gadget" > strings/0x409/product
mkdir -p functions/ecm.usb0
mkdir -p configs/c.1
mkdir -p configs/c.1/strings/0x409
echo "ECM" > configs/c.1/strings/0x409/configuration

if [ ! -L configs/c.1/ecm.usb0 ]; then
        ln -s functions/ecm.usb0 configs/c.1
fi

echo 11e20000.usb > UDC
echo 11e20000.usb > /sys/bus/platform/drivers/renesas_usbhs/unbind

The displayed trace is as follows:

 Internal error: synchronous external abort: 0000000096000010 [#1] SMP
 CPU: 0 UID: 0 PID: 188 Comm: sh Tainted: G M 6.17.0-rc7-next-20250922-00010-g41050493b2bd #55 PREEMPT
 Tainted: [M]=MACHINE_CHECK
 Hardware name: Renesas SMARC EVK version 2 based on r9a08g045s33 (DT)
 pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
 pc : usbhs_sys_function_pullup+0x10/0x40 [renesas_usbhs]
 lr : usbhsg_update_pullup+0x3c/0x68 [renesas_usbhs]
 sp : ffff8000838b3920
 x29: ffff8000838b3920 x28: ffff00000d585780 x27: 0000000000000000
 x26: 0000000000000000 x25: 0000000000000000 x24: ffff00000c3e3810
 x23: ffff00000d5e5c80 x22: ffff00000d5e5d40 x21: 0000000000000000
 x20: 0000000000000000 x19: ffff00000d5e5c80 x18: 0000000000000020
 x17: 2e30303230316531 x16: 312d7968703a7968 x15: 3d454d414e5f4344
 x14: 000000000000002c x13: 0000000000000000 x12: 0000000000000000
 x11: ffff00000f358f38 x10: ffff00000f358db0 x9 : ffff00000b41f418
 x8 : 0101010101010101 x7 : 7f7f7f7f7f7f7f7f x6 : fefefeff6364626d
 x5 : 8080808000000000 x4 : 000000004b5ccb9d x3 : 0000000000000000
 x2 : 0000000000000000 x1 : ffff800083790000 x0 : ffff00000d5e5c80
 Call trace:
 usbhs_sys_function_pullup+0x10/0x40 [renesas_usbhs] (P)
 usbhsg_pullup+0x4c/0x7c [renesas_usbhs]
 usb_gadget_disconnect_locked+0x48/0xd4
 gadget_unbind_driver+0x44/0x114
 device_remove+0x4c/0x80
 device_release_driver_internal+0x1c8/0x224
 device_release_driver+0x18/0x24
 bus_remove_device+0xcc/0x10c
 device_del+0x14c/0x404
 usb_del_gadget+0x88/0xc0
 usb_del_gadget_udc+0x18/0x30
 usbhs_mod_gadget_remove+0x24/0x44 [renesas_usbhs]
 usbhs_mod_remove+0x20/0x30 [renesas_usbhs]
 usbhs_remove+0x98/0xdc [renesas_usbhs]
 platform_remove+0x20/0x30
 device_remove+0x4c/0x80
 device_release_driver_internal+0x1c8/0x224
 device_driver_detach+0x18/0x24
 unbind_store+0xb4/0xb8
 drv_attr_store+0x24/0x38
 sysfs_kf_write+0x7c/0x94
 kernfs_fop_write_iter+0x128/0x1b8
 vfs_write+0x2ac/0x350
 ksys_write+0x68/0xfc
 __arm64_sys_write+0x1c/0x28
 invoke_syscall+0x48/0x110
 el0_svc_common.constprop.0+0xc0/0xe0
 do_el0_svc+0x1c/0x28
 el0_svc+0x34/0xf0
 el0t_64_sync_handler+0xa0/0xe4
 el0t_64_sync+0x198/0x19c
 Code: 7100003f 1a9f07e1 531c6c22 f9400001 (79400021)
 ---[ end trace 0000000000000000 ]---
 note: sh[188] exited with irqs disabled
 note: sh[188] exited with preempt_count 1

The issue occurs because usbhs_sys_function_pullup(), which accesses the IP
registers, is executed after the USBHS clocks have been disabled. The
problem is reproducible on the Renesas RZ/G3S SoC starting with the
addition of module stop in the clock enable/disable APIs. With module stop
functionality enabled, a bus error is expected if a master accesses a
module whose clock has been stopped and module stop activated.

Disable the IP clocks at the end of remove.

Cc: stable@vger.kernel.org
Fixes: f1407d5c6624 ("usb: renesas_usbhs: Add Renesas USBHS common code")
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Patch was tested with continuous unbind/bind and the configuration
sequence described above on the devices with the following device trees:

- r8a774a1-hihope-rzg2m-ex.dts
- r8a774b1-hihope-rzg2n-ex.dts
- r8a774e1-hihope-rzg2h-ex.dts
- r9a07g043u11-smarc.dts
- r9a07g044c2-smarc.dts
- r9a07g044l2-smarc.dts
- r9a07g054l2-smarc.dts

 drivers/usb/renesas_usbhs/common.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c
index 8f536f2c500f..b8b55d08ac52 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -813,18 +813,18 @@ 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);
-
-	pm_runtime_disable(&pdev->dev);
-
 	usbhs_platform_call(priv, hardware_exit, pdev);
 	usbhsc_clk_put(priv);
 	reset_control_assert(priv->rsts);
 	usbhs_mod_remove(priv);
 	usbhs_fifo_remove(priv);
 	usbhs_pipe_remove(priv);
+
+	/* power off */
+	if (!usbhs_get_dparam(priv, runtime_pwctrl))
+		usbhsc_power_ctrl(priv, 0);
+
+	pm_runtime_disable(&pdev->dev);
 }
 
 static int usbhsc_suspend(struct device *dev)
-- 
2.43.0
Re: [PATCH] usb: renesas_usbhs: Fix synchronous external abort on unbind
Posted by Geert Uytterhoeven 3 months, 2 weeks ago
Hi Claudiu,

On Wed, 22 Oct 2025 at 15:06, Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> A synchronous external abort occurs on the Renesas RZ/G3S SoC if unbind is
> executed after the configuration sequence described above:

[...]

> The issue occurs because usbhs_sys_function_pullup(), which accesses the IP
> registers, is executed after the USBHS clocks have been disabled. The
> problem is reproducible on the Renesas RZ/G3S SoC starting with the
> addition of module stop in the clock enable/disable APIs. With module stop
> functionality enabled, a bus error is expected if a master accesses a
> module whose clock has been stopped and module stop activated.
>
> Disable the IP clocks at the end of remove.
>
> Cc: stable@vger.kernel.org
> Fixes: f1407d5c6624 ("usb: renesas_usbhs: Add Renesas USBHS common code")
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/usb/renesas_usbhs/common.c
> +++ b/drivers/usb/renesas_usbhs/common.c
> @@ -813,18 +813,18 @@ 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);
> -
> -       pm_runtime_disable(&pdev->dev);
> -
>         usbhs_platform_call(priv, hardware_exit, pdev);
>         usbhsc_clk_put(priv);

Shouldn't the usbhsc_clk_put() call be moved just before the
pm_runtime_disable() call, too, cfr. the error path in usbhs_probe()?

>         reset_control_assert(priv->rsts);
>         usbhs_mod_remove(priv);
>         usbhs_fifo_remove(priv);
>         usbhs_pipe_remove(priv);
> +
> +       /* power off */
> +       if (!usbhs_get_dparam(priv, runtime_pwctrl))
> +               usbhsc_power_ctrl(priv, 0);
> +
> +       pm_runtime_disable(&pdev->dev);
>  }
>
>  static int usbhsc_suspend(struct device *dev)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH] usb: renesas_usbhs: Fix synchronous external abort on unbind
Posted by Claudiu Beznea 3 months, 2 weeks ago
Hi, Geert,

On 10/23/25 11:07, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Wed, 22 Oct 2025 at 15:06, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> A synchronous external abort occurs on the Renesas RZ/G3S SoC if unbind is
>> executed after the configuration sequence described above:
> 
> [...]
> 
>> The issue occurs because usbhs_sys_function_pullup(), which accesses the IP
>> registers, is executed after the USBHS clocks have been disabled. The
>> problem is reproducible on the Renesas RZ/G3S SoC starting with the
>> addition of module stop in the clock enable/disable APIs. With module stop
>> functionality enabled, a bus error is expected if a master accesses a
>> module whose clock has been stopped and module stop activated.
>>
>> Disable the IP clocks at the end of remove.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: f1407d5c6624 ("usb: renesas_usbhs: Add Renesas USBHS common code")
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Thanks for your patch!
> 
>> --- a/drivers/usb/renesas_usbhs/common.c
>> +++ b/drivers/usb/renesas_usbhs/common.c
>> @@ -813,18 +813,18 @@ 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);
>> -
>> -       pm_runtime_disable(&pdev->dev);
>> -
>>         usbhs_platform_call(priv, hardware_exit, pdev);
>>         usbhsc_clk_put(priv);
> 
> Shouldn't the usbhsc_clk_put() call be moved just before the
> pm_runtime_disable() call, too, cfr. the error path in usbhs_probe()?

You're right! I missed it. Thank you for pointing it.

Claudiu