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. > > + /* de-assert PLL reset */ > > + setbits_le32(base + GPR_REG3, PLL_RST); > > + > > + /* enable PLL */ > > + setbits_le32(base + GPR_REG3, PLL_CKE); > > + > > + /* Check if PLL is locked */ > > + ret = readl_poll_sleep_timeout(base + GPR_REG1, val, > > + val & PLL_LOCK, 10, 100000); > > + if (ret) > > + dev_err(clk->dev, "failed to lock HSIO PLL\n"); > > + > > + return ret; > > +} > > + > > +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. > > [...] > > The series is starting to look much better compared to V1 btw , these ^ > are only nitpicks . Thanks for your detailed review, I appreciate it. -Sumit