Re: [PATCH 1/2] dt-bindings: mmc: mmc-pwrseq-simple: add support for reset control

[POPESCU Catalin]

On 06/10/2024 14:37, Krzysztof Kozlowski wrote:
> [Some people who received this message don't often get email from krzk@xxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
>
>
> On Fri, Oct 04, 2024 at 02:07:39PM +0200, Catalin Popescu wrote:
>> Add compatible value "mmc-pwrseq-simple-reset" to support reset control
>> instead of gpios. Reset controls being refcounted, they allow to use
>> shared resets or gpios across drivers. Support of reset control is
>> limited to one single reset control.
> Driver support is not that relevant to the bindings.
>
>> Signed-off-by: Catalin Popescu <catalin.popescu@xxxxxxxxxxxxxxxxxxxx>
>> ---
>>   .../bindings/mmc/mmc-pwrseq-simple.yaml       | 21 +++++++++++++++++--
>>   1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.yaml b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.yaml
>> index 00feaafc1063..da218260af01 100644
>> --- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.yaml
>> @@ -16,12 +16,13 @@ description:
>>
>>   properties:
>>     compatible:
>> -    const: mmc-pwrseq-simple
>> +    enum:
>> +      - mmc-pwrseq-simple
>> +      - mmc-pwrseq-simple-reset
> Nope, that's the same device.
>
>>     reset-gpios:
>>       minItems: 1
>>       # Put some limit to avoid false warnings
>> -    maxItems: 32
>>       description:
>>         contains a list of GPIO specifiers. The reset GPIOs are asserted
>>         at initialization and prior we start the power up procedure of the card.
>> @@ -50,6 +51,22 @@ properties:
>>   required:
>>     - compatible
>>
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - mmc-pwrseq-simple-reset
>> +    then:
>> +      properties:
>> +        reset-gpios:
>> +          maxItems: 1
>> +    else:
>> +      properties:
>> +        reset-gpios:
>> +          maxItems: 32
> So basically they are the same... Sorry, this all patch makes little
> sense to me. You are not doing here much. It's exactly the same device
> which you now describe in two ways (first no-go) but the two ways are
> actually the same (second no-go).

That's because the reset framework doesn't support a list of reset gpios 
(assuming that RESET_GPIO was enabled), but only a single reset gpio.

Both gpio and reset frameworks are expecting the same DT property 
("reset-gpios") so using a different compatible to differentiate b/w the 
"tool" (gpio or reset control) to use for the power sequence seemed fine 
to me.

>
> Best regards,
> Krzysztof
>