[PATCH 00/19] irqchip: atmel-aic: make unified AIC driver

[PATCH 00/19] irqchip: atmel-aic: make unified AIC driver

[Milo Kim]

This patch-set provides unified Atmel AIC (Advanced Interrupt Controller)
driver. Currently, there are two AIC drivers, AIC and AIC5.
Each driver consists of chip specific part (irq-atmel-aic.o or
irq-atmel-aic5.o) and shared code (irq-atmel-aic-common.o).
But consolidated AIC driver is just one file driver which supports both
IRQ chip systems.

How to handle two IRQ chips in one driver
-----------------------------------------
  Structure aic_reg_offset is used for device configuration.
  AIC5 IRQ chip uses SSR (Source Select Register) to select IRQ number.
  On the other hand, AIC IRQ chip has simple register access.
  To support both IRQ chips, aic_is_ssr_used() helper is used.

Patches
-------
  1 ~  5: fix IRQ priority issue, clean up RTC/RTT fixup code and etc.
  6 ~ 19: create unified IRQ chip operation with aic_reg_offset data.

Target boards
-------------
  Tested with two boards.
  * Arietta G25 (SoC: AT91SAM9G25)
  * Xplained board (SoC: SAMA5D3)

Number of driver files
----------------------
  AIC:              3 (irq-atmel-aic.c, irq-atmel-aic-common.c and h)
  AIC5:             3 (irq-atmel-aic5.c, irq-atmel-aic-common.c and h)
  Consolidated AIC: 1 (irq-aic.c)

Code size
---------
  AIC (irq-atmel-aic.o and irq-atmel-aic-common.o)
    text	   data	    bss	    dec	    hex	filename
    5137	    196	      4	   5337	   14d9	drivers/irqchip/built-in.o

  AIC5 (irq-atmel-aic5.o and irq-atmel-aic-common.o)
    text	   data	    bss	    dec	    hex	filename
    5548	    196	      4	   5748	   1674	drivers/irqchip/built-in.o

  Consolidated AIC (irq-aic.o)
    text	   data	    bss	    dec	    hex	filename
    4841	    196	      8	   5045	   13b5	drivers/irqchip/built-in.o

Lines of code
-------------
  AIC:              597
  AIC5:             688
  Consolidated AIC: 609

Milo Kim (19):
  irqchip: atmel-aic: fix wrong bit operation for IRQ priority
  irqchip: atmel-aic: clean up RTC interrupt code
  irqchip: atmel-aic: clean up RTT interrupt code
  irqchip: atmel-aic: replace magic numbers with named constant
  irqchip: atmel-aic: use simple constant to get number of interrupts
    per chip
  irqchip: atmel-aic: introduce register data structure
  irqchip: atmel-aic: make common IRQ domain translate function
  irqchip: atmel-aic: add common mask and unmask functions
  irqchip: atmel-aic: add common retrigger function
  irqchip: atmel-aic: add common set_type function
  irqchip: atmel-aic: add common PM IRQ chip operation
  irqchip: atmel-aic: use EOI register data in aic_reg_data
  irqchip: atmel-aic: clean up irq_chip_generic
  irqchip: atmel-aic: add common HW init function
  irqchip: atmel-aic: add common interrupt handler
  irqchip: atmel-aic: get total number of IRQs from device node
  irqchip: atmel-aic: use unified IRQ chip initialization function
  irqchip: atmel-aic: use unified AIC driver
  irqchip: atmel-aic: rename AIC driver and fix Kconfig

 arch/arm/mach-at91/Kconfig             |   2 +-
 drivers/irqchip/Kconfig                |   7 -
 drivers/irqchip/Makefile               |   3 +-
 drivers/irqchip/irq-aic.c              | 609 +++++++++++++++++++++++++++++++++
 drivers/irqchip/irq-atmel-aic-common.c | 280 ---------------
 drivers/irqchip/irq-atmel-aic-common.h |  41 ---
 drivers/irqchip/irq-atmel-aic.c        | 276 ---------------
 drivers/irqchip/irq-atmel-aic5.c       | 367 --------------------
 8 files changed, 611 insertions(+), 974 deletions(-)
 create mode 100644 drivers/irqchip/irq-aic.c
 delete mode 100644 drivers/irqchip/irq-atmel-aic-common.c
 delete mode 100644 drivers/irqchip/irq-atmel-aic-common.h
 delete mode 100644 drivers/irqchip/irq-atmel-aic.c
 delete mode 100644 drivers/irqchip/irq-atmel-aic5.c

-- 
2.6.4

[PATCH 01/19] irqchip: atmel-aic: fix wrong bit operation for IRQ priority

[Milo Kim]

Atmel AIC has common structure for SMR (Source Mode Register).

  bit[6:5] Interrupt source type
  bit[2:0] Priority level
  Other bits are unused.

To update new priority value, bit[2:0] should be cleared first and then
new priority level can be written. However, aic_common_set_priority()
helper clears source type bits instead of priority bits.
This patch fixes wrong mask bit operation.

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index b12a5d5..37199b9 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -86,7 +86,7 @@ int aic_common_set_priority(int priority, unsigned *val)
 	    priority > AT91_AIC_IRQ_MAX_PRIORITY)
 		return -EINVAL;
 
-	*val &= AT91_AIC_PRIOR;
+	*val &= ~AT91_AIC_PRIOR;
 	*val |= priority;
 
 	return 0;
-- 
2.6.4

Re: [PATCH 01/19] irqchip: atmel-aic: fix wrong bit operation for IRQ priority

[Boris Brezillon]

Hi Milo,

On Mon, 4 Jan 2016 13:28:25 +0900
Milo Kim <milo.kim@ti.com> wrote:

> Atmel AIC has common structure for SMR (Source Mode Register).
> 
>   bit[6:5] Interrupt source type
>   bit[2:0] Priority level
>   Other bits are unused.
> 
> To update new priority value, bit[2:0] should be cleared first and then
> new priority level can be written. However, aic_common_set_priority()
> helper clears source type bits instead of priority bits.
> This patch fixes wrong mask bit operation.
> 
> 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>

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Can you also add

Fixes: b1479ebb7720 ("irqchip: atmel-aic: Add atmel AIC/AIC5 drivers")
Cc: stable@vger.kernel.org #v3.17+

Thanks,

Boris

> ---
>  drivers/irqchip/irq-atmel-aic-common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
> index b12a5d5..37199b9 100644
> --- a/drivers/irqchip/irq-atmel-aic-common.c
> +++ b/drivers/irqchip/irq-atmel-aic-common.c
> @@ -86,7 +86,7 @@ int aic_common_set_priority(int priority, unsigned *val)
>  	    priority > AT91_AIC_IRQ_MAX_PRIORITY)
>  		return -EINVAL;
>  
> -	*val &= AT91_AIC_PRIOR;
> +	*val &= ~AT91_AIC_PRIOR;
>  	*val |= priority;
>  
>  	return 0;



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH 02/19] irqchip: atmel-aic: clean up RTC interrupt code

[Milo Kim]

Atmel AIC disables RTC interrupt on initialization. However, the controller
doesn't need to do it. There are two reasons.

* RTC driver disables interrupts on _probe()
  RTC_IDR is set to 0x1f which means all RTC interrupts are disabled.
  (drivers/rtc/rtc-at91rm9200.c)
  at91_rtc_write_idr(AT91_RTC_ACKUPD | AT91_RTC_ALARM |
		AT91_RTC_SECEV | AT91_RTC_TIMEV |
		AT91_RTC_CALEV);

* All RTC interrupts are disabled by default.
  If we try to read RTC_IMR(interrup mask status), then read value is 0
  by default. It means all RTC interrupts are disabled by default.
  We can also check the default status in the datasheet. Please note that
  all devices have same RTC registers.
  AT91RM9200
  AT91SAM9N12
  AT91SAM9X5
  AT91SAM9G45
  AT91SAM9RL
  SAMA5D3
  SAMA5D4

So, aic_common_rtc_irq_fixup() helper can be removed.

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 | 28 ----------------------------
 drivers/irqchip/irq-atmel-aic-common.h |  2 --
 drivers/irqchip/irq-atmel-aic.c        |  9 ---------
 drivers/irqchip/irq-atmel-aic5.c       | 13 -------------
 4 files changed, 52 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index 37199b9..f8261be 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -139,34 +139,6 @@ static void __init aic_common_ext_irq_of_init(struct irq_domain *domain)
 	}
 }
 
-#define AT91_RTC_IDR           0x24
-#define AT91_RTC_IMR           0x28
-#define AT91_RTC_IRQ_MASK      0x1f
-
-void __init aic_common_rtc_irq_fixup(struct device_node *root)
-{
-	struct device_node *np;
-	void __iomem *regs;
-
-	np = of_find_compatible_node(root, NULL, "atmel,at91rm9200-rtc");
-	if (!np)
-		np = of_find_compatible_node(root, NULL,
-					     "atmel,at91sam9x5-rtc");
-
-	if (!np)
-		return;
-
-	regs = of_iomap(np, 0);
-	of_node_put(np);
-
-	if (!regs)
-		return;
-
-	writel(AT91_RTC_IRQ_MASK, regs + AT91_RTC_IDR);
-
-	iounmap(regs);
-}
-
 #define AT91_RTT_MR		0x00			/* Real-time Mode Register */
 #define AT91_RTT_ALMIEN		(1 << 16)		/* Alarm Interrupt Enable */
 #define AT91_RTT_RTTINCIEN	(1 << 17)		/* Real Time Timer Increment Interrupt Enable */
diff --git a/drivers/irqchip/irq-atmel-aic-common.h b/drivers/irqchip/irq-atmel-aic-common.h
index 603f0a9..9027f15 100644
--- a/drivers/irqchip/irq-atmel-aic-common.h
+++ b/drivers/irqchip/irq-atmel-aic-common.h
@@ -32,8 +32,6 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 					     const struct irq_domain_ops *ops,
 					     const char *name, int nirqs);
 
-void __init aic_common_rtc_irq_fixup(struct device_node *root);
-
 void __init aic_common_rtt_irq_fixup(struct device_node *root);
 
 void __init aic_common_irq_fixup(const struct of_device_id *matches);
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index 8a0c7f2..4c60224 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -209,11 +209,6 @@ static const struct irq_domain_ops aic_irq_ops = {
 	.xlate	= aic_irq_domain_xlate,
 };
 
-static void __init at91rm9200_aic_irq_fixup(struct device_node *root)
-{
-	aic_common_rtc_irq_fixup(root);
-}
-
 static void __init at91sam9260_aic_irq_fixup(struct device_node *root)
 {
 	aic_common_rtt_irq_fixup(root);
@@ -221,16 +216,12 @@ static void __init at91sam9260_aic_irq_fixup(struct device_node *root)
 
 static void __init at91sam9g45_aic_irq_fixup(struct device_node *root)
 {
-	aic_common_rtc_irq_fixup(root);
 	aic_common_rtt_irq_fixup(root);
 }
 
 static const struct of_device_id aic_irq_fixups[] __initconst = {
-	{ .compatible = "atmel,at91rm9200", .data = at91rm9200_aic_irq_fixup },
 	{ .compatible = "atmel,at91sam9g45", .data = at91sam9g45_aic_irq_fixup },
-	{ .compatible = "atmel,at91sam9n12", .data = at91rm9200_aic_irq_fixup },
 	{ .compatible = "atmel,at91sam9rl", .data = at91sam9g45_aic_irq_fixup },
-	{ .compatible = "atmel,at91sam9x5", .data = at91rm9200_aic_irq_fixup },
 	{ .compatible = "atmel,at91sam9260", .data = at91sam9260_aic_irq_fixup },
 	{ .compatible = "atmel,at91sam9261", .data = at91sam9260_aic_irq_fixup },
 	{ .compatible = "atmel,at91sam9263", .data = at91sam9260_aic_irq_fixup },
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 62bb840..8b8f5e2 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -285,17 +285,6 @@ static const struct irq_domain_ops aic5_irq_ops = {
 	.xlate	= aic5_irq_domain_xlate,
 };
 
-static void __init sama5d3_aic_irq_fixup(struct device_node *root)
-{
-	aic_common_rtc_irq_fixup(root);
-}
-
-static const struct of_device_id aic5_irq_fixups[] __initconst = {
-	{ .compatible = "atmel,sama5d3", .data = sama5d3_aic_irq_fixup },
-	{ .compatible = "atmel,sama5d4", .data = sama5d3_aic_irq_fixup },
-	{ /* sentinel */ },
-};
-
 static int __init aic5_of_init(struct device_node *node,
 			       struct device_node *parent,
 			       int nirqs)
@@ -316,8 +305,6 @@ static int __init aic5_of_init(struct device_node *node,
 	if (IS_ERR(domain))
 		return PTR_ERR(domain);
 
-	aic_common_irq_fixup(aic5_irq_fixups);
-
 	aic5_domain = domain;
 	nchips = aic5_domain->revmap_size / 32;
 	for (i = 0; i < nchips; i++) {
-- 
2.6.4

Re: [PATCH 02/19] irqchip: atmel-aic: clean up RTC interrupt code

[Boris Brezillon]

Hi Milo,

On Mon, 4 Jan 2016 13:28:26 +0900
Milo Kim <milo.kim@ti.com> wrote:

> Atmel AIC disables RTC interrupt on initialization. However, the controller
> doesn't need to do it. There are two reasons.
> 
> * RTC driver disables interrupts on _probe()
>   RTC_IDR is set to 0x1f which means all RTC interrupts are disabled.
>   (drivers/rtc/rtc-at91rm9200.c)
>   at91_rtc_write_idr(AT91_RTC_ACKUPD | AT91_RTC_ALARM |
> 		AT91_RTC_SECEV | AT91_RTC_TIMEV |
> 		AT91_RTC_CALEV);
> 
> * All RTC interrupts are disabled by default.
>   If we try to read RTC_IMR(interrup mask status), then read value is 0
>   by default. It means all RTC interrupts are disabled by default.
>   We can also check the default status in the datasheet. Please note that
>   all devices have same RTC registers.
>   AT91RM9200
>   AT91SAM9N12
>   AT91SAM9X5
>   AT91SAM9G45
>   AT91SAM9RL
>   SAMA5D3
>   SAMA5D4
> 
> So, aic_common_rtc_irq_fixup() helper can be removed.

Please read b2f579b58e93 ("irqchip: atmel-aic: Add irq fixup
infrastructure") and 3d61467f9bab ("irqchip: atmel-aic: Implement RTC
irq fixup") messages before assuming you can simply remove these
functions.

There's a good reason behind this irq fixup infrastructure, and you're
blindly removing those functions without asking why we were doing that.

Best Regards,

Boris

> 
> 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 | 28 ----------------------------
>  drivers/irqchip/irq-atmel-aic-common.h |  2 --
>  drivers/irqchip/irq-atmel-aic.c        |  9 ---------
>  drivers/irqchip/irq-atmel-aic5.c       | 13 -------------
>  4 files changed, 52 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
> index 37199b9..f8261be 100644
> --- a/drivers/irqchip/irq-atmel-aic-common.c
> +++ b/drivers/irqchip/irq-atmel-aic-common.c
> @@ -139,34 +139,6 @@ static void __init aic_common_ext_irq_of_init(struct irq_domain *domain)
>  	}
>  }
>  
> -#define AT91_RTC_IDR           0x24
> -#define AT91_RTC_IMR           0x28
> -#define AT91_RTC_IRQ_MASK      0x1f
> -
> -void __init aic_common_rtc_irq_fixup(struct device_node *root)
> -{
> -	struct device_node *np;
> -	void __iomem *regs;
> -
> -	np = of_find_compatible_node(root, NULL, "atmel,at91rm9200-rtc");
> -	if (!np)
> -		np = of_find_compatible_node(root, NULL,
> -					     "atmel,at91sam9x5-rtc");
> -
> -	if (!np)
> -		return;
> -
> -	regs = of_iomap(np, 0);
> -	of_node_put(np);
> -
> -	if (!regs)
> -		return;
> -
> -	writel(AT91_RTC_IRQ_MASK, regs + AT91_RTC_IDR);
> -
> -	iounmap(regs);
> -}
> -
>  #define AT91_RTT_MR		0x00			/* Real-time Mode Register */
>  #define AT91_RTT_ALMIEN		(1 << 16)		/* Alarm Interrupt Enable */
>  #define AT91_RTT_RTTINCIEN	(1 << 17)		/* Real Time Timer Increment Interrupt Enable */
> diff --git a/drivers/irqchip/irq-atmel-aic-common.h b/drivers/irqchip/irq-atmel-aic-common.h
> index 603f0a9..9027f15 100644
> --- a/drivers/irqchip/irq-atmel-aic-common.h
> +++ b/drivers/irqchip/irq-atmel-aic-common.h
> @@ -32,8 +32,6 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
>  					     const struct irq_domain_ops *ops,
>  					     const char *name, int nirqs);
>  
> -void __init aic_common_rtc_irq_fixup(struct device_node *root);
> -
>  void __init aic_common_rtt_irq_fixup(struct device_node *root);
>  
>  void __init aic_common_irq_fixup(const struct of_device_id *matches);
> diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
> index 8a0c7f2..4c60224 100644
> --- a/drivers/irqchip/irq-atmel-aic.c
> +++ b/drivers/irqchip/irq-atmel-aic.c
> @@ -209,11 +209,6 @@ static const struct irq_domain_ops aic_irq_ops = {
>  	.xlate	= aic_irq_domain_xlate,
>  };
>  
> -static void __init at91rm9200_aic_irq_fixup(struct device_node *root)
> -{
> -	aic_common_rtc_irq_fixup(root);
> -}
> -
>  static void __init at91sam9260_aic_irq_fixup(struct device_node *root)
>  {
>  	aic_common_rtt_irq_fixup(root);
> @@ -221,16 +216,12 @@ static void __init at91sam9260_aic_irq_fixup(struct device_node *root)
>  
>  static void __init at91sam9g45_aic_irq_fixup(struct device_node *root)
>  {
> -	aic_common_rtc_irq_fixup(root);
>  	aic_common_rtt_irq_fixup(root);
>  }
>  
>  static const struct of_device_id aic_irq_fixups[] __initconst = {
> -	{ .compatible = "atmel,at91rm9200", .data = at91rm9200_aic_irq_fixup },
>  	{ .compatible = "atmel,at91sam9g45", .data = at91sam9g45_aic_irq_fixup },
> -	{ .compatible = "atmel,at91sam9n12", .data = at91rm9200_aic_irq_fixup },
>  	{ .compatible = "atmel,at91sam9rl", .data = at91sam9g45_aic_irq_fixup },
> -	{ .compatible = "atmel,at91sam9x5", .data = at91rm9200_aic_irq_fixup },
>  	{ .compatible = "atmel,at91sam9260", .data = at91sam9260_aic_irq_fixup },
>  	{ .compatible = "atmel,at91sam9261", .data = at91sam9260_aic_irq_fixup },
>  	{ .compatible = "atmel,at91sam9263", .data = at91sam9260_aic_irq_fixup },
> diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
> index 62bb840..8b8f5e2 100644
> --- a/drivers/irqchip/irq-atmel-aic5.c
> +++ b/drivers/irqchip/irq-atmel-aic5.c
> @@ -285,17 +285,6 @@ static const struct irq_domain_ops aic5_irq_ops = {
>  	.xlate	= aic5_irq_domain_xlate,
>  };
>  
> -static void __init sama5d3_aic_irq_fixup(struct device_node *root)
> -{
> -	aic_common_rtc_irq_fixup(root);
> -}
> -
> -static const struct of_device_id aic5_irq_fixups[] __initconst = {
> -	{ .compatible = "atmel,sama5d3", .data = sama5d3_aic_irq_fixup },
> -	{ .compatible = "atmel,sama5d4", .data = sama5d3_aic_irq_fixup },
> -	{ /* sentinel */ },
> -};
> -
>  static int __init aic5_of_init(struct device_node *node,
>  			       struct device_node *parent,
>  			       int nirqs)
> @@ -316,8 +305,6 @@ static int __init aic5_of_init(struct device_node *node,
>  	if (IS_ERR(domain))
>  		return PTR_ERR(domain);
>  
> -	aic_common_irq_fixup(aic5_irq_fixups);
> -
>  	aic5_domain = domain;
>  	nchips = aic5_domain->revmap_size / 32;
>  	for (i = 0; i < nchips; i++) {



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH 03/19] irqchip: atmel-aic: clean up RTT interrupt code

[Milo Kim]

Atmel AIC disables RTT(Real Time Timer) interrupt on initialization.
Alarm interrupt(ALMIEN) and RTT increment interrupt(RTTINCIEN) are used.
However, the controller doesn't need to do it.

* RTT driver disables interrupts on _probe().
  ALMIEN and RTTINCIEN bits in RTT_MR are cleared to disable RTT
  interrupts.
  (drivers/rtc/rtc-at91sam9.c)
  /* disable all interrupts (same as on shutdown path) */
  mr &= ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN);
  rtt_writel(rtc, MR, mr);

* All RTT interrupts are disabled by default.
  Reset value of ALMIEN and RTTINCIEN bits are 0.
  Please note that all devices have same RTT registers.
  AT91RM9G45
  AT91SAM9RL
  AT91SAM9260
  AT91SAM9261
  AT91SAM9263
  AT91SAM9G20

So, aic_common_rtt_irq_fixup() helper can be removed.
And aic_common_irq_fixup() also can be removed because no driver
calls it any more.

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 | 45 ----------------------------------
 drivers/irqchip/irq-atmel-aic-common.h |  4 ---
 drivers/irqchip/irq-atmel-aic.c        | 22 -----------------
 3 files changed, 71 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index f8261be..ef2c619 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -139,51 +139,6 @@ static void __init aic_common_ext_irq_of_init(struct irq_domain *domain)
 	}
 }
 
-#define AT91_RTT_MR		0x00			/* Real-time Mode Register */
-#define AT91_RTT_ALMIEN		(1 << 16)		/* Alarm Interrupt Enable */
-#define AT91_RTT_RTTINCIEN	(1 << 17)		/* Real Time Timer Increment Interrupt Enable */
-
-void __init aic_common_rtt_irq_fixup(struct device_node *root)
-{
-	struct device_node *np;
-	void __iomem *regs;
-
-	/*
-	 * The at91sam9263 SoC has 2 instances of the RTT block, hence we
-	 * iterate over the DT to find each occurrence.
-	 */
-	for_each_compatible_node(np, NULL, "atmel,at91sam9260-rtt") {
-		regs = of_iomap(np, 0);
-		if (!regs)
-			continue;
-
-		writel(readl(regs + AT91_RTT_MR) &
-		       ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN),
-		       regs + AT91_RTT_MR);
-
-		iounmap(regs);
-	}
-}
-
-void __init aic_common_irq_fixup(const struct of_device_id *matches)
-{
-	struct device_node *root = of_find_node_by_path("/");
-	const struct of_device_id *match;
-
-	if (!root)
-		return;
-
-	match = of_match_node(matches, root);
-	of_node_put(root);
-
-	if (match) {
-		void (*fixup)(struct device_node *) = match->data;
-		fixup(root);
-	}
-
-	of_node_put(root);
-}
-
 struct irq_domain *__init aic_common_of_init(struct device_node *node,
 					     const struct irq_domain_ops *ops,
 					     const char *name, int nirqs)
diff --git a/drivers/irqchip/irq-atmel-aic-common.h b/drivers/irqchip/irq-atmel-aic-common.h
index 9027f15..c178557 100644
--- a/drivers/irqchip/irq-atmel-aic-common.h
+++ b/drivers/irqchip/irq-atmel-aic-common.h
@@ -32,8 +32,4 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 					     const struct irq_domain_ops *ops,
 					     const char *name, int nirqs);
 
