All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Milo Kim <milo.kim@ti.com>
Cc: <tglx@linutronix.de>, <jason@lakedaemon.net>,
	<marc.zyngier@arm.com>, <alexandre.belloni@free-electrons.com>,
	<ludovic.desroches@atmel.com>, <nicolas.ferre@atmel.com>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 05/19] irqchip: atmel-aic: use simple constant to get number of interrupts per chip
Date: Mon, 4 Jan 2016 09:33:39 +0100	[thread overview]
Message-ID: <20160104093339.7df4605f@bbrezillon> (raw)
In-Reply-To: <1451881723-2478-6-git-send-email-milo.kim@ti.com>

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

  reply	other threads:[~2016-01-04  8:33 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-04  4:28 [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Milo Kim
2016-01-04  4:28 ` [PATCH 01/19] irqchip: atmel-aic: fix wrong bit operation for IRQ priority Milo Kim
2016-01-04  8:11   ` Boris Brezillon
2016-01-04  4:28 ` [PATCH 02/19] irqchip: atmel-aic: clean up RTC interrupt code Milo Kim
2016-01-04  8:16   ` Boris Brezillon
2016-01-04  4:28 ` [PATCH 03/19] irqchip: atmel-aic: clean up RTT " Milo Kim
2016-01-04  8:17   ` Boris Brezillon
2016-01-04  4:28 ` [PATCH 04/19] irqchip: atmel-aic: replace magic numbers with named constant Milo Kim
2016-01-04  8:29   ` Boris Brezillon
2016-01-04  4:28 ` [PATCH 05/19] irqchip: atmel-aic: use simple constant to get number of interrupts per chip Milo Kim
2016-01-04  8:33   ` Boris Brezillon [this message]
2016-01-04  4:28 ` [PATCH 06/19] irqchip: atmel-aic: introduce register data structure Milo Kim
2016-01-04  8:53   ` Boris Brezillon
2016-01-06  8:30     ` Milo Kim
2016-01-04  4:28 ` [PATCH 07/19] irqchip: atmel-aic: make common IRQ domain translate function Milo Kim
2016-01-04  4:28 ` [PATCH 08/19] irqchip: atmel-aic: add common mask and unmask functions Milo Kim
2016-01-04  8:48   ` Boris Brezillon
2016-01-04  4:28 ` [PATCH 09/19] irqchip: atmel-aic: add common retrigger function Milo Kim
2016-01-04  4:28 ` [PATCH 10/19] irqchip: atmel-aic: add common set_type function Milo Kim
2016-01-04  4:28 ` [PATCH 11/19] irqchip: atmel-aic: add common PM IRQ chip operation Milo Kim
2016-01-04  4:28 ` [PATCH 12/19] irqchip: atmel-aic: use EOI register data in aic_reg_data Milo Kim
2016-01-04  4:28 ` [PATCH 13/19] irqchip: atmel-aic: clean up irq_chip_generic Milo Kim
2016-01-04  4:28 ` [PATCH 14/19] irqchip: atmel-aic: add common HW init function Milo Kim
2016-01-04  4:28 ` [PATCH 15/19] irqchip: atmel-aic: add common interrupt handler Milo Kim
2016-01-04  4:28 ` [PATCH 16/19] irqchip: atmel-aic: get total number of IRQs from device node Milo Kim
2016-01-04  4:28 ` [PATCH 17/19] irqchip: atmel-aic: use unified IRQ chip initialization function Milo Kim
2016-01-04  4:28 ` [PATCH 18/19] irqchip: atmel-aic: use unified AIC driver Milo Kim
2016-01-04  4:28 ` [PATCH 19/19] irqchip: atmel-aic: rename AIC driver and fix Kconfig Milo Kim
2016-01-04  9:02 ` [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Boris Brezillon
2016-01-04  9:37   ` Nicolas Ferre
2016-01-06  7:55     ` Milo Kim
2016-01-06  7:48   ` Milo Kim
2016-01-06  9:07     ` Boris Brezillon
2016-01-06 14:49       ` Jason Cooper
2016-01-07  7:48       ` Milo Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160104093339.7df4605f@bbrezillon \
    --to=boris.brezillon@free-electrons.com \
    --cc=alexandre.belloni@free-electrons.com \
    --cc=jason@lakedaemon.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ludovic.desroches@atmel.com \
    --cc=marc.zyngier@arm.com \
    --cc=milo.kim@ti.com \
    --cc=nicolas.ferre@atmel.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.