LKML Archive on lore.kernel.org help / color / mirror / Atom feed
From: Mark Rutland <email@example.com> To: Mika Westerberg <firstname.lastname@example.org> Cc: Jiri Kosina <email@example.com>, Benjamin Tissoires <firstname.lastname@example.org>, Rob Herring <email@example.com>, Pawel Moll <Pawel.Moll@arm.com>, Ian Campbell <firstname.lastname@example.org>, Kumar Gala <email@example.com>, Jarkko Nikula <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org> Subject: Re: [PATCH 2/2] HID: i2c-hid: Add support for GPIO interrupts Date: Mon, 26 Jan 2015 16:01:20 +0000 [thread overview] Message-ID: <20150126160044.GK23313@leverpostej> (raw) In-Reply-To: <20150126151637.GH1451@lahna.fi.intel.com> On Mon, Jan 26, 2015 at 03:16:37PM +0000, Mika Westerberg wrote: > On Mon, Jan 26, 2015 at 02:50:01PM +0000, Mark Rutland wrote: > > On Mon, Jan 26, 2015 at 02:47:29PM +0000, Mika Westerberg wrote: > > > On Mon, Jan 26, 2015 at 02:37:24PM +0000, Mark Rutland wrote: > > > > On Mon, Jan 26, 2015 at 02:29:33PM +0000, Mika Westerberg wrote: > > > > > The HID over I2C specification allows to have the interrupt for a HID > > > > > device to be GPIO instead of directly connected to the IO-APIC. > > > > > > > > > > Add support for this so that when the driver does not find proper interrupt > > > > > number from the I2C client structure we check if the device has property > > > > > named "gpios". This is then assumed to be the GPIO that serves as an > > > > > interrupt for the device. > > > > > > > > > > Signed-off-by: Mika Westerberg <email@example.com> > > > > > --- > > > > > .../devicetree/bindings/hid/hid-over-i2c.txt | 5 +- > > > > > drivers/hid/i2c-hid/i2c-hid.c | 70 ++++++++++++++++------ > > > > > 2 files changed, 56 insertions(+), 19 deletions(-) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > > index 488edcb264c4..8f4a99dad3b9 100644 > > > > > --- a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > > +++ b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > > @@ -15,7 +15,10 @@ Required properties: > > > > > - reg: i2c slave address > > > > > - hid-descr-addr: HID descriptor address > > > > > - interrupt-parent: the phandle for the interrupt controller > > > > > -- interrupts: interrupt line > > > > > +- interrupts: interrupt line if the device uses IO-APIC interrupts > > > > > + > > > > > +Optional properties: > > > > > +- gpios: GPIO used as an interrupt if the device uses GPIO interrupts > > > > > > > > Elsewhere we've said that for a GPIO acting as an interrupt line, GPIO > > > > controller should be marked as an interrupt-controller, and the GPIO > > > > described as an interrupt line. That also gets you the appropriate > > > > configuration for the GPIO as an interrupt. > > > > > > > > Does this GPIO serve any other purpose than an ersatz interrupt line? > > > > > > It is just an interrupt. > > > > > > > If not, it should probably be described as an interrupt. From the PoV of > > > > this device, it's just an interrupt controller hooked up to the > > > > interrupt pin. > > > > > > What I'm trying to do is to get a GPIO that is described in ACPI (as > > > GpioInt() in _CRS) to be supported in this driver using gpiolib like > > > this: > > > > > > desc = gpiod_get(&client->dev, NULL); > > > > > > This calls to find "gpios" property which ends up finding the GpioInt() > > > in _CRS. > > > > I understand what you are trying to do, but I disagree on the principle. > > If it's logically an interrupt, it should be described as an interrupt. > > It is a GPIO line that is used as interrupt. It is not IO-APIC interrupt > or anything like that. It will be handled through a GPIO driver. I understand that the interrupt line is wired up to a GPIO controller, where the GPIO controller is able to raise an interrupt as required. However, from the PoV of the I2C device, this doesn't matter. From it's PoV it raises an interrupt, and that's all. It has no idea what happens to be wired up to its IRQ pin, and nor should it. > > If ACPI lacks the ability to describe things in that fashion, it's yet > > another reason that we shouldn't be pretending that DT and ACPI are the > > same thing. > > I'm not saying they are the same thing (they're not). I'm trying to get > a GPIO from ACPI translated to an interrupt so that the driver can use > it. Preferably so that the DT description does not prevent people from > using the same on non-ACPI platforms. If you're following the Microsoft HID over I2C ACPI spec, why is this DT binding document relevant. Assuming you're following the spec, you won't be using _DSD with "hid-over-i2c". If you're not following the spec then you aren't following the spec, so the spec is irrelevant. > ACPI can desribe Interrupt(), GpioInt() and GpioIo() just fine. It is > the Microsoft HID over I2C specification that says it should be > GpioInt() even though we have seen Interrupt() used there as well. Ok, so if the HID over I2C spec says that for ACPI, do that for ACPI. I don't follow why the DT binding should do this. Mark.
next prev parent reply other threads:[~2015-01-26 16:02 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-01-26 14:29 [PATCH 1/2] HID: i2c-hid: The interrupt should be level sensitive Mika Westerberg 2015-01-26 14:29 ` [PATCH 2/2] HID: i2c-hid: Add support for GPIO interrupts Mika Westerberg 2015-01-26 14:37 ` Mark Rutland 2015-01-26 14:47 ` Mika Westerberg 2015-01-26 14:50 ` Mark Rutland 2015-01-26 15:16 ` Mika Westerberg 2015-01-26 16:01 ` Mark Rutland [this message] 2015-01-26 16:13 ` Mika Westerberg 2015-01-26 16:39 ` Mark Rutland 2015-01-27 10:16 ` Mika Westerberg 2015-01-27 10:39 ` Mark Rutland 2015-01-27 10:59 ` Mika Westerberg 2015-01-27 11:14 ` Mark Rutland 2015-01-27 11:30 ` Mika Westerberg 2015-01-27 14:33 ` Mark Rutland 2015-01-27 14:41 ` Mika Westerberg 2015-01-27 15:06 ` Mark Rutland 2015-01-27 15:21 ` Mika Westerberg 2015-01-27 15:57 ` Mark Rutland 2015-01-27 17:10 ` Mika Westerberg 2015-01-29 20:09 ` [PATCH 1/2] HID: i2c-hid: The interrupt should be level sensitive Benjamin Tissoires 2015-02-17 12:24 ` Jiri Kosina
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20150126160044.GK23313@leverpostej \ --firstname.lastname@example.org \ --cc=Pawel.Moll@arm.com \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).