2020-05-14 20:13:43

by Christian Marangi

[permalink] [raw]
Subject: [PATCH v4 08/10] PCI: qcom: Add ipq8064 rev2 variant and set tx term offset

Add tx term offset support to pcie qcom driver need in some revision of
the ipq806x SoC. Ipq8064 have tx term offset set to 7. Ipq8064-v2 revision
and ipq8065 have the tx term offset set to 0.

Signed-off-by: Sham Muthayyan <[email protected]>
Signed-off-by: Ansuel Smith <[email protected]>
---
drivers/pci/controller/dwc/pcie-qcom.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index f5398b0d270c..ab6f1bdd24c3 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -45,6 +45,9 @@
#define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10

#define PCIE20_PARF_PHY_CTRL 0x40
+#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(20, 16)
+#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x) ((x) << 16)
+
#define PCIE20_PARF_PHY_REFCLK 0x4C
#define PHY_REFCLK_SSP_EN BIT(16)
#define PHY_REFCLK_USE_PAD BIT(12)
@@ -363,7 +366,8 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
val &= ~BIT(0);
writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);

- if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
+ if (of_device_is_compatible(node, "qcom,pcie-ipq8064") |
+ of_device_is_compatible(node, "qcom,pcie-ipq8064-v2")) {
writel(PCS_DEEMPH_TX_DEEMPH_GEN1(24) |
PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(24) |
PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(34),
@@ -374,9 +378,18 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
writel(PHY_RX0_EQ(4), pcie->parf + PCIE20_PARF_CONFIG_BITS);
}

+ if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
+ /* set TX termination offset */
+ val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
+ val &= ~PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK;
+ val |= PHY_CTRL_PHY_TX0_TERM_OFFSET(7);
+ writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
+ }
+
/* enable external reference clock */
val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
- val |= BIT(16);
+ val &= ~PHY_REFCLK_USE_PAD;
+ val |= PHY_REFCLK_SSP_EN;
writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);

/* wait for clock acquisition */
@@ -1452,6 +1465,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
static const struct of_device_id qcom_pcie_match[] = {
{ .compatible = "qcom,pcie-apq8084", .data = &ops_1_0_0 },
{ .compatible = "qcom,pcie-ipq8064", .data = &ops_2_1_0 },
+ { .compatible = "qcom,pcie-ipq8064-v2", .data = &ops_2_1_0 },
{ .compatible = "qcom,pcie-apq8064", .data = &ops_2_1_0 },
{ .compatible = "qcom,pcie-msm8996", .data = &ops_2_3_2 },
{ .compatible = "qcom,pcie-ipq8074", .data = &ops_2_3_3 },
--
2.25.1


2020-06-01 21:11:07

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 08/10] PCI: qcom: Add ipq8064 rev2 variant and set tx term offset

On Thu, May 14, 2020 at 10:07:09PM +0200, Ansuel Smith wrote:
> Add tx term offset support to pcie qcom driver need in some revision of
> the ipq806x SoC. Ipq8064 have tx term offset set to 7. Ipq8064-v2 revision
> and ipq8065 have the tx term offset set to 0.

Seems like this should be 2 patches or why isn't 'Ipq8064 have tx term
offset set to 7' done in the prior patch? One tweak is needed for
stable, but this isn't?

