[PATCH v3 0/7] ipq9574: Enable PCI-Express support

[PATCH v3 0/7] ipq9574: Enable PCI-Express support

[Alexandru Gagniuc]

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

[PATCH v3 1/7] dt-bindings: clock: Add PCIe pipe related clocks for IPQ9574

[Alexandru Gagniuc]

Add defines for the missing PCIe PIPE clocks.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 include/dt-bindings/clock/qcom,ipq9574-gcc.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/dt-bindings/clock/qcom,ipq9574-gcc.h b/include/dt-bindings/clock/qcom,ipq9574-gcc.h
index 08fd3a37acaa..52123c5a09fa 100644
--- a/include/dt-bindings/clock/qcom,ipq9574-gcc.h
+++ b/include/dt-bindings/clock/qcom,ipq9574-gcc.h
@@ -216,4 +216,8 @@
 #define GCC_CRYPTO_AHB_CLK				207
 #define GCC_USB0_PIPE_CLK				208
 #define GCC_USB0_SLEEP_CLK				209
+#define GCC_PCIE0_PIPE_CLK				210
+#define GCC_PCIE1_PIPE_CLK				211
+#define GCC_PCIE2_PIPE_CLK				212
+#define GCC_PCIE3_PIPE_CLK				213
 #endif
-- 
2.40.1

[PATCH v3 2/7] clk: qcom: gcc-ipq9574: Add PCIe pipe clocks

[Alexandru Gagniuc]

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 <mr.nuke.me@gmail.com>
---
 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

Re: [PATCH v3 2/7] clk: qcom: gcc-ipq9574: Add PCIe pipe clocks

[Dmitry Baryshkov]

On Mon, 15 Apr 2024 at 21:21, Alexandru Gagniuc <mr.nuke.me@gmail.com> 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 <mr.nuke.me@gmail.com>
> ---
>  drivers/clk/qcom/gcc-ipq9574.c | 76 ++++++++++++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH v3 2/7] clk: qcom: gcc-ipq9574: Add PCIe pipe clocks

[Stephen Boyd]

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.

[PATCH v3 3/7] dt-bindings: PCI: qcom: Add IPQ9574 PCIe controller

[Alexandru Gagniuc]

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 <mr.nuke.me@gmail.com>
---
 .../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

Re: [PATCH v3 3/7] dt-bindings: PCI: qcom: Add IPQ9574 PCIe controller

[Rob Herring]

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 <mr.nuke.me@gmail.com>
> ---
>  .../devicetree/bindings/pci/qcom,pcie.yaml    | 35 +++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 

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

[PATCH v3 4/7] PCI: qcom: Add support for IPQ9574

[Alexandru Gagniuc]

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 <mr.nuke.me@gmail.com>
---
 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

Re: [PATCH v3 4/7] PCI: qcom: Add support for IPQ9574

[Dmitry Baryshkov]

On Mon, 15 Apr 2024 at 21:22, Alexandru Gagniuc <mr.nuke.me@gmail.com> 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 <mr.nuke.me@gmail.com>
> ---
>  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

Re: [PATCH v3 4/7] PCI: qcom: Add support for IPQ9574

[mr.nuke.me]

On 4/15/24 15:04, Dmitry Baryshkov wrote:
> On Mon, 15 Apr 2024 at 21:22, Alexandru Gagniuc <mr.nuke.me@gmail.com> 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 <mr.nuke.me@gmail.com>
>> ---
>>   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
>>
>>
> 
> 

Re: [PATCH v3 4/7] PCI: qcom: Add support for IPQ9574

[Manivannan Sadhasivam]

On Mon, Apr 15, 2024 at 03:07:02PM -0500, mr.nuke.me@gmail.com wrote:
> 
> 
> On 4/15/24 15:04, Dmitry Baryshkov wrote:
> > On Mon, 15 Apr 2024 at 21:22, Alexandru Gagniuc <mr.nuke.me@gmail.com> 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 <mr.nuke.me@gmail.com>
> > > ---
> > >   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/20240417-pci-qcom-clk-bulk-v1-1-52ca19b3d6b2@linaro.org/

> 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
> > > 
> > > 
> > 
> > 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v3 4/7] PCI: qcom: Add support for IPQ9574

[mr.nuke.me]

Hi Mani.

On 4/17/24 02:06, Manivannan Sadhasivam wrote:
> On Mon, Apr 15, 2024 at 03:07:02PM -0500, mr.nuke.me@gmail.com wrote:
>>
>>
>> On 4/15/24 15:04, Dmitry Baryshkov wrote:
>>> On Mon, 15 Apr 2024 at 21:22, Alexandru Gagniuc <mr.nuke.me@gmail.com> 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 <mr.nuke.me@gmail.com>
>>>> ---
>>>>    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

Re: [PATCH v3 4/7] PCI: qcom: Add support for IPQ9574

