2023-07-04 09:34:15

by William Qiu

Subject: [RESEND v1 1/2] dt-binding: spi: constrain minItems of clocks and clock-names

The SPI controller only need apb_pclk clock to work properly on JH7110 SoC,
so there add minItems whose value is equal to 1. Other platforms do not
have this constraint.

Signed-off-by: William Qiu <[email protected]>
---
Documentation/devicetree/bindings/spi/spi-pl022.yaml | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-pl022.yaml b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
index 91e540a92faf..42bb34c39971 100644
--- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
@@ -11,6 +11,7 @@ maintainers:

allOf:
- $ref: spi-controller.yaml#
+ - $ref: /schemas/arm/primecell.yaml#

# We need a select here so we don't match all nodes with 'arm,primecell'
select:
@@ -34,12 +35,16 @@ properties:
maxItems: 1

clocks:
+ minItems: 1
maxItems: 2

clock-names:
- items:
- - const: sspclk
- - const: apb_pclk
+ oneOf:
+ - items:
+ - const: apb_pclk
+ - items:
+ - const: sspclk
+ - const: apb_pclk

pl022,autosuspend-delay:
description: delay in ms following transfer completion before the
--
2.34.1

2023-07-04 10:12:20

by Krzysztof Kozlowski

Subject: Re: [RESEND v1 1/2] dt-binding: spi: constrain minItems of clocks and clock-names

On 04/07/2023 11:21, William Qiu wrote:
> The SPI controller only need apb_pclk clock to work properly on JH7110 SoC,
> so there add minItems whose value is equal to 1. Other platforms do not
> have this constraint.
>
> Signed-off-by: William Qiu <[email protected]>

I don't get why this is resent, but subject prefix is still wrong. It's
dt-bindings.

> ---
> Documentation/devicetree/bindings/spi/spi-pl022.yaml | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/spi-pl022.yaml b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> index 91e540a92faf..42bb34c39971 100644
> --- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> @@ -11,6 +11,7 @@ maintainers:
>
> allOf:
> - $ref: spi-controller.yaml#
> + - $ref: /schemas/arm/primecell.yaml#

This looks unrelated, so keep it as separate commit with its own rationale.

>
> # We need a select here so we don't match all nodes with 'arm,primecell'
> select:
> @@ -34,12 +35,16 @@ properties:
> maxItems: 1
>
> clocks:
> + minItems: 1
> maxItems: 2
>
> clock-names:
> - items:
> - - const: sspclk
> - - const: apb_pclk
> + oneOf:
> + - items:
> + - const: apb_pclk
> + - items:
> + - const: sspclk
> + - const: apb_pclk

Are you sure that your clock is APB pclk in such case?

Best regards,
Krzysztof

2023-07-05 03:58:26

by William Qiu

Subject: Re: [RESEND v1 1/2] dt-binding: spi: constrain minItems of clocks and clock-names

On 2023/7/4 17:38, Krzysztof Kozlowski wrote:
> On 04/07/2023 11:21, William Qiu wrote:
>> The SPI controller only need apb_pclk clock to work properly on JH7110 SoC,
>> so there add minItems whose value is equal to 1. Other platforms do not
>> have this constraint.
>>
>> Signed-off-by: William Qiu <[email protected]>
>
> I don't get why this is resent, but subject prefix is still wrong. It's
> dt-bindings.
>
Will update.
>> ---
>> Documentation/devicetree/bindings/spi/spi-pl022.yaml | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-pl022.yaml b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
>> index 91e540a92faf..42bb34c39971 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
>> +++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
>> @@ -11,6 +11,7 @@ maintainers:
>>
>> allOf:
>> - $ref: spi-controller.yaml#
>> + - $ref: /schemas/arm/primecell.yaml#
>
> This looks unrelated, so keep it as separate commit with its own rationale.
>
Because "arm,primecell-periphid" is need in JH7110 SoC, so I added them in
one commit, so do I need to put them in two commit?
>>
>> # We need a select here so we don't match all nodes with 'arm,primecell'
>> select:
>> @@ -34,12 +35,16 @@ properties:
>> maxItems: 1
>>
>> clocks:
>> + minItems: 1
>> maxItems: 2
>>
>> clock-names:
>> - items:
>> - - const: sspclk
>> - - const: apb_pclk
>> + oneOf:
>> + - items:
>> + - const: apb_pclk
>> + - items:
>> + - const: sspclk
>> + - const: apb_pclk
>
> Are you sure that your clock is APB pclk in such case?
>
Yes, in JH7110 SoC is APB pclk in such case.

Thanks for taking time to review this patch series.
> Best regards,
> Krzysztof
>

2023-07-05 06:14:42

by Krzysztof Kozlowski

Subject: Re: [RESEND v1 1/2] dt-binding: spi: constrain minItems of clocks and clock-names

On 05/07/2023 05:37, William Qiu wrote:
>>> ---
>>> Documentation/devicetree/bindings/spi/spi-pl022.yaml | 11 ++++++++---
>>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/spi-pl022.yaml b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
>>> index 91e540a92faf..42bb34c39971 100644
>>> --- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
>>> @@ -11,6 +11,7 @@ maintainers:
>>>
>>> allOf:
>>> - $ref: spi-controller.yaml#
>>> + - $ref: /schemas/arm/primecell.yaml#
>>
>> This looks unrelated, so keep it as separate commit with its own rationale.
>>
> Because "arm,primecell-periphid" is need in JH7110 SoC, so I added them in
> one commit, so do I need to put them in two commit?

You need to provide rationale why this is needed. I would assume this is
needed for every primecell, not only JH7110, right?



Best regards,
Krzysztof

2023-07-05 06:36:44

by William Qiu

Subject: Re: [RESEND v1 1/2] dt-binding: spi: constrain minItems of clocks and clock-names

On 2023/7/5 14:00, Krzysztof Kozlowski wrote:
> On 05/07/2023 05:37, William Qiu wrote:
>>>> ---
>>>> Documentation/devicetree/bindings/spi/spi-pl022.yaml | 11 ++++++++---
>>>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-pl022.yaml b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
>>>> index 91e540a92faf..42bb34c39971 100644
>>>> --- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
>>>> +++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
>>>> @@ -11,6 +11,7 @@ maintainers:
>>>>
>>>> allOf:
>>>> - $ref: spi-controller.yaml#
>>>> + - $ref: /schemas/arm/primecell.yaml#
>>>
>>> This looks unrelated, so keep it as separate commit with its own rationale.
>>>
>> Because "arm,primecell-periphid" is need in JH7110 SoC, so I added them in
>> one commit, so do I need to put them in two commit?
>
> You need to provide rationale why this is needed. I would assume this is
> needed for every primecell, not only JH7110, right?
>
>
All right, I'll keep it as separate commit.
>
> Best regards,
> Krzysztof
>