Subject: [PATCH v10 0/4] PCI: qcom: ep: Add basic interconnect support

Add basic support for managing "pcie-mem" interconnect path by setting
a low constraint before enabling clocks and updating it after the link
is up based on link speed and width the device got enumerated.

changes from v9:
- addressed the comments by mani.
changes from v8:
- Added cpu to pcie path in dtsi and in dtsi binding.
changes from v7:
- setting icc bw to '0' in disable resources as suggested by mani.
changes from v6:
- addressed the comments as suggested by mani.
changes from v5:
- addressed the comments by mani.
changes from v4:
- rebased with linux-next.
- Added comments as suggested by mani.
- removed the arm: dts: qcom: sdx55: Add interconnect path
as that patch is already applied.
changes from v3:
- ran make DT_CHECKER_FLAGS=-m dt_binding_check and fixed
errors.
- Added macros in the qcom ep driver patch as suggested by Dmitry
changes from v2:
- changed the logic for getting speed and width as suggested
by bjorn.
- fixed compilation errors.


Krishna chaitanya chundru (4):
PCI: qcom-ep: Add ICC bandwidth voting support
arm: dts: qcom: sdx65: Add PCIe EP interconnect path
arm: dts: qcom: sdx55: Add CPU PCIe EP interconnect path
dt-bindings: PCI: qcom: ep: Add interconnects path

.../devicetree/bindings/pci/qcom,pcie-ep.yaml | 15 +++++
arch/arm/boot/dts/qcom/qcom-sdx55.dtsi | 5 +-
arch/arm/boot/dts/qcom/qcom-sdx65.dtsi | 4 ++
drivers/pci/controller/dwc/pcie-qcom-ep.c | 71 ++++++++++++++++++++++
4 files changed, 93 insertions(+), 2 deletions(-)

--
2.7.4



Subject: [PATCH v10 4/4] PCI: qcom-ep: Add ICC bandwidth voting support

Add support for voting interconnect (ICC) bandwidth based
on the link speed and width.

This commit is inspired from the basic interconnect support added
to pcie-qcom driver in commit c4860af88d0c ("PCI: qcom: Add basic
interconnect support").

The interconnect support is kept optional to be backward compatible
with legacy devicetrees.

Reviewed-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Krishna chaitanya chundru <[email protected]>
---
drivers/pci/controller/dwc/pcie-qcom-ep.c | 71 +++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 0fe7f06..7460b3f 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>
@@ -133,6 +134,11 @@
#define CORE_RESET_TIME_US_MAX 1005
#define WAKE_DELAY_US 2000 /* 2 ms */

+#define PCIE_GEN1_BW_MBPS 250
+#define PCIE_GEN2_BW_MBPS 500
+#define PCIE_GEN3_BW_MBPS 985
+#define PCIE_GEN4_BW_MBPS 1969
+
#define to_pcie_ep(x) dev_get_drvdata((x)->dev)

enum qcom_pcie_ep_link_status {
@@ -178,6 +184,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,8 +261,49 @@ 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(PCIE_GEN1_BW_MBPS);
+ break;
+ case 2:
+ bw = MBps_to_icc(PCIE_GEN2_BW_MBPS);
+ break;
+ case 3:
+ bw = MBps_to_icc(PCIE_GEN3_BW_MBPS);
+ break;
+ default:
+ dev_warn(pci->dev, "using default GEN4 bandwidth\n");
+ fallthrough;
+ case 4:
+ bw = MBps_to_icc(PCIE_GEN4_BW_MBPS);
+ 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)
{
+ struct dw_pcie *pci = &pcie_ep->pci;
int ret;

ret = clk_bulk_prepare_enable(pcie_ep->num_clks, pcie_ep->clks);
@@ -277,8 +326,24 @@ 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 peak bandwidth corresponding to single-lane Gen 1
+ * for the pcie-mem path.
+ */
+ ret = icc_set_bw(pcie_ep->icc_mem, 0, MBps_to_icc(PCIE_GEN1_BW_MBPS));
+ if (ret) {
+ dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
+ ret);
+ goto err_phy_off;
+ }
+
return 0;

+err_phy_off:
+ phy_power_off(pcie_ep->phy);
err_phy_exit:
phy_exit(pcie_ep->phy);
err_disable_clk:
@@ -289,6 +354,7 @@ static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)

