All of lore.kernel.org
 help / color / mirror / Atom feed
From: Grant Likely <grant.likely@linaro.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Alexander Holler <holler@ahsoftware.de>,
	Linux-OMAP <linux-omap@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Javier Martinez Canillas <javier.martinez@collabora.co.uk>,
	Enric Balletbo i Serra <eballetbo@gmail.com>,
	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>,
	Santosh Shilimkar <santosh.shilimkar@ti.com>,
	Kevin Hilman <khilman@linaro.org>, Balaji T K <balajitk@ti.com>,
	Tony Lindgren <tony@atomide.com>,
	Jon Hunter <jgchunter@gmail.com>
Subject: Re: [PATCH] RFC: interrupt consistency check for OF GPIO IRQs
Date: Mon, 29 Jul 2013 22:30:23 -0600	[thread overview]
Message-ID: <CACxGe6uNJNMdZ1HYGgCtLM2t2TWGCR6btjOHVmP=vDXXvvFfRg@mail.gmail.com> (raw)
In-Reply-To: <1375101368-17645-1-git-send-email-linus.walleij@linaro.org>

On Mon, Jul 29, 2013 at 6:36 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> To solve this dilemma, perform an interrupt consistency check
> when adding a GPIO chip: if the chip is both gpio-controller and
> interrupt-controller, walk all children of the device tree,
> check if these in turn reference the interrupt-controller, and
> if they do, loop over the interrupts used by that child and
> perform gpio_reques() and gpio_direction_input() on these,
> making them unreachable from the GPIO side.

Ugh, that's pretty awful, and it doesn't actually solve the root
problem of the GPIO and IRQ subsystems not cooperating. It's also a
very DT-centric solution even though we're going to see the exact same
issue on ACPI machines.

We have to solve the problem in a better way than that. Rearranging
your patch description, here are some of the points you brought up so
I can comment on them...

> This has the following undesired effects:
>
> - The GPIOlib subsystem is not aware that the line is in use
>   and willingly lets some other consumer perform gpio_request()
>   on it, leading to a complex resource conflict if it occurs.

If a gpio line is being both requested as a gpio and used as an
interrupt line, then either a) it's a bug, or b) the gpio line needs
to be used as input only so it is compatible with irq usage. b) should
be supportable.

> - The GPIO debugfs file claims this GPIO line is "free".

Surely we can fix this. I still don't see a problem of having the
controller request the gpio when it is claimed as an irq if we can get
around the problem of another user performing a /valid/ request on the
same GPIO line. The solution may be to have a special form of request
or flag that allows it to be shared.

> - The line direction of the interrupt GPIO line is not
>   explicitly set as input, even though it is obvious that such
>   a line need to be set up in this way, often making the system
>   depend on boot-on defaults for this kind of settings.

Should also be solvable if the gpio request problem is solved.

g.

WARNING: multiple messages have this Message-ID (diff)
From: Grant Likely <grant.likely@linaro.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Kevin Hilman <khilman@linaro.org>, Balaji T K <balajitk@ti.com>,
	Enric Balletbo i Serra <eballetbo@gmail.com>,
	Jon Hunter <jgchunter@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Tony Lindgren <tony@atomide.com>,
	Santosh Shilimkar <santosh.shilimkar@ti.com>,
	Alexander Holler <holler@ahsoftware.de>,
	Linux-OMAP <linux-omap@vger.kernel.org>,
	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>,
	Javier Martinez Canillas <javier.martinez@collabora.co.uk>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] RFC: interrupt consistency check for OF GPIO IRQs
Date: Mon, 29 Jul 2013 22:30:23 -0600	[thread overview]
Message-ID: <CACxGe6uNJNMdZ1HYGgCtLM2t2TWGCR6btjOHVmP=vDXXvvFfRg@mail.gmail.com> (raw)
In-Reply-To: <1375101368-17645-1-git-send-email-linus.walleij@linaro.org>

On Mon, Jul 29, 2013 at 6:36 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> To solve this dilemma, perform an interrupt consistency check
> when adding a GPIO chip: if the chip is both gpio-controller and
> interrupt-controller, walk all children of the device tree,
> check if these in turn reference the interrupt-controller, and
> if they do, loop over the interrupts used by that child and
> perform gpio_reques() and gpio_direction_input() on these,
> making them unreachable from the GPIO side.

Ugh, that's pretty awful, and it doesn't actually solve the root
problem of the GPIO and IRQ subsystems not cooperating. It's also a
very DT-centric solution even though we're going to see the exact same
issue on ACPI machines.

We have to solve the problem in a better way than that. Rearranging
your patch description, here are some of the points you brought up so
I can comment on them...

> This has the following undesired effects:
>
> - The GPIOlib subsystem is not aware that the line is in use
>   and willingly lets some other consumer perform gpio_request()
>   on it, leading to a complex resource conflict if it occurs.

