[PATCH 1/4] clk: imx: pll14xx: use register defines consistently

[PATCH 1/4] clk: imx: pll14xx: use register defines consistently

[Ahmad Fatoum]

From: Sascha Hauer <s.hauer@pengutronix.de>

clk: imx: pll14xx: Use register defines consistently
The driver has defines for the registers, but they are mostly unused.
Use the defines consistently throughout the driver. While at it rename
DIV_CTL to DIV_CTL0 because that's the name in the reference manual.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
[ ahmad: ported from Linux 485b4ff57c27, s/(readl|writel)_relaxed/\1/ ]
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/clk/imx/clk-pll14xx.c | 49 ++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index 8a773f19f807..c1d5a6b88f41 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -19,7 +19,8 @@
 #include "clk.h"
 
 #define GNRL_CTL	0x0
-#define DIV_CTL		0x4
+#define DIV_CTL0	0x4
+#define DIV_CTL1	0x8
 #define LOCK_STATUS	BIT(31)
 #define LOCK_SEL_MASK	BIT(29)
 #define CLKE_MASK	BIT(11)
@@ -118,7 +119,7 @@ static unsigned long clk_pll1416x_recalc_rate(struct clk_hw *hw,
 	u32 mdiv, pdiv, sdiv, pll_div;
 	u64 fvco = parent_rate;
 
-	pll_div = readl(pll->base + 4);
+	pll_div = readl(pll->base + DIV_CTL0);
 	mdiv = (pll_div & MDIV_MASK) >> MDIV_SHIFT;
 	pdiv = (pll_div & PDIV_MASK) >> PDIV_SHIFT;
 	sdiv = (pll_div & SDIV_MASK) >> SDIV_SHIFT;
@@ -137,8 +138,8 @@ static unsigned long clk_pll1443x_recalc_rate(struct clk_hw *hw,
 	short int kdiv;
 	u64 fvco = parent_rate;
 
-	pll_div_ctl0 = readl(pll->base + 4);
-	pll_div_ctl1 = readl(pll->base + 8);
+	pll_div_ctl0 = readl(pll->base + DIV_CTL0);
+	pll_div_ctl1 = readl(pll->base + DIV_CTL1);
 	mdiv = (pll_div_ctl0 & MDIV_MASK) >> MDIV_SHIFT;
 	pdiv = (pll_div_ctl0 & PDIV_MASK) >> PDIV_SHIFT;
 	sdiv = (pll_div_ctl0 & SDIV_MASK) >> SDIV_SHIFT;
@@ -168,7 +169,7 @@ static int clk_pll14xx_wait_lock(struct clk_pll14xx *pll)
 {
 	u32 val;
 
-	return readl_poll_timeout(pll->base, val, val & LOCK_STATUS,
+	return readl_poll_timeout(pll->base + GNRL_CTL, val, val & LOCK_STATUS,
 			LOCK_TIMEOUT_US);
 }
 
@@ -187,32 +188,32 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 		return -EINVAL;
 	}
 
-	tmp = readl(pll->base + 4);
+	tmp = readl(pll->base + DIV_CTL0);
 
 	if (!clk_pll14xx_mp_change(rate, tmp)) {
 		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
 		tmp |= rate->sdiv << SDIV_SHIFT;
-		writel(tmp, pll->base + 4);
+		writel(tmp, pll->base + DIV_CTL0);
 
 		return 0;
 	}
 
 	/* Bypass clock and set lock to pll output lock */
-	tmp = readl(pll->base);
+	tmp = readl(pll->base + GNRL_CTL);
 	tmp |= LOCK_SEL_MASK;
-	writel(tmp, pll->base);
+	writel(tmp, pll->base + GNRL_CTL);
 
 	/* Enable RST */
 	tmp &= ~RST_MASK;
-	writel(tmp, pll->base);
+	writel(tmp, pll->base + GNRL_CTL);
 
 	/* Enable BYPASS */
 	tmp |= BYPASS_MASK;
-	writel(tmp, pll->base);
+	writel(tmp, pll->base + GNRL_CTL);
 
 	div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
 		(rate->sdiv << SDIV_SHIFT);
-	writel(div_val, pll->base + 0x4);
+	writel(div_val, pll->base + DIV_CTL0);
 
 	/*
 	 * According to SPEC, t3 - t2 need to be greater than
@@ -224,7 +225,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	/* Disable RST */
 	tmp |= RST_MASK;
-	writel(tmp, pll->base);
+	writel(tmp, pll->base + GNRL_CTL);
 
 	/* Wait Lock */
 	ret = clk_pll14xx_wait_lock(pll);
@@ -233,7 +234,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	/* Bypass */
 	tmp &= ~BYPASS_MASK;
-	writel(tmp, pll->base);
+	writel(tmp, pll->base + GNRL_CTL);
 
 	return 0;
 }
@@ -269,32 +270,32 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 		return -EINVAL;
 	}
 
