Re: [PATCH v3 6/9] phy: renesas: phy-rcar-gen3-usb2: Add RZ/G3L support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Sleep-in-atomic bug due to calling regulator_hardware_enable() inside a spinlock and from a hardirq handler.
--

--- Patch [6]: [PATCH v3 6/9] phy: renesas: phy-rcar-gen3-usb2: Add RZ/G3L support ---
commit 502c90feba61fdb92ccdaff21a3e226e6113ab48
Author: Biju Das <biju.das.jz@bp.renesas.com>

phy: renesas: phy-rcar-gen3-usb2: Add RZ/G3L support

This commit adds support for the RZ/G3L SoC to the phy-rcar-gen3-usb2 driver
by adding the "renesas,usb2-phy-r9a08g046" compatible string. The PHY data
configuration is shared with RZ/G3S.

> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> index 9a45d840efeb2..d06fb52ed5f1b 100644
> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> @@ -721,6 +721,10 @@ static const struct of_device_id rcar_gen3_phy_usb2_match_table[] = {
>  		.compatible = "renesas,usb2-phy-r9a08g045",
>  		.data = &rz_g3s_phy_usb2_data,
>  	},
> +	{
> +		.compatible = "renesas,usb2-phy-r9a08g046",
> +		.data = &rz_g3s_phy_usb2_data,
> +	},

[Severity: High]
This is a pre-existing issue, but since this patch exposes this hardware to 
existing code paths, I wanted to ask about a potential sleep-in-atomic bug.

When a user writes to the role sysfs attribute, role_store() acquires a
spinlock and disables interrupts:

drivers/phy/renesas/phy-rcar-gen3-usb2.c:role_store() {
    guard(spinlock_irqsave)(&ch->lock);
    ...
    if (!is_b_device)    /* A-Peripheral */
        rcar_gen3_init_from_a_peri_to_a_host(ch);
}

This calls down to rcar_gen3_enable_vbus_ctrl(), which then triggers the
regulator operations:

drivers/phy/renesas/phy-rcar-gen3-usb2.c:rcar_gen3_enable_vbus_ctrl() {
    ...
    if (ch->otg_internal_reg) {
        regulator_hardware_enable(ch->vbus, vbus);
        return;
    }

    if (ch->phy_data->no_adp_ctrl || ch->phy_data->vblvl_ctrl) {
        if (ch->vbus)
            regulator_hardware_enable(ch->vbus, vbus);
    ...
}

For SoCs using the internal regulator (such as RZ/G3L), the enable callback
invokes pm_runtime_resume_and_get():

drivers/phy/renesas/phy-rcar-gen3-usb2.c:rcar_gen3_phy_usb2_regulator_endisable() {
    ...
    ret = pm_runtime_resume_and_get(dev);
    ...
}

Can this sequence cause a panic? 

If the device state is RPM_SUSPENDING, pm_runtime_resume_and_get() might sleep
waiting for the asynchronous suspend to finish, but we are inside an atomic
section holding ch->lock with interrupts disabled.

Similarly, does the rcar_gen3_phy_usb2_irq() hardirq handler also reach
this regulator_hardware_enable() path on OTG plug/unplug events? If so,
would it hit the same sleep-in-atomic issue?

>  	{
>  		.compatible = "renesas,usb2-phy-r9a09g057",
>  		.data = &rz_v2h_phy_usb2_data,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707143646.245177-1-biju.das.jz@bp.renesas.com?part=6

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy