2014-11-07 06:45:04

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 00/14] genirq endian fixes; bcm7120/brcmstb IRQ updates

V3->V4:

- Fix buildbot bisectability warning on patch 02/14 (missing include)

- Add kernel-doc text for the new reg_{readl,writel} struct members and
IRQ_GC_BE_IO flag


Kevin Cernekee (14):
sh: Eliminate unused irq_reg_{readl,writel} accessors
genirq: Generic chip: Change irq_reg_{readl,writel} arguments
genirq: Generic chip: Allow irqchip drivers to override
irq_reg_{readl,writel}
genirq: Generic chip: Add big endian I/O accessors
irqchip: brcmstb-l2: Eliminate dependency on ARM code
irqchip: bcm7120-l2: Eliminate bad IRQ check
irqchip: bcm7120-l2, brcmstb-l2: Remove ARM Kconfig dependency
irqchip: bcm7120-l2: Make sure all register accesses use base+offset
irqchip: bcm7120-l2: Fix missing nibble in gc->unused mask
irqchip: bcm7120-l2: Use gc->mask_cache to simplify suspend/resume
functions
irqchip: bcm7120-l2: Extend driver to support 64+ bit controllers
irqchip: bcm7120-l2: Decouple driver from brcmstb-l2
irqchip: bcm7120-l2: Convert driver to use irq_reg_{readl,writel}
irqchip: brcmstb-l2: Convert driver to use irq_reg_{readl,writel}

.../interrupt-controller/brcm,bcm7120-l2-intc.txt | 26 ++-
arch/arm/mach-bcm/Kconfig | 1 +
arch/sh/boards/mach-se/7343/irq.c | 3 -
arch/sh/boards/mach-se/7722/irq.c | 3 -
drivers/irqchip/Kconfig | 6 +-
drivers/irqchip/Makefile | 4 +-
drivers/irqchip/irq-atmel-aic.c | 40 ++---
drivers/irqchip/irq-atmel-aic5.c | 65 ++++----
drivers/irqchip/irq-bcm7120-l2.c | 174 +++++++++++++--------
drivers/irqchip/irq-brcmstb-l2.c | 41 +++--
drivers/irqchip/irq-sunxi-nmi.c | 4 +-
drivers/irqchip/irq-tb10x.c | 4 +-
include/linux/irq.h | 32 +++-
kernel/irq/generic-chip.c | 36 +++--
14 files changed, 263 insertions(+), 176 deletions(-)

--
2.1.1


2014-11-07 06:45:09

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 02/14] genirq: Generic chip: Change irq_reg_{readl,writel} arguments

Pass in the irq_chip_generic struct so we can use different readl/writel
settings for each irqchip driver, when appropriate. Compute
(gc->reg_base + reg_offset) in the helper function because this is pretty
much what all callers want to do anyway.

Compile-tested using the following configurations:

at91_dt_defconfig (CONFIG_ATMEL_AIC_IRQ=y)
sama5_defconfig (CONFIG_ATMEL_AIC5_IRQ=y)
sunxi_defconfig (CONFIG_ARCH_SUNXI=y)

tb10x (ARC) is untested.

Signed-off-by: Kevin Cernekee <[email protected]>
---
drivers/irqchip/irq-atmel-aic.c | 40 ++++++++++++-------------
drivers/irqchip/irq-atmel-aic5.c | 65 +++++++++++++++++++---------------------
drivers/irqchip/irq-sunxi-nmi.c | 4 +--
drivers/irqchip/irq-tb10x.c | 4 +--
include/linux/irq.h | 20 ++++++++-----
kernel/irq/generic-chip.c | 20 ++++++-------
6 files changed, 78 insertions(+), 75 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index 9a2cf3c..27fdd8c 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -65,11 +65,11 @@ aic_handle(struct pt_regs *regs)
u32 irqnr;
u32 irqstat;

- irqnr = irq_reg_readl(gc->reg_base + AT91_AIC_IVR);
- irqstat = irq_reg_readl(gc->reg_base + AT91_AIC_ISR);
+ irqnr = irq_reg_readl(gc, AT91_AIC_IVR);
+ irqstat = irq_reg_readl(gc, AT91_AIC_ISR);

if (!irqstat)
- irq_reg_writel(0, gc->reg_base + AT91_AIC_EOICR);
+ irq_reg_writel(gc, 0, AT91_AIC_EOICR);
else
handle_domain_irq(aic_domain, irqnr, regs);
}
@@ -80,7 +80,7 @@ static int aic_retrigger(struct irq_data *d)

/* Enable interrupt on AIC5 */
irq_gc_lock(gc);
- irq_reg_writel(d->mask, gc->reg_base + AT91_AIC_ISCR);
+ irq_reg_writel(gc, d->mask, AT91_AIC_ISCR);
irq_gc_unlock(gc);

return 0;
@@ -92,12 +92,12 @@ static int aic_set_type(struct irq_data *d, unsigned type)
unsigned int smr;
int ret;

- smr = irq_reg_readl(gc->reg_base + AT91_AIC_SMR(d->hwirq));
+ smr = irq_reg_readl(gc, AT91_AIC_SMR(d->hwirq));
ret = aic_common_set_type(d, type, &smr);
if (ret)
return ret;

- irq_reg_writel(smr, gc->reg_base + AT91_AIC_SMR(d->hwirq));
+ irq_reg_writel(gc, smr, AT91_AIC_SMR(d->hwirq));

return 0;
}
@@ -108,8 +108,8 @@ static void aic_suspend(struct irq_data *d)
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);

irq_gc_lock(gc);
- irq_reg_writel(gc->mask_cache, gc->reg_base + AT91_AIC_IDCR);
- irq_reg_writel(gc->wake_active, gc->reg_base + AT91_AIC_IECR);
+ irq_reg_writel(gc, gc->mask_cache, AT91_AIC_IDCR);
+ irq_reg_writel(gc, gc->wake_active, AT91_AIC_IECR);
irq_gc_unlock(gc);
}

@@ -118,8 +118,8 @@ static void aic_resume(struct irq_data *d)
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);

irq_gc_lock(gc);
- irq_reg_writel(gc->wake_active, gc->reg_base + AT91_AIC_IDCR);
- irq_reg_writel(gc->mask_cache, gc->reg_base + AT91_AIC_IECR);
+ irq_reg_writel(gc, gc->wake_active, AT91_AIC_IDCR);
+ irq_reg_writel(gc, gc->mask_cache, AT91_AIC_IECR);
irq_gc_unlock(gc);
}

@@ -128,8 +128,8 @@ static void aic_pm_shutdown(struct irq_data *d)
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);

irq_gc_lock(gc);
- irq_reg_writel(0xffffffff, gc->reg_base + AT91_AIC_IDCR);
- irq_reg_writel(0xffffffff, gc->reg_base + AT91_AIC_ICCR);
+ irq_reg_writel(gc, 0xffffffff, AT91_AIC_IDCR);
+ irq_reg_writel(gc, 0xffffffff, AT91_AIC_ICCR);
irq_gc_unlock(gc);
}
#else
@@ -148,24 +148,24 @@ static void __init aic_hw_init(struct irq_domain *domain)
* will not Lock out nIRQ
*/
for (i = 0; i < 8; i++)
- irq_reg_writel(0, gc->reg_base + AT91_AIC_EOICR);
+ irq_reg_writel(gc, 0, AT91_AIC_EOICR);

/*
* Spurious Interrupt ID in Spurious Vector Register.
* When there is no current interrupt, the IRQ Vector Register
* reads the value stored in AIC_SPU
*/
- irq_reg_writel(0xffffffff, gc->reg_base + AT91_AIC_SPU);
+ irq_reg_writel(gc, 0xffffffff, AT91_AIC_SPU);

/* No debugging in AIC: Debug (Protect) Control Register */
- irq_reg_writel(0, gc->reg_base + AT91_AIC_DCR);
+ irq_reg_writel(gc, 0, AT91_AIC_DCR);

/* Disable and clear all interrupts initially */
- irq_reg_writel(0xffffffff, gc->reg_base + AT91_AIC_IDCR);
- irq_reg_writel(0xffffffff, gc->reg_base + AT91_AIC_ICCR);
+ irq_reg_writel(gc, 0xffffffff, AT91_AIC_IDCR);
+ irq_reg_writel(gc, 0xffffffff, AT91_AIC_ICCR);

for (i = 0; i < 32; i++)
- irq_reg_writel(i, gc->reg_base + AT91_AIC_SVR(i));
+ irq_reg_writel(gc, i, AT91_AIC_SVR(i));
}

static int aic_irq_domain_xlate(struct irq_domain *d,
@@ -195,10 +195,10 @@ static int aic_irq_domain_xlate(struct irq_domain *d,
gc = dgc->gc[idx];

irq_gc_lock(gc);
- smr = irq_reg_readl(gc->reg_base + AT91_AIC_SMR(*out_hwirq));
+ smr = irq_reg_readl(gc, AT91_AIC_SMR(*out_hwirq));
ret = aic_common_set_priority(intspec[2], &smr);
if (!ret)
- irq_reg_writel(smr, gc->reg_base + AT91_AIC_SMR(*out_hwirq));
+ irq_reg_writel(gc, smr, AT91_AIC_SMR(*out_hwirq));
irq_gc_unlock(gc);

return ret;
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index a11aae8..a2e8c3f 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -75,11 +75,11 @@ aic5_handle(struct pt_regs *regs)
u32 irqnr;
u32 irqstat;

- irqnr = irq_reg_readl(gc->reg_base + AT91_AIC5_IVR);
- irqstat = irq_reg_readl(gc->reg_base + AT91_AIC5_ISR);
+ irqnr = irq_reg_readl(gc, AT91_AIC5_IVR);
+ irqstat = irq_reg_readl(gc, AT91_AIC5_ISR);

if (!irqstat)
- irq_reg_writel(0, gc->reg_base + AT91_AIC5_EOICR);
+ irq_reg_writel(gc, 0, AT91_AIC5_EOICR);
else
handle_domain_irq(aic5_domain, irqnr, regs);
}
@@ -92,8 +92,8 @@ static void aic5_mask(struct irq_data *d)

/* Disable interrupt on AIC5 */
irq_gc_lock(gc);
- irq_reg_writel(d->hwirq, gc->reg_base + AT91_AIC5_SSR);
- irq_reg_writel(1, gc->reg_base + AT91_AIC5_IDCR);
+ irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
+ irq_reg_writel(gc, 1, AT91_AIC5_IDCR);
gc->mask_cache &= ~d->mask;
irq_gc_unlock(gc);
}
@@ -106,8 +106,8 @@ static void aic5_unmask(struct irq_data *d)

/* Enable interrupt on AIC5 */
irq_gc_lock(gc);
- irq_reg_writel(d->hwirq, gc->reg_base + AT91_AIC5_SSR);
- irq_reg_writel(1, gc->reg_base + AT91_AIC5_IECR);
+ irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
+ irq_reg_writel(gc, 1, AT91_AIC5_IECR);
gc->mask_cache |= d->mask;
irq_gc_unlock(gc);
}
@@ -120,8 +120,8 @@ static int aic5_retrigger(struct irq_data *d)

/* Enable interrupt on AIC5 */
irq_gc_lock(gc);
- irq_reg_writel(d->hwirq, gc->reg_base + AT91_AIC5_SSR);
- irq_reg_writel(1, gc->reg_base + AT91_AIC5_ISCR);
+ irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
+ irq_reg_writel(gc, 1, AT91_AIC5_ISCR);
irq_gc_unlock(gc);

return 0;
@@ -136,11 +136,11 @@ static int aic5_set_type(struct irq_data *d, unsigned type)
int ret;

irq_gc_lock(gc);
- irq_reg_writel(d->hwirq, gc->reg_base + AT91_AIC5_SSR);
- smr = irq_reg_readl(gc->reg_base + AT91_AIC5_SMR);
+ irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
+ smr = irq_reg_readl(gc, AT91_AIC5_SMR);
ret = aic_common_set_type(d, type, &smr);
if (!ret)
- irq_reg_writel(smr, gc->reg_base + AT91_AIC5_SMR);
+ irq_reg_writel(gc, smr, AT91_AIC5_SMR);
irq_gc_unlock(gc);

return ret;
@@ -162,12 +162,11 @@ static void aic5_suspend(struct irq_data *d)
if ((mask & gc->mask_cache) == (mask & gc->wake_active))
continue;

- irq_reg_writel(i + gc->irq_base,
- bgc->reg_base + AT91_AIC5_SSR);
+ irq_reg_writel(bgc, i + gc->irq_base, AT91_AIC5_SSR);
if (mask & gc->wake_active)
- irq_reg_writel(1, bgc->reg_base + AT91_AIC5_IECR);
+ irq_reg_writel(bgc, 1, AT91_AIC5_IECR);
else
- irq_reg_writel(1, bgc->reg_base + AT91_AIC5_IDCR);
+ irq_reg_writel(bgc, 1, AT91_AIC5_IDCR);
}
irq_gc_unlock(bgc);
}
@@ -187,12 +186,11 @@ static void aic5_resume(struct irq_data *d)
if ((mask & gc->mask_cache) == (mask & gc->wake_active))
continue;

- irq_reg_writel(i + gc->irq_base,
- bgc->reg_base + AT91_AIC5_SSR);
+ irq_reg_writel(bgc, i + gc->irq_base, AT91_AIC5_SSR);
if (mask & gc->mask_cache)
- irq_reg_writel(1, bgc->reg_base + AT91_AIC5_IECR);
+ irq_reg_writel(bgc, 1, AT91_AIC5_IECR);
else
- irq_reg_writel(1, bgc->reg_base + AT91_AIC5_IDCR);
+ irq_reg_writel(bgc, 1, AT91_AIC5_IDCR);
}
irq_gc_unlock(bgc);
}
@@ -207,10 +205,9 @@ static void aic5_pm_shutdown(struct irq_data *d)

irq_gc_lock(bgc);
for (i = 0; i < dgc->irqs_per_chip; i++) {
- irq_reg_writel(i + gc->irq_base,
- bgc->reg_base + AT91_AIC5_SSR);
- irq_reg_writel(1, bgc->reg_base + AT91_AIC5_IDCR);
- irq_reg_writel(1, bgc->reg_base + AT91_AIC5_ICCR);
+ irq_reg_writel(bgc, i + gc->irq_base, AT91_AIC5_SSR);
+ irq_reg_writel(bgc, 1, AT91_AIC5_IDCR);
+ irq_reg_writel(bgc, 1, AT91_AIC5_ICCR);
}
irq_gc_unlock(bgc);
}
@@ -230,24 +227,24 @@ static void __init aic5_hw_init(struct irq_domain *domain)
* will not Lock out nIRQ
*/
for (i = 0; i < 8; i++)
- irq_reg_writel(0, gc->reg_base + AT91_AIC5_EOICR);
+ irq_reg_writel(gc, 0, AT91_AIC5_EOICR);

/*
* Spurious Interrupt ID in Spurious Vector Register.
* When there is no current interrupt, the IRQ Vector Register
* reads the value stored in AIC_SPU
*/
- irq_reg_writel(0xffffffff, gc->reg_base + AT91_AIC5_SPU);
+ irq_reg_writel(gc, 0xffffffff, AT91_AIC5_SPU);

/* No debugging in AIC: Debug (Protect) Control Register */
- irq_reg_writel(0, gc->reg_base + AT91_AIC5_DCR);
+ irq_reg_writel(gc, 0, AT91_AIC5_DCR);

/* Disable and clear all interrupts initially */
for (i = 0; i < domain->revmap_size; i++) {
- irq_reg_writel(i, gc->reg_base + AT91_AIC5_SSR);
- irq_reg_writel(i, gc->reg_base + AT91_AIC5_SVR);
- irq_reg_writel(1, gc->reg_base + AT91_AIC5_IDCR);
- irq_reg_writel(1, gc->reg_base + AT91_AIC5_ICCR);
+ irq_reg_writel(gc, i, AT91_AIC5_SSR);
+ irq_reg_writel(gc, i, AT91_AIC5_SVR);
+ irq_reg_writel(gc, 1, AT91_AIC5_IDCR);
+ irq_reg_writel(gc, 1, AT91_AIC5_ICCR);
}
}

@@ -273,11 +270,11 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
gc = dgc->gc[0];

irq_gc_lock(gc);
- irq_reg_writel(*out_hwirq, gc->reg_base + AT91_AIC5_SSR);
- smr = irq_reg_readl(gc->reg_base + AT91_AIC5_SMR);
+ irq_reg_writel(gc, *out_hwirq, AT91_AIC5_SSR);
+ smr = irq_reg_readl(gc, AT91_AIC5_SMR);
ret = aic_common_set_priority(intspec[2], &smr);
if (!ret)
- irq_reg_writel(intspec[2] | smr, gc->reg_base + AT91_AIC5_SMR);
+ irq_reg_writel(gc, intspec[2] | smr, AT91_AIC5_SMR);
irq_gc_unlock(gc);

return ret;
diff --git a/drivers/irqchip/irq-sunxi-nmi.c b/drivers/irqchip/irq-sunxi-nmi.c
index 12f547a..4a9ce5b 100644
--- a/drivers/irqchip/irq-sunxi-nmi.c
+++ b/drivers/irqchip/irq-sunxi-nmi.c
@@ -50,12 +50,12 @@ static struct sunxi_sc_nmi_reg_offs sun6i_reg_offs = {
static inline void sunxi_sc_nmi_write(struct irq_chip_generic *gc, u32 off,
u32 val)
{
- irq_reg_writel(val, gc->reg_base + off);
+ irq_reg_writel(gc, val, off);
}

static inline u32 sunxi_sc_nmi_read(struct irq_chip_generic *gc, u32 off)
{
- return irq_reg_readl(gc->reg_base + off);
+ return irq_reg_readl(gc, off);
}

static void sunxi_sc_nmi_handle_irq(unsigned int irq, struct irq_desc *desc)
diff --git a/drivers/irqchip/irq-tb10x.c b/drivers/irqchip/irq-tb10x.c
index 7c44c99..accc200 100644
--- a/drivers/irqchip/irq-tb10x.c
+++ b/drivers/irqchip/irq-tb10x.c
@@ -43,12 +43,12 @@
static inline void ab_irqctl_writereg(struct irq_chip_generic *gc, u32 reg,
u32 val)
{
- irq_reg_writel(val, gc->reg_base + reg);
+ irq_reg_writel(gc, val, reg);
}

static inline u32 ab_irqctl_readreg(struct irq_chip_generic *gc, u32 reg)
{
- return irq_reg_readl(gc->reg_base + reg);
+ return irq_reg_readl(gc, reg);
}

static int tb10x_irq_set_type(struct irq_data *data, unsigned int flow_type)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 03f48d9..ed1135d 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -20,6 +20,7 @@
#include <linux/errno.h>
#include <linux/topology.h>
#include <linux/wait.h>
+#include <linux/io.h>

#include <asm/irq.h>
#include <asm/ptrace.h>
@@ -639,13 +640,6 @@ void arch_teardown_hwirq(unsigned int irq);
void irq_init_desc(unsigned int irq);
#endif

-#ifndef irq_reg_writel
-# define irq_reg_writel(val, addr) writel(val, addr)
-#endif
-#ifndef irq_reg_readl
-# define irq_reg_readl(addr) readl(addr)
-#endif
-
/**
* struct irq_chip_regs - register offsets for struct irq_gci
* @enable: Enable register offset to reg_base
@@ -821,4 +815,16 @@ static inline void irq_gc_lock(struct irq_chip_generic *gc) { }
static inline void irq_gc_unlock(struct irq_chip_generic *gc) { }
#endif

+static inline void irq_reg_writel(struct irq_chip_generic *gc,
+ u32 val, int reg_offset)
+{
+ writel(val, gc->reg_base + reg_offset);
+}
+
+static inline u32 irq_reg_readl(struct irq_chip_generic *gc,
+ int reg_offset)
+{
+ return readl(gc->reg_base + reg_offset);
+}
+
#endif /* _LINUX_IRQ_H */
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index cf80e7b..db458c6 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -39,7 +39,7 @@ void irq_gc_mask_disable_reg(struct irq_data *d)
u32 mask = d->mask;

irq_gc_lock(gc);
- irq_reg_writel(mask, gc->reg_base + ct->regs.disable);
+ irq_reg_writel(gc, mask, ct->regs.disable);
*ct->mask_cache &= ~mask;
irq_gc_unlock(gc);
}
@@ -59,7 +59,7 @@ void irq_gc_mask_set_bit(struct irq_data *d)

irq_gc_lock(gc);
*ct->mask_cache |= mask;
- irq_reg_writel(*ct->mask_cache, gc->reg_base + ct->regs.mask);
+ irq_reg_writel(gc, *ct->mask_cache, ct->regs.mask);
irq_gc_unlock(gc);
}
EXPORT_SYMBOL_GPL(irq_gc_mask_set_bit);
@@ -79,7 +79,7 @@ void irq_gc_mask_clr_bit(struct irq_data *d)

