Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only

From: Leonard Crestez <leonard.crestez@nxp.com>
To: "broonie@kernel.org" <broonie@kernel.org>,
	"linus.walleij@linaro.org" <linus.walleij@linaro.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Andy Duan <fugang.duan@nxp.com>,
	Fabio Estevam <fabio.estevam@nxp.com>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	Anson Huang <anson.huang@nxp.com>,
	"tony@atomide.com" <tony@atomide.com>,
	"rppt@linux.vnet.ibm.com" <rppt@linux.vnet.ibm.com>,
	"jmkrzyszt@gmail.com" <jmkrzyszt@gmail.com>,
	"john.stultz@linaro.org" <john.stultz@linaro.org>
Subject: Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only
Date: Mon, 1 Oct 2018 18:53:37 +0000	[thread overview]
Message-ID: <afd78fe6a5e168cc9190afe3e27d12286e0dfafa.camel@nxp.com> (raw)
In-Reply-To: <20180906122436.25610-1-linus.walleij@linaro.org>

Hello,

This patch seems to break network booting on imx6sx-sdb in linux-next
because the enet phy regulator is not on. Reverting the patch fixes
boot.

Here is the regulator definition:

reg_enet_3v3: regulator-enet-3v3 {
	compatible = "regulator-fixed";
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_enet_3v3>;
	regulator-name = "enet_3v3";
	regulator-min-microvolt = <3300000>;
	regulator-max-microvolt = <3300000>;
	gpios = <&gpio2 6 GPIO_ACTIVE_LOW>;
};

Here are some prints with the patch enabled:

[    0.153150] reg-fixed-voltage regulator-enet-3v3: OUT_HIGH gflags 0x7
[    0.153171] reg_fixed_voltage_probe(179): regulator-enet-3v3 call gpiod_get_optional gflags=0x7 ena_gpio_invert
[    0.153218] regulator-enet-3v3 GPIO handle specifies active low - ignored
[    0.153233] of_gpio_flags_quirks(83): regulator-enet-3v3 set active low because !enable-active-high
[    0.153258] of_get_named_gpiod_flags: parsed 'gpios' property of node '/regulator-enet-3v3[0]' - status (0)
[    0.153310] gpio_value: 38 set 0
[    0.153332] gpio_direction: 38 out (0)
[    0.153364] enet_3v3: ena_gpiod=(ptrval) ena_gpio=0 init=0 valid=1
[    0.153377] enet_3v3: request GPIO
[    0.153393] enet_3v3: already have gpiod
[    0.153423] enet_3v3: 3300 mV 
...
[    3.867827] enet_3v3: GPIO enable count=0 inv=1
[    3.872432] enet_3v3: set value 0
[    3.875779] gpio_value: 38 set 1

That "gpio_value: 30 set 1" tracepoint is wrong, the line is set high.

It seems that gpiod_set_value will check FLAG_ACTIVE_LOW and
automatically invert so maybe ena_gpio_invert should not be used if a
full gpiod is passed to regulator?

--- drivers/regulator/fixed.c
+++ drivers/regulator/fixed.c
@@ -146,7 +146,6 @@ static int reg_fixed_voltage_probe(struct
platform_device *pdev)
 
        drvdata->desc.fixed_uV = config->microvolts;
 
-       cfg.ena_gpio_invert = !config->enable_high;
        if (config->enabled_at_boot) {
                if (config->enable_high)
                        gflags = GPIOD_OUT_HIGH;

All these high/low inversions and flags are extremely confusing to me.

Link to original thread: https://lkml.org/lkml/2018/9/6/485

--
Regards,
Leonard
