linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver
@ 2020-03-20 18:34 Ansuel Smith
  2020-03-20 18:34 ` [PATCH 02/12] devicetree: bindings: pci: add missing clks to qcom,pcie Ansuel Smith
                   ` (12 more replies)
  0 siblings, 13 replies; 32+ messages in thread
From: Ansuel Smith @ 2020-03-20 18:34 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Ansuel Smith, Sham Muthayyan, Andy Gross, Bjorn Andersson,
	Bjorn Helgaas, Rob Herring, Mark Rutland, Lorenzo Pieralisi,
	Andrew Murray, Philipp Zabel, linux-arm-msm, linux-pci,
	devicetree, linux-kernel

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

Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 38 ++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 5ea527a6bd9f..f958c535de6e 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -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(dev, "aux");
+	if (IS_ERR(res->aux_clk))
+		return PTR_ERR(res->aux_clk);
+
+	res->ref_clk = devm_clk_get(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,28 @@ 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);
 	if (ret) {
 		dev_err(dev, "cannot prepare/enable phy clock\n");
 		goto err_clk_phy;
 	}
 
-	ret = clk_prepare_enable(res->core_clk);
+	ret = clk_prepare_enable(res->aux_clk);
 	if (ret) {
-		dev_err(dev, "cannot prepare/enable core clock\n");
-		goto err_clk_core;
+		dev_err(dev, "cannot prepare/enable aux clock\n");
+		goto err_clk_aux;
+	}
+
+	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 +396,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);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 02/12] devicetree: bindings: pci: add missing clks to qcom,pcie
  2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
@ 2020-03-20 18:34 ` Ansuel Smith
  2020-03-31 17:30   ` Rob Herring
  2020-03-20 18:34 ` [PATCH 03/12] pcie: qcom: change duplicate pci reset to phy reset Ansuel Smith
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 32+ messages in thread
From: Ansuel Smith @ 2020-03-20 18:34 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Ansuel Smith, Andy Gross, Bjorn Andersson, Bjorn Helgaas,
	Rob Herring, Mark Rutland, Lorenzo Pieralisi, Andrew Murray,
	Philipp Zabel, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

Document missing clks used in ipq806x soc

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.txt | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 981b4de12807..becdbdc0fffa 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -90,6 +90,8 @@
 	Definition: Should contain the following entries
 			- "core"	Clocks the pcie hw block
 			- "phy"		Clocks the pcie PHY block
+			- "aux" 	Clocks the pcie AUX block
+			- "ref" 	Clocks the pcie ref block
 - clock-names:
 	Usage: required for apq8084/ipq4019
 	Value type: <stringlist>
@@ -277,8 +279,10 @@
 				<0 0 0 4 &intc 0 39 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
 		clocks = <&gcc PCIE_A_CLK>,
 			 <&gcc PCIE_H_CLK>,
-			 <&gcc PCIE_PHY_CLK>;
-		clock-names = "core", "iface", "phy";
+			 <&gcc PCIE_PHY_CLK>,
+			 <&gcc PCIE_AUX_CLK>,
+			 <&gcc PCIE_ALT_REF_CLK>;
+		clock-names = "core", "iface", "phy", "aux", "ref";
 		resets = <&gcc PCIE_ACLK_RESET>,
 			 <&gcc PCIE_HCLK_RESET>,
 			 <&gcc PCIE_POR_RESET>,
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 03/12] pcie: qcom: change duplicate pci reset to phy reset
  2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
  2020-03-20 18:34 ` [PATCH 02/12] devicetree: bindings: pci: add missing clks to qcom,pcie Ansuel Smith
@ 2020-03-20 18:34 ` Ansuel Smith
  2020-03-20 18:34 ` [PATCH 04/12] pcie: qcom: Fixed pcie_phy_clk branch issue Ansuel Smith
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 32+ messages in thread
From: Ansuel Smith @ 2020-03-20 18:34 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Abhishek Sahu, Ansuel Smith, Andy Gross, Bjorn Andersson,
	Bjorn Helgaas, Rob Herring, Mark Rutland, Lorenzo Pieralisi,
	Andrew Murray, Philipp Zabel, linux-arm-msm, linux-pci,
	devicetree, linux-kernel

From: Abhishek Sahu <absahu@codeaurora.org>

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

Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index f958c535de6e..1fcc7fed8443 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -284,7 +284,7 @@ static void qcom_pcie_deinit_2_1_0(struct qcom_pcie *pcie)
 	reset_control_assert(res->axi_reset);
 	reset_control_assert(res->ahb_reset);
 	reset_control_assert(res->por_reset);
-	reset_control_assert(res->pci_reset);
+	reset_control_assert(res->phy_reset);
 	clk_disable_unprepare(res->iface_clk);
 	clk_disable_unprepare(res->core_clk);
 	clk_disable_unprepare(res->phy_clk);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 04/12] pcie: qcom: Fixed pcie_phy_clk branch issue
  2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
  2020-03-20 18:34 ` [PATCH 02/12] devicetree: bindings: pci: add missing clks to qcom,pcie Ansuel Smith
  2020-03-20 18:34 ` [PATCH 03/12] pcie: qcom: change duplicate pci reset to phy reset Ansuel Smith
@ 2020-03-20 18:34 ` Ansuel Smith
  2020-03-20 18:34 ` [PATCH 05/12] pcie: qcom: add missing reset for ipq806x Ansuel Smith
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 32+ messages in thread
From: Ansuel Smith @ 2020-03-20 18:34 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Ansuel Smith, Abhishek Sahu, Andy Gross, Bjorn Andersson,
	Bjorn Helgaas, Rob Herring, Mark Rutland, Lorenzo Pieralisi,
	Andrew Murray, Philipp Zabel, linux-arm-msm, linux-pci,
	devicetree, linux-kernel

Following backtraces are observed in PCIe deinit operation.

 Hardware name: Qualcomm (Flattened Device Tree)
 (unwind_backtrace) from [] (show_stack+0x10/0x14)
 (show_stack) from [] (dump_stack+0x84/0x98)
 (dump_stack) from [] (warn_slowpath_common+0x9c/0xb8)
 (warn_slowpath_common) from [] (warn_slowpath_fmt+0x30/0x40)
 (warn_slowpath_fmt) from [] (clk_branch_wait+0x114/0x120)
 (clk_branch_wait) from [] (clk_core_disable+0xd0/0x1f4)
 (clk_core_disable) from [] (clk_disable+0x24/0x30)
 (clk_disable) from [] (qcom_pcie_deinit_v0+0x6c/0xb8)
 (qcom_pcie_deinit_v0) from [] (qcom_pcie_host_init+0xe0/0xe8)
 (qcom_pcie_host_init) from [] (dw_pcie_host_init+0x3b0/0x538)
 (dw_pcie_host_init) from [] (qcom_pcie_probe+0x20c/0x2e4)

pcie_phy_clk is generated for PCIe controller itself and the
GCC controls its branch operation. This error is coming since
the assert operations turn off the parent clock before branch
clock. Now this patch moves clk_disable_unprepare before assert
operations.

Similarly, during probe function, the clock branch operation
should be done after dessert operation. Currently, it does not
generate any error since bootloader enables the pcie_phy_clk
but the same error is coming during probe, if bootloader
disables pcie_phy_clk.

Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 1fcc7fed8443..596731b54728 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -280,6 +280,7 @@ static void qcom_pcie_deinit_2_1_0(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_resources_2_1_0 *res = &pcie->res.v2_1_0;
 
+	clk_disable_unprepare(res->phy_clk);
 	reset_control_assert(res->pci_reset);
 	reset_control_assert(res->axi_reset);
 	reset_control_assert(res->ahb_reset);
@@ -287,7 +288,6 @@ static void qcom_pcie_deinit_2_1_0(struct qcom_pcie *pcie)
 	reset_control_assert(res->phy_reset);
 	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);
@@ -325,12 +325,6 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 		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->aux_clk);
 	if (ret) {
 		dev_err(dev, "cannot prepare/enable aux clock\n");
@@ -383,6 +377,12 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 		return ret;
 	}
 
+	ret = clk_prepare_enable(res->phy_clk);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable phy clock\n");
+		goto err_deassert_ahb;
+	}
+
 	/* wait for clock acquisition */
 	usleep_range(1000, 1500);
 
@@ -400,8 +400,6 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 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);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 05/12] pcie: qcom: add missing reset for ipq806x
  2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
                   ` (2 preceding siblings ...)
  2020-03-20 18:34 ` [PATCH 04/12] pcie: qcom: Fixed pcie_phy_clk branch issue Ansuel Smith
@ 2020-03-20 18:34 ` Ansuel Smith
  2020-03-20 18:51   ` Bjorn Helgaas
  2020-03-23  6:06   ` Philipp Zabel
  2020-03-20 18:34 ` [PATCH 06/12] devicetree: bindings: pci: add ext reset to qcom,pcie Ansuel Smith
                   ` (8 subsequent siblings)
  12 siblings, 2 replies; 32+ messages in thread
From: Ansuel Smith @ 2020-03-20 18:34 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Ansuel Smith, Sham Muthayyan, Andy Gross, Bjorn Andersson,
	Bjorn Helgaas, Rob Herring, Mark Rutland, Lorenzo Pieralisi,
	Andrew Murray, Philipp Zabel, linux-arm-msm, linux-pci,
	devicetree, linux-kernel

Add missing ext reset used by ipq806x soc in
pcie qcom driver

Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 596731b54728..ecc22fd27ea6 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -95,6 +95,7 @@ struct qcom_pcie_resources_2_1_0 {
 	struct reset_control *ahb_reset;
 	struct reset_control *por_reset;
 	struct reset_control *phy_reset;
+	struct reset_control *ext_reset;
 	struct regulator_bulk_data supplies[QCOM_PCIE_2_1_0_MAX_SUPPLY];
 };
 
@@ -272,6 +273,10 @@ static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
 	if (IS_ERR(res->por_reset))
 		return PTR_ERR(res->por_reset);
 
+	res->ext_reset = devm_reset_control_get(dev, "ext");
+	if (IS_ERR(res->ext_reset))
+		return PTR_ERR(res->ext_reset);
+
 	res->phy_reset = devm_reset_control_get_exclusive(dev, "phy");
 	return PTR_ERR_OR_ZERO(res->phy_reset);
 }
@@ -285,6 +290,7 @@ static void qcom_pcie_deinit_2_1_0(struct qcom_pcie *pcie)
 	reset_control_assert(res->axi_reset);
 	reset_control_assert(res->ahb_reset);
 	reset_control_assert(res->por_reset);
+	reset_control_assert(res->ext_reset);
 	reset_control_assert(res->phy_reset);
 	clk_disable_unprepare(res->iface_clk);
 	clk_disable_unprepare(res->core_clk);
@@ -301,18 +307,18 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 	u32 val;
 	int ret;
 
+	ret = reset_control_assert(res->ahb_reset);
+	if (ret) {
+		dev_err(dev, "cannot assert ahb reset\n");
+		return ret;
+	}
+
 	ret = regulator_bulk_enable(ARRAY_SIZE(res->supplies), res->supplies);
 	if (ret < 0) {
 		dev_err(dev, "cannot enable regulators\n");
 		return ret;
 	}
 
-	ret = reset_control_assert(res->ahb_reset);
-	if (ret) {
-		dev_err(dev, "cannot assert ahb reset\n");
-		goto err_assert_ahb;
-	}
-
 	ret = clk_prepare_enable(res->iface_clk);
 	if (ret) {
 		dev_err(dev, "cannot prepare/enable iface clock\n");
@@ -343,6 +349,12 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 		goto err_deassert_ahb;
 	}
 
+	ret = reset_control_deassert(res->ext_reset);
+	if (ret) {
+		dev_err(dev, "cannot assert ext reset\n");
+		goto err_deassert_ahb;
+	}
+
 	/* enable PCIe clocks and resets */
 	val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
 	val &= ~BIT(0);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 06/12] devicetree: bindings: pci: add ext reset to qcom,pcie
  2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
                   ` (3 preceding siblings ...)
  2020-03-20 18:34 ` [PATCH 05/12] pcie: qcom: add missing reset for ipq806x Ansuel Smith
@ 2020-03-20 18:34 ` Ansuel Smith
  2020-03-31 17:31   ` Rob Herring
  2020-03-20 18:34 ` [PATCH 07/12] pcie: qcom: add tx term offset support Ansuel Smith
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 32+ messages in thread
From: Ansuel Smith @ 2020-03-20 18:34 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Ansuel Smith, Andy Gross, Bjorn Andersson, Bjorn Helgaas,
	Rob Herring, Mark Rutland, Lorenzo Pieralisi, Andrew Murray,
	Philipp Zabel, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

