| Message ID | 20190131133928.17985-3-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: davinci: modernize the irq support | expand |
On 1/31/19 7:38 AM, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > In order to support SPARSE_IRQ we first need to make davinci use the > generic irq handler for ARM. Translate the legacy assembly to C and > put the irq handlers into their respective drivers (aintc and cp-intc). > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > --- ... > diff --git a/arch/arm/mach-davinci/cp_intc.c b/arch/arm/mach-davinci/cp_intc.c ... > @@ -97,6 +98,16 @@ static struct irq_chip cp_intc_irq_chip = { > > static struct irq_domain *cp_intc_domain; > > +static asmlinkage void __exception_irq_entry > +cp_intc_handle_irq(struct pt_regs *regs) > +{ > + int irqnr = cp_intc_read(CP_INTC_PRIO_IDX); > + > + irqnr &= 0xff; What does applying the 0xff mask do? (perhaps needs a code comment if it is important and not obvious). > + > + handle_domain_irq(cp_intc_domain, irqnr, regs); > +} > + > static int cp_intc_host_map(struct irq_domain *h, unsigned int virq, > irq_hw_number_t hw) > { ... > diff --git a/arch/arm/mach-davinci/irq.c b/arch/arm/mach-davinci/irq.c ... > @@ -69,6 +76,19 @@ davinci_alloc_gc(void __iomem *base, unsigned int irq_start, unsigned int num) > IRQ_NOREQUEST | IRQ_NOPROBE, 0); > } > > +static asmlinkage void __exception_irq_entry > +davinci_handle_irq(struct pt_regs *regs) > +{ > + int irqnr = davinci_irq_readl(IRQ_IRQENTRY_OFFSET); > + struct pt_regs *old_regs = set_irq_regs(regs); > + > + irqnr >>= 2; > + irqnr -= 1; Same here. It is not obvious to me what sort of conversion is going on here. > + > + generic_handle_irq(irqnr); > + set_irq_regs(old_regs); > +} > + > /* ARM Interrupt Controller Initialization */ > void __init davinci_irq_init(void) > {
On 31/01/19 7:08 PM, Bartosz Golaszewski wrote: > diff --git a/arch/arm/mach-davinci/cp_intc.c b/arch/arm/mach-davinci/cp_intc.c > index 67805ca74ff8..b9aec3c48a6a 100644 > --- a/arch/arm/mach-davinci/cp_intc.c > +++ b/arch/arm/mach-davinci/cp_intc.c > @@ -19,6 +19,7 @@ > #include <linux/of_address.h> > #include <linux/of_irq.h> > > +#include <asm/exception.h> > #include <mach/common.h> > #include "cp_intc.h" > > @@ -97,6 +98,16 @@ static struct irq_chip cp_intc_irq_chip = { > > static struct irq_domain *cp_intc_domain; > > +static asmlinkage void __exception_irq_entry > +cp_intc_handle_irq(struct pt_regs *regs) > +{ > + int irqnr = cp_intc_read(CP_INTC_PRIO_IDX); > + > + irqnr &= 0xff; > + > + handle_domain_irq(cp_intc_domain, irqnr, regs); This leaves out spurious interrupt handling present in existing assembly code. Can you add it back. May be use omap_intc_handle_irq() as an example for handling spurious IRQs. > +} > + > diff --git a/arch/arm/mach-davinci/irq.c b/arch/arm/mach-davinci/irq.c > index 952dc126c390..3bbbef78d9ac 100644 > --- a/arch/arm/mach-davinci/irq.c > +++ b/arch/arm/mach-davinci/irq.c > @@ -28,11 +28,13 @@ > #include <mach/cputype.h> > #include <mach/common.h> > #include <asm/mach/irq.h> > +#include <asm/exception.h> > > #define FIQ_REG0_OFFSET 0x0000 > #define FIQ_REG1_OFFSET 0x0004 > #define IRQ_REG0_OFFSET 0x0008 > #define IRQ_REG1_OFFSET 0x000C > +#define IRQ_IRQENTRY_OFFSET 0x0014 > #define IRQ_ENT_REG0_OFFSET 0x0018 > #define IRQ_ENT_REG1_OFFSET 0x001C > #define IRQ_INCTL_REG_OFFSET 0x0020 > @@ -45,6 +47,11 @@ static inline void davinci_irq_writel(unsigned long value, int offset) > __raw_writel(value, davinci_intc_base + offset); > } > > +static inline unsigned long davinci_irq_readl(int offset) > +{ > + return __raw_readl(davinci_intc_base + offset); > +} Can we use readl_relaxed() here? I know there is existing __raw_writel() usage. May be add a patch to fix the existing code first. Thanks, Sekhar
śr., 6 lut 2019 o 13:39 Sekhar Nori <nsekhar@ti.com> napisał(a): > > On 31/01/19 7:08 PM, Bartosz Golaszewski wrote: > > > diff --git a/arch/arm/mach-davinci/cp_intc.c b/arch/arm/mach-davinci/cp_intc.c > > index 67805ca74ff8..b9aec3c48a6a 100644 > > --- a/arch/arm/mach-davinci/cp_intc.c > > +++ b/arch/arm/mach-davinci/cp_intc.c > > @@ -19,6 +19,7 @@ > > #include <linux/of_address.h> > > #include <linux/of_irq.h> > > > > +#include <asm/exception.h> > > #include <mach/common.h> > > #include "cp_intc.h" > > > > @@ -97,6 +98,16 @@ static struct irq_chip cp_intc_irq_chip = { > > > > static struct irq_domain *cp_intc_domain; > > > > +static asmlinkage void __exception_irq_entry > > +cp_intc_handle_irq(struct pt_regs *regs) > > +{ > > + int irqnr = cp_intc_read(CP_INTC_PRIO_IDX); > > + > > + irqnr &= 0xff; > > + > > + handle_domain_irq(cp_intc_domain, irqnr, regs); > > This leaves out spurious interrupt handling present in existing assembly > code. Can you add it back. May be use omap_intc_handle_irq() as an > example for handling spurious IRQs. > Hi Sekhar, I started looking at this one and noticed that the manual says PRI_INDX field in the GPIR register is in bits 0-9 (mask 0x3ff) while the assembly logically ANDs it with 0xff. I guess it's because there can be no more interrupts than 255 but I'd at least explain it in a comment. Or should we use the proper mask? What do you think? Bart > > +} > > + > > > diff --git a/arch/arm/mach-davinci/irq.c b/arch/arm/mach-davinci/irq.c > > index 952dc126c390..3bbbef78d9ac 100644 > > --- a/arch/arm/mach-davinci/irq.c > > +++ b/arch/arm/mach-davinci/irq.c > > @@ -28,11 +28,13 @@ > > #include <mach/cputype.h> > > #include <mach/common.h> > > #include <asm/mach/irq.h> > > +#include <asm/exception.h> > > > > #define FIQ_REG0_OFFSET 0x0000 > > #define FIQ_REG1_OFFSET 0x0004 > > #define IRQ_REG0_OFFSET 0x0008 > > #define IRQ_REG1_OFFSET 0x000C > > +#define IRQ_IRQENTRY_OFFSET 0x0014 > > #define IRQ_ENT_REG0_OFFSET 0x0018 > > #define IRQ_ENT_REG1_OFFSET 0x001C > > #define IRQ_INCTL_REG_OFFSET 0x0020 > > @@ -45,6 +47,11 @@ static inline void davinci_irq_writel(unsigned long value, int offset) > > __raw_writel(value, davinci_intc_base + offset); > > } > > > > +static inline unsigned long davinci_irq_readl(int offset) > > +{ > > + return __raw_readl(davinci_intc_base + offset); > > +} > > Can we use readl_relaxed() here? I know there is existing __raw_writel() > usage. May be add a patch to fix the existing code first. > > Thanks, > Sekhar
On 07/02/19 9:19 PM, Bartosz Golaszewski wrote: >>> +static asmlinkage void __exception_irq_entry >>> +cp_intc_handle_irq(struct pt_regs *regs) >>> +{ >>> + int irqnr = cp_intc_read(CP_INTC_PRIO_IDX); >>> + >>> + irqnr &= 0xff; >>> + >>> + handle_domain_irq(cp_intc_domain, irqnr, regs); >> >> This leaves out spurious interrupt handling present in existing assembly >> code. Can you add it back. May be use omap_intc_handle_irq() as an >> example for handling spurious IRQs. >> > > Hi Sekhar, > > I started looking at this one and noticed that the manual says > PRI_INDX field in the GPIR register is in bits 0-9 (mask 0x3ff) while > the assembly logically ANDs it with 0xff. I guess it's because there > can be no more interrupts than 255 but I'd at least explain it in a > comment. Or should we use the proper mask? What do you think? I think using mask 0x3ff to match TRM is fine. Thanks, Sekhar
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 664e918e2624..f7770fdcad68 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -589,6 +589,7 @@ config ARCH_DAVINCI select GENERIC_ALLOCATOR select GENERIC_CLOCKEVENTS select GENERIC_IRQ_CHIP + select GENERIC_IRQ_MULTI_HANDLER select GPIOLIB select HAVE_IDE select PM_GENERIC_DOMAINS if PM diff --git a/arch/arm/mach-davinci/cp_intc.c b/arch/arm/mach-davinci/cp_intc.c index 67805ca74ff8..b9aec3c48a6a 100644 --- a/arch/arm/mach-davinci/cp_intc.c +++ b/arch/arm/mach-davinci/cp_intc.c @@ -19,6 +19,7 @@ #include <linux/of_address.h> #include <linux/of_irq.h> +#include <asm/exception.h> #include <mach/common.h> #include "cp_intc.h" @@ -97,6 +98,16 @@ static struct irq_chip cp_intc_irq_chip = { static struct irq_domain *cp_intc_domain; +static asmlinkage void __exception_irq_entry +cp_intc_handle_irq(struct pt_regs *regs) +{ + int irqnr = cp_intc_read(CP_INTC_PRIO_IDX); + + irqnr &= 0xff; + + handle_domain_irq(cp_intc_domain, irqnr, regs); +} + static int cp_intc_host_map(struct irq_domain *h, unsigned int virq, irq_hw_number_t hw) { @@ -196,6 +207,8 @@ int __init cp_intc_of_init(struct device_node *node, struct device_node *parent) return -EINVAL; } + set_handle_irq(cp_intc_handle_irq); + /* Enable global interrupt */ cp_intc_write(1, CP_INTC_GLOBAL_ENABLE); diff --git a/arch/arm/mach-davinci/include/mach/entry-macro.S b/arch/arm/mach-davinci/include/mach/entry-macro.S deleted file mode 100644 index cf5f573eb5fd..000000000000 --- a/arch/arm/mach-davinci/include/mach/entry-macro.S +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Low-level IRQ helper macros for TI DaVinci-based platforms - * - * Author: Kevin Hilman, MontaVista Software, Inc. <source@mvista.com> - * - * 2007 (c) MontaVista Software, Inc. This file is licensed under - * the terms of the GNU General Public License version 2. This program - * is licensed "as is" without any warranty of any kind, whether express - * or implied. - */ -#include <mach/irqs.h> - - .macro get_irqnr_preamble, base, tmp - ldr \base, =davinci_intc_base - ldr \base, [\base] - .endm - - .macro get_irqnr_and_base, irqnr, irqstat, base, tmp -#if defined(CONFIG_AINTC) && defined(CONFIG_CP_INTC) - ldr \tmp, =davinci_intc_type - ldr \tmp, [\tmp] - cmp \tmp, #DAVINCI_INTC_TYPE_CP_INTC - beq 1001f -#endif -#if defined(CONFIG_AINTC) - ldr \tmp, [\base, #0x14] - movs \tmp, \tmp, lsr #2 - sub \irqnr, \tmp, #1 - b 1002f -#endif -#if defined(CONFIG_CP_INTC) -1001: ldr \irqnr, [\base, #0x80] /* get irq number */ - mov \tmp, \irqnr, lsr #31 - and \irqnr, \irqnr, #0xff /* irq is in bits 0-9 */ - and \tmp, \tmp, #0x1 - cmp \tmp, #0x1 -#endif -1002: - .endm diff --git a/arch/arm/mach-davinci/irq.c b/arch/arm/mach-davinci/irq.c index 952dc126c390..3bbbef78d9ac 100644 --- a/arch/arm/mach-davinci/irq.c +++ b/arch/arm/mach-davinci/irq.c @@ -28,11 +28,13 @@ #include <mach/cputype.h> #include <mach/common.h> #include <asm/mach/irq.h> +#include <asm/exception.h> #define FIQ_REG0_OFFSET 0x0000 #define FIQ_REG1_OFFSET 0x0004 #define IRQ_REG0_OFFSET 0x0008 #define IRQ_REG1_OFFSET 0x000C +#define IRQ_IRQENTRY_OFFSET 0x0014 #define IRQ_ENT_REG0_OFFSET 0x0018 #define IRQ_ENT_REG1_OFFSET 0x001C #define IRQ_INCTL_REG_OFFSET 0x0020 @@ -45,6 +47,11 @@ static inline void davinci_irq_writel(unsigned long value, int offset) __raw_writel(value, davinci_intc_base + offset); } +static inline unsigned long davinci_irq_readl(int offset) +{ + return __raw_readl(davinci_intc_base + offset); +} + static __init void davinci_alloc_gc(void __iomem *base, unsigned int irq_start, unsigned int num) { @@ -69,6 +76,19 @@ davinci_alloc_gc(void __iomem *base, unsigned int irq_start, unsigned int num) IRQ_NOREQUEST | IRQ_NOPROBE, 0); } +static asmlinkage void __exception_irq_entry +davinci_handle_irq(struct pt_regs *regs) +{ + int irqnr = davinci_irq_readl(IRQ_IRQENTRY_OFFSET); + struct pt_regs *old_regs = set_irq_regs(regs); + + irqnr >>= 2; + irqnr -= 1; + + generic_handle_irq(irqnr); + set_irq_regs(old_regs); +} + /* ARM Interrupt Controller Initialization */ void __init davinci_irq_init(void) { @@ -114,4 +134,5 @@ void __init davinci_irq_init(void) davinci_alloc_gc(davinci_intc_base + j, i, 32); irq_set_handler(IRQ_TINT1_TINT34, handle_level_irq); + set_handle_irq(davinci_handle_irq); }