* [PATCH] irqchip/versatile-fpga: Handle chained IRQs properly
@ 2020-03-18 17:09 Sungbo Eo
2020-03-18 17:48 ` Marc Zyngier
0 siblings, 1 reply; 7+ messages in thread
From: Sungbo Eo @ 2020-03-18 17:09 UTC (permalink / raw)
To: linux-oxnas, Linus Walleij, Thomas Gleixner, Jason Cooper,
Marc Zyngier, linux-arm-kernel, linux-kernel
Cc: Sungbo Eo, Neil Armstrong
Enclose the chained handler with chained_irq_{enter,exit}(), so that the
muxed interrupts get properly acked.
This patch also fixes a reboot bug on OX820 SoC, where the jiffies timer
interrupt is never acked. The kernel waits a clock tick forever in
calibrate_delay_converge(), which leads to a boot hang.
Signed-off-by: Sungbo Eo <mans0n@gorani.run>
Cc: Neil Armstrong <narmstrong@baylibre.com>
---
drivers/irqchip/irq-versatile-fpga.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-versatile-fpga.c b/drivers/irqchip/irq-versatile-fpga.c
index 928858dada75..08faab2fec3e 100644
--- a/drivers/irqchip/irq-versatile-fpga.c
+++ b/drivers/irqchip/irq-versatile-fpga.c
@@ -6,6 +6,7 @@
#include <linux/irq.h>
#include <linux/io.h>
#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
#include <linux/irqchip/versatile-fpga.h>
#include <linux/irqdomain.h>
#include <linux/module.h>
@@ -68,12 +69,15 @@ static void fpga_irq_unmask(struct irq_data *d)
static void fpga_irq_handle(struct irq_desc *desc)
{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
struct fpga_irq_data *f = irq_desc_get_handler_data(desc);
u32 status = readl(f->base + IRQ_STATUS);
+ chained_irq_enter(chip, desc);
+
if (status == 0) {
do_bad_IRQ(desc);
- return;
+ goto out;
}
do {
@@ -82,6 +86,9 @@ static void fpga_irq_handle(struct irq_desc *desc)
status &= ~(1 << irq);
generic_handle_irq(irq_find_mapping(f->domain, irq));
} while (status);
+
+out:
+ chained_irq_exit(chip, desc);
}
/*
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] irqchip/versatile-fpga: Handle chained IRQs properly
2020-03-18 17:09 [PATCH] irqchip/versatile-fpga: Handle chained IRQs properly Sungbo Eo
@ 2020-03-18 17:48 ` Marc Zyngier
2020-03-18 18:20 ` Sungbo Eo
0 siblings, 1 reply; 7+ messages in thread
From: Marc Zyngier @ 2020-03-18 17:48 UTC (permalink / raw)
To: Sungbo Eo
Cc: Jason Cooper, Neil Armstrong, Linus Walleij, linux-kernel,
Thomas Gleixner, linux-oxnas, linux-arm-kernel
Hi Sungbo,
On 2020-03-18 17:09, Sungbo Eo wrote:
> Enclose the chained handler with chained_irq_{enter,exit}(), so that
> the
> muxed interrupts get properly acked.
>
> This patch also fixes a reboot bug on OX820 SoC, where the jiffies
> timer
> interrupt is never acked. The kernel waits a clock tick forever in
> calibrate_delay_converge(), which leads to a boot hang.
Nice catch.
>
> Signed-off-by: Sungbo Eo <mans0n@gorani.run>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> ---
> drivers/irqchip/irq-versatile-fpga.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-versatile-fpga.c
> b/drivers/irqchip/irq-versatile-fpga.c
> index 928858dada75..08faab2fec3e 100644
> --- a/drivers/irqchip/irq-versatile-fpga.c
> +++ b/drivers/irqchip/irq-versatile-fpga.c
> @@ -6,6 +6,7 @@
> #include <linux/irq.h>
> #include <linux/io.h>
> #include <linux/irqchip.h>
> +#include <linux/irqchip/chained_irq.h>
> #include <linux/irqchip/versatile-fpga.h>
> #include <linux/irqdomain.h>
> #include <linux/module.h>
> @@ -68,12 +69,15 @@ static void fpga_irq_unmask(struct irq_data *d)
>
> static void fpga_irq_handle(struct irq_desc *desc)
> {
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> struct fpga_irq_data *f = irq_desc_get_handler_data(desc);
> u32 status = readl(f->base + IRQ_STATUS);
>
> + chained_irq_enter(chip, desc);
> +
It's probably not a big deal, but I'm not fond of starting talking to
the muxing irqchip before having done the chained_irq_enter() call.
Moving that read here would probably be safer.
> if (status == 0) {
> do_bad_IRQ(desc);
> - return;
> + goto out;
> }
>
> do {
> @@ -82,6 +86,9 @@ static void fpga_irq_handle(struct irq_desc *desc)
> status &= ~(1 << irq);
> generic_handle_irq(irq_find_mapping(f->domain, irq));
> } while (status);
> +
> +out:
> + chained_irq_exit(chip, desc);
> }
>
> /*
Otherwise looks good. If you send it again with the above fixed
and a Fixes: tag, I'll queue it.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] irqchip/versatile-fpga: Handle chained IRQs properly
2020-03-18 17:48 ` Marc Zyngier
@ 2020-03-18 18:20 ` Sungbo Eo
2020-03-18 19:11 ` Marc Zyngier
0 siblings, 1 reply; 7+ messages in thread
From: Sungbo Eo @ 2020-03-18 18:20 UTC (permalink / raw)
To: Marc Zyngier
Cc: Jason Cooper, Neil Armstrong, Linus Walleij, linux-kernel,
Thomas Gleixner, linux-oxnas, linux-arm-kernel
Hi Marc,
On 2020-03-19 02:48, Marc Zyngier wrote:
> Hi Sungbo,
>
> On 2020-03-18 17:09, Sungbo Eo wrote:
>> Enclose the chained handler with chained_irq_{enter,exit}(), so that the
>> muxed interrupts get properly acked.
>>
>> This patch also fixes a reboot bug on OX820 SoC, where the jiffies timer
>> interrupt is never acked. The kernel waits a clock tick forever in
>> calibrate_delay_converge(), which leads to a boot hang.
>
> Nice catch.
>
>>
>> Signed-off-by: Sungbo Eo <mans0n@gorani.run>
>> Cc: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> drivers/irqchip/irq-versatile-fpga.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-versatile-fpga.c
>> b/drivers/irqchip/irq-versatile-fpga.c
>> index 928858dada75..08faab2fec3e 100644
>> --- a/drivers/irqchip/irq-versatile-fpga.c
>> +++ b/drivers/irqchip/irq-versatile-fpga.c
>> @@ -6,6 +6,7 @@
>> #include <linux/irq.h>
>> #include <linux/io.h>
>> #include <linux/irqchip.h>
>> +#include <linux/irqchip/chained_irq.h>
>> #include <linux/irqchip/versatile-fpga.h>
>> #include <linux/irqdomain.h>
>> #include <linux/module.h>
>> @@ -68,12 +69,15 @@ static void fpga_irq_unmask(struct irq_data *d)
>>
>> static void fpga_irq_handle(struct irq_desc *desc)
>> {
>> + struct irq_chip *chip = irq_desc_get_chip(desc);
>> struct fpga_irq_data *f = irq_desc_get_handler_data(desc);
>> u32 status = readl(f->base + IRQ_STATUS);
>>
>> + chained_irq_enter(chip, desc);
>> +
>
> It's probably not a big deal, but I'm not fond of starting talking to
> the muxing irqchip before having done the chained_irq_enter() call.
>
> Moving that read here would probably be safer.
Oops, I missed it. Thanks for pointing it out.
>
>> if (status == 0) {
>> do_bad_IRQ(desc);
>> - return;
>> + goto out;
>> }
>>
>> do {
>> @@ -82,6 +86,9 @@ static void fpga_irq_handle(struct irq_desc *desc)
>> status &= ~(1 << irq);
>> generic_handle_irq(irq_find_mapping(f->domain, irq));
>> } while (status);
>> +
>> +out:
>> + chained_irq_exit(chip, desc);
>> }
>>
>> /*
>
> Otherwise looks good. If you send it again with the above fixed
> and a Fixes: tag, I'll queue it.
It seems the handler had been broken from the very beginning. Could you
give me a hint on how the tag should be like?
Thanks.
>
> Thanks,
>
> M.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] irqchip/versatile-fpga: Handle chained IRQs properly
2020-03-18 18:20 ` Sungbo Eo
@ 2020-03-18 19:11 ` Marc Zyngier
2020-03-19 2:34 ` [PATCH v2] " Sungbo Eo
0 siblings, 1 reply; 7+ messages in thread
From: Marc Zyngier @ 2020-03-18 19:11 UTC (permalink / raw)
To: Sungbo Eo
Cc: Jason Cooper, Neil Armstrong, Linus Walleij, linux-kernel,
Thomas Gleixner, linux-oxnas, linux-arm-kernel
On 2020-03-18 18:20, Sungbo Eo wrote:
> Hi Marc,
>
> On 2020-03-19 02:48, Marc Zyngier wrote:
>> Hi Sungbo,
>>
>> On 2020-03-18 17:09, Sungbo Eo wrote:
>>> Enclose the chained handler with chained_irq_{enter,exit}(), so that
>>> the
>>> muxed interrupts get properly acked.
>>>
>>> This patch also fixes a reboot bug on OX820 SoC, where the jiffies
>>> timer
>>> interrupt is never acked. The kernel waits a clock tick forever in
>>> calibrate_delay_converge(), which leads to a boot hang.
>>
>> Nice catch.
>>
>>>
>>> Signed-off-by: Sungbo Eo <mans0n@gorani.run>
>>> Cc: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>> drivers/irqchip/irq-versatile-fpga.c | 9 ++++++++-
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/irqchip/irq-versatile-fpga.c
>>> b/drivers/irqchip/irq-versatile-fpga.c
>>> index 928858dada75..08faab2fec3e 100644
>>> --- a/drivers/irqchip/irq-versatile-fpga.c
>>> +++ b/drivers/irqchip/irq-versatile-fpga.c
>>> @@ -6,6 +6,7 @@
>>> #include <linux/irq.h>
>>> #include <linux/io.h>
>>> #include <linux/irqchip.h>
>>> +#include <linux/irqchip/chained_irq.h>
>>> #include <linux/irqchip/versatile-fpga.h>
>>> #include <linux/irqdomain.h>
>>> #include <linux/module.h>
>>> @@ -68,12 +69,15 @@ static void fpga_irq_unmask(struct irq_data *d)
>>>
>>> static void fpga_irq_handle(struct irq_desc *desc)
>>> {
>>> + struct irq_chip *chip = irq_desc_get_chip(desc);
>>> struct fpga_irq_data *f = irq_desc_get_handler_data(desc);
>>> u32 status = readl(f->base + IRQ_STATUS);
>>>
>>> + chained_irq_enter(chip, desc);
>>> +
>>
>> It's probably not a big deal, but I'm not fond of starting talking to
>> the muxing irqchip before having done the chained_irq_enter() call.
>>
>> Moving that read here would probably be safer.
>
> Oops, I missed it. Thanks for pointing it out.
>
>>
>>> if (status == 0) {
>>> do_bad_IRQ(desc);
>>> - return;
>>> + goto out;
>>> }
>>>
>>> do {
>>> @@ -82,6 +86,9 @@ static void fpga_irq_handle(struct irq_desc *desc)
>>> status &= ~(1 << irq);
>>> generic_handle_irq(irq_find_mapping(f->domain, irq));
>>> } while (status);
>>> +
>>> +out:
>>> + chained_irq_exit(chip, desc);
>>> }
>>>
>>> /*
>>
>> Otherwise looks good. If you send it again with the above fixed
>> and a Fixes: tag, I'll queue it.
>
> It seems the handler had been broken from the very beginning. Could
> you give me a hint on how the tag should be like?
Indeed, it has been broken forever. I'm tempted to say:
Fixes: c41b16f8c9d9d ("ARM: integrator/versatile: consolidate FPGA IRQ
handling code")
even if it probably predates the introduction of the chained_irq_enter()
helpers.
This will ensure this gets backported to older kernels...
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] irqchip/versatile-fpga: Handle chained IRQs properly
2020-03-18 19:11 ` Marc Zyngier
@ 2020-03-19 2:34 ` Sungbo Eo
2020-03-19 8:44 ` Marc Zyngier
2020-03-27 10:28 ` Linus Walleij
0 siblings, 2 replies; 7+ messages in thread
From: Sungbo Eo @ 2020-03-19 2:34 UTC (permalink / raw)
To: Linus Walleij, Thomas Gleixner, Jason Cooper, Marc Zyngier,
linux-arm-kernel, linux-kernel, linux-oxnas
Cc: Sungbo Eo, Neil Armstrong
Enclose the chained handler with chained_irq_{enter,exit}(), so that the
muxed interrupts get properly acked.
This patch also fixes a reboot bug on OX820 SoC, where the jiffies timer
interrupt is never acked. The kernel waits a clock tick forever in
calibrate_delay_converge(), which leads to a boot hang.
Fixes: c41b16f8c9d9 ("ARM: integrator/versatile: consolidate FPGA IRQ handling code")
Signed-off-by: Sungbo Eo <mans0n@gorani.run>
Cc: Neil Armstrong <narmstrong@baylibre.com>
---
v2: moved readl below chained_irq_enter()
added Fixes tag
drivers/irqchip/irq-versatile-fpga.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-versatile-fpga.c b/drivers/irqchip/irq-versatile-fpga.c
index 928858dada75..70e2cfff8175 100644
--- a/drivers/irqchip/irq-versatile-fpga.c
+++ b/drivers/irqchip/irq-versatile-fpga.c
@@ -6,6 +6,7 @@
#include <linux/irq.h>
#include <linux/io.h>
#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
#include <linux/irqchip/versatile-fpga.h>
#include <linux/irqdomain.h>
#include <linux/module.h>
@@ -68,12 +69,16 @@ static void fpga_irq_unmask(struct irq_data *d)
static void fpga_irq_handle(struct irq_desc *desc)
{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
struct fpga_irq_data *f = irq_desc_get_handler_data(desc);
- u32 status = readl(f->base + IRQ_STATUS);
+ u32 status;
+
+ chained_irq_enter(chip, desc);
+ status = readl(f->base + IRQ_STATUS);
if (status == 0) {
do_bad_IRQ(desc);
- return;
+ goto out;
}
do {
@@ -82,6 +87,9 @@ static void fpga_irq_handle(struct irq_desc *desc)
status &= ~(1 << irq);
generic_handle_irq(irq_find_mapping(f->domain, irq));
} while (status);
+
+out:
+ chained_irq_exit(chip, desc);
}
/*
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] irqchip/versatile-fpga: Handle chained IRQs properly
2020-03-19 2:34 ` [PATCH v2] " Sungbo Eo
@ 2020-03-19 8:44 ` Marc Zyngier
2020-03-27 10:28 ` Linus Walleij
1 sibling, 0 replies; 7+ messages in thread
From: Marc Zyngier @ 2020-03-19 8:44 UTC (permalink / raw)
To: Sungbo Eo
Cc: Jason Cooper, Neil Armstrong, Linus Walleij, linux-kernel,
Thomas Gleixner, linux-oxnas, linux-arm-kernel
On 2020-03-19 02:34, Sungbo Eo wrote:
> Enclose the chained handler with chained_irq_{enter,exit}(), so that
> the
> muxed interrupts get properly acked.
>
> This patch also fixes a reboot bug on OX820 SoC, where the jiffies
> timer
> interrupt is never acked. The kernel waits a clock tick forever in
> calibrate_delay_converge(), which leads to a boot hang.
>
> Fixes: c41b16f8c9d9 ("ARM: integrator/versatile: consolidate FPGA IRQ
> handling code")
> Signed-off-by: Sungbo Eo <mans0n@gorani.run>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> ---
> v2: moved readl below chained_irq_enter()
> added Fixes tag
>
> drivers/irqchip/irq-versatile-fpga.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
Queued for 5.7.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] irqchip/versatile-fpga: Handle chained IRQs properly
2020-03-19 2:34 ` [PATCH v2] " Sungbo Eo
2020-03-19 8:44 ` Marc Zyngier
@ 2020-03-27 10:28 ` Linus Walleij
1 sibling, 0 replies; 7+ messages in thread
From: Linus Walleij @ 2020-03-27 10:28 UTC (permalink / raw)
To: Sungbo Eo
Cc: Jason Cooper, Neil Armstrong, Marc Zyngier, linux-kernel,
Thomas Gleixner, linux-oxnas, Linux ARM
On Thu, Mar 19, 2020 at 3:36 AM Sungbo Eo <mans0n@gorani.run> wrote:
> Enclose the chained handler with chained_irq_{enter,exit}(), so that the
> muxed interrupts get properly acked.
>
> This patch also fixes a reboot bug on OX820 SoC, where the jiffies timer
> interrupt is never acked. The kernel waits a clock tick forever in
> calibrate_delay_converge(), which leads to a boot hang.
>
> Fixes: c41b16f8c9d9 ("ARM: integrator/versatile: consolidate FPGA IRQ handling code")
> Signed-off-by: Sungbo Eo <mans0n@gorani.run>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> ---
> v2: moved readl below chained_irq_enter()
> added Fixes tag
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
I wonder how Integrator keeps working so well despite
this. I can't test it right now but I'm sure it is fine.
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-03-27 10:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-18 17:09 [PATCH] irqchip/versatile-fpga: Handle chained IRQs properly Sungbo Eo
2020-03-18 17:48 ` Marc Zyngier
2020-03-18 18:20 ` Sungbo Eo
2020-03-18 19:11 ` Marc Zyngier
2020-03-19 2:34 ` [PATCH v2] " Sungbo Eo
2020-03-19 8:44 ` Marc Zyngier
2020-03-27 10:28 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).