arm64: IPQ6018 PCIe support
Subsystems (8)
v5 cover arm64: IPQ6018 PCIe support
This series adds support for the single PCIe lane on IPQ6018 SoCs. The code is ported from downstream Codeaurora v5.4 kernel. The main difference from downstream code is the split of PCIe registers configuration from .init to .post_init, since it requires phy_power_on().
Tested on IPQ6010 based hardware.
Changes in v5:
* Remove comments from qcom_pcie_init_2_9_0() (Bjorn Andersson)
Changes in v4:
* Drop applied DT bits
* Add max-link-speed that was missing from the applied v2 patch
* Rebase the driver on v5.16-rc3
Changes in v3:
* Drop applied patches
* Rely on generic code for speed setup
* Drop unused macros
* Formatting fixes
Changes in v2:
* Add patch moving GEN3_RELATED macros to a common header
* Drop ATU configuration from pcie-qcom
* Remove local definition of common registers
* Use bulk clk and reset APIs
* Remove msi-parent from device-tree
Baruch Siach (2):
arm64: dts: qcom: ipq6018: add pcie max-link-speed
PCI: dwc: tegra: move GEN3_RELATED DBI register to common header
Selvam Sathappan Periakaruppan (1):
PCI: qcom: add support for IPQ60xx PCIe controller
arch/arm64/boot/dts/qcom/ipq6018.dtsi | 1 + drivers/pci/controller/dwc/pcie-designware.h | 7 + drivers/pci/controller/dwc/pcie-qcom.c | 137 +++++++++++++++++++ drivers/pci/controller/dwc/pcie-tegra194.c | 6 - 4 files changed, 145 insertions(+), 6 deletions(-)
Signature · 1 line
2.34.1
PATCH v5 1/3 arm64: dts: qcom: ipq6018: add pcie max-link-speed
From: Baruch Siach <[email protected]>
Add the generic 'max-link-speed' that correctly describes the IPQ6018 PCIe link generation limit. This allows the generic dwc code to configure the link speed correctly.
--- arch/arm64/boot/dts/qcom/ipq6018.dtsi | 1 + 1 file changed, 1 insertion(+)
arch/arm64/boot/dts/qcom/ipq6018.dtsi +1 -0
@@ -438,6 +438,7 @@ pcie0: pci@20000000 { linux,pci-domain = <0>; bus-range = <0x00 0xff>; num-lanes = <1>; + max-link-speed = <3>; #address-cells = <3>; #size-cells = <2>;
Signature · 1 line
2.34.1
PATCH v5 2/3 PCI: dwc: tegra: move GEN3_RELATED DBI register to common header
In reply to Baruch SiachFrom: Baruch Siach <[email protected]>
These are common dwc macros that will be used for other platforms.
--- drivers/pci/controller/dwc/pcie-designware.h | 6 ++++++ drivers/pci/controller/dwc/pcie-tegra194.c | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-)
drivers/pci/controller/dwc/pcie-designware.h +6 -0
@@ -74,6 +74,12 @@#define PCIE_MSI_INTR0_MASK 0x82C#define PCIE_MSI_INTR0_STATUS 0x830 +#define GEN3_RELATED_OFF 0x890 +#define GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL BIT(0) +#define GEN3_RELATED_OFF_GEN3_EQ_DISABLE BIT(16) +#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT 24 +#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK GENMASK(25, 24) +#define PCIE_PORT_MULTI_LANE_CTRL 0x8C0#define PORT_MLTI_UPCFG_SUPPORT BIT(7)
drivers/pci/controller/dwc/pcie-tegra194.c +0 -6
@@ -193,12 +193,6 @@#define GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_MASK GENMASK(23, 8)#define GEN3_EQ_CONTROL_OFF_FB_MODE_MASK GENMASK(3, 0) -#define GEN3_RELATED_OFF 0x890 -#define GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL BIT(0) -#define GEN3_RELATED_OFF_GEN3_EQ_DISABLE BIT(16) -#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT 24 -#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK GENMASK(25, 24) -#define PORT_LOGIC_AMBA_ERROR_RESPONSE_DEFAULT 0x8D0#define AMBA_ERROR_RESPONSE_CRS_SHIFT 3#define AMBA_ERROR_RESPONSE_CRS_MASK GENMASK(1, 0)
Signature · 1 line
2.34.1
PATCH v5 3/3 PCI: qcom: add support for IPQ60xx PCIe controller
In reply to Baruch SiachFrom: Selvam Sathappan Periakaruppan <[email protected]>
IPQ60xx series of SoCs have one port of PCIe gen 3. Add support for that platform.
The code is based on downstream[1] Codeaurora kernel v5.4 (branch win.linuxopenwrt.2.0).
Split out the DBI registers access part from .init into .post_init. DBI registers are only accessible after phy_power_on().
[1] https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/
---
v5:
* Remove comments from qcom_pcie_init_2_9_0() (Bjorn Andersson)
v4:
* Rebase on v5.16-rc1
v3:
* Drop speed setup; rely on generic code (Rob Herring)
* Drop unused CLK_RATE macros (Bjorn Helgaas)
* Minor formatting fixes (Bjorn Helgaas)
* Add reference to downstream Codeaurora kernel tree (Bjorn Helgaas)
v2:
* Drop ATU configuration; rely on common code instead
* Use more common register macros
* Use bulk clk and reset APIs
--- drivers/pci/controller/dwc/pcie-designware.h | 1 + drivers/pci/controller/dwc/pcie-qcom.c | 137 +++++++++++++++++++ 2 files changed, 138 insertions(+)
drivers/pci/controller/dwc/pcie-designware.h +1 -0
@@ -76,6 +76,7 @@#define GEN3_RELATED_OFF 0x890#define GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL BIT(0) +#define GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS BIT(13)#define GEN3_RELATED_OFF_GEN3_EQ_DISABLE BIT(16)#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT 24#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK GENMASK(25, 24)
drivers/pci/controller/dwc/pcie-qcom.c +137 -0
179 lines not on this page · raw message
Signature · 1 line
2.34.1
Re: [PATCH v5 3/3] PCI: qcom: add support for IPQ60xx PCIe controller
Capitalize subject line to match history, i.e.,
$ git log --oneline drivers/pci/controller/dwc/pcie-qcom.c 3f13d611aa6b PCI: qcom: Use __be16 type to store return value from cpu_to_be16() 4e0e90539bb0 PCI: qcom: Fix an error handling path in 'qcom_pcie_probe()' 45a3ec891370 PCI: qcom: Add sc8180x compatible aa9c0df98c29 PCI: qcom: Switch pcie_1_pipe_clk_src after PHY init in SC7280 b89ff410253d PCI: qcom: Replace ops with struct pcie_cfg in pcie match data 2cfef1971aea PCI: qcom: Use PHY_REFCLK_USE_PAD only for ipq8064 7081556f81f7 PCI: qcom: Add support for ddrss_sf_tbu clock 4c9398822106 PCI: qcom: Add support for configuring BDF to SID mapping for SM8250 e1dd639e374a PCI: qcom: Add SM8250 SoC support
A good one would be:
PCI: qcom: Add IPQ60xx support
We already know this is a PCIe controller, and it's nice to have the important words like "IPQ60xx" early in the message so they don't get truncated or wrapped.
Quoted 129 lines · Baruch Siach
From: Selvam Sathappan Periakaruppan <[email protected]>
IPQ60xx series of SoCs have one port of PCIe gen 3. Add support for that platform.
The code is based on downstream[1] Codeaurora kernel v5.4 (branch win.linuxopenwrt.2.0).
Split out the DBI registers access part from .init into .post_init. DBI registers are only accessible after
phy_power_on().[1] https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/
Signed-off-by Selvam Sathappan Periakaruppan <[email protected]>Signed-off-by Baruch Siach <[email protected]>---
v5:* Remove comments from
qcom_pcie_init_2_9_0()(Bjorn Andersson)v4:
* Rebase on v5.16-rc1
v3:
* Drop speed setup; rely on generic code (Rob Herring)* Drop unused
CLK_RATEmacros (Bjorn Helgaas)* Minor formatting fixes (Bjorn Helgaas)
* Add reference to downstream Codeaurora kernel tree (Bjorn Helgaas)
v2:
* Drop ATU configuration; rely on common code instead* Use more common register macros
* Use bulk clk and reset APIs
--- drivers/pci/controller/dwc/pcie-designware.h | 1 + drivers/pci/controller/dwc/pcie-qcom.c | 137 +++++++++++++++++++ 2 files changed, 138 insertions(+)drivers/pci/controller/dwc/pcie-designware.h +1
-0@@ -76,6 +76,7 @@#define GEN3_RELATED_OFF 0x890#define GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL BIT(0) +#define GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS BIT(13)#define GEN3_RELATED_OFF_GEN3_EQ_DISABLE BIT(16)#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT 24#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK GENMASK(25, 24)drivers/pci/controller/dwc/pcie-qcom.c +44
-0@@ -52,6 +52,10 @@#define PCIE20_PARF_DBI_BASE_ADDR 0x168#define PCIE20_PARF_SLV_ADDR_SPACE_SIZE 0x16C#define PCIE20_PARF_MHI_CLOCK_RESET_CTRL 0x174 +#define AHB_CLK_EN BIT(0) +#define MSTR_AXI_CLK_EN BIT(1) +#define BYPASS BIT(4) +#define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT 0x178#define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2 0x1A8#define PCIE20_PARF_LTSSM 0x1B0@@ -171,6 +175,11 @@ struct qcom_pcie_resources_2_7_0 {struct clk *ref_clk_src; }; +struct qcom_pcie_resources_2_9_0 { +struct clk_bulk_data clks[5 ]; +struct reset_control *rst; +}; +union qcom_pcie_resources {struct qcom_pcie_resources_1_0_0 v1_0_0;struct qcom_pcie_resources_2_1_0 v2_1_0;@@ -178,6 +187,7 @@ union qcom_pcie_resources {struct qcom_pcie_resources_2_3_3 v2_3_3;struct qcom_pcie_resources_2_4_0 v2_4_0;struct qcom_pcie_resources_2_7_0 v2_7_0; +struct qcom_pcie_resources_2_9_0 v2_9_0; };struct qcom_pcie ;@@ -1297,6 +1307,119 @@ static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie)clk_disable_unprepare (res->pipe_clk); } +static int qcom_pcie_get_resources_2_9_0 (struct qcom_pcie *pcie) +{ +struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0; +struct dw_pcie *pci = pcie->pci; +struct device *dev = pci->dev; +int ret; + + res->clks[0 ].id ="iface" ; + res->clks[1 ].id ="axi_m" ; + res->clks[2 ].id ="axi_s" ; + res->clks[3 ].id ="axi_bridge" ; + res->clks[4 ].id ="rchng" ; + + ret =devm_clk_bulk_get (dev,ARRAY_SIZE (res->clks), res->clks); +if (ret <0 ) +return ret; + + res->rst =devm_reset_control_array_get_exclusive (dev); +if (IS_ERR (res->rst)) +return PTR_ERR (res->rst); + +return 0 ; +} + +static void qcom_pcie_deinit_2_9_0 (struct qcom_pcie *pcie) +{ +struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0; + +clk_bulk_disable_unprepare (ARRAY_SIZE (res->clks), res->clks); +} + +static int qcom_pcie_init_2_9_0 (struct qcom_pcie *pcie) +{ +struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
drivers/pci/controller/dwc/pcie-qcom.c static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie) · line 1353
Quoted 10 lines
drivers/pci/controller/dwc/pcie-qcom.c +10
-0+struct device *dev = pcie->pci->dev; +int ret; + + ret =reset_control_assert (res->rst); +if (ret) { +dev_err (dev,"reset assert failed (%d)\n" , ret); +return ret; + } + +usleep_range (2000 ,2500 );
Where do these sleep durations come from? If they're specified somewhere by PCIe, can you include a citation, e.g., a section number in the spec.
Quoted 42 lines
drivers/pci/controller/dwc/pcie-qcom.c +42
-0+ ret =reset_control_deassert (res->rst); +if (ret) { +dev_err (dev,"reset deassert failed (%d)\n" , ret); +return ret; + } + +usleep_range (2000 ,2500 ); + + ret =clk_bulk_prepare_enable (ARRAY_SIZE (res->clks), res->clks); +if (ret) + goto err_reset; + +return 0 ; + +err_reset: +reset_control_assert (res->rst); + +return ret; +} + +static int qcom_pcie_post_init_2_9_0 (struct qcom_pcie *pcie) +{ +struct dw_pcie *pci = pcie->pci; +u16 offset =dw_pcie_find_capability (pci,PCI_CAP_ID_EXP ); +u32 val; +int i; + +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 ); + +writel (0 , pcie->parf +PCIE20_PARF_DBI_BASE_ADDR ); + +writel (DEVICE_TYPE_RC , pcie->parf +PCIE20_PARF_DEVICE_TYPE ); +writel (BYPASS |MSTR_AXI_CLK_EN |AHB_CLK_EN , + pcie->parf +PCIE20_PARF_MHI_CLOCK_RESET_CTRL ); +writel (GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS + |GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL , + pci->dbi_base +GEN3_RELATED_OFF );
drivers/pci/controller/dwc/pcie-qcom.c static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie) · line 1406
Quoted 10 lines
drivers/pci/controller/dwc/pcie-qcom.c +10
-0+ +writel (MST_WAKEUP_EN |SLV_WAKEUP_EN |MSTR_ACLK_CGC_DIS + |SLV_ACLK_CGC_DIS |CORE_CLK_CGC_DIS | +AUX_PWR_DET |L23_CLK_RMV_DIS |L1_CLK_RMV_DIS , + pcie->parf +PCIE20_PARF_SYS_CTRL ); + +writel (0 , pcie->parf +PCIE20_PARF_Q2A_FLUSH ); + +dw_pcie_dbi_ro_wr_en (pci); +writel (PCIE_CAP_LINK1_VAL , pci->dbi_base + offset +PCI_EXP_SLTCAP );
Is "PCIE_CAP_LINK1_VAL" misnamed? If it's being written to PCI_EXP_SLTCAP, seems like it should be named something about a *slot*, not a *link*.
Or is this an error and it should this be written to PCI_EXP_LNKCAP instead?
Could PCIE_CAP_LINK1_VAL be defined in terms of PCI_EXP_SLTCAP_* or PCI_EXP_LNKCAP_*?
drivers/pci/controller/dwc/pcie-qcom.c static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie) · line 1411
Quoted 4 lines
drivers/pci/controller/dwc/pcie-qcom.c +4
-0+/* Configure PCIe link capabilities for ASPM */ + val =readl (pci->dbi_base + offset +PCI_EXP_LNKCAP ); + val &= ~PCI_EXP_LNKCAP_ASPMS ; +writel (val, pci->dbi_base + offset +PCI_EXP_LNKCAP );
Thanks for using the standard PCI_EXP_LNKCAP and similar #defines. Does this mean you don't advertise any ASPM support on this platform? If so, that's fine, but the comment could be more specific. "Configure" doesn't give any hint about *how* they are configured.
Quoted 56 lines
drivers/pci/controller/dwc/pcie-qcom.c +24
-0+writel (PCI_EXP_DEVCTL2_COMP_TMOUT_DIS , pci->dbi_base + offset + +PCI_EXP_DEVCTL2 ); + +for (i =0 ; i <256 ; i++) +writel (0x0 , pcie->parf +PCIE20_PARF_BDF_TO_SID_TABLE_N + + (4 * i)); + +return 0 ; +} +static int qcom_pcie_link_up (struct dw_pcie *pci) {u16 offset =dw_pcie_find_capability (pci,PCI_CAP_ID_EXP );@@ -1487,6 +1610,15 @@ static const struct qcom_pcie_ops ops_1_9_0 = { .config_sid = qcom_pcie_config_sid_sm8250, }; +/* Qcom IP rev.: 2.9.0 Synopsys IP rev.: 5.00a */ +static const struct qcom_pcie_ops ops_2_9_0 = { + .get_resources = qcom_pcie_get_resources_2_9_0, + .init = qcom_pcie_init_2_9_0, + .post_init = qcom_pcie_post_init_2_9_0, + .deinit = qcom_pcie_deinit_2_9_0, + .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable, +}; +static const struct qcom_pcie_cfg apq8084_cfg = { .ops = &ops_1_0_0, };@@ -1520,6 +1652,10 @@ static const struct qcom_pcie_cfg sc7280_cfg = { .pipe_clk_need_muxing = true, }; +static const struct qcom_pcie_cfg ipq6018_cfg = { + .ops = &ops_2_9_0, +}; +static const struct dw_pcie_ops dw_pcie_ops = { .link_up = qcom_pcie_link_up, .start_link = qcom_pcie_start_link,@@ -1629,6 +1765,7 @@ static const struct of_device_id qcom_pcie_match[] = { { .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg }, { .compatible = "qcom,pcie-sc8180x", .data = &sm8250_cfg }, { .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg }, + { .compatible = "qcom,pcie-ipq6018", .data = &ipq6018_cfg }, { } };Signature · 7 lines
2.34.1 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v5 3/3] PCI: qcom: add support for IPQ60xx PCIe controller
Hi Bjorn,
Thanks for your review comments.
Quoted 157 lines · Bjorn Helgaas
Capitalize subject line to match history, i.e.,
$ git log --oneline drivers/pci/controller/dwc/pcie-qcom.c 3f13d611aa6b PCI: qcom: Use __be16 type to store return value from cpu_to_be16() 4e0e90539bb0 PCI: qcom: Fix an error handling path in 'qcom_pcie_probe()' 45a3ec891370 PCI: qcom: Add sc8180x compatible aa9c0df98c29 PCI: qcom: Switch pcie_1_pipe_clk_src after PHY init in SC7280 b89ff410253d PCI: qcom: Replace ops with struct pcie_cfg in pcie match data 2cfef1971aea PCI: qcom: Use PHY_REFCLK_USE_PAD only for ipq8064 7081556f81f7 PCI: qcom: Add support for ddrss_sf_tbu clock 4c9398822106 PCI: qcom: Add support for configuring BDF to SID mapping for SM8250 e1dd639e374a PCI: qcom: Add SM8250 SoC supportA good one would be:
PCI: qcom: Add IPQ60xx support
We already know this is a PCIe controller, and it's nice to have the important words like "IPQ60xx" early in the message so they don't get truncated or wrapped.
Quoted 135 lines · Baruch Siach
From: Selvam Sathappan Periakaruppan <[email protected]>
IPQ60xx series of SoCs have one port of PCIe gen 3. Add support for that platform.
The code is based on downstream[1] Codeaurora kernel v5.4 (branch win.linuxopenwrt.2.0).
Split out the DBI registers access part from .init into .post_init. DBI registers are only accessible after
phy_power_on().[1] https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/
Signed-off-by Selvam Sathappan Periakaruppan <[email protected]>Signed-off-by Baruch Siach <[email protected]>---
v5:* Remove comments from
qcom_pcie_init_2_9_0()(Bjorn Andersson)v4:
* Rebase on v5.16-rc1
v3:
* Drop speed setup; rely on generic code (Rob Herring)* Drop unused
CLK_RATEmacros (Bjorn Helgaas)* Minor formatting fixes (Bjorn Helgaas)
* Add reference to downstream Codeaurora kernel tree (Bjorn Helgaas)
v2:
* Drop ATU configuration; rely on common code instead* Use more common register macros
* Use bulk clk and reset APIs
--- drivers/pci/controller/dwc/pcie-designware.h | 1 + drivers/pci/controller/dwc/pcie-qcom.c | 137 +++++++++++++++++++ 2 files changed, 138 insertions(+)drivers/pci/controller/dwc/pcie-designware.h +1
-0@@ -76,6 +76,7 @@#define GEN3_RELATED_OFF 0x890#define GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL BIT(0) +#define GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS BIT(13)#define GEN3_RELATED_OFF_GEN3_EQ_DISABLE BIT(16)#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT 24#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK GENMASK(25, 24)drivers/pci/controller/dwc/pcie-qcom.c +50
-0@@ -52,6 +52,10 @@#define PCIE20_PARF_DBI_BASE_ADDR 0x168#define PCIE20_PARF_SLV_ADDR_SPACE_SIZE 0x16C#define PCIE20_PARF_MHI_CLOCK_RESET_CTRL 0x174 +#define AHB_CLK_EN BIT(0) +#define MSTR_AXI_CLK_EN BIT(1) +#define BYPASS BIT(4) +#define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT 0x178#define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2 0x1A8#define PCIE20_PARF_LTSSM 0x1B0@@ -171,6 +175,11 @@ struct qcom_pcie_resources_2_7_0 {struct clk *ref_clk_src; }; +struct qcom_pcie_resources_2_9_0 { +struct clk_bulk_data clks[5 ]; +struct reset_control *rst; +}; +union qcom_pcie_resources {struct qcom_pcie_resources_1_0_0 v1_0_0;struct qcom_pcie_resources_2_1_0 v2_1_0;@@ -178,6 +187,7 @@ union qcom_pcie_resources {struct qcom_pcie_resources_2_3_3 v2_3_3;struct qcom_pcie_resources_2_4_0 v2_4_0;struct qcom_pcie_resources_2_7_0 v2_7_0; +struct qcom_pcie_resources_2_9_0 v2_9_0; };struct qcom_pcie ;@@ -1297,6 +1307,119 @@ static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie)clk_disable_unprepare (res->pipe_clk); } +static int qcom_pcie_get_resources_2_9_0 (struct qcom_pcie *pcie) +{ +struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0; +struct dw_pcie *pci = pcie->pci; +struct device *dev = pci->dev; +int ret; + + res->clks[0 ].id ="iface" ; + res->clks[1 ].id ="axi_m" ; + res->clks[2 ].id ="axi_s" ; + res->clks[3 ].id ="axi_bridge" ; + res->clks[4 ].id ="rchng" ; + + ret =devm_clk_bulk_get (dev,ARRAY_SIZE (res->clks), res->clks); +if (ret <0 ) +return ret; + + res->rst =devm_reset_control_array_get_exclusive (dev); +if (IS_ERR (res->rst)) +return PTR_ERR (res->rst); + +return 0 ; +} + +static void qcom_pcie_deinit_2_9_0 (struct qcom_pcie *pcie) +{ +struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0; + +clk_bulk_disable_unprepare (ARRAY_SIZE (res->clks), res->clks); +} + +static int qcom_pcie_init_2_9_0 (struct qcom_pcie *pcie) +{ +struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0; +struct device *dev = pcie->pci->dev; +int ret; + + ret =reset_control_assert (res->rst); +if (ret) { +dev_err (dev,"reset assert failed (%d)\n" , ret);
Quoted 8 lines
drivers/pci/controller/dwc/pcie-qcom.c static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie) · line 1353
Quoted 4 lines
drivers/pci/controller/dwc/pcie-qcom.c +4
-0+return ret; + } + +usleep_range (2000 ,2500 );Where do these sleep durations come from? If they're specified somewhere by PCIe, can you include a citation, e.g., a section number in the spec.
As I mentioned before, I have no access to hardware documentation. I'm only porting working code from downstream kernel.
In a comment on v4 Bjorn Andersson mentioned "datasheet stating the minimum timing of the operations to be performed to get the PCIe controller into a known (clean) state".
https://lore.kernel.org/all/Ydd5Wh0KeADBQ%2Fh1@ripper/
I have no further details.
Quoted 57 lines
Quoted 42 lines
drivers/pci/controller/dwc/pcie-qcom.c +42
-0+ ret =reset_control_deassert (res->rst); +if (ret) { +dev_err (dev,"reset deassert failed (%d)\n" , ret); +return ret; + } + +usleep_range (2000 ,2500 ); + + ret =clk_bulk_prepare_enable (ARRAY_SIZE (res->clks), res->clks); +if (ret) + goto err_reset; + +return 0 ; + +err_reset: +reset_control_assert (res->rst); + +return ret; +} + +static int qcom_pcie_post_init_2_9_0 (struct qcom_pcie *pcie) +{ +struct dw_pcie *pci = pcie->pci; +u16 offset =dw_pcie_find_capability (pci,PCI_CAP_ID_EXP ); +u32 val; +int i; + +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 ); + +writel (0 , pcie->parf +PCIE20_PARF_DBI_BASE_ADDR ); + +writel (DEVICE_TYPE_RC , pcie->parf +PCIE20_PARF_DEVICE_TYPE ); +writel (BYPASS |MSTR_AXI_CLK_EN |AHB_CLK_EN , + pcie->parf +PCIE20_PARF_MHI_CLOCK_RESET_CTRL ); +writel (GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS + |GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL , + pci->dbi_base +GEN3_RELATED_OFF );drivers/pci/controller/dwc/pcie-qcom.c static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie) · line 1406
Quoted 10 lines
drivers/pci/controller/dwc/pcie-qcom.c +10
-0+ +writel (MST_WAKEUP_EN |SLV_WAKEUP_EN |MSTR_ACLK_CGC_DIS + |SLV_ACLK_CGC_DIS |CORE_CLK_CGC_DIS | +AUX_PWR_DET |L23_CLK_RMV_DIS |L1_CLK_RMV_DIS , + pcie->parf +PCIE20_PARF_SYS_CTRL ); + +writel (0 , pcie->parf +PCIE20_PARF_Q2A_FLUSH ); + +dw_pcie_dbi_ro_wr_en (pci); +writel (PCIE_CAP_LINK1_VAL , pci->dbi_base + offset +PCI_EXP_SLTCAP );Is "
PCIE_CAP_LINK1_VAL" misnamed? If it's being written toPCI_EXP_SLTCAP, seems like it should be named something about a *slot*, not a *link*.
Quoted 2 lines
Or is this an error and it should this be written to
PCI_EXP_LNKCAPinstead?
As above, I don't know. I can only mention that the exact same assignment is currently also in qcom_pcie_init_2_3_3().
Quoted 2 lines
Could
PCIE_CAP_LINK1_VALbe defined in terms of PCI_EXP_SLTCAP_* or PCI_EXP_LNKCAP_*?
It surely can. Whether it makes sense I can't tell.
Quoted 9 lines
drivers/pci/controller/dwc/pcie-qcom.c static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie) · line 1411
Quoted 4 lines
drivers/pci/controller/dwc/pcie-qcom.c +4
-0+/* Configure PCIe link capabilities for ASPM */ + val =readl (pci->dbi_base + offset +PCI_EXP_LNKCAP ); + val &= ~PCI_EXP_LNKCAP_ASPMS ; +writel (val, pci->dbi_base + offset +PCI_EXP_LNKCAP );Thanks for using the standard
PCI_EXP_LNKCAPand similar #defines. Does this mean you don't advertise any ASPM support on this platform? If so, that's fine, but the comment could be more specific. "Configure" doesn't give any hint about *how* they are configured.
Again, the same code is in qcom_pcie_init_2_3_3(). I'll just drop the comment in the next submission.
Thanks,
baruch
Quoted 50 lines
Quoted 50 lines
drivers/pci/controller/dwc/pcie-qcom.c +24
-0+writel (PCI_EXP_DEVCTL2_COMP_TMOUT_DIS , pci->dbi_base + offset + +PCI_EXP_DEVCTL2 ); + +for (i =0 ; i <256 ; i++) +writel (0x0 , pcie->parf +PCIE20_PARF_BDF_TO_SID_TABLE_N + + (4 * i)); + +return 0 ; +} +static int qcom_pcie_link_up (struct dw_pcie *pci) {u16 offset =dw_pcie_find_capability (pci,PCI_CAP_ID_EXP );@@ -1487,6 +1610,15 @@ static const struct qcom_pcie_ops ops_1_9_0 = { .config_sid = qcom_pcie_config_sid_sm8250, }; +/* Qcom IP rev.: 2.9.0 Synopsys IP rev.: 5.00a */ +static const struct qcom_pcie_ops ops_2_9_0 = { + .get_resources = qcom_pcie_get_resources_2_9_0, + .init = qcom_pcie_init_2_9_0, + .post_init = qcom_pcie_post_init_2_9_0, + .deinit = qcom_pcie_deinit_2_9_0, + .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable, +}; +static const struct qcom_pcie_cfg apq8084_cfg = { .ops = &ops_1_0_0, };@@ -1520,6 +1652,10 @@ static const struct qcom_pcie_cfg sc7280_cfg = { .pipe_clk_need_muxing = true, }; +static const struct qcom_pcie_cfg ipq6018_cfg = { + .ops = &ops_2_9_0, +}; +static const struct dw_pcie_ops dw_pcie_ops = { .link_up = qcom_pcie_link_up, .start_link = qcom_pcie_start_link,@@ -1629,6 +1765,7 @@ static const struct of_device_id qcom_pcie_match[] = { { .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg }, { .compatible = "qcom,pcie-sc8180x", .data = &sm8250_cfg }, { .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg }, + { .compatible = "qcom,pcie-ipq6018", .data = &ipq6018_cfg }, { } };Signature · 1 line
2.34.1
Signature · 8 lines
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- [email protected] - tel: +972.52.368.4656, http://www.tkos.co.il -
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernelRe: [PATCH v5 3/3] PCI: qcom: add support for IPQ60xx PCIe controller
Quoted 21 lines · Baruch Siach
Quoted 11 lines · Bjorn Helgaas
Quoted 9 lines · Baruch Siach
drivers/pci/controller/dwc/pcie-qcom.c +9
-0+static int qcom_pcie_init_2_9_0 (struct qcom_pcie *pcie) +{ +struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0; +struct device *dev = pcie->pci->dev; +int ret; + + ret =reset_control_assert (res->rst); +if (ret) { +dev_err (dev,"reset assert failed (%d)\n" , ret);Quoted 8 lines
drivers/pci/controller/dwc/pcie-qcom.c static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie) · line 1353
Quoted 4 lines
drivers/pci/controller/dwc/pcie-qcom.c +4
-0+return ret; + } + +usleep_range (2000 ,2500 );Where do these sleep durations come from? If they're specified somewhere by PCIe, can you include a citation, e.g., a section number in the spec.
Quoted 6 lines
As I mentioned before, I have no access to hardware documentation. I'm only porting working code from downstream kernel.
In a comment on v4 Bjorn Andersson mentioned "datasheet stating the minimum timing of the operations to be performed to get the PCIe controller into a known (clean) state".
Sorry if I'm repeating questions. We can help avoid that by adding a brief comment here like "black magic copied from working code at X" or whatever you *do* know. Repeated questions are wasted effort for both the author and reviewers.
Sleeps with unsourced durations are always suspect because nobody wants to wait longer than necessary, and if all we have is a bare number and nobody knows how it was derived, we can't be confident about changing it.
Quoted 3 lines
https://lore.kernel.org/all/Ydd5Wh0KeADBQ%2Fh1@ripper/
I have no further details.