| Message ID | 1358790842-2986-2-git-send-email-gregory.clement@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Dear Gregory CLEMENT, Just some minor nitpicks below. On Mon, 21 Jan 2013 18:53:57 +0100, Gregory CLEMENT wrote: > + if (hw < ARMADA_370_XP_MAX_PER_CPU_IRQS) { > + Unneeded empty line. > + irq_set_percpu_devid(virq); > + irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip, > + handle_percpu_devid_irq); > + > + } else { > + irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip, > + handle_level_irq); > + } Braces useless since there is only one statement in the else. > + set_irq_flags(virq, IRQF_VALID | IRQF_PROBE); Incorrect indentation for this line. Thomas
On 01/21/2013 07:17 PM, Thomas Petazzoni wrote: > Dear Gregory CLEMENT, > > Just some minor nitpicks below. OK I will take them into account for the V2. Thanks > > On Mon, 21 Jan 2013 18:53:57 +0100, Gregory CLEMENT wrote: > >> + if (hw < ARMADA_370_XP_MAX_PER_CPU_IRQS) { >> + > > Unneeded empty line. > >> + irq_set_percpu_devid(virq); >> + irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip, >> + handle_percpu_devid_irq); >> + >> + } else { >> + irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip, >> + handle_level_irq); >> + } > > Braces useless since there is only one statement in the else. > >> + set_irq_flags(virq, IRQF_VALID | IRQF_PROBE); > > Incorrect indentation for this line. > > Thomas >
Hi Thomas and Gregory, On Mon, Jan 21, 2013 at 11:07:10PM +0100, Gregory CLEMENT wrote: > On 01/21/2013 07:17 PM, Thomas Petazzoni wrote: > >> + irq_set_percpu_devid(virq); > >> + irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip, > >> + handle_percpu_devid_irq); > >> + > >> + } else { > >> + irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip, > >> + handle_level_irq); > >> + } > > > > Braces useless since there is only one statement in the else. > > IMHO, this is an exception to the rule. Since the first block is more than one line, we usually put braces on the single line block too. (or at least that's what Documentation/CodingStyle says). Regards,
On 01/22/2013 12:26 AM, Ezequiel Garcia wrote: > Hi Thomas and Gregory, > > On Mon, Jan 21, 2013 at 11:07:10PM +0100, Gregory CLEMENT wrote: >> On 01/21/2013 07:17 PM, Thomas Petazzoni wrote: >>>> + irq_set_percpu_devid(virq); >>>> + irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip, >>>> + handle_percpu_devid_irq); >>>> + >>>> + } else { >>>> + irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip, >>>> + handle_level_irq); >>>> + } >>> >>> Braces useless since there is only one statement in the else. >>> > > IMHO, this is an exception to the rule. > Since the first block is more than one line, > we usually put braces on the single line block too. > (or at least that's what Documentation/CodingStyle says). > You're right! I would have liked to say I have done it on purpose, but in fact these braces are here only because during development I had had multiples lines inside the else. However, as you pointed it, I will keep it. > Regards, >
Dear Gregory CLEMENT, On Tue, 22 Jan 2013 10:09:03 +0100, Gregory CLEMENT wrote: > You're right! > I would have liked to say I have done it on purpose, but in fact these > braces are here only because during development I had had multiples lines > inside the else. > However, as you pointed it, I will keep it. ACK, fine. Didn't know about this specific point of the CodingStyle. Best regards, Thomas
diff --git a/arch/arm/mach-mvebu/irq-armada-370-xp.c b/arch/arm/mach-mvebu/irq-armada-370-xp.c index f99a4a2..c007fdc 100644 --- a/arch/arm/mach-mvebu/irq-armada-370-xp.c +++ b/arch/arm/mach-mvebu/irq-armada-370-xp.c @@ -145,11 +145,19 @@ static int armada_370_xp_mpic_irq_map(struct irq_domain *h, { armada_370_xp_irq_mask(irq_get_irq_data(virq)); writel(hw, main_int_base + ARMADA_370_XP_INT_SET_ENABLE_OFFS); - - irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip, - handle_level_irq); irq_set_status_flags(virq, IRQ_LEVEL); - set_irq_flags(virq, IRQF_VALID | IRQF_PROBE); + + if (hw < ARMADA_370_XP_MAX_PER_CPU_IRQS) { + + irq_set_percpu_devid(virq); + irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip, + handle_percpu_devid_irq); + + } else { + irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip, + handle_level_irq); + } + set_irq_flags(virq, IRQF_VALID | IRQF_PROBE); return 0; } @@ -245,7 +253,7 @@ asmlinkage void __exception_irq_entry armada_370_xp_handle_irq(struct pt_regs if (irqnr > 1022) break; - if (irqnr >= 8) { + if (irqnr > 0) { irqnr = irq_find_mapping(armada_370_xp_mpic_domain, irqnr); handle_IRQ(irqnr, regs);
MPIC allows the use of private interrupt for each CPUs. The 28th first interrupts are per-cpu. This patch adds support to use them. Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> --- arch/arm/mach-mvebu/irq-armada-370-xp.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)