[Manivannan Sadhasivam]

On Fri, Apr 19, 2024 at 02:44:36PM -0500, mr.nuke.me@gmail.com wrote:
> Hi Mani.
> 
> On 4/17/24 02:06, Manivannan Sadhasivam wrote:
> > On Mon, Apr 15, 2024 at 03:07:02PM -0500, mr.nuke.me@gmail.com wrote:
> > > 
> > > 
> > > On 4/15/24 15:04, Dmitry Baryshkov wrote:
> > > > On Mon, 15 Apr 2024 at 21:22, Alexandru Gagniuc <mr.nuke.me@gmail.com> 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 <mr.nuke.me@gmail.com>
> > > > > ---
> > > > >    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

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v3 4/7] PCI: qcom: Add support for IPQ9574

[Manivannan Sadhasivam]

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 <mr.nuke.me@gmail.com>
> ---
>  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
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH v3 5/7] dt-bindings: phy: qcom,ipq8074-qmp-pcie: add ipq9574 gen3x2 PHY

[Alexandru Gagniuc]

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 <mr.nuke.me@gmail.com>
---
 .../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

Re: [PATCH v3 5/7] dt-bindings: phy: qcom,ipq8074-qmp-pcie: add ipq9574 gen3x2 PHY

[Rob Herring]

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 <mr.nuke.me@gmail.com>
> ---
>  .../phy/qcom,ipq8074-qmp-pcie-phy.yaml        | 36 ++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 

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

[PATCH v3 6/7] phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY

[Alexandru Gagniuc]

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 <mr.nuke.me@gmail.com>
---
 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

Re: [PATCH v3 6/7] phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY

[Dmitry Baryshkov]

On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <mr.nuke.me@gmail.com> 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 <mr.nuke.me@gmail.com>
> ---
>  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

Re: [PATCH v3 6/7] phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY

[mr.nuke.me]

On 4/15/24 15:10, Dmitry Baryshkov wrote:
> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <mr.nuke.me@gmail.com> 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 <mr.nuke.me@gmail.com>
>> ---
>>   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

Re: [PATCH v3 6/7] phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY

[Alex G.]

Hi Dmitry,

On 4/15/24 16:25, mr.nuke.me@gmail.com wrote:
> 
> 
> On 4/15/24 15:10, Dmitry Baryshkov wrote:
>> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <mr.nuke.me@gmail.com> 
>> 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 <mr.nuke.me@gmail.com>
>>> ---
>>>   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

Re: [PATCH v3 6/7] phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY

[Dmitry Baryshkov]

On Wed, 17 Apr 2024 at 00:25, Alex G. <mr.nuke.me@gmail.com> wrote:
>
> Hi Dmitry,
>
> On 4/15/24 16:25, mr.nuke.me@gmail.com wrote:
> >
> >
> > On 4/15/24 15:10, Dmitry Baryshkov wrote:
> >> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <mr.nuke.me@gmail.com>
> >> 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 <mr.nuke.me@gmail.com>
> >>> ---
> >>>   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

Re: [PATCH v3 6/7] phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY

[Varadarajan Narayanan]

On Wed, Apr 17, 2024 at 12:50:49AM +0300, Dmitry Baryshkov wrote:
> On Wed, 17 Apr 2024 at 00:25, Alex G. <mr.nuke.me@gmail.com> wrote:
> >
> > Hi Dmitry,
> >
> > On 4/15/24 16:25, mr.nuke.me@gmail.com wrote:
> > >
> > >
> > > On 4/15/24 15:10, Dmitry Baryshkov wrote:
> > >> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > >> 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 <mr.nuke.me@gmail.com>
> > >>> ---
> > >>>   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

Re: [PATCH v3 6/7] phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY

[Dmitry Baryshkov]

On Mon, 29 Apr 2024 at 09:20, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> On Wed, Apr 17, 2024 at 12:50:49AM +0300, Dmitry Baryshkov wrote:
> > On Wed, 17 Apr 2024 at 00:25, Alex G. <mr.nuke.me@gmail.com> wrote:
> > >
> > > Hi Dmitry,
> > >
> > > On 4/15/24 16:25, mr.nuke.me@gmail.com wrote:
> > > >
> > > >
> > > > On 4/15/24 15:10, Dmitry Baryshkov wrote:
> > > >> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > > >> 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 <mr.nuke.me@gmail.com>
> > > >>> ---
> > > >>>   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

Re: [PATCH v3 6/7] phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY

[Varadarajan Narayanan]

