Re: [PATCH 10/10] gpiolib: add of_xlate support

[Jules Maselbas <jmaselbas@kalray.eu>]

Hi Marco,

On Fri, Jun 02, 2023 at 09:49:21AM +0200, Marco Felsch wrote:
> The of_xlate function can be used to by drivers which need a other gpio
> encoding than:
> 
>   gpio = <&phandle gpio-nr gpio-flags>;
> 
> The of_xlate code is as close as possible to the kernel counter part
> which makes it easier to add 'struct gpio_desc' support later on.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/gpio/gpiolib.c | 51 ++++++++++++++++++++++++++++++++++++++
>  drivers/of/of_gpio.c   | 56 +++++++++++++++++++++++++++++++++---------
>  include/gpio.h         | 25 +++++++++++++++++++
>  3 files changed, 120 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index d1087aa583..ab9a0e30be 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -592,6 +592,43 @@ static int of_gpiochip_set_names(struct gpio_chip *chip)
>  	return 0;
>  }
>  
> +/**
> + * of_gpio_simple_xlate - translate gpiospec to the GPIO number and flags
> + * @gc:		pointer to the gpio_chip structure
> + * @gpiospec:	GPIO specifier as found in the device tree
> + * @flags:	a flags pointer to fill in
> + *
> + * This is simple translation function, suitable for the most 1:1 mapped
> + * GPIO chips. This function performs only one sanity check: whether GPIO
> + * is less than ngpios (that is specified in the gpio_chip).
> + */
> +static int of_gpio_simple_xlate(struct gpio_chip *gc,
> +				const struct of_phandle_args *gpiospec,
> +				u32 *flags)
> +{
> +	/*
> +	 * We're discouraging gpio_cells < 2, since that way you'll have to
> +	 * write your own xlate function (that will have to retrieve the GPIO
> +	 * number and the flags from a single gpio cell -- this is possible,
> +	 * but not recommended).
> +	 */
> +	if (gc->of_gpio_n_cells < 2) {
> +		WARN_ON(1);
> +		return -EINVAL;
> +	}
> +
> +	if (WARN_ON(gpiospec->args_count < gc->of_gpio_n_cells))
> +		return -EINVAL;
> +
> +	if (gpiospec->args[0] >= gc->ngpio)
> +		return -EINVAL;
> +
> +	if (flags)
> +		*flags = gpiospec->args[1];
> +
> +	return gc->base + gpiospec->args[0];
> +}
> +
>  static int of_gpiochip_add(struct gpio_chip *chip)
>  {
>  	struct device_node *np;
> @@ -601,6 +638,20 @@ static int of_gpiochip_add(struct gpio_chip *chip)
>  	if (!np)
>  		return 0;
>  
> +	if (!chip->ops->of_xlate)
> +		chip->ops->of_xlate = of_gpio_simple_xlate;
> +
> +	/*
> +	 * Seperate check since the 'struct gpio_ops' is alawys the same for
typos: s/Seperate/Separate/  and s/alawys/always/

> +	 * every 'struct gpio_chip' of the same instance (e.g. 'struct
> +	 * imx_gpio_chip').
> +	 */
> +	if (chip->ops->of_xlate == of_gpio_simple_xlate)
> +		chip->of_gpio_n_cells = 2;
> +
> +	if (chip->of_gpio_n_cells > MAX_PHANDLE_ARGS)
> +		return -EINVAL;
> +
>  	ret = of_gpiochip_set_names(chip);
>  	if (ret)
>  		return ret;

Thanks :)