There are four PCIe ports on IPQ9574, pcie0 thru pcie3. This series
addresses pcie2, which is a gen3x2 port. The board I have only uses
pcie2, and that's the only one enabled in this series.
I believe this makes sense as a monolithic series, as the individual
pieces are not that useful by themselves.
In v2, I've had some issues regarding the dt schema checks. For
transparency, I used the following test invocations to test v3:
make dt_binding_check DT_SCHEMA_FILES=qcom,pcie.yaml:qcom,ipq8074-qmp-pcie-phy.yaml
make dtbs_check DT_SCHEMA_FILES=qcom,pcie.yaml:qcom,ipq8074-qmp-pcie-phy.yaml
Changes since v2:
- reworked resets in qcom,pcie.yaml to resolve dt schema errors
- constrained "reg" in qcom,pcie.yaml
- reworked min/max intems in qcom,ipq8074-qmp-pcie-phy.yaml
- dropped msi-parent for pcie node, as it is handled by "msi" IRQ
Changes since v1:
- updated new tables in phy-qcom-qmp-pcie.c to use lowercase hex numbers
- reorganized qcom,ipq8074-qmp-pcie-phy.yaml to use a single list of clocks
- reorganized qcom,pcie.yaml to include clocks+resets per compatible
- Renamed "pcie2_qmp_phy" label to "pcie2_phy"
- moved "ranges" property of pcie@20000000 higher up
Alexandru Gagniuc (7):
dt-bindings: clock: Add PCIe pipe related clocks for IPQ9574
clk: qcom: gcc-ipq9574: Add PCIe pipe clocks
dt-bindings: PCI: qcom: Add IPQ9574 PCIe controller
PCI: qcom: Add support for IPQ9574
dt-bindings: phy: qcom,ipq8074-qmp-pcie: add ipq9574 gen3x2 PHY
phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY
arm64: dts: qcom: ipq9574: add PCIe2 nodes
.../devicetree/bindings/pci/qcom,pcie.yaml | 35 +++++
.../phy/qcom,ipq8074-qmp-pcie-phy.yaml | 36 ++++-
arch/arm64/boot/dts/qcom/ipq9574.dtsi | 93 +++++++++++-
drivers/clk/qcom/gcc-ipq9574.c | 76 ++++++++++
drivers/pci/controller/dwc/pcie-qcom.c | 13 +-
drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
.../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
include/dt-bindings/clock/qcom,ipq9574-gcc.h | 4 +
8 files changed, 400 insertions(+), 7 deletions(-)
--
2.40.1
The IPQ9574 has four PCIe "pipe" clocks. These clocks are required by
PCIe PHYs. Port the pipe clocks from the downstream 5.4 kernel.
Signed-off-by: Alexandru Gagniuc <[email protected]>
---
drivers/clk/qcom/gcc-ipq9574.c | 76 ++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/drivers/clk/qcom/gcc-ipq9574.c b/drivers/clk/qcom/gcc-ipq9574.c
index 0a3f846695b8..c748d2f124f3 100644
--- a/drivers/clk/qcom/gcc-ipq9574.c
+++ b/drivers/clk/qcom/gcc-ipq9574.c
@@ -1569,6 +1569,24 @@ static struct clk_regmap_phy_mux pcie0_pipe_clk_src = {
},
};
+static struct clk_branch gcc_pcie0_pipe_clk = {
+ .halt_reg = 0x28044,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x28044,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gcc_pcie0_pipe_clk",
+ .parent_hws = (const struct clk_hw *[]) {
+ &pcie0_pipe_clk_src.clkr.hw
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
static struct clk_regmap_phy_mux pcie1_pipe_clk_src = {
.reg = 0x29064,
.clkr = {
@@ -1583,6 +1601,24 @@ static struct clk_regmap_phy_mux pcie1_pipe_clk_src = {
},
};
+static struct clk_branch gcc_pcie1_pipe_clk = {
+ .halt_reg = 0x29044,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x29044,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gcc_pcie1_pipe_clk",
+ .parent_hws = (const struct clk_hw *[]) {
+ &pcie1_pipe_clk_src.clkr.hw
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
static struct clk_regmap_phy_mux pcie2_pipe_clk_src = {
.reg = 0x2a064,
.clkr = {
@@ -1597,6 +1633,24 @@ static struct clk_regmap_phy_mux pcie2_pipe_clk_src = {
},
};
+static struct clk_branch gcc_pcie2_pipe_clk = {
+ .halt_reg = 0x2a044,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x2a044,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data) {
+ .name = "gcc_pcie2_pipe_clk",
+ .parent_hws = (const struct clk_hw *[]) {
+ &pcie2_pipe_clk_src.clkr.hw
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
static struct clk_regmap_phy_mux pcie3_pipe_clk_src = {
.reg = 0x2b064,
.clkr = {
@@ -1611,6 +1665,24 @@ static struct clk_regmap_phy_mux pcie3_pipe_clk_src = {
},
};
+static struct clk_branch gcc_pcie3_pipe_clk = {
+ .halt_reg = 0x2b044,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x2b044,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data) {
+ .name = "gcc_pcie3_pipe_clk",
+ .parent_hws = (const struct clk_hw *[]) {
+ &pcie3_pipe_clk_src.clkr.hw
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
static const struct freq_tbl ftbl_pcie_rchng_clk_src[] = {
F(24000000, P_XO, 1, 0, 0),
F(100000000, P_GPLL0, 8, 0, 0),
@@ -4141,6 +4213,10 @@ static struct clk_regmap *gcc_ipq9574_clks[] = {
[GCC_SNOC_PCIE1_1LANE_S_CLK] = &gcc_snoc_pcie1_1lane_s_clk.clkr,
[GCC_SNOC_PCIE2_2LANE_S_CLK] = &gcc_snoc_pcie2_2lane_s_clk.clkr,
[GCC_SNOC_PCIE3_2LANE_S_CLK] = &gcc_snoc_pcie3_2lane_s_clk.clkr,
+ [GCC_PCIE0_PIPE_CLK] = &gcc_pcie0_pipe_clk.clkr,
+ [GCC_PCIE1_PIPE_CLK] = &gcc_pcie1_pipe_clk.clkr,
+ [GCC_PCIE2_PIPE_CLK] = &gcc_pcie2_pipe_clk.clkr,
+ [GCC_PCIE3_PIPE_CLK] = &gcc_pcie3_pipe_clk.clkr,
};
static const struct qcom_reset_map gcc_ipq9574_resets[] = {
--
2.40.1
IPQ9574 has PCIe controllers which are almost identical to IPQ6018.
The difference is that the "iface" clock is not required, and the
"sleep" reset is replaced by an "aux" reset. Document these
differences along with the compatible string.
Signed-off-by: Alexandru Gagniuc <[email protected]>
---
.../devicetree/bindings/pci/qcom,pcie.yaml | 35 +++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index cf9a6910b542..2d9bbcc95c31 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -26,6 +26,7 @@ properties:
- qcom,pcie-ipq8064-v2
- qcom,pcie-ipq8074
- qcom,pcie-ipq8074-gen3
+ - qcom,pcie-ipq9574
- qcom,pcie-msm8996
- qcom,pcie-qcs404
- qcom,pcie-sdm845
@@ -161,6 +162,7 @@ allOf:
enum:
- qcom,pcie-ipq6018
- qcom,pcie-ipq8074-gen3
+ - qcom,pcie-ipq9574
then:
properties:
reg:
@@ -397,6 +399,37 @@ allOf:
- const: axi_m_sticky # AXI Master Sticky reset
- const: axi_s_sticky # AXI Slave Sticky reset
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,pcie-ipq9574
+ then:
+ properties:
+ clocks:
+ minItems: 4
+ maxItems: 4
+ clock-names:
+ items:
+ - const: axi_m # AXI Master clock
+ - const: axi_s # AXI Slave clock
+ - const: axi_bridge # AXI bridge clock
+ - const: rchng
+ resets:
+ minItems: 8
+ maxItems: 8
+ reset-names:
+ items:
+ - const: pipe # PIPE reset
+ - const: aux # AUX reset
+ - const: sticky # Core Sticky reset
+ - const: axi_m # AXI Master reset
+ - const: axi_s # AXI Slave reset
+ - const: axi_s_sticky # AXI Slave Sticky reset
+ - const: axi_m_sticky # AXI Master Sticky reset
+ - const: ahb # AHB Reset
+
- if:
properties:
compatible:
@@ -507,6 +540,7 @@ allOf:
- qcom,pcie-ipq8064v2
- qcom,pcie-ipq8074
- qcom,pcie-ipq8074-gen3
+ - qcom,pcie-ipq9574
- qcom,pcie-qcs404
then:
required:
@@ -566,6 +600,7 @@ allOf:
- qcom,pcie-ipq8064-v2
- qcom,pcie-ipq8074
- qcom,pcie-ipq8074-gen3
+ - qcom,pcie-ipq9574
- qcom,pcie-qcs404
then:
properties:
--
2.40.1
Add support for the PCIe on IPQ9574. The main difference from ipq6018
is that the "iface" clock is not necessarry. Add a special case in
qcom_pcie_get_resources_2_9_0() to handle this.
Signed-off-by: Alexandru Gagniuc <[email protected]>
---
drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 14772edcf0d3..10560d6d6336 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1101,15 +1101,19 @@ 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;
+ int ret, num_clks = ARRAY_SIZE(res->clks) - 1;
- res->clks[0].id = "iface";
+ res->clks[0].id = "rchng";
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 (!of_device_is_compatible(dev->of_node, "qcom,pcie-ipq9574")) {
+ res->clks[4].id = "iface";
+ num_clks++;
+ }
+
+ ret = devm_clk_bulk_get(dev, num_clks, res->clks);
if (ret < 0)
return ret;
@@ -1664,6 +1668,7 @@ static const struct of_device_id qcom_pcie_match[] = {
{ .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
{ .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
{ .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
+ { .compatible = "qcom,pcie-ipq9574", .data = &cfg_2_9_0 },
{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
{ .compatible = "qcom,pcie-sa8540p", .data = &cfg_sc8280xp },
--
2.40.1
The IPQ9574 gen3x2 PHY is very similar to IPQ6018. It requires two
extra clocks named "anoc" and "snoc". Document this, and add a
new compatible string for this PHY.
Signed-off-by: Alexandru Gagniuc <[email protected]>
---
.../phy/qcom,ipq8074-qmp-pcie-phy.yaml | 36 ++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
index 634cec5d57ea..89949f4e89db 100644
--- a/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
@@ -19,19 +19,24 @@ properties:
- qcom,ipq6018-qmp-pcie-phy
- qcom,ipq8074-qmp-gen3-pcie-phy
- qcom,ipq8074-qmp-pcie-phy
+ - qcom,ipq9574-qmp-gen3x2-pcie-phy
reg:
items:
- description: serdes
clocks:
- maxItems: 3
+ minItems: 3
+ maxItems: 5
clock-names:
+ minItems: 3
items:
- const: aux
- const: cfg_ahb
- const: pipe
+ - const: anoc
+ - const: snoc
resets:
maxItems: 2
@@ -61,6 +66,35 @@ required:
- clock-output-names
- "#phy-cells"
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,ipq6018-qmp-pcie-phy
+ - qcom,ipq8074-qmp-gen3-pcie-phy
+ - qcom,ipq8074-qmp-pcie-phy
+ then:
+ properties:
+ clocks:
+ maxItems: 3
+ clock-names:
+ maxItems: 3
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,ipq9574-qmp-gen3x2-pcie-phy
+ then:
+ properties:
+ clocks:
+ minItems: 5
+ clock-names:
+ minItems: 5
+
additionalProperties: false
examples:
--
2.40.1
Add support for the gen3x2 PCIe PHY on IPQ9574, ported form downstream
5.4 kernel. Only the serdes and pcs_misc tables are new, the others
being reused from IPQ8074 and IPQ6018 PHYs.
Signed-off-by: Alexandru Gagniuc <[email protected]>
---
drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
.../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
2 files changed, 149 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index 8836bb1ff0cc..a4a79ddf50a5 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -487,6 +487,100 @@ static const struct qmp_phy_init_tbl ipq8074_pcie_gen3_pcs_misc_tbl[] = {
QMP_PHY_INIT_CFG(QPHY_V4_PCS_PCIE_ENDPOINT_REFCLK_DRIVE, 0xc1),
};
+static const struct qmp_phy_init_tbl ipq9574_gen3x2_pcie_serdes_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_PLL_BIAS_EN_CLKBUFLR_EN, 0x18),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_BIAS_EN_CTRL_BY_PSM, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CLK_SELECT, 0x31),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_PLL_IVCO, 0x0f),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_BG_TRIM, 0x0f),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CMN_CONFIG, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_LOCK_CMP_EN, 0x42),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_RESETSM_CNTRL, 0x20),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SVS_MODE_CLK_SEL, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_VCO_TUNE_MAP, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SVS_MODE_CLK_SEL, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_VCO_TUNE_TIMER1, 0xff),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_VCO_TUNE_TIMER2, 0x3f),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CORE_CLK_EN, 0x30),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_HSCLK_SEL, 0x21),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_DEC_START_MODE0, 0x68),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_DIV_FRAC_START3_MODE0, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_DIV_FRAC_START2_MODE0, 0xaa),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_DIV_FRAC_START1_MODE0, 0xab),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_LOCK_CMP2_MODE0, 0x14),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_LOCK_CMP1_MODE0, 0xd4),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CP_CTRL_MODE0, 0x09),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_PLL_RCTRL_MODE0, 0x16),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_PLL_CCTRL_MODE0, 0x28),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_INTEGLOOP_GAIN1_MODE0, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_INTEGLOOP_GAIN0_MODE0, 0xa0),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_VCO_TUNE2_MODE0, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_VCO_TUNE1_MODE0, 0x24),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SVS_MODE_CLK_SEL, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CORE_CLK_EN, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CORECLK_DIV, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CLK_SELECT, 0x32),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SYS_CLK_CTRL, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SYSCLK_BUF_ENABLE, 0x07),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SYSCLK_EN_SEL, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_BG_TIMER, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_HSCLK_SEL, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_DEC_START_MODE1, 0x53),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_DIV_FRAC_START3_MODE1, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_DIV_FRAC_START2_MODE1, 0x55),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_DIV_FRAC_START1_MODE1, 0x55),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_LOCK_CMP2_MODE1, 0x29),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_LOCK_CMP1_MODE1, 0xaa),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CP_CTRL_MODE1, 0x09),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_PLL_RCTRL_MODE1, 0x16),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_PLL_CCTRL_MODE1, 0x28),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_INTEGLOOP_GAIN1_MODE1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_INTEGLOOP_GAIN0_MODE1, 0xa0),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_VCO_TUNE2_MODE1, 0x03),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_VCO_TUNE1_MODE1, 0xb4),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SVS_MODE_CLK_SEL, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CORE_CLK_EN, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CORECLK_DIV_MODE1, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_COM_SSC_EN_CENTER, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SSC_PER1, 0x7d),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SSC_PER2, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER2, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SSC_STEP_SIZE1_MODE0, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SSC_STEP_SIZE2_MODE0, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SSC_STEP_SIZE1_MODE1, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_SSC_STEP_SIZE2_MODE1, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CLK_EP_DIV_MODE0, 0x19),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CLK_EP_DIV_MODE1, 0x28),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CLK_ENABLE1, 0x90),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_HSCLK_SEL, 0x89),
+ QMP_PHY_INIT_CFG(QSERDES_PLL_CLK_ENABLE1, 0x10),
+};
+
+static const struct qmp_phy_init_tbl ipq9574_gen3x2_pcie_pcs_misc_tbl[] = {
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_OSC_DTCT_ACTIONS, 0x00),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG2, 0x1d),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_L1P1_WAKEUP_DLY_TIME_AUXCLK_H, 0x00),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_L1P1_WAKEUP_DLY_TIME_AUXCLK_L, 0x01),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_L1P2_WAKEUP_DLY_TIME_AUXCLK_H, 0x00),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_L1P2_WAKEUP_DLY_TIME_AUXCLK_L, 0x01),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_EQ_CONFIG1, 0x14),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_EQ_CONFIG1, 0x10),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_EQ_CONFIG2, 0x0b),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_PRESET_P10_PRE, 0x00),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_PRESET_P10_POST, 0x58),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG4, 0x07),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_OSC_DTCT_CONFIG1, 0x00),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_OSC_DTCT_CONFIG2, 0x52),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_OSC_DTCT_CONFIG4, 0x19),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_INT_AUX_CLK_CONFIG1, 0x00),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_OSC_DTCT_MODE2_CONFIG2, 0x49),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_OSC_DTCT_MODE2_CONFIG4, 0x2a),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_OSC_DTCT_MODE2_CONFIG5, 0x02),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_OSC_DTCT_MODE2_CONFIG6, 0x03),
+ QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_DRIVE, 0xc1),
+};
+
static const struct qmp_phy_init_tbl sdm845_qmp_pcie_serdes_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_V3_COM_BIAS_EN_CLKBUFLR_EN, 0x14),
QMP_PHY_INIT_CFG(QSERDES_V3_COM_CLK_SELECT, 0x30),
@@ -2448,7 +2542,7 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
/* list of clocks required by phy */
static const char * const qmp_pciephy_clk_l[] = {
- "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
+ "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux", "anoc", "snoc"
};
/* list of regulators */
@@ -2499,6 +2593,16 @@ static const struct qmp_pcie_offsets qmp_pcie_offsets_v4x1 = {
.rx = 0x0400,
};
+static const struct qmp_pcie_offsets qmp_pcie_offsets_ipq9574 = {
+ .serdes = 0,
+ .pcs = 0x1000,
+ .pcs_misc = 0x1400,
+ .tx = 0x0200,
+ .rx = 0x0400,
+ .tx2 = 0x0600,
+ .rx2 = 0x0800,
+};
+
static const struct qmp_pcie_offsets qmp_pcie_offsets_v4x2 = {
.serdes = 0,
.pcs = 0x0a00,
@@ -2728,6 +2832,33 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x1_pciephy_cfg = {
.phy_status = PHYSTATUS,
};
+static const struct qmp_phy_cfg ipq9574_pciephy_gen3x2_cfg = {
+ .lanes = 2,
+
+ .offsets = &qmp_pcie_offsets_ipq9574,
+
+ .tbls = {
+ .serdes = ipq9574_gen3x2_pcie_serdes_tbl,
+ .serdes_num = ARRAY_SIZE(ipq9574_gen3x2_pcie_serdes_tbl),
+ .tx = ipq8074_pcie_gen3_tx_tbl,
+ .tx_num = ARRAY_SIZE(ipq8074_pcie_gen3_tx_tbl),
+ .rx = ipq6018_pcie_rx_tbl,
+ .rx_num = ARRAY_SIZE(ipq6018_pcie_rx_tbl),
+ .pcs = ipq6018_pcie_pcs_tbl,
+ .pcs_num = ARRAY_SIZE(ipq6018_pcie_pcs_tbl),
+ .pcs_misc = ipq9574_gen3x2_pcie_pcs_misc_tbl,
+ .pcs_misc_num = ARRAY_SIZE(ipq9574_gen3x2_pcie_pcs_misc_tbl),
+ },
+ .reset_list = ipq8074_pciephy_reset_l,
+ .num_resets = ARRAY_SIZE(ipq8074_pciephy_reset_l),
+ .vreg_list = NULL,
+ .num_vregs = 0,
+ .regs = pciephy_v4_regs_layout,
+
+ .pwrdn_ctrl = SW_PWRDN | REFCLK_DRV_DSBL,
+ .phy_status = PHYSTATUS,
+};
+
static const struct qmp_phy_cfg sm8250_qmp_gen3x2_pciephy_cfg = {
.lanes = 2,
@@ -3935,6 +4066,9 @@ static const struct of_device_id qmp_pcie_of_match_table[] = {
}, {
.compatible = "qcom,ipq8074-qmp-pcie-phy",
.data = &ipq8074_pciephy_cfg,
+ }, {
+ .compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy",
+ .data = &ipq9574_pciephy_gen3x2_cfg,
}, {
.compatible = "qcom,msm8998-qmp-pcie-phy",
.data = &msm8998_pciephy_cfg,
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
index a469ae2a10a1..fa15a03055de 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
@@ -11,8 +11,22 @@
#define QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG2 0x0c
#define QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG4 0x14
#define QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_DRIVE 0x20
+#define QPHY_V5_PCS_PCIE_L1P1_WAKEUP_DLY_TIME_AUXCLK_L 0x44
+#define QPHY_V5_PCS_PCIE_L1P1_WAKEUP_DLY_TIME_AUXCLK_H 0x48
+#define QPHY_V5_PCS_PCIE_L1P2_WAKEUP_DLY_TIME_AUXCLK_L 0x4c
+#define QPHY_V5_PCS_PCIE_L1P2_WAKEUP_DLY_TIME_AUXCLK_H 0x50
#define QPHY_V5_PCS_PCIE_INT_AUX_CLK_CONFIG1 0x54
+#define QPHY_V5_PCS_PCIE_OSC_DTCT_CONFIG1 0x5c
+#define QPHY_V5_PCS_PCIE_OSC_DTCT_CONFIG2 0x60
+#define QPHY_V5_PCS_PCIE_OSC_DTCT_CONFIG4 0x68
+#define QPHY_V5_PCS_PCIE_OSC_DTCT_MODE2_CONFIG2 0x7c
+#define QPHY_V5_PCS_PCIE_OSC_DTCT_MODE2_CONFIG4 0x84
+#define QPHY_V5_PCS_PCIE_OSC_DTCT_MODE2_CONFIG5 0x88
+#define QPHY_V5_PCS_PCIE_OSC_DTCT_MODE2_CONFIG6 0x8c
#define QPHY_V5_PCS_PCIE_OSC_DTCT_ACTIONS 0x94
+#define QPHY_V5_PCS_PCIE_EQ_CONFIG1 0xa4
#define QPHY_V5_PCS_PCIE_EQ_CONFIG2 0xa8
+#define QPHY_V5_PCS_PCIE_PRESET_P10_PRE 0xc0
+#define QPHY_V5_PCS_PCIE_PRESET_P10_POST 0xe4
#endif
--
2.40.1
On ipq9574, there are 4 PCIe controllers. Describe the pcie2 node, and
its PHY in devicetree.
Only pcie2 is described, because only hardware using that controller
was available for testing.
Signed-off-by: Alexandru Gagniuc <[email protected]>
---
arch/arm64/boot/dts/qcom/ipq9574.dtsi | 93 ++++++++++++++++++++++++++-
1 file changed, 92 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 7f2e5cbf3bbb..f075e2715300 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -300,7 +300,7 @@ gcc: clock-controller@1800000 {
<0>,
<0>,
<0>,
- <0>,
+ <&pcie2_phy>,
<0>,
<0>;
#clock-cells = <1>;
@@ -745,6 +745,97 @@ frame@b128000 {
status = "disabled";
};
};
+
+ pcie2_phy: phy@8c000 {
+ compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
+ reg = <0x0008c000 0x14f4>;
+
+ clocks = <&gcc GCC_PCIE2_AUX_CLK>,
+ <&gcc GCC_PCIE2_AHB_CLK>,
+ <&gcc GCC_PCIE2_PIPE_CLK>,
+ <&gcc GCC_ANOC_PCIE2_2LANE_M_CLK>,
+ <&gcc GCC_SNOC_PCIE2_2LANE_S_CLK>;
+ clock-names = "aux",
+ "cfg_ahb",
+ "pipe",
+ "anoc",
+ "snoc";
+
+ clock-output-names = "pcie_phy2_pipe_clk";
+ #clock-cells = <0>;
+ #phy-cells = <0>;
+
+ resets = <&gcc GCC_PCIE2_PHY_BCR>,
+ <&gcc GCC_PCIE2PHY_PHY_BCR>;
+ reset-names = "phy",
+ "common";
+ status = "disabled";
+ };
+
+ pcie2: pcie@20000000 {
+ compatible = "qcom,pcie-ipq9574";
+ reg = <0x20000000 0xf1d>,
+ <0x20000f20 0xa8>,
+ <0x20001000 0x1000>,
+ <0x00088000 0x4000>,
+ <0x20100000 0x1000>;
+ reg-names = "dbi", "elbi", "atu", "parf", "config";
+
+ ranges = <0x81000000 0x0 0x20200000 0x20200000 0x0 0x00100000>, /* I/O */
+ <0x82000000 0x0 0x20300000 0x20300000 0x0 0x07d00000>; /* MEM */
+
+ device_type = "pci";
+ linux,pci-domain = <3>;
+ bus-range = <0x00 0xff>;
+ num-lanes = <2>;
+ max-link-speed = <3>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ phys = <&pcie2_phy>;
+ phy-names = "pciephy";
+
+ interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "msi";
+
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 0x7>;
+ interrupt-map = <0 0 0 1 &intc 0 0 164
+ IRQ_TYPE_LEVEL_HIGH>, /* int_a */
+ <0 0 0 2 &intc 0 0 165
+ IRQ_TYPE_LEVEL_HIGH>, /* int_b */
+ <0 0 0 3 &intc 0 0 186
+ IRQ_TYPE_LEVEL_HIGH>, /* int_c */
+ <0 0 0 4 &intc 0 0 187
+ IRQ_TYPE_LEVEL_HIGH>; /* int_d */
+
+ clocks = <&gcc GCC_PCIE2_AXI_M_CLK>,
+ <&gcc GCC_PCIE2_AXI_S_CLK>,
+ <&gcc GCC_PCIE2_AXI_S_BRIDGE_CLK>,
+ <&gcc GCC_PCIE2_RCHNG_CLK>;
+ clock-names = "axi_m",
+ "axi_s",
+ "axi_bridge",
+ "rchng";
+
+ resets = <&gcc GCC_PCIE2_PIPE_ARES>,
+ <&gcc GCC_PCIE2_AUX_ARES>,
+ <&gcc GCC_PCIE2_CORE_STICKY_ARES>,
+ <&gcc GCC_PCIE2_AXI_M_ARES>,
+ <&gcc GCC_PCIE2_AXI_S_ARES>,
+ <&gcc GCC_PCIE2_AXI_S_STICKY_ARES>,
+ <&gcc GCC_PCIE2_AXI_M_STICKY_ARES>,
+ <&gcc GCC_PCIE2_AHB_ARES>;
+ reset-names = "pipe",
+ "aux",
+ "sticky",
+ "axi_m",
+ "axi_s",
+ "axi_s_sticky",
+ "axi_m_sticky",
+ "ahb";
+ status = "disabled";
+ };
};
thermal-zones {
--
2.40.1
On Mon, 15 Apr 2024 at 21:21, Alexandru Gagniuc <[email protected]> wrote:
>
> The IPQ9574 has four PCIe "pipe" clocks. These clocks are required by
> PCIe PHYs. Port the pipe clocks from the downstream 5.4 kernel.
>
> Signed-off-by: Alexandru Gagniuc <[email protected]>
> ---
> drivers/clk/qcom/gcc-ipq9574.c | 76 ++++++++++++++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Mon, 15 Apr 2024 at 21:22, Alexandru Gagniuc <[email protected]> wrote:
>
> Add support for the PCIe on IPQ9574. The main difference from ipq6018
> is that the "iface" clock is not necessarry. Add a special case in
> qcom_pcie_get_resources_2_9_0() to handle this.
>
> Signed-off-by: Alexandru Gagniuc <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 14772edcf0d3..10560d6d6336 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1101,15 +1101,19 @@ 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;
> + int ret, num_clks = ARRAY_SIZE(res->clks) - 1;
>
> - res->clks[0].id = "iface";
> + res->clks[0].id = "rchng";
> 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 (!of_device_is_compatible(dev->of_node, "qcom,pcie-ipq9574")) {
> + res->clks[4].id = "iface";
> + num_clks++;
> + }
> +
> + ret = devm_clk_bulk_get(dev, num_clks, res->clks);
Just use devm_clk_bulk_get_optional() here.
> if (ret < 0)
> return ret;
>
> @@ -1664,6 +1668,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> { .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
> { .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
> { .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
> + { .compatible = "qcom,pcie-ipq9574", .data = &cfg_2_9_0 },
> { .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
> { .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> { .compatible = "qcom,pcie-sa8540p", .data = &cfg_sc8280xp },
> --
> 2.40.1
>
>
--
With best wishes
Dmitry
On 4/15/24 15:04, Dmitry Baryshkov wrote:
> On Mon, 15 Apr 2024 at 21:22, Alexandru Gagniuc <[email protected]> wrote:
>>
>> Add support for the PCIe on IPQ9574. The main difference from ipq6018
>> is that the "iface" clock is not necessarry. Add a special case in
>> qcom_pcie_get_resources_2_9_0() to handle this.
>>
>> Signed-off-by: Alexandru Gagniuc <[email protected]>
>> ---
>> drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 14772edcf0d3..10560d6d6336 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -1101,15 +1101,19 @@ 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;
>> + int ret, num_clks = ARRAY_SIZE(res->clks) - 1;
>>
>> - res->clks[0].id = "iface";
>> + res->clks[0].id = "rchng";
>> 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 (!of_device_is_compatible(dev->of_node, "qcom,pcie-ipq9574")) {
>> + res->clks[4].id = "iface";
>> + num_clks++;
>> + }
>> +
>> + ret = devm_clk_bulk_get(dev, num_clks, res->clks);
>
> Just use devm_clk_bulk_get_optional() here.
Thank you! I wasn't sure if this was the correct solution here. I will
get this updated in v4.
Alex
>> if (ret < 0)
>> return ret;
>>
>> @@ -1664,6 +1668,7 @@ static const struct of_device_id qcom_pcie_match[] = {
>> { .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
>> { .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
>> { .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
>> + { .compatible = "qcom,pcie-ipq9574", .data = &cfg_2_9_0 },
>> { .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
>> { .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
>> { .compatible = "qcom,pcie-sa8540p", .data = &cfg_sc8280xp },
>> --
>> 2.40.1
>>
>>
>
>
On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <[email protected]> wrote:
>
> Add support for the gen3x2 PCIe PHY on IPQ9574, ported form downstream
> 5.4 kernel. Only the serdes and pcs_misc tables are new, the others
> being reused from IPQ8074 and IPQ6018 PHYs.
>
> Signed-off-by: Alexandru Gagniuc <[email protected]>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
> .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
> 2 files changed, 149 insertions(+), 1 deletion(-)
>
[skipped]
> @@ -2448,7 +2542,7 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
>
> /* list of clocks required by phy */
> static const char * const qmp_pciephy_clk_l[] = {
> - "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
> + "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux", "anoc", "snoc"
Are the NoC clocks really necessary to drive the PHY? I think they are
usually connected to the controllers, not the PHYs.
> };
>
> /* list of regulators */
> @@ -2499,6 +2593,16 @@ static const struct qmp_pcie_offsets qmp_pcie_offsets_v4x1 = {
> .rx = 0x0400,
> };
>
> +static const struct qmp_pcie_offsets qmp_pcie_offsets_ipq9574 = {
> + .serdes = 0,
> + .pcs = 0x1000,
> + .pcs_misc = 0x1400,
> + .tx = 0x0200,
> + .rx = 0x0400,
> + .tx2 = 0x0600,
> + .rx2 = 0x0800,
> +};
> +
> static const struct qmp_pcie_offsets qmp_pcie_offsets_v4x2 = {
> .serdes = 0,
> .pcs = 0x0a00,
> @@ -2728,6 +2832,33 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x1_pciephy_cfg = {
> .phy_status = PHYSTATUS,
> };
>
> +static const struct qmp_phy_cfg ipq9574_pciephy_gen3x2_cfg = {
> + .lanes = 2,
> +
> + .offsets = &qmp_pcie_offsets_ipq9574,
> +
> + .tbls = {
> + .serdes = ipq9574_gen3x2_pcie_serdes_tbl,
> + .serdes_num = ARRAY_SIZE(ipq9574_gen3x2_pcie_serdes_tbl),
> + .tx = ipq8074_pcie_gen3_tx_tbl,
> + .tx_num = ARRAY_SIZE(ipq8074_pcie_gen3_tx_tbl),
> + .rx = ipq6018_pcie_rx_tbl,
> + .rx_num = ARRAY_SIZE(ipq6018_pcie_rx_tbl),
> + .pcs = ipq6018_pcie_pcs_tbl,
> + .pcs_num = ARRAY_SIZE(ipq6018_pcie_pcs_tbl),
> + .pcs_misc = ipq9574_gen3x2_pcie_pcs_misc_tbl,
> + .pcs_misc_num = ARRAY_SIZE(ipq9574_gen3x2_pcie_pcs_misc_tbl),
> + },
> + .reset_list = ipq8074_pciephy_reset_l,
> + .num_resets = ARRAY_SIZE(ipq8074_pciephy_reset_l),
> + .vreg_list = NULL,
> + .num_vregs = 0,
> + .regs = pciephy_v4_regs_layout,
So, is it v4 or v5?
> +
> + .pwrdn_ctrl = SW_PWRDN | REFCLK_DRV_DSBL,
> + .phy_status = PHYSTATUS,
> +};
> +
> static const struct qmp_phy_cfg sm8250_qmp_gen3x2_pciephy_cfg = {
> .lanes = 2,
>
--
With best wishes
Dmitry
On 4/15/24 15:10, Dmitry Baryshkov wrote:
> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <[email protected]> wrote:
>>
>> Add support for the gen3x2 PCIe PHY on IPQ9574, ported form downstream
>> 5.4 kernel. Only the serdes and pcs_misc tables are new, the others
>> being reused from IPQ8074 and IPQ6018 PHYs.
>>
>> Signed-off-by: Alexandru Gagniuc <[email protected]>
>> ---
>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
>> .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
>> 2 files changed, 149 insertions(+), 1 deletion(-)
>>
>
> [skipped]
>
>> @@ -2448,7 +2542,7 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
>>
>> /* list of clocks required by phy */
>> static const char * const qmp_pciephy_clk_l[] = {
>> - "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
>> + "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux", "anoc", "snoc"
>
> Are the NoC clocks really necessary to drive the PHY? I think they are
> usually connected to the controllers, not the PHYs.
The system will hang if these clocks are not enabled. They are also
attached to the PHY in the QCA 5.4 downstream kernel.
>> };
>>
>> /* list of regulators */
>> @@ -2499,6 +2593,16 @@ static const struct qmp_pcie_offsets qmp_pcie_offsets_v4x1 = {
>> .rx = 0x0400,
>> };
>>
>> +static const struct qmp_pcie_offsets qmp_pcie_offsets_ipq9574 = {
>> + .serdes = 0,
>> + .pcs = 0x1000,
>> + .pcs_misc = 0x1400,
>> + .tx = 0x0200,
>> + .rx = 0x0400,
>> + .tx2 = 0x0600,
>> + .rx2 = 0x0800,
>> +};
>> +
>> static const struct qmp_pcie_offsets qmp_pcie_offsets_v4x2 = {
>> .serdes = 0,
>> .pcs = 0x0a00,
>> @@ -2728,6 +2832,33 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x1_pciephy_cfg = {
>> .phy_status = PHYSTATUS,
>> };
>>
>> +static const struct qmp_phy_cfg ipq9574_pciephy_gen3x2_cfg = {
>> + .lanes = 2,
>> +
>> + .offsets = &qmp_pcie_offsets_ipq9574,
>> +
>> + .tbls = {
>> + .serdes = ipq9574_gen3x2_pcie_serdes_tbl,
>> + .serdes_num = ARRAY_SIZE(ipq9574_gen3x2_pcie_serdes_tbl),
>> + .tx = ipq8074_pcie_gen3_tx_tbl,
>> + .tx_num = ARRAY_SIZE(ipq8074_pcie_gen3_tx_tbl),
>> + .rx = ipq6018_pcie_rx_tbl,
>> + .rx_num = ARRAY_SIZE(ipq6018_pcie_rx_tbl),
>> + .pcs = ipq6018_pcie_pcs_tbl,
>> + .pcs_num = ARRAY_SIZE(ipq6018_pcie_pcs_tbl),
>> + .pcs_misc = ipq9574_gen3x2_pcie_pcs_misc_tbl,
>> + .pcs_misc_num = ARRAY_SIZE(ipq9574_gen3x2_pcie_pcs_misc_tbl),
>> + },
>> + .reset_list = ipq8074_pciephy_reset_l,
>> + .num_resets = ARRAY_SIZE(ipq8074_pciephy_reset_l),
>> + .vreg_list = NULL,
>> + .num_vregs = 0,
>> + .regs = pciephy_v4_regs_layout,
>
> So, is it v4 or v5?
Please give me a day or so to go over my notes and give you a more
coherent explanation of why this versioning was chosen. I am only
working from the QCA 5.4 downstream sources. I don't have any
documentation for the silicon
Alex
>
>> +
>> + .pwrdn_ctrl = SW_PWRDN | REFCLK_DRV_DSBL,
>> + .phy_status = PHYSTATUS,
>> +};
>> +
>> static const struct qmp_phy_cfg sm8250_qmp_gen3x2_pciephy_cfg = {
>> .lanes = 2,
>>
>
>
>
> --
> With best wishes
> Dmitry
Hi Dmitry,
On 4/15/24 16:25, [email protected] wrote:
>
>
> On 4/15/24 15:10, Dmitry Baryshkov wrote:
>> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <[email protected]>
>> wrote:
>>>
>>> Add support for the gen3x2 PCIe PHY on IPQ9574, ported form downstream
>>> 5.4 kernel. Only the serdes and pcs_misc tables are new, the others
>>> being reused from IPQ8074 and IPQ6018 PHYs.
>>>
>>> Signed-off-by: Alexandru Gagniuc <[email protected]>
>>> ---
>>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
>>> .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
>>> 2 files changed, 149 insertions(+), 1 deletion(-)
>>>
>>
>> [skipped]
>>
>>> @@ -2448,7 +2542,7 @@ static inline void qphy_clrbits(void __iomem
>>> *base, u32 offset, u32 val)
>>>
>>> /* list of clocks required by phy */
>>> static const char * const qmp_pciephy_clk_l[] = {
>>> - "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
>>> + "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
>>> "anoc", "snoc"
>>
>> Are the NoC clocks really necessary to drive the PHY? I think they are
>> usually connected to the controllers, not the PHYs.
>
> The system will hang if these clocks are not enabled. They are also
> attached to the PHY in the QCA 5.4 downstream kernel.
>
They are named "anoc_lane", and "snoc_lane" in the downstream kernel.
Would you like me to use these names instead?
e>>> };
>>>
>>> /* list of regulators */
>>> @@ -2499,6 +2593,16 @@ static const struct qmp_pcie_offsets
>>> qmp_pcie_offsets_v4x1 = {
>>> .rx = 0x0400,
>>> };
>>>
>>> +static const struct qmp_pcie_offsets qmp_pcie_offsets_ipq9574 = {
>>> + .serdes = 0,
>>> + .pcs = 0x1000,
>>> + .pcs_misc = 0x1400,
>>> + .tx = 0x0200,
>>> + .rx = 0x0400,
>>> + .tx2 = 0x0600,
>>> + .rx2 = 0x0800,
>>> +};
>>> +
>>> static const struct qmp_pcie_offsets qmp_pcie_offsets_v4x2 = {
>>> .serdes = 0,
>>> .pcs = 0x0a00,
>>> @@ -2728,6 +2832,33 @@ static const struct qmp_phy_cfg
>>> sm8250_qmp_gen3x1_pciephy_cfg = {
>>> .phy_status = PHYSTATUS,
>>> };
>>>
>>> +static const struct qmp_phy_cfg ipq9574_pciephy_gen3x2_cfg = {
>>> + .lanes = 2,
>>> +
>>> + .offsets = &qmp_pcie_offsets_ipq9574,
>>> +
>>> + .tbls = {
>>> + .serdes = ipq9574_gen3x2_pcie_serdes_tbl,
>>> + .serdes_num =
>>> ARRAY_SIZE(ipq9574_gen3x2_pcie_serdes_tbl),
>>> + .tx = ipq8074_pcie_gen3_tx_tbl,
>>> + .tx_num = ARRAY_SIZE(ipq8074_pcie_gen3_tx_tbl),
>>> + .rx = ipq6018_pcie_rx_tbl,
>>> + .rx_num = ARRAY_SIZE(ipq6018_pcie_rx_tbl),
>>> + .pcs = ipq6018_pcie_pcs_tbl,
>>> + .pcs_num = ARRAY_SIZE(ipq6018_pcie_pcs_tbl),
>>> + .pcs_misc = ipq9574_gen3x2_pcie_pcs_misc_tbl,
>>> + .pcs_misc_num =
>>> ARRAY_SIZE(ipq9574_gen3x2_pcie_pcs_misc_tbl),
>>> + },
>>> + .reset_list = ipq8074_pciephy_reset_l,
>>> + .num_resets = ARRAY_SIZE(ipq8074_pciephy_reset_l),
>>> + .vreg_list = NULL,
>>> + .num_vregs = 0,
>>> + .regs = pciephy_v4_regs_layout,
>>
>> So, is it v4 or v5?
>
> Please give me a day or so to go over my notes and give you a more
> coherent explanation of why this versioning was chosen. I am only
> working from the QCA 5.4 downstream sources. I don't have any
> documentation for the silicon
The downstream QCA kernel uses the same table for ipq6018, ipq8074-gen3,
and ipq9574. It is named "ipq_pciephy_gen3_regs_layout". Thus, it made
sense to use the same upstream table for ipq9574, "pciephy_v4_regs_layout".
As far as the register tables go, the pcs/pcs_misc are squashed into the
same table in the downstream 5.4 kernel. I was able to separate the two
tables because the pcs_misc registers were defined with an offset of
0x400. For example:
/* QMP V2 PHY for PCIE gen3 2 Lane ports - PCS Misc registers */
#define PCS_PCIE_X2_POWER_STATE_CONFIG2 0x40c
#define PCS_PCIE_X2_POWER_STATE_CONFIG4 0x414
#define PCS_PCIE_X2_ENDPOINT_REFCLK_DRIVE 0x420
#define PCS_PCIE_X2_L1P1_WAKEUP_DLY_TIME_AUXCLK_L 0x444
#define PCS_PCIE_X2_L1P1_WAKEUP_DLY_TIME_AUXCLK_H 0x448
#define PCS_PCIE_X2_L1P2_WAKEUP_DLY_TIME_AUXCLK_L 0x44c
#define PCS_PCIE_X2_L1P2_WAKEUP_DLY_TIME_AUXCLK_H 0x450
..
Here, QPHY_V4_PCS_PCIE_POWER_STATE_CONFIG2 = 0xc would be correct,
assuming a pcs_misc offset of 0x400. However, starting with
ENDPOINT_REFCLK_DRIVE, the register would be
QPHY_V4_PCS_PCIE_ENDPOINT_REFCLK_DRIVE = 0x1c. Our offsets are off-by 0x4.
The existing V5 offsets, on the other hand, were all correct. For this
reason, I considered that V5 is the most likely place to add the missing
PCS misc definitions.
Is this explanation sufficiently convincing? Where does the v4/v5 scheme
in upstream kernel originate?
Alex
On Wed, 17 Apr 2024 at 00:25, Alex G. <[email protected]> wrote:
>
> Hi Dmitry,
>
> On 4/15/24 16:25, [email protected] wrote:
> >
> >
> > On 4/15/24 15:10, Dmitry Baryshkov wrote:
> >> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <[email protected]>
> >> wrote:
> >>>
> >>> Add support for the gen3x2 PCIe PHY on IPQ9574, ported form downstream
> >>> 5.4 kernel. Only the serdes and pcs_misc tables are new, the others
> >>> being reused from IPQ8074 and IPQ6018 PHYs.
> >>>
> >>> Signed-off-by: Alexandru Gagniuc <[email protected]>
> >>> ---
> >>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
> >>> .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
> >>> 2 files changed, 149 insertions(+), 1 deletion(-)
> >>>
> >>
> >> [skipped]
> >>
> >>> @@ -2448,7 +2542,7 @@ static inline void qphy_clrbits(void __iomem
> >>> *base, u32 offset, u32 val)
> >>>
> >>> /* list of clocks required by phy */
> >>> static const char * const qmp_pciephy_clk_l[] = {
> >>> - "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
> >>> + "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
> >>> "anoc", "snoc"
> >>
> >> Are the NoC clocks really necessary to drive the PHY? I think they are
> >> usually connected to the controllers, not the PHYs.
> >
> > The system will hang if these clocks are not enabled. They are also
> > attached to the PHY in the QCA 5.4 downstream kernel.
Interesting.
I see that Varadarajan is converting these clocks into interconnects.
Maybe it's better to wait for those patches to land and use
interconnects instead. I think it would better suit the
infrastructure.
Varadarajan, could you please comment, are these interconnects
connected to the PHY too or just to the PCIe controller?
> >
> They are named "anoc_lane", and "snoc_lane" in the downstream kernel.
> Would you like me to use these names instead?
I'm fine either way.
> e>>> };
> >>>
> >>> /* list of regulators */
> >>> @@ -2499,6 +2593,16 @@ static const struct qmp_pcie_offsets
> >>> qmp_pcie_offsets_v4x1 = {
> >>> .rx = 0x0400,
> >>> };
> >>>
> >>> +static const struct qmp_pcie_offsets qmp_pcie_offsets_ipq9574 = {
> >>> + .serdes = 0,
> >>> + .pcs = 0x1000,
> >>> + .pcs_misc = 0x1400,
> >>> + .tx = 0x0200,
> >>> + .rx = 0x0400,
> >>> + .tx2 = 0x0600,
> >>> + .rx2 = 0x0800,
> >>> +};
> >>> +
> >>> static const struct qmp_pcie_offsets qmp_pcie_offsets_v4x2 = {
> >>> .serdes = 0,
> >>> .pcs = 0x0a00,
> >>> @@ -2728,6 +2832,33 @@ static const struct qmp_phy_cfg
> >>> sm8250_qmp_gen3x1_pciephy_cfg = {
> >>> .phy_status = PHYSTATUS,
> >>> };
> >>>
> >>> +static const struct qmp_phy_cfg ipq9574_pciephy_gen3x2_cfg = {
> >>> + .lanes = 2,
> >>> +
> >>> + .offsets = &qmp_pcie_offsets_ipq9574,
> >>> +
> >>> + .tbls = {
> >>> + .serdes = ipq9574_gen3x2_pcie_serdes_tbl,
> >>> + .serdes_num =
> >>> ARRAY_SIZE(ipq9574_gen3x2_pcie_serdes_tbl),
> >>> + .tx = ipq8074_pcie_gen3_tx_tbl,
> >>> + .tx_num = ARRAY_SIZE(ipq8074_pcie_gen3_tx_tbl),
> >>> + .rx = ipq6018_pcie_rx_tbl,
> >>> + .rx_num = ARRAY_SIZE(ipq6018_pcie_rx_tbl),
> >>> + .pcs = ipq6018_pcie_pcs_tbl,
> >>> + .pcs_num = ARRAY_SIZE(ipq6018_pcie_pcs_tbl),
> >>> + .pcs_misc = ipq9574_gen3x2_pcie_pcs_misc_tbl,
> >>> + .pcs_misc_num =
> >>> ARRAY_SIZE(ipq9574_gen3x2_pcie_pcs_misc_tbl),
> >>> + },
> >>> + .reset_list = ipq8074_pciephy_reset_l,
> >>> + .num_resets = ARRAY_SIZE(ipq8074_pciephy_reset_l),
> >>> + .vreg_list = NULL,
> >>> + .num_vregs = 0,
> >>> + .regs = pciephy_v4_regs_layout,
> >>
> >> So, is it v4 or v5?
> >
> > Please give me a day or so to go over my notes and give you a more
> > coherent explanation of why this versioning was chosen. I am only
> > working from the QCA 5.4 downstream sources. I don't have any
> > documentation for the silicon
>
> The downstream QCA kernel uses the same table for ipq6018, ipq8074-gen3,
> and ipq9574. It is named "ipq_pciephy_gen3_regs_layout". Thus, it made
> sense to use the same upstream table for ipq9574, "pciephy_v4_regs_layout".
>
> As far as the register tables go, the pcs/pcs_misc are squashed into the
> same table in the downstream 5.4 kernel. I was able to separate the two
> tables because the pcs_misc registers were defined with an offset of
> 0x400. For example:
>
> /* QMP V2 PHY for PCIE gen3 2 Lane ports - PCS Misc registers */
> #define PCS_PCIE_X2_POWER_STATE_CONFIG2 0x40c
> #define PCS_PCIE_X2_POWER_STATE_CONFIG4 0x414
> #define PCS_PCIE_X2_ENDPOINT_REFCLK_DRIVE 0x420
> #define PCS_PCIE_X2_L1P1_WAKEUP_DLY_TIME_AUXCLK_L 0x444
> #define PCS_PCIE_X2_L1P1_WAKEUP_DLY_TIME_AUXCLK_H 0x448
> #define PCS_PCIE_X2_L1P2_WAKEUP_DLY_TIME_AUXCLK_L 0x44c
> #define PCS_PCIE_X2_L1P2_WAKEUP_DLY_TIME_AUXCLK_H 0x450
> ...
>
> Here, QPHY_V4_PCS_PCIE_POWER_STATE_CONFIG2 = 0xc would be correct,
> assuming a pcs_misc offset of 0x400. However, starting with
> ENDPOINT_REFCLK_DRIVE, the register would be
> QPHY_V4_PCS_PCIE_ENDPOINT_REFCLK_DRIVE = 0x1c. Our offsets are off-by 0x4.
>
> The existing V5 offsets, on the other hand, were all correct. For this
> reason, I considered that V5 is the most likely place to add the missing
> PCS misc definitions.
Ok, sounds sane. Please use _v5 for the regs layout.
>
> Is this explanation sufficiently convincing? Where does the v4/v5 scheme
> in upstream kernel originate?
Sometimes it's vendor kernels, sometimes it's a feedback from devs
that have access to actual specs.
--
With best wishes
Dmitry
On Mon, Apr 15, 2024 at 03:07:02PM -0500, [email protected] wrote:
>
>
> On 4/15/24 15:04, Dmitry Baryshkov wrote:
> > On Mon, 15 Apr 2024 at 21:22, Alexandru Gagniuc <[email protected]> wrote:
> > >
> > > Add support for the PCIe on IPQ9574. The main difference from ipq6018
> > > is that the "iface" clock is not necessarry. Add a special case in
> > > qcom_pcie_get_resources_2_9_0() to handle this.
> > >
> > > Signed-off-by: Alexandru Gagniuc <[email protected]>
> > > ---
> > > drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++----
> > > 1 file changed, 9 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index 14772edcf0d3..10560d6d6336 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -1101,15 +1101,19 @@ 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;
> > > + int ret, num_clks = ARRAY_SIZE(res->clks) - 1;
> > >
> > > - res->clks[0].id = "iface";
> > > + res->clks[0].id = "rchng";
> > > 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 (!of_device_is_compatible(dev->of_node, "qcom,pcie-ipq9574")) {
> > > + res->clks[4].id = "iface";
> > > + num_clks++;
> > > + }
> > > +
> > > + ret = devm_clk_bulk_get(dev, num_clks, res->clks);
> >
> > Just use devm_clk_bulk_get_optional() here.
>
> Thank you! I wasn't sure if this was the correct solution here. I will get
> this updated in v4.
>
Please rebase on top of [1] and mention the dependency in cover letter.
- Mani
[1] https://lore.kernel.org/linux-pci/[email protected]/
> Alex
>
> > > if (ret < 0)
> > > return ret;
> > >
> > > @@ -1664,6 +1668,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> > > { .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
> > > { .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
> > > { .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
> > > + { .compatible = "qcom,pcie-ipq9574", .data = &cfg_2_9_0 },
> > > { .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
> > > { .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> > > { .compatible = "qcom,pcie-sa8540p", .data = &cfg_sc8280xp },
> > > --
> > > 2.40.1
> > >
> > >
> >
> >
--
மணிவண்ணன் சதாசிவம்
On Mon, Apr 15, 2024 at 01:20:49PM -0500, Alexandru Gagniuc wrote:
> Add support for the PCIe on IPQ9574. The main difference from ipq6018
> is that the "iface" clock is not necessarry. Add a special case in
> qcom_pcie_get_resources_2_9_0() to handle this.
>
Could you add more information about the PCIe controller used in this SoC? Like
controller version, supported data rate, PCIe generation etc...
- Mani
> Signed-off-by: Alexandru Gagniuc <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 14772edcf0d3..10560d6d6336 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1101,15 +1101,19 @@ 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;
> + int ret, num_clks = ARRAY_SIZE(res->clks) - 1;
>
> - res->clks[0].id = "iface";
> + res->clks[0].id = "rchng";
> 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 (!of_device_is_compatible(dev->of_node, "qcom,pcie-ipq9574")) {
> + res->clks[4].id = "iface";
> + num_clks++;
> + }
> +
> + ret = devm_clk_bulk_get(dev, num_clks, res->clks);
> if (ret < 0)
> return ret;
>
> @@ -1664,6 +1668,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> { .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
> { .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
> { .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
> + { .compatible = "qcom,pcie-ipq9574", .data = &cfg_2_9_0 },
> { .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
> { .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> { .compatible = "qcom,pcie-sa8540p", .data = &cfg_sc8280xp },
> --
> 2.40.1
>
--
மணிவண்ணன் சதாசிவம்
On Mon, Apr 15, 2024 at 01:20:52PM -0500, Alexandru Gagniuc wrote:
> On ipq9574, there are 4 PCIe controllers. Describe the pcie2 node, and
> its PHY in devicetree.
>
> Only pcie2 is described, because only hardware using that controller
> was available for testing.
>
You should describe all the instances in DT. Since the unused ones will be
'disabled', it won't affect anyone.
> Signed-off-by: Alexandru Gagniuc <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 93 ++++++++++++++++++++++++++-
> 1 file changed, 92 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> index 7f2e5cbf3bbb..f075e2715300 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> @@ -300,7 +300,7 @@ gcc: clock-controller@1800000 {
> <0>,
> <0>,
> <0>,
> - <0>,
> + <&pcie2_phy>,
> <0>,
> <0>;
> #clock-cells = <1>;
> @@ -745,6 +745,97 @@ frame@b128000 {
> status = "disabled";
> };
> };
> +
> + pcie2_phy: phy@8c000 {
> + compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
> + reg = <0x0008c000 0x14f4>;
> +
> + clocks = <&gcc GCC_PCIE2_AUX_CLK>,
> + <&gcc GCC_PCIE2_AHB_CLK>,
> + <&gcc GCC_PCIE2_PIPE_CLK>,
> + <&gcc GCC_ANOC_PCIE2_2LANE_M_CLK>,
> + <&gcc GCC_SNOC_PCIE2_2LANE_S_CLK>;
> + clock-names = "aux",
> + "cfg_ahb",
> + "pipe",
> + "anoc",
> + "snoc";
> +
> + clock-output-names = "pcie_phy2_pipe_clk";
> + #clock-cells = <0>;
> + #phy-cells = <0>;
> +
> + resets = <&gcc GCC_PCIE2_PHY_BCR>,
> + <&gcc GCC_PCIE2PHY_PHY_BCR>;
> + reset-names = "phy",
> + "common";
> + status = "disabled";
> + };
> +
> + pcie2: pcie@20000000 {
> + compatible = "qcom,pcie-ipq9574";
> + reg = <0x20000000 0xf1d>,
> + <0x20000f20 0xa8>,
> + <0x20001000 0x1000>,
> + <0x00088000 0x4000>,
> + <0x20100000 0x1000>;
> + reg-names = "dbi", "elbi", "atu", "parf", "config";
> +
> + ranges = <0x81000000 0x0 0x20200000 0x20200000 0x0 0x00100000>, /* I/O */
Please use below range:
<0x01000000 0x0 0x00000000 0x20200000 0x0 0x00100000>
<0x02000000 0x0 0x20300000 0x20300000 0x0 0x07d00000>
> + <0x82000000 0x0 0x20300000 0x20300000 0x0 0x07d00000>; /* MEM */
> +
> + device_type = "pci";
> + linux,pci-domain = <3>;
> + bus-range = <0x00 0xff>;
> + num-lanes = <2>;
> + max-link-speed = <3>;
> + #address-cells = <3>;
> + #size-cells = <2>;
> +
> + phys = <&pcie2_phy>;
> + phy-names = "pciephy";
> +
> + interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "msi";
> +
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 0x7>;
> + interrupt-map = <0 0 0 1 &intc 0 0 164
> + IRQ_TYPE_LEVEL_HIGH>, /* int_a */
> + <0 0 0 2 &intc 0 0 165
> + IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> + <0 0 0 3 &intc 0 0 186
> + IRQ_TYPE_LEVEL_HIGH>, /* int_c */
> + <0 0 0 4 &intc 0 0 187
> + IRQ_TYPE_LEVEL_HIGH>; /* int_d */
Use a single line for each INTX entry even if it exceeds 80 column width.
- Mani
--
மணிவண்ணன் சதாசிவம்
On Mon, 15 Apr 2024 13:20:48 -0500, Alexandru Gagniuc wrote:
> IPQ9574 has PCIe controllers which are almost identical to IPQ6018.
> The difference is that the "iface" clock is not required, and the
> "sleep" reset is replaced by an "aux" reset. Document these
> differences along with the compatible string.
>
> Signed-off-by: Alexandru Gagniuc <[email protected]>
> ---
> .../devicetree/bindings/pci/qcom,pcie.yaml | 35 +++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
Acked-by: Rob Herring (Arm) <[email protected]>
On Mon, 15 Apr 2024 13:20:50 -0500, Alexandru Gagniuc wrote:
> The IPQ9574 gen3x2 PHY is very similar to IPQ6018. It requires two
> extra clocks named "anoc" and "snoc". Document this, and add a
> new compatible string for this PHY.
>
> Signed-off-by: Alexandru Gagniuc <[email protected]>
> ---
> .../phy/qcom,ipq8074-qmp-pcie-phy.yaml | 36 ++++++++++++++++++-
> 1 file changed, 35 insertions(+), 1 deletion(-)
>
Acked-by: Rob Herring (Arm) <[email protected]>
On 4/17/24 02:34, Manivannan Sadhasivam wrote:
> On Mon, Apr 15, 2024 at 01:20:52PM -0500, Alexandru Gagniuc wrote:
>> On ipq9574, there are 4 PCIe controllers. Describe the pcie2 node, and
>> its PHY in devicetree.
>>
>> Only pcie2 is described, because only hardware using that controller
>> was available for testing.
>>
>
> You should describe all the instances in DT. Since the unused ones will be
> 'disabled', it won't affect anyone.
My concern with untested but "disabled" descriptions is that someone may
think it's supported, try to enable it on their board, and run into
issues. Theoretically, we could describe pcie3, as it uses the same
gen3x2 phy.
The pcie0 and pcie1 use a gen3x1 phy, which I do not support in this
series. I would have to leave the "phys" and "phy-names" for these
nodes, leading to an incomplete description
Given this info, do you still wish that I add all other pcie nodes?
>> Signed-off-by: Alexandru Gagniuc <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 93 ++++++++++++++++++++++++++-
>> 1 file changed, 92 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> index 7f2e5cbf3bbb..f075e2715300 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> @@ -300,7 +300,7 @@ gcc: clock-controller@1800000 {
>> <0>,
>> <0>,
>> <0>,
>> - <0>,
>> + <&pcie2_phy>,
>> <0>,
>> <0>;
>> #clock-cells = <1>;
>> @@ -745,6 +745,97 @@ frame@b128000 {
>> status = "disabled";
>> };
>> };
>> +
>> + pcie2_phy: phy@8c000 {
>> + compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
>> + reg = <0x0008c000 0x14f4>;
>> +
>> + clocks = <&gcc GCC_PCIE2_AUX_CLK>,
>> + <&gcc GCC_PCIE2_AHB_CLK>,
>> + <&gcc GCC_PCIE2_PIPE_CLK>,
>> + <&gcc GCC_ANOC_PCIE2_2LANE_M_CLK>,
>> + <&gcc GCC_SNOC_PCIE2_2LANE_S_CLK>;
>> + clock-names = "aux",
>> + "cfg_ahb",
>> + "pipe",
>> + "anoc",
>> + "snoc";
>> +
>> + clock-output-names = "pcie_phy2_pipe_clk";
>> + #clock-cells = <0>;
>> + #phy-cells = <0>;
>> +
>> + resets = <&gcc GCC_PCIE2_PHY_BCR>,
>> + <&gcc GCC_PCIE2PHY_PHY_BCR>;
>> + reset-names = "phy",
>> + "common";
>> + status = "disabled";
>> + };
>> +
>> + pcie2: pcie@20000000 {
>> + compatible = "qcom,pcie-ipq9574";
>> + reg = <0x20000000 0xf1d>,
>> + <0x20000f20 0xa8>,
>> + <0x20001000 0x1000>,
>> + <0x00088000 0x4000>,
>> + <0x20100000 0x1000>;
>> + reg-names = "dbi", "elbi", "atu", "parf", "config";
>> +
>> + ranges = <0x81000000 0x0 0x20200000 0x20200000 0x0 0x00100000>, /* I/O */
>
> Please use below range:
>
> <0x01000000 0x0 0x00000000 0x20200000 0x0 0x00100000>
> <0x02000000 0x0 0x20300000 0x20300000 0x0 0x07d00000>
>
Of course, thank you.
>> + <0x82000000 0x0 0x20300000 0x20300000 0x0 0x07d00000>; /* MEM */
>> +
>> + device_type = "pci";
>> + linux,pci-domain = <3>;
>> + bus-range = <0x00 0xff>;
>> + num-lanes = <2>;
>> + max-link-speed = <3>;
>> + #address-cells = <3>;
>> + #size-cells = <2>;
>> +
>> + phys = <&pcie2_phy>;
>> + phy-names = "pciephy";
>> +
>> + interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "msi";
>> +
>> + #interrupt-cells = <1>;
>> + interrupt-map-mask = <0 0 0 0x7>;
>> + interrupt-map = <0 0 0 1 &intc 0 0 164
>> + IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>> + <0 0 0 2 &intc 0 0 165
>> + IRQ_TYPE_LEVEL_HIGH>, /* int_b */
>> + <0 0 0 3 &intc 0 0 186
>> + IRQ_TYPE_LEVEL_HIGH>, /* int_c */
>> + <0 0 0 4 &intc 0 0 187
>> + IRQ_TYPE_LEVEL_HIGH>; /* int_d */
>
> Use a single line for each INTX entry even if it exceeds 80 column width.
Yes. Will do.
> - Mani
>
On 4/15/2024 11:50 PM, Alexandru Gagniuc wrote:
> There are four PCIe ports on IPQ9574, pcie0 thru pcie3. This series
> addresses pcie2, which is a gen3x2 port. The board I have only uses
> pcie2, and that's the only one enabled in this series.
>
> I believe this makes sense as a monolithic series, as the individual
> pieces are not that useful by themselves.
>
> In v2, I've had some issues regarding the dt schema checks. For
> transparency, I used the following test invocations to test v3:
>
> make dt_binding_check DT_SCHEMA_FILES=qcom,pcie.yaml:qcom,ipq8074-qmp-pcie-phy.yaml
> make dtbs_check DT_SCHEMA_FILES=qcom,pcie.yaml:qcom,ipq8074-qmp-pcie-phy.yaml
>
>
Alexandru,
Thanks for your contributions to the Qualcomm IPQ chipsets.
I would like to inform you that we have also submitted the patches to
enable the PCIe support on IPQ9574[1][2] and waiting for the ICC
support[3] to land to enable the NOC clocks.
[1]
https://lore.kernel.org/linux-arm-msm/[email protected]/
[2]
https://lore.kernel.org/linux-arm-msm/[email protected]/
[3]
https://lore.kernel.org/linux-arm-msm/[email protected]/
Please take a look at these patches as well.
Thanks,
Kathiravan T.
> Changes since v2:
> - reworked resets in qcom,pcie.yaml to resolve dt schema errors
> - constrained "reg" in qcom,pcie.yaml
> - reworked min/max intems in qcom,ipq8074-qmp-pcie-phy.yaml
> - dropped msi-parent for pcie node, as it is handled by "msi" IRQ
>
> Changes since v1:
> - updated new tables in phy-qcom-qmp-pcie.c to use lowercase hex numbers
> - reorganized qcom,ipq8074-qmp-pcie-phy.yaml to use a single list of clocks
> - reorganized qcom,pcie.yaml to include clocks+resets per compatible
> - Renamed "pcie2_qmp_phy" label to "pcie2_phy"
> - moved "ranges" property of pcie@20000000 higher up
>
> Alexandru Gagniuc (7):
> dt-bindings: clock: Add PCIe pipe related clocks for IPQ9574
> clk: qcom: gcc-ipq9574: Add PCIe pipe clocks
> dt-bindings: PCI: qcom: Add IPQ9574 PCIe controller
> PCI: qcom: Add support for IPQ9574
> dt-bindings: phy: qcom,ipq8074-qmp-pcie: add ipq9574 gen3x2 PHY
> phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY
> arm64: dts: qcom: ipq9574: add PCIe2 nodes
>
> .../devicetree/bindings/pci/qcom,pcie.yaml | 35 +++++
> .../phy/qcom,ipq8074-qmp-pcie-phy.yaml | 36 ++++-
> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 93 +++++++++++-
> drivers/clk/qcom/gcc-ipq9574.c | 76 ++++++++++
> drivers/pci/controller/dwc/pcie-qcom.c | 13 +-
> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
> .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
> include/dt-bindings/clock/qcom,ipq9574-gcc.h | 4 +
> 8 files changed, 400 insertions(+), 7 deletions(-)
>
Hi Mani.
On 4/17/24 02:06, Manivannan Sadhasivam wrote:
> On Mon, Apr 15, 2024 at 03:07:02PM -0500, [email protected] wrote:
>>
>>
>> On 4/15/24 15:04, Dmitry Baryshkov wrote:
>>> On Mon, 15 Apr 2024 at 21:22, Alexandru Gagniuc <[email protected]> wrote:
>>>>
>>>> Add support for the PCIe on IPQ9574. The main difference from ipq6018
>>>> is that the "iface" clock is not necessarry. Add a special case in
>>>> qcom_pcie_get_resources_2_9_0() to handle this.
>>>>
>>>> Signed-off-by: Alexandru Gagniuc <[email protected]>
>>>> ---
>>>> drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++----
>>>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>>> index 14772edcf0d3..10560d6d6336 100644
>>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>>> @@ -1101,15 +1101,19 @@ 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;
>>>> + int ret, num_clks = ARRAY_SIZE(res->clks) - 1;
>>>>
>>>> - res->clks[0].id = "iface";
>>>> + res->clks[0].id = "rchng";
>>>> 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 (!of_device_is_compatible(dev->of_node, "qcom,pcie-ipq9574")) {
>>>> + res->clks[4].id = "iface";
>>>> + num_clks++;
>>>> + }
>>>> +
>>>> + ret = devm_clk_bulk_get(dev, num_clks, res->clks);
>>>
>>> Just use devm_clk_bulk_get_optional() here.
>>
>> Thank you! I wasn't sure if this was the correct solution here. I will get
>> this updated in v4.
>>
>
> Please rebase on top of [1] and mention the dependency in cover letter.
I am very hesitant to depend on another patch series. Is it okay if I
include your patch in v4 of this series?
Alex
Hi Kathiravan,
On 4/19/24 09:28, Kathiravan Thirumoorthy wrote:
>
>
> On 4/15/2024 11:50 PM, Alexandru Gagniuc wrote:
>> There are four PCIe ports on IPQ9574, pcie0 thru pcie3. This series
>> addresses pcie2, which is a gen3x2 port. The board I have only uses
>> pcie2, and that's the only one enabled in this series.
>>
>> I believe this makes sense as a monolithic series, as the individual
>> pieces are not that useful by themselves.
>>
>> In v2, I've had some issues regarding the dt schema checks. For
>> transparency, I used the following test invocations to test v3:
>>
>> make dt_binding_check
>> DT_SCHEMA_FILES=qcom,pcie.yaml:qcom,ipq8074-qmp-pcie-phy.yaml
>> make dtbs_check
>> DT_SCHEMA_FILES=qcom,pcie.yaml:qcom,ipq8074-qmp-pcie-phy.yaml
>>
>>
>
> Alexandru,
>
> Thanks for your contributions to the Qualcomm IPQ chipsets.
>
> I would like to inform you that we have also submitted the patches to
> enable the PCIe support on IPQ9574[1][2] and waiting for the ICC
> support[3] to land to enable the NOC clocks.
>
> [1]
> https://lore.kernel.org/linux-arm-msm/[email protected]/
> [2]
> https://lore.kernel.org/linux-arm-msm/[email protected]/
> [3]
> https://lore.kernel.org/linux-arm-msm/[email protected]/
>
> Please take a look at these patches as well.
I think I've seen [1] before -- I thought the series was abandoned.
Since we have the dt-schema and applicability on mainline resolved here,
do you want to use this series as the base for any new PCIe work?
Alex
> Thanks,
> Kathiravan T.
>
>
>> Changes since v2:
>> - reworked resets in qcom,pcie.yaml to resolve dt schema errors
>> - constrained "reg" in qcom,pcie.yaml
>> - reworked min/max intems in qcom,ipq8074-qmp-pcie-phy.yaml
>> - dropped msi-parent for pcie node, as it is handled by "msi" IRQ
>>
>> Changes since v1:
>> - updated new tables in phy-qcom-qmp-pcie.c to use lowercase hex
>> numbers
>> - reorganized qcom,ipq8074-qmp-pcie-phy.yaml to use a single list of
>> clocks
>> - reorganized qcom,pcie.yaml to include clocks+resets per compatible
>> - Renamed "pcie2_qmp_phy" label to "pcie2_phy"
>> - moved "ranges" property of pcie@20000000 higher up
>>
>> Alexandru Gagniuc (7):
>> dt-bindings: clock: Add PCIe pipe related clocks for IPQ9574
>> clk: qcom: gcc-ipq9574: Add PCIe pipe clocks
>> dt-bindings: PCI: qcom: Add IPQ9574 PCIe controller
>> PCI: qcom: Add support for IPQ9574
>> dt-bindings: phy: qcom,ipq8074-qmp-pcie: add ipq9574 gen3x2 PHY
>> phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY
>> arm64: dts: qcom: ipq9574: add PCIe2 nodes
>>
>> .../devicetree/bindings/pci/qcom,pcie.yaml | 35 +++++
>> .../phy/qcom,ipq8074-qmp-pcie-phy.yaml | 36 ++++-
>> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 93 +++++++++++-
>> drivers/clk/qcom/gcc-ipq9574.c | 76 ++++++++++
>> drivers/pci/controller/dwc/pcie-qcom.c | 13 +-
>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
>> .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
>> include/dt-bindings/clock/qcom,ipq9574-gcc.h | 4 +
>> 8 files changed, 400 insertions(+), 7 deletions(-)
>>
Quoting Alexandru Gagniuc (2024-04-15 11:20:47)
> diff --git a/drivers/clk/qcom/gcc-ipq9574.c b/drivers/clk/qcom/gcc-ipq9574.c
> index 0a3f846695b8..c748d2f124f3 100644
> --- a/drivers/clk/qcom/gcc-ipq9574.c
> +++ b/drivers/clk/qcom/gcc-ipq9574.c
> @@ -1569,6 +1569,24 @@ static struct clk_regmap_phy_mux pcie0_pipe_clk_src = {
> },
> };
>
> +static struct clk_branch gcc_pcie0_pipe_clk = {
> + .halt_reg = 0x28044,
> + .halt_check = BRANCH_HALT_DELAY,
> + .clkr = {
> + .enable_reg = 0x28044,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "gcc_pcie0_pipe_clk",
> + .parent_hws = (const struct clk_hw *[]) {
> + &pcie0_pipe_clk_src.clkr.hw
> + },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + .ops = &clk_branch2_ops,
> + },
> + },
> +};
> +
> static struct clk_regmap_phy_mux pcie1_pipe_clk_src = {
> .reg = 0x29064,
> .clkr = {
> @@ -1583,6 +1601,24 @@ static struct clk_regmap_phy_mux pcie1_pipe_clk_src = {
> },
> };
>
> +static struct clk_branch gcc_pcie1_pipe_clk = {
> + .halt_reg = 0x29044,
> + .halt_check = BRANCH_HALT_DELAY,
> + .clkr = {
> + .enable_reg = 0x29044,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
const clk_init_data please.
On Thu, Apr 18, 2024 at 10:33:23AM -0500, [email protected] wrote:
>
>
> On 4/17/24 02:34, Manivannan Sadhasivam wrote:
> > On Mon, Apr 15, 2024 at 01:20:52PM -0500, Alexandru Gagniuc wrote:
> > > On ipq9574, there are 4 PCIe controllers. Describe the pcie2 node, and
> > > its PHY in devicetree.
> > >
> > > Only pcie2 is described, because only hardware using that controller
> > > was available for testing.
> > >
> >
> > You should describe all the instances in DT. Since the unused ones will be
> > 'disabled', it won't affect anyone.
>
> My concern with untested but "disabled" descriptions is that someone may
> think it's supported, try to enable it on their board, and run into issues.
> Theoretically, we could describe pcie3, as it uses the same gen3x2 phy.
>
Okay.
> The pcie0 and pcie1 use a gen3x1 phy, which I do not support in this series.
> I would have to leave the "phys" and "phy-names" for these nodes, leading to
> an incomplete description
>
Fine then. Please describe at least pcie3. Also add a TODO above the first pcie
node mentioning that someone need to populate others too.
- Mani
> Given this info, do you still wish that I add all other pcie nodes?
>
> > > Signed-off-by: Alexandru Gagniuc <[email protected]>
> > > ---
> > > arch/arm64/boot/dts/qcom/ipq9574.dtsi | 93 ++++++++++++++++++++++++++-
> > > 1 file changed, 92 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> > > index 7f2e5cbf3bbb..f075e2715300 100644
> > > --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> > > @@ -300,7 +300,7 @@ gcc: clock-controller@1800000 {
> > > <0>,
> > > <0>,
> > > <0>,
> > > - <0>,
> > > + <&pcie2_phy>,
> > > <0>,
> > > <0>;
> > > #clock-cells = <1>;
> > > @@ -745,6 +745,97 @@ frame@b128000 {
> > > status = "disabled";
> > > };
> > > };
> > > +
> > > + pcie2_phy: phy@8c000 {
> > > + compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
> > > + reg = <0x0008c000 0x14f4>;
> > > +
> > > + clocks = <&gcc GCC_PCIE2_AUX_CLK>,
> > > + <&gcc GCC_PCIE2_AHB_CLK>,
> > > + <&gcc GCC_PCIE2_PIPE_CLK>,
> > > + <&gcc GCC_ANOC_PCIE2_2LANE_M_CLK>,
> > > + <&gcc GCC_SNOC_PCIE2_2LANE_S_CLK>;
> > > + clock-names = "aux",
> > > + "cfg_ahb",
> > > + "pipe",
> > > + "anoc",
> > > + "snoc";
> > > +
> > > + clock-output-names = "pcie_phy2_pipe_clk";
> > > + #clock-cells = <0>;
> > > + #phy-cells = <0>;
> > > +
> > > + resets = <&gcc GCC_PCIE2_PHY_BCR>,
> > > + <&gcc GCC_PCIE2PHY_PHY_BCR>;
> > > + reset-names = "phy",
> > > + "common";
> > > + status = "disabled";
> > > + };
> > > +
> > > + pcie2: pcie@20000000 {
> > > + compatible = "qcom,pcie-ipq9574";
> > > + reg = <0x20000000 0xf1d>,
> > > + <0x20000f20 0xa8>,
> > > + <0x20001000 0x1000>,
> > > + <0x00088000 0x4000>,
> > > + <0x20100000 0x1000>;
> > > + reg-names = "dbi", "elbi", "atu", "parf", "config";
> > > +
> > > + ranges = <0x81000000 0x0 0x20200000 0x20200000 0x0 0x00100000>, /* I/O */
> >
> > Please use below range:
> >
> > <0x01000000 0x0 0x00000000 0x20200000 0x0 0x00100000>
> > <0x02000000 0x0 0x20300000 0x20300000 0x0 0x07d00000>
> >
> Of course, thank you.
>
> > > + <0x82000000 0x0 0x20300000 0x20300000 0x0 0x07d00000>; /* MEM */
> > > +
> > > + device_type = "pci";
> > > + linux,pci-domain = <3>;
> > > + bus-range = <0x00 0xff>;
> > > + num-lanes = <2>;
> > > + max-link-speed = <3>;
> > > + #address-cells = <3>;
> > > + #size-cells = <2>;
> > > +
> > > + phys = <&pcie2_phy>;
> > > + phy-names = "pciephy";
> > > +
> > > + interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
> > > + interrupt-names = "msi";
> > > +
> > > + #interrupt-cells = <1>;
> > > + interrupt-map-mask = <0 0 0 0x7>;
> > > + interrupt-map = <0 0 0 1 &intc 0 0 164
> > > + IRQ_TYPE_LEVEL_HIGH>, /* int_a */
> > > + <0 0 0 2 &intc 0 0 165
> > > + IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> > > + <0 0 0 3 &intc 0 0 186
> > > + IRQ_TYPE_LEVEL_HIGH>, /* int_c */
> > > + <0 0 0 4 &intc 0 0 187
> > > + IRQ_TYPE_LEVEL_HIGH>; /* int_d */
> >
> > Use a single line for each INTX entry even if it exceeds 80 column width.
>
> Yes. Will do.
>
> > - Mani
> >
--
மணிவண்ணன் சதாசிவம்
On Fri, Apr 19, 2024 at 02:44:36PM -0500, [email protected] wrote:
> Hi Mani.
>
> On 4/17/24 02:06, Manivannan Sadhasivam wrote:
> > On Mon, Apr 15, 2024 at 03:07:02PM -0500, [email protected] wrote:
> > >
> > >
> > > On 4/15/24 15:04, Dmitry Baryshkov wrote:
> > > > On Mon, 15 Apr 2024 at 21:22, Alexandru Gagniuc <[email protected]> wrote:
> > > > >
> > > > > Add support for the PCIe on IPQ9574. The main difference from ipq6018
> > > > > is that the "iface" clock is not necessarry. Add a special case in
> > > > > qcom_pcie_get_resources_2_9_0() to handle this.
> > > > >
> > > > > Signed-off-by: Alexandru Gagniuc <[email protected]>
> > > > > ---
> > > > > drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++----
> > > > > 1 file changed, 9 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > index 14772edcf0d3..10560d6d6336 100644
> > > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > @@ -1101,15 +1101,19 @@ 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;
> > > > > + int ret, num_clks = ARRAY_SIZE(res->clks) - 1;
> > > > >
> > > > > - res->clks[0].id = "iface";
> > > > > + res->clks[0].id = "rchng";
> > > > > 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 (!of_device_is_compatible(dev->of_node, "qcom,pcie-ipq9574")) {
> > > > > + res->clks[4].id = "iface";
> > > > > + num_clks++;
> > > > > + }
> > > > > +
> > > > > + ret = devm_clk_bulk_get(dev, num_clks, res->clks);
> > > >
> > > > Just use devm_clk_bulk_get_optional() here.
> > >
> > > Thank you! I wasn't sure if this was the correct solution here. I will get
> > > this updated in v4.
> > >
> >
> > Please rebase on top of [1] and mention the dependency in cover letter.
>
> I am very hesitant to depend on another patch series. Is it okay if I
> include your patch in v4 of this series?
>
Feel free to.
- Mani
--
மணிவண்ணன் சதாசிவம்
[...]
>>>> +
>>>> + #interrupt-cells = <1>;
>>>> + interrupt-map-mask = <0 0 0 0x7>;
>>>> + interrupt-map = <0 0 0 1 &intc 0 0 164
>>>> + IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>>>> + <0 0 0 2 &intc 0 0 165
>>>> + IRQ_TYPE_LEVEL_HIGH>, /* int_b */
>>>> + <0 0 0 3 &intc 0 0 186
>>>> + IRQ_TYPE_LEVEL_HIGH>, /* int_c */
>>>> + <0 0 0 4 &intc 0 0 187
>>>> + IRQ_TYPE_LEVEL_HIGH>; /* int_d */
>>>
>>> Use a single line for each INTX entry even if it exceeds 80 column width.
>>
>> Yes. Will do.
And drop the /* int_x */ comments
Konrad
On 4/20/2024 1:17 AM, [email protected] wrote:
> Hi Kathiravan,
>
> On 4/19/24 09:28, Kathiravan Thirumoorthy wrote:
>>
>>
>> On 4/15/2024 11:50 PM, Alexandru Gagniuc wrote:
>>> There are four PCIe ports on IPQ9574, pcie0 thru pcie3. This series
>>> addresses pcie2, which is a gen3x2 port. The board I have only uses
>>> pcie2, and that's the only one enabled in this series.
>>>
>>> I believe this makes sense as a monolithic series, as the individual
>>> pieces are not that useful by themselves.
>>>
>>> In v2, I've had some issues regarding the dt schema checks. For
>>> transparency, I used the following test invocations to test v3:
>>>
>>> make dt_binding_check
>>> DT_SCHEMA_FILES=qcom,pcie.yaml:qcom,ipq8074-qmp-pcie-phy.yaml
>>> make dtbs_check
>>> DT_SCHEMA_FILES=qcom,pcie.yaml:qcom,ipq8074-qmp-pcie-phy.yaml
>>>
>>>
>>
>> Alexandru,
>>
>> Thanks for your contributions to the Qualcomm IPQ chipsets.
>>
>> I would like to inform you that we have also submitted the patches to
>> enable the PCIe support on IPQ9574[1][2] and waiting for the ICC
>> support[3] to land to enable the NOC clocks.
>>
>> [1]
>> https://lore.kernel.org/linux-arm-msm/[email protected]/
>> [2]
>> https://lore.kernel.org/linux-arm-msm/[email protected]/
>> [3]
>> https://lore.kernel.org/linux-arm-msm/[email protected]/
>>
>> Please take a look at these patches as well.
>
> I think I've seen [1] before -- I thought the series was abandoned.
> Since we have the dt-schema and applicability on mainline resolved here,
> do you want to use this series as the base for any new PCIe work?
Sure Alex. I believe some of the code review comments are already
addressed in the series which I pointed out. If you could have picked
those and re-posted the next version, it could have been better.
>
> Alex
>
>> Thanks,
>> Kathiravan T.
>>
>>
>>> Changes since v2:
>>> - reworked resets in qcom,pcie.yaml to resolve dt schema errors
>>> - constrained "reg" in qcom,pcie.yaml
>>> - reworked min/max intems in qcom,ipq8074-qmp-pcie-phy.yaml
>>> - dropped msi-parent for pcie node, as it is handled by "msi" IRQ
>>>
>>> Changes since v1:
>>> - updated new tables in phy-qcom-qmp-pcie.c to use lowercase hex
>>> numbers
>>> - reorganized qcom,ipq8074-qmp-pcie-phy.yaml to use a single list
>>> of clocks
>>> - reorganized qcom,pcie.yaml to include clocks+resets per compatible
>>> - Renamed "pcie2_qmp_phy" label to "pcie2_phy"
>>> - moved "ranges" property of pcie@20000000 higher up
>>>
>>> Alexandru Gagniuc (7):
>>> dt-bindings: clock: Add PCIe pipe related clocks for IPQ9574
>>> clk: qcom: gcc-ipq9574: Add PCIe pipe clocks
>>> dt-bindings: PCI: qcom: Add IPQ9574 PCIe controller
>>> PCI: qcom: Add support for IPQ9574
>>> dt-bindings: phy: qcom,ipq8074-qmp-pcie: add ipq9574 gen3x2 PHY
>>> phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY
>>> arm64: dts: qcom: ipq9574: add PCIe2 nodes
>>>
>>> .../devicetree/bindings/pci/qcom,pcie.yaml | 35 +++++
>>> .../phy/qcom,ipq8074-qmp-pcie-phy.yaml | 36 ++++-
>>> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 93 +++++++++++-
>>> drivers/clk/qcom/gcc-ipq9574.c | 76 ++++++++++
>>> drivers/pci/controller/dwc/pcie-qcom.c | 13 +-
>>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
>>> .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
>>> include/dt-bindings/clock/qcom,ipq9574-gcc.h | 4 +
>>> 8 files changed, 400 insertions(+), 7 deletions(-)
>>>
On Wed, Apr 17, 2024 at 12:50:49AM +0300, Dmitry Baryshkov wrote:
> On Wed, 17 Apr 2024 at 00:25, Alex G. <[email protected]> wrote:
> >
> > Hi Dmitry,
> >
> > On 4/15/24 16:25, [email protected] wrote:
> > >
> > >
> > > On 4/15/24 15:10, Dmitry Baryshkov wrote:
> > >> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <[email protected]>
> > >> wrote:
> > >>>
> > >>> Add support for the gen3x2 PCIe PHY on IPQ9574, ported form downstream
> > >>> 5.4 kernel. Only the serdes and pcs_misc tables are new, the others
> > >>> being reused from IPQ8074 and IPQ6018 PHYs.
> > >>>
> > >>> Signed-off-by: Alexandru Gagniuc <[email protected]>
> > >>> ---
> > >>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
> > >>> .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
> > >>> 2 files changed, 149 insertions(+), 1 deletion(-)
> > >>>
> > >>
> > >> [skipped]
> > >>
> > >>> @@ -2448,7 +2542,7 @@ static inline void qphy_clrbits(void __iomem
> > >>> *base, u32 offset, u32 val)
> > >>>
> > >>> /* list of clocks required by phy */
> > >>> static const char * const qmp_pciephy_clk_l[] = {
> > >>> - "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
> > >>> + "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
> > >>> "anoc", "snoc"
> > >>
> > >> Are the NoC clocks really necessary to drive the PHY? I think they are
> > >> usually connected to the controllers, not the PHYs.
> > >
> > > The system will hang if these clocks are not enabled. They are also
> > > attached to the PHY in the QCA 5.4 downstream kernel.
>
> Interesting.
> I see that Varadarajan is converting these clocks into interconnects.
> Maybe it's better to wait for those patches to land and use
> interconnects instead. I think it would better suit the
> infrastructure.
>
> Varadarajan, could you please comment, are these interconnects
> connected to the PHY too or just to the PCIe controller?
Sorry for the late response. Missed this e-mail.
These 2 clks are related to AXI port clk on Aggnoc/SNOC, not
directly connected to PCIE wrapper, but it should be enabled to
generate pcie traffic.
Thanks
Varada
> > They are named "anoc_lane", and "snoc_lane" in the downstream kernel.
> > Would you like me to use these names instead?
>
> I'm fine either way.
>
> > e>>> };
> > >>>
> > >>> /* list of regulators */
> > >>> @@ -2499,6 +2593,16 @@ static const struct qmp_pcie_offsets
> > >>> qmp_pcie_offsets_v4x1 = {
> > >>> .rx = 0x0400,
> > >>> };
> > >>>
> > >>> +static const struct qmp_pcie_offsets qmp_pcie_offsets_ipq9574 = {
> > >>> + .serdes = 0,
> > >>> + .pcs = 0x1000,
> > >>> + .pcs_misc = 0x1400,
> > >>> + .tx = 0x0200,
> > >>> + .rx = 0x0400,
> > >>> + .tx2 = 0x0600,
> > >>> + .rx2 = 0x0800,
> > >>> +};
> > >>> +
> > >>> static const struct qmp_pcie_offsets qmp_pcie_offsets_v4x2 = {
> > >>> .serdes = 0,
> > >>> .pcs = 0x0a00,
> > >>> @@ -2728,6 +2832,33 @@ static const struct qmp_phy_cfg
> > >>> sm8250_qmp_gen3x1_pciephy_cfg = {
> > >>> .phy_status = PHYSTATUS,
> > >>> };
> > >>>
> > >>> +static const struct qmp_phy_cfg ipq9574_pciephy_gen3x2_cfg = {
> > >>> + .lanes = 2,
> > >>> +
> > >>> + .offsets = &qmp_pcie_offsets_ipq9574,
> > >>> +
> > >>> + .tbls = {
> > >>> + .serdes = ipq9574_gen3x2_pcie_serdes_tbl,
> > >>> + .serdes_num =
> > >>> ARRAY_SIZE(ipq9574_gen3x2_pcie_serdes_tbl),
> > >>> + .tx = ipq8074_pcie_gen3_tx_tbl,
> > >>> + .tx_num = ARRAY_SIZE(ipq8074_pcie_gen3_tx_tbl),
> > >>> + .rx = ipq6018_pcie_rx_tbl,
> > >>> + .rx_num = ARRAY_SIZE(ipq6018_pcie_rx_tbl),
> > >>> + .pcs = ipq6018_pcie_pcs_tbl,
> > >>> + .pcs_num = ARRAY_SIZE(ipq6018_pcie_pcs_tbl),
> > >>> + .pcs_misc = ipq9574_gen3x2_pcie_pcs_misc_tbl,
> > >>> + .pcs_misc_num =
> > >>> ARRAY_SIZE(ipq9574_gen3x2_pcie_pcs_misc_tbl),
> > >>> + },
> > >>> + .reset_list = ipq8074_pciephy_reset_l,
> > >>> + .num_resets = ARRAY_SIZE(ipq8074_pciephy_reset_l),
> > >>> + .vreg_list = NULL,
> > >>> + .num_vregs = 0,
> > >>> + .regs = pciephy_v4_regs_layout,
> > >>
> > >> So, is it v4 or v5?
> > >
> > > Please give me a day or so to go over my notes and give you a more
> > > coherent explanation of why this versioning was chosen. I am only
> > > working from the QCA 5.4 downstream sources. I don't have any
> > > documentation for the silicon
> >
> > The downstream QCA kernel uses the same table for ipq6018, ipq8074-gen3,
> > and ipq9574. It is named "ipq_pciephy_gen3_regs_layout". Thus, it made
> > sense to use the same upstream table for ipq9574, "pciephy_v4_regs_layout".
> >
> > As far as the register tables go, the pcs/pcs_misc are squashed into the
> > same table in the downstream 5.4 kernel. I was able to separate the two
> > tables because the pcs_misc registers were defined with an offset of
> > 0x400. For example:
> >
> > /* QMP V2 PHY for PCIE gen3 2 Lane ports - PCS Misc registers */
> > #define PCS_PCIE_X2_POWER_STATE_CONFIG2 0x40c
> > #define PCS_PCIE_X2_POWER_STATE_CONFIG4 0x414
> > #define PCS_PCIE_X2_ENDPOINT_REFCLK_DRIVE 0x420
> > #define PCS_PCIE_X2_L1P1_WAKEUP_DLY_TIME_AUXCLK_L 0x444
> > #define PCS_PCIE_X2_L1P1_WAKEUP_DLY_TIME_AUXCLK_H 0x448
> > #define PCS_PCIE_X2_L1P2_WAKEUP_DLY_TIME_AUXCLK_L 0x44c
> > #define PCS_PCIE_X2_L1P2_WAKEUP_DLY_TIME_AUXCLK_H 0x450
> > ...
> >
> > Here, QPHY_V4_PCS_PCIE_POWER_STATE_CONFIG2 = 0xc would be correct,
> > assuming a pcs_misc offset of 0x400. However, starting with
> > ENDPOINT_REFCLK_DRIVE, the register would be
> > QPHY_V4_PCS_PCIE_ENDPOINT_REFCLK_DRIVE = 0x1c. Our offsets are off-by 0x4.
> >
> > The existing V5 offsets, on the other hand, were all correct. For this
> > reason, I considered that V5 is the most likely place to add the missing
> > PCS misc definitions.
>
> Ok, sounds sane. Please use _v5 for the regs layout.
>
> >
> > Is this explanation sufficiently convincing? Where does the v4/v5 scheme
> > in upstream kernel originate?
>
> Sometimes it's vendor kernels, sometimes it's a feedback from devs
> that have access to actual specs.
>
>
> --
> With best wishes
> Dmitry
On Mon, 29 Apr 2024 at 09:20, Varadarajan Narayanan
<[email protected]> wrote:
>
> On Wed, Apr 17, 2024 at 12:50:49AM +0300, Dmitry Baryshkov wrote:
> > On Wed, 17 Apr 2024 at 00:25, Alex G. <[email protected]> wrote:
> > >
> > > Hi Dmitry,
> > >
> > > On 4/15/24 16:25, [email protected] wrote:
> > > >
> > > >
> > > > On 4/15/24 15:10, Dmitry Baryshkov wrote:
> > > >> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <[email protected]>
> > > >> wrote:
> > > >>>
> > > >>> Add support for the gen3x2 PCIe PHY on IPQ9574, ported form downstream
> > > >>> 5.4 kernel. Only the serdes and pcs_misc tables are new, the others
> > > >>> being reused from IPQ8074 and IPQ6018 PHYs.
> > > >>>
> > > >>> Signed-off-by: Alexandru Gagniuc <[email protected]>
> > > >>> ---
> > > >>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
> > > >>> .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
> > > >>> 2 files changed, 149 insertions(+), 1 deletion(-)
> > > >>>
> > > >>
> > > >> [skipped]
> > > >>
> > > >>> @@ -2448,7 +2542,7 @@ static inline void qphy_clrbits(void __iomem
> > > >>> *base, u32 offset, u32 val)
> > > >>>
> > > >>> /* list of clocks required by phy */
> > > >>> static const char * const qmp_pciephy_clk_l[] = {
> > > >>> - "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
> > > >>> + "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
> > > >>> "anoc", "snoc"
> > > >>
> > > >> Are the NoC clocks really necessary to drive the PHY? I think they are
> > > >> usually connected to the controllers, not the PHYs.
> > > >
> > > > The system will hang if these clocks are not enabled. They are also
> > > > attached to the PHY in the QCA 5.4 downstream kernel.
> >
> > Interesting.
> > I see that Varadarajan is converting these clocks into interconnects.
> > Maybe it's better to wait for those patches to land and use
> > interconnects instead. I think it would better suit the
> > infrastructure.
> >
> > Varadarajan, could you please comment, are these interconnects
> > connected to the PHY too or just to the PCIe controller?
>
> Sorry for the late response. Missed this e-mail.
>
> These 2 clks are related to AXI port clk on Aggnoc/SNOC, not
> directly connected to PCIE wrapper, but it should be enabled to
> generate pcie traffic.
So, are they required for the PHY or are they required for the PCIe
controller only?
>
> Thanks
> Varada
>
> > > They are named "anoc_lane", and "snoc_lane" in the downstream kernel.
> > > Would you like me to use these names instead?
> >
> > I'm fine either way.
> >
--
With best wishes
Dmitry
On Mon, Apr 29, 2024 at 01:55:32PM +0300, Dmitry Baryshkov wrote:
> On Mon, 29 Apr 2024 at 09:20, Varadarajan Narayanan
> <[email protected]> wrote:
> >
> > On Wed, Apr 17, 2024 at 12:50:49AM +0300, Dmitry Baryshkov wrote:
> > > On Wed, 17 Apr 2024 at 00:25, Alex G. <[email protected]> wrote:
> > > >
> > > > Hi Dmitry,
> > > >
> > > > On 4/15/24 16:25, [email protected] wrote:
> > > > >
> > > > >
> > > > > On 4/15/24 15:10, Dmitry Baryshkov wrote:
> > > > >> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <[email protected]>
> > > > >> wrote:
> > > > >>>
> > > > >>> Add support for the gen3x2 PCIe PHY on IPQ9574, ported form downstream
> > > > >>> 5.4 kernel. Only the serdes and pcs_misc tables are new, the others
> > > > >>> being reused from IPQ8074 and IPQ6018 PHYs.
> > > > >>>
> > > > >>> Signed-off-by: Alexandru Gagniuc <[email protected]>
> > > > >>> ---
> > > > >>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
> > > > >>> .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
> > > > >>> 2 files changed, 149 insertions(+), 1 deletion(-)
> > > > >>>
> > > > >>
> > > > >> [skipped]
> > > > >>
> > > > >>> @@ -2448,7 +2542,7 @@ static inline void qphy_clrbits(void __iomem
> > > > >>> *base, u32 offset, u32 val)
> > > > >>>
> > > > >>> /* list of clocks required by phy */
> > > > >>> static const char * const qmp_pciephy_clk_l[] = {
> > > > >>> - "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
> > > > >>> + "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
> > > > >>> "anoc", "snoc"
> > > > >>
> > > > >> Are the NoC clocks really necessary to drive the PHY? I think they are
> > > > >> usually connected to the controllers, not the PHYs.
> > > > >
> > > > > The system will hang if these clocks are not enabled. They are also
> > > > > attached to the PHY in the QCA 5.4 downstream kernel.
> > >
> > > Interesting.
> > > I see that Varadarajan is converting these clocks into interconnects.
> > > Maybe it's better to wait for those patches to land and use
> > > interconnects instead. I think it would better suit the
> > > infrastructure.
> > >
> > > Varadarajan, could you please comment, are these interconnects
> > > connected to the PHY too or just to the PCIe controller?
> >
> > Sorry for the late response. Missed this e-mail.
> >
> > These 2 clks are related to AXI port clk on Aggnoc/SNOC, not
> > directly connected to PCIE wrapper, but it should be enabled to
> > generate pcie traffic.
>
> So, are they required for the PHY or are they required for the PCIe
> controller only?
These 2 clks are required for PCIe controller only.
PCIE controller need these clks to send/receive axi pkts.
Thanks
Varada
> > > > They are named "anoc_lane", and "snoc_lane" in the downstream kernel.
> > > > Would you like me to use these names instead?
> > >
> > > I'm fine either way.
> > >
>
>
>
> --
> With best wishes
> Dmitry
On 4/30/24 1:31 AM, Varadarajan Narayanan wrote:
> On Mon, Apr 29, 2024 at 01:55:32PM +0300, Dmitry Baryshkov wrote:
>> On Mon, 29 Apr 2024 at 09:20, Varadarajan Narayanan
>> <[email protected]> wrote:
>>>
>>> On Wed, Apr 17, 2024 at 12:50:49AM +0300, Dmitry Baryshkov wrote:
>>>> On Wed, 17 Apr 2024 at 00:25, Alex G. <[email protected]> wrote:
>>>>>
>>>>> Hi Dmitry,
>>>>>
>>>>> On 4/15/24 16:25, [email protected] wrote:
>>>>>>
>>>>>>
>>>>>> On 4/15/24 15:10, Dmitry Baryshkov wrote:
>>>>>>> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <[email protected]>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Add support for the gen3x2 PCIe PHY on IPQ9574, ported form downstream
>>>>>>>> 5.4 kernel. Only the serdes and pcs_misc tables are new, the others
>>>>>>>> being reused from IPQ8074 and IPQ6018 PHYs.
>>>>>>>>
>>>>>>>> Signed-off-by: Alexandru Gagniuc <[email protected]>
>>>>>>>> ---
>>>>>>>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
>>>>>>>> .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
>>>>>>>> 2 files changed, 149 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>
>>>>>>> [skipped]
>>>>>>>
>>>>>>>> @@ -2448,7 +2542,7 @@ static inline void qphy_clrbits(void __iomem
>>>>>>>> *base, u32 offset, u32 val)
>>>>>>>>
>>>>>>>> /* list of clocks required by phy */
>>>>>>>> static const char * const qmp_pciephy_clk_l[] = {
>>>>>>>> - "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
>>>>>>>> + "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
>>>>>>>> "anoc", "snoc"
>>>>>>>
>>>>>>> Are the NoC clocks really necessary to drive the PHY? I think they are
>>>>>>> usually connected to the controllers, not the PHYs.
>>>>>>
>>>>>> The system will hang if these clocks are not enabled. They are also
>>>>>> attached to the PHY in the QCA 5.4 downstream kernel.
>>>>
>>>> Interesting.
>>>> I see that Varadarajan is converting these clocks into interconnects.
>>>> Maybe it's better to wait for those patches to land and use
>>>> interconnects instead. I think it would better suit the
>>>> infrastructure.
>>>>
>>>> Varadarajan, could you please comment, are these interconnects
>>>> connected to the PHY too or just to the PCIe controller?
>>>
>>> Sorry for the late response. Missed this e-mail.
>>>
>>> These 2 clks are related to AXI port clk on Aggnoc/SNOC, not
>>> directly connected to PCIE wrapper, but it should be enabled to
>>> generate pcie traffic.
>>
>> So, are they required for the PHY or are they required for the PCIe
>> controller only?
>
> These 2 clks are required for PCIe controller only.
> PCIE controller need these clks to send/receive axi pkts.
Very interesting information, thank you!
Dmitry, In light of this information do you want me to move these clocks
out of the PHY and into the PCIe controller?
Alex
> Thanks
> Varada
>
>>>>> They are named "anoc_lane", and "snoc_lane" in the downstream kernel.
>>>>> Would you like me to use these names instead?
>>>>
>>>> I'm fine either way.
>>>>
>>
>>
>>
>> --
>> With best wishes
>> Dmitry
On Wed, 1 May 2024 at 00:51, <[email protected]> wrote:
>
> On 4/30/24 1:31 AM, Varadarajan Narayanan wrote:
> > On Mon, Apr 29, 2024 at 01:55:32PM +0300, Dmitry Baryshkov wrote:
> >> On Mon, 29 Apr 2024 at 09:20, Varadarajan Narayanan
> >> <[email protected]> wrote:
> >>>
> >>> On Wed, Apr 17, 2024 at 12:50:49AM +0300, Dmitry Baryshkov wrote:
> >>>> On Wed, 17 Apr 2024 at 00:25, Alex G. <[email protected]> wrote:
> >>>>>
> >>>>> Hi Dmitry,
> >>>>>
> >>>>> On 4/15/24 16:25, [email protected] wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 4/15/24 15:10, Dmitry Baryshkov wrote:
> >>>>>>> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <[email protected]>
> >>>>>>> wrote:
> >>>>>>>>
> >>>>>>>> Add support for the gen3x2 PCIe PHY on IPQ9574, ported form downstream
> >>>>>>>> 5.4 kernel. Only the serdes and pcs_misc tables are new, the others
> >>>>>>>> being reused from IPQ8074 and IPQ6018 PHYs.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Alexandru Gagniuc <[email protected]>
> >>>>>>>> ---
> >>>>>>>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 136 +++++++++++++++++-
> >>>>>>>> .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 14 ++
> >>>>>>>> 2 files changed, 149 insertions(+), 1 deletion(-)
> >>>>>>>>
> >>>>>>>
> >>>>>>> [skipped]
> >>>>>>>
> >>>>>>>> @@ -2448,7 +2542,7 @@ static inline void qphy_clrbits(void __iomem
> >>>>>>>> *base, u32 offset, u32 val)
> >>>>>>>>
> >>>>>>>> /* list of clocks required by phy */
> >>>>>>>> static const char * const qmp_pciephy_clk_l[] = {
> >>>>>>>> - "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
> >>>>>>>> + "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
> >>>>>>>> "anoc", "snoc"
> >>>>>>>
> >>>>>>> Are the NoC clocks really necessary to drive the PHY? I think they are
> >>>>>>> usually connected to the controllers, not the PHYs.
> >>>>>>
> >>>>>> The system will hang if these clocks are not enabled. They are also
> >>>>>> attached to the PHY in the QCA 5.4 downstream kernel.
> >>>>
> >>>> Interesting.
> >>>> I see that Varadarajan is converting these clocks into interconnects.
> >>>> Maybe it's better to wait for those patches to land and use
> >>>> interconnects instead. I think it would better suit the
> >>>> infrastructure.
> >>>>
> >>>> Varadarajan, could you please comment, are these interconnects
> >>>> connected to the PHY too or just to the PCIe controller?
> >>>
> >>> Sorry for the late response. Missed this e-mail.
> >>>
> >>> These 2 clks are related to AXI port clk on Aggnoc/SNOC, not
> >>> directly connected to PCIE wrapper, but it should be enabled to
> >>> generate pcie traffic.
> >>
> >> So, are they required for the PHY or are they required for the PCIe
> >> controller only?
> >
> > These 2 clks are required for PCIe controller only.
> > PCIE controller need these clks to send/receive axi pkts.
>
> Very interesting information, thank you!
>
> Dmitry, In light of this information do you want me to move these clocks
> out of the PHY and into the PCIe controller?
That's what I was thinking about.
>
> Alex
>
> > Thanks
> > Varada
> >
> >>>>> They are named "anoc_lane", and "snoc_lane" in the downstream kernel.
> >>>>> Would you like me to use these names instead?
> >>>>
> >>>> I'm fine either way.
> >>>>
> >>
> >>
> >>
> >> --
> >> With best wishes
> >> Dmitry
--
With best wishes
Dmitry