All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] of/platform: Fix no irq domain found errors when populating interrupts
@ 2014-04-10 21:38 Tony Lindgren
  2014-04-11  0:29 ` Rob Herring
  2014-04-11  9:20 ` Russell King - ARM Linux
  0 siblings, 2 replies; 81+ messages in thread
From: Tony Lindgren @ 2014-04-10 21:38 UTC (permalink / raw)
  To: Greg KH
  Cc: Arnd Bergmann, Grant Likely, Paul Walmsley, Rob Herring,
	Thierry Reding, Russell King - ARM Linux, linux-kernel

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 populating the interrupt resources at the driver probe
time instead.

Note that at least currently we cannot dynamically allocate the resources as bus
specific code may add legacy resources with platform_device_add_resources()
before the driver probe. At least omap_device_alloc() currently relies on
num_resources to determine if legacy resources should be added. Some of these
will clear automatically when mach-omap2 boots with DT only, but there are
probably other places too where platform_device_add_resources() modifies
things before driver probe.

This patch was discussed quite a bit earlier, but so far it seems we don't
have any better options to fix the problem. For the earlier discussion,
please see:

https://lkml.org/lkml/2013/11/22/520

The addition of of_platform_probe() is based on patches posted earlier by
Thierry Reding <thierry.reding@gmail.com>.

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

---

Greg, this is still broken, can you please pick this up?

--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -483,6 +483,10 @@ static int platform_drv_probe(struct device *_dev)
 
 	acpi_dev_pm_attach(_dev, true);
 
+	ret = of_platform_probe(dev);
+	if (ret)
+		return ret;
+
 	ret = drv->probe(dev);
 	if (ret)
 		acpi_dev_pm_detach(_dev, true);
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -141,7 +141,7 @@ struct platform_device *of_device_alloc(struct device_node *np,
 				  struct device *parent)
 {
 	struct platform_device *dev;
-	int rc, i, num_reg = 0, num_irq;
+	int num_reg = 0, num_irq;
 	struct resource *res, temp_res;
 
 	dev = platform_device_alloc("", -1);
@@ -154,7 +154,14 @@ struct platform_device *of_device_alloc(struct device_node *np,
 			num_reg++;
 	num_irq = of_irq_count(np);
 
-	/* Populate the resource table */
+	/*
+	 * Only allocate the resources for us to use later on. Note that bus
+	 * specific code may also add in additional legacy resources using
+	 * platform_device_add_resources(), and may even rely on us allocating
+	 * the basic resources here to do so. So we cannot allocate the
+	 * resources lazily until the legacy code has been fixed to not rely
+	 * on allocating resources here.
+	 */
 	if (num_irq || num_reg) {
 		res = kzalloc(sizeof(*res) * (num_irq + num_reg), GFP_KERNEL);
 		if (!res) {
@@ -164,11 +171,7 @@ struct platform_device *of_device_alloc(struct device_node *np,
 
 		dev->num_resources = num_reg + num_irq;
 		dev->resource = res;
-		for (i = 0; i < num_reg; i++, res++) {
-			rc = of_address_to_resource(np, i, res);
-			WARN_ON(rc);
-		}
-		WARN_ON(of_irq_to_resource_table(np, res, num_irq) != num_irq);
+		/* See of_device_resource_populate for populating the data */
 	}
 
 	dev->dev.of_node = of_node_get(np);
@@ -187,6 +190,50 @@ struct platform_device *of_device_alloc(struct device_node *np,
 EXPORT_SYMBOL(of_device_alloc);
 
 /**
+ * of_device_resource_populate - Populate device resources from device tree
+ * @dev: pointer to platform device
+ *
+ * The device interrupts are not necessarily available for all
+ * irqdomains initially so we need to populate them lazily at
+ * device probe time from of_platform_populate.
+ */
+static int of_device_resource_populate(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	int rc, i, num_reg = 0, num_irq;
+	struct resource *res, temp_res;
+
+	res = pdev->resource;
+
+	/*
+	 * Count the io and irq resources again. Currently we cannot rely on
+	 * pdev->num_resources as bus specific code may have changed that
+	 * with platform_device_add_resources(). But the resources we allocated
+	 * earlier are still there and available for us to populate.
+	 */
+	if (of_can_translate_address(np))
+		while (of_address_to_resource(np, num_reg, &temp_res) == 0)
+			num_reg++;
+	num_irq = of_irq_count(np);
+
+	if (pdev->num_resources < num_reg + num_irq) {
+		dev_WARN(&pdev->dev, "not enough resources %i < %i\n",
+			 pdev->num_resources, num_reg + num_irq);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < num_reg; i++, res++) {
+		rc = of_address_to_resource(np, i, res);
+		WARN_ON(rc);
+	}
+
+	if (num_irq)
+		WARN_ON(of_irq_to_resource_table(np, res, num_irq) != num_irq);
+
+	return 0;
+}
+
+/**
  * of_platform_device_create_pdata - Alloc, initialize and register an of_device
  * @np: pointer to node to create device for
  * @bus_id: name to assign device
@@ -485,4 +532,35 @@ int of_platform_populate(struct device_node *root,
 	return rc;
 }
 EXPORT_SYMBOL_GPL(of_platform_populate);
+
+/**
+ * of_platform_probe() - OF specific initialization at probe time
+ * @pdev: pointer to a platform device
+ *
+ * This function is called by the driver core to perform devicetree-specific
+ * setup for a given platform device at probe time. If a device's resources
+ * as specified in the device tree are not available yet, this function can
+ * return -EPROBE_DEFER and cause the device to be probed again later, when
+ * other drivers that potentially provide the missing resources have been
+ * probed in turn.
+ *
+ * Note that because of the above, all code executed by this function must
+ * be prepared to be run multiple times on the same device (i.e. it must be
+ * idempotent).
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int of_platform_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	if (!pdev->dev.of_node)
+		return 0;
+
+	ret = of_device_resource_populate(pdev);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
 #endif /* CONFIG_OF_ADDRESS */
--- a/include/linux/of_platform.h
+++ b/include/linux/of_platform.h
@@ -72,6 +72,8 @@ extern int of_platform_populate(struct device_node *root,
 				const struct of_device_id *matches,
 				const struct of_dev_auxdata *lookup,
 				struct device *parent);
+
+extern int of_platform_probe(struct platform_device *pdev);
 #else
 static inline int of_platform_populate(struct device_node *root,
 					const struct of_device_id *matches,
@@ -80,6 +82,11 @@ static inline int of_platform_populate(struct device_node *root,
 {
 	return -ENODEV;
 }
+
+static inline int of_platform_probe(struct platform_device *pdev)
+{
+	return 0;
+}
 #endif
 
 #endif	/* _LINUX_OF_PLATFORM_H */

^ permalink raw reply	[flat|nested] 81+ messages in thread
* [PATCH] of/platform: Fix no irq domain found errors when populating interrupts
@ 2013-11-23  0:43 ` Tony Lindgren
  0 siblings, 0 replies; 81+ messages in thread
From: Tony Lindgren @ 2013-11-23  0:43 UTC (permalink / raw)
  To: Grant Likely, Rob Herring; +Cc: devicetree, linux-kernel, linux-arm-kernel

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 !
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 populating the interrupt resources based
on a notifier call instead.

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

---

Rob & Grant, care to merge this for the -rc if this looks OK to you?

These happen for example when using interrupts-extended for omap
wake-up interrupts where the irq domain is created by pinctrl-single.c
at module_init time.

--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -130,6 +130,56 @@ void of_device_make_bus_id(struct device *dev)
 	dev_set_name(dev, "%s.%d", node->name, magic - 1);
 }
 
+/*
+ * The device interrupts are not necessarily available for all
+ * irqdomains initially so we need to populate them using a
+ * notifier.
+ */
+static int of_device_resource_notify(struct notifier_block *nb,
+				     unsigned long event, void *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct device_node *np = pdev->dev.of_node;
+	struct resource *res = pdev->resource;
+	struct resource *irqr = NULL;
+	int num_irq, i, found = 0;
+
+	if (event != BUS_NOTIFY_BIND_DRIVER)
+		return 0;
+
+	if (!np)
+		goto out;
+
+	num_irq = of_irq_count(np);
+	if (!num_irq)
+		goto out;
+
+	for (i = 0; i < pdev->num_resources; i++, res++) {
+		if (res->flags != IORESOURCE_IRQ ||
+		    res->start != -EPROBE_DEFER ||
+		    res->end != -EPROBE_DEFER)
+			continue;
+
+		if (!irqr)
+			irqr = res;
+		found++;
+	}
+
+	if (!found)
+		goto out;
+
+	if (found != num_irq) {
+		dev_WARN(dev, "error populating irq resources: %i != %i\n",
+			 found, num_irq);
+		goto out;
+	}
+
+	WARN_ON(of_irq_to_resource_table(np, irqr, num_irq) != num_irq);
+
+out:
+	return NOTIFY_DONE;
+}
+
 /**
  * of_device_alloc - Allocate and initialize an of_device
  * @np: device node to assign to device
@@ -168,7 +218,13 @@ 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);
+
+		/* See of_device_resource_notify for populating interrupts */
+		for (i = 0; i < num_irq; i++, res++) {
+			res->flags = IORESOURCE_IRQ;
+			res->start = -EPROBE_DEFER;
+			res->end = -EPROBE_DEFER;
+		}
 	}
 
 	dev->dev.of_node = of_node_get(np);
@@ -447,6 +503,8 @@ int of_platform_bus_probe(struct device_node *root,
 }
 EXPORT_SYMBOL(of_platform_bus_probe);
 
+static struct notifier_block resource_nb;
+
 /**
  * of_platform_populate() - Populate platform_devices from device tree data
  * @root: parent of the first level to probe or NULL for the root of the tree
@@ -478,6 +536,11 @@ int of_platform_populate(struct device_node *root,
 	if (!root)
 		return -EINVAL;
 
+	if (!resource_nb.notifier_call) {
+		resource_nb.notifier_call = of_device_resource_notify,
+			bus_register_notifier(&platform_bus_type, &resource_nb);
+	}
+
 	for_each_child_of_node(root, child) {
 		rc = of_platform_bus_create(child, matches, lookup, parent, true);
 		if (rc)

^ permalink raw reply	[flat|nested] 81+ messages in thread

end of thread, other threads:[~2014-04-18 23:19 UTC | newest]

Thread overview: 81+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-10 21:38 [PATCH] of/platform: Fix no irq domain found errors when populating interrupts Tony Lindgren
2014-04-11  0:29 ` Rob Herring
2014-04-11 18:43   ` Thierry Reding
2014-04-18 20:55     ` Tony Lindgren
2014-04-11  9:20 ` Russell King - ARM Linux
2014-04-11 18:36   ` Thierry Reding
2014-04-18 21:37     ` Tony Lindgren
2014-04-18 23:18       ` Russell King - ARM Linux
  -- strict thread matches above, loose matches on Subject: below --
2013-11-23  0:43 Tony Lindgren
2013-11-23  0:43 ` Tony Lindgren
2013-11-23  0:43 ` Tony Lindgren
2013-11-23  0:55 ` Russell King - ARM Linux
2013-11-23  0:55   ` Russell King - ARM Linux
2013-11-23  0:55   ` Russell King - ARM Linux
2013-11-23  1:08   ` Tony Lindgren
2013-11-23  1:08     ` Tony Lindgren
2013-11-23  1:15     ` Tony Lindgren
2013-11-23  1:15       ` Tony Lindgren
2013-11-23  1:50       ` Tony Lindgren
2013-11-23  1:50         ` Tony Lindgren
2013-11-23  1:50         ` Tony Lindgren
2013-11-23 15:42         ` Rob Herring
2013-11-23 15:42           ` Rob Herring
2013-11-23 15:42           ` Rob Herring
2013-11-23 16:32           ` Tony Lindgren
2013-11-23 16:32             ` Tony Lindgren
2013-11-23 16:32             ` Tony Lindgren
2013-11-25  9:34             ` Thierry Reding
2013-11-25  9:34               ` Thierry Reding
2013-11-25  9:34               ` Thierry Reding
2013-11-25 19:46               ` Tony Lindgren
2013-11-25 19:46                 ` Tony Lindgren
2013-11-25 19:46                 ` Tony Lindgren
2013-11-24 21:27         ` Grant Likely
2013-11-24 21:27           ` Grant Likely
2013-12-10  3:39           ` Paul Walmsley
2013-12-10  3:39             ` Paul Walmsley
2013-12-30 22:10             ` Paul Walmsley
2013-12-30 22:10               ` Paul Walmsley
2013-12-30 22:10               ` Paul Walmsley
2013-12-31 16:33               ` Rob Herring
2013-12-31 16:33                 ` Rob Herring
2013-12-31 16:33                 ` Rob Herring
2014-01-06 23:41                 ` Paul Walmsley
2014-01-06 23:41                   ` Paul Walmsley
2014-01-06 23:41                   ` Paul Walmsley
2014-01-08  1:19                   ` Tony Lindgren
2014-01-08  1:19                     ` Tony Lindgren
2014-01-08  1:19                     ` Tony Lindgren
2013-11-23  1:07 ` Tony Lindgren
2013-11-23  1:07   ` Tony Lindgren
2013-11-23  1:07   ` Tony Lindgren
2013-11-24 21:36 ` Grant Likely
2013-11-24 21:36   ` Grant Likely
2013-11-24 21:36   ` Grant Likely
2013-11-25  9:25   ` Thierry Reding
2013-11-25  9:25     ` Thierry Reding
2013-11-25  9:25     ` Thierry Reding
     [not found]     ` < 20131125094954.GF22043@ulmo.nvidia.com>
2013-11-25  9:49     ` Thierry Reding
2013-11-25  9:49       ` Thierry Reding
2013-11-25  9:49       ` Thierry Reding
2013-11-25 19:50       ` Tony Lindgren
2013-11-25 19:50         ` Tony Lindgren
2013-11-27 15:56       ` Grant Likely
2013-11-27 15:56         ` Grant Likely
2013-11-28 15:46         ` Thierry Reding
2013-11-28 15:46           ` Thierry Reding
2013-11-28 15:46           ` Thierry Reding
2013-12-11 13:45           ` Grant Likely
2013-12-11 13:45             ` Grant Likely
2013-12-11 15:12             ` Thierry Reding
2013-12-11 15:12               ` Thierry Reding
2013-12-11 15:12               ` Thierry Reding
2013-12-11 16:43               ` Tony Lindgren
2013-12-11 16:43                 ` Tony Lindgren
2013-11-27 15:54     ` Grant Likely
2013-11-27 15:54       ` Grant Likely
2013-11-27 15:54       ` Grant Likely
2013-11-27 21:53   ` Tony Lindgren
2013-11-27 21:53     ` Tony Lindgren
2013-11-27 21:53     ` Tony Lindgren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.