On Tue, Sep 17, 2013 at 01:04:06PM +0000, Strashko, Grygorii wrote:

Hi Thierry,

On 09/16/2013 11:32 AM, Thierry Reding wrote:> Interrupt references are currently resolved very early (when a device is

quoted

created). This has the disadvantage that it will fail in cases where the
interrupt parent hasn't been probed and no IRQ domain for it has been
registered yet. To work around that various drivers use explicit
initcall ordering to force interrupt parents to be probed before devices
that need them are created. That's error prone and doesn't always work.
If a platform device uses an interrupt line connected to a different
platform device (such as a GPIO controller), both will be created in the
same batch, and the GPIO controller won't have been probed by its driver
when the depending platform device is created. Interrupt resolution will
fail in that case.

Another common workaround is for drivers to explicitly resolve interrupt
references at probe time. This is suboptimal, however, because it will
require every driver to duplicate the code.

This patch adds support for late interrupt resolution to the platform
driver core, by resolving the references right before a device driver's
.probe() function will be called. This not only delays the resolution
until a much later time (giving interrupt parents a better chance of
being probed in the meantime), but it also allows the platform driver
core to queue the device for deferred probing if the interrupt parent
hasn't registered its IRQ domain yet.

Signed-off-by: Thierry Reding <redacted>
---
  drivers/base/platform.c     |  4 ++++
  drivers/of/platform.c       | 43 +++++++++++++++++++++++++++++++++++++------
  include/linux/of_platform.h |  7 +++++++
  3 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..8dcf835 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c

@@ -481,6 +481,10 @@ static int platform_drv_probe(struct device *_dev)

Should it be the part of really_probe()? Isn't it?

really_probe() takes a struct device and is in fact called by all types
of devices. This code, however, is highly platform_device specific, so I
don't think we can do it in really_probe().

Unfortunately every device type has its own way of storing interrupts.
Platform devices store them as resources, I2C clients store them as a
separate field in struct i2c_client, etc.

quoted

+int of_platform_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	int num_irq, ret = 0;
+
+	if (!pdev->dev.of_node)
+		return 0;
+
+	num_irq = of_irq_count(pdev->dev.of_node);
+	if (num_irq > 0) {
+		struct resource *res = pdev->resource;
+		int num_reg = pdev->num_resources;
+		int num = num_reg + num_irq;
+
+		res = krealloc(res, num * sizeof(*res), GFP_KERNEL);
+		if (!res)
+			return -ENOMEM;
+
+		pdev->num_resources = num;
+		pdev->resource = res;
+		res += num_reg;

What will happen if Driver probe is failed or deferred?
Seems resource table size will grow each time the Driver probe is
deferred or failed.

That's a very good point. I think what we can do is check whether the
total number of resources that the device has (pdev->num_resources)
corresponds to num_reg + num_irq and skip in that case. That and...

quoted

+		ret = of_irq_to_resource_table(np, res, num_irq);
+		if (ret < 0)
+			return ret;

... updating pdev->num_resources after this point should cover all
cases. Do you see any other potential problems?

Thierry