lkml.org 
[lkml]   [2016]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 06/19] irqchip: atmel-aic: introduce register data structure
From
Date
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


\
 
 \ /
  Last update: 2016-01-06 09:41    [W:0.081 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site