Thread (22 messages) 22 messages, 4 authors, 2015-02-17

Re: [PATCH 2/2] HID: i2c-hid: Add support for GPIO interrupts

From: Mark Rutland <mark.rutland@arm.com>
Date: 2015-01-27 14:34:07
Also in: lkml

On Tue, Jan 27, 2015 at 11:30:41AM +0000, Mika Westerberg wrote:
On Tue, Jan 27, 2015 at 11:14:58AM +0000, Mark Rutland wrote:
quoted
On Tue, Jan 27, 2015 at 10:59:31AM +0000, Mika Westerberg wrote:
quoted
On Tue, Jan 27, 2015 at 10:39:25AM +0000, Mark Rutland wrote:
quoted
quoted
If the above is not the right way to use GPIOs as interrupt, can you
please tell me how it is done then?

So lets say we have a device which generates an interrupt:

	device@f00 {
		compatible = "some-interrupting-device";
		reg = <0xf00 0x100>;
		interrupts = < ... >;
	};

It's intended that this is connected to an interrupt controller:

	ic: interrupt-controller@b00 {
		compatible = "some-interrupt-controller";
		reg = <0xb00 0x100>;
		#interrupt-cells = <1>;
	};

	device@f00 {
		compatible = "some-interrupting-device";
		reg = <0xf00 0x100>;
		interrupt-parent = <&ic>;
		interrupts = <0x3>;
	};

But in some cases, this gets connected to a GPIO controller. In these
cases, the device is still logically generating an interrupt, and the
fact that the endpoint is an interrupt controller is irrelevant from the
PoV of the device. So we acknowledge that the GPIO controller is also
capable of acting as an interrupt controller, and mark it as such:

	gc: gpio-controller@000 {
		compatible = "some-gpio-controller";
		reg = <0x000 0x100>;
		#gpio-cells = <1>;
		#interrupt-cells = <1>;
	};

	device@f00 {
		compatible = "some-interrupting-device";
		reg = <0xf00 0x100>;
		interrupt-parent = <&gc>;
		interrupts = <0x1>;
	};

Thus the device binding only describes the logical interrupt, and the
driver only needs to handle interrupts.
OK.
quoted
In cases where the binding/driver actually care about the GPIO being a
GPIO (e.g. for card detect in an MMC controller), describing the GPIO as
a GPIO makes sense, and we can try gpio_to_irq as an optimisation over
polling the state of the GPIO.
Well, I've seen touch panels where you actually need to switch the GPIO
to be output and do some magic before you can use the same GPIO as an
interrupt.
Ok. That's a nasty case, but surely in that case the relevant GPIO
shoiuld be a GpioIO object for output?
I can't remember the details anymore, possibly it was GpioIo().

Nothing prevents you from using GpioIo() as an interrupt.
Certainly. As I mention above, in the case of something like a card
detect pin, it makes sense to be able to acquire an interrupt for the
GPIO.
quoted
quoted
quoted
quoted
BTW, passing NULL to gpiod_get() implies property named "gpios" in DT
(which is why I added it to the documentation).
Sure. My concern is that we should not need to deal with GPIOs in this
case were the GPIO is only there to function as an interrupt.

Given that GpioInt seems to describe an interrupt which happens to be
backed by a GPIO, I don't understand what it is necessary to translate
this as a GPIO rather than an interrupt. If it were going to be used as
a GPIO, then it would be a GpioIO object, no?
OK, so where do you propose we handle the translation if not in the
driver? Also keep in mind that some of the devices may have multiple
GpioInt()s.
To me it seems that GpioInt objects should be translated to interrupts
by some core code. How are interrupts described and handed in ACPI? Are
they resource along the lines of GpioInts, or are they a completely
separate class of device property?
They are similar resources in _CRS, like GpioIo/GpioInt etc. Below is
from another touch panel:

	Name (_CRS, ResourceTemplate () {
		I2cSerialBus (0x004C, ControllerInitiated, 0x00061A80,
			AddressingMode7Bit, "\\_SB.PCI0.I2C1", 0x00, ResourceConsumer,,)
		Interrupt (ResourceConsumer, Level, ActiveLow, Exclusive, ,, )
		{
		    0x00000022,
		}
	})

If we see one of the above we automatically add it to client->irq in
case of I2C device.
Ok, that allays my fear w.r.t. ordering of the resources.

As I see it, the fact that we convert GpioInt entries to GPIOs rather
than irqs when parsing _CRS is the issue here, and to me it makes no
sense that we do so. Were we to treat them as interrupts, the binding is
fine as-is, and we'd do the same thing in DT and ACPI.

The reason GpioInt is separate from GpioIo is that a GpioInt _is_ an
interrupt (which happens to be backed by a GPIO), and is not something
that necessarily makes sense as a GPIO.

So why do we currently ignore the GpioInt/GpioIo distinction and treat
GpioInts as GPIOs rather than interrupts?

Thanks,
Mark.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help