Re: [PATCH V2] regulator: gpio: Reword the binding document

[Marek Vasut <marek.vasut@gmail.com>]

On 3/5/19 11:07 AM, Harald Geyer wrote:
> marek.vasut@gmail.com writes:
>> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>>
>> Reword the binding document to make it clear how the propeties work
>> and which properties affect which other properties.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>> Cc: Harald Geyer <harald@ccbib.org>
>> Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: linux-renesas-soc@vger.kernel.org
>> To: devicetree@vger.kernel.org
>> ---
>> V2: - Make "gpios" a mandatory property
>>     - Reword "gpio-states" property description
>>     - Change "enable-gpio" to "enable-gpios" to match modern DT rules
>> Note: The recent gpio-regulator rework caused breakage. While the
>>       changes in the gpio-regulator code were according to the DT
>>       binding document, they stopped working with older DTs. Make
>>       the binding document clearer to prevent such breakage in the
>>       future.
> 
> Thanks for the update. I think it addresses all my concerns except for
> one:
> 
>> +- gpios-states	: State of GPIO pins in "gpios" array that is set until
>> +			  changed by the first consumer. 0: LOW, 1: HIGH.
>> +			  Default is LOW if nothing else is specified.
> 
> I still believe this not true: There is no guarantee that the regulator
> core won't change the state of GPIO pins before the first consumer comes
> up.

Why would it do that ? That would completely invalidate any remaining
useful-ness of this property.

> I still think my proposal describes the property more acurately:
> gpios-states : On operating systems, that don't support reading back gpio
>                values in output mode (most notably linux), this array
>                provides the state of GPIO pins set when requesting them
>                from the gpio controller. Systems, that are capable of
>                preserving state when requesting the lines, are free to
>                ignore this property. 0: LOW, 1: HIGH. Default is LOW if
>                nothing else is specified.
> 
> Since we had this discussion already in the V1 thread and you clearly don't
> agree with me, the maintainers will need to decide. You can add 
> Reviewed-by: Harald Geyer <harald@ccbib.org>
> once Rob and/or Mark have addressed this issue.

I think we're just looking at this from two different perspectives and
for whatever reason can't reconcile them.

> Thanks for your work!
> Harald
> 


-- 
Best regards,
Marek Vasut