[PATCH] genirq: allow an alternative setup for the mask cache

[PATCH] genirq: allow an alternative setup for the mask cache

[Holger Brunck]

The same interrupt mask cache (stored within struct irq_chip_generic)
is shared between all the irq_chip_type instances by default. But this
does not work on Orion SOCs which have separate mask registers for edge
and level interrupts. Therefore refactor the code that we always use a
pointer to access the mask register. By default it points to
gc->mask_cache for Orion SOCs it points to ct->mask_cache which is
setup in irq_setup_alt_chip().

Signed-off-by: Holger Brunck <holger.brunck@keymile.com>
---
 include/linux/irq.h       |    3 +++
 kernel/irq/generic-chip.c |   19 ++++++++++++-------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index bc4e066..6bf2447 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -654,6 +654,7 @@ struct irq_chip_type {
 	struct irq_chip_regs	regs;
 	irq_flow_handler_t	handler;
 	u32			type;
+	u32			mask_cache;
 };
 
 /**
@@ -663,6 +664,7 @@ struct irq_chip_type {
  * @irq_base:		Interrupt base nr for this chip
  * @irq_cnt:		Number of interrupts handled by this chip
  * @mask_cache:		Cached mask register
+ * @pmask_cache:	pointer to cached mask register
  * @type_cache:		Cached type register
  * @polarity_cache:	Cached polarity register
  * @wake_enabled:	Interrupt can wakeup from suspend
@@ -684,6 +686,7 @@ struct irq_chip_generic {
 	unsigned int		irq_base;
 	unsigned int		irq_cnt;
 	u32			mask_cache;
+	u32			*pmask_cache;
 	u32			type_cache;
 	u32			polarity_cache;
 	u32			wake_enabled;
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index c89295a..1be14a3 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -43,7 +43,7 @@ void irq_gc_mask_disable_reg(struct irq_data *d)
 
 	irq_gc_lock(gc);
 	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->disable);
-	gc->mask_cache &= ~mask;
+	*gc->pmask_cache &= ~mask;
 	irq_gc_unlock(gc);
 }
 
@@ -60,8 +60,8 @@ void irq_gc_mask_set_bit(struct irq_data *d)
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache |= mask;
-	irq_reg_writel(gc->mask_cache, gc->reg_base + cur_regs(d)->mask);
+	*gc->pmask_cache |= mask;
+	irq_reg_writel(*gc->pmask_cache, gc->reg_base + cur_regs(d)->mask);
 	irq_gc_unlock(gc);
 }
 
@@ -78,8 +78,8 @@ void irq_gc_mask_clr_bit(struct irq_data *d)
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache &= ~mask;
-	irq_reg_writel(gc->mask_cache, gc->reg_base + cur_regs(d)->mask);
+	*gc->pmask_cache &= ~mask;
+	irq_reg_writel(*gc->pmask_cache, gc->reg_base + cur_regs(d)->mask);
 	irq_gc_unlock(gc);
 }
 
@@ -97,7 +97,7 @@ void irq_gc_unmask_enable_reg(struct irq_data *d)
 
 	irq_gc_lock(gc);
 	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->enable);
-	gc->mask_cache |= mask;
+	*gc->pmask_cache |= mask;
 	irq_gc_unlock(gc);
 }
 
@@ -243,9 +243,12 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
 	list_add_tail(&gc->list, &gc_list);
 	raw_spin_unlock(&gc_lock);
 
+	/* Setup pointer to mask_cache */
+	gc->pmask_cache = &gc->mask_cache;
+
 	/* Init mask cache ? */
 	if (flags & IRQ_GC_INIT_MASK_CACHE)
-		gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
+		*gc->pmask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
 
 	for (i = gc->irq_base; msk; msk >>= 1, i++) {
 		if (!(msk & 0x01))
@@ -275,6 +278,8 @@ int irq_setup_alt_chip(struct irq_data *d, unsigned int type)
 	struct irq_chip_type *ct = gc->chip_types;
 	unsigned int i;
 
+	/* Setup pointer to the mask_cache */
+	gc->pmask_cache = &ct->mask_cache;
 	for (i = 0; i < gc->num_ct; i++, ct++) {
 		if (ct->type & type) {
 			d->chip = &ct->chip;
-- 
1.7.1

[PATCH] genirq: allow an alternative setup for the mask cache

[Simon Guinot]

On Thu, Mar 14, 2013 at 05:10:30PM +0100, Holger Brunck wrote:
> The same interrupt mask cache (stored within struct irq_chip_generic)
> is shared between all the irq_chip_type instances by default. But this
> does not work on Orion SOCs which have separate mask registers for edge
> and level interrupts. Therefore refactor the code that we always use a
> pointer to access the mask register. By default it points to
> gc->mask_cache for Orion SOCs it points to ct->mask_cache which is
> setup in irq_setup_alt_chip().

Thanks for patch. I was really willing to do it but you have been
faster.

> 
> Signed-off-by: Holger Brunck <holger.brunck@keymile.com>

You may add here:

Reported-by: Joey Oravec <joravec@drewtech.com>

Joey took some time to report and describe this bug.

> ---
>  include/linux/irq.h       |    3 +++
>  kernel/irq/generic-chip.c |   19 ++++++++++++-------
>  2 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index bc4e066..6bf2447 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -654,6 +654,7 @@ struct irq_chip_type {
>  	struct irq_chip_regs	regs;
>  	irq_flow_handler_t	handler;
>  	u32			type;
> +	u32			mask_cache;
>  };
>  
>  /**
> @@ -663,6 +664,7 @@ struct irq_chip_type {
>   * @irq_base:		Interrupt base nr for this chip
>   * @irq_cnt:		Number of interrupts handled by this chip
>   * @mask_cache:		Cached mask register
> + * @pmask_cache:	pointer to cached mask register
>   * @type_cache:		Cached type register
>   * @polarity_cache:	Cached polarity register
>   * @wake_enabled:	Interrupt can wakeup from suspend
> @@ -684,6 +686,7 @@ struct irq_chip_generic {
>  	unsigned int		irq_base;
>  	unsigned int		irq_cnt;
>  	u32			mask_cache;
> +	u32			*pmask_cache;
>  	u32			type_cache;
>  	u32			polarity_cache;
>  	u32			wake_enabled;
> diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
> index c89295a..1be14a3 100644
> --- a/kernel/irq/generic-chip.c
> +++ b/kernel/irq/generic-chip.c
> @@ -43,7 +43,7 @@ void irq_gc_mask_disable_reg(struct irq_data *d)
>  
>  	irq_gc_lock(gc);
>  	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->disable);
> -	gc->mask_cache &= ~mask;
> +	*gc->pmask_cache &= ~mask;
>  	irq_gc_unlock(gc);
>  }
>  
> @@ -60,8 +60,8 @@ void irq_gc_mask_set_bit(struct irq_data *d)
>  	u32 mask = 1 << (d->irq - gc->irq_base);
>  
>  	irq_gc_lock(gc);
> -	gc->mask_cache |= mask;
> -	irq_reg_writel(gc->mask_cache, gc->reg_base + cur_regs(d)->mask);
> +	*gc->pmask_cache |= mask;
> +	irq_reg_writel(*gc->pmask_cache, gc->reg_base + cur_regs(d)->mask);
>  	irq_gc_unlock(gc);
>  }
>  
> @@ -78,8 +78,8 @@ void irq_gc_mask_clr_bit(struct irq_data *d)
>  	u32 mask = 1 << (d->irq - gc->irq_base);
>  
>  	irq_gc_lock(gc);
> -	gc->mask_cache &= ~mask;
> -	irq_reg_writel(gc->mask_cache, gc->reg_base + cur_regs(d)->mask);
> +	*gc->pmask_cache &= ~mask;
> +	irq_reg_writel(*gc->pmask_cache, gc->reg_base + cur_regs(d)->mask);
>  	irq_gc_unlock(gc);
>  }
>  > @@ -97,7 +97,7 @@ void irq_gc_unmask_enable_reg(struct irq_data *d)
>  
>  	irq_gc_lock(gc);
>  	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->enable);
> -	gc->mask_cache |= mask;
> +	*gc->pmask_cache |= mask;
>  	irq_gc_unlock(gc);
>  }
>  
> @@ -243,9 +243,12 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
>  	list_add_tail(&gc->list, &gc_list);
>  	raw_spin_unlock(&gc_lock);
>  
> +	/* Setup pointer to mask_cache */
> +	gc->pmask_cache = &gc->mask_cache;

You need a flag here to choose between gc->mask_cache and
ct->mask_cache.

> +
>  	/* Init mask cache ? */
>  	if (flags & IRQ_GC_INIT_MASK_CACHE)
> -		gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
> +		*gc->pmask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
>  
>  	for (i = gc->irq_base; msk; msk >>= 1, i++) {
>  		if (!(msk & 0x01))
> @@ -275,6 +278,8 @@ int irq_setup_alt_chip(struct irq_data *d, unsigned int type)
>  	struct irq_chip_type *ct = gc->chip_types;
>  	unsigned int i;
>  
> +	/* Setup pointer to the mask_cache */
> +	gc->pmask_cache = &ct->mask_cache;

You also need to check the flag here, to know if you need to switch
pmask_cache on ct->mask_cache.

>  	for (i = 0; i < gc->num_ct; i++, ct++) {
>  		if (ct->type & type) {
>  			d->chip = &ct->chip;

Additionally, you need to update drivers/gpio/gpio-mvebu.c which use
gc->mask_cache. The gpio-mvebu driver is also impacted by the bug. 

Regards,

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130314/c5428cd1/attachment.sig>

[PATCH] genirq: allow an alternative setup for the mask cache

[Holger Brunck]

On 03/14/2013 06:45 PM, Simon Guinot wrote:
> On Thu, Mar 14, 2013 at 05:10:30PM +0100, Holger Brunck wrote:
>> The same interrupt mask cache (stored within struct irq_chip_generic)
>> is shared between all the irq_chip_type instances by default. But this
>> does not work on Orion SOCs which have separate mask registers for edge
>> and level interrupts. Therefore refactor the code that we always use a
>> pointer to access the mask register. By default it points to
>> gc->mask_cache for Orion SOCs it points to ct->mask_cache which is
>> setup in irq_setup_alt_chip().
> 
> Thanks for patch. I was really willing to do it but you have been
> faster.
> 
>>
>> Signed-off-by: Holger Brunck <holger.brunck@keymile.com>
> 
> You may add here:
> 
> Reported-by: Joey Oravec <joravec@drewtech.com>
> 
> Joey took some time to report and describe this bug.
> 

ok will do.

>> ---
>>  include/linux/irq.h       |    3 +++
>>  kernel/irq/generic-chip.c |   19 ++++++++++++-------
>>  2 files changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index bc4e066..6bf2447 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -654,6 +654,7 @@ struct irq_chip_type {
>>  	struct irq_chip_regs	regs;
>>  	irq_flow_handler_t	handler;
>>  	u32			type;
>> +	u32			mask_cache;
>>  };
>>  
>>  /**
>> @@ -663,6 +664,7 @@ struct irq_chip_type {
>>   * @irq_base:		Interrupt base nr for this chip
>>   * @irq_cnt:		Number of interrupts handled by this chip
>>   * @mask_cache:		Cached mask register
>> + * @pmask_cache:	pointer to cached mask register
>>   * @type_cache:		Cached type register
>>   * @polarity_cache:	Cached polarity register
>>   * @wake_enabled:	Interrupt can wakeup from suspend
>> @@ -684,6 +686,7 @@ struct irq_chip_generic {
>>  	unsigned int		irq_base;
>>  	unsigned int		irq_cnt;
>>  	u32			mask_cache;
>> +	u32			*pmask_cache;
>>  	u32			type_cache;
>>  	u32			polarity_cache;
>>  	u32			wake_enabled;
>> diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
>> index c89295a..1be14a3 100644
>> --- a/kernel/irq/generic-chip.c
>> +++ b/kernel/irq/generic-chip.c
>> @@ -43,7 +43,7 @@ void irq_gc_mask_disable_reg(struct irq_data *d)
>>  
>>  	irq_gc_lock(gc);
>>  	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->disable);
>> -	gc->mask_cache &= ~mask;
>> +	*gc->pmask_cache &= ~mask;
>>  	irq_gc_unlock(gc);
>>  }
>>  
>> @@ -60,8 +60,8 @@ void irq_gc_mask_set_bit(struct irq_data *d)
>>  	u32 mask = 1 << (d->irq - gc->irq_base);
>>  
>>  	irq_gc_lock(gc);
>> -	gc->mask_cache |= mask;
>> -	irq_reg_writel(gc->mask_cache, gc->reg_base + cur_regs(d)->mask);
>> +	*gc->pmask_cache |= mask;
>> +	irq_reg_writel(*gc->pmask_cache, gc->reg_base + cur_regs(d)->mask);
>>  	irq_gc_unlock(gc);
>>  }
>>  
>> @@ -78,8 +78,8 @@ void irq_gc_mask_clr_bit(struct irq_data *d)
>>  	u32 mask = 1 << (d->irq - gc->irq_base);
>>  
>>  	irq_gc_lock(gc);
>> -	gc->mask_cache &= ~mask;
>> -	irq_reg_writel(gc->mask_cache, gc->reg_base + cur_regs(d)->mask);
>> +	*gc->pmask_cache &= ~mask;
>> +	irq_reg_writel(*gc->pmask_cache, gc->reg_base + cur_regs(d)->mask);
>>  	irq_gc_unlock(gc);
>>  }
>>  > @@ -97,7 +97,7 @@ void irq_gc_unmask_enable_reg(struct irq_data *d)
>>  
>>  	irq_gc_lock(gc);
>>  	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->enable);
>> -	gc->mask_cache |= mask;
>> +	*gc->pmask_cache |= mask;
>>  	irq_gc_unlock(gc);
>>  }
>>  
>> @@ -243,9 +243,12 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
>>  	list_add_tail(&gc->list, &gc_list);
>>  	raw_spin_unlock(&gc_lock);
>>  
>> +	/* Setup pointer to mask_cache */
>> +	gc->pmask_cache = &gc->mask_cache;
> 
> You need a flag here to choose between gc->mask_cache and
> ct->mask_cache.
> 

hm, even here? Isn't it enough to do this if irq_setup_alt_chip is called?

>> +
>>  	/* Init mask cache ? */
>>  	if (flags & IRQ_GC_INIT_MASK_CACHE)
>> -		gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
>> +		*gc->pmask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
>>  
>>  	for (i = gc->irq_base; msk; msk >>= 1, i++) {
>>  		if (!(msk & 0x01))
>> @@ -275,6 +278,8 @@ int irq_setup_alt_chip(struct irq_data *d, unsigned int type)
>>  	struct irq_chip_type *ct = gc->chip_types;
>>  	unsigned int i;
>>  
>> +	/* Setup pointer to the mask_cache */
>> +	gc->pmask_cache = &ct->mask_cache;
> 
> You also need to check the flag here, to know if you need to switch
> pmask_cache on ct->mask_cache.
>

yes. I'll fix this in v2.

>>  	for (i = 0; i < gc->num_ct; i++, ct++) {
>>  		if (ct->type & type) {
>>  			d->chip = &ct->chip;
> 
> Additionally, you need to update drivers/gpio/gpio-mvebu.c which use
> gc->mask_cache. The gpio-mvebu driver is also impacted by the bug. 
>

I have no chance to test this, so couldn't you or someone else do this on top of
my changes?

Thanks,
Holger

[PATCH] genirq: allow an alternative setup for the mask cache

[Thomas Gleixner]

On Fri, 15 Mar 2013, Holger Brunck wrote:
> On 03/14/2013 06:45 PM, Simon Guinot wrote:
> >> @@ -243,9 +243,12 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
> >>  	list_add_tail(&gc->list, &gc_list);
> >>  	raw_spin_unlock(&gc_lock);
> >>  
> >> +	/* Setup pointer to mask_cache */
> >> +	gc->pmask_cache = &gc->mask_cache;
> > 
> > You need a flag here to choose between gc->mask_cache and
> > ct->mask_cache.
> > 
> 
> hm, even here? Isn't it enough to do this if irq_setup_alt_chip is called?

Yes, you need to do it at both places. We install ct[0] by default, so
if nothing calls setup_alt_chip() you lost.
 
Thanks,

	tglx

[PATCH] genirq: allow an alternative setup for the mask cache

[Gerlando Falauto]

Hi Simon,

On 03/14/2013 06:45 PM, Simon Guinot wrote:
> On Thu, Mar 14, 2013 at 05:10:30PM +0100, Holger Brunck wrote:
>> @@ -243,9 +243,12 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
>>   	list_add_tail(&gc->list, &gc_list);
>>   	raw_spin_unlock(&gc_lock);
>>
>> +	/* Setup pointer to mask_cache */
>> +	gc->pmask_cache = &gc->mask_cache;
>
> You need a flag here to choose between gc->mask_cache and
> ct->mask_cache.

I'm sorry, I don't understand...
Shouldn't pmask_cache be a pointer within ct, so that the pointers for 
all instances could either be the same (in case of a single register) or 
all different (in case of multiple registers)?
I already have a patch which seems to work, but I'm afraid I've come to 
YAM (Yet Another Misunderstanding (R) ) in regards to Thomas' suggestion.

>> +
>>   	/* Init mask cache ? */
>>   	if (flags & IRQ_GC_INIT_MASK_CACHE)
>> -		gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
>> +		*gc->pmask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
>>
>>   	for (i = gc->irq_base; msk; msk >>= 1, i++) {
>>   		if (!(msk & 0x01))
>> @@ -275,6 +278,8 @@ int irq_setup_alt_chip(struct irq_data *d, unsigned int type)
>>   	struct irq_chip_type *ct = gc->chip_types;
>>   	unsigned int i;
>>
>> +	/* Setup pointer to the mask_cache */
>> +	gc->pmask_cache = &ct->mask_cache;
>
> You also need to check the flag here, to know if you need to switch
> pmask_cache on ct->mask_cache.
>
>>   	for (i = 0; i < gc->num_ct; i++, ct++) {
>>   		if (ct->type & type) {
>>   			d->chip = &ct->chip;
>
> Additionally, you need to update drivers/gpio/gpio-mvebu.c which use
> gc->mask_cache. The gpio-mvebu driver is also impacted by the bug.

I understand gpio-mvebu.c is a new driver for all three variants of 
Marvell Chips (Orion, MV78200, ArmadaXP).
Which ones are affected by this bug (i.e. have separate mask registers)? 
Definitely Orion, but what about the other two?

Thanks a lot,
Gerlando

[PATCH] genirq: allow an alternative setup for the mask cache

[Thomas Gleixner]

On Fri, 15 Mar 2013, Gerlando Falauto wrote:

> Hi Simon,
> 
> On 03/14/2013 06:45 PM, Simon Guinot wrote:
> > On Thu, Mar 14, 2013 at 05:10:30PM +0100, Holger Brunck wrote:
> > > @@ -243,9 +243,12 @@ void irq_setup_generic_chip(struct irq_chip_generic
> > > *gc, u32 msk,
> > >   	list_add_tail(&gc->list, &gc_list);
> > >   	raw_spin_unlock(&gc_lock);
> > > 
> > > +	/* Setup pointer to mask_cache */
> > > +	gc->pmask_cache = &gc->mask_cache;
> > 
> > You need a flag here to choose between gc->mask_cache and
> > ct->mask_cache.
> 
> I'm sorry, I don't understand...
> Shouldn't pmask_cache be a pointer within ct, so that the pointers for all
> instances could either be the same (in case of a single register) or all
> different (in case of multiple registers)?

You are right. gc->pmask_cache wont solve the issue.

We need a ct->pmask_cache, so the various instances of gc->ct[] can
have their own selector, which default to gc-mask_cache if not
requested to be separate by a flag. In that case we need to initialize
the ct->mask_cache at setup time for each ct. No modifications to
setup_alt_chip() required then.

> I already have a patch which seems to work, but I'm afraid I've come to YAM
> (Yet Another Misunderstanding (R) ) in regards to Thomas' suggestion.

No, that was my brain going backwards :)

Care to send your patch ?

Thanks,

	tglx

[PATCH] genirq: allow an alternative setup for the mask cache

[Thomas Gleixner]

On Thu, 14 Mar 2013, Holger Brunck wrote:

> The same interrupt mask cache (stored within struct irq_chip_generic)
> is shared between all the irq_chip_type instances by default. But this
> does not work on Orion SOCs which have separate mask registers for edge
> and level interrupts. Therefore refactor the code that we always use a
> pointer to access the mask register. By default it points to
> gc->mask_cache for Orion SOCs it points to ct->mask_cache which is
> setup in irq_setup_alt_chip().

Sigh.
 
> @@ -275,6 +278,8 @@ int irq_setup_alt_chip(struct irq_data *d, unsigned int type)
>  	struct irq_chip_type *ct = gc->chip_types;
>  	unsigned int i;
>  
> +	/* Setup pointer to the mask_cache */
> +	gc->pmask_cache = &ct->mask_cache;

You cannot do this unconditionally as this might break existing
drivers.

See my previous mail:

I'd rather refactor the core code so it uses a pointer to the
mask_cache. The default would be to let it point to gc->mask_cache and
optionally let it point to ct->mask_cache. We'd need to store the flag
in the gc struct so we can redirect the pointer to the ct->mask_cache
in irq_setup_alt_chip().

I wrote that for a reason.

Thanks,

	tglx

[PATCH] genirq: allow an alternative setup for the mask cache

[Ezequiel Garcia]

Hi,

On Thu, Mar 14, 2013 at 1:10 PM, Holger Brunck
<holger.brunck@keymile.com> wrote:
> The same interrupt mask cache (stored within struct irq_chip_generic)
> is shared between all the irq_chip_type instances by default. But this
> does not work on Orion SOCs which have separate mask registers for edge
> and level interrupts. Therefore refactor the code that we always use a
> pointer to access the mask register. By default it points to
> gc->mask_cache for Orion SOCs it points to ct->mask_cache which is
> setup in irq_setup_alt_chip().
>

When you say this does not work for Orion SoC, are you aware there
are two GPIO drivers? One at plat-orion and *another* one at drivers/gpio?

Have you tried and reproduced this regression with both of them?
-- 
    Ezequiel

[PATCH] genirq: allow an alternative setup for the mask cache

[Gerlando Falauto]

Hi Ezequiel,

On 03/14/2013 08:42 PM, Ezequiel Garcia wrote:
> Hi,
>
> On Thu, Mar 14, 2013 at 1:10 PM, Holger Brunck
> <holger.brunck@keymile.com> wrote:
>> The same interrupt mask cache (stored within struct irq_chip_generic)
>> is shared between all the irq_chip_type instances by default. But this
>> does not work on Orion SOCs which have separate mask registers for edge
>> and level interrupts. Therefore refactor the code that we always use a
>> pointer to access the mask register. By default it points to
>> gc->mask_cache for Orion SOCs it points to ct->mask_cache which is
>> setup in irq_setup_alt_chip().
>>
>
> When you say this does not work for Orion SoC, are you aware there
> are two GPIO drivers? One at plat-orion and *another* one at drivers/gpio?

Yes, we are now aware of that, but apparently this was new driver was 
introduced around 3.6 which we are not yet using.

> Have you tried and reproduced this regression with both of them?

No, not really. Would you be able to test this?
BTW, do you know whether all three variants of Marvell Chipsets use 
separate registers, or it's only the Orion?

Thank you,
Gerlando

[PATCH v2 0/2] refactoring for mask_cache

[Gerlando Falauto]

Hi everyone,
here is a 2-patch set to address the issue found with Orion,
hoping I understood correctly what Thomas meant.
Even though I'm also providing changes for mvebu, I only
tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.
We currently do not have a working 3.6+ configuration for our Kirkwood
boards (3.6 is apparently where this mvebu gpio driver was introduced),
so I would be glad if someone could give it a try.
I also have no idea whether the three Marvell variants all have separate
mask registers.

Gerlando Falauto (2):
  genirq: cosmetic: remove cur_regs
  genirq: move mask_cache into struct irq_chip_type

 arch/arm/plat-orion/gpio.c            |    3 +-
 arch/arm/plat-samsung/irq-vic-timer.c |    6 ++--
 arch/mips/jz4740/irq.c                |    3 +-
 drivers/gpio/gpio-mvebu.c             |   23 +++++++------
 include/linux/irq.h                   |    7 ++--
 kernel/irq/generic-chip.c             |   57 +++++++++++++++++++++------------
 6 files changed, 63 insertions(+), 36 deletions(-)

-- 
1.7.10.1

[PATCH 1/2] genirq: cosmetic: remove cur_regs

[Gerlando Falauto]

Since we already have an irq_data_get_chip_type() function which returns
a pointer to irq_chip_type, use that instead of cur_regs().
This will also simplify subsequent patches.

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 kernel/irq/generic-chip.c |   31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index c89295a..0e6ba78 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -16,11 +16,6 @@
 static LIST_HEAD(gc_list);
 static DEFINE_RAW_SPINLOCK(gc_lock);
 
-static inline struct irq_chip_regs *cur_regs(struct irq_data *d)
-{
-	return &container_of(d->chip, struct irq_chip_type, chip)->regs;
-}
-
 /**
  * irq_gc_noop - NOOP function
  * @d: irq_data
@@ -39,10 +34,11 @@ void irq_gc_noop(struct irq_data *d)
 void irq_gc_mask_disable_reg(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->disable);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.disable);
 	gc->mask_cache &= ~mask;
 	irq_gc_unlock(gc);
 }
@@ -57,11 +53,12 @@ void irq_gc_mask_disable_reg(struct irq_data *d)
 void irq_gc_mask_set_bit(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
 	gc->mask_cache |= mask;
-	irq_reg_writel(gc->mask_cache, gc->reg_base + cur_regs(d)->mask);
+	irq_reg_writel(gc->mask_cache, gc->reg_base + ct->regs.mask);
 	irq_gc_unlock(gc);
 }
 
@@ -75,11 +72,12 @@ void irq_gc_mask_set_bit(struct irq_data *d)
 void irq_gc_mask_clr_bit(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
 	gc->mask_cache &= ~mask;
-	irq_reg_writel(gc->mask_cache, gc->reg_base + cur_regs(d)->mask);
+	irq_reg_writel(gc->mask_cache, gc->reg_base + ct->regs.mask);
 	irq_gc_unlock(gc);
 }
 
@@ -93,10 +91,11 @@ void irq_gc_mask_clr_bit(struct irq_data *d)
 void irq_gc_unmask_enable_reg(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->enable);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.enable);
 	gc->mask_cache |= mask;
 	irq_gc_unlock(gc);
 }
@@ -108,10 +107,11 @@ void irq_gc_unmask_enable_reg(struct irq_data *d)
 void irq_gc_ack_set_bit(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.ack);
 	irq_gc_unlock(gc);
 }
 
@@ -122,10 +122,11 @@ void irq_gc_ack_set_bit(struct irq_data *d)
 void irq_gc_ack_clr_bit(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = ~(1 << (d->irq - gc->irq_base));
 
 	irq_gc_lock(gc);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.ack);
 	irq_gc_unlock(gc);
 }
 
@@ -136,11 +137,12 @@ void irq_gc_ack_clr_bit(struct irq_data *d)
 void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->mask);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.mask);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.ack);
 	irq_gc_unlock(gc);
 }
 
@@ -151,10 +153,11 @@ void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
 void irq_gc_eoi(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->eoi);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.eoi);
 	irq_gc_unlock(gc);
 }
 
-- 
1.7.10.1

[PATCH 2/2] genirq: move mask_cache into struct irq_chip_type

[Gerlando Falauto]

This fixes a regression introduced by e59347a
"arm: orion: Use generic irq chip".

The same interrupt mask cache (stored within struct irq_chip_generic)
is shared between all the irq_chip_type instances. As each irq_chip_type
can use a distinct mask register, sharing a single mask cache may not be
correct. For instance in the case of Orion SoCs, which have separate
mask registers for edge and level interrupts.

This patch moves mask_cache from struct irq_chip_generic into struct
irq_chip_type. Note that the interrupt support for Samsung SoCs is also
slightly affected.

Since there are also cases where all irq_chip_type instances share a
common mask register, introduce a pointer to the mask register cache and
a new flag IRQ_GC_SEPARATE_MASK_REGISTERS to explicitly enable this separate
treatment. When this flag is not set, pointers for all irq_chip_type
instances point to the the mask register for the first instance so
essentially the old behavior is retained.

Reported-by: Joey Oravec <joravec@drewtech.com>
Signed-off-by: Simon Guinot <sguinot@lacie.com>
Signed-off-by: Holger Brunck <holger.brunck@keymile.com>
Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 arch/arm/plat-orion/gpio.c            |    3 ++-
 arch/arm/plat-samsung/irq-vic-timer.c |    6 ++++--
 arch/mips/jz4740/irq.c                |    3 ++-
 drivers/gpio/gpio-mvebu.c             |   23 ++++++++++++++---------
 include/linux/irq.h                   |    7 +++++--
 kernel/irq/generic-chip.c             |   30 +++++++++++++++++++++---------
 6 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/arch/arm/plat-orion/gpio.c b/arch/arm/plat-orion/gpio.c
index c29ee7e..a4dc04a 100644
--- a/arch/arm/plat-orion/gpio.c
+++ b/arch/arm/plat-orion/gpio.c
@@ -522,7 +522,8 @@ void __init orion_gpio_init(struct device_node *np,
 	ct->handler = handle_edge_irq;
 	ct->chip.name = ochip->chip.label;
 
-	irq_setup_generic_chip(gc, IRQ_MSK(ngpio), IRQ_GC_INIT_MASK_CACHE,
+	irq_setup_generic_chip(gc, IRQ_MSK(ngpio), IRQ_GC_INIT_MASK_CACHE |
+			       IRQ_GC_SEPARATE_MASK_REGISTERS,
 			       IRQ_NOREQUEST, IRQ_LEVEL | IRQ_NOPROBE);
 
 	/* Setup irq domain on top of the generic chip. */
diff --git a/arch/arm/plat-samsung/irq-vic-timer.c b/arch/arm/plat-samsung/irq-vic-timer.c
index f980cf3..a37ded2 100644
--- a/arch/arm/plat-samsung/irq-vic-timer.c
+++ b/arch/arm/plat-samsung/irq-vic-timer.c
@@ -37,9 +37,11 @@ static void s3c_irq_demux_vic_timer(unsigned int irq, struct irq_desc *desc)
 static void s3c_irq_timer_ack(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_irq_chip_type(d);
+
 	u32 mask = (1 << 5) << (d->irq - gc->irq_base);
 
-	irq_reg_writel(mask | gc->mask_cache, gc->reg_base);
+	irq_reg_writel(mask | *ct->pmask_cache, gc->reg_base);
 }
 
 /**
@@ -89,7 +91,7 @@ void __init s3c_init_vic_timer_irq(unsigned int num, unsigned int timer_irq)
 	irq_setup_generic_chip(s3c_tgc, IRQ_MSK(num), IRQ_GC_INIT_MASK_CACHE,
 			       IRQ_NOREQUEST | IRQ_NOPROBE, 0);
 	/* Clear the upper bits of the mask_cache*/
-	s3c_tgc->mask_cache &= 0x1f;
+	*ct->pmask_cache &= 0x1f;
 
 	for (i = 0; i < num; i++, timer_irq++) {
 		irq_set_chained_handler(pirq[i], s3c_irq_demux_vic_timer);
diff --git a/arch/mips/jz4740/irq.c b/arch/mips/jz4740/irq.c
index fc57ded..228ee33 100644
--- a/arch/mips/jz4740/irq.c
+++ b/arch/mips/jz4740/irq.c
@@ -68,7 +68,8 @@ void jz4740_irq_suspend(struct irq_data *data)
 void jz4740_irq_resume(struct irq_data *data)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
-	jz4740_irq_set_mask(gc, gc->mask_cache);
+	struct irq_chip_type *ct = irq_data_get_irq_chip_type(data);
+	jz4740_irq_set_mask(gc, *ct->pmask_cache);
 }
 
 static struct irqaction jz4740_cascade_action = {
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 6819d63..31ae5c4 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -301,48 +301,52 @@ static void mvebu_gpio_irq_ack(struct irq_data *d)
 static void mvebu_gpio_edge_irq_mask(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	struct mvebu_gpio_chip *mvchip = gc->private;
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache &= ~mask;
-	writel_relaxed(gc->mask_cache, mvebu_gpioreg_edge_mask(mvchip));
+	*ct->pmask_cache &= ~mask;
+	writel_relaxed(*ct->pmask_cache, mvebu_gpioreg_edge_mask(mvchip));
 	irq_gc_unlock(gc);
 }
 
 static void mvebu_gpio_edge_irq_unmask(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	struct mvebu_gpio_chip *mvchip = gc->private;
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache |= mask;
-	writel_relaxed(gc->mask_cache, mvebu_gpioreg_edge_mask(mvchip));
+	*ct->pmask_cache |= mask;
+	writel_relaxed(*ct->pmask_cache, mvebu_gpioreg_edge_mask(mvchip));
 	irq_gc_unlock(gc);
 }
 
 static void mvebu_gpio_level_irq_mask(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	struct mvebu_gpio_chip *mvchip = gc->private;
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache &= ~mask;
-	writel_relaxed(gc->mask_cache, mvebu_gpioreg_level_mask(mvchip));
+	*ct->pmask_cache &= ~mask;
+	writel_relaxed(*ct->pmask_cache, mvebu_gpioreg_level_mask(mvchip));
 	irq_gc_unlock(gc);
 }
 
 static void mvebu_gpio_level_irq_unmask(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	struct mvebu_gpio_chip *mvchip = gc->private;
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache |= mask;
-	writel_relaxed(gc->mask_cache, mvebu_gpioreg_level_mask(mvchip));
+	*ct->pmask_cache |= mask;
+	writel_relaxed(*ct->pmask_cache, mvebu_gpioreg_level_mask(mvchip));
 	irq_gc_unlock(gc);
 }
 
@@ -649,7 +653,8 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
 	ct->handler = handle_edge_irq;
 	ct->chip.name = mvchip->chip.label;
 
-	irq_setup_generic_chip(gc, IRQ_MSK(ngpios), 0,
+	irq_setup_generic_chip(gc, IRQ_MSK(ngpios),
+			       IRQ_GC_SEPARATE_MASK_REGISTERS,
 			       IRQ_NOREQUEST, IRQ_LEVEL | IRQ_NOPROBE);
 
 	/* Setup irq domain on top of the generic chip. */
diff --git a/include/linux/irq.h b/include/linux/irq.h
index fdf2c4a..063fffb 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -636,6 +636,8 @@ struct irq_chip_regs {
  * @regs:		Register offsets for this chip
  * @handler:		Flow handler associated with this chip
  * @type:		Chip can handle these flow types
+ * @mask_cache:		Cached mask register
+ * @pmask_cache:	Pointer to cached mask register
  *
  * A irq_generic_chip can have several instances of irq_chip_type when
  * it requires different functions and register offsets for different
@@ -646,6 +648,8 @@ struct irq_chip_type {
 	struct irq_chip_regs	regs;
 	irq_flow_handler_t	handler;
 	u32			type;
+	u32			mask_cache;
+	u32			*pmask_cache;
 };
 
 /**
@@ -654,7 +658,6 @@ struct irq_chip_type {
  * @reg_base:		Register base address (virtual)
  * @irq_base:		Interrupt base nr for this chip
  * @irq_cnt:		Number of interrupts handled by this chip
- * @mask_cache:		Cached mask register
  * @type_cache:		Cached type register
  * @polarity_cache:	Cached polarity register
  * @wake_enabled:	Interrupt can wakeup from suspend
@@ -675,7 +678,6 @@ struct irq_chip_generic {
 	void __iomem		*reg_base;
 	unsigned int		irq_base;
 	unsigned int		irq_cnt;
-	u32			mask_cache;
 	u32			type_cache;
 	u32			polarity_cache;
 	u32			wake_enabled;
@@ -696,6 +698,7 @@ struct irq_chip_generic {
 enum irq_gc_flags {
 	IRQ_GC_INIT_MASK_CACHE		= 1 << 0,
 	IRQ_GC_INIT_NESTED_LOCK		= 1 << 1,
+	IRQ_GC_SEPARATE_MASK_REGISTERS  = 1 << 2,
 };
 
 /* Generic chip callback functions */
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index 0e6ba78..3daeed3 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -39,7 +39,7 @@ void irq_gc_mask_disable_reg(struct irq_data *d)
 
 	irq_gc_lock(gc);
 	irq_reg_writel(mask, gc->reg_base + ct->regs.disable);
-	gc->mask_cache &= ~mask;
+	*ct->pmask_cache &= ~mask;
 	irq_gc_unlock(gc);
 }
 
@@ -57,8 +57,8 @@ void irq_gc_mask_set_bit(struct irq_data *d)
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache |= mask;
-	irq_reg_writel(gc->mask_cache, gc->reg_base + ct->regs.mask);
+	*ct->pmask_cache |= mask;
+	irq_reg_writel(*ct->pmask_cache, gc->reg_base + ct->regs.mask);
 	irq_gc_unlock(gc);
 }
 
@@ -76,8 +76,8 @@ void irq_gc_mask_clr_bit(struct irq_data *d)
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache &= ~mask;
-	irq_reg_writel(gc->mask_cache, gc->reg_base + ct->regs.mask);
+	*ct->pmask_cache &= ~mask;
+	irq_reg_writel(*ct->pmask_cache, gc->reg_base + ct->regs.mask);
 	irq_gc_unlock(gc);
 }
 
@@ -96,7 +96,7 @@ void irq_gc_unmask_enable_reg(struct irq_data *d)
 
 	irq_gc_lock(gc);
 	irq_reg_writel(mask, gc->reg_base + ct->regs.enable);
-	gc->mask_cache |= mask;
+	*ct->pmask_cache |= mask;
 	irq_gc_unlock(gc);
 }
 
@@ -246,9 +246,21 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
 	list_add_tail(&gc->list, &gc_list);
 	raw_spin_unlock(&gc_lock);
 
-	/* Init mask cache ? */
-	if (flags & IRQ_GC_INIT_MASK_CACHE)
-		gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
+	for (i = 0; i < gc->num_ct; i++) {
+		if (flags & IRQ_GC_SEPARATE_MASK_REGISTERS)
+			/* Define mask cache pointer */
+			ct[i].pmask_cache = &ct[i].mask_cache;
+		else
+			/* They all point to the same mask cache */
+			ct[i].pmask_cache = &ct[0].mask_cache;
+
+		/* Init mask cache ? */
+		if ((flags & IRQ_GC_INIT_MASK_CACHE)
+		 && ((flags & IRQ_GC_SEPARATE_MASK_REGISTER)
+		  || (i == 0)))
+			*ct[i].pmask_cache =
+				irq_reg_readl(gc->reg_base + ct[i].regs.mask);
+	}
 
 	for (i = gc->irq_base; msk; msk >>= 1, i++) {
 		if (!(msk & 0x01))
-- 
1.7.10.1

[PATCH 2/2] genirq: move mask_cache into struct irq_chip_type

[Thomas Gleixner]

Gerlando,

On Fri, 15 Mar 2013, Gerlando Falauto wrote:

> This fixes a regression introduced by e59347a
> "arm: orion: Use generic irq chip".
> 
> The same interrupt mask cache (stored within struct irq_chip_generic)
> is shared between all the irq_chip_type instances. As each irq_chip_type
> can use a distinct mask register, sharing a single mask cache may not be
> correct. For instance in the case of Orion SoCs, which have separate
> mask registers for edge and level interrupts.
> 
> This patch moves mask_cache from struct irq_chip_generic into struct
> irq_chip_type. Note that the interrupt support for Samsung SoCs is also
> slightly affected.

The patch is correct, but we want a more incremental approach.

Step 1: 
     Add the pointer and the mask_cache to ct
     init all ct->pointers in the setup code to gc->mask_cache
     switch core code over to use the ct->pointer

     Functional equivilent!

Step 2:
     Convert each user out of core to the ct->pointer (separate patches)

     Functional equivilent!

Step 3:
     Rename gc->mask_cache to gc->shared_mask_cache

     And we keep that instead of using ct[0].mask_cache simply because
     it makes the code simpler to understand.

Step 4:
     Add the extra flag and the initializer for the separate mask_caches

Step5:
     Convert the affected SOCs (separate patches)

Otherwise I only have a coding style related request:

> @@ -246,9 +246,21 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
>  	list_add_tail(&gc->list, &gc_list);
>  	raw_spin_unlock(&gc_lock);
>  
> -	/* Init mask cache ? */
> -	if (flags & IRQ_GC_INIT_MASK_CACHE)
> -		gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
> +	for (i = 0; i < gc->num_ct; i++) {
> +		if (flags & IRQ_GC_SEPARATE_MASK_REGISTERS)
> +			/* Define mask cache pointer */
> +			ct[i].pmask_cache = &ct[i].mask_cache;
> +		else
> +			/* They all point to the same mask cache */
> +			ct[i].pmask_cache = &ct[0].mask_cache;
> +
> +		/* Init mask cache ? */
> +		if ((flags & IRQ_GC_INIT_MASK_CACHE)
> +		 && ((flags & IRQ_GC_SEPARATE_MASK_REGISTER)
> +		  || (i == 0)))
> +			*ct[i].pmask_cache =
> +				irq_reg_readl(gc->reg_base + ct[i].regs.mask);
> +	}

My eyes are burning, my head is spinning and I have a hard time not to
use swearwords.

	bool mskperct = flags & IRQ_GC_SEPARATE_MASK_REGISTERS;
	bool mskinit = flags & IRQ_GC_INIT_MASK_CACHE;
	u32 *shmsk = &gc->shared_mask_cache;

	if (!mskperct && mskinit)
	     	*shmsk = irq_reg_readl(gc->reg_base + ct->regs.mask);

	for (i = 0; i < gc->num_ct: i++, ct++) {
	       ct->pmask_cache = mskperct ? &ct->mask_cache : shmsk;
	       
	       if (mskperct && mskinit)
	       	  	ct->mask_cache = irq_reg_readl(gc->reg_base +
				       	 	       ct->regs.mask);
	}

Or something like that, perhaps ?

Thanks,

	tglx

[PATCH 2/2] genirq: move mask_cache into struct irq_chip_type

[Gerlando Falauto]

Hi Thomas,

On 03/15/2013 09:47 PM, Thomas Gleixner wrote:
> Gerlando,
>
> On Fri, 15 Mar 2013, Gerlando Falauto wrote:

[...]
>
> The patch is correct, but we want a more incremental approach.

OK, will do.

> Otherwise I only have a coding style related request:
>
>> @@ -246,9 +246,21 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
>>   	list_add_tail(&gc->list, &gc_list);
>>   	raw_spin_unlock(&gc_lock);
>>
>> -	/* Init mask cache ? */
>> -	if (flags & IRQ_GC_INIT_MASK_CACHE)
>> -		gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
>> +	for (i = 0; i < gc->num_ct; i++) {
>> +		if (flags & IRQ_GC_SEPARATE_MASK_REGISTERS)
>> +			/* Define mask cache pointer */
>> +			ct[i].pmask_cache = &ct[i].mask_cache;
>> +		else
>> +			/* They all point to the same mask cache */
>> +			ct[i].pmask_cache = &ct[0].mask_cache;
>> +
>> +		/* Init mask cache ? */
>> +		if ((flags & IRQ_GC_INIT_MASK_CACHE)
>> +		 && ((flags & IRQ_GC_SEPARATE_MASK_REGISTER)
>> +		  || (i == 0)))
>> +			*ct[i].pmask_cache =
>> +				irq_reg_readl(gc->reg_base + ct[i].regs.mask);
>> +	}
>
> My eyes are burning, my head is spinning and I have a hard time not to
> use swearwords.

You're right... and I appreciate you NOT using such words. :-)

Just one question:

> 	bool mskperct = flags & IRQ_GC_SEPARATE_MASK_REGISTERS;
> 	bool mskinit = flags & IRQ_GC_INIT_MASK_CACHE;
> 	u32 *shmsk = &gc->shared_mask_cache;
>
> 	if (!mskperct && mskinit)
> 	     	*shmsk = irq_reg_readl(gc->reg_base + ct->regs.mask);
>
> 	for (i = 0; i < gc->num_ct: i++, ct++) {
> 	       ct->pmask_cache = mskperct ? &ct->mask_cache : shmsk;

What is wrong with using ct[i] instead? I find it a bit more readable.

> Or something like that, perhaps ?

Totally agree.

Thanks a lot for your patience!
Gerlando

[PATCH 2/2] genirq: move mask_cache into struct irq_chip_type

[Thomas Gleixner]

On Mon, 18 Mar 2013, Gerlando Falauto wrote:
> On 03/15/2013 09:47 PM, Thomas Gleixner wrote:
> You're right... and I appreciate you NOT using such words. :-)

I try hard :)
 
> Just one question:
> 
> > 	bool mskperct = flags & IRQ_GC_SEPARATE_MASK_REGISTERS;
> > 	bool mskinit = flags & IRQ_GC_INIT_MASK_CACHE;
> > 	u32 *shmsk = &gc->shared_mask_cache;
> > 
> > 	if (!mskperct && mskinit)
> > 	     	*shmsk = irq_reg_readl(gc->reg_base + ct->regs.mask);
> > 
> > 	for (i = 0; i < gc->num_ct: i++, ct++) {
> > 	       ct->pmask_cache = mskperct ? &ct->mask_cache : shmsk;
> 
> What is wrong with using ct[i] instead? I find it a bit more readable.

Either way is fine. I like the pointers more, but that's my personal
preference and I let you chose whatever you want

Thanks,

	tglx

[PATCH v2 0/2] refactoring for mask_cache

[Andrew Lunn]

On Fri, Mar 15, 2013 at 08:36:13PM +0100, Gerlando Falauto wrote:
> Hi everyone,
> here is a 2-patch set to address the issue found with Orion,
> hoping I understood correctly what Thomas meant.
> Even though I'm also providing changes for mvebu, I only
> tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.
> We currently do not have a working 3.6+ configuration for our Kirkwood
> boards (3.6 is apparently where this mvebu gpio driver was introduced),
> so I would be glad if someone could give it a try.

Hi Garlando

Could you describe how to reproduce the issue, so i can test it on
Kirkwood. I have a kirkwood with one GPIO button which i can use for
tests.

Thanks
	Andrew

[PATCH v2 0/2] refactoring for mask_cache

[Simon Guinot]

On Fri, Mar 15, 2013 at 10:25:45PM +0100, Andrew Lunn wrote:
> On Fri, Mar 15, 2013 at 08:36:13PM +0100, Gerlando Falauto wrote:
> > Hi everyone,
> > here is a 2-patch set to address the issue found with Orion,
> > hoping I understood correctly what Thomas meant.
> > Even though I'm also providing changes for mvebu, I only
> > tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.
> > We currently do not have a working 3.6+ configuration for our Kirkwood
> > boards (3.6 is apparently where this mvebu gpio driver was introduced),
> > so I would be glad if someone could give it a try.
> 
> Hi Garlando
> 
> Could you describe how to reproduce the issue, so i can test it on
> Kirkwood. I have a kirkwood with one GPIO button which i can use for
> tests.

Hi Andrew,

Maybe you could add debugfs entries to the GPIO drivers plat-orion and
gpio-mvebu, in order to allow to dump the GPIO registers. It could even
be reusable for other purposes.

With this debug informations, you can:

1 - Register a level interrupt for GPIO A. This GPIO could be unused.
    Maybe you can do that from some board setup code, once that the
    GPIO subsystem is initialized.
2 - Register an edge interrupt for GPIO B. To do that, you could simply
    load the gpio-key driver. It should eat your GPIO button.
3 - Check the GPIO edge mask register (via debugfs). With the issue,
    GPIO A and GPIO B are unmasked.

Or alternatively, I guess there is a more straightforward way:

1 - Disable the GPIO key driver.
2 - Register a level interrupt for the GPIO button.
3 - Register an edge interrupt for an unused GPIO (take care about
    the active state).
4 - Push and release the button. With the issue, your system should
    freeze. The edge interrupt for the GPIO button is asserted and is
    never acknowledged.

If you want, I can take care of testing the patches.

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130316/06196c25/attachment.sig>

[PATCH v3 0/9] refactoring for mask_cache

[Gerlando Falauto]

Hi everyone,
here is a patchset to address the issue found with Orion, in incremental
stages as Thomas suggested.
a) we introduce the new fields and pointer (though only the shared one is used)
b) we convert all drivers to use it 
c) we rename the field so to force the use of the per-ct pointer
d) we add per-ct mask cache, provided the new flag 
   IRQ_GC_SEPARATE_MASK_REGISTERS is enabled
e) we enable the flag for orion-gpio and mvebu drivers

So even though I'm also providing changes for mvebu, I only
tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.
We currently do not have a working 3.6+ configuration for our Kirkwood
boards (3.6 is apparently where this mvebu gpio driver was introduced),
so I would be glad if someone could give it a try.
I also have no idea whether the three Marvell variants all have separate
mask registers (which is what the last patch assumes).

Gerlando Falauto (9):
  genirq: cosmetic: remove cur_regs
  genirq: add mask_cache and pmask_cache into struct irq_chip_type
  gpio: mvebu: convert to usage of *pmask_cache within irq_chip_type
  MIPS: JZ4740: convert to usage of *pmask_cache within irq_chip_type
  ARM: SAMSUNG: convert to usage of *pmask_cache within irq_chip_type
  genirq: rename mask_cache to shared_mask_cache
  genirq: handle separate mask registers
  orion-gpio: enable IRQ_GC_SEPARATE_MASK_REGISTERS
  gpio: mvebu: enable IRQ_GC_SEPARATE_MASK_REGISTERS

 arch/arm/plat-orion/gpio.c            |    3 +-
 arch/arm/plat-samsung/irq-vic-timer.c |    6 ++--
 arch/mips/jz4740/irq.c                |    3 +-
 drivers/gpio/gpio-mvebu.c             |   23 ++++++++------
 include/linux/irq.h                   |    9 ++++--
 kernel/irq/generic-chip.c             |   55 +++++++++++++++++++++------------
 6 files changed, 64 insertions(+), 35 deletions(-)

-- 
1.7.10.1

[PATCH v3 1/9] genirq: cosmetic: remove cur_regs

[Gerlando Falauto]

Since we already have an irq_data_get_chip_type() function which returns
a pointer to irq_chip_type, use that instead of cur_regs().
This will also simplify subsequent patches.

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 kernel/irq/generic-chip.c |   31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index c89295a..0e6ba78 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -16,11 +16,6 @@
 static LIST_HEAD(gc_list);
 static DEFINE_RAW_SPINLOCK(gc_lock);
 
-static inline struct irq_chip_regs *cur_regs(struct irq_data *d)
-{
-	return &container_of(d->chip, struct irq_chip_type, chip)->regs;
-}
-
 /**
  * irq_gc_noop - NOOP function
  * @d: irq_data
@@ -39,10 +34,11 @@ void irq_gc_noop(struct irq_data *d)
 void irq_gc_mask_disable_reg(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->disable);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.disable);
 	gc->mask_cache &= ~mask;
 	irq_gc_unlock(gc);
 }
@@ -57,11 +53,12 @@ void irq_gc_mask_disable_reg(struct irq_data *d)
 void irq_gc_mask_set_bit(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
 	gc->mask_cache |= mask;
-	irq_reg_writel(gc->mask_cache, gc->reg_base + cur_regs(d)->mask);
+	irq_reg_writel(gc->mask_cache, gc->reg_base + ct->regs.mask);
 	irq_gc_unlock(gc);
 }
 
@@ -75,11 +72,12 @@ void irq_gc_mask_set_bit(struct irq_data *d)
 void irq_gc_mask_clr_bit(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
 	gc->mask_cache &= ~mask;
-	irq_reg_writel(gc->mask_cache, gc->reg_base + cur_regs(d)->mask);
+	irq_reg_writel(gc->mask_cache, gc->reg_base + ct->regs.mask);
 	irq_gc_unlock(gc);
 }
 
@@ -93,10 +91,11 @@ void irq_gc_mask_clr_bit(struct irq_data *d)
 void irq_gc_unmask_enable_reg(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->enable);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.enable);
 	gc->mask_cache |= mask;
 	irq_gc_unlock(gc);
 }
@@ -108,10 +107,11 @@ void irq_gc_unmask_enable_reg(struct irq_data *d)
 void irq_gc_ack_set_bit(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.ack);
 	irq_gc_unlock(gc);
 }
 
@@ -122,10 +122,11 @@ void irq_gc_ack_set_bit(struct irq_data *d)
 void irq_gc_ack_clr_bit(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = ~(1 << (d->irq - gc->irq_base));
 
 	irq_gc_lock(gc);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.ack);
 	irq_gc_unlock(gc);
 }
 
@@ -136,11 +137,12 @@ void irq_gc_ack_clr_bit(struct irq_data *d)
 void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->mask);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.mask);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.ack);
 	irq_gc_unlock(gc);
 }
 
@@ -151,10 +153,11 @@ void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
 void irq_gc_eoi(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->eoi);
+	irq_reg_writel(mask, gc->reg_base + ct->regs.eoi);
 	irq_gc_unlock(gc);
 }
 
-- 
1.7.10.1

[PATCH v3 2/9] genirq: add mask_cache and pmask_cache into struct irq_chip_type

[Gerlando Falauto]

Today the same interrupt mask cache (stored within struct irq_chip_generic)
is shared between all the irq_chip_type instances. As there are instances
where each irq_chip_type uses a distinct mask register (as it is the case
for Orion SoCs), sharing a single mask cache may be incorrect.
So add a distinct pointer for each irq_chip_type, which for now
points to the original mask register within irq_chip_generic.
So no functional changes here.

Reported-by: Joey Oravec <joravec@drewtech.com>
Signed-off-by: Simon Guinot <sguinot@lacie.com>
Signed-off-by: Holger Brunck <holger.brunck@keymile.com>
Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 include/linux/irq.h       |    4 ++++
 kernel/irq/generic-chip.c |   16 ++++++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index fdf2c4a..05d7fbd 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -636,6 +636,8 @@ struct irq_chip_regs {
  * @regs:		Register offsets for this chip
  * @handler:		Flow handler associated with this chip
  * @type:		Chip can handle these flow types
+ * @mask_cache:		Cached mask register
+ * @pmask_cache:	Pointer to cached mask register
  *
  * A irq_generic_chip can have several instances of irq_chip_type when
  * it requires different functions and register offsets for different
@@ -646,6 +648,8 @@ struct irq_chip_type {
 	struct irq_chip_regs	regs;
 	irq_flow_handler_t	handler;
 	u32			type;
+	u32			mask_cache;
+	u32			*pmask_cache;
 };
 
 /**
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index 0e6ba78..c8ec24d 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -39,7 +39,7 @@ void irq_gc_mask_disable_reg(struct irq_data *d)
 
 	irq_gc_lock(gc);
 	irq_reg_writel(mask, gc->reg_base + ct->regs.disable);
-	gc->mask_cache &= ~mask;
+	*ct->pmask_cache &= ~mask;
 	irq_gc_unlock(gc);
 }
 
@@ -57,8 +57,8 @@ void irq_gc_mask_set_bit(struct irq_data *d)
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache |= mask;
-	irq_reg_writel(gc->mask_cache, gc->reg_base + ct->regs.mask);
+	*ct->pmask_cache |= mask;
+	irq_reg_writel(*ct->pmask_cache, gc->reg_base + ct->regs.mask);
 	irq_gc_unlock(gc);
 }
 
@@ -76,8 +76,8 @@ void irq_gc_mask_clr_bit(struct irq_data *d)
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache &= ~mask;
-	irq_reg_writel(gc->mask_cache, gc->reg_base + ct->regs.mask);
+	*ct->pmask_cache &= ~mask;
+	irq_reg_writel(*ct->pmask_cache, gc->reg_base + ct->regs.mask);
 	irq_gc_unlock(gc);
 }
 
@@ -96,7 +96,7 @@ void irq_gc_unmask_enable_reg(struct irq_data *d)
 
 	irq_gc_lock(gc);
 	irq_reg_writel(mask, gc->reg_base + ct->regs.enable);
-	gc->mask_cache |= mask;
+	*ct->pmask_cache |= mask;
 	irq_gc_unlock(gc);
 }
 
@@ -250,6 +250,10 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
 	if (flags & IRQ_GC_INIT_MASK_CACHE)
 		gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
 
+	/* Initialize mask cache pointer */
+	for (i = 0; i < gc->num_ct; i++)
+		ct[i].pmask_cache = &gc->mask_cache;
+
 	for (i = gc->irq_base; msk; msk >>= 1, i++) {
 		if (!(msk & 0x01))
 			continue;
-- 
1.7.10.1

[PATCH v3 2/9] genirq: add mask_cache and pmask_cache into struct irq_chip_type

[Thomas Gleixner]

On Mon, 18 Mar 2013, Gerlando Falauto wrote:

> Today the same interrupt mask cache (stored within struct irq_chip_generic)
> is shared between all the irq_chip_type instances. As there are instances
> where each irq_chip_type uses a distinct mask register (as it is the case
> for Orion SoCs), sharing a single mask cache may be incorrect.
> So add a distinct pointer for each irq_chip_type, which for now
> points to the original mask register within irq_chip_generic.
> So no functional changes here.
> 
> Reported-by: Joey Oravec <joravec@drewtech.com>
> Signed-off-by: Simon Guinot <sguinot@lacie.com>
> Signed-off-by: Holger Brunck <holger.brunck@keymile.com>
> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>

Who wrote the patch?

If it was not you, then please add a From: real author next time.

If it was you, what are the other Signed-off-bys for?

SOB is added by the author and by people who handle the patch passing
it to the next person.

Thanks,

	tglx

[PATCH v3 3/9] gpio: mvebu: convert to usage of *pmask_cache within irq_chip_type

[Gerlando Falauto]

Since we have now introduced pmask_cache within irq_chip_type to also
handle per-chip-type mask registers, convert gpio-mvebu driver to use
this new pointer.

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 drivers/gpio/gpio-mvebu.c |   20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 456663c..81384f4 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -302,48 +302,52 @@ static void mvebu_gpio_irq_ack(struct irq_data *d)
 static void mvebu_gpio_edge_irq_mask(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	struct mvebu_gpio_chip *mvchip = gc->private;
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache &= ~mask;
-	writel_relaxed(gc->mask_cache, mvebu_gpioreg_edge_mask(mvchip));
+	*ct->pmask_cache &= ~mask;
+	writel_relaxed(*ct->pmask_cache, mvebu_gpioreg_edge_mask(mvchip));
 	irq_gc_unlock(gc);
 }
 
 static void mvebu_gpio_edge_irq_unmask(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	struct mvebu_gpio_chip *mvchip = gc->private;
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache |= mask;
-	writel_relaxed(gc->mask_cache, mvebu_gpioreg_edge_mask(mvchip));
+	*ct->pmask_cache |= mask;
+	writel_relaxed(*ct->pmask_cache, mvebu_gpioreg_edge_mask(mvchip));
 	irq_gc_unlock(gc);
 }
 
 static void mvebu_gpio_level_irq_mask(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	struct mvebu_gpio_chip *mvchip = gc->private;
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache &= ~mask;
-	writel_relaxed(gc->mask_cache, mvebu_gpioreg_level_mask(mvchip));
+	*ct->pmask_cache &= ~mask;
+	writel_relaxed(*ct->pmask_cache, mvebu_gpioreg_level_mask(mvchip));
 	irq_gc_unlock(gc);
 }
 
 static void mvebu_gpio_level_irq_unmask(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
 	struct mvebu_gpio_chip *mvchip = gc->private;
 	u32 mask = 1 << (d->irq - gc->irq_base);
 
 	irq_gc_lock(gc);
-	gc->mask_cache |= mask;
-	writel_relaxed(gc->mask_cache, mvebu_gpioreg_level_mask(mvchip));
+	*ct->pmask_cache |= mask;
+	writel_relaxed(*ct->pmask_cache, mvebu_gpioreg_level_mask(mvchip));
 	irq_gc_unlock(gc);
 }
 
-- 
1.7.10.1

[PATCH v3 4/9] MIPS: JZ4740: convert to usage of *pmask_cache within irq_chip_type

[Gerlando Falauto]

Since we have now introduced pmask_cache within irq_chip_type to also
handle per-chip-type mask registers, convert jz4740 irq driver to use
this new pointer.

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 arch/mips/jz4740/irq.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/jz4740/irq.c b/arch/mips/jz4740/irq.c
index fc57ded..228ee33 100644
--- a/arch/mips/jz4740/irq.c
+++ b/arch/mips/jz4740/irq.c
@@ -68,7 +68,8 @@ void jz4740_irq_suspend(struct irq_data *data)
 void jz4740_irq_resume(struct irq_data *data)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
-	jz4740_irq_set_mask(gc, gc->mask_cache);
+	struct irq_chip_type *ct = irq_data_get_irq_chip_type(data);
+	jz4740_irq_set_mask(gc, *ct->pmask_cache);
 }
 
 static struct irqaction jz4740_cascade_action = {
-- 
1.7.10.1

[PATCH v3 5/9] ARM: SAMSUNG: convert to usage of *pmask_cache within irq_chip_type

[Gerlando Falauto]

Since we have now introduced pmask_cache within irq_chip_type to also
handle per-chip-type mask registers, convert samsung irq-vic-timer driver
to use this new pointer.

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 arch/arm/plat-samsung/irq-vic-timer.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-samsung/irq-vic-timer.c b/arch/arm/plat-samsung/irq-vic-timer.c
index f980cf3..a37ded2 100644
--- a/arch/arm/plat-samsung/irq-vic-timer.c
+++ b/arch/arm/plat-samsung/irq-vic-timer.c
@@ -37,9 +37,11 @@ static void s3c_irq_demux_vic_timer(unsigned int irq, struct irq_desc *desc)
 static void s3c_irq_timer_ack(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_irq_chip_type(d);
+
 	u32 mask = (1 << 5) << (d->irq - gc->irq_base);
 
-	irq_reg_writel(mask | gc->mask_cache, gc->reg_base);
+	irq_reg_writel(mask | *ct->pmask_cache, gc->reg_base);
 }
 
 /**
@@ -89,7 +91,7 @@ void __init s3c_init_vic_timer_irq(unsigned int num, unsigned int timer_irq)
 	irq_setup_generic_chip(s3c_tgc, IRQ_MSK(num), IRQ_GC_INIT_MASK_CACHE,
 			       IRQ_NOREQUEST | IRQ_NOPROBE, 0);
 	/* Clear the upper bits of the mask_cache*/
-	s3c_tgc->mask_cache &= 0x1f;
+	*ct->pmask_cache &= 0x1f;
 
 	for (i = 0; i < num; i++, timer_irq++) {
 		irq_set_chained_handler(pirq[i], s3c_irq_demux_vic_timer);
-- 
1.7.10.1

[PATCH v3 6/9] genirq: rename mask_cache to shared_mask_cache

[Gerlando Falauto]

after introducing pmask_cached pointer within irq_chip_type, rename
mask_cache into shared_mask_cache so to state clearly that drivers
should not use it anymore, but should only access *pmask_cache from
irq_chip_type instead.

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 include/linux/irq.h       |    4 ++--
 kernel/irq/generic-chip.c |    5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 05d7fbd..5aca310 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -658,7 +658,7 @@ struct irq_chip_type {
  * @reg_base:		Register base address (virtual)
  * @irq_base:		Interrupt base nr for this chip
  * @irq_cnt:		Number of interrupts handled by this chip
- * @mask_cache:		Cached mask register
+ * @shared_mask_cache:	Cached mask register shared among all irq_chip_type's
  * @type_cache:		Cached type register
  * @polarity_cache:	Cached polarity register
  * @wake_enabled:	Interrupt can wakeup from suspend
@@ -679,7 +679,7 @@ struct irq_chip_generic {
 	void __iomem		*reg_base;
 	unsigned int		irq_base;
 	unsigned int		irq_cnt;
-	u32			mask_cache;
+	u32			shared_mask_cache;
 	u32			type_cache;
 	u32			polarity_cache;
 	u32			wake_enabled;
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index c8ec24d..b5cb991 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -248,11 +248,12 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
 
 	/* Init mask cache ? */
 	if (flags & IRQ_GC_INIT_MASK_CACHE)
-		gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
+		gc->shared_mask_cache =
+			irq_reg_readl(gc->reg_base + ct->regs.mask);
 
 	/* Initialize mask cache pointer */
 	for (i = 0; i < gc->num_ct; i++)
-		ct[i].pmask_cache = &gc->mask_cache;
+		ct[i].pmask_cache = &gc->shared_mask_cache;
 
 	for (i = gc->irq_base; msk; msk >>= 1, i++) {
 		if (!(msk & 0x01))
-- 
1.7.10.1

[PATCH v3 7/9] genirq: handle separate mask registers

[Gerlando Falauto]

There are cases where all irq_chip_type instances have separate mask registers,
making a shared mask register cache unsuitable for the purpose.
So introduce a new flag IRQ_GC_SEPARATE_MASK_REGISTERS to explicitly enable
this separate treatment.
When this flag is not set, pointers for all irq_chip_type instances point
to the the shared mask register as it has been done so far.

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 include/linux/irq.h       |    1 +
 kernel/irq/generic-chip.c |   15 +++++++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 5aca310..9feb06f 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -700,6 +700,7 @@ struct irq_chip_generic {
 enum irq_gc_flags {
 	IRQ_GC_INIT_MASK_CACHE		= 1 << 0,
 	IRQ_GC_INIT_NESTED_LOCK		= 1 << 1,
+	IRQ_GC_SEPARATE_MASK_REGISTERS  = 1 << 2,
 };
 
 /* Generic chip callback functions */
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index b5cb991..ae5ce41 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -241,19 +241,26 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
 {
 	struct irq_chip_type *ct = gc->chip_types;
 	unsigned int i;
+	bool mskperct = flags & IRQ_GC_SEPARATE_MASK_REGISTERS;
+	bool mskinit = flags & IRQ_GC_INIT_MASK_CACHE;
 
 	raw_spin_lock(&gc_lock);
 	list_add_tail(&gc->list, &gc_list);
 	raw_spin_unlock(&gc_lock);
 
 	/* Init mask cache ? */
-	if (flags & IRQ_GC_INIT_MASK_CACHE)
+	if (mskinit && !mskperct)
 		gc->shared_mask_cache =
 			irq_reg_readl(gc->reg_base + ct->regs.mask);
 
-	/* Initialize mask cache pointer */
-	for (i = 0; i < gc->num_ct; i++)
-		ct[i].pmask_cache = &gc->shared_mask_cache;
+	/* Initialize mask cache pointers */
+	for (i = 0; i < gc->num_ct; i++) {
+		ct[i].pmask_cache = mskperct ?
+			&ct[i].mask_cache : &gc->shared_mask_cache;
+		if (mskinit && mskperct)
+			ct[i].mask_cache =
+				irq_reg_readl(gc->reg_base + ct[i].regs.mask);
+	}
 
 	for (i = gc->irq_base; msk; msk >>= 1, i++) {
 		if (!(msk & 0x01))
-- 
1.7.10.1

[PATCH v3 8/9] orion-gpio: enable IRQ_GC_SEPARATE_MASK_REGISTERS

[Gerlando Falauto]

enable handling of separate mask registers for Orion SoC GPIOs,
fixing indeed the regression introduced by e59347a
"arm: orion: Use generic irq chip".

Reported-by: Joey Oravec <joravec@drewtech.com>
Signed-off-by: Simon Guinot <sguinot@lacie.com>
Signed-off-by: Holger Brunck <holger.brunck@keymile.com>
Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 arch/arm/plat-orion/gpio.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/plat-orion/gpio.c b/arch/arm/plat-orion/gpio.c
index c29ee7e..a4dc04a 100644
--- a/arch/arm/plat-orion/gpio.c
+++ b/arch/arm/plat-orion/gpio.c
@@ -522,7 +522,8 @@ void __init orion_gpio_init(struct device_node *np,
 	ct->handler = handle_edge_irq;
 	ct->chip.name = ochip->chip.label;
 
-	irq_setup_generic_chip(gc, IRQ_MSK(ngpio), IRQ_GC_INIT_MASK_CACHE,
+	irq_setup_generic_chip(gc, IRQ_MSK(ngpio), IRQ_GC_INIT_MASK_CACHE |
+			       IRQ_GC_SEPARATE_MASK_REGISTERS,
 			       IRQ_NOREQUEST, IRQ_LEVEL | IRQ_NOPROBE);
 
 	/* Setup irq domain on top of the generic chip. */
-- 
1.7.10.1

[PATCH v3 9/9] gpio: mvebu: enable IRQ_GC_SEPARATE_MASK_REGISTERS

[Gerlando Falauto]

enable handling of separate mask registers for all three SoC variants
handled by this driver.

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 drivers/gpio/gpio-mvebu.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 81384f4..c01376d 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -660,7 +660,8 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
 	ct->handler = handle_edge_irq;
 	ct->chip.name = mvchip->chip.label;
 
-	irq_setup_generic_chip(gc, IRQ_MSK(ngpios), 0,
+	irq_setup_generic_chip(gc, IRQ_MSK(ngpios),
+			       IRQ_GC_SEPARATE_MASK_REGISTERS,
 			       IRQ_NOREQUEST, IRQ_LEVEL | IRQ_NOPROBE);
 
 	/* Setup irq domain on top of the generic chip. */
-- 
1.7.10.1

[PATCH v3 0/9] refactoring for mask_cache

[Simon Guinot]

On Mon, Mar 18, 2013 at 03:00:46PM +0100, Gerlando Falauto wrote:
> Hi everyone,
> here is a patchset to address the issue found with Orion, in incremental
> stages as Thomas suggested.
> a) we introduce the new fields and pointer (though only the shared one is used)
> b) we convert all drivers to use it 
> c) we rename the field so to force the use of the per-ct pointer
> d) we add per-ct mask cache, provided the new flag 
>    IRQ_GC_SEPARATE_MASK_REGISTERS is enabled
> e) we enable the flag for orion-gpio and mvebu drivers
> 
> So even though I'm also providing changes for mvebu, I only
> tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.
> We currently do not have a working 3.6+ configuration for our Kirkwood
> boards (3.6 is apparently where this mvebu gpio driver was introduced),
> so I would be glad if someone could give it a try.
> I also have no idea whether the three Marvell variants all have separate
> mask registers (which is what the last patch assumes).

Accordingly to MV78230/78x60 (Armada-XP) functional specification, we
have separate mask registers for the GPIO level and edge interrupts.

I believe that the GPIO mask register is only shared on Orion and
Kirkwood based SoCs.

Regards,

Simon

> 
> Gerlando Falauto (9):
>   genirq: cosmetic: remove cur_regs
>   genirq: add mask_cache and pmask_cache into struct irq_chip_type
>   gpio: mvebu: convert to usage of *pmask_cache within irq_chip_type
>   MIPS: JZ4740: convert to usage of *pmask_cache within irq_chip_type
>   ARM: SAMSUNG: convert to usage of *pmask_cache within irq_chip_type
>   genirq: rename mask_cache to shared_mask_cache
>   genirq: handle separate mask registers
>   orion-gpio: enable IRQ_GC_SEPARATE_MASK_REGISTERS
>   gpio: mvebu: enable IRQ_GC_SEPARATE_MASK_REGISTERS
> 
>  arch/arm/plat-orion/gpio.c            |    3 +-
>  arch/arm/plat-samsung/irq-vic-timer.c |    6 ++--
>  arch/mips/jz4740/irq.c                |    3 +-
>  drivers/gpio/gpio-mvebu.c             |   23 ++++++++------
>  include/linux/irq.h                   |    9 ++++--
>  kernel/irq/generic-chip.c             |   55 +++++++++++++++++++++------------
>  6 files changed, 64 insertions(+), 35 deletions(-)
> 
> -- 
> 1.7.10.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130318/e0391611/attachment-0001.sig>

[PATCH v3 0/9] refactoring for mask_cache

[Simon Guinot]

On Mon, Mar 18, 2013 at 03:28:17PM +0100, Simon Guinot wrote:
> On Mon, Mar 18, 2013 at 03:00:46PM +0100, Gerlando Falauto wrote:
> > Hi everyone,
> > here is a patchset to address the issue found with Orion, in incremental
> > stages as Thomas suggested.
> > a) we introduce the new fields and pointer (though only the shared one is used)
> > b) we convert all drivers to use it 
> > c) we rename the field so to force the use of the per-ct pointer
> > d) we add per-ct mask cache, provided the new flag 
> >    IRQ_GC_SEPARATE_MASK_REGISTERS is enabled
> > e) we enable the flag for orion-gpio and mvebu drivers
> > 
> > So even though I'm also providing changes for mvebu, I only
> > tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.
> > We currently do not have a working 3.6+ configuration for our Kirkwood
> > boards (3.6 is apparently where this mvebu gpio driver was introduced),
> > so I would be glad if someone could give it a try.
> > I also have no idea whether the three Marvell variants all have separate
> > mask registers (which is what the last patch assumes).
> 
> Accordingly to MV78230/78x60 (Armada-XP) functional specification, we
> have separate mask registers for the GPIO level and edge interrupts.
> 
> I believe that the GPIO mask register is only shared on Orion and
> Kirkwood based SoCs.

Of course, I was meaning that all the three Marvell variants (Orion,
Kirkwood and Armada) are using separate mask registers for the GPIO
interrupts :)

Simon

> 
> Regards,
> 
> Simon
> 
> > 
> > Gerlando Falauto (9):
> >   genirq: cosmetic: remove cur_regs
> >   genirq: add mask_cache and pmask_cache into struct irq_chip_type
> >   gpio: mvebu: convert to usage of *pmask_cache within irq_chip_type
> >   MIPS: JZ4740: convert to usage of *pmask_cache within irq_chip_type
> >   ARM: SAMSUNG: convert to usage of *pmask_cache within irq_chip_type
> >   genirq: rename mask_cache to shared_mask_cache
> >   genirq: handle separate mask registers
> >   orion-gpio: enable IRQ_GC_SEPARATE_MASK_REGISTERS
> >   gpio: mvebu: enable IRQ_GC_SEPARATE_MASK_REGISTERS
> > 
> >  arch/arm/plat-orion/gpio.c            |    3 +-
> >  arch/arm/plat-samsung/irq-vic-timer.c |    6 ++--
> >  arch/mips/jz4740/irq.c                |    3 +-
> >  drivers/gpio/gpio-mvebu.c             |   23 ++++++++------
> >  include/linux/irq.h                   |    9 ++++--
> >  kernel/irq/generic-chip.c             |   55 +++++++++++++++++++++------------
> >  6 files changed, 64 insertions(+), 35 deletions(-)
> > 
> > -- 
> > 1.7.10.1


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130318/cc8bb7cc/attachment-0001.sig>

[PATCH v3 0/9] refactoring for mask_cache

[Ezequiel Garcia]

Hi Gerlando,

On Mon, Mar 18, 2013 at 03:00:46PM +0100, Gerlando Falauto wrote:
> here is a patchset to address the issue found with Orion, in incremental
> stages as Thomas suggested.
> a) we introduce the new fields and pointer (though only the shared one is used)
> b) we convert all drivers to use it 
> c) we rename the field so to force the use of the per-ct pointer
> d) we add per-ct mask cache, provided the new flag 
>    IRQ_GC_SEPARATE_MASK_REGISTERS is enabled
> e) we enable the flag for orion-gpio and mvebu drivers
> 
> So even though I'm also providing changes for mvebu, I only
> tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.

Great job! Since this is a really old bug you're fixing I believe that the
patchset applies for stable as well as mainline.

According to Documentation/stable_kernel_rules.txt all you need to do
is add a 'Cc: stable at vger.kernel.org' tag in your sign-off area.

Stable people will take care of picking the patch when it hits
mainline. You should receive a mail notification about patches
being included in stable kernels.

Thanks,
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH v3 0/9] refactoring for mask_cache

[Gerlando Falauto]

Hi Ezequiel,

On 03/19/2013 11:03 AM, Ezequiel Garcia wrote:
> Hi Gerlando,
>
thanks for the information!

[...]
> According to Documentation/stable_kernel_rules.txt all you need to do
> is add a 'Cc: stable at vger.kernel.org' tag in your sign-off area.

While we're at it, is there any other people I should have added in the 
sign-off area? I mean, the original reporter (Joey Oravec), the author 
of the first working fix (Simon Guinot), and so on... should they all be 
quoted on *each* patch?

> Stable people will take care of picking the patch when it hits
> mainline. You should receive a mail notification about patches
> being included in stable kernels.

Thank you so much. I guess I'd rather have to wait for Thomas' blessing 
first (maybe in the form of an Acked-By), don't I?

Thanks!
Gerlando

[PATCH v3 0/9] refactoring for mask_cache

[Ezequiel Garcia]

On Tue, Mar 19, 2013 at 11:09:36AM +0100, Gerlando Falauto wrote:
> 
> thanks for the information!
> 

You're welcome :-)

> > Stable people will take care of picking the patch when it hits
> > mainline. You should receive a mail notification about patches
> > being included in stable kernels.
> 
> Thank you so much. I guess I'd rather have to wait for Thomas' blessing 
> first (maybe in the form of an Acked-By), don't I?
> 

AFAIK no, you don't have to wait for Thomas.

The stable rules are like this:
You submit a patch you think it applies for stable (see Documentation/stable_kernel_rules)
adding a 'Cc: stable at vger.kernel.org' tag. If the suitable maintainer accepts the patch,
it will eventually get merged, and when it hits mainline, the stable tag will tell stable
people to add this patch into their trees (anyone please correct me if I'm wrong).

As Jason suggests, you should also state which commit is your patch fixing.

Thanks,
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH v3 0/9] refactoring for mask_cache

[Jason Cooper]

On Tue, Mar 19, 2013 at 07:03:53AM -0300, Ezequiel Garcia wrote:
> Hi Gerlando,
> 
> On Mon, Mar 18, 2013 at 03:00:46PM +0100, Gerlando Falauto wrote:
> > here is a patchset to address the issue found with Orion, in incremental
> > stages as Thomas suggested.
> > a) we introduce the new fields and pointer (though only the shared one is used)
> > b) we convert all drivers to use it 
> > c) we rename the field so to force the use of the per-ct pointer
> > d) we add per-ct mask cache, provided the new flag 
> >    IRQ_GC_SEPARATE_MASK_REGISTERS is enabled
> > e) we enable the flag for orion-gpio and mvebu drivers
> > 
> > So even though I'm also providing changes for mvebu, I only
> > tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.
> 
> Great job! Since this is a really old bug you're fixing I believe that the
> patchset applies for stable as well as mainline.
> 
> According to Documentation/stable_kernel_rules.txt all you need to do
> is add a 'Cc: stable at vger.kernel.org' tag in your sign-off area.
> 
> Stable people will take care of picking the patch when it hits
> mainline. You should receive a mail notification about patches
> being included in stable kernels.

Yes, and if you have an idea of when the regression was introduced,
perhaps even which commit, that would be *extremely* helpful.
Otherwise, the stable folks have to expend a lot of time tracking it
down.

thx,

Jason.

[PATCH v3 0/9] refactoring for mask_cache

[Gerlando Falauto]

On 03/19/2013 12:06 PM, Jason Cooper wrote:
> On Tue, Mar 19, 2013 at 07:03:53AM -0300, Ezequiel Garcia wrote:
>> Hi Gerlando,
>>
>> On Mon, Mar 18, 2013 at 03:00:46PM +0100, Gerlando Falauto wrote:
>>> here is a patchset to address the issue found with Orion, in incremental
>>> stages as Thomas suggested.
>>> a) we introduce the new fields and pointer (though only the shared one is used)
>>> b) we convert all drivers to use it
>>> c) we rename the field so to force the use of the per-ct pointer
>>> d) we add per-ct mask cache, provided the new flag
>>>     IRQ_GC_SEPARATE_MASK_REGISTERS is enabled
>>> e) we enable the flag for orion-gpio and mvebu drivers
>>>
>>> So even though I'm also providing changes for mvebu, I only
>>> tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.
>>
>> Great job! Since this is a really old bug you're fixing I believe that the
>> patchset applies for stable as well as mainline.
>>
>> According to Documentation/stable_kernel_rules.txt all you need to do
>> is add a 'Cc: stable at vger.kernel.org' tag in your sign-off area.
>>
>> Stable people will take care of picking the patch when it hits
>> mainline. You should receive a mail notification about patches
>> being included in stable kernels.
>
> Yes, and if you have an idea of when the regression was introduced,
> perhaps even which commit, that would be *extremely* helpful.

Uhm, you're right, that piece of information sort of got lost while 
reworking the whole thing (from a single patch to a 9-piece series!).
Here it is:

This fixes a regression introduced by e59347a
"arm: orion: Use generic irq chip".

Question is, where (out of the 9 patches) should that be mentioned?
On all of them?

> Otherwise, the stable folks have to expend a lot of time tracking it
> down.

Let's try and save them that... :-)

Thank you!
Gerlando

[PATCH v3 0/9] refactoring for mask_cache

[Jason Cooper]

On Tue, Mar 19, 2013 at 12:10:11PM +0100, Gerlando Falauto wrote:
> On 03/19/2013 12:06 PM, Jason Cooper wrote:
> >On Tue, Mar 19, 2013 at 07:03:53AM -0300, Ezequiel Garcia wrote:
> >>Hi Gerlando,
> >>
> >>On Mon, Mar 18, 2013 at 03:00:46PM +0100, Gerlando Falauto wrote:
> >>>here is a patchset to address the issue found with Orion, in incremental
> >>>stages as Thomas suggested.
> >>>a) we introduce the new fields and pointer (though only the shared one is used)
> >>>b) we convert all drivers to use it
> >>>c) we rename the field so to force the use of the per-ct pointer
> >>>d) we add per-ct mask cache, provided the new flag
> >>>    IRQ_GC_SEPARATE_MASK_REGISTERS is enabled
> >>>e) we enable the flag for orion-gpio and mvebu drivers
> >>>
> >>>So even though I'm also providing changes for mvebu, I only
> >>>tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.
> >>
> >>Great job! Since this is a really old bug you're fixing I believe that the
> >>patchset applies for stable as well as mainline.
> >>
> >>According to Documentation/stable_kernel_rules.txt all you need to do
> >>is add a 'Cc: stable at vger.kernel.org' tag in your sign-off area.
> >>
> >>Stable people will take care of picking the patch when it hits
> >>mainline. You should receive a mail notification about patches
> >>being included in stable kernels.
> >
> >Yes, and if you have an idea of when the regression was introduced,
> >perhaps even which commit, that would be *extremely* helpful.
> 
> Uhm, you're right, that piece of information sort of got lost while
> reworking the whole thing (from a single patch to a 9-piece
> series!).
> Here it is:
> 
> This fixes a regression introduced by e59347a
> "arm: orion: Use generic irq chip".

Great!

$ git tag --contains e59347a | grep '^v[23]\.[0-9]*\.' | \
	sed -e 's/\.[0-9]*$/.x/' | sort -uV
v3.0.x
v3.1.x
v3.2.x
v3.3.x
v3.4.x
v3.5.x
v3.6.x
v3.7.x
v3.8.x

So:

 Cc: <stable@vger.kernel.org> # 3.0.x

oughta do it for the first patch...

> Question is, where (out of the 9 patches) should that be mentioned?
> On all of them?

That's where it gets more complicated.  Short answer, yes.  The long
answer:  You need to tell the stable team about the dependencies in your
series.  After reading bullet 3 of "Procedures for submitting patches"
in stable_kernel_rules.txt, I would break up your series into two
chunks.

The first is that which can be applied to v3.0.x on up (everything
fixing the regression and plat-orion/gpio.c).  The second being the fix
to gpio-mvebu.c.  Anything affecting gpio-mvebu.c should only be for
v3.7.x and newer.  It would still list the dependency on v3.0.x series
of patches.

Each patch in your series should then have a longer and longer list of
 Cc: stable... lines as they list the previous patches in the series.

Those commit IDs are going to change once LinusW (I presume) applies
them to his tree, so he'll have to edit each commit message to point the
the correct commit.

LinusW, do you want me to handle this?  If you prefer to take it:

Whole series:

Acked-by: Jason Cooper <jason@lakedaemon.net>

Gerlando, Feel free to ask if you have any questions.

Also note, this is something the sub-system maintainers usually handle.
We appreciate all the help we can get, though.  ;-)  Just letting us
know which patch introduced the regression makes this a lot easier.

thx,

Jason.

[PATCH v3 0/9] refactoring for mask_cache

[Jason Cooper]

On Tue, Mar 19, 2013 at 07:44:31AM -0400, Jason Cooper wrote:
> Those commit IDs are going to change once LinusW (I presume) applies
> them to his tree, so he'll have to edit each commit message to point the
> the correct commit.
> 
> LinusW, do you want me to handle this?

Never mind, coffee underflow to /dev/brain.  One drawback to mutt, can't
view thread while typing messages...

> If you prefer to take it:
> 
> Acked-by: Jason Cooper <jason@lakedaemon.net>

for the plat-orion/gpio.c bits and if it helps, the gpio-mvebu.c bits.

thx,

Jason.

[PATCH v3 0/9] refactoring for mask_cache

[Gerlando Falauto]

Hi,

sorry about the delay (been busy with other stuff).
First of all thanks for your hints. I have some questions.
This is my first time, so please be gentle... :-)

On 03/19/2013 12:56 PM, Jason Cooper wrote:
> On Tue, Mar 19, 2013 at 07:44:31AM -0400, Jason Cooper wrote:
>> Those commit IDs are going to change once LinusW (I presume) applies
>> them to his tree, so he'll have to edit each commit message to point the
>> the correct commit.
>>
>> LinusW, do you want me to handle this?
>
> Never mind, coffee underflow to /dev/brain.  One drawback to mutt, can't
> view thread while typing messages...

1) I had no idea who LinusW is, I assume Linus Walleij who should be the 
maintainer of linux-gpio. And linux-gpio should have nothing to do with 
this discussion... so I should assume you never mentioned him, right?

2) About the SOB part... most of this is inspired by Simon's original 
(single) patch and Holger's reworking. Should they be given credit or 
not? If yes, how? If I don't get an answer I would just assume no and 
I'll take all the blame and credit all by myself. :-)

3) Jason, did you mean you would pull it to 
http://git.infradead.org/users/jcooper/linux.git? Shall I change the SOB 
lines and resubmit? Or should I submit it somewhere else instead?

4) About submitting to -stable, I guess it wouldn't make any sense until 
it gets included upstream 
(https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/ so to 
speak). Is that right?
Or at least to jcooper's tree, through which it should eventually make 
it mainline through a merge (therefore keeping its commit-id). That right?

I did read Documentation/stable_kernel_rules.txt and what I understand 
is that it would be enough to submit to stable at vger.kernel.org only the 
last one, while providing a list of

  Cc: <stable@vger.kernel.org> # 3.0.x: 123456 first patch
  Cc: <stable@vger.kernel.org> # 3.0.x: 123457 second patch
  Cc: <stable@vger.kernel.org> # 3.0.x: 123458 third patch

using the mainstream (or jcooper's) commit IDs.

I don't know whether I'd have to repeat that for each version (3.0 will 
include only a subset of this series, while 3.7 and following will need 
the whole nine yards, and what's in between... well, something in between).

Please let me know what to do. Again, it's my first time so please be 
gentle. :-)

Thanks again!
Gerlando

>> If you prefer to take it:
>>
>> Acked-by: Jason Cooper <jason@lakedaemon.net>
>
> for the plat-orion/gpio.c bits and if it helps, the gpio-mvebu.c bits.
>
> thx,
>
> Jason.
>

[PATCH v3 0/9] refactoring for mask_cache

[Thomas Gleixner]

On Wed, 20 Mar 2013, Gerlando Falauto wrote:
> On 03/19/2013 12:56 PM, Jason Cooper wrote:
> > On Tue, Mar 19, 2013 at 07:44:31AM -0400, Jason Cooper wrote:
> > > Those commit IDs are going to change once LinusW (I presume) applies
> > > them to his tree, so he'll have to edit each commit message to point the
> > > the correct commit.
> > > 
> > > LinusW, do you want me to handle this?
> > 
> > Never mind, coffee underflow to /dev/brain.  One drawback to mutt, can't
> > view thread while typing messages...
> 
> 1) I had no idea who LinusW is, I assume Linus Walleij who should be the
> maintainer of linux-gpio. And linux-gpio should have nothing to do with this
> discussion... so I should assume you never mentioned him, right?

Jason had a brain hickup as he told. So please ignore that part :)
 
> 2) About the SOB part... most of this is inspired by Simon's original (single)
> patch and Holger's reworking. Should they be given credit or not? If yes, how?
> If I don't get an answer I would just assume no and I'll take all the blame
> and credit all by myself. :-)

Yes, you can give them credit. There are only a few official
"....-by:" tags, but you can chose from the inofficial ones:

Suggested-by:
Originally-by:
Original-patch-by:
Requested-by:
Original-idea-by:
Based-on-patch-by:

That's an incomplete list of already in-use tags, but one of those is
probably sufficient.
 
> 3) Jason, did you mean you would pull it to
> http://git.infradead.org/users/jcooper/linux.git? Shall I change the SOB lines
> and resubmit? Or should I submit it somewhere else instead?

That needs to go through my tree, really. And please just post it to
LKML with the relevant people CC'ed.

> 4) About submitting to -stable, I guess it wouldn't make any sense until it
> gets included upstream

Right. Though I'm a bit concerned about the amount of change
involved. So it's simpler to do:

Cc: <stable@vger.kernel.org> # simple resolution will be sent in reply

Then let the people who have this problem reply on that patch series
with a simple revert of that generic_chip() conversion commit
concering their particular subarchitecture instead of forcing a
nightmare of complex (and maybe untested) changes onto stable.

Thanks,

	tglx

[PATCH v3 0/9] refactoring for mask_cache

[Gerlando Falauto]

Hi Thomas,

first of all thanks for your patience.

On 03/20/2013 10:42 PM, Thomas Gleixner wrote:
> On Wed, 20 Mar 2013, Gerlando Falauto wrote:
>> On 03/19/2013 12:56 PM, Jason Cooper wrote:
>>> On Tue, Mar 19, 2013 at 07:44:31AM -0400, Jason Cooper wrote:
>
>> 3) Jason, did you mean you would pull it to
>> http://git.infradead.org/users/jcooper/linux.git? Shall I change the SOB lines
>> and resubmit? Or should I submit it somewhere else instead?
>
> That needs to go through my tree, really. And please just post it to
> LKML with the relevant people CC'ed.

So drop linux-kernel-arm?

>> 4) About submitting to -stable, I guess it wouldn't make any sense until it
>> gets included upstream
>
> Right. Though I'm a bit concerned about the amount of change
> involved. So it's simpler to do:
>
> Cc: <stable@vger.kernel.org> # simple resolution will be sent in reply

You mean per-patch (in the SOB area)? Or just in the cover letter? 
[Since when does the cover letter have a SOB area?]
Or just adding it as a plain "Cc:" recipient?
In any case, before waiting for mainstream inclusion?

I'm sorry for such stupid questions, but I'm getting even more confused...

> Then let the people who have this problem reply on that patch series
> with a simple revert of that generic_chip() conversion commit
> concering their particular subarchitecture instead of forcing a
> nightmare of complex (and maybe untested) changes onto stable.

I think I understand the idea, though you've lost me completely as far 
as the process is concerned...

A pointer to a FMTR (assuming there is one that I could be RTFM-ed to) 
would be very much appreciated. Sorry again for my newbieness.

Thanks again!
Gerlando

[PATCH v3 0/9] refactoring for mask_cache

[Simon Guinot]

On Wed, Mar 20, 2013 at 06:40:29PM +0100, Gerlando Falauto wrote:
> Hi,
> 
> sorry about the delay (been busy with other stuff).
> First of all thanks for your hints. I have some questions.
> This is my first time, so please be gentle... :-)
> 
> On 03/19/2013 12:56 PM, Jason Cooper wrote:
> >On Tue, Mar 19, 2013 at 07:44:31AM -0400, Jason Cooper wrote:
> >>Those commit IDs are going to change once LinusW (I presume) applies
> >>them to his tree, so he'll have to edit each commit message to point the
> >>the correct commit.
> >>
> >>LinusW, do you want me to handle this?
> >
> >Never mind, coffee underflow to /dev/brain.  One drawback to mutt, can't
> >view thread while typing messages...
> 
> 1) I had no idea who LinusW is, I assume Linus Walleij who should be
> the maintainer of linux-gpio. And linux-gpio should have nothing to
> do with this discussion... so I should assume you never mentioned
> him, right?
> 
> 2) About the SOB part... most of this is inspired by Simon's
> original (single) patch and Holger's reworking. Should they be given
> credit or not? If yes, how? If I don't get an answer I would just
> assume no and I'll take all the blame and credit all by myself. :-)

Please drop all the SOBs with my name. Except for giving up a patch
almost two years, I am not involved in this patch series development. 
I am quite sure there is no credit in that :)

Thanks.

Simon

> 
> 3) Jason, did you mean you would pull it to
> http://git.infradead.org/users/jcooper/linux.git? Shall I change the
> SOB lines and resubmit? Or should I submit it somewhere else
> instead?
> 
> 4) About submitting to -stable, I guess it wouldn't make any sense
> until it gets included upstream
> (https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/ so
> to speak). Is that right?
> Or at least to jcooper's tree, through which it should eventually
> make it mainline through a merge (therefore keeping its commit-id).
> That right?
> 
> I did read Documentation/stable_kernel_rules.txt and what I
> understand is that it would be enough to submit to
> stable at vger.kernel.org only the last one, while providing a list of
> 
>  Cc: <stable@vger.kernel.org> # 3.0.x: 123456 first patch
>  Cc: <stable@vger.kernel.org> # 3.0.x: 123457 second patch
>  Cc: <stable@vger.kernel.org> # 3.0.x: 123458 third patch
> 
> using the mainstream (or jcooper's) commit IDs.
> 
> I don't know whether I'd have to repeat that for each version (3.0
> will include only a subset of this series, while 3.7 and following
> will need the whole nine yards, and what's in between... well,
> something in between).
> 
> Please let me know what to do. Again, it's my first time so please
> be gentle. :-)
> 
> Thanks again!
> Gerlando
> 
> >>If you prefer to take it:
> >>
> >>Acked-by: Jason Cooper <jason@lakedaemon.net>
> >
> >for the plat-orion/gpio.c bits and if it helps, the gpio-mvebu.c bits.
> >
> >thx,
> >
> >Jason.
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130321/736911a4/attachment.sig>

[PATCH v3 0/9] refactoring for mask_cache

[Ezequiel Garcia]

Hi Jason,

On Tue, Mar 19, 2013 at 07:06:20AM -0400, Jason Cooper wrote:
> On Tue, Mar 19, 2013 at 07:03:53AM -0300, Ezequiel Garcia wrote:
> > On Mon, Mar 18, 2013 at 03:00:46PM +0100, Gerlando Falauto wrote:
> > > here is a patchset to address the issue found with Orion, in incremental
> > > stages as Thomas suggested.
> > > a) we introduce the new fields and pointer (though only the shared one is used)
> > > b) we convert all drivers to use it 
> > > c) we rename the field so to force the use of the per-ct pointer
> > > d) we add per-ct mask cache, provided the new flag 
> > >    IRQ_GC_SEPARATE_MASK_REGISTERS is enabled
> > > e) we enable the flag for orion-gpio and mvebu drivers
> > > 
> > > So even though I'm also providing changes for mvebu, I only
> > > tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.
> > 
> > Great job! Since this is a really old bug you're fixing I believe that the
> > patchset applies for stable as well as mainline.
> > 
> > According to Documentation/stable_kernel_rules.txt all you need to do
> > is add a 'Cc: stable at vger.kernel.org' tag in your sign-off area.
> > 
> > Stable people will take care of picking the patch when it hits
> > mainline. You should receive a mail notification about patches
> > being included in stable kernels.
> 
> Yes, and if you have an idea of when the regression was introduced,
> perhaps even which commit, that would be *extremely* helpful.
> Otherwise, the stable folks have to expend a lot of time tracking it
> down.
> 

Gerlando explains the problematic commit in the changelog of patch 8/9
of this series:

  enable handling of separate mask registers for Orion SoC GPIOs,
  fixing indeed the regression introduced by e59347a
  "arm: orion: Use generic irq chip".

@Gerlando, perhaps you should add this comment to the cover-letter?
After all, this fix is the whole point of the patchset.

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH v3 0/9] refactoring for mask_cache

[Simon Guinot]

On Mon, Mar 18, 2013 at 03:00:46PM +0100, Gerlando Falauto wrote:
> Hi everyone,
> here is a patchset to address the issue found with Orion, in incremental
> stages as Thomas suggested.
> a) we introduce the new fields and pointer (though only the shared one is used)
> b) we convert all drivers to use it 
> c) we rename the field so to force the use of the per-ct pointer
> d) we add per-ct mask cache, provided the new flag 
>    IRQ_GC_SEPARATE_MASK_REGISTERS is enabled
> e) we enable the flag for orion-gpio and mvebu drivers
> 
> So even though I'm also providing changes for mvebu, I only
> tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.
> We currently do not have a working 3.6+ configuration for our Kirkwood
> boards (3.6 is apparently where this mvebu gpio driver was introduced),
> so I would be glad if someone could give it a try.
> I also have no idea whether the three Marvell variants all have separate
> mask registers (which is what the last patch assumes).
> 
> Gerlando Falauto (9):
>   genirq: cosmetic: remove cur_regs
>   genirq: add mask_cache and pmask_cache into struct irq_chip_type
>   gpio: mvebu: convert to usage of *pmask_cache within irq_chip_type
>   MIPS: JZ4740: convert to usage of *pmask_cache within irq_chip_type
>   ARM: SAMSUNG: convert to usage of *pmask_cache within irq_chip_type
>   genirq: rename mask_cache to shared_mask_cache
>   genirq: handle separate mask registers
>   orion-gpio: enable IRQ_GC_SEPARATE_MASK_REGISTERS
>   gpio: mvebu: enable IRQ_GC_SEPARATE_MASK_REGISTERS
> 
>  arch/arm/plat-orion/gpio.c            |    3 +-
>  arch/arm/plat-samsung/irq-vic-timer.c |    6 ++--
>  arch/mips/jz4740/irq.c                |    3 +-
>  drivers/gpio/gpio-mvebu.c             |   23 ++++++++------
>  include/linux/irq.h                   |    9 ++++--
>  kernel/irq/generic-chip.c             |   55 +++++++++++++++++++++------------
>  6 files changed, 64 insertions(+), 35 deletions(-)

Hi Gerlando,

On a Network Space v2 Max board (Kirkwood SoC), I have verified that
both the GPIO drivers mvebu-gpio and orion-gpio are impacted by the bug.

I have also checked that this patch series fixes the bug for each of
them.

Thanks

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130321/4a827868/attachment.sig>

[PATCH v3 0/9] refactoring for mask_cache

[Gerlando Falauto]

Hi Simon,
On 03/21/2013 11:51 AM, Simon Guinot wrote:
> On Mon, Mar 18, 2013 at 03:00:46PM +0100, Gerlando Falauto wrote:
>> Hi everyone,
>> here is a patchset to address the issue found with Orion, in incremental
>> stages as Thomas suggested.
>> a) we introduce the new fields and pointer (though only the shared one is used)
>> b) we convert all drivers to use it
>> c) we rename the field so to force the use of the per-ct pointer
>> d) we add per-ct mask cache, provided the new flag
>>     IRQ_GC_SEPARATE_MASK_REGISTERS is enabled
>> e) we enable the flag for orion-gpio and mvebu drivers
>>
>> So even though I'm also providing changes for mvebu, I only
>> tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.
>> We currently do not have a working 3.6+ configuration for our Kirkwood
>> boards (3.6 is apparently where this mvebu gpio driver was introduced),
>> so I would be glad if someone could give it a try.
>> I also have no idea whether the three Marvell variants all have separate
>> mask registers (which is what the last patch assumes).
>>
>> Gerlando Falauto (9):
>>    genirq: cosmetic: remove cur_regs
>>    genirq: add mask_cache and pmask_cache into struct irq_chip_type
>>    gpio: mvebu: convert to usage of *pmask_cache within irq_chip_type
>>    MIPS: JZ4740: convert to usage of *pmask_cache within irq_chip_type
>>    ARM: SAMSUNG: convert to usage of *pmask_cache within irq_chip_type
>>    genirq: rename mask_cache to shared_mask_cache
>>    genirq: handle separate mask registers
>>    orion-gpio: enable IRQ_GC_SEPARATE_MASK_REGISTERS
>>    gpio: mvebu: enable IRQ_GC_SEPARATE_MASK_REGISTERS
>>
>>   arch/arm/plat-orion/gpio.c            |    3 +-
>>   arch/arm/plat-samsung/irq-vic-timer.c |    6 ++--
>>   arch/mips/jz4740/irq.c                |    3 +-
>>   drivers/gpio/gpio-mvebu.c             |   23 ++++++++------
>>   include/linux/irq.h                   |    9 ++++--
>>   kernel/irq/generic-chip.c             |   55 +++++++++++++++++++++------------
>>   6 files changed, 64 insertions(+), 35 deletions(-)
>
> Hi Gerlando,
>
> On a Network Space v2 Max board (Kirkwood SoC), I have verified that
> both the GPIO drivers mvebu-gpio and orion-gpio are impacted by the bug.
>
> I have also checked that this patch series fixes the bug for each of
> them.
>
> Thanks

Great!
Will you then offer a Tested-By? (on the last two patches, I guess)?

Thanks!
Gerlando

[PATCH v3 0/9] refactoring for mask_cache

[Ezequiel Garcia]

Hi Gerlando,

On Mon, Mar 18, 2013 at 03:00:46PM +0100, Gerlando Falauto wrote:
> Hi everyone,
> here is a patchset to address the issue found with Orion, in incremental
> stages as Thomas suggested.
> a) we introduce the new fields and pointer (though only the shared one is used)
> b) we convert all drivers to use it 
> c) we rename the field so to force the use of the per-ct pointer
> d) we add per-ct mask cache, provided the new flag 
>    IRQ_GC_SEPARATE_MASK_REGISTERS is enabled
> e) we enable the flag for orion-gpio and mvebu drivers
> 
> So even though I'm also providing changes for mvebu, I only
> tested the patch on a 3.0.40 kernel with the plat-orion/gpio.c driver.
> We currently do not have a working 3.6+ configuration for our Kirkwood
> boards (3.6 is apparently where this mvebu gpio driver was introduced),
> so I would be glad if someone could give it a try.
> I also have no idea whether the three Marvell variants all have separate
> mask registers (which is what the last patch assumes).
> 
> Gerlando Falauto (9):
>   genirq: cosmetic: remove cur_regs
>   genirq: add mask_cache and pmask_cache into struct irq_chip_type
>   gpio: mvebu: convert to usage of *pmask_cache within irq_chip_type
>   MIPS: JZ4740: convert to usage of *pmask_cache within irq_chip_type
>   ARM: SAMSUNG: convert to usage of *pmask_cache within irq_chip_type
>   genirq: rename mask_cache to shared_mask_cache
>   genirq: handle separate mask registers
>   orion-gpio: enable IRQ_GC_SEPARATE_MASK_REGISTERS
>   gpio: mvebu: enable IRQ_GC_SEPARATE_MASK_REGISTERS
> 
>  arch/arm/plat-orion/gpio.c            |    3 +-
>  arch/arm/plat-samsung/irq-vic-timer.c |    6 ++--
>  arch/mips/jz4740/irq.c                |    3 +-
>  drivers/gpio/gpio-mvebu.c             |   23 ++++++++------
>  include/linux/irq.h                   |    9 ++++--
>  kernel/irq/generic-chip.c             |   55 +++++++++++++++++++++------------
>  6 files changed, 64 insertions(+), 35 deletions(-)
> 

What's the plan for this patchset?

Given this fixes a long time issue, perhaps we might want to try to have
it included in v3.10, which is soon to come.

Thanks,
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com