Re: [PATCH 01/19 v3] regulator: fixed: Convert to use GPIO descriptor only

From: Linus Walleij <linus.walleij@linaro.org>
To: Mark Brown <broonie@kernel.org>
Cc: Thierry Reding <thierry.reding@gmail.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Alexander Shiyan <shc_work@mail.ru>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	Aaro Koskinen <aaro.koskinen@iki.fi>,
	Tony Lindgren <tony@atomide.com>,
	Mike Rapoport <rppt@linux.vnet.ibm.com>,
	Robert Jarzmik <robert.jarzmik@free.fr>,
	Philipp Zabel <philipp.zabel@gmail.com>,
	Daniel Mack <zonque@gmail.com>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Russell King <rmk+kernel@armlinux.org.uk>
Subject: Re: [PATCH 01/19 v3] regulator: fixed: Convert to use GPIO descriptor only
Date: Tue, 12 Jun 2018 10:15:40 +0200	[thread overview]
Message-ID: <CACRpkdYxTNWcvJ41TJ+uLyVuGzbdip9uOOgDdiyyjYt1Vd8pfg@mail.gmail.com> (raw)
In-Reply-To: <20180611150054.GC11580@sirena.org.uk>

On Mon, Jun 11, 2018 at 5:00 PM, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Jun 11, 2018 at 03:11:14PM +0200, Linus Walleij wrote:
>
>> The DT binding for "regulator-fixed" specifies that enable-active-high
>> should be set for polarity inversion. For historical reasons, I guess,
>> we screwed up. The example in the binding uses that.
>
> No, it just says that if the enable is active high then we should
> specify it using that boolean property.  It explicitly says that enables
> might be active low
>
> | - enable-active-high: Polarity of GPIO is Active high
> | If this property is missing, the default assumed is Active low.

Yeah my code gets it right I think, my head doesn't always :/

The assumed default is active low and then we tag this
bool property on the regulator if we want it to be active high.

> This binding predates the GPIO binding having any gpiolib level polarity
> inversion.

Yeah, that's how it is.

The confusion stems from Tegra using kinda both.

I think I have it handled right in the gpiolib core by assuming
it is active low, and explicitlt ignoring any attempts to use
the GPIO flags to flag it as active low. It will be active low
anyway in the Tegra case (after my patch to the second
compatible string) albeit giving a warning in the
dmesg.

>> Sadly this only handled the undocumented fixed
>> regulator binding "reg-fixed-voltage". So I need to fix it
>> for "regulator-fixed" as well, and then it "should work".
>
> The normal fixed voltage regulator parses enable-active-high?

Yeah currently. However I moved the handling to the gpiolib
parsing code to centralize polarity inversion handling, in an
attempt so keep things under control. So in the patch series
this flag handling moves over to the gpiolib.

I was thinking to do the same with other subsystems, such as
MMC to get legacy GPIO flags under control and contained.

Yours,
Linus Walleij