[PATCH 3/3 v3] powerpc/mpic: FSL MPIC error interrupt support.
Sethi Varun-B16395
B16395 at freescale.com
Sat Aug 4 04:52:17 EST 2012
> -----Original Message-----
> From: Kumar Gala [mailto:galak at kernel.crashing.org]
> Sent: Friday, August 03, 2012 6:49 PM
> To: Sethi Varun-B16395
> Cc: linuxppc-dev at lists.ozlabs.org; Hamciuc Bogdan-BHAMCIU1
> Subject: Re: [PATCH 3/3 v3] powerpc/mpic: FSL MPIC error interrupt
> support.
>
>
> On Jul 31, 2012, at 9:42 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 at freescale.com>
> > Signed-off-by: Bogdan Hamciuc <bogdan.hamciuc at freescale.com> [In the
> > initial version of the patch we were using handle_simple_irq as the
> > handler for cascaded error interrupts, this resulted in issues in case
> > of threaded isrs (with RT kernel). This issue was debugged by Bogdan
> > and decision was taken to use the handle_level_irq handler]
>
> [ fix commit message to wrap at 75 char columns ]
>
> >
> > ---
> > arch/powerpc/include/asm/mpic.h | 13 +++
> > arch/powerpc/sysdev/Makefile | 2 +-
> > arch/powerpc/sysdev/fsl_mpic_err.c | 152
> ++++++++++++++++++++++++++++++++++++
> > arch/powerpc/sysdev/mpic.c | 41 ++++++++++-
> > arch/powerpc/sysdev/mpic.h | 22 +++++
> > 5 files changed, 228 insertions(+), 2 deletions(-) create mode 100644
> > arch/powerpc/sysdev/fsl_mpic_err.c
> >
> > diff --git a/arch/powerpc/include/asm/mpic.h
> > b/arch/powerpc/include/asm/mpic.h index e14d35d..5cc8000 100644
> > --- a/arch/powerpc/include/asm/mpic.h
> > +++ b/arch/powerpc/include/asm/mpic.h
> > @@ -118,6 +118,9 @@
> > #define MPIC_MAX_CPUS 32
> > #define MPIC_MAX_ISU 32
> >
> > +#define MPIC_MAX_ERR 32
> > +#define MPIC_FSL_ERR_INT 16
> > +
> > /*
> > * Tsi108 implementation of MPIC has many differences from the original
> > one */ @@ -270,6 +273,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;
> > @@ -283,6 +287,8 @@ struct mpic
> > /* vector numbers used for internal sources (ipi/timers) */
> > unsigned int ipi_vecs[4];
> > unsigned int timer_vecs[8];
> > + /* 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;
> > @@ -306,6 +312,11 @@ struct mpic
> > struct mpic_reg_bank cpuregs[MPIC_MAX_CPUS];
> > struct mpic_reg_bank isus[MPIC_MAX_ISU];
> >
> > + /* ioremap'ed base for error interrupt registers */
> > + u32 __iomem *err_regs;
> > + /* error interrupt config */
> > + u32 err_int_config_done;
> > +
>
> Is this really needed ?
[Sethi Varun-B16395] check for verifying if mpic_err_int_init was successful or not.
>
> > /* Protected sources */
> > unsigned long *protected;
> >
> > @@ -370,6 +381,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
>
> Can't we use BRR for this?
[Sethi Varun-B16395] This flag is set based on the BRR check. This is checked at various places.
>
> >
> > /* MPIC HW modification ID */
> > #define MPIC_REGSET_MASK 0xf0000000
> > diff --git a/arch/powerpc/sysdev/Makefile
> > b/arch/powerpc/sysdev/Makefile index 1bd7ecb..a57600b 100644
> > --- a/arch/powerpc/sysdev/Makefile
> > +++ b/arch/powerpc/sysdev/Makefile
> > @@ -15,7 +15,7 @@ obj-$(CONFIG_PPC_DCR_NATIVE) += dcr-low.o
> > obj-$(CONFIG_PPC_PMI) += pmi.o
> > obj-$(CONFIG_U3_DART) += dart_iommu.o
> > obj-$(CONFIG_MMIO_NVRAM) += mmio_nvram.o
> > -obj-$(CONFIG_FSL_SOC) += fsl_soc.o
> > +obj-$(CONFIG_FSL_SOC) += fsl_soc.o fsl_mpic_err.o
> > obj-$(CONFIG_FSL_PCI) += fsl_pci.o $(fsl-msi-obj-y)
> > obj-$(CONFIG_FSL_PMC) += fsl_pmc.o
> > obj-$(CONFIG_FSL_LBC) += fsl_lbc.o
> > diff --git a/arch/powerpc/sysdev/fsl_mpic_err.c
> > b/arch/powerpc/sysdev/fsl_mpic_err.c
> > new file mode 100644
> > index 0000000..1ebfa36
> > --- /dev/null
> > +++ b/arch/powerpc/sysdev/fsl_mpic_err.c
> > @@ -0,0 +1,152 @@
> > +/*
> > + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> > + *
> > + * Author: Varun Sethi <varun.sethi at freescale.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * as published by the Free Software Foundation; version 2 of the
> > + * License.
> > + *
> > + */
> > +
> > +#include <linux/irq.h>
> > +#include <linux/smp.h>
> > +#include <linux/interrupt.h>
> > +
> > +#include <asm/io.h>
> > +#include <asm/irq.h>
> > +#include <asm/mpic.h>
> > +
> > +#include "mpic.h"
> > +
> > +#define MPIC_ERR_INT_BASE 0x3900
> > +#define MPIC_ERR_INT_EISR 0x0000
> > +#define MPIC_ERR_INT_EIMR 0x0010
> > +
> > +static inline u32 mpic_fsl_err_read(u32 __iomem *base, unsigned int
> > +err_reg) {
> > + return in_be32(base + (err_reg >> 2)); }
> > +
> > +static inline void mpic_fsl_err_write(u32 __iomem *base, u32 value) {
> > + out_be32(base + (MPIC_ERR_INT_EIMR >> 2), value); }
> > +
> > +static void fsl_mpic_mask_err(struct irq_data *d) {
> > + u32 eimr;
> > + struct mpic *mpic = irq_data_get_irq_chip_data(d);
> > + unsigned int src = virq_to_hw(d->irq) - mpic->err_int_vecs[0];
> > +
> > + eimr = mpic_fsl_err_read(mpic->err_regs, MPIC_ERR_INT_EIMR);
> > + eimr |= (1 << (31 - src));
> > + mpic_fsl_err_write(mpic->err_regs, eimr); }
> > +
> > +static void fsl_mpic_unmask_err(struct irq_data *d) {
> > + u32 eimr;
> > + struct mpic *mpic = irq_data_get_irq_chip_data(d);
> > + unsigned int src = virq_to_hw(d->irq) - mpic->err_int_vecs[0];
> > +
> > + eimr = mpic_fsl_err_read(mpic->err_regs, MPIC_ERR_INT_EIMR);
> > + eimr &= ~(1 << (31 - src));
> > + mpic_fsl_err_write(mpic->err_regs, eimr); }
> > +
> > +static struct irq_chip fsl_mpic_err_chip = {
> > + .irq_disable = fsl_mpic_mask_err,
> > + .irq_mask = fsl_mpic_mask_err,
> > + .irq_unmask = fsl_mpic_unmask_err,
> > +};
> > +
> > +void mpic_setup_error_int(struct mpic *mpic, int intvec) {
> > + int i;
> > +
> > + mpic->err_regs = ioremap(mpic->paddr + MPIC_ERR_INT_BASE, 0x1000);
> > + if (!mpic->err_regs) {
> > + pr_err("could not map mpic error registers\n");
> > + return;
>
> we should propagate an error back up if ioremap failed.
[Sethi Varun-B16395] Should we fail mpic_alloc when this fails?
>
> > + }
> > + mpic->hc_err = fsl_mpic_err_chip;
> > + mpic->hc_err.name = mpic->name;
> > + mpic->flags |= MPIC_FSL_HAS_EIMR;
> > + /* allocate interrupt vectors for error interrupts */
> > + for (i = MPIC_MAX_ERR - 1; i >= 0; i--)
> > + mpic->err_int_vecs[i] = --intvec;
> > +
> > +}
> > +
> > +int mpic_map_error_int(struct mpic *mpic, unsigned int virq,
> > +irq_hw_number_t hw) {
> > + if ((mpic->flags & MPIC_FSL_HAS_EIMR) &&
> > + (hw >= mpic->err_int_vecs[0] &&
> > + hw <= mpic->err_int_vecs[MPIC_MAX_ERR - 1])) {
> > + WARN_ON(mpic->flags & MPIC_SECONDARY);
> > +
> > + pr_debug("mpic: mapping as Error Interrupt\n");
> > + irq_set_chip_data(virq, mpic);
> > + irq_set_chip_and_handler(virq, &mpic->hc_err,
> > + handle_level_irq);
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static irqreturn_t fsl_error_int_handler(int irq, void *data) {
> > + struct mpic *mpic = (struct mpic *) data;
> > + u32 eisr, eimr;
> > + int errint;
> > + unsigned int cascade_irq;
> > +
> > + eisr = mpic_fsl_err_read(mpic->err_regs, MPIC_ERR_INT_EISR);
> > + eimr = mpic_fsl_err_read(mpic->err_regs, MPIC_ERR_INT_EIMR);
> > +
> > + if (!(eisr & ~eimr))
> > + return IRQ_NONE;
> > +
> > + while (eisr) {
> > + errint = __builtin_clz(eisr);
> > + cascade_irq = irq_linear_revmap(mpic->irqhost,
> > + mpic->err_int_vecs[errint]);
> > + WARN_ON(cascade_irq == NO_IRQ);
> > + if (cascade_irq != NO_IRQ) {
> > + generic_handle_irq(cascade_irq);
> > + } else {
> > + eimr |= 1 << (31 - errint);
> > + mpic_fsl_err_write(mpic->err_regs, eimr);
> > + }
> > + eisr &= ~(1 << (31 - errint));
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +int mpic_err_int_init(struct mpic *mpic, irq_hw_number_t irqnum) {
> > + unsigned int virq;
> > + int ret;
> > +
> > + virq = irq_create_mapping(mpic->irqhost, irqnum);
> > + if (virq == NO_IRQ) {
> > + pr_err("Error interrupt setup failed\n");
> > + return 0;
> > + }
> > +
> > + /* Mask all error interrupts */
> > + mpic_fsl_err_write(mpic->err_regs, ~0);
> > +
> > + ret = request_irq(virq, fsl_error_int_handler, IRQF_NO_THREAD,
> > + "mpic-error-int", mpic);
> > + if (ret) {
> > + pr_err("Failed to register error interrupt handler\n");
> > + return 0;
> > + }
> > +
> > + return 1;
> > +}
> > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> > index 7e32db7..2a0b632 100644
> > --- a/arch/powerpc/sysdev/mpic.c
> > +++ b/arch/powerpc/sysdev/mpic.c
> > @@ -1026,6 +1026,9 @@ static int mpic_host_map(struct irq_domain *h,
> unsigned int virq,
> > return 0;
> > }
> >
> > + if (mpic_map_error_int(mpic, virq, hw))
> > + return 0;
> > +
> > if (hw >= mpic->num_sources)
> > return -EINVAL;
> >
> > @@ -1085,7 +1088,16 @@ static int mpic_host_xlate(struct irq_domain *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->err_int_config_done)
> > + break;
> > +
>
> Under what case would we call mpic_host_xlate and have not called
> mpic_init?
>
[Sethi Varun-B16395] Never, but we shouldn't translate the error interrupt specifier
If mpic_err_int_init failed.
-Varun
More information about the Linuxppc-dev
mailing list