[PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.

[PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.

[Varun Sethi]

All SOC device error interrupts are muxed and delivered to the core as a single
MPIC error interrupt. Currently all the device drivers requiring access to device
errors have to register for the MPIC error interrupt as a shared interrupt.

With this patch we add interrupt demuxing capability in the mpic driver, allowing
device drivers to register for their individual error interrupts. This is achieved
by handling error interrupts in a cascaded fashion.

MPIC error interrupt is handled by the "error_int_handler", which subsequently demuxes
it using the EISR and delivers it to the respective drivers. 

The error interrupt capability is dependent on the MPIC EIMR register, which was
introduced in FSL MPIC version 4.1 (P4080 rev2). So, error interrupt demuxing capability
is dependent on the MPIC version and can be used for versions >= 4.1.

Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
---
 arch/powerpc/include/asm/mpic.h |   18 +++++
 arch/powerpc/sysdev/mpic.c      |  155 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 171 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
index 3929b4b..db51015 100644
--- a/arch/powerpc/include/asm/mpic.h
+++ b/arch/powerpc/include/asm/mpic.h
@@ -114,12 +114,21 @@
 #define MPIC_FSL_BRR1			0x00000
 #define 	MPIC_FSL_BRR1_VER			0x0000ffff
 
+/*
+ * Error interrupt registers
+ */
+
+#define MPIC_ERR_INT_BASE	0x3900
+#define MPIC_ERR_INT_EISR	0x0000
+#define MPIC_ERR_INT_EIMR	0x0010
+
 #define MPIC_MAX_IRQ_SOURCES	2048
 #define MPIC_MAX_CPUS		32
 #define MPIC_MAX_ISU		32
 
 #define MPIC_MAX_TIMER    8
 #define MPIC_MAX_IPI      4
+#define MPIC_MAX_ERR      32
 
 /*
  * Tsi108 implementation of MPIC has many differences from the original one
@@ -273,6 +282,7 @@ struct mpic
 	struct irq_chip		hc_ipi;
 #endif
 	struct irq_chip		hc_tm;
+	struct irq_chip		hc_err;
 	const char		*name;
 	/* Flags */
 	unsigned int		flags;
@@ -289,6 +299,8 @@ struct mpic
 	/* vector numbers used for internal sources (ipi/timers) */
 	unsigned int		ipi_vecs[MPIC_MAX_IPI];
 	unsigned int		timer_vecs[MPIC_MAX_TIMER];
+	/* vector numbers used for FSL MPIC error interrupts */
+	unsigned int		err_int_vecs[MPIC_MAX_ERR];
 
 	/* Spurious vector to program into unused sources */
 	unsigned int		spurious_vec;
@@ -311,6 +323,10 @@ struct mpic
 	struct mpic_reg_bank	tmregs;
 	struct mpic_reg_bank	cpuregs[MPIC_MAX_CPUS];
 	struct mpic_reg_bank	isus[MPIC_MAX_ISU];
+	struct mpic_reg_bank	err_regs;
+
+	/* error interrupt config */
+	u32			err_int_config_done;
 
 	/* Protected sources */
 	unsigned long		*protected;
@@ -376,6 +392,8 @@ struct mpic
 #define MPIC_NO_RESET			0x00004000
 /* Freescale MPIC (compatible includes "fsl,mpic") */
 #define MPIC_FSL			0x00008000
+/* Freescale MPIC supports EIMR (error interrupt mask register)*/
+#define MPIC_FSL_HAS_EIMR		0x00010000
 
 /* MPIC HW modification ID */
 #define MPIC_REGSET_MASK		0xf0000000
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index c4da1d5..b0ff465 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -221,6 +221,17 @@ static inline void _mpic_ipi_write(struct mpic *mpic, unsigned int ipi, u32 valu
 	_mpic_write(mpic->reg_type, &mpic->gregs, offset, value);
 }
 
+static inline u32 _mpic_err_read(struct mpic *mpic, unsigned int err_reg)
+{
+	return _mpic_read(mpic->reg_type, &mpic->err_regs, err_reg);
+}
+
+static inline void _mpic_err_write(struct mpic *mpic, unsigned int err_reg,
+				   u32 value)
+{
+	_mpic_write(mpic->reg_type, &mpic->err_regs, err_reg, value);
+}
+
 static inline unsigned int mpic_tm_offset(struct mpic *mpic, unsigned int tm)
 {
 	return (tm >> 2) * MPIC_TIMER_GROUP_STRIDE +
@@ -295,6 +306,8 @@ static inline void _mpic_irq_write(struct mpic *mpic, unsigned int src_no,
 #define mpic_ipi_write(i,v)	_mpic_ipi_write(mpic,(i),(v))
 #define mpic_tm_read(i)		_mpic_tm_read(mpic,(i))
 #define mpic_tm_write(i,v)	_mpic_tm_write(mpic,(i),(v))
+#define mpic_err_read(i)	_mpic_err_read(mpic, (i))
+#define mpic_err_write(i, v)	_mpic_err_write(mpic, (i), (v))
 #define mpic_cpu_read(i)	_mpic_cpu_read(mpic,(i))
 #define mpic_cpu_write(i,v)	_mpic_cpu_write(mpic,(i),(v))
 #define mpic_irq_read(s,r)	_mpic_irq_read(mpic,(s),(r))
@@ -821,6 +834,86 @@ static void mpic_mask_tm(struct irq_data *d)
 	mpic_tm_read(src);
 }
 
+static void mpic_mask_err(struct irq_data *d)
+{
+	u32 eimr;
+	struct mpic *mpic = mpic_from_irq_data(d);
+	unsigned int src = virq_to_hw(d->irq) - mpic->err_int_vecs[0];
+	unsigned int err_reg_offset = MPIC_INFO(ERR_INT_EIMR);
+
+	eimr = mpic_err_read(err_reg_offset);
+	eimr |= (0x80000000 >> src);
+	mpic_err_write(err_reg_offset, eimr);
+}
+
+static void mpic_unmask_err(struct irq_data *d)
+{
+	u32 eimr;
+	struct mpic *mpic = mpic_from_irq_data(d);
+	unsigned int src = virq_to_hw(d->irq) - mpic->err_int_vecs[0];
+	unsigned int err_reg_offset = MPIC_INFO(ERR_INT_EIMR);
+
+	eimr = mpic_err_read(err_reg_offset);
+	eimr &= ~(0x80000000 >> src);
+	mpic_err_write(err_reg_offset, eimr);
+}
+
+static irqreturn_t error_int_handler(int irq, void *data)
+{
+	struct mpic *mpic = (struct mpic *) data;
+	unsigned int eisr_offset = MPIC_INFO(ERR_INT_EISR);
+	unsigned int eimr_offset = MPIC_INFO(ERR_INT_EIMR);
+	u32 eisr, eimr;
+	int errint;
+	unsigned int cascade_irq;
+
+	eisr = mpic_err_read(eisr_offset);
+	eimr = mpic_err_read(eimr_offset);
+
+	if (!(eisr & ~eimr))
+		return IRQ_NONE;
+
+	while (eisr) {
+		errint = __ffs(eisr);
+		cascade_irq = irq_linear_revmap(mpic->irqhost,
+				 mpic->err_int_vecs[31 - errint]);
+		WARN_ON(cascade_irq == NO_IRQ);
+		if (cascade_irq != NO_IRQ) {
+			generic_handle_irq(cascade_irq);
+		} else {
+			eimr |=  1 << errint;
+			mpic_err_write(eimr_offset, eimr);
+		}
+		eisr &= ~(1 << errint);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int mpic_err_int_init(struct mpic *mpic, irq_hw_number_t irqnum)
+{
+	unsigned int virq;
+	unsigned int offset = MPIC_INFO(ERR_INT_EIMR);
+	int ret;
+
+	virq = irq_create_mapping(mpic->irqhost, irqnum);
+	if (virq == NO_IRQ) {
+		pr_err("Error interrupt setup failed\n");
+		return -ENOSPC;
+	}
+
+	mpic_err_write(offset, ~0);
+
+	ret = request_irq(virq, error_int_handler, IRQF_NO_THREAD,
+		    "mpic-error-int", mpic);
+	if (ret) {
+		pr_err("Failed to register error interrupt handler\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 int mpic_set_affinity(struct irq_data *d, const struct cpumask *cpumask,
 		      bool force)
 {
@@ -947,6 +1040,12 @@ static struct irq_chip mpic_ipi_chip = {
 };
 #endif /* CONFIG_SMP */
 
+static struct irq_chip mpic_err_chip = {
+	.irq_disable	= mpic_mask_err,
+	.irq_mask	= mpic_mask_err,
+	.irq_unmask	= mpic_unmask_err,
+};
+
 static struct irq_chip mpic_tm_chip = {
 	.irq_mask	= mpic_mask_tm,
 	.irq_unmask	= mpic_unmask_tm,
@@ -984,8 +1083,19 @@ static int mpic_host_map(struct irq_host *h, unsigned int virq,
 	if (mpic->protected && test_bit(hw, mpic->protected))
 		return -EINVAL;
 
+	if ((mpic->flags & MPIC_FSL_HAS_EIMR) &&
+	    hw >= mpic->err_int_vecs[0]) {
+		WARN_ON(mpic->flags & MPIC_SECONDARY);
+
+		DBG("mpic: mapping as Error Interrupt\n");
+		irq_set_chip_data(virq, mpic);
+		irq_set_chip_and_handler(virq, &mpic->hc_err,
+					 handle_simple_irq);
+		return 0;
+	}
 #ifdef CONFIG_SMP
-	else if (hw >= mpic->ipi_vecs[0]) {
+	if (hw >= mpic->ipi_vecs[0] &&
+	    hw <= mpic->ipi_vecs[MPIC_MAX_IPI - 1]) {
 		WARN_ON(mpic->flags & MPIC_SECONDARY);
 
 		DBG("mpic: mapping as IPI\n");
@@ -1066,7 +1176,23 @@ static int mpic_host_xlate(struct irq_host *h, struct device_node *ct,
 		 */
 		switch (intspec[2]) {
 		case 0:
-		case 1: /* no EISR/EIMR support for now, treat as shared IRQ */
+			break;
+		case 1:
+			if (!(mpic->flags & MPIC_FSL_HAS_EIMR))
+				break;
+
+			if (intspec[3] >= ARRAY_SIZE(mpic->err_int_vecs))
+				return -EINVAL;
+
+			if (!mpic->err_int_config_done) {
+				int ret;
+				ret = mpic_err_int_init(mpic, intspec[0]);
+				if (ret)
+					return ret;
+				mpic->err_int_config_done = 1;
+			}
+
+			*out_hwirq = mpic->err_int_vecs[intspec[3]];
 			break;
 		case 2:
 			if (intspec[0] >= ARRAY_SIZE(mpic->ipi_vecs))
@@ -1140,6 +1266,11 @@ static void mpic_alloc_int_sources(struct mpic *mpic, int intvec_top)
 
 	intvec = intvec_top;
 
+	if (mpic->flags & MPIC_FSL_HAS_EIMR) {
+		for (i = MPIC_MAX_ERR - 1; i >= 0; i--)
+			mpic->err_int_vecs[i] = --intvec;
+	}
+
 	for (i = MPIC_MAX_IPI - 1; i >= 0; i--)
 		mpic->ipi_vecs[i] = --intvec;
 
@@ -1284,6 +1415,8 @@ struct mpic * __init mpic_alloc(struct device_node *node,
 	mpic_map(mpic, mpic->paddr, &mpic->tmregs, MPIC_INFO(TIMER_BASE), 0x1000);
 
 	if (mpic->flags & MPIC_FSL) {
+		u32 brr1, version;
+
 		/*
 		 * Yes, Freescale really did put global registers in the
 		 * magic per-cpu area -- and they don't even show up in the
@@ -1291,6 +1424,24 @@ struct mpic * __init mpic_alloc(struct device_node *node,
 		 */
 		mpic_map(mpic, mpic->paddr, &mpic->thiscpuregs,
 			 MPIC_CPU_THISBASE, 0x1000);
+
+		brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
+				MPIC_FSL_BRR1);
+		version = brr1 & MPIC_FSL_BRR1_VER;
+
+		/* Error interrupt mask register (EIMR) is required for
+		 * handling individual device error interrupts. EIMR
+		 * was added in MPIC version 4.1.
+		 */
+		if (version >= 0x401) {
+			mpic->hc_err = mpic_err_chip;
+			mpic->hc_err.name = mpic->name;
+			/* Map error interrupt registers */
+			mpic_map(mpic, mpic->paddr, &mpic->err_regs,
+				 MPIC_INFO(ERR_INT_BASE), 0x1000);
+			mpic->flags |= MPIC_FSL_HAS_EIMR;
+		}
+
 	}
 
 	/* Reset */
-- 
1.7.2.2

Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.

[Kumar Gala]

On Mar 27, 2012, at 7:17 AM, Varun Sethi wrote:

> All SOC device error interrupts are muxed and delivered to the core as =
a single
> MPIC error interrupt. Currently all the device drivers requiring =
access to device
> errors have to register for the MPIC error interrupt as a shared =
interrupt.
>=20
> With this patch we add interrupt demuxing capability in the mpic =
driver, allowing
> device drivers to register for their individual error interrupts. This =
is achieved
> by handling error interrupts in a cascaded fashion.
>=20
> MPIC error interrupt is handled by the "error_int_handler", which =
subsequently demuxes
> it using the EISR and delivers it to the respective drivers.=20
>=20
> The error interrupt capability is dependent on the MPIC EIMR register, =
which was
> introduced in FSL MPIC version 4.1 (P4080 rev2). So, error interrupt =
demuxing capability
> is dependent on the MPIC version and can be used for versions >=3D =
4.1.
>=20
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> ---

This isn't quite right.  Doing the 'request_irq' on behalf of the driver =
isn't treating the error interrupts as a real cascade, in addition to =
how you are hooking things up.

I think you need to add proper irq_domain_ops, etc.  Thus when we map =
the virq the proper thing can happen.

I'd also suggest maybe thinking about pulling this code into an =
mpic_fsl.c

> arch/powerpc/include/asm/mpic.h |   18 +++++
> arch/powerpc/sysdev/mpic.c      |  155 =
++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 171 insertions(+), 2 deletions(-)
>=20
> diff --git a/arch/powerpc/include/asm/mpic.h =
b/arch/powerpc/include/asm/mpic.h
> index 3929b4b..db51015 100644
> --- a/arch/powerpc/include/asm/mpic.h
> +++ b/arch/powerpc/include/asm/mpic.h
> @@ -114,12 +114,21 @@
> #define MPIC_FSL_BRR1			0x00000
> #define 	MPIC_FSL_BRR1_VER			0x0000ffff
>=20
> +/*
> + * Error interrupt registers
> + */
> +
> +#define MPIC_ERR_INT_BASE	0x3900
> +#define MPIC_ERR_INT_EISR	0x0000
> +#define MPIC_ERR_INT_EIMR	0x0010
> +
> #define MPIC_MAX_IRQ_SOURCES	2048
> #define MPIC_MAX_CPUS		32
> #define MPIC_MAX_ISU		32
>=20
> #define MPIC_MAX_TIMER    8
> #define MPIC_MAX_IPI      4
> +#define MPIC_MAX_ERR      32

Should probably be 64

> /*
>  * Tsi108 implementation of MPIC has many differences from the =
original one
> @@ -273,6 +282,7 @@ struct mpic
> 	struct irq_chip		hc_ipi;
> #endif
> 	struct irq_chip		hc_tm;
> +	struct irq_chip		hc_err;
> 	const char		*name;
> 	/* Flags */
> 	unsigned int		flags;
> @@ -289,6 +299,8 @@ struct mpic
> 	/* vector numbers used for internal sources (ipi/timers) */
> 	unsigned int		ipi_vecs[MPIC_MAX_IPI];
> 	unsigned int		timer_vecs[MPIC_MAX_TIMER];
> +	/* vector numbers used for FSL MPIC error interrupts */
> +	unsigned int		err_int_vecs[MPIC_MAX_ERR];
>=20
> 	/* Spurious vector to program into unused sources */
> 	unsigned int		spurious_vec;
> @@ -311,6 +323,10 @@ struct mpic
> 	struct mpic_reg_bank	tmregs;
> 	struct mpic_reg_bank	cpuregs[MPIC_MAX_CPUS];
> 	struct mpic_reg_bank	isus[MPIC_MAX_ISU];
> +	struct mpic_reg_bank	err_regs;
> +
> +	/* error interrupt config */
> +	u32			err_int_config_done;
>=20
> 	/* Protected sources */
> 	unsigned long		*protected;
> @@ -376,6 +392,8 @@ struct mpic
> #define MPIC_NO_RESET			0x00004000
> /* Freescale MPIC (compatible includes "fsl,mpic") */
> #define MPIC_FSL			0x00008000
> +/* Freescale MPIC supports EIMR (error interrupt mask register)*/
> +#define MPIC_FSL_HAS_EIMR		0x00010000
>=20
> /* MPIC HW modification ID */
> #define MPIC_REGSET_MASK		0xf0000000

> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index c4da1d5..b0ff465 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -221,6 +221,17 @@ static inline void _mpic_ipi_write(struct mpic =
*mpic, unsigned int ipi, u32 valu
> 	_mpic_write(mpic->reg_type, &mpic->gregs, offset, value);
> }
>=20
> +static inline u32 _mpic_err_read(struct mpic *mpic, unsigned int =
err_reg)
> +{
> +	return _mpic_read(mpic->reg_type, &mpic->err_regs, err_reg);
> +}
> +
> +static inline void _mpic_err_write(struct mpic *mpic, unsigned int =
err_reg,
> +				   u32 value)
> +{
> +	_mpic_write(mpic->reg_type, &mpic->err_regs, err_reg, value);
> +}
> +
> static inline unsigned int mpic_tm_offset(struct mpic *mpic, unsigned =
int tm)
> {
> 	return (tm >> 2) * MPIC_TIMER_GROUP_STRIDE +
> @@ -295,6 +306,8 @@ static inline void _mpic_irq_write(struct mpic =
*mpic, unsigned int src_no,
> #define mpic_ipi_write(i,v)	_mpic_ipi_write(mpic,(i),(v))
> #define mpic_tm_read(i)		_mpic_tm_read(mpic,(i))
> #define mpic_tm_write(i,v)	_mpic_tm_write(mpic,(i),(v))
> +#define mpic_err_read(i)	_mpic_err_read(mpic, (i))
> +#define mpic_err_write(i, v)	_mpic_err_write(mpic, (i), (v))
> #define mpic_cpu_read(i)	_mpic_cpu_read(mpic,(i))
> #define mpic_cpu_write(i,v)	_mpic_cpu_write(mpic,(i),(v))
> #define mpic_irq_read(s,r)	_mpic_irq_read(mpic,(s),(r))
> @@ -821,6 +834,86 @@ static void mpic_mask_tm(struct irq_data *d)
> 	mpic_tm_read(src);
> }
>=20
> +static void mpic_mask_err(struct irq_data *d)
> +{
> +	u32 eimr;
> +	struct mpic *mpic =3D mpic_from_irq_data(d);
> +	unsigned int src =3D virq_to_hw(d->irq) - mpic->err_int_vecs[0];
> +	unsigned int err_reg_offset =3D MPIC_INFO(ERR_INT_EIMR);
> +
> +	eimr =3D mpic_err_read(err_reg_offset);
> +	eimr |=3D (0x80000000 >> src);

This seems funny, add a comment about src # and bit # being opposite.  =
Maybe do:

	eimr |=3D (1 << (31 - src));

> +	mpic_err_write(err_reg_offset, eimr);
> +}
> +
> +static void mpic_unmask_err(struct irq_data *d)
> +{
> +	u32 eimr;
> +	struct mpic *mpic =3D mpic_from_irq_data(d);
> +	unsigned int src =3D virq_to_hw(d->irq) - mpic->err_int_vecs[0];
> +	unsigned int err_reg_offset =3D MPIC_INFO(ERR_INT_EIMR);
> +
> +	eimr =3D mpic_err_read(err_reg_offset);
> +	eimr &=3D ~(0x80000000 >> src);

same as above

> +	mpic_err_write(err_reg_offset, eimr);
> +}
> +
> +static irqreturn_t error_int_handler(int irq, void *data)
> +{
> +	struct mpic *mpic =3D (struct mpic *) data;
> +	unsigned int eisr_offset =3D MPIC_INFO(ERR_INT_EISR);
> +	unsigned int eimr_offset =3D MPIC_INFO(ERR_INT_EIMR);
> +	u32 eisr, eimr;
> +	int errint;
> +	unsigned int cascade_irq;
> +
> +	eisr =3D mpic_err_read(eisr_offset);
> +	eimr =3D mpic_err_read(eimr_offset);
> +
> +	if (!(eisr & ~eimr))
> +		return IRQ_NONE;
> +
> +	while (eisr) {
> +		errint =3D __ffs(eisr);
> +		cascade_irq =3D irq_linear_revmap(mpic->irqhost,
> +				 mpic->err_int_vecs[31 - errint]);

Should 31, be replaced w/MPIC_MAX_ERR - 1 ?

> +		WARN_ON(cascade_irq =3D=3D NO_IRQ);
> +		if (cascade_irq !=3D NO_IRQ) {
> +			generic_handle_irq(cascade_irq);
> +		} else {
> +			eimr |=3D  1 << errint;
> +			mpic_err_write(eimr_offset, eimr);
> +		}
> +		eisr &=3D ~(1 << errint);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int mpic_err_int_init(struct mpic *mpic, irq_hw_number_t =
irqnum)
> +{
> +	unsigned int virq;
> +	unsigned int offset =3D MPIC_INFO(ERR_INT_EIMR);
> +	int ret;
> +
> +	virq =3D irq_create_mapping(mpic->irqhost, irqnum);
> +	if (virq =3D=3D NO_IRQ) {
> +		pr_err("Error interrupt setup failed\n");
> +		return -ENOSPC;
> +	}
> +
> +	mpic_err_write(offset, ~0);
> +
> +	ret =3D request_irq(virq, error_int_handler, IRQF_NO_THREAD,
> +		    "mpic-error-int", mpic);
> +	if (ret) {
> +		pr_err("Failed to register error interrupt handler\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> int mpic_set_affinity(struct irq_data *d, const struct cpumask =
*cpumask,
> 		      bool force)
> {
> @@ -947,6 +1040,12 @@ static struct irq_chip mpic_ipi_chip =3D {
> };
> #endif /* CONFIG_SMP */
>=20
> +static struct irq_chip mpic_err_chip =3D {
> +	.irq_disable	=3D mpic_mask_err,
> +	.irq_mask	=3D mpic_mask_err,
> +	.irq_unmask	=3D mpic_unmask_err,
> +};
> +
> static struct irq_chip mpic_tm_chip =3D {
> 	.irq_mask	=3D mpic_mask_tm,
> 	.irq_unmask	=3D mpic_unmask_tm,
> @@ -984,8 +1083,19 @@ static int mpic_host_map(struct irq_host *h, =
unsigned int virq,
> 	if (mpic->protected && test_bit(hw, mpic->protected))
> 		return -EINVAL;
>=20
> +	if ((mpic->flags & MPIC_FSL_HAS_EIMR) &&
> +	    hw >=3D mpic->err_int_vecs[0]) {
> +		WARN_ON(mpic->flags & MPIC_SECONDARY);
> +
> +		DBG("mpic: mapping as Error Interrupt\n");
> +		irq_set_chip_data(virq, mpic);
> +		irq_set_chip_and_handler(virq, &mpic->hc_err,
> +					 handle_simple_irq);
> +		return 0;
> +	}
> #ifdef CONFIG_SMP
> -	else if (hw >=3D mpic->ipi_vecs[0]) {
> +	if (hw >=3D mpic->ipi_vecs[0] &&
> +	    hw <=3D mpic->ipi_vecs[MPIC_MAX_IPI - 1]) {
> 		WARN_ON(mpic->flags & MPIC_SECONDARY);
>=20
> 		DBG("mpic: mapping as IPI\n");
> @@ -1066,7 +1176,23 @@ static int mpic_host_xlate(struct irq_host *h, =
struct device_node *ct,
> 		 */
> 		switch (intspec[2]) {
> 		case 0:
> -		case 1: /* no EISR/EIMR support for now, treat as shared =
IRQ */
> +			break;
> +		case 1:
> +			if (!(mpic->flags & MPIC_FSL_HAS_EIMR))
> +				break;
> +
> +			if (intspec[3] >=3D =
ARRAY_SIZE(mpic->err_int_vecs))
> +				return -EINVAL;
> +
> +			if (!mpic->err_int_config_done) {
> +				int ret;
> +				ret =3D mpic_err_int_init(mpic, =
intspec[0]);
> +				if (ret)
> +					return ret;
> +				mpic->err_int_config_done =3D 1;
> +			}
> +
> +			*out_hwirq =3D mpic->err_int_vecs[intspec[3]];
> 			break;
> 		case 2:
> 			if (intspec[0] >=3D ARRAY_SIZE(mpic->ipi_vecs))
> @@ -1140,6 +1266,11 @@ static void mpic_alloc_int_sources(struct mpic =
*mpic, int intvec_top)
>=20
> 	intvec =3D intvec_top;
>=20
> +	if (mpic->flags & MPIC_FSL_HAS_EIMR) {
> +		for (i =3D MPIC_MAX_ERR - 1; i >=3D 0; i--)
> +			mpic->err_int_vecs[i] =3D --intvec;
> +	}
> +
> 	for (i =3D MPIC_MAX_IPI - 1; i >=3D 0; i--)
> 		mpic->ipi_vecs[i] =3D --intvec;
>=20
> @@ -1284,6 +1415,8 @@ struct mpic * __init mpic_alloc(struct =
device_node *node,
> 	mpic_map(mpic, mpic->paddr, &mpic->tmregs, =
MPIC_INFO(TIMER_BASE), 0x1000);
>=20
> 	if (mpic->flags & MPIC_FSL) {
> +		u32 brr1, version;
> +
> 		/*
> 		 * Yes, Freescale really did put global registers in the
> 		 * magic per-cpu area -- and they don't even show up in =
the
> @@ -1291,6 +1424,24 @@ struct mpic * __init mpic_alloc(struct =
device_node *node,
> 		 */
> 		mpic_map(mpic, mpic->paddr, &mpic->thiscpuregs,
> 			 MPIC_CPU_THISBASE, 0x1000);
> +
> +		brr1 =3D _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
> +				MPIC_FSL_BRR1);
> +		version =3D brr1 & MPIC_FSL_BRR1_VER;
> +
> +		/* Error interrupt mask register (EIMR) is required for
> +		 * handling individual device error interrupts. EIMR
> +		 * was added in MPIC version 4.1.
> +		 */
> +		if (version >=3D 0x401) {
> +			mpic->hc_err =3D mpic_err_chip;
> +			mpic->hc_err.name =3D mpic->name;
> +			/* Map error interrupt registers */
> +			mpic_map(mpic, mpic->paddr, &mpic->err_regs,
> +				 MPIC_INFO(ERR_INT_BASE), 0x1000);
> +			mpic->flags |=3D MPIC_FSL_HAS_EIMR;
> +		}
> +
> 	}
>=20
> 	/* Reset */
> --=20
> 1.7.2.2
>=20
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.

[Scott Wood]

On 03/27/2012 08:59 AM, Kumar Gala wrote:
> 
> On Mar 27, 2012, at 7:17 AM, Varun Sethi wrote:
> 
>> All SOC device error interrupts are muxed and delivered to the core as a single
>> MPIC error interrupt. Currently all the device drivers requiring access to device
>> errors have to register for the MPIC error interrupt as a shared interrupt.
>>
>> With this patch we add interrupt demuxing capability in the mpic driver, allowing
>> device drivers to register for their individual error interrupts. This is achieved
>> by handling error interrupts in a cascaded fashion.
>>
>> MPIC error interrupt is handled by the "error_int_handler", which subsequently demuxes
>> it using the EISR and delivers it to the respective drivers. 
>>
>> The error interrupt capability is dependent on the MPIC EIMR register, which was
>> introduced in FSL MPIC version 4.1 (P4080 rev2). So, error interrupt demuxing capability
>> is dependent on the MPIC version and can be used for versions >= 4.1.
>>
>> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
>> ---
> 
> This isn't quite right.  Doing the 'request_irq' on behalf of the driver isn't treating the error interrupts as a real cascade, in addition to how you are hooking things up.

Define "real cascade".  If you mean the chained handler stuff that's how
Varun initially did it and I asked him to change it, because it gets a
lot trickier to get things right, and I didn't see what it was buying us.

The request_irq() isn't on behalf of the individual drivers, it's
requesting the cascade itself (not a shared interrupt).

> I think you need to add proper irq_domain_ops, etc.  Thus when we map the virq the proper thing can happen

What proper thing is not happening?

Maybe it would be cleaner from an interrupt number management
perspective to keep things more separate, but do you have a functional
issue in mind?

> I'd also suggest maybe thinking about pulling this code into an mpic_fsl.c
> 
>> arch/powerpc/include/asm/mpic.h |   18 +++++
>> arch/powerpc/sysdev/mpic.c      |  155 ++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 171 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
>> index 3929b4b..db51015 100644
>> --- a/arch/powerpc/include/asm/mpic.h
>> +++ b/arch/powerpc/include/asm/mpic.h
>> @@ -114,12 +114,21 @@
>> #define MPIC_FSL_BRR1			0x00000
>> #define 	MPIC_FSL_BRR1_VER			0x0000ffff
>>
>> +/*
>> + * Error interrupt registers
>> + */
>> +
>> +#define MPIC_ERR_INT_BASE	0x3900
>> +#define MPIC_ERR_INT_EISR	0x0000
>> +#define MPIC_ERR_INT_EIMR	0x0010
>> +
>> #define MPIC_MAX_IRQ_SOURCES	2048
>> #define MPIC_MAX_CPUS		32
>> #define MPIC_MAX_ISU		32
>>
>> #define MPIC_MAX_TIMER    8
>> #define MPIC_MAX_IPI      4
>> +#define MPIC_MAX_ERR      32
> 
> Should probably be 64

This patch supports MPIC 4.1 and EISR0.  When support is added for EISR1
(didn't realize this was coming until your comment prompted me to
check...), this should be updated, but this change alone would not make
it work.

>> +static void mpic_mask_err(struct irq_data *d)
>> +{
>> +	u32 eimr;
>> +	struct mpic *mpic = mpic_from_irq_data(d);
>> +	unsigned int src = virq_to_hw(d->irq) - mpic->err_int_vecs[0];
>> +	unsigned int err_reg_offset = MPIC_INFO(ERR_INT_EIMR);
>> +
>> +	eimr = mpic_err_read(err_reg_offset);
>> +	eimr |= (0x80000000 >> src);
> 
> This seems funny, add a comment about src # and bit # being opposite.

People in PPC-land should be used to this craziness by now. :-P

>  Maybe do:
> 
> 	eimr |= (1 << (31 - src));

I'm not sure it's really any clearer that way...

>> +static irqreturn_t error_int_handler(int irq, void *data)
>> +{
>> +	struct mpic *mpic = (struct mpic *) data;
>> +	unsigned int eisr_offset = MPIC_INFO(ERR_INT_EISR);
>> +	unsigned int eimr_offset = MPIC_INFO(ERR_INT_EIMR);
>> +	u32 eisr, eimr;
>> +	int errint;
>> +	unsigned int cascade_irq;
>> +
>> +	eisr = mpic_err_read(eisr_offset);
>> +	eimr = mpic_err_read(eimr_offset);
>> +
>> +	if (!(eisr & ~eimr))
>> +		return IRQ_NONE;
>> +
>> +	while (eisr) {
>> +		errint = __ffs(eisr);
>> +		cascade_irq = irq_linear_revmap(mpic->irqhost,
>> +				 mpic->err_int_vecs[31 - errint]);
> 
> Should 31, be replaced w/MPIC_MAX_ERR - 1 ?

No, the 31 here is due to the backwards bit numbering within the
register.  It'd be better to say "errint = 31 - __ffs(eisr)" -- or
perhaps "errint = __builtin_clz(eisr)".

-Scott

Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.

[Kumar Gala]

On Mar 27, 2012, at 2:07 PM, Scott Wood wrote:

> On 03/27/2012 08:59 AM, Kumar Gala wrote:
>>=20
>> On Mar 27, 2012, at 7:17 AM, Varun Sethi wrote:
>>=20
>>> All SOC device error interrupts are muxed and delivered to the core =
as a single
>>> MPIC error interrupt. Currently all the device drivers requiring =
access to device
>>> errors have to register for the MPIC error interrupt as a shared =
interrupt.
>>>=20
>>> With this patch we add interrupt demuxing capability in the mpic =
driver, allowing
>>> device drivers to register for their individual error interrupts. =
This is achieved
>>> by handling error interrupts in a cascaded fashion.
>>>=20
>>> MPIC error interrupt is handled by the "error_int_handler", which =
subsequently demuxes
>>> it using the EISR and delivers it to the respective drivers.=20
>>>=20
>>> The error interrupt capability is dependent on the MPIC EIMR =
register, which was
>>> introduced in FSL MPIC version 4.1 (P4080 rev2). So, error interrupt =
demuxing capability
>>> is dependent on the MPIC version and can be used for versions >=3D =
4.1.
>>>=20
>>> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
>>> ---
>>=20
>> This isn't quite right.  Doing the 'request_irq' on behalf of the =
driver isn't treating the error interrupts as a real cascade, in =
addition to how you are hooking things up.
>=20
> Define "real cascade".  If you mean the chained handler stuff that's =
how
> Varun initially did it and I asked him to change it, because it gets a
> lot trickier to get things right, and I didn't see what it was buying =
us.
>=20
> The request_irq() isn't on behalf of the individual drivers, it's
> requesting the cascade itself (not a shared interrupt).

I missed the bit related to err_int_config_done, need to re-look at it.  =
Was thinking request_irq() was getting called for every error interrupt.

>> I think you need to add proper irq_domain_ops, etc.  Thus when we map =
the virq the proper thing can happen
>=20
> What proper thing is not happening?
>=20
> Maybe it would be cleaner from an interrupt number management
> perspective to keep things more separate, but do you have a functional
> issue in mind?
>=20
>> I'd also suggest maybe thinking about pulling this code into an =
mpic_fsl.c
>>=20
>>> arch/powerpc/include/asm/mpic.h |   18 +++++
>>> arch/powerpc/sysdev/mpic.c      |  155 =
++++++++++++++++++++++++++++++++++++++-
>>> 2 files changed, 171 insertions(+), 2 deletions(-)
>>>=20
>>> diff --git a/arch/powerpc/include/asm/mpic.h =
b/arch/powerpc/include/asm/mpic.h
>>> index 3929b4b..db51015 100644
>>> --- a/arch/powerpc/include/asm/mpic.h
>>> +++ b/arch/powerpc/include/asm/mpic.h
>>> @@ -114,12 +114,21 @@
>>> #define MPIC_FSL_BRR1			0x00000
>>> #define 	MPIC_FSL_BRR1_VER			0x0000ffff
>>>=20
>>> +/*
>>> + * Error interrupt registers
>>> + */
>>> +
>>> +#define MPIC_ERR_INT_BASE	0x3900
>>> +#define MPIC_ERR_INT_EISR	0x0000
>>> +#define MPIC_ERR_INT_EIMR	0x0010
>>> +
>>> #define MPIC_MAX_IRQ_SOURCES	2048
>>> #define MPIC_MAX_CPUS		32
>>> #define MPIC_MAX_ISU		32
>>>=20
>>> #define MPIC_MAX_TIMER    8
>>> #define MPIC_MAX_IPI      4
>>> +#define MPIC_MAX_ERR      32
>>=20
>> Should probably be 64
>=20
> This patch supports MPIC 4.1 and EISR0.  When support is added for =
EISR1
> (didn't realize this was coming until your comment prompted me to
> check...), this should be updated, but this change alone would not =
make
> it work.

Would prefer we handle this now rather than later (T4240 is going to =
need EISR1 support).

>>> +static void mpic_mask_err(struct irq_data *d)
>>> +{
>>> +	u32 eimr;
>>> +	struct mpic *mpic =3D mpic_from_irq_data(d);
>>> +	unsigned int src =3D virq_to_hw(d->irq) - mpic->err_int_vecs[0];
>>> +	unsigned int err_reg_offset =3D MPIC_INFO(ERR_INT_EIMR);
>>> +
>>> +	eimr =3D mpic_err_read(err_reg_offset);
>>> +	eimr |=3D (0x80000000 >> src);
>>=20
>> This seems funny, add a comment about src # and bit # being opposite.
>=20
> People in PPC-land should be used to this craziness by now. :-P

:), as I said a comment would be helpful.

>> Maybe do:
>>=20
>> 	eimr |=3D (1 << (31 - src));
>=20
> I'm not sure it's really any clearer that way...
>=20
>>> +static irqreturn_t error_int_handler(int irq, void *data)
>>> +{
>>> +	struct mpic *mpic =3D (struct mpic *) data;
>>> +	unsigned int eisr_offset =3D MPIC_INFO(ERR_INT_EISR);
>>> +	unsigned int eimr_offset =3D MPIC_INFO(ERR_INT_EIMR);
>>> +	u32 eisr, eimr;
>>> +	int errint;
>>> +	unsigned int cascade_irq;
>>> +
>>> +	eisr =3D mpic_err_read(eisr_offset);
>>> +	eimr =3D mpic_err_read(eimr_offset);
>>> +
>>> +	if (!(eisr & ~eimr))
>>> +		return IRQ_NONE;
>>> +
>>> +	while (eisr) {
>>> +		errint =3D __ffs(eisr);
>>> +		cascade_irq =3D irq_linear_revmap(mpic->irqhost,
>>> +				 mpic->err_int_vecs[31 - errint]);
>>=20
>> Should 31, be replaced w/MPIC_MAX_ERR - 1 ?
>=20
> No, the 31 here is due to the backwards bit numbering within the
> register.  It'd be better to say "errint =3D 31 - __ffs(eisr)" -- or
> perhaps "errint =3D __builtin_clz(eisr)".

gotcha

- k=

RE: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.

[Sethi Varun-B16395]

>>> +/*
> >>> + * Error interrupt registers
> >>> + */
> >>> +
> >>> +#define MPIC_ERR_INT_BASE	0x3900
> >>> +#define MPIC_ERR_INT_EISR	0x0000
> >>> +#define MPIC_ERR_INT_EIMR	0x0010
> >>> +
> >>> #define MPIC_MAX_IRQ_SOURCES	2048
> >>> #define MPIC_MAX_CPUS		32
> >>> #define MPIC_MAX_ISU		32
> >>>
> >>> #define MPIC_MAX_TIMER    8
> >>> #define MPIC_MAX_IPI      4
> >>> +#define MPIC_MAX_ERR      32
> >>
> >> Should probably be 64
> >
> > This patch supports MPIC 4.1 and EISR0.  When support is added for
> > EISR1 (didn't realize this was coming until your comment prompted me
> > to check...), this should be updated, but this change alone would not
> > make it work.
>=20
> Would prefer we handle this now rather than later (T4240 is going to need
> EISR1 support).
Hi Kumar,
As of now I don't have a proper mechanism to test this functionality. I wil=
l
submit a follow up patch for EISR1/EIMR1 support once I have a mechanism to
test this functionality.

Regards
Varun
=20

Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.

[Scott Wood]

On 06/18/2012 02:12 PM, Sethi Varun-B16395 wrote:
> 
> 
>>>> +/*
>>>>> + * Error interrupt registers
>>>>> + */
>>>>> +
>>>>> +#define MPIC_ERR_INT_BASE	0x3900
>>>>> +#define MPIC_ERR_INT_EISR	0x0000
>>>>> +#define MPIC_ERR_INT_EIMR	0x0010
>>>>> +
>>>>> #define MPIC_MAX_IRQ_SOURCES	2048
>>>>> #define MPIC_MAX_CPUS		32
>>>>> #define MPIC_MAX_ISU		32
>>>>>
>>>>> #define MPIC_MAX_TIMER    8
>>>>> #define MPIC_MAX_IPI      4
>>>>> +#define MPIC_MAX_ERR      32
>>>>
>>>> Should probably be 64
>>>
>>> This patch supports MPIC 4.1 and EISR0.  When support is added for
>>> EISR1 (didn't realize this was coming until your comment prompted me
>>> to check...), this should be updated, but this change alone would not
>>> make it work.
>>
>> Would prefer we handle this now rather than later (T4240 is going to need
>> EISR1 support).
> Hi Kumar,
> As of now I don't have a proper mechanism to test this functionality. I will
> submit a follow up patch for EISR1/EIMR1 support once I have a mechanism to
> test this functionality.

You could still write the code in a way that scales to multiple EISRs,
and test that it works with EISR0.

-Scott

RE: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.

[Sethi Varun-B16395]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, June 19, 2012 12:47 AM
> To: Sethi Varun-B16395
> Cc: Kumar Gala; Wood Scott-B07421; Linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.
>=20
> On 06/18/2012 02:12 PM, Sethi Varun-B16395 wrote:
> >
> >
> >>>> +/*
> >>>>> + * Error interrupt registers
> >>>>> + */
> >>>>> +
> >>>>> +#define MPIC_ERR_INT_BASE	0x3900
> >>>>> +#define MPIC_ERR_INT_EISR	0x0000
> >>>>> +#define MPIC_ERR_INT_EIMR	0x0010
> >>>>> +
> >>>>> #define MPIC_MAX_IRQ_SOURCES	2048
> >>>>> #define MPIC_MAX_CPUS		32
> >>>>> #define MPIC_MAX_ISU		32
> >>>>>
> >>>>> #define MPIC_MAX_TIMER    8
> >>>>> #define MPIC_MAX_IPI      4
> >>>>> +#define MPIC_MAX_ERR      32
> >>>>
> >>>> Should probably be 64
> >>>
> >>> This patch supports MPIC 4.1 and EISR0.  When support is added for
> >>> EISR1 (didn't realize this was coming until your comment prompted me
> >>> to check...), this should be updated, but this change alone would
> >>> not make it work.
> >>
> >> Would prefer we handle this now rather than later (T4240 is going to
> >> need
> >> EISR1 support).
> > Hi Kumar,
> > As of now I don't have a proper mechanism to test this functionality.
> > I will submit a follow up patch for EISR1/EIMR1 support once I have a
> > mechanism to test this functionality.
>=20
> You could still write the code in a way that scales to multiple EISRs,
> and test that it works with EISR0.
>=20
Yes, but I would like to submit the patch once I have tested it.

-Varun

Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.

[Scott Wood]

On 06/18/2012 02:19 PM, Sethi Varun-B16395 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Tuesday, June 19, 2012 12:47 AM
>> To: Sethi Varun-B16395
>> Cc: Kumar Gala; Wood Scott-B07421; Linuxppc-dev@lists.ozlabs.org
>> Subject: Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.
>>
>> On 06/18/2012 02:12 PM, Sethi Varun-B16395 wrote:
>>>
>>>
>>>>>> +/*
>>>>>>> + * Error interrupt registers
>>>>>>> + */
>>>>>>> +
>>>>>>> +#define MPIC_ERR_INT_BASE	0x3900
>>>>>>> +#define MPIC_ERR_INT_EISR	0x0000
>>>>>>> +#define MPIC_ERR_INT_EIMR	0x0010
>>>>>>> +
>>>>>>> #define MPIC_MAX_IRQ_SOURCES	2048
>>>>>>> #define MPIC_MAX_CPUS		32
>>>>>>> #define MPIC_MAX_ISU		32
>>>>>>>
>>>>>>> #define MPIC_MAX_TIMER    8
>>>>>>> #define MPIC_MAX_IPI      4
>>>>>>> +#define MPIC_MAX_ERR      32
>>>>>>
>>>>>> Should probably be 64
>>>>>
>>>>> This patch supports MPIC 4.1 and EISR0.  When support is added for
>>>>> EISR1 (didn't realize this was coming until your comment prompted me
>>>>> to check...), this should be updated, but this change alone would
>>>>> not make it work.
>>>>
>>>> Would prefer we handle this now rather than later (T4240 is going to
>>>> need
>>>> EISR1 support).
>>> Hi Kumar,
>>> As of now I don't have a proper mechanism to test this functionality.
>>> I will submit a follow up patch for EISR1/EIMR1 support once I have a
>>> mechanism to test this functionality.
>>
>> You could still write the code in a way that scales to multiple EISRs,
>> and test that it works with EISR0.
>>
> Yes, but I would like to submit the patch once I have tested it.

So test it the way I described, and submit. :-P

-Scott

RE: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.

[Sethi Varun-B16395]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, June 19, 2012 12:53 AM
> To: Sethi Varun-B16395
> Cc: Wood Scott-B07421; Kumar Gala; Linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.
>=20
> On 06/18/2012 02:19 PM, Sethi Varun-B16395 wrote:
> >
> >
> >> -----Original Message-----
> >> From: Wood Scott-B07421
> >> Sent: Tuesday, June 19, 2012 12:47 AM
> >> To: Sethi Varun-B16395
> >> Cc: Kumar Gala; Wood Scott-B07421; Linuxppc-dev@lists.ozlabs.org
> >> Subject: Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt
> support.
> >>
> >> On 06/18/2012 02:12 PM, Sethi Varun-B16395 wrote:
> >>>
> >>>
> >>>>>> +/*
> >>>>>>> + * Error interrupt registers
> >>>>>>> + */
> >>>>>>> +
> >>>>>>> +#define MPIC_ERR_INT_BASE	0x3900
> >>>>>>> +#define MPIC_ERR_INT_EISR	0x0000
> >>>>>>> +#define MPIC_ERR_INT_EIMR	0x0010
> >>>>>>> +
> >>>>>>> #define MPIC_MAX_IRQ_SOURCES	2048
> >>>>>>> #define MPIC_MAX_CPUS		32
> >>>>>>> #define MPIC_MAX_ISU		32
> >>>>>>>
> >>>>>>> #define MPIC_MAX_TIMER    8
> >>>>>>> #define MPIC_MAX_IPI      4
> >>>>>>> +#define MPIC_MAX_ERR      32
> >>>>>>
> >>>>>> Should probably be 64
> >>>>>
> >>>>> This patch supports MPIC 4.1 and EISR0.  When support is added for
> >>>>> EISR1 (didn't realize this was coming until your comment prompted
> >>>>> me to check...), this should be updated, but this change alone
> >>>>> would not make it work.
> >>>>
> >>>> Would prefer we handle this now rather than later (T4240 is going
> >>>> to need
> >>>> EISR1 support).
> >>> Hi Kumar,
> >>> As of now I don't have a proper mechanism to test this functionality.
> >>> I will submit a follow up patch for EISR1/EIMR1 support once I have
> >>> a mechanism to test this functionality.
> >>
> >> You could still write the code in a way that scales to multiple
> >> EISRs, and test that it works with EISR0.
> >>
> > Yes, but I would like to submit the patch once I have tested it.
>=20
> So test it the way I described, and submit. :-P
There just seem to be 32 error interrupts even in case of T4240, that means=
 there is no
need to handle multiple EISRs.

I have already submitted a revised patch for handling MPIC error interrupts=
.
[PATCH 3/3 v2] powerpc/mpic: FSL MPIC error interrupt support

-Varun

Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt support.

[Kumar Gala]

On Jul 5, 2012, at 11:02 PM, Sethi Varun-B16395 wrote:

>=20
>=20
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Tuesday, June 19, 2012 12:53 AM
>> To: Sethi Varun-B16395
>> Cc: Wood Scott-B07421; Kumar Gala; Linuxppc-dev@lists.ozlabs.org
>> Subject: Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt =
support.
>>=20
>> On 06/18/2012 02:19 PM, Sethi Varun-B16395 wrote:
>>>=20
>>>=20
>>>> -----Original Message-----
>>>> From: Wood Scott-B07421
>>>> Sent: Tuesday, June 19, 2012 12:47 AM
>>>> To: Sethi Varun-B16395
>>>> Cc: Kumar Gala; Wood Scott-B07421; Linuxppc-dev@lists.ozlabs.org
>>>> Subject: Re: [PATCH 4/4] powerpc/mpic: FSL MPIC error interrupt
>> support.
>>>>=20
>>>> On 06/18/2012 02:12 PM, Sethi Varun-B16395 wrote:
>>>>>=20
>>>>>=20
>>>>>>>> +/*
>>>>>>>>> + * Error interrupt registers
>>>>>>>>> + */
>>>>>>>>> +
>>>>>>>>> +#define MPIC_ERR_INT_BASE	0x3900
>>>>>>>>> +#define MPIC_ERR_INT_EISR	0x0000
>>>>>>>>> +#define MPIC_ERR_INT_EIMR	0x0010
>>>>>>>>> +
>>>>>>>>> #define MPIC_MAX_IRQ_SOURCES	2048
>>>>>>>>> #define MPIC_MAX_CPUS		32
>>>>>>>>> #define MPIC_MAX_ISU		32
>>>>>>>>>=20
>>>>>>>>> #define MPIC_MAX_TIMER    8
>>>>>>>>> #define MPIC_MAX_IPI      4
>>>>>>>>> +#define MPIC_MAX_ERR      32
>>>>>>>>=20
>>>>>>>> Should probably be 64
>>>>>>>=20
>>>>>>> This patch supports MPIC 4.1 and EISR0.  When support is added =
for
>>>>>>> EISR1 (didn't realize this was coming until your comment =
prompted
>>>>>>> me to check...), this should be updated, but this change alone
>>>>>>> would not make it work.
>>>>>>=20
>>>>>> Would prefer we handle this now rather than later (T4240 is going
>>>>>> to need
>>>>>> EISR1 support).
>>>>> Hi Kumar,
>>>>> As of now I don't have a proper mechanism to test this =
functionality.
>>>>> I will submit a follow up patch for EISR1/EIMR1 support once I =
have
>>>>> a mechanism to test this functionality.
>>>>=20
>>>> You could still write the code in a way that scales to multiple
>>>> EISRs, and test that it works with EISR0.
>>>>=20
>>> Yes, but I would like to submit the patch once I have tested it.
>>=20
>> So test it the way I described, and submit. :-P
> There just seem to be 32 error interrupts even in case of T4240, that =
means there is no
> need to handle multiple EISRs.

Ok, but I had some other comments about this patch.

> I have already submitted a revised patch for handling MPIC error =
interrupts.
> [PATCH 3/3 v2] powerpc/mpic: FSL MPIC error interrupt support

Please resubmit the full sequence of patches at this point.

- k=