Re: [PATCH v3 1/5] iio: proximity: sx9500: Assign interrupt from GpioIo()

[Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>]

On Sun, Nov 19, 2017 at 03:24:11PM +0000, Jonathan Cameron wrote:
> On Mon, 6 Nov 2017 11:35:56 +0200
> Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> 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 <andriy.shevchenko@xxxxxxxxxxxxxxx> 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 <andriy.shevchenko@xxxxxxxxxxxxxxx>  
> > > 
> > > 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 <mika.westerberg@xxxxxxxxxxxxxxx>
> 
> 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!
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html