irq_gc_lock(gc);
*ct->mask_cache &= ~mask;
- irq_reg_writel(*ct->mask_cache, gc->reg_base + ct->regs.mask);
+ irq_reg_writel(gc, *ct->mask_cache, ct->regs.mask);
irq_gc_unlock(gc);
}
EXPORT_SYMBOL_GPL(irq_gc_mask_clr_bit);
@@ -98,7 +98,7 @@ void irq_gc_unmask_enable_reg(struct irq_data *d)
u32 mask = d->mask;

irq_gc_lock(gc);
- irq_reg_writel(mask, gc->reg_base + ct->regs.enable);
+ irq_reg_writel(gc, mask, ct->regs.enable);
*ct->mask_cache |= mask;
irq_gc_unlock(gc);
}
@@ -114,7 +114,7 @@ void irq_gc_ack_set_bit(struct irq_data *d)
u32 mask = d->mask;

irq_gc_lock(gc);
- irq_reg_writel(mask, gc->reg_base + ct->regs.ack);
+ irq_reg_writel(gc, mask, ct->regs.ack);
irq_gc_unlock(gc);
}
EXPORT_SYMBOL_GPL(irq_gc_ack_set_bit);
@@ -130,7 +130,7 @@ void irq_gc_ack_clr_bit(struct irq_data *d)
u32 mask = ~d->mask;

irq_gc_lock(gc);
- irq_reg_writel(mask, gc->reg_base + ct->regs.ack);
+ irq_reg_writel(gc, mask, ct->regs.ack);
irq_gc_unlock(gc);
}

@@ -145,8 +145,8 @@ void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
u32 mask = d->mask;

irq_gc_lock(gc);
- irq_reg_writel(mask, gc->reg_base + ct->regs.mask);
- irq_reg_writel(mask, gc->reg_base + ct->regs.ack);
+ irq_reg_writel(gc, mask, ct->regs.mask);
+ irq_reg_writel(gc, mask, ct->regs.ack);
irq_gc_unlock(gc);
}

@@ -161,7 +161,7 @@ void irq_gc_eoi(struct irq_data *d)
u32 mask = d->mask;

irq_gc_lock(gc);
- irq_reg_writel(mask, gc->reg_base + ct->regs.eoi);
+ irq_reg_writel(gc, mask, ct->regs.eoi);
irq_gc_unlock(gc);
}

@@ -245,7 +245,7 @@ irq_gc_init_mask_cache(struct irq_chip_generic *gc, enum irq_gc_flags flags)
}
ct[i].mask_cache = mskptr;
if (flags & IRQ_GC_INIT_MASK_CACHE)
- *mskptr = irq_reg_readl(gc->reg_base + mskreg);
+ *mskptr = irq_reg_readl(gc, mskreg);
}
}

--
2.1.1

2014-11-07 06:45:22

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 06/14] irqchip: bcm7120-l2: Eliminate bad IRQ check

This check may be prone to race conditions, e.g.

1) Some external event (e.g. GPIO level) causes an IRQ to become pending
2) Peripheral asserts the L2 IRQ
3) CPU takes an interrupt
4) The event from #1 goes away
5) bcm7120_l2_intc_irq_handle() reads back a 0 status

Unlike the hardware supported by brcmstb-l2, the bcm7120-l2 controller
does not latch the IRQ status. Bits can change if the inputs to the
controller change. Also, do_bad_IRQ() is an ARM-specific macro.

So let's just nuke it.

Signed-off-by: Kevin Cernekee <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
---
drivers/irqchip/irq-bcm7120-l2.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
index b9f4fb8..7086fe0 100644
--- a/drivers/irqchip/irq-bcm7120-l2.c
+++ b/drivers/irqchip/irq-bcm7120-l2.c
@@ -27,8 +27,6 @@

#include "irqchip.h"

-#include <asm/mach/irq.h>
-
/* Register offset in the L2 interrupt controller */
#define IRQEN 0x00
#define IRQSTAT 0x04
@@ -51,19 +49,12 @@ static void bcm7120_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
chained_irq_enter(chip, desc);

status = __raw_readl(b->base + IRQSTAT);
-
- if (status == 0) {
- do_bad_IRQ(irq, desc);
- goto out;
- }
-
- do {
+ while (status) {
irq = ffs(status) - 1;
status &= ~(1 << irq);
generic_handle_irq(irq_find_mapping(b->domain, irq));
- } while (status);
+ }

-out:
chained_irq_exit(chip, desc);
}

--
2.1.1

2014-11-07 06:45:38

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 13/14] irqchip: bcm7120-l2: Convert driver to use irq_reg_{readl,writel}

On BE MIPS systems this needs to use the new IRQ_GC_BE_IO gc_flag. In
all other cases it will use the standard readl/writel accessors.

The initial irq_fwd_mask setup runs before "gc" is initialized, so it
is unchanged for now. This could potentially be a problem on an ARM
system that boots in LE mode but runs a BE kernel, but currently none
of the supported ARM platforms are ever expected to run BE.

Signed-off-by: Kevin Cernekee <[email protected]>
---
drivers/irqchip/irq-bcm7120-l2.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
index e53a3a6..e7c6155 100644
--- a/drivers/irqchip/irq-bcm7120-l2.c
+++ b/drivers/irqchip/irq-bcm7120-l2.c
@@ -13,6 +13,7 @@
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/module.h>
+#include <linux/kconfig.h>
#include <linux/platform_device.h>
#include <linux/of.h>
#include <linux/of_irq.h>
@@ -60,8 +61,7 @@ static void bcm7120_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
int hwirq;

irq_gc_lock(gc);
- pending = __raw_readl(b->base[idx] + IRQSTAT) &
- gc->mask_cache;
+ pending = irq_reg_readl(gc, IRQSTAT) & gc->mask_cache;
irq_gc_unlock(gc);

for_each_set_bit(hwirq, &pending, IRQS_PER_WORD) {
@@ -79,10 +79,8 @@ static void bcm7120_l2_intc_suspend(struct irq_data *d)
struct bcm7120_l2_intc_data *b = gc->private;

irq_gc_lock(gc);
- if (b->can_wake) {
- __raw_writel(gc->mask_cache | gc->wake_active,
- gc->reg_base + IRQEN);
- }
+ if (b->can_wake)
+ irq_reg_writel(gc, gc->mask_cache | gc->wake_active, IRQEN);
irq_gc_unlock(gc);
}

@@ -92,7 +90,7 @@ static void bcm7120_l2_intc_resume(struct irq_data *d)

/* Restore the saved mask */
irq_gc_lock(gc);
- __raw_writel(gc->mask_cache, gc->reg_base + IRQEN);
+ irq_reg_writel(gc, gc->mask_cache, IRQEN);
irq_gc_unlock(gc);
}

@@ -132,7 +130,7 @@ int __init bcm7120_l2_intc_of_init(struct device_node *dn,
const __be32 *map_mask;
int num_parent_irqs;
int ret = 0, len;
- unsigned int idx, irq;
+ unsigned int idx, irq, flags;

data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
@@ -195,9 +193,15 @@ int __init bcm7120_l2_intc_of_init(struct device_node *dn,
goto out_unmap;
}

+ /* MIPS chips strapped for BE will automagically configure the
+ * peripheral registers for CPU-native byte order.
+ */
+ flags = IRQ_GC_INIT_MASK_CACHE;
+ if (IS_ENABLED(CONFIG_MIPS) && IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+ flags |= IRQ_GC_BE_IO;
+
ret = irq_alloc_domain_generic_chips(data->domain, IRQS_PER_WORD, 1,
- dn->full_name, handle_level_irq, clr, 0,
- IRQ_GC_INIT_MASK_CACHE);
+ dn->full_name, handle_level_irq, clr, 0, flags);
if (ret) {
pr_err("failed to allocate generic irq chip\n");
goto out_free_domain;
--
2.1.1

2014-11-07 06:45:26

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 07/14] irqchip: bcm7120-l2, brcmstb-l2: Remove ARM Kconfig dependency

This can compile for MIPS (or anything else) now.

Signed-off-by: Kevin Cernekee <[email protected]>
Acked-by: Arnd Bergmann <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
---
drivers/irqchip/Kconfig | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index b21f12f..09c79d1 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -50,7 +50,6 @@ config ATMEL_AIC5_IRQ

config BRCMSTB_L2_IRQ
bool
- depends on ARM
select GENERIC_IRQ_CHIP
select IRQ_DOMAIN

--
2.1.1

2014-11-07 06:45:29

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 10/14] irqchip: bcm7120-l2: Use gc->mask_cache to simplify suspend/resume functions

The cached value already incorporates irq_fwd_mask, and was saved the
last time an IRQ was enabled/disabled.

Signed-off-by: Kevin Cernekee <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
---
drivers/irqchip/irq-bcm7120-l2.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
index b70679f8..9841121 100644
--- a/drivers/irqchip/irq-bcm7120-l2.c
+++ b/drivers/irqchip/irq-bcm7120-l2.c
@@ -37,7 +37,6 @@ struct bcm7120_l2_intc_data {
bool can_wake;
u32 irq_fwd_mask;
u32 irq_map_mask;
- u32 saved_mask;
};

static void bcm7120_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
@@ -62,14 +61,11 @@ static void bcm7120_l2_intc_suspend(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct bcm7120_l2_intc_data *b = gc->private;
- u32 reg;

irq_gc_lock(gc);
- /* Save the current mask and the interrupt forward mask */
- b->saved_mask = __raw_readl(b->base + IRQEN) | b->irq_fwd_mask;
if (b->can_wake) {
- reg = b->saved_mask | gc->wake_active;
- __raw_writel(reg, b->base + IRQEN);
+ __raw_writel(gc->mask_cache | gc->wake_active,
+ b->base + IRQEN);
}
irq_gc_unlock(gc);
}
@@ -77,11 +73,10 @@ static void bcm7120_l2_intc_suspend(struct irq_data *d)
static void bcm7120_l2_intc_resume(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- struct bcm7120_l2_intc_data *b = gc->private;

/* Restore the saved mask */
irq_gc_lock(gc);
- __raw_writel(b->saved_mask, b->base + IRQEN);
+ __raw_writel(gc->mask_cache, b->base + IRQEN);
irq_gc_unlock(gc);
}

--
2.1.1

2014-11-07 06:45:34

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 11/14] irqchip: bcm7120-l2: Extend driver to support 64+ bit controllers

