[PATCH RTF 0/2] gpiolib: of: Handle threecell gpios

[PATCH RTF 0/2] gpiolib: of: Handle threecell gpios

[Linus Walleij]

This adds some code in the gpiolib OF core to deal with
several gpio chip instances per OF node.

The change was prompted by the need of the Spacemit GPIO
controller.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Linus Walleij (2):
      gpiolib: of: Use local variables
      gpiolib: of: Handle threecell GPIO chips

 drivers/gpio/gpiolib-of.c   | 126 ++++++++++++++++++++++++++++++++++++--------
 include/linux/gpio/driver.h |  24 ++++++++-
 2 files changed, 126 insertions(+), 24 deletions(-)
---
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
change-id: 20250217-gpio-ranges-fourcell-85888ad219da

Best regards,
-- 
Linus Walleij <linus.walleij@linaro.org>

[PATCH RTF 1/2] gpiolib: of: Use local variables

[Linus Walleij]

Instead of modifying the contents of the array of valued read
in from a phandle, use local variables to store the values.
This makes the code easier to read and the array immutable.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib-of.c | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 2e537ee979f3e2b6e8d5f86f3e121a66f2a8e083..86405218f4e2ddc951a1a9d168e886400652bf60 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -1057,6 +1057,9 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
 	const char *name;
 	static const char group_names_propname[] = "gpio-ranges-group-names";
 	bool has_group_names;
+	int offset; /* Offset of the first GPIO line on the chip */
+	int pin; /* Pin base number in the range */
+	int count; /* Number of pins/GPIO lines to map */
 
 	np = dev_of_node(&chip->gpiodev->dev);
 	if (!np)
@@ -1075,13 +1078,17 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
 		if (!pctldev)
 			return -EPROBE_DEFER;
 
+		offset = pinspec.args[0];
+		pin = pinspec.args[1];
+		count = pinspec.args[2];
+
 		/* Ignore ranges outside of this GPIO chip */
-		if (pinspec.args[0] >= (chip->offset + chip->ngpio))
+		if (offset >= (chip->offset + chip->ngpio))
 			continue;
-		if (pinspec.args[0] + pinspec.args[2] <= chip->offset)
+		if (offset + count <= chip->offset)
 			continue;
 
