[PATCH 4/7] imx8mp: power-domain: Expose high performance PLL clock

[Sumit Garg]

On Tue, 20 Feb 2024 at 21:02, Marek Vasut <marex at denx.de> wrote:
>
> On 2/20/24 14:10, Sumit Garg wrote:
> > PCIe PHY can use it when there is no external refclock provided.
>
> Commit message needs to be fixed.

How about the following?

    Expose high performance PLL clock, so the PCIe PHY can
    use it when there is no external refclock provided.

>
> > +static int hsio_pll_enable(struct udevice *dev)
> > +{
> > +     struct imx8mp_hsiomix_priv *priv = dev_get_priv(dev);
> > +     unsigned long start;
> > +     u32 val;
> > +
> > +     /* Setup HSIO PLL */
> > +     val = readl(priv->base + GPR_REG2);
> > +     val &= ~(P_PLL_MASK | M_PLL_MASK | S_PLL_MASK);
> > +     val |= (FIELD_PREP(P_PLL_MASK, 12) | FIELD_PREP(M_PLL_MASK, 800) |
> > +             FIELD_PREP(S_PLL_MASK, 4));
> > +     writel(val, priv->base + GPR_REG2);
>
> clrsetbits_le32()

Ack

>
> > +     /* de-assert PLL reset */
> > +     setbits_le32(priv->base + GPR_REG3, PLL_RST);
> > +
> > +     /* enable PLL */
> > +     setbits_le32(priv->base + GPR_REG3, PLL_CKE);
> > +
> > +     /* Check if PLL is locked */
> > +     start = get_timer(0);
>
> wait_for_bit() or readl_poll_timeout()

Let me use readl_poll_timeout() instead.

>
> > +     for (;;) {
> > +             if (readl(priv->base + GPR_REG1) & PLL_LOCK)
> > +                     break;
> > +
> > +             if (get_timer(start) > 100) {
> > +                     dev_err(dev, "failed to lock HSIO PLL\n");
> > +                     return -ETIMEDOUT;
> > +             }
> > +
> > +             udelay(10);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static void hsio_pll_disable(struct udevice *dev)
> > +{
> > +     struct imx8mp_hsiomix_priv *priv = dev_get_priv(dev);
> > +
> > +     /* de-assert PLL reset */
> > +     clrbits_le32(priv->base + GPR_REG3, PLL_RST);
> > +
> > +     /* enable PLL */
> > +     clrbits_le32(priv->base + GPR_REG3, PLL_CKE);
> > +}
> > +
> >   static int imx8mp_hsiomix_on(struct power_domain *power_domain)
> >   {
> >       struct udevice *dev = power_domain->dev;
> > @@ -69,16 +127,23 @@ static int imx8mp_hsiomix_on(struct power_domain *power_domain)
> >       if (ret)
> >               goto err_clk_pcie;
> >
> > -     if (power_domain->id == IMX8MP_HSIOBLK_PD_USB)
> > +     if (power_domain->id == IMX8MP_HSIOBLK_PD_USB) {
> >               setbits_le32(priv->base + GPR_REG0, USB_CLOCK_MODULE_EN);
> > -     else if (power_domain->id == IMX8MP_HSIOBLK_PD_PCIE)
> > +     } else if (power_domain->id == IMX8MP_HSIOBLK_PD_PCIE) {
> >               setbits_le32(priv->base + GPR_REG0, PCIE_CLOCK_MODULE_EN);
> > -     else if (power_domain->id == IMX8MP_HSIOBLK_PD_PCIE_PHY)
> > +     } else if (power_domain->id == IMX8MP_HSIOBLK_PD_PCIE_PHY) {
> >               setbits_le32(priv->base + GPR_REG0, PCIE_PHY_APB_RST |
> >                                                   PCIE_PHY_INIT_RST);
> >
> > +             ret = hsio_pll_enable(dev);
>
> Is this how Linux handles this PLL ?
>
> Seems like this should be either syscon or clock driver .

It isn't similar to what Linux does but I can't find suitable
infrastructure in U-Boot to expose it as a regular clock. Are there
any APIs available similar to devm_of_clk_add_hw_provider() in Linux?

-Sumit

>
> [...]