> -----Original Message----- > From: Kumar Gala [mailto:ga...@kernel.crashing.org] > Sent: Friday, August 03, 2012 6:49 PM > To: Sethi Varun-B16395 > Cc: linuxppc-dev@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.se...@freescale.com> > > Signed-off-by: Bogdan Hamciuc <bogdan.hamc...@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.se...@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 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev