| Message ID | 1395413185-29763-3-git-send-email-jjhiblot@traphandler.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 21, 2014 at 9:46 AM, Jean-Jacques Hiblot <jjhiblot@traphandler.com> wrote: > The goal of this patch is to allow drivers to be probed even if at the time of > the DT parsing some of their IRQ ressources are not available yet. > > In the current situation, the IRQ resources of a platform device are filled from > the DT at the time the device is created (of_device_alloc()). The drawback of > this is that a device sitting close to the top of the DT (ahb for example) but > depending on ressources that are initialized later (IRQ domain dynamically > created for example) will fail to probe because the ressources don't exist > at this time. s/ressources/resources/ > > This patch fills the resource structure only before the device is probed and > will defer the probe if the IRQ resource is not yet available. > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com> > --- > drivers/base/platform.c | 5 ++ > drivers/of/platform.c | 114 ++++++++++++++++++++++++++++++++++---------- > include/linux/device.h | 1 + > include/linux/of_platform.h | 10 ++++ > 4 files changed, 105 insertions(+), 25 deletions(-) > > diff --git a/drivers/base/platform.c b/drivers/base/platform.c > index bc78848..cee9b8d 100644 > --- a/drivers/base/platform.c > +++ b/drivers/base/platform.c > @@ -481,6 +481,10 @@ static int platform_drv_probe(struct device *_dev) > struct platform_device *dev = to_platform_device(_dev); > int ret; > > + ret = of_platform_device_prepare(dev); > + if (ret) > + goto error; > + > if (ACPI_HANDLE(_dev)) > acpi_dev_pm_attach(_dev, true); > > @@ -488,6 +492,7 @@ static int platform_drv_probe(struct device *_dev) > if (ret && ACPI_HANDLE(_dev)) > acpi_dev_pm_detach(_dev, true); > > +error: > if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) { > dev_warn(_dev, "probe deferral not supported\n"); > ret = -ENXIO; > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index 404d1da..ba6be4a 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -141,41 +141,17 @@ 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; > - struct resource *res, temp_res; > > dev = platform_device_alloc("", -1); > if (!dev) > return NULL; > > - /* count the io and irq resources */ > - 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); > - > - /* Populate the resource table */ > - if (num_irq || num_reg) { > - res = kzalloc(sizeof(*res) * (num_irq + num_reg), GFP_KERNEL); > - if (!res) { > - platform_device_put(dev); > - return NULL; > - } > - > - 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); > - } > - > dev->dev.of_node = of_node_get(np); > #if defined(CONFIG_MICROBLAZE) > dev->dev.dma_mask = &dev->archdata.dma_mask; > #endif > dev->dev.parent = parent; > + dev->dev.of_device = 1; > > if (bus_id) > dev_set_name(&dev->dev, "%s", bus_id); > @@ -233,6 +209,94 @@ static struct platform_device *of_platform_device_create_pdata( > return dev; > } > > +static int of_reg_count(struct device_node *np) This belongs in of/address.c. It can be more efficiently written as: int of_reg_count(struct device_node *dev) { const __be32 *prop; unsigned int psize; struct device_node *parent; struct of_bus *bus; int na, ns; /* Get parent & match bus type */ parent = of_get_parent(dev); if (!parent) return 0; bus = of_match_bus(parent); bus->count_cells(dev, &na, &ns); of_node_put(parent); if (!OF_CHECK_COUNTS(na, ns)) return 0; /* Get "reg" or "assigned-addresses" property */ prop = of_get_property(dev, bus->addresses, &psize); if (!prop) return 0; psize /= 4; return psize / (na + ns); } > +{ > + int nreg = 0; > + if (of_can_translate_address(np)) { > + struct resource temp_res; > + while (of_address_to_resource(np, nreg, &temp_res) == 0) > + nreg++; > + } > + return nreg; > +} > + > +int of_platform_device_prepare(struct platform_device *dev) > +{ > + struct device_node *np; > + int i, irq_index; > + struct resource *res; > + > + /* > + * This function applies only devices described in the DT and > + * created by of_device_alloc(). > + * Other platform devices have their ressources already populated. > + */ > + np = dev->dev.of_node; > + if (!np || !dev->dev.of_device) > + return 0; > + > + /* Populate the resource table */ > + if (!dev->resource) { > + int rc, nreg = 0, nirq; > + /* count the io and irq resources */ > + nreg = of_reg_count(np); > + nirq = of_irq_count(np); I think of_irq_count should be changed to return errors from of_irq_parse_one. It's complicated by the fact you want to distinguish checking the index is too big versus all other errors. > + > + if (!nirq && !nreg) > + return 0; > + > + res = kzalloc(sizeof(*res) * (nirq + nreg), GFP_KERNEL); > + if (!res) > + return -ENOMEM; > + > + dev->resource = res; > + dev->num_resources = nreg + nirq; > + > + for (i = 0; i < nreg; i++, res++) { > + rc = of_address_to_resource(np, i, res); > + if (WARN_ON(rc)) { > + /* THIS IS BAD; don't try to defer probing */ > + dev->num_resources = 0; > + dev->resource = NULL; > + kfree(res); You are incrementing res, so this kfree is wrong. Using "res + i" instead would work, but... > + return rc; > + } > + } > + > + if (!rc && of_irq_to_resource_table(np, res, nirq) != nirq) ...then res needs to change here. Doesn't this give a rc may not be initialized warning? You don't need to check rc. You need to check !nirq instead. > + /* > + * Not all irqs can be translated. redo the irq > + * resources to check if the probe can be deferred > + */ > + goto redo_irq_resources; > + > + return 0; But then, you don't really need any of this. You can just fall-thru and the code below will work. > + } > + > +redo_irq_resources: > + /* See which IRQ resources need to be redone */ > + irq_index = 0; > + for (i = 0, res = dev->resource; i < dev->num_resources; i++, res++) { > + if (!res->flags) { > + int rc; > + /* > + * If the IRQ can't be translated to a resource, check > + * if its IRQ domain exists. > + */ > + rc = of_irq_to_resource(np, irq_index, res); > + if (!rc) { > + if (of_find_irq_domain(np, irq_index) == NULL) > + return -EPROBE_DEFER; > + return rc; > + } I would move this check into of_irq_to_resource_table and also make it skip entries with IORESOURCE_IRQ set. Then this can become: for (i = 0, res = dev->resource; i < dev->num_resources; i++, res++) { if (resource_type(res) != IORESOURCE_MEM) break; } rc = of_irq_to_resource_table(np, res, dev->num_resources - i); return (rc < 0) ? rc : 0; Rob
* Rob Herring <robherring2@gmail.com> [140411 10:33]: > On Fri, Mar 21, 2014 at 9:46 AM, Jean-Jacques Hiblot > <jjhiblot@traphandler.com> wrote: > > The goal of this patch is to allow drivers to be probed even if at the time of > > the DT parsing some of their IRQ ressources are not available yet. > > > > In the current situation, the IRQ resources of a platform device are filled from > > the DT at the time the device is created (of_device_alloc()). The drawback of > > this is that a device sitting close to the top of the DT (ahb for example) but > > depending on ressources that are initialized later (IRQ domain dynamically > > created for example) will fail to probe because the ressources don't exist > > at this time. > > s/ressources/resources/ While I've tested these two patches and they fix the issue for me. I have some serious doubts again about this whole ------ up string parsing pile of ---- called device tree. Do we really need to sprinkle more of_* hacks to the irq subsystem? There's nothing wrong with with the irq subsystem. Platform bus is just calling the irq subsystem at the wrong time with uninitialized data. It seems that we're again papering over the fact that there's nothing coordinating the setting up of various resources with device tree. That seems to be the repeating never ending pattern as we've already seen with GPIOs, pinctrl, and IRQchips. We end up adding all kinds of cross subsystem calls that were never needed earlier. Regards, Tony
On Fri, Apr 18, 2014 at 3:52 PM, Tony Lindgren <tony@atomide.com> wrote: > * Rob Herring <robherring2@gmail.com> [140411 10:33]: >> On Fri, Mar 21, 2014 at 9:46 AM, Jean-Jacques Hiblot >> <jjhiblot@traphandler.com> wrote: >> > The goal of this patch is to allow drivers to be probed even if at the time of >> > the DT parsing some of their IRQ ressources are not available yet. >> > >> > In the current situation, the IRQ resources of a platform device are filled from >> > the DT at the time the device is created (of_device_alloc()). The drawback of >> > this is that a device sitting close to the top of the DT (ahb for example) but >> > depending on ressources that are initialized later (IRQ domain dynamically >> > created for example) will fail to probe because the ressources don't exist >> > at this time. >> >> s/ressources/resources/ > > While I've tested these two patches and they fix the issue for me. I have > some serious doubts again about this whole ------ up string parsing > pile of ---- called device tree. > > Do we really need to sprinkle more of_* hacks to the irq subsystem? > > There's nothing wrong with with the irq subsystem. Platform bus is just > calling the irq subsystem at the wrong time with uninitialized data. This patch doesn't even touch the irq subsystem. > It seems that we're again papering over the fact that there's nothing > coordinating the setting up of various resources with device tree. > That seems to be the repeating never ending pattern as we've already > seen with GPIOs, pinctrl, and IRQchips. We end up adding all kinds of > cross subsystem calls that were never needed earlier. This problem may be made worse with DT, but this problem has existed for as long as I have worked on the kernel. It is worse with DT since platforms have less control over the init ordering and loose some ability to do tricks like changing the initcall levels. We didn't even try to do pinctrl in any generic way before DT. Rob
* Rob Herring <robherring2@gmail.com> [140418 14:39]: > On Fri, Apr 18, 2014 at 3:52 PM, Tony Lindgren <tony@atomide.com> wrote: > > * Rob Herring <robherring2@gmail.com> [140411 10:33]: > >> On Fri, Mar 21, 2014 at 9:46 AM, Jean-Jacques Hiblot > >> <jjhiblot@traphandler.com> wrote: > >> > The goal of this patch is to allow drivers to be probed even if at the time of > >> > the DT parsing some of their IRQ ressources are not available yet. > >> > > >> > In the current situation, the IRQ resources of a platform device are filled from > >> > the DT at the time the device is created (of_device_alloc()). The drawback of > >> > this is that a device sitting close to the top of the DT (ahb for example) but > >> > depending on ressources that are initialized later (IRQ domain dynamically > >> > created for example) will fail to probe because the ressources don't exist > >> > at this time. > >> > >> s/ressources/resources/ > > > > While I've tested these two patches and they fix the issue for me. I have > > some serious doubts again about this whole ------ up string parsing > > pile of ---- called device tree. > > > > Do we really need to sprinkle more of_* hacks to the irq subsystem? > > > > There's nothing wrong with with the irq subsystem. Platform bus is just > > calling the irq subsystem at the wrong time with uninitialized data. > > This patch doesn't even touch the irq subsystem. Heh, it depends on the previous patch in this series adding another of_blah_blah function to drivers/of/irq.c. > > It seems that we're again papering over the fact that there's nothing > > coordinating the setting up of various resources with device tree. > > That seems to be the repeating never ending pattern as we've already > > seen with GPIOs, pinctrl, and IRQchips. We end up adding all kinds of > > cross subsystem calls that were never needed earlier. > > This problem may be made worse with DT, but this problem has existed > for as long as I have worked on the kernel. It is worse with DT since > platforms have less control over the init ordering and loose some > ability to do tricks like changing the initcall levels. We didn't even > try to do pinctrl in any generic way before DT. Oh come on, let's stop pretending it's not broken. And it's way worse with device tree as there's nothing making sure the resources for a driver are set up before the driver probes. And we've been unable to fix just this issue alone for about six months now. It's also broken beyond that. It's called of_platform_bus yet it won't even pass the platform_data as auxdata to the devices on a sub-bus instantatiated like I2C. Regards, Tony
On Fri, Apr 18, 2014 at 02:58:48PM -0700, Tony Lindgren wrote: > Oh come on, let's stop pretending it's not broken. And it's way worse with > device tree as there's nothing making sure the resources for a driver > are set up before the driver probes. And we've been unable to fix just > this issue alone for about six months now. It's also broken beyond that. > It's called of_platform_bus yet it won't even pass the platform_data > as auxdata to the devices on a sub-bus instantatiated like I2C. Isn't there a much simpler solution to the platform device IRQ problem? Rather than trying to fix it at the point where the resources are created, why not just *not* have DT create the IRQ resources in the first place, and instead have platform_get_irq() (which is the function which should be used to get an IRQ) be the actor to do whatever is necessary to return the IRQ(s) ? Yes, I know we have some drivers which use platform_get_resources() with IORESOURCE_IRQ, but they should really use the right accessor. And those who just dereference the resource array directly... get what's coming (though of course they have to be fixed.) It has the benefit that you're in a path where you /can/ return -EPROBE_DEFER too and not have to mess around with notifiers or other silly stuff like that.
* Russell King - ARM Linux <linux@arm.linux.org.uk> [140418 16:04]: > On Fri, Apr 18, 2014 at 02:58:48PM -0700, Tony Lindgren wrote: > > Oh come on, let's stop pretending it's not broken. And it's way worse with > > device tree as there's nothing making sure the resources for a driver > > are set up before the driver probes. And we've been unable to fix just > > this issue alone for about six months now. It's also broken beyond that. > > It's called of_platform_bus yet it won't even pass the platform_data > > as auxdata to the devices on a sub-bus instantatiated like I2C. > > Isn't there a much simpler solution to the platform device IRQ problem? > > Rather than trying to fix it at the point where the resources are > created, why not just *not* have DT create the IRQ resources in the > first place, and instead have platform_get_irq() (which is the function > which should be used to get an IRQ) be the actor to do whatever is > necessary to return the IRQ(s) ? Yeah why not. I don't see why we would need to do all this of_* special trickery for much anything beyond parsing the binding. > Yes, I know we have some drivers which use platform_get_resources() with > IORESOURCE_IRQ, but they should really use the right accessor. And those > who just dereference the resource array directly... get what's coming > (though of course they have to be fixed.) $ git grep IORESOURCE_IRQ drivers/ | grep platform_get_resource | wc -l 179 But might be scriptable to some extent.. > It has the benefit that you're in a path where you /can/ return > -EPROBE_DEFER too and not have to mess around with notifiers or other > silly stuff like that. And then maybe we can make of_platform_probe() or some bus function do the most of the -EPROBE_DEFER ping pong before the driver even probes? Regards, Tony
On Fri, Apr 18, 2014 at 6:24 PM, Tony Lindgren <tony@atomide.com> wrote: > * Russell King - ARM Linux <linux@arm.linux.org.uk> [140418 16:04]: >> On Fri, Apr 18, 2014 at 02:58:48PM -0700, Tony Lindgren wrote: >> > Oh come on, let's stop pretending it's not broken. And it's way worse with >> > device tree as there's nothing making sure the resources for a driver >> > are set up before the driver probes. And we've been unable to fix just >> > this issue alone for about six months now. It's also broken beyond that. >> > It's called of_platform_bus yet it won't even pass the platform_data >> > as auxdata to the devices on a sub-bus instantatiated like I2C. >> >> Isn't there a much simpler solution to the platform device IRQ problem? >> >> Rather than trying to fix it at the point where the resources are >> created, why not just *not* have DT create the IRQ resources in the >> first place, and instead have platform_get_irq() (which is the function >> which should be used to get an IRQ) be the actor to do whatever is >> necessary to return the IRQ(s) ? > > Yeah why not. I don't see why we would need to do all this of_* special > trickery for much anything beyond parsing the binding. That can work, but it will still need something like of_find_irq_domain() to determine whether to return -EPROBE_DEFER or not. You could also go in the other direction and don't create the device until the resources can be resolved. Unlike any of the other solutions, that would work for amba bus as well although we may never have a case where we need this with the amba bus. This would require making of_platform_populate be callable multiple times, but there are already some other reasons for wanting to do that. Specifically, I would like the core code to call of_platform_populate with default options and then only platforms with non-default options need a call to of_platform_populate. >> Yes, I know we have some drivers which use platform_get_resources() with >> IORESOURCE_IRQ, but they should really use the right accessor. And those >> who just dereference the resource array directly... get what's coming >> (though of course they have to be fixed.) > > $ git grep IORESOURCE_IRQ drivers/ | grep platform_get_resource | wc -l > 179 Certainly, this is worthwhile clean-up no matter what the solution. Rob
* Rob Herring <robherring2@gmail.com> [140421 06:47]: > On Fri, Apr 18, 2014 at 6:24 PM, Tony Lindgren <tony@atomide.com> wrote: > > * Russell King - ARM Linux <linux@arm.linux.org.uk> [140418 16:04]: > >> On Fri, Apr 18, 2014 at 02:58:48PM -0700, Tony Lindgren wrote: > >> > Oh come on, let's stop pretending it's not broken. And it's way worse with > >> > device tree as there's nothing making sure the resources for a driver > >> > are set up before the driver probes. And we've been unable to fix just > >> > this issue alone for about six months now. It's also broken beyond that. > >> > It's called of_platform_bus yet it won't even pass the platform_data > >> > as auxdata to the devices on a sub-bus instantatiated like I2C. > >> > >> Isn't there a much simpler solution to the platform device IRQ problem? > >> > >> Rather than trying to fix it at the point where the resources are > >> created, why not just *not* have DT create the IRQ resources in the > >> first place, and instead have platform_get_irq() (which is the function > >> which should be used to get an IRQ) be the actor to do whatever is > >> necessary to return the IRQ(s) ? > > > > Yeah why not. I don't see why we would need to do all this of_* special > > trickery for much anything beyond parsing the binding. > > That can work, but it will still need something like > of_find_irq_domain() to determine whether to return -EPROBE_DEFER or > not. Right. Naturally let's do whatever it takes to first fix this issue in a minimal way first for the -rc cycle so we can do the longer term changes needed. > You could also go in the other direction and don't create the device > until the resources can be resolved. Unlike any of the other > solutions, that would work for amba bus as well although we may never > have a case where we need this with the amba bus. This would require > making of_platform_populate be callable multiple times, but there are > already some other reasons for wanting to do that. Specifically, I > would like the core code to call of_platform_populate with default > options and then only platforms with non-default options need a call > to of_platform_populate. I like this idea as this would also probably remove the the numerous dmesg errors we are currently getting for drivers reprobing with -EPROBE_DEFER. In the long term we should have platform bus just call a set of standardized functions implemented by whatever the data source might be. That way we can limit the use of of_* functions in device drivers to just parsing of custom bindings in the drivers and use bus specific functions for everything else. > >> Yes, I know we have some drivers which use platform_get_resources() with > >> IORESOURCE_IRQ, but they should really use the right accessor. And those > >> who just dereference the resource array directly... get what's coming > >> (though of course they have to be fixed.) > > > > $ git grep IORESOURCE_IRQ drivers/ | grep platform_get_resource | wc -l > > 179 > > Certainly, this is worthwhile clean-up no matter what the solution. Yeah agreed. But let's also consider the IORESOURCE_IRQ as just another source for for the bus or driver data in addition to the DT parsed data. Both sources of data should work just fine with platform_bus even without cleaning up the drivers. Regards, Tony
On Mon, 21 Apr 2014 08:47:41 -0500, Rob Herring <robherring2@gmail.com> wrote: > On Fri, Apr 18, 2014 at 6:24 PM, Tony Lindgren <tony@atomide.com> wrote: > > * Russell King - ARM Linux <linux@arm.linux.org.uk> [140418 16:04]: > >> On Fri, Apr 18, 2014 at 02:58:48PM -0700, Tony Lindgren wrote: > >> > Oh come on, let's stop pretending it's not broken. And it's way worse with > >> > device tree as there's nothing making sure the resources for a driver > >> > are set up before the driver probes. And we've been unable to fix just > >> > this issue alone for about six months now. It's also broken beyond that. > >> > It's called of_platform_bus yet it won't even pass the platform_data > >> > as auxdata to the devices on a sub-bus instantatiated like I2C. > >> > >> Isn't there a much simpler solution to the platform device IRQ problem? > >> > >> Rather than trying to fix it at the point where the resources are > >> created, why not just *not* have DT create the IRQ resources in the > >> first place, and instead have platform_get_irq() (which is the function > >> which should be used to get an IRQ) be the actor to do whatever is > >> necessary to return the IRQ(s) ? > > > > Yeah why not. I don't see why we would need to do all this of_* special > > trickery for much anything beyond parsing the binding. > > That can work, but it will still need something like > of_find_irq_domain() to determine whether to return -EPROBE_DEFER or > not. > > You could also go in the other direction and don't create the device > until the resources can be resolved. Unlike any of the other > solutions, that would work for amba bus as well although we may never > have a case where we need this with the amba bus. This would require > making of_platform_populate be callable multiple times, but there are > already some other reasons for wanting to do that. Specifically, I > would like the core code to call of_platform_populate with default > options and then only platforms with non-default options need a call > to of_platform_populate. No it wouldn't. It would require of_platform_populate to do the right thing internally and know when to create devices from nodes that get flagged by calling of_platform_populate(). > >> Yes, I know we have some drivers which use platform_get_resources() with > >> IORESOURCE_IRQ, but they should really use the right accessor. And those > >> who just dereference the resource array directly... get what's coming > >> (though of course they have to be fixed.) > > > > $ git grep IORESOURCE_IRQ drivers/ | grep platform_get_resource | wc -l > > 179 > > Certainly, this is worthwhile clean-up no matter what the solution. Yes, and it works no matter what the data source is; platform_data, dt, acpi, and works on any platform. g.
diff --git a/drivers/base/platform.c b/drivers/base/platform.c index bc78848..cee9b8d 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -481,6 +481,10 @@ static int platform_drv_probe(struct device *_dev) struct platform_device *dev = to_platform_device(_dev); int ret; + ret = of_platform_device_prepare(dev); + if (ret) + goto error; + if (ACPI_HANDLE(_dev)) acpi_dev_pm_attach(_dev, true); @@ -488,6 +492,7 @@ static int platform_drv_probe(struct device *_dev) if (ret && ACPI_HANDLE(_dev)) acpi_dev_pm_detach(_dev, true); +error: if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) { dev_warn(_dev, "probe deferral not supported\n"); ret = -ENXIO; diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 404d1da..ba6be4a 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -141,41 +141,17 @@ 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; - struct resource *res, temp_res; dev = platform_device_alloc("", -1); if (!dev) return NULL; - /* count the io and irq resources */ - 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); - - /* Populate the resource table */ - if (num_irq || num_reg) { - res = kzalloc(sizeof(*res) * (num_irq + num_reg), GFP_KERNEL); - if (!res) { - platform_device_put(dev); - return NULL; - } - - 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); - } - dev->dev.of_node = of_node_get(np); #if defined(CONFIG_MICROBLAZE) dev->dev.dma_mask = &dev->archdata.dma_mask; #endif dev->dev.parent = parent; + dev->dev.of_device = 1; if (bus_id) dev_set_name(&dev->dev, "%s", bus_id); @@ -233,6 +209,94 @@ static struct platform_device *of_platform_device_create_pdata( return dev; } +static int of_reg_count(struct device_node *np) +{ + int nreg = 0; + if (of_can_translate_address(np)) { + struct resource temp_res; + while (of_address_to_resource(np, nreg, &temp_res) == 0) + nreg++; + } + return nreg; +} + +int of_platform_device_prepare(struct platform_device *dev) +{ + struct device_node *np; + int i, irq_index; + struct resource *res; + + /* + * This function applies only devices described in the DT and + * created by of_device_alloc(). + * Other platform devices have their ressources already populated. + */ + np = dev->dev.of_node; + if (!np || !dev->dev.of_device) + return 0; + + /* Populate the resource table */ + if (!dev->resource) { + int rc, nreg = 0, nirq; + /* count the io and irq resources */ + nreg = of_reg_count(np); + nirq = of_irq_count(np); + + if (!nirq && !nreg) + return 0; + + res = kzalloc(sizeof(*res) * (nirq + nreg), GFP_KERNEL); + if (!res) + return -ENOMEM; + + dev->resource = res; + dev->num_resources = nreg + nirq; + + for (i = 0; i < nreg; i++, res++) { + rc = of_address_to_resource(np, i, res); + if (WARN_ON(rc)) { + /* THIS IS BAD; don't try to defer probing */ + dev->num_resources = 0; + dev->resource = NULL; + kfree(res); + return rc; + } + } + + if (!rc && of_irq_to_resource_table(np, res, nirq) != nirq) + /* + * Not all irqs can be translated. redo the irq + * resources to check if the probe can be deferred + */ + goto redo_irq_resources; + + return 0; + } + +redo_irq_resources: + /* See which IRQ resources need to be redone */ + irq_index = 0; + for (i = 0, res = dev->resource; i < dev->num_resources; i++, res++) { + if (!res->flags) { + int rc; + /* + * If the IRQ can't be translated to a resource, check + * if its IRQ domain exists. + */ + rc = of_irq_to_resource(np, irq_index, res); + if (!rc) { + if (of_find_irq_domain(np, irq_index) == NULL) + return -EPROBE_DEFER; + return rc; + } + irq_index++; + } else if (res->flags & IORESOURCE_IRQ) + irq_index++; + } + return 0; +} +EXPORT_SYMBOL(of_platform_device_prepare); + /** * of_platform_device_create - Alloc, initialize and register an of_device * @np: pointer to node to create device for diff --git a/include/linux/device.h b/include/linux/device.h index 952b010..eb6ac66 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -785,6 +785,7 @@ struct device { bool offline_disabled:1; bool offline:1; + bool of_device:1; }; static inline struct device *kobj_to_dev(struct kobject *kobj) diff --git a/include/linux/of_platform.h b/include/linux/of_platform.h index 05cb4a9..4e487ff 100644 --- a/include/linux/of_platform.h +++ b/include/linux/of_platform.h @@ -53,6 +53,16 @@ struct of_dev_auxdata { extern const struct of_device_id of_default_bus_match_table[]; +/* Populate the resource for a platform device */ +#ifdef CONFIG_OF +int of_platform_device_prepare(struct platform_device *dev); +#else +static inline int of_platform_device_prepare( + struct platform_device *dev) +{ + return 0; +} +#endif /* Platform drivers register/unregister */ extern struct platform_device *of_device_alloc(struct device_node *np, const char *bus_id,
The goal of this patch is to allow drivers to be probed even if at the time of the DT parsing some of their IRQ ressources are not available yet. In the current situation, the IRQ resources of a platform device are filled from the DT at the time the device is created (of_device_alloc()). The drawback of this is that a device sitting close to the top of the DT (ahb for example) but depending on ressources that are initialized later (IRQ domain dynamically created for example) will fail to probe because the ressources don't exist at this time. This patch fills the resource structure only before the device is probed and will defer the probe if the IRQ resource is not yet available. Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com> --- drivers/base/platform.c | 5 ++ drivers/of/platform.c | 114 ++++++++++++++++++++++++++++++++++---------- include/linux/device.h | 1 + include/linux/of_platform.h | 10 ++++ 4 files changed, 105 insertions(+), 25 deletions(-)