Document ext reset used in ipq806x soc by qcom pcie driver

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index becdbdc0fffa..6efcef040741 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -179,6 +179,7 @@
 			- "pwr"			PWR reset
 			- "ahb"			AHB reset
 			- "phy_ahb"		PHY AHB reset
+			- "ext"			EXT reset
 
 - reset-names:
 	Usage: required for ipq8074
@@ -287,8 +288,9 @@
 			 <&gcc PCIE_HCLK_RESET>,
 			 <&gcc PCIE_POR_RESET>,
 			 <&gcc PCIE_PCI_RESET>,
-			 <&gcc PCIE_PHY_RESET>;
-		reset-names = "axi", "ahb", "por", "pci", "phy";
+			 <&gcc PCIE_PHY_RESET>,
+			 <&gcc PCIE_EXT_RESET>;
+		reset-names = "axi", "ahb", "por", "pci", "phy", "ext";
 		pinctrl-0 = <&pcie_pins_default>;
 		pinctrl-names = "default";
 	};
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 07/12] pcie: qcom: add tx term offset support
  2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
                   ` (4 preceding siblings ...)
  2020-03-20 18:34 ` [PATCH 06/12] devicetree: bindings: pci: add ext reset to qcom,pcie Ansuel Smith
@ 2020-03-20 18:34 ` Ansuel Smith
  2020-03-20 19:22   ` Bjorn Helgaas
  2020-04-01 20:40   ` Bjorn Andersson
  2020-03-20 18:34 ` [PATCH 08/12] devicetree: bindings: pci: add phy-tx0-term-offset to qcom,pcie Ansuel Smith
                   ` (6 subsequent siblings)
  12 siblings, 2 replies; 32+ messages in thread
From: Ansuel Smith @ 2020-03-20 18:34 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Sham Muthayyan, Ansuel Smith, Andy Gross, Bjorn Andersson,
	Bjorn Helgaas, Rob Herring, Mark Rutland, Lorenzo Pieralisi,
	Andrew Murray, Philipp Zabel, linux-arm-msm, linux-pci,
	devicetree, linux-kernel

From: Sham Muthayyan <smuthayy@codeaurora.org>

Add tx term offset support to pcie qcom driver
need in some revision of the ipq806x soc

Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index ecc22fd27ea6..8009e3117765 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -45,7 +45,13 @@
 #define PCIE_CAP_CPL_TIMEOUT_DISABLE		0x10
 
 #define PCIE20_PARF_PHY_CTRL			0x40
+#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	(0x1f << 16)
+#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		(x << 16)
+
 #define PCIE20_PARF_PHY_REFCLK			0x4C
+#define REF_SSP_EN				BIT(16)
+#define REF_USE_PAD				BIT(12)
+
 #define PCIE20_PARF_DBI_BASE_ADDR		0x168
 #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE		0x16C
 #define PCIE20_PARF_MHI_CLOCK_RESET_CTRL	0x174
@@ -77,6 +83,18 @@
 #define DBI_RO_WR_EN				1
 
 #define PERST_DELAY_US				1000
+/* PARF registers */
+#define PCIE20_PARF_PCS_DEEMPH			0x34
+#define PCS_DEEMPH_TX_DEEMPH_GEN1(x)		(x << 16)
+#define PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(x)	(x << 8)
+#define PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(x)	(x << 0)
+
+#define PCIE20_PARF_PCS_SWING			0x38
+#define PCS_SWING_TX_SWING_FULL(x)		(x << 8)
+#define PCS_SWING_TX_SWING_LOW(x)		(x << 0)
+
+#define PCIE20_PARF_CONFIG_BITS			0x50
+#define PHY_RX0_EQ(x)				(x << 24)
 
 #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
 #define SLV_ADDR_SPACE_SZ			0x10000000
@@ -97,6 +115,7 @@ struct qcom_pcie_resources_2_1_0 {
 	struct reset_control *phy_reset;
 	struct reset_control *ext_reset;
 	struct regulator_bulk_data supplies[QCOM_PCIE_2_1_0_MAX_SUPPLY];
+	uint8_t phy_tx0_term_offset;
 };
 
 struct qcom_pcie_resources_1_0_0 {
@@ -184,6 +203,16 @@ struct qcom_pcie {
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
 
+static inline void
+writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)
+{
+	u32 val = readl(addr);
+
+	val &= ~clear_mask;
+	val |= set_mask;
+	writel(val, addr);
+}
+
 static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
 {
 	gpiod_set_value_cansleep(pcie->reset, 1);
@@ -277,6 +306,10 @@ static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
 	if (IS_ERR(res->ext_reset))
 		return PTR_ERR(res->ext_reset);
 
+	if (of_property_read_u8(dev->of_node, "phy-tx0-term-offset",
+				&res->phy_tx0_term_offset))
+		res->phy_tx0_term_offset = 0;
+
 	res->phy_reset = devm_reset_control_get_exclusive(dev, "phy");
 	return PTR_ERR_OR_ZERO(res->phy_reset);
 }
@@ -304,7 +337,6 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 	struct qcom_pcie_resources_2_1_0 *res = &pcie->res.v2_1_0;
 	struct dw_pcie *pci = pcie->pci;
 	struct device *dev = pci->dev;
-	u32 val;
 	int ret;
 
 	ret = reset_control_assert(res->ahb_reset);
@@ -355,15 +387,26 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 		goto err_deassert_ahb;
 	}
 
-	/* enable PCIe clocks and resets */
-	val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
-	val &= ~BIT(0);
-	writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
+	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
+
+	/* Set Tx termination offset */
+	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL,
+		      PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK,
+		      PHY_CTRL_PHY_TX0_TERM_OFFSET(res->phy_tx0_term_offset));
+
+	/* PARF programming */
+	writel(PCS_DEEMPH_TX_DEEMPH_GEN1(0x18) |
+	       PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(0x18) |
+	       PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(0x22),
+	       pcie->parf + PCIE20_PARF_PCS_DEEMPH);
+	writel(PCS_SWING_TX_SWING_FULL(0x78) |
+	       PCS_SWING_TX_SWING_LOW(0x78),
+	       pcie->parf + PCIE20_PARF_PCS_SWING);
+	writel(PHY_RX0_EQ(0x4), pcie->parf + PCIE20_PARF_CONFIG_BITS);
 
-	/* enable external reference clock */
-	val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
-	val |= BIT(16);
-	writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
+	/* Enable reference clock */
+	writel_masked(pcie->parf + PCIE20_PARF_PHY_REFCLK,
+		      REF_USE_PAD, REF_SSP_EN);
 
 	ret = reset_control_deassert(res->phy_reset);
 	if (ret) {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 08/12] devicetree: bindings: pci: add phy-tx0-term-offset to qcom,pcie
  2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
                   ` (5 preceding siblings ...)
  2020-03-20 18:34 ` [PATCH 07/12] pcie: qcom: add tx term offset support Ansuel Smith
@ 2020-03-20 18:34 ` Ansuel Smith
  2020-03-31 17:32   ` Rob Herring
  2020-04-01 20:41   ` Bjorn Andersson
  2020-03-20 18:34 ` [PATCH 09/12] pcie: qcom: Programming the PCIE iATU for IPQ806x Ansuel Smith
                   ` (5 subsequent siblings)
  12 siblings, 2 replies; 32+ messages in thread
From: Ansuel Smith @ 2020-03-20 18:34 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Ansuel Smith, Andy Gross, Bjorn Andersson, Bjorn Helgaas,
	Rob Herring, Mark Rutland, Lorenzo Pieralisi, Andrew Murray,
	Philipp Zabel, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

Document phy-tx0-term-offset propriety to qcom pcie driver

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 6efcef040741..8c1d014f37b0 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -254,6 +254,12 @@
 			- "perst-gpios"	PCIe endpoint reset signal line
 			- "wake-gpios"	PCIe endpoint wake signal line
 
+- phy-tx0-term-offset:
+	Usage: optional
+	Value type: <u32>
+	Definition: If not defined is 0. In ipq806x is set to 7. In newer
+				revision (v2.0) the offset is zero.
+
 * Example for ipq/apq8064
 	pcie@1b500000 {
 		compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064", "snps,dw-pcie";
@@ -293,6 +299,7 @@
 		reset-names = "axi", "ahb", "por", "pci", "phy", "ext";
 		pinctrl-0 = <&pcie_pins_default>;
 		pinctrl-names = "default";
+		phy-tx0-term-offset = <7>;
 	};
 
 * Example for apq8084
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 09/12] pcie: qcom: Programming the PCIE iATU for IPQ806x
  2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
                   ` (6 preceding siblings ...)
  2020-03-20 18:34 ` [PATCH 08/12] devicetree: bindings: pci: add phy-tx0-term-offset to qcom,pcie Ansuel Smith
@ 2020-03-20 18:34 ` Ansuel Smith
  2020-03-20 19:26   ` Bjorn Helgaas
  2020-04-01 13:21   ` Stanimir Varbanov
  2020-03-20 18:34 ` [PATCH 10/12] pcie: qcom: add Force GEN1 support Ansuel Smith
                   ` (4 subsequent siblings)
  12 siblings, 2 replies; 32+ messages in thread
From: Ansuel Smith @ 2020-03-20 18:34 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Sham Muthayyan, Ansuel Smith, Andy Gross, Bjorn Andersson,
	Bjorn Helgaas, Rob Herring, Mark Rutland, Lorenzo Pieralisi,
	Andrew Murray, Philipp Zabel, linux-arm-msm, linux-pci,
	devicetree, linux-kernel

From: Sham Muthayyan <smuthayy@codeaurora.org>

Resolved PCIE EP detection errors caused due to missing iATU programming.

Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 78 ++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 8009e3117765..e26ba8f63d4f 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -77,6 +77,30 @@
 #define PCIE20_CAP_LINK_1			(PCIE20_CAP + 0x14)
 #define PCIE_CAP_LINK1_VAL			0x2FD7F
 
+#define PCIE20_CAP_LINKCTRLSTATUS		(PCIE20_CAP + 0x10)
+
+#define PCIE20_AXI_MSTR_RESP_COMP_CTRL0		0x818
+#define PCIE20_AXI_MSTR_RESP_COMP_CTRL1		0x81c
+
+#define PCIE20_PLR_IATU_VIEWPORT		0x900
+#define PCIE20_PLR_IATU_REGION_OUTBOUND		(0x0 << 31)
+#define PCIE20_PLR_IATU_REGION_INDEX(x)		(x << 0)
+
+#define PCIE20_PLR_IATU_CTRL1			0x904
+#define PCIE20_PLR_IATU_TYPE_CFG0		(0x4 << 0)
+#define PCIE20_PLR_IATU_TYPE_MEM		(0x0 << 0)
+
+#define PCIE20_PLR_IATU_CTRL2			0x908
+#define PCIE20_PLR_IATU_ENABLE			BIT(31)
+
+#define PCIE20_PLR_IATU_LBAR			0x90C
+#define PCIE20_PLR_IATU_UBAR			0x910
+#define PCIE20_PLR_IATU_LAR			0x914
+#define PCIE20_PLR_IATU_LTAR			0x918
+#define PCIE20_PLR_IATU_UTAR			0x91c
+
+#define MSM_PCIE_DEV_CFG_ADDR			0x01000000
+
 #define PCIE20_PARF_Q2A_FLUSH			0x1AC
 
 #define PCIE20_MISC_CONTROL_1_REG		0x8BC
@@ -251,6 +275,57 @@ static void qcom_pcie_2_1_0_ltssm_enable(struct qcom_pcie *pcie)
 	writel(val, pcie->elbi + PCIE20_ELBI_SYS_CTRL);
 }
 
+static void qcom_pcie_prog_viewport_cfg0(struct qcom_pcie *pcie, u32 busdev)
+{
+	struct pcie_port *pp = &pcie->pci->pp;
+
+	/*
+	 * program and enable address translation region 0 (device config
+	 * address space); region type config;
+	 * axi config address range to device config address range
+	 */
+	writel(PCIE20_PLR_IATU_REGION_OUTBOUND |
+	       PCIE20_PLR_IATU_REGION_INDEX(0),
+	       pcie->pci->dbi_base + PCIE20_PLR_IATU_VIEWPORT);
+
+	writel(PCIE20_PLR_IATU_TYPE_CFG0, pcie->pci->dbi_base + PCIE20_PLR_IATU_CTRL1);
+	writel(PCIE20_PLR_IATU_ENABLE, pcie->pci->dbi_base + PCIE20_PLR_IATU_CTRL2);
+	writel(pp->cfg0_base, pcie->pci->dbi_base + PCIE20_PLR_IATU_LBAR);
+	writel((pp->cfg0_base >> 32), pcie->pci->dbi_base + PCIE20_PLR_IATU_UBAR);
+	writel((pp->cfg0_base + pp->cfg0_size - 1),
+	       pcie->pci->dbi_base + PCIE20_PLR_IATU_LAR);
+	writel(busdev, pcie->pci->dbi_base + PCIE20_PLR_IATU_LTAR);
+	writel(0, pcie->pci->dbi_base + PCIE20_PLR_IATU_UTAR);
+}
+
+static void qcom_pcie_prog_viewport_mem2_outbound(struct qcom_pcie *pcie)
+{
+	struct pcie_port *pp = &pcie->pci->pp;
+
+	/*
+	 * program and enable address translation region 2 (device resource
+	 * address space); region type memory;
+	 * axi device bar address range to device bar address range
+	 */
+	writel(PCIE20_PLR_IATU_REGION_OUTBOUND |
+	       PCIE20_PLR_IATU_REGION_INDEX(2),
+	       pcie->pci->dbi_base + PCIE20_PLR_IATU_VIEWPORT);
+
+	writel(PCIE20_PLR_IATU_TYPE_MEM, pcie->pci->dbi_base + PCIE20_PLR_IATU_CTRL1);
+	writel(PCIE20_PLR_IATU_ENABLE, pcie->pci->dbi_base + PCIE20_PLR_IATU_CTRL2);
+	writel(pp->mem_base, pcie->pci->dbi_base + PCIE20_PLR_IATU_LBAR);
+	writel((pp->mem_base >> 32), pcie->pci->dbi_base + PCIE20_PLR_IATU_UBAR);
+	writel(pp->mem_base + pp->mem_size - 1,
+	       pcie->pci->dbi_base + PCIE20_PLR_IATU_LAR);
+	writel(pp->mem_bus_addr, pcie->pci->dbi_base + PCIE20_PLR_IATU_LTAR);
+	writel(upper_32_bits(pp->mem_bus_addr),
+	       pcie->pci->dbi_base + PCIE20_PLR_IATU_UTAR);
+
+	/* 256B PCIE buffer setting */
+	writel(0x1, pcie->pci->dbi_base + PCIE20_AXI_MSTR_RESP_COMP_CTRL0);
+	writel(0x1, pcie->pci->dbi_base + PCIE20_AXI_MSTR_RESP_COMP_CTRL1);
+}
+
 static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_resources_2_1_0 *res = &pcie->res.v2_1_0;
@@ -448,6 +523,9 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 	writel(CFG_BRIDGE_SB_INIT,
 	       pci->dbi_base + PCIE20_AXI_MSTR_RESP_COMP_CTRL1);
 
+	qcom_pcie_prog_viewport_cfg0(pcie, MSM_PCIE_DEV_CFG_ADDR);
+	qcom_pcie_prog_viewport_mem2_outbound(pcie);
+
 	return 0;
 
 err_deassert_ahb:
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 10/12] pcie: qcom: add Force GEN1 support
  2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
                   ` (7 preceding siblings ...)
  2020-03-20 18:34 ` [PATCH 09/12] pcie: qcom: Programming the PCIE iATU for IPQ806x Ansuel Smith
@ 2020-03-20 18:34 ` Ansuel Smith
  2020-03-20 19:37   ` Bjorn Helgaas
  2020-03-20 18:34 ` [PATCH 11/12] devicetree: bindings: pci: add force_gen1 for qcom,pcie Ansuel Smith
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 32+ messages in thread
From: Ansuel Smith @ 2020-03-20 18:34 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Sham Muthayyan, Ansuel Smith, Andy Gross, Bjorn Andersson,
	Bjorn Helgaas, Rob Herring, Mark Rutland, Lorenzo Pieralisi,
	Andrew Murray, Philipp Zabel, linux-arm-msm, linux-pci,
	devicetree, linux-kernel

From: Sham Muthayyan <smuthayy@codeaurora.org>

Add Force GEN1 support needed in some ipq806x board
that needs to limit some pcie line to gen1 for some
hardware limitation

Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index e26ba8f63d4f..03130a3206b4 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -123,6 +123,8 @@
 #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
 #define SLV_ADDR_SPACE_SZ			0x10000000
 
+#define PCIE20_LNK_CONTROL2_LINK_STATUS2        0xA0
+
 #define DEVICE_TYPE_RC				0x4
 
 #define QCOM_PCIE_2_1_0_MAX_SUPPLY	3
@@ -223,6 +225,7 @@ struct qcom_pcie {
 	struct phy *phy;
 	struct gpio_desc *reset;
 	const struct qcom_pcie_ops *ops;
+	uint32_t force_gen1;
 };
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
@@ -515,6 +518,11 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 
 	/* wait for clock acquisition */
 	usleep_range(1000, 1500);
+	if (pcie->force_gen1) {
+		writel_relaxed((readl_relaxed(
+			pcie->pci->dbi_base + PCIE20_LNK_CONTROL2_LINK_STATUS2) | 1),
+			pcie->pci->dbi_base + PCIE20_LNK_CONTROL2_LINK_STATUS2);
+	}
 
 
 	/* Set the Max TLP size to 2K, instead of using default of 4K */
@@ -1487,6 +1495,8 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	struct dw_pcie *pci;
 	struct qcom_pcie *pcie;
 	int ret;
+	uint32_t force_gen1 = 0;
+	struct device_node *np = pdev->dev.of_node;
 
 	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
 	if (!pcie)
@@ -1517,6 +1527,9 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 		goto err_pm_runtime_put;
 	}
 
+	of_property_read_u32(np, "force_gen1", &force_gen1);
+	pcie->force_gen1 = force_gen1;
+
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "parf");
 	pcie->parf = devm_ioremap_resource(dev, res);
 	if (IS_ERR(pcie->parf)) {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 11/12] devicetree: bindings: pci: add force_gen1 for qcom,pcie
  2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
                   ` (8 preceding siblings ...)
  2020-03-20 18:34 ` [PATCH 10/12] pcie: qcom: add Force GEN1 support Ansuel Smith
