From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752397AbcADI3J (ORCPT ); Mon, 4 Jan 2016 03:29:09 -0500 Received: from down.free-electrons.com ([37.187.137.238]:34696 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751274AbcADI3I (ORCPT ); Mon, 4 Jan 2016 03:29:08 -0500 Date: Mon, 4 Jan 2016 09:29:04 +0100 From: Boris Brezillon To: Milo Kim Cc: , , , , , , Subject: Re: [PATCH 04/19] irqchip: atmel-aic: replace magic numbers with named constant Message-ID: <20160104092904.19e68c69@bbrezillon> In-Reply-To: <1451881723-2478-5-git-send-email-milo.kim@ti.com> References: <1451881723-2478-1-git-send-email-milo.kim@ti.com> <1451881723-2478-5-git-send-email-milo.kim@ti.com> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.27; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 4 Jan 2016 13:28:28 +0900 Milo Kim 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 > Cc: Jason Cooper > Cc: Marc Zyngier > Cc: Alexandre Belloni > Cc: Boris BREZILLON > Cc: Ludovic Desroches > Cc: Nicolas Ferre > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Milo Kim > --- > 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