On 2/26/24 2:43 PM, Sumit Garg wrote:
On Mon, 26 Feb 2024 at 18:31, Marek Vasut <ma...@denx.de> wrote:
On 2/26/24 1:39 PM, Sumit Garg wrote:
On Mon, 26 Feb 2024 at 14:24, Marek Vasut <ma...@denx.de> wrote:
On 2/26/24 9:04 AM, Sumit Garg wrote:
Expose the high performance PLL as a regular Linux clock, so the
PCIe PHY can use it when there is no external refclock provided.
Inspired from counterpart Linux kernel v6.8-rc3 driver:
drivers/pmdomain/imx/imx8mp-blk-ctrl.c
Commit ID, please, see previous comments in this series.
[...]
+static int hsio_pll_clk_enable(struct clk *clk)
+{
+ void *base = (void *)dev_get_driver_data(clk->dev);
+ u32 val;
+ int ret;
+
+ /* Setup HSIO PLL */
+ clrsetbits_le32(base + GPR_REG2,
+ P_PLL_MASK | M_PLL_MASK | S_PLL_MASK,
+ FIELD_PREP(P_PLL_MASK, 12) |
+ FIELD_PREP(M_PLL_MASK, 800) |
+ FIELD_PREP(S_PLL_MASK, 4));
These magic numbers 12, 800, 4 could use explanation (why these
numbers?) and a dedicated macro .
I can't find any information in TRM as to how HSIO PLL computation is
done. However, I can extend the comment above to say that this
configuration leads to a 100 MHz PHY clock as per
clk_hsio_pll_recalc_rate() in Linux kernel driver.
Try the CCM section 5.1.5.4.4 SSCG and Fractional PLLs I think,
That section also does mention following:
"The ARM PLL, GPU PLL, VPU PLL, DRAM PLL, Audio PLL1/2, and Video PLL1 are
fractional PLLs. The frequency on these can be tuned to be very
accurate to meet audio
and video interface requirements."
without any mention of HSIO PLL.
this is
likely PLL14xxx from Samsung, so
See ^
f_out = f_in * M / P * 2^S
I think f_in is something like 24 MHz, so 24 * 800 / 12 * 16 = 100 MHz
P - predivider
S - subdivider (output divider)
M - multiplier
IMO, we shouldn't document something based on our analysis but rather
it should be based on facts.
Frankly, all the PLLs in the MX8M are very much similar, so either wait
for someone from NXP to confirm this, or try and find some statement in
the MX8MPRM .
+static int hsio_pll_clk_disable(struct clk *clk)
+{
+ void *base = (void *)dev_get_driver_data(clk->dev);
+
+ clrbits_le32(base + GPR_REG3, PLL_CKE);
+ clrbits_le32(base + GPR_REG3, PLL_RST);
Can you clear both at once, or do they have to be cleared in sequence ?
I generally follow the reverse sequence of how the configuration is
done during hsio_pll_clk_enable(). These bits have seperate functions
related to clock and reset. So I would prefer to keep them as they
are.
Linux does this in drivers/pmdomain/imx/imx8mp-blk-ctrl.c :
120 static void clk_hsio_pll_unprepare(struct clk_hw *hw)
121 {
122 struct clk_hsio_pll *clk = to_clk_hsio_pll(hw);
123
124 regmap_update_bits(clk->regmap, GPR_REG3, PLL_RST | PLL_CKE, 0);
125 }
And just above it:
static int clk_hsio_pll_prepare(struct clk_hw *hw)
{
...
/* de-assert PLL reset */
regmap_update_bits(clk->regmap, GPR_REG3, PLL_RST, PLL_RST);
/* enable PLL */
regmap_update_bits(clk->regmap, GPR_REG3, PLL_CKE, PLL_CKE);
...
}
That's an anti-pattern in the Linux kernel driver which we shouldn't
copy, right?
This is the enable part which is done in sequence, the part I quoted is
the disable part.