static void qcom_pcie_disable_resources(struct qcom_pcie_ep *pcie_ep)
{
+ icc_set_bw(pcie_ep->icc_mem, 0, 0);
phy_power_off(pcie_ep->phy);
phy_exit(pcie_ep->phy);
clk_bulk_disable_unprepare(pcie_ep->num_clks, pcie_ep->clks);
@@ -550,6 +616,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;
}

@@ -573,6 +643,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);
pci_epc_bme_notify(pci->ep.epc);
} else if (FIELD_GET(PARF_INT_ALL_PM_TURNOFF, status)) {
dev_dbg(dev, "Received PM Turn-off event! Entering L23\n");
--
2.7.4


Subject: [PATCH v10 1/4] dt-bindings: PCI: qcom: ep: Add interconnects path

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" & "cpu-pcie" interconnect path as a required
property to the bindings.

Signed-off-by: Krishna chaitanya chundru <[email protected]>
Acked-by: Manivannan Sadhasivam <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
index 8111122..e553341 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
@@ -71,6 +71,14 @@ properties:
description: GPIO used as WAKE# output signal
maxItems: 1

+ interconnects:
+ maxItems: 2
+
+ interconnect-names:
+ items:
+ - const: pcie-mem
+ - const: cpu-pcie
+
resets:
maxItems: 1

@@ -98,6 +106,8 @@ required:
- interrupts
- interrupt-names
- reset-gpios
+ - interconnects
+ - interconnect-names
- resets
- reset-names
- power-domains
@@ -167,7 +177,9 @@ examples:
- |
#include <dt-bindings/clock/qcom,gcc-sdx55.h>
#include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interconnect/qcom,sdx55.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
+
pcie_ep: pcie-ep@1c00000 {
compatible = "qcom,sdx55-pcie-ep";
reg = <0x01c00000 0x3000>,
@@ -194,6 +206,9 @@ examples:
interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>;
interrupt-names = "global", "doorbell";
+ interconnects = <&system_noc MASTER_PCIE &mc_virt SLAVE_EBI_CH0>,
+ <&mem_noc MASTER_AMPSS_M0 &system_noc SLAVE_PCIE_0>;
+ interconnect-names = "pcie-mem", "cpu-pcie";
reset-gpios = <&tlmm 57 GPIO_ACTIVE_LOW>;
wake-gpios = <&tlmm 53 GPIO_ACTIVE_LOW>;
resets = <&gcc GCC_PCIE_BCR>;
--
2.7.4


Subject: [PATCH v10 2/4] arm: dts: qcom: sdx65: Add PCIe EP interconnect path

Add pcie-mem & cpu-pcie interconnect path ifor PCIe EP to sdx65 platform.

Reviewed-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Krishna chaitanya chundru <[email protected]>
---
arch/arm/boot/dts/qcom/qcom-sdx65.dtsi | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-sdx65.dtsi b/arch/arm/boot/dts/qcom/qcom-sdx65.dtsi
index 1a35830..69fe7e5 100644
--- a/arch/arm/boot/dts/qcom/qcom-sdx65.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-sdx65.dtsi
@@ -332,6 +332,10 @@
<GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>;
interrupt-names = "global", "doorbell";

+ interconnects = <&system_noc MASTER_PCIE_0 &mc_virt SLAVE_EBI1>,
+ <&mem_noc MASTER_APPSS_PROC &system_noc SLAVE_PCIE_0>;
+ interconnect-names = "pcie-mem", "cpu-pcie";
+
resets = <&gcc GCC_PCIE_BCR>;
reset-names = "core";

--
2.7.4


Subject: [PATCH v10 3/4] arm: dts: qcom: sdx55: Add CPU PCIe EP interconnect path

Add cpu-pcie interconnect path for PCIe EP to sdx55 platform.

Signed-off-by: Krishna chaitanya chundru <[email protected]>
---
arch/arm/boot/dts/qcom/qcom-sdx55.dtsi | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi b/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi
index df3cd9c..a7c0c26 100644
--- a/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi
@@ -422,8 +422,9 @@
interrupt-names = "global",
"doorbell";

- interconnects = <&system_noc MASTER_PCIE &mc_virt SLAVE_EBI_CH0>;
- interconnect-names = "pcie-mem";
+ interconnects = <&system_noc MASTER_PCIE &mc_virt SLAVE_EBI_CH0>,
+ <&mem_noc MASTER_AMPSS_M0 &system_noc SLAVE_PCIE_0>;
+ interconnect-names = "pcie-mem", "cpu-pcie";

resets = <&gcc GCC_PCIE_BCR>;
reset-names = "core";
--
2.7.4


2023-07-19 08:15:21

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v10 3/4] arm: dts: qcom: sdx55: Add CPU PCIe EP interconnect path

On Wed, Jul 19, 2023 at 12:50:17PM +0530, Krishna chaitanya chundru wrote:
> Add cpu-pcie interconnect path for PCIe EP to sdx55 platform.
>
> Signed-off-by: Krishna chaitanya chundru <[email protected]>

Reviewed-by: Manivannan Sadhasivam <[email protected]>

- Mani

> ---
> arch/arm/boot/dts/qcom/qcom-sdx55.dtsi | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi b/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi
> index df3cd9c..a7c0c26 100644
> --- a/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi
> +++ b/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi
> @@ -422,8 +422,9 @@
> interrupt-names = "global",
> "doorbell";
>
> - interconnects = <&system_noc MASTER_PCIE &mc_virt SLAVE_EBI_CH0>;
> - interconnect-names = "pcie-mem";
> + interconnects = <&system_noc MASTER_PCIE &mc_virt SLAVE_EBI_CH0>,
> + <&mem_noc MASTER_AMPSS_M0 &system_noc SLAVE_PCIE_0>;
> + interconnect-names = "pcie-mem", "cpu-pcie";
>
> resets = <&gcc GCC_PCIE_BCR>;
> reset-names = "core";
> --
> 2.7.4
>

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

2023-07-28 03:54:04

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v10 0/4] PCI: qcom: ep: Add basic interconnect support

