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@xxxxxxxxx> 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:
signature.asc
Description: PGP signature