Subject: [PATCH v1 0/3] PCI: qcom: sc7280: add missing clocks

Add missing pcie clocks support.

Krishna chaitanya chundru (3):
PCI: qcom: Add missing sc7280 clocks in PCIe driver
dt-bindings: pci: QCOM sc7280 add missing clocks.
arm64: dts: qcom: sc7280: Add missing pcie clocks

Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 ++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 4 ++++
drivers/pci/controller/dwc/pcie-qcom.c | 3 +++
3 files changed, 9 insertions(+)

--
2.7.4


Subject: [PATCH v1 3/3] arm64: dts: qcom: sc7280: Add missing pcie clocks

Add missing pcie clocks.

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

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index e66fc67..a5ce095 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2043,6 +2043,8 @@
<&gcc GCC_PCIE_1_SLV_AXI_CLK>,
<&gcc GCC_PCIE_1_SLV_Q2A_AXI_CLK>,
<&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>,
+ <&gcc GCC_AGGRE_NOC_PCIE_CENTER_SF_AXI_CLK>,
+ <&gcc GCC_AGGRE_NOC_PCIE_1_AXI_CLK>,
<&gcc GCC_DDRSS_PCIE_SF_CLK>;

clock-names = "pipe",
@@ -2055,6 +2057,8 @@
"bus_slave",
"slave_q2a",
"tbu",
+ "aggre0",
+ "aggre1",
"ddrss_sf_tbu";

assigned-clocks = <&gcc GCC_PCIE_1_AUX_CLK>;
--
2.7.4

2022-06-24 15:26:35

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v1 3/3] arm64: dts: qcom: sc7280: Add missing pcie clocks

On 24/06/2022 11:19, Krishna chaitanya chundru wrote:
> Add missing pcie clocks.
>
> Signed-off-by: Krishna chaitanya chundru <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 4 ++++
> 1 file changed, 4 insertions(+)
>

Thank you for your patch. There is something to discuss/improve.

> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index e66fc67..a5ce095 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -2043,6 +2043,8 @@
> <&gcc GCC_PCIE_1_SLV_AXI_CLK>,
> <&gcc GCC_PCIE_1_SLV_Q2A_AXI_CLK>,
> <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>,
> + <&gcc GCC_AGGRE_NOC_PCIE_CENTER_SF_AXI_CLK>,
> + <&gcc GCC_AGGRE_NOC_PCIE_1_AXI_CLK>,
> <&gcc GCC_DDRSS_PCIE_SF_CLK>;
>
> clock-names = "pipe",
> @@ -2055,6 +2057,8 @@
> "bus_slave",
> "slave_q2a",
> "tbu",
> + "aggre0",
> + "aggre1",
> "ddrss_sf_tbu";

Unfortunately wrong order.

Please test your patches with `make dtbs_check`.


Best regards,
Krzysztof

Subject: [PATCH v2 0/3] PCI: qcom: sc7280: add missing aggre0, aggre1 and ddrs sf tbu clocks

Add missing aggre0, aggre1 and ddrs sf tbu clocks supports to PCIe node.

Without voting these clocks, PCIe link is going down when system is
suspended as these clocks can get turned off as no-one is voting for them.

Krishna chaitanya chundru (3):
PCI: qcom: Add sc7280 aggre0, aggre1 and ddr sf tbu clocks in PCIe
driver
dt-bindings: pci: QCOM Add missing sc7280 aggre0, aggre1 clocks
arm64: dts: qcom: sc7280: Add missing aggre0, aggre1 clocks

Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 6 ++++--
arch/arm64/boot/dts/qcom/sc7280.dtsi | 4 ++++
drivers/pci/controller/dwc/pcie-qcom.c | 3 +++
3 files changed, 11 insertions(+), 2 deletions(-)

--
2.7.4

Subject: [PATCH v2 3/3] arm64: dts: qcom: sc7280: Add missing aggre0, aggre1 clocks

Add missing aggre0, aggre1 clocks to pcie node.

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

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index e66fc67..a5ce095 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2043,6 +2043,8 @@
<&gcc GCC_PCIE_1_SLV_AXI_CLK>,
<&gcc GCC_PCIE_1_SLV_Q2A_AXI_CLK>,
<&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>,
+ <&gcc GCC_AGGRE_NOC_PCIE_CENTER_SF_AXI_CLK>,
+ <&gcc GCC_AGGRE_NOC_PCIE_1_AXI_CLK>,
<&gcc GCC_DDRSS_PCIE_SF_CLK>;

clock-names = "pipe",
@@ -2055,6 +2057,8 @@
"bus_slave",
"slave_q2a",
"tbu",
+ "aggre0",
+ "aggre1",
"ddrss_sf_tbu";

assigned-clocks = <&gcc GCC_PCIE_1_AUX_CLK>;
--
2.7.4

Subject: [PATCH v2 2/3] dt-bindings: pci: QCOM Adding sc7280 aggre0, aggre1 clocks

Adding aggre0 and aggre1 clock entries to PCIe node.

Signed-off-by: Krishna chaitanya chundru <[email protected]>
---
Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 0b69b12..8f29bdd 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -423,8 +423,8 @@ allOf:
then:
properties:
clocks:
- minItems: 11
- maxItems: 11
+ minItems: 13
+ maxItems: 13
clock-names:
items:
- const: pipe # PIPE clock
@@ -437,6 +437,8 @@ allOf:
- const: bus_slave # Slave AXI clock
- const: slave_q2a # Slave Q2A clock
- const: tbu # PCIe TBU clock
+ - const: aggre0 # Aggre NoC PCIe CENTER SF AXI clock
+ - const: aggre1 # Aggre NoC PCIe1 AXI clock
- const: ddrss_sf_tbu # PCIe SF TBU clock
resets:
maxItems: 1
--
2.7.4

2022-07-04 08:40:39

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] dt-bindings: pci: QCOM Adding sc7280 aggre0, aggre1 clocks

On 01/07/2022 18:11, Krishna chaitanya chundru wrote:
> Adding aggre0 and aggre1 clock entries to PCIe node.
>
> Signed-off-by: Krishna chaitanya chundru <[email protected]>
> ---
> Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 0b69b12..8f29bdd 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -423,8 +423,8 @@ allOf:
> then:
> properties:
> clocks:
> - minItems: 11
> - maxItems: 11
> + minItems: 13
> + maxItems: 13
> clock-names:
> items:
> - const: pipe # PIPE clock
> @@ -437,6 +437,8 @@ allOf:
> - const: bus_slave # Slave AXI clock
> - const: slave_q2a # Slave Q2A clock
> - const: tbu # PCIe TBU clock
> + - const: aggre0 # Aggre NoC PCIe CENTER SF AXI clock
> + - const: aggre1 # Aggre NoC PCIe1 AXI clock

You ignored my comments from v1 - please don't. This is not accepted.

Also, please do not send new versions of patchset as reply to some other
threads. It's extremely confusing to find it under something else.

Best regards,
Krzysztof

2022-07-04 09:01:26

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] arm64: dts: qcom: sc7280: Add missing aggre0, aggre1 clocks

On 01/07/2022 18:11, Krishna chaitanya chundru wrote:
> Add missing aggre0, aggre1 clocks to pcie node.
>
> Signed-off-by: Krishna chaitanya chundru <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index e66fc67..a5ce095 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -2043,6 +2043,8 @@
> <&gcc GCC_PCIE_1_SLV_AXI_CLK>,
> <&gcc GCC_PCIE_1_SLV_Q2A_AXI_CLK>,
> <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>,
> + <&gcc GCC_AGGRE_NOC_PCIE_CENTER_SF_AXI_CLK>,
> + <&gcc GCC_AGGRE_NOC_PCIE_1_AXI_CLK>,
> <&gcc GCC_DDRSS_PCIE_SF_CLK>;


Still no.

Please respond to comments first.

Best regards,
Krzysztof

Subject: Re: [PATCH v2 2/3] dt-bindings: pci: QCOM Adding sc7280 aggre0, aggre1 clocks


On 7/4/2022 1:54 PM, Krzysztof Kozlowski wrote:
> On 01/07/2022 18:11, Krishna chaitanya chundru wrote:
>> Adding aggre0 and aggre1 clock entries to PCIe node.
>>
>> Signed-off-by: Krishna chaitanya chundru <[email protected]>
>> ---
>> Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> index 0b69b12..8f29bdd 100644
>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> @@ -423,8 +423,8 @@ allOf:
>> then:
>> properties:
>> clocks:
>> - minItems: 11
>> - maxItems: 11
>> + minItems: 13
>> + maxItems: 13
>> clock-names:
>> items:
>> - const: pipe # PIPE clock
>> @@ -437,6 +437,8 @@ allOf:
>> - const: bus_slave # Slave AXI clock
>> - const: slave_q2a # Slave Q2A clock
>> - const: tbu # PCIe TBU clock
>> + - const: aggre0 # Aggre NoC PCIe CENTER SF AXI clock
>> + - const: aggre1 # Aggre NoC PCIe1 AXI clock
> You ignored my comments from v1 - please don't. This is not accepted.
>
> Also, please do not send new versions of patchset as reply to some other
> threads. It's extremely confusing to find it under something else.
>
> Best regards,
> Krzysztof
Hi

Krzysztof,

Sorry for confusion created which replying this patch.

The only comment I got from v1 from you is to run make dtbs_check.

I ran that command I found the errors and fixed them and I ran the make dtbs_check again
before on v2 and made sure there are no errors.

Can you please tell me is there any steps I missed.

Thanks & Regards,
Krishna Chaitanya.

2022-07-06 15:24:53

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] dt-bindings: pci: QCOM Adding sc7280 aggre0, aggre1 clocks

On 06/07/2022 13:55, Krishna Chaitanya Chundru wrote:
>
> On 7/4/2022 1:54 PM, Krzysztof Kozlowski wrote:
>> On 01/07/2022 18:11, Krishna chaitanya chundru wrote:
>>> Adding aggre0 and aggre1 clock entries to PCIe node.
>>>
>>> Signed-off-by: Krishna chaitanya chundru <[email protected]>
>>> ---
>>> Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>> index 0b69b12..8f29bdd 100644
>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>> @@ -423,8 +423,8 @@ allOf:
>>> then:
>>> properties:
>>> clocks:
>>> - minItems: 11
>>> - maxItems: 11
>>> + minItems: 13
>>> + maxItems: 13
>>> clock-names:
>>> items:
>>> - const: pipe # PIPE clock
>>> @@ -437,6 +437,8 @@ allOf:
>>> - const: bus_slave # Slave AXI clock
>>> - const: slave_q2a # Slave Q2A clock
>>> - const: tbu # PCIe TBU clock
>>> + - const: aggre0 # Aggre NoC PCIe CENTER SF AXI clock
>>> + - const: aggre1 # Aggre NoC PCIe1 AXI clock
>> You ignored my comments from v1 - please don't. This is not accepted.
>>
>> Also, please do not send new versions of patchset as reply to some other
>> threads. It's extremely confusing to find it under something else.
>>
>> Best regards,
>> Krzysztof
> Hi
>
> Krzysztof,
>
> Sorry for confusion created which replying this patch.
>
> The only comment I got from v1 from you is to run make dtbs_check.
>
> I ran that command I found the errors and fixed them and I ran the make dtbs_check again
> before on v2 and made sure there are no errors.
>
> Can you please tell me is there any steps I missed.

The comment was:
"This won't work. You need to update other entry."

and then a conditional: "If you test it with
`make dtbs_check` you will see the errors."

So let's run it together:

/home/krzk/dev/linux/linux/out/arch/arm64/boot/dts/qcom/sc7280-idp.dtb:
pci@1c08000: clocks: [[42, 55], [42, 56], [41, 0], [39, 0], [42, 50],
[42, 52], [42, 53], [42, 57], [42, 58], [42, 177], [42, 178], [42, 8],
[42, 21]] is too long

From schema:
/home/krzk/dev/linux/linux/Documentation/devicetree/bindings/pci/qcom,pcie.yaml

/home/krzk/dev/linux/linux/out/arch/arm64/boot/dts/qcom/sc7280-idp.dtb:
pci@1c08000: clock-names: ['pipe', 'pipe_mux', 'phy_pipe', 'ref', 'aux',
'cfg', 'bus_master', 'bus_slave', 'slave_q2a', 'tbu', 'aggre0',
'aggre1', 'ddrss_sf_tbu'] is too long


clocks and clock-names can be maximum 12 items, as defined by schema in
"properties:" section. You cannot extend it in one place to 13 but leave
12 in other, because both constraints are applicable.

If you test it, you will see the errors.

Best regards,
Krzysztof

Subject: Re: [PATCH v2 2/3] dt-bindings: pci: QCOM Adding sc7280 aggre0, aggre1 clocks


On 7/6/2022 8:29 PM, Krzysztof Kozlowski wrote:
> On 06/07/2022 13:55, Krishna Chaitanya Chundru wrote:
>> On 7/4/2022 1:54 PM, Krzysztof Kozlowski wrote:
>>> On 01/07/2022 18:11, Krishna chaitanya chundru wrote:
>>>> Adding aggre0 and aggre1 clock entries to PCIe node.
>>>>
>>>> Signed-off-by: Krishna chaitanya chundru <[email protected]>
>>>> ---
>>>> Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 6 ++++--
>>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>> index 0b69b12..8f29bdd 100644
>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>> @@ -423,8 +423,8 @@ allOf:
>>>> then:
>>>> properties:
>>>> clocks:
>>>> - minItems: 11
>>>> - maxItems: 11
>>>> + minItems: 13
>>>> + maxItems: 13
>>>> clock-names:
>>>> items:
>>>> - const: pipe # PIPE clock
>>>> @@ -437,6 +437,8 @@ allOf:
>>>> - const: bus_slave # Slave AXI clock
>>>> - const: slave_q2a # Slave Q2A clock
>>>> - const: tbu # PCIe TBU clock
>>>> + - const: aggre0 # Aggre NoC PCIe CENTER SF AXI clock
>>>> + - const: aggre1 # Aggre NoC PCIe1 AXI clock
>>> You ignored my comments from v1 - please don't. This is not accepted.
>>>
>>> Also, please do not send new versions of patchset as reply to some other
>>> threads. It's extremely confusing to find it under something else.
>>>
>>> Best regards,
>>> Krzysztof
>> Hi
>>
>> Krzysztof,
>>
>> Sorry for confusion created which replying this patch.
>>
>> The only comment I got from v1 from you is to run make dtbs_check.
>>
>> I ran that command I found the errors and fixed them and I ran the make dtbs_check again
>> before on v2 and made sure there are no errors.
>>
>> Can you please tell me is there any steps I missed.
> The comment was:
> "This won't work. You need to update other entry."
>
> and then a conditional: "If you test it with
> `make dtbs_check` you will see the errors."
>
> So let's run it together:
>
> /home/krzk/dev/linux/linux/out/arch/arm64/boot/dts/qcom/sc7280-idp.dtb:
> pci@1c08000: clocks: [[42, 55], [42, 56], [41, 0], [39, 0], [42, 50],
> [42, 52], [42, 53], [42, 57], [42, 58], [42, 177], [42, 178], [42, 8],
> [42, 21]] is too long
>
> From schema:
> /home/krzk/dev/linux/linux/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>
> /home/krzk/dev/linux/linux/out/arch/arm64/boot/dts/qcom/sc7280-idp.dtb:
> pci@1c08000: clock-names: ['pipe', 'pipe_mux', 'phy_pipe', 'ref', 'aux',
> 'cfg', 'bus_master', 'bus_slave', 'slave_q2a', 'tbu', 'aggre0',
> 'aggre1', 'ddrss_sf_tbu'] is too long
>
>
> clocks and clock-names can be maximum 12 items, as defined by schema in
> "properties:" section. You cannot extend it in one place to 13 but leave
> 12 in other, because both constraints are applicable.
>
> If you test it, you will see the errors.
>
> Best regards,
> Krzysztof

Hi Krzysztof,

Sorry for very late reply.

If we increase the common definitions of clocks properties to "13" it is
sufficient right.


diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 92402f1..c9e268d 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -53,11 +53,11 @@ properties:
   # Platform constraints are described later.
   clocks:
     minItems: 3
-    maxItems: 12
+    maxItems: 13

   clock-names:
     minItems: 3
-    maxItems: 12
+    maxItems: 13

   resets:

Thanks & Regards,

Krishna Chaitanya.

2022-08-30 06:27:27

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] dt-bindings: pci: QCOM Adding sc7280 aggre0, aggre1 clocks

On 29/08/2022 20:45, Krishna Chaitanya Chundru wrote:
> Sorry for very late reply.
>
> If we increase the common definitions of clocks properties to "13" it is
> sufficient right.
>
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 92402f1..c9e268d 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -53,11 +53,11 @@ properties:
>    # Platform constraints are described later.
>    clocks:
>      minItems: 3
> -    maxItems: 12
> +    maxItems: 13
>
>    clock-names:
>      minItems: 3
> -    maxItems: 12
> +    maxItems: 13
>

Yes.

Best regards,
Krzysztof