OMAP: DSS2: Common IRQ handler for all OMAPs

OMAP2 has an irq line dedicated for DISPC interrupts, there is no DSI on omap2.
OMAP3 has a common irq line for DISPC and DSI interrupts. OMAP4 has seperate irq lines for DISPC and DSI Interrupts.

Use dss_features to have a common DSS irq handler for all OMAP revisions.

Also, use a member of the global dss structure to store the irq number as it is used in 2 functions.

Quoted 9 lines · archit taneja

OMAP2 has an irq line dedicated for DISPC interrupts, there is no DSI on omap2.
OMAP3 has a common irq line for DISPC and DSI interrupts. OMAP4 has seperate irq lines for DISPC and DSI Interrupts.

Use dss_features to have a common DSS irq handler for all OMAP revisions.

Also, use a member of the global dss structure to store the irq number as it is used in 2 functions.

It's good to remove the cpu_is_xxxx() calls, but I'm not quite sure about this patch...

Could we use shared interrupt handlers here, so that dss.c would handle only DISPC interrupts (or should it be even in dispc.c?) and dsi.c would handle DSI interrupts?

On OMAP3 both dss.c and dsi.c would register to the same interrupt, and they would need to check if the interrupt was really for them. On OMAP4 the code could be the same, even though the check is unnecessary.

Also, as I mentioned in the email I sent some minutes ago, this patch fixes the free_irq call in dss_exit. Things like that should never be fixed silently, even if it's trivial like in this case.

Hi,

Quoted 17 lines · Tomi Valkeinen

Quoted 9 lines · archit taneja

OMAP2 has an irq line dedicated for DISPC interrupts, there is no DSI on omap2.
OMAP3 has a common irq line for DISPC and DSI interrupts. OMAP4 has seperate irq lines for DISPC and DSI Interrupts.

Use dss_features to have a common DSS irq handler for all OMAP revisions.

Also, use a member of the global dss structure to store the irq number as it is used in 2 functions.

It's good to remove the cpu_is_xxxx() calls, but I'm not quite sure about this patch...

Could we use shared interrupt handlers here, so that dss.c would handle only DISPC interrupts (or should it be even in dispc.c?) and dsi.c would handle DSI interrupts?

Quoted 4 lines

On OMAP3 both dss.c and dsi.c would register to the same interrupt, and they would need to check if the interrupt was really for them. On OMAP4 the code could be the same, even though the check is unnecessary.

Quoted 4 lines

Also, as I mentioned in the email I sent some minutes ago, this patch fixes the free_irq call in dss_exit. Things like that should never be fixed silently, even if it's trivial like in this case.

does it make sense to install an irq_chip for that ? I mean, can you mask/unmask dss and or dsi IRQs ? If you can, then it might make sense to take a look into GENIRQ and install an irq_chip for that. Then both dsi and dss would be able to use standard request_irq() API.

Hi,

Quoted 14 lines · Felipe

Hi,

Quoted 11 lines · Tomi Valkeinen

Quoted 9 lines · archit taneja

OMAP2 has an irq line dedicated for DISPC interrupts, there is no DSI on omap2.
OMAP3 has a common irq line for DISPC and DSI interrupts. OMAP4 has seperate irq lines for DISPC and DSI Interrupts.

Use dss_features to have a common DSS irq handler for all OMAP revisions.

Also, use a member of the global dss structure to store the irq number as it is used in 2 functions.

Quoted 6 lines

Quoted 6 lines

It's good to remove the cpu_is_xxxx() calls, but I'm not quite sure about this patch...

Could we use shared interrupt handlers here, so that dss.c would handle only DISPC interrupts (or should it be even in dispc.c?) and dsi.c would handle DSI interrupts?

Could you elaborate this a bit more?

Quoted 5 lines

Quoted 5 lines

On OMAP3 both dss.c and dsi.c would register to the same interrupt, and they would need to check if the interrupt was really for them. On OMAP4 the code could be the same, even though the check is unnecessary.

The code can't be exactly the same. The DSS_IRQSTATUS register used on OMAP3 doesn't exist on OMAP4. A read to this register on OMAP4 would cause a hang/crash.

I meant something like this:

dispc.c:

dispc_init()

{
	/* did we have a pdev for dispc? if not, this needs to be dss.pdev */
	request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, "dispc irq", foo);
}

irq_handler()

{
	if (irq_can_be_shared) {
		check if the irq is for us. exit if not;
	}

	handle;
}

dsi.c:

dsi_init()

{
	request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, "dsi irq", foo);
}

irq_handler()

{
	if (irq_can_be_shared) {
		check if the irq is for us. exit if not;
	}

	handle;
}

Hi,

On Tuesday 15 February 2011 12:57 PM, Valkeinen, Tomi wrote:

<snip>

Quoted 32 lines

I meant something like this:

dispc.c:

dispc_init()

{
	/* did we have a pdev for dispc? if not, this needs to be dss.pdev */
	request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, "dispc irq", foo);
}

irq_handler()

{
	if (irq_can_be_shared) {
		check if the irq is for us. exit if not;
	}

	handle;
}

dsi.c:

dsi_init()

{
	request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, "dsi irq", foo);
}

irq_handler()

{
	if (irq_can_be_shared) {
		check if the irq is for us. exit if not;
	}

Quoted 3 lines

	handle;
}

This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers?

Quoted 42 lines · Archit

Hi,

On Tuesday 15 February 2011 12:57 PM, Valkeinen, Tomi wrote:

<snip>

Quoted 35 lines

I meant something like this:

dispc.c:

dispc_init()

{
	/* did we have a pdev for dispc? if not, this needs to be dss.pdev */
	request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, "dispc irq", foo);
}

irq_handler()

{
	if (irq_can_be_shared) {
		check if the irq is for us. exit if not;
	}

	handle;
}

dsi.c:

dsi_init()

{
	request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, "dsi irq", foo);
}

irq_handler()

{
	if (irq_can_be_shared) {
		check if the irq is for us. exit if not;
	}

	handle;
}

Quoted 2 lines

This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers?

That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean?

On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4.

Tomi

Quoted 38 lines · Archit

Hi,

On Tuesday 15 February 2011 12:57 PM, Valkeinen, Tomi wrote:

<snip>

Quoted 32 lines

I meant something like this:

dispc.c:

dispc_init()

{
	/* did we have a pdev for dispc? if not, this needs to be dss.pdev */
	request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, "dispc irq", foo);
}

irq_handler()

{
	if (irq_can_be_shared) {
		check if the irq is for us. exit if not;
	}

	handle;
}

dsi.c:

dsi_init()

{
	request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, "dsi irq", foo);
}

irq_handler()

{
	if (irq_can_be_shared) {
		check if the irq is for us. exit if not;
	}

Quoted 6 lines

Quoted 3 lines

	handle;
}

This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers?

That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean?

On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4.

Tomi

Copying Benoit,

Quoted 56 lines · Tomi

Quoted 38 lines · Archit

Hi,

On Tuesday 15 February 2011 12:57 PM, Valkeinen, Tomi wrote:

<snip>

Quoted 32 lines

I meant something like this:

dispc.c:

dispc_init()

{
	/* did we have a pdev for dispc? if not, this needs to be dss.pdev */
	request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, "dispc irq", foo);
}

irq_handler()

{
	if (irq_can_be_shared) {
		check if the irq is for us. exit if not;
	}

	handle;
}

dsi.c:

dsi_init()

{
	request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, "dsi irq", foo);
}

irq_handler()

{
	if (irq_can_be_shared) {
		check if the irq is for us. exit if not;
	}

Quoted 6 lines

Quoted 3 lines

	handle;
}

This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers?

That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean?

On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4.

Quoted 2 lines

Tomi

Benoit,

Is it okay to have the same irq entry for 2 different hwmods? This requirement comes from OMAP3 where dispc and dsi have a common irq line, where as on OMAP4 dispc and dsi have separate irq lines.

We basically want to get rid of a central dss irq handler (hence, remove irq entries for dss_core hwmod) and instead have separate irq handlers for each module which may or may not share an irq line.

Hi Archit,

Quoted 70 lines · Archit

Copying Benoit,

Quoted 55 lines · Tomi

Quoted 42 lines · Archit

Hi,

On Tuesday 15 February 2011 12:57 PM, Valkeinen, Tomi wrote:

<snip>

Quoted 35 lines

I meant something like this:

dispc.c:

dispc_init()
{
       /* did we have a pdev for dispc? if not, this needs to be dss.pdev */
       request_irq(platform_get_irq(dispc.pdev, 0), irq_handler, IRQF_SHARED, "dispc irq", foo);
}

irq_handler()
{
       if (irq_can_be_shared) {
               check if the irq is for us. exit if not; }

       handle;
}

dsi.c:

dsi_init()
{
       request_irq(platform_get_irq(dsi.pdev, 0), irq_handler, IRQF_SHARED, "dsi irq", foo);
}

irq_handler()
{
       if (irq_can_be_shared) {
               check if the irq is for us. exit if not; }

Quoted 6 lines

Quoted 3 lines

       handle;
}

This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers?

That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean?

Quoted 8 lines

On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4.

  Tomi

Benoit,

Quoted 3 lines

Is it okay to have the same irq entry for 2 different hwmods? This requirement comes from OMAP3 where dispc and dsi have a common irq line, where as on OMAP4 dispc and dsi have separate irq lines.

Well, no. I explained that in one of my comment about hwmod modification. The hwmod data are reflecting the exact HW capabilities.
So, if there is a change in the HW, the hwmod will be different. It is up to the driver to adapt to this change.

The driver has to evolve to handle properly the new HW capabilities while keeping the old stuff working.

Quoted 3 lines

We basically want to get rid of a central dss irq handler (hence, remove irq entries for dss_core hwmod) and instead have separate irq handlers for each module which may or may not share an irq line.

Then you need to hack your driver but not the hwmod data:-(

Regards,
Benoit

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Benoit,

Quoted 73 lines · Benoit

Hi Archit,

Quoted 70 lines · Archit

Copying Benoit,

Quoted 55 lines · Tomi

Quoted 42 lines · Archit

Hi,

On Tuesday 15 February 2011 12:57 PM, Valkeinen, Tomi wrote:

<snip>

Quoted 35 lines

I meant something like this:

dispc.c:

dispc_init()

{
        /* did we have a pdev for dispc? if not, this needs to be

dss.pdev */
request_irq(platform_get_irq(dispc.pdev, 0), irq_handler,
IRQF_SHARED, "dispc irq", foo);
}

irq_handler()

{
        if (irq_can_be_shared) {
                check if the irq is for us. exit if not;
        }

        handle;
}

dsi.c:

dsi_init()
{
request_irq(platform_get_irq(dsi.pdev, 0), irq_handler,
IRQF_SHARED, "dsi irq", foo);
}

irq_handler()

{
        if (irq_can_be_shared) {
                check if the irq is for us. exit if not;
        }

Quoted 6 lines

Quoted 3 lines

        handle;
}

This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers?

That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean?

Quoted 8 lines

On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4.

Tomi

Benoit,

Quoted 8 lines

Quoted 3 lines

Is it okay to have the same irq entry for 2 different hwmods? This requirement comes from OMAP3 where dispc and dsi have a common irq line, where as on OMAP4 dispc and dsi have separate irq lines.

Well, no. I explained that in one of my comment about hwmod modification. The hwmod data are reflecting the exact HW capabilities.
So, if there is a change in the HW, the hwmod will be different. It is up to the driver to adapt to this change.

I guess what Archit wanted to say is, for hw IPs DISPC and DSI, on OMAP3, have a common IRQ line, so could both their hwmod databases have the same IRQ added for them? This would us call, for a common IRQ line shared w/ DISPC and DSI, like
mentioned in Tomi's sample code above.

How is hwmod data for these cases handled today? [shared IRQ, different platform devices?]

Best regards,
~Sumit.

Quoted 18 lines

The driver has to evolve to handle properly the new HW capabilities while keeping the old stuff working.

Quoted 3 lines

We basically want to get rid of a central dss irq handler (hence, remove irq entries for dss_core hwmod) and instead have separate irq handlers for each module which may or may not share an irq line.

Then you need to hack your driver but not the hwmod data:-(

Regards,
Benoit

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 9 lines

Quoted 9 lines

Quoted 4 lines

Is it okay to have the same irq entry for 2 different hwmods? This requirement comes from OMAP3 where dispc and dsi have a common irq line, where as on OMAP4 dispc and dsi have separate irq lines.

Well, no. I explained that in one of my comment about hwmod modification. The hwmod data are reflecting the exact HW capabilities.
So, if there is a change in the HW, the hwmod will be different. It is up to the driver to adapt to this change.

Quoted 5 lines

I guess what Archit wanted to say is, for hw IPs DISPC and DSI, on OMAP3, have a common IRQ line, so could both their hwmod databases have the same IRQ added for them? This would us call, for a common IRQ line shared w/ DISPC and DSI, like
mentioned in Tomi's sample code above.

OK, thanks for the clarification, actually I missed a little bit the point :-(

So in fact the 2 modules share that same IRQ today, and you just want to populate both hwmod with the same input.
If this is a real OR between the two IRQ lines, meaning the dispc cannot mask the dsi IRQ or the opposite, then having the same IRQ number in the two different hwmods is a correct representation of the HW.

Hi,

<snip>

Quoted 15 lines

Quoted 9 lines

Quoted 9 lines

Quoted 4 lines

Is it okay to have the same irq entry for 2 different hwmods? This requirement comes from OMAP3 where dispc and dsi have a common irq line, where as on OMAP4 dispc and dsi have separate irq lines.

Well, no. I explained that in one of my comment about hwmod modification. The hwmod data are reflecting the exact HW capabilities.
So, if there is a change in the HW, the hwmod will be different. It is up to the driver to adapt to this change.

Quoted 5 lines

I guess what Archit wanted to say is, for hw IPs DISPC and DSI, on OMAP3, have a common IRQ line, so could both their hwmod databases have the same IRQ added for them? This would us call, for a common IRQ line shared w/ DISPC and DSI, like
mentioned in Tomi's sample code above.

Quoted 8 lines

OK, thanks for the clarification, actually I missed a little bit the point :-(

So in fact the 2 modules share that same IRQ today, and you just want to populate both hwmod with the same input.
If this is a real OR between the two IRQ lines, meaning the dispc cannot mask the dsi IRQ or the opposite, then having the same IRQ number in the two different hwmods is a correct representation of the HW.

There is a real OR between the 2 irq lines in OMAP3, as there is no DSS_IRQENABLE, but there is a DSS_IRQSTATUS.

You can mask one of DISPC or DSI by zeroing all the bits in DISPC_IRQENABLE or DSI_IRQENABLE respectively. But there is no higher level register to mask them.

Quoted 2 lines

This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers?

That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean?

On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4.

Hi,

Quoted 3 lines · Tomi Valkeinen

Quoted 2 lines

This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers?

Quoted 3 lines

That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean?

IMO, for omap3 it would be better to have irq_chip there. Then you can keep e.g. DISPC IRQ disabled until dispc.c calls request_irq(). What happens today if you have IRQ enabled but dispc isn't ready to act on those ?

Quoted 5 lines

On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4.

What if another HW requests the wrong IRQ number and it ends up being your dispc IRQ line ?

Quoted 8 lines · Tomi Valkeinen

Quoted 6 lines

Quoted 2 lines

This approach looks clean, but isn't IRQF_SHARED used the other way around. One irq line and multiple handlers?

That is the case here, isn't it (on omap3)? One interrupt line (the DSS irq, the same returned both from dsi.pdev and dispc.pdev), and two handlers, one in dispc and one in dsi? Or what do you mean?

Quoted 4 lines

IMO, for omap3 it would be better to have irq_chip there. Then you can keep e.g. DISPC IRQ disabled until dispc.c calls request_irq(). What happens today if you have IRQ enabled but dispc isn't ready to act on those ?

Currently we have a single interrupt handler, which then calls either dispc and/or dsi handler. Dispc and dsi are always ready to handle those.

I don't think it would be a good solution if irq_chip would be used only for omap3. Then we'd have totally different solutions for different omap versions. But if irq_chip can be easily used for all omap versions, then perhaps.

Quoted 8 lines

Quoted 5 lines

On omap2 there's no dsi code ran, so dispc is the only one requesting the irq, and thus IRQF_SHARED is extra. In omap4 there are separate irq lines (dsi.pdev and dispc.pdev return different irqs), and so IRQF_SHARED is again extra. But I don't see any harm in IRQF_SHARED even in omap2/4.

What if another HW requests the wrong IRQ number and it ends up being your dispc IRQ line ?

Are you asking what happens if we have a bug in kernel code? Anything can happen =). But I don't see that as a reason not to use IRQF_SHARED.

Quoted 5 lines

does it make sense to install an irq_chip for that ? I mean, can you mask/unmask dss and or dsi IRQs ? If you can, then it might make sense to take a look into GENIRQ and install an irq_chip for that. Then both dsi and dss would be able to use standard request_irq() API.

We could disable dsi IRQs by masking all the possible interrupt events in DSI_IRQSTATUS. The same goes for dispc. Is this what you meant by masking/unmasking irqs?

Hi,

Quoted 9 lines · archit taneja

Quoted 5 lines

does it make sense to install an irq_chip for that ? I mean, can you mask/unmask dss and or dsi IRQs ? If you can, then it might make sense to take a look into GENIRQ and install an irq_chip for that. Then both dsi and dss would be able to use standard request_irq() API.

We could disable dsi IRQs by masking all the possible interrupt events in DSI_IRQSTATUS. The same goes for dispc. Is this what you meant by masking/unmasking irqs?

yes it is. Then it makes sense to have an irq_chip for those two irqs, I think.

/proc/interrupt will reflect how the hardware works (DSI and DISPC IRQs being handled by DSS), both dsi and dispc can use normal request_irq() without setting IRQF_SHARED, etc etc. All you need to do is:

static struct irq_chip dss_irq_chip = {
	.name			= "DSS",
	.irq_bus_lock		= dss_bus_lock,
	.irq_bus_sync_unlock	= dss_bus_sync_unlock,
	.irq_mask		= dss_irq_mask,
	.irq_unmask		= dss_irq_unmask,
	.irq_ack		= dss_irq_ack,
};

then, somewhere during probe() you have to:

for (irq = irq_base; irq< irq_end; irq++) {
#ifdef CONFIG_ARM
set_irq_flags(irq, IRQF_VALID)

#else
	set_irq_noprobe(irq);

#endif

	set_irq_data(irq, dss_device_structure_pointer);
	set_irq_chip_and_handler(irq,&dss_irq_chip,
		handle_simple_irq);
}

and on exit() you just need to cleanup:

for (irq = irq_base; irq< irq_end; irq++) {

#ifdef CONFIG_ARM
	set_irq_flags(irq, 0)

#endif

	set_irq_chip_and_handler(irq, NULL, NULL);
	set_irq_data(irq, NULL);
}

Quoted 32 lines

yes it is. Then it makes sense to have an irq_chip for those two irqs, I think.

/proc/interrupt will reflect how the hardware works (DSI and DISPC IRQs being handled by DSS), both dsi and dispc can use normal request_irq() without setting IRQF_SHARED, etc etc. All you need to do is:

static struct irq_chip dss_irq_chip = {
	.name			= "DSS",
	.irq_bus_lock		= dss_bus_lock,
	.irq_bus_sync_unlock	= dss_bus_sync_unlock,
	.irq_mask		= dss_irq_mask,
	.irq_unmask		= dss_irq_unmask,
	.irq_ack		= dss_irq_ack,
};

then, somewhere during probe() you have to:

for (irq = irq_base; irq< irq_end; irq++) {
#ifdef CONFIG_ARM
set_irq_flags(irq, IRQF_VALID)

#else
	set_irq_noprobe(irq);

#endif

	set_irq_data(irq, dss_device_structure_pointer);
	set_irq_chip_and_handler(irq,&dss_irq_chip,
		handle_simple_irq);
}

and on exit() you just need to cleanup:

Quoted 9 lines

for (irq = irq_base; irq< irq_end; irq++) {

#ifdef CONFIG_ARM
	set_irq_flags(irq, 0)

#endif

	set_irq_chip_and_handler(irq, NULL, NULL);
	set_irq_data(irq, NULL);
}

Thanks for the info, but this seems to be suitable for the case when there are multiple irq events coming from the same interrupt line. On OMAP4 we have 4 different IRQ lines going to ARM, i.e 4 lines defined in irqs-44xx.h.

We are looking for a common irq handler located somewhere centrally, and each module can hook their irq line to this handler.

Does irq_chip do this? what does irq_base and irq_end signify?

Hi,

Quoted 18 lines · Tomi Valkeinen

Quoted 9 lines · archit taneja

OMAP2 has an irq line dedicated for DISPC interrupts, there is no DSI on omap2.
OMAP3 has a common irq line for DISPC and DSI interrupts. OMAP4 has seperate irq lines for DISPC and DSI Interrupts.

Use dss_features to have a common DSS irq handler for all OMAP revisions.

Also, use a member of the global dss structure to store the irq number as it is used in 2 functions.

It's good to remove the cpu_is_xxxx() calls, but I'm not quite sure about this patch...

Could we use shared interrupt handlers here, so that dss.c would handle only DISPC interrupts (or should it be even in dispc.c?) and dsi.c would handle DSI interrupts?

Quoted 7 lines

On OMAP3 both dss.c and dsi.c would register to the same interrupt, and they would need to check if the interrupt was really for them. On OMAP4 the code could be the same, even though the check is unnecessary.

Also, as I mentioned in the email I sent some minutes ago, this patch fixes the free_irq call in dss_exit. Things like that should never be fixed silently, even if it's trivial like in this case.

does it make sense to install an irq_chip for that ? I mean, can you mask/unmask dss and or dsi IRQs ? If you can, then it might make sense to take a look into GENIRQ and install an irq_chip for that. Then both dsi and dss would be able to use standard request_irq() API.

Take a look at drivers/mfd/twl*irq.c and drivers/cbus/retu.c (the last one on linux-omap only) there are examples of how that should be implemented. Actually twl*irq.c is wrong, as it bypasses the threaded IRQ stuff. I have some patches for those, but I'm not sure they are working correctly yet, needs more testing. My twl4030-irq patches are available at [1] for reference.

I'm not familiar with genirq/irq_chip. But yes, as Archit said, we can mask/unmask DSS interrupts. I mean, there's only one interrupt line, but DSS has irqstatus and irqenable registers that can be used.

If I understood correctly, irq_chip could be used to manage DSS and DSI interrupts, but I don't know right away what that would mean in practice, and would it make things easier or not. But this could be something that needs more study, as there's a custom isr handling system in DSS, and it would be nice if that could be replaced with genirq.

But I guess the main problem still remains with genirqs also: we have one interrupt line on omap3, used by two separate modules. And one irq on omap2, used by one module, and two irqs on omap4, used by two modules.

Tomi