| Message ID | 1432565608-26036-7-git-send-email-tomeu.vizoso@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tomeu, On Mon, May 25, 2015 at 04:53:10PM +0200, Tomeu Vizoso wrote: > This function ensures that the device that encloses the passed device > node is registered, and thus probed if the corresponding driver has been > registered already. Even if the driver has already been registered this code will not guarantee that the device has been probed if driver enabled asynchronous probing (async probing should appear in 4.2 and the end goal is to have async probing enabled by default for most drivers). Also, why are we concentrating on platform drivers only? What about other devices, for example a gpio expander behind i2c bus? I am also concerned about adding OF-specific hooks into every subsystem out there. Can we make process of instantiating OF devices iterative instead and add them only if their parent has been already probed and also devices corresponding to all phandles they reference have also been probed? We could repeat scanning of the device tree every time we bind a driver and bulk-add leftovers at late_initcall time. This would mean that we could keep all logic in OF code (and maybe ACPI will add their own implementation) and keep other subsystems unaware. Thanks.
On 26 May 2015 at 20:56, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > Hi Tomeu, > > On Mon, May 25, 2015 at 04:53:10PM +0200, Tomeu Vizoso wrote: >> This function ensures that the device that encloses the passed device >> node is registered, and thus probed if the corresponding driver has been >> registered already. > > Even if the driver has already been registered this code will not > guarantee that the device has been probed if driver enabled asynchronous > probing (async probing should appear in 4.2 and the end goal is to have > async probing enabled by default for most drivers). Ok, I will look at that. Do you know if there's any board plugged into kernelci.org in which most drivers have async probing enabled already in linux-next? > Also, why are we concentrating on platform drivers only? What about > other devices, for example a gpio expander behind i2c bus? The current code will register the i2c master device when a device requests one of those gpios, and the gpio master device will be registered when the i2c master is probed. > I am also concerned about adding OF-specific hooks into every subsystem > out there. Well, those hooks are in the OF-specific code that we already added in every subsystem out there. But yeah, it would be great if we didn't had to add them. > Can we make process of instantiating OF devices iterative > instead and add them only if their parent has been already probed and This is what happens today when the mach code calls of_platform_populate on the root node, right? Or are you thinking of something different? > also devices corresponding to all phandles they reference have also been > probed? We could repeat scanning of the device tree every time we bind > a driver and bulk-add leftovers at late_initcall time. Yeah, this possibility was discussed in the thread linked from the cover letter. The main problem is that the knowledge required to infer from the phandles what devices are dependencies is in the DT bindings documentation. That knowledge is already codified in both each driver's probe function (for example when regulator_get_optional(dev, "phy") is called) and the functions that resolve dependencies when a device requests them (of_get_regulator(dev, "phy") in this example). That's why I thought of this approach, the main advantage of which is that it makes use of all that existing code without having to modify each driver and subsystem core. There's several ways to address this problem but require substantial refactoring. I just wanted to propose this alternative because it hadn't been discussed before and because I think it brings an interesting cost/benefit ratio. > This would > mean that we could keep all logic in OF code (and maybe ACPI will add > their own implementation) and keep other subsystems unaware. Yeah, that would be cool to have, but TBH I don't know if it's worth it. Regards, Tomeu > Thanks. > > -- > Dmitry > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index a01f57c..cc5d808 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -499,6 +499,57 @@ void of_platform_depopulate(struct device *parent) } EXPORT_SYMBOL_GPL(of_platform_depopulate); +/** + * of_platform_device_ensure - Ensure that device has been registered + * @np: pointer to node to create device for + * + * Ensures that a device has been registered for the given node. + */ +void of_platform_device_ensure(struct device_node *np) +{ + struct device_node *iter, *node = NULL; + int rc; + + /* Nothing to be done here */ + if (of_node_check_flag(np, OF_POPULATED)) + return; + + /* Too early for us to do anything useful */ + if (!platform_bus.kobj.state_initialized) + return; + + /* Find the node of the platform device enclosing this node */ + for (iter = np; + iter && iter->parent; + iter = iter->parent) { + if (of_get_property(iter, "compatible", NULL)) + node = iter; + + /* + * If the requested and requester nodes have a common ancestor + * that is being populated at this moment, register the device + * associated with the child of this ancestor that includes + * the requested node. + */ + if (of_node_check_flag(iter->parent, OF_POPULATED) && + of_match_node(of_default_bus_match_table, iter->parent)) + break; + + if (of_node_is_root(iter->parent)) + break; + } + + if (!node) + return; + + rc = of_platform_bus_create(node, of_default_bus_match_table, NULL, + NULL, true); + if (rc) + pr_debug("%s: creation of platform device failed\n", + np->full_name); +} +EXPORT_SYMBOL_GPL(of_platform_device_ensure); + #ifdef CONFIG_OF_DYNAMIC static int of_platform_notify(struct notifier_block *nb, unsigned long action, void *arg) diff --git a/include/linux/of_platform.h b/include/linux/of_platform.h index 611a691..df17890 100644 --- a/include/linux/of_platform.h +++ b/include/linux/of_platform.h @@ -73,6 +73,7 @@ extern int of_platform_populate(struct device_node *root, const struct of_dev_auxdata *lookup, struct device *parent); extern void of_platform_depopulate(struct device *parent); +extern void of_platform_device_ensure(struct device_node *np); #else static inline int of_platform_populate(struct device_node *root, const struct of_device_id *matches, @@ -82,6 +83,7 @@ static inline int of_platform_populate(struct device_node *root, return -ENODEV; } static inline void of_platform_depopulate(struct device *parent) { } +static inline void of_platform_device_ensure(struct device_node *np) { } #endif #if defined(CONFIG_OF_DYNAMIC) && defined(CONFIG_OF_ADDRESS)
This function ensures that the device that encloses the passed device node is registered, and thus probed if the corresponding driver has been registered already. This function can be used by drivers to ensure that a dependency is fulfilled. Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> --- drivers/of/platform.c | 51 +++++++++++++++++++++++++++++++++++++++++++++ include/linux/of_platform.h | 2 ++ 2 files changed, 53 insertions(+)