| Message ID | 20250408025930.1863551-7-hongxing.zhu@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add some enhancements for i.MX95 PCIe | expand |
On Tue, Apr 08, 2025 at 10:59:29AM +0800, Richard Zhu wrote: > Add PLL clock lock check for i.MX95 PCIe. > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > Reviewed-by: Frank Li <Frank.Li@nxp.com> > --- > drivers/pci/controller/dwc/pci-imx6.c | 28 +++++++++++++++++++++++++-- > 1 file changed, 26 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index 7dcc9d88740d..c1d128ec255d 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -45,6 +45,9 @@ > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > +#define IMX95_PCIE_PHY_MPLLA_CTRL 0x10 > +#define IMX95_PCIE_PHY_MPLL_STATE BIT(30) > + > #define IMX95_PCIE_SS_RW_REG_0 0xf0 > #define IMX95_PCIE_REF_CLKEN BIT(23) > #define IMX95_PCIE_PHY_CR_PARA_SEL BIT(9) > @@ -479,6 +482,23 @@ static void imx7d_pcie_wait_for_phy_pll_lock(struct imx_pcie *imx_pcie) > dev_err(dev, "PCIe PLL lock timeout\n"); > } > > +static int imx95_pcie_wait_for_phy_pll_lock(struct imx_pcie *imx_pcie) > +{ > + u32 val; > + struct device *dev = imx_pcie->pci->dev; > + > + if (regmap_read_poll_timeout(imx_pcie->iomuxc_gpr, > + IMX95_PCIE_PHY_MPLLA_CTRL, val, > + val & IMX95_PCIE_PHY_MPLL_STATE, > + PHY_PLL_LOCK_WAIT_USLEEP_MAX, > + PHY_PLL_LOCK_WAIT_TIMEOUT)) { > + dev_err(dev, "PCIe PLL lock timeout\n"); > + return -ETIMEDOUT; > + } > + > + return 0; > +} > + > static int imx_setup_phy_mpll(struct imx_pcie *imx_pcie) > { > unsigned long phy_rate = 0; > @@ -824,6 +844,8 @@ static int imx95_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert) > regmap_read_bypassed(imx_pcie->iomuxc_gpr, IMX95_PCIE_RST_CTRL, > &val); > udelay(10); > + } else { > + return imx95_pcie_wait_for_phy_pll_lock(imx_pcie); Is this PLL lock related to COLD_RESET? It doesn't look like it. If unrelated, it should be called wherever required. imx95_pcie_core_reset() is supposed to only assert/deassert the COLD_RESET. If related, please explain how. - Mani
> -----Original Message----- > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > Sent: 2025年4月13日 23:33 > To: Hongxing Zhu <hongxing.zhu@nxp.com> > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org; > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de; > kernel@pengutronix.de; festevam@gmail.com; linux-pci@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH v5 6/7] PCI: imx6: Add PLL clock lock check for i.MX95 > PCIe > > On Tue, Apr 08, 2025 at 10:59:29AM +0800, Richard Zhu wrote: > > Add PLL clock lock check for i.MX95 PCIe. > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > Reviewed-by: Frank Li <Frank.Li@nxp.com> > > --- > > drivers/pci/controller/dwc/pci-imx6.c | 28 > > +++++++++++++++++++++++++-- > > 1 file changed, 26 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > b/drivers/pci/controller/dwc/pci-imx6.c > > index 7dcc9d88740d..c1d128ec255d 100644 > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > @@ -45,6 +45,9 @@ > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > > > +#define IMX95_PCIE_PHY_MPLLA_CTRL 0x10 > > +#define IMX95_PCIE_PHY_MPLL_STATE BIT(30) > > + > > #define IMX95_PCIE_SS_RW_REG_0 0xf0 > > #define IMX95_PCIE_REF_CLKEN BIT(23) > > #define IMX95_PCIE_PHY_CR_PARA_SEL BIT(9) > > @@ -479,6 +482,23 @@ static void > imx7d_pcie_wait_for_phy_pll_lock(struct imx_pcie *imx_pcie) > > dev_err(dev, "PCIe PLL lock timeout\n"); } > > > > +static int imx95_pcie_wait_for_phy_pll_lock(struct imx_pcie > > +*imx_pcie) { > > + u32 val; > > + struct device *dev = imx_pcie->pci->dev; > > + > > + if (regmap_read_poll_timeout(imx_pcie->iomuxc_gpr, > > + IMX95_PCIE_PHY_MPLLA_CTRL, val, > > + val & IMX95_PCIE_PHY_MPLL_STATE, > > + PHY_PLL_LOCK_WAIT_USLEEP_MAX, > > + PHY_PLL_LOCK_WAIT_TIMEOUT)) { > > + dev_err(dev, "PCIe PLL lock timeout\n"); > > + return -ETIMEDOUT; > > + } > > + > > + return 0; > > +} > > + > > static int imx_setup_phy_mpll(struct imx_pcie *imx_pcie) { > > unsigned long phy_rate = 0; > > @@ -824,6 +844,8 @@ static int imx95_pcie_core_reset(struct imx_pcie > *imx_pcie, bool assert) > > regmap_read_bypassed(imx_pcie->iomuxc_gpr, > IMX95_PCIE_RST_CTRL, > > &val); > > udelay(10); > > + } else { > > + return imx95_pcie_wait_for_phy_pll_lock(imx_pcie); > > Is this PLL lock related to COLD_RESET? It doesn't look like it. If unrelated, it > should be called wherever required. imx95_pcie_core_reset() is supposed to > only assert/deassert the COLD_RESET. > > If related, please explain how. Thanks for your kindly review. To make sure the HW state is correct to continue the sequential initializations. The PLL lock or not check would be kicked off after the COLD_RESET is de-asserted for i.MX95 PCIe. So, the PLL lock check is added at the end of de-assertion in imx95_pcie_core_reset() function. Best Regards Richard Zhu > > - Mani > > -- > மணிவண்ணன் சதாசிவம்
On Mon, Apr 14, 2025 at 03:16:46AM +0000, Hongxing Zhu wrote: > > -----Original Message----- > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Sent: 2025年4月13日 23:33 > > To: Hongxing Zhu <hongxing.zhu@nxp.com> > > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; > > lpieralisi@kernel.org; kw@linux.com; robh@kernel.org; > > bhelgaas@google.com; shawnguo@kernel.org; s.hauer@pengutronix.de; > > kernel@pengutronix.de; festevam@gmail.com; linux-pci@vger.kernel.org; > > linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev; > > linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v5 6/7] PCI: imx6: Add PLL clock lock check for i.MX95 > > PCIe > > > > On Tue, Apr 08, 2025 at 10:59:29AM +0800, Richard Zhu wrote: > > > Add PLL clock lock check for i.MX95 PCIe. > > > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > > Reviewed-by: Frank Li <Frank.Li@nxp.com> > > > --- > > > drivers/pci/controller/dwc/pci-imx6.c | 28 > > > +++++++++++++++++++++++++-- > > > 1 file changed, 26 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > > b/drivers/pci/controller/dwc/pci-imx6.c > > > index 7dcc9d88740d..c1d128ec255d 100644 > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > @@ -45,6 +45,9 @@ > > > #define IMX95_PCIE_PHY_GEN_CTRL 0x0 > > > #define IMX95_PCIE_REF_USE_PAD BIT(17) > > > > > > +#define IMX95_PCIE_PHY_MPLLA_CTRL 0x10 > > > +#define IMX95_PCIE_PHY_MPLL_STATE BIT(30) > > > + > > > #define IMX95_PCIE_SS_RW_REG_0 0xf0 > > > #define IMX95_PCIE_REF_CLKEN BIT(23) > > > #define IMX95_PCIE_PHY_CR_PARA_SEL BIT(9) > > > @@ -479,6 +482,23 @@ static void > > imx7d_pcie_wait_for_phy_pll_lock(struct imx_pcie *imx_pcie) > > > dev_err(dev, "PCIe PLL lock timeout\n"); } > > > > > > +static int imx95_pcie_wait_for_phy_pll_lock(struct imx_pcie > > > +*imx_pcie) { > > > + u32 val; > > > + struct device *dev = imx_pcie->pci->dev; > > > + > > > + if (regmap_read_poll_timeout(imx_pcie->iomuxc_gpr, > > > + IMX95_PCIE_PHY_MPLLA_CTRL, val, > > > + val & IMX95_PCIE_PHY_MPLL_STATE, > > > + PHY_PLL_LOCK_WAIT_USLEEP_MAX, > > > + PHY_PLL_LOCK_WAIT_TIMEOUT)) { > > > + dev_err(dev, "PCIe PLL lock timeout\n"); > > > + return -ETIMEDOUT; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > static int imx_setup_phy_mpll(struct imx_pcie *imx_pcie) { > > > unsigned long phy_rate = 0; > > > @@ -824,6 +844,8 @@ static int imx95_pcie_core_reset(struct imx_pcie > > *imx_pcie, bool assert) > > > regmap_read_bypassed(imx_pcie->iomuxc_gpr, > > IMX95_PCIE_RST_CTRL, > > > &val); > > > udelay(10); > > > + } else { > > > + return imx95_pcie_wait_for_phy_pll_lock(imx_pcie); > > > > Is this PLL lock related to COLD_RESET? It doesn't look like it. If unrelated, it > > should be called wherever required. imx95_pcie_core_reset() is supposed to > > only assert/deassert the COLD_RESET. > > > > If related, please explain how. > Thanks for your kindly review. > To make sure the HW state is correct to continue the sequential initializations. > The PLL lock or not check would be kicked off after the COLD_RESET is > de-asserted for i.MX95 PCIe. > So, the PLL lock check is added at the end of de-assertion in > imx95_pcie_core_reset() function. > But imx95_pcie_core_reset() is not doing anything for deassert other than waiting for PLL lock. Hence my question. - Mani
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index 7dcc9d88740d..c1d128ec255d 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -45,6 +45,9 @@ #define IMX95_PCIE_PHY_GEN_CTRL 0x0 #define IMX95_PCIE_REF_USE_PAD BIT(17) +#define IMX95_PCIE_PHY_MPLLA_CTRL 0x10 +#define IMX95_PCIE_PHY_MPLL_STATE BIT(30) + #define IMX95_PCIE_SS_RW_REG_0 0xf0 #define IMX95_PCIE_REF_CLKEN BIT(23) #define IMX95_PCIE_PHY_CR_PARA_SEL BIT(9) @@ -479,6 +482,23 @@ static void imx7d_pcie_wait_for_phy_pll_lock(struct imx_pcie *imx_pcie) dev_err(dev, "PCIe PLL lock timeout\n"); } +static int imx95_pcie_wait_for_phy_pll_lock(struct imx_pcie *imx_pcie) +{ + u32 val; + struct device *dev = imx_pcie->pci->dev; + + if (regmap_read_poll_timeout(imx_pcie->iomuxc_gpr, + IMX95_PCIE_PHY_MPLLA_CTRL, val, + val & IMX95_PCIE_PHY_MPLL_STATE, + PHY_PLL_LOCK_WAIT_USLEEP_MAX, + PHY_PLL_LOCK_WAIT_TIMEOUT)) { + dev_err(dev, "PCIe PLL lock timeout\n"); + return -ETIMEDOUT; + } + + return 0; +} + static int imx_setup_phy_mpll(struct imx_pcie *imx_pcie) { unsigned long phy_rate = 0; @@ -824,6 +844,8 @@ static int imx95_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert) regmap_read_bypassed(imx_pcie->iomuxc_gpr, IMX95_PCIE_RST_CTRL, &val); udelay(10); + } else { + return imx95_pcie_wait_for_phy_pll_lock(imx_pcie); } return 0; @@ -843,11 +865,13 @@ static void imx_pcie_assert_core_reset(struct imx_pcie *imx_pcie) static int imx_pcie_deassert_core_reset(struct imx_pcie *imx_pcie) { + int ret = 0; + reset_control_deassert(imx_pcie->pciephy_reset); reset_control_deassert(imx_pcie->apps_reset); if (imx_pcie->drvdata->core_reset) - imx_pcie->drvdata->core_reset(imx_pcie, false); + ret = imx_pcie->drvdata->core_reset(imx_pcie, false); /* Some boards don't have PCIe reset GPIO. */ if (imx_pcie->reset_gpiod) { @@ -857,7 +881,7 @@ static int imx_pcie_deassert_core_reset(struct imx_pcie *imx_pcie) msleep(100); } - return 0; + return ret; } static int imx_pcie_wait_for_speed_change(struct imx_pcie *imx_pcie)