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 <[email protected]>
---
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(-)
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;
}
--
2.1.0
On Thu, 27 Nov 2014 19:42:01 +0100, 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 <[email protected]>
> ---
> 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.
What about just making a lubbock CPLD a special separate device?
Then it will have normal probe callback and a possibility to return
-EPROBE_DEFER? If only syscon (drivers/mfd/syscon.c) could support
irq generation, it would fit ideally.
--
With best wishes
Dmitry
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 <[email protected]>
> ---
> 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 <[email protected]> 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.
--
Robert
On Fri, Nov 28, 2014 at 9:30 PM, Robert Jarzmik <[email protected]> wrote:
> Thomas Gleixner <[email protected]> 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 <[email protected]> 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.
--
Robert