Messages in this thread | | | Subject | Re: [PATCH 06/19] irqchip: atmel-aic: introduce register data structure | From | Milo Kim <> | Date | Wed, 6 Jan 2016 17:30:13 +0900 |
| |
On 01/04/2016 05:53 PM, Boris Brezillon wrote: > On Mon, 4 Jan 2016 13:28:30 +0900 > Milo Kim <milo.kim@ti.com> wrote: > >> Structure, 'aic_reg_offset' describes for device specific register offset. >> Each offset is used for IRQ chip operation. AIC and AIC5 have different >> register values, but the structure can be shared. >> >> Please note that this is not complete patch, it's a preceding step for >> making unified AIC driver. >> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Jason Cooper <jason@lakedaemon.net> >> Cc: Marc Zyngier <marc.zyngier@arm.com> >> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com> >> Cc: Boris BREZILLON <boris.brezillon@free-electrons.com> >> Cc: Ludovic Desroches <ludovic.desroches@atmel.com> >> Cc: Nicolas Ferre <nicolas.ferre@atmel.com> >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Milo Kim <milo.kim@ti.com> >> --- >> drivers/irqchip/irq-atmel-aic-common.c | 91 ++++++++++++++++++++++++++++++++++ >> 1 file changed, 91 insertions(+) >> >> diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c >> index 5effd52..f840165 100644 >> --- a/drivers/irqchip/irq-atmel-aic-common.c >> +++ b/drivers/irqchip/irq-atmel-aic-common.c >> @@ -24,6 +24,32 @@ >> >> #include "irq-atmel-aic-common.h" >> >> +#define AT91_AIC_SMR_BASE 0 >> +#define AT91_AIC_SVR_BASE 0x80 >> +#define AT91_AIC_IVR 0x100 >> +#define AT91_AIC_ISR 0x108 >> +#define AT91_AIC_IECR 0x120 >> +#define AT91_AIC_IDCR 0x124 >> +#define AT91_AIC_ICCR 0x128 >> +#define AT91_AIC_ISCR 0x12c >> +#define AT91_AIC_EOICR 0x130 >> +#define AT91_AIC_SPU 0x134 >> +#define AT91_AIC_DCR 0x138 >> +#define AT91_INVALID_OFFSET (-1) >> + >> +#define AT91_AIC5_SSR 0x0 >> +#define AT91_AIC5_SMR 0x4 >> +#define AT91_AIC5_SVR 0x8 >> +#define AT91_AIC5_IVR 0x10 >> +#define AT91_AIC5_ISR 0x18 >> +#define AT91_AIC5_EOICR 0x38 >> +#define AT91_AIC5_SPU 0x3c >> +#define AT91_AIC5_IECR 0x40 >> +#define AT91_AIC5_IDCR 0x44 >> +#define AT91_AIC5_ICCR 0x48 >> +#define AT91_AIC5_ISCR 0x4c >> +#define AT91_AIC5_DCR 0x6c >> + >> #define AT91_AIC_PRIOR GENMASK(2, 0) >> #define AT91_AIC_IRQ_MIN_PRIORITY 0 >> #define AT91_AIC_IRQ_MAX_PRIORITY 7 >> @@ -38,6 +64,71 @@ struct aic_chip_data { >> u32 ext_irqs; >> }; >> >> +/** >> + * struct aic_reg_offset >> + * >> + * @eoi: End of interrupt command register >> + * @smr: Source mode register >> + * @ssr: Source select register >> + * @iscr: Interrupt set command register >> + * @idcr: Interrupt disable command register >> + * @iccr: Interrupt clear command register >> + * @iecr: Interrupt enable command register >> + * @spu: Spurious interrupt vector register >> + * @dcr: Debug control register >> + * @svr: Source vector register >> + * @ivr: Interrupt vector register >> + * @isr: Interrupt status register >> + * >> + * Each value means register offset. >> + */ >> +struct aic_reg_offset { >> + int eoi; >> + int smr; >> + int ssr; >> + int iscr; >> + int idcr; >> + int iccr; >> + int iecr; >> + int spu; >> + int dcr; >> + int svr; >> + int ivr; >> + int isr; >> +}; >> + >> +static const struct aic_reg_offset aic_regs = { >> + .eoi = AT91_AIC_EOICR, >> + .smr = AT91_AIC_SMR_BASE, >> + .ssr = AT91_INVALID_OFFSET, /* No SSR exists */ >> + .iscr = AT91_AIC_ISCR, >> + .idcr = AT91_AIC_IDCR, >> + .iccr = AT91_AIC_ICCR, >> + .iecr = AT91_AIC_IECR, >> + .spu = AT91_AIC_SPU, >> + .dcr = AT91_AIC_DCR, >> + .svr = AT91_AIC_SVR_BASE, >> + .ivr = AT91_AIC_IVR, >> + .isr = AT91_AIC_ISR, >> +}; >> + >> +static const struct aic_reg_offset aic5_regs = { >> + .eoi = AT91_AIC5_EOICR, >> + .smr = AT91_AIC5_SMR, >> + .ssr = AT91_AIC5_SSR, >> + .iscr = AT91_AIC5_ISCR, >> + .idcr = AT91_AIC5_IDCR, >> + .iccr = AT91_AIC5_ICCR, >> + .iecr = AT91_AIC5_IECR, >> + .spu = AT91_AIC5_SPU, >> + .dcr = AT91_AIC5_DCR, >> + .svr = AT91_AIC5_SVR, >> + .ivr = AT91_AIC5_IVR, >> + .isr = AT91_AIC5_ISR, >> +}; >> + >> +static const struct aic_reg_offset *aic_reg_data; >> + > > Can we avoid adding this global variable: this information can probably > be added to the aic_chip_data struct.
Chip private data, 'aic_chip_data' is allocated per each chip. If we move aic_reg_data into aic_chip_data, then this data will be allocated in each IRQ chip data as well. Then, AIC5 will allocate several register data. (nchips >= 2 in AIC5)
aic = kcalloc(nchips, sizeof(*aic), GFP_KERNEL); if (!aic) { ret = -ENOMEM; goto err_iounmap; }
No need to use multiple register data.
Best regards, Milo
| |