rc4 and later log message: gpiochip_add_data_with_key: get_direction failed

rc4 and later log message: gpiochip_add_data_with_key: get_direction failed

[Genes Lists]

[-- Attachment #1: Type: text/plain, Size: 558 bytes --]


mainline starting with rc4. Same with rc5 and commit

  b7c90e3e717abff6fe06445b98be306b732bbd2b

there are now 194 lines logged on boot with:
    
 gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -22

Dont recall seeing these before.

# gpiodetect
gpiochip0 [INTC1055:00] (360 lines)
gpiochip1 [INTC100B:00] (64 lines)

Machine (dell xps 13 plus laptop) seems to be working normally.

Hoping someone will know how to quiet loggin down some without a bisect
but if its really needed I can.

thank you.

-- 
Gene


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: rc4 and later log message: gpiochip_add_data_with_key: get_direction failed

[Genes Lists]

[-- Attachment #1: Type: text/plain, Size: 335 bytes --]

On Sat, 2025-03-08 at 15:45 -0500, Genes Lists wrote:
> ...
> 
> there are now 194 lines logged on boot with:
>     
>  gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -
> 22
> 


For completeness - same log noise with rc5 and through commit
4d872d51bc9d7b899c1f61534e3dbde72613f627.


-- 
Gene


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: rc4 and later log message: gpiochip_add_data_with_key: get_direction failed

[Bartosz Golaszewski]

On Tue, 11 Mar 2025 15:03:59 +0100, Genes Lists <lists@sapience.com> said:
> On Sat, 2025-03-08 at 15:45 -0500, Genes Lists wrote:
>> ...
>>
>> there are now 194 lines logged on boot with:
>>
>>  gpio gpiochip0: gpiochip_add_data_with_key: get_direction failed: -
>> 22
>>
>
>
> For completeness - same log noise with rc5 and through commit
> 4d872d51bc9d7b899c1f61534e3dbde72613f627.
>
>
> --
> Gene
>
>

Hi Gene!

There are two problems here. The issue you're seeing is fixed in next but
not in mainline due to my omission. I will send a patch for that.

On the other hand, the pinctrl driver in question should be fixed too.
Can you try the following change:

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c
b/drivers/pinctrl/intel/pinctrl-intel.c
index d889c7c878e2..0c6925b53d9f 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1068,7 +1068,11 @@ static int intel_gpio_get_direction(struct
gpio_chip *chip, unsigned int offset)

        pin = intel_gpio_to_pin(pctrl, offset, NULL, NULL);
        if (pin < 0)
-               return -EINVAL;
+               /*
+                * For pins configured to functions other than GPIO, default
+                * to the safe INPUT value.
+                */
+               return GPIO_LINE_DIRECTION_IN;

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

?

FYI: This was uncovered by commit 9d846b1aebbe ("gpiolib: check the
return value of gpio_chip::get_direction()").

Bart

Re: rc4 and later log message: gpiochip_add_data_with_key: get_direction failed

[Genes Lists]

[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]

On Tue, 2025-03-11 at 10:40 -0700, Bartosz Golaszewski wrote:
> On Tue, 11 Mar 2025 15:03:59 +0100, Genes Lists <lists@sapience.com>
> said:
> > On Sat, 2025-03-08 at 15:45 -0500, Genes Lists wrote:
> > > 
> > >  gpio gpiochip0: gpiochip_add_data_with_key: get_direction
> > > failed: -
> > > 22
> 
> Hi Gene!
> 
> There are two problems here. The issue you're seeing is fixed in next
> but
> not in mainline due to my omission. I will send a patch for that.
> 
> On the other hand, the pinctrl driver in question should be fixed
> too.
> Can you try the following change:
> ...

> Bart


Thank you Bart.

I tested your pinctl patch on mainline (on commit
0b46b049d6eccd947c361018439fcb596e741d7a) and the same messages are
logged.  Presumably I also need the other patch from next as well.

I can wait till it gets to mainline or can pull from next - if so,
which commit(s) would I need to do that?

It's seems pretty minor, so happy to wait or can patch mainline and
test if you prefer.

thanks,

-- 
Gene


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: rc4 and later log message: gpiochip_add_data_with_key: get_direction failed

[Genes Lists]

[-- Attachment #1: Type: text/plain, Size: 216 bytes --]

On Tue, 2025-03-11 at 15:30 -0400, Genes Lists wrote:
> 
> Thank you Bart.
> 
Sorry - I forgot to mention that the number of warnings with your
pinctl patch  is down to 87 from 194.
 

thanks
-- 
Gene


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: rc4 and later log message: gpiochip_add_data_with_key: get_direction failed

[Bartosz Golaszewski]

On Tue, Mar 11, 2025 at 8:30 PM Genes Lists <lists@sapience.com> wrote:
>
> On Tue, 2025-03-11 at 10:40 -0700, Bartosz Golaszewski wrote:
> > On Tue, 11 Mar 2025 15:03:59 +0100, Genes Lists <lists@sapience.com>
> > said:
> > > On Sat, 2025-03-08 at 15:45 -0500, Genes Lists wrote:
> > > >
> > > >  gpio gpiochip0: gpiochip_add_data_with_key: get_direction
> > > > failed: -
> > > > 22
> >
> > Hi Gene!
> >
> > There are two problems here. The issue you're seeing is fixed in next
> > but
> > not in mainline due to my omission. I will send a patch for that.
> >
> > On the other hand, the pinctrl driver in question should be fixed
> > too.
> > Can you try the following change:
> > ...
>
> > Bart
>
>
> Thank you Bart.
>
> I tested your pinctl patch on mainline (on commit
> 0b46b049d6eccd947c361018439fcb596e741d7a) and the same messages are
> logged.  Presumably I also need the other patch from next as well.
>

No, they are independent. The other one should fix your issue. Could
you please test it[1] and leave your Tested-by under it on the list?

Bartosz

[1] https://lore.kernel.org/all/20250311175631.83779-1-brgl@bgdev.pl/

Re: rc4 and later log message: gpiochip_add_data_with_key: get_direction failed

[Genes Lists]

[-- Attachment #1.1: Type: text/plain, Size: 372 bytes --]

On Tue, 2025-03-11 at 20:50 +0100, Bartosz Golaszewski wrote:
> > 
> 
> No, they are independent. The other one should fix your issue. Could
> you please test it[1] and leave your Tested-by under it on the list?
> 
> Bartosz
> 
> [1] https://lore.kernel.org/all/20250311175631.83779-1-brgl@bgdev.pl/

Done and confirm works fine.

Thank you!


-- 
Gene


[-- Attachment #1.2: Type: text/html, Size: 1323 bytes --]

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: rc4 and later log message: gpiochip_add_data_with_key: get_direction failed

[Genes Lists]

[-- Attachment #1.1: Type: text/plain, Size: 1660 bytes --]

On Tue, 2025-03-11 at 10:40 -0700, Bartosz Golaszewski wrote:
> On Tue, 11 Mar 2025 15:03:59 +0100, Genes Lists <lists@sapience.com>
> said:
> > On Sat, 2025-03-08 at 15:45 -0500, Genes Lists wrote:
> > > ......

> > 
> 
> There are two problems here. The issue you're seeing is fixed in next
> but
> not in mainline due to my omission. I will send a patch for that.
> 
> On the other hand, the pinctrl driver in question should be fixed
> too.
> Can you try the following change:
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c
> b/drivers/pinctrl/intel/pinctrl-intel.c
> index d889c7c878e2..0c6925b53d9f 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -1068,7 +1068,11 @@ static int intel_gpio_get_direction(struct
> gpio_chip *chip, unsigned int offset)
> 
>         pin = intel_gpio_to_pin(pctrl, offset, NULL, NULL);
>         if (pin < 0)
> -               return -EINVAL;
> +               /*
> +                * For pins configured to functions other than GPIO,
> default
> +                * to the safe INPUT value.
> +                */
> +               return GPIO_LINE_DIRECTION_IN;
> 
>         reg = intel_get_padcfg(pctrl, pin, PADCFG0);
>         if (!reg)
> 
> ?
> 
> FYI: This was uncovered by commit 9d846b1aebbe ("gpiolib: check the
> return value of gpio_chip::get_direction()").
> 
> Bart

Hi Bart - I don't see this pincntrl patch in mainline yet -  what's
your thinking on this?

thanks!


-- 
Gene


[-- Attachment #1.2: Type: text/html, Size: 3686 bytes --]

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: rc4 and later log message: gpiochip_add_data_with_key: get_direction failed -> missing pinctrl patch

[Genes Lists]

[-- Attachment #1.1: Type: text/plain, Size: 1951 bytes --]

On Sun, 2025-03-16 at 08:48 -0400, Genes Lists wrote:
> On Tue, 2025-03-11 at 10:40 -0700, Bartosz Golaszewski wrote:
> > On Tue, 11 Mar 2025 15:03:59 +0100, Genes Lists
> > <lists@sapience.com> said:
> > > On Sat, 2025-03-08 at 15:45 -0500, Genes Lists wrote:
> > > > ......
> 
> > 
> > There are two problems here. The issue you're seeing is fixed in
> > next but
> > not in mainline due to my omission. I will send a patch for that.
> > 
> > On the other hand, the pinctrl driver in question should be fixed
> > too.
> > Can you try the following change:
> > 
> > diff --git a/drivers/pinctrl/intel/pinctrl-intel.c
> > b/drivers/pinctrl/intel/pinctrl-intel.c
> > index d889c7c878e2..0c6925b53d9f 100644
> > --- a/drivers/pinctrl/intel/pinctrl-intel.c
> > +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> > @@ -1068,7 +1068,11 @@ static int intel_gpio_get_direction(struct
> > gpio_chip *chip, unsigned int offset)
> > 
> >         pin = intel_gpio_to_pin(pctrl, offset, NULL, NULL);
> >         if (pin < 0)
> > -               return -EINVAL;
> > +               /*
> > +                * For pins configured to functions other than
> > GPIO, default
> > +                * to the safe INPUT value.
> > +                */
> > +               return GPIO_LINE_DIRECTION_IN;
> > 
> >         reg = intel_get_padcfg(pctrl, pin, PADCFG0);
> >         if (!reg)
> > 
> > ?
> > 
> > FYI: This was uncovered by commit 9d846b1aebbe ("gpiolib: check the
> > return value of gpio_chip::get_direction()").
> > 
> > Bart
> 
> Hi Bart - I don't see this pincntrl patch in mainline yet -  what's
> your thinking on this?
> 
> thanks!
> 
> 

Following up - same question - this patch is not in mainline as of
today.  

Can you share your thoughts/plans on this one?

thanks.


-- 
Gene

[-- Attachment #1.2: Type: text/html, Size: 4155 bytes --]

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: rc4 and later log message: gpiochip_add_data_with_key: get_direction failed -> missing pinctrl patch

[Bartosz Golaszewski]

On Fri, Mar 28, 2025 at 11:00 AM Genes Lists <lists@sapience.com> wrote:
>
> On Sun, 2025-03-16 at 08:48 -0400, Genes Lists wrote:
>
> On Tue, 2025-03-11 at 10:40 -0700, Bartosz Golaszewski wrote:
>
> On Tue, 11 Mar 2025 15:03:59 +0100, Genes Lists <lists@sapience.com> said:
>
> On Sat, 2025-03-08 at 15:45 -0500, Genes Lists wrote:
>
> ......
>
>
>
> There are two problems here. The issue you're seeing is fixed in next but
> not in mainline due to my omission. I will send a patch for that.
>
> On the other hand, the pinctrl driver in question should be fixed too.
> Can you try the following change:
>
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c
> b/drivers/pinctrl/intel/pinctrl-intel.c
> index d889c7c878e2..0c6925b53d9f 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -1068,7 +1068,11 @@ static int intel_gpio_get_direction(struct
> gpio_chip *chip, unsigned int offset)
>
>         pin = intel_gpio_to_pin(pctrl, offset, NULL, NULL);
>         if (pin < 0)
> -               return -EINVAL;
> +               /*
> +                * For pins configured to functions other than GPIO, default
> +                * to the safe INPUT value.
> +                */
> +               return GPIO_LINE_DIRECTION_IN;
>
>         reg = intel_get_padcfg(pctrl, pin, PADCFG0);
>         if (!reg)
>
> ?
>
> FYI: This was uncovered by commit 9d846b1aebbe ("gpiolib: check the
> return value of gpio_chip::get_direction()").
>
> Bart
>
>
> Hi Bart - I don't see this pincntrl patch in mainline yet -  what's your thinking on this?
>
> thanks!
>
>
>
> Following up - same question - this patch is not in mainline as of today.
>
> Can you share your thoughts/plans on this one?
>
> thanks.
>
>
> --
>
> Gene
>

My thoughts are that this is in mainline as commit 0102fbf52b93e
("gpiolib: don't check the retval of get_direction() when registering
a chip")[1].

Bart

[1] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0102fbf52b93e609fec0dab53b1fb4fe69113f5e

Re: rc4 and later log message: gpiochip_add_data_with_key: get_direction failed -> pincntrl patch?

[Genes Lists]

[-- Attachment #1: Type: text/plain, Size: 1959 bytes --]

On Sun, 2025-03-16 at 08:48 -0400, Genes Lists wrote:
> On Tue, 2025-03-11 at 10:40 -0700, Bartosz Golaszewski wrote:
> > On Tue, 11 Mar 2025 15:03:59 +0100, Genes Lists
> > <lists@sapience.com> said:
> > > On Sat, 2025-03-08 at 15:45 -0500, Genes Lists wrote:
> > > > ......
> 
> > 
> > There are two problems here. The issue you're seeing is fixed in
> > next but
> > not in mainline due to my omission. I will send a patch for that.
> > 
> > On the other hand, the pinctrl driver in question should be fixed
> > too.
> > Can you try the following change:
> > 
> > diff --git a/drivers/pinctrl/intel/pinctrl-intel.c
> > b/drivers/pinctrl/intel/pinctrl-intel.c
> > index d889c7c878e2..0c6925b53d9f 100644
> > --- a/drivers/pinctrl/intel/pinctrl-intel.c
> > +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> > @@ -1068,7 +1068,11 @@ static int intel_gpio_get_direction(struct
> > gpio_chip *chip, unsigned int offset)
> > 
> >         pin = intel_gpio_to_pin(pctrl, offset, NULL, NULL);
> >         if (pin < 0)
> > -               return -EINVAL;
> > +               /*
> > +                * For pins configured to functions other than
> > GPIO, default
> > +                * to the safe INPUT value.
> > +                */
> > +               return GPIO_LINE_DIRECTION_IN;
> > 
> >         reg = intel_get_padcfg(pctrl, pin, PADCFG0);
> >         if (!reg)
> > 
> > ?
> > 
> > FYI: This was uncovered by commit 9d846b1aebbe ("gpiolib: check the
> > return value of gpio_chip::get_direction()").
> > 
> > Bart
> 
> Hi Bart - I don't see this pincntrl patch in mainline yet -  what's
> your thinking on this?
> 
> thanks!
> 
(resending in plain text - sorry about that)

Hi Bart - I don't see this pincntrl patch in linus tree -  what's your
thinking around this?

thanks!


-- 
Gene

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: rc4 and later log message: gpiochip_add_data_with_key: get_direction failed -> pincntrl patch?

[Bartosz Golaszewski]

On Sat, Mar 29, 2025 at 11:59 AM Genes Lists <lists@sapience.com> wrote:
>
> On Sun, 2025-03-16 at 08:48 -0400, Genes Lists wrote:
> > On Tue, 2025-03-11 at 10:40 -0700, Bartosz Golaszewski wrote:
> > > On Tue, 11 Mar 2025 15:03:59 +0100, Genes Lists
> > > <lists@sapience.com> said:
> > > > On Sat, 2025-03-08 at 15:45 -0500, Genes Lists wrote:
> > > > > ......
> >
> > >
> > > There are two problems here. The issue you're seeing is fixed in
> > > next but
> > > not in mainline due to my omission. I will send a patch for that.
> > >
> > > On the other hand, the pinctrl driver in question should be fixed
> > > too.
> > > Can you try the following change:
> > >
> > > diff --git a/drivers/pinctrl/intel/pinctrl-intel.c
> > > b/drivers/pinctrl/intel/pinctrl-intel.c
> > > index d889c7c878e2..0c6925b53d9f 100644
> > > --- a/drivers/pinctrl/intel/pinctrl-intel.c
> > > +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> > > @@ -1068,7 +1068,11 @@ static int intel_gpio_get_direction(struct
> > > gpio_chip *chip, unsigned int offset)
> > >
> > >         pin = intel_gpio_to_pin(pctrl, offset, NULL, NULL);
> > >         if (pin < 0)
> > > -               return -EINVAL;
> > > +               /*
> > > +                * For pins configured to functions other than
> > > GPIO, default
> > > +                * to the safe INPUT value.
> > > +                */
> > > +               return GPIO_LINE_DIRECTION_IN;
> > >
> > >         reg = intel_get_padcfg(pctrl, pin, PADCFG0);
> > >         if (!reg)
> > >
> > > ?
> > >
> > > FYI: This was uncovered by commit 9d846b1aebbe ("gpiolib: check the
> > > return value of gpio_chip::get_direction()").
> > >
> > > Bart
> >
> > Hi Bart - I don't see this pincntrl patch in mainline yet -  what's
> > your thinking on this?
> >
> > thanks!
> >
> (resending in plain text - sorry about that)
>
> Hi Bart - I don't see this pincntrl patch in linus tree -  what's your
> thinking around this?
>

I already answered here:
https://lore.kernel.org/all/CAMRc=Me0-Tv3ZZDsdijuErYZTswbjQ0obMGh_XtUn+cT-uCZ3A@mail.gmail.com/

Bartosz