2022-10-19 14:53:01

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] spi: dt-bindings: amlogic, meson-gx-spicc: Add pinctrl names for SPI signal states

On 19/10/2022 10:01, Amjad Ouled-Ameur wrote:
> SPI pins of the SPICC Controller in Meson-GX needs to be controlled by
> pin biais when idle. Therefore define three pinctrl names:
> - default: SPI pins are controlled by spi function.
> - idle-high: SCLK pin is pulled-up, but MOSI/MISO are still controlled
> by spi function.
> - idle-low: SCLK pin is pulled-down, but MOSI/MISO are still controlled
> by spi function.
>


> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - amlogic,meson-gx-spicc
> +
> + then:
> + properties:
> + pinctrl-names:
> + minItems: 1
> + items:
> + - const: default
> + - const: idle-high
> + - const: idle-low

You should also define in such case pinctrl-0 and others.

Best regards,
Krzysztof


2022-10-19 17:09:38

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] spi: dt-bindings: amlogic, meson-gx-spicc: Add pinctrl names for SPI signal states

On 19/10/2022 16:29, Krzysztof Kozlowski wrote:
> On 19/10/2022 10:01, Amjad Ouled-Ameur wrote:
>> SPI pins of the SPICC Controller in Meson-GX needs to be controlled by
>> pin biais when idle. Therefore define three pinctrl names:
>> - default: SPI pins are controlled by spi function.
>> - idle-high: SCLK pin is pulled-up, but MOSI/MISO are still controlled
>> by spi function.
>> - idle-low: SCLK pin is pulled-down, but MOSI/MISO are still controlled
>> by spi function.
>>
>
>
>> +
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - amlogic,meson-gx-spicc
>> +
>> + then:
>> + properties:
>> + pinctrl-names:
>> + minItems: 1
>> + items:
>> + - const: default
>> + - const: idle-high
>> + - const: idle-low
>
> You should also define in such case pinctrl-0 and others.

Ok I thought it would be covered by the pinctrl-consumer.yaml
but yeah we should allow pinctrl-1 and pinctrl-2 here aswell by adding:

pinctrl-1: true
pinctrl-2: true

>
> Best regards,
> Krzysztof
>
Neil

2022-10-20 13:39:40

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] spi: dt-bindings: amlogic, meson-gx-spicc: Add pinctrl names for SPI signal states

>>> + properties:
>>> + pinctrl-names:
>>> + minItems: 1
>>> + items:
>>> + - const: default
>>> + - const: idle-high
>>> + - const: idle-low
>>
>> You should also define in such case pinctrl-0 and others.
>
> Ok I thought it would be covered by the pinctrl-consumer.yaml
> but yeah we should allow pinctrl-1 and pinctrl-2 here aswell by adding:
>
> pinctrl-1: true
> pinctrl-2: true
>
>

Yes.

Best regards,
Krzysztof

2022-10-21 13:41:44

by Amjad Ouled-Ameur

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] spi: dt-bindings: amlogic, meson-gx-spicc: Add pinctrl names for SPI signal states

Hi

On 10/20/22 14:49, Krzysztof Kozlowski wrote:
>>>> + properties:
>>>> + pinctrl-names:
>>>> + minItems: 1
>>>> + items:
>>>> + - const: default
>>>> + - const: idle-high
>>>> + - const: idle-low
>>> You should also define in such case pinctrl-0 and others.
>> Ok I thought it would be covered by the pinctrl-consumer.yaml
>> but yeah we should allow pinctrl-1 and pinctrl-2 here aswell by adding:
>>
>> pinctrl-1: true
>> pinctrl-2: true
>>
>>
In such case, should I define pinctrl- as part of the if statement, as shown below,

or before allOf ?

[...]

  - if:
      properties:
        compatible:
          contains:
            enum:
              - amlogic,meson-gx-spicc

    then:
      properties:
        pinctrl-0: true
        pinctrl-1: true
        pinctrl-2: true

        pinctrl-names:
          minItems: 1
          items:
            - const: default
            - const: idle-high
            - const: idle-low

[...]

Regards

Amjad

> Yes.
>
> Best regards,
> Krzysztof
>

2022-10-21 14:18:50

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] spi: dt-bindings: amlogic, meson-gx-spicc: Add pinctrl names for SPI signal states

On 21/10/2022 08:54, Amjad Ouled-Ameur wrote:
> Hi
>
> On 10/20/22 14:49, Krzysztof Kozlowski wrote:
>>>>> + properties:
>>>>> + pinctrl-names:
>>>>> + minItems: 1
>>>>> + items:
>>>>> + - const: default
>>>>> + - const: idle-high
>>>>> + - const: idle-low
>>>> You should also define in such case pinctrl-0 and others.
>>> Ok I thought it would be covered by the pinctrl-consumer.yaml
>>> but yeah we should allow pinctrl-1 and pinctrl-2 here aswell by adding:
>>>
>>> pinctrl-1: true
>>> pinctrl-2: true
>>>
>>>
> In such case, should I define pinctrl- as part of the if statement, as shown below,
>
> or before allOf ?

The same as pinctrl-names, so part of allOf.

>
> [...]
>
>   - if:
>       properties:
>         compatible:
>           contains:
>             enum:
>               - amlogic,meson-gx-spicc
>
>     then:
>       properties:
>         pinctrl-0: true
>         pinctrl-1: true
>         pinctrl-2: true
>
>         pinctrl-names:
>           minItems: 1
>           items:
>             - const: default
>             - const: idle-high
>             - const: idle-low

Best regards,
Krzysztof