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

Groups > linux.kernel > #1229341 > unrolled thread

[PATCH 1/3] irqchip: atmel-aic5: fix bug with mask/unmask

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/3] irqchip: atmel-aic5: fix bug with mask/unmask Ludovic Desroches <ludovic.desroches@atmel.com> - 2015-09-21 15:50 +0200
    [PATCH 2/3] irqchip: atmel-aic5: fix variable naming Ludovic Desroches <ludovic.desroches@atmel.com> - 2015-09-21 15:50 +0200
      Re: [PATCH 2/3] irqchip: atmel-aic5: fix variable naming Nicolas Ferre <nicolas.ferre@atmel.com> - 2015-09-22 10:30 +0200
      [tip:irq/core] irqchip/atmel-aic5:   Use explicit variable name for the base chip tip-bot for Ludovic Desroches <tipbot@zytor.com> - 2015-09-22 16:20 +0200
    [PATCH 3/3] irqchip: atmel-aic5: simplify base chip selection Ludovic Desroches <ludovic.desroches@atmel.com> - 2015-09-21 15:50 +0200
      Re: [PATCH 3/3] irqchip: atmel-aic5: simplify base chip selection Nicolas Ferre <nicolas.ferre@atmel.com> - 2015-09-22 10:30 +0200
      [tip:irq/core] irqchip/atmel-aic5: Simplify base chip selection tip-bot for Ludovic Desroches <tipbot@zytor.com> - 2015-09-22 16:20 +0200
    Re: [PATCH 1/3] irqchip: atmel-aic5: fix bug with mask/unmask Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-09-22 09:50 +0200
    Re: [PATCH 1/3] irqchip: atmel-aic5: fix bug with mask/unmask Nicolas Ferre <nicolas.ferre@atmel.com> - 2015-09-22 10:30 +0200
    Re: [PATCH 1/3] irqchip: atmel-aic5: fix bug with mask/unmask Thomas Gleixner <tglx@linutronix.de> - 2015-09-22 12:30 +0200
      Re: [PATCH 1/3] irqchip: atmel-aic5: fix bug with mask/unmask Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-09-22 14:00 +0200
        Re: [PATCH 1/3] irqchip: atmel-aic5: fix bug with mask/unmask Thomas Gleixner <tglx@linutronix.de> - 2015-09-22 16:00 +0200
          Re: [PATCH 1/3] irqchip: atmel-aic5: fix bug with mask/unmask Ludovic Desroches <ludovic.desroches@atmel.com> - 2015-09-22 16:20 +0200
    Re: [tip:irq/urgent] irqchip/atmel-aic5: Use proper mask cache in  mask/unmask() Thomas Gleixner <tglx@linutronix.de> - 2015-09-22 12:40 +0200
    [tip:irq/urgent] irqchip/atmel-aic5:   Use proper mask cache in mask/unmask() tip-bot for Ludovic Desroches <tipbot@zytor.com> - 2015-09-22 12:40 +0200
    [tip:irq/urgent] irqchip/atmel-aic5:   Use per chip mask caches in mask/unmask() tip-bot for Ludovic Desroches <tipbot@zytor.com> - 2015-09-22 16:10 +0200

#1229341 — [PATCH 1/3] irqchip: atmel-aic5: fix bug with mask/unmask

When masking/unmasking interrupts, mask_cache is updated and used later
for suspend/resume. Unfortunately, it always was the mask_cache
associated with the first irq chip which was updated. So when performing
resume, only irqs 0-31 could be enabled and maybe not the good ones!

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
Fixes: b1479ebb7720 ("irqchip: atmel-aic: Add atmel AIC/AIC5 drivers")
Cc: stable@vger.kernel.org #3.18
---

Sasha,

This fix won't apply without conflicts because of irq_reg_writel changes. I
can provide you a fix for 3.18 if you need.

Regards

Ludovic


 drivers/irqchip/irq-atmel-aic5.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 9da9942..6c5fd25 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -88,28 +88,30 @@ static void aic5_mask(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
 	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *gc = dgc->gc[0];
+	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 
 	/* Disable interrupt on AIC5 */
-	irq_gc_lock(gc);
+	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(gc);
+	irq_gc_unlock(bgc);
 }
 
 static void aic5_unmask(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
 	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *gc = dgc->gc[0];
+	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 
 	/* Enable interrupt on AIC5 */
-	irq_gc_lock(gc);
+	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(gc);
+	irq_gc_unlock(bgc);
 }
 
 static int aic5_retrigger(struct irq_data *d)
-- 
2.5.0

--
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] | [next] | [standalone]

#1229355 — [PATCH 2/3] irqchip: atmel-aic5: fix variable naming