On Mon, Apr 29, 2024 at 01:55:32PM +0300, Dmitry Baryshkov wrote:
> On Mon, 29 Apr 2024 at 09:20, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
> >
> > On Wed, Apr 17, 2024 at 12:50:49AM +0300, Dmitry Baryshkov wrote:
> > > On Wed, 17 Apr 2024 at 00:25, Alex G. <mr.nuke.me@gmail.com> wrote:
> > > >
> > > > Hi Dmitry,
> > > >
> > > > On 4/15/24 16:25, mr.nuke.me@gmail.com wrote:
> > > > >
> > > > >
> > > > > On 4/15/24 15:10, Dmitry Baryshkov wrote:
> > > > >> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > > > >> 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 <mr.nuke.me@gmail.com>
> > > > >>> ---
> > > > >>>   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

Re: [PATCH v3 6/7] phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY

[mr.nuke.me]

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
>> <quic_varada@quicinc.com> wrote:
>>>
>>> On Wed, Apr 17, 2024 at 12:50:49AM +0300, Dmitry Baryshkov wrote:
>>>> On Wed, 17 Apr 2024 at 00:25, Alex G. <mr.nuke.me@gmail.com> wrote:
>>>>>
>>>>> Hi Dmitry,
>>>>>
>>>>> On 4/15/24 16:25, mr.nuke.me@gmail.com wrote:
>>>>>>
>>>>>>
>>>>>> On 4/15/24 15:10, Dmitry Baryshkov wrote:
>>>>>>> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <mr.nuke.me@gmail.com>
>>>>>>> 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 <mr.nuke.me@gmail.com>
>>>>>>>> ---
>>>>>>>>    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

Re: [PATCH v3 6/7] phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY

[Dmitry Baryshkov]

On Wed, 1 May 2024 at 00:51, <mr.nuke.me@gmail.com> 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
> >> <quic_varada@quicinc.com> wrote:
> >>>
> >>> On Wed, Apr 17, 2024 at 12:50:49AM +0300, Dmitry Baryshkov wrote:
> >>>> On Wed, 17 Apr 2024 at 00:25, Alex G. <mr.nuke.me@gmail.com> wrote:
> >>>>>
> >>>>> Hi Dmitry,
> >>>>>
> >>>>> On 4/15/24 16:25, mr.nuke.me@gmail.com wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 4/15/24 15:10, Dmitry Baryshkov wrote:
> >>>>>>> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <mr.nuke.me@gmail.com>
> >>>>>>> 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 <mr.nuke.me@gmail.com>
> >>>>>>>> ---
> >>>>>>>>    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

[PATCH v3 7/7] arm64: dts: qcom: ipq9574: add PCIe2 nodes

[Alexandru Gagniuc]

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 <mr.nuke.me@gmail.com>
---
 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

Re: [PATCH v3 7/7] arm64: dts: qcom: ipq9574: add PCIe2 nodes

[Manivannan Sadhasivam]

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 <mr.nuke.me@gmail.com>
> ---
>  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

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v3 7/7] arm64: dts: qcom: ipq9574: add PCIe2 nodes

[mr.nuke.me]

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 <mr.nuke.me@gmail.com>
>> ---
>>   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
> 

Re: [PATCH v3 7/7] arm64: dts: qcom: ipq9574: add PCIe2 nodes

[Manivannan Sadhasivam]

On Thu, Apr 18, 2024 at 10:33:23AM -0500, mr.nuke.me@gmail.com 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 <mr.nuke.me@gmail.com>
> > > ---
> > >   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
> > 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v3 7/7] arm64: dts: qcom: ipq9574: add PCIe2 nodes

[Konrad Dybcio]

[...]


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

Re: [PATCH v3 0/7] ipq9574: Enable PCI-Express support

[Kathiravan Thirumoorthy]

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/20230519090219.15925-1-quic_devipriy@quicinc.com/
[2] 
https://lore.kernel.org/linux-arm-msm/20230519085723.15601-1-quic_devipriy@quicinc.com/
[3] 
https://lore.kernel.org/linux-arm-msm/20240418092305.2337429-1-quic_varada@quicinc.com/

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(-)
> 

Re: [PATCH v3 0/7] ipq9574: Enable PCI-Express support

[mr.nuke.me]

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/20230519090219.15925-1-quic_devipriy@quicinc.com/
> [2] 
> https://lore.kernel.org/linux-arm-msm/20230519085723.15601-1-quic_devipriy@quicinc.com/
> [3] 
> https://lore.kernel.org/linux-arm-msm/20240418092305.2337429-1-quic_varada@quicinc.com/
> 
> 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(-)
>>

Re: [PATCH v3 0/7] ipq9574: Enable PCI-Express support

[Kathiravan Thirumoorthy]

On 4/20/2024 1:17 AM, mr.nuke.me@gmail.com 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/20230519090219.15925-1-quic_devipriy@quicinc.com/
>> [2] 
>> https://lore.kernel.org/linux-arm-msm/20230519085723.15601-1-quic_devipriy@quicinc.com/
>> [3] 
>> https://lore.kernel.org/linux-arm-msm/20240418092305.2337429-1-quic_varada@quicinc.com/
>>
>> 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(-)
>>>