On Wed, Nov 6, 2013 at 5:44 PM, Sebastian Hesselbarth
[off-list ref] wrote:

quoted

quoted

+struct dwapb_gpio {
+       struct  device          *dev;
+       void __iomem            *regs;
+       struct dwapb_gpio_port  *ports;
+       unsigned int            nr_ports;
+       struct irq_domain       *domain;
+       int                     hwirq;

I'm not sure I fully understand what hwirq is in this context - is it
the IRQ line from the Synopsys block to the system interrupt controller?
If so I don't think this covers all configurations - the Picochip
devices for instance have each GPIO in port A as an individual IRQ going
to the VIC.

Usually, hwirq is the interrupt as seen from this very device, i.e. in
this case it is 0 up to 32 referencing the portA gpio line that
triggered an interrupt. In this drivers context 'hwirq' above is
actually the virtual irq number Linux made up to identify an interrupt
coming from this IP block.

Therefore, above should really be irq instead of hwirq - but with a
more detailed review, I think it can be removed completely.

quoted

It looks here like hwirq is used for all of the interrupt registers so
only one GPIO interrupt is supported?

Looking in _probe, currently only one upstream interrupt is requested.
Jamie is right, that you should grab all interrupts - but you can forget
their virtual number after registering the handler. There are helper
functions to get it back in irq handler.

The IP configuration I have has only one irq line going to the gic,
(that is the "hwirq" here) so I had one hwirq (the gic) for each IP
instance.  The domain maps a Linux IRQ number for each gpio line.

I will need to rework this to also support both this (one irq per IP
block) and the picochip (one irq per gpio line coming out of the IP
block).

quoted

quoted

+static void __exit dwapb_gpio_exit(void)
+{
+       platform_driver_unregister(&dwapb_gpio_driver);
+}
+module_exit(dwapb_gpio_exit);

We can replace the registration and unregistration with
module_platform_driver() now.

+1 (again)

OK

Alan

Sebastian