-	tmp = readl(pll->base + 4);
+	tmp = readl(pll->base + DIV_CTL0);
 
 	if (!clk_pll14xx_mp_change(rate, tmp)) {
 		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
 		tmp |= rate->sdiv << SDIV_SHIFT;
-		writel(tmp, pll->base + 4);
+		writel(tmp, pll->base + DIV_CTL0);
 
 		tmp = rate->kdiv << KDIV_SHIFT;
-		writel(tmp, pll->base + 8);
+		writel(tmp, pll->base + DIV_CTL1);
 
 		return 0;
 	}
 
 	/* Enable RST */
-	tmp = readl(pll->base);
+	tmp = readl(pll->base + GNRL_CTL);
 	tmp &= ~RST_MASK;
-	writel(tmp, pll->base);
+	writel(tmp, pll->base + GNRL_CTL);
 
 	/* Enable BYPASS */
 	tmp |= BYPASS_MASK;
-	writel(tmp, pll->base);
+	writel(tmp, pll->base + GNRL_CTL);
 
 	div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
 		(rate->sdiv << SDIV_SHIFT);
-	writel(div_val, pll->base + 0x4);
-	writel(rate->kdiv << KDIV_SHIFT, pll->base + 0x8);
+	writel(div_val, pll->base + DIV_CTL0);
+	writel(rate->kdiv << KDIV_SHIFT, pll->base + DIV_CTL1);
 
 	/*
 	 * According to SPEC, t3 - t2 need to be greater than
@@ -306,7 +307,7 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	/* Disable RST */
 	tmp |= RST_MASK;
-	writel(tmp, pll->base);
+	writel(tmp, pll->base + GNRL_CTL);
 
 	/* Wait Lock*/
 	ret = clk_pll14xx_wait_lock(pll);
@@ -315,7 +316,7 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	/* Bypass */
 	tmp &= ~BYPASS_MASK;
-	writel(tmp, pll->base);
+	writel(tmp, pll->base + GNRL_CTL);
 
 	return 0;
 }
-- 
2.47.3

[PATCH 2/4] clk: imx: pll14xx: Drop wrong shifting

[Ahmad Fatoum]

From: Sascha Hauer <s.hauer@pengutronix.de>

