LKML Archive on lore.kernel.org help / color / mirror / Atom feed
From: Mark Rutland <firstname.lastname@example.org> To: Mika Westerberg <email@example.com> Cc: Jiri Kosina <firstname.lastname@example.org>, Benjamin Tissoires <email@example.com>, Rob Herring <firstname.lastname@example.org>, Pawel Moll <Pawel.Moll@arm.com>, Ian Campbell <email@example.com>, Kumar Gala <firstname.lastname@example.org>, Jarkko Nikula <email@example.com>, "firstname.lastname@example.org" <email@example.com>, "firstname.lastname@example.org" <email@example.com>, Linus Walleij <firstname.lastname@example.org>, Alexandre Courbot <email@example.com> Subject: Re: [PATCH 2/2] HID: i2c-hid: Add support for GPIO interrupts Date: Tue, 27 Jan 2015 15:57:47 +0000 [thread overview] Message-ID: <20150127155747.GJ17721@leverpostej> (raw) In-Reply-To: <20150127152127.GV1451@lahna.fi.intel.com> On Tue, Jan 27, 2015 at 03:21:27PM +0000, Mika Westerberg wrote: > On Tue, Jan 27, 2015 at 03:06:06PM +0000, Mark Rutland wrote: > > On Tue, Jan 27, 2015 at 02:41:27PM +0000, Mika Westerberg wrote: > > > On Tue, Jan 27, 2015 at 02:33:34PM +0000, Mark Rutland wrote: > > > > 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. > > > > > > I would rather say that GpioInt *is* a GPIO. That can then used as an > > > interrupt but it should not prevent you from using it as GPIO instead. > > > For example if you just want to poll that something is 0 or 1. That > > > should be possible as well and nothing say that you cannot do that for > > > GpioInt(). > > > > >From my POV a GpioInt is logically an interrupt, or it would be a > > GpioIo. That doesn't necessarily mean it's invalid to try to query its > > state as a GPIO, but I do not think that it makes sense to handle it by > > default as a GPIO given that it was handed to us as a GpioInt so that it > > can be used as an interrupt. > > > > If it's just the case that ACPI and DT differ w.r.t. how this case (an > > interrupt line wired to a GPIO) is described, that in itself is fine; > > different standards have different models. > > > > However, I do not think we must change the DT binding and violate > > established DT practice simply becuase on the ACPI side things are > > different (nor would it make sense to do things the other way around). > > If the two have different rules, then we should handle those rules > > separately rather than trying to force the two together when they > > clearly don't fit. > > I agree. > > > In the driver that can easily be achieved with separate probe paths. > > Yes, that's what this patch is doing. It has different paths for > interrupt and GPIO cases. If we find that there is an interrupt number > already, then we use that directly. > > If not we try to look for a GPIO which we could use as an interrupt. My comment about separate probe paths was w.r.t. ACPI and DT. In DT, acquiring a GPIO here shouldn't be necessary (and ideally shouldn't be allowed) because the GPIO should be described as an interrupt (we need this to have the correct flags). > The reason why this is not done only in the ACPI probe path is that > there is nothing ACPI specific in gpiod_get() and friends. While the high-level APIs are not specific to ACPI or DT, the behaviour we want for DT is subtly different top the behaviour you want for ACPI. In DT we don't necessarily have the relevant flags (e.g. edge vs level, active high vs active low) unless the GPIO is described as an interrupt, so permitting it to be described as a GPIO permits situations which we cannot handle correctly. In the DT case therefore we must not acquire the interrupt GPIO as a GPIO; it must be described as an interrupt. So if we need to acquire a GPIO in the ACPI case, this should be limited to the ACPI case. Looking further at ACPI, the flags issue also seems to be a problem there. If the interrupt GPIO were described as a GpioIo, then you could acquire it as a GPIO with gpiod_get(), but wouldn't have the EdgeLevel and ActiveLevel properties, and therefore cannot necessarily correctly convert the GPIO to an irq, no? Thanks, Mark.
next prev parent reply other threads:[~2015-01-27 15:58 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 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 [this message] 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=20150127155747.GJ17721@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 \ --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).