Re: [PATCH 1/2] clk: en7523: do not sleep in the EN7581 PCIe clock disable

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] clk: en7523: data race in en7523_reset_update() [1]
--

commit 1564f332e9a1734f7c62abce0ffb9fd4c355e412
Author: John Crispin <john@phrozen.org>

clk: en7523: do not sleep in the EN7581 PCIe clock disable

This commit removes the usleep_range() call from en7581_pci_disable() to
avoid sleeping in atomic context when the clock core disables the PCIe
clock under the enable spinlock.

> diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
> index 24fdec2e6198c..8f63482cbb7ba 100644
> --- a/drivers/clk/clk-en7523.c
> +++ b/drivers/clk/clk-en7523.c
> @@ -689,7 +689,6 @@ static void en7581_pci_disable(struct clk_hw *hw)
>  	mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1;
>  	val = readl(np_base + REG_PCI_CONTROL);
>  	writel(val & ~mask, np_base + REG_PCI_CONTROL);
> -	usleep_range(1000, 2000);
>  }

[Severity: High]
This problem wasn't introduced by this patch, but can concurrent accesses
to REG_PCI_CONTROL and the reset bank registers race [1] and lose bit
updates?

en7581_pci_disable(), en7581_pci_enable(), and en7523_reset_update() perform
read-modify-write sequences on shared NP SCU registers (including
REG_PCI_CONTROL on EN7581) without a driver-level spinlock.  Because the
reset controller framework does not serialize .assert and .deassert
callbacks, two callers updating bits in the same register bank, or a PCIe
clock toggle running alongside a reset update on REG_PCI_CONTROL, can
interleave:

CPU0                              CPU1
en7523_reset_update()             en7523_reset_update()
  val = readl(addr);
                                    val = readl(addr);
  val |= BIT(id1);
  writel(val, addr);
                                    val &= ~BIT(id2);
                                    writel(val, addr);  <-- id1 update lost

This overwrites hardware state and can leave peripherals stuck in reset or
corrupt PCIe clock/reset bits.

[1] https://sashiko.dev/#/bug/linux-ef84cdb6-0114-41fc-b76f-33e8bdd47a6f

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111085650.632025-1-john@phrozen.org?part=1