Re: [PATCH] IIO: Adds ACPI support for ST gyroscopes

[Linus Walleij <linus.walleij@linaro.org>]

On Mon, Mar 30, 2015 at 3:52 PM, Octavian Purdila
<octavian.purdila@intel.com> wrote:
> On Mon, Mar 30, 2015 at 4:33 PM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
>> On Mon, Mar 30, 2015 at 03:55:14PM +0300, Octavian Purdila wrote:
>>> On Mon, Mar 30, 2015 at 12:52 PM, Mika Westerberg
>>> <mika.westerberg@linux.intel.com> wrote:
>>> > On Fri, Mar 27, 2015 at 11:36:25AM +0100, Linus Walleij wrote:
>>> >> On Fri, Mar 27, 2015 at 11:06 AM, Mika Westerberg
>>> >> <mika.westerberg@linux.intel.com> wrote:
>>> >> > On Thu, Mar 26, 2015 at 06:28:19PM +0200, Octavian Purdila wrote:
>>> >>
>>> >> >> For the sleep case I think the GPIO controller needs to do the pin
>>> >> >> enable and set input direction operation in it's irq_bus_sync_unlock.
>>> >> >
>>> >> > I wonder how DT handles all this? Is it the boot firmware that sets up
>>> >> > the pins accordingly or is there something we are missing?
>>> >>
>>> >> DT systems mostly do not have firmware for power usecases, they
>>> >> handle it all using pin control. I would more say that is a feature of
>>> >> all-SW systems without power-firmware ideas, without ACPI and
>>> >> without PSCI (well PSCI systems do not care about much more
>>> >> than CPU power down in firmware anyway...)
>>> >
>>> > OK, thanks.
>>> >
>>> > In case of ACPI (where firmware does lot more) it is supposed to
>>> > configure pins based on what is connected, if the firmware knows that.
>>> > Due to bugs in the boot firmware that obviously does not happen in all
>>> > cases (like this one).
>>> >
>>>
>>> Ah, interesting, I was not aware that the firmware was supposed to do
>>> the pin configuration. In this case I think your patch can be merged
>>> as it is Mika, mine doesn't make sense anymore.
>>
>> Unfortunately because of bugs we can't rely that the firmware (BIOS)
>> gets those always right :-(
>>
>>> This particular case is special since we did not performed the tests
>>> on a full system that has the component integrated. We instead used
>>> and I2C to USB bridge to which we connected the component and we
>>> loaded the ACPI table dynamically.
>>
>> Regardless of how did the device appear this always works:
>>
>>   1) request the GPIO (with GPIOD_IN)
>>   2) convert it to IRQ using gpiod_to_irq()
>>
>> Since we cannot be sure that the firmware has configured the pin
>> properly, rather than adding automatic IRQ <-> GPIO translation for ACPI
>> GpioInt I think we are better off if drivers explictly request their
>> GPIOs and configure them as needed.
>
>
> Yes, this is what we are doing now and it works well.
>
> But I am not yet ready to give-up on the automatic IRQ <-> GPIO translation :)
>
> What if we can do the pin configuration in gpiolib right after the
> GPIO controller is initialized. I am thinking of searching the ACPI
> namespace and looking for resources that have GpioInt entries for that
> particular GPIO controller and use them to set the pins to input (and
> ideally also set the other properties like pullups, pulldowns, etc.)

I would rather say that since the gpiolib and irqchip APIs are
supposed to be orthogonal, the irqchip request/unmask or
whatever functions are enabling IRQs should just go and poke
the necessary bits in the GPIO registers to turn the pin into
an input.

spinlocks to avoid collision with gpiolib usecases if necessary,
make GPIO side of the driver not assume it has sole control
over the registers or their contents, use gpiolib_irqchip or other
mechanism that will mark the line as used for IRQs.

Yours,
Linus Walleij