[PATCH] PCI: imx6: Override the CLKREQ low in the initialization

[PATCH] PCI: imx6: Override the CLKREQ low in the initialization

[Richard Zhu]

The CLKREQ# signal is an open drain, active low signal that is driven
low by the remote Endpoint device. But it might not be driven low if no
Endpoint device is connected.

On i.MX8MM PCIe, phy_init() would be failed and system boot hang if the
reference clock is not toggled.

Follow with i.MX8MQ PCIe, to make sure the reference clock on, override
the CLKREQ# low during initialization to fix this issue.

Fixes: 178e244cb6e2 ("PCI: imx: Add the imx8mm pcie support")
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index d7f0db01f3c3..a334341a1789 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -447,10 +447,6 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
 	case IMX7D:
 		break;
 	case IMX8MM:
-		ret = clk_prepare_enable(imx6_pcie->pcie_aux);
-		if (ret)
-			dev_err(dev, "unable to enable pcie_aux clock\n");
-		break;
 	case IMX8MQ:
 		ret = clk_prepare_enable(imx6_pcie->pcie_aux);
 		if (ret) {
-- 
2.25.1

Re: [PATCH] PCI: imx6: Override the CLKREQ low in the initialization

[Bjorn Helgaas]

In subject, maybe:

  PCI: imx6: Assert i.MX8MM CLKREQ# even if no device present

On Wed, Feb 23, 2022 at 04:38:33PM +0800, Richard Zhu wrote:
> The CLKREQ# signal is an open drain, active low signal that is driven
> low by the remote Endpoint device. But it might not be driven low if no
> Endpoint device is connected.
> 
> On i.MX8MM PCIe, phy_init() would be failed and system boot hang if the
> reference clock is not toggled.

s/would be failed/may fail/
s/boot hang/boot may hang/
s/if the reference clock is not toggled/if no Endpoint is connected to assert CLKREQ#/

> Follow with i.MX8MQ PCIe, to make sure the reference clock on, override
> the CLKREQ# low during initialization to fix this issue.

  Handle this as on i.MX8MQ, where we explicitly assert CLKREQ# so the
  PHY can be initialized.

(I don't really understand the hardware here, so disregard if this
doesn't say what it needs to.)

> Fixes: 178e244cb6e2 ("PCI: imx: Add the imx8mm pcie support")
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index d7f0db01f3c3..a334341a1789 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -447,10 +447,6 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  	case IMX7D:
>  		break;
>  	case IMX8MM:
> -		ret = clk_prepare_enable(imx6_pcie->pcie_aux);
> -		if (ret)
> -			dev_err(dev, "unable to enable pcie_aux clock\n");
> -		break;
>  	case IMX8MQ:
>  		ret = clk_prepare_enable(imx6_pcie->pcie_aux);
>  		if (ret) {
> -- 
> 2.25.1
> 

RE: [PATCH] PCI: imx6: Override the CLKREQ low in the initialization

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2022年2月24日 2:32
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> lorenzo.pieralisi@arm.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] PCI: imx6: Override the CLKREQ low in the initialization
> 
> In subject, maybe:
> 
>   PCI: imx6: Assert i.MX8MM CLKREQ# even if no device present
> 
> On Wed, Feb 23, 2022 at 04:38:33PM +0800, Richard Zhu wrote:
> > The CLKREQ# signal is an open drain, active low signal that is driven
> > low by the remote Endpoint device. But it might not be driven low if
> > no Endpoint device is connected.
> >
> > On i.MX8MM PCIe, phy_init() would be failed and system boot hang if
> > the reference clock is not toggled.
> 
> s/would be failed/may fail/
> s/boot hang/boot may hang/
> s/if the reference clock is not toggled/if no Endpoint is connected to assert
> CLKREQ#/
> 
> > Follow with i.MX8MQ PCIe, to make sure the reference clock on,
> > override the CLKREQ# low during initialization to fix this issue.
> 
>   Handle this as on i.MX8MQ, where we explicitly assert CLKREQ# so the
>   PHY can be initialized.
> 
> (I don't really understand the hardware here, so disregard if this doesn't say
> what it needs to.)
Hi Bjorn:
Thanks a lot.
Your description is much better than mine.
Would be changed later.

Best Regards
Richard Zhu

> 
> > Fixes: 178e244cb6e2 ("PCI: imx: Add the imx8mm pcie support")
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index d7f0db01f3c3..a334341a1789 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -447,10 +447,6 @@ static int imx6_pcie_enable_ref_clk(struct
> imx6_pcie *imx6_pcie)
> >  	case IMX7D:
> >  		break;
> >  	case IMX8MM:
> > -		ret = clk_prepare_enable(imx6_pcie->pcie_aux);
> > -		if (ret)
> > -			dev_err(dev, "unable to enable pcie_aux clock\n");
> > -		break;
> >  	case IMX8MQ:
> >  		ret = clk_prepare_enable(imx6_pcie->pcie_aux);
> >  		if (ret) {
> > --
> > 2.25.1
> >