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

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.

Fixes: f3b5a8d9b50d ("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>

authored by Claudiu Beznea and committed by Vinod Koul 55a387eb de76809f

+26 -23
+26 -23
drivers/phy/renesas/phy-rcar-gen3-usb2.c
··· 9 * Copyright (C) 2014 Cogent Embedded, Inc. 10 */ 11 12 #include <linux/extcon-provider.h> 13 #include <linux/interrupt.h> 14 #include <linux/io.h> ··· 119 struct regulator *vbus; 120 struct reset_control *rstc; 121 struct work_struct work; 122 - struct mutex lock; /* protects rphys[...].powered */ 123 enum usb_dr_mode dr_mode; 124 u32 obint_enable_bits; 125 bool extcon_host; ··· 349 bool is_b_device; 350 enum phy_mode cur_mode, new_mode; 351 352 if (!ch->is_otg_channel || !rcar_gen3_is_any_otg_rphy_initialized(ch)) 353 return -EIO; 354 ··· 418 val = readl(usb2_base + USB2_ADPCTRL); 419 writel(val | USB2_ADPCTRL_IDPULLUP, usb2_base + USB2_ADPCTRL); 420 } 421 - msleep(20); 422 423 writel(0xffffffff, usb2_base + USB2_OBINTSTA); 424 writel(ch->obint_enable_bits, usb2_base + USB2_OBINTEN); ··· 439 if (pm_runtime_suspended(dev)) 440 goto rpm_put; 441 442 - status = readl(usb2_base + USB2_OBINTSTA); 443 - if (status & ch->obint_enable_bits) { 444 - dev_vdbg(dev, "%s: %08x\n", __func__, status); 445 - writel(ch->obint_enable_bits, usb2_base + USB2_OBINTSTA); 446 - rcar_gen3_device_recognition(ch); 447 - ret = IRQ_HANDLED; 448 } 449 450 rpm_put: ··· 460 struct rcar_gen3_chan *channel = rphy->ch; 461 void __iomem *usb2_base = channel->base; 462 u32 val; 463 464 /* Initialize USB2 part */ 465 val = readl(usb2_base + USB2_INT_ENABLE); ··· 486 void __iomem *usb2_base = channel->base; 487 u32 val; 488 489 rphy->initialized = false; 490 491 val = readl(usb2_base + USB2_INT_ENABLE); ··· 507 u32 val; 508 int ret = 0; 509 510 - mutex_lock(&channel->lock); 511 - if (!rcar_gen3_are_all_rphys_power_off(channel)) 512 - goto out; 513 - 514 if (channel->vbus) { 515 ret = regulator_enable(channel->vbus); 516 if (ret) 517 - goto out; 518 } 519 520 val = readl(usb2_base + USB2_USBCTR); 521 val |= USB2_USBCTR_PLL_RST; ··· 527 out: 528 /* The powered flag should be set for any other phys anyway */ 529 rphy->powered = true; 530 - mutex_unlock(&channel->lock); 531 532 return 0; 533 } ··· 537 struct rcar_gen3_chan *channel = rphy->ch; 538 int ret = 0; 539 540 - mutex_lock(&channel->lock); 541 - rphy->powered = false; 542 - 543 - if (!rcar_gen3_are_all_rphys_power_off(channel)) 544 - goto out; 545 546 if (channel->vbus) 547 ret = regulator_disable(channel->vbus); 548 - 549 - out: 550 - mutex_unlock(&channel->lock); 551 552 return ret; 553 } ··· 753 if (phy_data->no_adp_ctrl) 754 channel->obint_enable_bits = USB2_OBINT_IDCHG_EN; 755 756 - mutex_init(&channel->lock); 757 for (i = 0; i < NUM_OF_PHYS; i++) { 758 channel->rphys[i].phy = devm_phy_create(dev, NULL, 759 phy_data->phy_usb2_ops);
··· 9 * Copyright (C) 2014 Cogent Embedded, Inc. 10 */ 11 12 + #include <linux/cleanup.h> 13 #include <linux/extcon-provider.h> 14 #include <linux/interrupt.h> 15 #include <linux/io.h> ··· 118 struct regulator *vbus; 119 struct reset_control *rstc; 120 struct work_struct work; 121 + spinlock_t lock; /* protects access to hardware and driver data structure. */ 122 enum usb_dr_mode dr_mode; 123 u32 obint_enable_bits; 124 bool extcon_host; ··· 348 bool is_b_device; 349 enum phy_mode cur_mode, new_mode; 350 351 + guard(spinlock_irqsave)(&ch->lock); 352 + 353 if (!ch->is_otg_channel || !rcar_gen3_is_any_otg_rphy_initialized(ch)) 354 return -EIO; 355 ··· 415 val = readl(usb2_base + USB2_ADPCTRL); 416 writel(val | USB2_ADPCTRL_IDPULLUP, usb2_base + USB2_ADPCTRL); 417 } 418 + mdelay(20); 419 420 writel(0xffffffff, usb2_base + USB2_OBINTSTA); 421 writel(ch->obint_enable_bits, usb2_base + USB2_OBINTEN); ··· 436 if (pm_runtime_suspended(dev)) 437 goto rpm_put; 438 439 + scoped_guard(spinlock, &ch->lock) { 440 + status = readl(usb2_base + USB2_OBINTSTA); 441 + if (status & ch->obint_enable_bits) { 442 + dev_vdbg(dev, "%s: %08x\n", __func__, status); 443 + writel(ch->obint_enable_bits, usb2_base + USB2_OBINTSTA); 444 + rcar_gen3_device_recognition(ch); 445 + ret = IRQ_HANDLED; 446 + } 447 } 448 449 rpm_put: ··· 455 struct rcar_gen3_chan *channel = rphy->ch; 456 void __iomem *usb2_base = channel->base; 457 u32 val; 458 + 459 + guard(spinlock_irqsave)(&channel->lock); 460 461 /* Initialize USB2 part */ 462 val = readl(usb2_base + USB2_INT_ENABLE); ··· 479 void __iomem *usb2_base = channel->base; 480 u32 val; 481 482 + guard(spinlock_irqsave)(&channel->lock); 483 + 484 rphy->initialized = false; 485 486 val = readl(usb2_base + USB2_INT_ENABLE); ··· 498 u32 val; 499 int ret = 0; 500 501 if (channel->vbus) { 502 ret = regulator_enable(channel->vbus); 503 if (ret) 504 + return ret; 505 } 506 + 507 + guard(spinlock_irqsave)(&channel->lock); 508 + 509 + if (!rcar_gen3_are_all_rphys_power_off(channel)) 510 + goto out; 511 512 val = readl(usb2_base + USB2_USBCTR); 513 val |= USB2_USBCTR_PLL_RST; ··· 517 out: 518 /* The powered flag should be set for any other phys anyway */ 519 rphy->powered = true; 520 521 return 0; 522 } ··· 528 struct rcar_gen3_chan *channel = rphy->ch; 529 int ret = 0; 530 531 + scoped_guard(spinlock_irqsave, &channel->lock) 532 + rphy->powered = false; 533 534 if (channel->vbus) 535 ret = regulator_disable(channel->vbus); 536 537 return ret; 538 } ··· 750 if (phy_data->no_adp_ctrl) 751 channel->obint_enable_bits = USB2_OBINT_IDCHG_EN; 752 753 + spin_lock_init(&channel->lock); 754 for (i = 0; i < NUM_OF_PHYS; i++) { 755 channel->rphys[i].phy = devm_phy_create(dev, NULL, 756 phy_data->phy_usb2_ops);