Message ID | b80d65600641e6dcf00da53ae797f4a40a80e2d0.1716976062.git.geert+renesas@glider.be |
---|---|
State | New |
Headers | show |
Series | [v2] regulator: gpio: Correct default GPIO state to LOW | expand |
On Wed, May 29, 2024 at 11:49:51AM +0200, Geert Uytterhoeven wrote: > According to the GPIO regulator DT bindings[1], the default GPIO state > is LOW. However, the driver defaults to HIGH. > Before the conversion to descriptors in commit d6cd33ad71029a3f > ("regulator: gpio: Convert to use descriptors"), the default state used > by the driver was rather ill-defined, too: That was 4 years ago... > I have no idea if this has any impact. > I guess most/all DTS files have proper gpios-states properties? That seems optimistic, and a grep in mainline shows some users but not really as many as I'd intuitively expect. > - /* Default to high per specification */ > + /* Default to low per specification */ > if (ret) > - config->gflags[i] = GPIOD_OUT_HIGH; > + config->gflags[i] = GPIOD_OUT_LOW; > else The risk here is that we start glitching the power where previously we didn't. This does make me nervous.
Hi Mark, On Wed, May 29, 2024 at 3:15 PM Mark Brown <broonie@kernel.org> wrote: > On Wed, May 29, 2024 at 11:49:51AM +0200, Geert Uytterhoeven wrote: > > According to the GPIO regulator DT bindings[1], the default GPIO state > > is LOW. However, the driver defaults to HIGH. > > > Before the conversion to descriptors in commit d6cd33ad71029a3f > > ("regulator: gpio: Convert to use descriptors"), the default state used > > by the driver was rather ill-defined, too: > > That was 4 years ago... > > > I have no idea if this has any impact. > > I guess most/all DTS files have proper gpios-states properties? > > That seems optimistic, and a grep in mainline shows some users but not > really as many as I'd intuitively expect. > > > - /* Default to high per specification */ > > + /* Default to low per specification */ > > if (ret) > > - config->gflags[i] = GPIOD_OUT_HIGH; > > + config->gflags[i] = GPIOD_OUT_LOW; > > else > > The risk here is that we start glitching the power where previously we > didn't. This does make me nervous. /me too The alternative is to change the GPIO regulator DT bindings document to match the code... Gr{oetje,eeting}s, Geert
On Wed, May 29, 2024 at 03:19:04PM +0200, Geert Uytterhoeven wrote: > On Wed, May 29, 2024 at 3:15 PM Mark Brown <broonie@kernel.org> wrote: > > The risk here is that we start glitching the power where previously we > > didn't. This does make me nervous. > /me too > The alternative is to change the GPIO regulator DT bindings document > to match the code... Yup, that makes me feel a lot safer than changing the code.
diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c index 65927fa2ef161cda..5dfed8bae0c4cfdc 100644 --- a/drivers/regulator/gpio-regulator.c +++ b/drivers/regulator/gpio-regulator.c @@ -176,9 +176,9 @@ of_get_gpio_regulator_config(struct device *dev, struct device_node *np, ret = of_property_read_u32_index(np, "gpios-states", i, &val); - /* Default to high per specification */ + /* Default to low per specification */ if (ret) - config->gflags[i] = GPIOD_OUT_HIGH; + config->gflags[i] = GPIOD_OUT_LOW; else config->gflags[i] = val ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;