On Wed, Jul 19, 2023 at 12:50:14PM +0530, Krishna chaitanya chundru wrote:
> Add basic support for managing "pcie-mem" interconnect path by setting
> a low constraint before enabling clocks and updating it after the link
> is up based on link speed and width the device got enumerated.
>

Krzysztof, can this series be merged for 6.6? Bjorn A will pick the dts patches.

- Mani

> changes from v9:
> - addressed the comments by mani.
> changes from v8:
> - Added cpu to pcie path in dtsi and in dtsi binding.
> changes from v7:
> - setting icc bw to '0' in disable resources as suggested by mani.
> changes from v6:
> - addressed the comments as suggested by mani.
> changes from v5:
> - addressed the comments by mani.
> changes from v4:
> - rebased with linux-next.
> - Added comments as suggested by mani.
> - removed the arm: dts: qcom: sdx55: Add interconnect path
> as that patch is already applied.
> changes from v3:
> - ran make DT_CHECKER_FLAGS=-m dt_binding_check and fixed
> errors.
> - Added macros in the qcom ep driver patch as suggested by Dmitry
> changes from v2:
> - changed the logic for getting speed and width as suggested
> by bjorn.
> - fixed compilation errors.
>
>
> Krishna chaitanya chundru (4):
> PCI: qcom-ep: Add ICC bandwidth voting support
> arm: dts: qcom: sdx65: Add PCIe EP interconnect path
> arm: dts: qcom: sdx55: Add CPU PCIe EP interconnect path
> dt-bindings: PCI: qcom: ep: Add interconnects path
>
> .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 15 +++++
> arch/arm/boot/dts/qcom/qcom-sdx55.dtsi | 5 +-
> arch/arm/boot/dts/qcom/qcom-sdx65.dtsi | 4 ++
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 71 ++++++++++++++++++++++
> 4 files changed, 93 insertions(+), 2 deletions(-)
>
> --
> 2.7.4
>

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

2023-08-29 12:59:26

by Krzysztof Wilczyński

[permalink] [raw]
Subject: Re: [PATCH v10 0/4] PCI: qcom: ep: Add basic interconnect support

Hello,

> > Add basic support for managing "pcie-mem" interconnect path by setting
> > a low constraint before enabling clocks and updating it after the link
> > is up based on link speed and width the device got enumerated.
> >
>
> Krzysztof, can this series be merged for 6.6? Bjorn A will pick the dts patches.

Applied. Sorry for the delay.

That said, it's now up to Bjorn (Bjorn Helgaas) to take it, or let it slide
for 6.7, depending on how far he has got with his Pull Request.

Krzysztof

2023-08-29 13:36:07

by Krzysztof Wilczyński

[permalink] [raw]
Subject: Re: [PATCH v10 0/4] PCI: qcom: ep: Add basic interconnect support

Hello,

> Add basic support for managing "pcie-mem" interconnect path by setting
> a low constraint before enabling clocks and updating it after the link
> is up based on link speed and width the device got enumerated.

Applied to controller/qcom-ep, thank you!

[1/4] dt-bindings: PCI: qcom: ep: Add interconnects path
https://git.kernel.org/pci/pci/c/e590ad240ff0
[4/4] PCI: qcom-ep: Add ICC bandwidth voting support
https://git.kernel.org/pci/pci/c/0c104996e6a8

Krzysztof

Subject: Re: [PATCH v10 0/4] PCI: qcom: ep: Add basic interconnect support


On 8/29/2023 5:42 PM, Krzysztof Kozlowski wrote:
> On 29/08/2023 14:10, Krishna Chaitanya Chundru wrote:
>>>>> Krzysztof, can this series be merged for 6.6? Bjorn A will pick the dts patches.
>>>>>
>>>>> - Mani
>>>> A Gentle ping
>>>>
>>> Whom do you ping and why me? If you choose not to use
>>> scripts/get_maintainers.pl, it's your right, but then you might get
>>> maintainers wrong and no surprise patches got not accepted...
>>>
>>> Plus, it's merge window, so why pinging now?
>>>
>>> Best regards,
>>> Krzysztof
>> Krzyszto,
>>
>> The series is already reviewed and there are some patches which is
>> reviewed by you also.
>>
>> I am using this command to send patches it looks like this script is
>> fetching based upon the source file where there was change due to that
>> only I was seeing the problem of all patches are not going to all the
>> maintainers.
>>
>>  --cc-cmd=scripts/get_maintainer.pl ./patch-series
>>
>> I was trying to install b4 and make sure to send all patches to all the
>> maintainers next time on wards.
> This looks good.
>
>> we pinged it now so that as this is already reviewed and no comments on
>> this series so that this can picked up.
> And what is has anything to do with me? You got everything needed from
> me, don't you?
>
> Anyway, do not ping during merge window.
>
>
> Best regards,
> Krzysztof

I meant to ping all maintainers I should pinged as new reply to cover
message instead of replying back to mani's comment.

sure Krzysztof we will make sure we don't ping in merge window.

- KC


I

>

2023-08-29 14:07:46

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v10 0/4] PCI: qcom: ep: Add basic interconnect support

On Tue, Aug 29, 2023 at 05:40:10PM +0530, Krishna Chaitanya Chundru wrote:
>
> On 8/29/2023 5:26 PM, Krzysztof Kozlowski wrote:
> > On 29/08/2023 13:41, Krishna Chaitanya Chundru wrote:
> > > On 7/28/2023 8:26 AM, Manivannan Sadhasivam wrote:
> > > > On Wed, Jul 19, 2023 at 12:50:14PM +0530, Krishna chaitanya chundru wrote:
> > > > > Add basic support for managing "pcie-mem" interconnect path by setting
> > > > > a low constraint before enabling clocks and updating it after the link
> > > > > is up based on link speed and width the device got enumerated.
> > > > >
> > > > Krzysztof, can this series be merged for 6.6? Bjorn A will pick the dts patches.
> > > >
> > > > - Mani
> > > A Gentle ping
> > >
> > Whom do you ping and why me? If you choose not to use
> > scripts/get_maintainers.pl, it's your right, but then you might get
> > maintainers wrong and no surprise patches got not accepted...
> >
> > Plus, it's merge window, so why pinging now?
> >
> > Best regards,
> > Krzysztof
>
> Krzyszto,
>
> The series is already reviewed and there are some patches which is reviewed
> by you also.
>
> I am using this command to send patches it looks like this script is
> fetching based upon the source file where there was change due to that only
> I was seeing the problem of all patches are not going to all the
> maintainers.
>
>  --cc-cmd=scripts/get_maintainer.pl ./patch-series
>
> I was trying to install b4 and make sure to send all patches to all the
> maintainers next time on wards.
>
> we pinged it now so that as this is already reviewed and no comments on this
> series so that this can picked up.
>

FWIW, it was me who pinged Krzysztof first. But I was referring to Krzysztof
Wilczyński, who is the PCI controller drivers maintainer. I thought you CCed all
the relevant people, but just noticed that you didn't :/ Please fix it as you
promised.

Anyway, since the merge window is now open, this series has to wait for 6.7.

- Mani

>
> - KC

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