> -----Original Message-----
> From: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> Sent: Saturday, October 16, 2021 2:50 AM
> To: Richard Zhu <hongxing.zhu@xxxxxxx>
> Cc: l.stach@xxxxxxxxxxxxxx; bhelgaas@xxxxxxxxxx;
> lorenzo.pieralisi@xxxxxxx; linux-pci@xxxxxxxxxxxxxxx; dl-linux-imx
> <linux-imx@xxxxxxx>; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx
> Subject: Re: [RESEND v2 4/5] PCI: imx6: Fix the clock reference handling
> unbalance when link never came up
>
> On Fri, Oct 15, 2021 at 02:05:40PM +0800, Richard Zhu wrote:
> > When link never came up, driver probe would be failed with error -110.
> > To keep usage counter balance of the clocks, disable the previous
> > enabled clocks when link is down.
> > Move definitions of the imx6_pcie_clk_disable() function to the proper
> > place. Because it wouldn't be used in imx6_pcie_suspend_noirq() only.
>
> Add blank line between paragraphs.
>
> Can you please split this into two patches:
>
> 1) the imx6_pcie_clk_disable() move
> 2) the actual fix
>
> It's hard to tell exactly where the fix is when things are mixed together.
>
[Richard Zhu] Okay, would split this patch into two patches later.
One is used for imx6_pcie_clk_disable() move, and no function changes.
The other one is the actual fix. Thanks.
Best Regards
Richard Zhu
> > Signed-off-by: Richard Zhu <hongxing.zhu@xxxxxxx>
> > ---
> > drivers/pci/controller/dwc/pci-imx6.c | 47
> > ++++++++++++++-------------
> > 1 file changed, 24 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index cc837f8bf6d4..d6a5d99ffa52 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -514,6 +514,29 @@ static int imx6_pcie_clk_enable(struct imx6_pcie
> *imx6_pcie)
> > return ret;
> > }
> >
> > +static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie) {
> > + clk_disable_unprepare(imx6_pcie->pcie);
> > + clk_disable_unprepare(imx6_pcie->pcie_phy);
> > + clk_disable_unprepare(imx6_pcie->pcie_bus);
> > +
> > + switch (imx6_pcie->drvdata->variant) {
> > + case IMX6SX:
> > + clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> > + break;
> > + case IMX7D:
> > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > + IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> > + IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> > + break;
> > + case IMX8MQ:
> > + clk_disable_unprepare(imx6_pcie->pcie_aux);
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie
> > *imx6_pcie) {
> > u32 val;
> > @@ -853,6 +876,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);
> > + imx6_pcie_clk_disable(imx6_pcie);
> > if (imx6_pcie->vpcie && regulator_is_enabled(imx6_pcie->vpcie) > 0)
> > regulator_disable(imx6_pcie->vpcie);
> > return ret;
> > @@ -941,29 +965,6 @@ static void imx6_pcie_pm_turnoff(struct
> imx6_pcie *imx6_pcie)
> > usleep_range(1000, 10000);
> > }
> >
> > -static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie) -{
> > - clk_disable_unprepare(imx6_pcie->pcie);
> > - clk_disable_unprepare(imx6_pcie->pcie_phy);
> > - clk_disable_unprepare(imx6_pcie->pcie_bus);
> > -
> > - switch (imx6_pcie->drvdata->variant) {
> > - case IMX6SX:
> > - clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> > - break;
> > - case IMX7D:
> > - regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> > - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> > - break;
> > - case IMX8MQ:
> > - clk_disable_unprepare(imx6_pcie->pcie_aux);
> > - break;
> > - default:
> > - break;
> > - }
> > -}
> > -
> > static int imx6_pcie_suspend_noirq(struct device *dev) {
> > struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> > --
> > 2.25.1
> >
