Message-Id: <200805271349.24884.david-b@pacbell.net>
Date:	Tue, 27 May 2008 13:49:24 -0700
From:	David Brownell <david-b@...bell.net>
To:	Uwe Kleine-König <Uwe.Kleine-Koenig@...i.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: configure gpio for interrupt function

On Tuesday 27 May 2008, Uwe Kleine-König wrote:
> Hello David,
> 
> 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.

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 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!
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().

An alternate implementation would make the irq_chip for those GPIOs
understand the GPIO-to-EXT_IRQ mappings, and use them for those
pins that are inputs *and* have request_irq() called.  It sounds
as messy as I'm sure the code would be.


> 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.)


> (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.


> So I suggest to extend the gpio API with two functions like:
> 
> 	int gpio_direction_inputirq(unsigned gpio, unsigned flags);
> 	void gpio_ackirq(unsigned gpio);
> 
> with flags being similar to the IRQF_TRIGGER_* defines in
> include/linux/interrupt.h.  I need gpio_ackirq because ns9xxx requires
> that an edge triggered irq is acknowledged separatly in the "External
> Interrupt X Control register"[2].  (If you have better names ...)
> 
> Moreover this makes the API more flexible because the irq type can be
> configured.

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.

  - irq_to_gpio() may fail in all cases

  - request_irq() kicks in irq_chip logic to ensure that the pin
    is either input GPIO (and remuxes it) or already EXT_INT ...

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

  - set_irq_type() configures the IRQ type via irq_chip hooks

Right?

 
> OTOH I'd like to deprecate irq_to_gpio.  $(git grep -l irq_to_gpio
> drivers) suggests that this function is only used in two places.  One of
> them[3] is AT91 specific, the other[4] seems specific for the
> RouterBoard 532.
> (There are some other users below arch/, but I expect no breakage here.
> I expect that every arch that uses the function have it implemented
> anyhow.)

That's true for that AT91 case, for example.


> What do you think?

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.

- Dave



> 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
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists