Re: [PATCH RESEND v3 3/5] phy: renesas: rcar-gen3-usb2: Lock around hardware registers and driver data

[Xia Fukun]

On 5/7/2025 8:50 PM, Claudiu wrote:

> @@ -348,6 +349,8 @@ static ssize_t role_store(struct device *dev, struct device_attribute *attr,
>  	bool is_b_device;
>  	enum phy_mode cur_mode, new_mode;
>  
> +	guard(spinlock_irqsave)(&ch->lock);
> +
>  	if (!ch->is_otg_channel || !rcar_gen3_is_any_otg_rphy_initialized(ch))
>  		return -EIO;
>  
> @@ -415,7 +418,7 @@ static void rcar_gen3_init_otg(struct rcar_gen3_chan *ch)
>  		val = readl(usb2_base + USB2_ADPCTRL);
>  		writel(val | USB2_ADPCTRL_IDPULLUP, usb2_base + USB2_ADPCTRL);
>  	}
> -	msleep(20);
> +	mdelay(20);
>  
>  	writel(0xffffffff, usb2_base + USB2_OBINTSTA);
>  	writel(ch->obint_enable_bits, usb2_base + USB2_OBINTEN);

The fix for the rcar_gen3_init_otg function has not been backported to
the 5.10-stable branch, which leads to a "scheduling while atomic" bug
in that branch:

rcar_gen3_phy_usb2_init()                    ← phy_init() callback
  → guard(spinlock_irqsave)(&ch->lock)       ← acquires spinlock, disabling preemption + IRQs
  → rcar_gen3_init_otg(ch)
      → msleep(20)                           ← BUG: scheduling while atomic!

The patch for 5.10 is available at the following link:
https://web.git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.10.y&id=0f86a559900fbe96af08a3a226a86fac27b02b8f

Perhaps the stable patch for the 5.10 branch needs to be updated to
fix this serious bug?

> @@ -456,6 +461,8 @@ static int rcar_gen3_phy_usb2_init(struct phy *p)
>  	void __iomem *usb2_base = channel->base;
>  	u32 val;
>  
> +	guard(spinlock_irqsave)(&channel->lock);
> +
>  	/* Initialize USB2 part */
>  	val = readl(usb2_base + USB2_INT_ENABLE);
>  	val |= USB2_INT_ENABLE_UCOM_INTEN | rphy->int_enable_bits;
> @@ -479,6 +486,8 @@ static int rcar_gen3_phy_usb2_exit(struct phy *p)
>  	void __iomem *usb2_base = channel->base;
>  	u32 val;
>  
> +	guard(spinlock_irqsave)(&channel->lock);
> +
>  	rphy->initialized = false;
>  
>  	val = readl(usb2_base + USB2_INT_ENABLE);