Multiple fixes in PCIe qcom driver

Aux and Ref clk are missing in PCIe qcom driver.
Add support in the driver to fix PCIe initialization in ipq806x.

Fixes 82a823833f4e PCI: qcom: Add Qualcomm PCIe controller driver

Signed-off-by Sham Muthayyan <[email protected]>

Signed-off-by Ansuel Smith <[email protected]>

Cc [email protected] # v4.5+

---
 drivers/pci/controller/dwc/pcie-qcom.c | 44 ++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 6 deletions(-)

drivers/pci/controller/dwc/pcie-qcom.c +2 -0

index 5ea527a6bd9f..2a39dfdccfc8 100644
@@ -88,6 +88,8 @@ struct qcom_pcie_resources_2_1_0 {
 	struct clk *iface_clk;
 	struct clk *core_clk;
 	struct clk *phy_clk;
+	struct clk *aux_clk;
+	struct clk *ref_clk;
 	struct reset_control *pci_reset;
 	struct reset_control *axi_reset;
 	struct reset_control *ahb_reset;

drivers/pci/controller/dwc/pcie-qcom.c +3 -0

+
 	res->pci_reset = devm_reset_control_get_exclusive(dev, "pci");
 	if (IS_ERR(res->pci_reset))
 		return PTR_ERR(res->pci_reset);

@@ -278,6 +288,8 @@ static void qcom_pcie_deinit_2_1_0(struct qcom_pcie *pcie)
 	clk_disable_unprepare(res->iface_clk);
 	clk_disable_unprepare(res->core_clk);
 	clk_disable_unprepare(res->phy_clk);
+	clk_disable_unprepare(res->aux_clk);
+	clk_disable_unprepare(res->ref_clk);
 	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
 }
 

drivers/pci/controller/dwc/pcie-qcom.c +25 -6

+	if (ret) {
+		dev_err(dev, "cannot prepare/enable core clock\n");
+		goto err_clk_core;
+	}
+
 	ret = clk_prepare_enable(res->phy_clk);
 	if (ret) {
 		dev_err(dev, "cannot prepare/enable phy clock\n");
 		goto err_clk_phy;
 	}
 
-	ret = clk_prepare_enable(res->core_clk);
-	if (ret) {
-		dev_err(dev, "cannot prepare/enable core clock\n");
-		goto err_clk_core;
+	if (res->aux_clk) {
+		ret = clk_prepare_enable(res->aux_clk);
+		if (ret) {
+			dev_err(dev, "cannot prepare/enable aux clock\n");
+			goto err_clk_aux;
+		}
+	}
+
+	if (res->ref_clk) {
+		ret = clk_prepare_enable(res->ref_clk);
+		if (ret) {
+			dev_err(dev, "cannot prepare/enable ref clock\n");
+			goto err_clk_ref;
+		}
 	}
 
 	ret = reset_control_deassert(res->ahb_reset);

@@ -372,10 +400,14 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 	return 0;
 
 err_deassert_ahb:
-	clk_disable_unprepare(res->core_clk);
-err_clk_core:
+	clk_disable_unprepare(res->ref_clk);
+err_clk_ref:
+	clk_disable_unprepare(res->aux_clk);
+err_clk_aux:
 	clk_disable_unprepare(res->phy_clk);
 err_clk_phy:
+	clk_disable_unprepare(res->core_clk);
+err_clk_core:
 	clk_disable_unprepare(res->iface_clk);
 err_assert_ahb:
 	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);

Signature · 1 line

2.25.1

Aux and Ref clk are missing in PCIe qcom driver.
Add support in the driver to fix PCIe initialization in ipq806x.

Fixes 82a823833f4e PCI: qcom: Add Qualcomm PCIe controller driver

Signed-off-by Sham Muthayyan <[email protected]>

Signed-off-by Ansuel Smith <[email protected]>

Cc [email protected] # v4.5+

---
 drivers/pci/controller/dwc/pcie-qcom.c | 44 ++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 6 deletions(-)

drivers/pci/controller/dwc/pcie-qcom.c +18 -0

index 5ea527a6bd9f..2a39dfdccfc8 100644
@@ -88,6 +88,8 @@ struct qcom_pcie_resources_2_1_0 {
 	struct clk *iface_clk;
 	struct clk *core_clk;
 	struct clk *phy_clk;
+	struct clk *aux_clk;
+	struct clk *ref_clk;
 	struct reset_control *pci_reset;
 	struct reset_control *axi_reset;
 	struct reset_control *ahb_reset;

@@ -246,6 +248,14 @@ static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
 	if (IS_ERR(res->phy_clk))
 		return PTR_ERR(res->phy_clk);
 
+	res->aux_clk = devm_clk_get_optional(dev, "aux");
+	if (IS_ERR(res->aux_clk))
+		return PTR_ERR(res->aux_clk);
+
+	res->ref_clk = devm_clk_get_optional(dev, "ref");
+	if (IS_ERR(res->ref_clk))
+		return PTR_ERR(res->ref_clk);
+
 	res->pci_reset = devm_reset_control_get_exclusive(dev, "pci");
 	if (IS_ERR(res->pci_reset))
 		return PTR_ERR(res->pci_reset);

@@ -278,6 +288,8 @@ static void qcom_pcie_deinit_2_1_0(struct qcom_pcie *pcie)
 	clk_disable_unprepare(res->iface_clk);
 	clk_disable_unprepare(res->core_clk);
 	clk_disable_unprepare(res->phy_clk);
+	clk_disable_unprepare(res->aux_clk);
+	clk_disable_unprepare(res->ref_clk);
 	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
 }
 

@@ -307,16 +319,32 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 		goto err_assert_ahb;
 	}
 
+	ret = clk_prepare_enable(res->core_clk);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable core clock\n");
+		goto err_clk_core;
+	}
+
 	ret = clk_prepare_enable(res->phy_clk);

drivers/pci/controller/dwc/pcie-qcom.c +11 -2

+		ret = clk_prepare_enable(res->ref_clk);
+		if (ret) {
+			dev_err(dev, "cannot prepare/enable ref clock\n");
+			goto err_clk_ref;
+		}
 	}
 
 	ret = reset_control_deassert(res->ahb_reset);

@@ -372,10 +400,14 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 	return 0;
 
 err_deassert_ahb:
-	clk_disable_unprepare(res->core_clk);
-err_clk_core:
+	clk_disable_unprepare(res->ref_clk);
+err_clk_ref:
+	clk_disable_unprepare(res->aux_clk);
+err_clk_aux:
 	clk_disable_unprepare(res->phy_clk);
 err_clk_phy:
+	clk_disable_unprepare(res->core_clk);
+err_clk_core:
 	clk_disable_unprepare(res->iface_clk);
 err_assert_ahb:
 	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);

From: Abhishek Sahu <[email protected]>

The deinit issues reset_control_assert for PCI twice and does not contain phy reset.

Signed-off-by Abhishek Sahu <[email protected]>

Signed-off-by Ansuel Smith <[email protected]>

---
 drivers/pci/controller/dwc/pcie-qcom.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

Add missing ext reset used by ipq8064 SoC in PCIe qcom driver.

Fixes 82a823833f4e PCI: qcom: Add Qualcomm PCIe controller driver

Signed-off-by Sham Muthayyan <[email protected]>

Signed-off-by Ansuel Smith <[email protected]>

Cc [email protected] # v4.5+

---
 drivers/pci/controller/dwc/pcie-qcom.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Add missing ext reset used by ipq8064 SoC in PCIe qcom driver.

Fixes 82a823833f4e PCI: qcom: Add Qualcomm PCIe controller driver

Signed-off-by Sham Muthayyan <[email protected]>

Signed-off-by Ansuel Smith <[email protected]>

Cc [email protected] # v4.5+

---
 drivers/pci/controller/dwc/pcie-qcom.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

drivers/pci/controller/dwc/pcie-qcom.c +0 -0

index 7a8901efc031..921030a64bab 100644

Use qcom_clear_and_set_dword instead of use the same code many times in the entire driver.

Signed-off-by Ansuel Smith <[email protected]>

---
 drivers/pci/controller/dwc/pcie-qcom.c | 108 ++++++++++---------------
 1 file changed, 41 insertions(+), 67 deletions(-)

drivers/pci/controller/dwc/pcie-qcom.c +0 -0

index 921030a64bab..a4fd5baada34 100644
@@ -184,6 +184,16 @@ struct qcom_pcie {
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)

It is now supported the editing of Tx De-Emphasis, Tx Swing and Rx equalization params on ipq8064. Document this new optional params.

Signed-off-by Ansuel Smith <[email protected]>

