On Fri, Jan 27, 2012 at 02:35:54PM -0700, Grant Likely wrote:
> Hey everyone,
>
> This patch series is ready for much wider consumption now. I'd like
> to get it into linux-next ASAP because there will be ARM board support
> depending on it. I'll wait a few days before I ask Stephen to pull
> this in.
Grant,
Can you answer me this: does this irqdomain support require DT?
The question comes up because OMAP has converted some of their support
to require irq domain support for their PMICs, and it seems irq domain
support requires DT. This seems to have made the whole of OMAP
essentially become a DT-only platform.
Removing the dependency on IRQ_DOMAIN brings up these build errors
in the twl-core code (that being the PMIC for OMAP CPUs):
drivers/mfd/twl-core.c: In function 'twl_probe':
drivers/mfd/twl-core.c:1229: error: invalid use of undefined type 'struct irq_domain'
drivers/mfd/twl-core.c:1230: error: invalid use of undefined type 'struct irq_domain'
drivers/mfd/twl-core.c:1235: error: implicit declaration of function 'irq_domain_add'
That's a bit of a problem, because afaik there aren't the DT descriptions
for the boards I have yet, so it's causing me to see regressions when
building and booting kernels with CONFIG_OF=n.
The more core-code we end up with which requires DT, the worse this
problem is going to become - and obviously saying "everyone must now
convert to DT" is, even today, a mammoth task.
Now, here's the thing: I believe that IRQ domains - at least as far as
the hwirq stuff - should be available irrespective of whether we have
the rest of the IRQ domain support code in place, so that IRQ support
code doesn't have to keep playing games to decode from the global
space to the per-controller number space.
I believe that would certainly help the current OMAP problems, where
the current lack of CONFIG_IRQ_DOMAIN basically makes the kernel oops
on boot.
How we fix this regression for 3.4 I've no idea at present, I'm trying
to work out what the real dependencies are for OMAP on this stuff.
Finally, do we need asm/irq.h in our asm/prom.h ? That's causing
fragility between DT and non-DT builds, because people are finding
that their DT builds work without their mach/irqs.h includes but
fail when built with non-DT. The only thing which DT might need -
at the most - is NR_IRQS, but I'd hope with things like irq domains
it doesn't actually require it.
On Sat, Feb 04, 2012 at 10:17:48PM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 27, 2012 at 02:35:54PM -0700, Grant Likely wrote:
> > Hey everyone,
> >
> > This patch series is ready for much wider consumption now. I'd like
> > to get it into linux-next ASAP because there will be ARM board support
> > depending on it. I'll wait a few days before I ask Stephen to pull
> > this in.
>
> Grant,
>
> Can you answer me this: does this irqdomain support require DT?
>
> The question comes up because OMAP has converted some of their support
> to require irq domain support for their PMICs, and it seems irq domain
> support requires DT. This seems to have made the whole of OMAP
> essentially become a DT-only platform.
>
> Removing the dependency on IRQ_DOMAIN brings up these build errors
> in the twl-core code (that being the PMIC for OMAP CPUs):
>
> drivers/mfd/twl-core.c: In function 'twl_probe':
> drivers/mfd/twl-core.c:1229: error: invalid use of undefined type 'struct irq_domain'
> drivers/mfd/twl-core.c:1230: error: invalid use of undefined type 'struct irq_domain'
> drivers/mfd/twl-core.c:1235: error: implicit declaration of function 'irq_domain_add'
>
> That's a bit of a problem, because afaik there aren't the DT descriptions
> for the boards I have yet, so it's causing me to see regressions when
> building and booting kernels with CONFIG_OF=n.
>
> The more core-code we end up with which requires DT, the worse this
> problem is going to become - and obviously saying "everyone must now
> convert to DT" is, even today, a mammoth task.
>
> Now, here's the thing: I believe that IRQ domains - at least as far as
> the hwirq stuff - should be available irrespective of whether we have
> the rest of the IRQ domain support code in place, so that IRQ support
> code doesn't have to keep playing games to decode from the global
> space to the per-controller number space.
>
> I believe that would certainly help the current OMAP problems, where
> the current lack of CONFIG_IRQ_DOMAIN basically makes the kernel oops
> on boot.
>
> How we fix this regression for 3.4 I've no idea at present, I'm trying
> to work out what the real dependencies are for OMAP on this stuff.
Actually, it turns out to be not that hard, because twl doesn't actually
make use of the IRQ domain stuff:
commit aeb5032b3f8b9ab69daa545777433fa94b3494c4
Author: Benoit Cousson <[email protected]>
AuthorDate: Mon Aug 29 16:20:23 2011 +0200
Commit: Samuel Ortiz <[email protected]>
CommitDate: Mon Jan 9 00:37:40 2012 +0100
mfd: twl-core: Add initial DT support for twl4030/twl6030
[[email protected]: Fix IRQ_DOMAIN dependency in kconfig]
Adding any dependency - especially one which wouldn't be enabled - for
a new feature which wasn't required before is going to break existing
users, so this shouldn't have been done in the first place.
A better fix to preserve existing users would've been as below - yes
it means more ifdefs, but if irq domain is to remain a DT only thing
then we're going to end up with _lots_ of this stuff.
I'd much prefer to see irq domain become more widely available so it
doesn't require these ifdefs everywhere.
drivers/mfd/Kconfig | 2 +-
drivers/mfd/twl-core.c | 4 ++++
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 28a301b..bd60ce0 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -200,7 +200,7 @@ config MENELAUS
config TWL4030_CORE
bool "Texas Instruments TWL4030/TWL5030/TWL6030/TPS659x0 Support"
- depends on I2C=y && GENERIC_HARDIRQS && IRQ_DOMAIN
+ depends on I2C=y && GENERIC_HARDIRQS
help
Say yes here if you have TWL4030 / TWL6030 family chip on your board.
This core driver provides register access and IRQ handling
diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index e04e04d..5913aaa 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -263,7 +263,9 @@ struct twl_client {
static struct twl_client twl_modules[TWL_NUM_SLAVES];
+#ifdef CONFIG_IRQ_DOMAIN
static struct irq_domain domain;
+#endif
/* mapping the module id to slave id and base address */
struct twl_mapping {
@@ -1226,6 +1228,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
pdata->irq_base = status;
pdata->irq_end = pdata->irq_base + nr_irqs;
+#ifdef CONFIG_IRQ_DOMAIN
domain.irq_base = pdata->irq_base;
domain.nr_irq = nr_irqs;
#ifdef CONFIG_OF_IRQ
@@ -1233,6 +1236,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
domain.ops = &irq_domain_simple_ops;
#endif
irq_domain_add(&domain);
+#endif
if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C) == 0) {
dev_dbg(&client->dev, "can't talk I2C?\n");
On Sat, 2012-02-04 at 22:17 +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 27, 2012 at 02:35:54PM -0700, Grant Likely wrote:
> > Hey everyone,
> >
> > This patch series is ready for much wider consumption now. I'd like
> > to get it into linux-next ASAP because there will be ARM board support
> > depending on it. I'll wait a few days before I ask Stephen to pull
> > this in.
>
> Grant,
>
> Can you answer me this: does this irqdomain support require DT?
The original powerpc code this is based on didn't require it. It was an
explicit design decision and I remember insisting that Grant follows it,
but I haven't yet reviewed his last batch.
DT is orthogonal. You have "helpers" that use the DT to resolve the
domain of an interrupt source and do the mapping for you, but I made
sure that you call always still create domains and map interrupts using
explicit domain pointers & hw numbers.
(And I need them to deal with ancient broken device-tree's on some
platforms such as oldworld PowerMacs).
> The question comes up because OMAP has converted some of their support
> to require irq domain support for their PMICs, and it seems irq domain
> support requires DT. This seems to have made the whole of OMAP
> essentially become a DT-only platform.
.../...
> Now, here's the thing: I believe that IRQ domains - at least as far as
> the hwirq stuff - should be available irrespective of whether we have
> the rest of the IRQ domain support code in place, so that IRQ support
> code doesn't have to keep playing games to decode from the global
> space to the per-controller number space.
>
> I believe that would certainly help the current OMAP problems, where
> the current lack of CONFIG_IRQ_DOMAIN basically makes the kernel oops
> on boot.
>
> How we fix this regression for 3.4 I've no idea at present, I'm trying
> to work out what the real dependencies are for OMAP on this stuff.
>
> Finally, do we need asm/irq.h in our asm/prom.h ? That's causing
> fragility between DT and non-DT builds, because people are finding
> that their DT builds work without their mach/irqs.h includes but
> fail when built with non-DT. The only thing which DT might need -
> at the most - is NR_IRQS, but I'd hope with things like irq domains
> it doesn't actually require it.
Cheers,
Ben.
* Russell King - ARM Linux <[email protected]> [120204 14:00]:
>
> Actually, it turns out to be not that hard, because twl doesn't actually
> make use of the IRQ domain stuff:
>
> commit aeb5032b3f8b9ab69daa545777433fa94b3494c4
> Author: Benoit Cousson <[email protected]>
> AuthorDate: Mon Aug 29 16:20:23 2011 +0200
> Commit: Samuel Ortiz <[email protected]>
> CommitDate: Mon Jan 9 00:37:40 2012 +0100
>
> mfd: twl-core: Add initial DT support for twl4030/twl6030
>
> [[email protected]: Fix IRQ_DOMAIN dependency in kconfig]
>
> Adding any dependency - especially one which wouldn't be enabled - for
> a new feature which wasn't required before is going to break existing
> users, so this shouldn't have been done in the first place.
>
> A better fix to preserve existing users would've been as below - yes
> it means more ifdefs, but if irq domain is to remain a DT only thing
> then we're going to end up with _lots_ of this stuff.
>
> I'd much prefer to see irq domain become more widely available so it
> doesn't require these ifdefs everywhere.
Your patch below looks like a correct fix to me to the problem
you and Grazvydas are seeing:
Acked-by: Tony Lindgren <[email protected]>
> drivers/mfd/Kconfig | 2 +-
> drivers/mfd/twl-core.c | 4 ++++
> 2 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 28a301b..bd60ce0 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -200,7 +200,7 @@ config MENELAUS
>
> config TWL4030_CORE
> bool "Texas Instruments TWL4030/TWL5030/TWL6030/TPS659x0 Support"
> - depends on I2C=y && GENERIC_HARDIRQS && IRQ_DOMAIN
> + depends on I2C=y && GENERIC_HARDIRQS
> help
> Say yes here if you have TWL4030 / TWL6030 family chip on your board.
> This core driver provides register access and IRQ handling
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index e04e04d..5913aaa 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -263,7 +263,9 @@ struct twl_client {
>
> static struct twl_client twl_modules[TWL_NUM_SLAVES];
>
> +#ifdef CONFIG_IRQ_DOMAIN
> static struct irq_domain domain;
> +#endif
>
> /* mapping the module id to slave id and base address */
> struct twl_mapping {
> @@ -1226,6 +1228,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
> pdata->irq_base = status;
> pdata->irq_end = pdata->irq_base + nr_irqs;
>
> +#ifdef CONFIG_IRQ_DOMAIN
> domain.irq_base = pdata->irq_base;
> domain.nr_irq = nr_irqs;
> #ifdef CONFIG_OF_IRQ
> @@ -1233,6 +1236,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
> domain.ops = &irq_domain_simple_ops;
> #endif
> irq_domain_add(&domain);
> +#endif
>
> if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C) == 0) {
> dev_dbg(&client->dev, "can't talk I2C?\n");
> _______________________________________________
> devicetree-discuss mailing list
> [email protected]
> https://lists.ozlabs.org/listinfo/devicetree-discuss
On Sat, Feb 04, 2012 at 05:38:53PM -0800, Tony Lindgren wrote:
> * Russell King - ARM Linux <[email protected]> [120204 14:00]:
> >
> > Actually, it turns out to be not that hard, because twl doesn't actually
> > make use of the IRQ domain stuff:
> >
> > commit aeb5032b3f8b9ab69daa545777433fa94b3494c4
> > Author: Benoit Cousson <[email protected]>
> > AuthorDate: Mon Aug 29 16:20:23 2011 +0200
> > Commit: Samuel Ortiz <[email protected]>
> > CommitDate: Mon Jan 9 00:37:40 2012 +0100
> >
> > mfd: twl-core: Add initial DT support for twl4030/twl6030
> >
> > [[email protected]: Fix IRQ_DOMAIN dependency in kconfig]
> >
> > Adding any dependency - especially one which wouldn't be enabled - for
> > a new feature which wasn't required before is going to break existing
> > users, so this shouldn't have been done in the first place.
> >
> > A better fix to preserve existing users would've been as below - yes
> > it means more ifdefs, but if irq domain is to remain a DT only thing
> > then we're going to end up with _lots_ of this stuff.
> >
> > I'd much prefer to see irq domain become more widely available so it
> > doesn't require these ifdefs everywhere.
>
> Your patch below looks like a correct fix to me to the problem
> you and Grazvydas are seeing:
>
> Acked-by: Tony Lindgren <[email protected]>
It's not quite correct, because OMAP4 has issues in this area as well
(which does select IRQ_DOMAIN but can be without OF.) The result is
an oops from irq_domain_add() because domain->ops is NULL.
The right solution is three fold:
1. Wrap the bits of code in CONFIG_IRQ_DOMAIN
2. Get rid of the #ifdef CONFIG_OF there, so the 'ops' member can be
initialized.
3. Fix the OMAP vp code not to oops when voltdm->pmic is NULL
which I have in my combined patch for fixing OMAP so far.
Russell,
On 02/04/2012 04:17 PM, Russell King - ARM Linux wrote:
> On Fri, Jan 27, 2012 at 02:35:54PM -0700, Grant Likely wrote:
>> Hey everyone,
>>
>> This patch series is ready for much wider consumption now. I'd like
>> to get it into linux-next ASAP because there will be ARM board support
>> depending on it. I'll wait a few days before I ask Stephen to pull
>> this in.
>
> Grant,
>
> Can you answer me this: does this irqdomain support require DT?
>
No. It's the other way around, DT requires irqdomain. The GIC and VIC
code for example can be built with or w/o DT enabled, but both select
IRQ_DOMAIN.
FYI, I just submitted a patch that selects IRQ_DOMAIN for all of ARM:
http://www.gossamer-threads.com/lists/linux/kernel/1487231?page=last
Either we do that or we select IRQ_DOMAIN one irq_chip at a time. With
the "new" irq_domain code, we could probably do better to shrink the
code needed in the non-DT case.
> The question comes up because OMAP has converted some of their support
> to require irq domain support for their PMICs, and it seems irq domain
> support requires DT. This seems to have made the whole of OMAP
> essentially become a DT-only platform.
I think we should select DT or not at the sub-arch level. Trying to
support both builds is a needless headache.
>
> Removing the dependency on IRQ_DOMAIN brings up these build errors
> in the twl-core code (that being the PMIC for OMAP CPUs):
>
> drivers/mfd/twl-core.c: In function 'twl_probe':
> drivers/mfd/twl-core.c:1229: error: invalid use of undefined type 'struct irq_domain'
> drivers/mfd/twl-core.c:1230: error: invalid use of undefined type 'struct irq_domain'
> drivers/mfd/twl-core.c:1235: error: implicit declaration of function 'irq_domain_add'
>
> That's a bit of a problem, because afaik there aren't the DT descriptions
> for the boards I have yet, so it's causing me to see regressions when
> building and booting kernels with CONFIG_OF=n.
>
> The more core-code we end up with which requires DT, the worse this
> problem is going to become - and obviously saying "everyone must now
> convert to DT" is, even today, a mammoth task.
>
> Now, here's the thing: I believe that IRQ domains - at least as far as
> the hwirq stuff - should be available irrespective of whether we have
> the rest of the IRQ domain support code in place, so that IRQ support
> code doesn't have to keep playing games to decode from the global
> space to the per-controller number space.
>
> I believe that would certainly help the current OMAP problems, where
> the current lack of CONFIG_IRQ_DOMAIN basically makes the kernel oops
> on boot.
>
> How we fix this regression for 3.4 I've no idea at present, I'm trying
> to work out what the real dependencies are for OMAP on this stuff.
>
> Finally, do we need asm/irq.h in our asm/prom.h ? That's causing
> fragility between DT and non-DT builds, because people are finding
> that their DT builds work without their mach/irqs.h includes but
> fail when built with non-DT. The only thing which DT might need -
> at the most - is NR_IRQS, but I'd hope with things like irq domains
> it doesn't actually require it.
Doesn't look like it is needed.
Rob
> _______________________________________________
> devicetree-discuss mailing list
> [email protected]
> https://lists.ozlabs.org/listinfo/devicetree-discuss
On Sat, Feb 04, 2012 at 10:17:48PM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 27, 2012 at 02:35:54PM -0700, Grant Likely wrote:
> > Hey everyone,
> >
> > This patch series is ready for much wider consumption now. I'd like
> > to get it into linux-next ASAP because there will be ARM board support
> > depending on it. I'll wait a few days before I ask Stephen to pull
> > this in.
>
> Grant,
>
> Can you answer me this: does this irqdomain support require DT?
No, it should not. Any situation where it does is a bug.
> Now, here's the thing: I believe that IRQ domains - at least as far as
> the hwirq stuff - should be available irrespective of whether we have
> the rest of the IRQ domain support code in place, so that IRQ support
> code doesn't have to keep playing games to decode from the global
> space to the per-controller number space.
Correct. That's the intent. My new series flushes out irq_domain quite a
bit better and gets all architectures doing the same thing if they use
irq_domains. I've done some testing on both CONFIG_OF and !CONFIG_OF builds,
but I'm going to do some more to make sure I've not missed anything.
> I believe that would certainly help the current OMAP problems, where
> the current lack of CONFIG_IRQ_DOMAIN basically makes the kernel oops
> on boot.
>
> How we fix this regression for 3.4 I've no idea at present, I'm trying
> to work out what the real dependencies are for OMAP on this stuff.
>
> Finally, do we need asm/irq.h in our asm/prom.h ? That's causing
> fragility between DT and non-DT builds, because people are finding
> that their DT builds work without their mach/irqs.h includes but
> fail when built with non-DT. The only thing which DT might need -
> at the most - is NR_IRQS, but I'd hope with things like irq domains
> it doesn't actually require it.
I don't think so. There may be a file or two that break because they're
not including everything they need, but I don't think anything in the
header requires it.
The irq_domain code is well isolated. The header file doesn't need to
be including it.
g.
On Sun, Feb 05, 2012 at 04:13:48PM +0000, Russell King - ARM Linux wrote:
> It's not quite correct, because OMAP4 has issues in this area as well
> (which does select IRQ_DOMAIN but can be without OF.) The result is
> an oops from irq_domain_add() because domain->ops is NULL.
> The right solution is three fold:
> 1. Wrap the bits of code in CONFIG_IRQ_DOMAIN
> 2. Get rid of the #ifdef CONFIG_OF there, so the 'ops' member can be
> initialized.
> 3. Fix the OMAP vp code not to oops when voltdm->pmic is NULL
> which I have in my combined patch for fixing OMAP so far.
It'd also help if we supported null ops, I sent patches for that a few
times over the 3.3 cycle since I was running into it on my systems but
apparently to /dev/null and further changes in this area have made the
patches not apply any more.
On Tue, Feb 07, 2012 at 03:26:27PM +0000, Mark Brown wrote:
> On Sun, Feb 05, 2012 at 04:13:48PM +0000, Russell King - ARM Linux wrote:
>
> > It's not quite correct, because OMAP4 has issues in this area as well
> > (which does select IRQ_DOMAIN but can be without OF.) The result is
> > an oops from irq_domain_add() because domain->ops is NULL.
>
> > The right solution is three fold:
>
> > 1. Wrap the bits of code in CONFIG_IRQ_DOMAIN
> > 2. Get rid of the #ifdef CONFIG_OF there, so the 'ops' member can be
> > initialized.
> > 3. Fix the OMAP vp code not to oops when voltdm->pmic is NULL
>
> > which I have in my combined patch for fixing OMAP so far.
>
> It'd also help if we supported null ops, I sent patches for that a few
> times over the 3.3 cycle since I was running into it on my systems but
> apparently to /dev/null and further changes in this area have made the
> patches not apply any more.
I've avoided allowing null ops so that the drivers are forced to be explicit
about what they want, and it makes for (IMHO) simpler to follow code in the
core.
Sorry I missed your patches though.
g.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/