linux-kernel - [PATCH 4/4] clk: imx: fracn-gppll: fix pll power up

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20241014-imx-clk-v1-v1-4-ee75876d3102@nxp.com>
Date: Mon, 14 Oct 2024 17:11:25 +0800
From: "Peng Fan (OSS)" <peng.fan@....nxp.com>
To: Abel Vesa <abelvesa@...nel.org>, 
 Michael Turquette <mturquette@...libre.com>, 
 Stephen Boyd <sboyd@...nel.org>, Shawn Guo <shawnguo@...nel.org>, 
 Sascha Hauer <s.hauer@...gutronix.de>, 
 Pengutronix Kernel Team <kernel@...gutronix.de>, 
 Fabio Estevam <festevam@...il.com>, Aisheng Dong <aisheng.dong@....com>
Cc: linux-clk@...r.kernel.org, imx@...ts.linux.dev, 
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
 Peng Fan <peng.fan@....com>, Jacky Bai <ping.bai@....com>
Subject: [PATCH 4/4] clk: imx: fracn-gppll: fix pll power up

From: Peng Fan <peng.fan@....com>

To i.MX93 which features dual Cortex-A55 cores and DSU, when using
writel_relaxed to write value to PLL registers, the value might be
buffered. To make sure the value has been written into the hardware,
using readl to read back the register could make sure the value
written into hardware.

current PLL power up flow can be simplified as below:
  1. writel_relaxed to set the PLL POWERUP bit;
  2. readl_poll_timeout to check the PLL lock bit:
     a). timeout = ktime_add_us(ktime_get(), timeout_us);
     b). readl the pll the lock reg;
     c). check if the pll lock bit ready
     d). check if timeout

But in some corner cases, both the write in step 1 and read in
step 2 will be blocked by other bus transaction in the SoC for a
long time, saying the value into real hardware is just before step b).
That means the timeout counting has begins for quite sometime since
step a), but value still not written into real hardware until bus
released just at a point before step b).

Then there maybe chances that the pll lock bit is not ready
when readl done but the timeout happens. readl_poll_timeout will
err return due to timeout. To avoid such unexpected failure,
read back the reg to make sure the write has been done in HW
reg.

Introduce fence_write for this purpose.

Since we are here, to avoid udelay to run before writel_relaxed, use
fence_write before udelay.

Fixes: 1b26cb8a77a4 ("clk: imx: support fracn gppll")
Co-developed-by: Jacky Bai <ping.bai@....com>
Signed-off-by: Jacky Bai <ping.bai@....com>
Signed-off-by: Peng Fan <peng.fan@....com>
---
 drivers/clk/imx/clk-fracn-gppll.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/imx/clk-fracn-gppll.c b/drivers/clk/imx/clk-fracn-gppll.c
index 4749c3e0b7051cf53876664808aa28742f6861f7..6a4b3745d3d9aa1b583c9bd390c884bcb689e4c7 100644
--- a/drivers/clk/imx/clk-fracn-gppll.c
+++ b/drivers/clk/imx/clk-fracn-gppll.c
@@ -63,6 +63,12 @@
 		.odiv	=	(_odiv),			\
 	}
 
+#define fence_write(val, reg)			\
+	do {					\
+		writel_relaxed(val, reg);	\
+		readl(reg);			\
+	} while (0)
+
 struct clk_fracn_gppll {
 	struct clk_hw			hw;
 	void __iomem			*base;
@@ -253,10 +259,10 @@ static int clk_fracn_gppll_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	pll_div = FIELD_PREP(PLL_RDIV_MASK, rate->rdiv) | rate->odiv |
 		FIELD_PREP(PLL_MFI_MASK, rate->mfi);
-	writel_relaxed(pll_div, pll->base + PLL_DIV);
+	fence_write(pll_div, pll->base + PLL_DIV);
 	if (pll->flags & CLK_FRACN_GPPLL_FRACN) {
 		writel_relaxed(rate->mfd, pll->base + PLL_DENOMINATOR);
-		writel_relaxed(FIELD_PREP(PLL_MFN_MASK, rate->mfn), pll->base + PLL_NUMERATOR);
+		fence_write(FIELD_PREP(PLL_MFN_MASK, rate->mfn), pll->base + PLL_NUMERATOR);
 	}
 
 	/* Wait for 5us according to fracn mode pll doc */
@@ -264,7 +270,7 @@ static int clk_fracn_gppll_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	/* Enable Powerup */
 	tmp |= POWERUP_MASK;
-	writel_relaxed(tmp, pll->base + PLL_CTRL);
+	fence_write(tmp, pll->base + PLL_CTRL);
 
 	/* Wait Lock */
 	ret = clk_fracn_gppll_wait_lock(pll);
@@ -301,7 +307,7 @@ static int clk_fracn_gppll_prepare(struct clk_hw *hw)
 	writel_relaxed(val, pll->base + PLL_CTRL);
 
 	val |= POWERUP_MASK;
-	writel_relaxed(val, pll->base + PLL_CTRL);
+	fence_write(val, pll->base + PLL_CTRL);
 
 	ret = clk_fracn_gppll_wait_lock(pll);
 	if (ret)

-- 
2.37.1