Re: [PATCH] gpio: of: Apply regulator-gpio quirk only to enable-gpios

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1847 bytes --]

On Wed, Feb 20, 2019 at 10:01:50AM +0100, Linus Walleij wrote:
> On Tue, Feb 19, 2019 at 5:30 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> 
> > Handling broken DTs only adds to the complexity, but I think this cannot
> > be helped, since those DTs can be stored in some ROM.
> 
> I agree, but if the broken DT needs to be supported going forward
> I would like it to be quite explicit, so that the code down in
> gpiolib-of.c does this:
> 
> /*
>  * Comment about this machine
>  */
> if (of_machine_is_compatible("nvidia,problematic-board") {
>    /* Quirk for this specific DT for this machine*/
> }
> 
> This makes the standard behavior clear and avoids the risk
> of applying hairy machine-specific quirks to any other machine,
> but even more importantly it makes it easy for people reading
> the code to see what is going on and why.

At the risk of repeating myself: the device tree bindings for regulator-
fixed are very clear: any flags in the specifier are ignored and the
polarity of the GPIO is determined exclusively by the enable-active-high
property.

This means that, contrary to what I said earlier, the Jetson TX1 device
tree is not actually buggy. Is it confusing and inconsistent? Yes it is.
But it was never wrong according to the bindings.

The commit that you merged broke the ABI set forth in the bindings by
not applying the quirks necessary to adhere to the bindings for these
fixed regulators. It was supposed to only avoid the quirks for GPIO
regulators, which are an entirely different thing.

Given the above, I don't think adding the quirk for a specific machine
is acceptable. There could be any number of device trees that rely on
the literal interpretation of the device tree bindings and they would
all remain broken if we added a quirk only for Jetson TX1.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]