Re: [PATCH 06/19] irqchip: atmel-aic: introduce register data structure

From: Boris Brezillon
Date: Mon Jan 04 2016 - 03:53:17 EST


On Mon, 4 Jan 2016 13:28:30 +0900
Milo Kim <milo.kim@xxxxxx> 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@xxxxxxxxxxxxx>
> Cc: Jason Cooper <jason@xxxxxxxxxxxxxx>
> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
> Cc: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxxxxxxxxx>
> Cc: Boris BREZILLON <boris.brezillon@xxxxxxxxxxxxxxxxxx>
> Cc: Ludovic Desroches <ludovic.desroches@xxxxxxxxx>
> Cc: Nicolas Ferre <nicolas.ferre@xxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Milo Kim <milo.kim@xxxxxx>
> ---
> 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.

> static void aic_common_shutdown(struct irq_data *d)
> {
> struct irq_chip_type *ct = irq_data_get_chip_type(d);



--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/