Add gpiochip_add_pin_range_sparse() function to add a range for GPIO<->pin
mapping, using a list of non consecutive pins.
Previously, it was only possible to add range of consecutive pins using
gpiochip_add_pin_range_sparse().
The struct pinctrl_gpio_range has a 'pins' member which allows to set a
list of pins (which can be non consecutive).
gpiochip_add_pin_range_sparse() is identical to gpiochip_add_pin_range(),
except it set 'pins' member instead of 'pin_base' member.
Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
---
drivers/gpio/gpiolib.c | 74 +++++++++++++++++++++++++++++++++------------
include/linux/gpio/driver.h | 12 ++++++++
2 files changed, 66 insertions(+), 20 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 16d190c1d6802..5a6d97116be9f 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2234,26 +2234,9 @@ int gpiochip_add_pingroup_range(struct gpio_chip *gc,
}
EXPORT_SYMBOL_GPL(gpiochip_add_pingroup_range);
-/**
- * gpiochip_add_pin_range() - add a range for GPIO <-> pin mapping
- * @gc: the gpiochip to add the range for
- * @pinctl_name: the dev_name() of the pin controller to map to
- * @gpio_offset: the start offset in the current gpio_chip number space
- * @pin_offset: the start offset in the pin controller number space
- * @npins: the number of pins from the offset of each pin space (GPIO and
- * pin controller) to accumulate in this range
- *
- * Calling this function directly from a DeviceTree-supported
- * pinctrl driver is DEPRECATED. Please see Section 2.1 of
- * Documentation/devicetree/bindings/gpio/gpio.txt on how to
- * bind pinctrl and gpio drivers via the "gpio-ranges" property.
- *
- * Returns:
- * 0 on success, or a negative errno on failure.
- */
-int gpiochip_add_pin_range(struct gpio_chip *gc, const char *pinctl_name,
- unsigned int gpio_offset, unsigned int pin_offset,
- unsigned int npins)
+static int __gpiochip_add_pin_range(struct gpio_chip *gc, const char *pinctl_name,
+ unsigned int gpio_offset, unsigned int pin_offset,
+ unsigned int const *pins, unsigned int npins)
{
struct gpio_pin_range *pin_range;
struct gpio_device *gdev = gc->gpiodev;
@@ -2271,6 +2254,7 @@ int gpiochip_add_pin_range(struct gpio_chip *gc, const char *pinctl_name,
pin_range->range.name = gc->label;
pin_range->range.base = gdev->base + gpio_offset;
pin_range->range.pin_base = pin_offset;
+ pin_range->range.pins = pins;
pin_range->range.npins = npins;
pin_range->pctldev = pinctrl_find_and_add_gpio_range(pinctl_name,
&pin_range->range);
@@ -2289,8 +2273,58 @@ int gpiochip_add_pin_range(struct gpio_chip *gc, const char *pinctl_name,
return 0;
}
+
+/**
+ * gpiochip_add_pin_range() - add a range for GPIO <-> pin mapping
+ * @gc: the gpiochip to add the range for
+ * @pinctl_name: the dev_name() of the pin controller to map to
+ * @gpio_offset: the start offset in the current gpio_chip number space
+ * @pin_offset: the start offset in the pin controller number space
+ * @npins: the number of pins from the offset of each pin space (GPIO and
+ * pin controller) to accumulate in this range
+ *
+ * Calling this function directly from a DeviceTree-supported
+ * pinctrl driver is DEPRECATED. Please see Section 2.1 of
+ * Documentation/devicetree/bindings/gpio/gpio.txt on how to
+ * bind pinctrl and gpio drivers via the "gpio-ranges" property.
+ *
+ * Returns:
+ * 0 on success, or a negative errno on failure.
+ */
+int gpiochip_add_pin_range(struct gpio_chip *gc, const char *pinctl_name,
+ unsigned int gpio_offset, unsigned int pin_offset,
+ unsigned int npins)
+{
+ return __gpiochip_add_pin_range(gc, pinctl_name, gpio_offset,
+ pin_offset, NULL, npins);
+}
EXPORT_SYMBOL_GPL(gpiochip_add_pin_range);
+/**
+ * gpiochip_add_pin_range_sparse() - add a range for GPIO <-> pin mapping
+ * @gc: the gpiochip to add the range for
+ * @pinctl_name: the dev_name() of the pin controller to map to
+ * @gpio_offset: the start offset in the current gpio_chip number space
+ * @pin_list: the list of pins to accumulate in this range
+ * @npins: the number of pins to accumulate in this range
+ *
+ * Calling this function directly from a DeviceTree-supported
+ * pinctrl driver is DEPRECATED. Please see Section 2.1 of
+ * Documentation/devicetree/bindings/gpio/gpio.txt on how to
+ * bind pinctrl and gpio drivers via the "gpio-ranges" property.
+ *
+ * Returns:
+ * 0 on success, or a negative errno on failure.
+ */
+int gpiochip_add_pin_range_sparse(struct gpio_chip *gc, const char *pinctl_name,
+ unsigned int gpio_offset, unsigned int const *pins,
+ unsigned int npins)
+{
+ return __gpiochip_add_pin_range(gc, pinctl_name, gpio_offset, 0, pins,
+ npins);
+}
+EXPORT_SYMBOL_GPL(gpiochip_add_pin_range_sparse);
+
/**
* gpiochip_remove_pin_ranges() - remove all the GPIO <-> pin mappings
* @gc: the chip to remove all the mappings for
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 2dd7cb9cc270a..0402f94ec6a02 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -751,6 +751,9 @@ struct gpio_pin_range {
int gpiochip_add_pin_range(struct gpio_chip *gc, const char *pinctl_name,
unsigned int gpio_offset, unsigned int pin_offset,
unsigned int npins);
+int gpiochip_add_pin_range_sparse(struct gpio_chip *gc, const char *pinctl_name,
+ unsigned int gpio_offset, unsigned int const *pins,
+ unsigned int npins);
int gpiochip_add_pingroup_range(struct gpio_chip *gc,
struct pinctrl_dev *pctldev,
unsigned int gpio_offset, const char *pin_group);
@@ -765,6 +768,15 @@ gpiochip_add_pin_range(struct gpio_chip *gc, const char *pinctl_name,
{
return 0;
}
+
+static inline int
+gpiochip_add_pin_range_sparse(struct gpio_chip *gc, const char *pinctl_name,
+ unsigned int gpio_offset, unsigned int const *pins,
+ unsigned int npins)
+{
+ return 0;
+}
+
static inline int
gpiochip_add_pingroup_range(struct gpio_chip *gc,
struct pinctrl_dev *pctldev,
--
2.39.5On Mon, Mar 17, 2025 at 04:37:59PM +0100, Thomas Richard wrote:
> Add gpiochip_add_pin_range_sparse() function to add a range for GPIO<->pin
> mapping, using a list of non consecutive pins.
> Previously, it was only possible to add range of consecutive pins using
> gpiochip_add_pin_range_sparse().
>
> The struct pinctrl_gpio_range has a 'pins' member which allows to set a
> list of pins (which can be non consecutive).
> gpiochip_add_pin_range_sparse() is identical to gpiochip_add_pin_range(),
> except it set 'pins' member instead of 'pin_base' member.
...
> +static int __gpiochip_add_pin_range(struct gpio_chip *gc, const char *pinctl_name,
> + unsigned int gpio_offset, unsigned int pin_offset,
> + unsigned int const *pins, unsigned int npins)
I really do not like the __ naming here.
Can we rather create a better one? E.g., gpiochip_add_pin_range_with_pins().
...
> +/**
> + * gpiochip_add_pin_range_sparse() - add a range for GPIO <-> pin mapping
> + * @gc: the gpiochip to add the range for
> + * @pinctl_name: the dev_name() of the pin controller to map to
> + * @gpio_offset: the start offset in the current gpio_chip number space
> + * @pin_list: the list of pins to accumulate in this range
> + * @npins: the number of pins to accumulate in this range
> + * Calling this function directly from a DeviceTree-supported
> + * pinctrl driver is DEPRECATED. Please see Section 2.1 of
> + * Documentation/devicetree/bindings/gpio/gpio.txt on how to
> + * bind pinctrl and gpio drivers via the "gpio-ranges" property.
New API can't be deprecated. You probably want to say
"NOTE, this API is not supposed to be used on DeviceTree-supported platforms."
or something like that.
Also it's not clear which function should be used to clean up this. I would
clarify that: "When tearing down the driver don't forget to remove added ranges
with help of gpiochip_remove_pin_ranges()."
> + * Returns:
> + * 0 on success, or a negative errno on failure.
> + */
> +int gpiochip_add_pin_range_sparse(struct gpio_chip *gc, const char *pinctl_name,
> + unsigned int gpio_offset, unsigned int const *pins,
> + unsigned int npins)
> +{
> + return __gpiochip_add_pin_range(gc, pinctl_name, gpio_offset, 0, pins,
> + npins);
> +}
> +EXPORT_SYMBOL_GPL(gpiochip_add_pin_range_sparse);
To me the gpiochip_add_sparse_pin_range() name sounds better.
...
> int gpiochip_add_pin_range(struct gpio_chip *gc, const char *pinctl_name,
> unsigned int gpio_offset, unsigned int pin_offset,
> unsigned int npins);
> +int gpiochip_add_pin_range_sparse(struct gpio_chip *gc, const char *pinctl_name,
> + unsigned int gpio_offset, unsigned int const *pins,
> + unsigned int npins);
> int gpiochip_add_pingroup_range(struct gpio_chip *gc,
> struct pinctrl_dev *pctldev,
> unsigned int gpio_offset, const char *pin_group);
> +static inline int
> +gpiochip_add_pin_range_sparse(struct gpio_chip *gc, const char *pinctl_name,
> + unsigned int gpio_offset, unsigned int const *pins,
> + unsigned int npins)
> +{
> + return 0;
> +}
Yeah, two stubs, two almost identical doc sections, no explanations of pins in
the core function...
I would rather refactor this to just rename the current function while adding
parameter to it, but leave it is being exported, just add a description to the
new parameter into the kernel doc. Make two new out of it as static inline:rs.
--
With Best Regards,
Andy Shevchenko
On 3/17/25 17:59, Andy Shevchenko wrote: > On Mon, Mar 17, 2025 at 04:37:59PM +0100, Thomas Richard wrote: >> Add gpiochip_add_pin_range_sparse() function to add a range for GPIO<->pin >> mapping, using a list of non consecutive pins. >> Previously, it was only possible to add range of consecutive pins using >> gpiochip_add_pin_range_sparse(). >> >> The struct pinctrl_gpio_range has a 'pins' member which allows to set a >> list of pins (which can be non consecutive). >> gpiochip_add_pin_range_sparse() is identical to gpiochip_add_pin_range(), >> except it set 'pins' member instead of 'pin_base' member. > > ... > >> +static int __gpiochip_add_pin_range(struct gpio_chip *gc, const char *pinctl_name, >> + unsigned int gpio_offset, unsigned int pin_offset, >> + unsigned int const *pins, unsigned int npins) > > I really do not like the __ naming here. > Can we rather create a better one? E.g., gpiochip_add_pin_range_with_pins(). > > ... > >> +/** >> + * gpiochip_add_pin_range_sparse() - add a range for GPIO <-> pin mapping >> + * @gc: the gpiochip to add the range for >> + * @pinctl_name: the dev_name() of the pin controller to map to >> + * @gpio_offset: the start offset in the current gpio_chip number space >> + * @pin_list: the list of pins to accumulate in this range >> + * @npins: the number of pins to accumulate in this range > >> + * Calling this function directly from a DeviceTree-supported >> + * pinctrl driver is DEPRECATED. Please see Section 2.1 of >> + * Documentation/devicetree/bindings/gpio/gpio.txt on how to >> + * bind pinctrl and gpio drivers via the "gpio-ranges" property. > > New API can't be deprecated. You probably want to say > "NOTE, this API is not supposed to be used on DeviceTree-supported platforms." > or something like that. > > Also it's not clear which function should be used to clean up this. I would > clarify that: "When tearing down the driver don't forget to remove added ranges > with help of gpiochip_remove_pin_ranges()." gpiochip_remove_pin_ranges() is automatically called when the gpiochip is removed [1] [1] https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/gpio/gpiolib.c#L1189 Regards, Thomas
© 2016 - 2025 Red Hat, Inc.