| Message ID | Y3Qc6et/d+vhd71Q@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: mvebu: switch to using gpiod API in pm-board code | expand |
> - ret = gpio_direction_output(pic_gpios[i], 0); > - if (ret < 0) { > - gpio_free(pic_gpios[i]); > + pic_gpios[i] = fwnode_gpiod_get_index(of_fwnode_handle(np), > + "ctrl", i, GPIOD_OUT_HIGH, > + name); The old code passes value=0 to gpio_direction_output(). For fwnode_gpiod_get_index() you pass GPIOD_OUT_HIGH. Is this correct? Andrew
On Wed, Nov 16, 2022 at 02:10:29AM +0100, Andrew Lunn wrote: > > - ret = gpio_direction_output(pic_gpios[i], 0); > > - if (ret < 0) { > > - gpio_free(pic_gpios[i]); > > + pic_gpios[i] = fwnode_gpiod_get_index(of_fwnode_handle(np), > > + "ctrl", i, GPIOD_OUT_HIGH, > > + name); > > The old code passes value=0 to gpio_direction_output(). For > fwnode_gpiod_get_index() you pass GPIOD_OUT_HIGH. Is this correct? Yes, gpiod API works on logical states, whereas old gpio API used signal levels. In arch/arm/boot/dts/armada-xp-gp.dts ctrl-gpios are described as "active low": cpus { pm_pic { ctrl-gpios = <&gpio0 16 GPIO_ACTIVE_LOW>, <&gpio0 17 GPIO_ACTIVE_LOW>, <&gpio0 18 GPIO_ACTIVE_LOW>; }; }; so gpiolib will translate GPIOD_OUT_HIGH to 0 when setting final state of the pin. There are discussions to rename GPIOD_OUT_HIGH and friends to something like active/inactive for better clarity, but that has not happened yet. Thanks.
On Tue, Nov 15, 2022 at 05:40:35PM -0800, Dmitry Torokhov wrote: > On Wed, Nov 16, 2022 at 02:10:29AM +0100, Andrew Lunn wrote: > > > - ret = gpio_direction_output(pic_gpios[i], 0); > > > - if (ret < 0) { > > > - gpio_free(pic_gpios[i]); > > > + pic_gpios[i] = fwnode_gpiod_get_index(of_fwnode_handle(np), > > > + "ctrl", i, GPIOD_OUT_HIGH, > > > + name); > > > > The old code passes value=0 to gpio_direction_output(). For > > fwnode_gpiod_get_index() you pass GPIOD_OUT_HIGH. Is this correct? > > Yes, gpiod API works on logical states, whereas old gpio API used signal > levels. In arch/arm/boot/dts/armada-xp-gp.dts ctrl-gpios are described > as "active low": > > cpus { > pm_pic { > ctrl-gpios = <&gpio0 16 GPIO_ACTIVE_LOW>, > <&gpio0 17 GPIO_ACTIVE_LOW>, > <&gpio0 18 GPIO_ACTIVE_LOW>; > }; > }; > > so gpiolib will translate GPIOD_OUT_HIGH to 0 when setting final state > of the pin. Ah, yes. I knew that once, but it has gotten forgotten. Thanks for the explanation and reminder. Andrew
On Tue, Nov 15, 2022 at 03:12:41PM -0800, Dmitry Torokhov wrote: > This switches PM code to use the newer gpiod API instead of legacy > gpio API that we want to retire. > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
Dmitry Torokhov <dmitry.torokhov@gmail.com> writes: > This switches PM code to use the newer gpiod API instead of legacy > gpio API that we want to retire. > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> Applied on mvebu/dt Thanks, Gregory > --- > arch/arm/mach-mvebu/pm-board.c | 28 +++++++++------------------- > 1 file changed, 9 insertions(+), 19 deletions(-) > > diff --git a/arch/arm/mach-mvebu/pm-board.c b/arch/arm/mach-mvebu/pm-board.c > index 7fa1806acd65..beec22e17e89 100644 > --- a/arch/arm/mach-mvebu/pm-board.c > +++ b/arch/arm/mach-mvebu/pm-board.c > @@ -8,19 +8,19 @@ > */ > > #include <linux/delay.h> > -#include <linux/gpio.h> > +#include <linux/err.h> > +#include <linux/gpio/consumer.h> > #include <linux/init.h> > #include <linux/io.h> > #include <linux/of.h> > #include <linux/of_address.h> > -#include <linux/of_gpio.h> > #include <linux/slab.h> > #include "common.h" > > #define ARMADA_PIC_NR_GPIOS 3 > > static void __iomem *gpio_ctrl; > -static int pic_gpios[ARMADA_PIC_NR_GPIOS]; > +static struct gpio_desc *pic_gpios[ARMADA_PIC_NR_GPIOS]; > static int pic_raw_gpios[ARMADA_PIC_NR_GPIOS]; > > static void mvebu_armada_pm_enter(void __iomem *sdram_reg, u32 srcmd) > @@ -90,27 +90,17 @@ static int __init mvebu_armada_pm_init(void) > char *name; > struct of_phandle_args args; > > - pic_gpios[i] = of_get_named_gpio(np, "ctrl-gpios", i); > - if (pic_gpios[i] < 0) { > - ret = -ENODEV; > - goto out; > - } > - > name = kasprintf(GFP_KERNEL, "pic-pin%d", i); > if (!name) { > ret = -ENOMEM; > goto out; > } > > - ret = gpio_request(pic_gpios[i], name); > - if (ret < 0) { > - kfree(name); > - goto out; > - } > - > - ret = gpio_direction_output(pic_gpios[i], 0); > - if (ret < 0) { > - gpio_free(pic_gpios[i]); > + pic_gpios[i] = fwnode_gpiod_get_index(of_fwnode_handle(np), > + "ctrl", i, GPIOD_OUT_HIGH, > + name); > + ret = PTR_ERR_OR_ZERO(pic_gpios[i]); > + if (ret) { > kfree(name); > goto out; > } > @@ -118,7 +108,7 @@ static int __init mvebu_armada_pm_init(void) > ret = of_parse_phandle_with_fixed_args(np, "ctrl-gpios", 2, > i, &args); > if (ret < 0) { > - gpio_free(pic_gpios[i]); > + gpiod_put(pic_gpios[i]); > kfree(name); > goto out; > } > -- > 2.38.1.431.g37b22c650d-goog > > > -- > Dmitry
diff --git a/arch/arm/mach-mvebu/pm-board.c b/arch/arm/mach-mvebu/pm-board.c index 7fa1806acd65..beec22e17e89 100644 --- a/arch/arm/mach-mvebu/pm-board.c +++ b/arch/arm/mach-mvebu/pm-board.c @@ -8,19 +8,19 @@ */ #include <linux/delay.h> -#include <linux/gpio.h> +#include <linux/err.h> +#include <linux/gpio/consumer.h> #include <linux/init.h> #include <linux/io.h> #include <linux/of.h> #include <linux/of_address.h> -#include <linux/of_gpio.h> #include <linux/slab.h> #include "common.h" #define ARMADA_PIC_NR_GPIOS 3 static void __iomem *gpio_ctrl; -static int pic_gpios[ARMADA_PIC_NR_GPIOS]; +static struct gpio_desc *pic_gpios[ARMADA_PIC_NR_GPIOS]; static int pic_raw_gpios[ARMADA_PIC_NR_GPIOS]; static void mvebu_armada_pm_enter(void __iomem *sdram_reg, u32 srcmd) @@ -90,27 +90,17 @@ static int __init mvebu_armada_pm_init(void) char *name; struct of_phandle_args args; - pic_gpios[i] = of_get_named_gpio(np, "ctrl-gpios", i); - if (pic_gpios[i] < 0) { - ret = -ENODEV; - goto out; - } - name = kasprintf(GFP_KERNEL, "pic-pin%d", i); if (!name) { ret = -ENOMEM; goto out; } - ret = gpio_request(pic_gpios[i], name); - if (ret < 0) { - kfree(name); - goto out; - } - - ret = gpio_direction_output(pic_gpios[i], 0); - if (ret < 0) { - gpio_free(pic_gpios[i]); + pic_gpios[i] = fwnode_gpiod_get_index(of_fwnode_handle(np), + "ctrl", i, GPIOD_OUT_HIGH, + name); + ret = PTR_ERR_OR_ZERO(pic_gpios[i]); + if (ret) { kfree(name); goto out; } @@ -118,7 +108,7 @@ static int __init mvebu_armada_pm_init(void) ret = of_parse_phandle_with_fixed_args(np, "ctrl-gpios", 2, i, &args); if (ret < 0) { - gpio_free(pic_gpios[i]); + gpiod_put(pic_gpios[i]); kfree(name); goto out; }
This switches PM code to use the newer gpiod API instead of legacy gpio API that we want to retire. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- arch/arm/mach-mvebu/pm-board.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-)