diff mbox series

pinctrl: at91: allow use of of gpio-line-names property

Message ID 4d17866a-d9a4-a3d7-189a-781d18dbea00@axentia.se
State New
Headers show
Series pinctrl: at91: allow use of of gpio-line-names property | expand

Commit Message

Peter Rosin Dec. 6, 2021, 11:32 p.m. UTC 
If no line name is given (by not having a gpio-line-names property,
or by having empty "" strings for some lines), fall back to the
existing pioC12-style line name scheme.

It is useful to be able to explicitly name lines from the schematics
or its function, rather than having the MCU names forced upon every
user.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/pinctrl/pinctrl-at91.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

I don't know if it's sane to fall back to the pioC12-style on empty
strings, or if someone adding a gpio-line-names property should be
responsible for filling in those names "by hand". I generally don't
care what "unused" pins are named, so either is fine by me...

Cheers,
Peter

Comments

Alexander Dahl Dec. 8, 2021, 12:33 p.m. UTC | #1 
Hello Peter,

Am Tue, Dec 07, 2021 at 12:32:03AM +0100 schrieb Peter Rosin:
> If no line name is given (by not having a gpio-line-names property,
> or by having empty "" strings for some lines), fall back to the
> existing pioC12-style line name scheme.
> 
> It is useful to be able to explicitly name lines from the schematics
> or its function, rather than having the MCU names forced upon every
> user.

+1 from me. 

I asked about this some months ago, but I saw no clear
direction in the discussion. So for reference:

https://lore.kernel.org/linux-gpio/946021874.11132.1615900079722@seven.thorsis.com/

HTH & Greets
Alex

> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  drivers/pinctrl/pinctrl-at91.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> I don't know if it's sane to fall back to the pioC12-style on empty
> strings, or if someone adding a gpio-line-names property should be
> responsible for filling in those names "by hand". I generally don't
> care what "unused" pins are named, so either is fine by me...
> 
> Cheers,
> Peter
> 
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 6022496bb6a9..4f108d07e6ad 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -1821,7 +1821,7 @@ static int at91_gpio_probe(struct platform_device *pdev)
>  	int irq, i;
>  	int alias_idx = of_alias_get_id(np, "gpio");
>  	uint32_t ngpio;
> -	char **names;
> +	const char **names;
>  
>  	BUG_ON(alias_idx >= ARRAY_SIZE(gpio_chips));
>  	if (gpio_chips[alias_idx]) {
> @@ -1890,8 +1890,15 @@ static int at91_gpio_probe(struct platform_device *pdev)
>  		goto clk_enable_err;
>  	}
>  
> -	for (i = 0; i < chip->ngpio; i++)
> -		names[i] = kasprintf(GFP_KERNEL, "pio%c%d", alias_idx + 'A', i);
> +	if (of_property_read_string_array(np, "gpio-line-names",
> +					  names, chip->ngpio) != chip->ngpio)
> +		memset(names, 0, chip->ngpio * sizeof(char *));
> +
> +	for (i = 0; i < chip->ngpio; i++) {
> +		if (!names[i] || !names[i][0])
> +			names[i] = kasprintf(GFP_KERNEL,
> +					     "pio%c%d", alias_idx + 'A', i);
> +	}
>  
>  	chip->names = (const char *const *)names;
>  
> -- 
> 2.20.1
>
Andy Shevchenko Dec. 8, 2021, 4:28 p.m. UTC | #2 
On Wed, Dec 8, 2021 at 4:51 PM Alexander Dahl <ada@thorsis.com> wrote:
> Am Tue, Dec 07, 2021 at 12:32:03AM +0100 schrieb Peter Rosin:
> > If no line name is given (by not having a gpio-line-names property,
> > or by having empty "" strings for some lines), fall back to the
> > existing pioC12-style line name scheme.
> >
> > It is useful to be able to explicitly name lines from the schematics
> > or its function, rather than having the MCU names forced upon every
> > user.
>
> +1 from me.
>
> I asked about this some months ago, but I saw no clear
> direction in the discussion. So for reference:
>
> https://lore.kernel.org/linux-gpio/946021874.11132.1615900079722@seven.thorsis.com/

Thanks for the reminder. AFAICS from that discussion we kinda agreed
on the names being excluded from the ABI path. Hence there is good and
bad news.

Bad one: NAK to this patch.
Good one: Please, fix this in the gpiolib respective functions to make
it once for all.

Disclaimer, I'm not a maintainer of this subsystem, I might be
perfectly wrong in my understanding of the state of affairs, let's
hear what Linus and Bart can tell us about the subject. Above are just
my wishes as a contributor to and consumer of this subsystem on how
things should be done.
Peter Rosin Dec. 9, 2021, 11:23 a.m. UTC | #3 
Hi!

On 2021-12-08 17:28, Andy Shevchenko wrote:
> On Wed, Dec 8, 2021 at 4:51 PM Alexander Dahl <ada@thorsis.com> wrote:
>> Am Tue, Dec 07, 2021 at 12:32:03AM +0100 schrieb Peter Rosin:
>>> If no line name is given (by not having a gpio-line-names property,
>>> or by having empty "" strings for some lines), fall back to the
>>> existing pioC12-style line name scheme.
>>>
>>> It is useful to be able to explicitly name lines from the schematics
>>> or its function, rather than having the MCU names forced upon every
>>> user.
>>
>> +1 from me.
>>
>> I asked about this some months ago, but I saw no clear
>> direction in the discussion. So for reference:
>>
>> https://lore.kernel.org/linux-gpio/946021874.11132.1615900079722@seven.thorsis.com/
> 
> Thanks for the reminder. AFAICS from that discussion we kinda agreed
> on the names being excluded from the ABI path. Hence there is good and
> bad news.
> 
> Bad one: NAK to this patch.
> Good one: Please, fix this in the gpiolib respective functions to make
> it once for all.
> 
> Disclaimer, I'm not a maintainer of this subsystem, I might be
> perfectly wrong in my understanding of the state of affairs, let's
> hear what Linus and Bart can tell us about the subject. Above are just
> my wishes as a contributor to and consumer of this subsystem on how
> things should be done.

Understood, I'll send a new patch for gpiolib.c instead.

Cheers,
Peter
diff mbox series

Patch

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 6022496bb6a9..4f108d07e6ad 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -1821,7 +1821,7 @@  static int at91_gpio_probe(struct platform_device *pdev)
 	int irq, i;
 	int alias_idx = of_alias_get_id(np, "gpio");
 	uint32_t ngpio;
-	char **names;
+	const char **names;
 
 	BUG_ON(alias_idx >= ARRAY_SIZE(gpio_chips));
 	if (gpio_chips[alias_idx]) {
@@ -1890,8 +1890,15 @@  static int at91_gpio_probe(struct platform_device *pdev)
 		goto clk_enable_err;
 	}
 
-	for (i = 0; i < chip->ngpio; i++)
-		names[i] = kasprintf(GFP_KERNEL, "pio%c%d", alias_idx + 'A', i);
+	if (of_property_read_string_array(np, "gpio-line-names",
+					  names, chip->ngpio) != chip->ngpio)
+		memset(names, 0, chip->ngpio * sizeof(char *));
+
+	for (i = 0; i < chip->ngpio; i++) {
+		if (!names[i] || !names[i][0])
+			names[i] = kasprintf(GFP_KERNEL,
+					     "pio%c%d", alias_idx + 'A', i);
+	}
 
 	chip->names = (const char *const *)names;