Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1300466 > unrolled thread

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

Started byMilo Kim <milo.kim@ti.com>
First post2016-01-04 05:30 +0100
Last post2016-01-07 08:50 +0100
Articles 13 on this page of 33 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
    [PATCH 07/19] irqchip: atmel-aic: make common IRQ domain translate function Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
    [PATCH 09/19] irqchip: atmel-aic: add common retrigger function Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
    [PATCH 13/19] irqchip: atmel-aic: clean up irq_chip_generic Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
    [PATCH 01/19] irqchip: atmel-aic: fix wrong bit operation for IRQ priority Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
      Re: [PATCH 01/19] irqchip: atmel-aic: fix wrong bit operation for  IRQ priority Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 09:20 +0100
    [PATCH 17/19] irqchip: atmel-aic: use unified IRQ chip initialization function Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
    [PATCH 06/19] irqchip: atmel-aic: introduce register data structure Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
      Re: [PATCH 06/19] irqchip: atmel-aic: introduce register data  structure Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 10:00 +0100
        Re: [PATCH 06/19] irqchip: atmel-aic: introduce register data  structure Milo Kim <milo.kim@ti.com> - 2016-01-06 09:40 +0100
    [PATCH 04/19] irqchip: atmel-aic: replace magic numbers with named constant Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
      Re: [PATCH 04/19] irqchip: atmel-aic: replace magic numbers with  named constant Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 09:30 +0100
    [PATCH 19/19] irqchip: atmel-aic: rename AIC driver and fix Kconfig Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
    [PATCH 11/19] irqchip: atmel-aic: add common PM IRQ chip operation Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
    [PATCH 12/19] irqchip: atmel-aic: use EOI register data in aic_reg_data Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
    [PATCH 14/19] irqchip: atmel-aic: add common HW init function Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
    [PATCH 10/19] irqchip: atmel-aic: add common set_type function Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
    [PATCH 08/19] irqchip: atmel-aic: add common mask and unmask functions Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
      Re: [PATCH 08/19] irqchip: atmel-aic: add common mask and unmask  functions Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 09:50 +0100
    [PATCH 02/19] irqchip: atmel-aic: clean up RTC interrupt code Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
      Re: [PATCH 02/19] irqchip: atmel-aic: clean up RTC interrupt code Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 09:20 +0100
    [PATCH 05/19] irqchip: atmel-aic: use simple constant to get number of interrupts per chip Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
      Re: [PATCH 05/19] irqchip: atmel-aic: use simple constant to get  number of interrupts per chip Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 09:40 +0100
    [PATCH 03/19] irqchip: atmel-aic: clean up RTT interrupt code Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
      Re: [PATCH 03/19] irqchip: atmel-aic: clean up RTT interrupt code Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 09:20 +0100
    [PATCH 15/19] irqchip: atmel-aic: add common interrupt handler Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
    Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 10:10 +0100
      Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Nicolas Ferre <nicolas.ferre@atmel.com> - 2016-01-04 10:40 +0100
        Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Milo Kim <milo.kim@ti.com> - 2016-01-06 09:00 +0100
      Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Milo Kim <milo.kim@ti.com> - 2016-01-06 08:50 +0100
        Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-06 10:10 +0100
          Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Jason Cooper <jason@lakedaemon.net> - 2016-01-06 15:50 +0100
          Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Milo Kim <milo.kim@ti.com> - 2016-01-07 08:50 +0100

Page 2 of 2 — ← Prev page 1 [2]

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

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-01-04 09:20 +0100
SubjectRe: [PATCH 02/19] irqchip: atmel-aic: clean up RTC interrupt code
Message-ID<qN8wW-VH-7@gated-at.bofh.it>
In reply to#1300481
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

FromMilo Kim <milo.kim@ti.com>
Date2016-01-04 05:40 +0100
Subject[PATCH 05/19] irqchip: atmel-aic: use simple constant to get number of interrupts per chip
Message-ID<qN563-6Ig-25@gated-at.bofh.it>
In reply to#1300466
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-01-04 09:40 +0100
SubjectRe: [PATCH 05/19] irqchip: atmel-aic: use simple constant to get number of interrupts per chip
Message-ID<qN8Qh-12y-5@gated-at.bofh.it>
In reply to#1300483
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

FromMilo Kim <milo.kim@ti.com>
Date2016-01-04 05:40 +0100
Subject[PATCH 03/19] irqchip: atmel-aic: clean up RTT interrupt code
Message-ID<qN563-6Ig-27@gated-at.bofh.it>
In reply to#1300466
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-01-04 09:20 +0100
SubjectRe: [PATCH 03/19] irqchip: atmel-aic: clean up RTT interrupt code
Message-ID<qN8wW-VH-13@gated-at.bofh.it>
In reply to#1300484
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

FromMilo Kim <milo.kim@ti.com>
Date2016-01-04 05:40 +0100
Subject[PATCH 15/19] irqchip: atmel-aic: add common interrupt handler
Message-ID<qN563-6Ig-29@gated-at.bofh.it>
In reply to#1300466
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300562

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-01-04 10:10 +0100
Message-ID<qN9jk-1sI-7@gated-at.bofh.it>
In reply to#1300466
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300574

FromNicolas Ferre <nicolas.ferre@atmel.com>
Date2016-01-04 10:40 +0100
Message-ID<qN9Ml-1Eg-1@gated-at.bofh.it>
In reply to#1300562
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302443

FromMilo Kim <milo.kim@ti.com>
Date2016-01-06 09:00 +0100
Message-ID<qNRaG-6Cx-3@gated-at.bofh.it>
In reply to#1300574
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302440

FromMilo Kim <milo.kim@ti.com>
Date2016-01-06 08:50 +0100
Message-ID<qNR10-6zh-3@gated-at.bofh.it>
In reply to#1300562
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302489

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-01-06 10:10 +0100
Message-ID<qNSgq-7wa-17@gated-at.bofh.it>
In reply to#1302440
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302825

FromJason Cooper <jason@lakedaemon.net>
Date2016-01-06 15:50 +0100
Message-ID<qNXzr-2po-1@gated-at.bofh.it>
In reply to#1302489
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303337

FromMilo Kim <milo.kim@ti.com>
Date2016-01-07 08:50 +0100
Message-ID<qOduy-4XK-17@gated-at.bofh.it>
In reply to#1302489
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web