| Message ID | 20221012132634.267970-1-s.hauer@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: imx6: Fix link initialisation when the phy is ref clk provider | expand |
Would you mind rewording the subject so it says something more specific about what the patch does? E.g., PCI: imx6: Initialize PHY before deasserting core reset It may be that this ordering is only *required* when the PHY is the ref clk provider, but the patch doesn't test for that; it *always* initializes the PHY first. On Wed, Oct 12, 2022 at 03:26:34PM +0200, Sascha Hauer wrote: > When the phy is the reference clock provider then it must be initialised > and powered on before the reset on the client is deasserted, otherwise > the link will never come up. The order was changed in cf236e0c0d59. > Restore the correct order to make the driver work again on boards where > the phy provides the reference clock. s/phy/PHY/ several places above > Fixes: cf236e0c0d59 ("PCI: imx6: Do not hide PHY driver callbacks and refine the error handling") > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/pci/controller/dwc/pci-imx6.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index b5f0de455a7bd..211eb55d6d34b 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -942,12 +942,6 @@ static int imx6_pcie_host_init(struct dw_pcie_rp *pp) > } > } > > - ret = imx6_pcie_deassert_core_reset(imx6_pcie); > - if (ret < 0) { > - dev_err(dev, "pcie deassert core reset failed: %d\n", ret); > - goto err_phy_off; > - } > - > if (imx6_pcie->phy) { > ret = phy_power_on(imx6_pcie->phy); > if (ret) { > @@ -955,6 +949,13 @@ static int imx6_pcie_host_init(struct dw_pcie_rp *pp) > goto err_phy_off; > } > } > + > + ret = imx6_pcie_deassert_core_reset(imx6_pcie); > + if (ret < 0) { > + dev_err(dev, "pcie deassert core reset failed: %d\n", ret); > + goto err_phy_off; > + } > + > imx6_setup_phy_mpll(imx6_pcie); > > return 0; > -- > 2.30.2 >
On Thu, Oct 13, 2022 at 09:04:50AM -0500, Bjorn Helgaas wrote: > Would you mind rewording the subject so it says something more > specific about what the patch does? E.g., > > PCI: imx6: Initialize PHY before deasserting core reset Ok. > > It may be that this ordering is only *required* when the PHY is the > ref clk provider, but the patch doesn't test for that; it *always* > initializes the PHY first. Yes, it is only required when the PHY is the ref clk provider. It shouldn't hurt though when the ref clk is generated elsewhere. The PCI host driver doesn't know about the ref clk provider, that knowledge is limited to the phy driver only. Sascha > > On Wed, Oct 12, 2022 at 03:26:34PM +0200, Sascha Hauer wrote: > > When the phy is the reference clock provider then it must be initialised > > and powered on before the reset on the client is deasserted, otherwise > > the link will never come up. The order was changed in cf236e0c0d59. > > Restore the correct order to make the driver work again on boards where > > the phy provides the reference clock. > > s/phy/PHY/ several places above > > > Fixes: cf236e0c0d59 ("PCI: imx6: Do not hide PHY driver callbacks and refine the error handling") > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > drivers/pci/controller/dwc/pci-imx6.c | 13 +++++++------ > > 1 file changed, 7 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > index b5f0de455a7bd..211eb55d6d34b 100644 > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > @@ -942,12 +942,6 @@ static int imx6_pcie_host_init(struct dw_pcie_rp *pp) > > } > > } > > > > - ret = imx6_pcie_deassert_core_reset(imx6_pcie); > > - if (ret < 0) { > > - dev_err(dev, "pcie deassert core reset failed: %d\n", ret); > > - goto err_phy_off; > > - } > > - > > if (imx6_pcie->phy) { > > ret = phy_power_on(imx6_pcie->phy); > > if (ret) { > > @@ -955,6 +949,13 @@ static int imx6_pcie_host_init(struct dw_pcie_rp *pp) > > goto err_phy_off; > > } > > } > > + > > + ret = imx6_pcie_deassert_core_reset(imx6_pcie); > > + if (ret < 0) { > > + dev_err(dev, "pcie deassert core reset failed: %d\n", ret); > > + goto err_phy_off; > > + } > > + > > imx6_setup_phy_mpll(imx6_pcie); > > > > return 0; > > -- > > 2.30.2 > > >
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index b5f0de455a7bd..211eb55d6d34b 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -942,12 +942,6 @@ static int imx6_pcie_host_init(struct dw_pcie_rp *pp) } } - ret = imx6_pcie_deassert_core_reset(imx6_pcie); - if (ret < 0) { - dev_err(dev, "pcie deassert core reset failed: %d\n", ret); - goto err_phy_off; - } - if (imx6_pcie->phy) { ret = phy_power_on(imx6_pcie->phy); if (ret) { @@ -955,6 +949,13 @@ static int imx6_pcie_host_init(struct dw_pcie_rp *pp) goto err_phy_off; } } + + ret = imx6_pcie_deassert_core_reset(imx6_pcie); + if (ret < 0) { + dev_err(dev, "pcie deassert core reset failed: %d\n", ret); + goto err_phy_off; + } + imx6_setup_phy_mpll(imx6_pcie); return 0;
When the phy is the reference clock provider then it must be initialised and powered on before the reset on the client is deasserted, otherwise the link will never come up. The order was changed in cf236e0c0d59. Restore the correct order to make the driver work again on boards where the phy provides the reference clock. Fixes: cf236e0c0d59 ("PCI: imx6: Do not hide PHY driver callbacks and refine the error handling") Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/pci/controller/dwc/pci-imx6.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)