The code tries to mask the bits in SDIV_MASK from 'tmp'. SDIV_MASK
already contains the shifted value, so shifting it again is wrong.
No functional change though as SDIV_SHIFT is zero.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Abel Vesa <abel.vesa@nxp.com>
Link: https://lore.kernel.org/r/20220304125256.2125023-3-s.hauer@pengutronix.de
Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
[ahmad: cherry-picked from Linux commit d77461a616b659bd88d]
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/clk/imx/clk-pll14xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index c1d5a6b88f41..a4aa9ce548cb 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -191,7 +191,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 	tmp = readl(pll->base + DIV_CTL0);
 
 	if (!clk_pll14xx_mp_change(rate, tmp)) {
-		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
+		tmp &= ~SDIV_MASK;
 		tmp |= rate->sdiv << SDIV_SHIFT;
 		writel(tmp, pll->base + DIV_CTL0);
 
@@ -273,7 +273,7 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 	tmp = readl(pll->base + DIV_CTL0);
 
 	if (!clk_pll14xx_mp_change(rate, tmp)) {
-		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
+		tmp &= ~SDIV_MASK;
 		tmp |= rate->sdiv << SDIV_SHIFT;
 		writel(tmp, pll->base + DIV_CTL0);
 
-- 
2.47.3

[PATCH 3/4] clk: imx: pll14xx: Use FIELD_GET/FIELD_PREP

[Ahmad Fatoum]

From: Sascha Hauer <s.hauer@pengutronix.de>

Linux has these marvelous FIELD_GET/FIELD_PREP macros for easy access
to bitfields in registers. Use them and remove the now unused *_SHIFT
defines.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Abel Vesa <abel.vesa@nxp.com>
Link: https://lore.kernel.org/r/20220304125256.2125023-4-s.hauer@pengutronix.de
Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
[ahmad: ported from Linux commit 58f4980ccbecf63]
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/clk/imx/clk-pll14xx.c | 40 +++++++++++++++++------------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index a4aa9ce548cb..939320c0590f 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -15,6 +15,7 @@
 #include <clock.h>
 #include <soc/imx8m/clk-early.h>
 #include <linux/math64.h>
+#include <linux/bitfield.h>
 
 #include "clk.h"
 
@@ -26,13 +27,9 @@
 #define CLKE_MASK	BIT(11)
 #define RST_MASK	BIT(9)
 #define BYPASS_MASK	BIT(4)
-#define MDIV_SHIFT	12
 #define MDIV_MASK	GENMASK(21, 12)
-#define PDIV_SHIFT	4
 #define PDIV_MASK	GENMASK(9, 4)
-#define SDIV_SHIFT	0
 #define SDIV_MASK	GENMASK(2, 0)
-#define KDIV_SHIFT	0
 #define KDIV_MASK	GENMASK(15, 0)
 
 #define LOCK_TIMEOUT_US		10000
@@ -120,9 +117,9 @@ static unsigned long clk_pll1416x_recalc_rate(struct clk_hw *hw,
 	u64 fvco = parent_rate;
 
 	pll_div = readl(pll->base + DIV_CTL0);
-	mdiv = (pll_div & MDIV_MASK) >> MDIV_SHIFT;
-	pdiv = (pll_div & PDIV_MASK) >> PDIV_SHIFT;
-	sdiv = (pll_div & SDIV_MASK) >> SDIV_SHIFT;
+	mdiv = FIELD_GET(MDIV_MASK, pll_div);
+	pdiv = FIELD_GET(PDIV_MASK, pll_div);
+	sdiv = FIELD_GET(SDIV_MASK, pll_div);
 
 	fvco *= mdiv;
 	do_div(fvco, pdiv << sdiv);
@@ -140,10 +137,10 @@ static unsigned long clk_pll1443x_recalc_rate(struct clk_hw *hw,
 
 	pll_div_ctl0 = readl(pll->base + DIV_CTL0);
 	pll_div_ctl1 = readl(pll->base + DIV_CTL1);
-	mdiv = (pll_div_ctl0 & MDIV_MASK) >> MDIV_SHIFT;
-	pdiv = (pll_div_ctl0 & PDIV_MASK) >> PDIV_SHIFT;
-	sdiv = (pll_div_ctl0 & SDIV_MASK) >> SDIV_SHIFT;
-	kdiv = pll_div_ctl1 & KDIV_MASK;
+	mdiv = FIELD_GET(MDIV_MASK, pll_div_ctl0);
+	pdiv = FIELD_GET(PDIV_MASK, pll_div_ctl0);
+	sdiv = FIELD_GET(SDIV_MASK, pll_div_ctl0);
+	kdiv = FIELD_GET(KDIV_MASK, pll_div_ctl1);
 
 	/* fvco = (m * 65536 + k) * Fin / (p * 65536) */
 	fvco *= (mdiv * 65536 + kdiv);
@@ -159,8 +156,8 @@ static inline bool clk_pll14xx_mp_change(const struct imx_pll14xx_rate_table *ra
 {
 	u32 old_mdiv, old_pdiv;
 
-	old_mdiv = (pll_div & MDIV_MASK) >> MDIV_SHIFT;
-	old_pdiv = (pll_div & PDIV_MASK) >> PDIV_SHIFT;
+	old_mdiv = FIELD_GET(MDIV_MASK, pll_div);
+	old_pdiv = FIELD_GET(PDIV_MASK, pll_div);
 
 	return rate->mdiv != old_mdiv || rate->pdiv != old_pdiv;
 }
@@ -192,7 +189,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	if (!clk_pll14xx_mp_change(rate, tmp)) {
 		tmp &= ~SDIV_MASK;
-		tmp |= rate->sdiv << SDIV_SHIFT;
+		tmp |= FIELD_PREP(SDIV_MASK, rate->sdiv);
 		writel(tmp, pll->base + DIV_CTL0);
 
 		return 0;
@@ -211,8 +208,8 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 	tmp |= BYPASS_MASK;
 	writel(tmp, pll->base + GNRL_CTL);
 
-	div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
-		(rate->sdiv << SDIV_SHIFT);
+	div_val = FIELD_PREP(MDIV_MASK, rate->mdiv) | FIELD_PREP(PDIV_MASK, rate->pdiv) |
+		FIELD_PREP(SDIV_MASK, rate->sdiv);
 	writel(div_val, pll->base + DIV_CTL0);
 
 	/*
@@ -274,10 +271,10 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	if (!clk_pll14xx_mp_change(rate, tmp)) {
 		tmp &= ~SDIV_MASK;
-		tmp |= rate->sdiv << SDIV_SHIFT;
+		tmp |= FIELD_PREP(SDIV_MASK, rate->sdiv);
 		writel(tmp, pll->base + DIV_CTL0);
 
-		tmp = rate->kdiv << KDIV_SHIFT;
+		tmp = FIELD_PREP(KDIV_MASK, rate->kdiv);
 		writel(tmp, pll->base + DIV_CTL1);
 
 		return 0;
@@ -292,10 +289,11 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 	tmp |= BYPASS_MASK;
 	writel(tmp, pll->base + GNRL_CTL);
 
-	div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
-		(rate->sdiv << SDIV_SHIFT);
+	div_val = FIELD_PREP(MDIV_MASK, rate->mdiv) |
+		  FIELD_PREP(PDIV_MASK, rate->pdiv) |
+		  FIELD_PREP(SDIV_MASK, rate->sdiv);
 	writel(div_val, pll->base + DIV_CTL0);
-	writel(rate->kdiv << KDIV_SHIFT, pll->base + DIV_CTL1);
+	writel(FIELD_PREP(KDIV_MASK, rate->kdiv), pll->base + DIV_CTL1);
 
 	/*
 	 * According to SPEC, t3 - t2 need to be greater than
-- 
2.47.3

[PATCH 4/4] clk: imx: pll14xx: hit 393216000 and 361267200 Hz exactly

[Ahmad Fatoum]

The table entries for the two audio PLL rates only approximate them:
they program 393215995 and 361267196 Hz. Linux commit 72d00e560d10
("clk: imx: pll14xx: dynamically configure PLL for
393216000/361267200Hz") dropped them in favor of dynamically computed
dividers that hit the rates exactly.

barebox has no dynamic rate calculation, so replace the entries with
the equivalent hardcoded values that I had listed in the Linux
commit above.

This is visible after boot: Linux finds the requested rate reachable
from barebox's dividers by adjusting kdiv alone, keeps them and ends up
a few Hz short, whereas after U-Boot, which doesn't touch the audio
PLLs, it locked in exactly.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/clk/imx/clk-pll14xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index 939320c0590f..3390a3ffc754 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -64,8 +64,8 @@ static const struct imx_pll14xx_rate_table imx_pll1443x_tbl[] = {
 	PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
 	PLL_1443X_RATE(594000000U, 198, 2, 2, 0),
 	PLL_1443X_RATE(519750000U, 173, 2, 2, 16384),
-	PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
-	PLL_1443X_RATE(361267200U, 361, 3, 3, 17511),
+	PLL_1443X_RATE(393216000U, 655, 5, 3, 23593),
+	PLL_1443X_RATE(361267200U, 497, 33, 0, -16882),
 };
 
 struct imx_pll14xx_clk imx_1443x_pll = {
-- 
2.47.3

Re: [PATCH 1/4] clk: imx: pll14xx: use register defines consistently

[Sascha Hauer]

On Wed, 30 Sep 2026 12:15:53 +0200, Ahmad Fatoum wrote:
> clk: imx: pll14xx: Use register defines consistently
> The driver has defines for the registers, but they are mostly unused.
> Use the defines consistently throughout the driver. While at it rename
> DIV_CTL to DIV_CTL0 because that's the name in the reference manual.
> 
> 

Applied, thanks!

[1/4] clk: imx: pll14xx: use register defines consistently
      https://git.pengutronix.de/cgit/barebox/commit/?id=53213b51d712 (link may not be stable)
[2/4] clk: imx: pll14xx: Drop wrong shifting
      https://git.pengutronix.de/cgit/barebox/commit/?id=a64e07dc04be (link may not be stable)
[3/4] clk: imx: pll14xx: Use FIELD_GET/FIELD_PREP
      https://git.pengutronix.de/cgit/barebox/commit/?id=56f4cf3140af (link may not be stable)
[4/4] clk: imx: pll14xx: hit 393216000 and 361267200 Hz exactly
      https://git.pengutronix.de/cgit/barebox/commit/?id=48151b0fa372 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>