| Message ID | 1655110538-10914-7-git-send-email-hongxing.zhu@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI: imx6: refine codes and add the error propagation | expand |
On Mon, Jun 13, 2022 at 04:55:37PM +0800, Richard Zhu wrote: > let the driver probe successfully, return zero in imx6_pcie_start_link() > when PCIe link is down. > > Because i.MX PCIe doesn't support hot-plug feature. > In this link down scenario, only start the PCIe link training in resume > when the link is up before system suspend to avoid the long latency in > the link training period. This looks like two changes that should be separate patches: 1) Make driver probe successful even if link is down. 2) Reduce resume time by only starting the link if it was up before suspend. > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > --- > drivers/pci/controller/dwc/pci-imx6.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index c02748393aac..ac6ec2d691a0 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -67,6 +67,7 @@ struct imx6_pcie { > struct dw_pcie *pci; > int reset_gpio; > bool gpio_active_high; > + bool link_is_up; > struct clk *pcie_bus; > struct clk *pcie_phy; > struct clk *pcie_inbound_axi; > @@ -845,7 +846,9 @@ static int imx6_pcie_start_link(struct dw_pcie *pci) > /* Start LTSSM. */ > imx6_pcie_ltssm_enable(dev); > > - dw_pcie_wait_for_link(pci); > + ret = dw_pcie_wait_for_link(pci); > + if (ret) > + goto err_reset_phy; > > if (pci->link_gen == 2) { > /* Allow Gen2 mode after the link is up. */ > @@ -881,11 +884,14 @@ static int imx6_pcie_start_link(struct dw_pcie *pci) > } > > /* Make sure link training is finished as well! */ > - dw_pcie_wait_for_link(pci); > + ret = dw_pcie_wait_for_link(pci); > + if (ret) > + goto err_reset_phy; > } else { > dev_info(dev, "Link: Gen2 disabled\n"); > } > > + imx6_pcie->link_is_up = true; > tmp = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA); > dev_info(dev, "Link up, Gen%i\n", tmp & PCI_EXP_LNKSTA_CLS); > return 0; > @@ -895,7 +901,7 @@ static int imx6_pcie_start_link(struct dw_pcie *pci) > dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0), > dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG1)); > imx6_pcie_reset_phy(imx6_pcie); > - return ret; > + return 0; > } > > static int imx6_pcie_host_init(struct pcie_port *pp) > @@ -1022,10 +1028,8 @@ static int imx6_pcie_resume_noirq(struct device *dev) > imx6_pcie_init_phy(imx6_pcie); > imx6_pcie_deassert_core_reset(imx6_pcie); > dw_pcie_setup_rc(pp); > - > - ret = imx6_pcie_start_link(imx6_pcie->pci); > - if (ret < 0) > - dev_info(dev, "pcie link is down after resume.\n"); > + if (imx6_pcie->link_is_up) > + imx6_pcie_start_link(imx6_pcie->pci); > > return 0; > } > -- > 2.25.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> -----Original Message----- > From: Bjorn Helgaas <helgaas@kernel.org> > Sent: 2022年6月14日 6:21 > To: Hongxing Zhu <hongxing.zhu@nxp.com> > Cc: l.stach@pengutronix.de; bhelgaas@google.com; robh+dt@kernel.org; > broonie@kernel.org; lorenzo.pieralisi@arm.com; festevam@gmail.com; > francesco.dolcini@toradex.com; linux-pci@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com> > Subject: Re: [PATCH v10 6/7] PCI: imx6: Mark the link down as none fatal error > > On Mon, Jun 13, 2022 at 04:55:37PM +0800, Richard Zhu wrote: > > let the driver probe successfully, return zero in > > imx6_pcie_start_link() when PCIe link is down. > > > > Because i.MX PCIe doesn't support hot-plug feature. > > In this link down scenario, only start the PCIe link training in > > resume when the link is up before system suspend to avoid the long > > latency in the link training period. > > This looks like two changes that should be separate patches: > > 1) Make driver probe successful even if link is down. > > 2) Reduce resume time by only starting the link if it was up before > suspend. Yes, you're right. They are two changes. Would split them into two patches later. Thanks a lot for your kindly help. Best Regards Richard Zhu > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > --- > > drivers/pci/controller/dwc/pci-imx6.c | 18 +++++++++++------- > > 1 file changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > b/drivers/pci/controller/dwc/pci-imx6.c > > index c02748393aac..ac6ec2d691a0 100644 > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > @@ -67,6 +67,7 @@ struct imx6_pcie { > > struct dw_pcie *pci; > > int reset_gpio; > > bool gpio_active_high; > > + bool link_is_up; > > struct clk *pcie_bus; > > struct clk *pcie_phy; > > struct clk *pcie_inbound_axi; > > @@ -845,7 +846,9 @@ static int imx6_pcie_start_link(struct dw_pcie *pci) > > /* Start LTSSM. */ > > imx6_pcie_ltssm_enable(dev); > > > > - dw_pcie_wait_for_link(pci); > > + ret = dw_pcie_wait_for_link(pci); > > + if (ret) > > + goto err_reset_phy; > > > > if (pci->link_gen == 2) { > > /* Allow Gen2 mode after the link is up. */ @@ -881,11 +884,14 > @@ > > static int imx6_pcie_start_link(struct dw_pcie *pci) > > } > > > > /* Make sure link training is finished as well! */ > > - dw_pcie_wait_for_link(pci); > > + ret = dw_pcie_wait_for_link(pci); > > + if (ret) > > + goto err_reset_phy; > > } else { > > dev_info(dev, "Link: Gen2 disabled\n"); > > } > > > > + imx6_pcie->link_is_up = true; > > tmp = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA); > > dev_info(dev, "Link up, Gen%i\n", tmp & PCI_EXP_LNKSTA_CLS); > > return 0; > > @@ -895,7 +901,7 @@ static int imx6_pcie_start_link(struct dw_pcie *pci) > > dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0), > > dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG1)); > > imx6_pcie_reset_phy(imx6_pcie); > > - return ret; > > + return 0; > > } > > > > static int imx6_pcie_host_init(struct pcie_port *pp) @@ -1022,10 > > +1028,8 @@ static int imx6_pcie_resume_noirq(struct device *dev) > > imx6_pcie_init_phy(imx6_pcie); > > imx6_pcie_deassert_core_reset(imx6_pcie); > > dw_pcie_setup_rc(pp); > > - > > - ret = imx6_pcie_start_link(imx6_pcie->pci); > > - if (ret < 0) > > - dev_info(dev, "pcie link is down after resume.\n"); > > + if (imx6_pcie->link_is_up) > > + imx6_pcie_start_link(imx6_pcie->pci); > > > > return 0; > > } > > -- > > 2.25.1 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=05%7 > C0 > > > 1%7Chongxing.zhu%40nxp.com%7Ce77b8f84d1cf4a0b105808da4d8b0e18%7 > C686ea1 > > > d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637907556889007090%7CUnk > nown%7CTW > > > FpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXV > CI6 > > > Mn0%3D%7C3000%7C%7C%7C&sdata=vd0JWz9d5xDP4MpPNTsq5UG2L > gmx%2B%2FrNE > > tnF4UsZKCM%3D&reserved=0
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index c02748393aac..ac6ec2d691a0 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -67,6 +67,7 @@ struct imx6_pcie { struct dw_pcie *pci; int reset_gpio; bool gpio_active_high; + bool link_is_up; struct clk *pcie_bus; struct clk *pcie_phy; struct clk *pcie_inbound_axi; @@ -845,7 +846,9 @@ static int imx6_pcie_start_link(struct dw_pcie *pci) /* Start LTSSM. */ imx6_pcie_ltssm_enable(dev); - dw_pcie_wait_for_link(pci); + ret = dw_pcie_wait_for_link(pci); + if (ret) + goto err_reset_phy; if (pci->link_gen == 2) { /* Allow Gen2 mode after the link is up. */ @@ -881,11 +884,14 @@ static int imx6_pcie_start_link(struct dw_pcie *pci) } /* Make sure link training is finished as well! */ - dw_pcie_wait_for_link(pci); + ret = dw_pcie_wait_for_link(pci); + if (ret) + goto err_reset_phy; } else { dev_info(dev, "Link: Gen2 disabled\n"); } + imx6_pcie->link_is_up = true; tmp = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA); dev_info(dev, "Link up, Gen%i\n", tmp & PCI_EXP_LNKSTA_CLS); return 0; @@ -895,7 +901,7 @@ static int imx6_pcie_start_link(struct dw_pcie *pci) dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0), dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG1)); imx6_pcie_reset_phy(imx6_pcie); - return ret; + return 0; } static int imx6_pcie_host_init(struct pcie_port *pp) @@ -1022,10 +1028,8 @@ static int imx6_pcie_resume_noirq(struct device *dev) imx6_pcie_init_phy(imx6_pcie); imx6_pcie_deassert_core_reset(imx6_pcie); dw_pcie_setup_rc(pp); - - ret = imx6_pcie_start_link(imx6_pcie->pci); - if (ret < 0) - dev_info(dev, "pcie link is down after resume.\n"); + if (imx6_pcie->link_is_up) + imx6_pcie_start_link(imx6_pcie->pci); return 0; }
let the driver probe successfully, return zero in imx6_pcie_start_link() when PCIe link is down. Because i.MX PCIe doesn't support hot-plug feature. In this link down scenario, only start the PCIe link training in resume when the link is up before system suspend to avoid the long latency in the link training period. Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> --- drivers/pci/controller/dwc/pci-imx6.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)