From nobody Mon Jun 8 16:30:19 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 813A233123F; Thu, 28 May 2026 07:08:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779952115; cv=none; b=X6jjv0roYKetX+nyQi6NkzxDTPTn4UYvLEutZF8XO4UvPS0gRv+V3gzy6SUdMCmPW6MY14OggYlg6v+22hP47/CgKdTaQBD3ki3DWgrUh0XscC6adHVUCxxVAPlwwew+8j79IZNDl+k3iByGGsS8X9k88Voi3+qLS5hoNnFMNmg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779952115; c=relaxed/simple; bh=CIGhuM+3rUFwEK0N3+Cy3IFTwiBE4qQODvtnIhIIIwY=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=WP7ncutxymGMsd5XO2hLWoH+0SPQYSTtBbOfwZigZhWRzSVXYA+ZewLgcKL/LUFd/zw7hWhL+4BERAsKMF1HCh0rFnxQMwtU0tzGd+CfRIjGjHEY3bh9Nih2Qh050liJWZHVVndH/Wid5FgKmRgAFLouLMJaNs3G4pfTYNn4HOU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=HipAD32P; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="HipAD32P" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0529D1F000E9; Thu, 28 May 2026 07:08:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1779952113; bh=xZcxL5fUeXKaziqhPPhIhoJJPfCSwi6XWBCXnJGQMQo=; h=From:To:Cc:Subject:Date; b=HipAD32Pcx++g0cu9n2D85LKG1FUfPkABz5m3BdOHMwxDbzQHpjDPKQCVtI3GE1B8 6LNkiS3hcYDwgB8onG5KphVY82rbmb7lw8s2YTfhRiLiYG1W3CIcFP6IsDNBe8+jac dRLDCEB04vcvaSMkpi5wDI5/LELQ6s6McvkDvfgnEupQTdDd2tMFwIrpgh4GbWYPUQ k2+KJO7+op6SK/3GGARKg+YvMdmCLbG/c/4wu5qAFvtSF3cygOl0Dfwl0V4La6y1bQ QDvGasFE3aOSKeDqbVc/nQFBVsTMSpg4qUgH6HRe/7pUZVBpb0fo1zc19+UK5Y3J4o ToeNc7f8ywQJg== From: Claudiu Beznea To: yoshihiro.shimoda.uh@renesas.com, vkoul@kernel.org, neil.armstrong@linaro.org, geert+renesas@glider.be, magnus.damm@gmail.com, prabhakar.mahadev-lad.rj@bp.renesas.com Cc: claudiu.beznea@kernel.org, claudiu.beznea@tuxon.dev, linux-renesas-soc@vger.kernel.org, linux-phy@lists.infradead.org, linux-kernel@vger.kernel.org, Claudiu Beznea , stable@vger.kernel.org, Pavel Machek , Nobuhiro Iwamatsu Subject: [PATCH v3] phy: renesas: rcar-gen3-usb2: Avoid long delay in atomic context Date: Thu, 28 May 2026 10:08:26 +0300 Message-ID: <20260528070826.478813-1-claudiu.beznea@kernel.org> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Claudiu Beznea The OTG PHY initialization sequence needs to wait for 20 ms at a specific step, as described in commit 72c0339c115b ("phy: renesas: rcar-gen3-usb2: follow the hardware manual procedure"). Commit 55a387ebb921 ("phy: renesas: rcar-gen3-usb2: Lock around hardware registers and driver data") tried to address various problems in the rcar-gen3-usb2 driver and converted the mutex protecting HW register accesses to a spin lock, leaving, however, a long delay in the critical section protected by the spin lock. This may become a problem, especially on RT kernels. To address this, release the spin lock before sleeping for 20 ms as required by the HW manual and reacquire it afterwards. To avoid other threads entering the critical section and configuring the HW while the software is waiting for the OTG initialization to complete, introduce the otg_initializing variable alongside the otg_init_done completion. Any other thread trying to configure the HW while the OTG PHY initialization is in progress waits for the completion instead of immediately returning errors to PHY users. The IRQs were also disabled while waiting for the OTG PHY initialization to complete, as the interrupt handler may also apply HW settings. Fixes: 55a387ebb921 ("phy: renesas: rcar-gen3-usb2: Lock around hardware re= gisters and driver data") Cc: stable@vger.kernel.org Reported-by: Pavel Machek Closes: https://lore.kernel.org/all/afhkX2Ys2BG1gnqy@duo.ucw.cz Reported-by: Nobuhiro Iwamatsu Closes: https://lore.kernel.org/all/afhkX2Ys2BG1gnqy@duo.ucw.cz Signed-off-by: Claudiu Beznea --- Changes in v3: - initialize ret value in role_store() - jump to exit label in role_store() to have the same exit path - dropped scoped_guard() in rcar_gen3_phy_usb2_irq() to avoid mixing goto statements with scoped_guard() - take into account USB2_INT_ENABLE_UCOM_INTEN when masking/unmasking IRQs - increase the completion timeout to 40ms - in rcar_gen3_phy_usb2_power_on() disable regulator on failure path Changes in v2: - dropped the rcar_gen3_phy_wait_otg_init() and used inline the code; with = it the comment that was previously in rcar_gen3_phy_wait_otg_init() was added in struct rcar_gen3_chan for above the org_initializing variable - dropped READ_ONCE()/WRITE_ONCE() arround struct rcar_gen3_chan::otg_initi= alizing - checked for struct rcar_gen3_chan::otg_initializing before checking for rcar_gen3_is_any_otg_rphy_initialized() in all code places - added rcar_gen3_phy_usb2_irqs_mask_all() and rcar_gen3_phy_usb2_irqs_unma= sk() to mask/unmask only the interrupts at the controller level since the line= is shared - along with it dropped disable_irq_nosync()/enable_irq() from v1 along with struct rcar_gen3_chan::irq - increased the completion timeout to 30 ms drivers/phy/renesas/phy-rcar-gen3-usb2.c | 244 +++++++++++++++++++---- 1 file changed, 203 insertions(+), 41 deletions(-) diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas= /phy-rcar-gen3-usb2.c index 9a45d840efeb..2231b0475fdd 100644 --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c @@ -138,12 +138,20 @@ struct rcar_gen3_chan { struct rcar_gen3_phy rphys[NUM_OF_PHYS]; struct regulator *vbus; struct work_struct work; + struct completion otg_init_done; spinlock_t lock; /* protects access to hardware and driver data structure= . */ enum usb_dr_mode dr_mode; bool extcon_host; bool is_otg_channel; bool uses_otg_pins; bool otg_internal_reg; + /* + * The OTG can be initialized only once and needs to release the spinlock + * and wait for 20 ms due to hardware constraints. If a thread executes + * PHY configuration code while the OTG PHY is waiting for the 20 ms, the + * thread will have to wait for the OTG PHY initialization to complete. + */ + bool otg_initializing; }; =20 struct rcar_gen3_phy_drv_data { @@ -392,26 +400,52 @@ static ssize_t role_store(struct device *dev, struct = device_attribute *attr, struct rcar_gen3_chan *ch =3D dev_get_drvdata(dev); bool is_b_device; enum phy_mode cur_mode, new_mode; + unsigned long flags; + int ret =3D -EIO; =20 - guard(spinlock_irqsave)(&ch->lock); + spin_lock_irqsave(&ch->lock, flags); =20 - if (!ch->is_otg_channel || !rcar_gen3_is_any_otg_rphy_initialized(ch)) - return -EIO; + if (!ch->is_otg_channel) + goto unlock; + + if (ch->otg_initializing) { + unsigned long timeout =3D msecs_to_jiffies(40); + + spin_unlock_irqrestore(&ch->lock, flags); =20 - if (sysfs_streq(buf, "host")) + ret =3D wait_for_completion_timeout(&ch->otg_init_done, timeout); + ret =3D ret ? 0 : -ETIMEDOUT; + if (ret) + goto exit; + + spin_lock_irqsave(&ch->lock, flags); + } + + if (!rcar_gen3_is_any_otg_rphy_initialized(ch)) { + ret =3D -EIO; + goto unlock; + } + + if (sysfs_streq(buf, "host")) { new_mode =3D PHY_MODE_USB_HOST; - else if (sysfs_streq(buf, "peripheral")) + } else if (sysfs_streq(buf, "peripheral")) { new_mode =3D PHY_MODE_USB_DEVICE; - else - return -EINVAL; + } else { + ret =3D -EINVAL; + goto unlock; + } =20 /* is_b_device: true is B-Device. false is A-Device. */ is_b_device =3D rcar_gen3_check_id(ch); cur_mode =3D rcar_gen3_get_phy_mode(ch); =20 /* If current and new mode is the same, this returns the error */ - if (cur_mode =3D=3D new_mode) - return -EINVAL; + if (cur_mode =3D=3D new_mode) { + ret =3D -EINVAL; + goto unlock; + } + + ret =3D 0; =20 if (new_mode =3D=3D PHY_MODE_USB_HOST) { /* And is_host must be false */ if (!is_b_device) /* A-Peripheral */ @@ -425,7 +459,10 @@ static ssize_t role_store(struct device *dev, struct d= evice_attribute *attr, rcar_gen3_init_for_peri(ch); } =20 - return count; +unlock: + spin_unlock_irqrestore(&ch->lock, flags); +exit: + return ret ?: count; } =20 static ssize_t role_show(struct device *dev, struct device_attribute *attr, @@ -441,14 +478,11 @@ static ssize_t role_show(struct device *dev, struct d= evice_attribute *attr, } static DEVICE_ATTR_RW(role); =20 -static void rcar_gen3_init_otg(struct rcar_gen3_chan *ch) +static void rcar_gen3_init_otg_phase0(struct rcar_gen3_chan *ch) { void __iomem *usb2_base =3D ch->base; u32 val; =20 - if (!ch->is_otg_channel || rcar_gen3_is_any_otg_rphy_initialized(ch)) - return; - /* Should not use functions of read-modify-write a register */ val =3D readl(usb2_base + USB2_LINECTRL1); val =3D (val & ~USB2_LINECTRL1_DP_RPD) | USB2_LINECTRL1_DPRPD_EN | @@ -471,7 +505,11 @@ static void rcar_gen3_init_otg(struct rcar_gen3_chan *= ch) writel(val | USB2_ADPCTRL_IDPULLUP, usb2_base + USB2_ADPCTRL); } } - mdelay(20); +} + +static void rcar_gen3_init_otg_phase1(struct rcar_gen3_chan *ch) +{ + void __iomem *usb2_base =3D ch->base; =20 writel(0xffffffff, usb2_base + USB2_OBINTSTA); writel(ch->phy_data->obint_enable_bits, usb2_base + USB2_OBINTEN); @@ -502,6 +540,7 @@ static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, void= *_ch) void __iomem *usb2_base =3D ch->base; struct device *dev =3D ch->dev; irqreturn_t ret =3D IRQ_NONE; + unsigned long flags; u32 status; =20 pm_runtime_get_noresume(dev); @@ -509,33 +548,85 @@ static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, vo= id *_ch) if (pm_runtime_suspended(dev)) goto rpm_put; =20 - scoped_guard(spinlock, &ch->lock) { - status =3D readl(usb2_base + USB2_OBINTSTA); - if (status & ch->phy_data->obint_enable_bits) { - dev_vdbg(dev, "%s: %08x\n", __func__, status); - if (ch->phy_data->vblvl_ctrl) - writel(USB2_OBINTSTA_CLEAR, usb2_base + USB2_OBINTSTA); - else - writel(ch->phy_data->obint_enable_bits, usb2_base + USB2_OBINTSTA); - rcar_gen3_device_recognition(ch); - rcar_gen3_configure_vblvl_ctrl(ch); - ret =3D IRQ_HANDLED; - } + spin_lock_irqsave(&ch->lock, flags); + + if (ch->otg_initializing) { + ret =3D IRQ_NONE; + goto unlock; + } + + status =3D readl(usb2_base + USB2_OBINTSTA); + if (status & ch->phy_data->obint_enable_bits) { + dev_vdbg(dev, "%s: %08x\n", __func__, status); + if (ch->phy_data->vblvl_ctrl) + writel(USB2_OBINTSTA_CLEAR, usb2_base + USB2_OBINTSTA); + else + writel(ch->phy_data->obint_enable_bits, usb2_base + USB2_OBINTSTA); + rcar_gen3_device_recognition(ch); + rcar_gen3_configure_vblvl_ctrl(ch); + ret =3D IRQ_HANDLED; } =20 +unlock: + spin_unlock_irqrestore(&ch->lock, flags); rpm_put: pm_runtime_put_noidle(dev); return ret; } =20 +static void rcar_gen3_phy_usb2_irqs_mask_all(struct rcar_gen3_chan *channe= l, + u32 *masked_irqs_bits) +{ + u32 val, bitmask =3D USB2_INT_ENABLE_UCOM_INTEN; + void __iomem *usb2_base =3D channel->base; + + for (unsigned int i =3D 0; i < NUM_OF_PHYS; i++) + bitmask |=3D channel->rphys[i].int_enable_bits; + + val =3D readl(usb2_base + USB2_INT_ENABLE); + *masked_irqs_bits =3D val & bitmask; + val &=3D ~bitmask; + writel(val, usb2_base + USB2_INT_ENABLE); +} + +static void rcar_gen3_phy_usb2_irqs_unmask(struct rcar_gen3_chan *channel, + u32 irqs_bits) +{ + u32 val, bitmask =3D USB2_INT_ENABLE_UCOM_INTEN; + void __iomem *usb2_base =3D channel->base; + + for (unsigned int i =3D 0; i < NUM_OF_PHYS; i++) + bitmask |=3D channel->rphys[i].int_enable_bits; + + val =3D readl(usb2_base + USB2_INT_ENABLE); + val &=3D ~bitmask; + val |=3D irqs_bits; + writel(val, usb2_base + USB2_INT_ENABLE); +} + static int rcar_gen3_phy_usb2_init(struct phy *p) { struct rcar_gen3_phy *rphy =3D phy_get_drvdata(p); struct rcar_gen3_chan *channel =3D rphy->ch; void __iomem *usb2_base =3D channel->base; + unsigned long flags; u32 val; =20 - guard(spinlock_irqsave)(&channel->lock); + spin_lock_irqsave(&channel->lock, flags); + + if (channel->otg_initializing) { + unsigned long timeout =3D msecs_to_jiffies(40); + int ret; + + spin_unlock_irqrestore(&channel->lock, flags); + + ret =3D wait_for_completion_timeout(&channel->otg_init_done, timeout); + ret =3D ret ? 0 : -ETIMEDOUT; + if (ret) + return ret; + + spin_lock_irqsave(&channel->lock, flags); + } =20 /* Initialize USB2 part */ val =3D readl(usb2_base + USB2_INT_ENABLE); @@ -548,8 +639,24 @@ static int rcar_gen3_phy_usb2_init(struct phy *p) } =20 /* Initialize otg part (only if we initialize a PHY with IRQs). */ - if (rphy->int_enable_bits) - rcar_gen3_init_otg(channel); + if (rphy->int_enable_bits && channel->is_otg_channel && + !rcar_gen3_is_any_otg_rphy_initialized(channel)) { + u32 masked_irq_bits =3D 0; + + rcar_gen3_init_otg_phase0(channel); + rcar_gen3_phy_usb2_irqs_mask_all(channel, &masked_irq_bits); + reinit_completion(&channel->otg_init_done); + channel->otg_initializing =3D true; + spin_unlock_irqrestore(&channel->lock, flags); + + fsleep(20000); + + spin_lock_irqsave(&channel->lock, flags); + channel->otg_initializing =3D false; + complete_all(&channel->otg_init_done); + rcar_gen3_phy_usb2_irqs_unmask(channel, masked_irq_bits); + rcar_gen3_init_otg_phase1(channel); + } =20 if (channel->phy_data->vblvl_ctrl) { /* SIDDQ mode release */ @@ -568,6 +675,8 @@ static int rcar_gen3_phy_usb2_init(struct phy *p) =20 rphy->initialized =3D true; =20 + spin_unlock_irqrestore(&channel->lock, flags); + return 0; } =20 @@ -576,9 +685,24 @@ static int rcar_gen3_phy_usb2_exit(struct phy *p) struct rcar_gen3_phy *rphy =3D phy_get_drvdata(p); struct rcar_gen3_chan *channel =3D rphy->ch; void __iomem *usb2_base =3D channel->base; + unsigned long flags; u32 val; =20 - guard(spinlock_irqsave)(&channel->lock); + spin_lock_irqsave(&channel->lock, flags); + + if (channel->otg_initializing) { + unsigned long timeout =3D msecs_to_jiffies(40); + int ret; + + spin_unlock_irqrestore(&channel->lock, flags); + + ret =3D wait_for_completion_timeout(&channel->otg_init_done, timeout); + ret =3D ret ? 0 : -ETIMEDOUT; + if (ret) + return ret; + + spin_lock_irqsave(&channel->lock, flags); + } =20 rphy->initialized =3D false; =20 @@ -588,6 +712,7 @@ static int rcar_gen3_phy_usb2_exit(struct phy *p) val &=3D ~USB2_INT_ENABLE_UCOM_INTEN; writel(val, usb2_base + USB2_INT_ENABLE); =20 + spin_unlock_irqrestore(&channel->lock, flags); return 0; } =20 @@ -596,6 +721,7 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *p) struct rcar_gen3_phy *rphy =3D phy_get_drvdata(p); struct rcar_gen3_chan *channel =3D rphy->ch; void __iomem *usb2_base =3D channel->base; + unsigned long flags; u32 val; int ret =3D 0; =20 @@ -605,7 +731,20 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *p) return ret; } =20 - guard(spinlock_irqsave)(&channel->lock); + spin_lock_irqsave(&channel->lock, flags); + + if (channel->otg_initializing) { + unsigned long timeout =3D msecs_to_jiffies(40); + + spin_unlock_irqrestore(&channel->lock, flags); + + ret =3D wait_for_completion_timeout(&channel->otg_init_done, timeout); + ret =3D ret ? 0 : -ETIMEDOUT; + if (ret) + goto disable_regulator; + + spin_lock_irqsave(&channel->lock, flags); + } =20 if (!rcar_gen3_are_all_rphys_power_off(channel)) goto out; @@ -620,27 +759,49 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *p) /* The powered flag should be set for any other phys anyway */ rphy->powered =3D true; =20 - return 0; + spin_unlock_irqrestore(&channel->lock, flags); + +disable_regulator: + if (ret && channel->vbus && !channel->otg_internal_reg) + regulator_disable(channel->vbus); + + return ret; } =20 static int rcar_gen3_phy_usb2_power_off(struct phy *p) { struct rcar_gen3_phy *rphy =3D phy_get_drvdata(p); struct rcar_gen3_chan *channel =3D rphy->ch; + unsigned long flags; int ret =3D 0; =20 - scoped_guard(spinlock_irqsave, &channel->lock) { - rphy->powered =3D false; + spin_lock_irqsave(&channel->lock, flags); =20 - if (rcar_gen3_are_all_rphys_power_off(channel)) { - u32 val =3D readl(channel->base + USB2_USBCTR); + if (channel->otg_initializing) { + unsigned long timeout =3D msecs_to_jiffies(40); =20 - val |=3D USB2_USBCTR_PLL_RST; - writel(val, channel->base + USB2_USBCTR); - } + spin_unlock_irqrestore(&channel->lock, flags); + + ret =3D wait_for_completion_timeout(&channel->otg_init_done, timeout); + ret =3D ret ? 0 : -ETIMEDOUT; + if (ret) + return ret; + + spin_lock_irqsave(&channel->lock, flags); } =20 - if (channel->vbus && !channel->otg_internal_reg) + rphy->powered =3D false; + + if (rcar_gen3_are_all_rphys_power_off(channel)) { + u32 val =3D readl(channel->base + USB2_USBCTR); + + val |=3D USB2_USBCTR_PLL_RST; + writel(val, channel->base + USB2_USBCTR); + } + + spin_unlock_irqrestore(&channel->lock, flags); + + if (!ret && channel->vbus && !channel->otg_internal_reg) ret =3D regulator_disable(channel->vbus); =20 return ret; @@ -1007,6 +1168,7 @@ static int rcar_gen3_phy_usb2_probe(struct platform_d= evice *pdev) return ret; =20 spin_lock_init(&channel->lock); + init_completion(&channel->otg_init_done); for (i =3D 0; i < NUM_OF_PHYS; i++) { channel->rphys[i].phy =3D devm_phy_create(dev, NULL, channel->phy_data->phy_usb2_ops); --=20 2.43.0