Messages in this thread | | | From | Alexey Brodkin <> | Subject | Re: [PATCH] gpio-dwapb: reset mask register on probe | Date | Thu, 19 Mar 2015 08:44:01 +0000 |
| |
Hi Linus,
On Tue, 2015-03-17 at 17:54 +0100, Linus Walleij wrote: > >> + /* Mask out and disable all interrupts */ > >> + dwapb_write(gpio, GPIO_INTMASK, 0xffffffff); > >> + dwapb_write(gpio, GPIO_INTEN, 0); > > > > This looks good to me - it's always a good idea to make sure defaults > > are set as we expect. > > So should I cook a patch doing just these two lines? > > But the initial patch unmasking all IRQs then? Is that even needed?
Oops, my bad here :)
I intentionally wrote 0 to MASK register to return it to default state (if pre-bootloader messes it). As I explained if we really mask all interrupts (together with disabling them all via INTEN) then interrupts will never happen.
If we look at DW APB GPIO databook it says: --->8--- Whenever a 1 is written to a bit of this register [GPIO_INTEN], it configures the corresponding bit on Port A to become an interrupt; otherwise, Port A operates as a normal GPIO signal. --->8---
While for GPIO_INTMASK it says: --->8--- Controls whether an interrupt on Port A can create an interrupt for the interrupt controller by not masking it. By default, all interrupts bits are unmasked. --->8---
So IMHO we need to update "gpio-dwapb" driver in the following manner:
[1] In dwapb_configure_irqs() in accordance to "snps,nr-gpios" in the first bank set a value in GPIO_INTEN. This way we turn N pins in that port/bank in "interrupt" mode from their default gpio mode.
[2] Don't expose dwapb_irq_enable()/dwapb_irq_disable() through ct->chip.irq_enable/ct->chip.irq_disable. Because we don't want to toggle modes of pins, right?
[3] Use ct->chip.irq_mask/ct->chip.irq_unmask for purpose of real enabling/disabling interrupts.
If I'm not missing something that would be implementation that matches real device specification.
If we do these changes then indeed we'll want to make sure GPIO_INTMASK is initialized with all interrupts masked out: --->8--- dwapb_write(gpio, GPIO_INTMASK, 0xffffffff); --->8---
Let me know if my proposal makes sense and then I'll send patch that implements it.
-Alexey
| |