If a gpio line is being both requested as a gpio and used as an
interrupt line, then either a) it's a bug, or b) the gpio line needs
to be used as input only so it is compatible with irq usage. b) should
be supportable.

> - The GPIO debugfs file claims this GPIO line is "free".

Surely we can fix this. I still don't see a problem of having the
controller request the gpio when it is claimed as an irq if we can get
around the problem of another user performing a /valid/ request on the
same GPIO line. The solution may be to have a special form of request
or flag that allows it to be shared.

> - The line direction of the interrupt GPIO line is not
>   explicitly set as input, even though it is obvious that such
>   a line need to be set up in this way, often making the system
>   depend on boot-on defaults for this kind of settings.

Should also be solvable if the gpio request problem is solved.

g.

WARNING: multiple messages have this Message-ID (diff)
From: grant.likely@linaro.org (Grant Likely)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] RFC: interrupt consistency check for OF GPIO IRQs
Date: Mon, 29 Jul 2013 22:30:23 -0600	[thread overview]
Message-ID: <CACxGe6uNJNMdZ1HYGgCtLM2t2TWGCR6btjOHVmP=vDXXvvFfRg@mail.gmail.com> (raw)
In-Reply-To: <1375101368-17645-1-git-send-email-linus.walleij@linaro.org>

On Mon, Jul 29, 2013 at 6:36 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> To solve this dilemma, perform an interrupt consistency check
> when adding a GPIO chip: if the chip is both gpio-controller and
> interrupt-controller, walk all children of the device tree,
> check if these in turn reference the interrupt-controller, and
> if they do, loop over the interrupts used by that child and
> perform gpio_reques() and gpio_direction_input() on these,
> making them unreachable from the GPIO side.

Ugh, that's pretty awful, and it doesn't actually solve the root
problem of the GPIO and IRQ subsystems not cooperating. It's also a
very DT-centric solution even though we're going to see the exact same
issue on ACPI machines.

We have to solve the problem in a better way than that. Rearranging
your patch description, here are some of the points you brought up so
I can comment on them...

> This has the following undesired effects:
>
> - The GPIOlib subsystem is not aware that the line is in use
>   and willingly lets some other consumer perform gpio_request()
>   on it, leading to a complex resource conflict if it occurs.

If a gpio line is being both requested as a gpio and used as an
interrupt line, then either a) it's a bug, or b) the gpio line needs
to be used as input only so it is compatible with irq usage. b) should
be supportable.

> - The GPIO debugfs file claims this GPIO line is "free".

Surely we can fix this. I still don't see a problem of having the
controller request the gpio when it is claimed as an irq if we can get
around the problem of another user performing a /valid/ request on the
same GPIO line. The solution may be to have a special form of request
or flag that allows it to be shared.

> - The line direction of the interrupt GPIO line is not
>   explicitly set as input, even though it is obvious that such
>   a line need to be set up in this way, often making the system
>   depend on boot-on defaults for this kind of settings.

Should also be solvable if the gpio request problem is solved.

g.

  reply	other threads:[~2013-07-30  4:30 UTC|newest]

Thread overview: 106+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-29 12:36 [PATCH] RFC: interrupt consistency check for OF GPIO IRQs Linus Walleij
2013-07-30  4:30 ` Grant Likely [this message]
2013-07-30  4:30   ` Grant Likely
2013-07-30  4:30   ` Grant Likely
2013-07-30 23:44   ` Linus Walleij
2013-07-30 23:44     ` Linus Walleij
2013-07-30 23:44     ` Linus Walleij
2013-07-31  8:35     ` Javier Martinez Canillas
2013-07-31  8:35       ` Javier Martinez Canillas
2013-07-31  8:35       ` Javier Martinez Canillas
2013-08-02  9:57       ` Alexander Holler
2013-08-02  9:57         ` Alexander Holler
2013-08-02  9:57         ` Alexander Holler
2013-08-02 15:35         ` Alexander Holler
2013-08-02 15:35           ` Alexander Holler
2013-08-02 15:35           ` Alexander Holler
2013-08-03  7:23           ` Alexander Holler
2013-08-03  7:23             ` Alexander Holler
2013-08-03  7:23             ` Alexander Holler
2013-09-10  7:00       ` Joel Fernandes
2013-09-10  7:00         ` Joel Fernandes
2013-09-10  7:00         ` Joel Fernandes
2013-09-10 13:17         ` Javier Martinez Canillas
2013-09-10 13:17           ` Javier Martinez Canillas
2013-09-10 13:17           ` Javier Martinez Canillas
2013-09-10 15:00           ` Joel Fernandes
2013-09-10 15:00             ` Joel Fernandes
2013-09-10 15:00             ` Joel Fernandes
2013-09-10 15:48             ` Javier Martinez Canillas
2013-09-10 15:48               ` Javier Martinez Canillas
2013-09-10 15:48               ` Javier Martinez Canillas
2013-09-10 16:25               ` Joel Fernandes
2013-09-10 16:25                 ` Joel Fernandes
2013-09-10 16:25                 ` Joel Fernandes
2013-09-11  7:05             ` Alexander Holler
2013-09-11  7:05               ` Alexander Holler
2013-09-11  7:05               ` Alexander Holler
2013-09-11  7:16               ` Alexander Holler
2013-09-11  7:16                 ` Alexander Holler
2013-09-11  7:16                 ` Alexander Holler
2013-09-11  7:30                 ` Alexander Holler
2013-09-11  7:30                   ` Alexander Holler
2013-09-11  7:30                   ` Alexander Holler
2013-09-11  7:36                   ` Alexander Holler
2013-09-11  7:36                     ` Alexander Holler
2013-09-11  7:36                     ` Alexander Holler
2013-08-13  9:52     ` Lars Poeschel
2013-08-13  9:52       ` Lars Poeschel
2013-08-13  9:52       ` Lars Poeschel
2013-08-19 22:04     ` Laurent Pinchart
2013-08-19 22:04       ` Laurent Pinchart
2013-08-19 22:04       ` Laurent Pinchart
2013-08-21 22:02       ` Linus Walleij
2013-08-21 22:02         ` Linus Walleij
2013-08-21 22:02         ` Linus Walleij
2013-09-06 15:32         ` Laurent Pinchart
2013-09-06 15:32           ` Laurent Pinchart
2013-09-06 15:32           ` Laurent Pinchart
2013-09-11 15:30         ` Alexander Holler
2013-09-11 15:30           ` Alexander Holler
2013-09-11 15:30           ` Alexander Holler
2013-09-11 16:14           ` Javier Martinez Canillas
2013-09-11 16:14             ` Javier Martinez Canillas
2013-09-11 16:14             ` Javier Martinez Canillas
2013-09-11 17:42             ` Alexander Holler
2013-09-11 17:42               ` Alexander Holler
2013-09-11 17:42               ` Alexander Holler
2013-09-12  8:55               ` Alexander Holler
2013-09-12  8:55                 ` Alexander Holler
2013-09-12  8:55                 ` Alexander Holler
2013-09-12 10:11                 ` Javier Martinez Canillas
2013-09-12 10:11                   ` Javier Martinez Canillas
2013-09-12 10:11                   ` Javier Martinez Canillas
2013-09-12 10:28                   ` Alexander Holler
2013-09-12 10:28                     ` Alexander Holler
2013-09-12 10:28                     ` Alexander Holler
2013-09-12 11:09                     ` Alexander Holler
2013-09-12 11:09                       ` Alexander Holler
2013-09-12 11:09                       ` Alexander Holler
2013-09-12 11:26                       ` Alexander Holler
2013-09-12 11:26                         ` Alexander Holler
2013-09-12 11:26                         ` Alexander Holler
2013-09-12 11:37                         ` Alexander Holler
2013-09-12 11:37                           ` Alexander Holler
2013-09-12 11:37                           ` Alexander Holler
2013-09-12 15:19                           ` Stephen Warren
2013-09-12 15:19                             ` Stephen Warren
2013-09-12 15:19                             ` Stephen Warren
2013-09-12 15:57                             ` Alexander Holler
2013-09-12 15:57                               ` Alexander Holler
2013-09-12 15:57                               ` Alexander Holler
2013-09-18  0:36                               ` Grant Likely
2013-09-18  0:36                                 ` Grant Likely
2013-09-18  0:36                                 ` Grant Likely
2013-10-20 12:41                                 ` Laurent Pinchart
2013-10-20 12:41                                   ` Laurent Pinchart
2013-10-20 12:41                                   ` Laurent Pinchart
2013-10-20 15:51                                   ` Tony Lindgren
2013-10-20 15:51                                     ` Tony Lindgren
2013-10-20 15:51                                     ` Tony Lindgren
2013-10-20 21:35                                   ` Stephen Warren
2013-10-20 21:35                                     ` Stephen Warren
2013-10-20 21:35                                     ` Stephen Warren
2013-10-21 23:26                                     ` Laurent Pinchart
2013-10-21 23:26                                       ` Laurent Pinchart
2013-10-21 23:26                                       ` Laurent Pinchart

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='CACxGe6uNJNMdZ1HYGgCtLM2t2TWGCR6btjOHVmP=vDXXvvFfRg@mail.gmail.com' \
    --to=grant.likely@linaro.org \
    --cc=balajitk@ti.com \
    --cc=devicetree@vger.kernel.org \
    --cc=eballetbo@gmail.com \
    --cc=holler@ahsoftware.de \
    --cc=javier.martinez@collabora.co.uk \
    --cc=jgchunter@gmail.com \
    --cc=khilman@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=plagnioj@jcrosoft.com \
    --cc=santosh.shilimkar@ti.com \
    --cc=tony@atomide.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.