Most implementations of the bcm7120-l2 controller only have a single
32-bit enable word + 32-bit status word. But some instances have added
more enable/status pairs in order to support 64+ IRQs (which are all
ORed into one parent IRQ input). Make the following changes to allow
the driver to support this:

- Extend DT bindings so that multiple words can be specified for the
reg property, various masks, etc.

- Add loops to the probe/handle functions to deal with each word
separately

- Allocate 1 generic-chip for every 32 IRQs, so we can still use the
clr/set helper functions

- Update the documentation

This uses one domain per bcm7120-l2 DT node. If the DT node defines
multiple enable/status pairs (i.e. >=64 IRQs) then the driver will
create a single IRQ domain with 2+ generic chips. Multiple generic chips
are required because the generic-chip code can only handle one
enable/status register pair per instance.

Signed-off-by: Kevin Cernekee <[email protected]>
---
.../interrupt-controller/brcm,bcm7120-l2-intc.txt | 26 ++--
drivers/irqchip/irq-bcm7120-l2.c | 144 ++++++++++++++-------
2 files changed, 113 insertions(+), 57 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm7120-l2-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm7120-l2-intc.txt
index ff812a8..bae1f21 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm7120-l2-intc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm7120-l2-intc.txt
@@ -13,7 +13,12 @@ Such an interrupt controller has the following hardware design:
or if they will output an interrupt signal at this 2nd level interrupt
controller, in particular for UARTs

-- not all 32-bits within the interrupt controller actually map to an interrupt
+- typically has one 32-bit enable word and one 32-bit status word, but on
+ some hardware may have more than one enable/status pair
+
+- no atomic set/clear operations
+
+- not all bits within the interrupt controller actually map to an interrupt

The typical hardware layout for this controller is represented below:

@@ -48,7 +53,9 @@ The typical hardware layout for this controller is represented below:
Required properties:

- compatible: should be "brcm,bcm7120-l2-intc"
-- reg: specifies the base physical address and size of the registers
+- reg: specifies the base physical address and size of the registers;
+ multiple pairs may be specified, with the first pair handling IRQ offsets
+ 0..31 and the second pair handling 32..63
- interrupt-controller: identifies the node as an interrupt controller
- #interrupt-cells: specifies the number of cells needed to encode an interrupt
source, should be 1.
@@ -59,18 +66,21 @@ Required properties:
- brcm,int-map-mask: 32-bits bit mask describing how many and which interrupts
are wired to this 2nd level interrupt controller, and how they match their
respective interrupt parents. Should match exactly the number of interrupts
- specified in the 'interrupts' property.
+ specified in the 'interrupts' property, multiplied by the number of
+ enable/status register pairs implemented by this controller. For
+ multiple parent IRQs with multiple enable/status words, this looks like:
+ <irq0_w0 irq0_w1 irq1_w0 irq1_w1 ...>

Optional properties:

- brcm,irq-can-wake: if present, this means the L2 controller can be used as a
wakeup source for system suspend/resume.

-- brcm,int-fwd-mask: if present, a 32-bits bit mask to configure for the
- interrupts which have a mux gate, typically UARTs. Setting these bits will
- make their respective interrupts outputs bypass this 2nd level interrupt
- controller completely, it completely transparent for the interrupt controller
- parent
+- brcm,int-fwd-mask: if present, a bit mask to configure the interrupts which
+ have a mux gate, typically UARTs. Setting these bits will make their
+ respective interrupt outputs bypass this 2nd level interrupt controller
+ completely; it is completely transparent for the interrupt controller
+ parent. This should have one 32-bit word per enable/status pair.

Example:

diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
index 9841121..ef4d32c 100644
--- a/drivers/irqchip/irq-bcm7120-l2.c
+++ b/drivers/irqchip/irq-bcm7120-l2.c
@@ -23,6 +23,7 @@
#include <linux/io.h>
#include <linux/irqdomain.h>
#include <linux/reboot.h>
+#include <linux/bitops.h>
#include <linux/irqchip/chained_irq.h>

#include "irqchip.h"
@@ -31,27 +32,42 @@
#define IRQEN 0x00
#define IRQSTAT 0x04

+#define MAX_WORDS 4
+#define IRQS_PER_WORD 32
+
struct bcm7120_l2_intc_data {
- void __iomem *base;
+ unsigned int n_words;
+ void __iomem *base[MAX_WORDS];
struct irq_domain *domain;
bool can_wake;
- u32 irq_fwd_mask;
- u32 irq_map_mask;
+ u32 irq_fwd_mask[MAX_WORDS];
+ u32 irq_map_mask[MAX_WORDS];
};

static void bcm7120_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
{
struct bcm7120_l2_intc_data *b = irq_desc_get_handler_data(desc);
struct irq_chip *chip = irq_desc_get_chip(desc);
- u32 status;
+ unsigned int idx;

chained_irq_enter(chip, desc);

- status = __raw_readl(b->base + IRQSTAT);
- while (status) {
- irq = ffs(status) - 1;
- status &= ~(1 << irq);
- generic_handle_irq(irq_find_mapping(b->domain, irq));
+ for (idx = 0; idx < b->n_words; idx++) {
+ int base = idx * IRQS_PER_WORD;
+ struct irq_chip_generic *gc =
+ irq_get_domain_generic_chip(b->domain, base);
+ unsigned long pending;
+ int hwirq;
+
+ irq_gc_lock(gc);
+ pending = __raw_readl(b->base[idx] + IRQSTAT) &
+ gc->mask_cache;
+ irq_gc_unlock(gc);
+
+ for_each_set_bit(hwirq, &pending, IRQS_PER_WORD) {
+ generic_handle_irq(irq_find_mapping(b->domain,
+ base + hwirq));
+ }
}

chained_irq_exit(chip, desc);
@@ -65,7 +81,7 @@ static void bcm7120_l2_intc_suspend(struct irq_data *d)
irq_gc_lock(gc);
if (b->can_wake) {
__raw_writel(gc->mask_cache | gc->wake_active,
- b->base + IRQEN);
+ gc->reg_base + IRQEN);
}
irq_gc_unlock(gc);
}
@@ -76,7 +92,7 @@ static void bcm7120_l2_intc_resume(struct irq_data *d)

/* Restore the saved mask */
irq_gc_lock(gc);
- __raw_writel(gc->mask_cache, b->base + IRQEN);
+ __raw_writel(gc->mask_cache, gc->reg_base + IRQEN);
irq_gc_unlock(gc);
}

@@ -85,6 +101,7 @@ static int bcm7120_l2_intc_init_one(struct device_node *dn,
int irq, const __be32 *map_mask)
{
int parent_irq;
+ unsigned int idx;

parent_irq = irq_of_parse_and_map(dn, irq);
if (parent_irq < 0) {
@@ -92,7 +109,12 @@ static int bcm7120_l2_intc_init_one(struct device_node *dn,
return parent_irq;
}

- data->irq_map_mask |= be32_to_cpup(map_mask + irq);
+ /* For multiple parent IRQs with multiple words, this looks like:
+ * <irq0_w0 irq0_w1 irq1_w0 irq1_w1 ...>
+ */
+ for (idx = 0; idx < data->n_words; idx++)
+ data->irq_map_mask[idx] |=
+ be32_to_cpup(map_mask + irq * data->n_words + idx);

irq_set_handler_data(parent_irq, data);
irq_set_chained_handler(parent_irq, bcm7120_l2_intc_irq_handle);
@@ -109,26 +131,41 @@ int __init bcm7120_l2_intc_of_init(struct device_node *dn,
struct irq_chip_type *ct;
const __be32 *map_mask;
int num_parent_irqs;
- int ret = 0, len, irq;
+ int ret = 0, len;
+ unsigned int idx, irq;

data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;

- data->base = of_iomap(dn, 0);
- if (!data->base) {
+ for (idx = 0; idx < MAX_WORDS; idx++) {
+ data->base[idx] = of_iomap(dn, idx);
+ if (!data->base[idx])
+ break;
+ data->n_words = idx + 1;
+ }
+ if (!data->n_words) {
pr_err("failed to remap intc L2 registers\n");
ret = -ENOMEM;
- goto out_free;
+ goto out_unmap;
}

- if (of_property_read_u32(dn, "brcm,int-fwd-mask", &data->irq_fwd_mask))
- data->irq_fwd_mask = 0;
-
- /* Enable all interrupt specified in the interrupt forward mask and have
- * the other disabled
+ /* Enable all interrupts specified in the interrupt forward mask;
+ * disable all others. If the property doesn't exist (-EINVAL),
+ * assume all zeroes.
*/
- __raw_writel(data->irq_fwd_mask, data->base + IRQEN);
+ ret = of_property_read_u32_array(dn, "brcm,int-fwd-mask",
+ data->irq_fwd_mask, data->n_words);
+ if (ret == 0 || ret == -EINVAL) {
+ for (idx = 0; idx < data->n_words; idx++)
+ __raw_writel(data->irq_fwd_mask[idx],
+ data->base[idx] + IRQEN);
+ } else {
+ /* property exists but has the wrong number of words */
+ pr_err("invalid int-fwd-mask property\n");
+ ret = -EINVAL;
+ goto out_unmap;
+ }

num_parent_irqs = of_irq_count(dn);
if (num_parent_irqs <= 0) {
@@ -138,7 +175,8 @@ int __init bcm7120_l2_intc_of_init(struct device_node *dn,
}

map_mask = of_get_property(dn, "brcm,int-map-mask", &len);
- if (!map_mask || (len != (sizeof(*map_mask) * num_parent_irqs))) {
+ if (!map_mask ||
+ (len != (sizeof(*map_mask) * num_parent_irqs * data->n_words))) {
pr_err("invalid brcm,int-map-mask property\n");
ret = -EINVAL;
goto out_unmap;
@@ -150,14 +188,14 @@ int __init bcm7120_l2_intc_of_init(struct device_node *dn,
goto out_unmap;
}

- data->domain = irq_domain_add_linear(dn, 32,
- &irq_generic_chip_ops, NULL);
+ data->domain = irq_domain_add_linear(dn, IRQS_PER_WORD * data->n_words,
+ &irq_generic_chip_ops, NULL);
if (!data->domain) {
ret = -ENOMEM;
goto out_unmap;
}

- ret = irq_alloc_domain_generic_chips(data->domain, 32, 1,
+ ret = irq_alloc_domain_generic_chips(data->domain, IRQS_PER_WORD, 1,
dn->full_name, handle_level_irq, clr, 0,
IRQ_GC_INIT_MASK_CACHE);
if (ret) {
@@ -165,39 +203,47 @@ int __init bcm7120_l2_intc_of_init(struct device_node *dn,
goto out_free_domain;
}

- gc = irq_get_domain_generic_chip(data->domain, 0);
- gc->unused = 0xffffffff & ~data->irq_map_mask;
- gc->reg_base = data->base;
- gc->private = data;
- ct = gc->chip_types;
-
- ct->regs.mask = IRQEN;
- ct->chip.irq_mask = irq_gc_mask_clr_bit;
- ct->chip.irq_unmask = irq_gc_mask_set_bit;
- ct->chip.irq_ack = irq_gc_noop;
- ct->chip.irq_suspend = bcm7120_l2_intc_suspend;
- ct->chip.irq_resume = bcm7120_l2_intc_resume;
-
- if (of_property_read_bool(dn, "brcm,irq-can-wake")) {
+ if (of_property_read_bool(dn, "brcm,irq-can-wake"))
data->can_wake = true;
- /* This IRQ chip can wake the system, set all relevant child
- * interupts in wake_enabled mask
- */
- gc->wake_enabled = 0xffffffff;
- gc->wake_enabled &= ~gc->unused;
- ct->chip.irq_set_wake = irq_gc_set_wake;
+
+ for (idx = 0; idx < data->n_words; idx++) {
+ irq = idx * IRQS_PER_WORD;
+ gc = irq_get_domain_generic_chip(data->domain, irq);
+
+ gc->unused = 0xffffffff & ~data->irq_map_mask[idx];
+ gc->reg_base = data->base[idx];
+ gc->private = data;
+ ct = gc->chip_types;
+
+ ct->regs.mask = IRQEN;
+ ct->chip.irq_mask = irq_gc_mask_clr_bit;
+ ct->chip.irq_unmask = irq_gc_mask_set_bit;
+ ct->chip.irq_ack = irq_gc_noop;
+ ct->chip.irq_suspend = bcm7120_l2_intc_suspend;
+ ct->chip.irq_resume = bcm7120_l2_intc_resume;
+
+ if (data->can_wake) {
+ /* This IRQ chip can wake the system, set all
+ * relevant child interupts in wake_enabled mask
+ */
+ gc->wake_enabled = 0xffffffff;
+ gc->wake_enabled &= ~gc->unused;
+ ct->chip.irq_set_wake = irq_gc_set_wake;
+ }
}

pr_info("registered BCM7120 L2 intc (mem: 0x%p, parent IRQ(s): %d)\n",
- data->base, num_parent_irqs);
+ data->base[0], num_parent_irqs);

return 0;

out_free_domain:
irq_domain_remove(data->domain);
out_unmap:
- iounmap(data->base);
-out_free:
+ for (idx = 0; idx < MAX_WORDS; idx++) {
+ if (data->base[idx])
+ iounmap(data->base[idx]);
+ }
kfree(data);
return ret;
}
--
2.1.1

2014-11-07 06:46:34

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 14/14] irqchip: brcmstb-l2: Convert driver to use irq_reg_{readl,writel}

This effectively converts the __raw_ accessors to the non-__raw_
equivalents. To handle BE, we pass IRQ_GC_BE_IO, similar to what was
done in irq-bcm7120-l2.c.

Since irq_reg_writel now takes an irq_chip_generic argument, writel must
be used for the initial hardware reset in the probe function. But that
operation never needs endian swapping, so it's probably not a big deal.

Signed-off-by: Kevin Cernekee <[email protected]>
---
drivers/irqchip/irq-brcmstb-l2.c | 34 ++++++++++++++++++++++------------
1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/irqchip/irq-brcmstb-l2.c b/drivers/irqchip/irq-brcmstb-l2.c
index c9bdf20..4aa653a 100644
--- a/drivers/irqchip/irq-brcmstb-l2.c
+++ b/drivers/irqchip/irq-brcmstb-l2.c
@@ -18,6 +18,7 @@
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/module.h>
+#include <linux/kconfig.h>
#include <linux/platform_device.h>
#include <linux/spinlock.h>
#include <linux/of.h>
@@ -53,13 +54,14 @@ struct brcmstb_l2_intc_data {
static void brcmstb_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
{
struct brcmstb_l2_intc_data *b = irq_desc_get_handler_data(desc);
+ struct irq_chip_generic *gc = irq_get_domain_generic_chip(b->domain, 0);
struct irq_chip *chip = irq_desc_get_chip(desc);
u32 status;

chained_irq_enter(chip, desc);

- status = __raw_readl(b->base + CPU_STATUS) &
- ~(__raw_readl(b->base + CPU_MASK_STATUS));
+ status = irq_reg_readl(gc, CPU_STATUS) &
+ ~(irq_reg_readl(gc, CPU_MASK_STATUS));

if (status == 0) {
raw_spin_lock(&desc->lock);
@@ -71,7 +73,7 @@ static void brcmstb_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
do {
irq = ffs(status) - 1;
/* ack at our level */
- __raw_writel(1 << irq, b->base + CPU_CLEAR);
+ irq_reg_writel(gc, 1 << irq, CPU_CLEAR);
status &= ~(1 << irq);
generic_handle_irq(irq_find_mapping(b->domain, irq));
} while (status);
@@ -86,12 +88,12 @@ static void brcmstb_l2_intc_suspend(struct irq_data *d)

irq_gc_lock(gc);
/* Save the current mask */
- b->saved_mask = __raw_readl(b->base + CPU_MASK_STATUS);
+ b->saved_mask = irq_reg_readl(gc, CPU_MASK_STATUS);

if (b->can_wake) {
/* Program the wakeup mask */
- __raw_writel(~gc->wake_active, b->base + CPU_MASK_SET);
- __raw_writel(gc->wake_active, b->base + CPU_MASK_CLEAR);
+ irq_reg_writel(gc, ~gc->wake_active, CPU_MASK_SET);
+ irq_reg_writel(gc, gc->wake_active, CPU_MASK_CLEAR);
}
irq_gc_unlock(gc);
}
@@ -103,11 +105,11 @@ static void brcmstb_l2_intc_resume(struct irq_data *d)

irq_gc_lock(gc);
/* Clear unmasked non-wakeup interrupts */
- __raw_writel(~b->saved_mask & ~gc->wake_active, b->base + CPU_CLEAR);
+ irq_reg_writel(gc, ~b->saved_mask & ~gc->wake_active, CPU_CLEAR);

/* Restore the saved mask */
- __raw_writel(b->saved_mask, b->base + CPU_MASK_SET);
- __raw_writel(~b->saved_mask, b->base + CPU_MASK_CLEAR);
+ irq_reg_writel(gc, b->saved_mask, CPU_MASK_SET);
+ irq_reg_writel(gc, ~b->saved_mask, CPU_MASK_CLEAR);
irq_gc_unlock(gc);
}

@@ -119,6 +121,7 @@ int __init brcmstb_l2_intc_of_init(struct device_node *np,
struct irq_chip_generic *gc;
struct irq_chip_type *ct;
int ret;
+ unsigned int flags;

data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
@@ -132,8 +135,8 @@ int __init brcmstb_l2_intc_of_init(struct device_node *np,
}

/* Disable all interrupts by default */
- __raw_writel(0xffffffff, data->base + CPU_MASK_SET);
- __raw_writel(0xffffffff, data->base + CPU_CLEAR);
+ writel(0xffffffff, data->base + CPU_MASK_SET);
+ writel(0xffffffff, data->base + CPU_CLEAR);

data->parent_irq = irq_of_parse_and_map(np, 0);
if (data->parent_irq < 0) {
@@ -149,9 +152,16 @@ int __init brcmstb_l2_intc_of_init(struct device_node *np,
goto out_unmap;
}

+ /* MIPS chips strapped for BE will automagically configure the
+ * peripheral registers for CPU-native byte order.
+ */
+ flags = 0;
+ if (IS_ENABLED(CONFIG_MIPS) && IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+ flags |= IRQ_GC_BE_IO;
+
/* Allocate a single Generic IRQ chip for this node */
ret = irq_alloc_domain_generic_chips(data->domain, 32, 1,
- np->full_name, handle_edge_irq, clr, 0, 0);
+ np->full_name, handle_edge_irq, clr, 0, flags);
if (ret) {
pr_err("failed to allocate generic irq chip\n");
goto out_free_domain;
--
2.1.1

2014-11-07 06:45:18

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 05/14] irqchip: brcmstb-l2: Eliminate dependency on ARM code

The irq-brcmstb-l2 driver has a single dependency on the ARM code, the
do_bad_IRQ macro. Expand this macro in-place so that the driver can be
built on non-ARM platforms.

Signed-off-by: Kevin Cernekee <[email protected]>
Acked-by: Arnd Bergmann <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
---
drivers/irqchip/irq-brcmstb-l2.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-brcmstb-l2.c b/drivers/irqchip/irq-brcmstb-l2.c
index c15c840..c9bdf20 100644
--- a/drivers/irqchip/irq-brcmstb-l2.c
+++ b/drivers/irqchip/irq-brcmstb-l2.c
@@ -19,6 +19,7 @@
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/spinlock.h>
#include <linux/of.h>
#include <linux/of_irq.h>
#include <linux/of_address.h>
@@ -30,8 +31,6 @@
#include <linux/irqchip.h>
#include <linux/irqchip/chained_irq.h>

-#include <asm/mach/irq.h>
-
#include "irqchip.h"

/* Register offsets in the L2 interrupt controller */
@@ -63,7 +62,9 @@ static void brcmstb_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
~(__raw_readl(b->base + CPU_MASK_STATUS));

if (status == 0) {
- do_bad_IRQ(irq, desc);
+ raw_spin_lock(&desc->lock);
+ handle_bad_irq(irq, desc);
+ raw_spin_unlock(&desc->lock);
goto out;
}

--
2.1.1

2014-11-07 06:46:55

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 12/14] irqchip: bcm7120-l2: Decouple driver from brcmstb-l2

Some chips, such as BCM6328, only require bcm7120-l2. Some BCM7xxx STB
configurations only require brcmstb-l2. Treat them as two separate
entities, and update the mach-bcm dependencies to reflect the change.

Signed-off-by: Kevin Cernekee <[email protected]>
Acked-by: Arnd Bergmann <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
---
arch/arm/mach-bcm/Kconfig | 1 +
drivers/irqchip/Kconfig | 5 +++++
drivers/irqchip/Makefile | 4 ++--
drivers/irqchip/irq-bcm7120-l2.c | 2 +-
4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index 2abad74..bf47eb0 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -125,6 +125,7 @@ config ARCH_BRCMSTB
select HAVE_ARM_ARCH_TIMER
select BRCMSTB_GISB_ARB
select BRCMSTB_L2_IRQ
+ select BCM7120_L2_IRQ
help
Say Y if you intend to run the kernel on a Broadcom ARM-based STB
chipset.
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 09c79d1..afdc1f3 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -48,6 +48,11 @@ config ATMEL_AIC5_IRQ
select MULTI_IRQ_HANDLER
select SPARSE_IRQ

+config BCM7120_L2_IRQ
+ bool
+ select GENERIC_IRQ_CHIP
+ select IRQ_DOMAIN
+
config BRCMSTB_L2_IRQ
bool
select GENERIC_IRQ_CHIP
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 173bb5f..f0909d0 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -35,6 +35,6 @@ obj-$(CONFIG_TB10X_IRQC) += irq-tb10x.o
obj-$(CONFIG_XTENSA) += irq-xtensa-pic.o
obj-$(CONFIG_XTENSA_MX) += irq-xtensa-mx.o
obj-$(CONFIG_IRQ_CROSSBAR) += irq-crossbar.o
-obj-$(CONFIG_BRCMSTB_L2_IRQ) += irq-brcmstb-l2.o \
- irq-bcm7120-l2.o
+obj-$(CONFIG_BCM7120_L2_IRQ) += irq-bcm7120-l2.o
+obj-$(CONFIG_BRCMSTB_L2_IRQ) += irq-brcmstb-l2.o
obj-$(CONFIG_KEYSTONE_IRQ) += irq-keystone.o
diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
index ef4d32c..e53a3a6 100644
--- a/drivers/irqchip/irq-bcm7120-l2.c
+++ b/drivers/irqchip/irq-bcm7120-l2.c
@@ -247,5 +247,5 @@ out_unmap:
kfree(data);
return ret;
}
-IRQCHIP_DECLARE(brcmstb_l2_intc, "brcm,bcm7120-l2-intc",
+IRQCHIP_DECLARE(bcm7120_l2_intc, "brcm,bcm7120-l2-intc",
bcm7120_l2_intc_of_init);
--
2.1.1

2014-11-07 06:47:22

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 09/14] irqchip: bcm7120-l2: Fix missing nibble in gc->unused mask

This mask should have been 0xffff_ffff, not 0x0fff_ffff.

The change should not have an effect on current users (STB) because bits
31:27 are unused.

Signed-off-by: Kevin Cernekee <[email protected]>
Acked-by: Arnd Bergmann <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
---
drivers/irqchip/irq-bcm7120-l2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
index 22d3fa1..b70679f8 100644
--- a/drivers/irqchip/irq-bcm7120-l2.c
+++ b/drivers/irqchip/irq-bcm7120-l2.c
@@ -171,7 +171,7 @@ int __init bcm7120_l2_intc_of_init(struct device_node *dn,
}

gc = irq_get_domain_generic_chip(data->domain, 0);
- gc->unused = 0xfffffff & ~data->irq_map_mask;
+ gc->unused = 0xffffffff & ~data->irq_map_mask;
gc->reg_base = data->base;
gc->private = data;
ct = gc->chip_types;
--
2.1.1

2014-11-07 06:47:42

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 08/14] irqchip: bcm7120-l2: Make sure all register accesses use base+offset

A couple of accesses to IRQEN (base+0x00) just used "base" directly, so
they would break if IRQEN ever became nonzero. Make sure that all
reads/writes specify the register offset constant.

Signed-off-by: Kevin Cernekee <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
---
drivers/irqchip/irq-bcm7120-l2.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
index 7086fe0..22d3fa1 100644
--- a/drivers/irqchip/irq-bcm7120-l2.c
+++ b/drivers/irqchip/irq-bcm7120-l2.c
@@ -66,10 +66,10 @@ static void bcm7120_l2_intc_suspend(struct irq_data *d)

irq_gc_lock(gc);
/* Save the current mask and the interrupt forward mask */
- b->saved_mask = __raw_readl(b->base) | b->irq_fwd_mask;
+ b->saved_mask = __raw_readl(b->base + IRQEN) | b->irq_fwd_mask;
if (b->can_wake) {
reg = b->saved_mask | gc->wake_active;
- __raw_writel(reg, b->base);
+ __raw_writel(reg, b->base + IRQEN);
}
irq_gc_unlock(gc);
}
@@ -81,7 +81,7 @@ static void bcm7120_l2_intc_resume(struct irq_data *d)

/* Restore the saved mask */
irq_gc_lock(gc);
- __raw_writel(b->saved_mask, b->base);
+ __raw_writel(b->saved_mask, b->base + IRQEN);
irq_gc_unlock(gc);
}

--
2.1.1

2014-11-07 06:48:08

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 03/14] genirq: Generic chip: Allow irqchip drivers to override irq_reg_{readl,writel}

Currently, these I/O accessors always assume little endian 32-bit
registers (readl/writel). On some systems the IRQ registers need to be
accessed in BE mode or using 16-bit loads/stores, so we will provide a
way to override the default behavior.

Signed-off-by: Kevin Cernekee <[email protected]>
---
include/linux/irq.h | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index ed1135d..0fecd95 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -686,6 +686,8 @@ struct irq_chip_type {
* struct irq_chip_generic - Generic irq chip data structure
* @lock: Lock to protect register and cache data access
* @reg_base: Register base address (virtual)
+ * @reg_readl: Alternate I/O accessor (defaults to readl if NULL)
+ * @reg_writel: Alternate I/O accessor (defaults to writel if NULL)
* @irq_base: Interrupt base nr for this chip
* @irq_cnt: Number of interrupts handled by this chip
* @mask_cache: Cached mask register shared between all chip types
@@ -710,6 +712,8 @@ struct irq_chip_type {
struct irq_chip_generic {
raw_spinlock_t lock;
void __iomem *reg_base;
+ u32 (*reg_readl)(void __iomem *addr);
+ void (*reg_writel)(u32 val, void __iomem *addr);
unsigned int irq_base;
unsigned int irq_cnt;
u32 mask_cache;
@@ -818,13 +822,19 @@ static inline void irq_gc_unlock(struct irq_chip_generic *gc) { }
static inline void irq_reg_writel(struct irq_chip_generic *gc,
u32 val, int reg_offset)
{
- writel(val, gc->reg_base + reg_offset);
+ if (gc->reg_writel)
+ gc->reg_writel(val, gc->reg_base + reg_offset);
+ else
+ writel(val, gc->reg_base + reg_offset);
}

static inline u32 irq_reg_readl(struct irq_chip_generic *gc,
int reg_offset)
{
- return readl(gc->reg_base + reg_offset);
+ if (gc->reg_readl)
+ return gc->reg_readl(gc->reg_base + reg_offset);
+ else
+ return readl(gc->reg_base + reg_offset);
}

#endif /* _LINUX_IRQ_H */
--
2.1.1

2014-11-07 06:48:05

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 04/14] genirq: Generic chip: Add big endian I/O accessors

Use io{read,write}32be if the caller specified IRQ_GC_BE_IO when creating
the irqchip.

Signed-off-by: Kevin Cernekee <[email protected]>
---
include/linux/irq.h | 2 ++
kernel/irq/generic-chip.c | 16 ++++++++++++++++
2 files changed, 18 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 0fecd95..8588e5e 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -738,12 +738,14 @@ struct irq_chip_generic {
* the parent irq. Usually GPIO implementations
* @IRQ_GC_MASK_CACHE_PER_TYPE: Mask cache is chip type private
* @IRQ_GC_NO_MASK: Do not calculate irq_data->mask
+ * @IRQ_GC_BE_IO: Use big-endian register accesses (default: LE)
*/
enum irq_gc_flags {
IRQ_GC_INIT_MASK_CACHE = 1 << 0,
IRQ_GC_INIT_NESTED_LOCK = 1 << 1,
IRQ_GC_MASK_CACHE_PER_TYPE = 1 << 2,
IRQ_GC_NO_MASK = 1 << 3,
+ IRQ_GC_BE_IO = 1 << 4,
};

/*
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index db458c6..61024e8 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -191,6 +191,16 @@ int irq_gc_set_wake(struct irq_data *d, unsigned int on)
return 0;
}

+static u32 irq_readl_be(void __iomem *addr)
+{
+ return ioread32be(addr);
+}
+
+static void irq_writel_be(u32 val, void __iomem *addr)
+{
+ iowrite32be(val, addr);
+}
+
static void
irq_init_generic_chip(struct irq_chip_generic *gc, const char *name,
int num_ct, unsigned int irq_base,
@@ -300,7 +310,13 @@ int irq_alloc_domain_generic_chips(struct irq_domain *d, int irqs_per_chip,
dgc->gc[i] = gc = tmp;
irq_init_generic_chip(gc, name, num_ct, i * irqs_per_chip,
NULL, handler);
+
gc->domain = d;
+ if (gcflags & IRQ_GC_BE_IO) {
+ gc->reg_readl = &irq_readl_be;
+ gc->reg_writel = &irq_writel_be;
+ }
+
raw_spin_lock_irqsave(&gc_lock, flags);
list_add_tail(&gc->list, &gc_list);
raw_spin_unlock_irqrestore(&gc_lock, flags);
--
2.1.1

2014-11-07 06:49:04

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH V4 01/14] sh: Eliminate unused irq_reg_{readl,writel} accessors

Defining these macros way down in arch/sh/.../irq.c doesn't cause
kernel/irq/generic-chip.c to use them. As far as I can tell this code
has no effect.

Signed-off-by: Kevin Cernekee <[email protected]>
---
arch/sh/boards/mach-se/7343/irq.c | 3 ---
arch/sh/boards/mach-se/7722/irq.c | 3 ---
2 files changed, 6 deletions(-)

diff --git a/arch/sh/boards/mach-se/7343/irq.c b/arch/sh/boards/mach-se/7343/irq.c
index 7646bf0..1087dba 100644
--- a/arch/sh/boards/mach-se/7343/irq.c
+++ b/arch/sh/boards/mach-se/7343/irq.c
@@ -14,9 +14,6 @@
#define DRV_NAME "SE7343-FPGA"
#define pr_fmt(fmt) DRV_NAME ": " fmt

-#define irq_reg_readl ioread16
-#define irq_reg_writel iowrite16
-
#include <linux/init.h>
#include <linux/irq.h>
#include <linux/interrupt.h>
diff --git a/arch/sh/boards/mach-se/7722/irq.c b/arch/sh/boards/mach-se/7722/irq.c
index f5e2af1..00e6992 100644
--- a/arch/sh/boards/mach-se/7722/irq.c
+++ b/arch/sh/boards/mach-se/7722/irq.c
@@ -11,9 +11,6 @@
#define DRV_NAME "SE7722-FPGA"
#define pr_fmt(fmt) DRV_NAME ": " fmt

-#define irq_reg_readl ioread16
-#define irq_reg_writel iowrite16
-
#include <linux/init.h>
#include <linux/irq.h>
#include <linux/interrupt.h>
--
2.1.1

2014-11-07 12:28:04

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH V4 00/14] genirq endian fixes; bcm7120/brcmstb IRQ updates

Thomas,

On Thu, Nov 06, 2014 at 10:44:15PM -0800, Kevin Cernekee wrote:
> V3->V4:
>
> - Fix buildbot bisectability warning on patch 02/14 (missing include)

Dammit. :( Even if I had created a topic branch for this series, I
still would have put the genirq changes in /core, then based a brcm
branch off of that.

I'm inclined to think I should just drop v3 and apply v4, as a revert
commit wouldn't solve the bisectability issue.

thoughts?

thx,

Jason.

>
> - Add kernel-doc text for the new reg_{readl,writel} struct members and
> IRQ_GC_BE_IO flag
>
>
> Kevin Cernekee (14):
> sh: Eliminate unused irq_reg_{readl,writel} accessors
> genirq: Generic chip: Change irq_reg_{readl,writel} arguments
> genirq: Generic chip: Allow irqchip drivers to override
> irq_reg_{readl,writel}
> genirq: Generic chip: Add big endian I/O accessors
> irqchip: brcmstb-l2: Eliminate dependency on ARM code
> irqchip: bcm7120-l2: Eliminate bad IRQ check
> irqchip: bcm7120-l2, brcmstb-l2: Remove ARM Kconfig dependency
> irqchip: bcm7120-l2: Make sure all register accesses use base+offset
> irqchip: bcm7120-l2: Fix missing nibble in gc->unused mask
> irqchip: bcm7120-l2: Use gc->mask_cache to simplify suspend/resume
> functions
> irqchip: bcm7120-l2: Extend driver to support 64+ bit controllers
> irqchip: bcm7120-l2: Decouple driver from brcmstb-l2
> irqchip: bcm7120-l2: Convert driver to use irq_reg_{readl,writel}
> irqchip: brcmstb-l2: Convert driver to use irq_reg_{readl,writel}
>
> .../interrupt-controller/brcm,bcm7120-l2-intc.txt | 26 ++-
> arch/arm/mach-bcm/Kconfig | 1 +
> arch/sh/boards/mach-se/7343/irq.c | 3 -
> arch/sh/boards/mach-se/7722/irq.c | 3 -
> drivers/irqchip/Kconfig | 6 +-
> drivers/irqchip/Makefile | 4 +-
> drivers/irqchip/irq-atmel-aic.c | 40 ++---
> drivers/irqchip/irq-atmel-aic5.c | 65 ++++----
> drivers/irqchip/irq-bcm7120-l2.c | 174 +++++++++++++--------
> drivers/irqchip/irq-brcmstb-l2.c | 41 +++--
> drivers/irqchip/irq-sunxi-nmi.c | 4 +-
> drivers/irqchip/irq-tb10x.c | 4 +-
> include/linux/irq.h | 32 +++-
> kernel/irq/generic-chip.c | 36 +++--
> 14 files changed, 263 insertions(+), 176 deletions(-)
>
> --
> 2.1.1
>

2014-11-09 04:30:26

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH V4 00/14] genirq endian fixes; bcm7120/brcmstb IRQ updates

On Fri, Nov 07, 2014 at 07:27:45AM -0500, Jason Cooper wrote:
> Thomas,
>
> On Thu, Nov 06, 2014 at 10:44:15PM -0800, Kevin Cernekee wrote:
> > V3->V4:
> >
> > - Fix buildbot bisectability warning on patch 02/14 (missing include)
>
> Dammit. :( Even if I had created a topic branch for this series, I
> still would have put the genirq changes in /core, then based a brcm
> branch off of that.
>
> I'm inclined to think I should just drop v3 and apply v4, as a revert
> commit wouldn't solve the bisectability issue.

Ok, no complaints, so it's done.

thx,

Jason.

2014-11-10 08:13:53

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH V4 01/14] sh: Eliminate unused irq_reg_{readl,writel} accessors

On Fri, Nov 7, 2014 at 7:44 AM, Kevin Cernekee <[email protected]> wrote:
> Defining these macros way down in arch/sh/.../irq.c doesn't cause
> kernel/irq/generic-chip.c to use them. As far as I can tell this code
> has no effect.
>
> Signed-off-by: Kevin Cernekee <[email protected]>

Compared preprocessor and asm output before and after, no difference
except for line numbers, so

Tested-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2014-11-10 22:00:09

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH V4 04/14] genirq: Generic chip: Add big endian I/O accessors

Kevin Cernekee <[email protected]> writes:

> Use io{read,write}32be if the caller specified IRQ_GC_BE_IO when creating
> the irqchip.
>
> Signed-off-by: Kevin Cernekee <[email protected]>

I bisected a couple ARM boot failures in next-20141110 on atmel sama5 platforms down to
this patch, though I'm not quite yet sure how it's causing the failure.
I'm not getting any console output, so haven't been able to dig deeper
yet. Maybe the atmel maintainers (Cc'd) can help dig.

I've confirmed that reverting $SUBJECT patch (commit
b79055952badbd73710685643bab44104f2509ea2) on top of next-20141110 gets
things booting again.

Also, it only happens with sama5_defconfig, not with multi_v7_defconfig.

Kevin

2014-11-10 22:12:10

by Kevin Cernekee

[permalink] [raw]
Subject: Re: [PATCH V4 04/14] genirq: Generic chip: Add big endian I/O accessors

On Mon, Nov 10, 2014 at 2:00 PM, Kevin Hilman <[email protected]> wrote:
> Kevin Cernekee <[email protected]> writes:
>
>> Use io{read,write}32be if the caller specified IRQ_GC_BE_IO when creating
>> the irqchip.
>>
>> Signed-off-by: Kevin Cernekee <[email protected]>
>
> I bisected a couple ARM boot failures in next-20141110 on atmel sama5 platforms down to
> this patch, though I'm not quite yet sure how it's causing the failure.
> I'm not getting any console output, so haven't been able to dig deeper
> yet. Maybe the atmel maintainers (Cc'd) can help dig.
>
> I've confirmed that reverting $SUBJECT patch (commit
> b79055952badbd73710685643bab44104f2509ea2) on top of next-20141110 gets
> things booting again.
>
> Also, it only happens with sama5_defconfig, not with multi_v7_defconfig.

In drivers/irqchip/irq-atmel-aic-common.c I see:

ret = irq_alloc_domain_generic_chips(domain, 32, 1, name,
handle_level_irq, 0, 0,
IRQCHIP_SKIP_SET_WAKE);

and IRQCHIP_SKIP_SET_WAKE is (1 << 4), same as IRQ_GC_BE_IO.

Is it possible that the caller is passing values intended for
irq_chip->flags into a function expecting
irq_domain_chip_generic->gc_flags ?

2014-11-10 23:03:07

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH V4 04/14] genirq: Generic chip: Add big endian I/O accessors

Adding Boris in Cc: as he wrote that part.

On 10/11/2014 at 14:11:44 -0800, Kevin Cernekee wrote :
> On Mon, Nov 10, 2014 at 2:00 PM, Kevin Hilman <[email protected]> wrote:
> > Kevin Cernekee <[email protected]> writes:
> >
> >> Use io{read,write}32be if the caller specified IRQ_GC_BE_IO when creating
> >> the irqchip.
> >>
> >> Signed-off-by: Kevin Cernekee <[email protected]>
> >
> > I bisected a couple ARM boot failures in next-20141110 on atmel sama5 platforms down to
> > this patch, though I'm not quite yet sure how it's causing the failure.
> > I'm not getting any console output, so haven't been able to dig deeper
> > yet. Maybe the atmel maintainers (Cc'd) can help dig.
> >
> > I've confirmed that reverting $SUBJECT patch (commit
> > b79055952badbd73710685643bab44104f2509ea2) on top of next-20141110 gets
> > things booting again.
> >
> > Also, it only happens with sama5_defconfig, not with multi_v7_defconfig.
>
> In drivers/irqchip/irq-atmel-aic-common.c I see:
>
> ret = irq_alloc_domain_generic_chips(domain, 32, 1, name,
> handle_level_irq, 0, 0,
> IRQCHIP_SKIP_SET_WAKE);
>
> and IRQCHIP_SKIP_SET_WAKE is (1 << 4), same as IRQ_GC_BE_IO.
>
> Is it possible that the caller is passing values intended for
> irq_chip->flags into a function expecting
> irq_domain_chip_generic->gc_flags ?

--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-11-11 10:03:16

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH V4 04/14] genirq: Generic chip: Add big endian I/O accessors

Hi,

On Tue, 11 Nov 2014 00:03:01 +0100
Alexandre Belloni <[email protected]> wrote:

> Adding Boris in Cc: as he wrote that part.

Thanks for putting me in the loop.

>
> On 10/11/2014 at 14:11:44 -0800, Kevin Cernekee wrote :
> > On Mon, Nov 10, 2014 at 2:00 PM, Kevin Hilman <[email protected]> wrote:
> > > Kevin Cernekee <[email protected]> writes:
> > >
> > >> Use io{read,write}32be if the caller specified IRQ_GC_BE_IO when creating
> > >> the irqchip.
> > >>
> > >> Signed-off-by: Kevin Cernekee <[email protected]>
> > >
> > > I bisected a couple ARM boot failures in next-20141110 on atmel sama5 platforms down to
> > > this patch, though I'm not quite yet sure how it's causing the failure.
> > > I'm not getting any console output, so haven't been able to dig deeper
> > > yet. Maybe the atmel maintainers (Cc'd) can help dig.
> > >
> > > I've confirmed that reverting $SUBJECT patch (commit
> > > b79055952badbd73710685643bab44104f2509ea2) on top of next-20141110 gets
> > > things booting again.
> > >
> > > Also, it only happens with sama5_defconfig, not with multi_v7_defconfig.
> >
> > In drivers/irqchip/irq-atmel-aic-common.c I see:
> >
> > ret = irq_alloc_domain_generic_chips(domain, 32, 1, name,
> > handle_level_irq, 0, 0,
> > IRQCHIP_SKIP_SET_WAKE);
> >
> > and IRQCHIP_SKIP_SET_WAKE is (1 << 4), same as IRQ_GC_BE_IO.
> >
> > Is it possible that the caller is passing values intended for
> > irq_chip->flags into a function expecting
> > irq_domain_chip_generic->gc_flags ?

Indeed, I don't know what I tried to do in the first place but this is
completely wrong.
First because the last argument is not a valid flag as you pointed out,
then because I clearly have set irq_set_wake and thus setting
IRQCHIP_SKIP_SET_WAKE makes no sense.

I also realized I should directly pass handle_fasteoi_irq and not
handle_level_irq for the handler, that clr flags (IRQ_NOREQUEST |
IRQ_NOPROBE | IRQ_NOAUTOEN) are missing and that IRQ_GC_INIT_MASK_CACHE
is missing too.

I'll propose a patch fixing all those bugs.

Sorry for the inconvenience :-(.

Regards,

Boris

--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

2014-11-11 13:34:06

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH] irqchip: atmel-aic: fix irqdomain initialization

First of all IRQCHIP_SKIP_SET_WAKE is not a valid irq_gc_flags and thus
should not be passed as the last argument of
irq_alloc_domain_generic_chips.

Then pass the correct handler (handle_fasteoi_irq) to
irq_alloc_domain_generic_chips instead of manually re-setting it in the
initialization loop.

And eventually initialize default irq flags to the pseudo standard:
IRQ_REQUEST | IRQ_PROBE | IRQ_AUTOEN.

Signed-off-by: Boris Brezillon <[email protected]>
---
Hello Kevin,

This patch has not been tested yet but it should solve the issue you've
experienced with the IRQ_GC_BE_IO flag and the atmel-aic driver.

I'll test it tomorrow and let you know if it actually works.

Regards,

Boris

drivers/irqchip/irq-atmel-aic-common.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index 656cfe3..d111ac7 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -243,8 +243,9 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
}

ret = irq_alloc_domain_generic_chips(domain, 32, 1, name,
- handle_level_irq, 0, 0,
- IRQCHIP_SKIP_SET_WAKE);
+ handle_fasteoi_irq,
+ IRQ_NOREQUEST | IRQ_NOPROBE |
+ IRQ_NOAUTOEN, 0, 0);
if (ret)
goto err_domain_remove;

@@ -256,7 +257,6 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
gc->unused = 0;
gc->wake_enabled = ~0;
gc->chip_types[0].type = IRQ_TYPE_SENSE_MASK;
- gc->chip_types[0].handler = handle_fasteoi_irq;
gc->chip_types[0].chip.irq_eoi = irq_gc_eoi;
gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
gc->chip_types[0].chip.irq_shutdown = aic_common_shutdown;
--
1.9.1

2014-11-11 15:45:44

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH] irqchip: atmel-aic: fix irqdomain initialization

Boris Brezillon <[email protected]> writes:

> First of all IRQCHIP_SKIP_SET_WAKE is not a valid irq_gc_flags and thus
> should not be passed as the last argument of
> irq_alloc_domain_generic_chips.
>
> Then pass the correct handler (handle_fasteoi_irq) to
> irq_alloc_domain_generic_chips instead of manually re-setting it in the
> initialization loop.
>
> And eventually initialize default irq flags to the pseudo standard:
> IRQ_REQUEST | IRQ_PROBE | IRQ_AUTOEN.
>
> Signed-off-by: Boris Brezillon <[email protected]>

> ---
> Hello Kevin,
>
> This patch has not been tested yet but it should solve the issue you've
> experienced with the IRQ_GC_BE_IO flag and the atmel-aic driver.
>
> I'll test it tomorrow and let you know if it actually works.

Well, it at least boots on my xplained and my sama5d35-ek:

Tested-by: Kevin Hilman <[email protected]>

I'll let Jason pick this up as a fix through his tree.

Kevin

2014-11-11 22:58:21

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH] irqchip: atmel-aic: fix irqdomain initialization

On Tue, Nov 11, 2014 at 02:33:36PM +0100, Boris Brezillon wrote:
> First of all IRQCHIP_SKIP_SET_WAKE is not a valid irq_gc_flags and thus
> should not be passed as the last argument of
> irq_alloc_domain_generic_chips.
>
> Then pass the correct handler (handle_fasteoi_irq) to
> irq_alloc_domain_generic_chips instead of manually re-setting it in the
> initialization loop.
>
> And eventually initialize default irq flags to the pseudo standard:
> IRQ_REQUEST | IRQ_PROBE | IRQ_AUTOEN.
>
> Signed-off-by: Boris Brezillon <[email protected]>
> ---
> Hello Kevin,
>
> This patch has not been tested yet but it should solve the issue you've
> experienced with the IRQ_GC_BE_IO flag and the atmel-aic driver.
>
> I'll test it tomorrow and let you know if it actually works.
>
> Regards,
>
> Boris
>
> drivers/irqchip/irq-atmel-aic-common.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)

Applied to irqchip/urgent with Kevin's Tested-by. Also, flagged for
stable, v3.17 and up.

Boris, once this is in mainline, you may want to generate a patch
against older versions (in the arch dir) for the stable team.

thx,

Jason.

2014-11-12 09:43:49

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH] irqchip: atmel-aic: fix irqdomain initialization

Hi Jason,

On Tue, 11 Nov 2014 17:58:00 -0500
Jason Cooper <[email protected]> wrote:

> On Tue, Nov 11, 2014 at 02:33:36PM +0100, Boris Brezillon wrote:
> > First of all IRQCHIP_SKIP_SET_WAKE is not a valid irq_gc_flags and thus
> > should not be passed as the last argument of
> > irq_alloc_domain_generic_chips.
> >
> > Then pass the correct handler (handle_fasteoi_irq) to
> > irq_alloc_domain_generic_chips instead of manually re-setting it in the
> > initialization loop.
> >
> > And eventually initialize default irq flags to the pseudo standard:
> > IRQ_REQUEST | IRQ_PROBE | IRQ_AUTOEN.
> >
> > Signed-off-by: Boris Brezillon <[email protected]>
> > ---
> > Hello Kevin,
> >
> > This patch has not been tested yet but it should solve the issue you've
> > experienced with the IRQ_GC_BE_IO flag and the atmel-aic driver.
> >
> > I'll test it tomorrow and let you know if it actually works.

Tested it, and it seems to work.

> >
> > Regards,
> >
> > Boris
> >
> > drivers/irqchip/irq-atmel-aic-common.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
>
> Applied to irqchip/urgent with Kevin's Tested-by. Also, flagged for
> stable, v3.17 and up.

Thanks!

>
> Boris, once this is in mainline, you may want to generate a patch
> against older versions (in the arch dir) for the stable team.

Actually, the old irq driver does not use the generic irqchip
infrastructure and thus is not impacted by this bug.

Regards,

Boris

--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

2014-11-14 16:38:54

by Ralf Baechle

[permalink] [raw]
Subject: Re: [PATCH V4 01/14] sh: Eliminate unused irq_reg_{readl,writel} accessors

On Mon, Nov 10, 2014 at 09:13:49AM +0100, Geert Uytterhoeven wrote:

> On Fri, Nov 7, 2014 at 7:44 AM, Kevin Cernekee <[email protected]> wrote:
> > Defining these macros way down in arch/sh/.../irq.c doesn't cause
> > kernel/irq/generic-chip.c to use them. As far as I can tell this code
> > has no effect.
> >
> > Signed-off-by: Kevin Cernekee <[email protected]>
>
> Compared preprocessor and asm output before and after, no difference
> except for line numbers, so
>
> Tested-by: Geert Uytterhoeven <[email protected]>

I noticed the remainder of of this series is now in Jason Cooper's irqchip
tree. Is anybody still taking care of SH? If not I can funnel it
through the MIPS tree.

Ralf

2014-11-14 17:05:45

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH V4 01/14] sh: Eliminate unused irq_reg_{readl,writel} accessors

On Fri, Nov 14, 2014 at 05:38:44PM +0100, Ralf Baechle wrote:
> On Mon, Nov 10, 2014 at 09:13:49AM +0100, Geert Uytterhoeven wrote:
>
> > On Fri, Nov 7, 2014 at 7:44 AM, Kevin Cernekee <[email protected]> wrote:
> > > Defining these macros way down in arch/sh/.../irq.c doesn't cause
> > > kernel/irq/generic-chip.c to use them. As far as I can tell this code
> > > has no effect.
> > >
> > > Signed-off-by: Kevin Cernekee <[email protected]>
> >
> > Compared preprocessor and asm output before and after, no difference
> > except for line numbers, so
> >
> > Tested-by: Geert Uytterhoeven <[email protected]>
>
> I noticed the remainder of of this series is now in Jason Cooper's irqchip
> tree. Is anybody still taking care of SH? If not I can funnel it
> through the MIPS tree.

Are the SH maintainers active? I admit I know very little about the
arch. I'm more than happy to pick it up and keep it with the rest of
the series, I just didn't want to step on toes...

thx,

Jason.

2014-11-16 09:34:49

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH V4 01/14] sh: Eliminate unused irq_reg_{readl,writel} accessors

Hi Jason,

On Fri, Nov 14, 2014 at 6:05 PM, Jason Cooper <[email protected]> wrote:
> On Fri, Nov 14, 2014 at 05:38:44PM +0100, Ralf Baechle wrote:
>> On Mon, Nov 10, 2014 at 09:13:49AM +0100, Geert Uytterhoeven wrote:
>>
>> > On Fri, Nov 7, 2014 at 7:44 AM, Kevin Cernekee <[email protected]> wrote:
>> > > Defining these macros way down in arch/sh/.../irq.c doesn't cause
>> > > kernel/irq/generic-chip.c to use them. As far as I can tell this code
>> > > has no effect.
>> > >
>> > > Signed-off-by: Kevin Cernekee <[email protected]>
>> >
>> > Compared preprocessor and asm output before and after, no difference
>> > except for line numbers, so
>> >
>> > Tested-by: Geert Uytterhoeven <[email protected]>
>>
>> I noticed the remainder of of this series is now in Jason Cooper's irqchip
>> tree. Is anybody still taking care of SH? If not I can funnel it
>> through the MIPS tree.
>
> Are the SH maintainers active? I admit I know very little about the
> arch. I'm more than happy to pick it up and keep it with the rest of
> the series, I just didn't want to step on toes...

SH is orphaned, so it would be great if you could pick it up.
Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds