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

Groups > linux.kernel > #1300555

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

Show all headers | View raw

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
--
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/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[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

csiph-web