diff mbox series

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

Message ID 1656691899-21315-3-git-send-email-quic_krichai@quicinc.com (mailing list archive)
State Not Applicable
Headers show
Series [v2,1/3] PCI: qcom: Add sc7280 aggre0, aggre1 and ddr sf tbu clocks in PCIe driver | expand

Commit Message

Krishna Chaitanya Chundru July 1, 2022, 4:11 p.m. UTC
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(-)

Comments

Krzysztof Kozlowski July 4, 2022, 8:24 a.m. UTC | #1
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
Krishna Chaitanya Chundru July 6, 2022, 11:55 a.m. UTC | #2
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.
Krzysztof Kozlowski July 6, 2022, 2:59 p.m. UTC | #3
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
Krishna Chaitanya Chundru Aug. 29, 2022, 5:45 p.m. UTC | #4
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.
Krzysztof Kozlowski Aug. 30, 2022, 6:23 a.m. UTC | #5
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
diff mbox series

Patch

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