| Message ID | 501551E4.6020806@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Jul 29, 2012 at 11:08 PM, Daniel Mack <zonque@gmail.com> wrote: > Hi Haojian, > > On 28.07.2012 17:42, Haojian Zhuang wrote: >> On Sat, Jul 28, 2012 at 5:56 PM, Daniel Mack <zonque@gmail.com> wrote: >>> On 28.07.2012 09:17, Haojian Zhuang wrote: >>>> On Fri, Jul 27, 2012 at 3:16 AM, Daniel Mack <zonque@gmail.com> wrote: >>>>> Properly register on-chip interrupt using the irqdomain logic. The >>>>> number of interrupts is taken from the devicetree node. >>>>> >>>>> Signed-off-by: Daniel Mack <zonque@gmail.com> >>>>> --- >>>>> arch/arm/mach-pxa/irq.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++ >>>>> arch/arm/mach-pxa/pxa3xx.c | 17 +++++++++-- >>>>> 2 files changed, 88 insertions(+), 2 deletions(-) >>>>> >>>>> +#ifdef CONFIG_OF >>>>> +static struct irq_domain *pxa_irq_domain; >>>>> + >>>>> +static int pxa_irq_map(struct irq_domain *h, unsigned int virq, >>>>> + irq_hw_number_t hw) >>>>> +{ >>>>> + int irq, i = hw % 32; >>>>> + void __iomem *base = irq_base(hw / 32); >>>>> + >>>>> + /* initialize interrupt priority */ >>>>> + if (cpu_has_ipr()) >>>>> + __raw_writel(i | IPR_VALID, IRQ_BASE + IPR(i)); >>>> Since we have DT support at here. Could we use property for interrupt priority? >>> >>> Not sure what you mean here. Can you elaborate? I couldn't find any >>> reference to IRQ priorities in other platforms either. >>> >>> Maybe we can also add that in a separate patch, which would also help in >>> tracking possible regressions du to such a change? >>> >> cpu_has_ipr() returns true if CPU isn't PXA25x. >> My point is that we can avoid to use cpu_is_xxx() while DT is used. We only need >> to append a property "marvell,intc-priority" is DTS. So the code could >> be changed >> in below. >> if (of_find_property(np, "marvell,intc-priority", NULL)) >> __raw_writel(i | IPR_VALID, IRQ_BASE + IPR(i)); >> >>>>> + irq = PXA_IRQ(virq); >>>> #ifdef CONFIG_PXA_HAVE_ISA_IRQS >>>> #define PXA_ISA_IRQ(x) (x) >>>> #define PXA_ISA_IRQ_NUM (16) >>>> #else >>>> #define PXA_ISA_IRQ_NUM (0) >>>> #endif >>>> >>>> Could we avoid to use PXA_IRQ() at here? We can make use of >>>> NR_IRQS_LEGACY that is 16. Since you already use irq_alloc_descs() >>>> to allocate irqs that virtual irq number starts from 16. So you needn't >>>> use PXA_IRQ() any more. >>> >>> Ok, I changed this. Note that there's still need to subtract >>> NR_IRQS_LEGACY from the virq that is passed in to the .map function, >>> because early_irq_init() in kernel/irq/irqdesc.c will pre-allocate the >>> IRQs the platform claims to have natively, which defaults to 16 on PXA, >>> unless the machine descriptor sets nr_irqs, which it doesn't in case of DT. >>> >> You needn't subtract NR_IRQS_LEGACY. PXA25x hwirq starts from >> 16 & PXA27x/PXA3xx hwirq starts from 0. While DT is used, irq_alloc_descs() >> allocates virq from NR_IRQS_LEGACY. For PXA25x, there's exactly match. >> For PXA27x/PXA3xx, there's a little different. But it doesn't matter. We needn't >> force virq starting from 0 on PXA27x/PXA3xx. The first virq starts from 16 is >> also OK. > > Ok, now I got you. By simply ignoring the virq passed in and only taking > into account the hw irq, this is of course possible. > > Please see the attached patch. Does that look better to you? I removed > the cpu_has_ipr() inline function and made it a variable that is used > and initalized from both the DT and the legacy code. > > > Daniel > Yes, both of these two are fixed perfectly. Now let's focus on this in below. I just find it. + if (cpu_has_ipr) + __raw_writel(bit | IPR_VALID, IRQ_BASE + IPR(bit)); #define IRQ_BASE io_p2v(0x40d00000) IRQ_BASE is defined in arch/arm/mach-pxa/irq.c. It's OK for non-DT mode. If we want to support DT, I hope that all registers mapping should be covered by of_iomap(). We should discard this kind of static register mapping. You can find some reference in current code base. Regards Haojian
On Sun, Jul 29, 2012 at 11:08 PM, Daniel Mack <zonque@gmail.com> wrote: > Hi Haojian, > > On 28.07.2012 17:42, Haojian Zhuang wrote: >> On Sat, Jul 28, 2012 at 5:56 PM, Daniel Mack <zonque@gmail.com> wrote: >>> On 28.07.2012 09:17, Haojian Zhuang wrote: >>>> On Fri, Jul 27, 2012 at 3:16 AM, Daniel Mack <zonque@gmail.com> wrote: >>>>> Properly register on-chip interrupt using the irqdomain logic. The >>>>> number of interrupts is taken from the devicetree node. >>>>> >>>>> Signed-off-by: Daniel Mack <zonque@gmail.com> >>>>> --- >>>>> arch/arm/mach-pxa/irq.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++ >>>>> arch/arm/mach-pxa/pxa3xx.c | 17 +++++++++-- >>>>> 2 files changed, 88 insertions(+), 2 deletions(-) >>>>> >>>>> +#ifdef CONFIG_OF >>>>> +static struct irq_domain *pxa_irq_domain; >>>>> + >>>>> +static int pxa_irq_map(struct irq_domain *h, unsigned int virq, >>>>> + irq_hw_number_t hw) >>>>> +{ >>>>> + int irq, i = hw % 32; >>>>> + void __iomem *base = irq_base(hw / 32); >>>>> + >>>>> + /* initialize interrupt priority */ >>>>> + if (cpu_has_ipr()) >>>>> + __raw_writel(i | IPR_VALID, IRQ_BASE + IPR(i)); >>>> Since we have DT support at here. Could we use property for interrupt priority? >>> >>> Not sure what you mean here. Can you elaborate? I couldn't find any >>> reference to IRQ priorities in other platforms either. >>> >>> Maybe we can also add that in a separate patch, which would also help in >>> tracking possible regressions du to such a change? >>> >> cpu_has_ipr() returns true if CPU isn't PXA25x. >> My point is that we can avoid to use cpu_is_xxx() while DT is used. We only need >> to append a property "marvell,intc-priority" is DTS. So the code could >> be changed >> in below. >> if (of_find_property(np, "marvell,intc-priority", NULL)) >> __raw_writel(i | IPR_VALID, IRQ_BASE + IPR(i)); >> > > Please see the attached patch. Does that look better to you? I removed > the cpu_has_ipr() inline function and made it a variable that is used > and initalized from both the DT and the legacy code. > > > Daniel > Don't forget to append this property into .dtsi file. Only PXA25x don't support this property. Regards Haojian
From 734b792760ee3bef6b6d2ef7ecb7f047f02ec62f Mon Sep 17 00:00:00 2001 From: Daniel Mack <zonque@gmail.com> Date: Sun, 22 Jul 2012 19:50:22 +0200 Subject: [PATCH] ARM: pxa: add devicetree code for irq handling Properly register on-chip interrupt using the irqdomain logic. The number of interrupts is taken from the devicetree node. cpu_has_ipr() was converted from an inline function to a static bool variable, so it can be set using the "marvell,intc-priority" property inside the device node of the tree. Signed-off-by: Daniel Mack <zonque@gmail.com> --- arch/arm/mach-pxa/irq.c | 88 +++++++++++++++++++++++++++++++++++++++++----- arch/arm/mach-pxa/pxa3xx.c | 17 +++++++-- 2 files changed, 95 insertions(+), 10 deletions(-) diff --git a/arch/arm/mach-pxa/irq.c b/arch/arm/mach-pxa/irq.c index 5dae15e..dedd10b 100644 --- a/arch/arm/mach-pxa/irq.c +++ b/arch/arm/mach-pxa/irq.c @@ -17,6 +17,8 @@ #include <linux/syscore_ops.h> #include <linux/io.h> #include <linux/irq.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> #include <asm/exception.h> @@ -49,11 +51,7 @@ */ static int pxa_internal_irq_nr; - -static inline int cpu_has_ipr(void) -{ - return !cpu_is_pxa25x(); -} +static bool cpu_has_ipr; static inline void __iomem *irq_base(int i) { @@ -128,6 +126,7 @@ void __init pxa_init_irq(int irq_nr, int (*fn)(struct irq_data *, unsigned int)) BUG_ON(irq_nr > MAX_INTERNAL_IRQS); pxa_internal_irq_nr = irq_nr; + cpu_has_ipr = !cpu_is_pxa25x(); for (n = 0; n < irq_nr; n += 32) { void __iomem *base = irq_base(n >> 5); @@ -136,7 +135,7 @@ void __init pxa_init_irq(int irq_nr, int (*fn)(struct irq_data *, unsigned int)) __raw_writel(0, base + ICLR); /* all IRQs are IRQ, not FIQ */ for (i = n; (i < (n + 32)) && (i < irq_nr); i++) { /* initialize interrupt priority */ - if (cpu_has_ipr()) + if (cpu_has_ipr) __raw_writel(i | IPR_VALID, IRQ_BASE + IPR(i)); irq = PXA_IRQ(i); @@ -168,7 +167,7 @@ static int pxa_irq_suspend(void) __raw_writel(0, base + ICMR); } - if (cpu_has_ipr()) { + if (cpu_has_ipr) { for (i = 0; i < pxa_internal_irq_nr; i++) saved_ipr[i] = __raw_readl(IRQ_BASE + IPR(i)); } @@ -187,7 +186,7 @@ static void pxa_irq_resume(void) __raw_writel(0, base + ICLR); } - if (cpu_has_ipr()) + if (cpu_has_ipr) for (i = 0; i < pxa_internal_irq_nr; i++) __raw_writel(saved_ipr[i], IRQ_BASE + IPR(i)); @@ -202,3 +201,76 @@ struct syscore_ops pxa_irq_syscore_ops = { .suspend = pxa_irq_suspend, .resume = pxa_irq_resume, }; + +#ifdef CONFIG_OF +static struct irq_domain *pxa_irq_domain; + +static int pxa_irq_map(struct irq_domain *h, unsigned int virq, + irq_hw_number_t hw) +{ + int bit = hw % 32; + void __iomem *base = irq_base(hw / 32); + + /* initialize interrupt priority */ + if (cpu_has_ipr) + __raw_writel(bit | IPR_VALID, IRQ_BASE + IPR(bit)); + + irq_set_chip_and_handler(hw, &pxa_internal_irq_chip, + handle_level_irq); + irq_set_chip_data(hw, base); + set_irq_flags(hw, IRQF_VALID); + + return 0; +} + +static struct irq_domain_ops pxa_irq_ops = { + .map = pxa_irq_map, + .xlate = irq_domain_xlate_onecell, +}; + +static const struct of_device_id intc_ids[] __initconst = { + { .compatible = "marvell,pxa-intc", }, + {} +}; + +void __init pxa_dt_irq_init(int (*fn)(struct irq_data *, unsigned int)) +{ + struct device_node *node; + const struct of_device_id *of_id; + struct pxa_intc_conf *conf; + int nr_irqs, irq_base, ret; + + node = of_find_matching_node(NULL, intc_ids); + if (!node) { + pr_err("Failed to find interrupt controller in arch-pxa\n"); + return; + } + of_id = of_match_node(intc_ids, node); + conf = of_id->data; + + ret = of_property_read_u32(node, "mrvl,intc-nr-irqs", &nr_irqs); + if (ret) { + pr_err("Not found mrvl,intc-nr-irqs property\n"); + return; + } + + if (of_find_property(node, "marvell,intc-priority", NULL)) + cpu_has_ipr = 1; + + irq_base = irq_alloc_descs(-1, 0, nr_irqs, 0); + if (irq_base < 0) { + pr_err("Failed to allocate IRQ numbers\n"); + return; + } + + pxa_irq_domain = irq_domain_add_legacy(node, nr_irqs, 0, 0, + &pxa_irq_ops, NULL); + if (!pxa_irq_domain) + panic("Unable to add PXA IRQ domain\n"); + + irq_set_default_host(pxa_irq_domain); + pxa_init_irq(nr_irqs, fn); + + return; +} +#endif /* CONFIG_OF */ diff --git a/arch/arm/mach-pxa/pxa3xx.c b/arch/arm/mach-pxa/pxa3xx.c index dffb7e8..1827d3c 100644 --- a/arch/arm/mach-pxa/pxa3xx.c +++ b/arch/arm/mach-pxa/pxa3xx.c @@ -40,6 +40,8 @@ #define PECR_IE(n) ((1 << ((n) * 2)) << 28) #define PECR_IS(n) ((1 << ((n) * 2)) << 29) +extern void __init pxa_dt_irq_init(int (*fn)(struct irq_data *, unsigned int)); + static DEFINE_PXA3_CKEN(pxa3xx_ffuart, FFUART, 14857000, 1); static DEFINE_PXA3_CKEN(pxa3xx_btuart, BTUART, 14857000, 1); static DEFINE_PXA3_CKEN(pxa3xx_stuart, STUART, 14857000, 1); @@ -382,7 +384,7 @@ static void __init pxa_init_ext_wakeup_irq(int (*fn)(struct irq_data *, pxa_ext_wakeup_chip.irq_set_wake = fn; } -void __init pxa3xx_init_irq(void) +static void __init __pxa3xx_init_irq(void) { /* enable CP6 access */ u32 value; @@ -390,10 +392,21 @@ void __init pxa3xx_init_irq(void) value |= (1 << 6); __asm__ __volatile__("mcr p15, 0, %0, c15, c1, 0\n": :"r"(value)); - pxa_init_irq(56, pxa3xx_set_wake); pxa_init_ext_wakeup_irq(pxa3xx_set_wake); } +void __init pxa3xx_init_irq(void) +{ + __pxa3xx_init_irq(); + pxa_init_irq(56, pxa3xx_set_wake); +} + +void __init pxa3xx_dt_init_irq(void) +{ + __pxa3xx_init_irq(); + pxa_dt_irq_init(pxa3xx_set_wake); +} + static struct map_desc pxa3xx_io_desc[] __initdata = { { /* Mem Ctl */ .virtual = (unsigned long)SMEMC_VIRT, -- 1.7.11.2