linux-kernel - Re: [PATCH RFC v2 1/6] gpiolib: add gpiochip_add_pin_range

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d7433fe4-289a-4b56-83bb-98073543367b@bootlin.com>
Date: Wed, 9 Apr 2025 15:49:13 +0200
From: Thomas Richard <thomas.richard@...tlin.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Linus Walleij <linus.walleij@...aro.org>,
 Bartosz Golaszewski <brgl@...ev.pl>,
 Geert Uytterhoeven <geert+renesas@...der.be>, linux-gpio@...r.kernel.org,
 linux-kernel@...r.kernel.org, thomas.petazzoni@...tlin.com,
 DanieleCleri@...on.eu, GaryWang@...on.com.tw,
 Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH RFC v2 1/6] gpiolib: add gpiochip_add_pin_range_sparse()
 function

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