---
 .../devicetree/bindings/pci/qcom,pcie.txt     | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)

Documentation/devicetree/bindings/pci/qcom,pcie.txt +0 -0

index 6efcef040741..8cc5aea8a1da 100644

Documentation/devicetree/bindings/pci/qcom,pcie.txt +31 -0

+
+- qcom,tx-deemph-gen2-3p5db:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: Gen2 (3.5db) De-emphasis value.
+		    For ipq806x should be set to 24.
+
+- qcom,tx-deemph-gen2-6db:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: Gen2 (6db) De-emphasis value.
+		    For ipq806x should be set to 34.
+
+- qcom,tx-swing-full:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: TX launch amplitude swing_full value.
+		    For ipq806x should be set to 120.
+
+- qcom,tx-swing-low:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: TX launch amplitude swing_low value.
+		    For ipq806x should be set to 120.
+
+- qcom,rx0-eq:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: RX0 equalization value.
+		    For ipq806x should be set to 4.
+
 * Example for ipq/apq8064
 	pcie@1b500000 {
 		compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064", "snps,dw-pcie";

Signature · 1 line

2.25.1

Review

Quoted 9 lines

It is now supported the editing of Tx De-Emphasis, Tx Swing and Rx equalization params on ipq8064. Document this new optional params.

Signed-off-by Ansuel Smith <[email protected]>

---
 .../devicetree/bindings/pci/qcom,pcie.txt     | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/pci/qcom,pcie.txt

Quoted 7 lines

index 6efcef040741..8cc5aea8a1da 100644

Documentation/devicetree/bindings/pci/qcom,pcie.txt +0 -0

@@ -254,6 +254,42 @@
 			- "perst-gpios"	PCIe endpoint reset signal line
 			- "wake-gpios"	PCIe endpoint wake signal line
 

Documentation/devicetree/bindings/pci/qcom,pcie.txt line 261

Quoted 5 lines

Documentation/devicetree/bindings/pci/qcom,pcie.txt +5 -0

+- qcom,tx-deemph-gen1:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: Gen1 De-emphasis value.
+		    For ipq806x should be set to 24.

Unless these need to be tuned per board, then the compatible string for ipq806x should imply all these settings.

Quoted 24 lines

Documentation/devicetree/bindings/pci/qcom,pcie.txt +24 -0

+
+- qcom,tx-deemph-gen2-3p5db:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: Gen2 (3.5db) De-emphasis value.
+		    For ipq806x should be set to 24.
+
+- qcom,tx-deemph-gen2-6db:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: Gen2 (6db) De-emphasis value.
+		    For ipq806x should be set to 34.
+
+- qcom,tx-swing-full:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: TX launch amplitude swing_full value.
+		    For ipq806x should be set to 120.
+
+- qcom,tx-swing-low:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: TX launch amplitude swing_low value.
+		    For ipq806x should be set to 120.

Documentation/devicetree/bindings/pci/qcom,pcie.txt

Quoted 10 lines

Documentation/devicetree/bindings/pci/qcom,pcie.txt +7 -0

+
+- qcom,rx0-eq:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: RX0 equalization value.
+		    For ipq806x should be set to 4.
+
 * Example for ipq/apq8064
 	pcie@1b500000 {
 		compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064",

"snps,dw-pcie";

Quoted 3 lines

--
2.25.1

Quoted 18 lines · Ansuel Smith

Review

Quoted 9 lines

It is now supported the editing of Tx De-Emphasis, Tx Swing and Rx equalization params on ipq8064. Document this new optional params.

Signed-off-by Ansuel Smith <[email protected]>

---
 .../devicetree/bindings/pci/qcom,pcie.txt     | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/pci/qcom,pcie.txt

Quoted 7 lines

index 6efcef040741..8cc5aea8a1da 100644

Documentation/devicetree/bindings/pci/qcom,pcie.txt +0 -0

@@ -254,6 +254,42 @@
 			- "perst-gpios"	PCIe endpoint reset signal line
 			- "wake-gpios"	PCIe endpoint wake signal line
 

Quoted 9 lines

Documentation/devicetree/bindings/pci/qcom,pcie.txt line 261

Quoted 5 lines

Documentation/devicetree/bindings/pci/qcom,pcie.txt +5 -0

+- qcom,tx-deemph-gen1:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: Gen1 De-emphasis value.
+		    For ipq806x should be set to 24.

Unless these need to be tuned per board, then the compatible string for ipq806x should imply all these settings.

It was requested by v2 to make this settings tunable. These don't change are all the same for every ipq806x SoC. The original implementation had this value hardcoded for ipq806x. Should I restore this and drop this patch?

Quoted 28 lines · On Thu, May 07, 2020 at 09:34:35PM +0200, [email protected] wrote:

Quoted 27 lines · Ansuel Smith

Review

Quoted 9 lines

It is now supported the editing of Tx De-Emphasis, Tx Swing and Rx equalization params on ipq8064. Document this new optional params.

Signed-off-by Ansuel Smith <[email protected]>

---
 .../devicetree/bindings/pci/qcom,pcie.txt     | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/pci/qcom,pcie.txt

Documentation/devicetree/bindings/pci/qcom,pcie.txt line 261

Quoted 12 lines

index 6efcef040741..8cc5aea8a1da 100644

Documentation/devicetree/bindings/pci/qcom,pcie.txt +5 -0

@@ -254,6 +254,42 @@
 			- "perst-gpios"	PCIe endpoint reset signal line
 			- "wake-gpios"	PCIe endpoint wake signal line
 
+- qcom,tx-deemph-gen1:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: Gen1 De-emphasis value.
+		    For ipq806x should be set to 24.

Unless these need to be tuned per board, then the compatible string for ipq806x should imply all these settings.

Quoted 3 lines

It was requested by v2 to make this settings tunable. These don't change are all the same for every ipq806x SoC. The original implementation had this value hardcoded for ipq806x. Should I restore this and drop this patch?

Yes, please.

Rob

Quoted 1 line

On Thu, May 07, 2020 at 09:34:35PM +0200, [email protected]

Quoted 3 lines · wrote:

Quoted 3 lines

Quoted 3 lines · Ansuel Smith

Quoted 2 lines

It is now supported the editing of Tx De-Emphasis, Tx Swing and Rx equalization params on ipq8064. Document this new optional

params.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Signed-off-by Ansuel Smith <[email protected]>

---
 .../devicetree/bindings/pci/qcom,pcie.txt     | 36

+++++++++++++++++++

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

Review

Quoted 3 lines

 1 file changed, 36 insertions(+)

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/pci/qcom,pcie.txt

Quoted 7 lines

index 6efcef040741..8cc5aea8a1da 100644

Documentation/devicetree/bindings/pci/qcom,pcie.txt +0 -0

@@ -254,6 +254,42 @@
 			- "perst-gpios"	PCIe endpoint reset signal line
 			- "wake-gpios"	PCIe endpoint wake signal line
 

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Documentation/devicetree/bindings/pci/qcom,pcie.txt line 261

Quoted 5 lines

Documentation/devicetree/bindings/pci/qcom,pcie.txt +5 -0

+- qcom,tx-deemph-gen1:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: Gen1 De-emphasis value.
+		    For ipq806x should be set to 24.

Unless these need to be tuned per board, then the compatible string

for

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

ipq806x should imply all these settings.

It was requested by v2 to make this settings tunable. These don't change

are

Quoted 4 lines

Quoted 2 lines

all the same for every ipq806x SoC. The original implementation had this value hardcoded for ipq806x. Should I restore this and drop this patch?

Yes, please.

I still think that the values for tx deemph and tx swing should be tunable. But I can live with them in the driver if they not break support for apq8064.

The default values in the registers for apq8064 and ipq806x are:

default your change
TX_DEEMPH_GEN1 21 24
TX_DEEMPH_GEN2_3_5DB 21 24
TX_DEEMPH_GEN2_6DB 32 34

TX_SWING_FULL 121 120
TX_SWING_LOW 121 120

So until now (without your change) apq8064 worked with default values.

Quoted 3 lines

Rob

--
regards,
Stan

Quoted 52 lines · Rob Herring

Quoted 1 line

On Thu, May 07, 2020 at 09:34:35PM +0200, [email protected]

Quoted 3 lines · wrote:

Quoted 3 lines

Quoted 3 lines · Ansuel Smith

Quoted 2 lines

It is now supported the editing of Tx De-Emphasis, Tx Swing and Rx equalization params on ipq8064. Document this new optional

params.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Signed-off-by Ansuel Smith <[email protected]>

---
 .../devicetree/bindings/pci/qcom,pcie.txt     | 36

+++++++++++++++++++

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

Review

Quoted 3 lines

 1 file changed, 36 insertions(+)

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/pci/qcom,pcie.txt

Quoted 7 lines

index 6efcef040741..8cc5aea8a1da 100644

Documentation/devicetree/bindings/pci/qcom,pcie.txt +0 -0

@@ -254,6 +254,42 @@
 			- "perst-gpios"	PCIe endpoint reset signal line
 			- "wake-gpios"	PCIe endpoint wake signal line
 

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Documentation/devicetree/bindings/pci/qcom,pcie.txt line 261

Quoted 5 lines

Documentation/devicetree/bindings/pci/qcom,pcie.txt +5 -0

+- qcom,tx-deemph-gen1:
+	Usage: optional (available for ipq/apq8064)
+	Value type: <u32>
+	Definition: Gen1 De-emphasis value.
+		    For ipq806x should be set to 24.

Unless these need to be tuned per board, then the compatible string

for

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

ipq806x should imply all these settings.

It was requested by v2 to make this settings tunable. These don't change

are

Quoted 4 lines

Quoted 2 lines

all the same for every ipq806x SoC. The original implementation had this value hardcoded for ipq806x. Should I restore this and drop this patch?

Yes, please.

I still think that the values for tx deemph and tx swing should be tunable. But I can live with them in the driver if they not break support for apq8064.

The default values in the registers for apq8064 and ipq806x are:

default your change
TX_DEEMPH_GEN1 21 24
TX_DEEMPH_GEN2_3_5DB 21 24
TX_DEEMPH_GEN2_6DB 32 34

Quoted 5 lines

TX_SWING_FULL 121 120
TX_SWING_LOW 121 120

So until now (without your change) apq8064 worked with default values.

I will limit this to ipq8064(-v2) if this could be a problem.

From: Sham Muthayyan <[email protected]>

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 | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

drivers/pci/controller/dwc/pcie-qcom.c +4 -0

index da8058fd1925..372d2c8508b5 100644
@@ -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(12, 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)

@@ -118,6 +121,7 @@ struct qcom_pcie_resources_2_1_0 {
 	u32 tx_swing_full;
 	u32 tx_swing_low;
 	u32 rx0_eq;
+	u8 phy_tx0_term_offset;
 };
 
 struct qcom_pcie_resources_1_0_0 {

drivers/pci/controller/dwc/pcie-qcom.c +3 -0

+	else
+		res->phy_tx0_term_offset = 0;
+

Quoted 7 lines

From: Sham Muthayyan <[email protected]>

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.

Review

Quoted 7 lines

Signed-off-by Sham Muthayyan <[email protected]>

Signed-off-by Ansuel Smith <[email protected]>

---
 drivers/pci/controller/dwc/pcie-qcom.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

(quoted diff) +0 -0

b/drivers/pci/controller/dwc/pcie-qcom.c

drivers/pci/controller/dwc/pcie-qcom.c line 48

Quoted 8 lines

index da8058fd1925..372d2c8508b5 100644

drivers/pci/controller/dwc/pcie-qcom.c +1 -0

@@ -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(12,

16)

drivers/pci/controller/dwc/pcie-qcom.c static int

Quoted 14 lines

drivers/pci/controller/dwc/pcie-qcom.c +3 -0

+#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)

@@ -118,6 +121,7 @@ struct qcom_pcie_resources_2_1_0 {
 	u32 tx_swing_full;
 	u32 tx_swing_low;
 	u32 rx0_eq;
+	u8 phy_tx0_term_offset;
 };
 
 struct qcom_pcie_resources_1_0_0 {

@@ -318,6 +322,11 @@ static int

qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)

drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie) · line 326

Quoted 5 lines

drivers/pci/controller/dwc/pcie-qcom.c +2 -0

 	if (IS_ERR(res->ext_reset))
 		return PTR_ERR(res->ext_reset);
 
+	if (of_device_is_compatible(dev->of_node, "qcom,pcie-ipq8064"))
+		res->phy_tx0_term_offset = 7;

Based on my other comments, you'll want to turn this into match data.

Quoted 3 lines

drivers/pci/controller/dwc/pcie-qcom.c +3 -0

+	else
+		res->phy_tx0_term_offset = 0;
+

From: Sham Muthayyan <[email protected]>

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 | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

drivers/pci/controller/dwc/pcie-qcom.c +0 -0

index da8058fd1925..372d2c8508b5 100644

drivers/pci/controller/dwc/pcie-qcom.c +3 -0

+#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)

@@ -118,6 +121,7 @@ struct qcom_pcie_resources_2_1_0 {
 	u32 tx_swing_full;
 	u32 tx_swing_low;
 	u32 rx0_eq;
+	u8 phy_tx0_term_offset;
 };
 
 struct qcom_pcie_resources_1_0_0 {

drivers/pci/controller/dwc/pcie-qcom.c +3 -0

+			PHY_CTRL_PHY_TX0_TERM_OFFSET(res->phy_tx0_term_offset));
+
 	writel(PCS_DEEMPH_TX_DEEMPH_GEN1(res->tx_deemph_gen1) |
 	       PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(res->tx_deemph_gen2_3p5db) |
 	       PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(res->tx_deemph_gen2_6db),

@@ -1485,6 +1499,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 },

Hi Ansuel,

Quoted 7 lines · Ansuel Smith

From: Sham Muthayyan <[email protected]>

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.

Review

Quoted 7 lines

Signed-off-by Sham Muthayyan <[email protected]>

Signed-off-by Ansuel Smith <[email protected]>

---
 drivers/pci/controller/dwc/pcie-qcom.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

(quoted diff) +0 -0

b/drivers/pci/controller/dwc/pcie-qcom.c

drivers/pci/controller/dwc/pcie-qcom.c line 48

Quoted 8 lines

index da8058fd1925..372d2c8508b5 100644

drivers/pci/controller/dwc/pcie-qcom.c +1 -0

@@ -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(12,

16)

The mask definition is not correct. Should be GENMASK(20, 16)

drivers/pci/controller/dwc/pcie-qcom.c static int

Quoted 14 lines

drivers/pci/controller/dwc/pcie-qcom.c +3 -0

+#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)

@@ -118,6 +121,7 @@ struct qcom_pcie_resources_2_1_0 {
 	u32 tx_swing_full;
 	u32 tx_swing_low;
 	u32 rx0_eq;
+	u8 phy_tx0_term_offset;
 };
 
 struct qcom_pcie_resources_1_0_0 {

@@ -318,6 +322,11 @@ static int

qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)

drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie) · line 326

Quoted 5 lines

drivers/pci/controller/dwc/pcie-qcom.c +2 -0

 	if (IS_ERR(res->ext_reset))
 		return PTR_ERR(res->ext_reset);
 
+	if (of_device_is_compatible(dev->of_node, "qcom,pcie-ipq8064"))
+		res->phy_tx0_term_offset = 7;

Before your change the phy_tx0_term_offser was 0 for apq8064, but here you change it to 7, why?

drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_init_2_1_0(struct

Quoted 7 lines

drivers/pci/controller/dwc/pcie-qcom.c +3 -0

+	else
+		res->phy_tx0_term_offset = 0;
+
 	res->phy_reset = devm_reset_control_get_exclusive(dev, "phy");
 	return PTR_ERR_OR_ZERO(res->phy_reset);
 }

@@ -402,6 +411,11 @@ static int qcom_pcie_init_2_1_0(struct

qcom_pcie *pcie)

drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_init_2_1_0(struct · line 412

Quoted 2 lines

drivers/pci/controller/dwc/pcie-qcom.c +0 -0

 	/* enable PCIe clocks and resets */
 	qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL,

BIT(0), 0);

drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie) · line 416

Quoted 4 lines

drivers/pci/controller/dwc/pcie-qcom.c +3 -0

+	/* set TX termination offset */
+	qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL,
+			PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK,

As the mask definition is incorrect you actually clear 12 to 16 bit in the register where is another PHY parameter. Is that was intentional?

drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_probe(struct

Quoted 9 lines

drivers/pci/controller/dwc/pcie-qcom.c +2 -0

+			PHY_CTRL_PHY_TX0_TERM_OFFSET(res-
 phy_tx0_term_offset));
+
 	writel(PCS_DEEMPH_TX_DEEMPH_GEN1(res->tx_deemph_gen1) |
 	       PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(res-

tx_deemph_gen2_3p5db) |
PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(res-
tx_deemph_gen2_6db),

drivers/pci/controller/dwc/pcie-qcom.c +0 -0

@@ -1485,6 +1499,7 @@ static int qcom_pcie_probe(struct

platform_device *pdev)

drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_probe(struct platform_device *pdev) · line 1505

Quoted 8 lines

drivers/pci/controller/dwc/pcie-qcom.c +1 -0

 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

Quoted 45 lines

Hi Ansuel,

Quoted 7 lines · Ansuel Smith

From: Sham Muthayyan <[email protected]>

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.

Review

Quoted 7 lines

Signed-off-by Sham Muthayyan <[email protected]>

Signed-off-by Ansuel Smith <[email protected]>

---
 drivers/pci/controller/dwc/pcie-qcom.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

(quoted diff) +0 -0

b/drivers/pci/controller/dwc/pcie-qcom.c

drivers/pci/controller/dwc/pcie-qcom.c line 48

Quoted 8 lines

index da8058fd1925..372d2c8508b5 100644

drivers/pci/controller/dwc/pcie-qcom.c +1 -0

@@ -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(12,

16)

The mask definition is not correct. Should be GENMASK(20, 16)

drivers/pci/controller/dwc/pcie-qcom.c static int

Quoted 14 lines

drivers/pci/controller/dwc/pcie-qcom.c +3 -0

+#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)

@@ -118,6 +121,7 @@ struct qcom_pcie_resources_2_1_0 {
 	u32 tx_swing_full;
 	u32 tx_swing_low;
 	u32 rx0_eq;
+	u8 phy_tx0_term_offset;
 };
 
 struct qcom_pcie_resources_1_0_0 {

@@ -318,6 +322,11 @@ static int

qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)

Quoted 9 lines

drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie) · line 326