>
> Signed-off-by: Sham Muthayyan <[email protected]>
> Signed-off-by: Ansuel Smith <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index f5398b0d270c..ab6f1bdd24c3 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -45,6 +45,9 @@
> #define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
>
> #define PCIE20_PARF_PHY_CTRL 0x40
> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(20, 16)
> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x) ((x) << 16)
> +
> #define PCIE20_PARF_PHY_REFCLK 0x4C
> #define PHY_REFCLK_SSP_EN BIT(16)
> #define PHY_REFCLK_USE_PAD BIT(12)
> @@ -363,7 +366,8 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
> val &= ~BIT(0);
> writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
>
> - if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
> + if (of_device_is_compatible(node, "qcom,pcie-ipq8064") |
> + of_device_is_compatible(node, "qcom,pcie-ipq8064-v2")) {
> writel(PCS_DEEMPH_TX_DEEMPH_GEN1(24) |
> PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(24) |
> PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(34),
> @@ -374,9 +378,18 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
> writel(PHY_RX0_EQ(4), pcie->parf + PCIE20_PARF_CONFIG_BITS);
> }
>
> + if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
> + /* set TX termination offset */
> + val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
> + val &= ~PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK;
> + val |= PHY_CTRL_PHY_TX0_TERM_OFFSET(7);
> + writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> + }
> +
> /* enable external reference clock */
> val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> - val |= BIT(16);
> + val &= ~PHY_REFCLK_USE_PAD;
> + val |= PHY_REFCLK_SSP_EN;
> writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
>
> /* wait for clock acquisition */
> @@ -1452,6 +1465,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> static const struct of_device_id qcom_pcie_match[] = {
> { .compatible = "qcom,pcie-apq8084", .data = &ops_1_0_0 },
> { .compatible = "qcom,pcie-ipq8064", .data = &ops_2_1_0 },
> + { .compatible = "qcom,pcie-ipq8064-v2", .data = &ops_2_1_0 },
> { .compatible = "qcom,pcie-apq8064", .data = &ops_2_1_0 },
> { .compatible = "qcom,pcie-msm8996", .data = &ops_2_3_2 },
> { .compatible = "qcom,pcie-ipq8074", .data = &ops_2_3_3 },
> --
> 2.25.1
>

2020-06-02 07:56:09

by Stanimir Varbanov

[permalink] [raw]
Subject: Re: [PATCH v4 08/10] PCI: qcom: Add ipq8064 rev2 variant and set tx term offset

Hi,

On 5/14/20 11:07 PM, Ansuel Smith wrote:
> Add tx term offset support to pcie qcom driver need in some revision of
> the ipq806x SoC. Ipq8064 have tx term offset set to 7. Ipq8064-v2 revision
> and ipq8065 have the tx term offset set to 0.
>
> Signed-off-by: Sham Muthayyan <[email protected]>
> Signed-off-by: Ansuel Smith <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index f5398b0d270c..ab6f1bdd24c3 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -45,6 +45,9 @@
> #define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
>
> #define PCIE20_PARF_PHY_CTRL 0x40
> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(20, 16)

I see you changed the mask, did you found the issue in previous v3 mask
and shift?

> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x) ((x) << 16)
> +
> #define PCIE20_PARF_PHY_REFCLK 0x4C
> #define PHY_REFCLK_SSP_EN BIT(16)
> #define PHY_REFCLK_USE_PAD BIT(12)
> @@ -363,7 +366,8 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
> val &= ~BIT(0);
> writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
>
> - if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
> + if (of_device_is_compatible(node, "qcom,pcie-ipq8064") |

this should be logical OR

> + of_device_is_compatible(node, "qcom,pcie-ipq8064-v2")) {
> writel(PCS_DEEMPH_TX_DEEMPH_GEN1(24) |
> PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(24) |
> PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(34),
> @@ -374,9 +378,18 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
> writel(PHY_RX0_EQ(4), pcie->parf + PCIE20_PARF_CONFIG_BITS);
> }
>
> + if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
> + /* set TX termination offset */
> + val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
> + val &= ~PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK;
> + val |= PHY_CTRL_PHY_TX0_TERM_OFFSET(7);
> + writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> + }
> +
> /* enable external reference clock */
> val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> - val |= BIT(16);
> + val &= ~PHY_REFCLK_USE_PAD;
> + val |= PHY_REFCLK_SSP_EN;
> writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
>
> /* wait for clock acquisition */
> @@ -1452,6 +1465,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> static const struct of_device_id qcom_pcie_match[] = {
> { .compatible = "qcom,pcie-apq8084", .data = &ops_1_0_0 },
> { .compatible = "qcom,pcie-ipq8064", .data = &ops_2_1_0 },
> + { .compatible = "qcom,pcie-ipq8064-v2", .data = &ops_2_1_0 },
> { .compatible = "qcom,pcie-apq8064", .data = &ops_2_1_0 },
> { .compatible = "qcom,pcie-msm8996", .data = &ops_2_3_2 },
> { .compatible = "qcom,pcie-ipq8074", .data = &ops_2_3_3 },
>

--
regards,
Stan

2020-06-02 09:34:46

by Christian Marangi

[permalink] [raw]
Subject: R: [PATCH v4 08/10] PCI: qcom: Add ipq8064 rev2 variant and set tx term offset



> -----Messaggio originale-----
> Da: Stanimir Varbanov <[email protected]>
> Inviato: martedì 2 giugno 2020 09:54
> A: Ansuel Smith <[email protected]>; Bjorn Andersson
> <[email protected]>
> Cc: Sham Muthayyan <[email protected]>; Andy Gross
> <[email protected]>; Bjorn Helgaas <[email protected]>; Rob Herring
> <[email protected]>; Mark Rutland <[email protected]>; Lorenzo
> Pieralisi <[email protected]>; Andrew Murray
> <[email protected]>; Philipp Zabel
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Oggetto: Re: [PATCH v4 08/10] PCI: qcom: Add ipq8064 rev2 variant and
> set tx term offset
>
> Hi,
>
> On 5/14/20 11:07 PM, Ansuel Smith wrote:
> > Add tx term offset support to pcie qcom driver need in some revision of
> > the ipq806x SoC. Ipq8064 have tx term offset set to 7. Ipq8064-v2
> revision
> > and ipq8065 have the tx term offset set to 0.
> >
> > Signed-off-by: Sham Muthayyan <[email protected]>
> > Signed-off-by: Ansuel Smith <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pcie-qcom.c | 18 ++++++++++++++++--
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> > index f5398b0d270c..ab6f1bdd24c3 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -45,6 +45,9 @@
> > #define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
> >
> > #define PCIE20_PARF_PHY_CTRL 0x40
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(20,
> 16)
>
> I see you changed the mask, did you found the issue in previous v3 mask
> and shift?
>

I checked the original code and the old GENMASK declaration was wrong as you
suggested.

> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x) ((x) << 16)
> > +
> > #define PCIE20_PARF_PHY_REFCLK 0x4C
> > #define PHY_REFCLK_SSP_EN BIT(16)
> > #define PHY_REFCLK_USE_PAD BIT(12)
> > @@ -363,7 +366,8 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> > val &= ~BIT(0);
> > writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> >
> > - if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
> > + if (of_device_is_compatible(node, "qcom,pcie-ipq8064") |
>
> this should be logical OR
>

Will change in v5 since I will have to split this

> > + of_device_is_compatible(node, "qcom,pcie-ipq8064-v2")) {
> > writel(PCS_DEEMPH_TX_DEEMPH_GEN1(24) |
> > PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(24) |
> > PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(34),
> > @@ -374,9 +378,18 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> > writel(PHY_RX0_EQ(4), pcie->parf +
> PCIE20_PARF_CONFIG_BITS);
> > }
> >
> > + if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
> > + /* set TX termination offset */
> > + val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
> > + val &= ~PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK;
> > + val |= PHY_CTRL_PHY_TX0_TERM_OFFSET(7);
> > + writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> > + }
> > +
> > /* enable external reference clock */
> > val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> > - val |= BIT(16);
> > + val &= ~PHY_REFCLK_USE_PAD;
> > + val |= PHY_REFCLK_SSP_EN;
> > writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
> >
> > /* wait for clock acquisition */
> > @@ -1452,6 +1465,7 @@ static int qcom_pcie_probe(struct
> platform_device *pdev)
> > static const struct of_device_id qcom_pcie_match[] = {
> > { .compatible = "qcom,pcie-apq8084", .data = &ops_1_0_0 },
> > { .compatible = "qcom,pcie-ipq8064", .data = &ops_2_1_0 },
> > + { .compatible = "qcom,pcie-ipq8064-v2", .data = &ops_2_1_0 },
> > { .compatible = "qcom,pcie-apq8064", .data = &ops_2_1_0 },
> > { .compatible = "qcom,pcie-msm8996", .data = &ops_2_3_2 },
> > { .compatible = "qcom,pcie-ipq8074", .data = &ops_2_3_3 },
> >
>
> --
> regards,
> Stan

2020-06-02 09:38:40

by Christian Marangi

[permalink] [raw]
Subject: R: [PATCH v4 08/10] PCI: qcom: Add ipq8064 rev2 variant and set tx term offset



> -----Messaggio originale-----
> Da: Rob Herring <[email protected]>
> Inviato: luned? 1 giugno 2020 23:09
> A: Ansuel Smith <[email protected]>
> Cc: Bjorn Andersson <[email protected]>; Sham Muthayyan
> <[email protected]>; Andy Gross <[email protected]>; Bjorn
> Helgaas <[email protected]>; Mark Rutland
> <[email protected]>; Stanimir Varbanov <[email protected]>;
> Lorenzo Pieralisi <[email protected]>; Andrew Murray
> <[email protected]>; Philipp Zabel
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Oggetto: Re: [PATCH v4 08/10] PCI: qcom: Add ipq8064 rev2 variant and
> set tx term offset
>
> On Thu, May 14, 2020 at 10:07:09PM +0200, Ansuel Smith wrote:
> > Add tx term offset support to pcie qcom driver need in some revision of
> > the ipq806x SoC. Ipq8064 have tx term offset set to 7. Ipq8064-v2
> revision
> > and ipq8065 have the tx term offset set to 0.
>
> Seems like this should be 2 patches or why isn't 'Ipq8064 have tx term
> offset set to 7' done in the prior patch? One tweak is needed for
> stable, but this isn't?
>

Ok i will split this in 2 patch and set for stable the tx term patch.

> >
> > Signed-off-by: Sham Muthayyan <[email protected]>
> > Signed-off-by: Ansuel Smith <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pcie-qcom.c | 18 ++++++++++++++++--
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> > index f5398b0d270c..ab6f1bdd24c3 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -45,6 +45,9 @@
> > #define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
> >
> > #define PCIE20_PARF_PHY_CTRL 0x40
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(20,
> 16)
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x) ((x) << 16)
> > +
> > #define PCIE20_PARF_PHY_REFCLK 0x4C
> > #define PHY_REFCLK_SSP_EN BIT(16)
> > #define PHY_REFCLK_USE_PAD BIT(12)
> > @@ -363,7 +366,8 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> > val &= ~BIT(0);
> > writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> >
> > - if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
> > + if (of_device_is_compatible(node, "qcom,pcie-ipq8064") |
> > + of_device_is_compatible(node, "qcom,pcie-ipq8064-v2")) {
> > writel(PCS_DEEMPH_TX_DEEMPH_GEN1(24) |
> > PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(24) |
> > PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(34),
> > @@ -374,9 +378,18 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> > writel(PHY_RX0_EQ(4), pcie->parf +
> PCIE20_PARF_CONFIG_BITS);
> > }
> >
> > + if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
> > + /* set TX termination offset */
> > + val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
> > + val &= ~PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK;
> > + val |= PHY_CTRL_PHY_TX0_TERM_OFFSET(7);
> > + writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> > + }
> > +
> > /* enable external reference clock */
> > val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> > - val |= BIT(16);
> > + val &= ~PHY_REFCLK_USE_PAD;
> > + val |= PHY_REFCLK_SSP_EN;
> > writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
> >
> > /* wait for clock acquisition */
> > @@ -1452,6 +1465,7 @@ static int qcom_pcie_probe(struct
> platform_device *pdev)
> > static const struct of_device_id qcom_pcie_match[] = {
> > { .compatible = "qcom,pcie-apq8084", .data = &ops_1_0_0 },
> > { .compatible = "qcom,pcie-ipq8064", .data = &ops_2_1_0 },
> > + { .compatible = "qcom,pcie-ipq8064-v2", .data = &ops_2_1_0 },
> > { .compatible = "qcom,pcie-apq8064", .data = &ops_2_1_0 },
> > { .compatible = "qcom,pcie-msm8996", .data = &ops_2_3_2 },
> > { .compatible = "qcom,pcie-ipq8074", .data = &ops_2_3_3 },
> > --
> > 2.25.1
> >