All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <Uwe.Kleine-Koenig@digi.com>
To: David Brownell <david-b@pacbell.net>
Cc: <linux-kernel@vger.kernel.org>
Subject: Re: configure gpio for interrupt function
Date: Wed, 28 May 2008 09:17:47 +0200	[thread overview]
Message-ID: <20080528071747.GA7599@digi.com> (raw)
In-Reply-To: <200805271349.24884.david-b@pacbell.net>

Hello David,

David Brownell wrote:
> On Tuesday 27 May 2008, Uwe Kleine-König wrote:
> > I want to use a driver that in turn uses the gpio API (i.e.
> > drivers/input/keyboard/gpio_keys.c).  My problem with that is, that my
> > machine[1]'s external irqs don't fit into the GPIO API model.  The main
> > problem is that gpio_direction_input isn't enough to get an irq.
> > I need to explicitly set the gpio to an EXT_IRQ function.
> 
> Let me describe your situation a bit differently:  the GPIO
> hardware on your platform doesn't support IRQs, so you want to
> overlay the EXT_IRQ functions as transparently as you can.
OK, if you want ...

> I don't happen to have come across GPIO controllers without
> IRQ support before, unless maybe you consider PXA to work
> like that.  (PXA docs write "GPIO" most places it'd be more
> clear to say "pin", so that's highly confusing.)  I've used
> systems with EXT_INT function options on pins that support
> full GPIO functionality (including pin change IRQs) though.
I think s3c2443 is similar.  For them just using the input mode for a
gpio isn't enough to get an irq, too.  (At least if I believe my
collegues who work with that processor :))

> > (I can workaround that because the EXT_IRQ function allows to read the
> > state of the gpio.  So I can use the EXT_IRQ function if input is
> > requested.)
>.
> That's part of the reason pinmux and GPIO are orthogonal!
IMHO pinmux and GPIO are not orthogonal.  The GPIO API just choosed not
to bother about pinmuxing.

> Your platforms GPIO code can treat EXT_INT pins as input
> GPIOs; it has no reason to care.
???

> So the *cleanest* way to handle this would be as an EXT_IRQ with
> an affiliated GPIO signal, instead of a single funky GPIO.  That
> mapping could be done by gpio_to_irq().
 
> > Another problem is that I cannot implement irq_to_gpio correctly,
> > because several gpio's share the same irq.
> 
> Sure, no problem.  irq_to_gpio() can't succeed for all GPIOs.
> On your platform, I might be inclined to make it fail in all
> cases to force drivers to record both IRQ and GPIO.
> 
> (And yes, that implies gpio_keys needs to generalize a bit.
> Right now it assumes a one-to-one mapping.)
OK.

> > (Note this makes the workaround above less reliable.  Consider two
> > gpios sharing the same irq.  I cannot assert that only one of them can
> > trigger the interrupt.)
> 
> This is another reason to prefer the "cleanest" solution above.
> Drivers could request_irq() the EXT_IRQ(N) signal as IRQF_SHARED,
> and the handler could check the relevant IRQ as needed.
> 
> Of course, this may require code using your platform to know
> that it needs to track two values (EXT_IRQ and GPIO) not one.
> 
> I don't see a need to extend the API.  The current APIs will do what
> you need, if you use them correctly:
> 
>   - gpio_to_irq() encapsulates your CPU's EXT_IRQ(x) pinmux logic;
>     possibly just a table lookup.
OK.

>   - irq_to_gpio() may fail in all cases
OK.  (But then a general driver cannot make use of this function.  IMHO
this should be noted in the docs.  And then you can deprecate it ;))

>   - request_irq() kicks in irq_chip logic to ensure that the pin
>     is either input GPIO (and remuxes it) or already EXT_INT ...
There isn't "the pin".  E.g. IRQ_NS9XXX_EXT0 can trigger on gpios 1, 9,
16 and 105 on ns9215.  These are four different pins.  What should I do
when two of them are in input mode?  (I can remember for which gpios
gpio_to_irq was called, but this looks messy.)

Can I rely on drivers to first call gpio_direction_input and only then
request_irq?  (Otherwise gpio_direction_input needs some addional logic,
too.)  I have no real example, but I can imagine situations where the
gpio that should interrupt the cpu is used as output from time to time.
This might need some handling, too.

As far as I understand now this won't solve the problem of two gpios
sharing the same irq where only one of them should trigger the irq.

>   - free_irq() can remux EXT_INT as GPIO, if it changed muxing in
>     the first place.  (so gpio_direction_output can work, later.)
OK.

>   - set_irq_type() configures the IRQ type via irq_chip hooks
OK.  This is a function I only found yesterday.

> Deprecating irq_to_gpio() seems a bit strong to me, but maybe that's
> because I've never worked on hardware that can't support it.  ISTR that
> Russell King disliked it a lot, but couldn't explain "why" to me.  This
> may be a good enough reason to do so:  an example showing non-portability
> (albeit because the IRQ in question is _not_ a GPIO irq).
> 
> Why don't you whip up a patch doing the right thing for irq_to_gpio(),
> and send it along.  Right now I'd be inclined to sign off on it.
irq_to_gpio() already does the right thing[5] :-)
The task for request_irq isn't clear to me (yet).  Implementing only
gpio_to_irq() isn't nice, because then gpio_key never gets an irq
(instead of just failing because gpio_to_irq() returns an error).
Another problem is that there isn't any support for ns9215 in the
vanilla kernel yet.

Other than that acking the irq is still open.  I don't really want to
bother the generic irq-ack routine to do something like:

	if (irq_is_external(irq) && extirq_is_configured_edgesensive(irq))
		ackextirq(irq)

And only for that using a 2nd chip seems overkill to me.

Best regards
Uwe

> > [1] http://www.digi.com/products/embeddedsolutions/ns9215.jsp
> >     Interesting sections are "Pinout -> General purpose I/O (GPIO)" and
> >     "I/O Control Module".  E.g. GPIO4's func1 is "Ext Int Ch 2"
> > [2] System Control Module -> External Interrupt 0­3 Control register
> > [3] drivers/mmc/host/at91_mci.c
> > [4] drivers/ata/pata_rb532_cf.c
[5]
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=include/asm-arm/arch-ns9xxx/gpio.h;hb=HEAD#l34

-- 
Uwe Kleine-König, Software Engineer
Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany
Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962

      reply	other threads:[~2008-05-28  7:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-27 13:02 configure gpio for interrupt function Uwe Kleine-König
2008-05-27 20:49 ` David Brownell
2008-05-28  7:17   ` Uwe Kleine-König [this message]

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=20080528071747.GA7599@digi.com \
    --to=uwe.kleine-koenig@digi.com \
    --cc=david-b@pacbell.net \
    --cc=linux-kernel@vger.kernel.org \
    /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.