From ab7c1e163b525316a870a494dd4ea196e7a6c455 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 15 Apr 2020 18:45:00 +0200 Subject: firmware: Drop unused pages field from struct firmware The struct firmware contains a page table pointer that was used only internally in the past. Since the actual page tables are referred from struct fw_priv and should be never from struct firmware, we can drop this unused field gracefully. Signed-off-by: Takashi Iwai Acked-by: Luis Chamberlain Link: https://lore.kernel.org/r/20200415164500.28749-1-tiwai@suse.de Signed-off-by: Greg Kroah-Hartman --- include/linux/firmware.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include/linux') diff --git a/include/linux/firmware.h b/include/linux/firmware.h index 4bbd0afd91b7..cb3e2c06ed8a 100644 --- a/include/linux/firmware.h +++ b/include/linux/firmware.h @@ -12,7 +12,6 @@ struct firmware { size_t size; const u8 *data; - struct page **pages; /* firmware loader private fields */ void *priv; -- cgit v1.2.3-71-gd317 From 716a7a25969003d82ab738179c3f1068a120ed11 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Thu, 14 May 2020 22:34:59 -0700 Subject: driver core: fw_devlink: Add support for batching fwnode parsing The amount of time spent parsing fwnodes of devices can become really high if the devices are added in an non-ideal order. Worst case can be O(N^2) when N devices are added. But this can be optimized to O(N) by adding all the devices and then parsing all their fwnodes in one batch. This commit adds fw_devlink_pause() and fw_devlink_resume() to allow doing this. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20200515053500.215929-4-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 1 + drivers/base/core.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++--- drivers/base/dd.c | 8 ++++ include/linux/fwnode.h | 2 + 4 files changed, 120 insertions(+), 7 deletions(-) (limited to 'include/linux') diff --git a/drivers/base/base.h b/drivers/base/base.h index 40fb069a8a7e..95c22c0f9036 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -153,6 +153,7 @@ extern char *make_class_name(const char *name, struct kobject *kobj); extern int devres_release_all(struct device *dev); extern void device_block_probing(void); extern void device_unblock_probing(void); +extern void driver_deferred_probe_force_trigger(void); /* /sys/devices directory */ extern struct kset *devices_kset; diff --git a/drivers/base/core.c b/drivers/base/core.c index f585d92e09d0..84c569726d75 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -49,6 +49,9 @@ static LIST_HEAD(wait_for_suppliers); static DEFINE_MUTEX(wfs_lock); static LIST_HEAD(deferred_sync); static unsigned int defer_sync_state_count = 1; +static unsigned int defer_fw_devlink_count; +static DEFINE_MUTEX(defer_fw_devlink_lock); +static bool fw_devlink_is_permissive(void); #ifdef CONFIG_SRCU static DEFINE_MUTEX(device_links_lock); @@ -527,7 +530,7 @@ static void device_link_add_missing_supplier_links(void) int ret = fwnode_call_int_op(dev->fwnode, add_links, dev); if (!ret) list_del_init(&dev->links.needs_suppliers); - else if (ret != -ENODEV) + else if (ret != -ENODEV || fw_devlink_is_permissive()) dev->links.need_for_probe = false; } mutex_unlock(&wfs_lock); @@ -1177,17 +1180,116 @@ static void fw_devlink_link_device(struct device *dev) { int fw_ret; - device_link_add_missing_supplier_links(); + if (!fw_devlink_flags) + return; + + mutex_lock(&defer_fw_devlink_lock); + if (!defer_fw_devlink_count) + device_link_add_missing_supplier_links(); + + /* + * The device's fwnode not having add_links() doesn't affect if other + * consumers can find this device as a supplier. So, this check is + * intentionally placed after device_link_add_missing_supplier_links(). + */ + if (!fwnode_has_op(dev->fwnode, add_links)) + goto out; - if (fw_devlink_flags && fwnode_has_op(dev->fwnode, add_links)) { + /* + * If fw_devlink is being deferred, assume all devices have mandatory + * suppliers they need to link to later. Then, when the fw_devlink is + * resumed, all these devices will get a chance to try and link to any + * suppliers they have. + */ + if (!defer_fw_devlink_count) { fw_ret = fwnode_call_int_op(dev->fwnode, add_links, dev); - if (fw_ret == -ENODEV && !fw_devlink_is_permissive()) - device_link_wait_for_mandatory_supplier(dev); - else if (fw_ret) - device_link_wait_for_optional_supplier(dev); + if (fw_ret == -ENODEV && fw_devlink_is_permissive()) + fw_ret = -EAGAIN; + } else { + fw_ret = -ENODEV; } + + if (fw_ret == -ENODEV) + device_link_wait_for_mandatory_supplier(dev); + else if (fw_ret) + device_link_wait_for_optional_supplier(dev); + +out: + mutex_unlock(&defer_fw_devlink_lock); } +/** + * fw_devlink_pause - Pause parsing of fwnode to create device links + * + * Calling this function defers any fwnode parsing to create device links until + * fw_devlink_resume() is called. Both these functions are ref counted and the + * caller needs to match the calls. + * + * While fw_devlink is paused: + * - Any device that is added won't have its fwnode parsed to create device + * links. + * - The probe of the device will also be deferred during this period. + * - Any devices that were already added, but waiting for suppliers won't be + * able to link to newly added devices. + * + * Once fw_devlink_resume(): + * - All the fwnodes that was not parsed will be parsed. + * - All the devices that were deferred probing will be reattempted if they + * aren't waiting for any more suppliers. + * + * This pair of functions, is mainly meant to optimize the parsing of fwnodes + * when a lot of devices that need to link to each other are added in a short + * interval of time. For example, adding all the top level devices in a system. + * + * For example, if N devices are added and: + * - All the consumers are added before their suppliers + * - All the suppliers of the N devices are part of the N devices + * + * Then: + * + * - With the use of fw_devlink_pause() and fw_devlink_resume(), each device + * will only need one parsing of its fwnode because it is guaranteed to find + * all the supplier devices already registered and ready to link to. It won't + * have to do another pass later to find one or more suppliers it couldn't + * find in the first parse of the fwnode. So, we'll only need O(N) fwnode + * parses. + * + * - Without the use of fw_devlink_pause() and fw_devlink_resume(), we would + * end up doing O(N^2) parses of fwnodes because every device that's added is + * guaranteed to trigger a parse of the fwnode of every device added before + * it. This O(N^2) parse is made worse by the fact that when a fwnode of a + * device is parsed, all it descendant devices might need to have their + * fwnodes parsed too (even if the devices themselves aren't added). + */ +void fw_devlink_pause(void) +{ + mutex_lock(&defer_fw_devlink_lock); + defer_fw_devlink_count++; + mutex_unlock(&defer_fw_devlink_lock); +} + +/** fw_devlink_resume - Resume parsing of fwnode to create device links + * + * This function is used in conjunction with fw_devlink_pause() and is ref + * counted. See documentation for fw_devlink_pause() for more details. + */ +void fw_devlink_resume(void) +{ + mutex_lock(&defer_fw_devlink_lock); + if (!defer_fw_devlink_count) { + WARN(true, "Unmatched fw_devlink pause/resume!"); + goto out; + } + + defer_fw_devlink_count--; + if (defer_fw_devlink_count) + goto out; + + device_link_add_missing_supplier_links(); + driver_deferred_probe_force_trigger(); +out: + mutex_unlock(&defer_fw_devlink_lock); +} /* Device links support end. */ int (*platform_notify)(struct device *dev) = NULL; diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 48ca81cb8ebc..63991d97adcc 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -160,6 +160,14 @@ static bool driver_deferred_probe_enable = false; * again. */ static void driver_deferred_probe_trigger(void) +{ + if (!driver_deferred_probe_enable) + return; + + driver_deferred_probe_force_trigger(); +} + +void driver_deferred_probe_force_trigger(void) { if (!driver_deferred_probe_enable) return; diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h index e0abafbb17f8..9506f8ec0974 100644 --- a/include/linux/fwnode.h +++ b/include/linux/fwnode.h @@ -171,5 +171,7 @@ struct fwnode_operations { #define get_dev_from_fwnode(fwnode) get_device((fwnode)->dev) extern u32 fw_devlink_get_flags(void); +void fw_devlink_pause(void); +void fw_devlink_resume(void); #endif -- cgit v1.2.3-71-gd317 From 46d26819a5056f4831649c5887ad5c71a16d86f7 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Sun, 24 May 2020 17:30:40 +0200 Subject: software node: implement software_node_unregister() Sometimes it is better to unregister individual nodes instead of trying to do them all at once with software_node_unregister_nodes(), so create software_node_unregister() so that you can unregister them one at a time. This is especially important when creating nodes in a hierarchy, with parent -> children representations. Children always need to be removed before a parent is, as the swnode logic assumes this is going to be the case. Fix up the lib/test_printf.c fwnode_pointer() test which to use this new function as it had the problem of tearing things down in the backwards order. Fixes: f1ce39df508d ("lib/test_printf: Add tests for %pfw printk modifier") Cc: stable Cc: Andy Shevchenko Cc: Brendan Higgins Cc: Dmitry Torokhov Cc: Petr Mladek Cc: Rafael J. Wysocki Cc: Rasmus Villemoes Cc: Sakari Ailus Cc: Sergey Senozhatsky Cc: Steven Rostedt Reported-by: Naresh Kamboju Reported-by: kernel test robot Reported-by: Randy Dunlap Tested-by: Petr Mladek Tested-by: Randy Dunlap Tested-by: Guenter Roeck Reviewed-by: Heikki Krogerus Acked-by: Randy Dunlap Link: https://lore.kernel.org/r/20200524153041.2361-1-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- drivers/base/swnode.c | 27 +++++++++++++++++++++------ include/linux/property.h | 1 + lib/test_printf.c | 4 +++- 3 files changed, 25 insertions(+), 7 deletions(-) (limited to 'include/linux') diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c index de8d3543e8fe..770b1f47a625 100644 --- a/drivers/base/swnode.c +++ b/drivers/base/swnode.c @@ -712,17 +712,18 @@ EXPORT_SYMBOL_GPL(software_node_register_nodes); * @nodes: Zero terminated array of software nodes to be unregistered * * Unregister multiple software nodes at once. + * + * NOTE: Be careful using this call if the nodes had parent pointers set up in + * them before registering. If so, it is wiser to remove the nodes + * individually, in the correct order (child before parent) instead of relying + * on the sequential order of the list of nodes in the array. */ void software_node_unregister_nodes(const struct software_node *nodes) { - struct swnode *swnode; int i; - for (i = 0; nodes[i].name; i++) { - swnode = software_node_to_swnode(&nodes[i]); - if (swnode) - fwnode_remove_software_node(&swnode->fwnode); - } + for (i = 0; nodes[i].name; i++) + software_node_unregister(&nodes[i]); } EXPORT_SYMBOL_GPL(software_node_unregister_nodes); @@ -741,6 +742,20 @@ int software_node_register(const struct software_node *node) } EXPORT_SYMBOL_GPL(software_node_register); +/** + * software_node_unregister - Unregister static software node + * @node: The software node to be unregistered + */ +void software_node_unregister(const struct software_node *node) +{ + struct swnode *swnode; + + swnode = software_node_to_swnode(node); + if (swnode) + fwnode_remove_software_node(&swnode->fwnode); +} +EXPORT_SYMBOL_GPL(software_node_unregister); + struct fwnode_handle * fwnode_create_software_node(const struct property_entry *properties, const struct fwnode_handle *parent) diff --git a/include/linux/property.h b/include/linux/property.h index d86de017c689..0d4099b4ce1f 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -441,6 +441,7 @@ int software_node_register_nodes(const struct software_node *nodes); void software_node_unregister_nodes(const struct software_node *nodes); int software_node_register(const struct software_node *node); +void software_node_unregister(const struct software_node *node); int software_node_notify(struct device *dev, unsigned long action); diff --git a/lib/test_printf.c b/lib/test_printf.c index 6b1622f4d7c2..fc63b8959d42 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -637,7 +637,9 @@ static void __init fwnode_pointer(void) test(second_name, "%pfwP", software_node_fwnode(&softnodes[1])); test(third_name, "%pfwP", software_node_fwnode(&softnodes[2])); - software_node_unregister_nodes(softnodes); + software_node_unregister(&softnodes[2]); + software_node_unregister(&softnodes[1]); + software_node_unregister(&softnodes[0]); } static void __init -- cgit v1.2.3-71-gd317