[PATCH] PCI: qcom: don't clear out PHY_REFCLK_USE_PAD

[PATCH] PCI: qcom: don't clear out PHY_REFCLK_USE_PAD

[Ilia Mirkin]

This makes PCIe links come up again on ifc6410 (apq8064).

Fixes: de3c4bf6489 ("PCI: qcom: Add support for tx term offset for rev 2.1.0")
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 3aac77a295ba..985b11cf6481 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -387,7 +387,6 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 
 	/* enable external reference clock */
 	val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
-	val &= ~PHY_REFCLK_USE_PAD;
 	val |= PHY_REFCLK_SSP_EN;
 	writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
 
-- 
2.26.2

R: [PATCH] PCI: qcom: don't clear out PHY_REFCLK_USE_PAD

[ansuelsmth]

> -----Messaggio originale-----
> Da: Ilia Mirkin <ibmirkin@gmail.com> Per conto di Ilia Mirkin
> Inviato: lunedì 7 settembre 2020 03:13
> A: ansuelsmth@gmail.com
> Cc: linux-arm-msm@vger.kernel.org; linux-pci@vger.kernel.org; Ilia Mirkin
> <imirkin@alum.mit.edu>
> Oggetto: [PATCH] PCI: qcom: don't clear out PHY_REFCLK_USE_PAD
> 
> This makes PCIe links come up again on ifc6410 (apq8064).
> 
> Fixes: de3c4bf6489 ("PCI: qcom: Add support for tx term offset for rev
> 2.1.0")
> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> index 3aac77a295ba..985b11cf6481 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -387,7 +387,6 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie
> *pcie)
> 
>  	/* enable external reference clock */
>  	val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> -	val &= ~PHY_REFCLK_USE_PAD;

To make sure this doesn't brake ipq806x, why not limit the &= to the ipq806x
compatible like we do up in the code? (or use the use_pad only if apq8064 
compatible is not detected, to address ipq8064-v2 added later?)

>  	val |= PHY_REFCLK_SSP_EN;
>  	writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
> 
> --
> 2.26.2

Re: [PATCH] PCI: qcom: don't clear out PHY_REFCLK_USE_PAD

[Ilia Mirkin]

On Sun, Sep 6, 2020 at 9:18 PM <ansuelsmth@gmail.com> wrote:
>
>
>
> > -----Messaggio originale-----
> > Da: Ilia Mirkin <ibmirkin@gmail.com> Per conto di Ilia Mirkin
> > Inviato: lunedì 7 settembre 2020 03:13
> > A: ansuelsmth@gmail.com
> > Cc: linux-arm-msm@vger.kernel.org; linux-pci@vger.kernel.org; Ilia Mirkin
> > <imirkin@alum.mit.edu>
> > Oggetto: [PATCH] PCI: qcom: don't clear out PHY_REFCLK_USE_PAD
> >
> > This makes PCIe links come up again on ifc6410 (apq8064).
> >
> > Fixes: de3c4bf6489 ("PCI: qcom: Add support for tx term offset for rev
> > 2.1.0")
> > Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> > b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 3aac77a295ba..985b11cf6481 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -387,7 +387,6 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie
> > *pcie)
> >
> >       /* enable external reference clock */
> >       val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> > -     val &= ~PHY_REFCLK_USE_PAD;
>
> To make sure this doesn't brake ipq806x, why not limit the &= to the ipq806x
> compatible like we do up in the code? (or use the use_pad only if apq8064
> compatible is not detected, to address ipq8064-v2 added later?)

Do you mean something like

if (!of_device_is_compatible(node, "qcom,pcie-apq8064"))
val &= ~PHY_REFCLK_USE_PAD;

I'm not sure what's considered acceptable in these cases. It does seem
odd that this bit should not be cleared on apq8064 but should be on
ipq8064 -- perhaps there's more going on there? Unfortunately I
haven't the faintest clue as to what it is...

  -ilia

R: [PATCH] PCI: qcom: don't clear out PHY_REFCLK_USE_PAD

[ansuelsmth]

> -----Messaggio originale-----
> Da: Ilia Mirkin <imirkin@alum.mit.edu>
> Inviato: lunedì 7 settembre 2020 05:29
> A: Ansuel Smith <ansuelsmth@gmail.com>
> Cc: linux-arm-msm@vger.kernel.org; Linux PCI <linux-pci@vger.kernel.org>
> Oggetto: Re: [PATCH] PCI: qcom: don't clear out PHY_REFCLK_USE_PAD
> 
> On Sun, Sep 6, 2020 at 9:18 PM <ansuelsmth@gmail.com> wrote:
> >
> >
> >
> > > -----Messaggio originale-----
> > > Da: Ilia Mirkin <ibmirkin@gmail.com> Per conto di Ilia Mirkin
> > > Inviato: lunedì 7 settembre 2020 03:13
> > > A: ansuelsmth@gmail.com
> > > Cc: linux-arm-msm@vger.kernel.org; linux-pci@vger.kernel.org; Ilia
> Mirkin
> > > <imirkin@alum.mit.edu>
> > > Oggetto: [PATCH] PCI: qcom: don't clear out PHY_REFCLK_USE_PAD
> > >
> > > This makes PCIe links come up again on ifc6410 (apq8064).
> > >
> > > Fixes: de3c4bf6489 ("PCI: qcom: Add support for tx term offset for rev
> > > 2.1.0")
> > > Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-qcom.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> > > b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index 3aac77a295ba..985b11cf6481 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -387,7 +387,6 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie
> > > *pcie)
> > >
> > >       /* enable external reference clock */
> > >       val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> > > -     val &= ~PHY_REFCLK_USE_PAD;
> >
> > To make sure this doesn't brake ipq806x, why not limit the &= to the
> ipq806x
> > compatible like we do up in the code? (or use the use_pad only if
> apq8064
> > compatible is not detected, to address ipq8064-v2 added later?)
> 
> Do you mean something like
> 
> if (!of_device_is_compatible(node, "qcom,pcie-apq8064"))
> val &= ~PHY_REFCLK_USE_PAD;
> 
> I'm not sure what's considered acceptable in these cases. It does seem
> odd that this bit should not be cleared on apq8064 but should be on
> ipq8064 -- perhaps there's more going on there? Unfortunately I
> haven't the faintest clue as to what it is...
> 
>   -ilia

Ok i did some test... Can confirm that the condition is needed.
ipq806x needs the USE_PAD or the kernel just hangs. 
When the pci interface is init the regs are 1019... For ipq806x this need to
change to 10019 (external ref clk enabled and something else disabled that
we don't know without documentation)
So to sum up... without the condition this patch would cause a regression for
ipq8064/5.