-void __init aic_common_rtt_irq_fixup(struct device_node *root);
-
-void __init aic_common_irq_fixup(const struct of_device_id *matches);
-
 #endif /* __IRQ_ATMEL_AIC_COMMON_H */
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index 4c60224..6fcd680 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -209,26 +209,6 @@ static const struct irq_domain_ops aic_irq_ops = {
 	.xlate	= aic_irq_domain_xlate,
 };
 
-static void __init at91sam9260_aic_irq_fixup(struct device_node *root)
-{
-	aic_common_rtt_irq_fixup(root);
-}
-
-static void __init at91sam9g45_aic_irq_fixup(struct device_node *root)
-{
-	aic_common_rtt_irq_fixup(root);
-}
-
-static const struct of_device_id aic_irq_fixups[] __initconst = {
-	{ .compatible = "atmel,at91sam9g45", .data = at91sam9g45_aic_irq_fixup },
-	{ .compatible = "atmel,at91sam9rl", .data = at91sam9g45_aic_irq_fixup },
-	{ .compatible = "atmel,at91sam9260", .data = at91sam9260_aic_irq_fixup },
-	{ .compatible = "atmel,at91sam9261", .data = at91sam9260_aic_irq_fixup },
-	{ .compatible = "atmel,at91sam9263", .data = at91sam9260_aic_irq_fixup },
-	{ .compatible = "atmel,at91sam9g20", .data = at91sam9260_aic_irq_fixup },
-	{ /* sentinel */ },
-};
-
 static int __init aic_of_init(struct device_node *node,
 			      struct device_node *parent)
 {
@@ -243,8 +223,6 @@ static int __init aic_of_init(struct device_node *node,
 	if (IS_ERR(domain))
 		return PTR_ERR(domain);
 
-	aic_common_irq_fixup(aic_irq_fixups);
-
 	aic_domain = domain;
 	gc = irq_get_domain_generic_chip(domain, 0);
 
-- 
2.6.4

Re: [PATCH 03/19] irqchip: atmel-aic: clean up RTT interrupt code

[Boris Brezillon]

On Mon, 4 Jan 2016 13:28:27 +0900
Milo Kim <milo.kim@ti.com> wrote:

> Atmel AIC disables RTT(Real Time Timer) interrupt on initialization.
> Alarm interrupt(ALMIEN) and RTT increment interrupt(RTTINCIEN) are used.
> However, the controller doesn't need to do it.
> 
> * RTT driver disables interrupts on _probe().
>   ALMIEN and RTTINCIEN bits in RTT_MR are cleared to disable RTT
>   interrupts.
>   (drivers/rtc/rtc-at91sam9.c)
>   /* disable all interrupts (same as on shutdown path) */
>   mr &= ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN);
>   rtt_writel(rtc, MR, mr);
> 
> * All RTT interrupts are disabled by default.
>   Reset value of ALMIEN and RTTINCIEN bits are 0.
>   Please note that all devices have same RTT registers.
>   AT91RM9G45
>   AT91SAM9RL
>   AT91SAM9260
>   AT91SAM9261
>   AT91SAM9263
>   AT91SAM9G20
> 
> So, aic_common_rtt_irq_fixup() helper can be removed.
> And aic_common_irq_fixup() also can be removed because no driver
> calls it any more.

Same as for patch 2, you cannot remove these functions without
impacting the system behavior.

> 
> 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 | 45 ----------------------------------
>  drivers/irqchip/irq-atmel-aic-common.h |  4 ---
>  drivers/irqchip/irq-atmel-aic.c        | 22 -----------------
>  3 files changed, 71 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
> index f8261be..ef2c619 100644
> --- a/drivers/irqchip/irq-atmel-aic-common.c
> +++ b/drivers/irqchip/irq-atmel-aic-common.c
> @@ -139,51 +139,6 @@ static void __init aic_common_ext_irq_of_init(struct irq_domain *domain)
>  	}
>  }
>  
> -#define AT91_RTT_MR		0x00			/* Real-time Mode Register */
> -#define AT91_RTT_ALMIEN		(1 << 16)		/* Alarm Interrupt Enable */
> -#define AT91_RTT_RTTINCIEN	(1 << 17)		/* Real Time Timer Increment Interrupt Enable */
> -
> -void __init aic_common_rtt_irq_fixup(struct device_node *root)
> -{
> -	struct device_node *np;
> -	void __iomem *regs;
> -
> -	/*
> -	 * The at91sam9263 SoC has 2 instances of the RTT block, hence we
> -	 * iterate over the DT to find each occurrence.
> -	 */
> -	for_each_compatible_node(np, NULL, "atmel,at91sam9260-rtt") {
> -		regs = of_iomap(np, 0);
> -		if (!regs)
> -			continue;
> -
> -		writel(readl(regs + AT91_RTT_MR) &
> -		       ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN),
> -		       regs + AT91_RTT_MR);
> -
> -		iounmap(regs);
> -	}
> -}
> -
> -void __init aic_common_irq_fixup(const struct of_device_id *matches)
> -{
> -	struct device_node *root = of_find_node_by_path("/");
> -	const struct of_device_id *match;
> -
> -	if (!root)
> -		return;
> -
> -	match = of_match_node(matches, root);
> -	of_node_put(root);
> -
> -	if (match) {
> -		void (*fixup)(struct device_node *) = match->data;
> -		fixup(root);
> -	}
> -
> -	of_node_put(root);
> -}
> -
>  struct irq_domain *__init aic_common_of_init(struct device_node *node,
>  					     const struct irq_domain_ops *ops,
>  					     const char *name, int nirqs)
> diff --git a/drivers/irqchip/irq-atmel-aic-common.h b/drivers/irqchip/irq-atmel-aic-common.h
> index 9027f15..c178557 100644
> --- a/drivers/irqchip/irq-atmel-aic-common.h
> +++ b/drivers/irqchip/irq-atmel-aic-common.h
> @@ -32,8 +32,4 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
>  					     const struct irq_domain_ops *ops,
>  					     const char *name, int nirqs);
>  
> -void __init aic_common_rtt_irq_fixup(struct device_node *root);
> -
> -void __init aic_common_irq_fixup(const struct of_device_id *matches);
> -
>  #endif /* __IRQ_ATMEL_AIC_COMMON_H */
> diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
> index 4c60224..6fcd680 100644
> --- a/drivers/irqchip/irq-atmel-aic.c
> +++ b/drivers/irqchip/irq-atmel-aic.c
> @@ -209,26 +209,6 @@ static const struct irq_domain_ops aic_irq_ops = {
>  	.xlate	= aic_irq_domain_xlate,
>  };
>  
> -static void __init at91sam9260_aic_irq_fixup(struct device_node *root)
> -{
> -	aic_common_rtt_irq_fixup(root);
> -}
> -
> -static void __init at91sam9g45_aic_irq_fixup(struct device_node *root)
> -{
> -	aic_common_rtt_irq_fixup(root);
> -}
> -
> -static const struct of_device_id aic_irq_fixups[] __initconst = {
> -	{ .compatible = "atmel,at91sam9g45", .data = at91sam9g45_aic_irq_fixup },
> -	{ .compatible = "atmel,at91sam9rl", .data = at91sam9g45_aic_irq_fixup },
> -	{ .compatible = "atmel,at91sam9260", .data = at91sam9260_aic_irq_fixup },
> -	{ .compatible = "atmel,at91sam9261", .data = at91sam9260_aic_irq_fixup },
> -	{ .compatible = "atmel,at91sam9263", .data = at91sam9260_aic_irq_fixup },
> -	{ .compatible = "atmel,at91sam9g20", .data = at91sam9260_aic_irq_fixup },
> -	{ /* sentinel */ },
> -};
> -
>  static int __init aic_of_init(struct device_node *node,
>  			      struct device_node *parent)
>  {
> @@ -243,8 +223,6 @@ static int __init aic_of_init(struct device_node *node,
>  	if (IS_ERR(domain))
>  		return PTR_ERR(domain);
>  
> -	aic_common_irq_fixup(aic_irq_fixups);
> -
>  	aic_domain = domain;
>  	gc = irq_get_domain_generic_chip(domain, 0);
>  



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH 04/19] irqchip: atmel-aic: replace magic numbers with named constant

[Milo Kim]

One AIC IRQ chip can have 32 interrupt source.
To enhance code readability, magic number is replaced with named constant,
'AIC_IRQS_PER_CHIP'.

aic_hw_init() initializes vector registers up to total number of
AIC interrupts. This magic number is replaced with NR_AIC_IRQS.

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 | 11 ++++++-----
 drivers/irqchip/irq-atmel-aic-common.h |  1 +
 drivers/irqchip/irq-atmel-aic.c        |  2 +-
 drivers/irqchip/irq-atmel-aic5.c       |  4 ++--
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index ef2c619..5effd52 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -135,7 +135,7 @@ static void __init aic_common_ext_irq_of_init(struct irq_domain *domain)
 		}
 
 		aic = gc->private;
-		aic->ext_irqs |= (1 << (hwirq % 32));
+		aic->ext_irqs |= (1 << (hwirq % AIC_IRQS_PER_CHIP));
 	}
 }
 
@@ -151,7 +151,7 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 	int ret;
 	int i;
 
-	nchips = DIV_ROUND_UP(nirqs, 32);
+	nchips = DIV_ROUND_UP(nirqs, AIC_IRQS_PER_CHIP);
 
 	reg_base = of_iomap(node, 0);
 	if (!reg_base)
@@ -163,13 +163,14 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 		goto err_iounmap;
 	}
 
-	domain = irq_domain_add_linear(node, nchips * 32, ops, aic);
+	domain = irq_domain_add_linear(node, nchips * AIC_IRQS_PER_CHIP, ops,
+				       aic);
 	if (!domain) {
 		ret = -ENOMEM;
 		goto err_free_aic;
 	}
 