Quoted 5 lines

drivers/pci/controller/dwc/pcie-qcom.c +2 -0

 	if (IS_ERR(res->ext_reset))
 		return PTR_ERR(res->ext_reset);
 
+	if (of_device_is_compatible(dev->of_node, "qcom,pcie-ipq8064"))
+		res->phy_tx0_term_offset = 7;

Before your change the phy_tx0_term_offser was 0 for apq8064, but here you change it to 7, why?

apq8064 board should use qcom,pcie-apq8064 right? This should be set to 0 only with pcie-ipq8064 compatible. Tell me if I'm wrong.

Document qcom,pcie-ipq8064-v2 needed to use different phy_tx0_term_offset. In ipq8064 phy_tx0_term_offset is 7.
In ipq8064 v2 other SoC it's set to 0 by default.

Signed-off-by Ansuel Smith <[email protected]>

---
 Documentation/devicetree/bindings/pci/qcom,pcie.txt | 1 +
 1 file changed, 1 insertion(+)

This contains multiple fix for PCIe qcom driver.
Some optional reset and clocks were missing.
Fix a problem with no PARF programming that cause kernel lock on load. Add support to force gen 1 speed if needed. (due to hardware limitation) Add ipq8064 rev 2 support that use a different tx termination offset.

v3:
* Fix check reported by checkpatch --strict
* Rename force_gen1 to gen
* Fix spelling error
* Better describe qcom_clear_and_set_dword
* Make PARF deemph and equalization configurable

v2:
* Drop iATU programming (already done in pcie init)
* Use max-link-speed instead of force-gen1 custom definition
* Drop MRRS to 256B (Can't find a realy reason why this was suggested)
* Introduce a new variant for different revision of ipq8064

Abhishek Sahu (1):
PCI: qcom: change duplicate PCI reset to phy reset

Ansuel Smith (8):
PCI: qcom: add missing ipq806x clocks in PCIe driver

devicetree: bindings: pci: add missing clks to qcom,pcie
PCI: qcom: add missing reset for ipq806x
devicetree: bindings: pci: add ext reset to qcom,pcie

PCI: qcom: introduce qcom_clear_and_set_dword
PCI: qcom: add support for defining some PARF params
devicetree: bindings: pci: document PARF params bindings
devicetree: bindings: pci: add ipq8064 rev 2 variant to qcom,pcie

Sham Muthayyan (2):
PCI: qcom: add ipq8064 rev2 variant and set tx term offset
PCI: qcom: add Force GEN1 support

 .../devicetree/bindings/pci/qcom,pcie.txt     |  51 +++-
 drivers/pci/controller/dwc/pcie-qcom.c        | 241 ++++++++++++------
 2 files changed, 211 insertions(+), 81 deletions(-)

Signature · 1 line

2.25.1