[PATCH v2 1/3] gpiolib: acpi: Respect bias settings for GpioInt() resource

[PATCH v2 1/3] gpiolib: acpi: Respect bias settings for GpioInt() resource

[Andy Shevchenko]

In some cases the GpioInt() resource is coming with bias settings
which may affect system functioning. Respect bias settings for
GpioInt() resource by calling acpi_gpio_update_gpiod_*flags() API
in acpi_dev_gpio_irq_get().

Reported-by: Jamie McClymont <jamie@kwiius.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
v2: preserved ordering of IRQ map (Hans, Mika), added Rb tag (Mika)
 drivers/gpio/gpiolib-acpi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 834a12f3219e..3a39e8a93226 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -942,6 +942,7 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 
 		if (info.gpioint && idx++ == index) {
 			unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
+			enum gpiod_flags dflags = GPIOD_ASIS;
 			char label[32];
 			int irq;
 
@@ -952,8 +953,11 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 			if (irq < 0)
 				return irq;
 
+			acpi_gpio_update_gpiod_flags(&dflags, &info);
+			acpi_gpio_update_gpiod_lookup_flags(&lflags, &info);
+
 			snprintf(label, sizeof(label), "GpioInt() %d", index);
-			ret = gpiod_configure_flags(desc, label, lflags, info.flags);
+			ret = gpiod_configure_flags(desc, label, lflags, dflags);
 			if (ret < 0)
 				return ret;
 
-- 
2.28.0

[PATCH v2 2/3] gpiolib: acpi: Use named item for enum gpiod_flags variable

[Andy Shevchenko]

Use named item instead of plain integer for enum gpiod_flags
to make it clear that even 0 has its own meaning.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
v2: added Rb tag (Mika)
 drivers/gpio/gpiolib-acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 3a39e8a93226..c127b410a7a2 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1136,7 +1136,7 @@ acpi_gpiochip_parse_own_gpio(struct acpi_gpio_chip *achip,
 	int ret;
 
 	*lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
-	*dflags = 0;
+	*dflags = GPIOD_ASIS;
 	*name = NULL;
 
 	ret = fwnode_property_read_u32_array(fwnode, "gpios", gpios,
-- 
2.28.0

[PATCH v2 3/3] gpiolib: of: Use named item for enum gpiod_flags variable

[Andy Shevchenko]

Use named item instead of plain integer for enum gpiod_flags
to make it clear that even 0 has its own meaning.

Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: no changes
 drivers/gpio/gpiolib-of.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 2f895a2b8411..892a513b7a64 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -593,7 +593,7 @@ static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
 
 	xlate_flags = 0;
 	*lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
-	*dflags = 0;
+	*dflags = GPIOD_ASIS;
 
 	ret = of_property_read_u32(chip_np, "#gpio-cells", &tmp);
 	if (ret)
-- 
2.28.0

Re: [PATCH v2 1/3] gpiolib: acpi: Respect bias settings for GpioInt() resource

[Andy Shevchenko]

On Thu, Oct 22, 2020 at 07:58:45PM +0300, Andy Shevchenko wrote:
> In some cases the GpioInt() resource is coming with bias settings
> which may affect system functioning. Respect bias settings for
> GpioInt() resource by calling acpi_gpio_update_gpiod_*flags() API
> in acpi_dev_gpio_irq_get().
> 
> Reported-by: Jamie McClymont <jamie@kwiius.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
> v2: preserved ordering of IRQ map (Hans, Mika), added Rb tag (Mika)

Missed comment as per v1:

This one highly depends on Intel pin control driver changes (for now [1],
but might be more), so it's probably not supposed to be backported (at least
right now).

[1]: https://lore.kernel.org/linux-gpio/20201014104638.84043-1-andriy.shevchenko@linux.intel.com/T/

>  drivers/gpio/gpiolib-acpi.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 834a12f3219e..3a39e8a93226 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -942,6 +942,7 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
>  
>  		if (info.gpioint && idx++ == index) {
>  			unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
> +			enum gpiod_flags dflags = GPIOD_ASIS;
>  			char label[32];
>  			int irq;
>  
> @@ -952,8 +953,11 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
>  			if (irq < 0)
>  				return irq;
>  
> +			acpi_gpio_update_gpiod_flags(&dflags, &info);
> +			acpi_gpio_update_gpiod_lookup_flags(&lflags, &info);
> +
>  			snprintf(label, sizeof(label), "GpioInt() %d", index);
> -			ret = gpiod_configure_flags(desc, label, lflags, info.flags);
> +			ret = gpiod_configure_flags(desc, label, lflags, dflags);
>  			if (ret < 0)
>  				return ret;
>  
> -- 
> 2.28.0
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/3] gpiolib: of: Use named item for enum gpiod_flags variable

[Bartosz Golaszewski]

On Thu, Oct 22, 2020 at 6:58 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Use named item instead of plain integer for enum gpiod_flags
> to make it clear that even 0 has its own meaning.
>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: no changes
>  drivers/gpio/gpiolib-of.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 2f895a2b8411..892a513b7a64 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -593,7 +593,7 @@ static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
>
>         xlate_flags = 0;
>         *lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
> -       *dflags = 0;
> +       *dflags = GPIOD_ASIS;
>
>         ret = of_property_read_u32(chip_np, "#gpio-cells", &tmp);
>         if (ret)
> --
> 2.28.0
>

This patch doesn't seem to depend on the others in this series so I
applied it to my tree. Let me know if that's not the case. I'll let
you take the ACPI patches.

Bartosz

Re: [PATCH v2 1/3] gpiolib: acpi: Respect bias settings for GpioInt() resource

[Andy Shevchenko]

On Thu, Oct 22, 2020 at 9:15 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Thu, Oct 22, 2020 at 07:58:45PM +0300, Andy Shevchenko wrote:

> Missed comment as per v1:
>
> This one highly depends on Intel pin control driver changes (for now [1],
> but might be more), so it's probably not supposed to be backported (at least
> right now).
>
> [1]: https://lore.kernel.org/linux-gpio/20201014104638.84043-1-andriy.shevchenko@linux.intel.com/T/

I probably have to elaborate what above implies from integration p.o.v.

I think the best way is to collect tags from GPIO maintainers and I
can incorporate this into our Intel pin control branch which I will
share with you as PR against GPIO and pin control subsystems.

I'm also all ears for alternatives.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/3] gpiolib: of: Use named item for enum gpiod_flags variable

[Andy Shevchenko]

On Mon, Oct 26, 2020 at 4:40 PM Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
>
> On Thu, Oct 22, 2020 at 6:58 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > Use named item instead of plain integer for enum gpiod_flags
> > to make it clear that even 0 has its own meaning.

> This patch doesn't seem to depend on the others in this series so I
> applied it to my tree. Let me know if that's not the case. I'll let
> you take the ACPI patches.

It's fine, thanks! I have just sent an additional message to elaborate
my vision, but again, the approach for this patch you chose is just
fine.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/3] gpiolib: acpi: Respect bias settings for GpioInt() resource

[Andy Shevchenko]

On Mon, Oct 26, 2020 at 04:41:14PM +0200, Andy Shevchenko wrote:
> On Thu, Oct 22, 2020 at 9:15 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Oct 22, 2020 at 07:58:45PM +0300, Andy Shevchenko wrote:
> 
> > Missed comment as per v1:
> >
> > This one highly depends on Intel pin control driver changes (for now [1],
> > but might be more), so it's probably not supposed to be backported (at least
> > right now).
> >
> > [1]: https://lore.kernel.org/linux-gpio/20201014104638.84043-1-andriy.shevchenko@linux.intel.com/T/
> 
> I probably have to elaborate what above implies from integration p.o.v.
> 
> I think the best way is to collect tags from GPIO maintainers and I
> can incorporate this into our Intel pin control branch which I will
> share with you as PR against GPIO and pin control subsystems.
> 
> I'm also all ears for alternatives.

Linus, Bart, what do you think about this series?

(Linus, the patch 3/3 has been already applied by Bart as it's independent to ACPI anyway)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/3] gpiolib: acpi: Respect bias settings for GpioInt() resource

[Andy Shevchenko]

On Mon, Nov 2, 2020 at 6:51 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Mon, Oct 26, 2020 at 04:41:14PM +0200, Andy Shevchenko wrote:
...

> Linus, Bart, what do you think about this series?
>
> (Linus, the patch 3/3 has been already applied by Bart as it's independent to ACPI anyway)

I have sent a v3

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/3] gpiolib: of: Use named item for enum gpiod_flags variable

[Linus Walleij]

On Mon, Oct 26, 2020 at 3:43 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Oct 26, 2020 at 4:40 PM Bartosz Golaszewski
> <bgolaszewski@baylibre.com> wrote:
> >
> > On Thu, Oct 22, 2020 at 6:58 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > Use named item instead of plain integer for enum gpiod_flags
> > > to make it clear that even 0 has its own meaning.
>
> > This patch doesn't seem to depend on the others in this series so I
> > applied it to my tree. Let me know if that's not the case. I'll let
> > you take the ACPI patches.
>
> It's fine, thanks! I have just sent an additional message to elaborate
> my vision, but again, the approach for this patch you chose is just
> fine.

I might have lost track of the state of this patch set and mix it
up conceptually with some other patch sets so bear with me if
there are mistakes.

I see that Bartosz applied this one patch at least.

Yours,
Linus Walleij

Re: [PATCH v2 3/3] gpiolib: of: Use named item for enum gpiod_flags variable

[Andy Shevchenko]

On Thu, Nov 05, 2020 at 10:05:52AM +0100, Linus Walleij wrote:
> On Mon, Oct 26, 2020 at 3:43 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Oct 26, 2020 at 4:40 PM Bartosz Golaszewski
> > <bgolaszewski@baylibre.com> wrote:
> > >
> > > On Thu, Oct 22, 2020 at 6:58 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > Use named item instead of plain integer for enum gpiod_flags
> > > > to make it clear that even 0 has its own meaning.
> >
> > > This patch doesn't seem to depend on the others in this series so I
> > > applied it to my tree. Let me know if that's not the case. I'll let
> > > you take the ACPI patches.
> >
> > It's fine, thanks! I have just sent an additional message to elaborate
> > my vision, but again, the approach for this patch you chose is just
> > fine.
> 
> I might have lost track of the state of this patch set and mix it
> up conceptually with some other patch sets so bear with me if
> there are mistakes.

Consider to review / Ack [1]. I hope I addressed all / most of the comments.

> I see that Bartosz applied this one patch at least.

Yes.

[1]: https://lore.kernel.org/linux-gpio/20201102191722.81502-1-andriy.shevchenko@linux.intel.com/

-- 
With Best Regards,
Andy Shevchenko