Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Paul Menzel]

Dear Bartosz,


On the Intel Kaby Lake Dell XPS 13 9360, Linux 6.14-rc3+ with your 
commit 9d846b1aebbe (gpiolib: check the return value of 
gpio_chip::get_direction()) prints 52 new warnings:

     $ dmesg
     […]
     [    0.000000] DMI: Dell Inc. XPS 13 9360/0596KF, BIOS 2.21.0 
06/02/2022
     […]
     [    5.148927] pci 0000:00:1d.0: PCI bridge to [bus 3c]
     [    5.150955] gpio gpiochip0: gpiochip_add_data_with_key: 
get_direction failed: -22
     [50 times the same]
     [    5.151639] gpio gpiochip0: gpiochip_add_data_with_key: 
get_direction failed: -22
     [    5.151768] ACPI: PCI: Interrupt link LNKA configured for IRQ 11
     […]
     $ lspci -nn -k -s 1d.0
     00:1d.0 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI 
Express Root Port #9 [8086:9d18] (rev f1)
     	Subsystem: Dell Device [1028:075b]
     	Kernel driver in use: pcieport

Judging from the commit messages, this is expected. But what should a 
user seeing this do now?

Also, it probably should not be applied to the stable series, as people 
might monitor warnings and new warnings in stable series might be 
unexpected.


Kind regards,

Paul


[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9d846b1aebbe488f245f1aa463802ff9c34cc078

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Bartosz Golaszewski]

On Fri, Feb 21, 2025 at 9:40 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Bartosz,
>
>
> On the Intel Kaby Lake Dell XPS 13 9360, Linux 6.14-rc3+ with your
> commit 9d846b1aebbe (gpiolib: check the return value of
> gpio_chip::get_direction()) prints 52 new warnings:
>
>      $ dmesg
>      […]
>      [    0.000000] DMI: Dell Inc. XPS 13 9360/0596KF, BIOS 2.21.0
> 06/02/2022
>      […]
>      [    5.148927] pci 0000:00:1d.0: PCI bridge to [bus 3c]
>      [    5.150955] gpio gpiochip0: gpiochip_add_data_with_key:
> get_direction failed: -22
>      [50 times the same]
>      [    5.151639] gpio gpiochip0: gpiochip_add_data_with_key:
> get_direction failed: -22
>      [    5.151768] ACPI: PCI: Interrupt link LNKA configured for IRQ 11
>      […]
>      $ lspci -nn -k -s 1d.0
>      00:1d.0 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI
> Express Root Port #9 [8086:9d18] (rev f1)
>         Subsystem: Dell Device [1028:075b]
>         Kernel driver in use: pcieport
>
> Judging from the commit messages, this is expected. But what should a
> user seeing this do now?
>
> Also, it probably should not be applied to the stable series, as people
> might monitor warnings and new warnings in stable series might be
> unexpected.
>
>
> Kind regards,
>
> Paul
>
>
> [1]:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9d846b1aebbe488f245f1aa463802ff9c34cc078
>

Hi!

What GPIO driver is it using? It's likely that it's not using the
provider API correctly and this change uncovered it, I'd like to take
a look at it and fix it.

Bart

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Paul Menzel]

Dear Bartosz,


Thank you for your quick reply.

Am 21.02.25 um 21:53 schrieb Bartosz Golaszewski:
> On Fri, Feb 21, 2025 at 9:40 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:

>> On the Intel Kaby Lake Dell XPS 13 9360, Linux 6.14-rc3+ with your
>> commit 9d846b1aebbe (gpiolib: check the return value of
>> gpio_chip::get_direction()) prints 52 new warnings:
>>
>>       $ dmesg
>>       […]
>>       [    0.000000] DMI: Dell Inc. XPS 13 9360/0596KF, BIOS 2.21.0 06/02/2022
>>       […]
>>       [    5.148927] pci 0000:00:1d.0: PCI bridge to [bus 3c]
>>       [    5.150955] gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22
>>       [50 times the same]
>>       [    5.151639] gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22
>>       [    5.151768] ACPI: PCI: Interrupt link LNKA configured for IRQ 11
>>       […]
>>       $ lspci -nn -k -s 1d.0
>>       00:1d.0 PCI bridge [0604]: Intel Corporation Sunrise Point-LP PCI Express Root Port #9 [8086:9d18] (rev f1)
>>          Subsystem: Dell Device [1028:075b]
>>          Kernel driver in use: pcieport
>>
>> Judging from the commit messages, this is expected. But what should a
>> user seeing this do now?
>>
>> Also, it probably should not be applied to the stable series, as people
>> might monitor warnings and new warnings in stable series might be
>> unexpected.


>> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9d846b1aebbe488f245f1aa463802ff9c34cc078

> What GPIO driver is it using? It's likely that it's not using the
> provider API correctly and this change uncovered it, I'd like to take
> a look at it and fix it.

How do I find out? The commands below do not return anything.

     $ lsmod | grep gpio
     $ lspci -nn | grep -i gpio
     $ sudo dmesg | grep gpio
     [    5.150955] gpio gpiochip0: gpiochip_add_data_with_key: 
get_direction failed: -22
     [Just these lines match.]


Kind regards,

Paul

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Bartosz Golaszewski]

On Fri, Feb 21, 2025 at 10:02 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> > What GPIO driver is it using? It's likely that it's not using the
> > provider API correctly and this change uncovered it, I'd like to take
> > a look at it and fix it.
>
> How do I find out? The commands below do not return anything.
>
>      $ lsmod | grep gpio
>      $ lspci -nn | grep -i gpio
>      $ sudo dmesg | grep gpio
>      [    5.150955] gpio gpiochip0: gpiochip_add_data_with_key:
> get_direction failed: -22
>      [Just these lines match.]
>
>
> Kind regards,
>
> Paul

If you have libgpiod-tools installed, you can post the output of
gpiodetect here.

Bart

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Paul Menzel]

Dear Bortosz,


Am 23.02.25 um 21:54 schrieb Bartosz Golaszewski:
> On Fri, Feb 21, 2025 at 10:02 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>>
>>> What GPIO driver is it using? It's likely that it's not using the
>>> provider API correctly and this change uncovered it, I'd like to take
>>> a look at it and fix it.
>>
>> How do I find out? The commands below do not return anything.
>>
>>       $ lsmod | grep gpio
>>       $ lspci -nn | grep -i gpio
>>       $ sudo dmesg | grep gpio
>>       [    5.150955] gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22
>>       [Just these lines match.]

> If you have libgpiod-tools installed, you can post the output of
> gpiodetect here.

     $ sudo gpiodetect
     gpiochip0 [INT344B:00] (152 lines)

(In Debian the package name is *gpiod*.)


Kind regards,

Paul

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[brgl]

On Sun, 23 Feb 2025 23:04:05 +0100, Paul Menzel <pmenzel@molgen.mpg.de> said:
> Dear Bortosz,
>
>
> Am 23.02.25 um 21:54 schrieb Bartosz Golaszewski:
>> On Fri, Feb 21, 2025 at 10:02 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>>>
>>>> What GPIO driver is it using? It's likely that it's not using the
>>>> provider API correctly and this change uncovered it, I'd like to take
>>>> a look at it and fix it.
>>>
>>> How do I find out? The commands below do not return anything.
>>>
>>>       $ lsmod | grep gpio
>>>       $ lspci -nn | grep -i gpio
>>>       $ sudo dmesg | grep gpio
>>>       [    5.150955] gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22
>>>       [Just these lines match.]
>
>> If you have libgpiod-tools installed, you can post the output of
>> gpiodetect here.
>
>      $ sudo gpiodetect
>      gpiochip0 [INT344B:00] (152 lines)
>

So it's pinctrl-intel, specifically this function in
drivers/pinctrl/intel/pinctrl-intel.c:

static int intel_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
{
	struct intel_pinctrl *pctrl = gpiochip_get_data(chip);
	void __iomem *reg;
	u32 padcfg0;
	int pin;

	pin = intel_gpio_to_pin(pctrl, offset, NULL, NULL);
	if (pin < 0)
		return -EINVAL;

	reg = intel_get_padcfg(pctrl, pin, PADCFG0);
	if (!reg)
		return -EINVAL;

	scoped_guard(raw_spinlock_irqsave, &pctrl->lock)
		padcfg0 = readl(reg);

	if (padcfg0 & PADCFG0_PMODE_MASK)
		return -EINVAL;

	if (__intel_gpio_get_direction(padcfg0) & PAD_CONNECT_OUTPUT)
		return GPIO_LINE_DIRECTION_OUT;

	return GPIO_LINE_DIRECTION_IN;
}

Can you add some logs and see which -EINVAL is returned here specifically?

