gpiolib: acpi: Respect bias settings for GpioInt() resource

PATCH Superseded Andy Shevchenko 4 participants 11 messages Oct 22, 2020 – Nov 5, 2020 GPIO

Share & tools

Andy Shevchenko

author

2020-10-22 16:58 UTC #0 lore raw

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

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 <[email protected]>

Signed-off-by Andy Shevchenko <[email protected]>

Reviewed-by Mika Westerberg <[email protected]>

---
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(-)

drivers/gpio/gpiolib-acpi.c +5 -1

index 834a12f3219e..3a39e8a93226 100644
@@ -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;

Signature · 1 line

2.28.0

Andy Shevchenko

author

2020-10-22 16:58 UTC #1 lore raw

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

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 <[email protected]>

Reviewed-by Mika Westerberg <[email protected]>

---
v2: added Rb tag (Mika)

 drivers/gpio/gpiolib-acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpio/gpiolib-acpi.c +1 -1

index 3a39e8a93226..c127b410a7a2 100644
@@ -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,

Signature · 1 line

2.28.0

Andy Shevchenko

author

2020-10-22 16:58 UTC #2 lore raw

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

In reply to 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 <[email protected]>

Signed-off-by Andy Shevchenko <[email protected]>

---
v2: no changes

 drivers/gpio/gpiolib-of.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpio/gpiolib-of.c +1 -1

index 2f895a2b8411..892a513b7a64 100644
@@ -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)

Signature · 1 line

2.28.0

Bartosz Golaszewski

2020-10-26 14:38 UTC #3 lore raw

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

Quoted 25 lines · 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 <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
---
v2: no changes
 drivers/gpio/gpiolib-of.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpio/gpiolib-of.c +1 -2
index 2f895a2b8411..892a513b7a64 100644
@@ -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)
--

Quoted 2 lines

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

Andy Shevchenko

2020-10-26 14:43 UTC #4 lore raw

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

Quoted 10 lines · Bartosz Golaszewski

Quoted 3 lines · Andy Shevchenko
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.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Linus Walleij

2020-11-05 09:06 UTC #5 lore raw

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

Quoted 9 lines · Andy Shevchenko

Quoted 6 lines · Bartosz Golaszewski
Quoted 3 lines · Andy Shevchenko
Use named item instead of plain integer for enum gpiod_flags to make it clear that even 0 has its own meaning.

Quoted 7 lines

Quoted 3 lines
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

Andy Shevchenko

2020-11-05 16:25 UTC #6 lore raw

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

Quoted 15 lines · Linus Walleij

Quoted 13 lines · Andy Shevchenko
Quoted 10 lines · Bartosz Golaszewski
Quoted 3 lines · Andy Shevchenko
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.

Quoted 7 lines

Quoted 3 lines
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.

Quoted 1 line

I see that Bartosz applied this one patch at least.

Yes.

[1]: https://lore.kernel.org/linux-gpio/[email protected]/

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

author

2020-10-22 17:02 UTC #7 lore raw

In reply to Andy Shevchenko

Quoted 10 lines · 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 <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Mika Westerberg <[email protected]>
---
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/[email protected]/T/

Quoted 31 lines

 drivers/gpio/gpiolib-acpi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/gpio/gpiolib-acpi.c +5 -1

index 834a12f3219e..3a39e8a93226 100644
@@ -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;

Signature · 1 line

2.28.0

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

2020-10-26 14:40 UTC #8 lore raw

Quoted 9 lines · Andy Shevchenko

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/[email protected]/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.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

author

2020-11-02 16:50 UTC #9 lore raw

Quoted 12 lines · Andy Shevchenko

Quoted 9 lines · Andy Shevchenko
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/[email protected]/T/

Quoted 7 lines

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)

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

2020-11-03 08:35 UTC #10 lore raw

Quoted 1 line · Andy Shevchenko

On Mon, Oct 26, 2020 at 04:41:14PM +0200, Andy Shevchenko wrote:

...

Quoted 3 lines

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

Signature · 2 lines

With Best Regards,
Andy Shevchenko