-	ret = irq_alloc_domain_generic_chips(domain, 32, 1, name,
+	ret = irq_alloc_domain_generic_chips(domain, AIC_IRQS_PER_CHIP, 1, name,
 					     handle_fasteoi_irq,
 					     IRQ_NOREQUEST | IRQ_NOPROBE |
 					     IRQ_NOAUTOEN, 0, 0);
@@ -177,7 +178,7 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 		goto err_domain_remove;
 
 	for (i = 0; i < nchips; i++) {
-		gc = irq_get_domain_generic_chip(domain, i * 32);
+		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
 
 		gc->reg_base = reg_base;
 
diff --git a/drivers/irqchip/irq-atmel-aic-common.h b/drivers/irqchip/irq-atmel-aic-common.h
index c178557..4c0b471 100644
--- a/drivers/irqchip/irq-atmel-aic-common.h
+++ b/drivers/irqchip/irq-atmel-aic-common.h
@@ -16,6 +16,7 @@
 #ifndef __IRQ_ATMEL_AIC_COMMON_H
 #define __IRQ_ATMEL_AIC_COMMON_H
 
+#define AIC_IRQS_PER_CHIP	32
 
 int aic_common_set_type(struct irq_data *d, unsigned type, unsigned *val);
 
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index 6fcd680..c499949 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -164,7 +164,7 @@ static void __init aic_hw_init(struct irq_domain *domain)
 	irq_reg_writel(gc, 0xffffffff, AT91_AIC_IDCR);
 	irq_reg_writel(gc, 0xffffffff, AT91_AIC_ICCR);
 
-	for (i = 0; i < 32; i++)
+	for (i = 0; i < NR_AIC_IRQS; i++)
 		irq_reg_writel(gc, i, AT91_AIC_SVR(i));
 }
 
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 8b8f5e2..f5848c8 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -306,9 +306,9 @@ static int __init aic5_of_init(struct device_node *node,
 		return PTR_ERR(domain);
 
 	aic5_domain = domain;
-	nchips = aic5_domain->revmap_size / 32;
+	nchips = aic5_domain->revmap_size / AIC_IRQS_PER_CHIP;
 	for (i = 0; i < nchips; i++) {
-		gc = irq_get_domain_generic_chip(domain, i * 32);
+		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
 
 		gc->chip_types[0].regs.eoi = AT91_AIC5_EOICR;
 		gc->chip_types[0].chip.irq_mask = aic5_mask;
-- 
2.6.4

Re: [PATCH 04/19] irqchip: atmel-aic: replace magic numbers with named constant

[Boris Brezillon]

On Mon, 4 Jan 2016 13:28:28 +0900
Milo Kim <milo.kim@ti.com> wrote:

> One AIC IRQ chip can have 32 interrupt source.
> To enhance code readability, magic number is replaced with named constant,
> 'AIC_IRQS_PER_CHIP'.
> 
> aic_hw_init() initializes vector registers up to total number of
> AIC interrupts. This magic number is replaced with NR_AIC_IRQS.

Actually this is a limitation of the generic chip infrastructure
(the irq_chip_generic uses u32 fields to store its internal status, and
each irq is attributed a bit position, which explains the 32 irqs per
chip limit), so I think this macro should be defined in
include/linux/irq.h:

#define MAX_IRQS_PER_GENERIC_CHIP	32

> 
> 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 | 11 ++++++-----
>  drivers/irqchip/irq-atmel-aic-common.h |  1 +
>  drivers/irqchip/irq-atmel-aic.c        |  2 +-
>  drivers/irqchip/irq-atmel-aic5.c       |  4 ++--
>  4 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
> index ef2c619..5effd52 100644
> --- a/drivers/irqchip/irq-atmel-aic-common.c
> +++ b/drivers/irqchip/irq-atmel-aic-common.c
> @@ -135,7 +135,7 @@ static void __init aic_common_ext_irq_of_init(struct irq_domain *domain)
>  		}
>  
>  		aic = gc->private;
> -		aic->ext_irqs |= (1 << (hwirq % 32));
> +		aic->ext_irqs |= (1 << (hwirq % AIC_IRQS_PER_CHIP));
>  	}
>  }
>  
> @@ -151,7 +151,7 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
>  	int ret;
>  	int i;
>  
> -	nchips = DIV_ROUND_UP(nirqs, 32);
> +	nchips = DIV_ROUND_UP(nirqs, AIC_IRQS_PER_CHIP);
>  
>  	reg_base = of_iomap(node, 0);
>  	if (!reg_base)
> @@ -163,13 +163,14 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
>  		goto err_iounmap;
>  	}
>  
> -	domain = irq_domain_add_linear(node, nchips * 32, ops, aic);
> +	domain = irq_domain_add_linear(node, nchips * AIC_IRQS_PER_CHIP, ops,
> +				       aic);
>  	if (!domain) {
>  		ret = -ENOMEM;
>  		goto err_free_aic;
>  	}
>  
> -	ret = irq_alloc_domain_generic_chips(domain, 32, 1, name,
> +	ret = irq_alloc_domain_generic_chips(domain, AIC_IRQS_PER_CHIP, 1, name,
>  					     handle_fasteoi_irq,
>  					     IRQ_NOREQUEST | IRQ_NOPROBE |
>  					     IRQ_NOAUTOEN, 0, 0);
> @@ -177,7 +178,7 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
>  		goto err_domain_remove;
>  
>  	for (i = 0; i < nchips; i++) {
> -		gc = irq_get_domain_generic_chip(domain, i * 32);
> +		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
>  
>  		gc->reg_base = reg_base;
>  
> diff --git a/drivers/irqchip/irq-atmel-aic-common.h b/drivers/irqchip/irq-atmel-aic-common.h
> index c178557..4c0b471 100644
> --- a/drivers/irqchip/irq-atmel-aic-common.h
> +++ b/drivers/irqchip/irq-atmel-aic-common.h
> @@ -16,6 +16,7 @@
>  #ifndef __IRQ_ATMEL_AIC_COMMON_H
>  #define __IRQ_ATMEL_AIC_COMMON_H
>  
> +#define AIC_IRQS_PER_CHIP	32
>  
>  int aic_common_set_type(struct irq_data *d, unsigned type, unsigned *val);
>  
> diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
> index 6fcd680..c499949 100644
> --- a/drivers/irqchip/irq-atmel-aic.c
> +++ b/drivers/irqchip/irq-atmel-aic.c
> @@ -164,7 +164,7 @@ static void __init aic_hw_init(struct irq_domain *domain)
>  	irq_reg_writel(gc, 0xffffffff, AT91_AIC_IDCR);
>  	irq_reg_writel(gc, 0xffffffff, AT91_AIC_ICCR);
>  
> -	for (i = 0; i < 32; i++)
> +	for (i = 0; i < NR_AIC_IRQS; i++)
>  		irq_reg_writel(gc, i, AT91_AIC_SVR(i));
>  }
>  
> diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
> index 8b8f5e2..f5848c8 100644
> --- a/drivers/irqchip/irq-atmel-aic5.c
> +++ b/drivers/irqchip/irq-atmel-aic5.c
> @@ -306,9 +306,9 @@ static int __init aic5_of_init(struct device_node *node,
>  		return PTR_ERR(domain);
>  
>  	aic5_domain = domain;
> -	nchips = aic5_domain->revmap_size / 32;
> +	nchips = aic5_domain->revmap_size / AIC_IRQS_PER_CHIP;
>  	for (i = 0; i < nchips; i++) {
> -		gc = irq_get_domain_generic_chip(domain, i * 32);
> +		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
>  
>  		gc->chip_types[0].regs.eoi = AT91_AIC5_EOICR;
>  		gc->chip_types[0].chip.irq_mask = aic5_mask;



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH 05/19] irqchip: atmel-aic: use simple constant to get number of interrupts per chip

[Milo Kim]

Number of interrupts per each chip is determined when IRQ controller
allocates IRQ chip by calling irq_alloc_domain_generic_chips().
This number is fixed by atmel-aic-common part. The value is 32.
So each AIC driver can use this value directly in IRQ chip operation.

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.c  | 2 +-
 drivers/irqchip/irq-atmel-aic5.c | 9 +++------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index c499949..f2c0fd9 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -188,7 +188,7 @@ static int aic_irq_domain_xlate(struct irq_domain *d,
 	if (ret)
 		return ret;
 
-	idx = intspec[0] / dgc->irqs_per_chip;
+	idx = intspec[0] / AIC_IRQS_PER_CHIP;
 	if (idx >= dgc->num_chips)
 		return -EINVAL;
 
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index f5848c8..50d540b 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -153,14 +153,13 @@ static int aic5_set_type(struct irq_data *d, unsigned type)
 static void aic5_suspend(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
-	struct irq_domain_chip_generic *dgc = domain->gc;
 	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	int i;
 	u32 mask;
 
 	irq_gc_lock(bgc);
-	for (i = 0; i < dgc->irqs_per_chip; i++) {
+	for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
 		mask = 1 << i;
 		if ((mask & gc->mask_cache) == (mask & gc->wake_active))
 			continue;
@@ -177,14 +176,13 @@ static void aic5_suspend(struct irq_data *d)
 static void aic5_resume(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
-	struct irq_domain_chip_generic *dgc = domain->gc;
 	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	int i;
 	u32 mask;
 
 	irq_gc_lock(bgc);
-	for (i = 0; i < dgc->irqs_per_chip; i++) {
+	for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
 		mask = 1 << i;
 		if ((mask & gc->mask_cache) == (mask & gc->wake_active))
 			continue;
@@ -201,13 +199,12 @@ static void aic5_resume(struct irq_data *d)
 static void aic5_pm_shutdown(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
-	struct irq_domain_chip_generic *dgc = domain->gc;
 	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	int i;
 
 	irq_gc_lock(bgc);
-	for (i = 0; i < dgc->irqs_per_chip; i++) {
+	for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
 		irq_reg_writel(bgc, i + gc->irq_base, AT91_AIC5_SSR);
 		irq_reg_writel(bgc, 1, AT91_AIC5_IDCR);
 		irq_reg_writel(bgc, 1, AT91_AIC5_ICCR);
-- 
2.6.4

Re: [PATCH 05/19] irqchip: atmel-aic: use simple constant to get number of interrupts per chip

[Boris Brezillon]

On Mon, 4 Jan 2016 13:28:29 +0900
Milo Kim <milo.kim@ti.com> wrote:

> Number of interrupts per each chip is determined when IRQ controller
> allocates IRQ chip by calling irq_alloc_domain_generic_chips().
> This number is fixed by atmel-aic-common part. The value is 32.
> So each AIC driver can use this value directly in IRQ chip operation.

Sorry, but I don't like the idea of hardcoding the number of irqs per
chip just to optimize some functions that are rarely called.
What if atmel creates a chip using the AIC but exposing less than 32
irqs? You'll have to change all those places.

Do you have a strong reason to do this kind of optimization?

> 
> 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.c  | 2 +-
>  drivers/irqchip/irq-atmel-aic5.c | 9 +++------
>  2 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
> index c499949..f2c0fd9 100644
> --- a/drivers/irqchip/irq-atmel-aic.c
> +++ b/drivers/irqchip/irq-atmel-aic.c
> @@ -188,7 +188,7 @@ static int aic_irq_domain_xlate(struct irq_domain *d,
>  	if (ret)
>  		return ret;
>  
> -	idx = intspec[0] / dgc->irqs_per_chip;
> +	idx = intspec[0] / AIC_IRQS_PER_CHIP;
>  	if (idx >= dgc->num_chips)
>  		return -EINVAL;
>  
> diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
> index f5848c8..50d540b 100644
> --- a/drivers/irqchip/irq-atmel-aic5.c
> +++ b/drivers/irqchip/irq-atmel-aic5.c
> @@ -153,14 +153,13 @@ static int aic5_set_type(struct irq_data *d, unsigned type)
>  static void aic5_suspend(struct irq_data *d)
>  {
>  	struct irq_domain *domain = d->domain;
> -	struct irq_domain_chip_generic *dgc = domain->gc;
>  	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
>  	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
>  	int i;
>  	u32 mask;
>  
>  	irq_gc_lock(bgc);
> -	for (i = 0; i < dgc->irqs_per_chip; i++) {
> +	for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
>  		mask = 1 << i;
>  		if ((mask & gc->mask_cache) == (mask & gc->wake_active))
>  			continue;
> @@ -177,14 +176,13 @@ static void aic5_suspend(struct irq_data *d)
>  static void aic5_resume(struct irq_data *d)
>  {
>  	struct irq_domain *domain = d->domain;
> -	struct irq_domain_chip_generic *dgc = domain->gc;
>  	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
>  	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
>  	int i;
>  	u32 mask;
>  
>  	irq_gc_lock(bgc);
> -	for (i = 0; i < dgc->irqs_per_chip; i++) {
> +	for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
>  		mask = 1 << i;
>  		if ((mask & gc->mask_cache) == (mask & gc->wake_active))
>  			continue;
> @@ -201,13 +199,12 @@ static void aic5_resume(struct irq_data *d)
>  static void aic5_pm_shutdown(struct irq_data *d)
>  {
>  	struct irq_domain *domain = d->domain;
> -	struct irq_domain_chip_generic *dgc = domain->gc;
>  	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
>  	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
>  	int i;
>  
>  	irq_gc_lock(bgc);
> -	for (i = 0; i < dgc->irqs_per_chip; i++) {
> +	for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
>  		irq_reg_writel(bgc, i + gc->irq_base, AT91_AIC5_SSR);
>  		irq_reg_writel(bgc, 1, AT91_AIC5_IDCR);
>  		irq_reg_writel(bgc, 1, AT91_AIC5_ICCR);



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

[Milo Kim]

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;
+
 static void aic_common_shutdown(struct irq_data *d)
 {
 	struct irq_chip_type *ct = irq_data_get_chip_type(d);
-- 
2.6.4

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

[Boris Brezillon]

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.

>  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

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

[Milo Kim]

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

[PATCH 07/19] irqchip: atmel-aic: make common IRQ domain translate function

[Milo Kim]

AIC5 and AIC have common translate interrupt specifiers.
HW IRQ number, interrupt type (flag) and interrupt priority.

For interrupt priority update, those systems look different handing.
AIC5 writes SSR (Source Select Register) first and read SMR (Source Mode
Register) to update interrupt priority.
On the other hand, AIC reads SMR and update the priority into same register.
New helper, 'aic_update_smr()' provides unified handling.

aic_is_ssr_used():
  Check whether IRQ chip uses SSR for IRQ selection or not.
  AIC5 uses this register. AIC does not have it.

aic_update_smr():
  AIC has separate register for mode selection, SMR.
  Offset value depends on IRQ source number. (SMR + hwirq * 4)
  On the other hands, AIC5 requires two register access, SSR and SMR.
  This helper selects the interrupt source on updating prority level or
  IRQ flag.

aic_irq_domain_xlate():
  Common domain translator handles interrupt specifiers as below.
  1. Get HW IRQ number, IRQ flag and priority from interrupt specifiers.
  2. If SSR is used, write hwirq into SSR for IRQ selection.
  3. Read value of SMR
  4. Update priority into SMR

  In case of AIC, total number of interrupts is 32. It means selected index
  is always zero in xlate(), so generic IRQ chip data can be retrieved from
  irq_get_domain_generic_chip(d, 0).

Then, both IRQ domains can use common operations, aic_irq_ops, so ops
argument in aic_common_of_init() can be removed.

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 | 94 +++++++++++++++++++++-------------
 drivers/irqchip/irq-atmel-aic-common.h | 10 ----
 drivers/irqchip/irq-atmel-aic.c        | 44 +---------------
 drivers/irqchip/irq-atmel-aic5.c       | 37 +------------
 4 files changed, 61 insertions(+), 124 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index f840165..94c9dad 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -129,6 +129,63 @@ static const struct aic_reg_offset aic5_regs = {
 
 static const struct aic_reg_offset *aic_reg_data;
 
+static inline bool aic_is_ssr_used(void)
+{
+	return aic_reg_data->ssr != AT91_INVALID_OFFSET;
+}
+
+static void aic_update_smr(struct irq_chip_generic *gc, int hwirq,
+			   u32 mask, u32 val)
+{
+	int reg = aic_reg_data->smr;
+	u32 tmp = 0;
+
+	if (aic_is_ssr_used())
+		irq_reg_writel(gc, hwirq, aic_reg_data->ssr);
+	else
+		reg += hwirq * 4;
+
+	tmp = irq_reg_readl(gc, reg);
+	tmp &= mask;
+	tmp |= val;
+
+	irq_reg_writel(gc, tmp, reg);
+}
+
+static int aic_irq_domain_xlate(struct irq_domain *d, struct device_node *node,
+				const u32 *intspec, unsigned int intsize,
+				irq_hw_number_t *out_hwirq,
+				unsigned int *out_type)
+{
+	struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0);
+	bool condition = (intsize < 3) ||
+			 (intspec[2] < AT91_AIC_IRQ_MIN_PRIORITY) ||
+			 (intspec[2] > AT91_AIC_IRQ_MAX_PRIORITY);
+
+	if (!gc || WARN_ON(condition))
+		return -EINVAL;
+
+	/*
+	 * intspec[0]: HW IRQ number
+	 * intspec[1]: IRQ flag
+	 * intspec[2]: IRQ priority
+	 */
+
+	*out_hwirq = intspec[0];
+	*out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
+
+	irq_gc_lock(gc);
+	aic_update_smr(gc, *out_hwirq, ~AT91_AIC_PRIOR, intspec[2]);
+	irq_gc_unlock(gc);
+
+	return 0;
+}
+
+static const struct irq_domain_ops aic_irq_ops = {
+	.map	= irq_map_generic_chip,
+	.xlate	= aic_irq_domain_xlate,
+};
+
 static void aic_common_shutdown(struct irq_data *d)
 {
 	struct irq_chip_type *ct = irq_data_get_chip_type(d);
@@ -171,38 +228,6 @@ int aic_common_set_type(struct irq_data *d, unsigned type, unsigned *val)
 	return 0;
 }
 
-int aic_common_set_priority(int priority, unsigned *val)
-{
-	if (priority < AT91_AIC_IRQ_MIN_PRIORITY ||
-	    priority > AT91_AIC_IRQ_MAX_PRIORITY)
-		return -EINVAL;
-
-	*val &= ~AT91_AIC_PRIOR;
-	*val |= priority;
-
-	return 0;
-}
-
-int aic_common_irq_domain_xlate(struct irq_domain *d,
-				struct device_node *ctrlr,
-				const u32 *intspec,
-				unsigned int intsize,
-				irq_hw_number_t *out_hwirq,
-				unsigned int *out_type)
-{
-	if (WARN_ON(intsize < 3))
-		return -EINVAL;
-
-	if (WARN_ON((intspec[2] < AT91_AIC_IRQ_MIN_PRIORITY) ||
-		    (intspec[2] > AT91_AIC_IRQ_MAX_PRIORITY)))
-		return -EINVAL;
-
-	*out_hwirq = intspec[0];
-	*out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
-
-	return 0;
-}
-
 static void __init aic_common_ext_irq_of_init(struct irq_domain *domain)
 {
 	struct device_node *node = irq_domain_get_of_node(domain);
@@ -231,7 +256,6 @@ static void __init aic_common_ext_irq_of_init(struct irq_domain *domain)
 }
 
 struct irq_domain *__init aic_common_of_init(struct device_node *node,
-					     const struct irq_domain_ops *ops,
 					     const char *name, int nirqs)
 {
 	struct irq_chip_generic *gc;
@@ -254,8 +278,8 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 		goto err_iounmap;
 	}
 
-	domain = irq_domain_add_linear(node, nchips * AIC_IRQS_PER_CHIP, ops,
-				       aic);
+	domain = irq_domain_add_linear(node, nchips * AIC_IRQS_PER_CHIP,
+				       &aic_irq_ops, aic);
 	if (!domain) {
 		ret = -ENOMEM;
 		goto err_free_aic;
diff --git a/drivers/irqchip/irq-atmel-aic-common.h b/drivers/irqchip/irq-atmel-aic-common.h
index 4c0b471..e068349 100644
--- a/drivers/irqchip/irq-atmel-aic-common.h
+++ b/drivers/irqchip/irq-atmel-aic-common.h
@@ -20,17 +20,7 @@
 
 int aic_common_set_type(struct irq_data *d, unsigned type, unsigned *val);
 
-int aic_common_set_priority(int priority, unsigned *val);
-
-int aic_common_irq_domain_xlate(struct irq_domain *d,
-				struct device_node *ctrlr,
-				const u32 *intspec,
-				unsigned int intsize,
-				irq_hw_number_t *out_hwirq,
-				unsigned int *out_type);
-
 struct irq_domain *__init aic_common_of_init(struct device_node *node,
-					     const struct irq_domain_ops *ops,
 					     const char *name, int nirqs);
 
 #endif /* __IRQ_ATMEL_AIC_COMMON_H */
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index f2c0fd9..46ce3ca 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -168,47 +168,6 @@ static void __init aic_hw_init(struct irq_domain *domain)
 		irq_reg_writel(gc, i, AT91_AIC_SVR(i));
 }
 
-static int aic_irq_domain_xlate(struct irq_domain *d,
-				struct device_node *ctrlr,
-				const u32 *intspec, unsigned int intsize,
-				irq_hw_number_t *out_hwirq,
-				unsigned int *out_type)
-{
-	struct irq_domain_chip_generic *dgc = d->gc;
-	struct irq_chip_generic *gc;
-	unsigned smr;
-	int idx;
-	int ret;
-
-	if (!dgc)
-		return -EINVAL;
-
-	ret = aic_common_irq_domain_xlate(d, ctrlr, intspec, intsize,
-					  out_hwirq, out_type);
-	if (ret)
-		return ret;
-
-	idx = intspec[0] / AIC_IRQS_PER_CHIP;
-	if (idx >= dgc->num_chips)
-		return -EINVAL;
-
-	gc = dgc->gc[idx];
-
-	irq_gc_lock(gc);
-	smr = irq_reg_readl(gc, AT91_AIC_SMR(*out_hwirq));
-	ret = aic_common_set_priority(intspec[2], &smr);
-	if (!ret)
-		irq_reg_writel(gc, smr, AT91_AIC_SMR(*out_hwirq));
-	irq_gc_unlock(gc);
-
-	return ret;
-}
-
-static const struct irq_domain_ops aic_irq_ops = {
-	.map	= irq_map_generic_chip,
-	.xlate	= aic_irq_domain_xlate,
-};
-
 static int __init aic_of_init(struct device_node *node,
 			      struct device_node *parent)
 {
@@ -218,8 +177,7 @@ static int __init aic_of_init(struct device_node *node,
 	if (aic_domain)
 		return -EEXIST;
 
-	domain = aic_common_of_init(node, &aic_irq_ops, "atmel-aic",
-				    NR_AIC_IRQS);
+	domain = aic_common_of_init(node, "atmel-aic", NR_AIC_IRQS);
 	if (IS_ERR(domain))
 		return PTR_ERR(domain);
 
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 50d540b..ecaa7e0 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -248,40 +248,6 @@ static void __init aic5_hw_init(struct irq_domain *domain)
 	}
 }
 
-static int aic5_irq_domain_xlate(struct irq_domain *d,
-				 struct device_node *ctrlr,
-				 const u32 *intspec, unsigned int intsize,
-				 irq_hw_number_t *out_hwirq,
-				 unsigned int *out_type)
-{
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(d, 0);
-	unsigned smr;
-	int ret;
-
-	if (!bgc)
-		return -EINVAL;
-
-	ret = aic_common_irq_domain_xlate(d, ctrlr, intspec, intsize,
-					  out_hwirq, out_type);
-	if (ret)
-		return ret;
-
-	irq_gc_lock(bgc);
-	irq_reg_writel(bgc, *out_hwirq, AT91_AIC5_SSR);
-	smr = irq_reg_readl(bgc, AT91_AIC5_SMR);
-	ret = aic_common_set_priority(intspec[2], &smr);
-	if (!ret)
-		irq_reg_writel(bgc, intspec[2] | smr, AT91_AIC5_SMR);
-	irq_gc_unlock(bgc);
-
-	return ret;
-}
-
-static const struct irq_domain_ops aic5_irq_ops = {
-	.map	= irq_map_generic_chip,
-	.xlate	= aic5_irq_domain_xlate,
-};
-
 static int __init aic5_of_init(struct device_node *node,
 			       struct device_node *parent,
 			       int nirqs)
@@ -297,8 +263,7 @@ static int __init aic5_of_init(struct device_node *node,
 	if (aic5_domain)
 		return -EEXIST;
 
-	domain = aic_common_of_init(node, &aic5_irq_ops, "atmel-aic5",
-				    nirqs);
+	domain = aic_common_of_init(node, "atmel-aic5", nirqs);
 	if (IS_ERR(domain))
 		return PTR_ERR(domain);
 
-- 
2.6.4

[PATCH 08/19] irqchip: atmel-aic: add common mask and unmask functions

[Milo Kim]

AIC has one register access to enable/disable an interrupt.
AIC5 requires two register accesses - SSR and IECR/IDCR.
This patch unifies interrupt mask and unmask operations.

Mask and unmask operations are moved into aic_common_of_init().
AIC5 can have multiple IRQ chips, mask/unmask should be assigned per chip.
In case of AIC, it's also good because AIC has one IRQ chip.
So looping count is just one time to configure mask/unmask functions.

struct irq_domain *__init aic_common_of_init(struct device_node *node,
					     const char *name, int nirqs)
{
	...

	for (i = 0; i < nchips; i++) {
		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);

		...
		gc->chip_types[0].chip.irq_mask = aic_mask;
		gc->chip_types[0].chip.irq_unmask = aic_unmask;
		gc->private = &aic[i];
	}
}

In AIC, register configuration for enabling and disabling IRQ can be
replaced with irq_mask and irq_unmask. This is for using unified mask and
unmask functions (aic_mask and aic_unmask).

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 | 52 ++++++++++++++++++++++++++++++++++
 drivers/irqchip/irq-atmel-aic.c        |  4 ---
 drivers/irqchip/irq-atmel-aic5.c       | 36 -----------------------
 3 files changed, 52 insertions(+), 40 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index 94c9dad..533b3e9 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -193,6 +193,56 @@ static void aic_common_shutdown(struct irq_data *d)
 	ct->chip.irq_mask(d);
 }
 
+static void aic_mask(struct irq_data *d)
+{
+	struct irq_domain *domain = d->domain;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	u32 mask = d->mask;
+
+	/*
+	 * Disable interrupt. We always take the lock of the
+	 * first irq chip as all chips share the same registers.
+	 */
+	irq_gc_lock(bgc);
+
+	if (aic_is_ssr_used()) {
+		irq_reg_writel(gc, d->hwirq, aic_reg_data->ssr);
+		irq_reg_writel(gc, 1, aic_reg_data->idcr);
+	} else {
+		irq_reg_writel(gc, mask, aic_reg_data->idcr);
+	}
+
+	gc->mask_cache &= ~mask;
+
+	irq_gc_unlock(bgc);
+}
+
+static void aic_unmask(struct irq_data *d)
+{
+	struct irq_domain *domain = d->domain;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	u32 mask = d->mask;
+
+	/*
+	 * Enable interrupt. We always take the lock of the
+	 * first irq chip as all chips share the same registers.
+	 */
+	irq_gc_lock(bgc);
+
+	if (aic_is_ssr_used()) {
+		irq_reg_writel(gc, d->hwirq, aic_reg_data->ssr);
+		irq_reg_writel(gc, 1, aic_reg_data->iecr);
+	} else {
+		irq_reg_writel(gc, mask, aic_reg_data->iecr);
+	}
+
+	gc->mask_cache |= mask;
+
+	irq_gc_unlock(bgc);
+}
+
 int aic_common_set_type(struct irq_data *d, unsigned type, unsigned *val)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
@@ -303,6 +353,8 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 		gc->chip_types[0].chip.irq_eoi = irq_gc_eoi;
 		gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
 		gc->chip_types[0].chip.irq_shutdown = aic_common_shutdown;
+		gc->chip_types[0].chip.irq_mask = aic_mask;
+		gc->chip_types[0].chip.irq_unmask = aic_unmask;
 		gc->private = &aic[i];
 	}
 
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index 46ce3ca..df12249d 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -185,10 +185,6 @@ static int __init aic_of_init(struct device_node *node,
 	gc = irq_get_domain_generic_chip(domain, 0);
 
 	gc->chip_types[0].regs.eoi = AT91_AIC_EOICR;
-	gc->chip_types[0].regs.enable = AT91_AIC_IECR;
-	gc->chip_types[0].regs.disable = AT91_AIC_IDCR;
-	gc->chip_types[0].chip.irq_mask = irq_gc_mask_disable_reg;
-	gc->chip_types[0].chip.irq_unmask = irq_gc_unmask_enable_reg;
 	gc->chip_types[0].chip.irq_retrigger = aic_retrigger;
 	gc->chip_types[0].chip.irq_set_type = aic_set_type;
 	gc->chip_types[0].chip.irq_suspend = aic_suspend;
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index ecaa7e0..e610780 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -83,40 +83,6 @@ aic5_handle(struct pt_regs *regs)
 		handle_domain_irq(aic5_domain, irqnr, regs);
 }
 
-static void aic5_mask(struct irq_data *d)
-{
-	struct irq_domain *domain = d->domain;
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-
-	/*
-	 * Disable interrupt on AIC5. We always take the lock of the
-	 * first irq chip as all chips share the same registers.
-	 */
-	irq_gc_lock(bgc);
-	irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
-	irq_reg_writel(gc, 1, AT91_AIC5_IDCR);
-	gc->mask_cache &= ~d->mask;
-	irq_gc_unlock(bgc);
-}
-
-static void aic5_unmask(struct irq_data *d)
-{
-	struct irq_domain *domain = d->domain;
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-
-	/*
-	 * Enable interrupt on AIC5. We always take the lock of the
-	 * first irq chip as all chips share the same registers.
-	 */
-	irq_gc_lock(bgc);
-	irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
-	irq_reg_writel(gc, 1, AT91_AIC5_IECR);
-	gc->mask_cache |= d->mask;
-	irq_gc_unlock(bgc);
-}
-
 static int aic5_retrigger(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
@@ -273,8 +239,6 @@ static int __init aic5_of_init(struct device_node *node,
 		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
 
 		gc->chip_types[0].regs.eoi = AT91_AIC5_EOICR;
-		gc->chip_types[0].chip.irq_mask = aic5_mask;
-		gc->chip_types[0].chip.irq_unmask = aic5_unmask;
 		gc->chip_types[0].chip.irq_retrigger = aic5_retrigger;
 		gc->chip_types[0].chip.irq_set_type = aic5_set_type;
 		gc->chip_types[0].chip.irq_suspend = aic5_suspend;
-- 
2.6.4

Re: [PATCH 08/19] irqchip: atmel-aic: add common mask and unmask functions

[Boris Brezillon]

On Mon, 4 Jan 2016 13:28:32 +0900
Milo Kim <milo.kim@ti.com> wrote:

> AIC has one register access to enable/disable an interrupt.
> AIC5 requires two register accesses - SSR and IECR/IDCR.
> This patch unifies interrupt mask and unmask operations.
> 
> Mask and unmask operations are moved into aic_common_of_init().
> AIC5 can have multiple IRQ chips, mask/unmask should be assigned per chip.
> In case of AIC, it's also good because AIC has one IRQ chip.
> So looping count is just one time to configure mask/unmask functions.
> 
> struct irq_domain *__init aic_common_of_init(struct device_node *node,
> 					     const char *name, int nirqs)
> {
> 	...
> 
> 	for (i = 0; i < nchips; i++) {
> 		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
> 
> 		...
> 		gc->chip_types[0].chip.irq_mask = aic_mask;
> 		gc->chip_types[0].chip.irq_unmask = aic_unmask;
> 		gc->private = &aic[i];
> 	}
> }
> 
> In AIC, register configuration for enabling and disabling IRQ can be
> replaced with irq_mask and irq_unmask. This is for using unified mask and
> unmask functions (aic_mask and aic_unmask).
> 
> 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 | 52 ++++++++++++++++++++++++++++++++++
>  drivers/irqchip/irq-atmel-aic.c        |  4 ---
>  drivers/irqchip/irq-atmel-aic5.c       | 36 -----------------------
>  3 files changed, 52 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
> index 94c9dad..533b3e9 100644
> --- a/drivers/irqchip/irq-atmel-aic-common.c
> +++ b/drivers/irqchip/irq-atmel-aic-common.c
> @@ -193,6 +193,56 @@ static void aic_common_shutdown(struct irq_data *d)
>  	ct->chip.irq_mask(d);
>  }
>  
> +static void aic_mask(struct irq_data *d)
> +{
> +	struct irq_domain *domain = d->domain;
> +	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	u32 mask = d->mask;
> +
> +	/*
> +	 * Disable interrupt. We always take the lock of the
> +	 * first irq chip as all chips share the same registers.
> +	 */
> +	irq_gc_lock(bgc);
> +
> +	if (aic_is_ssr_used()) {
> +		irq_reg_writel(gc, d->hwirq, aic_reg_data->ssr);
> +		irq_reg_writel(gc, 1, aic_reg_data->idcr);
> +	} else {
> +		irq_reg_writel(gc, mask, aic_reg_data->idcr);
> +	}
> +
> +	gc->mask_cache &= ~mask;
> +
> +	irq_gc_unlock(bgc);
> +}
> +
> +static void aic_unmask(struct irq_data *d)
> +{
> +	struct irq_domain *domain = d->domain;
> +	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	u32 mask = d->mask;
> +
> +	/*
> +	 * Enable interrupt. We always take the lock of the
> +	 * first irq chip as all chips share the same registers.
> +	 */
> +	irq_gc_lock(bgc);
> +
> +	if (aic_is_ssr_used()) {
> +		irq_reg_writel(gc, d->hwirq, aic_reg_data->ssr);
> +		irq_reg_writel(gc, 1, aic_reg_data->iecr);
> +	} else {
> +		irq_reg_writel(gc, mask, aic_reg_data->iecr);
> +	}

In other words, you prefer to add extra conditional statements in the
critical irq path rather than keeping two different drivers for two IPs
that are not so similar.

Here is my opinion: if you want to get rid of the aic-common* files,
fine, but please keep 2 different drivers for the AIC and AIC5 IPs and
duplicate the common code in each driver.
I understand that code factorization is important (and this is exactly
why I created aic-common), but it's pointless to try to factorize things
that are completely different, and AIC and AIC5 fall in this case (look
at the number of aic_is_ssr_used() you're adding in your series).

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH 09/19] irqchip: atmel-aic: add common retrigger function

[Milo Kim]

AIC has one register access to retrigger an interrupt.
AIC5 requires two register accesses - SSR and ISCR.
This patch unifies interrupt retrigger operation.

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 | 21 +++++++++++++++++++++
 drivers/irqchip/irq-atmel-aic.c        | 13 -------------
 drivers/irqchip/irq-atmel-aic5.c       | 15 ---------------
 3 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index 533b3e9..ec93725 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -243,6 +243,26 @@ static void aic_unmask(struct irq_data *d)
 	irq_gc_unlock(bgc);
 }
 
+static int aic_retrigger(struct irq_data *d)
+{
+	struct irq_domain *domain = d->domain;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
+
+	/* Set interrupt */
+	irq_gc_lock(bgc);
+
+	if (aic_is_ssr_used()) {
+		irq_reg_writel(bgc, d->hwirq, aic_reg_data->ssr);
+		irq_reg_writel(bgc, 1, aic_reg_data->iscr);
+	} else {
+		irq_reg_writel(bgc, d->mask, aic_reg_data->iscr);
+	}
+
+	irq_gc_unlock(bgc);
+
+	return 0;
+}
+
 int aic_common_set_type(struct irq_data *d, unsigned type, unsigned *val)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
@@ -355,6 +375,7 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 		gc->chip_types[0].chip.irq_shutdown = aic_common_shutdown;
 		gc->chip_types[0].chip.irq_mask = aic_mask;
 		gc->chip_types[0].chip.irq_unmask = aic_unmask;
+		gc->chip_types[0].chip.irq_retrigger = aic_retrigger;
 		gc->private = &aic[i];
 	}
 
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index df12249d..c218716 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -74,18 +74,6 @@ aic_handle(struct pt_regs *regs)
 		handle_domain_irq(aic_domain, irqnr, regs);
 }
 
-static int aic_retrigger(struct irq_data *d)
-{
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-
-	/* Enable interrupt on AIC5 */
-	irq_gc_lock(gc);
-	irq_reg_writel(gc, d->mask, AT91_AIC_ISCR);
-	irq_gc_unlock(gc);
-
-	return 0;
-}
-
 static int aic_set_type(struct irq_data *d, unsigned type)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
@@ -185,7 +173,6 @@ static int __init aic_of_init(struct device_node *node,
 	gc = irq_get_domain_generic_chip(domain, 0);
 
 	gc->chip_types[0].regs.eoi = AT91_AIC_EOICR;
-	gc->chip_types[0].chip.irq_retrigger = aic_retrigger;
 	gc->chip_types[0].chip.irq_set_type = aic_set_type;
 	gc->chip_types[0].chip.irq_suspend = aic_suspend;
 	gc->chip_types[0].chip.irq_resume = aic_resume;
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index e610780..49d40b4 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -83,20 +83,6 @@ aic5_handle(struct pt_regs *regs)
 		handle_domain_irq(aic5_domain, irqnr, regs);
 }
 
-static int aic5_retrigger(struct irq_data *d)
-{
-	struct irq_domain *domain = d->domain;
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
-
-	/* Enable interrupt on AIC5 */
-	irq_gc_lock(bgc);
-	irq_reg_writel(bgc, d->hwirq, AT91_AIC5_SSR);
-	irq_reg_writel(bgc, 1, AT91_AIC5_ISCR);
-	irq_gc_unlock(bgc);
-
-	return 0;
-}
-
 static int aic5_set_type(struct irq_data *d, unsigned type)
 {
 	struct irq_domain *domain = d->domain;
@@ -239,7 +225,6 @@ static int __init aic5_of_init(struct device_node *node,
 		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
 
 		gc->chip_types[0].regs.eoi = AT91_AIC5_EOICR;
-		gc->chip_types[0].chip.irq_retrigger = aic5_retrigger;
 		gc->chip_types[0].chip.irq_set_type = aic5_set_type;
 		gc->chip_types[0].chip.irq_suspend = aic5_suspend;
 		gc->chip_types[0].chip.irq_resume = aic5_resume;
-- 
2.6.4

[PATCH 10/19] irqchip: atmel-aic: add common set_type function

[Milo Kim]

set_type() operation updates IRQ flag for selected IRQ.
By using aic_update_smr(), this operation can be unified.

Common set_type() handles interrupt type as below.
  1. Set register value corresponding to IRQ type.
  2. If SSR is used, write hwirq into SSR for IRQ selection
  3. Read value of SMR
  4. Update IRQ type into SMR

aic_update_smr() handles step 2, 3 and 4.

Please note that there are two helpers for getting IRQ chip generic data.
One is irq_data_get_irq_chip_data(d) named 'gc'.
The other is irq_get_domain_generic_chip(domain, 0) called 'bgc'.
'gc' is used for getting external IRQ configuration which is specified in
chip private data, 'aic'. Low level IRQ/falling IRQ detection is only
valid for external interrupts. So 'aic->ext_irqs' should be checked.
'bgc' is used for general operations like lock/unlocking IRQ data and
accessing AIC/AIC5 registers.

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 | 20 ++++++++++++--------
 drivers/irqchip/irq-atmel-aic-common.h |  2 --
 drivers/irqchip/irq-atmel-aic.c        | 17 -----------------
 drivers/irqchip/irq-atmel-aic5.c       | 19 -------------------
 4 files changed, 12 insertions(+), 46 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index ec93725..4c404f9 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -263,37 +263,40 @@ static int aic_retrigger(struct irq_data *d)
 	return 0;
 }
 
-int aic_common_set_type(struct irq_data *d, unsigned type, unsigned *val)
+static int aic_set_type(struct irq_data *d, unsigned int type)
 {
+	struct irq_domain *domain = d->domain;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	struct aic_chip_data *aic = gc->private;
-	unsigned aic_type;
+	u32 val;
 
 	switch (type) {
 	case IRQ_TYPE_LEVEL_HIGH:
-		aic_type = AT91_AIC_SRCTYPE_HIGH;
+		val = AT91_AIC_SRCTYPE_HIGH;
 		break;
 	case IRQ_TYPE_EDGE_RISING:
-		aic_type = AT91_AIC_SRCTYPE_RISING;
+		val = AT91_AIC_SRCTYPE_RISING;
 		break;
 	case IRQ_TYPE_LEVEL_LOW:
 		if (!(d->mask & aic->ext_irqs))
 			return -EINVAL;
 
-		aic_type = AT91_AIC_SRCTYPE_LOW;
+		val = AT91_AIC_SRCTYPE_LOW;
 		break;
 	case IRQ_TYPE_EDGE_FALLING:
 		if (!(d->mask & aic->ext_irqs))
 			return -EINVAL;
 
-		aic_type = AT91_AIC_SRCTYPE_FALLING;
+		val = AT91_AIC_SRCTYPE_FALLING;
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	*val &= ~AT91_AIC_SRCTYPE;
-	*val |= aic_type;
+	irq_gc_lock(bgc);
+	aic_update_smr(bgc, d->hwirq, ~AT91_AIC_SRCTYPE, val);
+	irq_gc_unlock(bgc);
 
 	return 0;
 }
@@ -376,6 +379,7 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 		gc->chip_types[0].chip.irq_mask = aic_mask;
 		gc->chip_types[0].chip.irq_unmask = aic_unmask;
 		gc->chip_types[0].chip.irq_retrigger = aic_retrigger;
+		gc->chip_types[0].chip.irq_set_type = aic_set_type;
 		gc->private = &aic[i];
 	}
 
diff --git a/drivers/irqchip/irq-atmel-aic-common.h b/drivers/irqchip/irq-atmel-aic-common.h
index e068349..bf721b8 100644
--- a/drivers/irqchip/irq-atmel-aic-common.h
+++ b/drivers/irqchip/irq-atmel-aic-common.h
@@ -18,8 +18,6 @@
 
 #define AIC_IRQS_PER_CHIP	32
 
-int aic_common_set_type(struct irq_data *d, unsigned type, unsigned *val);
-
 struct irq_domain *__init aic_common_of_init(struct device_node *node,
 					     const char *name, int nirqs);
 
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index c218716..9166733 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -74,22 +74,6 @@ aic_handle(struct pt_regs *regs)
 		handle_domain_irq(aic_domain, irqnr, regs);
 }
 
-static int aic_set_type(struct irq_data *d, unsigned type)
-{
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-	unsigned int smr;
-	int ret;
-
-	smr = irq_reg_readl(gc, AT91_AIC_SMR(d->hwirq));
-	ret = aic_common_set_type(d, type, &smr);
-	if (ret)
-		return ret;
-
-	irq_reg_writel(gc, smr, AT91_AIC_SMR(d->hwirq));
-
-	return 0;
-}
-
 #ifdef CONFIG_PM
 static void aic_suspend(struct irq_data *d)
 {
@@ -173,7 +157,6 @@ static int __init aic_of_init(struct device_node *node,
 	gc = irq_get_domain_generic_chip(domain, 0);
 
 	gc->chip_types[0].regs.eoi = AT91_AIC_EOICR;
-	gc->chip_types[0].chip.irq_set_type = aic_set_type;
 	gc->chip_types[0].chip.irq_suspend = aic_suspend;
 	gc->chip_types[0].chip.irq_resume = aic_resume;
 	gc->chip_types[0].chip.irq_pm_shutdown = aic_pm_shutdown;
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 49d40b4..972eba7 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -83,24 +83,6 @@ aic5_handle(struct pt_regs *regs)
 		handle_domain_irq(aic5_domain, irqnr, regs);
 }
 
-static int aic5_set_type(struct irq_data *d, unsigned type)
-{
-	struct irq_domain *domain = d->domain;
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
-	unsigned int smr;
-	int ret;
-
-	irq_gc_lock(bgc);
-	irq_reg_writel(bgc, d->hwirq, AT91_AIC5_SSR);
-	smr = irq_reg_readl(bgc, AT91_AIC5_SMR);
-	ret = aic_common_set_type(d, type, &smr);
-	if (!ret)
-		irq_reg_writel(bgc, smr, AT91_AIC5_SMR);
-	irq_gc_unlock(bgc);
-
-	return ret;
-}
-
 #ifdef CONFIG_PM
 static void aic5_suspend(struct irq_data *d)
 {
@@ -225,7 +207,6 @@ static int __init aic5_of_init(struct device_node *node,
 		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
 
 		gc->chip_types[0].regs.eoi = AT91_AIC5_EOICR;
-		gc->chip_types[0].chip.irq_set_type = aic5_set_type;
 		gc->chip_types[0].chip.irq_suspend = aic5_suspend;
 		gc->chip_types[0].chip.irq_resume = aic5_resume;
 		gc->chip_types[0].chip.irq_pm_shutdown = aic5_pm_shutdown;
-- 
2.6.4

[PATCH 11/19] irqchip: atmel-aic: add common PM IRQ chip operation

[Milo Kim]

To control IDCR, IECR and ICCR, AIC5 selects the number by checking
each mask bit. AIC provides simple register access.
This patch unifies suspend/resume/pm_shutdown operation.

aic_common_shutdown() is renamed as aic_irq_shutdown() to differentiate it
from aic_pm_shutdowm().

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 | 113 ++++++++++++++++++++++++++++++++-
 drivers/irqchip/irq-atmel-aic.c        |  39 ------------
 drivers/irqchip/irq-atmel-aic5.c       |  71 ---------------------
 3 files changed, 110 insertions(+), 113 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index 4c404f9..3a723f4 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -186,7 +186,7 @@ static const struct irq_domain_ops aic_irq_ops = {
 	.xlate	= aic_irq_domain_xlate,
 };
 
-static void aic_common_shutdown(struct irq_data *d)
+static void aic_irq_shutdown(struct irq_data *d)
 {
 	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 
@@ -301,6 +301,109 @@ static int aic_set_type(struct irq_data *d, unsigned int type)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+
+enum aic_pm_mode {
+	AIC_PM_SUSPEND,
+	AIC_PM_RESUME,
+};
+
+static void aic_pm_ctrl_ssr(struct irq_data *d, enum aic_pm_mode mode)
+{
+	struct irq_domain *domain = d->domain;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	u32 mask;
+	u32 which;
+	int i;
+
+	if (mode == AIC_PM_SUSPEND)
+		which = gc->wake_active;
+	else
+		which = gc->mask_cache;
+
+	irq_gc_lock(bgc);
+
+	for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
+		mask = 1 << i;
+		if ((mask & gc->mask_cache) == (mask & gc->wake_active))
+			continue;
+
+		irq_reg_writel(bgc, i + gc->irq_base, aic_reg_data->ssr);
+
+		if (mask & which)
+			irq_reg_writel(bgc, 1, aic_reg_data->iecr);
+		else
+			irq_reg_writel(bgc, 1, aic_reg_data->idcr);
+	}
+
+	irq_gc_unlock(bgc);
+}
+
+static void aic_pm_ctrl(struct irq_data *d, enum aic_pm_mode mode)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	u32 mask_idcr;
+	u32 mask_iecr;
+
+	if (mode == AIC_PM_SUSPEND) {
+		mask_idcr = gc->mask_cache;
+		mask_iecr = gc->wake_active;
+	} else {
+		mask_idcr = gc->wake_active;
+		mask_iecr = gc->mask_cache;
+	}
+
+	irq_gc_lock(gc);
+	irq_reg_writel(gc, mask_idcr, aic_reg_data->idcr);
+	irq_reg_writel(gc, mask_iecr, aic_reg_data->iecr);
+	irq_gc_unlock(gc);
+}
+
+static void aic_suspend(struct irq_data *d)
+{
+	if (aic_is_ssr_used())
+		aic_pm_ctrl_ssr(d, AIC_PM_SUSPEND);
+	else
+		aic_pm_ctrl(d, AIC_PM_SUSPEND);
+}
+
+static void aic_resume(struct irq_data *d)
+{
+	if (aic_is_ssr_used())
+		aic_pm_ctrl_ssr(d, AIC_PM_RESUME);
+	else
+		aic_pm_ctrl(d, AIC_PM_RESUME);
+}
+
+static void aic_pm_shutdown(struct irq_data *d)
+{
+	struct irq_domain *domain = d->domain;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	int i;
+
+	if (aic_is_ssr_used()) {
+		irq_gc_lock(bgc);
+		for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
+			irq_reg_writel(bgc, i + gc->irq_base, aic_reg_data->ssr);
+			irq_reg_writel(bgc, 1, aic_reg_data->idcr);
+			irq_reg_writel(bgc, 1, aic_reg_data->iccr);
+		}
+		irq_gc_unlock(bgc);
+	} else {
+		irq_gc_lock(gc);
+		irq_reg_writel(gc, 0xffffffff, aic_reg_data->idcr);
+		irq_reg_writel(gc, 0xffffffff, aic_reg_data->iccr);
+		irq_gc_unlock(gc);
+	}
+}
+#else
+#define aic_suspend	NULL
+#define aic_resume	NULL
+#define aic_pm_shutdown	NULL
+#endif /* CONFIG_PM */
+
 static void __init aic_common_ext_irq_of_init(struct irq_domain *domain)
 {
 	struct device_node *node = irq_domain_get_of_node(domain);
@@ -369,17 +472,21 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
 
 		gc->reg_base = reg_base;
-
 		gc->unused = 0;
 		gc->wake_enabled = ~0;
+
 		gc->chip_types[0].type = IRQ_TYPE_SENSE_MASK;
 		gc->chip_types[0].chip.irq_eoi = irq_gc_eoi;
 		gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
-		gc->chip_types[0].chip.irq_shutdown = aic_common_shutdown;
+		gc->chip_types[0].chip.irq_shutdown = aic_irq_shutdown;
 		gc->chip_types[0].chip.irq_mask = aic_mask;
 		gc->chip_types[0].chip.irq_unmask = aic_unmask;
 		gc->chip_types[0].chip.irq_retrigger = aic_retrigger;
 		gc->chip_types[0].chip.irq_set_type = aic_set_type;
+		gc->chip_types[0].chip.irq_suspend = aic_suspend;
+		gc->chip_types[0].chip.irq_resume = aic_resume;
+		gc->chip_types[0].chip.irq_pm_shutdown = aic_pm_shutdown;
+
 		gc->private = &aic[i];
 	}
 
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index 9166733..5f943b5 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -74,42 +74,6 @@ aic_handle(struct pt_regs *regs)
 		handle_domain_irq(aic_domain, irqnr, regs);
 }
 
-#ifdef CONFIG_PM
-static void aic_suspend(struct irq_data *d)
-{
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-
-	irq_gc_lock(gc);
-	irq_reg_writel(gc, gc->mask_cache, AT91_AIC_IDCR);
-	irq_reg_writel(gc, gc->wake_active, AT91_AIC_IECR);
-	irq_gc_unlock(gc);
-}
-
-static void aic_resume(struct irq_data *d)
-{
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-
-	irq_gc_lock(gc);
-	irq_reg_writel(gc, gc->wake_active, AT91_AIC_IDCR);
-	irq_reg_writel(gc, gc->mask_cache, AT91_AIC_IECR);
-	irq_gc_unlock(gc);
-}
-
-static void aic_pm_shutdown(struct irq_data *d)
-{
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-
-	irq_gc_lock(gc);
-	irq_reg_writel(gc, 0xffffffff, AT91_AIC_IDCR);
-	irq_reg_writel(gc, 0xffffffff, AT91_AIC_ICCR);
-	irq_gc_unlock(gc);
-}
-#else
-#define aic_suspend		NULL
-#define aic_resume		NULL
-#define aic_pm_shutdown		NULL
-#endif /* CONFIG_PM */
-
 static void __init aic_hw_init(struct irq_domain *domain)
 {
 	struct irq_chip_generic *gc = irq_get_domain_generic_chip(domain, 0);
@@ -157,9 +121,6 @@ static int __init aic_of_init(struct device_node *node,
 	gc = irq_get_domain_generic_chip(domain, 0);
 
 	gc->chip_types[0].regs.eoi = AT91_AIC_EOICR;
-	gc->chip_types[0].chip.irq_suspend = aic_suspend;
-	gc->chip_types[0].chip.irq_resume = aic_resume;
-	gc->chip_types[0].chip.irq_pm_shutdown = aic_pm_shutdown;
 
 	aic_hw_init(domain);
 	set_handle_irq(aic_handle);
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 972eba7..ec89178 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -83,74 +83,6 @@ aic5_handle(struct pt_regs *regs)
 		handle_domain_irq(aic5_domain, irqnr, regs);
 }
 
-#ifdef CONFIG_PM
-static void aic5_suspend(struct irq_data *d)
-{
-	struct irq_domain *domain = d->domain;
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-	int i;
-	u32 mask;
-
-	irq_gc_lock(bgc);
-	for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
-		mask = 1 << i;
-		if ((mask & gc->mask_cache) == (mask & gc->wake_active))
-			continue;
-
-		irq_reg_writel(bgc, i + gc->irq_base, AT91_AIC5_SSR);
-		if (mask & gc->wake_active)
-			irq_reg_writel(bgc, 1, AT91_AIC5_IECR);
-		else
-			irq_reg_writel(bgc, 1, AT91_AIC5_IDCR);
-	}
-	irq_gc_unlock(bgc);
-}
-
-static void aic5_resume(struct irq_data *d)
-{
-	struct irq_domain *domain = d->domain;
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-	int i;
-	u32 mask;
-
-	irq_gc_lock(bgc);
-	for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
-		mask = 1 << i;
-		if ((mask & gc->mask_cache) == (mask & gc->wake_active))
-			continue;
-
-		irq_reg_writel(bgc, i + gc->irq_base, AT91_AIC5_SSR);
-		if (mask & gc->mask_cache)
-			irq_reg_writel(bgc, 1, AT91_AIC5_IECR);
-		else
-			irq_reg_writel(bgc, 1, AT91_AIC5_IDCR);
-	}
-	irq_gc_unlock(bgc);
-}
-
-static void aic5_pm_shutdown(struct irq_data *d)
-{
-	struct irq_domain *domain = d->domain;
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-	int i;
-
-	irq_gc_lock(bgc);
-	for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
-		irq_reg_writel(bgc, i + gc->irq_base, AT91_AIC5_SSR);
-		irq_reg_writel(bgc, 1, AT91_AIC5_IDCR);
-		irq_reg_writel(bgc, 1, AT91_AIC5_ICCR);
-	}
-	irq_gc_unlock(bgc);
-}
-#else
-#define aic5_suspend		NULL
-#define aic5_resume		NULL
-#define aic5_pm_shutdown	NULL
-#endif /* CONFIG_PM */
-
 static void __init aic5_hw_init(struct irq_domain *domain)
 {
 	struct irq_chip_generic *gc = irq_get_domain_generic_chip(domain, 0);
@@ -207,9 +139,6 @@ static int __init aic5_of_init(struct device_node *node,
 		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
 
 		gc->chip_types[0].regs.eoi = AT91_AIC5_EOICR;
-		gc->chip_types[0].chip.irq_suspend = aic5_suspend;
-		gc->chip_types[0].chip.irq_resume = aic5_resume;
-		gc->chip_types[0].chip.irq_pm_shutdown = aic5_pm_shutdown;
 	}
 
 	aic5_hw_init(domain);
-- 
2.6.4

[PATCH 12/19] irqchip: atmel-aic: use EOI register data in aic_reg_data

[Milo Kim]

Structure 'aic_reg_data' has EOI data, chip type data can be moved to
common part.

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 | 1 +
 drivers/irqchip/irq-atmel-aic.c        | 2 --
 drivers/irqchip/irq-atmel-aic5.c       | 2 --
 3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index 3a723f4..777cf33 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -476,6 +476,7 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 		gc->wake_enabled = ~0;
 
 		gc->chip_types[0].type = IRQ_TYPE_SENSE_MASK;
+		gc->chip_types[0].regs.eoi = aic_reg_data->eoi;
 		gc->chip_types[0].chip.irq_eoi = irq_gc_eoi;
 		gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
 		gc->chip_types[0].chip.irq_shutdown = aic_irq_shutdown;
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index 5f943b5..bab93a0 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -120,8 +120,6 @@ static int __init aic_of_init(struct device_node *node,
 	aic_domain = domain;
 	gc = irq_get_domain_generic_chip(domain, 0);
 
-	gc->chip_types[0].regs.eoi = AT91_AIC_EOICR;
-
 	aic_hw_init(domain);
 	set_handle_irq(aic_handle);
 
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index ec89178..b314aa4 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -137,8 +137,6 @@ static int __init aic5_of_init(struct device_node *node,
 	nchips = aic5_domain->revmap_size / AIC_IRQS_PER_CHIP;
 	for (i = 0; i < nchips; i++) {
 		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
-
-		gc->chip_types[0].regs.eoi = AT91_AIC5_EOICR;
 	}
 
 	aic5_hw_init(domain);
-- 
2.6.4

[PATCH 13/19] irqchip: atmel-aic: clean up irq_chip_generic

[Milo Kim]

Now, aic_common_of_init() handles all IRQ chip operations.
Remove unused code for getting irq_chip_generic data.

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.c  | 3 ---
 drivers/irqchip/irq-atmel-aic5.c | 8 --------
 2 files changed, 11 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index bab93a0..721ecb6 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -107,7 +107,6 @@ static void __init aic_hw_init(struct irq_domain *domain)
 static int __init aic_of_init(struct device_node *node,
 			      struct device_node *parent)
 {
-	struct irq_chip_generic *gc;
 	struct irq_domain *domain;
 
 	if (aic_domain)
@@ -118,8 +117,6 @@ static int __init aic_of_init(struct device_node *node,
 		return PTR_ERR(domain);
 
 	aic_domain = domain;
-	gc = irq_get_domain_generic_chip(domain, 0);
-
 	aic_hw_init(domain);
 	set_handle_irq(aic_handle);
 
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index b314aa4..ff487089 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -118,10 +118,7 @@ static int __init aic5_of_init(struct device_node *node,
 			       struct device_node *parent,
 			       int nirqs)
 {
-	struct irq_chip_generic *gc;
 	struct irq_domain *domain;
-	int nchips;
-	int i;
 
 	if (nirqs > NR_AIC5_IRQS)
 		return -EINVAL;
@@ -134,11 +131,6 @@ static int __init aic5_of_init(struct device_node *node,
 		return PTR_ERR(domain);
 
 	aic5_domain = domain;
-	nchips = aic5_domain->revmap_size / AIC_IRQS_PER_CHIP;
-	for (i = 0; i < nchips; i++) {
-		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
-	}
-
 	aic5_hw_init(domain);
 	set_handle_irq(aic5_handle);
 
-- 
2.6.4

[PATCH 14/19] irqchip: atmel-aic: add common HW init function

[Milo Kim]

AIC and AIC5 have common interrupt initialization process.
With aic_reg_data configuration, chip specific init functions can be
combined into one function, aic_common_hw_init().

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 | 42 ++++++++++++++++++++++++++++++++++
 drivers/irqchip/irq-atmel-aic.c        | 31 -------------------------
 drivers/irqchip/irq-atmel-aic5.c       | 32 --------------------------
 3 files changed, 42 insertions(+), 63 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index 777cf33..67f9204 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -24,6 +24,8 @@
 
 #include "irq-atmel-aic-common.h"
 
+#define NR_AIC_IRQS			32
+
 #define AT91_AIC_SMR_BASE		0
 #define AT91_AIC_SVR_BASE		0x80
 #define AT91_AIC_IVR			0x100
@@ -431,6 +433,45 @@ static void __init aic_common_ext_irq_of_init(struct irq_domain *domain)
 	}
 }
 
+static void __init aic_hw_init(struct irq_domain *domain)
+{
+	struct irq_chip_generic *gc = irq_get_domain_generic_chip(domain, 0);
+	int i;
+
+	/*
+	 * Perform 8 End Of Interrupt Command to make sure AIC
+	 * will not Lock out nIRQ
+	 */
+	for (i = 0; i < 8; i++)
+		irq_reg_writel(gc, 0, aic_reg_data->eoi);
+
+	/*
+	 * Spurious Interrupt ID in Spurious Vector Register.
+	 * When there is no current interrupt, the IRQ Vector Register
+	 * reads the value stored in AIC_SPU
+	 */
+	irq_reg_writel(gc, 0xffffffff, aic_reg_data->spu);
+
+	/* No debugging in AIC: Debug (Protect) Control Register */
+	irq_reg_writel(gc, 0, aic_reg_data->dcr);
+
+	/* Disable and clear all interrupts initially */
+	if (aic_is_ssr_used()) {
+		for (i = 0; i < domain->revmap_size; i++) {
+			irq_reg_writel(gc, i, aic_reg_data->ssr);
+			irq_reg_writel(gc, i, aic_reg_data->svr);
+			irq_reg_writel(gc, 1, aic_reg_data->idcr);
+			irq_reg_writel(gc, 1, aic_reg_data->iccr);
+		}
+	} else {
+		irq_reg_writel(gc, 0xffffffff, aic_reg_data->idcr);
+		irq_reg_writel(gc, 0xffffffff, aic_reg_data->iccr);
+
+		for (i = 0; i < NR_AIC_IRQS; i++)
+			irq_reg_writel(gc, i, aic_reg_data->svr + (i * 4));
+	}
+}
+
 struct irq_domain *__init aic_common_of_init(struct device_node *node,
 					     const char *name, int nirqs)
 {
@@ -492,6 +533,7 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 	}
 
 	aic_common_ext_irq_of_init(domain);
+	aic_hw_init(domain);
 
 	return domain;
 
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index 721ecb6..ef2cfb8 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -74,36 +74,6 @@ aic_handle(struct pt_regs *regs)
 		handle_domain_irq(aic_domain, irqnr, regs);
 }
 
-static void __init aic_hw_init(struct irq_domain *domain)
-{
-	struct irq_chip_generic *gc = irq_get_domain_generic_chip(domain, 0);
-	int i;
-
-	/*
-	 * Perform 8 End Of Interrupt Command to make sure AIC
-	 * will not Lock out nIRQ
-	 */
-	for (i = 0; i < 8; i++)
-		irq_reg_writel(gc, 0, AT91_AIC_EOICR);
-
-	/*
-	 * Spurious Interrupt ID in Spurious Vector Register.
-	 * When there is no current interrupt, the IRQ Vector Register
-	 * reads the value stored in AIC_SPU
-	 */
-	irq_reg_writel(gc, 0xffffffff, AT91_AIC_SPU);
-
-	/* No debugging in AIC: Debug (Protect) Control Register */
-	irq_reg_writel(gc, 0, AT91_AIC_DCR);
-
-	/* Disable and clear all interrupts initially */
-	irq_reg_writel(gc, 0xffffffff, AT91_AIC_IDCR);
-	irq_reg_writel(gc, 0xffffffff, AT91_AIC_ICCR);
-
-	for (i = 0; i < NR_AIC_IRQS; i++)
-		irq_reg_writel(gc, i, AT91_AIC_SVR(i));
-}
-
 static int __init aic_of_init(struct device_node *node,
 			      struct device_node *parent)
 {
@@ -117,7 +87,6 @@ static int __init aic_of_init(struct device_node *node,
 		return PTR_ERR(domain);
 
 	aic_domain = domain;
-	aic_hw_init(domain);
 	set_handle_irq(aic_handle);
 
 	return 0;
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index ff487089..4551bf6 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -83,37 +83,6 @@ aic5_handle(struct pt_regs *regs)
 		handle_domain_irq(aic5_domain, irqnr, regs);
 }
 
-static void __init aic5_hw_init(struct irq_domain *domain)
-{
-	struct irq_chip_generic *gc = irq_get_domain_generic_chip(domain, 0);
-	int i;
-
-	/*
-	 * Perform 8 End Of Interrupt Command to make sure AIC
-	 * will not Lock out nIRQ
-	 */
-	for (i = 0; i < 8; i++)
-		irq_reg_writel(gc, 0, AT91_AIC5_EOICR);
-
-	/*
-	 * Spurious Interrupt ID in Spurious Vector Register.
-	 * When there is no current interrupt, the IRQ Vector Register
-	 * reads the value stored in AIC_SPU
-	 */
-	irq_reg_writel(gc, 0xffffffff, AT91_AIC5_SPU);
-
-	/* No debugging in AIC: Debug (Protect) Control Register */
-	irq_reg_writel(gc, 0, AT91_AIC5_DCR);
-
-	/* Disable and clear all interrupts initially */
-	for (i = 0; i < domain->revmap_size; i++) {
-		irq_reg_writel(gc, i, AT91_AIC5_SSR);
-		irq_reg_writel(gc, i, AT91_AIC5_SVR);
-		irq_reg_writel(gc, 1, AT91_AIC5_IDCR);
-		irq_reg_writel(gc, 1, AT91_AIC5_ICCR);
-	}
-}
-
 static int __init aic5_of_init(struct device_node *node,
 			       struct device_node *parent,
 			       int nirqs)
@@ -131,7 +100,6 @@ static int __init aic5_of_init(struct device_node *node,
 		return PTR_ERR(domain);
 
 	aic5_domain = domain;
-	aic5_hw_init(domain);
 	set_handle_irq(aic5_handle);
 
 	return 0;
-- 
2.6.4

[PATCH 15/19] irqchip: atmel-aic: add common interrupt handler

[Milo Kim]

AIC and AIC5 have same interrupt handling process.
  1. Read IVR (Interrupt Vector Register) to get a HW IRQ number.
  2. Read ISR (Interrupt Status Register) to get current IRQ source number.
  3. Indicate the interrupt handling is complete if no interrupt condition.
     Otherwise, handle current interrupt.

With aic_reg_data configuration, two handlers can be combined.
And irq_domain also can be moved to common part.

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 | 24 ++++++++++++++++++++++++
 drivers/irqchip/irq-atmel-aic.c        | 25 -------------------------
 drivers/irqchip/irq-atmel-aic5.c       | 24 ------------------------
 3 files changed, 24 insertions(+), 49 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index 67f9204..3d8cc8d 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -17,11 +17,15 @@
 #include <linux/errno.h>
 #include <linux/io.h>
 #include <linux/irq.h>
+#include <linux/irqdesc.h>
 #include <linux/irqdomain.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/slab.h>
 
+#include <asm/exception.h>
+#include <asm/mach/irq.h>
+
 #include "irq-atmel-aic-common.h"
 
 #define NR_AIC_IRQS			32
@@ -129,8 +133,23 @@ static const struct aic_reg_offset aic5_regs = {
 	.isr	= AT91_AIC5_ISR,
 };
 
+static struct irq_domain *aic_domain;
 static const struct aic_reg_offset *aic_reg_data;
 
+static asmlinkage void __exception_irq_entry
+aic_handle(struct pt_regs *regs)
+{
+	struct irq_chip_generic *gc = irq_get_domain_generic_chip(aic_domain,
+								  0);
+	u32 hwirq = irq_reg_readl(gc, aic_reg_data->ivr);
+	u32 status = irq_reg_readl(gc, aic_reg_data->isr);
+
+	if (!status)
+		irq_reg_writel(gc, 0, aic_reg_data->eoi);
+	else
+		handle_domain_irq(aic_domain, hwirq, regs);
+}
+
 static inline bool aic_is_ssr_used(void)
 {
 	return aic_reg_data->ssr != AT91_INVALID_OFFSET;
@@ -483,6 +502,9 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 	int ret;
 	int i;
 
+	if (aic_domain)
+		return ERR_PTR(-EEXIST);
+
 	nchips = DIV_ROUND_UP(nirqs, AIC_IRQS_PER_CHIP);
 
 	reg_base = of_iomap(node, 0);
@@ -532,8 +554,10 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 		gc->private = &aic[i];
 	}
 
+	aic_domain = domain;
 	aic_common_ext_irq_of_init(domain);
 	aic_hw_init(domain);
+	set_handle_irq(aic_handle);
 
 	return domain;
 
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index ef2cfb8..44cedce 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -55,40 +55,15 @@
 #define AT91_AIC_SPU			0x134
 #define AT91_AIC_DCR			0x138
 
-static struct irq_domain *aic_domain;
-
-static asmlinkage void __exception_irq_entry
-aic_handle(struct pt_regs *regs)
-{
-	struct irq_domain_chip_generic *dgc = aic_domain->gc;
-	struct irq_chip_generic *gc = dgc->gc[0];
-	u32 irqnr;
-	u32 irqstat;
-
-	irqnr = irq_reg_readl(gc, AT91_AIC_IVR);
-	irqstat = irq_reg_readl(gc, AT91_AIC_ISR);
-
-	if (!irqstat)
-		irq_reg_writel(gc, 0, AT91_AIC_EOICR);
-	else
-		handle_domain_irq(aic_domain, irqnr, regs);
-}
-
 static int __init aic_of_init(struct device_node *node,
 			      struct device_node *parent)
 {
 	struct irq_domain *domain;
 
-	if (aic_domain)
-		return -EEXIST;
-
 	domain = aic_common_of_init(node, "atmel-aic", NR_AIC_IRQS);
 	if (IS_ERR(domain))
 		return PTR_ERR(domain);
 
-	aic_domain = domain;
-	set_handle_irq(aic_handle);
-
 	return 0;
 }
 IRQCHIP_DECLARE(at91rm9200_aic, "atmel,at91rm9200-aic", aic_of_init);
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 4551bf6..d09cefe 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -65,24 +65,6 @@
 #define AT91_AIC5_FFDR			0x54
 #define AT91_AIC5_FFSR			0x58
 
-static struct irq_domain *aic5_domain;
-
-static asmlinkage void __exception_irq_entry
-aic5_handle(struct pt_regs *regs)
-{
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(aic5_domain, 0);
-	u32 irqnr;
-	u32 irqstat;
-
-	irqnr = irq_reg_readl(bgc, AT91_AIC5_IVR);
-	irqstat = irq_reg_readl(bgc, AT91_AIC5_ISR);
-
-	if (!irqstat)
-		irq_reg_writel(bgc, 0, AT91_AIC5_EOICR);
-	else
-		handle_domain_irq(aic5_domain, irqnr, regs);
-}
-
 static int __init aic5_of_init(struct device_node *node,
 			       struct device_node *parent,
 			       int nirqs)
@@ -92,16 +74,10 @@ static int __init aic5_of_init(struct device_node *node,
 	if (nirqs > NR_AIC5_IRQS)
 		return -EINVAL;
 
-	if (aic5_domain)
-		return -EEXIST;
-
 	domain = aic_common_of_init(node, "atmel-aic5", nirqs);
 	if (IS_ERR(domain))
 		return PTR_ERR(domain);
 
-	aic5_domain = domain;
-	set_handle_irq(aic5_handle);
-
 	return 0;
 }
 
-- 
2.6.4

[PATCH 16/19] irqchip: atmel-aic: get total number of IRQs from device node

[Milo Kim]

aic_common_of_init() needs an argument for number of interrupts.
Argument, 'nirqs' can be ignored if device compatible string is used.
This patch provides unified way to get total number of interrupts.
Chip specific register data is assigned as well.

Use single constant total AIC IRQ number, 'NR_AT91RM9200_IRQS'.

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 | 38 ++++++++++++++++++++++++++++++----
 drivers/irqchip/irq-atmel-aic-common.h |  4 +---
 drivers/irqchip/irq-atmel-aic.c        |  2 +-
 drivers/irqchip/irq-atmel-aic5.c       | 37 +++++----------------------------
 4 files changed, 41 insertions(+), 40 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index 3d8cc8d..deec551 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -28,7 +28,11 @@
 
 #include "irq-atmel-aic-common.h"
 
-#define NR_AIC_IRQS			32
+#define AIC_IRQS_PER_CHIP		32
+#define NR_AT91RM9200_IRQS		32
+#define NR_SAMA5D2_IRQS			77
+#define NR_SAMA5D3_IRQS			48
+#define NR_SAMA5D4_IRQS			68
 
 #define AT91_AIC_SMR_BASE		0
 #define AT91_AIC_SVR_BASE		0x80
@@ -425,6 +429,30 @@ static void aic_pm_shutdown(struct irq_data *d)
 #define aic_pm_shutdown	NULL
 #endif /* CONFIG_PM */
 
+static int __init aic_get_num_chips(struct device_node *node)
+{
+	int nirqs;
+
+	/* Get total number of IRQs and configure register data */
+	if (of_device_is_compatible(node, "atmel,at91rm9200-aic")) {
+		nirqs = NR_AT91RM9200_IRQS;
+		aic_reg_data = &aic_regs;
+	} else if (of_device_is_compatible(node, "atmel,sama5d2-aic")) {
+		nirqs = NR_SAMA5D2_IRQS;
+		aic_reg_data = &aic5_regs;
+	} else if (of_device_is_compatible(node, "atmel,sama5d3-aic")) {
+		nirqs = NR_SAMA5D3_IRQS;
+		aic_reg_data = &aic5_regs;
+	} else if (of_device_is_compatible(node, "atmel,sama5d4-aic")) {
+		nirqs = NR_SAMA5D4_IRQS;
+		aic_reg_data = &aic5_regs;
+	} else {
+		return -EINVAL;
+	}
+
+	return DIV_ROUND_UP(nirqs, AIC_IRQS_PER_CHIP);
+}
+
 static void __init aic_common_ext_irq_of_init(struct irq_domain *domain)
 {
 	struct device_node *node = irq_domain_get_of_node(domain);
@@ -486,13 +514,13 @@ static void __init aic_hw_init(struct irq_domain *domain)
 		irq_reg_writel(gc, 0xffffffff, aic_reg_data->idcr);
 		irq_reg_writel(gc, 0xffffffff, aic_reg_data->iccr);
 
-		for (i = 0; i < NR_AIC_IRQS; i++)
+		for (i = 0; i < NR_AT91RM9200_IRQS; i++)
 			irq_reg_writel(gc, i, aic_reg_data->svr + (i * 4));
 	}
 }
 
 struct irq_domain *__init aic_common_of_init(struct device_node *node,
-					     const char *name, int nirqs)
+					     const char *name)
 {
 	struct irq_chip_generic *gc;
 	struct irq_domain *domain;
@@ -505,7 +533,9 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 	if (aic_domain)
 		return ERR_PTR(-EEXIST);
 
-	nchips = DIV_ROUND_UP(nirqs, AIC_IRQS_PER_CHIP);
+	nchips = aic_get_num_chips(node);
+	if (nchips < 0)
+		return ERR_PTR(-EINVAL);
 
 	reg_base = of_iomap(node, 0);
 	if (!reg_base)
diff --git a/drivers/irqchip/irq-atmel-aic-common.h b/drivers/irqchip/irq-atmel-aic-common.h
index bf721b8..4170133 100644
--- a/drivers/irqchip/irq-atmel-aic-common.h
+++ b/drivers/irqchip/irq-atmel-aic-common.h
@@ -16,9 +16,7 @@
 #ifndef __IRQ_ATMEL_AIC_COMMON_H
 #define __IRQ_ATMEL_AIC_COMMON_H
 
-#define AIC_IRQS_PER_CHIP	32
-
 struct irq_domain *__init aic_common_of_init(struct device_node *node,
-					     const char *name, int nirqs);
+					     const char *name);
 
 #endif /* __IRQ_ATMEL_AIC_COMMON_H */
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index 44cedce..980197f 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -60,7 +60,7 @@ static int __init aic_of_init(struct device_node *node,
 {
 	struct irq_domain *domain;
 
-	domain = aic_common_of_init(node, "atmel-aic", NR_AIC_IRQS);
+	domain = aic_common_of_init(node, "atmel-aic");
 	if (IS_ERR(domain))
 		return PTR_ERR(domain);
 
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index d09cefe..dcf4711 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -66,44 +66,17 @@
 #define AT91_AIC5_FFSR			0x58
 
 static int __init aic5_of_init(struct device_node *node,
-			       struct device_node *parent,
-			       int nirqs)
+			       struct device_node *parent)
 {
 	struct irq_domain *domain;
 
-	if (nirqs > NR_AIC5_IRQS)
-		return -EINVAL;
-
-	domain = aic_common_of_init(node, "atmel-aic5", nirqs);
+	domain = aic_common_of_init(node, "atmel-aic5");
 	if (IS_ERR(domain))
 		return PTR_ERR(domain);
 
 	return 0;
 }
 
-#define NR_SAMA5D2_IRQS		77
-
-static int __init sama5d2_aic5_of_init(struct device_node *node,
-				       struct device_node *parent)
-{
-	return aic5_of_init(node, parent, NR_SAMA5D2_IRQS);
-}
-IRQCHIP_DECLARE(sama5d2_aic5, "atmel,sama5d2-aic", sama5d2_aic5_of_init);
-
-#define NR_SAMA5D3_IRQS		48
-
-static int __init sama5d3_aic5_of_init(struct device_node *node,
-				       struct device_node *parent)
-{
-	return aic5_of_init(node, parent, NR_SAMA5D3_IRQS);
-}
-IRQCHIP_DECLARE(sama5d3_aic5, "atmel,sama5d3-aic", sama5d3_aic5_of_init);
-
-#define NR_SAMA5D4_IRQS		68
-
-static int __init sama5d4_aic5_of_init(struct device_node *node,
-				       struct device_node *parent)
-{
-	return aic5_of_init(node, parent, NR_SAMA5D4_IRQS);
-}
-IRQCHIP_DECLARE(sama5d4_aic5, "atmel,sama5d4-aic", sama5d4_aic5_of_init);
+IRQCHIP_DECLARE(sama5d2_aic5, "atmel,sama5d2-aic", aic5_of_init);
+IRQCHIP_DECLARE(sama5d3_aic5, "atmel,sama5d3-aic", aic5_of_init);
+IRQCHIP_DECLARE(sama5d4_aic5, "atmel,sama5d4-aic", aic5_of_init);
-- 
2.6.4

[PATCH 17/19] irqchip: atmel-aic: use unified IRQ chip initialization function

[Milo Kim]

Now, AIC and AIC5 have common IRQ chip initialization function.
All IRQ chips are declared in common part.
Return type is changed from irq_domain pointer to integer.
IRQ domain name is fixed as 'atmel-aic'.
Delete irq-atmel-aic-common.h because it is not used any more.

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 | 29 +++++++++++++++++------------
 drivers/irqchip/irq-atmel-aic-common.h | 22 ----------------------
 drivers/irqchip/irq-atmel-aic.c        | 15 ---------------
 drivers/irqchip/irq-atmel-aic5.c       | 18 ------------------
 4 files changed, 17 insertions(+), 67 deletions(-)
 delete mode 100644 drivers/irqchip/irq-atmel-aic-common.h

diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index deec551..cd89d635 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -15,10 +15,14 @@
  */
 
 #include <linux/errno.h>
+#include <linux/init.h>
 #include <linux/io.h>
 #include <linux/irq.h>
+#include <linux/irqchip.h>
 #include <linux/irqdesc.h>
 #include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/slab.h>
@@ -26,8 +30,6 @@
 #include <asm/exception.h>
 #include <asm/mach/irq.h>
 
-#include "irq-atmel-aic-common.h"
-
 #define AIC_IRQS_PER_CHIP		32
 #define NR_AT91RM9200_IRQS		32
 #define NR_SAMA5D2_IRQS			77
@@ -519,8 +521,8 @@ static void __init aic_hw_init(struct irq_domain *domain)
 	}
 }
 
-struct irq_domain *__init aic_common_of_init(struct device_node *node,
-					     const char *name)
+static int __init aic_of_init(struct device_node *node,
+			      struct device_node *parent)
 {
 	struct irq_chip_generic *gc;
 	struct irq_domain *domain;
@@ -531,15 +533,15 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 	int i;
 
 	if (aic_domain)
-		return ERR_PTR(-EEXIST);
+		return -EEXIST;
 
 	nchips = aic_get_num_chips(node);
 	if (nchips < 0)
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
 
 	reg_base = of_iomap(node, 0);
 	if (!reg_base)
-		return ERR_PTR(-ENOMEM);
+		return -ENOMEM;
 
 	aic = kcalloc(nchips, sizeof(*aic), GFP_KERNEL);
 	if (!aic) {
@@ -554,8 +556,8 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 		goto err_free_aic;
 	}
 
-	ret = irq_alloc_domain_generic_chips(domain, AIC_IRQS_PER_CHIP, 1, name,
-					     handle_fasteoi_irq,
+	ret = irq_alloc_domain_generic_chips(domain, AIC_IRQS_PER_CHIP, 1,
+					     "atmel-aic", handle_fasteoi_irq,
 					     IRQ_NOREQUEST | IRQ_NOPROBE |
 					     IRQ_NOAUTOEN, 0, 0);
 	if (ret)
@@ -589,7 +591,7 @@ struct irq_domain *__init aic_common_of_init(struct device_node *node,
 	aic_hw_init(domain);
 	set_handle_irq(aic_handle);
 
-	return domain;
+	return 0;
 
 err_domain_remove:
 	irq_domain_remove(domain);
@@ -599,6 +601,9 @@ err_free_aic:
 
 err_iounmap:
 	iounmap(reg_base);
-
-	return ERR_PTR(ret);
+	return ret;
 }
+IRQCHIP_DECLARE(at91rm9200_aic, "atmel,at91rm9200-aic", aic_of_init);
+IRQCHIP_DECLARE(sama5d2_aic5, "atmel,sama5d2-aic", aic_of_init);
+IRQCHIP_DECLARE(sama5d3_aic5, "atmel,sama5d3-aic", aic_of_init);
+IRQCHIP_DECLARE(sama5d4_aic5, "atmel,sama5d4-aic", aic_of_init);
diff --git a/drivers/irqchip/irq-atmel-aic-common.h b/drivers/irqchip/irq-atmel-aic-common.h
deleted file mode 100644
index 4170133..0000000
--- a/drivers/irqchip/irq-atmel-aic-common.h
+++ /dev/null
@@ -1,22 +0,0 @@
-/*
- * Atmel AT91 common AIC (Advanced Interrupt Controller) header file
- *
- *  Copyright (C) 2004 SAN People
- *  Copyright (C) 2004 ATMEL
- *  Copyright (C) Rick Bronson
- *  Copyright (C) 2014 Free Electrons
- *
- *  Author: Boris BREZILLON <boris.brezillon@free-electrons.com>
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2.  This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
- */
-
-#ifndef __IRQ_ATMEL_AIC_COMMON_H
-#define __IRQ_ATMEL_AIC_COMMON_H
-
-struct irq_domain *__init aic_common_of_init(struct device_node *node,
-					     const char *name);
-
-#endif /* __IRQ_ATMEL_AIC_COMMON_H */
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index 980197f..335a94e 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -31,8 +31,6 @@
 #include <asm/exception.h>
 #include <asm/mach/irq.h>
 
-#include "irq-atmel-aic-common.h"
-
 /* Number of irq lines managed by AIC */
 #define NR_AIC_IRQS	32
 
@@ -54,16 +52,3 @@
 #define AT91_AIC_EOICR			0x130
 #define AT91_AIC_SPU			0x134
 #define AT91_AIC_DCR			0x138
-
-static int __init aic_of_init(struct device_node *node,
-			      struct device_node *parent)
-{
-	struct irq_domain *domain;
-
-	domain = aic_common_of_init(node, "atmel-aic");
-	if (IS_ERR(domain))
-		return PTR_ERR(domain);
-
-	return 0;
-}
-IRQCHIP_DECLARE(at91rm9200_aic, "atmel,at91rm9200-aic", aic_of_init);
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index dcf4711..082fdde 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -31,8 +31,6 @@
 #include <asm/exception.h>
 #include <asm/mach/irq.h>
 
-#include "irq-atmel-aic-common.h"
-
 /* Number of irq lines managed by AIC */
 #define NR_AIC5_IRQS	128
 
@@ -64,19 +62,3 @@
 #define AT91_AIC5_FFER			0x50
 #define AT91_AIC5_FFDR			0x54
 #define AT91_AIC5_FFSR			0x58
-
-static int __init aic5_of_init(struct device_node *node,
-			       struct device_node *parent)
-{
-	struct irq_domain *domain;
-
-	domain = aic_common_of_init(node, "atmel-aic5");
-	if (IS_ERR(domain))
-		return PTR_ERR(domain);
-
-	return 0;
-}
-
-IRQCHIP_DECLARE(sama5d2_aic5, "atmel,sama5d2-aic", aic5_of_init);
-IRQCHIP_DECLARE(sama5d3_aic5, "atmel,sama5d3-aic", aic5_of_init);
-IRQCHIP_DECLARE(sama5d4_aic5, "atmel,sama5d4-aic", aic5_of_init);
-- 
2.6.4

[PATCH 18/19] irqchip: atmel-aic: use unified AIC driver

[Milo Kim]

AIC and AIC5 use same driver file, so IRQ chip specific driver files can
be deleted.
aic_common_ext_irq_of_init() is renamed as aic_ext_irq_of_init().
Fix checkpatch warning.

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/Makefile               |  4 +--
 drivers/irqchip/irq-atmel-aic-common.c | 10 +++---
 drivers/irqchip/irq-atmel-aic.c        | 54 ----------------------------
 drivers/irqchip/irq-atmel-aic5.c       | 64 ----------------------------------
 4 files changed, 7 insertions(+), 125 deletions(-)
 delete mode 100644 drivers/irqchip/irq-atmel-aic.c
 delete mode 100644 drivers/irqchip/irq-atmel-aic5.c

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 83d1fce..6e43333 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -28,8 +28,8 @@ obj-$(CONFIG_ARM_GIC_V3_ITS)		+= irq-gic-v3-its.o irq-gic-v3-its-pci-msi.o irq-g
 obj-$(CONFIG_HISILICON_IRQ_MBIGEN)	+= irq-mbigen.o
 obj-$(CONFIG_ARM_NVIC)			+= irq-nvic.o
 obj-$(CONFIG_ARM_VIC)			+= irq-vic.o
-obj-$(CONFIG_ATMEL_AIC_IRQ)		+= irq-atmel-aic-common.o irq-atmel-aic.o
-obj-$(CONFIG_ATMEL_AIC5_IRQ)	+= irq-atmel-aic-common.o irq-atmel-aic5.o
+obj-$(CONFIG_ATMEL_AIC_IRQ)		+= irq-atmel-aic-common.o
+obj-$(CONFIG_ATMEL_AIC5_IRQ)	+= irq-atmel-aic-common.o
 obj-$(CONFIG_I8259)			+= irq-i8259.o
 obj-$(CONFIG_IMGPDC_IRQ)		+= irq-imgpdc.o
 obj-$(CONFIG_IRQ_MIPS_CPU)		+= irq-mips-cpu.o
diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index cd89d635..1d3978e 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -1,6 +1,5 @@
 /*
- * Atmel AT91 common AIC (Advanced Interrupt Controller) code shared by
- * irq-atmel-aic and irq-atmel-aic5 drivers
+ * Atmel AIC (Advanced Interrupt Controller) Driver
  *
  *  Copyright (C) 2004 SAN People
  *  Copyright (C) 2004 ATMEL
@@ -413,7 +412,8 @@ static void aic_pm_shutdown(struct irq_data *d)
 	if (aic_is_ssr_used()) {
 		irq_gc_lock(bgc);
 		for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
-			irq_reg_writel(bgc, i + gc->irq_base, aic_reg_data->ssr);
+			irq_reg_writel(bgc, i + gc->irq_base,
+				       aic_reg_data->ssr);
 			irq_reg_writel(bgc, 1, aic_reg_data->idcr);
 			irq_reg_writel(bgc, 1, aic_reg_data->iccr);
 		}
@@ -455,7 +455,7 @@ static int __init aic_get_num_chips(struct device_node *node)
 	return DIV_ROUND_UP(nirqs, AIC_IRQS_PER_CHIP);
 }
 
-static void __init aic_common_ext_irq_of_init(struct irq_domain *domain)
+static void __init aic_ext_irq_of_init(struct irq_domain *domain)
 {
 	struct device_node *node = irq_domain_get_of_node(domain);
 	struct irq_chip_generic *gc;
@@ -587,7 +587,7 @@ static int __init aic_of_init(struct device_node *node,
 	}
 
 	aic_domain = domain;
-	aic_common_ext_irq_of_init(domain);
+	aic_ext_irq_of_init(domain);
 	aic_hw_init(domain);
 	set_handle_irq(aic_handle);
 
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
deleted file mode 100644
index 335a94e..0000000
--- a/drivers/irqchip/irq-atmel-aic.c
+++ /dev/null
@@ -1,54 +0,0 @@
-/*
- * Atmel AT91 AIC (Advanced Interrupt Controller) driver
- *
- *  Copyright (C) 2004 SAN People
- *  Copyright (C) 2004 ATMEL
- *  Copyright (C) Rick Bronson
- *  Copyright (C) 2014 Free Electrons
- *
- *  Author: Boris BREZILLON <boris.brezillon@free-electrons.com>
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2.  This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
- */
-
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/mm.h>
-#include <linux/bitmap.h>
-#include <linux/types.h>
-#include <linux/irq.h>
-#include <linux/irqchip.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/of_irq.h>
-#include <linux/irqdomain.h>
-#include <linux/err.h>
-#include <linux/slab.h>
-#include <linux/io.h>
-
-#include <asm/exception.h>
-#include <asm/mach/irq.h>
-
-/* Number of irq lines managed by AIC */
-#define NR_AIC_IRQS	32
-
-#define AT91_AIC_SMR(n)			((n) * 4)
-
-#define AT91_AIC_SVR(n)			(0x80 + ((n) * 4))
-#define AT91_AIC_IVR			0x100
-#define AT91_AIC_FVR			0x104
-#define AT91_AIC_ISR			0x108
-
-#define AT91_AIC_IPR			0x10c
-#define AT91_AIC_IMR			0x110
-#define AT91_AIC_CISR			0x114
-
-#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
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
deleted file mode 100644
index 082fdde..0000000
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * Atmel AT91 AIC5 (Advanced Interrupt Controller) driver
- *
- *  Copyright (C) 2004 SAN People
- *  Copyright (C) 2004 ATMEL
- *  Copyright (C) Rick Bronson
- *  Copyright (C) 2014 Free Electrons
- *
- *  Author: Boris BREZILLON <boris.brezillon@free-electrons.com>
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2.  This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
- */
-
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/mm.h>
-#include <linux/bitmap.h>
-#include <linux/types.h>
-#include <linux/irq.h>
-#include <linux/irqchip.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/of_irq.h>
-#include <linux/irqdomain.h>
-#include <linux/err.h>
-#include <linux/slab.h>
-#include <linux/io.h>
-
-#include <asm/exception.h>
-#include <asm/mach/irq.h>
-
-/* Number of irq lines managed by AIC */
-#define NR_AIC5_IRQS	128
-
-#define AT91_AIC5_SSR		0x0
-#define AT91_AIC5_INTSEL_MSK	(0x7f << 0)
-
-#define AT91_AIC5_SMR			0x4
-
-#define AT91_AIC5_SVR			0x8
-#define AT91_AIC5_IVR			0x10
-#define AT91_AIC5_FVR			0x14
-#define AT91_AIC5_ISR			0x18
-
-#define AT91_AIC5_IPR0			0x20
-#define AT91_AIC5_IPR1			0x24
-#define AT91_AIC5_IPR2			0x28
-#define AT91_AIC5_IPR3			0x2c
-#define AT91_AIC5_IMR			0x30
-#define AT91_AIC5_CISR			0x34
-
-#define AT91_AIC5_IECR			0x40
-#define AT91_AIC5_IDCR			0x44
-#define AT91_AIC5_ICCR			0x48
-#define AT91_AIC5_ISCR			0x4c
-#define AT91_AIC5_EOICR			0x38
-#define AT91_AIC5_SPU			0x3c
-#define AT91_AIC5_DCR			0x6c
-
-#define AT91_AIC5_FFER			0x50
-#define AT91_AIC5_FFDR			0x54
-#define AT91_AIC5_FFSR			0x58
-- 
2.6.4

[PATCH 19/19] irqchip: atmel-aic: rename AIC driver and fix Kconfig

[Milo Kim]

'irq-aic' is consolidated interrupt driver for Atmel SoCs.

Rename the driver file:
  irq-atmel-aic-common.c -> irq-aic.c

Kconfig:
  ATMEL_AIC5_IRQ is removed

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>
---
 arch/arm/mach-at91/Kconfig             |   2 +-
 drivers/irqchip/Kconfig                |   7 -
 drivers/irqchip/Makefile               |   3 +-
 drivers/irqchip/irq-aic.c              | 609 +++++++++++++++++++++++++++++++++
 drivers/irqchip/irq-atmel-aic-common.c | 609 ---------------------------------
 5 files changed, 611 insertions(+), 619 deletions(-)
 create mode 100644 drivers/irqchip/irq-aic.c
 delete mode 100644 drivers/irqchip/irq-atmel-aic-common.c

diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
index 23be2e4..315195c 100644
--- a/arch/arm/mach-at91/Kconfig
+++ b/arch/arm/mach-at91/Kconfig
@@ -122,7 +122,7 @@ config SOC_SAM_V7
 
 config SOC_SAMA5
 	bool
-	select ATMEL_AIC5_IRQ
+	select ATMEL_AIC_IRQ
 	select ATMEL_SDRAMC
 	select MEMORY
 	select SOC_SAM_V7
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index b5f5133..b926f79 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -67,13 +67,6 @@ config ATMEL_AIC_IRQ
 	select MULTI_IRQ_HANDLER
 	select SPARSE_IRQ
 
-config ATMEL_AIC5_IRQ
-	bool
-	select GENERIC_IRQ_CHIP
-	select IRQ_DOMAIN
-	select MULTI_IRQ_HANDLER
-	select SPARSE_IRQ
-
 config I8259
 	bool
 	select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 6e43333..8d6494f 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -28,8 +28,7 @@ obj-$(CONFIG_ARM_GIC_V3_ITS)		+= irq-gic-v3-its.o irq-gic-v3-its-pci-msi.o irq-g
 obj-$(CONFIG_HISILICON_IRQ_MBIGEN)	+= irq-mbigen.o
 obj-$(CONFIG_ARM_NVIC)			+= irq-nvic.o
 obj-$(CONFIG_ARM_VIC)			+= irq-vic.o
-obj-$(CONFIG_ATMEL_AIC_IRQ)		+= irq-atmel-aic-common.o
-obj-$(CONFIG_ATMEL_AIC5_IRQ)	+= irq-atmel-aic-common.o
+obj-$(CONFIG_ATMEL_AIC_IRQ)		+= irq-aic.o
 obj-$(CONFIG_I8259)			+= irq-i8259.o
 obj-$(CONFIG_IMGPDC_IRQ)		+= irq-imgpdc.o
 obj-$(CONFIG_IRQ_MIPS_CPU)		+= irq-mips-cpu.o
diff --git a/drivers/irqchip/irq-aic.c b/drivers/irqchip/irq-aic.c
new file mode 100644
index 0000000..1d3978e
--- /dev/null
+++ b/drivers/irqchip/irq-aic.c
@@ -0,0 +1,609 @@
+/*
+ * Atmel AIC (Advanced Interrupt Controller) Driver
+ *
+ *  Copyright (C) 2004 SAN People
+ *  Copyright (C) 2004 ATMEL
+ *  Copyright (C) Rick Bronson
+ *  Copyright (C) 2014 Free Electrons
+ *
+ *  Author: Boris BREZILLON <boris.brezillon@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqdesc.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#include <asm/exception.h>
+#include <asm/mach/irq.h>
+
+#define AIC_IRQS_PER_CHIP		32
+#define NR_AT91RM9200_IRQS		32
+#define NR_SAMA5D2_IRQS			77
+#define NR_SAMA5D3_IRQS			48
+#define NR_SAMA5D4_IRQS			68
+
+#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
+
+#define AT91_AIC_SRCTYPE		GENMASK(6, 5)
+#define AT91_AIC_SRCTYPE_LOW		(0 << 5)
+#define AT91_AIC_SRCTYPE_FALLING	(1 << 5)
+#define AT91_AIC_SRCTYPE_HIGH		(2 << 5)
+#define AT91_AIC_SRCTYPE_RISING		(3 << 5)
+
+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 struct irq_domain *aic_domain;
+static const struct aic_reg_offset *aic_reg_data;
+
+static asmlinkage void __exception_irq_entry
+aic_handle(struct pt_regs *regs)
+{
+	struct irq_chip_generic *gc = irq_get_domain_generic_chip(aic_domain,
+								  0);
+	u32 hwirq = irq_reg_readl(gc, aic_reg_data->ivr);
+	u32 status = irq_reg_readl(gc, aic_reg_data->isr);
+
+	if (!status)
+		irq_reg_writel(gc, 0, aic_reg_data->eoi);
+	else
+		handle_domain_irq(aic_domain, hwirq, regs);
+}
+
+static inline bool aic_is_ssr_used(void)
+{
+	return aic_reg_data->ssr != AT91_INVALID_OFFSET;
+}
+
+static void aic_update_smr(struct irq_chip_generic *gc, int hwirq,
+			   u32 mask, u32 val)
+{
+	int reg = aic_reg_data->smr;
+	u32 tmp = 0;
+
+	if (aic_is_ssr_used())
+		irq_reg_writel(gc, hwirq, aic_reg_data->ssr);
+	else
+		reg += hwirq * 4;
+
+	tmp = irq_reg_readl(gc, reg);
+	tmp &= mask;
+	tmp |= val;
+
+	irq_reg_writel(gc, tmp, reg);
+}
+
+static int aic_irq_domain_xlate(struct irq_domain *d, struct device_node *node,
+				const u32 *intspec, unsigned int intsize,
+				irq_hw_number_t *out_hwirq,
+				unsigned int *out_type)
+{
+	struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0);
+	bool condition = (intsize < 3) ||
+			 (intspec[2] < AT91_AIC_IRQ_MIN_PRIORITY) ||
+			 (intspec[2] > AT91_AIC_IRQ_MAX_PRIORITY);
+
+	if (!gc || WARN_ON(condition))
+		return -EINVAL;
+
+	/*
+	 * intspec[0]: HW IRQ number
+	 * intspec[1]: IRQ flag
+	 * intspec[2]: IRQ priority
+	 */
+
+	*out_hwirq = intspec[0];
+	*out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
+
+	irq_gc_lock(gc);
+	aic_update_smr(gc, *out_hwirq, ~AT91_AIC_PRIOR, intspec[2]);
+	irq_gc_unlock(gc);
+
+	return 0;
+}
+
+static const struct irq_domain_ops aic_irq_ops = {
+	.map	= irq_map_generic_chip,
+	.xlate	= aic_irq_domain_xlate,
+};
+
+static void aic_irq_shutdown(struct irq_data *d)
+{
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
+
+	ct->chip.irq_mask(d);
+}
+
+static void aic_mask(struct irq_data *d)
+{
+	struct irq_domain *domain = d->domain;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	u32 mask = d->mask;
+
+	/*
+	 * Disable interrupt. We always take the lock of the
+	 * first irq chip as all chips share the same registers.
+	 */
+	irq_gc_lock(bgc);
+
+	if (aic_is_ssr_used()) {
+		irq_reg_writel(gc, d->hwirq, aic_reg_data->ssr);
+		irq_reg_writel(gc, 1, aic_reg_data->idcr);
+	} else {
+		irq_reg_writel(gc, mask, aic_reg_data->idcr);
+	}
+
+	gc->mask_cache &= ~mask;
+
+	irq_gc_unlock(bgc);
+}
+
+static void aic_unmask(struct irq_data *d)
+{
+	struct irq_domain *domain = d->domain;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	u32 mask = d->mask;
+
+	/*
+	 * Enable interrupt. We always take the lock of the
+	 * first irq chip as all chips share the same registers.
+	 */
+	irq_gc_lock(bgc);
+
+	if (aic_is_ssr_used()) {
+		irq_reg_writel(gc, d->hwirq, aic_reg_data->ssr);
+		irq_reg_writel(gc, 1, aic_reg_data->iecr);
+	} else {
+		irq_reg_writel(gc, mask, aic_reg_data->iecr);
+	}
+
+	gc->mask_cache |= mask;
+
+	irq_gc_unlock(bgc);
+}
+
+static int aic_retrigger(struct irq_data *d)
+{
+	struct irq_domain *domain = d->domain;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
+
+	/* Set interrupt */
+	irq_gc_lock(bgc);
+
+	if (aic_is_ssr_used()) {
+		irq_reg_writel(bgc, d->hwirq, aic_reg_data->ssr);
+		irq_reg_writel(bgc, 1, aic_reg_data->iscr);
+	} else {
+		irq_reg_writel(bgc, d->mask, aic_reg_data->iscr);
+	}
+
+	irq_gc_unlock(bgc);
+
+	return 0;
+}
+
+static int aic_set_type(struct irq_data *d, unsigned int type)
+{
+	struct irq_domain *domain = d->domain;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct aic_chip_data *aic = gc->private;
+	u32 val;
+
+	switch (type) {
+	case IRQ_TYPE_LEVEL_HIGH:
+		val = AT91_AIC_SRCTYPE_HIGH;
+		break;
+	case IRQ_TYPE_EDGE_RISING:
+		val = AT91_AIC_SRCTYPE_RISING;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		if (!(d->mask & aic->ext_irqs))
+			return -EINVAL;
+
+		val = AT91_AIC_SRCTYPE_LOW;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		if (!(d->mask & aic->ext_irqs))
+			return -EINVAL;
+
+		val = AT91_AIC_SRCTYPE_FALLING;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	irq_gc_lock(bgc);
+	aic_update_smr(bgc, d->hwirq, ~AT91_AIC_SRCTYPE, val);
+	irq_gc_unlock(bgc);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+
+enum aic_pm_mode {
+	AIC_PM_SUSPEND,
+	AIC_PM_RESUME,
+};
+
+static void aic_pm_ctrl_ssr(struct irq_data *d, enum aic_pm_mode mode)
+{
+	struct irq_domain *domain = d->domain;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	u32 mask;
+	u32 which;
+	int i;
+
+	if (mode == AIC_PM_SUSPEND)
+		which = gc->wake_active;
+	else
+		which = gc->mask_cache;
+
+	irq_gc_lock(bgc);
+
+	for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
+		mask = 1 << i;
+		if ((mask & gc->mask_cache) == (mask & gc->wake_active))
+			continue;
+
+		irq_reg_writel(bgc, i + gc->irq_base, aic_reg_data->ssr);
+
+		if (mask & which)
+			irq_reg_writel(bgc, 1, aic_reg_data->iecr);
+		else
+			irq_reg_writel(bgc, 1, aic_reg_data->idcr);
+	}
+
+	irq_gc_unlock(bgc);
+}
+
+static void aic_pm_ctrl(struct irq_data *d, enum aic_pm_mode mode)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	u32 mask_idcr;
+	u32 mask_iecr;
+
+	if (mode == AIC_PM_SUSPEND) {
+		mask_idcr = gc->mask_cache;
+		mask_iecr = gc->wake_active;
+	} else {
+		mask_idcr = gc->wake_active;
+		mask_iecr = gc->mask_cache;
+	}
+
+	irq_gc_lock(gc);
+	irq_reg_writel(gc, mask_idcr, aic_reg_data->idcr);
+	irq_reg_writel(gc, mask_iecr, aic_reg_data->iecr);
+	irq_gc_unlock(gc);
+}
+
+static void aic_suspend(struct irq_data *d)
+{
+	if (aic_is_ssr_used())
+		aic_pm_ctrl_ssr(d, AIC_PM_SUSPEND);
+	else
+		aic_pm_ctrl(d, AIC_PM_SUSPEND);
+}
+
+static void aic_resume(struct irq_data *d)
+{
+	if (aic_is_ssr_used())
+		aic_pm_ctrl_ssr(d, AIC_PM_RESUME);
+	else
+		aic_pm_ctrl(d, AIC_PM_RESUME);
+}
+
+static void aic_pm_shutdown(struct irq_data *d)
+{
+	struct irq_domain *domain = d->domain;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	int i;
+
+	if (aic_is_ssr_used()) {
+		irq_gc_lock(bgc);
+		for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
+			irq_reg_writel(bgc, i + gc->irq_base,
+				       aic_reg_data->ssr);
+			irq_reg_writel(bgc, 1, aic_reg_data->idcr);
+			irq_reg_writel(bgc, 1, aic_reg_data->iccr);
+		}
+		irq_gc_unlock(bgc);
+	} else {
+		irq_gc_lock(gc);
+		irq_reg_writel(gc, 0xffffffff, aic_reg_data->idcr);
+		irq_reg_writel(gc, 0xffffffff, aic_reg_data->iccr);
+		irq_gc_unlock(gc);
+	}
+}
+#else
+#define aic_suspend	NULL
+#define aic_resume	NULL
+#define aic_pm_shutdown	NULL
+#endif /* CONFIG_PM */
+
+static int __init aic_get_num_chips(struct device_node *node)
+{
+	int nirqs;
+
+	/* Get total number of IRQs and configure register data */
+	if (of_device_is_compatible(node, "atmel,at91rm9200-aic")) {
+		nirqs = NR_AT91RM9200_IRQS;
+		aic_reg_data = &aic_regs;
+	} else if (of_device_is_compatible(node, "atmel,sama5d2-aic")) {
+		nirqs = NR_SAMA5D2_IRQS;
+		aic_reg_data = &aic5_regs;
+	} else if (of_device_is_compatible(node, "atmel,sama5d3-aic")) {
+		nirqs = NR_SAMA5D3_IRQS;
+		aic_reg_data = &aic5_regs;
+	} else if (of_device_is_compatible(node, "atmel,sama5d4-aic")) {
+		nirqs = NR_SAMA5D4_IRQS;
+		aic_reg_data = &aic5_regs;
+	} else {
+		return -EINVAL;
+	}
+
+	return DIV_ROUND_UP(nirqs, AIC_IRQS_PER_CHIP);
+}
+
+static void __init aic_ext_irq_of_init(struct irq_domain *domain)
+{
+	struct device_node *node = irq_domain_get_of_node(domain);
+	struct irq_chip_generic *gc;
+	struct aic_chip_data *aic;
+	struct property *prop;
+	const __be32 *p;
+	u32 hwirq;
+
+	gc = irq_get_domain_generic_chip(domain, 0);
+
+	aic = gc->private;
+	aic->ext_irqs |= 1;
+
+	of_property_for_each_u32(node, "atmel,external-irqs", prop, p, hwirq) {
+		gc = irq_get_domain_generic_chip(domain, hwirq);
+		if (!gc) {
+			pr_warn("AIC: external irq %d >= %d skip it\n",
+				hwirq, domain->revmap_size);
+			continue;
+		}
+
+		aic = gc->private;
+		aic->ext_irqs |= (1 << (hwirq % AIC_IRQS_PER_CHIP));
+	}
+}
+
+static void __init aic_hw_init(struct irq_domain *domain)
+{
+	struct irq_chip_generic *gc = irq_get_domain_generic_chip(domain, 0);
+	int i;
+
+	/*
+	 * Perform 8 End Of Interrupt Command to make sure AIC
+	 * will not Lock out nIRQ
+	 */
+	for (i = 0; i < 8; i++)
+		irq_reg_writel(gc, 0, aic_reg_data->eoi);
+
+	/*
+	 * Spurious Interrupt ID in Spurious Vector Register.
+	 * When there is no current interrupt, the IRQ Vector Register
+	 * reads the value stored in AIC_SPU
+	 */
+	irq_reg_writel(gc, 0xffffffff, aic_reg_data->spu);
+
+	/* No debugging in AIC: Debug (Protect) Control Register */
+	irq_reg_writel(gc, 0, aic_reg_data->dcr);
+
+	/* Disable and clear all interrupts initially */
+	if (aic_is_ssr_used()) {
+		for (i = 0; i < domain->revmap_size; i++) {
+			irq_reg_writel(gc, i, aic_reg_data->ssr);
+			irq_reg_writel(gc, i, aic_reg_data->svr);
+			irq_reg_writel(gc, 1, aic_reg_data->idcr);
+			irq_reg_writel(gc, 1, aic_reg_data->iccr);
+		}
+	} else {
+		irq_reg_writel(gc, 0xffffffff, aic_reg_data->idcr);
+		irq_reg_writel(gc, 0xffffffff, aic_reg_data->iccr);
+
+		for (i = 0; i < NR_AT91RM9200_IRQS; i++)
+			irq_reg_writel(gc, i, aic_reg_data->svr + (i * 4));
+	}
+}
+
+static int __init aic_of_init(struct device_node *node,
+			      struct device_node *parent)
+{
+	struct irq_chip_generic *gc;
+	struct irq_domain *domain;
+	struct aic_chip_data *aic;
+	void __iomem *reg_base;
+	int nchips;
+	int ret;
+	int i;
+
+	if (aic_domain)
+		return -EEXIST;
+
+	nchips = aic_get_num_chips(node);
+	if (nchips < 0)
+		return -EINVAL;
+
+	reg_base = of_iomap(node, 0);
+	if (!reg_base)
+		return -ENOMEM;
+
+	aic = kcalloc(nchips, sizeof(*aic), GFP_KERNEL);
+	if (!aic) {
+		ret = -ENOMEM;
+		goto err_iounmap;
+	}
+
+	domain = irq_domain_add_linear(node, nchips * AIC_IRQS_PER_CHIP,
+				       &aic_irq_ops, aic);
+	if (!domain) {
+		ret = -ENOMEM;
+		goto err_free_aic;
+	}
+
+	ret = irq_alloc_domain_generic_chips(domain, AIC_IRQS_PER_CHIP, 1,
+					     "atmel-aic", handle_fasteoi_irq,
+					     IRQ_NOREQUEST | IRQ_NOPROBE |
+					     IRQ_NOAUTOEN, 0, 0);
+	if (ret)
+		goto err_domain_remove;
+
+	for (i = 0; i < nchips; i++) {
+		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
+
+		gc->reg_base = reg_base;
+		gc->unused = 0;
+		gc->wake_enabled = ~0;
+
+		gc->chip_types[0].type = IRQ_TYPE_SENSE_MASK;
+		gc->chip_types[0].regs.eoi = aic_reg_data->eoi;
+		gc->chip_types[0].chip.irq_eoi = irq_gc_eoi;
+		gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
+		gc->chip_types[0].chip.irq_shutdown = aic_irq_shutdown;
+		gc->chip_types[0].chip.irq_mask = aic_mask;
+		gc->chip_types[0].chip.irq_unmask = aic_unmask;
+		gc->chip_types[0].chip.irq_retrigger = aic_retrigger;
+		gc->chip_types[0].chip.irq_set_type = aic_set_type;
+		gc->chip_types[0].chip.irq_suspend = aic_suspend;
+		gc->chip_types[0].chip.irq_resume = aic_resume;
+		gc->chip_types[0].chip.irq_pm_shutdown = aic_pm_shutdown;
+
+		gc->private = &aic[i];
+	}
+
+	aic_domain = domain;
+	aic_ext_irq_of_init(domain);
+	aic_hw_init(domain);
+	set_handle_irq(aic_handle);
+
+	return 0;
+
+err_domain_remove:
+	irq_domain_remove(domain);
+
+err_free_aic:
+	kfree(aic);
+
+err_iounmap:
+	iounmap(reg_base);
+	return ret;
+}
+IRQCHIP_DECLARE(at91rm9200_aic, "atmel,at91rm9200-aic", aic_of_init);
+IRQCHIP_DECLARE(sama5d2_aic5, "atmel,sama5d2-aic", aic_of_init);
+IRQCHIP_DECLARE(sama5d3_aic5, "atmel,sama5d3-aic", aic_of_init);
+IRQCHIP_DECLARE(sama5d4_aic5, "atmel,sama5d4-aic", aic_of_init);
diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
deleted file mode 100644
index 1d3978e..0000000
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ /dev/null
@@ -1,609 +0,0 @@
-/*
- * Atmel AIC (Advanced Interrupt Controller) Driver
- *
- *  Copyright (C) 2004 SAN People
- *  Copyright (C) 2004 ATMEL
- *  Copyright (C) Rick Bronson
- *  Copyright (C) 2014 Free Electrons
- *
- *  Author: Boris BREZILLON <boris.brezillon@free-electrons.com>
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2.  This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
- */
-
-#include <linux/errno.h>
-#include <linux/init.h>
-#include <linux/io.h>
-#include <linux/irq.h>
-#include <linux/irqchip.h>
-#include <linux/irqdesc.h>
-#include <linux/irqdomain.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/slab.h>
-
-#include <asm/exception.h>
-#include <asm/mach/irq.h>
-
-#define AIC_IRQS_PER_CHIP		32
-#define NR_AT91RM9200_IRQS		32
-#define NR_SAMA5D2_IRQS			77
-#define NR_SAMA5D3_IRQS			48
-#define NR_SAMA5D4_IRQS			68
-
-#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
-
-#define AT91_AIC_SRCTYPE		GENMASK(6, 5)
-#define AT91_AIC_SRCTYPE_LOW		(0 << 5)
-#define AT91_AIC_SRCTYPE_FALLING	(1 << 5)
-#define AT91_AIC_SRCTYPE_HIGH		(2 << 5)
-#define AT91_AIC_SRCTYPE_RISING		(3 << 5)
-
-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 struct irq_domain *aic_domain;
-static const struct aic_reg_offset *aic_reg_data;
-
-static asmlinkage void __exception_irq_entry
-aic_handle(struct pt_regs *regs)
-{
-	struct irq_chip_generic *gc = irq_get_domain_generic_chip(aic_domain,
-								  0);
-	u32 hwirq = irq_reg_readl(gc, aic_reg_data->ivr);
-	u32 status = irq_reg_readl(gc, aic_reg_data->isr);
-
-	if (!status)
-		irq_reg_writel(gc, 0, aic_reg_data->eoi);
-	else
-		handle_domain_irq(aic_domain, hwirq, regs);
-}
-
-static inline bool aic_is_ssr_used(void)
-{
-	return aic_reg_data->ssr != AT91_INVALID_OFFSET;
-}
-
-static void aic_update_smr(struct irq_chip_generic *gc, int hwirq,
-			   u32 mask, u32 val)
-{
-	int reg = aic_reg_data->smr;
-	u32 tmp = 0;
-
-	if (aic_is_ssr_used())
-		irq_reg_writel(gc, hwirq, aic_reg_data->ssr);
-	else
-		reg += hwirq * 4;
-
-	tmp = irq_reg_readl(gc, reg);
-	tmp &= mask;
-	tmp |= val;
-
-	irq_reg_writel(gc, tmp, reg);
-}
-
-static int aic_irq_domain_xlate(struct irq_domain *d, struct device_node *node,
-				const u32 *intspec, unsigned int intsize,
-				irq_hw_number_t *out_hwirq,
-				unsigned int *out_type)
-{
-	struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0);
-	bool condition = (intsize < 3) ||
-			 (intspec[2] < AT91_AIC_IRQ_MIN_PRIORITY) ||
-			 (intspec[2] > AT91_AIC_IRQ_MAX_PRIORITY);
-
-	if (!gc || WARN_ON(condition))
-		return -EINVAL;
-
-	/*
-	 * intspec[0]: HW IRQ number
-	 * intspec[1]: IRQ flag
-	 * intspec[2]: IRQ priority
-	 */
-
-	*out_hwirq = intspec[0];
-	*out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
-
-	irq_gc_lock(gc);
-	aic_update_smr(gc, *out_hwirq, ~AT91_AIC_PRIOR, intspec[2]);
-	irq_gc_unlock(gc);
-
-	return 0;
-}
-
-static const struct irq_domain_ops aic_irq_ops = {
-	.map	= irq_map_generic_chip,
-	.xlate	= aic_irq_domain_xlate,
-};
-
-static void aic_irq_shutdown(struct irq_data *d)
-{
-	struct irq_chip_type *ct = irq_data_get_chip_type(d);
-
-	ct->chip.irq_mask(d);
-}
-
-static void aic_mask(struct irq_data *d)
-{
-	struct irq_domain *domain = d->domain;
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-	u32 mask = d->mask;
-
-	/*
-	 * Disable interrupt. We always take the lock of the
-	 * first irq chip as all chips share the same registers.
-	 */
-	irq_gc_lock(bgc);
-
-	if (aic_is_ssr_used()) {
-		irq_reg_writel(gc, d->hwirq, aic_reg_data->ssr);
-		irq_reg_writel(gc, 1, aic_reg_data->idcr);
-	} else {
-		irq_reg_writel(gc, mask, aic_reg_data->idcr);
-	}
-
-	gc->mask_cache &= ~mask;
-
-	irq_gc_unlock(bgc);
-}
-
-static void aic_unmask(struct irq_data *d)
-{
-	struct irq_domain *domain = d->domain;
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-	u32 mask = d->mask;
-
-	/*
-	 * Enable interrupt. We always take the lock of the
-	 * first irq chip as all chips share the same registers.
-	 */
-	irq_gc_lock(bgc);
-
-	if (aic_is_ssr_used()) {
-		irq_reg_writel(gc, d->hwirq, aic_reg_data->ssr);
-		irq_reg_writel(gc, 1, aic_reg_data->iecr);
-	} else {
-		irq_reg_writel(gc, mask, aic_reg_data->iecr);
-	}
-
-	gc->mask_cache |= mask;
-
-	irq_gc_unlock(bgc);
-}
-
-static int aic_retrigger(struct irq_data *d)
-{
-	struct irq_domain *domain = d->domain;
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
-
-	/* Set interrupt */
-	irq_gc_lock(bgc);
-
-	if (aic_is_ssr_used()) {
-		irq_reg_writel(bgc, d->hwirq, aic_reg_data->ssr);
-		irq_reg_writel(bgc, 1, aic_reg_data->iscr);
-	} else {
-		irq_reg_writel(bgc, d->mask, aic_reg_data->iscr);
-	}
-
-	irq_gc_unlock(bgc);
-
-	return 0;
-}
-
-static int aic_set_type(struct irq_data *d, unsigned int type)
-{
-	struct irq_domain *domain = d->domain;
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-	struct aic_chip_data *aic = gc->private;
-	u32 val;
-
-	switch (type) {
-	case IRQ_TYPE_LEVEL_HIGH:
-		val = AT91_AIC_SRCTYPE_HIGH;
-		break;
-	case IRQ_TYPE_EDGE_RISING:
-		val = AT91_AIC_SRCTYPE_RISING;
-		break;
-	case IRQ_TYPE_LEVEL_LOW:
-		if (!(d->mask & aic->ext_irqs))
-			return -EINVAL;
-
-		val = AT91_AIC_SRCTYPE_LOW;
-		break;
-	case IRQ_TYPE_EDGE_FALLING:
-		if (!(d->mask & aic->ext_irqs))
-			return -EINVAL;
-
-		val = AT91_AIC_SRCTYPE_FALLING;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	irq_gc_lock(bgc);
-	aic_update_smr(bgc, d->hwirq, ~AT91_AIC_SRCTYPE, val);
-	irq_gc_unlock(bgc);
-
-	return 0;
-}
-
-#ifdef CONFIG_PM
-
-enum aic_pm_mode {
-	AIC_PM_SUSPEND,
-	AIC_PM_RESUME,
-};
-
-static void aic_pm_ctrl_ssr(struct irq_data *d, enum aic_pm_mode mode)
-{
-	struct irq_domain *domain = d->domain;
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-	u32 mask;
-	u32 which;
-	int i;
-
-	if (mode == AIC_PM_SUSPEND)
-		which = gc->wake_active;
-	else
-		which = gc->mask_cache;
-
-	irq_gc_lock(bgc);
-
-	for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
-		mask = 1 << i;
-		if ((mask & gc->mask_cache) == (mask & gc->wake_active))
-			continue;
-
-		irq_reg_writel(bgc, i + gc->irq_base, aic_reg_data->ssr);
-
-		if (mask & which)
-			irq_reg_writel(bgc, 1, aic_reg_data->iecr);
-		else
-			irq_reg_writel(bgc, 1, aic_reg_data->idcr);
-	}
-
-	irq_gc_unlock(bgc);
-}
-
-static void aic_pm_ctrl(struct irq_data *d, enum aic_pm_mode mode)
-{
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-	u32 mask_idcr;
-	u32 mask_iecr;
-
-	if (mode == AIC_PM_SUSPEND) {
-		mask_idcr = gc->mask_cache;
-		mask_iecr = gc->wake_active;
-	} else {
-		mask_idcr = gc->wake_active;
-		mask_iecr = gc->mask_cache;
-	}
-
-	irq_gc_lock(gc);
-	irq_reg_writel(gc, mask_idcr, aic_reg_data->idcr);
-	irq_reg_writel(gc, mask_iecr, aic_reg_data->iecr);
-	irq_gc_unlock(gc);
-}
-
-static void aic_suspend(struct irq_data *d)
-{
-	if (aic_is_ssr_used())
-		aic_pm_ctrl_ssr(d, AIC_PM_SUSPEND);
-	else
-		aic_pm_ctrl(d, AIC_PM_SUSPEND);
-}
-
-static void aic_resume(struct irq_data *d)
-{
-	if (aic_is_ssr_used())
-		aic_pm_ctrl_ssr(d, AIC_PM_RESUME);
-	else
-		aic_pm_ctrl(d, AIC_PM_RESUME);
-}
-
-static void aic_pm_shutdown(struct irq_data *d)
-{
-	struct irq_domain *domain = d->domain;
-	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
-	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
-	int i;
-
-	if (aic_is_ssr_used()) {
-		irq_gc_lock(bgc);
-		for (i = 0; i < AIC_IRQS_PER_CHIP; i++) {
-			irq_reg_writel(bgc, i + gc->irq_base,
-				       aic_reg_data->ssr);
-			irq_reg_writel(bgc, 1, aic_reg_data->idcr);
-			irq_reg_writel(bgc, 1, aic_reg_data->iccr);
-		}
-		irq_gc_unlock(bgc);
-	} else {
-		irq_gc_lock(gc);
-		irq_reg_writel(gc, 0xffffffff, aic_reg_data->idcr);
-		irq_reg_writel(gc, 0xffffffff, aic_reg_data->iccr);
-		irq_gc_unlock(gc);
-	}
-}
-#else
-#define aic_suspend	NULL
-#define aic_resume	NULL
-#define aic_pm_shutdown	NULL
-#endif /* CONFIG_PM */
-
-static int __init aic_get_num_chips(struct device_node *node)
-{
-	int nirqs;
-
-	/* Get total number of IRQs and configure register data */
-	if (of_device_is_compatible(node, "atmel,at91rm9200-aic")) {
-		nirqs = NR_AT91RM9200_IRQS;
-		aic_reg_data = &aic_regs;
-	} else if (of_device_is_compatible(node, "atmel,sama5d2-aic")) {
-		nirqs = NR_SAMA5D2_IRQS;
-		aic_reg_data = &aic5_regs;
-	} else if (of_device_is_compatible(node, "atmel,sama5d3-aic")) {
-		nirqs = NR_SAMA5D3_IRQS;
-		aic_reg_data = &aic5_regs;
-	} else if (of_device_is_compatible(node, "atmel,sama5d4-aic")) {
-		nirqs = NR_SAMA5D4_IRQS;
-		aic_reg_data = &aic5_regs;
-	} else {
-		return -EINVAL;
-	}
-
-	return DIV_ROUND_UP(nirqs, AIC_IRQS_PER_CHIP);
-}
-
-static void __init aic_ext_irq_of_init(struct irq_domain *domain)
-{
-	struct device_node *node = irq_domain_get_of_node(domain);
-	struct irq_chip_generic *gc;
-	struct aic_chip_data *aic;
-	struct property *prop;
-	const __be32 *p;
-	u32 hwirq;
-
-	gc = irq_get_domain_generic_chip(domain, 0);
-
-	aic = gc->private;
-	aic->ext_irqs |= 1;
-
-	of_property_for_each_u32(node, "atmel,external-irqs", prop, p, hwirq) {
-		gc = irq_get_domain_generic_chip(domain, hwirq);
-		if (!gc) {
-			pr_warn("AIC: external irq %d >= %d skip it\n",
-				hwirq, domain->revmap_size);
-			continue;
-		}
-
-		aic = gc->private;
-		aic->ext_irqs |= (1 << (hwirq % AIC_IRQS_PER_CHIP));
-	}
-}
-
-static void __init aic_hw_init(struct irq_domain *domain)
-{
-	struct irq_chip_generic *gc = irq_get_domain_generic_chip(domain, 0);
-	int i;
-
-	/*
-	 * Perform 8 End Of Interrupt Command to make sure AIC
-	 * will not Lock out nIRQ
-	 */
-	for (i = 0; i < 8; i++)
-		irq_reg_writel(gc, 0, aic_reg_data->eoi);
-
-	/*
-	 * Spurious Interrupt ID in Spurious Vector Register.
-	 * When there is no current interrupt, the IRQ Vector Register
-	 * reads the value stored in AIC_SPU
-	 */
-	irq_reg_writel(gc, 0xffffffff, aic_reg_data->spu);
-
-	/* No debugging in AIC: Debug (Protect) Control Register */
-	irq_reg_writel(gc, 0, aic_reg_data->dcr);
-
-	/* Disable and clear all interrupts initially */
-	if (aic_is_ssr_used()) {
-		for (i = 0; i < domain->revmap_size; i++) {
-			irq_reg_writel(gc, i, aic_reg_data->ssr);
-			irq_reg_writel(gc, i, aic_reg_data->svr);
-			irq_reg_writel(gc, 1, aic_reg_data->idcr);
-			irq_reg_writel(gc, 1, aic_reg_data->iccr);
-		}
-	} else {
-		irq_reg_writel(gc, 0xffffffff, aic_reg_data->idcr);
-		irq_reg_writel(gc, 0xffffffff, aic_reg_data->iccr);
-
-		for (i = 0; i < NR_AT91RM9200_IRQS; i++)
-			irq_reg_writel(gc, i, aic_reg_data->svr + (i * 4));
-	}
-}
-
-static int __init aic_of_init(struct device_node *node,
-			      struct device_node *parent)
-{
-	struct irq_chip_generic *gc;
-	struct irq_domain *domain;
-	struct aic_chip_data *aic;
-	void __iomem *reg_base;
-	int nchips;
-	int ret;
-	int i;
-
-	if (aic_domain)
-		return -EEXIST;
-
-	nchips = aic_get_num_chips(node);
-	if (nchips < 0)
-		return -EINVAL;
-
-	reg_base = of_iomap(node, 0);
-	if (!reg_base)
-		return -ENOMEM;
-
-	aic = kcalloc(nchips, sizeof(*aic), GFP_KERNEL);
-	if (!aic) {
-		ret = -ENOMEM;
-		goto err_iounmap;
-	}
-
-	domain = irq_domain_add_linear(node, nchips * AIC_IRQS_PER_CHIP,
-				       &aic_irq_ops, aic);
-	if (!domain) {
-		ret = -ENOMEM;
-		goto err_free_aic;
-	}
-
-	ret = irq_alloc_domain_generic_chips(domain, AIC_IRQS_PER_CHIP, 1,
-					     "atmel-aic", handle_fasteoi_irq,
-					     IRQ_NOREQUEST | IRQ_NOPROBE |
-					     IRQ_NOAUTOEN, 0, 0);
-	if (ret)
-		goto err_domain_remove;
-
-	for (i = 0; i < nchips; i++) {
-		gc = irq_get_domain_generic_chip(domain, i * AIC_IRQS_PER_CHIP);
-
-		gc->reg_base = reg_base;
-		gc->unused = 0;
-		gc->wake_enabled = ~0;
-
-		gc->chip_types[0].type = IRQ_TYPE_SENSE_MASK;
-		gc->chip_types[0].regs.eoi = aic_reg_data->eoi;
-		gc->chip_types[0].chip.irq_eoi = irq_gc_eoi;
-		gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
-		gc->chip_types[0].chip.irq_shutdown = aic_irq_shutdown;
-		gc->chip_types[0].chip.irq_mask = aic_mask;
-		gc->chip_types[0].chip.irq_unmask = aic_unmask;
-		gc->chip_types[0].chip.irq_retrigger = aic_retrigger;
-		gc->chip_types[0].chip.irq_set_type = aic_set_type;
-		gc->chip_types[0].chip.irq_suspend = aic_suspend;
-		gc->chip_types[0].chip.irq_resume = aic_resume;
-		gc->chip_types[0].chip.irq_pm_shutdown = aic_pm_shutdown;
-
-		gc->private = &aic[i];
-	}
-
-	aic_domain = domain;
-	aic_ext_irq_of_init(domain);
-	aic_hw_init(domain);
-	set_handle_irq(aic_handle);
-
-	return 0;
-
-err_domain_remove:
-	irq_domain_remove(domain);
-
-err_free_aic:
-	kfree(aic);
-
-err_iounmap:
-	iounmap(reg_base);
-	return ret;
-}
-IRQCHIP_DECLARE(at91rm9200_aic, "atmel,at91rm9200-aic", aic_of_init);
-IRQCHIP_DECLARE(sama5d2_aic5, "atmel,sama5d2-aic", aic_of_init);
-IRQCHIP_DECLARE(sama5d3_aic5, "atmel,sama5d3-aic", aic_of_init);
-IRQCHIP_DECLARE(sama5d4_aic5, "atmel,sama5d4-aic", aic_of_init);
-- 
2.6.4

Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver

[Boris Brezillon]

Hi Milo,

On Mon, 4 Jan 2016 13:28:24 +0900
Milo Kim <milo.kim@ti.com> wrote:

> This patch-set provides unified Atmel AIC (Advanced Interrupt Controller)
> driver. Currently, there are two AIC drivers, AIC and AIC5.
> Each driver consists of chip specific part (irq-atmel-aic.o or
> irq-atmel-aic5.o) and shared code (irq-atmel-aic-common.o).
> But consolidated AIC driver is just one file driver which supports both
> IRQ chip systems.

Sorry, but what's the real motivation behind this rework?

> 
> How to handle two IRQ chips in one driver
> -----------------------------------------
>   Structure aic_reg_offset is used for device configuration.
>   AIC5 IRQ chip uses SSR (Source Select Register) to select IRQ number.
>   On the other hand, AIC IRQ chip has simple register access.
>   To support both IRQ chips, aic_is_ssr_used() helper is used.
> 
> Patches
> -------
>   1 ~  5: fix IRQ priority issue, clean up RTC/RTT fixup code and etc.

As explained in my review, those irq fixup are essential, and cannot
remove them.

>   6 ~ 19: create unified IRQ chip operation with aic_reg_offset data.

I started to review those patches, but honestly I don't see the point of
this rework, since you're trying to merge drivers for 2 IPs that are
completely different from a functional POV (except for a few tiny things
like priority or irq type definition).

Before reviewing the remaining patches, I'd like to know more about your
real motivations for pushing those changes?

> 
> Target boards
> -------------
>   Tested with two boards.
>   * Arietta G25 (SoC: AT91SAM9G25)
>   * Xplained board (SoC: SAMA5D3)
> 
> Number of driver files
> ----------------------
>   AIC:              3 (irq-atmel-aic.c, irq-atmel-aic-common.c and h)
>   AIC5:             3 (irq-atmel-aic5.c, irq-atmel-aic-common.c and h)
>   Consolidated AIC: 1 (irq-aic.c)
> 
> Code size
> ---------
>   AIC (irq-atmel-aic.o and irq-atmel-aic-common.o)
>     text	   data	    bss	    dec	    hex	filename
>     5137	    196	      4	   5337	   14d9	drivers/irqchip/built-in.o
> 
>   AIC5 (irq-atmel-aic5.o and irq-atmel-aic-common.o)
>     text	   data	    bss	    dec	    hex	filename
>     5548	    196	      4	   5748	   1674	drivers/irqchip/built-in.o
> 
>   Consolidated AIC (irq-aic.o)
>     text	   data	    bss	    dec	    hex	filename
>     4841	    196	      8	   5045	   13b5	drivers/irqchip/built-in.o
> 
> Lines of code
> -------------
>   AIC:              597
>   AIC5:             688
>   Consolidated AIC: 609


Please, redo the same thing, but after keeping the IRQ fixup stuff, and
I'm pretty sure the text section of the AIC/AIC5 and the consolidated
version will be much closer.

Not to mention that you're adding a bunch of if () statements in the
critical IRQ path, which is never a good think for latency concern.

Best Regards,

Boris

> 
> Milo Kim (19):
>   irqchip: atmel-aic: fix wrong bit operation for IRQ priority
>   irqchip: atmel-aic: clean up RTC interrupt code
>   irqchip: atmel-aic: clean up RTT interrupt code
>   irqchip: atmel-aic: replace magic numbers with named constant
>   irqchip: atmel-aic: use simple constant to get number of interrupts
>     per chip
>   irqchip: atmel-aic: introduce register data structure
>   irqchip: atmel-aic: make common IRQ domain translate function
>   irqchip: atmel-aic: add common mask and unmask functions
>   irqchip: atmel-aic: add common retrigger function
>   irqchip: atmel-aic: add common set_type function
>   irqchip: atmel-aic: add common PM IRQ chip operation
>   irqchip: atmel-aic: use EOI register data in aic_reg_data
>   irqchip: atmel-aic: clean up irq_chip_generic
>   irqchip: atmel-aic: add common HW init function
>   irqchip: atmel-aic: add common interrupt handler
>   irqchip: atmel-aic: get total number of IRQs from device node
>   irqchip: atmel-aic: use unified IRQ chip initialization function
>   irqchip: atmel-aic: use unified AIC driver
>   irqchip: atmel-aic: rename AIC driver and fix Kconfig
> 
>  arch/arm/mach-at91/Kconfig             |   2 +-
>  drivers/irqchip/Kconfig                |   7 -
>  drivers/irqchip/Makefile               |   3 +-
>  drivers/irqchip/irq-aic.c              | 609 +++++++++++++++++++++++++++++++++
>  drivers/irqchip/irq-atmel-aic-common.c | 280 ---------------
>  drivers/irqchip/irq-atmel-aic-common.h |  41 ---
>  drivers/irqchip/irq-atmel-aic.c        | 276 ---------------
>  drivers/irqchip/irq-atmel-aic5.c       | 367 --------------------
>  8 files changed, 611 insertions(+), 974 deletions(-)
>  create mode 100644 drivers/irqchip/irq-aic.c
>  delete mode 100644 drivers/irqchip/irq-atmel-aic-common.c
>  delete mode 100644 drivers/irqchip/irq-atmel-aic-common.h
>  delete mode 100644 drivers/irqchip/irq-atmel-aic.c
>  delete mode 100644 drivers/irqchip/irq-atmel-aic5.c
> 



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver

[Nicolas Ferre]

Le 04/01/2016 10:02, Boris Brezillon a écrit :
> Hi Milo,
> 
> On Mon, 4 Jan 2016 13:28:24 +0900
> Milo Kim <milo.kim@ti.com> wrote:
> 
>> This patch-set provides unified Atmel AIC (Advanced Interrupt Controller)
>> driver. Currently, there are two AIC drivers, AIC and AIC5.
>> Each driver consists of chip specific part (irq-atmel-aic.o or
>> irq-atmel-aic5.o) and shared code (irq-atmel-aic-common.o).
>> But consolidated AIC driver is just one file driver which supports both
>> IRQ chip systems.
> 
> Sorry, but what's the real motivation behind this rework?

I was about to ask the same question.

I'm really sorry that you spent so much time and effort for something
that I'm absolutely against: reworking stable code that had been
maturing for years and that is now nicely stable... for no obvious reason...

So unless there is a very strong benefit, it's a pretty firm NACK for
this series. We will certainly take some time to review the bits that
can be taken further to enhance our current drivers.

I also advice you to talk to us before starting such a big project and
you'll notice that we are open to changes and have many aspect that you
can work on with AT91.

Bye,


>> How to handle two IRQ chips in one driver
>> -----------------------------------------
>>   Structure aic_reg_offset is used for device configuration.
>>   AIC5 IRQ chip uses SSR (Source Select Register) to select IRQ number.
>>   On the other hand, AIC IRQ chip has simple register access.
>>   To support both IRQ chips, aic_is_ssr_used() helper is used.
>>
>> Patches
>> -------
>>   1 ~  5: fix IRQ priority issue, clean up RTC/RTT fixup code and etc.
> 
> As explained in my review, those irq fixup are essential, and cannot
> remove them.
> 
>>   6 ~ 19: create unified IRQ chip operation with aic_reg_offset data.
> 
> I started to review those patches, but honestly I don't see the point of
> this rework, since you're trying to merge drivers for 2 IPs that are
> completely different from a functional POV (except for a few tiny things
> like priority or irq type definition).
> 
> Before reviewing the remaining patches, I'd like to know more about your
> real motivations for pushing those changes?
> 
>>
>> Target boards
>> -------------
>>   Tested with two boards.
>>   * Arietta G25 (SoC: AT91SAM9G25)
>>   * Xplained board (SoC: SAMA5D3)
>>
>> Number of driver files
>> ----------------------
>>   AIC:              3 (irq-atmel-aic.c, irq-atmel-aic-common.c and h)
>>   AIC5:             3 (irq-atmel-aic5.c, irq-atmel-aic-common.c and h)
>>   Consolidated AIC: 1 (irq-aic.c)
>>
>> Code size
>> ---------
>>   AIC (irq-atmel-aic.o and irq-atmel-aic-common.o)
>>     text	   data	    bss	    dec	    hex	filename
>>     5137	    196	      4	   5337	   14d9	drivers/irqchip/built-in.o
>>
>>   AIC5 (irq-atmel-aic5.o and irq-atmel-aic-common.o)
>>     text	   data	    bss	    dec	    hex	filename
>>     5548	    196	      4	   5748	   1674	drivers/irqchip/built-in.o
>>
>>   Consolidated AIC (irq-aic.o)
>>     text	   data	    bss	    dec	    hex	filename
>>     4841	    196	      8	   5045	   13b5	drivers/irqchip/built-in.o
>>
>> Lines of code
>> -------------
>>   AIC:              597
>>   AIC5:             688
>>   Consolidated AIC: 609
> 
> 
> Please, redo the same thing, but after keeping the IRQ fixup stuff, and
> I'm pretty sure the text section of the AIC/AIC5 and the consolidated
> version will be much closer.
> 
> Not to mention that you're adding a bunch of if () statements in the
> critical IRQ path, which is never a good think for latency concern.
> 
> Best Regards,
> 
> Boris
> 
>>
>> Milo Kim (19):
>>   irqchip: atmel-aic: fix wrong bit operation for IRQ priority
>>   irqchip: atmel-aic: clean up RTC interrupt code
>>   irqchip: atmel-aic: clean up RTT interrupt code
>>   irqchip: atmel-aic: replace magic numbers with named constant
>>   irqchip: atmel-aic: use simple constant to get number of interrupts
>>     per chip
>>   irqchip: atmel-aic: introduce register data structure
>>   irqchip: atmel-aic: make common IRQ domain translate function
>>   irqchip: atmel-aic: add common mask and unmask functions
>>   irqchip: atmel-aic: add common retrigger function
>>   irqchip: atmel-aic: add common set_type function
>>   irqchip: atmel-aic: add common PM IRQ chip operation
>>   irqchip: atmel-aic: use EOI register data in aic_reg_data
>>   irqchip: atmel-aic: clean up irq_chip_generic
>>   irqchip: atmel-aic: add common HW init function
>>   irqchip: atmel-aic: add common interrupt handler
>>   irqchip: atmel-aic: get total number of IRQs from device node
>>   irqchip: atmel-aic: use unified IRQ chip initialization function
>>   irqchip: atmel-aic: use unified AIC driver
>>   irqchip: atmel-aic: rename AIC driver and fix Kconfig
>>
>>  arch/arm/mach-at91/Kconfig             |   2 +-
>>  drivers/irqchip/Kconfig                |   7 -
>>  drivers/irqchip/Makefile               |   3 +-
>>  drivers/irqchip/irq-aic.c              | 609 +++++++++++++++++++++++++++++++++
>>  drivers/irqchip/irq-atmel-aic-common.c | 280 ---------------
>>  drivers/irqchip/irq-atmel-aic-common.h |  41 ---
>>  drivers/irqchip/irq-atmel-aic.c        | 276 ---------------
>>  drivers/irqchip/irq-atmel-aic5.c       | 367 --------------------
>>  8 files changed, 611 insertions(+), 974 deletions(-)
>>  create mode 100644 drivers/irqchip/irq-aic.c
>>  delete mode 100644 drivers/irqchip/irq-atmel-aic-common.c
>>  delete mode 100644 drivers/irqchip/irq-atmel-aic-common.h
>>  delete mode 100644 drivers/irqchip/irq-atmel-aic.c
>>  delete mode 100644 drivers/irqchip/irq-atmel-aic5.c
>>
> 
> 
> 


-- 
Nicolas Ferre

Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver

[Milo Kim]

Hi Nicolas,

On 01/04/2016 06:37 PM, Nicolas Ferre wrote:
> Le 04/01/2016 10:02, Boris Brezillon a écrit :
>> Hi Milo,
>>
>> On Mon, 4 Jan 2016 13:28:24 +0900
>> Milo Kim <milo.kim@ti.com> wrote:
>>
>>> This patch-set provides unified Atmel AIC (Advanced Interrupt Controller)
>>> driver. Currently, there are two AIC drivers, AIC and AIC5.
>>> Each driver consists of chip specific part (irq-atmel-aic.o or
>>> irq-atmel-aic5.o) and shared code (irq-atmel-aic-common.o).
>>> But consolidated AIC driver is just one file driver which supports both
>>> IRQ chip systems.
>>
>> Sorry, but what's the real motivation behind this rework?
>
> I was about to ask the same question.
>
> I'm really sorry that you spent so much time and effort for something
> that I'm absolutely against: reworking stable code that had been
> maturing for years and that is now nicely stable... for no obvious reason...

That's fine. It's not a problem. I enjoyed the time :)
I was also concerning about a regression. Let me try to rework common 
part then.

>
> So unless there is a very strong benefit, it's a pretty firm NACK for
> this series. We will certainly take some time to review the bits that
> can be taken further to enhance our current drivers.

Sure, I totally agree.

>
> I also advice you to talk to us before starting such a big project and
> you'll notice that we are open to changes and have many aspect that you
> can work on with AT91.

I'll keep in mind. Thanks for your advice.

Best regards,
Milo

Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver

[Milo Kim]

Hi Boris,

Thanks a lot for your comments.

On 01/04/2016 06:02 PM, Boris Brezillon wrote:
> Hi Milo,
>
> On Mon, 4 Jan 2016 13:28:24 +0900
> Milo Kim <milo.kim@ti.com> wrote:
>
>> This patch-set provides unified Atmel AIC (Advanced Interrupt Controller)
>> driver. Currently, there are two AIC drivers, AIC and AIC5.
>> Each driver consists of chip specific part (irq-atmel-aic.o or
>> irq-atmel-aic5.o) and shared code (irq-atmel-aic-common.o).
>> But consolidated AIC driver is just one file driver which supports both
>> IRQ chip systems.
>
> Sorry, but what's the real motivation behind this rework?

During my driver development on Atmel boards, I just found major 
difference between two IRQ chips is how to select HW IRQ number. Other 
parts could be merged into single driver like OMAP.

>
>>
>> How to handle two IRQ chips in one driver
>> -----------------------------------------
>>    Structure aic_reg_offset is used for device configuration.
>>    AIC5 IRQ chip uses SSR (Source Select Register) to select IRQ number.
>>    On the other hand, AIC IRQ chip has simple register access.
>>    To support both IRQ chips, aic_is_ssr_used() helper is used.
>>
>> Patches
>> -------
>>    1 ~  5: fix IRQ priority issue, clean up RTC/RTT fixup code and etc.
>
> As explained in my review, those irq fixup are essential, and cannot
> remove them.

My apologies for this regression. I should check your log carefully. 
Thanks for pointing this out.
I'm just thinking about boot code modification for this. How about 
supporting RTC/RTT fixup code in at91bootstrap?

>
>>    6 ~ 19: create unified IRQ chip operation with aic_reg_offset data.
>
> I started to review those patches, but honestly I don't see the point of
> this rework, since you're trying to merge drivers for 2 IPs that are
> completely different from a functional POV (except for a few tiny things
> like priority or irq type definition).
>
> Before reviewing the remaining patches, I'd like to know more about your
> real motivations for pushing those changes?

Yeap, thanks for your time. My idea is simple.

"Different IRQ chip operation can be consolidated if simple data 
structure is used."

If AIC and AIC5 must be separate, then I'll try to rework 
irq-atmel-common part.

>
>>
>> Target boards
>> -------------
>>    Tested with two boards.
>>    * Arietta G25 (SoC: AT91SAM9G25)
>>    * Xplained board (SoC: SAMA5D3)
>>
>> Number of driver files
>> ----------------------
>>    AIC:              3 (irq-atmel-aic.c, irq-atmel-aic-common.c and h)
>>    AIC5:             3 (irq-atmel-aic5.c, irq-atmel-aic-common.c and h)
>>    Consolidated AIC: 1 (irq-aic.c)
>>
>> Code size
>> ---------
>>    AIC (irq-atmel-aic.o and irq-atmel-aic-common.o)
>>      text	   data	    bss	    dec	    hex	filename
>>      5137	    196	      4	   5337	   14d9	drivers/irqchip/built-in.o
>>
>>    AIC5 (irq-atmel-aic5.o and irq-atmel-aic-common.o)
>>      text	   data	    bss	    dec	    hex	filename
>>      5548	    196	      4	   5748	   1674	drivers/irqchip/built-in.o
>>
>>    Consolidated AIC (irq-aic.o)
>>      text	   data	    bss	    dec	    hex	filename
>>      4841	    196	      8	   5045	   13b5	drivers/irqchip/built-in.o
>>
>> Lines of code
>> -------------
>>    AIC:              597
>>    AIC5:             688
>>    Consolidated AIC: 609
>
>
> Please, redo the same thing, but after keeping the IRQ fixup stuff, and
> I'm pretty sure the text section of the AIC/AIC5 and the consolidated
> version will be much closer.

Let me check it later if I could have a chance to create the 2nd patch ;)

Best regards,
Milo

Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver

[Boris Brezillon]

Hi Milo,

On Wed, 6 Jan 2016 16:48:23 +0900
Milo Kim <milo.kim@ti.com> wrote:

> Hi Boris,
> 
> Thanks a lot for your comments.
> 
> On 01/04/2016 06:02 PM, Boris Brezillon wrote:
> > Hi Milo,
> >
> > On Mon, 4 Jan 2016 13:28:24 +0900
> > Milo Kim <milo.kim@ti.com> wrote:
> >
> >> This patch-set provides unified Atmel AIC (Advanced Interrupt Controller)
> >> driver. Currently, there are two AIC drivers, AIC and AIC5.
> >> Each driver consists of chip specific part (irq-atmel-aic.o or
> >> irq-atmel-aic5.o) and shared code (irq-atmel-aic-common.o).
> >> But consolidated AIC driver is just one file driver which supports both
> >> IRQ chip systems.
> >
> > Sorry, but what's the real motivation behind this rework?
> 
> During my driver development on Atmel boards, I just found major 
> difference between two IRQ chips is how to select HW IRQ number. Other 
> parts could be merged into single driver like OMAP.

Except that this major difference is a central aspect, and if you look
at your changes, you'll see that you're introducing
'if (aic_is_ssr_used())' statements in pretty much all irqchip
callbacks.
As I said, I'm not against code factorization, but it's not really
one to me, because you're adding extra conditional path all over the
code to differentiate the two chips, which means those are not so
similar.

Could you list the common aspects shared by those two controllers?

> 
> >
> >>
> >> How to handle two IRQ chips in one driver
> >> -----------------------------------------
> >>    Structure aic_reg_offset is used for device configuration.
> >>    AIC5 IRQ chip uses SSR (Source Select Register) to select IRQ number.
> >>    On the other hand, AIC IRQ chip has simple register access.
> >>    To support both IRQ chips, aic_is_ssr_used() helper is used.
> >>
> >> Patches
> >> -------
> >>    1 ~  5: fix IRQ priority issue, clean up RTC/RTT fixup code and etc.
> >
> > As explained in my review, those irq fixup are essential, and cannot
> > remove them.
> 
> My apologies for this regression. I should check your log carefully. 
> Thanks for pointing this out.
> I'm just thinking about boot code modification for this. How about 
> supporting RTC/RTT fixup code in at91bootstrap?

Nope, because at91bootstrap is not mandatory, you can use u-boot's
or barebox's SPL, or even develop your own bootstrap code. Not to
mention that kexeced kernels can also experience some bugs. It should
not happen if all drivers implement a ->shutdown() function disabling
theie interrupt, but I'm pretty sure that's not currently the case.
Anyway, we have to support existing systems, so removing those fixup is
simply not an option.

> 
> >
> >>    6 ~ 19: create unified IRQ chip operation with aic_reg_offset data.
> >
> > I started to review those patches, but honestly I don't see the point of
> > this rework, since you're trying to merge drivers for 2 IPs that are
> > completely different from a functional POV (except for a few tiny things
> > like priority or irq type definition).
> >
> > Before reviewing the remaining patches, I'd like to know more about your
> > real motivations for pushing those changes?
> 
> Yeap, thanks for your time. My idea is simple.
> 
> "Different IRQ chip operation can be consolidated if simple data 
> structure is used."

As pointed, I don't think that's a good idea, but let's see what others
say.
Thomas, Jason, any comments?

> 
> If AIC and AIC5 must be separate, then I'll try to rework 
> irq-atmel-common part.

What would you like to rework? Could you describe in more details
what's bothering you in this code?

Also, you can join #at91 on IRC (freenode server) if you want to
discuss that in a more interactive way.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver

[Jason Cooper]

Hey Milo,

On Wed, Jan 06, 2016 at 10:07:55AM +0100, Boris Brezillon wrote:
> On Wed, 6 Jan 2016 16:48:23 +0900 Milo Kim <milo.kim@ti.com> wrote:
> > On 01/04/2016 06:02 PM, Boris Brezillon wrote:
> > > On Mon, 4 Jan 2016 13:28:24 +0900 Milo Kim <milo.kim@ti.com> wrote:
> > >
> > >> This patch-set provides unified Atmel AIC (Advanced Interrupt Controller)
> > >> driver. Currently, there are two AIC drivers, AIC and AIC5.
> > >> Each driver consists of chip specific part (irq-atmel-aic.o or
> > >> irq-atmel-aic5.o) and shared code (irq-atmel-aic-common.o).
> > >> But consolidated AIC driver is just one file driver which supports both
> > >> IRQ chip systems.
> > >
> > > Sorry, but what's the real motivation behind this rework?
> > 
> > During my driver development on Atmel boards, I just found major 
> > difference between two IRQ chips is how to select HW IRQ number. Other 
> > parts could be merged into single driver like OMAP.
> 
> Except that this major difference is a central aspect, and if you look
> at your changes, you'll see that you're introducing
> 'if (aic_is_ssr_used())' statements in pretty much all irqchip
> callbacks.
> 
> As I said, I'm not against code factorization, but it's not really
> one to me, because you're adding extra conditional path all over the
> code to differentiate the two chips, which means those are not so
> similar
...
> > > Before reviewing the remaining patches, I'd like to know more about your
> > > real motivations for pushing those changes?
> > 
> > Yeap, thanks for your time. My idea is simple.
> > 
> > "Different IRQ chip operation can be consolidated if simple data 
> > structure is used."
> 
> As pointed, I don't think that's a good idea, but let's see what others
> say.
> Thomas, Jason, any comments?

I'm with Nicolas on this one.  I appreciate the effort, but it's best to
discuss the proposal with at91/irqchip maintainers prior to investing so
much effort.

sorry,

Jason.

Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver

[Milo Kim]

Hi Boris,

>> If AIC and AIC5 must be separate, then I'll try to rework
>> irq-atmel-common part.
>
> What would you like to rework? Could you describe in more details
> what's bothering you in this code?

I was about to modify irq-atmel-aic-common as below.

   * AIC5 handles for-loop twice during initialization.
   1) after IRQ generic chip is allocated successfully.
   2) map AIC5 IRQ chip operation after aic_common_of_init() returns.
   To reduce for-loop count, move each IRQ chip initialization code into 
common part.

   * AIC and AIC5 have same interrupt handling procedure, so each IRQ 
handler can be moved to common part.

BTW, I've sent another patch-set about fixing few things. Could you 
check them?

Best regards,
Milo