Mailing List Archive: Linux: Kernel

[PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically

Index | Next | Previous | View Threaded

peter.ujfalusi at ti

May 3, 2012, 5:54 AM

Post #1 of 16 (113 views)
Permalink

[PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically

Use irq_alloc_descs() to get the IRQ number range dynamically instead of
the hardwired use if pdata->irq_base.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi [at] ti>
---
drivers/mfd/twl6040-core.c | 3 +--
drivers/mfd/twl6040-irq.c | 13 +++++++++++--
2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/twl6040-core.c b/drivers/mfd/twl6040-core.c
index 7a92d95..c50fba7 100644
--- a/drivers/mfd/twl6040-core.c
+++ b/drivers/mfd/twl6040-core.c
@@ -515,7 +515,7 @@ static int __devinit twl6040_probe(struct i2c_client *client,
}

/* In order to operate correctly we need valid interrupt config */
- if (!client->irq || !pdata->irq_base) {
+ if (!client->irq) {
dev_err(&client->dev, "Invalid IRQ configuration\n");
return -EINVAL;
}
@@ -552,7 +552,6 @@ static int __devinit twl6040_probe(struct i2c_client *client,

twl6040->dev = &client->dev;
twl6040->irq = client->irq;
- twl6040->irq_base = pdata->irq_base;

mutex_init(&twl6040->mutex);
mutex_init(&twl6040->io_mutex);
diff --git a/drivers/mfd/twl6040-irq.c b/drivers/mfd/twl6040-irq.c
index 008022c..914978e 100644
--- a/drivers/mfd/twl6040-irq.c
+++ b/drivers/mfd/twl6040-irq.c
@@ -23,6 +23,7 @@

#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/err.h>
#include <linux/irq.h>
#include <linux/interrupt.h>
#include <linux/mfd/core.h>
@@ -138,7 +139,7 @@ static irqreturn_t twl6040_irq_thread(int irq, void *data)

int twl6040_irq_init(struct twl6040 *twl6040)
{
- int i, nr_irqs, ret;
+ int i, nr_irqs, irq_base, ret;
u8 val;

mutex_init(&twl6040->irq_mutex);
@@ -149,8 +150,16 @@ int twl6040_irq_init(struct twl6040 *twl6040)
twl6040_reg_write(twl6040, TWL6040_REG_INTMR, TWL6040_ALLINT_MSK);

nr_irqs = ARRAY_SIZE(twl6040_irqs);
+
+ irq_base = irq_alloc_descs(-1, 0, nr_irqs, 0);
+ if (IS_ERR_VALUE(irq_base)) {
+ dev_err(twl6040->dev, "Fail to allocate IRQ descs\n");
+ return irq_base;
+ }
+ twl6040->irq_base = irq_base;
+
/* Register them with genirq */
- for (i = twl6040->irq_base; i < twl6040->irq_base + nr_irqs; i++) {
+ for (i = irq_base; i < irq_base + nr_irqs; i++) {
irq_set_chip_data(i, twl6040);
irq_set_chip_and_handler(i, &twl6040_irq_chip,
handle_level_irq);
--
1.7.8.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

broonie at opensource

May 3, 2012, 6:20 AM

Post #2 of 16 (106 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On Thu, May 03, 2012 at 03:54:24PM +0300, Peter Ujfalusi wrote:

> /* In order to operate correctly we need valid interrupt config */
> - if (!client->irq || !pdata->irq_base) {
> + if (!client->irq) {

It looks like you're totally removing the use of irq_base which will
break any boards that didn't convert to DT. The usual idiom is to use
irq_base as the base for the range of requested IRQs if it's supplied,
otherwise set it to -1 to allow dynamic allocation. This should keep
existing users working without disruption.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

peter.ujfalusi at ti

May 3, 2012, 6:28 AM

Post #3 of 16 (102 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On 05/03/2012 04:20 PM, Mark Brown wrote:
> On Thu, May 03, 2012 at 03:54:24PM +0300, Peter Ujfalusi wrote:
>
>> /* In order to operate correctly we need valid interrupt config */
>> - if (!client->irq || !pdata->irq_base) {
>> + if (!client->irq) {
>
> It looks like you're totally removing the use of irq_base which will
> break any boards that didn't convert to DT. The usual idiom is to use
> irq_base as the base for the range of requested IRQs if it's supplied,
> otherwise set it to -1 to allow dynamic allocation. This should keep
> existing users working without disruption.

In the current board files the pdata->irq_base set to some #defined
value (which is a comfortably big number).
With irq_alloc_descs(-1, 0, nr_irqs, 0); the range which can accommodate
the number of IRQs twl6040 will be provided so there is no need to me to
use the board specified irq_base.
The irq_base will be removed from the pdata structure, but that change
will go via linux-omap to avoid compile breakage.
This part is in preparation for DT, yes, but it is working without DT.
The twl6040 irq range is mapped to different numbers, that's all.

--
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

broonie at opensource

May 3, 2012, 7:52 AM

Post #4 of 16 (108 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On Thu, May 03, 2012 at 04:28:59PM +0300, Peter Ujfalusi wrote:
> On 05/03/2012 04:20 PM, Mark Brown wrote:

> > It looks like you're totally removing the use of irq_base which will
> > break any boards that didn't convert to DT. The usual idiom is to use

> In the current board files the pdata->irq_base set to some #defined
> value (which is a comfortably big number).

Are you sure there aren't any boards out there which rely on the
interrupt base (eg, using a GPIO with the IRQ output of a chip)?

Attachments:

signature.asc (0.82 KB)

peter.ujfalusi at ti

May 3, 2012, 8:13 AM

Post #5 of 16 (100 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On 05/03/2012 05:52 PM, Mark Brown wrote:
> Are you sure there aren't any boards out there which rely on the
> interrupt base (eg, using a GPIO with the IRQ output of a chip)?

Yes, I'm sure.

--
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

broonie at opensource

May 3, 2012, 8:26 AM

Post #6 of 16 (102 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On Thu, May 03, 2012 at 06:13:16PM +0300, Peter Ujfalusi wrote:
> On 05/03/2012 05:52 PM, Mark Brown wrote:
> > Are you sure there aren't any boards out there which rely on the
> > interrupt base (eg, using a GPIO with the IRQ output of a chip)?

> Yes, I'm sure.

Because...?

Attachments:

signature.asc (0.82 KB)

peter.ujfalusi at ti

May 4, 2012, 1:38 AM

Post #7 of 16 (93 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On 05/03/2012 06:26 PM, Mark Brown wrote:
> On Thu, May 03, 2012 at 06:13:16PM +0300, Peter Ujfalusi wrote:
>> On 05/03/2012 05:52 PM, Mark Brown wrote:
>>> Are you sure there aren't any boards out there which rely on the
>>> interrupt base (eg, using a GPIO with the IRQ output of a chip)?
>
>> Yes, I'm sure.
>
> Because...?

The irq_base was used to map the nested interrupt numbers somewhere high
enough. twl6040 has one irq line towards the CPU (comes via
i2c_client->irq).
With this change we just change the mapping of the nested interrupt
range provided by twl6040 (instead of hardwired number we ask for
suitable range).
We have sdp4430 and PandaBoard with twl6040. They are fine. The other
non upstream boards should be fine as well with this change.

--
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

broonie at opensource

May 4, 2012, 2:08 AM

Post #8 of 16 (93 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On Fri, May 04, 2012 at 11:38:34AM +0300, Peter Ujfalusi wrote:

> The irq_base was used to map the nested interrupt numbers somewhere high
> enough. twl6040 has one irq line towards the CPU (comes via
> i2c_client->irq).
> With this change we just change the mapping of the nested interrupt
> range provided by twl6040 (instead of hardwired number we ask for
> suitable range).

You're not understanding the issue at all - the issue is that if
some driver outside the twl6040 driver is using an interrupt in that
range based off the irq_base that they supplied then you'll break them.
The most common case here is using GPIOs on the device as interrupts.

If this is safe you should at least be making it clear why...

> We have sdp4430 and PandaBoard with twl6040. They are fine. The other
> non upstream boards should be fine as well with this change.

...ideally in a more concrete way than just saying it works on your
reference boards.

Attachments:

signature.asc (0.82 KB)

peter.ujfalusi at ti

May 4, 2012, 3:37 AM

Post #9 of 16 (93 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On 05/04/2012 12:08 PM, Mark Brown wrote:
> You're not understanding the issue at all - the issue is that if
> some driver outside the twl6040 driver is using an interrupt in that
> range based off the irq_base that they supplied then you'll break them.
> The most common case here is using GPIOs on the device as interrupts.

The OMAP platform related drives has been already converted to use
irq_alloc_descs(-1, 0, nr_irqs, 0); to map their range (including GPIO,
twl6030, etc).
Neither of these are using or have irq_base passed from board files for
some time now. We still have the defines for the legacy define based
mapping of the ranges, but it is no longer in use.
To be fair: in arch/arm/mach-omap2/ there's one driver which has not
been converted. It is the gpmc (Flash).

> If this is safe you should at least be making it clear why...

I'll add short explanation to the commit message.

--
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

broonie at opensource

May 4, 2012, 4:22 AM

Post #10 of 16 (90 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On Fri, May 04, 2012 at 01:37:54PM +0300, Peter Ujfalusi wrote:
> On 05/04/2012 12:08 PM, Mark Brown wrote:

> > You're not understanding the issue at all - the issue is that if
> > some driver outside the twl6040 driver is using an interrupt in that
> > range based off the irq_base that they supplied then you'll break them.
> > The most common case here is using GPIOs on the device as interrupts.

> The OMAP platform related drives has been already converted to use
> irq_alloc_descs(-1, 0, nr_irqs, 0); to map their range (including GPIO,
> twl6030, etc).

How does this work for interrupts on things like SPI and I2C devices?

Attachments:

signature.asc (0.82 KB)

peter.ujfalusi at ti

May 4, 2012, 4:55 AM

Post #11 of 16 (92 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On 05/04/2012 02:22 PM, Mark Brown wrote:
>> The OMAP platform related drives has been already converted to use
>> irq_alloc_descs(-1, 0, nr_irqs, 0); to map their range (including GPIO,
>> twl6030, etc).
>
> How does this work for interrupts on things like SPI and I2C devices?

You mean on devices like twl6040, twl6030, twl4030 (I2C MFD devices)?
Or "irq expander" type of devices?

For the later I'm not sure.

--
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

broonie at opensource

May 4, 2012, 5:17 AM

Post #12 of 16 (90 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On Fri, May 04, 2012 at 02:55:37PM +0300, Peter Ujfalusi wrote:
> On 05/04/2012 02:22 PM, Mark Brown wrote:
> >> The OMAP platform related drives has been already converted to use
> >> irq_alloc_descs(-1, 0, nr_irqs, 0); to map their range (including GPIO,
> >> twl6030, etc).

> > How does this work for interrupts on things like SPI and I2C devices?

> You mean on devices like twl6040, twl6030, twl4030 (I2C MFD devices)?
> Or "irq expander" type of devices?

The latter, and also just any driver that delivers an interrupt via a
GPIO on OMAP - if the GPIO IRQ numbers are all dynamically allocated
then it gets hard to register an off-chip device and tell it which
interrupt to request.

Attachments:

signature.asc (0.82 KB)

peter.ujfalusi at ti

May 4, 2012, 5:33 AM

Post #13 of 16 (94 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On 05/04/2012 03:17 PM, Mark Brown wrote:
>> You mean on devices like twl6040, twl6030, twl4030 (I2C MFD devices)?
>> Or "irq expander" type of devices?
>
> The latter, and also just any driver that delivers an interrupt via a
> GPIO on OMAP - if the GPIO IRQ numbers are all dynamically allocated
> then it gets hard to register an off-chip device and tell it which
> interrupt to request.

For GPIO IRQ there's the gpio_to_irq() call which returns the mapped IRQ
number for the given GPIO.
If there is "irq expander" type of chip I assume it would have similar
way to get the IRQ number based on either GPIO number or some other
enumeration value.
The twl6040 does not have such a feature. The interrupts are generated
internally and it has one IRQ line towards the host. We have nested
interrupts for the childs (plug detect is handled by ASoC codec, Vibra
overcurrent is handled by the vibra driver, etc).
Client drivers got their interrupts via platform_get_irq(); they does
not need to know anything about irq_base, or where the IRQ number has
been mapped.

--
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

broonie at opensource

May 4, 2012, 5:47 AM

Post #14 of 16 (92 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On Fri, May 04, 2012 at 03:33:51PM +0300, Peter Ujfalusi wrote:
> On 05/04/2012 03:17 PM, Mark Brown wrote:

> > The latter, and also just any driver that delivers an interrupt via a
> > GPIO on OMAP - if the GPIO IRQ numbers are all dynamically allocated
> > then it gets hard to register an off-chip device and tell it which
> > interrupt to request.

> For GPIO IRQ there's the gpio_to_irq() call which returns the mapped IRQ
> number for the given GPIO.

This doesn't really work for board files, though - you can only call
gpio_to_irq() at runtime so if you're statically registering the devices
on the board you'd need to make the structures non-const, manage to find
the GPIO controller at runtime then do the lookup, update the struct and
finally register the device.

> If there is "irq expander" type of chip I assume it would have similar
> way to get the IRQ number based on either GPIO number or some other
> enumeration value.

No, there's no generic interface for this.

> The twl6040 does not have such a feature. The interrupts are generated
> internally and it has one IRQ line towards the host. We have nested
> interrupts for the childs (plug detect is handled by ASoC codec, Vibra
> overcurrent is handled by the vibra driver, etc).

OK, that's fine then - you should put this in the changelog to make it
clear that there are no external users who could be affected.

Attachments:

signature.asc (0.82 KB)

peter.ujfalusi at ti

May 6, 2012, 11:49 PM

Post #15 of 16 (83 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On 05/04/2012 03:47 PM, Mark Brown wrote:
> This doesn't really work for board files, though - you can only call
> gpio_to_irq() at runtime so if you're statically registering the devices
> on the board you'd need to make the structures non-const, manage to find
> the GPIO controller at runtime then do the lookup, update the struct and
> finally register the device.

I've seen some board files doing this already (obviously not for the
twl6040). Which is not the nicest thing...

>> If there is "irq expander" type of chip I assume it would have similar
>> way to get the IRQ number based on either GPIO number or some other
>> enumeration value.
>
> No, there's no generic interface for this.

This dynamic range allocation for IRQ, GPIO will work nicely with DT at
the end. In legacy way we will need to have workarounds. Or just forget
about booting without DT.

>> The twl6040 does not have such a feature. The interrupts are generated
>> internally and it has one IRQ line towards the host. We have nested
>> interrupts for the childs (plug detect is handled by ASoC codec, Vibra
>> overcurrent is handled by the vibra driver, etc).
>
> OK, that's fine then - you should put this in the changelog to make it
> clear that there are no external users who could be affected.

I'll do that.

Thank you,
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

broonie at opensource

May 7, 2012, 2:49 AM

Post #16 of 16 (85 views)
Permalink

Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically [In reply to]

On Mon, May 07, 2012 at 09:49:55AM +0300, Peter Ujfalusi wrote:
> On 05/04/2012 03:47 PM, Mark Brown wrote:

> >> If there is "irq expander" type of chip I assume it would have similar
> >> way to get the IRQ number based on either GPIO number or some other
> >> enumeration value.

> > No, there's no generic interface for this.

> This dynamic range allocation for IRQ, GPIO will work nicely with DT at
> the end. In legacy way we will need to have workarounds. Or just forget
> about booting without DT.

Device tree is only available for a small minority of architectures.
Just as you shouldn't assume that people only ever use the reference
boards you work on so you should also not assume that people are using
ARM (and ARM systems that support DT at at that).

Attachments:

signature.asc (0.82 KB)

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads