* [PATCH v2 0/2] irqchip: sun4i: Use handle_fasteoi_irq for all irqs @ 2014-03-15 15:04 Hans de Goede 2014-03-15 15:04 ` [PATCH v2 1/2] irqchip: sun4i: Use handle_fasteoi_irq for all interrupts Hans de Goede 2014-03-15 15:04 ` [PATCH v2 2/2] irqchip: sun4i: simplify sun4i_irq_ack Hans de Goede 0 siblings, 2 replies; 7+ messages in thread From: Hans de Goede @ 2014-03-15 15:04 UTC (permalink / raw) To: Thomas Gleixner, Maxime Ripard Cc: linux-arm-kernel, devicetree, linux-kernel, linux-sunxi Hi All, Here is v2 of my patchset for sun4i-irq.c to use handle_fasteoi_irq for all irqs + follow up clean-up patch. Changes since v2: -adjust commit msg based on Thomas' comments, and merge patch 1 and 2 as they make more sense as 1 patch Regards, Hans ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] irqchip: sun4i: Use handle_fasteoi_irq for all interrupts 2014-03-15 15:04 [PATCH v2 0/2] irqchip: sun4i: Use handle_fasteoi_irq for all irqs Hans de Goede @ 2014-03-15 15:04 ` Hans de Goede 2014-03-17 11:12 ` Maxime Ripard 2014-03-19 11:36 ` [tip:irq/core] " tip-bot for Hans de Goede 2014-03-15 15:04 ` [PATCH v2 2/2] irqchip: sun4i: simplify sun4i_irq_ack Hans de Goede 1 sibling, 2 replies; 7+ messages in thread From: Hans de Goede @ 2014-03-15 15:04 UTC (permalink / raw) To: Thomas Gleixner, Maxime Ripard Cc: linux-arm-kernel, devicetree, linux-kernel, linux-sunxi, Hans de Goede Since the sun4i irq chip does not require any action and clears the interrupt when the level goes back to inactive, we don't need to mask / unmask for non oneshot IRQs, to achieve this we make sun4i_irq_ack a nop for all irqs except irq 0 and use handle_fasteoi_irq for all interrupts. Now there might be a case when the device reactivates the interrupt before the RETI. But that does not matter as we run the primary interrupt handlers with interrupts disabled. This also allows us to get rid of needing to use 2 irq_chip structs, this means that the IRQCHIP_EOI_THREADED | IRQCHIP_EOI_IF_HANDLED will now influence all interrupts rather then just irq 0, but that does not matter as the eoi is now a nop anyways for all interrupts but irq 0. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/irqchip/irq-sun4i.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/drivers/irqchip/irq-sun4i.c b/drivers/irqchip/irq-sun4i.c index a0ed1ea..6a8c88d 100644 --- a/drivers/irqchip/irq-sun4i.c +++ b/drivers/irqchip/irq-sun4i.c @@ -45,6 +45,9 @@ static void sun4i_irq_ack(struct irq_data *irqd) int reg = irq / 32; u32 val; + if (irq != 0) + return; /* Only IRQ 0 / the ENMI needs to be acked */ + val = readl(sun4i_irq_base + SUN4I_IRQ_PENDING_REG(reg)); writel(val | (1 << irq_off), sun4i_irq_base + SUN4I_IRQ_PENDING_REG(reg)); @@ -76,13 +79,6 @@ static void sun4i_irq_unmask(struct irq_data *irqd) static struct irq_chip sun4i_irq_chip = { .name = "sun4i_irq", - .irq_mask = sun4i_irq_mask, - .irq_unmask = sun4i_irq_unmask, -}; - -/* IRQ 0 / the ENMI needs a late eoi call */ -static struct irq_chip sun4i_irq_chip_enmi = { - .name = "sun4i_irq", .irq_eoi = sun4i_irq_ack, .irq_mask = sun4i_irq_mask, .irq_unmask = sun4i_irq_unmask, @@ -92,13 +88,7 @@ static struct irq_chip sun4i_irq_chip_enmi = { static int sun4i_irq_map(struct irq_domain *d, unsigned int virq, irq_hw_number_t hw) { - if (hw == 0) - irq_set_chip_and_handler(virq, &sun4i_irq_chip_enmi, - handle_fasteoi_irq); - else - irq_set_chip_and_handler(virq, &sun4i_irq_chip, - handle_level_irq); - + irq_set_chip_and_handler(virq, &sun4i_irq_chip, handle_fasteoi_irq); set_irq_flags(virq, IRQF_VALID | IRQF_PROBE); return 0; -- 1.9.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] irqchip: sun4i: Use handle_fasteoi_irq for all interrupts 2014-03-15 15:04 ` [PATCH v2 1/2] irqchip: sun4i: Use handle_fasteoi_irq for all interrupts Hans de Goede @ 2014-03-17 11:12 ` Maxime Ripard 2014-03-19 11:36 ` [tip:irq/core] " tip-bot for Hans de Goede 1 sibling, 0 replies; 7+ messages in thread From: Maxime Ripard @ 2014-03-17 11:12 UTC (permalink / raw) To: Hans de Goede Cc: Thomas Gleixner, linux-arm-kernel, devicetree, linux-kernel, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 2743 bytes --] On Sat, Mar 15, 2014 at 04:04:53PM +0100, Hans de Goede wrote: > Since the sun4i irq chip does not require any action and clears the interrupt > when the level goes back to inactive, we don't need to mask / unmask for > non oneshot IRQs, to achieve this we make sun4i_irq_ack a nop for all irqs > except irq 0 and use handle_fasteoi_irq for all interrupts. > > Now there might be a case when the device reactivates the interrupt > before the RETI. But that does not matter as we run the primary > interrupt handlers with interrupts disabled. > > This also allows us to get rid of needing to use 2 irq_chip structs, this > means that the IRQCHIP_EOI_THREADED | IRQCHIP_EOI_IF_HANDLED will now influence > all interrupts rather then just irq 0, but that does not matter as the eoi > is now a nop anyways for all interrupts but irq 0. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/irqchip/irq-sun4i.c | 18 ++++-------------- > 1 file changed, 4 insertions(+), 14 deletions(-) > > diff --git a/drivers/irqchip/irq-sun4i.c b/drivers/irqchip/irq-sun4i.c > index a0ed1ea..6a8c88d 100644 > --- a/drivers/irqchip/irq-sun4i.c > +++ b/drivers/irqchip/irq-sun4i.c > @@ -45,6 +45,9 @@ static void sun4i_irq_ack(struct irq_data *irqd) > int reg = irq / 32; > u32 val; > > + if (irq != 0) > + return; /* Only IRQ 0 / the ENMI needs to be acked */ > + > val = readl(sun4i_irq_base + SUN4I_IRQ_PENDING_REG(reg)); > writel(val | (1 << irq_off), > sun4i_irq_base + SUN4I_IRQ_PENDING_REG(reg)); > @@ -76,13 +79,6 @@ static void sun4i_irq_unmask(struct irq_data *irqd) > > static struct irq_chip sun4i_irq_chip = { > .name = "sun4i_irq", > - .irq_mask = sun4i_irq_mask, > - .irq_unmask = sun4i_irq_unmask, > -}; > - > -/* IRQ 0 / the ENMI needs a late eoi call */ > -static struct irq_chip sun4i_irq_chip_enmi = { > - .name = "sun4i_irq", > .irq_eoi = sun4i_irq_ack, > .irq_mask = sun4i_irq_mask, > .irq_unmask = sun4i_irq_unmask, > @@ -92,13 +88,7 @@ static struct irq_chip sun4i_irq_chip_enmi = { > static int sun4i_irq_map(struct irq_domain *d, unsigned int virq, > irq_hw_number_t hw) > { > - if (hw == 0) > - irq_set_chip_and_handler(virq, &sun4i_irq_chip_enmi, > - handle_fasteoi_irq); > - else > - irq_set_chip_and_handler(virq, &sun4i_irq_chip, > - handle_level_irq); > - > + irq_set_chip_and_handler(virq, &sun4i_irq_chip, handle_fasteoi_irq); > set_irq_flags(virq, IRQF_VALID | IRQF_PROBE); Oh.. And you just did. Nevermind then. Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [tip:irq/core] irqchip: sun4i: Use handle_fasteoi_irq for all interrupts 2014-03-15 15:04 ` [PATCH v2 1/2] irqchip: sun4i: Use handle_fasteoi_irq for all interrupts Hans de Goede 2014-03-17 11:12 ` Maxime Ripard @ 2014-03-19 11:36 ` tip-bot for Hans de Goede 1 sibling, 0 replies; 7+ messages in thread From: tip-bot for Hans de Goede @ 2014-03-19 11:36 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, hdegoede, tglx, maxime.ripard Commit-ID: 915b78ce8ef0178305cb100e830832a866b42faa Gitweb: http://git.kernel.org/tip/915b78ce8ef0178305cb100e830832a866b42faa Author: Hans de Goede <hdegoede@redhat.com> AuthorDate: Sat, 15 Mar 2014 16:04:53 +0100 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Wed, 19 Mar 2014 12:34:29 +0100 irqchip: sun4i: Use handle_fasteoi_irq for all interrupts Since the sun4i irq chip does not require any action and clears the interrupt when the level goes back to inactive, we don't need to mask / unmask for non oneshot IRQs, to achieve this we make sun4i_irq_ack a nop for all irqs except irq 0 and use handle_fasteoi_irq for all interrupts. Now there might be a case when the device reactivates the interrupt before the RETI. But that does not matter as we run the primary interrupt handlers with interrupts disabled. This also allows us to get rid of needing to use 2 irq_chip structs, this means that the IRQCHIP_EOI_THREADED | IRQCHIP_EOI_IF_HANDLED will now influence all interrupts rather then just irq 0, but that does not matter as the eoi is now a nop anyways for all interrupts but irq 0. Signed-off-by: Hans de Goede <hdegoede@redhat.com> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Cc: linux-arm-kernel@lists.infradead.org Cc: linux-sunxi@googlegroups.com Link: http://lkml.kernel.org/r/1394895894-8891-2-git-send-email-hdegoede@redhat.com Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- drivers/irqchip/irq-sun4i.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/drivers/irqchip/irq-sun4i.c b/drivers/irqchip/irq-sun4i.c index 2029cc5..003a146 100644 --- a/drivers/irqchip/irq-sun4i.c +++ b/drivers/irqchip/irq-sun4i.c @@ -45,6 +45,9 @@ static void sun4i_irq_ack(struct irq_data *irqd) int reg = irq / 32; u32 val; + if (irq != 0) + return; /* Only IRQ 0 / the ENMI needs to be acked */ + val = readl(sun4i_irq_base + SUN4I_IRQ_PENDING_REG(reg)); writel(val | (1 << irq_off), sun4i_irq_base + SUN4I_IRQ_PENDING_REG(reg)); @@ -76,13 +79,6 @@ static void sun4i_irq_unmask(struct irq_data *irqd) static struct irq_chip sun4i_irq_chip = { .name = "sun4i_irq", - .irq_mask = sun4i_irq_mask, - .irq_unmask = sun4i_irq_unmask, -}; - -/* IRQ 0 / the ENMI needs a late eoi call */ -static struct irq_chip sun4i_irq_chip_enmi = { - .name = "sun4i_irq", .irq_eoi = sun4i_irq_ack, .irq_mask = sun4i_irq_mask, .irq_unmask = sun4i_irq_unmask, @@ -92,13 +88,7 @@ static struct irq_chip sun4i_irq_chip_enmi = { static int sun4i_irq_map(struct irq_domain *d, unsigned int virq, irq_hw_number_t hw) { - if (hw == 0) - irq_set_chip_and_handler(virq, &sun4i_irq_chip_enmi, - handle_fasteoi_irq); - else - irq_set_chip_and_handler(virq, &sun4i_irq_chip, - handle_level_irq); - + irq_set_chip_and_handler(virq, &sun4i_irq_chip, handle_fasteoi_irq); set_irq_flags(virq, IRQF_VALID | IRQF_PROBE); return 0; ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] irqchip: sun4i: simplify sun4i_irq_ack 2014-03-15 15:04 [PATCH v2 0/2] irqchip: sun4i: Use handle_fasteoi_irq for all irqs Hans de Goede 2014-03-15 15:04 ` [PATCH v2 1/2] irqchip: sun4i: Use handle_fasteoi_irq for all interrupts Hans de Goede @ 2014-03-15 15:04 ` Hans de Goede 2014-03-17 11:12 ` Maxime Ripard 2014-03-19 11:36 ` [tip:irq/core] irqchip: sun4i: Simplify sun4i_irq_ack tip-bot for Hans de Goede 1 sibling, 2 replies; 7+ messages in thread From: Hans de Goede @ 2014-03-15 15:04 UTC (permalink / raw) To: Thomas Gleixner, Maxime Ripard Cc: linux-arm-kernel, devicetree, linux-kernel, linux-sunxi, Hans de Goede Now that we only ack irq 0 the code can be simplified a lot. Also switch from read / modify / write to a simple write clear: 1) This is what the android code does (it has a hack for acking irq 0 in its unmask code doing this) 2) read / modify / write simply does not make sense for an irq status register like this, if the other bits are writeable (and the data sheet says they are not) they should be write 1 to clear, since otherwise a read / modify / write can race with a device raising an interrupt and then clear the pending bit unintentionally Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/irqchip/irq-sun4i.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/irqchip/irq-sun4i.c b/drivers/irqchip/irq-sun4i.c index 6a8c88d..75615b5 100644 --- a/drivers/irqchip/irq-sun4i.c +++ b/drivers/irqchip/irq-sun4i.c @@ -41,16 +41,11 @@ static asmlinkage void __exception_irq_entry sun4i_handle_irq(struct pt_regs *re static void sun4i_irq_ack(struct irq_data *irqd) { unsigned int irq = irqd_to_hwirq(irqd); - unsigned int irq_off = irq % 32; - int reg = irq / 32; - u32 val; if (irq != 0) return; /* Only IRQ 0 / the ENMI needs to be acked */ - val = readl(sun4i_irq_base + SUN4I_IRQ_PENDING_REG(reg)); - writel(val | (1 << irq_off), - sun4i_irq_base + SUN4I_IRQ_PENDING_REG(reg)); + writel(BIT(0), sun4i_irq_base + SUN4I_IRQ_PENDING_REG(0)); } static void sun4i_irq_mask(struct irq_data *irqd) -- 1.9.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] irqchip: sun4i: simplify sun4i_irq_ack 2014-03-15 15:04 ` [PATCH v2 2/2] irqchip: sun4i: simplify sun4i_irq_ack Hans de Goede @ 2014-03-17 11:12 ` Maxime Ripard 2014-03-19 11:36 ` [tip:irq/core] irqchip: sun4i: Simplify sun4i_irq_ack tip-bot for Hans de Goede 1 sibling, 0 replies; 7+ messages in thread From: Maxime Ripard @ 2014-03-17 11:12 UTC (permalink / raw) To: Hans de Goede Cc: Thomas Gleixner, linux-arm-kernel, devicetree, linux-kernel, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 900 bytes --] On Sat, Mar 15, 2014 at 04:04:54PM +0100, Hans de Goede wrote: > Now that we only ack irq 0 the code can be simplified a lot. > > Also switch from read / modify / write to a simple write clear: > 1) This is what the android code does (it has a hack for acking irq 0 > in its unmask code doing this) > 2) read / modify / write simply does not make sense for an irq status > register like this, if the other bits are writeable (and the data sheet says > they are not) they should be write 1 to clear, since otherwise a read / > modify / write can race with a device raising an interrupt and then clear > the pending bit unintentionally > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Thanks! -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [tip:irq/core] irqchip: sun4i: Simplify sun4i_irq_ack 2014-03-15 15:04 ` [PATCH v2 2/2] irqchip: sun4i: simplify sun4i_irq_ack Hans de Goede 2014-03-17 11:12 ` Maxime Ripard @ 2014-03-19 11:36 ` tip-bot for Hans de Goede 1 sibling, 0 replies; 7+ messages in thread From: tip-bot for Hans de Goede @ 2014-03-19 11:36 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, hdegoede, tglx, maxime.ripard Commit-ID: cc3b68fea29c3af018734501e166124c8eb04a6c Gitweb: http://git.kernel.org/tip/cc3b68fea29c3af018734501e166124c8eb04a6c Author: Hans de Goede <hdegoede@redhat.com> AuthorDate: Sat, 15 Mar 2014 16:04:54 +0100 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Wed, 19 Mar 2014 12:34:29 +0100 irqchip: sun4i: Simplify sun4i_irq_ack Now that we only ack irq 0 the code can be simplified a lot. Also switch from read / modify / write to a simple write clear: 1) This is what the android code does (it has a hack for acking irq 0 in its unmask code doing this) 2) read / modify / write simply does not make sense for an irq status register like this, if the other bits are writeable (and the data sheet says they are not) they should be write 1 to clear, since otherwise a read / modify / write can race with a device raising an interrupt and then clear the pending bit unintentionally Signed-off-by: Hans de Goede <hdegoede@redhat.com> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Cc: linux-arm-kernel@lists.infradead.org Cc: linux-sunxi@googlegroups.com Link: http://lkml.kernel.org/r/1394895894-8891-3-git-send-email-hdegoede@redhat.com Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- drivers/irqchip/irq-sun4i.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/irqchip/irq-sun4i.c b/drivers/irqchip/irq-sun4i.c index 003a146..6fcef4a 100644 --- a/drivers/irqchip/irq-sun4i.c +++ b/drivers/irqchip/irq-sun4i.c @@ -41,16 +41,11 @@ static void __exception_irq_entry sun4i_handle_irq(struct pt_regs *regs); static void sun4i_irq_ack(struct irq_data *irqd) { unsigned int irq = irqd_to_hwirq(irqd); - unsigned int irq_off = irq % 32; - int reg = irq / 32; - u32 val; if (irq != 0) return; /* Only IRQ 0 / the ENMI needs to be acked */ - val = readl(sun4i_irq_base + SUN4I_IRQ_PENDING_REG(reg)); - writel(val | (1 << irq_off), - sun4i_irq_base + SUN4I_IRQ_PENDING_REG(reg)); + writel(BIT(0), sun4i_irq_base + SUN4I_IRQ_PENDING_REG(0)); } static void sun4i_irq_mask(struct irq_data *irqd) ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-03-19 11:37 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-15 15:04 [PATCH v2 0/2] irqchip: sun4i: Use handle_fasteoi_irq for all irqs Hans de Goede 2014-03-15 15:04 ` [PATCH v2 1/2] irqchip: sun4i: Use handle_fasteoi_irq for all interrupts Hans de Goede 2014-03-17 11:12 ` Maxime Ripard 2014-03-19 11:36 ` [tip:irq/core] " tip-bot for Hans de Goede 2014-03-15 15:04 ` [PATCH v2 2/2] irqchip: sun4i: simplify sun4i_irq_ack Hans de Goede 2014-03-17 11:12 ` Maxime Ripard 2014-03-19 11:36 ` [tip:irq/core] irqchip: sun4i: Simplify sun4i_irq_ack tip-bot for Hans de Goede
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox