2014-04-23 22:58:04

by Rob Herring

[permalink] [raw]
Subject: [PATCH 0/2] DT irq deferred probe support

From: Rob Herring <[email protected]>

This is atleast the 4th attempt to fix a long standing issue with DT
irq resolution needing to support deferred probe when irq parent is not
yet initialized. This version implements Russell King's suggestion to do
irq resolution when platform_get_irq is called.

Tony, I squashed your warning fix (dropping the other part) and stole
much of your commit message. Let me know if you have any issues with
that.

Rob

Rob Herring (2):
of: selftest: add deferred probe interrupt test
of/irq: do irq resolution in platform_get_irq

drivers/base/platform.c | 7 +++++-
drivers/of/irq.c | 26 +++++++++++++++++++++
drivers/of/platform.c | 4 +++-
drivers/of/selftest.c | 32 ++++++++++++++++++++++++++
drivers/of/testcase-data/tests-interrupts.dtsi | 12 ++++++++++
include/linux/of_irq.h | 7 +++++-
6 files changed, 85 insertions(+), 3 deletions(-)

--
1.9.1


2014-04-23 22:58:07

by Rob Herring

[permalink] [raw]
Subject: [PATCH 1/2] of: selftest: add deferred probe interrupt test

From: Rob Herring <[email protected]>

Signed-off-by: Rob Herring <[email protected]>
---
drivers/of/selftest.c | 32 ++++++++++++++++++++++++++
drivers/of/testcase-data/tests-interrupts.dtsi | 12 ++++++++++
2 files changed, 44 insertions(+)

diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index ae44500..fe70b86 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_irq.h>
+#include <linux/of_platform.h>
#include <linux/list.h>
#include <linux/mutex.h>
#include <linux/slab.h>
@@ -427,6 +428,36 @@ static void __init of_selftest_match_node(void)
}
}

+static void __init of_selftest_platform_populate(void)
+{
+ int irq;
+ struct device_node *np;
+ struct platform_device *pdev;
+
+ np = of_find_node_by_path("/testcase-data");
+ of_platform_populate(np, of_default_bus_match_table, NULL, NULL);
+
+ /* Test that a missing irq domain returns -EPROBE_DEFER */
+ np = of_find_node_by_path("/testcase-data/testcase-device1");
+ pdev = of_find_device_by_node(np);
+ if (!pdev)
+ selftest(0, "device 1 creation failed\n");
+ irq = platform_get_irq(pdev, 0);
+ if (irq != -EPROBE_DEFER)
+ selftest(0, "device deferred probe failed - %d\n", irq);
+
+ /* Test that a parsing failure does not return -EPROBE_DEFER */
+ np = of_find_node_by_path("/testcase-data/testcase-device2");
+ pdev = of_find_device_by_node(np);
+ if (!pdev)
+ selftest(0, "device 2 creation failed\n");
+ irq = platform_get_irq(pdev, 0);
+ if (irq >= 0 || irq == -EPROBE_DEFER)
+ selftest(0, "device parsing error failed - %d\n", irq);
+
+ selftest(1, "passed");
+}
+
static int __init of_selftest(void)
{
struct device_node *np;
@@ -445,6 +476,7 @@ static int __init of_selftest(void)
of_selftest_parse_interrupts();
of_selftest_parse_interrupts_extended();
of_selftest_match_node();
+ of_selftest_platform_populate();
pr_info("end of selftest - %i passed, %i failed\n",
selftest_results.passed, selftest_results.failed);
return 0;
diff --git a/drivers/of/testcase-data/tests-interrupts.dtsi b/drivers/of/testcase-data/tests-interrupts.dtsi
index c843720..45cbdf8 100644
--- a/drivers/of/testcase-data/tests-interrupts.dtsi
+++ b/drivers/of/testcase-data/tests-interrupts.dtsi
@@ -54,5 +54,17 @@
<&test_intmap1 1 2>;
};
};
+
+ testcase-device1 {
+ compatible = "testcase-device";
+ interrupt-parent = <&test_intc0>;
+ interrupts = <1>;
+ };
+
+ testcase-device2 {
+ compatible = "testcase-device";
+ interrupts = <1>;
+ };
};
+
};
--
1.9.1

2014-04-23 22:58:56

by Rob Herring

[permalink] [raw]
Subject: [PATCH 2/2] of/irq: do irq resolution in platform_get_irq

From: Rob Herring <[email protected]>

Currently we get the following kind of errors if we try to use interrupt
phandles to irqchips that have not yet initialized:

irq: no irq domain found for /ocp/pinmux@48002030 !
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at drivers/of/platform.c:171 of_device_alloc+0x144/0x184()
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.12.0-00038-g42a9708 #1012
(show_stack+0x14/0x1c)
(dump_stack+0x6c/0xa0)
(warn_slowpath_common+0x64/0x84)
(warn_slowpath_null+0x1c/0x24)
(of_device_alloc+0x144/0x184)
(of_platform_device_create_pdata+0x44/0x9c)
(of_platform_bus_create+0xd0/0x170)
(of_platform_bus_create+0x12c/0x170)
(of_platform_populate+0x60/0x98)

This is because we're wrongly trying to populate resources that are not
yet available. It's perfectly valid to create irqchips dynamically, so
let's fix up the issue by resolving the interrupt resources when
platform_get_irq is called.

And then we also need to accept the fact that some irqdomains do not
exist that early on, and only get initialized later on. So we can
make the current WARN_ON into just into a pr_debug().

We still attempt to populate irq resources when we create the devices.
This allows current drivers which don't use platform_get_irq to continue
to function. Once all drivers are fixed, this code can be removed.

Suggested-by: Russell King <[email protected]>
Signed-off-by: Rob Herring <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/base/platform.c | 7 ++++++-
drivers/of/irq.c | 26 ++++++++++++++++++++++++++
drivers/of/platform.c | 4 +++-
include/linux/of_irq.h | 7 ++++++-
4 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index e714709..5b47210 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -13,6 +13,7 @@
#include <linux/string.h>
#include <linux/platform_device.h>
#include <linux/of_device.h>
+#include <linux/of_irq.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/dma-mapping.h>
@@ -87,7 +88,11 @@ int platform_get_irq(struct platform_device *dev, unsigned int num)
return -ENXIO;
return dev->archdata.irqs[num];
#else
- struct resource *r = platform_get_resource(dev, IORESOURCE_IRQ, num);
+ struct resource *r;
+ if (IS_ENABLED(CONFIG_OF_IRQ) && dev->dev.of_node)
+ return of_irq_get(dev->dev.of_node, num);
+
+ r = platform_get_resource(dev, IORESOURCE_IRQ, num);

return r ? r->start : -ENXIO;
#endif
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 9bcf2cf..ca01893 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -380,6 +380,32 @@ int of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
EXPORT_SYMBOL_GPL(of_irq_to_resource);

/**
+ * of_irq_get - Decode a node's IRQ and return it as a Linux irq number
+ * @dev: pointer to device tree node
+ * @index: zero-based index of the irq
+ *
+ * Returns Linux irq number on success, or -EPROBE_DEFER if the irq domain
+ * is not yet created.
+ *
+ */
+int of_irq_get(struct device_node *dev, int index)
+{
+ int rc;
+ struct of_phandle_args oirq;
+ struct irq_domain *domain;
+
+ rc = of_irq_parse_one(dev, index, &oirq);
+ if (rc)
+ return rc;
+
+ domain = irq_find_host(oirq.np);
+ if (!domain)
+ return -EPROBE_DEFER;
+
+ return irq_create_of_mapping(&oirq);
+}
+
+/**
* of_irq_count - Count the number of IRQs a node uses
* @dev: pointer to device tree node
*/
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 404d1da..bd47fbc 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -168,7 +168,9 @@ struct platform_device *of_device_alloc(struct device_node *np,
rc = of_address_to_resource(np, i, res);
WARN_ON(rc);
}
- WARN_ON(of_irq_to_resource_table(np, res, num_irq) != num_irq);
+ if (of_irq_to_resource_table(np, res, num_irq) != num_irq)
+ pr_debug("not all legacy IRQ resources mapped for %s\n",
+ np->name);
}

dev->dev.of_node = of_node_get(np);
diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
index 3f23b44..bc9e51a 100644
--- a/include/linux/of_irq.h
+++ b/include/linux/of_irq.h
@@ -34,21 +34,26 @@ static inline int of_irq_parse_oldworld(struct device_node *device, int index,
extern int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq);
extern int of_irq_parse_one(struct device_node *device, int index,
struct of_phandle_args *out_irq);
-extern unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data);
extern int of_irq_to_resource(struct device_node *dev, int index,
struct resource *r);
extern int of_irq_to_resource_table(struct device_node *dev,
struct resource *res, int nr_irqs);
+extern unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data);

extern void of_irq_init(const struct of_device_id *matches);

#ifdef CONFIG_OF_IRQ
extern int of_irq_count(struct device_node *dev);
+extern int of_irq_get(struct device_node *dev, int index);
#else
static inline int of_irq_count(struct device_node *dev)
{
return 0;
}
+static inline int of_irq_get(struct device_node *dev, int index)
+{
+ return 0;
+}
#endif

#if defined(CONFIG_OF)
--
1.9.1

2014-04-23 23:42:22

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 2/2] of/irq: do irq resolution in platform_get_irq

* Rob Herring <[email protected]> [140423 15:58]:
> From: Rob Herring <[email protected]>
>
> Currently we get the following kind of errors if we try to use interrupt
> phandles to irqchips that have not yet initialized:
>
> irq: no irq domain found for /ocp/pinmux@48002030 !
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at drivers/of/platform.c:171 of_device_alloc+0x144/0x184()
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.12.0-00038-g42a9708 #1012
> (show_stack+0x14/0x1c)
> (dump_stack+0x6c/0xa0)
> (warn_slowpath_common+0x64/0x84)
> (warn_slowpath_null+0x1c/0x24)
> (of_device_alloc+0x144/0x184)
> (of_platform_device_create_pdata+0x44/0x9c)
> (of_platform_bus_create+0xd0/0x170)
> (of_platform_bus_create+0x12c/0x170)
> (of_platform_populate+0x60/0x98)
>
> This is because we're wrongly trying to populate resources that are not
> yet available. It's perfectly valid to create irqchips dynamically, so
> let's fix up the issue by resolving the interrupt resources when
> platform_get_irq is called.
>
> And then we also need to accept the fact that some irqdomains do not
> exist that early on, and only get initialized later on. So we can
> make the current WARN_ON into just into a pr_debug().
>
> We still attempt to populate irq resources when we create the devices.
> This allows current drivers which don't use platform_get_irq to continue
> to function. Once all drivers are fixed, this code can be removed.
>
> Suggested-by: Russell King <[email protected]>
> Signed-off-by: Rob Herring <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>

Great, works for me. Hopefully this patch is non-intrusive enough for
people for the -rc cycle too?

Tested-by: Tony Lindgren <[email protected]>

> ---
> drivers/base/platform.c | 7 ++++++-
> drivers/of/irq.c | 26 ++++++++++++++++++++++++++
> drivers/of/platform.c | 4 +++-
> include/linux/of_irq.h | 7 ++++++-
> 4 files changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index e714709..5b47210 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -13,6 +13,7 @@
> #include <linux/string.h>
> #include <linux/platform_device.h>
> #include <linux/of_device.h>
> +#include <linux/of_irq.h>
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/dma-mapping.h>
> @@ -87,7 +88,11 @@ int platform_get_irq(struct platform_device *dev, unsigned int num)
> return -ENXIO;
> return dev->archdata.irqs[num];
> #else
> - struct resource *r = platform_get_resource(dev, IORESOURCE_IRQ, num);
> + struct resource *r;
> + if (IS_ENABLED(CONFIG_OF_IRQ) && dev->dev.of_node)
> + return of_irq_get(dev->dev.of_node, num);
> +
> + r = platform_get_resource(dev, IORESOURCE_IRQ, num);
>
> return r ? r->start : -ENXIO;
> #endif
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 9bcf2cf..ca01893 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -380,6 +380,32 @@ int of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
> EXPORT_SYMBOL_GPL(of_irq_to_resource);
>
> /**
> + * of_irq_get - Decode a node's IRQ and return it as a Linux irq number
> + * @dev: pointer to device tree node
> + * @index: zero-based index of the irq
> + *
> + * Returns Linux irq number on success, or -EPROBE_DEFER if the irq domain
> + * is not yet created.
> + *
> + */
> +int of_irq_get(struct device_node *dev, int index)
> +{
> + int rc;
> + struct of_phandle_args oirq;
> + struct irq_domain *domain;
> +
> + rc = of_irq_parse_one(dev, index, &oirq);
> + if (rc)
> + return rc;
> +
> + domain = irq_find_host(oirq.np);
> + if (!domain)
> + return -EPROBE_DEFER;
> +
> + return irq_create_of_mapping(&oirq);
> +}
> +
> +/**
> * of_irq_count - Count the number of IRQs a node uses
> * @dev: pointer to device tree node
> */
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 404d1da..bd47fbc 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -168,7 +168,9 @@ struct platform_device *of_device_alloc(struct device_node *np,
> rc = of_address_to_resource(np, i, res);
> WARN_ON(rc);
> }
> - WARN_ON(of_irq_to_resource_table(np, res, num_irq) != num_irq);
> + if (of_irq_to_resource_table(np, res, num_irq) != num_irq)
> + pr_debug("not all legacy IRQ resources mapped for %s\n",
> + np->name);
> }
>
> dev->dev.of_node = of_node_get(np);
> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> index 3f23b44..bc9e51a 100644
> --- a/include/linux/of_irq.h
> +++ b/include/linux/of_irq.h
> @@ -34,21 +34,26 @@ static inline int of_irq_parse_oldworld(struct device_node *device, int index,
> extern int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq);
> extern int of_irq_parse_one(struct device_node *device, int index,
> struct of_phandle_args *out_irq);
> -extern unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data);
> extern int of_irq_to_resource(struct device_node *dev, int index,
> struct resource *r);
> extern int of_irq_to_resource_table(struct device_node *dev,
> struct resource *res, int nr_irqs);
> +extern unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data);
>
> extern void of_irq_init(const struct of_device_id *matches);
>
> #ifdef CONFIG_OF_IRQ
> extern int of_irq_count(struct device_node *dev);
> +extern int of_irq_get(struct device_node *dev, int index);
> #else
> static inline int of_irq_count(struct device_node *dev)
> {
> return 0;
> }
> +static inline int of_irq_get(struct device_node *dev, int index)
> +{
> + return 0;
> +}
> #endif
>
> #if defined(CONFIG_OF)
> --
> 1.9.1
>

2014-04-23 23:43:33

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 0/2] DT irq deferred probe support

* Rob Herring <[email protected]> [140423 15:58]:
> From: Rob Herring <[email protected]>
>
> This is atleast the 4th attempt to fix a long standing issue with DT
> irq resolution needing to support deferred probe when irq parent is not
> yet initialized. This version implements Russell King's suggestion to do
> irq resolution when platform_get_irq is called.
>
> Tony, I squashed your warning fix (dropping the other part) and stole
> much of your commit message. Let me know if you have any issues with
> that.

Nope, looks good, just gave it a try and it fixes the problem for me.

Regards,

Tony

> Rob Herring (2):
> of: selftest: add deferred probe interrupt test
> of/irq: do irq resolution in platform_get_irq
>
> drivers/base/platform.c | 7 +++++-
> drivers/of/irq.c | 26 +++++++++++++++++++++
> drivers/of/platform.c | 4 +++-
> drivers/of/selftest.c | 32 ++++++++++++++++++++++++++
> drivers/of/testcase-data/tests-interrupts.dtsi | 12 ++++++++++
> include/linux/of_irq.h | 7 +++++-
> 6 files changed, 85 insertions(+), 3 deletions(-)
>
> --
> 1.9.1
>

2014-04-24 16:10:57

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 2/2] of/irq: do irq resolution in platform_get_irq

On Wed, 23 Apr 2014 16:42:13 -0700, Tony Lindgren <[email protected]> wrote:
> * Rob Herring <[email protected]> [140423 15:58]:
> > From: Rob Herring <[email protected]>
> >
> > Currently we get the following kind of errors if we try to use interrupt
> > phandles to irqchips that have not yet initialized:
> >
> > irq: no irq domain found for /ocp/pinmux@48002030 !
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 1 at drivers/of/platform.c:171 of_device_alloc+0x144/0x184()
> > Modules linked in:
> > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.12.0-00038-g42a9708 #1012
> > (show_stack+0x14/0x1c)
> > (dump_stack+0x6c/0xa0)
> > (warn_slowpath_common+0x64/0x84)
> > (warn_slowpath_null+0x1c/0x24)
> > (of_device_alloc+0x144/0x184)
> > (of_platform_device_create_pdata+0x44/0x9c)
> > (of_platform_bus_create+0xd0/0x170)
> > (of_platform_bus_create+0x12c/0x170)
> > (of_platform_populate+0x60/0x98)
> >
> > This is because we're wrongly trying to populate resources that are not
> > yet available. It's perfectly valid to create irqchips dynamically, so
> > let's fix up the issue by resolving the interrupt resources when
> > platform_get_irq is called.
> >
> > And then we also need to accept the fact that some irqdomains do not
> > exist that early on, and only get initialized later on. So we can
> > make the current WARN_ON into just into a pr_debug().
> >
> > We still attempt to populate irq resources when we create the devices.
> > This allows current drivers which don't use platform_get_irq to continue
> > to function. Once all drivers are fixed, this code can be removed.
> >
> > Suggested-by: Russell King <[email protected]>
> > Signed-off-by: Rob Herring <[email protected]>
> > Signed-off-by: Tony Lindgren <[email protected]>
>
> Great, works for me. Hopefully this patch is non-intrusive enough for
> people for the -rc cycle too?

Both patches look good. I've put them in my tree and will push it out
shortly. I want to make sure there are no regressions on PowerPC, so
I'll give it a few days in linux-next before asking Linus to pull.

Tony, how far back does this need to be backported?

g.

2014-04-24 17:19:29

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 2/2] of/irq: do irq resolution in platform_get_irq

* Grant Likely <[email protected]> [140424 09:11]:
> On Wed, 23 Apr 2014 16:42:13 -0700, Tony Lindgren <[email protected]> wrote:
> > * Rob Herring <[email protected]> [140423 15:58]:
> > > From: Rob Herring <[email protected]>
> > >
> > > Currently we get the following kind of errors if we try to use interrupt
> > > phandles to irqchips that have not yet initialized:
> > >
> > > irq: no irq domain found for /ocp/pinmux@48002030 !
> > > ------------[ cut here ]------------
> > > WARNING: CPU: 0 PID: 1 at drivers/of/platform.c:171 of_device_alloc+0x144/0x184()
> > > Modules linked in:
> > > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.12.0-00038-g42a9708 #1012
> > > (show_stack+0x14/0x1c)
> > > (dump_stack+0x6c/0xa0)
> > > (warn_slowpath_common+0x64/0x84)
> > > (warn_slowpath_null+0x1c/0x24)
> > > (of_device_alloc+0x144/0x184)
> > > (of_platform_device_create_pdata+0x44/0x9c)
> > > (of_platform_bus_create+0xd0/0x170)
> > > (of_platform_bus_create+0x12c/0x170)
> > > (of_platform_populate+0x60/0x98)
> > >
> > > This is because we're wrongly trying to populate resources that are not
> > > yet available. It's perfectly valid to create irqchips dynamically, so
> > > let's fix up the issue by resolving the interrupt resources when
> > > platform_get_irq is called.
> > >
> > > And then we also need to accept the fact that some irqdomains do not
> > > exist that early on, and only get initialized later on. So we can
> > > make the current WARN_ON into just into a pr_debug().
> > >
> > > We still attempt to populate irq resources when we create the devices.
> > > This allows current drivers which don't use platform_get_irq to continue
> > > to function. Once all drivers are fixed, this code can be removed.
> > >
> > > Suggested-by: Russell King <[email protected]>
> > > Signed-off-by: Rob Herring <[email protected]>
> > > Signed-off-by: Tony Lindgren <[email protected]>
> >
> > Great, works for me. Hopefully this patch is non-intrusive enough for
> > people for the -rc cycle too?
>
> Both patches look good. I've put them in my tree and will push it out
> shortly. I want to make sure there are no regressions on PowerPC, so
> I'll give it a few days in linux-next before asking Linus to pull.

Great, sounds good to me.

> Tony, how far back does this need to be backported?

The issue seems to have been around ever since the start of DT with
platform_bus, and people have been probably working around it with
the initcall levels and using irq_of_parse_and_map instead of
platform_get_irq.

I noticed the issue last year around the time interrupts-extended
binding patches were posted and I was adding the irqdomain support
to pinctrl-single.c for wake-up interrupts.

So the fix should probably go back to at least v3.12 or v3.10 as
those are recent longterm kernels. The patch seems to apply to both
of them with fuzz except for include/linux/of_irq.h.

Sligthly related to this patch, also 4a43d686fe33 (of/irq: Pass
trigger type in IRQ resource flags) should also get tagged stable
too if not done already as that keeps some legacy drivers working.

Regards,

Tony

2014-04-24 18:43:33

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 2/2] of/irq: do irq resolution in platform_get_irq

On Wed, Apr 23, 2014 at 05:57:41PM -0500, Rob Herring wrote:
> From: Rob Herring <[email protected]>
>
> Currently we get the following kind of errors if we try to use interrupt
> phandles to irqchips that have not yet initialized:
>
> irq: no irq domain found for /ocp/pinmux@48002030 !
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at drivers/of/platform.c:171 of_device_alloc+0x144/0x184()
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.12.0-00038-g42a9708 #1012
> (show_stack+0x14/0x1c)
> (dump_stack+0x6c/0xa0)
> (warn_slowpath_common+0x64/0x84)
> (warn_slowpath_null+0x1c/0x24)
> (of_device_alloc+0x144/0x184)
> (of_platform_device_create_pdata+0x44/0x9c)
> (of_platform_bus_create+0xd0/0x170)
> (of_platform_bus_create+0x12c/0x170)
> (of_platform_populate+0x60/0x98)
>
> This is because we're wrongly trying to populate resources that are not
> yet available. It's perfectly valid to create irqchips dynamically, so
> let's fix up the issue by resolving the interrupt resources when
> platform_get_irq is called.
>
> And then we also need to accept the fact that some irqdomains do not
> exist that early on, and only get initialized later on. So we can
> make the current WARN_ON into just into a pr_debug().
>
> We still attempt to populate irq resources when we create the devices.
> This allows current drivers which don't use platform_get_irq to continue
> to function. Once all drivers are fixed, this code can be removed.
>
> Suggested-by: Russell King <[email protected]>
> Signed-off-by: Rob Herring <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> drivers/base/platform.c | 7 ++++++-
> drivers/of/irq.c | 26 ++++++++++++++++++++++++++
> drivers/of/platform.c | 4 +++-
> include/linux/of_irq.h | 7 ++++++-
> 4 files changed, 41 insertions(+), 3 deletions(-)

Hehe... that's largely what we already had back in January[0]. Glad to
see that people could finally agree on what to do about this.

Thierry

[0]: https://lkml.org/lkml/2014/1/8/240


Attachments:
(No filename) (2.00 kB)
(No filename) (836.00 B)
Download all attachments

2014-04-24 20:47:26

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 2/2] of/irq: do irq resolution in platform_get_irq

On Thu, Apr 24, 2014 at 7:42 PM, Thierry Reding
<[email protected]> wrote:
> On Wed, Apr 23, 2014 at 05:57:41PM -0500, Rob Herring wrote:
>> From: Rob Herring <[email protected]>
>>
>> Currently we get the following kind of errors if we try to use interrupt
>> phandles to irqchips that have not yet initialized:
>>
>> irq: no irq domain found for /ocp/pinmux@48002030 !
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 1 at drivers/of/platform.c:171 of_device_alloc+0x144/0x184()
>> Modules linked in:
>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.12.0-00038-g42a9708 #1012
>> (show_stack+0x14/0x1c)
>> (dump_stack+0x6c/0xa0)
>> (warn_slowpath_common+0x64/0x84)
>> (warn_slowpath_null+0x1c/0x24)
>> (of_device_alloc+0x144/0x184)
>> (of_platform_device_create_pdata+0x44/0x9c)
>> (of_platform_bus_create+0xd0/0x170)
>> (of_platform_bus_create+0x12c/0x170)
>> (of_platform_populate+0x60/0x98)
>>
>> This is because we're wrongly trying to populate resources that are not
>> yet available. It's perfectly valid to create irqchips dynamically, so
>> let's fix up the issue by resolving the interrupt resources when
>> platform_get_irq is called.
>>
>> And then we also need to accept the fact that some irqdomains do not
>> exist that early on, and only get initialized later on. So we can
>> make the current WARN_ON into just into a pr_debug().
>>
>> We still attempt to populate irq resources when we create the devices.
>> This allows current drivers which don't use platform_get_irq to continue
>> to function. Once all drivers are fixed, this code can be removed.
>>
>> Suggested-by: Russell King <[email protected]>
>> Signed-off-by: Rob Herring <[email protected]>
>> Signed-off-by: Tony Lindgren <[email protected]>
>> ---
>> drivers/base/platform.c | 7 ++++++-
>> drivers/of/irq.c | 26 ++++++++++++++++++++++++++
>> drivers/of/platform.c | 4 +++-
>> include/linux/of_irq.h | 7 ++++++-
>> 4 files changed, 41 insertions(+), 3 deletions(-)
>
> Hehe... that's largely what we already had back in January[0]. Glad to
> see that people could finally agree on what to do about this.
>
> Thierry
>
> [0]: https://lkml.org/lkml/2014/1/8/240

Sometimes it takes prodding around with the other options to figure
out an earlier approach was on the right track. I'm very happy to
finally have a solution here that appears to be working.

I've pushed it out to my tree so it gets into linux-next. Please test
before I ask Linus to pull.

git://git.secretlab.ca/git/linux devicetree/merge

g.