Re: [PATCH] gpiolib: fix gpio-hog functionality

[Sascha Hauer <sha@pengutronix.de>]

On Thu, Jun 01, 2023 at 04:25:06PM +0200, Marco Felsch wrote:
> This reverts commit 3641d381e63321016e3bf09504852a6b2a2f879b.
> 
> Since the of_xlate support the gpio-hog support is broken because the
> 'gpio' property used to specify the gpio-hog pin does not contain any
> phandle. Due to the fact that of_xlate was never implemented the easiest
> way to fix the gpio-hog functionality is to revert the commit.

3641d381e63321016e3bf09504852a6b2a2f879b was introduced for sunxi. As
Jules is currently working on sunxi support this is likely needed soon,
so what's the second easiest way?

Sascha

> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/gpio/gpiolib.c | 58 ++++++++++++++++++++----------------------
>  drivers/of/of_gpio.c   |  5 ++--
>  include/gpio.h         |  4 +--
>  3 files changed, 30 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 6ec63b1119..b4a3a4e550 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -444,27 +444,44 @@ static int of_hog_gpio(struct device_node *np, struct gpio_chip *chip,
>  		       unsigned int idx)
>  {
>  	struct device_node *chip_np = chip->dev->of_node;
> -	struct of_phandle_args gpiospec;
>  	unsigned long flags = 0;
> -	u32 gpio_flags;
> +	u32 gpio_cells, gpio_num, gpio_flags;
>  	int ret, gpio;
>  	const char *name = NULL;
>  
> -	ret = of_parse_phandle_with_args(chip_np, "gpios", "#gpio-cells", idx,
> -					&gpiospec);
> +	ret = of_property_read_u32(chip_np, "#gpio-cells", &gpio_cells);
>  	if (ret)
>  		return ret;
>  
> -	gpio = gpio_of_xlate(chip->dev, &gpiospec, &gpio_flags);
> +	/*
> +	 * Support for GPIOs that don't have #gpio-cells set to 2 is
> +	 * not implemented
> +	 */
> +	if (WARN_ON(gpio_cells != 2))
> +		return -ENOTSUPP;
> +
> +	ret = of_property_read_u32_index(np, "gpios", idx * gpio_cells,
> +					 &gpio_num);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_read_u32_index(np, "gpios", idx * gpio_cells + 1,
> +					 &gpio_flags);
> +	if (ret)
> +		return ret;
> +
> +	if (gpio_flags & OF_GPIO_ACTIVE_LOW)
> +		flags |= GPIOF_ACTIVE_LOW;
> +
> +	gpio = gpio_get_num(chip->dev, gpio_num);
>  	if (gpio == -EPROBE_DEFER)
>  		return gpio;
> +
>  	if (gpio < 0) {
> -		dev_err(chip->dev, "unable to get gpio: %d\n", gpio);
> +		dev_err(chip->dev, "unable to get gpio %u\n", gpio_num);
>  		return gpio;
>  	}
>  
> -	if (gpio_flags & OF_GPIO_ACTIVE_LOW)
> -		flags |= GPIOF_ACTIVE_LOW;
>  
>  	/*
>  	 * Note that, in order to be compatible with Linux, the code
> @@ -625,23 +642,6 @@ void gpiochip_remove(struct gpio_chip *chip)
>  	list_del(&chip->list);
>  }
>  
> -static int of_gpio_simple_xlate(struct gpio_chip *chip,
> -				const struct of_phandle_args *gpiospec,
> -				u32 *flags)
> -{
> -	/*
> -	 * Support for GPIOs that don't have #gpio-cells set to 2 is
> -	 * not implemented
> -	 */
> -	if (WARN_ON(gpiospec->args_count != 2))
> -		return -ENOTSUPP;
> -
> -	if (flags)
> -		*flags = gpiospec->args[1];
> -
> -	return chip->base + gpiospec->args[0];
> -}
> -
>  struct gpio_chip *gpio_get_chip_by_dev(struct device *dev)
>  {
>  	struct gpio_chip *chip;
> @@ -654,8 +654,7 @@ struct gpio_chip *gpio_get_chip_by_dev(struct device *dev)
>  	return NULL;
>  }
>  
> -int gpio_of_xlate(struct device *dev, struct of_phandle_args *gpiospec,
> -		  int *flags)
> +int gpio_get_num(struct device *dev, int gpio)
>  {
>  	struct gpio_chip *chip;
>  
> @@ -666,10 +665,7 @@ int gpio_of_xlate(struct device *dev, struct of_phandle_args *gpiospec,
>  	if (!chip)
>  		return -EPROBE_DEFER;
>  
> -	if (chip->ops->of_xlate)
> -		return chip->ops->of_xlate(chip, gpiospec, flags);
> -	else
> -		return of_gpio_simple_xlate(chip, gpiospec, flags);
> +	return chip->base + gpio;
>  }
>  
>  struct gpio_chip *gpio_get_chip(int gpio)
> diff --git a/drivers/of/of_gpio.c b/drivers/of/of_gpio.c
> index b662563742..5da80e2493 100644
> --- a/drivers/of/of_gpio.c
> +++ b/drivers/of/of_gpio.c
> @@ -62,7 +62,6 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
>  {
>  	struct of_phandle_args out_args;
>  	struct device *dev;
> -	int of_flags;
>  	int ret;
>  
>  	ret = of_parse_phandle_with_args(np, propname, "#gpio-cells",
> @@ -80,7 +79,7 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
>  		return -EPROBE_DEFER;
>  	}
>  
> -	ret = gpio_of_xlate(dev, &out_args, &of_flags);
> +	ret = gpio_get_num(dev, out_args.args[0]);
>  	if (ret == -EPROBE_DEFER)
>  		return ret;
>  	if (ret < 0) {
> @@ -90,7 +89,7 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
>  	}
>  
>  	if (flags) {
> -		*flags = of_flags;
> +		*flags = out_args.args[1];
>  		of_gpio_flags_quirks(np, propname, flags, index);
>  	}
>  
> diff --git a/include/gpio.h b/include/gpio.h
> index 10ac7fd7c4..5f2c16584c 100644
> --- a/include/gpio.h
> +++ b/include/gpio.h
> @@ -177,7 +177,6 @@ struct gpio_ops {
>  	int (*get_direction)(struct gpio_chip *chip, unsigned offset);
>  	int (*get)(struct gpio_chip *chip, unsigned offset);
>  	void (*set)(struct gpio_chip *chip, unsigned offset, int value);
> -	int (*of_xlate)(struct gpio_chip *chip, const struct of_phandle_args *gpiospec, u32 *flags);
>  };
>  
>  struct gpio_chip {
> @@ -194,8 +193,7 @@ struct gpio_chip {
>  int gpiochip_add(struct gpio_chip *chip);
>  void gpiochip_remove(struct gpio_chip *chip);
>  
> -int gpio_of_xlate(struct device *dev, struct of_phandle_args *gpiospec,
> -		  int *flags);
> +int gpio_get_num(struct device *dev, int gpio);
>  struct gpio_chip *gpio_get_chip(int gpio);
>  
>  #endif /* __GPIO_H */
> -- 
> 2.39.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |