Re: [PATCH] gpiolib: acpi: Support GpioInt with active_low polarity

From: Ricardo Ribalda <ribalda@google.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Tomasz Figa <tfiga@google.com>
Subject: Re: [PATCH] gpiolib: acpi: Support GpioInt with active_low polarity
Date: Thu, 29 Oct 2020 19:08:47 +0100	[thread overview]
Message-ID: <CANiDSCsrtL1h+z_f7jQicgwz5nTc33wJGGCjZyeF9aGQJwED7A@mail.gmail.com> (raw)
In-Reply-To: <CAHp75Vc6LhqKvuAeOkVtTAniHGRMGV=7Pa71CNT7por=PRk9eQ@mail.gmail.com>

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