ARM: mvebu: switch to using gpiod API in pm-board code
Commit Message
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(-)
Comments
> - 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
@@ -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;
}