linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
To: Stephen Warren <swarren@wwwdotorg.org>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>,
	Kevin Hilman <khilman@linaro.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Lars Poeschel <larsi@wh2.tu-dresden.de>,
	Grant Likely <grant.likely@linaro.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ian.campbell@citrix.com>,
	Kumar Gala <galak@codeaurora.org>,
	Pawel Moll <pawel.moll@arm.com>,
	Tomasz Figa <tomasz.figa@gmail.com>,
	Enric Balletbo i Serra <eballetbo@gmail.com>,
	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>,
	Balaji T K <balajitk@ti.com>, Tony Lindgren <tony@atomide.com>,
	Jon Hunter <jgchunter@gmail.com>,
	linux-gpio@vger.kernel.org, linux-omap@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ
Date: Mon, 23 Sep 2013 18:31:50 +0200	[thread overview]
Message-ID: <52406CF6.3040807@collabora.co.uk> (raw)
In-Reply-To: <524068D4.5040909@wwwdotorg.org>

On 09/23/2013 06:14 PM, Stephen Warren wrote:
> On 09/22/2013 08:40 AM, Javier Martinez Canillas wrote:
>> To use a GPIO pin as an interrupt line, two previous configurations
>> have to be made:
>> 
>> a) Map the GPIO pin as an interrupt line into the Linux irq space
>> b) Enable the GPIO bank and configure the GPIO direction as input
>> 
>> Most GPIO/IRQ chip drivers just create a mapping for every single
>> GPIO pin with irq_create_mapping() on .probe so users usually can
>> assume a) and only have to do b) by using the following sequence:
>> 
>> gpio_request(gpio, "foo IRQ");
>> gpio_direction_input(gpio);
>> 
>> and then request a IRQ with:
>> 
>> irq = gpio_to_irq(gpio);
>> request_irq(irq, ...);
>> 
>> Some drivers know that their IRQ line is being driven by a GPIO
>> and use a similar sequence as the described above but others are
>> not aware or don't care wether their IRQ is a real line from an
>> interrupt controller or a GPIO pin acting as an IRQ.
>> ...
> 
> I think that explanation is a bit like retro-actively implying that
> drivers /should/ be aware of whether their IRQ is a GPIO or not, and
> should be acting differently. However, they should not.
>

I know the patch description is rather verbose but since we have been discussing
this a lot and people have different opinions I wanted to explain some context
and the motivation for the patch.

> I would much rather see a simpler patch description along the lines of:
> 
> The OMAP GPIO controller HW requires that a pin be configured in GPIO
> mode in order to operate as an interrupt input. Since drivers should not
> be aware of whether an interrupt pin is also a GPIO or not, the HW
> should be fully configured/enabled as an IRQ if a driver solely uses IRQ
> APIs such as request_irq, and never calls any GPIO-related APIs. As
> such, add the missing HW setup to the OMAP GPIO controller's irq_chip
> driver.
> 

Thanks for the suggestion, I'll use something like that when I do a proper post
as a PATCH and not RFC.

> The code change looks like it does what I would expect though.
> 

Great, let's see what is the feedback from Santosh and Kevin about the
implementation since they are the maintainers of this driver.

I really hope we can find a solution to this long standing issue.

Thanks a lot and best regards,
Javier

  reply	other threads:[~2013-09-23 16:31 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-22 14:40 [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ Javier Martinez Canillas
     [not found] ` <1379860848-29020-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
2013-09-23 16:14   ` Stephen Warren
2013-09-23 16:31     ` Javier Martinez Canillas [this message]
2013-09-23 16:45 ` Tony Lindgren
2013-09-23 17:00   ` Javier Martinez Canillas
2013-09-23 17:07     ` Tony Lindgren
2013-09-23 18:35       ` Santosh Shilimkar
     [not found]       ` <20130923170724.GG2684-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2013-09-24  7:39         ` Sricharan R
2013-09-24  7:54           ` Javier Martinez Canillas
2013-09-24  8:47             ` Sricharan R
2013-09-23 20:15 ` Linus Walleij
     [not found]   ` <CACRpkdb35DouKsZ+9aWfQmER1vmAaK1dR7VXjoFHTLRR+hBezg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-09-24  5:41     ` Javier Martinez Canillas
2013-09-24 15:27       ` Tony Lindgren

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=52406CF6.3040807@collabora.co.uk \
    --to=javier.martinez@collabora.co.uk \
    --cc=balajitk@ti.com \
    --cc=devicetree@vger.kernel.org \
    --cc=eballetbo@gmail.com \
    --cc=galak@codeaurora.org \
    --cc=grant.likely@linaro.org \
    --cc=ian.campbell@citrix.com \
    --cc=jgchunter@gmail.com \
    --cc=khilman@linaro.org \
    --cc=larsi@wh2.tu-dresden.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=plagnioj@jcrosoft.com \
    --cc=santosh.shilimkar@ti.com \
    --cc=swarren@wwwdotorg.org \
    --cc=tomasz.figa@gmail.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 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).