To avoid errors, use an explicit variable name when accessing the 'base'
generic chip.

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 drivers/irqchip/irq-atmel-aic5.c | 44 ++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 6c5fd25..abff79e 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -71,15 +71,15 @@ static asmlinkage void __exception_irq_entry
 aic5_handle(struct pt_regs *regs)
 {
 	struct irq_domain_chip_generic *dgc = aic5_domain->gc;
-	struct irq_chip_generic *gc = dgc->gc[0];
+	struct irq_chip_generic *bgc = dgc->gc[0];
 	u32 irqnr;
 	u32 irqstat;
 
-	irqnr = irq_reg_readl(gc, AT91_AIC5_IVR);
-	irqstat = irq_reg_readl(gc, AT91_AIC5_ISR);
+	irqnr = irq_reg_readl(bgc, AT91_AIC5_IVR);
+	irqstat = irq_reg_readl(bgc, AT91_AIC5_ISR);
 
 	if (!irqstat)
-		irq_reg_writel(gc, 0, AT91_AIC5_EOICR);
+		irq_reg_writel(bgc, 0, AT91_AIC5_EOICR);
 	else
 		handle_domain_irq(aic5_domain, irqnr, regs);
 }
@@ -118,13 +118,13 @@ static int aic5_retrigger(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
 	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *gc = dgc->gc[0];
+	struct irq_chip_generic *bgc = dgc->gc[0];
 
 	/* Enable interrupt on AIC5 */
-	irq_gc_lock(gc);
-	irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
-	irq_reg_writel(gc, 1, AT91_AIC5_ISCR);
-	irq_gc_unlock(gc);
+	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;
 }
@@ -133,17 +133,17 @@ static int aic5_set_type(struct irq_data *d, unsigned type)
 {
 	struct irq_domain *domain = d->domain;
 	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *gc = dgc->gc[0];
+	struct irq_chip_generic *bgc = dgc->gc[0];
 	unsigned int smr;
 	int ret;
 
-	irq_gc_lock(gc);
-	irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
-	smr = irq_reg_readl(gc, AT91_AIC5_SMR);
+	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(gc, smr, AT91_AIC5_SMR);
-	irq_gc_unlock(gc);
+		irq_reg_writel(bgc, smr, AT91_AIC5_SMR);
+	irq_gc_unlock(bgc);
 
 	return ret;
 }
@@ -257,7 +257,7 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
 				 unsigned int *out_type)
 {
 	struct irq_domain_chip_generic *dgc = d->gc;
-	struct irq_chip_generic *gc;
+	struct irq_chip_generic *bgc;
 	unsigned smr;
 	int ret;
 
@@ -269,15 +269,15 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
 	if (ret)
 		return ret;
 
-	gc = dgc->gc[0];
+	bgc = dgc->gc[0];
 
-	irq_gc_lock(gc);
-	irq_reg_writel(gc, *out_hwirq, AT91_AIC5_SSR);
-	smr = irq_reg_readl(gc, AT91_AIC5_SMR);
+	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(gc, intspec[2] | smr, AT91_AIC5_SMR);
-	irq_gc_unlock(gc);
+		irq_reg_writel(bgc, intspec[2] | smr, AT91_AIC5_SMR);
+	irq_gc_unlock(bgc);
 
 	return ret;
 }
-- 
2.5.0

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

#1229975 — Re: [PATCH 2/3] irqchip: atmel-aic5: fix variable naming

Le 21/09/2015 15:46, Ludovic Desroches a écrit :
> To avoid errors, use an explicit variable name when accessing the 'base'
> generic chip.
> 
> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>  drivers/irqchip/irq-atmel-aic5.c | 44 ++++++++++++++++++++--------------------
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
> index 6c5fd25..abff79e 100644
> --- a/drivers/irqchip/irq-atmel-aic5.c
> +++ b/drivers/irqchip/irq-atmel-aic5.c
> @@ -71,15 +71,15 @@ static asmlinkage void __exception_irq_entry
>  aic5_handle(struct pt_regs *regs)
>  {
>  	struct irq_domain_chip_generic *dgc = aic5_domain->gc;
> -	struct irq_chip_generic *gc = dgc->gc[0];
> +	struct irq_chip_generic *bgc = dgc->gc[0];
>  	u32 irqnr;
>  	u32 irqstat;
>  
> -	irqnr = irq_reg_readl(gc, AT91_AIC5_IVR);
> -	irqstat = irq_reg_readl(gc, AT91_AIC5_ISR);
> +	irqnr = irq_reg_readl(bgc, AT91_AIC5_IVR);
> +	irqstat = irq_reg_readl(bgc, AT91_AIC5_ISR);
>  
>  	if (!irqstat)
> -		irq_reg_writel(gc, 0, AT91_AIC5_EOICR);
> +		irq_reg_writel(bgc, 0, AT91_AIC5_EOICR);
>  	else
>  		handle_domain_irq(aic5_domain, irqnr, regs);
>  }
> @@ -118,13 +118,13 @@ static int aic5_retrigger(struct irq_data *d)
>  {
>  	struct irq_domain *domain = d->domain;
>  	struct irq_domain_chip_generic *dgc = domain->gc;
> -	struct irq_chip_generic *gc = dgc->gc[0];
> +	struct irq_chip_generic *bgc = dgc->gc[0];
>  
>  	/* Enable interrupt on AIC5 */
> -	irq_gc_lock(gc);
> -	irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
> -	irq_reg_writel(gc, 1, AT91_AIC5_ISCR);
> -	irq_gc_unlock(gc);
> +	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;
>  }
> @@ -133,17 +133,17 @@ static int aic5_set_type(struct irq_data *d, unsigned type)
>  {
>  	struct irq_domain *domain = d->domain;
>  	struct irq_domain_chip_generic *dgc = domain->gc;
> -	struct irq_chip_generic *gc = dgc->gc[0];
> +	struct irq_chip_generic *bgc = dgc->gc[0];
>  	unsigned int smr;
>  	int ret;
>  
> -	irq_gc_lock(gc);
> -	irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
> -	smr = irq_reg_readl(gc, AT91_AIC5_SMR);
> +	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(gc, smr, AT91_AIC5_SMR);
> -	irq_gc_unlock(gc);
> +		irq_reg_writel(bgc, smr, AT91_AIC5_SMR);
> +	irq_gc_unlock(bgc);
>  
>  	return ret;
>  }
> @@ -257,7 +257,7 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
>  				 unsigned int *out_type)
>  {
>  	struct irq_domain_chip_generic *dgc = d->gc;
> -	struct irq_chip_generic *gc;
> +	struct irq_chip_generic *bgc;
>  	unsigned smr;
>  	int ret;
>  
> @@ -269,15 +269,15 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
>  	if (ret)
>  		return ret;
>  
> -	gc = dgc->gc[0];
> +	bgc = dgc->gc[0];
>  
> -	irq_gc_lock(gc);
> -	irq_reg_writel(gc, *out_hwirq, AT91_AIC5_SSR);
> -	smr = irq_reg_readl(gc, AT91_AIC5_SMR);
> +	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(gc, intspec[2] | smr, AT91_AIC5_SMR);
> -	irq_gc_unlock(gc);
> +		irq_reg_writel(bgc, intspec[2] | smr, AT91_AIC5_SMR);
> +	irq_gc_unlock(bgc);
>  
>  	return ret;
>  }
> 


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

#1230247 — [tip:irq/core] irqchip/atmel-aic5: Use explicit variable name for the base chip

Commit-ID:  414a431ad6217a03e561fcb199048141db3fc024
Gitweb:     http://git.kernel.org/tip/414a431ad6217a03e561fcb199048141db3fc024
Author:     Ludovic Desroches <ludovic.desroches@atmel.com>
AuthorDate: Mon, 21 Sep 2015 15:46:05 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 22 Sep 2015 16:04:42 +0200

irqchip/atmel-aic5: Use explicit variable name for the base chip

To avoid errors, use an explicit variable name when accessing the 'base'
generic chip.

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
Acked-by: Nicholas Ferre <nicolas.ferre@atmel.com>
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: <sasha.levin@oracle.com>
Cc: <linux-arm-kernel@lists.infradead.org>
Cc: <alexandre.belloni@free-electrons.com>
Cc: <Wenyou.Yang@atmel.com>
Cc: <jason@lakedaemon.net>
Cc: <marc.zyngier@arm.com>
Link: http://lkml.kernel.org/r/1442843173-2390-2-git-send-email-ludovic.desroches@atmel.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-atmel-aic5.c | 44 ++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index f6d6804..ae2fcf9 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -71,15 +71,15 @@ static asmlinkage void __exception_irq_entry
 aic5_handle(struct pt_regs *regs)
 {
 	struct irq_domain_chip_generic *dgc = aic5_domain->gc;
-	struct irq_chip_generic *gc = dgc->gc[0];
+	struct irq_chip_generic *bgc = dgc->gc[0];
 	u32 irqnr;
 	u32 irqstat;
 
-	irqnr = irq_reg_readl(gc, AT91_AIC5_IVR);
-	irqstat = irq_reg_readl(gc, AT91_AIC5_ISR);
+	irqnr = irq_reg_readl(bgc, AT91_AIC5_IVR);
+	irqstat = irq_reg_readl(bgc, AT91_AIC5_ISR);
 
 	if (!irqstat)
-		irq_reg_writel(gc, 0, AT91_AIC5_EOICR);
+		irq_reg_writel(bgc, 0, AT91_AIC5_EOICR);
 	else
 		handle_domain_irq(aic5_domain, irqnr, regs);
 }
@@ -124,13 +124,13 @@ static int aic5_retrigger(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
 	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *gc = dgc->gc[0];
+	struct irq_chip_generic *bgc = dgc->gc[0];
 
 	/* Enable interrupt on AIC5 */
-	irq_gc_lock(gc);
-	irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
-	irq_reg_writel(gc, 1, AT91_AIC5_ISCR);
-	irq_gc_unlock(gc);
+	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;
 }
@@ -139,17 +139,17 @@ static int aic5_set_type(struct irq_data *d, unsigned type)
 {
 	struct irq_domain *domain = d->domain;
 	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *gc = dgc->gc[0];
+	struct irq_chip_generic *bgc = dgc->gc[0];
 	unsigned int smr;
 	int ret;
 
-	irq_gc_lock(gc);
-	irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
-	smr = irq_reg_readl(gc, AT91_AIC5_SMR);
+	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(gc, smr, AT91_AIC5_SMR);
-	irq_gc_unlock(gc);
+		irq_reg_writel(bgc, smr, AT91_AIC5_SMR);
+	irq_gc_unlock(bgc);
 
 	return ret;
 }
@@ -263,7 +263,7 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
 				 unsigned int *out_type)
 {
 	struct irq_domain_chip_generic *dgc = d->gc;
-	struct irq_chip_generic *gc;
+	struct irq_chip_generic *bgc;
 	unsigned smr;
 	int ret;
 
@@ -275,15 +275,15 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
 	if (ret)
 		return ret;
 
-	gc = dgc->gc[0];
+	bgc = dgc->gc[0];
 
-	irq_gc_lock(gc);
-	irq_reg_writel(gc, *out_hwirq, AT91_AIC5_SSR);
-	smr = irq_reg_readl(gc, AT91_AIC5_SMR);
+	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(gc, intspec[2] | smr, AT91_AIC5_SMR);
-	irq_gc_unlock(gc);
+		irq_reg_writel(bgc, intspec[2] | smr, AT91_AIC5_SMR);
+	irq_gc_unlock(bgc);
 
 	return ret;
 }
--
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]

#1229362 — [PATCH 3/3] irqchip: atmel-aic5: simplify base chip selection

Use irq_get_domain_generic_chip() to select the base chip.

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 drivers/irqchip/irq-atmel-aic5.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index abff79e..7264ec7 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -70,8 +70,7 @@ static struct irq_domain *aic5_domain;
 static asmlinkage void __exception_irq_entry
 aic5_handle(struct pt_regs *regs)
 {
-	struct irq_domain_chip_generic *dgc = aic5_domain->gc;
-	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(aic5_domain, 0);
 	u32 irqnr;
 	u32 irqstat;
 
@@ -87,8 +86,7 @@ aic5_handle(struct pt_regs *regs)
 static void aic5_mask(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
-	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *bgc = dgc->gc[0];
+	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 */
@@ -102,8 +100,7 @@ static void aic5_mask(struct irq_data *d)
 static void aic5_unmask(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
-	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *bgc = dgc->gc[0];
+	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 */
@@ -117,8 +114,7 @@ static void aic5_unmask(struct irq_data *d)
 static int aic5_retrigger(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
-	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
 
 	/* Enable interrupt on AIC5 */
 	irq_gc_lock(bgc);
@@ -132,8 +128,7 @@ static int aic5_retrigger(struct irq_data *d)
 static int aic5_set_type(struct irq_data *d, unsigned type)
 {
 	struct irq_domain *domain = d->domain;
-	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
 	unsigned int smr;
 	int ret;
 
@@ -153,7 +148,7 @@ 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 = dgc->gc[0];
+	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;
@@ -177,7 +172,7 @@ 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 = dgc->gc[0];
+	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;
@@ -201,7 +196,7 @@ 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 = dgc->gc[0];
+	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;
 
@@ -256,12 +251,11 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
 				 irq_hw_number_t *out_hwirq,
 				 unsigned int *out_type)
 {
-	struct irq_domain_chip_generic *dgc = d->gc;
-	struct irq_chip_generic *bgc;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(d, 0);
 	unsigned smr;
 	int ret;
 
-	if (!dgc)
+	if (!bgc)
 		return -EINVAL;
 
 	ret = aic_common_irq_domain_xlate(d, ctrlr, intspec, intsize,
@@ -269,8 +263,6 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
 	if (ret)
 		return ret;
 
-	bgc = dgc->gc[0];
-
 	irq_gc_lock(bgc);
 	irq_reg_writel(bgc, *out_hwirq, AT91_AIC5_SSR);
 	smr = irq_reg_readl(bgc, AT91_AIC5_SMR);
-- 
2.5.0

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

#1229974 — Re: [PATCH 3/3] irqchip: atmel-aic5: simplify base chip selection

Le 21/09/2015 15:46, Ludovic Desroches a écrit :
> Use irq_get_domain_generic_chip() to select the base chip.
> 
> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>  drivers/irqchip/irq-atmel-aic5.c | 28 ++++++++++------------------
>  1 file changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
> index abff79e..7264ec7 100644
> --- a/drivers/irqchip/irq-atmel-aic5.c
> +++ b/drivers/irqchip/irq-atmel-aic5.c
> @@ -70,8 +70,7 @@ static struct irq_domain *aic5_domain;
>  static asmlinkage void __exception_irq_entry
>  aic5_handle(struct pt_regs *regs)
>  {
> -	struct irq_domain_chip_generic *dgc = aic5_domain->gc;
> -	struct irq_chip_generic *bgc = dgc->gc[0];
> +	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(aic5_domain, 0);
>  	u32 irqnr;
>  	u32 irqstat;
>  
> @@ -87,8 +86,7 @@ aic5_handle(struct pt_regs *regs)
>  static void aic5_mask(struct irq_data *d)
>  {
>  	struct irq_domain *domain = d->domain;
> -	struct irq_domain_chip_generic *dgc = domain->gc;
> -	struct irq_chip_generic *bgc = dgc->gc[0];
> +	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 */
> @@ -102,8 +100,7 @@ static void aic5_mask(struct irq_data *d)
>  static void aic5_unmask(struct irq_data *d)
>  {
>  	struct irq_domain *domain = d->domain;
> -	struct irq_domain_chip_generic *dgc = domain->gc;
> -	struct irq_chip_generic *bgc = dgc->gc[0];
> +	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 */
> @@ -117,8 +114,7 @@ static void aic5_unmask(struct irq_data *d)
>  static int aic5_retrigger(struct irq_data *d)
>  {
>  	struct irq_domain *domain = d->domain;
> -	struct irq_domain_chip_generic *dgc = domain->gc;
> -	struct irq_chip_generic *bgc = dgc->gc[0];
> +	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
>  
>  	/* Enable interrupt on AIC5 */
>  	irq_gc_lock(bgc);
> @@ -132,8 +128,7 @@ static int aic5_retrigger(struct irq_data *d)
>  static int aic5_set_type(struct irq_data *d, unsigned type)
>  {
>  	struct irq_domain *domain = d->domain;
> -	struct irq_domain_chip_generic *dgc = domain->gc;
> -	struct irq_chip_generic *bgc = dgc->gc[0];
> +	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
>  	unsigned int smr;
>  	int ret;
>  
> @@ -153,7 +148,7 @@ 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 = dgc->gc[0];
> +	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;
> @@ -177,7 +172,7 @@ 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 = dgc->gc[0];
> +	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;
> @@ -201,7 +196,7 @@ 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 = dgc->gc[0];
> +	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;
>  
> @@ -256,12 +251,11 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
>  				 irq_hw_number_t *out_hwirq,
>  				 unsigned int *out_type)
>  {
> -	struct irq_domain_chip_generic *dgc = d->gc;
> -	struct irq_chip_generic *bgc;
> +	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(d, 0);
>  	unsigned smr;
>  	int ret;
>  
> -	if (!dgc)
> +	if (!bgc)
>  		return -EINVAL;
>  
>  	ret = aic_common_irq_domain_xlate(d, ctrlr, intspec, intsize,
> @@ -269,8 +263,6 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
>  	if (ret)
>  		return ret;
>  
> -	bgc = dgc->gc[0];
> -
>  	irq_gc_lock(bgc);
>  	irq_reg_writel(bgc, *out_hwirq, AT91_AIC5_SSR);
>  	smr = irq_reg_readl(bgc, AT91_AIC5_SMR);
> 


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

#1230256 — [tip:irq/core] irqchip/atmel-aic5: Simplify base chip selection

Commit-ID:  b55a3bb8650ddb096624175c55176d7fdbcad4ae
Gitweb:     http://git.kernel.org/tip/b55a3bb8650ddb096624175c55176d7fdbcad4ae
Author:     Ludovic Desroches <ludovic.desroches@atmel.com>
AuthorDate: Mon, 21 Sep 2015 15:46:06 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 22 Sep 2015 16:04:43 +0200

irqchip/atmel-aic5: Simplify base chip selection

Use irq_get_domain_generic_chip() to select the base chip.

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
Acked-by: Nicholas Ferre <nicolas.ferre@atmel.com>
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: <sasha.levin@oracle.com>
Cc: <linux-arm-kernel@lists.infradead.org>
Cc: <alexandre.belloni@free-electrons.com>
Cc: <Wenyou.Yang@atmel.com>
Cc: <jason@lakedaemon.net>
Cc: <marc.zyngier@arm.com>
Link: http://lkml.kernel.org/r/1442843173-2390-3-git-send-email-ludovic.desroches@atmel.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-atmel-aic5.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index ae2fcf9..62bb840 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -70,8 +70,7 @@ static struct irq_domain *aic5_domain;
 static asmlinkage void __exception_irq_entry
 aic5_handle(struct pt_regs *regs)
 {
-	struct irq_domain_chip_generic *dgc = aic5_domain->gc;
-	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(aic5_domain, 0);
 	u32 irqnr;
 	u32 irqstat;
 
@@ -87,8 +86,7 @@ aic5_handle(struct pt_regs *regs)
 static void aic5_mask(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
-	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 
 	/*
@@ -105,8 +103,7 @@ static void aic5_mask(struct irq_data *d)
 static void aic5_unmask(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
-	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 
 	/*
@@ -123,8 +120,7 @@ static void aic5_unmask(struct irq_data *d)
 static int aic5_retrigger(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
-	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
 
 	/* Enable interrupt on AIC5 */
 	irq_gc_lock(bgc);
@@ -138,8 +134,7 @@ static int aic5_retrigger(struct irq_data *d)
 static int aic5_set_type(struct irq_data *d, unsigned type)
 {
 	struct irq_domain *domain = d->domain;
-	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(domain, 0);
 	unsigned int smr;
 	int ret;
 
@@ -159,7 +154,7 @@ 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 = dgc->gc[0];
+	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;
@@ -183,7 +178,7 @@ 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 = dgc->gc[0];
+	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;
@@ -207,7 +202,7 @@ 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 = dgc->gc[0];
+	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;
 
@@ -262,12 +257,11 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
 				 irq_hw_number_t *out_hwirq,
 				 unsigned int *out_type)
 {
-	struct irq_domain_chip_generic *dgc = d->gc;
-	struct irq_chip_generic *bgc;
+	struct irq_chip_generic *bgc = irq_get_domain_generic_chip(d, 0);
 	unsigned smr;
 	int ret;
 
-	if (!dgc)
+	if (!bgc)
 		return -EINVAL;
 
 	ret = aic_common_irq_domain_xlate(d, ctrlr, intspec, intsize,
@@ -275,8 +269,6 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
 	if (ret)
 		return ret;
 
-	bgc = dgc->gc[0];
-
 	irq_gc_lock(bgc);
 	irq_reg_writel(bgc, *out_hwirq, AT91_AIC5_SSR);
 	smr = irq_reg_readl(bgc, AT91_AIC5_SMR);
--
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]

#1229956

Hi Ludovic,

On Mon, 21 Sep 2015 15:46:04 +0200
Ludovic Desroches <ludovic.desroches@atmel.com> wrote:

> When masking/unmasking interrupts, mask_cache is updated and used later
> for suspend/resume. Unfortunately, it always was the mask_cache
> associated with the first irq chip which was updated. So when performing
> resume, only irqs 0-31 could be enabled and maybe not the good ones!
> 
> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> Fixes: b1479ebb7720 ("irqchip: atmel-aic: Add atmel AIC/AIC5 drivers")
> Cc: stable@vger.kernel.org #3.18

To the whole series

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

Thanks,

Boris

> ---
> 
> Sasha,
> 
> This fix won't apply without conflicts because of irq_reg_writel changes. I
> can provide you a fix for 3.18 if you need.
> 
> Regards
> 
> Ludovic
> 
> 
>  drivers/irqchip/irq-atmel-aic5.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
> index 9da9942..6c5fd25 100644
> --- a/drivers/irqchip/irq-atmel-aic5.c
> +++ b/drivers/irqchip/irq-atmel-aic5.c
> @@ -88,28 +88,30 @@ static void aic5_mask(struct irq_data *d)
>  {
>  	struct irq_domain *domain = d->domain;
>  	struct irq_domain_chip_generic *dgc = domain->gc;
> -	struct irq_chip_generic *gc = dgc->gc[0];
> +	struct irq_chip_generic *bgc = dgc->gc[0];
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
>  
>  	/* Disable interrupt on AIC5 */
> -	irq_gc_lock(gc);
> +	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(gc);
> +	irq_gc_unlock(bgc);
>  }
>  
>  static void aic5_unmask(struct irq_data *d)
>  {
>  	struct irq_domain *domain = d->domain;
>  	struct irq_domain_chip_generic *dgc = domain->gc;
> -	struct irq_chip_generic *gc = dgc->gc[0];
> +	struct irq_chip_generic *bgc = dgc->gc[0];
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
>  
>  	/* Enable interrupt on AIC5 */
> -	irq_gc_lock(gc);
> +	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(gc);
> +	irq_gc_unlock(bgc);
>  }
>  
>  static int aic5_retrigger(struct irq_data *d)



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

#1229973

Le 21/09/2015 15:46, Ludovic Desroches a écrit :
> When masking/unmasking interrupts, mask_cache is updated and used later
> for suspend/resume. Unfortunately, it always was the mask_cache
> associated with the first irq chip which was updated. So when performing
> resume, only irqs 0-31 could be enabled and maybe not the good ones!
> 
> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> Fixes: b1479ebb7720 ("irqchip: atmel-aic: Add atmel AIC/AIC5 drivers")
> Cc: stable@vger.kernel.org #3.18

Pretty important fix indeed!

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>


Thomas, Jason, can we please have this series queued for the 4.3-rc phase?

Bye,


> ---
> 
> Sasha,
> 
> This fix won't apply without conflicts because of irq_reg_writel changes. I
> can provide you a fix for 3.18 if you need.
> 
> Regards
> 
> Ludovic
> 
> 
>  drivers/irqchip/irq-atmel-aic5.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
> index 9da9942..6c5fd25 100644
> --- a/drivers/irqchip/irq-atmel-aic5.c
> +++ b/drivers/irqchip/irq-atmel-aic5.c
> @@ -88,28 +88,30 @@ static void aic5_mask(struct irq_data *d)
>  {
>  	struct irq_domain *domain = d->domain;
>  	struct irq_domain_chip_generic *dgc = domain->gc;
> -	struct irq_chip_generic *gc = dgc->gc[0];
> +	struct irq_chip_generic *bgc = dgc->gc[0];
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
>  
>  	/* Disable interrupt on AIC5 */
> -	irq_gc_lock(gc);
> +	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(gc);
> +	irq_gc_unlock(bgc);
>  }
>  
>  static void aic5_unmask(struct irq_data *d)
>  {
>  	struct irq_domain *domain = d->domain;
>  	struct irq_domain_chip_generic *dgc = domain->gc;
> -	struct irq_chip_generic *gc = dgc->gc[0];
> +	struct irq_chip_generic *bgc = dgc->gc[0];
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
>  
>  	/* Enable interrupt on AIC5 */
> -	irq_gc_lock(gc);
> +	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(gc);
> +	irq_gc_unlock(bgc);
>  }
>  
>  static int aic5_retrigger(struct irq_data *d)
> 


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

#1230056

On Mon, 21 Sep 2015, Ludovic Desroches wrote:
> diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
> index 9da9942..6c5fd25 100644
> --- a/drivers/irqchip/irq-atmel-aic5.c
> +++ b/drivers/irqchip/irq-atmel-aic5.c
> @@ -88,28 +88,30 @@ static void aic5_mask(struct irq_data *d)
>  {
>  	struct irq_domain *domain = d->domain;
>  	struct irq_domain_chip_generic *dgc = domain->gc;
> -	struct irq_chip_generic *gc = dgc->gc[0];
> +	struct irq_chip_generic *bgc = dgc->gc[0];
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
>  
>  	/* Disable interrupt on AIC5 */
> -	irq_gc_lock(gc);
> +	irq_gc_lock(bgc);

Why is this locking dgc->gc[0] and fiddling with some other generic
chip?

>  	irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
>  	irq_reg_writel(gc, 1, AT91_AIC5_IDCR);

Thanks,

	tglx
--
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]

#1230102

Hi Thomas,

On Tue, 22 Sep 2015 12:27:08 +0200 (CEST)
Thomas Gleixner <tglx@linutronix.de> wrote:

> On Mon, 21 Sep 2015, Ludovic Desroches wrote:
> > diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
> > index 9da9942..6c5fd25 100644
> > --- a/drivers/irqchip/irq-atmel-aic5.c
> > +++ b/drivers/irqchip/irq-atmel-aic5.c
> > @@ -88,28 +88,30 @@ static void aic5_mask(struct irq_data *d)
> >  {
> >  	struct irq_domain *domain = d->domain;
> >  	struct irq_domain_chip_generic *dgc = domain->gc;
> > -	struct irq_chip_generic *gc = dgc->gc[0];
> > +	struct irq_chip_generic *bgc = dgc->gc[0];
> > +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> >  
> >  	/* Disable interrupt on AIC5 */
> > -	irq_gc_lock(gc);
> > +	irq_gc_lock(bgc);
> 
> Why is this locking dgc->gc[0] and fiddling with some other generic
> chip?

Actually, we always access the same set of registers for all irqs of the
domain, and thus need to take the same lock (I chose the one contained
in the first generic irqchip, but I guess it could work with the others
too, as long as we always take the same one) before accessing them
because the configuration is done in two steps:

1/ specify the irq line you want to configure
2/ set the new configuration

Regarding register accesses, all generic chips are configured to
point to the same registers, so accessing them from the 'base' generic
chip or from the generic chip attached to the irq_data struct is the
same, though I agree that using bgc would add some consistency to the
implementation.

This leaves the mask_cache value, which should be updated on the generic
chip attached to the irq_data (but since the lock is global to the
whole domain, it should work fine).

Please let us know if you see other problems, or have a better
solution to fix the bug.

Best Regards,

Boris

> 
> >  	irq_reg_writel(gc, d->hwirq, AT91_AIC5_SSR);
> >  	irq_reg_writel(gc, 1, AT91_AIC5_IDCR);
> 
> Thanks,
> 
> 	tglx



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

#1230236

On Tue, 22 Sep 2015, Boris Brezillon wrote:
> On Tue, 22 Sep 2015 12:27:08 +0200 (CEST)
> Thomas Gleixner <tglx@linutronix.de> wrote:
> > Why is this locking dgc->gc[0] and fiddling with some other generic
> > chip?
> 
> Actually, we always access the same set of registers for all irqs of the
> domain, and thus need to take the same lock (I chose the one contained
> in the first generic irqchip, but I guess it could work with the others
> too, as long as we always take the same one) before accessing them
> because the configuration is done in two steps:
> 
> 1/ specify the irq line you want to configure
> 2/ set the new configuration
> 
> Regarding register accesses, all generic chips are configured to
> point to the same registers, so accessing them from the 'base' generic
> chip or from the generic chip attached to the irq_data struct is the
> same, though I agree that using bgc would add some consistency to the
> implementation.

Fair enough. It just deserves a comment for the casual reader.

Thanks,

	tglx
--
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]

#1230248

On Tue, Sep 22, 2015 at 03:50:30PM +0200, Thomas Gleixner wrote:
> On Tue, 22 Sep 2015, Boris Brezillon wrote:
> > On Tue, 22 Sep 2015 12:27:08 +0200 (CEST)
> > Thomas Gleixner <tglx@linutronix.de> wrote:
> > > Why is this locking dgc->gc[0] and fiddling with some other generic
> > > chip?
> > 
> > Actually, we always access the same set of registers for all irqs of the
> > domain, and thus need to take the same lock (I chose the one contained
> > in the first generic irqchip, but I guess it could work with the others
> > too, as long as we always take the same one) before accessing them
> > because the configuration is done in two steps:
> > 
> > 1/ specify the irq line you want to configure
> > 2/ set the new configuration
> > 
> > Regarding register accesses, all generic chips are configured to
> > point to the same registers, so accessing them from the 'base' generic
> > chip or from the generic chip attached to the irq_data struct is the
> > same, though I agree that using bgc would add some consistency to the
> > implementation.
> 
> Fair enough. It just deserves a comment for the casual reader.
> 

Thanks for the addition of the comment.

Ludovic
--
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]

#1230061 — Re: [tip:irq/urgent] irqchip/atmel-aic5: Use proper mask cache in mask/unmask()

On Tue, 22 Sep 2015, tip-bot for Ludovic Desroches wrote:

> Commit-ID:  478cc7b33413a4ad50852239d931d55e622f1cde
> Gitweb:     http://git.kernel.org/tip/478cc7b33413a4ad50852239d931d55e622f1cde

FYI, I zapped that commit again after noticing that locking
strangeness.

Thanks,

	tglx
--
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]

#1230064 — [tip:irq/urgent] irqchip/atmel-aic5: Use proper mask cache in mask/unmask()

Commit-ID:  478cc7b33413a4ad50852239d931d55e622f1cde
Gitweb:     http://git.kernel.org/tip/478cc7b33413a4ad50852239d931d55e622f1cde
Author:     Ludovic Desroches <ludovic.desroches@atmel.com>
AuthorDate: Mon, 21 Sep 2015 15:46:04 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 22 Sep 2015 12:25:12 +0200

irqchip/atmel-aic5: Use proper mask cache in mask/unmask()

When masking/unmasking interrupts, mask_cache is updated and used later
for suspend/resume. Unfortunately, it always was the mask_cache
associated with the first irq chip which was updated. So when performing
resume, only irqs 0-31 could be enabled and maybe not the good ones!

Fixes: b1479ebb7720 ("irqchip: atmel-aic: Add atmel AIC/AIC5 drivers")
Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
Cc: <sasha.levin@oracle.com>
Cc: <linux-arm-kernel@lists.infradead.org>
Cc: <nicolas.ferre@atmel.com>
Cc: <alexandre.belloni@free-electrons.com>
Cc: <boris.brezillon@free-electrons.com>
Cc: <Wenyou.Yang@atmel.com>
Cc: <jason@lakedaemon.net>
Cc: <marc.zyngier@arm.com>
Cc: stable@vger.kernel.org #3.18
Link: http://lkml.kernel.org/r/1442843173-2390-1-git-send-email-ludovic.desroches@atmel.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-atmel-aic5.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 9da9942..6c5fd25 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -88,28 +88,30 @@ static void aic5_mask(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
 	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *gc = dgc->gc[0];
+	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 
 	/* Disable interrupt on AIC5 */
-	irq_gc_lock(gc);
+	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(gc);
+	irq_gc_unlock(bgc);
 }
 
 static void aic5_unmask(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
 	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *gc = dgc->gc[0];
+	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 
 	/* Enable interrupt on AIC5 */
-	irq_gc_lock(gc);
+	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(gc);
+	irq_gc_unlock(bgc);
 }
 
 static int aic5_retrigger(struct irq_data *d)
--
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]

#1230240 — [tip:irq/urgent] irqchip/atmel-aic5: Use per chip mask caches in mask/unmask()

Commit-ID:  d32dc9aa10c739363c775baf4499416b2e0dc11f
Gitweb:     http://git.kernel.org/tip/d32dc9aa10c739363c775baf4499416b2e0dc11f
Author:     Ludovic Desroches <ludovic.desroches@atmel.com>
AuthorDate: Mon, 21 Sep 2015 15:46:04 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 22 Sep 2015 15:55:51 +0200

irqchip/atmel-aic5: Use per chip mask caches in mask/unmask()

When masking/unmasking interrupts, mask_cache is updated and used later
for suspend/resume. Unfortunately, it always was the mask_cache
associated with the first irq chip which was updated. So when performing
resume, only irqs 0-31 could be enabled.

Fixes: b1479ebb7720 ("irqchip: atmel-aic: Add atmel AIC/AIC5 drivers")
Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
Cc: <sasha.levin@oracle.com>
Cc: <linux-arm-kernel@lists.infradead.org>
Cc: <nicolas.ferre@atmel.com>
Cc: <alexandre.belloni@free-electrons.com>
Cc: <boris.brezillon@free-electrons.com>
Cc: <Wenyou.Yang@atmel.com>
Cc: <jason@lakedaemon.net>
Cc: <marc.zyngier@arm.com>
Cc: stable@vger.kernel.org #3.18
Link: http://lkml.kernel.org/r/1442843173-2390-1-git-send-email-ludovic.desroches@atmel.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-atmel-aic5.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 9da9942..f6d6804 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -88,28 +88,36 @@ static void aic5_mask(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
 	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *gc = dgc->gc[0];
+	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 
-	/* Disable interrupt on AIC5 */
-	irq_gc_lock(gc);
+	/*
+	 * 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(gc);
+	irq_gc_unlock(bgc);
 }
 
 static void aic5_unmask(struct irq_data *d)
 {
 	struct irq_domain *domain = d->domain;
 	struct irq_domain_chip_generic *dgc = domain->gc;
-	struct irq_chip_generic *gc = dgc->gc[0];
+	struct irq_chip_generic *bgc = dgc->gc[0];
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 
-	/* Enable interrupt on AIC5 */
-	irq_gc_lock(gc);
+	/*
+	 * 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(gc);
+	irq_gc_unlock(bgc);
 }
 
 static int aic5_retrigger(struct irq_data *d)
--
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]

Back to top | Article view | linux.kernel

csiph-web