| Message ID | 1417113721-9062-1-git-send-email-robert.jarzmik@free.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 27 Nov 2014, Robert Jarzmik wrote: > When gpio-pxa was moved to drivers/pxa, it became a driver, and its > initialization and probing happen at postcore initcall. The lubbock code > used to install the chained lubbock interrupt handler at init_irq() > time. > > The consequence of the gpio-pxa change is that the installed chained irq > handler lubbock_irq_handler() was overwritten in pxa_gpio_probe(_dt)(), > removing : > - the handler > - the falling edge detection setting of GPIO0, which revealed the > interrupt request from the lubbock IO board. > > As a fix, move the gpio0 chained handler setup to a place where we have > the guarantee that pxa_gpio_probe() was called before, so that lubbock > handler becomes the true IRQ chained handler of GPIO0, demuxing the > lubbock IO board interrupts. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > --- > For Thomas: as a side note, I'm not very happy with this patch. What > makes me unhappy is that I don't know how to express the > dependency between gpio-pxa probe time and > irq_set_chained_handler(irq, lubbock_irq_handler). > > At the moment I rely on the fact that > lubbock_irq_device_init() is called as device initcall while > pxa_gpio_probe() is called as postcore initcall. Admittedly I'm confused. So what is the relationship between installing that chained handler and that gpio-pxa probe stuff? And why is the GPIO0 interrupt handled from arch code rather than from a regular driver setup, which depends on the availablity of the GPIO driver? Thanks, tglx
Thomas Gleixner <tglx@linutronix.de> writes: > So what is the relationship between installing that chained handler > and that gpio-pxa probe stuff? The relation is in gpio-pxa probe, look at the extract of pxa_gpio_probe() : pxa_gpio_probe() irq = gpio_to_irq(0); irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip, handle_edge_irq); set_irq_flags(irq, IRQF_VALID | IRQF_PROBE); irq_set_chained_handler(IRQ_GPIO0, pxa_gpio_demux_handler); Now look at the extract from the former lubbock_init_irq() : lubbock_init_irq() irq = PXA_GPIO_TO_IRQ(0); irq_set_chained_handler(irq, lubbock_irq_handler); irq_set_irq_type(irq, IRQ_TYPE_EDGE_FALLING); Given that gpio_to_irq(0) = PXA_GPIO_TO_IRQ(0), see how these 2 are fighting to install the handler, and how the resulting installed handler depends on the order of execution of pxa_gpio_to_irq() wrt lubbock_init_irq(). > And why is the GPIO0 interrupt handled from arch code rather than from > a regular driver setup, which depends on the availablity of the GPIO > driver? Ah that's a good question. Maybe the answer is that there is no driver in this case. When I say "no driver", it's because this interrupt is a consequence of the IO-Board (or motherboard) wiring topology. I think I need to add a bit of context, so pardon my crude ascii-art style, and see in the lubbock case, we have this wiring (list of IPs not exhaustive, and gates to mask each XXX irq not added) : IPs on Motherboard Gates on motherboard SoC +-------------+ +-------+ | SMC Lan | --lan irq--- | Latch | - +-------------+ | | \ +------PXA-----+ | | \ | | +-------------+ | | |+----------+ | | UDC Vbus | --vbus irq-- | Latch | -- NOR gate -- GPIO0 -- ||GPIO block| | +-------------+ | | line |+----------+ | | | / | | +-------------+ | | / +--------------+ | SA1111 | --sa11x irq--| Latch | - +-------------+ +-------+ The "gates on motherboard" is what lubbock.c is describing, ie. the interconnection on the motherboard. I don't see the device/driver model fitting to describe these gates, do you ? Cheers.
On Fri, Nov 28, 2014 at 9:30 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote: > Thomas Gleixner <tglx@linutronix.de> writes: > >> So what is the relationship between installing that chained handler >> and that gpio-pxa probe stuff? > The relation is in gpio-pxa probe, look at the extract of pxa_gpio_probe() : > pxa_gpio_probe() > irq = gpio_to_irq(0); > irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip, > handle_edge_irq); > set_irq_flags(irq, IRQF_VALID | IRQF_PROBE); > irq_set_chained_handler(IRQ_GPIO0, pxa_gpio_demux_handler); > > Now look at the extract from the former lubbock_init_irq() : > lubbock_init_irq() > irq = PXA_GPIO_TO_IRQ(0); > irq_set_chained_handler(irq, lubbock_irq_handler); > irq_set_irq_type(irq, IRQ_TYPE_EDGE_FALLING); > > Given that gpio_to_irq(0) = PXA_GPIO_TO_IRQ(0), see how these 2 are fighting to > install the handler, and how the resulting installed handler depends on the > order of execution of pxa_gpio_to_irq() wrt lubbock_init_irq(). > >> And why is the GPIO0 interrupt handled from arch code rather than from >> a regular driver setup, which depends on the availablity of the GPIO >> driver? > Ah that's a good question. Maybe the answer is that there is no driver in this > case. > When I say "no driver", it's because this interrupt is a consequence of the > IO-Board (or motherboard) wiring topology. > > I think I need to add a bit of context, so pardon my crude ascii-art style, and > see in the lubbock case, we have this wiring (list of IPs not exhaustive, and > gates to mask each XXX irq not added) : > > IPs on Motherboard Gates on motherboard SoC > > +-------------+ +-------+ > | SMC Lan | --lan irq--- | Latch | - > +-------------+ | | \ +------PXA-----+ > | | \ | | > +-------------+ | | |+----------+ | > | UDC Vbus | --vbus irq-- | Latch | -- NOR gate -- GPIO0 -- ||GPIO block| | > +-------------+ | | line |+----------+ | > | | / | | > +-------------+ | | / +--------------+ > | SA1111 | --sa11x irq--| Latch | - > +-------------+ +-------+ > > The "gates on motherboard" is what lubbock.c is describing, ie. the > interconnection on the motherboard. I don't see the device/driver model fitting > to describe these gates, do you ? > I think that it's a kind of irq muxing, just like lots of PMIC (power management IC). We should move the lubbock board irqs to a mfd driver, and register them as threaded irqs. Best Regards Haojian
Haojian Zhuang <haojian.zhuang@gmail.com> writes: > > I think that it's a kind of irq muxing, just like lots of PMIC (power > management IC). > We should move the lubbock board irqs to a mfd driver, and register them as > threaded irqs. I will have a look. I didn't consider mfd for motherboard gates, I'll think of it, thanks for the idea. Cheers.
diff --git a/arch/arm/mach-pxa/lubbock.c b/arch/arm/mach-pxa/lubbock.c index d8a1be6..1f138f9 100644 --- a/arch/arm/mach-pxa/lubbock.c +++ b/arch/arm/mach-pxa/lubbock.c @@ -172,9 +172,6 @@ static void __init lubbock_init_irq(void) handle_level_irq); set_irq_flags(irq, IRQF_VALID | IRQF_PROBE); } - - irq_set_chained_handler(PXA_GPIO_TO_IRQ(0), lubbock_irq_handler); - irq_set_irq_type(PXA_GPIO_TO_IRQ(0), IRQ_TYPE_EDGE_FALLING); } #ifdef CONFIG_PM @@ -190,7 +187,13 @@ static struct syscore_ops lubbock_irq_syscore_ops = { static int __init lubbock_irq_device_init(void) { + int irq; + if (machine_is_lubbock()) { + irq = PXA_GPIO_TO_IRQ(0); + irq_set_chained_handler(irq, lubbock_irq_handler); + irq_set_irq_type(irq, IRQ_TYPE_EDGE_FALLING); + register_syscore_ops(&lubbock_irq_syscore_ops); return 0; }
When gpio-pxa was moved to drivers/pxa, it became a driver, and its initialization and probing happen at postcore initcall. The lubbock code used to install the chained lubbock interrupt handler at init_irq() time. The consequence of the gpio-pxa change is that the installed chained irq handler lubbock_irq_handler() was overwritten in pxa_gpio_probe(_dt)(), removing : - the handler - the falling edge detection setting of GPIO0, which revealed the interrupt request from the lubbock IO board. As a fix, move the gpio0 chained handler setup to a place where we have the guarantee that pxa_gpio_probe() was called before, so that lubbock handler becomes the true IRQ chained handler of GPIO0, demuxing the lubbock IO board interrupts. Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> --- For Thomas: as a side note, I'm not very happy with this patch. What makes me unhappy is that I don't know how to express the dependency between gpio-pxa probe time and irq_set_chained_handler(irq, lubbock_irq_handler). At the moment I rely on the fact that lubbock_irq_device_init() is called as device initcall while pxa_gpio_probe() is called as postcore initcall. If you have a better idea I'm all ears. --- arch/arm/mach-pxa/lubbock.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)