From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754129AbbINOdO (ORCPT ); Mon, 14 Sep 2015 10:33:14 -0400 Received: from gabe.freedesktop.org ([131.252.210.177]:34688 "EHLO gabe.freedesktop.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751751AbbINOdN (ORCPT ); Mon, 14 Sep 2015 10:33:13 -0400 From: Eric Anholt To: Russell King - ARM Linux , Noralf =?utf-8?Q?Tr?= =?utf-8?Q?=C3=B8nnes?= Cc: tglx@linutronix.de, jason@lakedaemon.net, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] irqchip: bcm2835: Add FIQ support In-Reply-To: <20150914090853.GF21084@n2100.arm.linux.org.uk> References: <1434130016-26574-1-git-send-email-noralf@tronnes.org> <87d1zj6fq2.fsf@eliezer.anholt.net> <55F5CD80.3020700@tronnes.org> <20150914090853.GF21084@n2100.arm.linux.org.uk> User-Agent: Notmuch/0.20.2 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) Date: Mon, 14 Sep 2015 07:33:09 -0700 Message-ID: <87wpvtjcka.fsf@eliezer.anholt.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Russell King - ARM Linux writes: > On Sun, Sep 13, 2015 at 09:24:48PM +0200, Noralf Tr=C3=B8nnes wrote: >>=20 >> Den 22.07.2015 23:32, skrev Eric Anholt: >> >Noralf Tr=C3=B8nnes writes: >> > >> >>Add a duplicate irq range with an offset on the hwirq's so the >> >>driver can detect that enable_fiq() is used. >> >>Tested with downstream dwc_otg USB controller driver. >> >> >> >>Signed-off-by: Noralf Tr=C3=B8nnes >> >>--- >> >> arch/arm/mach-bcm/Kconfig | 1 + >> >> drivers/irqchip/irq-bcm2835.c | 53 +++++++++++++++++++++++++++++++++= +++++----- >> >> 2 files changed, 48 insertions(+), 6 deletions(-) >> >> >> >>diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig >> >>index 8b11f44..7cfef7b 100644 >> >>--- a/arch/arm/mach-bcm/Kconfig >> >>+++ b/arch/arm/mach-bcm/Kconfig >> >>@@ -114,6 +114,7 @@ config ARCH_BCM2835 >> >> select ARM_ERRATA_411920 >> >> select ARM_TIMER_SP804 >> >> select CLKSRC_OF >> >>+ select FIQ >> >> select PINCTRL >> >> select PINCTRL_BCM2835 >> >> help >> >>diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm28= 35.c >> >>index 5916d6c..db66246 100644 >> >>--- a/drivers/irqchip/irq-bcm2835.c >> >>+++ b/drivers/irqchip/irq-bcm2835.c >> >>@@ -56,7 +56,7 @@ >> >> #include "irqchip.h" >> >> /* Put the bank and irq (32 bits) into the hwirq */ >> >>-#define MAKE_HWIRQ(b, n) ((b << 5) | (n)) >> >>+#define MAKE_HWIRQ(b, n) (((b) << 5) | (n)) >> >> #define HWIRQ_BANK(i) (i >> 5) >> >> #define HWIRQ_BIT(i) BIT(i & 0x1f) >> >>@@ -72,9 +72,13 @@ >> >> | SHORTCUT1_MASK | SHORTCUT2_MASK) >> >> #define REG_FIQ_CONTROL 0x0c >> >>+#define REG_FIQ_ENABLE 0x80 >> >>+#define REG_FIQ_DISABLE 0 >> >> #define NR_BANKS 3 >> >> #define IRQS_PER_BANK 32 >> >>+#define NUMBER_IRQS MAKE_HWIRQ(NR_BANKS, 0) >> >>+#define FIQ_START (NR_IRQS_BANK0 + MAKE_HWIRQ(NR_BANKS - 1, 0)) >> >> static int reg_pending[] __initconst =3D { 0x00, 0x04, 0x08 }; >> >> static int reg_enable[] __initconst =3D { 0x18, 0x10, 0x14 }; >> >>@@ -98,14 +102,38 @@ static struct armctrl_ic intc __read_mostly; >> >> static void __exception_irq_entry bcm2835_handle_irq( >> >> struct pt_regs *regs); >> >>+static inline unsigned int hwirq_to_fiq(unsigned long hwirq) >> >>+{ >> >>+ hwirq -=3D NUMBER_IRQS; >> >>+ /* >> >>+ * The hwirq numbering used in this driver is: >> >>+ * BASE (0-7) GPU1 (32-63) GPU2 (64-95). >> >>+ * This differ from the one used in the FIQ register: >> >>+ * GPU1 (0-31) GPU2 (32-63) BASE (64-71) >> >>+ */ >> >>+ if (hwirq >=3D 32) >> >>+ return hwirq - 32; >> >>+ >> >>+ return hwirq + 64; >> >>+} >> >>+ >> >> static void armctrl_mask_irq(struct irq_data *d) >> >> { >> >>- writel_relaxed(HWIRQ_BIT(d->hwirq), intc.disable[HWIRQ_BANK(d->hwirq= )]); >> >>+ if (d->hwirq >=3D NUMBER_IRQS) >> >>+ writel_relaxed(REG_FIQ_DISABLE, intc.base + REG_FIQ_CONTROL); >> >>+ else >> >>+ writel_relaxed(HWIRQ_BIT(d->hwirq), >> >>+ intc.disable[HWIRQ_BANK(d->hwirq)]); >> >> } >> >> static void armctrl_unmask_irq(struct irq_data *d) >> >> { >> >>- writel_relaxed(HWIRQ_BIT(d->hwirq), intc.enable[HWIRQ_BANK(d->hwirq)= ]); >> >>+ if (d->hwirq >=3D NUMBER_IRQS) >> >>+ writel_relaxed(REG_FIQ_ENABLE | hwirq_to_fiq(d->hwirq), >> >>+ intc.base + REG_FIQ_CONTROL); >> >>+ else >> >>+ writel_relaxed(HWIRQ_BIT(d->hwirq), >> >>+ intc.enable[HWIRQ_BANK(d->hwirq)]); >> >> } >> >I found it nice for the 2836 controller to declare a new irqchip when >> >both the mask/unmask hooks needed to be changed for that class of >> >interrupt. However, it looks like these functions aren't going to be >> >called regularly, so it doesn't matter much. >> > >> >As far as interaction with my 2836 series, it looks like the only thing >> >downstream does is make the FIQ get handled on CPU1 instead of CPU0. I >> >think we'll be fine fixing this up later. >> > >> >Reviewed-by: Eric Anholt >>=20 >> It seems that this has slipped through the cracks, at least it didn't >> enter in the 4.3 merge window. > > ... to which I'm glad. > > What's the use-case for FIQs on this SMP platform? Where's the code that > makes use of the provided FIQs? Downstream (what Raspberry Pi users are generally using) is using FIQ for the USB driver to reduce interrupt latency. Notably, the ethernet chip hangs off USB, and I've heard the USB chip is flaky when interrupt latency is too high. I've only seen dmesg warnings and not lockups myself, but if FIQs improve performance, it may be worthwhile for us. Right now downstream is using its own platform ports called 2708/2708. They're considering swapping over to using our platform ports instead, and being able to use upstream's interrut controller would be a big step on that front. If they can use our platform support in general, it means that future platforms based on it will be more mergeable upstream (It's been 7 months and we still don't have a Raspberry Pi 2 port in tree. It's kind of embarrassing). So, while FIQ isn't used in upstream, I think it's worthwhile to merge. It is another step to bringing the downstream developers into the fold. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJV9tqmAAoJELXWKTbR/J7oRt8QAKhA1mCfebGljJ0BMZoXiC6c qowvV5bQQdhbb23r9/mfiloAWrrPLQYfXaVbmB+iR0PoAXIm7OtoZGN7ecIxIy76 kaPUoE2dwXlj0TpHPOjezxucca1vBxyBWjf5XoC61HH+6bUzeQ0zWC6ZpI6tHrif u7na5duINqyC67EeRQ8wE6CUCd9LY0a/776o7CbYA2o8MgentLqw4eCdFHt09y4b xairTn7FkrleEk2WI/C1ADncLS6QDqEBWH2BOD8eZW0siHBCWR3LG819NlYgoG8p lLWbACdtJK1tI7cHZgZQgqow1e7TRJsbJ4BBvzZtjjGwotwam02qZgUFQUhAWP/2 OoOK9P6HSMs7ZN5VLJgjjfxOEi5Q9H+RivqnY3vLbrJOx0bE2Qc45NTMbG++BNwq ktDtIlbPmcGYlHQiatNCKmt/geUD7IqKD7sOGgl73MzBTHw6qaz+MSmzHM0ra8jx LjnNY9KB7SeA2U5cqqD63oCDShIZKviwfT7L3z4GeYbmVGn1wtILfQ9iyElNF6hc b+BRnhA07/mR78A/GHwn/puD+Llf0p3K1yiEH0ixF0aF9wNz2w+afKQe+BStlQot pn/syhaAJlQRL2qiEriGLSkVNSBNtotIcyuQcRkpcrkUhatb2kMbpd63oHyHRcV9 6HINr+N0YJ4YsbkQXMNz =sYRF -----END PGP SIGNATURE----- --=-=-=-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: eric@anholt.net (Eric Anholt) Date: Mon, 14 Sep 2015 07:33:09 -0700 Subject: [PATCH] irqchip: bcm2835: Add FIQ support In-Reply-To: <20150914090853.GF21084@n2100.arm.linux.org.uk> References: <1434130016-26574-1-git-send-email-noralf@tronnes.org> <87d1zj6fq2.fsf@eliezer.anholt.net> <55F5CD80.3020700@tronnes.org> <20150914090853.GF21084@n2100.arm.linux.org.uk> Message-ID: <87wpvtjcka.fsf@eliezer.anholt.net> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Russell King - ARM Linux writes: > On Sun, Sep 13, 2015 at 09:24:48PM +0200, Noralf Tr?nnes wrote: >> >> Den 22.07.2015 23:32, skrev Eric Anholt: >> >Noralf Tr?nnes writes: >> > >> >>Add a duplicate irq range with an offset on the hwirq's so the >> >>driver can detect that enable_fiq() is used. >> >>Tested with downstream dwc_otg USB controller driver. >> >> >> >>Signed-off-by: Noralf Tr?nnes >> >>--- >> >> arch/arm/mach-bcm/Kconfig | 1 + >> >> drivers/irqchip/irq-bcm2835.c | 53 ++++++++++++++++++++++++++++++++++++++----- >> >> 2 files changed, 48 insertions(+), 6 deletions(-) >> >> >> >>diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig >> >>index 8b11f44..7cfef7b 100644 >> >>--- a/arch/arm/mach-bcm/Kconfig >> >>+++ b/arch/arm/mach-bcm/Kconfig >> >>@@ -114,6 +114,7 @@ config ARCH_BCM2835 >> >> select ARM_ERRATA_411920 >> >> select ARM_TIMER_SP804 >> >> select CLKSRC_OF >> >>+ select FIQ >> >> select PINCTRL >> >> select PINCTRL_BCM2835 >> >> help >> >>diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c >> >>index 5916d6c..db66246 100644 >> >>--- a/drivers/irqchip/irq-bcm2835.c >> >>+++ b/drivers/irqchip/irq-bcm2835.c >> >>@@ -56,7 +56,7 @@ >> >> #include "irqchip.h" >> >> /* Put the bank and irq (32 bits) into the hwirq */ >> >>-#define MAKE_HWIRQ(b, n) ((b << 5) | (n)) >> >>+#define MAKE_HWIRQ(b, n) (((b) << 5) | (n)) >> >> #define HWIRQ_BANK(i) (i >> 5) >> >> #define HWIRQ_BIT(i) BIT(i & 0x1f) >> >>@@ -72,9 +72,13 @@ >> >> | SHORTCUT1_MASK | SHORTCUT2_MASK) >> >> #define REG_FIQ_CONTROL 0x0c >> >>+#define REG_FIQ_ENABLE 0x80 >> >>+#define REG_FIQ_DISABLE 0 >> >> #define NR_BANKS 3 >> >> #define IRQS_PER_BANK 32 >> >>+#define NUMBER_IRQS MAKE_HWIRQ(NR_BANKS, 0) >> >>+#define FIQ_START (NR_IRQS_BANK0 + MAKE_HWIRQ(NR_BANKS - 1, 0)) >> >> static int reg_pending[] __initconst = { 0x00, 0x04, 0x08 }; >> >> static int reg_enable[] __initconst = { 0x18, 0x10, 0x14 }; >> >>@@ -98,14 +102,38 @@ static struct armctrl_ic intc __read_mostly; >> >> static void __exception_irq_entry bcm2835_handle_irq( >> >> struct pt_regs *regs); >> >>+static inline unsigned int hwirq_to_fiq(unsigned long hwirq) >> >>+{ >> >>+ hwirq -= NUMBER_IRQS; >> >>+ /* >> >>+ * The hwirq numbering used in this driver is: >> >>+ * BASE (0-7) GPU1 (32-63) GPU2 (64-95). >> >>+ * This differ from the one used in the FIQ register: >> >>+ * GPU1 (0-31) GPU2 (32-63) BASE (64-71) >> >>+ */ >> >>+ if (hwirq >= 32) >> >>+ return hwirq - 32; >> >>+ >> >>+ return hwirq + 64; >> >>+} >> >>+ >> >> static void armctrl_mask_irq(struct irq_data *d) >> >> { >> >>- writel_relaxed(HWIRQ_BIT(d->hwirq), intc.disable[HWIRQ_BANK(d->hwirq)]); >> >>+ if (d->hwirq >= NUMBER_IRQS) >> >>+ writel_relaxed(REG_FIQ_DISABLE, intc.base + REG_FIQ_CONTROL); >> >>+ else >> >>+ writel_relaxed(HWIRQ_BIT(d->hwirq), >> >>+ intc.disable[HWIRQ_BANK(d->hwirq)]); >> >> } >> >> static void armctrl_unmask_irq(struct irq_data *d) >> >> { >> >>- writel_relaxed(HWIRQ_BIT(d->hwirq), intc.enable[HWIRQ_BANK(d->hwirq)]); >> >>+ if (d->hwirq >= NUMBER_IRQS) >> >>+ writel_relaxed(REG_FIQ_ENABLE | hwirq_to_fiq(d->hwirq), >> >>+ intc.base + REG_FIQ_CONTROL); >> >>+ else >> >>+ writel_relaxed(HWIRQ_BIT(d->hwirq), >> >>+ intc.enable[HWIRQ_BANK(d->hwirq)]); >> >> } >> >I found it nice for the 2836 controller to declare a new irqchip when >> >both the mask/unmask hooks needed to be changed for that class of >> >interrupt. However, it looks like these functions aren't going to be >> >called regularly, so it doesn't matter much. >> > >> >As far as interaction with my 2836 series, it looks like the only thing >> >downstream does is make the FIQ get handled on CPU1 instead of CPU0. I >> >think we'll be fine fixing this up later. >> > >> >Reviewed-by: Eric Anholt >> >> It seems that this has slipped through the cracks, at least it didn't >> enter in the 4.3 merge window. > > ... to which I'm glad. > > What's the use-case for FIQs on this SMP platform? Where's the code that > makes use of the provided FIQs? Downstream (what Raspberry Pi users are generally using) is using FIQ for the USB driver to reduce interrupt latency. Notably, the ethernet chip hangs off USB, and I've heard the USB chip is flaky when interrupt latency is too high. I've only seen dmesg warnings and not lockups myself, but if FIQs improve performance, it may be worthwhile for us. Right now downstream is using its own platform ports called 2708/2708. They're considering swapping over to using our platform ports instead, and being able to use upstream's interrut controller would be a big step on that front. If they can use our platform support in general, it means that future platforms based on it will be more mergeable upstream (It's been 7 months and we still don't have a Raspberry Pi 2 port in tree. It's kind of embarrassing). So, while FIQ isn't used in upstream, I think it's worthwhile to merge. It is another step to bringing the downstream developers into the fold. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 818 bytes Desc: not available URL: