[RFC patch 00/20] Interrupt chip consolidation

[RFC patch 00/20] Interrupt chip consolidation

[Thomas Gleixner]

A search for patterns of interrupt chips and their implementations
came up with at least two very common variants:

 - mask register based, where a bit flips the masked state of the
   corresponding interrupt line. Most common is to unmask with the bit
   set, but the inverted version exists as well. Interestingly enough
   almost none of the chips implementations cache the register value
   and read it back from the chip before every write.
   
 - clear/set register based, where a bit written to one of the
   registers either clears or sets the interrupt mask.

There are more patterns, but for now I restricted the patches on those
two.

The core code provides now a generic irq chip mechanism which
dynamically allocates the data structures. The chip assumes that the
interrupt hardware is using 32bit wide registers. For each interrupt
bank (32 irqs) a separate chip is allocated and configured. If
possible the generic callbacks for mask/unmask/ack/eoi are used. There
is also a generic version for irq_set_wake, which just caches the
wakeup sources in the chip data structure to replace home brewn
caching mechanisms.

The core code has new callbacks irq_suspend/irq_resume as well, which
can be used to replace the sys device implementations for the various
irq chips.

I converted and cleaned up a bunch of irq chip implementations in
arch/arm to see whether this works out as intended.

Note, that this is mostly compile tested only, but I'd like to get
some feedback on that before going further. There is more room for
consolidation (especially a bunch of slightly different VIC copies
poked my eyes).

The core code adds about 500 lines, but it's massivly commented. The
binary size with all functionality compiled in is about 1500 bytes
which could be reduced with some ifdeffery, but it's probably not
worth the mess.

The diffstat for the arm part is:

 arch/arm/mach-msm/irq.c                                      |  151 ----
 linux-2.6/arch/arm/common/sa1111.c                           |  193 ------
 linux-2.6/arch/arm/common/vic.c                              |  262 +-------
 linux-2.6/arch/arm/mach-bcmring/irq.c                        |   98 ---
 linux-2.6/arch/arm/mach-davinci/irq.c                        |   90 --
 linux-2.6/arch/arm/mach-msm/Kconfig                          |    1 
 linux-2.6/arch/arm/mach-msm/Makefile                         |    2 
 linux-2.6/arch/arm/mach-msm/include/mach/irqs.h              |    3 
 linux-2.6/arch/arm/mach-msm/include/mach/sirc.h              |   43 -
 linux-2.6/arch/arm/mach-msm/irq-vic.c                        |  344 +----------
 linux-2.6/arch/arm/mach-msm/sirc.c                           |  172 +----
 linux-2.6/arch/arm/mach-pxa/irq.c                            |  192 ++----
 linux-2.6/arch/arm/mach-s3c64xx/irq.c                        |    7 
 linux-2.6/arch/arm/mach-tcc8k/irq.c                          |   83 --
 linux-2.6/arch/arm/plat-orion/gpio.c                         |  110 +--
 linux-2.6/arch/arm/plat-orion/include/plat/gpio.h            |    1 
 linux-2.6/arch/arm/plat-orion/irq.c                          |   49 -
 linux-2.6/arch/arm/plat-s5p/irq-gpioint.c                    |  115 ---
 linux-2.6/arch/arm/plat-s5p/irq.c                            |    6 
 linux-2.6/arch/arm/plat-samsung/include/plat/irq-vic-timer.h |    2 
 linux-2.6/arch/arm/plat-samsung/irq-uart.c                   |   82 --
 linux-2.6/arch/arm/plat-samsung/irq-vic-timer.c              |   73 --
 22 files changed, 455 insertions(+), 1624 deletions(-)

That converts 18 irq_chips and removes at least one duplicated VIC
implementation.

Please have a look and check also the not yet converted chips whether
they could make use of such an generic implementation. We better have
some oddball callback functions in the generic code than ten slightly
different instances of them all over the place.

Thanks,

	tglx

genirq: Implement a generic interrupt chip

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: genirq-genirq-chip.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/c8d08638/attachment.ksh>

genirq: Add chip suspend and resume callbacks

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: genirq-chip-suspend-resume-callbacks.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/82ebc6c3/attachment.ksh>

arm: vic: Use generic interrupt chip

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-vic-use-gc.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/019c3618/attachment.ksh>

arm: vic: Implement irq_suspend/resume callbacks

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-vic-gc-pm.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/2eacd6e2/attachment.ksh>

arm: orion: Use generic irq chip

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-orion-use-generic-chip.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/a11556bd/attachment.ksh>

arm: bcmring: Use generic irq chip implementation

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-bcmring-gc.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/75eba04e/attachment.ksh>

arm: davinci: Use generic irq chip

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-davinci-gc.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/29cee867/attachment.ksh>

arm: samsung: Convert irq-vic-timer to generic irq chip

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-samsung-irq-vic-timer.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/a4357dfb/attachment.ksh>

arm: samsung: Convert irq_uart to generic irq chip

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-samsung-uart.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/c9fee5f5/attachment.ksh>

arm: samsung: s5p: Convert irq-gpioint to generic irq chip

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-samsung-irq-gpioint.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/70cadc51/attachment.ksh>

arm: tcc8k: Convert to generic irq chip

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-tcc-gc.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/510a4869/attachment.ksh>

arm: sa1111: Convert to generic irq chip

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-sa1111-gc.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/af1d9276/attachment.ksh>

arm: msm: Convert sirc to generic irq chip

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-msm-sric.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/df217bcb/attachment.ksh>

arm: msm: Convert irq.c chip to generic irq chip

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-msm-irq.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/646062c6/attachment.ksh>

arm: msm: Convert irq-vic to generic irq chip

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-msm-irq-vic.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/1f7bbb55/attachment.ksh>

arm: msm: Cleanup the irq.c code some more

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-msm-unify.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/17784751/attachment.ksh>

arm: msm: Use irq-vic for all vic instances

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-msm-use-one-vic.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/cd1e5b96/attachment.ksh>

arm: msm: Consolidate more

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-msm-consolidate-more.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/a742690a/attachment.ksh>

arm: pxa: Convert SC and GPIO-l to generic irq chips

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-pxa-gc.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110416/2ec834e0/attachment.ksh>

samsung: s5p: Convert irq-gpioint to generic irq chip

[Kukjin Kim]

Thomas Gleixner wrote:
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Acked-by: Kukjin Kim <kgene.kim@samsung.com>

> ---
>  arch/arm/plat-s5p/irq-gpioint.c |  115
+++++++++----------------------------
> ---
>  1 file changed, 26 insertions(+), 89 deletions(-)
> 

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

samsung: Convert irq-vic-timer to generic irq chip

[Kukjin Kim]

Thomas Gleixner wrote:
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Acked-by: Kukjin Kim <kgene.kim@samsung.com>

> ---
>  arch/arm/mach-s3c64xx/irq.c                        |    7 --
>  arch/arm/plat-s5p/irq.c                            |    6 -
>  arch/arm/plat-samsung/include/plat/irq-vic-timer.h |    2
>  arch/arm/plat-samsung/irq-vic-timer.c              |   73
++++++++----------
> ---
>  4 files changed, 31 insertions(+), 57 deletions(-)
> 

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

samsung: Convert irq_uart to generic irq chip

[Kukjin Kim]

Thomas Gleixner wrote:
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Acked-by: Kukjin Kim <kgene.kim@samsung.com>

> ---
>  arch/arm/plat-samsung/irq-uart.c |   82
+++++-------------------------------
> ---
>  1 file changed, 11 insertions(+), 71 deletions(-)
> 

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

genirq: Implement a generic interrupt chip

[H Hartley Sweeten]

On Saturday, April 16, 2011 2:14 PM, Thomas Gleixner wrote:
>
> Implement a generic interrupt chip, which is configurable and is able
> to handle the most common irq chip implementations.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  include/linux/irq.h       |  123 ++++++++++++++++++++++
>  kernel/irq/Makefile       |    1 
>  kernel/irq/generic-chip.c |  251 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 375 insertions(+)
> 
> Index: linux-2.6/include/linux/irq.h
> ===================================================================
> --- linux-2.6.orig/include/linux/irq.h
> +++ linux-2.6/include/linux/irq.h

[snip]

> +/**
> + * struct irq_chip_type - Generic interrupt chip instance for a flow type
> + * @chip:		The real interrupt chip which provides the callbacks
> + * @regs:		Register offsets for this chip
> + * @handler:		Flow handler associated with this chip
> + * @type:		Chip can handle these flow types
> + *
> + * A irq_generic_chip can have several instances of irq_chip_type when
> + * it requires different functions and register offsets for different
> + * flow types.
> + */
> +struct irq_chip_type {
> +	struct irq_chip		chip;
> +	struct irq_chip_regs	regs;
> +	irq_flow_handler_t	handler;
> +	u32			type;
> +};
> +

[snip]

> Index: linux-2.6/kernel/irq/generic-chip.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6/kernel/irq/generic-chip.c

[snip]

> +/**
> + * irq_setup_generic_chip - Setup a range of interrupts with a generic chip
> + * @gc:		Generic irq chip holding all data
> + * @msk:	Bitmask holding the irqs to initialize relative to gc->irq_base
> + * @clr:	IRQ_* bits to clear
> + * @set:	IRQ_* bits to set
> + *
> + * Set up max. 32 interrupts starting from gc->irq_base. Note, this
> + * initializes all interrupts to the primary irq_chip_type and its
> + * associated handler.
> + */
> +void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
> +			    unsigned int clr, unsigned int set)
> +{
> +	struct irq_chip_type *ct = gc->chip_types;
> +	unsigned int i;
> +
> +	/* Init mask cache */
> +	if (ct->irq_mask == irq_gc_mask_clr_bit ||
> +	    ct->irq_mask == irq_gc_mask_set_bit)

struct irq_chip_type does not have a member named 'irq_mask'

Regards,
Hartley

vic: Use generic interrupt chip

[H Hartley Sweeten]

On Saturday, April 16, 2011 2:14 PM, Thomas Gleixner wrote:
>
> The VIC chips can be handled by the generic implementation. The
> implementation uses always uses handle_level_irq. We can replace the
> ack and mask functions by a combined mask_ack function which also gets
> rid of the double write to VIC_INT_ENABLE_CLEAR.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/arm/common/vic.c |  152 ++++++++++++--------------------------------------
>  1 file changed, 39 insertions(+), 113 deletions(-)
> 
> Index: linux-2.6/arch/arm/common/vic.c
> ===================================================================
> --- linux-2.6.orig/arch/arm/common/vic.c
> +++ linux-2.6/arch/arm/common/vic.c

[snip]

> @@ -178,15 +172,13 @@ late_initcall(vic_pm_init);
>  
>  /**
>   * vic_pm_register - Register a VIC for later power management control
> - * @base: The base address of the VIC.
> - * @irq: The base IRQ for the VIC.
> - * @resume_sources: bitmask of interrupts allowed for resume sources.
> + * @gc: The generic interrupt chip for the VIC
>   *
>   * Register the VIC with the system device tree so that it can be notified
>   * of suspend and resume requests and ensure that the correct actions are
>   * taken to re-instate the settings on resume.
>   */
> -static void __init vic_pm_register(void __iomem *base, unsigned int irq, u32 resume_sources)
> +static void __init vic_pm_register(struct irq_chip_generic *gc)
>  {
>  	struct vic_device *v;
>  
> @@ -194,85 +186,13 @@ static void __init vic_pm_register(void 
>  		printk(KERN_ERR "%s: too few VICs, increase CONFIG_ARM_VIC_NR\n", __func__);
>  	else {
>  		v = &vic_devices[vic_id];
> -		v->base = base;
> -		v->resume_sources = resume_sources;
> -		v->irq = irq;
> -		vic_id++;
> +		v->gc = gc;
>  	}
>  }

I think the vic_id++ is still needed here in order to handle multiple VIC devices.

Regards,
Hartley

genirq: Implement a generic interrupt chip

[Thomas Gleixner]

On Mon, 18 Apr 2011, H Hartley Sweeten wrote:
> On Saturday, April 16, 2011 2:14 PM, Thomas Gleixner wrote:
> > +void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
> > +			    unsigned int clr, unsigned int set)
> > +{
> > +	struct irq_chip_type *ct = gc->chip_types;
> > +	unsigned int i;
> > +
> > +	/* Init mask cache */
> > +	if (ct->irq_mask == irq_gc_mask_clr_bit ||
> > +	    ct->irq_mask == irq_gc_mask_set_bit)
> 
> struct irq_chip_type does not have a member named 'irq_mask'

Found that already. Needs to be ct->chip_types->irq_mask of course.

Someday I hope that the patch to quilt which forces a refresh of the
queue _BEFORE_ generating mail will surface :)

Thanks,

	tglx

genirq: Implement a generic interrupt chip

[Thomas Gleixner]

On Mon, 18 Apr 2011, Thomas Gleixner wrote:

> On Mon, 18 Apr 2011, H Hartley Sweeten wrote:
> > On Saturday, April 16, 2011 2:14 PM, Thomas Gleixner wrote:
> > > +void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
> > > +			    unsigned int clr, unsigned int set)
> > > +{
> > > +	struct irq_chip_type *ct = gc->chip_types;
> > > +	unsigned int i;
> > > +
> > > +	/* Init mask cache */
> > > +	if (ct->irq_mask == irq_gc_mask_clr_bit ||
> > > +	    ct->irq_mask == irq_gc_mask_set_bit)
> > 
> > struct irq_chip_type does not have a member named 'irq_mask'
> 
> Found that already. Needs to be ct->chip_types->irq_mask of course.

Gah, my brain is seriously damaged from staring into that
code. ct->chip.irq_mask of course.
 
> Someday I hope that the patch to quilt which forces a refresh of the
> queue _BEFORE_ generating mail will surface :)
> 
> Thanks,
> 
> 	tglx
> 

genirq: Implement a generic interrupt chip

[H Hartley Sweeten]

On Monday, April 18, 2011 12:44 PM, Thomas Gleixner wrote:
> On Mon, 18 Apr 2011, Thomas Gleixner wrote:
>
>> On Mon, 18 Apr 2011, H Hartley Sweeten wrote:
>>> On Saturday, April 16, 2011 2:14 PM, Thomas Gleixner wrote:
>>>> +void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
>>>> +			    unsigned int clr, unsigned int set)
>>>> +{
>>>> +	struct irq_chip_type *ct = gc->chip_types;
>>>> +	unsigned int i;
>>>> +
>>>> +	/* Init mask cache */
>>>> +	if (ct->irq_mask == irq_gc_mask_clr_bit ||
>>>> +	    ct->irq_mask == irq_gc_mask_set_bit)
>>> 
>>> struct irq_chip_type does not have a member named 'irq_mask'
>>
>> Found that already. Needs to be ct->chip_types->irq_mask of course.
>
> Gah, my brain is seriously damaged from staring into that
> code. ct->chip.irq_mask of course.

OK. With that fix, I compiled and boot tested these patches on an 
ep93xx based board:

genirq: Implement a generic interrupt chip
vic: Use generic interrupt chip

I do not have CONFIG_PM enabled so I'm not sure about the vic_id++
issue I pointed out earlier in vic: Use generic interrupt chip.

Other than that, for these two patches you can add (if you wish):

Tested-by: H Hartley Sweeten <hsweeten@visionengravers.com>

genirq: Implement a generic interrupt chip

[Tony Lindgren]

* Thomas Gleixner <tglx@linutronix.de> [110416 14:20]:
> --- linux-2.6.orig/include/linux/irq.h
> +++ linux-2.6/include/linux/irq.h
...

> +/**
> + * struct irq_chip_regs - register offsets for struct irq_gci
> + * @enable:	Enable register offset to reg_base
> + * @disable:	Disable register offset to reg_base
> + * @mask:	Mask register offset to reg_base
> + * @ack:	Ack register offset to reg_base
> + * @ack:	Secondary Ack register offset to reg_base

Two @ack lines here?

Tony

[RFC patch 00/20] Interrupt chip consolidation

[Lars-Peter Clausen]

On 04/16/2011 11:14 PM, Thomas Gleixner wrote:
> [...]
>
> That converts 18 irq_chips and removes at least one duplicated VIC
> implementation.
>
> Please have a look and check also the not yet converted chips whether
> they could make use of such an generic implementation. We better have
> some oddball callback functions in the generic code than ten slightly
> different instances of them all over the place.
>

I've adopted JZ4740 platform which is MIPS based SoC to the generic_irq_chip
and the diffstat looks ok:

 arch/mips/jz4740/gpio.c  |  116 +++++++++++++++-------------------------------
 arch/mips/jz4740/irq.c   |   95 +++++++++++++++----------------------
 arch/mips/jz4740/pm.c    |    2 -
 drivers/mfd/jz4740-adc.c |   73 +++++++----------------------
 4 files changed, 94 insertions(+), 192 deletions(-)

I've also noticed that all my chained demuxer handlers follow a similar scheme
and a quick grep for generic_handle_irq revealed that quite a few other
implementations also follow a similar scheme as well.

Something along the lines of ...

void irq_gc_chained_demux_handler(unsigned int irq, struct irq_desc *desc)
{
	struct irq_chip_generic *gc = irq_desc_get_handler_data(desc);
	struct irq_chip_type *ct = gc = irq_chip_generic_current_chip_type(gc);
	unsigned long pending = irq_reg_readl(ct->regs.pending);

	pending = irq_reg_readl(ct->regs.pending) & gc->mask_cache;

	for_each_set_bit (irq, &pending, gc->irq_cnt)
		generic_handle_irq(gc->irq_base + irq);
}

... could be used to replace those custom handlers in an irq_chip_generic based
implementation.

With this the diffstat for JZ4740 looks even better:
 4 files changed, 91 insertions(+), 232 deletions(-)

Unfortunately it doesn't completely fit into the current irq_chip_generic
implementation. For one there is no irq_chip_generic_current_chip_type, but I
guess that could be implementing by adding a field to irq_chip_generic pointing
to the current chip type. On the other hand I'm not sure if it is necessary to
have different pending registers for different chip types of the same
irq_chip_generic.

Another issue is that while the gpio unit on the JZ4740 supports different
trigger modes it only supports either rising or falling edge at one time. So
since it doesn't makes sense to implement switching between those two in a
driver triggering in both edges is emulated by the irq chip.
Whenever an interrupt occurs or it is unmask the current state is read the
hardware is configured to trigger when the state would change. So if the pin is
low the hardware is set to trigger on a rising edge.

And of course this is not the only gpio unit which requires this. On ARM Orion,
MSN, MXC and possible others have the same issue.
I wonder if you see an option to handle this in a generic way? Either in the
irq core or in the generic_irq_chip implementation, or is this something better
put into into a specialized gpio_irq_chip implementation?
This would at least require a callback by which the generic code could get the
current irq level, a specialized unmask and demuxer.

Also related to gpio irq chips is the lockdep class issue.
Would it make sense to add a lockdep class parameter to irq_setup_generic_chip
or is this something better put into a separate function along the lines of.

void irq_generic_chip_set_lockclass(struct irq_generic_chip *gc,
	struct lock_class_key *lockdep_class)
{
	unsigned int irq;
	for (irq = 0; irq < gc->irq_cnt; irq++)
		irq_set_lockdep_class(gc->irq_bas + irq, lockdep_class);
}


- Lars

[RFC patch 00/20] Interrupt chip consolidation

[Thomas Gleixner]

On Tue, 19 Apr 2011, Lars-Peter Clausen wrote:
> On 04/16/2011 11:14 PM, Thomas Gleixner wrote:
> > [...]
> >
> > That converts 18 irq_chips and removes at least one duplicated VIC
> > implementation.
> >
> > Please have a look and check also the not yet converted chips whether
> > they could make use of such an generic implementation. We better have
> > some oddball callback functions in the generic code than ten slightly
> > different instances of them all over the place.
> >
> 
> I've adopted JZ4740 platform which is MIPS based SoC to the generic_irq_chip
> and the diffstat looks ok:
> 
>  arch/mips/jz4740/gpio.c  |  116 +++++++++++++++-------------------------------
>  arch/mips/jz4740/irq.c   |   95 +++++++++++++++----------------------
>  arch/mips/jz4740/pm.c    |    2 -
>  drivers/mfd/jz4740-adc.c |   73 +++++++----------------------
>  4 files changed, 94 insertions(+), 192 deletions(-)

Nice!
 
> I've also noticed that all my chained demuxer handlers follow a similar scheme
> and a quick grep for generic_handle_irq revealed that quite a few other
> implementations also follow a similar scheme as well.
> 
> Something along the lines of ...
> 
> void irq_gc_chained_demux_handler(unsigned int irq, struct irq_desc *desc)
> {
> 	struct irq_chip_generic *gc = irq_desc_get_handler_data(desc);
> 	struct irq_chip_type *ct = gc = irq_chip_generic_current_chip_type(gc);
> 	unsigned long pending = irq_reg_readl(ct->regs.pending);
> 
> 	pending = irq_reg_readl(ct->regs.pending) & gc->mask_cache;
> 
> 	for_each_set_bit (irq, &pending, gc->irq_cnt)
> 		generic_handle_irq(gc->irq_base + irq);
> }
> 
> ... could be used to replace those custom handlers in an irq_chip_generic based
> implementation.
> 
> With this the diffstat for JZ4740 looks even better:
>  4 files changed, 91 insertions(+), 232 deletions(-)
> 
> Unfortunately it doesn't completely fit into the current irq_chip_generic
> implementation. For one there is no irq_chip_generic_current_chip_type, but I
> guess that could be implementing by adding a field to irq_chip_generic pointing
> to the current chip type. On the other hand I'm not sure if it is necessary to

No, you can get the current chip from irq_data.chip which always has a
pointer to the current active one. container_of will give you the
generic chip.

> have different pending registers for different chip types of the same
> irq_chip_generic.
> 
> Another issue is that while the gpio unit on the JZ4740 supports different
> trigger modes it only supports either rising or falling edge at one time. So
> since it doesn't makes sense to implement switching between those two in a
> driver triggering in both edges is emulated by the irq chip.

That's what about 5 dozen other irq chips do as well.

> Also related to gpio irq chips is the lockdep class issue.
> Would it make sense to add a lockdep class parameter to irq_setup_generic_chip
> or is this something better put into a separate function along the lines of.
> 
> void irq_generic_chip_set_lockclass(struct irq_generic_chip *gc,
> 	struct lock_class_key *lockdep_class)
> {
> 	unsigned int irq;
> 	for (irq = 0; irq < gc->irq_cnt; irq++)
> 		irq_set_lockdep_class(gc->irq_bas + irq, lockdep_class);
> }

Probably yes.

Thanks,

	tglx
 

[PATCH] arm: omap2/3: Use generic irq chip

[Tony Lindgren]

Use generic irq chip for omap2 & 3.

Note that this patch also leaves out the spurious IRQ warning
for omap3.

This warning should no longer be needed as the interrupt handlers
for various devices have implemented the necessayr read-back of
the posted write.

Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -73,83 +73,18 @@ static u32 intc_bank_read_reg(struct omap_irq_bank *bank, u16 reg)
 	return __raw_readl(bank->base_reg + reg);
 }
 
-static int previous_irq;
-
-/*
- * On 34xx we can get occasional spurious interrupts if the ack from
- * an interrupt handler does not get posted before we unmask. Warn about
- * the interrupt handlers that need to flush posted writes.
- */
-static int omap_check_spurious(unsigned int irq)
-{
-	u32 sir, spurious;
-
-	sir = intc_bank_read_reg(&irq_banks[0], INTC_SIR);
-	spurious = sir >> 7;
-
-	if (spurious) {
-		printk(KERN_WARNING "Spurious irq %i: 0x%08x, please flush "
-					"posted write for irq %i\n",
-					irq, sir, previous_irq);
-		return spurious;
-	}
-
-	return 0;
-}
-
 /* XXX: FIQ and additional INTC support (only MPU at the moment) */
 static void omap_ack_irq(struct irq_data *d)
 {
 	intc_bank_write_reg(0x1, &irq_banks[0], INTC_CONTROL);
 }
 
-static void omap_mask_irq(struct irq_data *d)
-{
-	unsigned int irq = d->irq;
-	int offset = irq & (~(IRQ_BITS_PER_REG - 1));
-
-	if (cpu_is_omap34xx() && !cpu_is_ti816x()) {
-		int spurious = 0;
-
-		/*
-		 * INT_34XX_GPT12_IRQ is also the spurious irq. Maybe because
-		 * it is the highest irq number?
-		 */
-		if (irq == INT_34XX_GPT12_IRQ)
-			spurious = omap_check_spurious(irq);
-
-		if (!spurious)
-			previous_irq = irq;
-	}
-
-	irq &= (IRQ_BITS_PER_REG - 1);
-
-	intc_bank_write_reg(1 << irq, &irq_banks[0], INTC_MIR_SET0 + offset);
-}
-
-static void omap_unmask_irq(struct irq_data *d)
-{
-	unsigned int irq = d->irq;
-	int offset = irq & (~(IRQ_BITS_PER_REG - 1));
-
-	irq &= (IRQ_BITS_PER_REG - 1);
-
-	intc_bank_write_reg(1 << irq, &irq_banks[0], INTC_MIR_CLEAR0 + offset);
-}
-
 static void omap_mask_ack_irq(struct irq_data *d)
 {
-	omap_mask_irq(d);
+	irq_gc_mask_disable_reg(d);
 	omap_ack_irq(d);
 }
 
-static struct irq_chip omap_irq_chip = {
-	.name		= "INTC",
-	.irq_ack	= omap_mask_ack_irq,
-	.irq_mask	= omap_mask_irq,
-	.irq_unmask	= omap_unmask_irq,
-};
-
 static void __init omap_irq_bank_init_one(struct omap_irq_bank *bank)
 {
 	unsigned long tmp;
@@ -186,11 +121,31 @@ int omap_irq_pending(void)
 	return 0;
 }
 
+static __init void
+omap_alloc_gc(void __iomem *base, unsigned int irq_start, unsigned int num)
+{
+	struct irq_chip_generic *gc;
+	struct irq_chip_type *ct;
+
+	gc = irq_alloc_generic_chip("INTC", 1, irq_start, base,
+					handle_level_irq);
+	ct = gc->chip_types;
+	ct->chip.irq_ack = omap_mask_ack_irq;
+	ct->chip.irq_mask = irq_gc_mask_disable_reg;
+	ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
+
+	ct->regs.ack = INTC_CONTROL;
+	ct->regs.enable = INTC_MIR_CLEAR0;
+	ct->regs.disable = INTC_MIR_SET0;
+	irq_setup_generic_chip(gc, IRQ_MSK(num),
+				IRQ_NOREQUEST | IRQ_NOPROBE, 0);
+}
+
 void __init omap_init_irq(void)
 {
 	unsigned long nr_of_irqs = 0;
 	unsigned int nr_banks = 0;
-	int i;
+	int i, j;
 
 	for (i = 0; i < ARRAY_SIZE(irq_banks); i++) {
 		unsigned long base = 0;
@@ -215,17 +170,15 @@ void __init omap_init_irq(void)
 
 		omap_irq_bank_init_one(bank);
 
+		for (i = 0, j= 0; i < bank->nr_irqs; i += 32, j += 0x20)
+			omap_alloc_gc(bank->base_reg + j, i, 32);
+
 		nr_of_irqs += bank->nr_irqs;
 		nr_banks++;
 	}
 
 	printk(KERN_INFO "Total of %ld interrupts on %d active controller%s\n",
 	       nr_of_irqs, nr_banks, nr_banks > 1 ? "s" : "");
-
-	for (i = 0; i < nr_of_irqs; i++) {
-		irq_set_chip_and_handler(i, &omap_irq_chip, handle_level_irq);
-		set_irq_flags(i, IRQF_VALID);
-	}
 }
 
 #ifdef CONFIG_ARCH_OMAP3

[PATCH] arm: omap2/3: Use generic irq chip

[Tony Lindgren]

Use generic irq chip for omap2 & 3.

Note that this patch also leaves out the spurious IRQ warning
for omap3.

This warning should no longer be needed as the interrupt handlers
for various devices have implemented the necessayr read-back of
the posted write.

Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -73,83 +73,18 @@ static u32 intc_bank_read_reg(struct omap_irq_bank *bank, u16 reg)
 	return __raw_readl(bank->base_reg + reg);
 }
 
-static int previous_irq;
-
-/*
- * On 34xx we can get occasional spurious interrupts if the ack from
- * an interrupt handler does not get posted before we unmask. Warn about
- * the interrupt handlers that need to flush posted writes.
- */
-static int omap_check_spurious(unsigned int irq)
-{
-	u32 sir, spurious;
-
-	sir = intc_bank_read_reg(&irq_banks[0], INTC_SIR);
-	spurious = sir >> 7;
-
-	if (spurious) {
-		printk(KERN_WARNING "Spurious irq %i: 0x%08x, please flush "
-					"posted write for irq %i\n",
-					irq, sir, previous_irq);
-		return spurious;
-	}
-
-	return 0;
-}
-
 /* XXX: FIQ and additional INTC support (only MPU at the moment) */
 static void omap_ack_irq(struct irq_data *d)
 {
 	intc_bank_write_reg(0x1, &irq_banks[0], INTC_CONTROL);
 }
 
-static void omap_mask_irq(struct irq_data *d)
-{
-	unsigned int irq = d->irq;
-	int offset = irq & (~(IRQ_BITS_PER_REG - 1));
-
-	if (cpu_is_omap34xx() && !cpu_is_ti816x()) {
-		int spurious = 0;
-
-		/*
-		 * INT_34XX_GPT12_IRQ is also the spurious irq. Maybe because
-		 * it is the highest irq number?
-		 */
-		if (irq == INT_34XX_GPT12_IRQ)
-			spurious = omap_check_spurious(irq);
-
-		if (!spurious)
-			previous_irq = irq;
-	}
-
-	irq &= (IRQ_BITS_PER_REG - 1);
-
-	intc_bank_write_reg(1 << irq, &irq_banks[0], INTC_MIR_SET0 + offset);
-}
-
-static void omap_unmask_irq(struct irq_data *d)
-{
-	unsigned int irq = d->irq;
-	int offset = irq & (~(IRQ_BITS_PER_REG - 1));
-
-	irq &= (IRQ_BITS_PER_REG - 1);
-
-	intc_bank_write_reg(1 << irq, &irq_banks[0], INTC_MIR_CLEAR0 + offset);
-}
-
 static void omap_mask_ack_irq(struct irq_data *d)
 {
-	omap_mask_irq(d);
+	irq_gc_mask_disable_reg(d);
 	omap_ack_irq(d);
 }
 
-static struct irq_chip omap_irq_chip = {
-	.name		= "INTC",
-	.irq_ack	= omap_mask_ack_irq,
-	.irq_mask	= omap_mask_irq,
-	.irq_unmask	= omap_unmask_irq,
-};
-
 static void __init omap_irq_bank_init_one(struct omap_irq_bank *bank)
 {
 	unsigned long tmp;
@@ -186,11 +121,31 @@ int omap_irq_pending(void)
 	return 0;
 }
 
+static __init void
+omap_alloc_gc(void __iomem *base, unsigned int irq_start, unsigned int num)
+{
+	struct irq_chip_generic *gc;
+	struct irq_chip_type *ct;
+
+	gc = irq_alloc_generic_chip("INTC", 1, irq_start, base,
+					handle_level_irq);
+	ct = gc->chip_types;
+	ct->chip.irq_ack = omap_mask_ack_irq;
+	ct->chip.irq_mask = irq_gc_mask_disable_reg;
+	ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
+
+	ct->regs.ack = INTC_CONTROL;
+	ct->regs.enable = INTC_MIR_CLEAR0;
+	ct->regs.disable = INTC_MIR_SET0;
+	irq_setup_generic_chip(gc, IRQ_MSK(num),
+				IRQ_NOREQUEST | IRQ_NOPROBE, 0);
+}
+
 void __init omap_init_irq(void)
 {
 	unsigned long nr_of_irqs = 0;
 	unsigned int nr_banks = 0;
-	int i;
+	int i, j;
 
 	for (i = 0; i < ARRAY_SIZE(irq_banks); i++) {
 		unsigned long base = 0;
@@ -215,17 +170,15 @@ void __init omap_init_irq(void)
 
 		omap_irq_bank_init_one(bank);
 
+		for (i = 0, j= 0; i < bank->nr_irqs; i += 32, j += 0x20)
+			omap_alloc_gc(bank->base_reg + j, i, 32);
+
 		nr_of_irqs += bank->nr_irqs;
 		nr_banks++;
 	}
 
 	printk(KERN_INFO "Total of %ld interrupts on %d active controller%s\n",
 	       nr_of_irqs, nr_banks, nr_banks > 1 ? "s" : "");
-
-	for (i = 0; i < nr_of_irqs; i++) {
-		irq_set_chip_and_handler(i, &omap_irq_chip, handle_level_irq);
-		set_irq_flags(i, IRQF_VALID);
-	}
 }
 
 #ifdef CONFIG_ARCH_OMAP3

genirq: Implement a generic interrupt chip

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [110419 00:55]:
> * Thomas Gleixner <tglx@linutronix.de> [110416 14:20]:
> > --- linux-2.6.orig/include/linux/irq.h
> > +++ linux-2.6/include/linux/irq.h
> ...
> 
> > +/**
> > + * struct irq_chip_regs - register offsets for struct irq_gci
> > + * @enable:	Enable register offset to reg_base
> > + * @disable:	Disable register offset to reg_base
> > + * @mask:	Mask register offset to reg_base
> > + * @ack:	Ack register offset to reg_base
> > + * @ack:	Secondary Ack register offset to reg_base
> 
> Two @ack lines here?

Other than that seems to work for omap2 & 3. Posted a patch
for you too.

Tested-by: Tony Lindgren <tony@atomide.com>

[RFC patch 00/20] Interrupt chip consolidation

[Lars-Peter Clausen]

On 04/19/2011 11:41 AM, Thomas Gleixner wrote:
> On Tue, 19 Apr 2011, Lars-Peter Clausen wrote:
>> On 04/16/2011 11:14 PM, Thomas Gleixner wrote:
>>> [...]
>  
>> I've also noticed that all my chained demuxer handlers follow a similar scheme
>> and a quick grep for generic_handle_irq revealed that quite a few other
>> implementations also follow a similar scheme as well.
>>
>> Something along the lines of ...
>>
>> void irq_gc_chained_demux_handler(unsigned int irq, struct irq_desc *desc)
>> {
>> 	struct irq_chip_generic *gc = irq_desc_get_handler_data(desc);
>> 	struct irq_chip_type *ct = gc = irq_chip_generic_current_chip_type(gc);
>> 	unsigned long pending = irq_reg_readl(ct->regs.pending);
>>
>> 	pending = irq_reg_readl(ct->regs.pending) & gc->mask_cache;
>>
>> 	for_each_set_bit (irq, &pending, gc->irq_cnt)
>> 		generic_handle_irq(gc->irq_base + irq);
>> }
>>
>> ... could be used to replace those custom handlers in an irq_chip_generic based
>> implementation.
>>
>> With this the diffstat for JZ4740 looks even better:
>>  4 files changed, 91 insertions(+), 232 deletions(-)
>>
>> Unfortunately it doesn't completely fit into the current irq_chip_generic
>> implementation. For one there is no irq_chip_generic_current_chip_type, but I
>> guess that could be implementing by adding a field to irq_chip_generic pointing
>> to the current chip type. On the other hand I'm not sure if it is necessary to
> 
> No, you can get the current chip from irq_data.chip which always has a
> pointer to the current active one. container_of will give you the
> generic chip.

Yes, but the chained demux handler is of course installed on the parent irq chip.
I guess we could do something like cur_regs(irq_get_irq_data(gc->irq_base))
though, but that is a lot of indirection.

> 
>> have different pending registers for different chip types of the same
>> irq_chip_generic.
>>
>> Another issue is that while the gpio unit on the JZ4740 supports different
>> trigger modes it only supports either rising or falling edge at one time. So
>> since it doesn't makes sense to implement switching between those two in a
>> driver triggering in both edges is emulated by the irq chip.
> 
> That's what about 5 dozen other irq chips do as well.

Yes, I've noticed as well. That is why I was asking if you see the possibility
to handle this in the core somehow.

- Lars

[RFC patch 00/20] Interrupt chip consolidation

[Thomas Gleixner]

On Tue, 19 Apr 2011, Lars-Peter Clausen wrote:
> On 04/19/2011 11:41 AM, Thomas Gleixner wrote:
> > On Tue, 19 Apr 2011, Lars-Peter Clausen wrote:
> >> On 04/16/2011 11:14 PM, Thomas Gleixner wrote:
> >>> [...]
> >  
> >> I've also noticed that all my chained demuxer handlers follow a similar scheme
> >> and a quick grep for generic_handle_irq revealed that quite a few other
> >> implementations also follow a similar scheme as well.
> >>
> >> Something along the lines of ...
> >>
> >> void irq_gc_chained_demux_handler(unsigned int irq, struct irq_desc *desc)
> >> {
> >> 	struct irq_chip_generic *gc = irq_desc_get_handler_data(desc);
> >> 	struct irq_chip_type *ct = gc = irq_chip_generic_current_chip_type(gc);
> >> 	unsigned long pending = irq_reg_readl(ct->regs.pending);
> >>
> >> 	pending = irq_reg_readl(ct->regs.pending) & gc->mask_cache;
> >>
> >> 	for_each_set_bit (irq, &pending, gc->irq_cnt)
> >> 		generic_handle_irq(gc->irq_base + irq);
> >> }
> >>
> >> ... could be used to replace those custom handlers in an irq_chip_generic based
> >> implementation.
> >>
> >> With this the diffstat for JZ4740 looks even better:
> >>  4 files changed, 91 insertions(+), 232 deletions(-)
> >>
> >> Unfortunately it doesn't completely fit into the current irq_chip_generic
> >> implementation. For one there is no irq_chip_generic_current_chip_type, but I
> >> guess that could be implementing by adding a field to irq_chip_generic pointing
> >> to the current chip type. On the other hand I'm not sure if it is necessary to
> > 
> > No, you can get the current chip from irq_data.chip which always has a
> > pointer to the current active one. container_of will give you the
> > generic chip.
> 
> Yes, but the chained demux handler is of course installed on the parent irq chip.
> I guess we could do something like cur_regs(irq_get_irq_data(gc->irq_base))
> though, but that is a lot of indirection.

Nah. I whip up a inline helper which lets you retrieve that info from
*desc.
 
> >> Another issue is that while the gpio unit on the JZ4740 supports different
> >> trigger modes it only supports either rising or falling edge at one time. So
> >> since it doesn't makes sense to implement switching between those two in a
> >> driver triggering in both edges is emulated by the irq chip.
> > 
> > That's what about 5 dozen other irq chips do as well.
> 
> Yes, I've noticed as well. That is why I was asking if you see the possibility
> to handle this in the core somehow.

If the patterns are similar enough then we definitely should do that.

Thanks,

	tglx

arm: davinci: Use generic irq chip

[Kevin Hilman]

Thomas Gleixner <tglx@linutronix.de> writes:

> Simple conversion which simply uses the fact that the second irq chip
> base address has offset 0x04 to the first one.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

For davinci changes:

Reviewed-and-Tested-by: Kevin Hilman <khilman@ti.com>

genirq: Implement a generic interrupt chip

[Abhijeet Dharmapurikar]

On 04/16/2011 02:14 PM, Thomas Gleixner wrote:
> Implement a generic interrupt chip, which is configurable and is able
> to handle the most common irq chip implementations.
>
> Signed-off-by: Thomas Gleixner<tglx@linutronix.de>
> ---
>   include/linux/irq.h       |  123 ++++++++++++++++++++++
>   kernel/irq/Makefile       |    1
>   kernel/irq/generic-chip.c |  251 ++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 375 insertions(+)
>
> Index: linux-2.6/include/linux/irq.h
> ===================================================================
> --- linux-2.6.orig/include/linux/irq.h
> +++ linux-2.6/include/linux/irq.h
> @@ -573,6 +573,129 @@ static inline int irq_reserve_irq(unsign
>   	return irq_reserve_irqs(irq, 1);
>   }
>
> +#ifndef irq_reg_writel
> +# define irq_reg_writel(val, addr)	writel(val, addr)
> +#endif
> +#ifndef irq_reg_readl
> +# define irq_reg_readl(addr)		readl(addr)
> +#endif
> +
> +/**
> + * struct irq_chip_regs - register offsets for struct irq_gci
> + * @enable:	Enable register offset to reg_base
> + * @disable:	Disable register offset to reg_base
> + * @mask:	Mask register offset to reg_base
> + * @ack:	Ack register offset to reg_base
> + * @ack:	Secondary Ack register offset to reg_base
> + * @eoi:	Eoi register offset to reg_base
> + * @type:	Type configuration register offset to reg_base
> + * @polarity:	Polarity configuration register offset to reg_base
> + */
> +struct irq_chip_regs {
> +	unsigned long		enable;
> +	unsigned long		disable;
> +	unsigned long		mask;
> +	unsigned long		ack;
> +	unsigned long		eoi;
> +	unsigned long		type;
> +	unsigned long		polarity;
> +};
> +
> +/**
> + * struct irq_chip_type - Generic interrupt chip instance for a flow type
> + * @chip:		The real interrupt chip which provides the callbacks
> + * @regs:		Register offsets for this chip
> + * @handler:		Flow handler associated with this chip
> + * @type:		Chip can handle these flow types
> + *
> + * A irq_generic_chip can have several instances of irq_chip_type when
> + * it requires different functions and register offsets for different
> + * flow types.
> + */
> +struct irq_chip_type {
> +	struct irq_chip		chip;
> +	struct irq_chip_regs	regs;
> +	irq_flow_handler_t	handler;
> +	u32			type;
> +};
> +
> +/**
> + * struct irq_chip_generic - Generic irq chip data structure
> + * @lock:		Lock to protect register and cache data access
> + * @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
> + * @wake_active:	Interrupt is marked as an wakeup from suspend source
> + * @num_ct:		Number of available irq_chip_type instances (usually 1)
> + * @private:		Private data for non generic chip callbacks
> + * @chip_types:		Array of interrupt irq_chip_types
> + *
> + * Note, that irq_chip_generic can have multiple irq_chip_type
> + * implementations which can be associated to a particular irq line of
> + * an irq_chip_generic instance. That allows to share and protect
> + * state in an irq_chip_generic instance when we need to implement
> + * different flow mechanisms (level/edge) for it.
> + */
> +struct irq_chip_generic {
> +	raw_spinlock_t		lock;
> +	void __iomem		*reg_base;
> +	unsigned int		irq_base;
> +	unsigned int		irq_cnt;
> +	u32			mask_cache;
> +	u32			type_cache;
> +	u32			polarity_cache;
> +	u32			wake_enabled;
> +	u32			wake_active;
> +	unsigned int		num_ct;
> +	void			*private;
> +	struct irq_chip_type	chip_types[0];
> +};
> +
> +/* Generic chip callback functions */
> +void irq_gc_noop(struct irq_data *d);
> +void irq_gc_mask_disable_reg(struct irq_data *d);
> +void irq_gc_mask_set_bit(struct irq_data *d);
> +void irq_gc_mask_clr_bit(struct irq_data *d);
> +void irq_gc_unmask_enable_reg(struct irq_data *d);
> +void irq_gc_ack(struct irq_data *d);
> +void irq_gc_mask_disable_reg_and_ack(struct irq_data *d);
> +void irq_gc_eoi(struct irq_data *d);
> +int irq_gc_set_wake(struct irq_data *d, unsigned int on);
> +
> +/* Setup functions for irq_chip_generic */
> +struct irq_chip_generic *
> +irq_alloc_generic_chip(const char *name, int nr_ct, unsigned int irq_base,
> +		       void __iomem *reg_base, irq_flow_handler_t handler);
> +void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
> +			    unsigned int clr, unsigned int set);
> +int irq_setup_alt_chip(struct irq_data *d, unsigned int type);
> +
> +static inline struct irq_chip_type *irq_data_get_chip_type(struct irq_data *d)
> +{
> +	return container_of(d->chip, struct irq_chip_type, chip);
> +}
> +
> +#define IRQ_MSK(n) (u32)((n)<  32 ? ((1<<  (n)) - 1) : UINT_MAX)
> +
> +#ifdef CONFIG_SMP
> +static inline void irq_gc_lock(struct irq_chip_generic *gc)
> +{
> +	raw_spin_lock(&gc->lock);
> +}
> +
> +static inline void irq_gc_unlock(struct irq_chip_generic *gc)
> +{
> +	raw_spin_unlock(&gc->lock);
> +}
> +#else
> +static inline void irq_gc_lock(struct irq_chip_generic *gc) { }
> +static inline void irq_gc_unlock(struct irq_chip_generic *gc) { }
> +#endif
> +

Shall we create a new header file for this, lets say geneirc_irq_chip.h? 
The generic irq chip api is different from what  irq.h provides, I think

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

arm: orion: Use generic irq chip

[Abhijeet Dharmapurikar]

On 04/16/2011 02:14 PM, Thomas Gleixner wrote:
> The core interrupt chip is a straight forward conversion. The gpio
> chip is implemented with two instances of the irq_chip_type which can
> be switched with the irq_set_type function. That allows us to use the
> generic callbacks and avoids the conditionals in them.
>
> Signed-off-by: Thomas Gleixner<tglx@linutronix.de>
> ---
>   arch/arm/plat-orion/gpio.c              |  110 ++++++++++----------------------
>   arch/arm/plat-orion/include/plat/gpio.h |    1
>   arch/arm/plat-orion/irq.c               |   49 ++------------
>   3 files changed, 44 insertions(+), 116 deletions(-)
>
> Index: linux-2.6/arch/arm/plat-orion/gpio.c
> ===================================================================
> --- linux-2.6.orig/arch/arm/plat-orion/gpio.c
> +++ linux-2.6/arch/arm/plat-orion/gpio.c
> @@ -321,59 +321,16 @@ EXPORT_SYMBOL(orion_gpio_set_blink);
>    *        polarity    LEVEL          mask
>    *
>    ****************************************************************************/
> -static void gpio_irq_ack(struct irq_data *d)
> -{
> -	struct orion_gpio_chip *ochip = irq_data_get_irq_chip_data(d);
> -	int type = irqd_get_trigger_type(d);
> -
> -	if (type&  (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING)) {
> -		int pin = d->irq - ochip->secondary_irq_base;
> -
> -		writel(~(1<<  pin), GPIO_EDGE_CAUSE(ochip));
> -	}
> -}
> -
> -static void gpio_irq_mask(struct irq_data *d)
> -{
> -	struct orion_gpio_chip *ochip = irq_data_get_irq_chip_data(d);
> -	int type = irqd_get_trigger_type(d);
> -	void __iomem *reg;
> -	int pin;
> -
> -	if (type&  (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING))
> -		reg = GPIO_EDGE_MASK(ochip);
> -	else
> -		reg = GPIO_LEVEL_MASK(ochip);
> -
> -	pin = d->irq - ochip->secondary_irq_base;
> -
> -	writel(readl(reg)&  ~(1<<  pin), reg);
> -}
> -
> -static void gpio_irq_unmask(struct irq_data *d)
> -{
> -	struct orion_gpio_chip *ochip = irq_data_get_irq_chip_data(d);
> -	int type = irqd_get_trigger_type(d);
> -	void __iomem *reg;
> -	int pin;
> -
> -	if (type&  (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING))
> -		reg = GPIO_EDGE_MASK(ochip);
> -	else
> -		reg = GPIO_LEVEL_MASK(ochip);
> -
> -	pin = d->irq - ochip->secondary_irq_base;
> -
> -	writel(readl(reg) | (1<<  pin), reg);
> -}
>
>   static int gpio_irq_set_type(struct irq_data *d, u32 type)
>   {
> -	struct orion_gpio_chip *ochip = irq_data_get_irq_chip_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 orion_gpio_chip *ochip = gc->private;
>   	int pin;
>   	u32 u;
>
> -	pin = d->irq - ochip->secondary_irq_base;
> +	pin = d->irq - gc->irq_base;
>
>   	u = readl(GPIO_IO_CONF(ochip))&  (1<<  pin);
>   	if (!u) {
> @@ -382,18 +339,14 @@ static int gpio_irq_set_type(struct irq_
>   		return -EINVAL;
>   	}
>
> -	/*
> -	 * Set edge/level type.
> -	 */
> -	if (type&  (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING)) {
> -		__irq_set_handler_locked(d->irq, handle_edge_irq);
> -	} else if (type&  (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW)) {
> -		__irq_set_handler_locked(d->irq, handle_level_irq);
> -	} else {
> -		printk(KERN_ERR "failed to set irq=%d (type=%d)\n",
> -		       d->irq, type);
> +	type&= IRQ_TYPE_SENSE_MASK;
> +	if (type == IRQ_TYPE_NONE)
>   		return -EINVAL;
> -	}
> +
> +	/* Check if we need to change chip and handler */
> +	if (!(ct->type&  type))
> +		if (irq_setup_alt_chip(d, type))
> +			return -EINVAL;
>
>   	/*
>   	 * Configure interrupt polarity.
> @@ -425,19 +378,12 @@ static int gpio_irq_set_type(struct irq_
>   	return 0;
>   }
>
> -struct irq_chip orion_gpio_irq_chip = {
> -	.name		= "orion_gpio_irq",
> -	.irq_ack	= gpio_irq_ack,
> -	.irq_mask	= gpio_irq_mask,
> -	.irq_unmask	= gpio_irq_unmask,
> -	.irq_set_type	= gpio_irq_set_type,
> -};
> -
>   void __init orion_gpio_init(int gpio_base, int ngpio,
>   			    u32 base, int mask_offset, int secondary_irq_base)
>   {
>   	struct orion_gpio_chip *ochip;
> -	int i;
> +	struct irq_chip_generic *gc;
> +	struct irq_chip_type *ct;
>
>   	if (orion_gpio_chip_count == ARRAY_SIZE(orion_gpio_chips))
>   		return;
> @@ -471,15 +417,29 @@ void __init orion_gpio_init(int gpio_bas
>   	writel(0, GPIO_EDGE_MASK(ochip));
>   	writel(0, GPIO_LEVEL_MASK(ochip));
>
> -	for (i = 0; i<  ngpio; i++) {
> -		unsigned int irq = secondary_irq_base + i;
> +	gc = irq_alloc_generic_chip("orion_gpio_irq", 1, secondary_irq_base,
> +				    ochip->base, handle_level_irq);

should this be 2 instead of 1 ?
	gc = irq_alloc_generic_chip("orion_gpio_irq", 2,  ...


> +	gc->private = ochip;
> +
> +	ct = gc->chip_types;
> +	ct->regs.mask = ochip->mask_offset + GPIO_LEVEL_MASK_OFF;
> +	ct->type = IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW;
> +	ct->chip.irq_mask = irq_gc_mask_clr_bit;
> +	ct->chip.irq_unmask = irq_gc_mask_set_bit;
> +	ct->chip.irq_set_type = gpio_irq_set_type;
> +
> +	ct++;
> +	ct->regs.mask = ochip->mask_offset + GPIO_EDGE_MASK_OFF;
> +	ct->regs.ack = GPIO_EDGE_CAUSE_OFF;
> +	ct->type = IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
> +	ct->chip.irq_ack = irq_gc_ack;
> +	ct->chip.irq_mask = irq_gc_mask_clr_bit;
> +	ct->chip.irq_unmask = irq_gc_mask_set_bit;
> +	ct->chip.irq_set_type = gpio_irq_set_type;
> +	ct->handler = handle_edge_irq;
>
> -		irq_set_chip_and_handler(irq,&orion_gpio_irq_chip,
> -					 handle_level_irq);
> -		irq_set_chip_data(irq, ochip);
> -		irq_set_status_flags(irq, IRQ_LEVEL);
> -		set_irq_flags(irq, IRQF_VALID);
> -	}
> +	irq_setup_generic_chip(gc, IRQ_MSK(ngpio), IRQ_NOREQUEST,
> +			       IRQ_LEVEL | IRQ_NOPROBE);
>   }

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

arm: orion: Use generic irq chip

[Thomas Gleixner]

On Wed, 20 Apr 2011, Abhijeet Dharmapurikar wrote:
> > -	for (i = 0; i<  ngpio; i++) {
> > -		unsigned int irq = secondary_irq_base + i;
> > +	gc = irq_alloc_generic_chip("orion_gpio_irq", 1, secondary_irq_base,
> > +				    ochip->base, handle_level_irq);
> 
> should this be 2 instead of 1 ?
> 	gc = irq_alloc_generic_chip("orion_gpio_irq", 2,  ...

Ouch, yes.

genirq: Implement a generic interrupt chip

[Thomas Gleixner]

On Wed, 20 Apr 2011, Abhijeet Dharmapurikar wrote:
> On 04/16/2011 02:14 PM, Thomas Gleixner wrote:
> > +#else
> > +static inline void irq_gc_lock(struct irq_chip_generic *gc) { }
> > +static inline void irq_gc_unlock(struct irq_chip_generic *gc) { }
> > +#endif
> > +
> 
> Shall we create a new header file for this, lets say geneirc_irq_chip.h? The
> generic irq chip api is different from what  irq.h provides, I think

And that header includes irq.h then. Where is the point ?

arm: msm: Convert sirc to generic irq chip

[Abhijeet Dharmapurikar]

On 04/16/2011 02:14 PM, Thomas Gleixner wrote:
> Signed-off-by: Thomas Gleixner<tglx@linutronix.de>
> ---
>   arch/arm/mach-msm/include/mach/sirc.h |   43 +++-----
>   arch/arm/mach-msm/sirc.c              |  166 ++++++++++------------------------
>   2 files changed, 68 insertions(+), 141 deletions(-)
>
> Index: linux-2.6/arch/arm/mach-msm/include/mach/sirc.h
> ===================================================================
> --- linux-2.6.orig/arch/arm/mach-msm/include/mach/sirc.h
> +++ linux-2.6/arch/arm/mach-msm/include/mach/sirc.h
> @@ -13,18 +13,13 @@
>   #ifndef __ASM_ARCH_MSM_SIRC_H
>   #define __ASM_ARCH_MSM_SIRC_H
>
> -struct sirc_regs_t {
> -	void    *int_enable;
> -	void    *int_enable_clear;
> -	void    *int_enable_set;
> -	void    *int_type;
> -	void    *int_polarity;
> -	void    *int_clear;
> -};
> -
>   struct sirc_cascade_regs {
> -	void    *int_status;
> -	unsigned int    cascade_irq;
> +	void __iomem	*reg_base;
> +	unsigned long	int_status;
> +	u32		status_mask;
> +	unsigned int	irq_base;
> +	unsigned int	irq_cnt;
> +	unsigned int	cascade_irq;
>   };
>
>   void msm_init_sirc(void);
> @@ -78,20 +73,18 @@ void msm_sirc_exit_sleep(void);
>   #define SIRC_MASK                     0x007FFFFF
>   #endif
>
> -#define LAST_SIRC_IRQ                 (FIRST_SIRC_IRQ + NR_SIRC_IRQS - 1)
> -
> -#define SPSS_SIRC_INT_SELECT          (MSM_SIRC_BASE + 0x00)
> -#define SPSS_SIRC_INT_ENABLE          (MSM_SIRC_BASE + 0x04)
> -#define SPSS_SIRC_INT_ENABLE_CLEAR    (MSM_SIRC_BASE + 0x08)
> -#define SPSS_SIRC_INT_ENABLE_SET      (MSM_SIRC_BASE + 0x0C)
> -#define SPSS_SIRC_INT_TYPE            (MSM_SIRC_BASE + 0x10)
> -#define SPSS_SIRC_INT_POLARITY        (MSM_SIRC_BASE + 0x14)
> -#define SPSS_SIRC_SECURITY            (MSM_SIRC_BASE + 0x18)
> -#define SPSS_SIRC_IRQ_STATUS          (MSM_SIRC_BASE + 0x1C)
> -#define SPSS_SIRC_IRQ1_STATUS         (MSM_SIRC_BASE + 0x20)
> -#define SPSS_SIRC_RAW_STATUS          (MSM_SIRC_BASE + 0x24)
> -#define SPSS_SIRC_INT_CLEAR           (MSM_SIRC_BASE + 0x28)
> -#define SPSS_SIRC_SOFT_INT            (MSM_SIRC_BASE + 0x2C)
> +#define SPSS_SIRC_INT_SELECT          0x00
> +#define SPSS_SIRC_INT_ENABLE          0x04
> +#define SPSS_SIRC_INT_ENABLE_CLEAR    0x08
> +#define SPSS_SIRC_INT_ENABLE_SET      0x0C
> +#define SPSS_SIRC_INT_TYPE            0x10
> +#define SPSS_SIRC_INT_POLARITY        0x14
> +#define SPSS_SIRC_SECURITY            0x18
> +#define SPSS_SIRC_IRQ_STATUS          0x1C
> +#define SPSS_SIRC_IRQ1_STATUS         0x20
> +#define SPSS_SIRC_RAW_STATUS          0x24
> +#define SPSS_SIRC_INT_CLEAR           0x28
> +#define SPSS_SIRC_SOFT_INT            0x2C
>
>   #endif
>
> Index: linux-2.6/arch/arm/mach-msm/sirc.c
> ===================================================================
> --- linux-2.6.orig/arch/arm/mach-msm/sirc.c
> +++ linux-2.6/arch/arm/mach-msm/sirc.c
> @@ -15,158 +15,92 @@
>    * 02110-1301, USA.
>    *
>    */
> -
> -#include<linux/io.h>
> -#include<linux/irq.h>
>   #include<linux/interrupt.h>
> -#include<asm/irq.h>
> -
> -static unsigned int int_enable;
> -static unsigned int wake_enable;
> +#include<linux/irq.h>
> +#include<linux/io.h>
>
> -static struct sirc_regs_t sirc_regs = {
> -	.int_enable       = SPSS_SIRC_INT_ENABLE,
> -	.int_enable_clear = SPSS_SIRC_INT_ENABLE_CLEAR,
> -	.int_enable_set   = SPSS_SIRC_INT_ENABLE_SET,
> -	.int_type         = SPSS_SIRC_INT_TYPE,
> -	.int_polarity     = SPSS_SIRC_INT_POLARITY,
> -	.int_clear        = SPSS_SIRC_INT_CLEAR,
> -};
> +#include<asm/irq.h>
>
>   static struct sirc_cascade_regs sirc_reg_table[] = {
>   	{
> +		.reg_base    = MSM_SIRC_BASE,
>   		.int_status  = SPSS_SIRC_IRQ_STATUS,
> +		.status_mask = SIRC_MASK,
> +		.irq_base    = FIRST_SIRC_IRQ,
> +		.irq_cnt     = NR_SIRC_IRQS,
>   		.cascade_irq = INT_SIRC_0,
>   	}
>   };
>
> -/* Mask off the given interrupt. Keep the int_enable mask in sync with
> -   the enable reg, so it can be restored after power collapse. */
> -static void sirc_irq_mask(struct irq_data *d)
> -{
> -	unsigned int mask;
> -
> -	mask = 1<<  (d->irq - FIRST_SIRC_IRQ);
> -	writel(mask, sirc_regs.int_enable_clear);
> -	int_enable&= ~mask;
> -	return;
> -}
> -
> -/* Unmask the given interrupt. Keep the int_enable mask in sync with
> -   the enable reg, so it can be restored after power collapse. */
> -static void sirc_irq_unmask(struct irq_data *d)
> -{
> -	unsigned int mask;
> -
> -	mask = 1<<  (d->irq - FIRST_SIRC_IRQ);
> -	writel(mask, sirc_regs.int_enable_set);
> -	int_enable |= mask;
> -	return;
> -}
> -
> -static void sirc_irq_ack(struct irq_data *d)
> -{
> -	unsigned int mask;
> -
> -	mask = 1<<  (d->irq - FIRST_SIRC_IRQ);
> -	writel(mask, sirc_regs.int_clear);
> -	return;
> -}
> -
> -static int sirc_irq_set_wake(struct irq_data *d, unsigned int on)
> -{
> -	unsigned int mask;
> -
> -	/* Used to set the interrupt enable mask during power collapse. */
> -	mask = 1<<  (d->irq - FIRST_SIRC_IRQ);
> -	if (on)
> -		wake_enable |= mask;
> -	else
> -		wake_enable&= ~mask;
> -
> -	return 0;
> -}
> -
>   static int sirc_irq_set_type(struct irq_data *d, unsigned int flow_type)
>   {
> -	unsigned int mask;
> -	unsigned int val;
> -
> -	mask = 1<<  (d->irq - FIRST_SIRC_IRQ);
> -	val = readl(sirc_regs.int_polarity);
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	u32 mask = 1<<  (d->irq - gc->irq_base);
>
>   	if (flow_type&  (IRQF_TRIGGER_LOW | IRQF_TRIGGER_FALLING))
> -		val |= mask;
> +		gc->polarity_cache |= mask;
>   	else
> -		val&= ~mask;
> -
> -	writel(val, sirc_regs.int_polarity);
> +		gc->polarity_cache&= ~mask;
> +	writel(gc->polarity_cache, gc->reg_base + gc->chip_types->regs.type);
>
> -	val = readl(sirc_regs.int_type);
>   	if (flow_type&  (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) {
> -		val |= mask;
> +		gc->type_cache |= mask;
>   		__irq_set_handler_locked(d->irq, handle_edge_irq);
>   	} else {
> -		val&= ~mask;
> +		gc->type_cache&= ~mask;
>   		__irq_set_handler_locked(d->irq, handle_level_irq);
>   	}
> -
> -	writel(val, sirc_regs.int_type);
> -
> +	writel(gc->type_cache, gc->reg_base + gc->chip_types->regs.polarity);
>   	return 0;
>   }
>
>   /* Finds the pending interrupt on the passed cascade irq and redrives it */
>   static void sirc_irq_handler(unsigned int irq, struct irq_desc *desc)
>   {
> -	unsigned int reg = 0;
> -	unsigned int sirq;
> -	unsigned int status;
> -
> -	while ((reg<  ARRAY_SIZE(sirc_reg_table))&&
> -		(sirc_reg_table[reg].cascade_irq != irq))
> -		reg++;
> -
> -	status = readl(sirc_reg_table[reg].int_status);
> -	status&= SIRC_MASK;
> -	if (status == 0)
> +	struct sirc_cascade_regs *cr = irq_desc_get_handler_data(desc);
> +	u32 status;
> +
> +	status = readl(cr->reg_base + cr->int_status);
> +	status&= cr->status_mask;
> +	if (!status)
>   		return;
>
> -	for (sirq = 0;
> -	     (sirq<  NR_SIRC_IRQS)&&  ((status&  (1U<<  sirq)) == 0);
> -	     sirq++)
> -		;
> -	generic_handle_irq(sirq+FIRST_SIRC_IRQ);
> +	generic_handle_irq(cr->irq_base + __ffs(status));
>
>   	desc->irq_data.chip->irq_ack(&desc->irq_data);
>   }
>
> -static struct irq_chip sirc_irq_chip = {
> -	.name          = "sirc",
> -	.irq_ack       = sirc_irq_ack,
> -	.irq_mask      = sirc_irq_mask,
> -	.irq_unmask    = sirc_irq_unmask,
> -	.irq_set_wake  = sirc_irq_set_wake,
> -	.irq_set_type  = sirc_irq_set_type,
> -};
> +static void msm_init_one_sirc(struct sirc_cascade_regs *cr)
> +{
> +	struct irq_chip_generic *gc;
> +	struct irq_chip_type *ct;
> +
> +	gc = irq_alloc_generic_chip("sirc", 1, cr->irq_base, cr->reg_base,
> +				    handle_level_irq);
> +	ct = gc->chip_types;
> +	ct->chip.irq_ack = irq_gc_ack;
> +	ct->chip.irq_mask = irq_gc_mask_disable_reg;
> +	ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
> +	ct->chip.irq_set_wake = irq_gc_set_wake;
> +	ct->chip.irq_set_type = sirc_irq_set_type;
> +	ct->regs.ack = SPSS_SIRC_INT_CLEAR;
> +	ct->regs.disable = SPSS_SIRC_INT_ENABLE_CLEAR;
> +	ct->regs.enable = SPSS_SIRC_INT_ENABLE_SET;
> +	ct->regs.type = SPSS_SIRC_INT_TYPE;
> +	ct->regs.polarity = SPSS_SIRC_INT_POLARITY;
> +
> +	irq_setup_generic_chip(gc, IRQ_MSK(sirc->irq_cnt),
> +			       IRQ_NOREQUEST | IRQ_NOPROBE, 0);
> +
> +	irq_set_chained_handler(cr->cascade_irq, sirc_irq_handler);
> +	irq_set_handler_data(cr->cascade_irq, cr);
> +	irq_set_irq_wake(cr->cascade_irq, 1);
> +}
>
>   void __init msm_init_sirc(void)
>   {
>   	int i;
>
> -	int_enable = 0;
> -	wake_enable = 0;
> -
> -	for (i = FIRST_SIRC_IRQ; i<  LAST_SIRC_IRQ; i++) {
> -		irq_set_chip_and_handler(i,&sirc_irq_chip, handle_edge_irq);
> -		set_irq_flags(i, IRQF_VALID);
> -	}
> -
> -	for (i = 0; i<  ARRAY_SIZE(sirc_reg_table); i++) {
> -		irq_set_chained_handler(sirc_reg_table[i].cascade_irq,
> -					sirc_irq_handler);
> -		irq_set_irq_wake(sirc_reg_table[i].cascade_irq, 1);
> -	}
> -	return;
> +	for (i = 0; i<  ARRAY_SIZE(sirc_reg_table); i++)
> +		msm_init_one_sirc(sirc_reg_table + i);
>   }

This looks good to me. Thanks a ton for cleaning it up.

Abhijeet
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

arm: msm: Convert irq-vic to generic irq chip

[Abhijeet Dharmapurikar]

On 04/16/2011 02:14 PM, Thomas Gleixner wrote:
> This removes a ton of completely useless code. The shadow registers,
> which are set but never used are maintained in the generic chip
> functions on a per chip basis as well.
>
> Signed-off-by: Thomas Gleixner<tglx@linutronix.de>
> ---
>   arch/arm/mach-msm/irq-vic.c |  342 ++++++--------------------------------------
>   1 file changed, 51 insertions(+), 291 deletions(-)
>
> Index: linux-2.6/arch/arm/mach-msm/irq-vic.c
> ===================================================================
> --- linux-2.6.orig/arch/arm/mach-msm/irq-vic.c
> +++ linux-2.6/arch/arm/mach-msm/irq-vic.c
> @@ -12,331 +12,93 @@
>    * GNU General Public License for more details.
>    *
>    */
> -
> -#include<linux/init.h>
> -#include<linux/module.h>
> -#include<linux/sched.h>
>   #include<linux/interrupt.h>
> -#include<linux/ptrace.h>
> -#include<linux/timer.h>
>   #include<linux/irq.h>
>   #include<linux/io.h>
>
> -#include<asm/cacheflush.h>
> -
>   #include<mach/hardware.h>
> -
>   #include<mach/msm_iomap.h>
>
> -#include "smd_private.h"
> -
> -enum {
> -	IRQ_DEBUG_SLEEP_INT_TRIGGER = 1U<<  0,
> -	IRQ_DEBUG_SLEEP_INT = 1U<<  1,
> -	IRQ_DEBUG_SLEEP_ABORT = 1U<<  2,
> -	IRQ_DEBUG_SLEEP = 1U<<  3,
> -	IRQ_DEBUG_SLEEP_REQUEST = 1U<<  4,
> -};
> -static int msm_irq_debug_mask;
> -module_param_named(debug_mask, msm_irq_debug_mask, int,
> -		   S_IRUGO | S_IWUSR | S_IWGRP);
> -
>   #define VIC_REG(off) (MSM_VIC_BASE + (off))
> -#define VIC_INT_TO_REG_ADDR(base, irq) (base + (irq / 32) * 4)
> -#define VIC_INT_TO_REG_INDEX(irq) ((irq>>  5)&  3)
>
> -#define VIC_INT_SELECT0     VIC_REG(0x0000)  /* 1: FIQ, 0: IRQ */
> -#define VIC_INT_SELECT1     VIC_REG(0x0004)  /* 1: FIQ, 0: IRQ */
> -#define VIC_INT_SELECT2     VIC_REG(0x0008)  /* 1: FIQ, 0: IRQ */
> -#define VIC_INT_SELECT3     VIC_REG(0x000C)  /* 1: FIQ, 0: IRQ */
> -#define VIC_INT_EN0         VIC_REG(0x0010)
> -#define VIC_INT_EN1         VIC_REG(0x0014)
> -#define VIC_INT_EN2         VIC_REG(0x0018)
> -#define VIC_INT_EN3         VIC_REG(0x001C)
> -#define VIC_INT_ENCLEAR0    VIC_REG(0x0020)
> -#define VIC_INT_ENCLEAR1    VIC_REG(0x0024)
> -#define VIC_INT_ENCLEAR2    VIC_REG(0x0028)
> -#define VIC_INT_ENCLEAR3    VIC_REG(0x002C)
> -#define VIC_INT_ENSET0      VIC_REG(0x0030)
> -#define VIC_INT_ENSET1      VIC_REG(0x0034)
> -#define VIC_INT_ENSET2      VIC_REG(0x0038)
> -#define VIC_INT_ENSET3      VIC_REG(0x003C)
> -#define VIC_INT_TYPE0       VIC_REG(0x0040)  /* 1: EDGE, 0: LEVEL  */
> -#define VIC_INT_TYPE1       VIC_REG(0x0044)  /* 1: EDGE, 0: LEVEL  */
> -#define VIC_INT_TYPE2       VIC_REG(0x0048)  /* 1: EDGE, 0: LEVEL  */
> -#define VIC_INT_TYPE3       VIC_REG(0x004C)  /* 1: EDGE, 0: LEVEL  */
> -#define VIC_INT_POLARITY0   VIC_REG(0x0050)  /* 1: NEG, 0: POS */
> -#define VIC_INT_POLARITY1   VIC_REG(0x0054)  /* 1: NEG, 0: POS */
> -#define VIC_INT_POLARITY2   VIC_REG(0x0058)  /* 1: NEG, 0: POS */
> -#define VIC_INT_POLARITY3   VIC_REG(0x005C)  /* 1: NEG, 0: POS */
> -#define VIC_NO_PEND_VAL     VIC_REG(0x0060)
> +#define VIC_INT_SELECT0		0x0000		/* 1: FIQ, 0: IRQ */
> +#define VIC_INT_EN0		0x0010
> +#define VIC_INT_ENCLEAR0	0x0020
> +#define VIC_INT_ENSET0		0x0030
> +#define VIC_INT_TYPE0		0x0040		/* 1: EDGE, 0: LEVEL  */
> +#define VIC_INT_POLARITY0	0x0050		/* 1: NEG, 0: POS */
> +#define VIC_INT_CLEAR0		0x00B0
>
>   #if defined(CONFIG_ARCH_MSM_SCORPION)
> -#define VIC_NO_PEND_VAL_FIQ VIC_REG(0x0064)
> -#define VIC_INT_MASTEREN    VIC_REG(0x0068)  /* 1: IRQ, 2: FIQ     */
> -#define VIC_CONFIG          VIC_REG(0x006C)  /* 1: USE SC VIC */
> +#define VIC_INT_MASTEREN	0x0068		/* 1: IRQ, 2: FIQ     */
> +#define VIC_CONFIG		0x006C		/* 1: USE ARM1136 VIC */
>   #else
> -#define VIC_INT_MASTEREN    VIC_REG(0x0064)  /* 1: IRQ, 2: FIQ     */
> -#define VIC_PROTECTION      VIC_REG(0x006C)  /* 1: ENABLE          */
> -#define VIC_CONFIG          VIC_REG(0x0068)  /* 1: USE ARM1136 VIC */
> +#define VIC_INT_MASTEREN	0x0064		/* 1: IRQ, 2: FIQ     */
> +#define VIC_CONFIG		0x0068		/* 1: USE ARM1136 VIC */
>   #endif
>
> -#define VIC_IRQ_STATUS0     VIC_REG(0x0080)
> -#define VIC_IRQ_STATUS1     VIC_REG(0x0084)
> -#define VIC_IRQ_STATUS2     VIC_REG(0x0088)
> -#define VIC_IRQ_STATUS3     VIC_REG(0x008C)
> -#define VIC_FIQ_STATUS0     VIC_REG(0x0090)
> -#define VIC_FIQ_STATUS1     VIC_REG(0x0094)
> -#define VIC_FIQ_STATUS2     VIC_REG(0x0098)
> -#define VIC_FIQ_STATUS3     VIC_REG(0x009C)
> -#define VIC_RAW_STATUS0     VIC_REG(0x00A0)
> -#define VIC_RAW_STATUS1     VIC_REG(0x00A4)
> -#define VIC_RAW_STATUS2     VIC_REG(0x00A8)
> -#define VIC_RAW_STATUS3     VIC_REG(0x00AC)
> -#define VIC_INT_CLEAR0      VIC_REG(0x00B0)
> -#define VIC_INT_CLEAR1      VIC_REG(0x00B4)
> -#define VIC_INT_CLEAR2      VIC_REG(0x00B8)
> -#define VIC_INT_CLEAR3      VIC_REG(0x00BC)
> -#define VIC_SOFTINT0        VIC_REG(0x00C0)
> -#define VIC_SOFTINT1        VIC_REG(0x00C4)
> -#define VIC_SOFTINT2        VIC_REG(0x00C8)
> -#define VIC_SOFTINT3        VIC_REG(0x00CC)
> -#define VIC_IRQ_VEC_RD      VIC_REG(0x00D0)  /* pending int # */
> -#define VIC_IRQ_VEC_PEND_RD VIC_REG(0x00D4)  /* pending vector addr */
> -#define VIC_IRQ_VEC_WR      VIC_REG(0x00D8)
> -
> -#if defined(CONFIG_ARCH_MSM_SCORPION)
> -#define VIC_FIQ_VEC_RD      VIC_REG(0x00DC)
> -#define VIC_FIQ_VEC_PEND_RD VIC_REG(0x00E0)
> -#define VIC_FIQ_VEC_WR      VIC_REG(0x00E4)
> -#define VIC_IRQ_IN_SERVICE  VIC_REG(0x00E8)
> -#define VIC_IRQ_IN_STACK    VIC_REG(0x00EC)
> -#define VIC_FIQ_IN_SERVICE  VIC_REG(0x00F0)
> -#define VIC_FIQ_IN_STACK    VIC_REG(0x00F4)
> -#define VIC_TEST_BUS_SEL    VIC_REG(0x00F8)
> -#define VIC_IRQ_CTRL_CONFIG VIC_REG(0x00FC)
> -#else
> -#define VIC_IRQ_IN_SERVICE  VIC_REG(0x00E0)
> -#define VIC_IRQ_IN_STACK    VIC_REG(0x00E4)
> -#define VIC_TEST_BUS_SEL    VIC_REG(0x00E8)
> -#endif
> -
> -#define VIC_VECTPRIORITY(n) VIC_REG(0x0200+((n) * 4))
> -#define VIC_VECTADDR(n)     VIC_REG(0x0400+((n) * 4))
> -
>   #if defined(CONFIG_ARCH_MSM7X30)
>   #define VIC_NUM_REGS	    4
>   #else
>   #define VIC_NUM_REGS	    2
>   #endif
>
> -#if VIC_NUM_REGS == 2
> -#define DPRINT_REGS(base_reg, format, ...)	      			\
> -	printk(KERN_INFO format " %x %x\n", ##__VA_ARGS__,		\
> -			readl(base_reg ## 0), readl(base_reg ## 1))
> -#define DPRINT_ARRAY(array, format, ...)				\
> -	printk(KERN_INFO format " %x %x\n", ##__VA_ARGS__,		\
> -			array[0], array[1])
> -#elif VIC_NUM_REGS == 4
> -#define DPRINT_REGS(base_reg, format, ...) \
> -	printk(KERN_INFO format " %x %x %x %x\n", ##__VA_ARGS__,	\
> -			readl(base_reg ## 0), readl(base_reg ## 1),	\
> -			readl(base_reg ## 2), readl(base_reg ## 3))
> -#define DPRINT_ARRAY(array, format, ...)				\
> -	printk(KERN_INFO format " %x %x %x %x\n", ##__VA_ARGS__,	\
> -			array[0], array[1],				\
> -			array[2], array[3])
> -#else
> -#error "VIC_NUM_REGS set to illegal value"
> -#endif
> -
> -static uint32_t msm_irq_smsm_wake_enable[2];
> -static struct {
> -	uint32_t int_en[2];
> -	uint32_t int_type;
> -	uint32_t int_polarity;
> -	uint32_t int_select;
> -} msm_irq_shadow_reg[VIC_NUM_REGS];
> -static uint32_t msm_irq_idle_disable[VIC_NUM_REGS];
> -
> -#define SMSM_FAKE_IRQ (0xff)
> -static uint8_t msm_irq_to_smsm[NR_IRQS] = {
> -	[INT_MDDI_EXT] = 1,
> -	[INT_MDDI_PRI] = 2,
> -	[INT_MDDI_CLIENT] = 3,
> -	[INT_USB_OTG] = 4,
> -
> -	[INT_PWB_I2C] = 5,
> -	[INT_SDC1_0] = 6,
> -	[INT_SDC1_1] = 7,
> -	[INT_SDC2_0] = 8,
> -
> -	[INT_SDC2_1] = 9,
> -	[INT_ADSP_A9_A11] = 10,
> -	[INT_UART1] = 11,
> -	[INT_UART2] = 12,
> -
> -	[INT_UART3] = 13,
> -	[INT_UART1_RX] = 14,
> -	[INT_UART2_RX] = 15,
> -	[INT_UART3_RX] = 16,
> -
> -	[INT_UART1DM_IRQ] = 17,
> -	[INT_UART1DM_RX] = 18,
> -	[INT_KEYSENSE] = 19,
> -#if !defined(CONFIG_ARCH_MSM7X30)
> -	[INT_AD_HSSD] = 20,
> -#endif
> -
> -	[INT_NAND_WR_ER_DONE] = 21,
> -	[INT_NAND_OP_DONE] = 22,
> -	[INT_TCHSCRN1] = 23,
> -	[INT_TCHSCRN2] = 24,
> -
> -	[INT_TCHSCRN_SSBI] = 25,
> -	[INT_USB_HS] = 26,
> -	[INT_UART2DM_RX] = 27,
> -	[INT_UART2DM_IRQ] = 28,
> -
> -	[INT_SDC4_1] = 29,
> -	[INT_SDC4_0] = 30,
> -	[INT_SDC3_1] = 31,
> -	[INT_SDC3_0] = 32,
> -
> -	/* fake wakeup interrupts */
> -	[INT_GPIO_GROUP1] = SMSM_FAKE_IRQ,
> -	[INT_GPIO_GROUP2] = SMSM_FAKE_IRQ,
> -	[INT_A9_M2A_0] = SMSM_FAKE_IRQ,
> -	[INT_A9_M2A_1] = SMSM_FAKE_IRQ,
> -	[INT_A9_M2A_5] = SMSM_FAKE_IRQ,
> -	[INT_GP_TIMER_EXP] = SMSM_FAKE_IRQ,
> -	[INT_DEBUG_TIMER_EXP] = SMSM_FAKE_IRQ,
> -	[INT_ADSP_A11] = SMSM_FAKE_IRQ,
> -#ifdef CONFIG_ARCH_QSD8X50
> -	[INT_SIRC_0] = SMSM_FAKE_IRQ,
> -	[INT_SIRC_1] = SMSM_FAKE_IRQ,
> -#endif
> -};
> -



The entire patch looks good to me.

However, let me explain that SMSM arrays are. MSM has a different 
interrupt controller that monitors few of the lines that the VIC does.
This parallel interrupt controller is activated when the MSM goes to low
power modes.

For idle case, if an interrupt not monitored by the parallel interrupt 
controller is enabled, we avoid going to low power modes. It is 
unreasonable to check all the shadow registers to find if such is the 
case, so we maintain the unmask status of such interrupts in
static uint32_t msm_irq_idle_disable[VIC_NUM_REGS]. If any bit is set we 
dont go to low power mode. This code is not upstreamed yet, I can submit 
it on top of this patch.

The reason for explaining this is to ask if we should extend the generic 
chip to have a function that returns if any interrupts besides the 
wakeup ones are unmasked. It would return

mask_cache & ~wakeup_active  if mask_cache bit set means interrupt is 
unmasked
~mask_cache & ~wakeup_active if mask_cache bit set means interrupts is 
masked

or you would prefer the driver do that check?



-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

arm: msm: Convert irq-vic to generic irq chip

[Thomas Gleixner]

On Thu, 21 Apr 2011, Abhijeet Dharmapurikar wrote:
> On 04/16/2011 02:14 PM, Thomas Gleixner wrote:
> However, let me explain that SMSM arrays are. MSM has a different interrupt
> controller that monitors few of the lines that the VIC does.
> This parallel interrupt controller is activated when the MSM goes to low
> power modes.
> 
> For idle case, if an interrupt not monitored by the parallel interrupt
> controller is enabled, we avoid going to low power modes. It is unreasonable
> to check all the shadow registers to find if such is the case, so we maintain
> the unmask status of such interrupts in
> static uint32_t msm_irq_idle_disable[VIC_NUM_REGS]. If any bit is set we dont
> go to low power mode. This code is not upstreamed yet, I can submit it on top
> of this patch.
> 
> The reason for explaining this is to ask if we should extend the generic chip
> to have a function that returns if any interrupts besides the wakeup ones are
> unmasked. It would return
> 
> mask_cache & ~wakeup_active  if mask_cache bit set means interrupt is unmasked
> ~mask_cache & ~wakeup_active if mask_cache bit set means interrupts is masked
> 
> or you would prefer the driver do that check?

That wants to be a function in the core as I assume that there are
similar usecases in tree.

Another question is whether you could move to the arm/common/vic.c
implementation completely.

Thanks,

	tglx

arm: msm: Convert irq-vic to generic irq chip

[Abhijeet Dharmapurikar]

On 04/21/2011 10:28 AM, Thomas Gleixner wrote:
> On Thu, 21 Apr 2011, Abhijeet Dharmapurikar wrote:
>> On 04/16/2011 02:14 PM, Thomas Gleixner wrote:
>
> Another question is whether you could move to the arm/common/vic.c
> implementation completely.

arm/common/vic.c doesnt have a set_type callback. We have some edge 
triggered interrupts as wakeup interrupts and need the PENDING flag in 
the descriptor set when disabled. Can we move msm_irq_set_type() to 
arm/common/vic.c ?

Additionally
- the register map is different between msm's vic and the common one.
- the common one uses some PL190 registers, not sure what that is. The 
VIC can operate in few different modes, perhaps it refers to vectored 
mode as opposed to non vectored mode, msm uses the non vectored mode.
- vic_init2() seems like it is initializing a cascaded interrupt 
controller. msm doesnt use cascaded vics.

I dont think msm can move to using the common vic.c.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

davinci: Use generic irq chip

[Nori, Sekhar]

Hi Thomas,

On Sun, Apr 17, 2011 at 02:44:08, Thomas Gleixner wrote:
> Simple conversion which simply uses the fact that the second irq chip
> base address has offset 0x04 to the first one.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Boot tested it on a DM365 EVM.

Tested-by: Sekhar Nori <nsekhar@ti.com>

> @@ -152,13 +103,8 @@ void __init davinci_irq_init(void)
>  		davinci_irq_writel(pri, i);
>  	}
>  
> -	/* set up genirq dispatch for ARM INTC */
> -	for (i = 0; i < davinci_soc_info.intc_irq_num; i++) {
> -		irq_set_chip(i, &davinci_irq_chip_0);
> -		set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
> -		if (i != IRQ_TINT1_TINT34)
> -			irq_set_handler(i, handle_edge_irq);
> -		else
> -			irq_set_handler(i, handle_level_irq);
> -	}
> +	for (i = 0, j= 0; i < davinci_soc_info.intc_irq_num; i += 32, j += 0x04)
                   ^
There is a space needed before the '=' as pointed by checkpatch.

Thanks,
Sekhar

arm: msm: Convert irq-vic to generic irq chip

[Linus Walleij]

2011/4/25 Abhijeet Dharmapurikar <adharmap@codeaurora.org>:

> - the common one uses some PL190 registers, not sure what that is.

PL190 is ARMs name for the VIC: PrimeCell 190.

Every vendor get these PrimeCells as part of their bargin for CPU
cores when licensing ARM IP. Then they tweak them to suit their
needs and fix hardware bugs.

It seems Qualcomm HW engineers did the same thing as everyone
else, tweak a PrimeCell by editing VHDL until it works...

Yours,
Linus Walleij