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

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

[Krishna chaitanya chundru]

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

[PATCH v1 1/3] PCI: qcom: Add missing sc7280 clocks in PCIe driver

[Krishna chaitanya chundru]

Add missing clocks in the pcie driver.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 2ea1375..a7202f0 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1548,7 +1548,10 @@ static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
 static const struct qcom_pcie_cfg sc7280_cfg = {
 	.ops = &ops_1_9_0,
 	.has_tbu_clk = true,
+	.has_ddrss_sf_tbu_clk = true,
 	.pipe_clk_need_muxing = true,
+	.has_aggre0_clk = true,
+	.has_aggre1_clk = true,
 };
 
 static const struct qcom_pcie_cfg sc8180x_cfg = {
-- 
2.7.4

Re: [PATCH v1 1/3] PCI: qcom: Add missing sc7280 clocks in PCIe driver

[Bjorn Helgaas]

[+cc Prasad, who added sc7280]

On Fri, Jun 24, 2022 at 02:49:49PM +0530, Krishna chaitanya chundru wrote:
> Add missing clocks in the pcie driver.

s/pcie/PCIe/ as in subject.  Also, please mention the names of the
clocks here, too.

Does this add new functionality, or did sc7280 just never work in the
first place?

If this fixes a problem, it's nice to mention the user-visible symptom
in the commit log to make it easier to connect this fix with the
problem.

Looks like sc7280 was added by Prasad with b89ff410253d ("PCI: qcom:
Replace ops with struct pcie_cfg in pcie match data"), and I assume it
was tested at that time.

If I had noticed, I would have asked that b89ff410253d be a strict
conversion to struct qcom_pcie_cfg with no functional changes, with
sc7280 support being added in a separate patch by itself.

> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 2ea1375..a7202f0 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1548,7 +1548,10 @@ static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
>  static const struct qcom_pcie_cfg sc7280_cfg = {
>  	.ops = &ops_1_9_0,
>  	.has_tbu_clk = true,
> +	.has_ddrss_sf_tbu_clk = true,
>  	.pipe_clk_need_muxing = true,
> +	.has_aggre0_clk = true,
> +	.has_aggre1_clk = true,
>  };
>  
>  static const struct qcom_pcie_cfg sc8180x_cfg = {
> -- 
> 2.7.4
> 

Re: [PATCH v1 1/3] PCI: qcom: Add missing sc7280 clocks in PCIe driver

[Bjorn Helgaas]

[update Prasad's email address since pmaliset@codeaurora.org bounced]

On Fri, Jun 24, 2022 at 12:05:03PM -0500, Bjorn Helgaas wrote:
> [+cc Prasad, who added sc7280]
> 
> On Fri, Jun 24, 2022 at 02:49:49PM +0530, Krishna chaitanya chundru wrote:
> > Add missing clocks in the pcie driver.
> 
> s/pcie/PCIe/ as in subject.  Also, please mention the names of the
> clocks here, too.
> 
> Does this add new functionality, or did sc7280 just never work in the
> first place?
> 
> If this fixes a problem, it's nice to mention the user-visible symptom
> in the commit log to make it easier to connect this fix with the
> problem.
> 
> Looks like sc7280 was added by Prasad with b89ff410253d ("PCI: qcom:
> Replace ops with struct pcie_cfg in pcie match data"), and I assume it
> was tested at that time.
> 
> If I had noticed, I would have asked that b89ff410253d be a strict
> conversion to struct qcom_pcie_cfg with no functional changes, with
> sc7280 support being added in a separate patch by itself.
> 
> > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 2ea1375..a7202f0 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -1548,7 +1548,10 @@ static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
> >  static const struct qcom_pcie_cfg sc7280_cfg = {
> >  	.ops = &ops_1_9_0,
> >  	.has_tbu_clk = true,
> > +	.has_ddrss_sf_tbu_clk = true,
> >  	.pipe_clk_need_muxing = true,
> > +	.has_aggre0_clk = true,
> > +	.has_aggre1_clk = true,
> >  };
> >  
> >  static const struct qcom_pcie_cfg sc8180x_cfg = {
> > -- 
> > 2.7.4
> > 

Re: [PATCH v1 1/3] PCI: qcom: Add missing sc7280 clocks in PCIe driver

[Krishna Chaitanya Chundru]

On 6/24/2022 10:35 PM, Bjorn Helgaas wrote:
> [+cc Prasad, who added sc7280]
> On Fri, Jun 24, 2022 at 02:49:49PM +0530, Krishna chaitanya chundru wrote:
>> Add missing clocks in the pcie driver.
> s/pcie/PCIe/ as in subject.  Also, please mention the names of the
> clocks here, too.
Will correct in next patch.
> Does this add new functionality, or did sc7280 just never work in the

When ever system goes to suspend the pcie link is also going down even 
though

we didn't unvote any clocks/regulators.

When we added these two missing clocks, the link is stable now after 
suspend and resume also.

We didn't test suspend/resume previously that is reason we didn't see 
any issue.

> first place?
>
> If this fixes a problem, it's nice to mention the user-visible symptom
> in the commit log to make it easier to connect this fix with the
> problem.
Will update in the next patch.
> Looks like sc7280 was added by Prasad with b89ff410253d ("PCI: qcom:
> Replace ops with struct pcie_cfg in pcie match data"), and I assume it
> was tested at that time.
>
> If I had noticed, I would have asked that b89ff410253d be a strict
> conversion to struct qcom_pcie_cfg with no functional changes, with
> sc7280 support being added in a separate patch by itself.
FYI. Prasad has left our org and no longer associated with our org now.
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 2ea1375..a7202f0 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -1548,7 +1548,10 @@ static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
>>   static const struct qcom_pcie_cfg sc7280_cfg = {
>>   	.ops = &ops_1_9_0,
>>   	.has_tbu_clk = true,
>> +	.has_ddrss_sf_tbu_clk = true,
>>   	.pipe_clk_need_muxing = true,
>> +	.has_aggre0_clk = true,
>> +	.has_aggre1_clk = true,
>>   };
>>   
>>   static const struct qcom_pcie_cfg sc8180x_cfg = {
>> -- 
>> 2.7.4
>>

[PATCH v1 2/3] dt-bindings: pci: QCOM sc7280 add missing clocks.

[Krishna chaitanya chundru]

Add missing clocks.

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

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 0b69b12..313b981 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -438,6 +438,8 @@ allOf:
             - const: slave_q2a # Slave Q2A clock
             - const: tbu # PCIe TBU clock
             - const: ddrss_sf_tbu # PCIe SF TBU clock
+            - const: aggre0 # Aggre NoC PCIE CENTER SF AXI clock
+            - const: aggre1 # Aggre NoC PCIe1 AXI clock
         resets:
           maxItems: 1
         reset-names:
-- 
2.7.4

Re: [PATCH v1 2/3] dt-bindings: pci: QCOM sc7280 add missing clocks.

[Krzysztof Kozlowski]

On 24/06/2022 11:19, Krishna chaitanya chundru wrote:
> Add missing clocks.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 0b69b12..313b981 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -438,6 +438,8 @@ allOf:
>              - const: slave_q2a # Slave Q2A clock
>              - const: tbu # PCIe TBU clock
>              - const: ddrss_sf_tbu # PCIe SF TBU clock
> +            - const: aggre0 # Aggre NoC PCIE CENTER SF AXI clock
> +            - const: aggre1 # Aggre NoC PCIe1 AXI clock

Hi,

This won't work. You need to update other entry. If you test it with
`make dtbs_check` you will see the errors.

Best regards,
Krzysztof

Re: [PATCH v1 2/3] dt-bindings: pci: QCOM sc7280 add missing clocks.

[Bjorn Helgaas]

On Fri, Jun 24, 2022 at 02:49:50PM +0530, Krishna chaitanya chundru wrote:
> Add missing clocks.

When you revise this, please make your subject line match the history:

  dt-bindings: PCI: qcom: Add schema for sc7280 chipset
  dt-bindings: PCI: qcom: Specify reg-names explicitly
  dt-bindings: PCI: qcom: Do not require resets on msm8996 platforms
  dt-bindings: PCI: qcom: Convert to YAML

So your subject line should be something like:

  dt-bindings: PCI: qcom: Add sc7280 aggre0 and aggre1 clocks

Please also include the names of the clocks you're adding in the
commit log.  This will make the commit log history more specific and
useful for future readers.

This also applies to the other patches.

> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 0b69b12..313b981 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -438,6 +438,8 @@ allOf:
>              - const: slave_q2a # Slave Q2A clock
>              - const: tbu # PCIe TBU clock
>              - const: ddrss_sf_tbu # PCIe SF TBU clock
> +            - const: aggre0 # Aggre NoC PCIE CENTER SF AXI clock
> +            - const: aggre1 # Aggre NoC PCIe1 AXI clock

I assume the stuff after "#" is comment and could thus be made
consistent: s/PCIE/PCIe/, is "PCIe1" a typo for "PCIe" or different?
Seems like weird/inconsistent capitalization in general ("CENTER"),
but if you're matching language in a spec, please do that and ignore
my comment :)

>          resets:
>            maxItems: 1
>          reset-names:
> -- 
> 2.7.4
> 

Re: [PATCH v1 2/3] dt-bindings: pci: QCOM sc7280 add missing clocks.

[Krishna Chaitanya Chundru]

On 6/24/2022 10:28 PM, Bjorn Helgaas wrote:
> On Fri, Jun 24, 2022 at 02:49:50PM +0530, Krishna chaitanya chundru wrote:
>> Add missing clocks.
> When you revise this, please make your subject line match the history:
>
>    dt-bindings: PCI: qcom: Add schema for sc7280 chipset
>    dt-bindings: PCI: qcom: Specify reg-names explicitly
>    dt-bindings: PCI: qcom: Do not require resets on msm8996 platforms
>    dt-bindings: PCI: qcom: Convert to YAML
>
> So your subject line should be something like:
>
>    dt-bindings: PCI: qcom: Add sc7280 aggre0 and aggre1 clocks
>
> Please also include the names of the clocks you're adding in the
> commit log.  This will make the commit log history more specific and
> useful for future readers.
>
> This also applies to the other patches.
Thanks Bjorn, I will try to incorporate your comments in the next patch.
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> index 0b69b12..313b981 100644
>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> @@ -438,6 +438,8 @@ allOf:
>>               - const: slave_q2a # Slave Q2A clock
>>               - const: tbu # PCIe TBU clock
>>               - const: ddrss_sf_tbu # PCIe SF TBU clock
>> +            - const: aggre0 # Aggre NoC PCIE CENTER SF AXI clock
>> +            - const: aggre1 # Aggre NoC PCIe1 AXI clock
> I assume the stuff after "#" is comment and could thus be made
> consistent: s/PCIE/PCIe/, is "PCIe1" a typo for "PCIe" or different?
> Seems like weird/inconsistent capitalization in general ("CENTER"),
> but if you're matching language in a spec, please do that and ignore
> my comment :)
>
>>           resets:
>>             maxItems: 1
>>           reset-names:
>> -- 
>> 2.7.4
>>

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

[Krishna chaitanya chundru]

Add missing pcie clocks.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 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

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

[Krzysztof Kozlowski]

On 24/06/2022 11:19, Krishna chaitanya chundru wrote:
> Add missing pcie clocks.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  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

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

[Krishna chaitanya chundru]

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

[PATCH v2 1/3] PCI: qcom: Add sc7280 aggre0, aggre1 and ddr sf tbu clocks in PCIe driver

[Krishna chaitanya chundru]

Add missing aggre0, aggre1 and ddrs sf tbu clocks in PCIe driver.

If these clocks are not voted-on, then PCIe link is going down when system
is suspended as these clocks can get turned off.

And these clocks are needed only on sc7280 target.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 2ea1375..a7202f0 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1548,7 +1548,10 @@ static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
 static const struct qcom_pcie_cfg sc7280_cfg = {
 	.ops = &ops_1_9_0,
 	.has_tbu_clk = true,
+	.has_ddrss_sf_tbu_clk = true,
 	.pipe_clk_need_muxing = true,
+	.has_aggre0_clk = true,
+	.has_aggre1_clk = true,
 };
 
 static const struct qcom_pcie_cfg sc8180x_cfg = {
-- 
2.7.4

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

[Krishna chaitanya chundru]

Adding aggre0 and aggre1 clock entries to PCIe node. 

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 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

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

[Krzysztof Kozlowski]

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 <quic_krichai@quicinc.com>
> ---
>  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

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

[Krishna Chaitanya Chundru]

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 <quic_krichai@quicinc.com>
>> ---
>>   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.

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

[Krzysztof Kozlowski]

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 <quic_krichai@quicinc.com>
>>> ---
>>>   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

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

[Krishna Chaitanya Chundru]

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 <quic_krichai@quicinc.com>
>>>> ---
>>>>    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.

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

[Krzysztof Kozlowski]

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

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

[Krishna chaitanya chundru]

Add missing aggre0, aggre1 clocks to pcie node.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 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

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

[Krzysztof Kozlowski]

On 01/07/2022 18:11, Krishna chaitanya chundru wrote:
> Add missing aggre0, aggre1 clocks to pcie node.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  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