From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752276AbcAFIai (ORCPT ); Wed, 6 Jan 2016 03:30:38 -0500 Received: from arroyo.ext.ti.com ([192.94.94.40]:43698 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751999AbcAFIaf (ORCPT ); Wed, 6 Jan 2016 03:30:35 -0500 Subject: Re: [PATCH 06/19] irqchip: atmel-aic: introduce register data structure To: Boris Brezillon References: <1451881723-2478-1-git-send-email-milo.kim@ti.com> <1451881723-2478-7-git-send-email-milo.kim@ti.com> <20160104095309.4a613aab@bbrezillon> CC: , , , , , , From: Milo Kim Message-ID: <568CD095.7010203@ti.com> Date: Wed, 6 Jan 2016 17:30:13 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <20160104095309.4a613aab@bbrezillon> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/04/2016 05:53 PM, Boris Brezillon wrote: > On Mon, 4 Jan 2016 13:28:30 +0900 > Milo Kim 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 >> Cc: Jason Cooper >> Cc: Marc Zyngier >> Cc: Alexandre Belloni >> Cc: Boris BREZILLON >> Cc: Ludovic Desroches >> Cc: Nicolas Ferre >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Milo Kim >> --- >> 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