@ 2020-03-20 18:34 ` Ansuel Smith
  2020-03-31 17:33   ` Rob Herring
  2020-04-01 13:17   ` Stanimir Varbanov
  2020-03-20 18:34 ` [PATCH 12/12] pcie: qcom: Set PCIE MRRS and MPS to 256B Ansuel Smith
                   ` (2 subsequent siblings)
  12 siblings, 2 replies; 32+ messages in thread
From: Ansuel Smith @ 2020-03-20 18:34 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Ansuel Smith, Andy Gross, Bjorn Andersson, Bjorn Helgaas,
	Rob Herring, Mark Rutland, Lorenzo Pieralisi, Andrew Murray,
	Philipp Zabel, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

Document force_gen1 optional definition to limit pcie
line to GEN1 speed

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 8c1d014f37b0..766876465c42 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -260,6 +260,11 @@
 	Definition: If not defined is 0. In ipq806x is set to 7. In newer
 				revision (v2.0) the offset is zero.
 
+- force_gen1:
+	Usage: optional
+	Value type: <u32>
+	Definition: Set 1 to force the pcie line to GEN1
+
 * Example for ipq/apq8064
 	pcie@1b500000 {
 		compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064", "snps,dw-pcie";
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 12/12] pcie: qcom: Set PCIE MRRS and MPS to 256B
  2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
                   ` (9 preceding siblings ...)
  2020-03-20 18:34 ` [PATCH 11/12] devicetree: bindings: pci: add force_gen1 for qcom,pcie Ansuel Smith
@ 2020-03-20 18:34 ` Ansuel Smith
  2020-03-20 19:46   ` Bjorn Helgaas
  2020-03-20 18:47 ` [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Bjorn Helgaas
  2020-04-01 13:01 ` Stanimir Varbanov
  12 siblings, 1 reply; 32+ messages in thread
From: Ansuel Smith @ 2020-03-20 18:34 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Sriram Palanisamy, Ansuel Smith, Andy Gross, Bjorn Andersson,
	Bjorn Helgaas, Rob Herring, Mark Rutland, Lorenzo Pieralisi,
	Andrew Murray, Philipp Zabel, linux-arm-msm, linux-pci,
	devicetree, linux-kernel

From: Sriram Palanisamy <gpalan@codeaurora.org>

Set Max Read Request Size and Max Payload Size to 256 bytes,
per chip team recommendation.

Signed-off-by: Gokul Sriram Palanisamy <gpalan@codeaurora.org>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 37 ++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 03130a3206b4..ad437c6f49a0 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -125,6 +125,14 @@
 
 #define PCIE20_LNK_CONTROL2_LINK_STATUS2        0xA0
 
+#define __set(v, a, b)	(((v) << (b)) & GENMASK(a, b))
+#define __mask(a, b)	(((1 << ((a) + 1)) - 1) & ~((1 << (b)) - 1))
+#define PCIE20_DEV_CAS			0x78
+#define PCIE20_MRRS_MASK		__mask(14, 12)
+#define PCIE20_MRRS(x)			__set(x, 14, 12)
+#define PCIE20_MPS_MASK			__mask(7, 5)
+#define PCIE20_MPS(x)			__set(x, 7, 5)
+
 #define DEVICE_TYPE_RC				0x4
 
 #define QCOM_PCIE_2_1_0_MAX_SUPPLY	3
@@ -1595,6 +1603,35 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static void qcom_pcie_fixup_final(struct pci_dev *pcidev)
+{
+	int cap, err;
+	u16 ctl, reg_val;
+
+	cap = pci_pcie_cap(pcidev);
+	if (!cap)
+		return;
+
+	err = pci_read_config_word(pcidev, cap + PCI_EXP_DEVCTL, &ctl);
+
+	if (err)
+		return;
+
+	reg_val = ctl;
+
+	if (((reg_val & PCIE20_MRRS_MASK) >> 12) > 1)
+		reg_val = (reg_val & ~(PCIE20_MRRS_MASK)) | PCIE20_MRRS(0x1);
+
+	if (((ctl & PCIE20_MPS_MASK) >> 5) > 1)
+		reg_val = (reg_val & ~(PCIE20_MPS_MASK)) | PCIE20_MPS(0x1);
+
+	err = pci_write_config_word(pcidev, cap + PCI_EXP_DEVCTL, reg_val);
+
+	if (err)
+		dev_err(&pcidev->dev, "pcie config write failed %d\n", err);
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, qcom_pcie_fixup_final);
+
 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 },
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver
  2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
                   ` (10 preceding siblings ...)
  2020-03-20 18:34 ` [PATCH 12/12] pcie: qcom: Set PCIE MRRS and MPS to 256B Ansuel Smith
@ 2020-03-20 18:47 ` Bjorn Helgaas
  2020-04-01 13:01 ` Stanimir Varbanov
  12 siblings, 0 replies; 32+ messages in thread
From: Bjorn Helgaas @ 2020-03-20 18:47 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Stanimir Varbanov, Sham Muthayyan, Andy Gross, Bjorn Andersson,
	Rob Herring, Mark Rutland, Lorenzo Pieralisi, Andrew Murray,
	Philipp Zabel, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

Please add a cover letter with the patches as responses to it.

Make your subjects match in capitalization, verb tense, etc.:

  $ git log --oneline drivers/pci/controller/dwc/pcie-qcom.c
  ed8cc3b1fc84 PCI: qcom: Add support for SDM845 PCIe controller
  64adde31c8e9 PCI: qcom: Ensure that PERST is asserted for at least 100 ms
  67021ae0bbe9 PCI: qcom: Add QCS404 PCIe controller support
  5aa180974e4d PCI: qcom: Use clk bulk API for 2.4.0 controllers
  322f03436692 PCI: qcom: Use default config space read function
  02b485e31d98 PCI: qcom: Don't deassert reset GPIO during probe
  6e5da6f7d824 PCI: qcom: Fix error handling in runtime PM support
  739cd35918b7 PCI: qcom: Drop unnecessary root_bus_nr setting

On Fri, Mar 20, 2020 at 07:34:43PM +0100, Ansuel Smith wrote:
> Aux and Ref clk are missing in pcie qcom driver.
> Add support in the driver to fix pcie inizialization
> in ipq806x

s/pcie/PCIe/ in English text like commit logs and comments.
s/inizialization/initialization/

It'd be useful to have enough context to know what "ipq806x" is.

Add period at end of sentence.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 05/12] pcie: qcom: add missing reset for ipq806x
  2020-03-20 18:34 ` [PATCH 05/12] pcie: qcom: add missing reset for ipq806x Ansuel Smith
@ 2020-03-20 18:51   ` Bjorn Helgaas
  2020-03-23  6:06   ` Philipp Zabel
  1 sibling, 0 replies; 32+ messages in thread
From: Bjorn Helgaas @ 2020-03-20 18:51 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Stanimir Varbanov, Sham Muthayyan, Andy Gross, Bjorn Andersson,
	Rob Herring, Mark Rutland, Lorenzo Pieralisi, Andrew Murray,
	Philipp Zabel, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

On Fri, Mar 20, 2020 at 07:34:47PM +0100, Ansuel Smith wrote:
> Add missing ext reset used by ipq806x soc in
> pcie qcom driver

You say "missing" -- does that mean this is a *new* requirement for
this ipq806x device, and previous devices work correctly without this
patch?

Or does this fix an omission and previous devices actually didn't work
correctly?

s/soc/SoC/
s/pcie/PCIe/

Period at end of sentence.  Please wrap to fill 75 columns.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 07/12] pcie: qcom: add tx term offset support
  2020-03-20 18:34 ` [PATCH 07/12] pcie: qcom: add tx term offset support Ansuel Smith
@ 2020-03-20 19:22   ` Bjorn Helgaas
  2020-04-01 20:40   ` Bjorn Andersson
  1 sibling, 0 replies; 32+ messages in thread
From: Bjorn Helgaas @ 2020-03-20 19:22 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Stanimir Varbanov, Sham Muthayyan, Andy Gross, Bjorn Andersson,
	Rob Herring, Mark Rutland, Lorenzo Pieralisi, Andrew Murray,
	Philipp Zabel, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

On Fri, Mar 20, 2020 at 07:34:49PM +0100, Ansuel Smith wrote:
> From: Sham Muthayyan <smuthayy@codeaurora.org>
> 
> Add tx term offset support to pcie qcom driver
> need in some revision of the ipq806x soc

The usual (s/pcie/PCIe/, s/soc/SoC/, wrap to fill 75 columns, add
period at end).  Apply to all patches.

> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	(0x1f << 16)
> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		(x << 16)

Since the rest of the file uses BIT(), I think it would make sense to
use GENMASK() here.  And below, of course.

> +static inline void
> +writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)

Follow coding style of rest of file, i.e.,

  static inline void writel_masked(...)

The name "writel_masked" suggests that we're writing "val & mask" to a
register, but that's not what's happening.  This is really a "clear
some bits and set others" operation.

The names are wordy, but we do have pci_clear_and_set_dword(),
pcie_capability_clear_and_set_word(), etc., functions that work
similarly.

> +{
> +	u32 val = readl(addr);
> +
> +	val &= ~clear_mask;
> +	val |= set_mask;
> +	writel(val, addr);
> +}

> +	/* Set Tx termination offset */

Capitalize consistently with other comments in the file.  Below also.
Hmm, I see the file is already a bit of a mess in that regard.  So
just do what you can.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 09/12] pcie: qcom: Programming the PCIE iATU for IPQ806x
  2020-03-20 18:34 ` [PATCH 09/12] pcie: qcom: Programming the PCIE iATU for IPQ806x Ansuel Smith
@ 2020-03-20 19:26   ` Bjorn Helgaas
  2020-04-01 13:21   ` Stanimir Varbanov
  1 sibling, 0 replies; 32+ messages in thread
From: Bjorn Helgaas @ 2020-03-20 19:26 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Stanimir Varbanov, Sham Muthayyan, Andy Gross, Bjorn Andersson,
	Rob Herring, Mark Rutland, Lorenzo Pieralisi, Andrew Murray,
	Philipp Zabel, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

s/Programming/Program/

Capitalize IPQ806x consistently (other patches mention "ipq806x").

On Fri, Mar 20, 2020 at 07:34:51PM +0100, Ansuel Smith wrote:
> From: Sham Muthayyan <smuthayy@codeaurora.org>
> 
> Resolved PCIE EP detection errors caused due to missing iATU programming.

s/Resolved/Resolve/
s/PCIE/PCIe/

Is this a bug fix?  If so, can you cite the commit that introduced the
bug?  This is useful when backporting fixes.  Same question applies to
the other patches as well.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 10/12] pcie: qcom: add Force GEN1 support
  2020-03-20 18:34 ` [PATCH 10/12] pcie: qcom: add Force GEN1 support Ansuel Smith
@ 2020-03-20 19:37   ` Bjorn Helgaas
  0 siblings, 0 replies; 32+ messages in thread
From: Bjorn Helgaas @ 2020-03-20 19:37 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Stanimir Varbanov, Sham Muthayyan, Andy Gross, Bjorn Andersson,
	Rob Herring, Mark Rutland, Lorenzo Pieralisi, Andrew Murray,
	Philipp Zabel, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

On Fri, Mar 20, 2020 at 07:34:52PM +0100, Ansuel Smith wrote:
> From: Sham Muthayyan <smuthayy@codeaurora.org>
> 
> Add Force GEN1 support needed in some ipq806x board
> that needs to limit some pcie line to gen1 for some
> hardware limitation

Usual commit log comments.

> +	uint32_t force_gen1;

unsigned int force_gen1 : 1

> +	of_property_read_u32(np, "force_gen1", &force_gen1);
> +	pcie->force_gen1 = force_gen1;

I think there's a more or less standard property you can use for this
instead of inventing a new one specific to this device.

Documentation/devicetree/bindings/pci/pci.txt:
"max-link-speed"

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 12/12] pcie: qcom: Set PCIE MRRS and MPS to 256B
  2020-03-20 18:34 ` [PATCH 12/12] pcie: qcom: Set PCIE MRRS and MPS to 256B Ansuel Smith
@ 2020-03-20 19:46   ` Bjorn Helgaas
  0 siblings, 0 replies; 32+ messages in thread
From: Bjorn Helgaas @ 2020-03-20 19:46 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Stanimir Varbanov, Sriram Palanisamy, Andy Gross,
	Bjorn Andersson, Rob Herring, Mark Rutland, Lorenzo Pieralisi,
	Andrew Murray, Philipp Zabel, linux-arm-msm, linux-pci,
	devicetree, linux-kernel

On Fri, Mar 20, 2020 at 07:34:54PM +0100, Ansuel Smith wrote:
> From: Sriram Palanisamy <gpalan@codeaurora.org>
> 
> Set Max Read Request Size and Max Payload Size to 256 bytes,
> per chip team recommendation.

Is this to avoid a device defect or to optimize performance?

This should not be done in an individual driver for performance
reasons because these parameters need to be managed at the system
level.

If this is to work around a device defect, we probably need to think
about a quirk that changes the device capabilities advertised by this
bridge and then changes to the PCI core code to take that into
account.

> Signed-off-by: Gokul Sriram Palanisamy <gpalan@codeaurora.org>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 37 ++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 03130a3206b4..ad437c6f49a0 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -125,6 +125,14 @@
>  
>  #define PCIE20_LNK_CONTROL2_LINK_STATUS2        0xA0
>  
> +#define __set(v, a, b)	(((v) << (b)) & GENMASK(a, b))
> +#define __mask(a, b)	(((1 << ((a) + 1)) - 1) & ~((1 << (b)) - 1))
> +#define PCIE20_DEV_CAS			0x78
> +#define PCIE20_MRRS_MASK		__mask(14, 12)
> +#define PCIE20_MRRS(x)			__set(x, 14, 12)
> +#define PCIE20_MPS_MASK			__mask(7, 5)
> +#define PCIE20_MPS(x)			__set(x, 7, 5)

These should all be the generic PCI_EXP_DEVCTL_READRQ and similar
#defines, since you use them on values from PCI_EXP_DEVCTL.

>  #define DEVICE_TYPE_RC				0x4
>  
>  #define QCOM_PCIE_2_1_0_MAX_SUPPLY	3
> @@ -1595,6 +1603,35 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  	return ret;
>  }
>  
> +static void qcom_pcie_fixup_final(struct pci_dev *pcidev)
> +{
> +	int cap, err;
> +	u16 ctl, reg_val;
> +
> +	cap = pci_pcie_cap(pcidev);
> +	if (!cap)
> +		return;
> +
> +	err = pci_read_config_word(pcidev, cap + PCI_EXP_DEVCTL, &ctl);
> +
> +	if (err)
> +		return;
> +
> +	reg_val = ctl;
> +
> +	if (((reg_val & PCIE20_MRRS_MASK) >> 12) > 1)
> +		reg_val = (reg_val & ~(PCIE20_MRRS_MASK)) | PCIE20_MRRS(0x1);
> +
> +	if (((ctl & PCIE20_MPS_MASK) >> 5) > 1)
> +		reg_val = (reg_val & ~(PCIE20_MPS_MASK)) | PCIE20_MPS(0x1);
> +
> +	err = pci_write_config_word(pcidev, cap + PCI_EXP_DEVCTL, reg_val);
> +
> +	if (err)
> +		dev_err(&pcidev->dev, "pcie config write failed %d\n", err);
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, qcom_pcie_fixup_final);
> +
>  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 },
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 05/12] pcie: qcom: add missing reset for ipq806x
  2020-03-20 18:34 ` [PATCH 05/12] pcie: qcom: add missing reset for ipq806x Ansuel Smith
  2020-03-20 18:51   ` Bjorn Helgaas
@ 2020-03-23  6:06   ` Philipp Zabel
  1 sibling, 0 replies; 32+ messages in thread
From: Philipp Zabel @ 2020-03-23  6:06 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Stanimir Varbanov, Sham Muthayyan, Andy Gross, Bjorn Andersson,
	Bjorn Helgaas, Rob Herring, Mark Rutland, Lorenzo Pieralisi,
	Andrew Murray, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

Hi Ansuel,

On Fri, Mar 20, 2020 at 07:34:47PM +0100, Ansuel Smith wrote:
> Add missing ext reset used by ipq806x soc in
> pcie qcom driver
> 
> Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 596731b54728..ecc22fd27ea6 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -95,6 +95,7 @@ struct qcom_pcie_resources_2_1_0 {
>  	struct reset_control *ahb_reset;
>  	struct reset_control *por_reset;
>  	struct reset_control *phy_reset;
> +	struct reset_control *ext_reset;
>  	struct regulator_bulk_data supplies[QCOM_PCIE_2_1_0_MAX_SUPPLY];
>  };
>  
> @@ -272,6 +273,10 @@ static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
>  	if (IS_ERR(res->por_reset))
>  		return PTR_ERR(res->por_reset);
>  
> +	res->ext_reset = devm_reset_control_get(dev, "ext");

Please use devm_reset_control_get_exclusive() instead.

> +	if (IS_ERR(res->ext_reset))
> +		return PTR_ERR(res->ext_reset);
> +
>  	res->phy_reset = devm_reset_control_get_exclusive(dev, "phy");
>  	return PTR_ERR_OR_ZERO(res->phy_reset);
>  }
> @@ -285,6 +290,7 @@ static void qcom_pcie_deinit_2_1_0(struct qcom_pcie *pcie)
>  	reset_control_assert(res->axi_reset);
>  	reset_control_assert(res->ahb_reset);
>  	reset_control_assert(res->por_reset);
> +	reset_control_assert(res->ext_reset);
>  	reset_control_assert(res->phy_reset);
>  	clk_disable_unprepare(res->iface_clk);
>  	clk_disable_unprepare(res->core_clk);
> @@ -301,18 +307,18 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
>  	u32 val;
>  	int ret;
>  
> +	ret = reset_control_assert(res->ahb_reset);
> +	if (ret) {
> +		dev_err(dev, "cannot assert ahb reset\n");
> +		return ret;
> +	}
> +
>  	ret = regulator_bulk_enable(ARRAY_SIZE(res->supplies), res->supplies);
>  	if (ret < 0) {
>  		dev_err(dev, "cannot enable regulators\n");
>  		return ret;
>  	}
>  
> -	ret = reset_control_assert(res->ahb_reset);
> -	if (ret) {
> -		dev_err(dev, "cannot assert ahb reset\n");
> -		goto err_assert_ahb;
> -	}
> -

This change is not described in the commit message.

regards
Philipp

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 02/12] devicetree: bindings: pci: add missing clks to qcom,pcie
  2020-03-20 18:34 ` [PATCH 02/12] devicetree: bindings: pci: add missing clks to qcom,pcie Ansuel Smith
@ 2020-03-31 17:30   ` Rob Herring
  0 siblings, 0 replies; 32+ messages in thread
From: Rob Herring @ 2020-03-31 17:30 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Stanimir Varbanov, Andy Gross, Bjorn Andersson, Bjorn Helgaas,
	Mark Rutland, Lorenzo Pieralisi, Andrew Murray, Philipp Zabel,
	linux-arm-msm, linux-pci, devicetree, linux-kernel

On Fri, Mar 20, 2020 at 07:34:44PM +0100, Ansuel Smith wrote:
> Document missing clks used in ipq806x soc
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.txt | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

What a mess the clocks are for this binding... 

Oh well,

Acked-by: Rob Herring <robh@kernel.org>

> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> index 981b4de12807..becdbdc0fffa 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> @@ -90,6 +90,8 @@
>  	Definition: Should contain the following entries
>  			- "core"	Clocks the pcie hw block
>  			- "phy"		Clocks the pcie PHY block
> +			- "aux" 	Clocks the pcie AUX block
> +			- "ref" 	Clocks the pcie ref block
>  - clock-names:
>  	Usage: required for apq8084/ipq4019
>  	Value type: <stringlist>
> @@ -277,8 +279,10 @@
>  				<0 0 0 4 &intc 0 39 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
>  		clocks = <&gcc PCIE_A_CLK>,
>  			 <&gcc PCIE_H_CLK>,
> -			 <&gcc PCIE_PHY_CLK>;
> -		clock-names = "core", "iface", "phy";
> +			 <&gcc PCIE_PHY_CLK>,
> +			 <&gcc PCIE_AUX_CLK>,
> +			 <&gcc PCIE_ALT_REF_CLK>;
> +		clock-names = "core", "iface", "phy", "aux", "ref";
>  		resets = <&gcc PCIE_ACLK_RESET>,
>  			 <&gcc PCIE_HCLK_RESET>,
>  			 <&gcc PCIE_POR_RESET>,
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 06/12] devicetree: bindings: pci: add ext reset to qcom,pcie
  2020-03-20 18:34 ` [PATCH 06/12] devicetree: bindings: pci: add ext reset to qcom,pcie Ansuel Smith
@ 2020-03-31 17:31   ` Rob Herring
  0 siblings, 0 replies; 32+ messages in thread
From: Rob Herring @ 2020-03-31 17:31 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Stanimir Varbanov, Ansuel Smith, Andy Gross, Bjorn Andersson,
	Bjorn Helgaas, Mark Rutland, Lorenzo Pieralisi, Andrew Murray,
	Philipp Zabel, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

On Fri, 20 Mar 2020 19:34:48 +0100, Ansuel Smith wrote:
> Document ext reset used in ipq806x soc by qcom pcie driver
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.txt | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 08/12] devicetree: bindings: pci: add phy-tx0-term-offset to qcom,pcie
  2020-03-20 18:34 ` [PATCH 08/12] devicetree: bindings: pci: add phy-tx0-term-offset to qcom,pcie Ansuel Smith
@ 2020-03-31 17:32   ` Rob Herring
  2020-04-01 12:09     ` R: " ansuelsmth
  2020-04-01 20:41   ` Bjorn Andersson
  1 sibling, 1 reply; 32+ messages in thread
From: Rob Herring @ 2020-03-31 17:32 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Stanimir Varbanov, Andy Gross, Bjorn Andersson, Bjorn Helgaas,
	Mark Rutland, Lorenzo Pieralisi, Andrew Murray, Philipp Zabel,
	linux-arm-msm, linux-pci, devicetree, linux-kernel

On Fri, Mar 20, 2020 at 07:34:50PM +0100, Ansuel Smith wrote:
> Document phy-tx0-term-offset propriety to qcom pcie driver

propriety?

> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> index 6efcef040741..8c1d014f37b0 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> @@ -254,6 +254,12 @@
>  			- "perst-gpios"	PCIe endpoint reset signal line
>  			- "wake-gpios"	PCIe endpoint wake signal line
>  
> +- phy-tx0-term-offset:

Needs a vendor prefix.

> +	Usage: optional
> +	Value type: <u32>
> +	Definition: If not defined is 0. In ipq806x is set to 7. In newer
> +				revision (v2.0) the offset is zero.
> +
>  * Example for ipq/apq8064
>  	pcie@1b500000 {
>  		compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064", "snps,dw-pcie";
> @@ -293,6 +299,7 @@
>  		reset-names = "axi", "ahb", "por", "pci", "phy", "ext";
>  		pinctrl-0 = <&pcie_pins_default>;
>  		pinctrl-names = "default";
> +		phy-tx0-term-offset = <7>;
>  	};
>  
>  * Example for apq8084
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 11/12] devicetree: bindings: pci: add force_gen1 for qcom,pcie
  2020-03-20 18:34 ` [PATCH 11/12] devicetree: bindings: pci: add force_gen1 for qcom,pcie Ansuel Smith
