On Tue, Sep 16, 2008 at 06:08:34PM -0400, Jon Smirl wrote:
[...]
> >> >> > Assume that GPIO 8 does not translate to any IRQ, but IRQ 8 is still
> >> >> > valid virq b/c it is mapped for another IRQ controller (particularly
> >> >> > lots of kernel code assumes that IRQ 8 is 8259 PIC's CMOS interrupt,
> >> >> > the PIC and IRQ8 is widely used on PowerPC).
> >> >>
> >> >> Set the base in the GPIO struct such that this won't happen. You can
> >> >> set the base greater than MAX_IRQ.
> >> >
> >> > And then you'll conflict with some other subsystem that decides to engage
> >> > in the same shenanigans.
> >>
> >> That comment was target at GPIO's that don't support interrupts. Give
> >> those GPIO numbers greater than MAX_IRQ in case someone tries to use
> >> them with the IRQ subsystem. Then they'll get errors.
> >
> > Or we can do the right thing, without messing all other gpio
> > controllers, i.e. implementing MAX_IRQ hacks. Right?
> >
> > I still don't see any problems with .to_irq callback, can you
> > point out any?
>
>
> You have to map between GPIO and IRQ inside the interrupt handlers so
> it has to be reasonably fast. This gets done on every shared interrupt
> so you will end up building mapping tables.
I don't get it. The mapping for your gpio controller will be 1:1.
But only for your GPIO controller. You don't have to create any tables.
That is,
static unsigned int your_controller_gpio_to_irq(stuct gpio_chip *gc,
unsigned int gpio)
{
return gc->base + gpio; /* guaranteed for this particular
irq/gpio controller bundle, because
gc->base == virq_base AND we
use 1:1 mapping. */
}
gpio_chip->to_irq = your_controller_gpio_to_irq;
Where is the table?
> Also, gpio_to_irq()
> doesn't take the gpio chip struct as a parameter.
You don't need this, since gpio_to_irq will call gpiolib's
__gpio_to_irq(), and gpiolib will call gpio_to_chip() to get the
chip struct. The approach is the same as we do for
gpio_{get,set}_value via gpiolib.
> Why does this mess with all of ther GPIO controllers? If they generate
> interrupts they obviously have to coordinate with the VIRQ system.
Btw, why do you need the gpio_to_irq call in the first place?
Why don't you just configure a gpio to serve as an interrupt source
(inside one of irq_host_ops), and just specify "interrupts = <>"
along side with "gpios = <>" in the "ir" node?
For example,
gpio_wkup: [EMAIL PROTECTED] {
compatible = "fsl,mpc5200b-gpio-wkup","fsl,mpc5200-gpio-wkup";
reg = <0xc00 0x40>;
interrupts = <0x1 0x8 0x0 0x0 0x3 0x0>;
interrupt-parent = <&mpc5200_pic>;
gpio-controller;
#gpio-cells = <2>;
interrupt-controller; <-- added
#interrupt-cells = <2>; <-- added
};
ir {
interrupts = <0 1>; <-- notice that irq-specific flags
placed where they should.
interrupt-parent = <&gpio_wkup>;
gpios = <&gpio_w 0 0>; <-- notice that 1:1 mapping is explicit
};
...and you don't need the gpio_to_irq.
(Plus. I would rather split the gpio-wkup node into two:
interrupt-controller, and gpio-controller).
> This may be an issue with the way gpio lib is designed, the API for
> the library assumes all gpios in the system are assigned unique
> identifiers.
>
> Is there any other problem with 1:1 other than it doesn't return an
> error if gpio_to_irq() is called with a gpio number that doesn't
> support irqs?
Yes. We might want non-1:1 mapping for other gpio controllers.
And we can't handle gpio_to_irq() for GPIO0 (yes, sure, we can
implement another hack: reserve GPIO0 for no use. ;-)
> You could always implement gpio_to_irq() like this:
>
> if (gpio < MAX_HW_IRQ)
> return -ENOSYSl
> return gpio;
Don't know anything about MAX_HW_IRQ... maybe NR_IRQS? I heard
some rumors about making NR_IRQS dynamic...
> Sure your proposal works too, it's just more complicated. 1:1 mapping
> is working for ARM, why does PowerPC need to be different? I initially
> started coding it the way you propose but then I stumbled across the
> ARM solution and it was way simpler.
I don't see why adding one more gpiolib callback would complicate
things. Today you're _forcing_ every gpio controller to have 1:1
irq:gpio mapping. I think later we will encounter more problems
with it and then we will blame our lack of foresight...
--
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
