From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mika Westerberg Subject: Re: [PATCH v3 1/5] iio: proximity: sx9500: Assign interrupt from GpioIo() Date: Mon, 20 Nov 2017 12:30:27 +0200 Message-ID: <20171120103027.GA22431@lahna.fi.intel.com> References: <20171103130340.42459-1-andriy.shevchenko@linux.intel.com> <20171104031119.00006e56@huawei.com> <20171106093556.GR2283@lahna.fi.intel.com> <20171119152411.12e4c7cb@archlinux> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20171119152411.12e4c7cb@archlinux> Sender: linux-acpi-owner@vger.kernel.org To: Jonathan Cameron Cc: Jonathan Cameron , Andy Shevchenko , linux-iio@vger.kernel.org, Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Linus Walleij , linux-acpi@vger.kernel.org, linux-gpio@vger.kernel.org List-Id: linux-gpio@vger.kernel.org On Sun, Nov 19, 2017 at 03:24:11PM +0000, Jonathan Cameron wrote: > On Mon, 6 Nov 2017 11:35:56 +0200 > Mika Westerberg wrote: > > > On Sat, Nov 04, 2017 at 03:11:19AM +0000, Jonathan Cameron wrote: > > > On Fri, 3 Nov 2017 15:03:36 +0200 > > > Andy Shevchenko wrote: > > > > > > > The commit 0f0796509c07 > > > > > > > > ("iio: remove gpio interrupt probing from drivers that use a single > > > > interrupt") > > > > > > > > removed custom IRQ assignment for the drivers which are enumerated via > > > > ACPI or OF. Unfortunately, some ACPI tables have IRQ line defined as > > > > GpioIo() resource and thus automatic IRQ allocation will fail. > > > > > > I'll ask the obvious question - is this allowed under the ACPI spec? > > > > Yes, it is perfectly fine. > > I'm unconvinced... > > > > > > > Partially revert the commit 0f0796509c07 to restore original > > > > behaviour. > > > > > > > > Signed-off-by: Andy Shevchenko > > > > > > I really don't like scattering fixes for broken ACPI tables through > > > drivers... Is there really no better solution to this? > > > > This is not about broken ACPI tables. We just currently have > > "convenience" stuff in the kernel that translates trivial things like a > > single ACPI GpioInt() resource directly to a device interrupt. If the > > table has multiple GpioInt()s or uses GpioIo() then it is up to the > > driver to handle device specific details. > > I agree on the multiple cases needing hanlding. > What bothers me is that there is no guarantee at all that a GpioIo > can even do an interrupt. > > (table 6.2.17 in the 6.1 spec for example makes it clear that we are > in a mess) If it is a gpioio lots of the interrupt specific stuff > cannot be supplied (all the stuff in byte 7) > > So as I read the ACPI specification any interrupt specified as GpioIO > is simply broken. Well, it is the same with DT if you just declare GPIOs as in "xxx-gpios" but still use them to trigger interrupts. Normally you would use GpioInt in ACPI case but sometimes there might actually be need to use the GPIO as input/output without interrupts. I remember there was some I2C connected touchpanel that required some magic to be done when it was put to low power states. In those cases GpioIo is more correct IMHO. It is also possible that the BIOS people just messed this up. > > > On patches like this best to pull in ACPI and GPIO on the cc list. > > > > > > Also cc'd Mika who made the original change to support gpioint. > > > > The patch looks fine to me, > > > > Acked-by: Mika Westerberg > > I'll probably take it anyway to support the platforms doing this particular > piece of fun as doubtlessly the chance of fixing the firmware is next > to nothing... Thanks!