[PATCH v3 1/3] dt-bindings: PCI: qcom: ep: Add interconnects path

[PATCH v3 1/3] dt-bindings: PCI: qcom: ep: Add interconnects path

[Krishna chaitanya chundru]

Some platforms may not boot if a device driver doesn't initialize
the interconnect path. Mostly it is handled by the bootloader but
we have starting to see cases where bootloader simply ignores them.

Add the "pcie-mem" interconnect path as a required property to the
bindings.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
index b3c22eb..656e362 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
@@ -70,6 +70,13 @@ properties:
     description: GPIO used as WAKE# output signal
     maxItems: 1
 
+  interconnects:
+    maxItems: 1
+
+  interconnect-names:
+    items:
+      - const: pcie-mem
+
   resets:
     maxItems: 1
 
@@ -97,6 +104,8 @@ required:
   - interrupts
   - interrupt-names
   - reset-gpios
+  - interconnects
+  - interconnect-names
   - resets
   - reset-names
   - power-domains
@@ -194,6 +203,8 @@ examples:
         interrupt-names = "global", "doorbell";
         reset-gpios = <&tlmm 57 GPIO_ACTIVE_LOW>;
         wake-gpios = <&tlmm 53 GPIO_ACTIVE_LOW>;
+	interconnects = <&system_noc MASTER_PCIE_0 &mc_virt SLAVE_EBI1>;
+	interconnect-names = "pci-mem";
         resets = <&gcc GCC_PCIE_BCR>;
         reset-names = "core";
         power-domains = <&gcc PCIE_GDSC>;
-- 
2.7.4

[PATCH v3 2/3] arm: dts: qcom: sdx55: Add interconnect path

[Krishna chaitanya chundru]

Add pcie-mem interconnect path to sdx55 target.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 arch/arm/boot/dts/qcom-sdx55.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-sdx55.dtsi b/arch/arm/boot/dts/qcom-sdx55.dtsi
index 342c3d1..38943d4 100644
--- a/arch/arm/boot/dts/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom-sdx55.dtsi
@@ -421,6 +421,10 @@
 				     <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "global",
 					  "doorbell";
+
+			interconnects = <&system_noc MASTER_PCIE_0 &mc_virt SLAVE_EBI1>;
+			interconnect-names = "pci-mem";
+
 			resets = <&gcc GCC_PCIE_BCR>;
 			reset-names = "core";
 			power-domains = <&gcc PCIE_GDSC>;
-- 
2.7.4

[PATCH v3 3/3] PCI: qcom-ep: Add ICC bandwidth voting support

[Krishna chaitanya chundru]

Add support to vote for ICC bandwidth based on the link speed and width.

This patch is inspired from pcie-qcom driver to add basic interconnect
support.

Link: https://lore.kernel.org/all/20221102090705.23634-1-johan+linaro@kernel.org/
Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-qcom-ep.c | 68 +++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 19b3283..baf831f 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -13,6 +13,7 @@
 #include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
+#include <linux/interconnect.h>
 #include <linux/mfd/syscon.h>
 #include <linux/phy/pcie.h>
 #include <linux/phy/phy.h>
@@ -28,6 +29,7 @@
 #define PARF_SYS_CTRL				0x00
 #define PARF_DB_CTRL				0x10
 #define PARF_PM_CTRL				0x20
+#define PARF_PM_STTS				0x24
 #define PARF_MHI_CLOCK_RESET_CTRL		0x174
 #define PARF_MHI_BASE_ADDR_LOWER		0x178
 #define PARF_MHI_BASE_ADDR_UPPER		0x17c
@@ -128,6 +130,9 @@
 /* DBI register fields */
 #define DBI_CON_STATUS_POWER_STATE_MASK		GENMASK(1, 0)
 
+#define DBI_LINKCTRLSTATUS			0x80
+#define DBI_LINKCTRLSTATUS_SHIFT		16
+
 #define XMLH_LINK_UP				0x400
 #define CORE_RESET_TIME_US_MIN			1000
 #define CORE_RESET_TIME_US_MAX			1005
@@ -178,6 +183,8 @@ struct qcom_pcie_ep {
 	struct phy *phy;
 	struct dentry *debugfs;
 
+	struct icc_path *icc_mem;
+
 	struct clk_bulk_data *clks;
 	int num_clks;
 
@@ -253,9 +260,51 @@ static void qcom_pcie_dw_stop_link(struct dw_pcie *pci)
 	disable_irq(pcie_ep->perst_irq);
 }
 
+static void qcom_pcie_ep_icc_update(struct qcom_pcie_ep *pcie_ep)
+{
+	struct dw_pcie *pci = &pcie_ep->pci;
+	u32 offset, status, bw;
+	int speed, width;
+	int ret;
+
+	if (!pcie_ep->icc_mem)
+		return;
+
+	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
+
+	speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status);
+	width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status);
+
+	switch (speed) {
+	case 1:
+		bw = MBps_to_icc(250);	/* BW for GEN1 per lane: 250MBps */
+		break;
+	case 2:
+		bw = MBps_to_icc(500);	/* BW for GEN2 per lane: 500MBps */
+		break;
+	case 3:
+		bw = MBps_to_icc(985);	/* BW for GEN3 per lane: 985MBps */
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		fallthrough;
+	case 4:
+		bw = MBps_to_icc(1969);	/* BW for GEN4 per lane:1969MBps */
+		break;
+	}
+
+	ret = icc_set_bw(pcie_ep->icc_mem, 0, width * bw);
+	if (ret) {
+		dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
+			ret);
+	}
+}
+
 static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)
 {
 	int ret;
+	struct dw_pcie *pci = &pcie_ep->pci;
 
 	ret = clk_bulk_prepare_enable(pcie_ep->num_clks, pcie_ep->clks);
 	if (ret)
@@ -277,6 +326,20 @@ static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)
 	if (ret)
 		goto err_phy_exit;
 
+	/*
+	 * Some Qualcomm platforms require interconnect bandwidth constraints
+	 * to be set before enabling interconnect clocks.
+	 *
+	 * Set an initial average bandwidth corresponding to GEN1x1(250 MBps)
+	 * for the pcie to mem path.
+	 */
+	ret = icc_set_bw(pcie_ep->icc_mem, 0, MBps_to_icc(250));
+	if (ret) {
+		dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
+			ret);
+		goto err_phy_exit;
+	}
+
 	return 0;
 
 err_phy_exit:
@@ -550,6 +613,10 @@ static int qcom_pcie_ep_get_resources(struct platform_device *pdev,
 	if (IS_ERR(pcie_ep->phy))
 		ret = PTR_ERR(pcie_ep->phy);
 
+	pcie_ep->icc_mem = devm_of_icc_get(dev, "pcie-mem");
+	if (IS_ERR(pcie_ep->icc_mem))
+		ret = PTR_ERR(pcie_ep->icc_mem);
+
 	return ret;
 }
 
@@ -572,6 +639,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
 	} else if (FIELD_GET(PARF_INT_ALL_BME, status)) {
 		dev_dbg(dev, "Received BME event. Link is enabled!\n");
 		pcie_ep->link_status = QCOM_PCIE_EP_LINK_ENABLED;
+		qcom_pcie_ep_icc_update(pcie_ep);
 	} else if (FIELD_GET(PARF_INT_ALL_PM_TURNOFF, status)) {
 		dev_dbg(dev, "Received PM Turn-off event! Entering L23\n");
 		val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
-- 
2.7.4

Re: [PATCH v3 1/3] dt-bindings: PCI: qcom: ep: Add interconnects path

[Konrad Dybcio]

On 9.06.2023 13:47, Krishna chaitanya chundru wrote:
> Some platforms may not boot if a device driver doesn't initialize
> the interconnect path. Mostly it is handled by the bootloader but
> we have starting to see cases where bootloader simply ignores them.
> 
> Add the "pcie-mem" interconnect path as a required property to the
> bindings.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
Hi, only patches 1 and 2 made it to both me and linux-arm-msm.

Consider using b4 (https://b4.docs.kernel.org/en/latest/index.html) to
avoid this.

>  Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
> index b3c22eb..656e362 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
> @@ -70,6 +70,13 @@ properties:
>      description: GPIO used as WAKE# output signal
>      maxItems: 1
>  
> +  interconnects:
> +    maxItems: 1
> +
> +  interconnect-names:
> +    items:
> +      - const: pcie-mem
> +
>    resets:
>      maxItems: 1
>  
> @@ -97,6 +104,8 @@ required:
>    - interrupts
>    - interrupt-names
>    - reset-gpios
> +  - interconnects
> +  - interconnect-names
>    - resets
>    - reset-names
>    - power-domains
> @@ -194,6 +203,8 @@ examples:
>          interrupt-names = "global", "doorbell";
>          reset-gpios = <&tlmm 57 GPIO_ACTIVE_LOW>;
>          wake-gpios = <&tlmm 53 GPIO_ACTIVE_LOW>;
> +	interconnects = <&system_noc MASTER_PCIE_0 &mc_virt SLAVE_EBI1>;
> +	interconnect-names = "pci-mem";
The indentation is off and my brain compiler says that it will not compile
without including some headers.

Konrad
>          resets = <&gcc GCC_PCIE_BCR>;
>          reset-names = "core";
>          power-domains = <&gcc PCIE_GDSC>;

Re: [PATCH v3 1/3] dt-bindings: PCI: qcom: ep: Add interconnects path

[Rob Herring]

On Fri, 09 Jun 2023 17:17:26 +0530, Krishna chaitanya chundru wrote:
> Some platforms may not boot if a device driver doesn't initialize
> the interconnect path. Mostly it is handled by the bootloader but
> we have starting to see cases where bootloader simply ignores them.
> 
> Add the "pcie-mem" interconnect path as a required property to the
> bindings.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml:206:1: [error] syntax error: found character '\t' that cannot start any token (syntax)

dtschema/dtc warnings/errors:
make[1]: *** Deleting file 'Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dts'
Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml:206:1: found a tab character where an indentation space is expected
make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml:206:1: found a tab character where an indentation space is expected
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml: ignoring, error parsing file
make: *** [Makefile:1512: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/1686311249-6857-2-git-send-email-quic_krichai@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v3 3/3] PCI: qcom-ep: Add ICC bandwidth voting support

[Dmitry Baryshkov]

On Fri, 9 Jun 2023 at 14:47, Krishna chaitanya chundru
<quic_krichai@quicinc.com> wrote:
>
> Add support to vote for ICC bandwidth based on the link speed and width.
>
> This patch is inspired from pcie-qcom driver to add basic interconnect
> support.
>
> Link: https://lore.kernel.org/all/20221102090705.23634-1-johan+linaro@kernel.org/

This link should be a part of the cover letter, not the commit msg. If
you want to refer to the previous commits, please use the standard
reference: commit abcdefabc ("PCI: qcom: Make foo and bar").

> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom-ep.c | 68 +++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 19b3283..baf831f 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -13,6 +13,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/interconnect.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/phy/pcie.h>
>  #include <linux/phy/phy.h>
> @@ -28,6 +29,7 @@
>  #define PARF_SYS_CTRL                          0x00
>  #define PARF_DB_CTRL                           0x10
>  #define PARF_PM_CTRL                           0x20
> +#define PARF_PM_STTS                           0x24
>  #define PARF_MHI_CLOCK_RESET_CTRL              0x174
>  #define PARF_MHI_BASE_ADDR_LOWER               0x178
>  #define PARF_MHI_BASE_ADDR_UPPER               0x17c
> @@ -128,6 +130,9 @@
>  /* DBI register fields */
>  #define DBI_CON_STATUS_POWER_STATE_MASK                GENMASK(1, 0)
>
> +#define DBI_LINKCTRLSTATUS                     0x80
> +#define DBI_LINKCTRLSTATUS_SHIFT               16
> +
>  #define XMLH_LINK_UP                           0x400
>  #define CORE_RESET_TIME_US_MIN                 1000
>  #define CORE_RESET_TIME_US_MAX                 1005
> @@ -178,6 +183,8 @@ struct qcom_pcie_ep {
>         struct phy *phy;
>         struct dentry *debugfs;
>
> +       struct icc_path *icc_mem;
> +
>         struct clk_bulk_data *clks;
>         int num_clks;
>
> @@ -253,9 +260,51 @@ static void qcom_pcie_dw_stop_link(struct dw_pcie *pci)
>         disable_irq(pcie_ep->perst_irq);
>  }
>
> +static void qcom_pcie_ep_icc_update(struct qcom_pcie_ep *pcie_ep)
> +{
> +       struct dw_pcie *pci = &pcie_ep->pci;
> +       u32 offset, status, bw;
> +       int speed, width;
> +       int ret;
> +
> +       if (!pcie_ep->icc_mem)
> +               return;
> +
> +       offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +       status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
> +
> +       speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status);
> +       width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status);
> +
> +       switch (speed) {
> +       case 1:
> +               bw = MBps_to_icc(250);  /* BW for GEN1 per lane: 250MBps */

Please extract these constants to the defines. This would save you
from duplicating 250 below.

> +               break;
> +       case 2:
> +               bw = MBps_to_icc(500);  /* BW for GEN2 per lane: 500MBps */
> +               break;
> +       case 3:
> +               bw = MBps_to_icc(985);  /* BW for GEN3 per lane: 985MBps */
> +               break;
> +       default:
> +               WARN_ON_ONCE(1);
> +               fallthrough;
> +       case 4:
> +               bw = MBps_to_icc(1969); /* BW for GEN4 per lane:1969MBps */
> +               break;
> +       }
> +
> +       ret = icc_set_bw(pcie_ep->icc_mem, 0, width * bw);
> +       if (ret) {
> +               dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
> +                       ret);
> +       }
> +}
> +
>  static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)
>  {
>         int ret;
> +       struct dw_pcie *pci = &pcie_ep->pci;
>
>         ret = clk_bulk_prepare_enable(pcie_ep->num_clks, pcie_ep->clks);
>         if (ret)
> @@ -277,6 +326,20 @@ static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)
>         if (ret)
>                 goto err_phy_exit;
>
> +       /*
> +        * Some Qualcomm platforms require interconnect bandwidth constraints
> +        * to be set before enabling interconnect clocks.
> +        *
> +        * Set an initial average bandwidth corresponding to GEN1x1(250 MBps)
> +        * for the pcie to mem path.
> +        */
> +       ret = icc_set_bw(pcie_ep->icc_mem, 0, MBps_to_icc(250));
> +       if (ret) {
> +               dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
> +                       ret);
> +               goto err_phy_exit;
> +       }
> +
>         return 0;
>
>  err_phy_exit:
> @@ -550,6 +613,10 @@ static int qcom_pcie_ep_get_resources(struct platform_device *pdev,
>         if (IS_ERR(pcie_ep->phy))
>                 ret = PTR_ERR(pcie_ep->phy);
>
> +       pcie_ep->icc_mem = devm_of_icc_get(dev, "pcie-mem");
> +       if (IS_ERR(pcie_ep->icc_mem))
> +               ret = PTR_ERR(pcie_ep->icc_mem);
> +
>         return ret;
>  }
>
> @@ -572,6 +639,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
>         } else if (FIELD_GET(PARF_INT_ALL_BME, status)) {
>                 dev_dbg(dev, "Received BME event. Link is enabled!\n");
>                 pcie_ep->link_status = QCOM_PCIE_EP_LINK_ENABLED;
> +               qcom_pcie_ep_icc_update(pcie_ep);
>         } else if (FIELD_GET(PARF_INT_ALL_PM_TURNOFF, status)) {
>                 dev_dbg(dev, "Received PM Turn-off event! Entering L23\n");
>                 val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
> --
> 2.7.4
>


-- 
With best wishes
Dmitry

Re: [PATCH v3 1/3] dt-bindings: PCI: qcom: ep: Add interconnects path

[Krishna Chaitanya Chundru]

On 6/9/2023 6:43 PM, Rob Herring wrote:
> On Fri, 09 Jun 2023 17:17:26 +0530, Krishna chaitanya chundru wrote:
>> Some platforms may not boot if a device driver doesn't initialize
>> the interconnect path. Mostly it is handled by the bootloader but
>> we have starting to see cases where bootloader simply ignores them.
>>
>> Add the "pcie-mem" interconnect path as a required property to the
>> bindings.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml:206:1: [error] syntax error: found character '\t' that cannot start any token (syntax)
>
> dtschema/dtc warnings/errors:
> make[1]: *** Deleting file 'Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dts'
> Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml:206:1: found a tab character where an indentation space is expected
> make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dts] Error 1
> make[1]: *** Waiting for unfinished jobs....
> ./Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml:206:1: found a tab character where an indentation space is expected
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml: ignoring, error parsing file
> make: *** [Makefile:1512: dt_binding_check] Error 2
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/1686311249-6857-2-git-send-email-quic_krichai@quicinc.com
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>
sorry for sending the patch without checking the patch. fixed the errors 
and sent new patch

Re: [PATCH v3 1/3] dt-bindings: PCI: qcom: ep: Add interconnects path

[Krishna Chaitanya Chundru]

On 6/9/2023 6:43 PM, Rob Herring wrote:
> On Fri, 09 Jun 2023 17:17:26 +0530, Krishna chaitanya chundru wrote:
>> Some platforms may not boot if a device driver doesn't initialize
>> the interconnect path. Mostly it is handled by the bootloader but
>> we have starting to see cases where bootloader simply ignores them.
>>
>> Add the "pcie-mem" interconnect path as a required property to the
>> bindings.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml:206:1: [error] syntax error: found character '\t' that cannot start any token (syntax)
>
> dtschema/dtc warnings/errors:
> make[1]: *** Deleting file 'Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dts'
> Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml:206:1: found a tab character where an indentation space is expected
> make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dts] Error 1
> make[1]: *** Waiting for unfinished jobs....
> ./Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml:206:1: found a tab character where an indentation space is expected
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml: ignoring, error parsing file
> make: *** [Makefile:1512: dt_binding_check] Error 2
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/1686311249-6857-2-git-send-email-quic_krichai@quicinc.com
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>
Sorry for sending patch without checking for errors, I fixed the errors 
in the new patch.

Re: [PATCH v3 3/3] PCI: qcom-ep: Add ICC bandwidth voting support

[Krishna Chaitanya Chundru]

On 6/9/2023 9:56 PM, Dmitry Baryshkov wrote:
> On Fri, 9 Jun 2023 at 14:47, Krishna chaitanya chundru
> <quic_krichai@quicinc.com> wrote:
>> Add support to vote for ICC bandwidth based on the link speed and width.
>>
>> This patch is inspired from pcie-qcom driver to add basic interconnect
>> support.
>>
>> Link: https://lore.kernel.org/all/20221102090705.23634-1-johan+linaro@kernel.org/
> This link should be a part of the cover letter, not the commit msg. If
> you want to refer to the previous commits, please use the standard
> reference: commit abcdefabc ("PCI: qcom: Make foo and bar").
done
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom-ep.c | 68 +++++++++++++++++++++++++++++++
>>   1 file changed, 68 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> index 19b3283..baf831f 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> @@ -13,6 +13,7 @@
>>   #include <linux/debugfs.h>
>>   #include <linux/delay.h>
>>   #include <linux/gpio/consumer.h>
>> +#include <linux/interconnect.h>
>>   #include <linux/mfd/syscon.h>
>>   #include <linux/phy/pcie.h>
>>   #include <linux/phy/phy.h>
>> @@ -28,6 +29,7 @@
>>   #define PARF_SYS_CTRL                          0x00
>>   #define PARF_DB_CTRL                           0x10
>>   #define PARF_PM_CTRL                           0x20
>> +#define PARF_PM_STTS                           0x24
>>   #define PARF_MHI_CLOCK_RESET_CTRL              0x174
>>   #define PARF_MHI_BASE_ADDR_LOWER               0x178
>>   #define PARF_MHI_BASE_ADDR_UPPER               0x17c
>> @@ -128,6 +130,9 @@
>>   /* DBI register fields */
>>   #define DBI_CON_STATUS_POWER_STATE_MASK                GENMASK(1, 0)
>>
>> +#define DBI_LINKCTRLSTATUS                     0x80
>> +#define DBI_LINKCTRLSTATUS_SHIFT               16
>> +
>>   #define XMLH_LINK_UP                           0x400
>>   #define CORE_RESET_TIME_US_MIN                 1000
>>   #define CORE_RESET_TIME_US_MAX                 1005
>> @@ -178,6 +183,8 @@ struct qcom_pcie_ep {
>>          struct phy *phy;
>>          struct dentry *debugfs;
>>
>> +       struct icc_path *icc_mem;
>> +
>>          struct clk_bulk_data *clks;
>>          int num_clks;
>>
>> @@ -253,9 +260,51 @@ static void qcom_pcie_dw_stop_link(struct dw_pcie *pci)
>>          disable_irq(pcie_ep->perst_irq);
>>   }
>>
>> +static void qcom_pcie_ep_icc_update(struct qcom_pcie_ep *pcie_ep)
>> +{
>> +       struct dw_pcie *pci = &pcie_ep->pci;
>> +       u32 offset, status, bw;
>> +       int speed, width;
>> +       int ret;
>> +
>> +       if (!pcie_ep->icc_mem)
>> +               return;
>> +
>> +       offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>> +       status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
>> +
>> +       speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status);
>> +       width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status);
>> +
>> +       switch (speed) {
>> +       case 1:
>> +               bw = MBps_to_icc(250);  /* BW for GEN1 per lane: 250MBps */
> Please extract these constants to the defines. This would save you
> from duplicating 250 below.
done
>> +               break;
>> +       case 2:
>> +               bw = MBps_to_icc(500);  /* BW for GEN2 per lane: 500MBps */
>> +               break;
>> +       case 3:
>> +               bw = MBps_to_icc(985);  /* BW for GEN3 per lane: 985MBps */
>> +               break;
>> +       default:
>> +               WARN_ON_ONCE(1);
>> +               fallthrough;
>> +       case 4:
>> +               bw = MBps_to_icc(1969); /* BW for GEN4 per lane:1969MBps */
>> +               break;
>> +       }
>> +
>> +       ret = icc_set_bw(pcie_ep->icc_mem, 0, width * bw);
>> +       if (ret) {
>> +               dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
>> +                       ret);
>> +       }
>> +}
>> +
>>   static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)
>>   {
>>          int ret;
>> +       struct dw_pcie *pci = &pcie_ep->pci;
>>
>>          ret = clk_bulk_prepare_enable(pcie_ep->num_clks, pcie_ep->clks);
>>          if (ret)
>> @@ -277,6 +326,20 @@ static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)
>>          if (ret)
>>                  goto err_phy_exit;
>>
>> +       /*
>> +        * Some Qualcomm platforms require interconnect bandwidth constraints
>> +        * to be set before enabling interconnect clocks.
>> +        *
>> +        * Set an initial average bandwidth corresponding to GEN1x1(250 MBps)
>> +        * for the pcie to mem path.
>> +        */
>> +       ret = icc_set_bw(pcie_ep->icc_mem, 0, MBps_to_icc(250));
>> +       if (ret) {
>> +               dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
>> +                       ret);
>> +               goto err_phy_exit;
>> +       }
>> +
>>          return 0;
>>
>>   err_phy_exit:
>> @@ -550,6 +613,10 @@ static int qcom_pcie_ep_get_resources(struct platform_device *pdev,
>>          if (IS_ERR(pcie_ep->phy))
>>                  ret = PTR_ERR(pcie_ep->phy);
>>
>> +       pcie_ep->icc_mem = devm_of_icc_get(dev, "pcie-mem");
>> +       if (IS_ERR(pcie_ep->icc_mem))
>> +               ret = PTR_ERR(pcie_ep->icc_mem);
>> +
>>          return ret;
>>   }
>>
>> @@ -572,6 +639,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
>>          } else if (FIELD_GET(PARF_INT_ALL_BME, status)) {
>>                  dev_dbg(dev, "Received BME event. Link is enabled!\n");
>>                  pcie_ep->link_status = QCOM_PCIE_EP_LINK_ENABLED;
>> +               qcom_pcie_ep_icc_update(pcie_ep);
>>          } else if (FIELD_GET(PARF_INT_ALL_PM_TURNOFF, status)) {
>>                  dev_dbg(dev, "Received PM Turn-off event! Entering L23\n");
>>                  val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
>> --
>> 2.7.4
>>
>