-		if (pinspec.args[2]) {
+		if (count) {
 			/* npins != 0: linear range */
 			if (has_group_names) {
 				of_property_read_string_index(np,
@@ -1095,27 +1102,27 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
 			}
 
 			/* Trim the range to fit this GPIO chip */
-			if (chip->offset > pinspec.args[0]) {
-				trim = chip->offset - pinspec.args[0];
-				pinspec.args[2] -= trim;
-				pinspec.args[1] += trim;
-				pinspec.args[0] = 0;
+			if (chip->offset > offset) {
+				trim = chip->offset - offset;
+				count -= trim;
+				pin += trim;
+				offset = 0;
 			} else {
-				pinspec.args[0] -= chip->offset;
+				offset -= chip->offset;
 			}
-			if ((pinspec.args[0] + pinspec.args[2]) > chip->ngpio)
-				pinspec.args[2] = chip->ngpio - pinspec.args[0];
+			if ((offset + count) > chip->ngpio)
+				count = chip->ngpio - offset;
 
 			ret = gpiochip_add_pin_range(chip,
 					pinctrl_dev_get_devname(pctldev),
-					pinspec.args[0],
-					pinspec.args[1],
-					pinspec.args[2]);
+					offset,
+					pin,
+					count);
 			if (ret)
 				return ret;
 		} else {
 			/* npins == 0: special range */
-			if (pinspec.args[1]) {
+			if (pin) {
 				pr_err("%pOF: Illegal gpio-range format.\n",
 					np);
 				break;
@@ -1140,7 +1147,7 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
 			}
 
 			ret = gpiochip_add_pingroup_range(chip, pctldev,
-						pinspec.args[0], name);
+						offset, name);
 			if (ret)
 				return ret;
 		}

-- 
2.48.1

[PATCH RTF 2/2] gpiolib: of: Handle threecell GPIO chips

[Linus Walleij]

When describing GPIO controllers in the device tree, the ambition
of device tree to describe the hardware may require a three-cell
scheme:

gpios = <&gpio instance offset flags>;

This implements support for this scheme in the gpiolib OF core.

Drivers that want to handle multiple gpiochip instances from one
OF node need to implement a callback similar to this to
determine if a certain gpio chip is a pointer to the right
instance (pseudo-code):

struct my_gpio {
    struct gpio_chip gcs[MAX_CHIPS];
};

static bool my_of_node_instance_match(struct gpio_chip *gc
                                      unsigned int instance)
{
    struct my_gpio *mg = gpiochip_get_data(gc);

    if (instance >= MAX_CHIPS)
        return false;
    return (gc == &mg->gcs[instance];
}

probe() {
    struct my_gpio *mg;
    struct gpio_chip *gc;
    int i, ret;

    for (i = 0; i++; i < MAX_CHIPS) {
        gc = &mg->gcs[i];
        /* This tells gpiolib we have several instances per node */
        gc->of_gpio_n_cells = 3;
	gc->of_node_instance_match = my_of_node_instance_match;
        gc->base = -1;
        ...

        ret = devm_gpiochip_add_data(dev, gc, mg);
        if (ret)
            return ret;
    }
}

Rename the "simple" of_xlate function to "twocell" which is closer
to what it actually does.

In the device tree bindings, the provide node needs
to specify #gpio-cells = <3>; where the first cell is the instance
number:

gpios = <&gpio instance offset flags>;

Conversely ranges need to have four cells:

gpio-ranges = <&pinctrl instance gpio_offset pin_offset count>;

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib-of.c   | 93 ++++++++++++++++++++++++++++++++++++++++-----
 include/linux/gpio/driver.h | 24 +++++++++++-
 2 files changed, 106 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 86405218f4e2ddc951a1a9d168e886400652bf60..614590a5bcd10e5605ecb66ebd956250e4ea1fd8 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -929,7 +929,7 @@ struct notifier_block gpio_of_notifier = {
 #endif /* CONFIG_OF_DYNAMIC */
 
 /**
- * of_gpio_simple_xlate - translate gpiospec to the GPIO number and flags
+ * of_gpio_twocell_xlate - translate twocell 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
@@ -941,9 +941,9 @@ struct notifier_block gpio_of_notifier = {
  * Returns:
  * GPIO number (>= 0) on success, negative errno on failure.
  */
-static int of_gpio_simple_xlate(struct gpio_chip *gc,
-				const struct of_phandle_args *gpiospec,
-				u32 *flags)
+static int of_gpio_twocell_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
@@ -968,6 +968,49 @@ static int of_gpio_simple_xlate(struct gpio_chip *gc,
 	return gpiospec->args[0];
 }
 
+/**
+ * of_gpio_threecell_xlate - translate threecell 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:n mapped
+ * GPIO chips, i.e. several GPIO chip instances from one device tree node.
+ * In this case the following binding is implied:
+ *
+ * foo-gpios = <&gpio instance offset flags>;
+ *
+ * Returns:
+ * GPIO number (>= 0) on success, negative errno on failure.
+ */
+static int of_gpio_threecell_xlate(struct gpio_chip *gc,
+				   const struct of_phandle_args *gpiospec,
+				   u32 *flags)
+{
+	if (gc->of_gpio_n_cells != 3) {
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
+	if (WARN_ON(gpiospec->args_count != 3))
+		return -EINVAL;
+
+	/*
+	 * Check chip instance number, the driver responds with true if
+	 * this is the chip we are looking for.
+	 */
+	if (!gc->of_node_instance_match(gc, gpiospec->args[0]))
+		return -EINVAL;
+
+	if (gpiospec->args[1] >= gc->ngpio)
+		return -EINVAL;
+
+	if (flags)
+		*flags = gpiospec->args[2];
+
+	return gpiospec->args[1];
+}
+
 #if IS_ENABLED(CONFIG_OF_GPIO_MM_GPIOCHIP)
 #include <linux/gpio/legacy-of-mm-gpiochip.h>
 /**
@@ -1068,7 +1111,15 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
 	has_group_names = of_property_present(np, group_names_propname);
 
 	for (;; index++) {
-		ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3,
+		/*
+		 * Ordinary phandles contain 2-3 cells:
+		 * gpios = <&gpio [instance] offset flags>;
+		 * Ranges always contain one more cell:
+		 * gpio-ranges <&pinctrl [gpio_instance] gpio_offet pin_offet count>;
+		 * This is why we parse chip->of_gpio_n_cells + 1 cells
+		 */
+		ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges",
+				chip->of_gpio_n_cells + 1,
 				index, &pinspec);
 		if (ret)
 			break;
@@ -1078,9 +1129,25 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
 		if (!pctldev)
 			return -EPROBE_DEFER;
 
-		offset = pinspec.args[0];
-		pin = pinspec.args[1];
-		count = pinspec.args[2];
+		if (chip->of_gpio_n_cells == 3) {
+			/* First cell is the gpiochip instance number */
+			offset = pinspec.args[1];
+			pin = pinspec.args[2];
+			count = pinspec.args[3];
+		} else {
+			offset = pinspec.args[0];
+			pin = pinspec.args[1];
+			count = pinspec.args[2];
+		}
+
+		/*
+		 * With multiple GPIO chips per node, check that this chip is the
+		 * right instance.
+		 */
+		if (chip->of_node_instance_match &&
+		    (chip->of_gpio_n_cells == 3) &&
+		    !chip->of_node_instance_match(chip, pinspec.args[0]))
+			continue;
 
 		/* Ignore ranges outside of this GPIO chip */
 		if (offset >= (chip->offset + chip->ngpio))
@@ -1170,8 +1237,14 @@ int of_gpiochip_add(struct gpio_chip *chip)
 		return 0;
 
 	if (!chip->of_xlate) {
-		chip->of_gpio_n_cells = 2;
-		chip->of_xlate = of_gpio_simple_xlate;
+		if (chip->of_gpio_n_cells == 3) {
+			if (!chip->of_node_instance_match)
+				return -EINVAL;
+			chip->of_xlate = of_gpio_threecell_xlate;
+		} else {
+			chip->of_gpio_n_cells = 2;
+			chip->of_xlate = of_gpio_twocell_xlate;
+		}
 	}
 
 	if (chip->of_gpio_n_cells > MAX_PHANDLE_ARGS)
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 2dd7cb9cc270a68ddedbcdd5d44e0d0f88dfa785..70a361f6aba06d4a11e5ca913ec79411d7a11b3c 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -516,10 +516,32 @@ struct gpio_chip {
 	/**
 	 * @of_gpio_n_cells:
 	 *
-	 * Number of cells used to form the GPIO specifier.
+	 * Number of cells used to form the GPIO specifier. The standard i 2
+	 * cells:
+	 *
+	 * gpios = <&gpio offset flags>;
+	 *
+	 * some complex GPIO controllers instantiate more than one chip per
+	 * device tree node and have 3 cells:
+	 *
+	 * gpios = <&gpio instance offset flags>;
+	 *
+	 * Legacy GPIO controllers may even have 1 cell:
+	 *
+	 * gpios = <&gpio offset>;
 	 */
 	unsigned int of_gpio_n_cells;
 
+	/**
+	 * of_node_instance_match:
+	 *
+	 * Determine if a chip is the right instance. Must be implemented by
+	 * any driver using more than one gpio_chip per device tree node.
+	 * Returns true if gc is the instance indicated by i (which is the
+	 * first cell in the phandles for GPIO lines and gpio-ranges).
+	 */
+	bool (*of_node_instance_match)(struct gpio_chip *gc, unsigned int i);
+
 	/**
 	 * @of_xlate:
 	 *

-- 
2.48.1

Re: [PATCH RTF 0/2] gpiolib: of: Handle threecell gpios

[Yixun Lan]

Hi Linus Walleij:

On 11:28 Tue 18 Feb     , Linus Walleij wrote:
> This adds some code in the gpiolib OF core to deal with
> several gpio chip instances per OF node.
> 
> The change was prompted by the need of the Spacemit GPIO
> controller.
> 
this patch works for me, many thanks
I will send a patch v6 of spacemit's gpio on top of this 

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Linus Walleij (2):
>       gpiolib: of: Use local variables
>       gpiolib: of: Handle threecell GPIO chips
> 
>  drivers/gpio/gpiolib-of.c   | 126 ++++++++++++++++++++++++++++++++++++--------
>  include/linux/gpio/driver.h |  24 ++++++++-
>  2 files changed, 126 insertions(+), 24 deletions(-)
> ---
> base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
> change-id: 20250217-gpio-ranges-fourcell-85888ad219da
> 
> Best regards,
> -- 
> Linus Walleij <linus.walleij@linaro.org>
> 

-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55

Re: [PATCH RTF 1/2] gpiolib: of: Use local variables

[Alex Elder]

On 2/18/25 4:28 AM, Linus Walleij wrote:
> Instead of modifying the contents of the array of valued read

s/valued/values/

> in from a phandle, use local variables to store the values.
> This makes the code easier to read and the array immutable.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

In addition to preparing for the next patch, this is much more
readable.

Reviewed-by: Alex Elder <elder@riscstar.com>

> ---
>   drivers/gpio/gpiolib-of.c | 39 +++++++++++++++++++++++----------------
>   1 file changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 2e537ee979f3e2b6e8d5f86f3e121a66f2a8e083..86405218f4e2ddc951a1a9d168e886400652bf60 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -1057,6 +1057,9 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
>   	const char *name;
>   	static const char group_names_propname[] = "gpio-ranges-group-names";
>   	bool has_group_names;
> +	int offset; /* Offset of the first GPIO line on the chip */
> +	int pin; /* Pin base number in the range */
> +	int count; /* Number of pins/GPIO lines to map */
>   
>   	np = dev_of_node(&chip->gpiodev->dev);
>   	if (!np)
> @@ -1075,13 +1078,17 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
>   		if (!pctldev)
>   			return -EPROBE_DEFER;
>   
> +		offset = pinspec.args[0];
> +		pin = pinspec.args[1];
> +		count = pinspec.args[2];
> +
>   		/* Ignore ranges outside of this GPIO chip */
> -		if (pinspec.args[0] >= (chip->offset + chip->ngpio))
> +		if (offset >= (chip->offset + chip->ngpio))
>   			continue;
> -		if (pinspec.args[0] + pinspec.args[2] <= chip->offset)
> +		if (offset + count <= chip->offset)
>   			continue;
>   
> -		if (pinspec.args[2]) {
> +		if (count) {
>   			/* npins != 0: linear range */
>   			if (has_group_names) {
>   				of_property_read_string_index(np,
> @@ -1095,27 +1102,27 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
>   			}
>   
>   			/* Trim the range to fit this GPIO chip */
> -			if (chip->offset > pinspec.args[0]) {
> -				trim = chip->offset - pinspec.args[0];
> -				pinspec.args[2] -= trim;
> -				pinspec.args[1] += trim;
> -				pinspec.args[0] = 0;
> +			if (chip->offset > offset) {
> +				trim = chip->offset - offset;
> +				count -= trim;
> +				pin += trim;
> +				offset = 0;
>   			} else {
> -				pinspec.args[0] -= chip->offset;
> +				offset -= chip->offset;
>   			}
> -			if ((pinspec.args[0] + pinspec.args[2]) > chip->ngpio)
> -				pinspec.args[2] = chip->ngpio - pinspec.args[0];
> +			if ((offset + count) > chip->ngpio)
> +				count = chip->ngpio - offset;
>   
>   			ret = gpiochip_add_pin_range(chip,
>   					pinctrl_dev_get_devname(pctldev),
> -					pinspec.args[0],
> -					pinspec.args[1],
> -					pinspec.args[2]);
> +					offset,
> +					pin,
> +					count);
>   			if (ret)
>   				return ret;
>   		} else {
>   			/* npins == 0: special range */
> -			if (pinspec.args[1]) {
> +			if (pin) {
>   				pr_err("%pOF: Illegal gpio-range format.\n",
>   					np);
>   				break;
> @@ -1140,7 +1147,7 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
>   			}
>   
>   			ret = gpiochip_add_pingroup_range(chip, pctldev,
> -						pinspec.args[0], name);
> +						offset, name);
>   			if (ret)
>   				return ret;
>   		}
> 

Re: [PATCH RTF 2/2] gpiolib: of: Handle threecell GPIO chips

[Alex Elder]

On 2/18/25 4:28 AM, Linus Walleij wrote:
> When describing GPIO controllers in the device tree, the ambition
> of device tree to describe the hardware may require a three-cell
> scheme:
> 
> gpios = <&gpio instance offset flags>;
> 
> This implements support for this scheme in the gpiolib OF core.
> 
> Drivers that want to handle multiple gpiochip instances from one
> OF node need to implement a callback similar to this to
> determine if a certain gpio chip is a pointer to the right
> instance (pseudo-code):
> 
> struct my_gpio {
>      struct gpio_chip gcs[MAX_CHIPS];
> };
> 
> static bool my_of_node_instance_match(struct gpio_chip *gc
>                                        unsigned int instance)
> {
>      struct my_gpio *mg = gpiochip_get_data(gc);
> 
>      if (instance >= MAX_CHIPS)
>          return false;
>      return (gc == &mg->gcs[instance];

This is pseudocode, but either "(" not needed, or ")" missing.

> }
> 
> probe() {
>      struct my_gpio *mg;
>      struct gpio_chip *gc;
>      int i, ret;
> 
>      for (i = 0; i++; i < MAX_CHIPS) {
>          gc = &mg->gcs[i];
>          /* This tells gpiolib we have several instances per node */
>          gc->of_gpio_n_cells = 3;
> 	gc->of_node_instance_match = my_of_node_instance_match;
>          gc->base = -1;
>          ...
> 
>          ret = devm_gpiochip_add_data(dev, gc, mg);
>          if (ret)
>              return ret;
>      }
> }
> 
> Rename the "simple" of_xlate function to "twocell" which is closer
> to what it actually does.
> 
> In the device tree bindings, the provide node needs
> to specify #gpio-cells = <3>; where the first cell is the instance
> number:
> 
> gpios = <&gpio instance offset flags>;
> 
> Conversely ranges need to have four cells:
> 
> gpio-ranges = <&pinctrl instance gpio_offset pin_offset count>;

I haven't looked for it, but is a DT binding update forthcoming?

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

This looks good.

Reviewed-by: Alex Elder <elder@riscstar.com>

> ---
>   drivers/gpio/gpiolib-of.c   | 93 ++++++++++++++++++++++++++++++++++++++++-----
>   include/linux/gpio/driver.h | 24 +++++++++++-
>   2 files changed, 106 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 86405218f4e2ddc951a1a9d168e886400652bf60..614590a5bcd10e5605ecb66ebd956250e4ea1fd8 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -929,7 +929,7 @@ struct notifier_block gpio_of_notifier = {
>   #endif /* CONFIG_OF_DYNAMIC */
>   
>   /**
> - * of_gpio_simple_xlate - translate gpiospec to the GPIO number and flags
> + * of_gpio_twocell_xlate - translate twocell 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
> @@ -941,9 +941,9 @@ struct notifier_block gpio_of_notifier = {
>    * Returns:
>    * GPIO number (>= 0) on success, negative errno on failure.
>    */
> -static int of_gpio_simple_xlate(struct gpio_chip *gc,
> -				const struct of_phandle_args *gpiospec,
> -				u32 *flags)
> +static int of_gpio_twocell_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
> @@ -968,6 +968,49 @@ static int of_gpio_simple_xlate(struct gpio_chip *gc,
>   	return gpiospec->args[0];
>   }
>   
> +/**
> + * of_gpio_threecell_xlate - translate threecell 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:n mapped
> + * GPIO chips, i.e. several GPIO chip instances from one device tree node.
> + * In this case the following binding is implied:
> + *
> + * foo-gpios = <&gpio instance offset flags>;
> + *
> + * Returns:
> + * GPIO number (>= 0) on success, negative errno on failure.
> + */
> +static int of_gpio_threecell_xlate(struct gpio_chip *gc,
> +				   const struct of_phandle_args *gpiospec,
> +				   u32 *flags)
> +{
> +	if (gc->of_gpio_n_cells != 3) {
> +		WARN_ON(1);
> +		return -EINVAL;
> +	}
> +
> +	if (WARN_ON(gpiospec->args_count != 3))
> +		return -EINVAL;
> +
> +	/*
> +	 * Check chip instance number, the driver responds with true if
> +	 * this is the chip we are looking for.
> +	 */
> +	if (!gc->of_node_instance_match(gc, gpiospec->args[0]))
> +		return -EINVAL;
> +
> +	if (gpiospec->args[1] >= gc->ngpio)
> +		return -EINVAL;
> +
> +	if (flags)
> +		*flags = gpiospec->args[2];
> +
> +	return gpiospec->args[1];
> +}
> +
>   #if IS_ENABLED(CONFIG_OF_GPIO_MM_GPIOCHIP)
>   #include <linux/gpio/legacy-of-mm-gpiochip.h>
>   /**
> @@ -1068,7 +1111,15 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
>   	has_group_names = of_property_present(np, group_names_propname);
>   
>   	for (;; index++) {
> -		ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3,
> +		/*
> +		 * Ordinary phandles contain 2-3 cells:
> +		 * gpios = <&gpio [instance] offset flags>;
> +		 * Ranges always contain one more cell:
> +		 * gpio-ranges <&pinctrl [gpio_instance] gpio_offet pin_offet count>;
> +		 * This is why we parse chip->of_gpio_n_cells + 1 cells
> +		 */
> +		ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges",
> +				chip->of_gpio_n_cells + 1,
>   				index, &pinspec);
>   		if (ret)
>   			break;
> @@ -1078,9 +1129,25 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
>   		if (!pctldev)
>   			return -EPROBE_DEFER;
>   
> -		offset = pinspec.args[0];
> -		pin = pinspec.args[1];
> -		count = pinspec.args[2];
> +		if (chip->of_gpio_n_cells == 3) {
> +			/* First cell is the gpiochip instance number */
> +			offset = pinspec.args[1];
> +			pin = pinspec.args[2];
> +			count = pinspec.args[3];
> +		} else {
> +			offset = pinspec.args[0];
> +			pin = pinspec.args[1];
> +			count = pinspec.args[2];
> +		}
> +
> +		/*
> +		 * With multiple GPIO chips per node, check that this chip is the
> +		 * right instance.
> +		 */
> +		if (chip->of_node_instance_match &&
> +		    (chip->of_gpio_n_cells == 3) &&
> +		    !chip->of_node_instance_match(chip, pinspec.args[0]))
> +			continue;
>   
>   		/* Ignore ranges outside of this GPIO chip */
>   		if (offset >= (chip->offset + chip->ngpio))
> @@ -1170,8 +1237,14 @@ int of_gpiochip_add(struct gpio_chip *chip)
>   		return 0;
>   
>   	if (!chip->of_xlate) {
> -		chip->of_gpio_n_cells = 2;
> -		chip->of_xlate = of_gpio_simple_xlate;
> +		if (chip->of_gpio_n_cells == 3) {
> +			if (!chip->of_node_instance_match)
> +				return -EINVAL;
> +			chip->of_xlate = of_gpio_threecell_xlate;
> +		} else {
> +			chip->of_gpio_n_cells = 2;
> +			chip->of_xlate = of_gpio_twocell_xlate;
> +		}
>   	}
>   
>   	if (chip->of_gpio_n_cells > MAX_PHANDLE_ARGS)
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index 2dd7cb9cc270a68ddedbcdd5d44e0d0f88dfa785..70a361f6aba06d4a11e5ca913ec79411d7a11b3c 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -516,10 +516,32 @@ struct gpio_chip {
>   	/**
>   	 * @of_gpio_n_cells:
>   	 *
> -	 * Number of cells used to form the GPIO specifier.
> +	 * Number of cells used to form the GPIO specifier. The standard i 2

s/standard i /standard is /

> +	 * cells:
> +	 *
> +	 * gpios = <&gpio offset flags>;
> +	 *
> +	 * some complex GPIO controllers instantiate more than one chip per
> +	 * device tree node and have 3 cells:
> +	 *
> +	 * gpios = <&gpio instance offset flags>;
> +	 *
> +	 * Legacy GPIO controllers may even have 1 cell:
> +	 *
> +	 * gpios = <&gpio offset>;
>   	 */
>   	unsigned int of_gpio_n_cells;
>   
> +	/**
> +	 * of_node_instance_match:
> +	 *
> +	 * Determine if a chip is the right instance. Must be implemented by
> +	 * any driver using more than one gpio_chip per device tree node.
> +	 * Returns true if gc is the instance indicated by i (which is the
> +	 * first cell in the phandles for GPIO lines and gpio-ranges).
> +	 */
> +	bool (*of_node_instance_match)(struct gpio_chip *gc, unsigned int i);
> +
>   	/**
>   	 * @of_xlate:
>   	 *
> 

Re: [PATCH RTF 0/2] gpiolib: of: Handle threecell gpios

[Yixun Lan]

On 11:28 Tue 18 Feb     , Linus Walleij wrote:
> This adds some code in the gpiolib OF core to deal with
> several gpio chip instances per OF node.
> 
> The change was prompted by the need of the Spacemit GPIO
> controller.
> 

Tested-by: Yixun Lan <dlan@gentoo.org>

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Linus Walleij (2):
>       gpiolib: of: Use local variables
>       gpiolib: of: Handle threecell GPIO chips
> 
>  drivers/gpio/gpiolib-of.c   | 126 ++++++++++++++++++++++++++++++++++++--------
>  include/linux/gpio/driver.h |  24 ++++++++-
>  2 files changed, 126 insertions(+), 24 deletions(-)
> ---
> base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
> change-id: 20250217-gpio-ranges-fourcell-85888ad219da
> 
> Best regards,
> -- 
> Linus Walleij <linus.walleij@linaro.org>
> 

-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55