* [PATCH] gpiolib: acpi: Support GpioInt with active_low polarity
@ 2020-10-28 17:17 Ricardo Ribalda
2020-10-28 18:27 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Ricardo Ribalda @ 2020-10-28 17:17 UTC (permalink / raw)
To: Mika Westerberg, Andy Shevchenko, Linus Walleij,
Bartosz Golaszewski, linux-gpio, linux-acpi, linux-kernel
Cc: Ricardo Ribalda
On the current implementation we only support active_high polarity for
GpioInt.
There can be cases where a GPIO has active_low polarity and it is also a
IRQ source.
De-couple the irq_polarity and active_low fields instead of re-use it.
With this patch we support ACPI devices such as:
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
GpioInt (Edge, ActiveBoth, Exclusive, PullDefault, 0x0000,
"\\_SB.PCI0.GPIO", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0064
}
})
Name (_DSD, Package (0x02) // _DSD: Device-Specific Data
{
ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */,
Package (0x01)
{
Package (0x02)
{
"privacy-gpio",
Package (0x04)
{
\_SB.PCI0.XHCI.RHUB.HS07,
Zero,
Zero,
One
}
}
}
})
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/gpio/gpiolib-acpi.c | 8 ++++----
drivers/gpio/gpiolib-acpi.h | 6 ++++--
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 834a12f3219e..bc33c1056391 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -624,7 +624,7 @@ int acpi_gpio_update_gpiod_lookup_flags(unsigned long *lookupflags,
break;
}
- if (info->polarity == GPIO_ACTIVE_LOW)
+ if (info->active_low)
*lookupflags |= GPIO_ACTIVE_LOW;
return 0;
@@ -665,6 +665,7 @@ static int acpi_populate_gpio_lookup(struct acpi_resource *ares, void *data)
agpio->pin_table[pin_index]);
lookup->info.pin_config = agpio->pin_config;
lookup->info.gpioint = gpioint;
+ lookup->info.active_low = !!lookup->active_low;
/*
* Polarity and triggering are only specified for GpioInt
@@ -675,11 +676,10 @@ static int acpi_populate_gpio_lookup(struct acpi_resource *ares, void *data)
*/
if (lookup->info.gpioint) {
lookup->info.flags = GPIOD_IN;
- lookup->info.polarity = agpio->polarity;
+ lookup->info.irq_polarity = agpio->polarity;
lookup->info.triggering = agpio->triggering;
} else {
lookup->info.flags = acpi_gpio_to_gpiod_flags(agpio);
- lookup->info.polarity = lookup->active_low;
}
}
@@ -958,7 +958,7 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
return ret;
irq_flags = acpi_dev_get_irq_type(info.triggering,
- info.polarity);
+ info.irq_polarity);
/* Set type if specified and different than the current one */
if (irq_flags != IRQ_TYPE_NONE &&
diff --git a/drivers/gpio/gpiolib-acpi.h b/drivers/gpio/gpiolib-acpi.h
index 1c6d65cf0629..816a2d7a21ed 100644
--- a/drivers/gpio/gpiolib-acpi.h
+++ b/drivers/gpio/gpiolib-acpi.h
@@ -16,7 +16,8 @@ struct acpi_device;
* @flags: GPIO initialization flags
* @gpioint: if %true this GPIO is of type GpioInt otherwise type is GpioIo
* @pin_config: pin bias as provided by ACPI
- * @polarity: interrupt polarity as provided by ACPI
+ * @irq_polarity: interrupt polarity as provided by ACPI
+ * @active_low: pin polarity as provided by ACPI
* @triggering: triggering type as provided by ACPI
* @quirks: Linux specific quirks as provided by struct acpi_gpio_mapping
*/
@@ -25,7 +26,8 @@ struct acpi_gpio_info {
enum gpiod_flags flags;
bool gpioint;
int pin_config;
- int polarity;
+ int irq_polarity;
+ bool active_low;
int triggering;
unsigned int quirks;
};
--
2.29.0.rc2.309.g374f81d7ae-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] gpiolib: acpi: Support GpioInt with active_low polarity
2020-10-28 17:17 [PATCH] gpiolib: acpi: Support GpioInt with active_low polarity Ricardo Ribalda
@ 2020-10-28 18:27 ` Andy Shevchenko
2020-10-29 15:34 ` Ricardo Ribalda
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2020-10-28 18:27 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Mika Westerberg, Linus Walleij, Bartosz Golaszewski, linux-gpio,
linux-acpi, linux-kernel
On Wed, Oct 28, 2020 at 06:17:57PM +0100, Ricardo Ribalda wrote:
> On the current implementation we only support active_high polarity for
> GpioInt.
>
> There can be cases where a GPIO has active_low polarity and it is also a
> IRQ source.
>
> De-couple the irq_polarity and active_low fields instead of re-use it.
>
> With this patch we support ACPI devices such as:
Is it real device on the market?!
This table is broken. _DSD GPIO active_low is only for GpioIo().
If it is a ChromeBook, please fix the firmware.
> Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
> {
> GpioInt (Edge, ActiveBoth, Exclusive, PullDefault, 0x0000,
> "\\_SB.PCI0.GPIO", 0x00, ResourceConsumer, ,
> )
> { // Pin list
> 0x0064
> }
> })
> Name (_DSD, Package (0x02) // _DSD: Device-Specific Data
> {
> ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */,
> Package (0x01)
> {
> Package (0x02)
> {
> "privacy-gpio",
> Package (0x04)
> {
> \_SB.PCI0.XHCI.RHUB.HS07,
> Zero,
> Zero,
> One
> }
> }
> }
> })
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] gpiolib: acpi: Support GpioInt with active_low polarity
2020-10-28 18:27 ` Andy Shevchenko
@ 2020-10-29 15:34 ` Ricardo Ribalda
2020-10-29 17:31 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Ricardo Ribalda @ 2020-10-29 15:34 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mika Westerberg, Linus Walleij, Bartosz Golaszewski, linux-gpio,
linux-acpi, linux-kernel
Hi Andy
On Thu, Oct 29, 2020 at 3:38 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Oct 28, 2020 at 06:17:57PM +0100, Ricardo Ribalda wrote:
> > On the current implementation we only support active_high polarity for
> > GpioInt.
> >
> > There can be cases where a GPIO has active_low polarity and it is also a
> > IRQ source.
> >
> > De-couple the irq_polarity and active_low fields instead of re-use it.
> >
> > With this patch we support ACPI devices such as:
>
> Is it real device on the market?!
Yes, it is a chromebook.
>
> This table is broken. _DSD GPIO active_low is only for GpioIo().
AFAIK the format of the _DSD is not in the ACPI standard. We have
decided its fields. (please correct me if I am wrong here)
On the other mail I have described why we need to make use of the
active_low on a GpioInt()
If there is another way of describing ActiveBoth + inverted polarity
please let me know and I will go that way.
Thanks
>
> If it is a ChromeBook, please fix the firmware.
>
> > Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
> > {
> > GpioInt (Edge, ActiveBoth, Exclusive, PullDefault, 0x0000,
> > "\\_SB.PCI0.GPIO", 0x00, ResourceConsumer, ,
> > )
> > { // Pin list
> > 0x0064
> > }
> > })
> > Name (_DSD, Package (0x02) // _DSD: Device-Specific Data
> > {
> > ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */,
> > Package (0x01)
> > {
> > Package (0x02)
> > {
> > "privacy-gpio",
> > Package (0x04)
> > {
> > \_SB.PCI0.XHCI.RHUB.HS07,
> > Zero,
> > Zero,
> > One
> > }
> > }
> > }
> > })
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] gpiolib: acpi: Support GpioInt with active_low polarity
2020-10-29 15:34 ` Ricardo Ribalda
@ 2020-10-29 17:31 ` Andy Shevchenko
2020-10-29 18:08 ` Ricardo Ribalda
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2020-10-29 17:31 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Andy Shevchenko, Mika Westerberg, Linus Walleij,
Bartosz Golaszewski, open list:GPIO SUBSYSTEM,
ACPI Devel Maling List, Linux Kernel Mailing List
On Thu, Oct 29, 2020 at 5:37 PM Ricardo Ribalda <ribalda@google.com> wrote:
> On Thu, Oct 29, 2020 at 3:38 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Wed, Oct 28, 2020 at 06:17:57PM +0100, Ricardo Ribalda wrote:
> > > On the current implementation we only support active_high polarity for
> > > GpioInt.
> > >
> > > There can be cases where a GPIO has active_low polarity and it is also a
> > > IRQ source.
> > >
> > > De-couple the irq_polarity and active_low fields instead of re-use it.
> > >
> > > With this patch we support ACPI devices such as:
> >
> > Is it real device on the market?!
>
> Yes, it is a chromebook.
You mean it's already on sale with this broken table?!
> > This table is broken. _DSD GPIO active_low is only for GpioIo().
>
> AFAIK the format of the _DSD is not in the ACPI standard. We have
> decided its fields. (please correct me if I am wrong here)
_DSD is a concept that is part of the spec, but each UUID and its
application is out of scope indeed.
GPIO application to _DSD is described in the in-kernel documentation.
Thanks for pointing out the issues it has.
> On the other mail I have described why we need to make use of the
> active_low on a GpioInt()
>
> If there is another way of describing ActiveBoth + inverted polarity
> please let me know and I will go that way.
I answered it there, please, continue this topic there.
NAK to the proposed change.
> > If it is a ChromeBook, please fix the firmware.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gpiolib: acpi: Support GpioInt with active_low polarity
2020-10-29 17:31 ` Andy Shevchenko
@ 2020-10-29 18:08 ` Ricardo Ribalda
2020-10-29 18:14 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Ricardo Ribalda @ 2020-10-29 18:08 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, Mika Westerberg, Linus Walleij,
Bartosz Golaszewski, open list:GPIO SUBSYSTEM,
ACPI Devel Maling List, Linux Kernel Mailing List, Tomasz Figa
Adding Tomasz in CC in case he wants to share more info about the device.
On Thu, Oct 29, 2020 at 6:31 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Oct 29, 2020 at 5:37 PM Ricardo Ribalda <ribalda@google.com> wrote:
> > On Thu, Oct 29, 2020 at 3:38 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > On Wed, Oct 28, 2020 at 06:17:57PM +0100, Ricardo Ribalda wrote:
> > > > On the current implementation we only support active_high polarity for
> > > > GpioInt.
> > > >
> > > > There can be cases where a GPIO has active_low polarity and it is also a
> > > > IRQ source.
> > > >
> > > > De-couple the irq_polarity and active_low fields instead of re-use it.
> > > >
> > > > With this patch we support ACPI devices such as:
> > >
> > > Is it real device on the market?!
> >
> > Yes, it is a chromebook.
>
> You mean it's already on sale with this broken table?!
I do not agree that it is broken. It follows the current standard ;)
>
> > > This table is broken. _DSD GPIO active_low is only for GpioIo().
> >
> > AFAIK the format of the _DSD is not in the ACPI standard. We have
> > decided its fields. (please correct me if I am wrong here)
>
> _DSD is a concept that is part of the spec, but each UUID and its
> application is out of scope indeed.
>
> GPIO application to _DSD is described in the in-kernel documentation.
> Thanks for pointing out the issues it has.
>
> > On the other mail I have described why we need to make use of the
> > active_low on a GpioInt()
> >
> > If there is another way of describing ActiveBoth + inverted polarity
> > please let me know and I will go that way.
>
> I answered it there, please, continue this topic there.
> NAK to the proposed change.
>
> > > If it is a ChromeBook, please fix the firmware.
Lets agree what is the best way to describe in acpi my usecase and I
will implement it.
>
> --
> With Best Regards,
> Andy Shevchenko
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gpiolib: acpi: Support GpioInt with active_low polarity
2020-10-29 18:08 ` Ricardo Ribalda
@ 2020-10-29 18:14 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2020-10-29 18:14 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Andy Shevchenko, Mika Westerberg, Linus Walleij,
Bartosz Golaszewski, open list:GPIO SUBSYSTEM,
ACPI Devel Maling List, Linux Kernel Mailing List, Tomasz Figa
On Thu, Oct 29, 2020 at 8:09 PM Ricardo Ribalda <ribalda@google.com> wrote:
> Adding Tomasz in CC in case he wants to share more info about the device.
OK, but let's discuss in another thread.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-29 18:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-28 17:17 [PATCH] gpiolib: acpi: Support GpioInt with active_low polarity Ricardo Ribalda
2020-10-28 18:27 ` Andy Shevchenko
2020-10-29 15:34 ` Ricardo Ribalda
2020-10-29 17:31 ` Andy Shevchenko
2020-10-29 18:08 ` Ricardo Ribalda
2020-10-29 18:14 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).