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(-)
--
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(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 5ea527a6bd9f..2a39dfdccfc8 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_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);
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);
--
2.25.1
Document missing clks used in ipq8064 soc.
Signed-off-by: Ansuel Smith <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
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
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(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 2a39dfdccfc8..7a8901efc031 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -280,14 +280,14 @@ 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);
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);
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;
- }
-
if (res->aux_clk) {
ret = clk_prepare_enable(res->aux_clk);
if (ret) {
@@ -387,6 +381,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);
@@ -404,8 +404,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
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(+)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 7a8901efc031..921030a64bab 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_optional_exclusive(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);
@@ -347,6 +353,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
Document ext reset used in ipq8064 SoC by qcom PCIe driver.
Signed-off-by: Ansuel Smith <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
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
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(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 921030a64bab..a4fd5baada34 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -184,6 +184,16 @@ struct qcom_pcie {
#define to_qcom_pcie(x) dev_get_drvdata((x)->dev)
+static void qcom_clear_and_set_dword(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);
@@ -214,12 +224,9 @@ static int qcom_pcie_establish_link(struct qcom_pcie *pcie)
static void qcom_pcie_2_1_0_ltssm_enable(struct qcom_pcie *pcie)
{
- u32 val;
-
/* enable link training */
- val = readl(pcie->elbi + PCIE20_ELBI_SYS_CTRL);
- val |= PCIE20_ELBI_SYS_CTRL_LT_ENABLE;
- writel(val, pcie->elbi + PCIE20_ELBI_SYS_CTRL);
+ qcom_clear_and_set_dword(pcie->elbi + PCIE20_ELBI_SYS_CTRL, 0,
+ PCIE20_ELBI_SYS_CTRL_LT_ENABLE);
}
static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
@@ -304,7 +311,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 = regulator_bulk_enable(ARRAY_SIZE(res->supplies), res->supplies);
@@ -360,14 +366,11 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
}
/* enable PCIe clocks and resets */
- val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
- val &= ~BIT(0);
- writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
/* enable external reference clock */
- val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
- val |= BIT(16);
- writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_REFCLK, 0,
+ BIT(16));
ret = reset_control_deassert(res->phy_reset);
if (ret) {
@@ -514,10 +517,9 @@ static int qcom_pcie_init_1_0_0(struct qcom_pcie *pcie)
writel(0, pcie->parf + PCIE20_PARF_DBI_BASE_ADDR);
if (IS_ENABLED(CONFIG_PCI_MSI)) {
- u32 val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
-
- val |= BIT(31);
- writel(val, pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
+ qcom_clear_and_set_dword(
+ pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT, 0,
+ BIT(31));
}
return 0;
@@ -537,12 +539,8 @@ static int qcom_pcie_init_1_0_0(struct qcom_pcie *pcie)
static void qcom_pcie_2_3_2_ltssm_enable(struct qcom_pcie *pcie)
{
- u32 val;
-
/* enable link training */
- val = readl(pcie->parf + PCIE20_PARF_LTSSM);
- val |= BIT(8);
- writel(val, pcie->parf + PCIE20_PARF_LTSSM);
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_LTSSM, 0, BIT(8));
}
static int qcom_pcie_get_resources_2_3_2(struct qcom_pcie *pcie)
@@ -603,7 +601,6 @@ static int qcom_pcie_init_2_3_2(struct qcom_pcie *pcie)
struct qcom_pcie_resources_2_3_2 *res = &pcie->res.v2_3_2;
struct dw_pcie *pci = pcie->pci;
struct device *dev = pci->dev;
- u32 val;
int ret;
ret = regulator_bulk_enable(ARRAY_SIZE(res->supplies), res->supplies);
@@ -637,25 +634,19 @@ static int qcom_pcie_init_2_3_2(struct qcom_pcie *pcie)
}
/* enable PCIe clocks and resets */
- val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
- val &= ~BIT(0);
- writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
/* change DBI base address */
writel(0, pcie->parf + PCIE20_PARF_DBI_BASE_ADDR);
/* MAC PHY_POWERDOWN MUX DISABLE */
- val = readl(pcie->parf + PCIE20_PARF_SYS_CTRL);
- val &= ~BIT(29);
- writel(val, pcie->parf + PCIE20_PARF_SYS_CTRL);
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_SYS_CTRL, BIT(29), 0);
- val = readl(pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
- val |= BIT(4);
- writel(val, pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL,
+ 0, BIT(4));
- val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2);
- val |= BIT(31);
- writel(val, pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2);
+ qcom_clear_and_set_dword(
+ pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2, 0, BIT(31));
return 0;
@@ -792,7 +783,6 @@ static int qcom_pcie_init_2_4_0(struct qcom_pcie *pcie)
struct qcom_pcie_resources_2_4_0 *res = &pcie->res.v2_4_0;
struct dw_pcie *pci = pcie->pci;
struct device *dev = pci->dev;
- u32 val;
int ret;
ret = reset_control_assert(res->axi_m_reset);
@@ -918,25 +908,19 @@ static int qcom_pcie_init_2_4_0(struct qcom_pcie *pcie)
goto err_clks;
/* enable PCIe clocks and resets */
- val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
- val &= ~BIT(0);
- writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
/* change DBI base address */
writel(0, pcie->parf + PCIE20_PARF_DBI_BASE_ADDR);
/* MAC PHY_POWERDOWN MUX DISABLE */
- val = readl(pcie->parf + PCIE20_PARF_SYS_CTRL);
- val &= ~BIT(29);
- writel(val, pcie->parf + PCIE20_PARF_SYS_CTRL);
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_SYS_CTRL, BIT(29), 0);
- val = readl(pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
- val |= BIT(4);
- writel(val, pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL,
+ 0, BIT(4));
- val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2);
- val |= BIT(31);
- writel(val, pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2);
+ qcom_clear_and_set_dword(
+ pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2, 0, BIT(31));
return 0;
@@ -1017,7 +1001,6 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie)
struct dw_pcie *pci = pcie->pci;
struct device *dev = pci->dev;
int i, ret;
- u32 val;
for (i = 0; i < ARRAY_SIZE(res->rst); i++) {
ret = reset_control_assert(res->rst[i]);
@@ -1077,9 +1060,7 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie)
writel(SLV_ADDR_SPACE_SZ,
pcie->parf + PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE);
- val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
- val &= ~BIT(0);
- writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
writel(0, pcie->parf + PCIE20_PARF_DBI_BASE_ADDR);
@@ -1093,9 +1074,8 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie)
writel(DBI_RO_WR_EN, pci->dbi_base + PCIE20_MISC_CONTROL_1_REG);
writel(PCIE_CAP_LINK1_VAL, pci->dbi_base + PCIE20_CAP_LINK_1);
- val = readl(pci->dbi_base + PCIE20_CAP_LINK_CAPABILITIES);
- val &= ~PCIE20_CAP_ACTIVE_STATE_LINK_PM_SUPPORT;
- writel(val, pci->dbi_base + PCIE20_CAP_LINK_CAPABILITIES);
+ qcom_clear_and_set_dword(pcie->dbi_base + PCIE20_CAP_LINK_CAPABILITIES,
+ PCIE20_CAP_ACTIVE_STATE_LINK_PM_SUPPORT, 0);
writel(PCIE_CAP_CPL_TIMEOUT_DISABLE, pci->dbi_base +
PCIE20_DEVICE_CONTROL2_STATUS2);
@@ -1159,7 +1139,6 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
struct dw_pcie *pci = pcie->pci;
struct device *dev = pci->dev;
- u32 val;
int ret;
ret = regulator_bulk_enable(ARRAY_SIZE(res->supplies), res->supplies);
@@ -1196,26 +1175,21 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
writel(DEVICE_TYPE_RC, pcie->parf + PCIE20_PARF_DEVICE_TYPE);
/* enable PCIe clocks and resets */
- val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
- val &= ~BIT(0);
- writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
/* change DBI base address */
writel(0, pcie->parf + PCIE20_PARF_DBI_BASE_ADDR);
/* MAC PHY_POWERDOWN MUX DISABLE */
- val = readl(pcie->parf + PCIE20_PARF_SYS_CTRL);
- val &= ~BIT(29);
- writel(val, pcie->parf + PCIE20_PARF_SYS_CTRL);
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_SYS_CTRL, BIT(29), 0);
- val = readl(pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
- val |= BIT(4);
- writel(val, pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL,
+ 0, BIT(4));
if (IS_ENABLED(CONFIG_PCI_MSI)) {
- val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
- val |= BIT(31);
- writel(val, pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
+ qcom_clear_and_set_dword(
+ pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL, 0,
+ BIT(31));
}
return 0;
--
2.25.1
Add support for Tx De-Emphasis, Tx Swing and Rx equalization definition
needed on some ipq8064 based device (Netgear R7800 for example). This
cause a total lock of the system on kernel load.
Fixes: 82a823833f4e PCI: qcom: Add Qualcomm PCIe controller driver
Signed-off-by: Ansuel Smith <[email protected]>
Cc: [email protected] # v4.5+
---
drivers/pci/controller/dwc/pcie-qcom.c | 47 ++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index a4fd5baada34..da8058fd1925 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -46,6 +46,9 @@
#define PCIE20_PARF_PHY_CTRL 0x40
#define PCIE20_PARF_PHY_REFCLK 0x4C
+#define PHY_REFCLK_SSP_EN BIT(16)
+#define PHY_REFCLK_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 +80,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 +112,12 @@ 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];
+ u32 tx_deemph_gen1;
+ u32 tx_deemph_gen2_3p5db;
+ u32 tx_deemph_gen2_6db;
+ u32 tx_swing_full;
+ u32 tx_swing_low;
+ u32 rx0_eq;
};
struct qcom_pcie_resources_1_0_0 {
@@ -234,8 +255,21 @@ 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;
struct dw_pcie *pci = pcie->pci;
struct device *dev = pci->dev;
+ struct device_node *node = dev->of_node;
int ret;
+ of_property_read_u32(node, "qcom,tx-deemph-gen1",
+ &res->tx_deemph_gen1);
+ of_property_read_u32(node, "qcom,tx-deemph-gen2-3p5db",
+ &res->tx_deemph_gen2_3p5db);
+ of_property_read_u32(node, "qcom,tx-deemph-gen2-6db",
+ &res->tx_deemph_gen2_6db);
+ of_property_read_u32(node, "qcom,tx-swing-full",
+ &res->tx_swing_full);
+ of_property_read_u32(node, "qcom,tx-swing-low",
+ &res->tx_swing_low);
+ of_property_read_u32(node, "qcom,rx0-eq", &res->rx0_eq);
+
res->supplies[0].supply = "vdda";
res->supplies[1].supply = "vdda_phy";
res->supplies[2].supply = "vdda_refclk";
@@ -368,9 +402,18 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
/* enable PCIe clocks and resets */
qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
+ 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),
+ pcie->parf + PCIE20_PARF_PCS_DEEMPH);
+ writel(PCS_SWING_TX_SWING_FULL(res->tx_swing_full) |
+ PCS_SWING_TX_SWING_LOW(res->tx_swing_low),
+ pcie->parf + PCIE20_PARF_PCS_SWING);
+ writel(PHY_RX0_EQ(res->rx0_eq), pcie->parf + PCIE20_PARF_CONFIG_BITS);
+
/* enable external reference clock */
- qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_REFCLK, 0,
- BIT(16));
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_REFCLK,
+ PHY_REFCLK_USE_PAD, PHY_REFCLK_SSP_EN;
ret = reset_control_deassert(res->phy_reset);
if (ret) {
--
2.25.1
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(+)
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 8cc5aea8a1da..c7102863d855 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -5,6 +5,7 @@
Value type: <stringlist>
Definition: Value should contain
- "qcom,pcie-ipq8064" for ipq8064
+ - "qcom,pcie-ipq8064-v2" for ipq8064 rev 2 or ipq8065
- "qcom,pcie-apq8064" for apq8064
- "qcom,pcie-apq8084" for apq8084
- "qcom,pcie-msm8996" for msm8996 or apq8096
--
2.25.1
From: Sham Muthayyan <[email protected]>
Add Force GEN1 support needed in some ipq8064 board that needs to limit
some PCIe line to gen1 for some hardware limitation.
This is set by the max-link-speed binding and needed by some soc based
on ipq8064. (for example Netgear R7800 router)
Signed-off-by: Sham Muthayyan <[email protected]>
Signed-off-by: Ansuel Smith <[email protected]>
---
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 372d2c8508b5..a568f28645e7 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -27,6 +27,7 @@
#include <linux/slab.h>
#include <linux/types.h>
+#include "../../pci.h"
#include "pcie-designware.h"
#define PCIE20_PARF_SYS_CTRL 0x00
@@ -99,6 +100,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
@@ -205,6 +208,7 @@ struct qcom_pcie {
struct phy *phy;
struct gpio_desc *reset;
const struct qcom_pcie_ops *ops;
+ int gen;
};
#define to_qcom_pcie(x) dev_get_drvdata((x)->dev)
@@ -462,6 +466,11 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
/* wait for clock acquisition */
usleep_range(1000, 1500);
+ if (pcie->gen == 1) {
+ qcom_clear_and_set_dword(pci->dbi_base +
+ PCIE20_LNK_CONTROL2_LINK_STATUS2, 0, 1);
+ }
+
/* Set the Max TLP size to 2K, instead of using default of 4K */
writel(CFG_REMOTE_RD_REQ_BRIDGE_SIZE_2K,
@@ -1431,6 +1440,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
goto err_pm_runtime_put;
}
+ pcie->gen = of_pci_get_max_link_speed(pdev->dev.of_node);
+ if (pcie->gen < 0)
+ pcie->gen = 2;
+
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "parf");
pcie->parf = devm_ioremap_resource(dev, res);
if (IS_ERR(pcie->parf)) {
--
2.25.1
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(+)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index da8058fd1925..372d2c8508b5 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -45,6 +45,9 @@
#define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
#define PCIE20_PARF_PHY_CTRL 0x40
+#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(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 {
@@ -318,6 +322,11 @@ 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_device_is_compatible(dev->of_node, "qcom,pcie-ipq8064"))
+ res->phy_tx0_term_offset = 7;
+ 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)
/* enable PCIe clocks and resets */
qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
+ /* set TX termination offset */
+ qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL,
+ PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK,
+ 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 },
--
2.25.1
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(+)
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 6efcef040741..8cc5aea8a1da 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -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.
+
+- 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";
--
2.25.1
On Fri, May 01, 2020 at 12:06:07AM +0200, Ansuel Smith wrote:
> 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
s/in PCIe driver// (obvious from context)
> 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
s/to qcom,pcie// (obvious from context after updating as below)
> 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
s/to qcom,pcie// (obvious from context after updating as below)
> Sham Muthayyan (2):
> PCI: qcom: add ipq8064 rev2 variant and set tx term offset
> PCI: qcom: add Force GEN1 support
Hi Ansuel, if you post this again, would you mind adjusting your
subject lines to match the history, e.g.,
$ git log --oneline drivers/pci/controller/dwc/pcie-qcom.c
604f3956524a PCI: qcom: Fix the fixup of PCI_VENDOR_ID_QCOM
ed8cc3b1fc84 PCI: qcom: Add support for SDM845 PCIe controller
64adde31c8e9 PCI: qcom: Ensure that PERST is asserted for at least 100 ms
...
$ git log --oneline Documentation/devicetree/bindings/pci/qcom,pcie.txt
5d28bee7c91e dt-bindings: PCI: qcom: Add support for SDM845 PCIe
29a50257a9d6 dt-bindings: PCI: qcom: Add QCS404 to the binding
(Capitalize first word, follow "dt-bindings: PCI: qcom" example).
Some of the commit logs also have random-length short lines. Please
wrap them to use the entire ~75 column width and add blank lines
between paragraphs.
Add ("..") on the Fixes: lines. See git log for common practice. I
use this alias to make them:
$ type gsr
gsr is aliased to `git --no-pager show -s --abbrev-commit --abbrev=12 --pretty=format:"%h (\"%s\")%n"'
$ gsr 82a823833f4e
82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
Some of the logs use "SoC", some use "soc". I prefer "SoC" because
"soc" really isn't an English word.
You don't need to post a new version just for these tweaks, but maybe
make them in your local copy so if you do post a v4 for some other
reason, they'll be included.
> .../devicetree/bindings/pci/qcom,pcie.txt | 51 +++-
> drivers/pci/controller/dwc/pcie-qcom.c | 241 ++++++++++++------
> 2 files changed, 211 insertions(+), 81 deletions(-)
>
> --
> 2.25.1
>
On Fri, May 01, 2020 at 12:06:08AM +0200, Ansuel Smith wrote:
> 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+
Doesn't strike me as stable material. Looks like new h/w enablement.
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 44 ++++++++++++++++++++++----
> 1 file changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 5ea527a6bd9f..2a39dfdccfc8 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_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);
Seems like you'd want to report an error for ipq608x? Based on the
commit msg, they aren't optional.
> +
> 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);
Perhaps use the bulk api.
> + 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);
> --
> 2.25.1
>
On Fri, 1 May 2020 00:06:10 +0200, Ansuel Smith wrote:
> 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(-)
>
Reviewed-by: Rob Herring <[email protected]>
On Fri, 1 May 2020 00:06:11 +0200, Ansuel Smith wrote:
> 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(+)
>
Reviewed-by: Rob Herring <[email protected]>
On Fri, May 01, 2020 at 12:06:13AM +0200, Ansuel Smith wrote:
> 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(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 921030a64bab..a4fd5baada34 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -184,6 +184,16 @@ struct qcom_pcie {
>
> #define to_qcom_pcie(x) dev_get_drvdata((x)->dev)
>
> +static void qcom_clear_and_set_dword(void __iomem *addr, u32 clear_mask,
> + u32 set_mask)
> +{
> + u32 val = readl(addr);
> +
> + val &= ~clear_mask;
> + val |= set_mask;
> + writel(val, addr);
> +}
If we wanted this kind of register accessor in the kernel, then we'd
have common ones. We don't because it hides the possible need for
locking on a RMW sequence.
Also, not a fix. Don't mix refactoring with fixes.
Rob
On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote:
> 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(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> index 6efcef040741..8cc5aea8a1da 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> @@ -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.
> +
> +- 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";
> --
> 2.25.1
>
On Fri, May 01, 2020 at 12:06:16AM +0200, Ansuel Smith wrote:
> 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(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index da8058fd1925..372d2c8508b5 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -45,6 +45,9 @@
> #define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
>
> #define PCIE20_PARF_PHY_CTRL 0x40
> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(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 {
> @@ -318,6 +322,11 @@ 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_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.
> + else
> + res->phy_tx0_term_offset = 0;
> +
On Fri, 1 May 2020 00:06:17 +0200, Ansuel Smith wrote:
> 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(+)
>
Acked-by: Rob Herring <[email protected]>
> On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote:
> > 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(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > index 6efcef040741..8cc5aea8a1da 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > @@ -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.
>
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?
Anyway thanks for the review.
> > +
> > +- 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";
> > --
> > 2.25.1
> >
Hi Ansuel,
On Fri, May 01, 2020 at 12:06:11AM +0200, Ansuel Smith wrote:
> 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(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 7a8901efc031..921030a64bab 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
[...]
> @@ -347,6 +353,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");
^
This probably should say "cannot deassert ext reset". Apart from this,
Reviewed-by: Philipp Zabel <[email protected]>
regards
Philipp
Hi Ansuel,
On 5/1/20 1:06 AM, Ansuel Smith wrote:
> 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(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 5ea527a6bd9f..2a39dfdccfc8 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_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);
> 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) {
you don't need this check, clk_prepare_enable handles NULL
> + 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) {
here too
> + 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);
>
--
regards,
Stan
> On Fri, May 01, 2020 at 12:06:16AM +0200, Ansuel Smith wrote:
> > 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(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> > index da8058fd1925..372d2c8508b5 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -45,6 +45,9 @@
> > #define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
> >
> > #define PCIE20_PARF_PHY_CTRL 0x40
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(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 {
> > @@ -318,6 +322,11 @@ 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_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.
>
I don't understand what you mean here. I really can't think of another way
to set this only for qcom,pci-ipq8064 as ipq8064-v2 and apq8064 use the
same get resource function. Should I create a different get_resources for
the other 2 device?
> > + else
> > + res->phy_tx0_term_offset = 0;
> > +
On Thu, May 07, 2020 at 09:34:35PM +0200, [email protected] wrote:
> > On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote:
> > > 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(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > > index 6efcef040741..8cc5aea8a1da 100644
> > > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > > @@ -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.
> >
>
> 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
On 5/12/20 6:45 PM, Rob Herring wrote:
> On Thu, May 07, 2020 at 09:34:35PM +0200, [email protected] wrote:
>>> On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote:
>>>> 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(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>>> index 6efcef040741..8cc5aea8a1da 100644
>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>>> @@ -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.
>>>
>>
>> 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.
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.
>
> Rob
>
--
regards,
Stan
> On 5/12/20 6:45 PM, Rob Herring wrote:
> > On Thu, May 07, 2020 at 09:34:35PM +0200, [email protected]
> wrote:
> >>> On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote:
> >>>> 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(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> >>> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> >>>> index 6efcef040741..8cc5aea8a1da 100644
> >>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> >>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> >>>> @@ -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.
> >>>
> >>
> >> 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.
>
> 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.
>
I will limit this to ipq8064(-v2) if this could be a problem.
> >
> > Rob
> >
>
> --
> regards,
> Stan
On 5/13/20 3:54 PM, [email protected] wrote:
>> Hi Ansuel,
>>
>> On 5/1/20 1:06 AM, Ansuel Smith wrote:
>>> 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(+)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
>> b/drivers/pci/controller/dwc/pcie-qcom.c
>>> index da8058fd1925..372d2c8508b5 100644
>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>> @@ -45,6 +45,9 @@
>>> #define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
>>>
>>> #define PCIE20_PARF_PHY_CTRL 0x40
>>> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(12,
>> 16)
>>
>> The mask definition is not correct. Should be GENMASK(20, 16)
>>
>>> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x) ((x) << 16)
>>> +
>>> #define PCIE20_PARF_PHY_REFCLK 0x4C
>>> #define PHY_REFCLK_SSP_EN BIT(16)
>>> #define PHY_REFCLK_USE_PAD BIT(12)
>>> @@ -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)
>>> 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.
Sorry, my fault. I read the compatible check above as apq8064 but it is ipq.
--
regards,
Stan
Hi Ansuel,
On 5/1/20 1:06 AM, Ansuel Smith wrote:
> 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(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index da8058fd1925..372d2c8508b5 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -45,6 +45,9 @@
> #define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
>
> #define PCIE20_PARF_PHY_CTRL 0x40
> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(12, 16)
The mask definition is not correct. Should be GENMASK(20, 16)
> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x) ((x) << 16)
> +
> #define PCIE20_PARF_PHY_REFCLK 0x4C
> #define PHY_REFCLK_SSP_EN BIT(16)
> #define PHY_REFCLK_USE_PAD BIT(12)
> @@ -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)
> 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?
> + 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)
> /* enable PCIe clocks and resets */
> qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 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?
> + 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 },
>
--
regards,
Stan
> Hi Ansuel,
>
> On 5/1/20 1:06 AM, Ansuel Smith wrote:
> > 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(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> > index da8058fd1925..372d2c8508b5 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -45,6 +45,9 @@
> > #define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
> >
> > #define PCIE20_PARF_PHY_CTRL 0x40
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(12,
> 16)
>
> The mask definition is not correct. Should be GENMASK(20, 16)
>
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x) ((x) << 16)
> > +
> > #define PCIE20_PARF_PHY_REFCLK 0x4C
> > #define PHY_REFCLK_SSP_EN BIT(16)
> > #define PHY_REFCLK_USE_PAD BIT(12)
> > @@ -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)
> > 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.
> > + 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)
> > /* enable PCIe clocks and resets */
> > qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL,
> BIT(0), 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?
>
Will check this and the wrong genmask.
> > + 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 },
> >
>
> --
> regards,
> Stan
Hi,
On 5/13/20 3:56 PM, [email protected] wrote:
>> On 5/12/20 6:45 PM, Rob Herring wrote:
>>> On Thu, May 07, 2020 at 09:34:35PM +0200, [email protected]
>> wrote:
>>>>> On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote:
>>>>>> 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(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>>>> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>>>>> index 6efcef040741..8cc5aea8a1da 100644
>>>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
>>>>>> @@ -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.
>>>>>
>>>>
>>>> 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.
>>
>> 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.
>>
>
> I will limit this to ipq8064(-v2) if this could be a problem.
I guess you can do it that way, but if new board appear in the future
with slightly different parameters (for example deemph_gen1 = 23 and so
on) do we need to add another compatible for that? At the end we will
have compatibles per board but not per SoC. :(
--
regards,
Stan