@ 2020-03-31 17:33   ` Rob Herring
  2020-04-01 12:09     ` R: " ansuelsmth
  2020-04-01 13:17   ` Stanimir Varbanov
  1 sibling, 1 reply; 32+ messages in thread
From: Rob Herring @ 2020-03-31 17:33 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Stanimir Varbanov, Andy Gross, Bjorn Andersson, Bjorn Helgaas,
	Mark Rutland, Lorenzo Pieralisi, Andrew Murray, Philipp Zabel,
	linux-arm-msm, linux-pci, devicetree, linux-kernel

On Fri, Mar 20, 2020 at 07:34:53PM +0100, Ansuel Smith wrote:
> Document force_gen1 optional definition to limit pcie
> line to GEN1 speed
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> index 8c1d014f37b0..766876465c42 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> @@ -260,6 +260,11 @@
>  	Definition: If not defined is 0. In ipq806x is set to 7. In newer
>  				revision (v2.0) the offset is zero.
>  
> +- force_gen1:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: Set 1 to force the pcie line to GEN1
> +

I believe we have a standard property 'link-speed' for this purpose.

>  * Example for ipq/apq8064
>  	pcie@1b500000 {
>  		compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064", "snps,dw-pcie";
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* R: [PATCH 11/12] devicetree: bindings: pci: add force_gen1 for qcom,pcie
  2020-03-31 17:33   ` Rob Herring
@ 2020-04-01 12:09     ` ansuelsmth
  0 siblings, 0 replies; 32+ messages in thread
From: ansuelsmth @ 2020-04-01 12:09 UTC (permalink / raw)
  To: 'Rob Herring'
  Cc: 'Stanimir Varbanov', 'Andy Gross',
	'Bjorn Andersson', 'Bjorn Helgaas',
	'Mark Rutland', 'Lorenzo Pieralisi',
	'Andrew Murray', 'Philipp Zabel',
	linux-arm-msm, linux-pci, devicetree, linux-kernel



> -----Messaggio originale-----
> Da: Rob Herring <robh@kernel.org>
> Inviato: martedì 31 marzo 2020 19:34
> A: Ansuel Smith <ansuelsmth@gmail.com>
> Cc: Stanimir Varbanov <svarbanov@mm-sol.com>; Andy Gross
> <agross@kernel.org>; Bjorn Andersson <bjorn.andersson@linaro.org>;
> Bjorn Helgaas <bhelgaas@google.com>; Mark Rutland
> <mark.rutland@arm.com>; Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>;
> Andrew Murray <amurray@thegoodpenguin.co.uk>; Philipp Zabel
> <p.zabel@pengutronix.de>; linux-arm-msm@vger.kernel.org; linux-
> pci@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Oggetto: Re: [PATCH 11/12] devicetree: bindings: pci: add force_gen1 for
> qcom,pcie
> 
> On Fri, Mar 20, 2020 at 07:34:53PM +0100, Ansuel Smith wrote:
> > Document force_gen1 optional definition to limit pcie
> > line to GEN1 speed
> >
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/pci/qcom,pcie.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > index 8c1d014f37b0..766876465c42 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > @@ -260,6 +260,11 @@
> >  	Definition: If not defined is 0. In ipq806x is set to 7. In newer
> >  				revision (v2.0) the offset is zero.
> >
> > +- force_gen1:
> > +	Usage: optional
> > +	Value type: <u32>
> > +	Definition: Set 1 to force the pcie line to GEN1
> > +
> 
> I believe we have a standard property 'link-speed' for this purpose.
> 

Yes this will be dropped in v2

> >  * Example for ipq/apq8064
> >  	pcie@1b500000 {
> >  		compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064",
> "snps,dw-pcie";
> > --
> > 2.25.1
> >


^ permalink raw reply	[flat|nested] 32+ messages in thread

* R: [PATCH 08/12] devicetree: bindings: pci: add phy-tx0-term-offset to qcom,pcie
  2020-03-31 17:32   ` Rob Herring
@ 2020-04-01 12:09     ` ansuelsmth
  0 siblings, 0 replies; 32+ messages in thread
From: ansuelsmth @ 2020-04-01 12:09 UTC (permalink / raw)
  To: 'Rob Herring'
  Cc: 'Stanimir Varbanov', 'Andy Gross',
	'Bjorn Andersson', 'Bjorn Helgaas',
	'Mark Rutland', 'Lorenzo Pieralisi',
	'Andrew Murray', 'Philipp Zabel',
	linux-arm-msm, linux-pci, devicetree, linux-kernel



> -----Messaggio originale-----
> Da: Rob Herring <robh@kernel.org>
> Inviato: martedì 31 marzo 2020 19:33
> A: Ansuel Smith <ansuelsmth@gmail.com>
> Cc: Stanimir Varbanov <svarbanov@mm-sol.com>; Andy Gross
> <agross@kernel.org>; Bjorn Andersson <bjorn.andersson@linaro.org>;
> Bjorn Helgaas <bhelgaas@google.com>; Mark Rutland
> <mark.rutland@arm.com>; Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>;
> Andrew Murray <amurray@thegoodpenguin.co.uk>; Philipp Zabel
> <p.zabel@pengutronix.de>; linux-arm-msm@vger.kernel.org; linux-
> pci@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Oggetto: Re: [PATCH 08/12] devicetree: bindings: pci: add phy-tx0-term-
> offset to qcom,pcie
> 
> On Fri, Mar 20, 2020 at 07:34:50PM +0100, Ansuel Smith wrote:
> > Document phy-tx0-term-offset propriety to qcom pcie driver
> 
> propriety?
> 
> >
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/pci/qcom,pcie.txt | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > index 6efcef040741..8c1d014f37b0 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > @@ -254,6 +254,12 @@
> >  			- "perst-gpios"	PCIe endpoint reset signal line
> >  			- "wake-gpios"	PCIe endpoint wake signal line
> >
> > +- phy-tx0-term-offset:
> 
> Needs a vendor prefix.
> 

So I should change to qcom,phy-tx0-term-offset  right?

> > +	Usage: optional
> > +	Value type: <u32>
> > +	Definition: If not defined is 0. In ipq806x is set to 7. In newer
> > +				revision (v2.0) the offset is zero.
> > +
> >  * Example for ipq/apq8064
> >  	pcie@1b500000 {
> >  		compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064",
> "snps,dw-pcie";
> > @@ -293,6 +299,7 @@
> >  		reset-names = "axi", "ahb", "por", "pci", "phy", "ext";
> >  		pinctrl-0 = <&pcie_pins_default>;
> >  		pinctrl-names = "default";
> > +		phy-tx0-term-offset = <7>;
> >  	};
> >
> >  * Example for apq8084
> > --
> > 2.25.1
> >


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver
  2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
                   ` (11 preceding siblings ...)
  2020-03-20 18:47 ` [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Bjorn Helgaas
@ 2020-04-01 13:01 ` Stanimir Varbanov
  12 siblings, 0 replies; 32+ messages in thread
From: Stanimir Varbanov @ 2020-04-01 13:01 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Sham Muthayyan, Andy Gross, Bjorn Andersson, Bjorn Helgaas,
	Rob Herring, Mark Rutland, Lorenzo Pieralisi, Andrew Murray,
	Philipp Zabel, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

Hi Ansuel,

As Bjorn already mentioned please make cover-letter in next version of
the patchset so that we know what is the purpose of the patches.

I've the impression that you want to use pcie-qcom platform driver as an
endpoint, am I wrong?

I'll review the patches these days.

On 3/20/20 8:34 PM, Ansuel Smith wrote:
> Aux and Ref clk are missing in pcie qcom driver.
> Add support in the driver to fix pcie inizialization
> in ipq806x
> 
> Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 38 ++++++++++++++++++++++----
>  1 file changed, 33 insertions(+), 5 deletions(-)


-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 11/12] devicetree: bindings: pci: add force_gen1 for qcom,pcie
  2020-03-20 18:34 ` [PATCH 11/12] devicetree: bindings: pci: add force_gen1 for qcom,pcie Ansuel Smith
  2020-03-31 17:33   ` Rob Herring
@ 2020-04-01 13:17   ` Stanimir Varbanov
  1 sibling, 0 replies; 32+ messages in thread
From: Stanimir Varbanov @ 2020-04-01 13:17 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Andy Gross, Bjorn Andersson, Bjorn Helgaas, Rob Herring,
	Mark Rutland, Lorenzo Pieralisi, Andrew Murray, Philipp Zabel,
	linux-arm-msm, linux-pci, devicetree, linux-kernel

Hi Ansuel,

Before inventing new DT property I'd suggest you to consult with [1].
There is already property max-link-speed for that purpose.

On 3/20/20 8:34 PM, Ansuel Smith wrote:
> Document force_gen1 optional definition to limit pcie
> line to GEN1 speed
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> index 8c1d014f37b0..766876465c42 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> @@ -260,6 +260,11 @@
>  	Definition: If not defined is 0. In ipq806x is set to 7. In newer
>  				revision (v2.0) the offset is zero.
>  
> +- force_gen1:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: Set 1 to force the pcie line to GEN1
> +
>  * Example for ipq/apq8064
>  	pcie@1b500000 {
>  		compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064", "snps,dw-pcie";
> 

-- 
regards,
Stan

[1] Documentation/devicetree/bindings/pci/pci.txt

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 09/12] pcie: qcom: Programming the PCIE iATU for IPQ806x
  2020-03-20 18:34 ` [PATCH 09/12] pcie: qcom: Programming the PCIE iATU for IPQ806x Ansuel Smith
  2020-03-20 19:26   ` Bjorn Helgaas
@ 2020-04-01 13:21   ` Stanimir Varbanov
  1 sibling, 0 replies; 32+ messages in thread
From: Stanimir Varbanov @ 2020-04-01 13:21 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Sham Muthayyan, Andy Gross, Bjorn Andersson, Bjorn Helgaas,
	Rob Herring, Mark Rutland, Lorenzo Pieralisi, Andrew Murray,
	Philipp Zabel, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

Hi Ansuel,

On 3/20/20 8:34 PM, Ansuel Smith wrote:
> From: Sham Muthayyan <smuthayy@codeaurora.org>
> 
> Resolved PCIE EP detection errors caused due to missing iATU programming.

NACK, the iATU programing is not belonging here. Did you check what
pcie-designware-*.c is doing with iATU?

If you want to support endpoint mode in pcie-qcom driver you have to see
how the other drivers is doing that.

> 
> Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 78 ++++++++++++++++++++++++++
>  1 file changed, 78 insertions(+)
> 


-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 07/12] pcie: qcom: add tx term offset support
  2020-03-20 18:34 ` [PATCH 07/12] pcie: qcom: add tx term offset support Ansuel Smith
  2020-03-20 19:22   ` Bjorn Helgaas
@ 2020-04-01 20:40   ` Bjorn Andersson
  2020-04-01 21:55     ` R: " ansuelsmth
  1 sibling, 1 reply; 32+ messages in thread
From: Bjorn Andersson @ 2020-04-01 20:40 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Stanimir Varbanov, Sham Muthayyan, Andy Gross, Bjorn Helgaas,
	Rob Herring, Mark Rutland, Lorenzo Pieralisi, Andrew Murray,
	Philipp Zabel, linux-arm-msm, linux-pci, devicetree,
	linux-kernel

On Fri 20 Mar 11:34 PDT 2020, Ansuel Smith wrote:

> From: Sham Muthayyan <smuthayy@codeaurora.org>
> 
> Add tx term offset support to pcie qcom driver
> need in some revision of the ipq806x soc
> 
> Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++++++----
>  1 file changed, 52 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index ecc22fd27ea6..8009e3117765 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -45,7 +45,13 @@
>  #define PCIE_CAP_CPL_TIMEOUT_DISABLE		0x10
>  
>  #define PCIE20_PARF_PHY_CTRL			0x40
> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	(0x1f << 16)
> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		(x << 16)
> +
>  #define PCIE20_PARF_PHY_REFCLK			0x4C
> +#define REF_SSP_EN				BIT(16)
> +#define REF_USE_PAD				BIT(12)
> +
>  #define PCIE20_PARF_DBI_BASE_ADDR		0x168
>  #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE		0x16C
>  #define PCIE20_PARF_MHI_CLOCK_RESET_CTRL	0x174
> @@ -77,6 +83,18 @@
>  #define DBI_RO_WR_EN				1
>  
>  #define PERST_DELAY_US				1000
> +/* PARF registers */
> +#define PCIE20_PARF_PCS_DEEMPH			0x34
> +#define PCS_DEEMPH_TX_DEEMPH_GEN1(x)		(x << 16)
> +#define PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(x)	(x << 8)
> +#define PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(x)	(x << 0)
> +
> +#define PCIE20_PARF_PCS_SWING			0x38
> +#define PCS_SWING_TX_SWING_FULL(x)		(x << 8)
> +#define PCS_SWING_TX_SWING_LOW(x)		(x << 0)
> +
> +#define PCIE20_PARF_CONFIG_BITS			0x50
> +#define PHY_RX0_EQ(x)				(x << 24)
>  
>  #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
>  #define SLV_ADDR_SPACE_SZ			0x10000000
> @@ -97,6 +115,7 @@ struct qcom_pcie_resources_2_1_0 {
>  	struct reset_control *phy_reset;
>  	struct reset_control *ext_reset;
>  	struct regulator_bulk_data supplies[QCOM_PCIE_2_1_0_MAX_SUPPLY];
> +	uint8_t phy_tx0_term_offset;
>  };
>  
>  struct qcom_pcie_resources_1_0_0 {
> @@ -184,6 +203,16 @@ struct qcom_pcie {
>  
>  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
>  
> +static inline void
> +writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)
> +{
> +	u32 val = readl(addr);
> +
> +	val &= ~clear_mask;
> +	val |= set_mask;
> +	writel(val, addr);
> +}
> +
>  static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
>  {
>  	gpiod_set_value_cansleep(pcie->reset, 1);
> @@ -277,6 +306,10 @@ static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
>  	if (IS_ERR(res->ext_reset))
>  		return PTR_ERR(res->ext_reset);
>  
> +	if (of_property_read_u8(dev->of_node, "phy-tx0-term-offset",
> +				&res->phy_tx0_term_offset))
> +		res->phy_tx0_term_offset = 0;

The appropriate way is to encode differences in hardware is to use
different compatibles for the two different versions of the hardware.

Regards,
Bjorn

> +
>  	res->phy_reset = devm_reset_control_get_exclusive(dev, "phy");
>  	return PTR_ERR_OR_ZERO(res->phy_reset);
>  }
> @@ -304,7 +337,6 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
>  	struct qcom_pcie_resources_2_1_0 *res = &pcie->res.v2_1_0;
>  	struct dw_pcie *pci = pcie->pci;
>  	struct device *dev = pci->dev;
> -	u32 val;
>  	int ret;
>  
>  	ret = reset_control_assert(res->ahb_reset);
> @@ -355,15 +387,26 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
>  		goto err_deassert_ahb;
>  	}
>  
> -	/* enable PCIe clocks and resets */
> -	val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
> -	val &= ~BIT(0);
> -	writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> +	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
> +
> +	/* Set Tx termination offset */
> +	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL,
> +		      PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK,
> +		      PHY_CTRL_PHY_TX0_TERM_OFFSET(res->phy_tx0_term_offset));
> +
> +	/* PARF programming */
> +	writel(PCS_DEEMPH_TX_DEEMPH_GEN1(0x18) |
> +	       PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(0x18) |
> +	       PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(0x22),
> +	       pcie->parf + PCIE20_PARF_PCS_DEEMPH);
> +	writel(PCS_SWING_TX_SWING_FULL(0x78) |
> +	       PCS_SWING_TX_SWING_LOW(0x78),
> +	       pcie->parf + PCIE20_PARF_PCS_SWING);
> +	writel(PHY_RX0_EQ(0x4), pcie->parf + PCIE20_PARF_CONFIG_BITS);
>  
> -	/* enable external reference clock */
> -	val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> -	val |= BIT(16);
> -	writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
> +	/* Enable reference clock */
> +	writel_masked(pcie->parf + PCIE20_PARF_PHY_REFCLK,
> +		      REF_USE_PAD, REF_SSP_EN);
>  
>  	ret = reset_control_deassert(res->phy_reset);
>  	if (ret) {
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 08/12] devicetree: bindings: pci: add phy-tx0-term-offset to qcom,pcie
  2020-03-20 18:34 ` [PATCH 08/12] devicetree: bindings: pci: add phy-tx0-term-offset to qcom,pcie Ansuel Smith
  2020-03-31 17:32   ` Rob Herring
@ 2020-04-01 20:41   ` Bjorn Andersson
  1 sibling, 0 replies; 32+ messages in thread
From: Bjorn Andersson @ 2020-04-01 20:41 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Stanimir Varbanov, Andy Gross, Bjorn Helgaas, Rob Herring,
	Mark Rutland, Lorenzo Pieralisi, Andrew Murray, Philipp Zabel,
	linux-arm-msm, linux-pci, devicetree, linux-kernel

On Fri 20 Mar 11:34 PDT 2020, Ansuel Smith wrote:

> Document phy-tx0-term-offset propriety to qcom pcie driver
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> index 6efcef040741..8c1d014f37b0 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> @@ -254,6 +254,12 @@
>  			- "perst-gpios"	PCIe endpoint reset signal line
>  			- "wake-gpios"	PCIe endpoint wake signal line
>  
> +- phy-tx0-term-offset:

If I understand your implementation correctly this difference in
hardware revision should be encoded in the compatible string.

Regards,
Bjorn

> +	Usage: optional
> +	Value type: <u32>
> +	Definition: If not defined is 0. In ipq806x is set to 7. In newer
> +				revision (v2.0) the offset is zero.
> +
>  * Example for ipq/apq8064
>  	pcie@1b500000 {
>  		compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064", "snps,dw-pcie";
> @@ -293,6 +299,7 @@
>  		reset-names = "axi", "ahb", "por", "pci", "phy", "ext";
>  		pinctrl-0 = <&pcie_pins_default>;
>  		pinctrl-names = "default";
> +		phy-tx0-term-offset = <7>;
>  	};
>  
>  * Example for apq8084
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* R: [PATCH 07/12] pcie: qcom: add tx term offset support
  2020-04-01 20:40   ` Bjorn Andersson
@ 2020-04-01 21:55     ` ansuelsmth
  2020-04-01 23:52       ` Bjorn Andersson
  0 siblings, 1 reply; 32+ messages in thread
From: ansuelsmth @ 2020-04-01 21:55 UTC (permalink / raw)
  To: 'Bjorn Andersson'
  Cc: 'Stanimir Varbanov', 'Sham Muthayyan',
	'Andy Gross', 'Bjorn Helgaas',
	'Rob Herring', 'Mark Rutland',
	'Lorenzo Pieralisi', 'Andrew Murray',
	'Philipp Zabel',
	linux-arm-msm, linux-pci, devicetree, linux-kernel

> On Fri 20 Mar 11:34 PDT 2020, Ansuel Smith wrote:
> 
> > From: Sham Muthayyan <smuthayy@codeaurora.org>
> >
> > Add tx term offset support to pcie qcom driver
> > need in some revision of the ipq806x soc
> >
> > Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 61
> ++++++++++++++++++++++----
> >  1 file changed, 52 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> > index ecc22fd27ea6..8009e3117765 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -45,7 +45,13 @@
> >  #define PCIE_CAP_CPL_TIMEOUT_DISABLE		0x10
> >
> >  #define PCIE20_PARF_PHY_CTRL			0x40
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	(0x1f << 16)
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		(x << 16)
> > +
> >  #define PCIE20_PARF_PHY_REFCLK			0x4C
> > +#define REF_SSP_EN				BIT(16)
> > +#define REF_USE_PAD				BIT(12)
> > +
> >  #define PCIE20_PARF_DBI_BASE_ADDR		0x168
> >  #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE		0x16C
> >  #define PCIE20_PARF_MHI_CLOCK_RESET_CTRL	0x174
> > @@ -77,6 +83,18 @@
> >  #define DBI_RO_WR_EN				1
> >
> >  #define PERST_DELAY_US				1000
> > +/* PARF registers */
> > +#define PCIE20_PARF_PCS_DEEMPH			0x34
> > +#define PCS_DEEMPH_TX_DEEMPH_GEN1(x)		(x << 16)
> > +#define PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(x)	(x << 8)
> > +#define PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(x)	(x << 0)
> > +
> > +#define PCIE20_PARF_PCS_SWING			0x38
> > +#define PCS_SWING_TX_SWING_FULL(x)		(x << 8)
> > +#define PCS_SWING_TX_SWING_LOW(x)		(x << 0)
> > +
> > +#define PCIE20_PARF_CONFIG_BITS			0x50
> > +#define PHY_RX0_EQ(x)				(x << 24)
> >
> >  #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
> >  #define SLV_ADDR_SPACE_SZ			0x10000000
> > @@ -97,6 +115,7 @@ struct qcom_pcie_resources_2_1_0 {
> >  	struct reset_control *phy_reset;
> >  	struct reset_control *ext_reset;
> >  	struct regulator_bulk_data
> supplies[QCOM_PCIE_2_1_0_MAX_SUPPLY];
> > +	uint8_t phy_tx0_term_offset;
> >  };
> >
> >  struct qcom_pcie_resources_1_0_0 {
> > @@ -184,6 +203,16 @@ struct qcom_pcie {
> >
> >  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
> >
> > +static inline void
> > +writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)
> > +{
> > +	u32 val = readl(addr);
> > +
> > +	val &= ~clear_mask;
> > +	val |= set_mask;
> > +	writel(val, addr);
> > +}
> > +
> >  static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
> >  {
> >  	gpiod_set_value_cansleep(pcie->reset, 1);
> > @@ -277,6 +306,10 @@ static int
> qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
> >  	if (IS_ERR(res->ext_reset))
> >  		return PTR_ERR(res->ext_reset);
> >
> > +	if (of_property_read_u8(dev->of_node, "phy-tx0-term-offset",
> > +				&res->phy_tx0_term_offset))
> > +		res->phy_tx0_term_offset = 0;
> 
> The appropriate way is to encode differences in hardware is to use
> different compatibles for the two different versions of the hardware.
> 
> Regards,
> Bjorn
> 

So a better way to handle this would be to check the SoC compatible?
AFAIK a different offset is only needed on ipq8064 revision 2 and ipq8065
but
it looks bad to add a special code just for that 2 SoC. 
I would prefer to handle this with the offset definition but If you think
this would be
the right way, I will follow that. Waiting for your response about this.

> > +
> >  	res->phy_reset = devm_reset_control_get_exclusive(dev, "phy");
> >  	return PTR_ERR_OR_ZERO(res->phy_reset);
> >  }
> > @@ -304,7 +337,6 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> >  	struct qcom_pcie_resources_2_1_0 *res = &pcie->res.v2_1_0;
> >  	struct dw_pcie *pci = pcie->pci;
> >  	struct device *dev = pci->dev;
> > -	u32 val;
> >  	int ret;
> >
> >  	ret = reset_control_assert(res->ahb_reset);
> > @@ -355,15 +387,26 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> >  		goto err_deassert_ahb;
> >  	}
> >
> > -	/* enable PCIe clocks and resets */
> > -	val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
> > -	val &= ~BIT(0);
> > -	writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> > +	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
> > +
> > +	/* Set Tx termination offset */
> > +	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL,
> > +		      PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK,
> > +		      PHY_CTRL_PHY_TX0_TERM_OFFSET(res-
> >phy_tx0_term_offset));
> > +
> > +	/* PARF programming */
> > +	writel(PCS_DEEMPH_TX_DEEMPH_GEN1(0x18) |
> > +	       PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(0x18) |
> > +	       PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(0x22),
> > +	       pcie->parf + PCIE20_PARF_PCS_DEEMPH);
> > +	writel(PCS_SWING_TX_SWING_FULL(0x78) |
> > +	       PCS_SWING_TX_SWING_LOW(0x78),
> > +	       pcie->parf + PCIE20_PARF_PCS_SWING);
> > +	writel(PHY_RX0_EQ(0x4), pcie->parf + PCIE20_PARF_CONFIG_BITS);
> >
> > -	/* enable external reference clock */
> > -	val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> > -	val |= BIT(16);
> > -	writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
> > +	/* Enable reference clock */
> > +	writel_masked(pcie->parf + PCIE20_PARF_PHY_REFCLK,
> > +		      REF_USE_PAD, REF_SSP_EN);
> >
> >  	ret = reset_control_deassert(res->phy_reset);
> >  	if (ret) {
> > --
> > 2.25.1
> >


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: R: [PATCH 07/12] pcie: qcom: add tx term offset support
  2020-04-01 21:55     ` R: " ansuelsmth
@ 2020-04-01 23:52       ` Bjorn Andersson
  0 siblings, 0 replies; 32+ messages in thread
From: Bjorn Andersson @ 2020-04-01 23:52 UTC (permalink / raw)
  To: ansuelsmth
  Cc: 'Stanimir Varbanov', 'Sham Muthayyan',
	'Andy Gross', 'Bjorn Helgaas',
	'Rob Herring', 'Mark Rutland',
	'Lorenzo Pieralisi', 'Andrew Murray',
	'Philipp Zabel',
	linux-arm-msm, linux-pci, devicetree, linux-kernel

On Wed 01 Apr 14:55 PDT 2020, ansuelsmth@gmail.com wrote:

> > On Fri 20 Mar 11:34 PDT 2020, Ansuel Smith wrote:
> > 
> > > From: Sham Muthayyan <smuthayy@codeaurora.org>
> > >
> > > Add tx term offset support to pcie qcom driver
> > > need in some revision of the ipq806x soc
> > >
> > > Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-qcom.c | 61
> > ++++++++++++++++++++++----
> > >  1 file changed, 52 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> > b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index ecc22fd27ea6..8009e3117765 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -45,7 +45,13 @@
> > >  #define PCIE_CAP_CPL_TIMEOUT_DISABLE		0x10
> > >
> > >  #define PCIE20_PARF_PHY_CTRL			0x40
> > > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	(0x1f << 16)
> > > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		(x << 16)
> > > +
> > >  #define PCIE20_PARF_PHY_REFCLK			0x4C
> > > +#define REF_SSP_EN				BIT(16)
> > > +#define REF_USE_PAD				BIT(12)
> > > +
> > >  #define PCIE20_PARF_DBI_BASE_ADDR		0x168
> > >  #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE		0x16C
> > >  #define PCIE20_PARF_MHI_CLOCK_RESET_CTRL	0x174
> > > @@ -77,6 +83,18 @@
> > >  #define DBI_RO_WR_EN				1
> > >
> > >  #define PERST_DELAY_US				1000
> > > +/* PARF registers */
> > > +#define PCIE20_PARF_PCS_DEEMPH			0x34
> > > +#define PCS_DEEMPH_TX_DEEMPH_GEN1(x)		(x << 16)
> > > +#define PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(x)	(x << 8)
> > > +#define PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(x)	(x << 0)
> > > +
> > > +#define PCIE20_PARF_PCS_SWING			0x38
> > > +#define PCS_SWING_TX_SWING_FULL(x)		(x << 8)
> > > +#define PCS_SWING_TX_SWING_LOW(x)		(x << 0)
> > > +
> > > +#define PCIE20_PARF_CONFIG_BITS			0x50
> > > +#define PHY_RX0_EQ(x)				(x << 24)
> > >
> > >  #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
> > >  #define SLV_ADDR_SPACE_SZ			0x10000000
> > > @@ -97,6 +115,7 @@ struct qcom_pcie_resources_2_1_0 {
> > >  	struct reset_control *phy_reset;
> > >  	struct reset_control *ext_reset;
> > >  	struct regulator_bulk_data
> > supplies[QCOM_PCIE_2_1_0_MAX_SUPPLY];
> > > +	uint8_t phy_tx0_term_offset;
> > >  };
> > >
> > >  struct qcom_pcie_resources_1_0_0 {
> > > @@ -184,6 +203,16 @@ struct qcom_pcie {
> > >
> > >  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
> > >
> > > +static inline void
> > > +writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)
> > > +{
> > > +	u32 val = readl(addr);
> > > +
> > > +	val &= ~clear_mask;
> > > +	val |= set_mask;
> > > +	writel(val, addr);
> > > +}
> > > +
> > >  static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
> > >  {
> > >  	gpiod_set_value_cansleep(pcie->reset, 1);
> > > @@ -277,6 +306,10 @@ static int
> > qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
> > >  	if (IS_ERR(res->ext_reset))
> > >  		return PTR_ERR(res->ext_reset);
> > >
> > > +	if (of_property_read_u8(dev->of_node, "phy-tx0-term-offset",
> > > +				&res->phy_tx0_term_offset))
> > > +		res->phy_tx0_term_offset = 0;
> > 
> > The appropriate way is to encode differences in hardware is to use
> > different compatibles for the two different versions of the hardware.
> > 
> > Regards,
> > Bjorn
> > 
> 
> So a better way to handle this would be to check the SoC compatible?
> AFAIK a different offset is only needed on ipq8064 revision 2 and ipq8065
> but
> it looks bad to add a special code just for that 2 SoC. 
> I would prefer to handle this with the offset definition but If you think
> this would be
> the right way, I will follow that. Waiting for your response about this.
> 

Yes, please do this by having different compatibles for the different
revisions of the hardware block.

You should be able to use the same implementation for the two
compatibles, just make the phy_tx0_term_offset depends on which was
used.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2020-04-01 23:52 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-20 18:34 [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Ansuel Smith
2020-03-20 18:34 ` [PATCH 02/12] devicetree: bindings: pci: add missing clks to qcom,pcie Ansuel Smith
2020-03-31 17:30   ` Rob Herring
2020-03-20 18:34 ` [PATCH 03/12] pcie: qcom: change duplicate pci reset to phy reset Ansuel Smith
2020-03-20 18:34 ` [PATCH 04/12] pcie: qcom: Fixed pcie_phy_clk branch issue Ansuel Smith
2020-03-20 18:34 ` [PATCH 05/12] pcie: qcom: add missing reset for ipq806x Ansuel Smith
2020-03-20 18:51   ` Bjorn Helgaas
2020-03-23  6:06   ` Philipp Zabel
2020-03-20 18:34 ` [PATCH 06/12] devicetree: bindings: pci: add ext reset to qcom,pcie Ansuel Smith
2020-03-31 17:31   ` Rob Herring
2020-03-20 18:34 ` [PATCH 07/12] pcie: qcom: add tx term offset support Ansuel Smith
2020-03-20 19:22   ` Bjorn Helgaas
2020-04-01 20:40   ` Bjorn Andersson
2020-04-01 21:55     ` R: " ansuelsmth
2020-04-01 23:52       ` Bjorn Andersson
2020-03-20 18:34 ` [PATCH 08/12] devicetree: bindings: pci: add phy-tx0-term-offset to qcom,pcie Ansuel Smith
2020-03-31 17:32   ` Rob Herring
2020-04-01 12:09     ` R: " ansuelsmth
2020-04-01 20:41   ` Bjorn Andersson
2020-03-20 18:34 ` [PATCH 09/12] pcie: qcom: Programming the PCIE iATU for IPQ806x Ansuel Smith
2020-03-20 19:26   ` Bjorn Helgaas
2020-04-01 13:21   ` Stanimir Varbanov
2020-03-20 18:34 ` [PATCH 10/12] pcie: qcom: add Force GEN1 support Ansuel Smith
2020-03-20 19:37   ` Bjorn Helgaas
2020-03-20 18:34 ` [PATCH 11/12] devicetree: bindings: pci: add force_gen1 for qcom,pcie Ansuel Smith
2020-03-31 17:33   ` Rob Herring
2020-04-01 12:09     ` R: " ansuelsmth
2020-04-01 13:17   ` Stanimir Varbanov
2020-03-20 18:34 ` [PATCH 12/12] pcie: qcom: Set PCIE MRRS and MPS to 256B Ansuel Smith
2020-03-20 19:46   ` Bjorn Helgaas
2020-03-20 18:47 ` [PATCH 01/12] pcie: qcom: add missing ipq806x clocks in pcie driver Bjorn Helgaas
2020-04-01 13:01 ` Stanimir Varbanov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).