Re: [PATCH RFC v2 1/6] gpiolib: add gpiochip_add_pin_range_sparse() function

[Thomas Richard]

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