phy: renesas: rcar-gen3-usb2: Lock around hardware registers and driver data

commit 55a387ebb9 upstream.

The phy-rcar-gen3-usb2 driver exposes four individual PHYs that are
requested and configured by PHY users. The struct phy_ops APIs access the
same set of registers to configure all PHYs. Additionally, PHY settings can
be modified through sysfs or an IRQ handler. While some struct phy_ops APIs
are protected by a driver-wide mutex, others rely on individual
PHY-specific mutexes.

This approach can lead to various issues, including:
1/ the IRQ handler may interrupt PHY settings in progress, racing with
   hardware configuration protected by a mutex lock
2/ due to msleep(20) in rcar_gen3_init_otg(), while a configuration thread
   suspends to wait for the delay, another thread may try to configure
   another PHY (with phy_init() + phy_power_on()); re-running the
   phy_init() goes to the exact same configuration code, re-running the
   same hardware configuration on the same set of registers (and bits)
   which might impact the result of the msleep for the 1st configuring
   thread
3/ sysfs can configure the hardware (though role_store()) and it can
   still race with the phy_init()/phy_power_on() APIs calling into the
   drivers struct phy_ops

To address these issues, add a spinlock to protect hardware register access
and driver private data structures (e.g., calls to
rcar_gen3_is_any_rphy_initialized()). Checking driver-specific data remains
necessary as all PHY instances share common settings. With this change,
the existing mutex protection is removed and the cleanup.h helpers are
used.

While at it, to keep the code simpler, do not skip
regulator_enable()/regulator_disable() APIs in
rcar_gen3_phy_usb2_power_on()/rcar_gen3_phy_usb2_power_off() as the
regulators enable/disable operations are reference counted anyway.

[claudiu.beznea:
 - in rcar_gen3_init_otg(): fixed conflict by droppping ch->soc_no_adp_ctrl check
 - in rcar_gen3_phy_usb2_irq() use spin_lock()/spin_unlock() as scoped_guard()
   is not avaialable in v5.10
 - in probe(): replace mutex_init() with spin_lock_init()
 - rcar_gen3_phy_usb2_power_off() replaced scoped_guard() as it is not
   available in v5.10
 - in rcar_gen3_phy_usb2_power_on() droppped guard to avoid compilation
   warning "ISO C90 forbids mixed declarations and code"]

Fixes: f3b5a8d9b5 ("phy: rcar-gen3-usb2: Add R-Car Gen3 USB2 PHY driver")
Cc: stable@vger.kernel.org
Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Link: https://lore.kernel.org/r/20250507125032.565017-4-claudiu.beznea.uj@bp.renesas.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Claudiu Beznea
2026-04-07 14:37:41 +03:00
committed by Greg Kroah-Hartman
parent 6f1d5cd7f0
commit 0f86a55990
+26 -15
View File
@@ -9,6 +9,7 @@
* Copyright (C) 2014 Cogent Embedded, Inc.
*/
#include <linux/cleanup.h>
#include <linux/extcon-provider.h>
#include <linux/interrupt.h>
#include <linux/io.h>
@@ -108,7 +109,7 @@ struct rcar_gen3_chan {
struct rcar_gen3_phy rphys[NUM_OF_PHYS];
struct regulator *vbus;
struct work_struct work;
struct mutex lock; /* protects rphys[...].powered */
spinlock_t lock; /* protects access to hardware and driver data structure. */
enum usb_dr_mode dr_mode;
bool extcon_host;
bool is_otg_channel;
@@ -317,6 +318,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;
@@ -404,6 +407,8 @@ static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, void *_ch)
if (pm_runtime_suspended(dev))
goto rpm_put;
spin_lock(&ch->lock);
status = readl(usb2_base + USB2_OBINTSTA);
if (status & USB2_OBINT_BITS) {
dev_vdbg(dev, "%s: %08x\n", __func__, status);
@@ -412,6 +417,8 @@ static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, void *_ch)
ret = IRQ_HANDLED;
}
spin_unlock(&ch->lock);
rpm_put:
pm_runtime_put_noidle(dev);
return ret;
@@ -424,6 +431,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;
@@ -450,6 +459,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);
@@ -466,19 +477,21 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *p)
struct rcar_gen3_phy *rphy = phy_get_drvdata(p);
struct rcar_gen3_chan *channel = rphy->ch;
void __iomem *usb2_base = channel->base;
unsigned long flags;
u32 val;
int ret = 0;
mutex_lock(&channel->lock);
if (!rcar_gen3_are_all_rphys_power_off(channel))
goto out;
if (channel->vbus) {
ret = regulator_enable(channel->vbus);
if (ret)
goto out;
return ret;
}
spin_lock_irqsave(&channel->lock, flags);
if (!rcar_gen3_are_all_rphys_power_off(channel))
goto out;
val = readl(usb2_base + USB2_USBCTR);
val |= USB2_USBCTR_PLL_RST;
writel(val, usb2_base + USB2_USBCTR);
@@ -488,7 +501,8 @@ static int rcar_gen3_phy_usb2_power_on(struct phy *p)
out:
/* The powered flag should be set for any other phys anyway */
rphy->powered = true;
mutex_unlock(&channel->lock);
spin_unlock_irqrestore(&channel->lock, flags);
return 0;
}
@@ -497,20 +511,16 @@ static int rcar_gen3_phy_usb2_power_off(struct phy *p)
{
struct rcar_gen3_phy *rphy = phy_get_drvdata(p);
struct rcar_gen3_chan *channel = rphy->ch;
unsigned long flags;
int ret = 0;
mutex_lock(&channel->lock);
spin_lock_irqsave(&channel->lock, flags);
rphy->powered = false;
if (!rcar_gen3_are_all_rphys_power_off(channel))
goto out;
spin_unlock_irqrestore(&channel->lock, flags);
if (channel->vbus)
ret = regulator_disable(channel->vbus);
out:
mutex_unlock(&channel->lock);
return ret;
}
@@ -650,7 +660,8 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
goto error;
}
mutex_init(&channel->lock);
spin_lock_init(&channel->lock);
for (i = 0; i < NUM_OF_PHYS; i++) {
channel->rphys[i].phy = devm_phy_create(dev, NULL,
phy_usb2_ops);