In any case: Linus: what should be our policy here? There are some pinctrl
drivers which return EINVAL if the pin in question is not in GPIO mode. I don't
think this is an error. Returning errors should be reserved for read failures
and so on. Are you fine with changing the logic here to explicitly default to
INPUT as until recently all errors would be interpreted as such anyway?

Bart

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Paul Menzel]

Dear Bartosz,


Thank you for your support.

Am 24.02.25 um 09:51 schrieb brgl@bgdev.pl:
> On Sun, 23 Feb 2025 23:04:05 +0100, Paul Menzel <pmenzel@molgen.mpg.de> said:

>> Am 23.02.25 um 21:54 schrieb Bartosz Golaszewski:
>>> On Fri, Feb 21, 2025 at 10:02 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>>>>
>>>>> What GPIO driver is it using? It's likely that it's not using the
>>>>> provider API correctly and this change uncovered it, I'd like to take
>>>>> a look at it and fix it.
>>>>
>>>> How do I find out? The commands below do not return anything.
>>>>
>>>>        $ lsmod | grep gpio
>>>>        $ lspci -nn | grep -i gpio
>>>>        $ sudo dmesg | grep gpio
>>>>        [    5.150955] gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22
>>>>        [Just these lines match.]
>>
>>> If you have libgpiod-tools installed, you can post the output of
>>> gpiodetect here.
>>
>>       $ sudo gpiodetect
>>       gpiochip0 [INT344B:00] (152 lines)
> 
> So it's pinctrl-intel, specifically this function in
> drivers/pinctrl/intel/pinctrl-intel.c:
> 
> static int intel_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
> {
> 	struct intel_pinctrl *pctrl = gpiochip_get_data(chip);
> 	void __iomem *reg;
> 	u32 padcfg0;
> 	int pin;
> 
> 	pin = intel_gpio_to_pin(pctrl, offset, NULL, NULL);
> 	if (pin < 0)
> 		return -EINVAL;
> 
> 	reg = intel_get_padcfg(pctrl, pin, PADCFG0);
> 	if (!reg)
> 		return -EINVAL;
> 
> 	scoped_guard(raw_spinlock_irqsave, &pctrl->lock)
> 		padcfg0 = readl(reg);
> 
> 	if (padcfg0 & PADCFG0_PMODE_MASK)
> 		return -EINVAL;
> 
> 	if (__intel_gpio_get_direction(padcfg0) & PAD_CONNECT_OUTPUT)
> 		return GPIO_LINE_DIRECTION_OUT;
> 
> 	return GPIO_LINE_DIRECTION_IN;
> }
> 
> Can you add some logs and see which -EINVAL is returned here specifically?

Sure. I used the diff below, and added `dyndbg="file pinctrl-intel.c 
+p"` added to `/boot/grub/grub.cfg`.

```
diff --git a/drivers/pinctrl/intel/pinctrl-intel.c 
b/drivers/pinctrl/intel/pinctrl-intel.c
index 527e4b87ae52..f0922d9e64ee 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1067,18 +1067,24 @@ static int intel_gpio_get_direction(struct 
gpio_chip *chip, unsigned int offset)
         int pin;

         pin = intel_gpio_to_pin(pctrl, offset, NULL, NULL);
-       if (pin < 0)
+       if (pin < 0) {
+               dev_dbg(pctrl->dev, "pin < 0");
                 return -EINVAL;
+       }

         reg = intel_get_padcfg(pctrl, pin, PADCFG0);
-       if (!reg)
+       if (!reg) {
+               dev_dbg(pctrl->dev, "not reg");
                 return -EINVAL;
+       }

         scoped_guard(raw_spinlock_irqsave, &pctrl->lock)
                 padcfg0 = readl(reg);

-       if (padcfg0 & PADCFG0_PMODE_MASK)
+       if (padcfg0 & PADCFG0_PMODE_MASK) {
+               dev_dbg(pctrl->dev, "padcfg0 = %x", padcfg0);
                 return -EINVAL;
+       }

         if (__intel_gpio_get_direction(padcfg0) & PAD_CONNECT_OUTPUT)
                 return GPIO_LINE_DIRECTION_OUT;
```

These are the logs:

```
[    0.198584] sunrisepoint-pinctrl INT344B:00: Community0 features: 
0x000000
[    0.198613] sunrisepoint-pinctrl INT344B:00: Community1 features: 
0x00000c
[    0.198629] sunrisepoint-pinctrl INT344B:00: Community2 features: 
0x000000
[    0.198687] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198688] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198693] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198694] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198699] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198700] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198704] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198705] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198709] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198710] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198715] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.198715] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198720] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198721] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198730] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198731] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198735] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.198736] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198741] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.198741] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198746] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198747] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198756] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.198757] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198766] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198767] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198812] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.198812] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198817] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.198818] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198822] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198823] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198837] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.198838] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198843] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198843] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198848] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198849] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198853] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.198854] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198863] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.198864] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198874] sunrisepoint-pinctrl INT344B:00: padcfg0 = 4000700
[    0.198875] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198879] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.198880] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198884] sunrisepoint-pinctrl INT344B:00: padcfg0 = 4000700
[    0.198885] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198938] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198939] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198944] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198945] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198950] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198951] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198972] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198973] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198978] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.198979] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.198989] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.198990] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199006] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.199007] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199011] sunrisepoint-pinctrl INT344B:00: padcfg0 = 84000700
[    0.199012] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199028] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.199029] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199034] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.199035] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199040] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.199041] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199045] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.199046] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199211] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.199211] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199217] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.199217] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199239] sunrisepoint-pinctrl INT344B:00: padcfg0 = 4000700
[    0.199240] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199255] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.199256] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199261] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.199262] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199267] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.199268] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199273] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000702
[    0.199274] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199278] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.199279] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199284] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.199285] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199301] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000502
[    0.199302] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199307] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.199308] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199312] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.199313] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199318] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.199319] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199324] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000700
[    0.199325] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199382] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000600
[    0.199383] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
[    0.199387] sunrisepoint-pinctrl INT344B:00: padcfg0 = 44000600
[    0.199388] gpio gpiochip0: gpiochip_add_data_with_key: get_direction 
failed: -22
```

With

     #define PADCFG0_PMODE_MASK              GENMASK(13, 10)

indeed one bit is always set in this range.

> In any case: Linus: what should be our policy here? There are some pinctrl
> drivers which return EINVAL if the pin in question is not in GPIO mode. I don't
> think this is an error. Returning errors should be reserved for read failures
> and so on. Are you fine with changing the logic here to explicitly default to
> INPUT as until recently all errors would be interpreted as such anyway?


Kind regards,

Paul

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Linus Walleij]

On Mon, Feb 24, 2025 at 9:51 AM <brgl@bgdev.pl> wrote:

> In any case: Linus: what should be our policy here? There are some pinctrl
> drivers which return EINVAL if the pin in question is not in GPIO mode. I don't
> think this is an error. Returning errors should be reserved for read failures
> and so on. Are you fine with changing the logic here to explicitly default to
> INPUT as until recently all errors would be interpreted as such anyway?

Oh hm I guess. There was no defined semantic until now anyway. Maybe
Andy has something to say about it though, it's very much his pin controller.

Yours,
Linus Walleij

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Andy Shevchenko]

On Tue, Feb 25, 2025 at 10:25:00PM +0100, Linus Walleij wrote:
> On Mon, Feb 24, 2025 at 9:51 AM <brgl@bgdev.pl> wrote:
> 
> > In any case: Linus: what should be our policy here? There are some pinctrl
> > drivers which return EINVAL if the pin in question is not in GPIO mode. I don't
> > think this is an error. Returning errors should be reserved for read failures
> > and so on. Are you fine with changing the logic here to explicitly default to
> > INPUT as until recently all errors would be interpreted as such anyway?
> 
> Oh hm I guess. There was no defined semantic until now anyway. Maybe
> Andy has something to say about it though, it's very much his pin controller.

Driver is doing correct things. If you want to be pedantic, we need to return
all possible pin states (which are currently absent from GPIO get_direction()
perspective) and even though it's not possible to tell from the pin muxer
p.o.v. If function is I2C, it's open-drain, if some other, it may be completely
different, but pin muxer might only guesstimate the state of the particular
function is and I do not think guesstimation is a right approach.

We may use the specific error code, though. and document that semantics.

-- 
With Best Regards,
Andy Shevchenko

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Andy Shevchenko]

On Wed, Feb 26, 2025 at 03:37:47PM +0200, Andy Shevchenko wrote:
> On Tue, Feb 25, 2025 at 10:25:00PM +0100, Linus Walleij wrote:
> > On Mon, Feb 24, 2025 at 9:51 AM <brgl@bgdev.pl> wrote:
> > 
> > > In any case: Linus: what should be our policy here? There are some pinctrl
> > > drivers which return EINVAL if the pin in question is not in GPIO mode. I don't
> > > think this is an error. Returning errors should be reserved for read failures
> > > and so on. Are you fine with changing the logic here to explicitly default to
> > > INPUT as until recently all errors would be interpreted as such anyway?
> > 
> > Oh hm I guess. There was no defined semantic until now anyway. Maybe
> > Andy has something to say about it though, it's very much his pin controller.
> 
> Driver is doing correct things. If you want to be pedantic, we need to return
> all possible pin states (which are currently absent from GPIO get_direction()
> perspective) and even though it's not possible to tell from the pin muxer
> p.o.v. If function is I2C, it's open-drain, if some other, it may be completely
> different, but pin muxer might only guesstimate the state of the particular
> function is and I do not think guesstimation is a right approach.
> 
> We may use the specific error code, though. and document that semantics.

Brief looking at the error descriptions and the practical use the best (and
unique enough) choice may be EBADSLT.

-- 
With Best Regards,
Andy Shevchenko

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Bartosz Golaszewski]

On Wed, 26 Feb 2025 at 14:47, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Feb 26, 2025 at 03:37:47PM +0200, Andy Shevchenko wrote:
> > On Tue, Feb 25, 2025 at 10:25:00PM +0100, Linus Walleij wrote:
> > > On Mon, Feb 24, 2025 at 9:51 AM <brgl@bgdev.pl> wrote:
> > >
> > > > In any case: Linus: what should be our policy here? There are some pinctrl
> > > > drivers which return EINVAL if the pin in question is not in GPIO mode. I don't
> > > > think this is an error. Returning errors should be reserved for read failures
> > > > and so on. Are you fine with changing the logic here to explicitly default to
> > > > INPUT as until recently all errors would be interpreted as such anyway?
> > >
> > > Oh hm I guess. There was no defined semantic until now anyway. Maybe
> > > Andy has something to say about it though, it's very much his pin controller.
> >
> > Driver is doing correct things. If you want to be pedantic, we need to return
> > all possible pin states (which are currently absent from GPIO get_direction()
> > perspective) and even though it's not possible to tell from the pin muxer
> > p.o.v. If function is I2C, it's open-drain, if some other, it may be completely
> > different, but pin muxer might only guesstimate the state of the particular
> > function is and I do not think guesstimation is a right approach.
> >
> > We may use the specific error code, though. and document that semantics.
>
> Brief looking at the error descriptions and the practical use the best (and
> unique enough) choice may be EBADSLT.
>

In any case, I proposed to revert to the previous behavior in
gpiochip_add_data() in my follow-up series so the issue should soon go
away.

Bart

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Andy Shevchenko]

On Wed, Feb 26, 2025 at 03:14:24PM +0100, Bartosz Golaszewski wrote:
> On Wed, 26 Feb 2025 at 14:47, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, Feb 26, 2025 at 03:37:47PM +0200, Andy Shevchenko wrote:
> > > On Tue, Feb 25, 2025 at 10:25:00PM +0100, Linus Walleij wrote:
> > > > On Mon, Feb 24, 2025 at 9:51 AM <brgl@bgdev.pl> wrote:
> > > >
> > > > > In any case: Linus: what should be our policy here? There are some pinctrl
> > > > > drivers which return EINVAL if the pin in question is not in GPIO mode. I don't
> > > > > think this is an error. Returning errors should be reserved for read failures
> > > > > and so on. Are you fine with changing the logic here to explicitly default to
> > > > > INPUT as until recently all errors would be interpreted as such anyway?
> > > >
> > > > Oh hm I guess. There was no defined semantic until now anyway. Maybe
> > > > Andy has something to say about it though, it's very much his pin controller.
> > >
> > > Driver is doing correct things. If you want to be pedantic, we need to return
> > > all possible pin states (which are currently absent from GPIO get_direction()
> > > perspective) and even though it's not possible to tell from the pin muxer
> > > p.o.v. If function is I2C, it's open-drain, if some other, it may be completely
> > > different, but pin muxer might only guesstimate the state of the particular
> > > function is and I do not think guesstimation is a right approach.
> > >
> > > We may use the specific error code, though. and document that semantics.
> >
> > Brief looking at the error descriptions and the practical use the best (and
> > unique enough) choice may be EBADSLT.
> 
> In any case, I proposed to revert to the previous behavior in
> gpiochip_add_data() in my follow-up series so the issue should soon go
> away.

Yes, I noted. The above is a material to discuss. We can make that semantics
documented and strict and then one may filter out those errors if/when
required.

-- 
With Best Regards,
Andy Shevchenko

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Paul Menzel]

Dear Andy, dear Bartosz,


Am 26.02.25 um 15:22 schrieb Andy Shevchenko:
> On Wed, Feb 26, 2025 at 03:14:24PM +0100, Bartosz Golaszewski wrote:
>> On Wed, 26 Feb 2025 at 14:47, Andy Shevchenko wrote:
>>> On Wed, Feb 26, 2025 at 03:37:47PM +0200, Andy Shevchenko wrote:
>>>> On Tue, Feb 25, 2025 at 10:25:00PM +0100, Linus Walleij wrote:
>>>>> On Mon, Feb 24, 2025 at 9:51 AM <brgl@bgdev.pl> wrote:
>>>>>
>>>>>> In any case: Linus: what should be our policy here? There are some pinctrl
>>>>>> drivers which return EINVAL if the pin in question is not in GPIO mode. I don't
>>>>>> think this is an error. Returning errors should be reserved for read failures
>>>>>> and so on. Are you fine with changing the logic here to explicitly default to
>>>>>> INPUT as until recently all errors would be interpreted as such anyway?
>>>>>
>>>>> Oh hm I guess. There was no defined semantic until now anyway. Maybe
>>>>> Andy has something to say about it though, it's very much his pin controller.
>>>>
>>>> Driver is doing correct things. If you want to be pedantic, we need to return
>>>> all possible pin states (which are currently absent from GPIO get_direction()
>>>> perspective) and even though it's not possible to tell from the pin muxer
>>>> p.o.v. If function is I2C, it's open-drain, if some other, it may be completely
>>>> different, but pin muxer might only guesstimate the state of the particular
>>>> function is and I do not think guesstimation is a right approach.
>>>>
>>>> We may use the specific error code, though. and document that semantics.
>>>
>>> Brief looking at the error descriptions and the practical use the best (and
>>> unique enough) choice may be EBADSLT.
>>
>> In any case, I proposed to revert to the previous behavior in
>> gpiochip_add_data() in my follow-up series so the issue should soon go
>> away.
> 
> Yes, I noted. The above is a material to discuss. We can make that semantics
> documented and strict and then one may filter out those errors if/when
> required.

I am still seeing this with 6.14.0-rc6-00022-gb7f94fcf5546. Do you know, 
if the reverts are going to be in the final 6.14 release?


Kind regards,

Paul

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Bartosz Golaszewski]

On Fri, Mar 14, 2025 at 12:54 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
> >>>
> >>> Brief looking at the error descriptions and the practical use the best (and
> >>> unique enough) choice may be EBADSLT.
> >>
> >> In any case, I proposed to revert to the previous behavior in
> >> gpiochip_add_data() in my follow-up series so the issue should soon go
> >> away.
> >
> > Yes, I noted. The above is a material to discuss. We can make that semantics
> > documented and strict and then one may filter out those errors if/when
> > required.
>
> I am still seeing this with 6.14.0-rc6-00022-gb7f94fcf5546. Do you know,
> if the reverts are going to be in the final 6.14 release?
>

linux-next should probably be a better point of reference. I'm about
to send out my PR to Linus so it'll be in 6.14-rc7 alright.

Bartosz

Re: Linux logs new warning `gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22`

[Paul Menzel]

Dear Bartosz,


Am 14.03.25 um 13:19 schrieb Bartosz Golaszewski:
> On Fri, Mar 14, 2025 at 12:54 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>>>>>
>>>>> Brief looking at the error descriptions and the practical use the best (and
>>>>> unique enough) choice may be EBADSLT.
>>>>
>>>> In any case, I proposed to revert to the previous behavior in
>>>> gpiochip_add_data() in my follow-up series so the issue should soon go
>>>> away.
>>>
>>> Yes, I noted. The above is a material to discuss. We can make that semantics
>>> documented and strict and then one may filter out those errors if/when
>>> required.
>>
>> I am still seeing this with 6.14.0-rc6-00022-gb7f94fcf5546. Do you know,
>> if the reverts are going to be in the final 6.14 release?
> 
> linux-next should probably be a better point of reference. I'm about
> to send out my PR to Linus so it'll be in 6.14-rc7 alright.

Awesome. Thank you!


Kind regards,

Paul