From 21774fd81a51ec1eccd59caf922d387977acd2aa Mon Sep 17 00:00:00 2001 From: Willem de Bruijn Date: Thu, 15 Oct 2020 14:57:26 -0400 Subject: [PATCH 01/39] kernfs: bring names in comments in line with code Fix two stragglers in the comments of the below rename operation. Fixes: adc5e8b58f48 ("kernfs: drop s_ prefix from kernfs_node members") Acked-by: Tejun Heo Signed-off-by: Willem de Bruijn Link: https://lore.kernel.org/r/20201015185726.1386868-1-willemdebruijn.kernel@gmail.com Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/dir.c | 2 +- include/linux/kernfs.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index 9aec80b9d7c6..ea3c95125bf1 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -1599,7 +1599,7 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent, return error; } -/* Relationship between s_mode and the DT_xxx types */ +/* Relationship between mode and the DT_xxx types */ static inline unsigned char dt_type(struct kernfs_node *kn) { return (kn->mode >> 12) & 15; diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h index 89f6a4214a70..9e8ca8743c26 100644 --- a/include/linux/kernfs.h +++ b/include/linux/kernfs.h @@ -116,7 +116,7 @@ struct kernfs_elem_attr { * kernfs node is represented by single kernfs_node. Most fields are * private to kernfs and shouldn't be accessed directly by kernfs users. * - * As long as s_count reference is held, the kernfs_node itself is + * As long as count reference is held, the kernfs_node itself is * accessible. Dereferencing elem or any other outer entity requires * active reference. */ From 7008e58c63bc8468e8d16154e25d780198b3ecfc Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Wed, 4 Nov 2020 12:54:30 -0800 Subject: [PATCH 02/39] driver core: Fix lockdep warning on wfs_lock There's a potential deadlock with the following cycle: wfs_lock --> device_links_lock --> kn->count Fix this by simply dropping the lock around a list_empty() check that's just exported to a sysfs file. The sysfs file output is an instantaneous check anyway and the lock doesn't really add any protection. Lockdep log: [ 48.808132] [ 48.808132] the existing dependency chain (in reverse order) is: [ 48.809069] [ 48.809069] -> #2 (kn->count){++++}: [ 48.809707] __kernfs_remove.llvm.7860393000964815146+0x2d4/0x460 [ 48.810537] kernfs_remove_by_name_ns+0x54/0x9c [ 48.811171] sysfs_remove_file_ns+0x18/0x24 [ 48.811762] device_del+0x2b8/0x5a8 [ 48.812269] __device_link_del+0x98/0xb8 [ 48.812829] device_links_driver_bound+0x210/0x2d8 [ 48.813496] driver_bound+0x44/0xf8 [ 48.814000] really_probe+0x340/0x6e0 [ 48.814526] driver_probe_device+0xb8/0x100 [ 48.815117] device_driver_attach+0x78/0xb8 [ 48.815708] __driver_attach+0xe0/0x194 [ 48.816255] bus_for_each_dev+0xa8/0x11c [ 48.816816] driver_attach+0x24/0x30 [ 48.817331] bus_add_driver+0x100/0x1e0 [ 48.817880] driver_register+0x78/0x114 [ 48.818427] __platform_driver_register+0x44/0x50 [ 48.819089] 0xffffffdbb3227038 [ 48.819551] do_one_initcall+0xd8/0x1e0 [ 48.820099] do_init_module+0xd8/0x298 [ 48.820636] load_module+0x3afc/0x44c8 [ 48.821173] __arm64_sys_finit_module+0xbc/0xf0 [ 48.821807] el0_svc_common+0xbc/0x1d0 [ 48.822344] el0_svc_handler+0x74/0x98 [ 48.822882] el0_svc+0x8/0xc [ 48.823310] [ 48.823310] -> #1 (device_links_lock){+.+.}: [ 48.824036] __mutex_lock_common+0xe0/0xe44 [ 48.824626] mutex_lock_nested+0x28/0x34 [ 48.825185] device_link_add+0xd4/0x4ec [ 48.825734] of_link_to_suppliers+0x158/0x204 [ 48.826347] of_fwnode_add_links+0x50/0x64 [ 48.826928] device_link_add_missing_supplier_links+0x90/0x11c [ 48.827725] fw_devlink_resume+0x58/0x130 [ 48.828296] of_platform_default_populate_init+0xb4/0xd0 [ 48.829030] do_one_initcall+0xd8/0x1e0 [ 48.829578] do_initcall_level+0xb8/0xcc [ 48.830137] do_basic_setup+0x60/0x7c [ 48.830662] kernel_init_freeable+0x128/0x1ac [ 48.831275] kernel_init+0x18/0x29c [ 48.831781] ret_from_fork+0x10/0x18 [ 48.832297] [ 48.832297] -> #0 (wfs_lock){+.+.}: [ 48.832922] __lock_acquire+0xe04/0x2e20 [ 48.833480] lock_acquire+0xbc/0xec [ 48.833984] __mutex_lock_common+0xe0/0xe44 [ 48.834577] mutex_lock_nested+0x28/0x34 [ 48.835136] waiting_for_supplier_show+0x3c/0x98 [ 48.835781] dev_attr_show+0x48/0xb4 [ 48.836295] sysfs_kf_seq_show+0xe8/0x184 [ 48.836864] kernfs_seq_show+0x48/0x8c [ 48.837401] seq_read+0x1c8/0x600 [ 48.837884] kernfs_fop_read+0x68/0x204 [ 48.838431] __vfs_read+0x60/0x214 [ 48.838925] vfs_read+0xbc/0x15c [ 48.839397] ksys_read+0x78/0xe4 [ 48.839869] __arm64_sys_read+0x1c/0x28 [ 48.840416] el0_svc_common+0xbc/0x1d0 [ 48.840953] el0_svc_handler+0x74/0x98 [ 48.841490] el0_svc+0x8/0xc [ 48.841917] [ 48.841917] other info that might help us debug this: [ 48.841917] [ 48.842920] Chain exists of: [ 48.842920] wfs_lock --> device_links_lock --> kn->count [ 48.842920] [ 48.844152] Possible unsafe locking scenario: [ 48.844152] [ 48.844895] CPU0 CPU1 [ 48.845463] ---- ---- [ 48.846032] lock(kn->count); [ 48.846417] lock(device_links_lock); [ 48.847203] lock(kn->count); [ 48.847902] lock(wfs_lock); [ 48.848276] [ 48.848276] *** DEADLOCK *** Reported-by: Cheng-Jui.Wang@mediatek.com Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201104205431.3795207-1-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index d661ada1518f..1165a80f8010 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1062,10 +1062,8 @@ static ssize_t waiting_for_supplier_show(struct device *dev, bool val; device_lock(dev); - mutex_lock(&wfs_lock); val = !list_empty(&dev->links.needs_suppliers) && dev->links.need_for_probe; - mutex_unlock(&wfs_lock); device_unlock(dev); return sysfs_emit(buf, "%u\n", val); } From 33c0c9bdf7a59051a654cd98b7d2b48ce0080967 Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Fri, 23 Oct 2020 18:32:56 +0200 Subject: [PATCH 03/39] drivers: base: fix some kernel-doc markups class_create is actually defined at the header. Fix the markup there and add a new one at the right place. While here, also fix some markups for functions that have different names between their prototypes and kernel-doc comments. Signed-off-by: Mauro Carvalho Chehab Link: https://lore.kernel.org/r/2fb6efd6a1f90d69ff73bf579566079cbb051e15.1603469755.git.mchehab+huawei@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/base/class.c | 2 +- drivers/base/devres.c | 2 +- drivers/base/firmware_loader/fallback.c | 2 +- include/linux/device/class.h | 14 ++++++++++++++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/drivers/base/class.c b/drivers/base/class.c index c3451481194e..7476f393df97 100644 --- a/drivers/base/class.c +++ b/drivers/base/class.c @@ -210,7 +210,7 @@ static void class_create_release(struct class *cls) } /** - * class_create - create a struct class structure + * __class_create - create a struct class structure * @owner: pointer to the module that is to "own" this struct class * @name: pointer to a string for the name of this class. * @key: the lock_class_key for this class; used by mutex lock debugging diff --git a/drivers/base/devres.c b/drivers/base/devres.c index 586e9a75c840..fb9d5289a620 100644 --- a/drivers/base/devres.c +++ b/drivers/base/devres.c @@ -149,7 +149,7 @@ void * __devres_alloc_node(dr_release_t release, size_t size, gfp_t gfp, int nid EXPORT_SYMBOL_GPL(__devres_alloc_node); #else /** - * devres_alloc - Allocate device resource data + * devres_alloc_node - Allocate device resource data * @release: Release function devres will be associated with * @size: Allocation size * @gfp: Allocation flags diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c index 4dec4b79ae06..91899d185e31 100644 --- a/drivers/base/firmware_loader/fallback.c +++ b/drivers/base/firmware_loader/fallback.c @@ -128,7 +128,7 @@ static ssize_t timeout_show(struct class *class, struct class_attribute *attr, } /** - * firmware_timeout_store() - set number of seconds to wait for firmware + * timeout_store() - set number of seconds to wait for firmware * @class: device class pointer * @attr: device attribute pointer * @buf: buffer to scan for timeout value diff --git a/include/linux/device/class.h b/include/linux/device/class.h index e8d470c457d1..e61ec5502019 100644 --- a/include/linux/device/class.h +++ b/include/linux/device/class.h @@ -256,6 +256,20 @@ extern void class_destroy(struct class *cls); /* This is a #define to keep the compiler from merging different * instances of the __key variable */ + +/** + * class_create - create a struct class structure + * @owner: pointer to the module that is to "own" this struct class + * @name: pointer to a string for the name of this class. + * + * This is used to create a struct class pointer that can then be used + * in calls to device_create(). + * + * Returns &struct class pointer on success, or ERR_PTR() on error. + * + * Note, the pointer created here is to be destroyed when finished by + * making a call to class_destroy(). + */ #define class_create(owner, name) \ ({ \ static struct lock_class_key __key; \ From 5812b32e01c6d86ba7a84110702b46d8a8531fe9 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 23 Nov 2020 11:23:12 +0100 Subject: [PATCH 04/39] of: fix linker-section match-table corruption Specify type alignment when declaring linker-section match-table entries to prevent gcc from increasing alignment and corrupting the various tables with padding (e.g. timers, irqchips, clocks, reserved memory). This is specifically needed on x86 where gcc (typically) aligns larger objects like struct of_device_id with static extent on 32-byte boundaries which at best prevents matching on anything but the first entry. Specifying alignment when declaring variables suppresses this optimisation. Here's a 64-bit example where all entries are corrupt as 16 bytes of padding has been inserted before the first entry: ffffffff8266b4b0 D __clk_of_table ffffffff8266b4c0 d __of_table_fixed_factor_clk ffffffff8266b5a0 d __of_table_fixed_clk ffffffff8266b680 d __clk_of_table_sentinel And here's a 32-bit example where the 8-byte-aligned table happens to be placed on a 32-byte boundary so that all but the first entry are corrupt due to the 28 bytes of padding inserted between entries: 812b3ec0 D __irqchip_of_table 812b3ec0 d __of_table_irqchip1 812b3fa0 d __of_table_irqchip2 812b4080 d __of_table_irqchip3 812b4160 d irqchip_of_match_end Verified on x86 using gcc-9.3 and gcc-4.9 (which uses 64-byte alignment), and on arm using gcc-7.2. Note that there are no in-tree users of these tables on x86 currently (even if they are included in the image). Fixes: 54196ccbe0ba ("of: consolidate linker section OF match table declarations") Fixes: f6e916b82022 ("irqchip: add basic infrastructure") Cc: stable # 3.9 Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20201123102319.8090-2-johan@kernel.org Signed-off-by: Greg Kroah-Hartman --- include/linux/of.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/of.h b/include/linux/of.h index 5d51891cbf1a..af655d264f10 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -1300,6 +1300,7 @@ static inline int of_get_available_child_count(const struct device_node *np) #define _OF_DECLARE(table, name, compat, fn, fn_type) \ static const struct of_device_id __of_table_##name \ __used __section("__" #table "_of_table") \ + __aligned(__alignof__(struct of_device_id)) \ = { .compatible = compat, \ .data = (fn == (fn_type)NULL) ? fn : fn } #else From 784b2c48ac12dcee27db001fb1a3c58c39380cb6 Mon Sep 17 00:00:00 2001 From: Dave Jiang Date: Fri, 4 Dec 2020 09:46:49 -0700 Subject: [PATCH 05/39] driver core: auxiliary bus: Fix auxiliary bus shutdown null auxdrv ptr If the probe of the auxdrv failed, the device->driver is set to NULL. During kernel shutdown, the bus shutdown will call auxdrv->shutdown and cause an invalid ptr dereference. Add check to make sure device->driver is not NULL before we proceed. Fixes: 7de3697e9cbd ("Add auxiliary bus support") Cc: Dave Ertman Signed-off-by: Dave Jiang Reviewed-by: Dan Williams Link: https://lore.kernel.org/r/160710040926.1889434.8840329810698403478.stgit@djiang5-desk3.ch.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/auxiliary.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c index f303daadf843..8336535f1e11 100644 --- a/drivers/base/auxiliary.c +++ b/drivers/base/auxiliary.c @@ -92,10 +92,15 @@ static int auxiliary_bus_remove(struct device *dev) static void auxiliary_bus_shutdown(struct device *dev) { - struct auxiliary_driver *auxdrv = to_auxiliary_drv(dev->driver); - struct auxiliary_device *auxdev = to_auxiliary_dev(dev); + struct auxiliary_driver *auxdrv = NULL; + struct auxiliary_device *auxdev; - if (auxdrv->shutdown) + if (dev->driver) { + auxdrv = to_auxiliary_drv(dev->driver); + auxdev = to_auxiliary_dev(dev); + } + + if (auxdrv && auxdrv->shutdown) auxdrv->shutdown(auxdev); } From 0288e7fa35b37fc91c6afec8c420f71d0ade853f Mon Sep 17 00:00:00 2001 From: Hui Su Date: Fri, 13 Nov 2020 21:21:43 +0800 Subject: [PATCH 06/39] fs/kernfs: remove the double check of dentry->inode In both kernfs_node_from_dentry() and in kernfs_dentry_node(), we will check the dentry->inode is NULL or not, which is superfluous. So remove the check in kernfs_node_from_dentry(). Acked-by: Tejun Heo Signed-off-by: Hui Su Link: https://lore.kernel.org/r/20201113132143.GA119541@rlk Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/dir.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index ea3c95125bf1..7a53eed69fef 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -604,8 +604,7 @@ const struct dentry_operations kernfs_dops = { */ struct kernfs_node *kernfs_node_from_dentry(struct dentry *dentry) { - if (dentry->d_sb->s_op == &kernfs_sops && - !d_really_is_negative(dentry)) + if (dentry->d_sb->s_op == &kernfs_sops) return kernfs_dentry_node(dentry); return NULL; } From 66482f640755b31cb94371ff6cef17400cda6db5 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 8 Dec 2020 20:03:26 +0100 Subject: [PATCH 07/39] driver: core: Fix list corruption after device_del() The device_links_purge() function (called from device_del()) tries to remove the links.needs_suppliers list entry, but it's using list_del(), hence it doesn't initialize after the removal. This is OK for normal cases where device_del() is called via device_destroy(). However, it's not guaranteed that the device object will be really deleted soon after device_del(). In a minor case like HD-audio codec reconfiguration that re-initializes the device after device_del(), it may lead to a crash by the corrupted list entry. As a simple fix, replace list_del() with list_del_init() in order to make the list intact after the device_del() call. Fixes: e2ae9bcc4aaa ("driver core: Add support for linking devices during device addition") Cc: Reviewed-by: Rafael J. Wysocki Signed-off-by: Takashi Iwai Link: https://lore.kernel.org/r/20201208190326.27531-1-tiwai@suse.de Cc: Saravana Kannan Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 1165a80f8010..ba5a3cac6571 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1384,7 +1384,7 @@ static void device_links_purge(struct device *dev) return; mutex_lock(&wfs_lock); - list_del(&dev->links.needs_suppliers); + list_del_init(&dev->links.needs_suppliers); mutex_unlock(&wfs_lock); /* From c95d64012ad7de2747923b0caf80e195e940606c Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:16 -0800 Subject: [PATCH 08/39] Revert "driver core: Avoid deferred probe due to fw_devlink_pause/resume()" This reverts commit 2451e746478a6a6e981cfa66b62b791ca93b90c8. fw_devlink_pause/resume() was an incomplete attempt at boot time optimization. That's going to get replaced by a much better optimization at the end of the series. Since fw_devlink_pause/resume() is going away, changes made for that can also go away. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-2-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 21 --------------------- include/linux/device.h | 3 +-- 2 files changed, 1 insertion(+), 23 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index ba5a3cac6571..58c7d17820a7 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -51,7 +51,6 @@ 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 LIST_HEAD(deferred_fw_devlink); static DEFINE_MUTEX(defer_fw_devlink_lock); static bool fw_devlink_is_permissive(void); @@ -1468,12 +1467,6 @@ static void fw_devlink_link_device(struct device *dev) fw_ret = -EAGAIN; } else { fw_ret = -ENODEV; - /* - * defer_hook is not used to add device to deferred_sync list - * until device is bound. Since deferred fw devlink also blocks - * probing, same list hook can be used for deferred_fw_devlink. - */ - list_add_tail(&dev->links.defer_hook, &deferred_fw_devlink); } if (fw_ret == -ENODEV) @@ -1542,9 +1535,6 @@ void fw_devlink_pause(void) */ void fw_devlink_resume(void) { - struct device *dev, *tmp; - LIST_HEAD(probe_list); - mutex_lock(&defer_fw_devlink_lock); if (!defer_fw_devlink_count) { WARN(true, "Unmatched fw_devlink pause/resume!"); @@ -1556,19 +1546,8 @@ void fw_devlink_resume(void) goto out; device_link_add_missing_supplier_links(); - list_splice_tail_init(&deferred_fw_devlink, &probe_list); out: mutex_unlock(&defer_fw_devlink_lock); - - /* - * bus_probe_device() can cause new devices to get added and they'll - * try to grab defer_fw_devlink_lock. So, this needs to be done outside - * the defer_fw_devlink_lock. - */ - list_for_each_entry_safe(dev, tmp, &probe_list, links.defer_hook) { - list_del_init(&dev->links.defer_hook); - bus_probe_device(dev); - } } /* Device links support end. */ diff --git a/include/linux/device.h b/include/linux/device.h index 5ed101be7b2e..da00f8e449bb 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -352,8 +352,7 @@ enum dl_dev_state { * @suppliers: List of links to supplier devices. * @consumers: List of links to consumer devices. * @needs_suppliers: Hook to global list of devices waiting for suppliers. - * @defer_hook: Hook to global list of devices that have deferred sync_state or - * deferred fw_devlink. + * @defer_hook: Hook to global list of devices that have deferred sync_state. * @need_for_probe: If needs_suppliers is on a list, this indicates if the * suppliers are needed for probe or not. * @status: Driver status information. From 3b052a3e30f2eb92dcae9fd89af48d5a13045737 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:17 -0800 Subject: [PATCH 09/39] Revert "driver core: Rename dev_links_info.defer_sync to defer_hook" This reverts commit ec7bd78498f29680f536451fbdf9464e851273ed. This field rename was done to reuse defer_syc list head for multiple lists. That's not needed anymore and this list head will only be used for defer sync. So revert this patch to avoid conflicts with the other reverts coming after this. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-3-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 22 +++++++++++----------- include/linux/device.h | 4 ++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 58c7d17820a7..fc4b0bc59935 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -959,11 +959,11 @@ static void __device_links_queue_sync_state(struct device *dev, */ dev->state_synced = true; - if (WARN_ON(!list_empty(&dev->links.defer_hook))) + if (WARN_ON(!list_empty(&dev->links.defer_sync))) return; get_device(dev); - list_add_tail(&dev->links.defer_hook, list); + list_add_tail(&dev->links.defer_sync, list); } /** @@ -981,8 +981,8 @@ static void device_links_flush_sync_list(struct list_head *list, { struct device *dev, *tmp; - list_for_each_entry_safe(dev, tmp, list, links.defer_hook) { - list_del_init(&dev->links.defer_hook); + list_for_each_entry_safe(dev, tmp, list, links.defer_sync) { + list_del_init(&dev->links.defer_sync); if (dev != dont_lock_dev) device_lock(dev); @@ -1020,12 +1020,12 @@ void device_links_supplier_sync_state_resume(void) if (defer_sync_state_count) goto out; - list_for_each_entry_safe(dev, tmp, &deferred_sync, links.defer_hook) { + list_for_each_entry_safe(dev, tmp, &deferred_sync, links.defer_sync) { /* * Delete from deferred_sync list before queuing it to - * sync_list because defer_hook is used for both lists. + * sync_list because defer_sync is used for both lists. */ - list_del_init(&dev->links.defer_hook); + list_del_init(&dev->links.defer_sync); __device_links_queue_sync_state(dev, &sync_list); } out: @@ -1043,8 +1043,8 @@ late_initcall(sync_state_resume_initcall); static void __device_links_supplier_defer_sync(struct device *sup) { - if (list_empty(&sup->links.defer_hook) && dev_has_sync_state(sup)) - list_add_tail(&sup->links.defer_hook, &deferred_sync); + if (list_empty(&sup->links.defer_sync) && dev_has_sync_state(sup)) + list_add_tail(&sup->links.defer_sync, &deferred_sync); } static void device_link_drop_managed(struct device_link *link) @@ -1272,7 +1272,7 @@ void device_links_driver_cleanup(struct device *dev) WRITE_ONCE(link->status, DL_STATE_DORMANT); } - list_del_init(&dev->links.defer_hook); + list_del_init(&dev->links.defer_sync); __device_links_no_driver(dev); device_links_write_unlock(); @@ -2405,7 +2405,7 @@ void device_initialize(struct device *dev) INIT_LIST_HEAD(&dev->links.consumers); INIT_LIST_HEAD(&dev->links.suppliers); INIT_LIST_HEAD(&dev->links.needs_suppliers); - INIT_LIST_HEAD(&dev->links.defer_hook); + INIT_LIST_HEAD(&dev->links.defer_sync); dev->links.status = DL_DEV_NO_DRIVER; } EXPORT_SYMBOL_GPL(device_initialize); diff --git a/include/linux/device.h b/include/linux/device.h index da00f8e449bb..1e771ea4dca6 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -352,7 +352,7 @@ enum dl_dev_state { * @suppliers: List of links to supplier devices. * @consumers: List of links to consumer devices. * @needs_suppliers: Hook to global list of devices waiting for suppliers. - * @defer_hook: Hook to global list of devices that have deferred sync_state. + * @defer_sync: Hook to global list of devices that have deferred sync_state. * @need_for_probe: If needs_suppliers is on a list, this indicates if the * suppliers are needed for probe or not. * @status: Driver status information. @@ -361,7 +361,7 @@ struct dev_links_info { struct list_head suppliers; struct list_head consumers; struct list_head needs_suppliers; - struct list_head defer_hook; + struct list_head defer_sync; bool need_for_probe; enum dl_dev_state status; }; From 96d8a9168ef5539beba9951dffe6eda07b74833e Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:18 -0800 Subject: [PATCH 10/39] Revert "driver core: Don't do deferred probe in parallel with kernel_init thread" This reverts commit cec72f3efc6272420c2c2c699607f03d09b93e41. Commit cec72f3efc62 ("driver core: Don't do deferred probe in parallel with kernel_init thread") was fixing a commit 716a7a259690 ("driver core: fw_devlink: Add support for batching fwnode parsing"). Since the commit being fixed itself is going to be reverted, the fix can also be reverted. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-4-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 1 + drivers/base/core.c | 1 + drivers/base/dd.c | 5 +++++ 3 files changed, 7 insertions(+) diff --git a/drivers/base/base.h b/drivers/base/base.h index 91cfb8405abd..c3562adf4789 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -156,6 +156,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 fc4b0bc59935..b1189697c29b 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1546,6 +1546,7 @@ void fw_devlink_resume(void) goto out; device_link_add_missing_supplier_links(); + driver_deferred_probe_force_trigger(); out: mutex_unlock(&defer_fw_devlink_lock); } diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 148e81969e04..001caa0b447c 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -167,6 +167,11 @@ 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) +{ /* * A successful probe means that all the devices in the pending list * should be triggered to be reprobed. Move all the deferred devices From 999032ece38be271ca28a6c0ba5f2e4dd4012425 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:19 -0800 Subject: [PATCH 11/39] Revert "driver core: Remove check in driver_deferred_probe_force_trigger()" This reverts commit fefcfc968723caf93318613a08e1f3ad07a6154f. The reverted commit is fixing commit 716a7a259690 ("driver core: fw_devlink: Add support for batching fwnode parsing"). Since the original commit will be reverted, the fix can be reverted too. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-5-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 001caa0b447c..b4be35fa7fda 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -172,6 +172,9 @@ static void driver_deferred_probe_trigger(void) void driver_deferred_probe_force_trigger(void) { + if (!driver_deferred_probe_enable) + return; + /* * A successful probe means that all the devices in the pending list * should be triggered to be reprobed. Move all the deferred devices From 087ad763c15477fba4324d7312866946b7f5ed30 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:20 -0800 Subject: [PATCH 12/39] Revert "of: platform: Batch fwnode parsing when adding all top level devices" This reverts commit 93d2e4322aa74c1ad1e8c2160608eb9a960d69ff. The fw_devlink_pause/resume() optimization attempt is getting replaced with a much more robust optimization by the end of this series. So, stop using those APIs. Acked-by: Rob Herring Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-6-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index b557a0fcd4ba..79bd5f5a1bf1 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -538,9 +538,7 @@ static int __init of_platform_default_populate_init(void) } /* Populate everything else. */ - fw_devlink_pause(); of_platform_default_populate(NULL, NULL, NULL); - fw_devlink_resume(); return 0; } From c84b90909e475a2eb4934b4d92fdd10e73e75805 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:21 -0800 Subject: [PATCH 13/39] Revert "driver core: fw_devlink: Add support for batching fwnode parsing" This reverts commit 716a7a25969003d82ab738179c3f1068a120ed11. The fw_devlink_pause/resume() APIs added by the commit being reverted were a first cut attempt at optimizing boot time. But these APIs don't fully solve the problem and are very fragile (can only be used for the top level devices being added). This series replaces them with a much better optimization that works for all device additions and also has the benefit of reducing the complexity of the firmware (DT, EFI) specific code and abstracting out common code to driver core. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-7-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 1 - drivers/base/core.c | 122 ++++------------------------------------- drivers/base/dd.c | 8 --- include/linux/fwnode.h | 2 - 4 files changed, 10 insertions(+), 123 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index c3562adf4789..91cfb8405abd 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -156,7 +156,6 @@ 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 b1189697c29b..670aa452bfe7 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -50,9 +50,6 @@ 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); @@ -758,7 +755,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 || fw_devlink_is_permissive()) + else if (ret != -ENODEV) dev->links.need_for_probe = false; } mutex_unlock(&wfs_lock); @@ -1440,116 +1437,17 @@ static void fw_devlink_link_device(struct device *dev) { int fw_ret; - 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 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()) - 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); + + if (fw_devlink_flags && fwnode_has_op(dev->fwnode, add_links)) { + 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); + } } + /* Device links support end. */ int (*platform_notify)(struct device *dev) = NULL; diff --git a/drivers/base/dd.c b/drivers/base/dd.c index b4be35fa7fda..148e81969e04 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -163,14 +163,6 @@ 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 9506f8ec0974..e0abafbb17f8 100644 --- a/include/linux/fwnode.h +++ b/include/linux/fwnode.h @@ -171,7 +171,5 @@ 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 From 01bb86b380a306bd937c96da36f66429f3362137 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:22 -0800 Subject: [PATCH 14/39] driver core: Add fwnode_init() There are multiple locations in the kernel where a struct fwnode_handle is initialized. Add fwnode_init() so that we have one way of initializing a fwnode_handle. Acked-by: Rob Herring Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-8-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/acpi/property.c | 2 +- drivers/acpi/scan.c | 2 +- drivers/base/swnode.c | 2 +- drivers/firmware/efi/efi-init.c | 8 ++++---- include/linux/fwnode.h | 6 ++++++ include/linux/of.h | 2 +- kernel/irq/irqdomain.c | 2 +- 7 files changed, 15 insertions(+), 9 deletions(-) diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c index d04de10a63e4..24e87b630573 100644 --- a/drivers/acpi/property.c +++ b/drivers/acpi/property.c @@ -76,7 +76,7 @@ static bool acpi_nondev_subnode_extract(const union acpi_object *desc, return false; dn->name = link->package.elements[0].string.pointer; - dn->fwnode.ops = &acpi_data_fwnode_ops; + fwnode_init(&dn->fwnode, &acpi_data_fwnode_ops); dn->parent = parent; INIT_LIST_HEAD(&dn->data.properties); INIT_LIST_HEAD(&dn->data.subnodes); diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index a896e5e87c93..0ac19f9274b8 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1589,7 +1589,7 @@ void acpi_init_device_object(struct acpi_device *device, acpi_handle handle, device->device_type = type; device->handle = handle; device->parent = acpi_bus_get_parent(handle); - device->fwnode.ops = &acpi_device_fwnode_ops; + fwnode_init(&device->fwnode, &acpi_device_fwnode_ops); acpi_set_device_status(device, sta); acpi_device_get_busid(device); acpi_set_pnp_ids(handle, &device->pnp, type); diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c index 010828fc785b..4a4b2008fbc2 100644 --- a/drivers/base/swnode.c +++ b/drivers/base/swnode.c @@ -653,7 +653,7 @@ swnode_register(const struct software_node *node, struct swnode *parent, swnode->parent = parent; swnode->allocated = allocated; swnode->kobj.kset = swnode_kset; - swnode->fwnode.ops = &software_node_ops; + fwnode_init(&swnode->fwnode, &software_node_ops); ida_init(&swnode->child_ids); INIT_LIST_HEAD(&swnode->entry); diff --git a/drivers/firmware/efi/efi-init.c b/drivers/firmware/efi/efi-init.c index f55a92ff12c0..b148f1459fb3 100644 --- a/drivers/firmware/efi/efi-init.c +++ b/drivers/firmware/efi/efi-init.c @@ -359,9 +359,7 @@ static const struct fwnode_operations efifb_fwnode_ops = { .add_links = efifb_add_links, }; -static struct fwnode_handle efifb_fwnode = { - .ops = &efifb_fwnode_ops, -}; +static struct fwnode_handle efifb_fwnode; static int __init register_gop_device(void) { @@ -375,8 +373,10 @@ static int __init register_gop_device(void) if (!pd) return -ENOMEM; - if (IS_ENABLED(CONFIG_PCI)) + if (IS_ENABLED(CONFIG_PCI)) { + fwnode_init(&efifb_fwnode, &efifb_fwnode_ops); pd->dev.fwnode = &efifb_fwnode; + } err = platform_device_add_data(pd, &screen_info, sizeof(screen_info)); if (err) diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h index e0abafbb17f8..5589799708b5 100644 --- a/include/linux/fwnode.h +++ b/include/linux/fwnode.h @@ -170,6 +170,12 @@ struct fwnode_operations { } while (false) #define get_dev_from_fwnode(fwnode) get_device((fwnode)->dev) +static inline void fwnode_init(struct fwnode_handle *fwnode, + const struct fwnode_operations *ops) +{ + fwnode->ops = ops; +} + extern u32 fw_devlink_get_flags(void); #endif diff --git a/include/linux/of.h b/include/linux/of.h index af655d264f10..df71a3a1bb8d 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -108,7 +108,7 @@ static inline void of_node_init(struct device_node *node) #if defined(CONFIG_OF_KOBJ) kobject_init(&node->kobj, &of_node_ktype); #endif - node->fwnode.ops = &of_fwnode_ops; + fwnode_init(&node->fwnode, &of_fwnode_ops); } #if defined(CONFIG_OF_KOBJ) diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index cf8b374b892d..06fce7e39033 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -91,7 +91,7 @@ struct fwnode_handle *__irq_domain_alloc_fwnode(unsigned int type, int id, fwid->type = type; fwid->name = n; fwid->pa = pa; - fwid->fwnode.ops = &irqchip_fwnode_ops; + fwnode_init(&fwid->fwnode, &irqchip_fwnode_ops); return &fwid->fwnode; } EXPORT_SYMBOL_GPL(__irq_domain_alloc_fwnode); From 7b337cb3ebde384cba7405b61dfb84200bf623bf Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:23 -0800 Subject: [PATCH 15/39] driver core: Add fwnode link support Add support for creating supplier-consumer links between fwnodes. It is intended for internal use the driver core and generic firmware support code (eg. Device Tree, ACPI), so it is simple by design and the API provided is limited. Acked-by: Rob Herring Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-9-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 98 ++++++++++++++++++++++++++++++++++++++++++ drivers/of/dynamic.c | 1 + include/linux/fwnode.h | 14 ++++++ 3 files changed, 113 insertions(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index 670aa452bfe7..972d42dedfc8 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -50,6 +50,104 @@ 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 DEFINE_MUTEX(fwnode_link_lock); + +/** + * fwnode_link_add - Create a link between two fwnode_handles. + * @con: Consumer end of the link. + * @sup: Supplier end of the link. + * + * Create a fwnode link between fwnode handles @con and @sup. The fwnode link + * represents the detail that the firmware lists @sup fwnode as supplying a + * resource to @con. + * + * The driver core will use the fwnode link to create a device link between the + * two device objects corresponding to @con and @sup when they are created. The + * driver core will automatically delete the fwnode link between @con and @sup + * after doing that. + * + * Attempts to create duplicate links between the same pair of fwnode handles + * are ignored and there is no reference counting. + */ +int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup) +{ + struct fwnode_link *link; + int ret = 0; + + mutex_lock(&fwnode_link_lock); + + list_for_each_entry(link, &sup->consumers, s_hook) + if (link->consumer == con) + goto out; + + link = kzalloc(sizeof(*link), GFP_KERNEL); + if (!link) { + ret = -ENOMEM; + goto out; + } + + link->supplier = sup; + INIT_LIST_HEAD(&link->s_hook); + link->consumer = con; + INIT_LIST_HEAD(&link->c_hook); + + list_add(&link->s_hook, &sup->consumers); + list_add(&link->c_hook, &con->suppliers); +out: + mutex_unlock(&fwnode_link_lock); + + return ret; +} + +/** + * fwnode_links_purge_suppliers - Delete all supplier links of fwnode_handle. + * @fwnode: fwnode whose supplier links need to be deleted + * + * Deletes all supplier links connecting directly to @fwnode. + */ +static void fwnode_links_purge_suppliers(struct fwnode_handle *fwnode) +{ + struct fwnode_link *link, *tmp; + + mutex_lock(&fwnode_link_lock); + list_for_each_entry_safe(link, tmp, &fwnode->suppliers, c_hook) { + list_del(&link->s_hook); + list_del(&link->c_hook); + kfree(link); + } + mutex_unlock(&fwnode_link_lock); +} + +/** + * fwnode_links_purge_consumers - Delete all consumer links of fwnode_handle. + * @fwnode: fwnode whose consumer links need to be deleted + * + * Deletes all consumer links connecting directly to @fwnode. + */ +static void fwnode_links_purge_consumers(struct fwnode_handle *fwnode) +{ + struct fwnode_link *link, *tmp; + + mutex_lock(&fwnode_link_lock); + list_for_each_entry_safe(link, tmp, &fwnode->consumers, s_hook) { + list_del(&link->s_hook); + list_del(&link->c_hook); + kfree(link); + } + mutex_unlock(&fwnode_link_lock); +} + +/** + * fwnode_links_purge - Delete all links connected to a fwnode_handle. + * @fwnode: fwnode whose links needs to be deleted + * + * Deletes all links connecting directly to a fwnode. + */ +void fwnode_links_purge(struct fwnode_handle *fwnode) +{ + fwnode_links_purge_suppliers(fwnode); + fwnode_links_purge_consumers(fwnode); +} #ifdef CONFIG_SRCU static DEFINE_MUTEX(device_links_lock); diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index fe64430b438a..9a824decf61f 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -356,6 +356,7 @@ void of_node_release(struct kobject *kobj) property_list_free(node->properties); property_list_free(node->deadprops); + fwnode_links_purge(of_fwnode_handle(node)); kfree(node->full_name); kfree(node->data); diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h index 5589799708b5..b88365187347 100644 --- a/include/linux/fwnode.h +++ b/include/linux/fwnode.h @@ -10,6 +10,7 @@ #define _LINUX_FWNODE_H_ #include +#include struct fwnode_operations; struct device; @@ -18,6 +19,15 @@ struct fwnode_handle { struct fwnode_handle *secondary; const struct fwnode_operations *ops; struct device *dev; + struct list_head suppliers; + struct list_head consumers; +}; + +struct fwnode_link { + struct fwnode_handle *supplier; + struct list_head s_hook; + struct fwnode_handle *consumer; + struct list_head c_hook; }; /** @@ -174,8 +184,12 @@ static inline void fwnode_init(struct fwnode_handle *fwnode, const struct fwnode_operations *ops) { fwnode->ops = ops; + INIT_LIST_HEAD(&fwnode->consumers); + INIT_LIST_HEAD(&fwnode->suppliers); } extern u32 fw_devlink_get_flags(void); +int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup); +void fwnode_links_purge(struct fwnode_handle *fwnode); #endif From ac66c5bbb4371073dcace77e47c234d2e36a006b Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:24 -0800 Subject: [PATCH 16/39] driver core: Allow only unprobed consumers for SYNC_STATE_ONLY device links SYNC_STATE_ONLY device links only affect the behavior of sync_state() callbacks. Specifically, they prevent sync_state() only callbacks from being called on a device if one or more of its consumers haven't probed. So, creating a SYNC_STATE_ONLY device link from an already probed consumer is useless. So, don't allow creating such device links. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-10-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index 972d42dedfc8..edde79fc3d33 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -649,6 +649,17 @@ struct device_link *device_link_add(struct device *consumer, goto out; } + /* + * SYNC_STATE_ONLY links are useless once a consumer device has probed. + * So, only create it if the consumer hasn't probed yet. + */ + if (flags & DL_FLAG_SYNC_STATE_ONLY && + consumer->links.status != DL_DEV_NO_DRIVER && + consumer->links.status != DL_DEV_PROBING) { + link = NULL; + goto out; + } + /* * DL_FLAG_AUTOREMOVE_SUPPLIER indicates that the link will be needed * longer than for DL_FLAG_AUTOREMOVE_CONSUMER and setting them both From b5d3e2fbcb10957521af14c4256cd0e5f68b9234 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:25 -0800 Subject: [PATCH 17/39] device property: Add fwnode_is_ancestor_of() and fwnode_get_next_parent_dev() Add fwnode_is_ancestor_of() helper function to check if a fwnode is an ancestor of another fwnode. Add fwnode_get_next_parent_dev() helper function that take as input a fwnode and finds the closest ancestor fwnode that has a corresponding struct device and returns that struct device. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-11-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/property.c | 52 ++++++++++++++++++++++++++++++++++++++++ include/linux/property.h | 3 +++ 2 files changed, 55 insertions(+) diff --git a/drivers/base/property.c b/drivers/base/property.c index 4c43d30145c6..35b95c6ac0c6 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -614,6 +614,31 @@ struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode) } EXPORT_SYMBOL_GPL(fwnode_get_next_parent); +/** + * fwnode_get_next_parent_dev - Find device of closest ancestor fwnode + * @fwnode: firmware node + * + * Given a firmware node (@fwnode), this function finds its closest ancestor + * firmware node that has a corresponding struct device and returns that struct + * device. + * + * The caller of this function is expected to call put_device() on the returned + * device when they are done. + */ +struct device *fwnode_get_next_parent_dev(struct fwnode_handle *fwnode) +{ + struct device *dev = NULL; + + fwnode_handle_get(fwnode); + do { + fwnode = fwnode_get_next_parent(fwnode); + if (fwnode) + dev = get_dev_from_fwnode(fwnode); + } while (fwnode && !dev); + fwnode_handle_put(fwnode); + return dev; +} + /** * fwnode_count_parents - Return the number of parents a node has * @fwnode: The node the parents of which are to be counted @@ -660,6 +685,33 @@ struct fwnode_handle *fwnode_get_nth_parent(struct fwnode_handle *fwnode, } EXPORT_SYMBOL_GPL(fwnode_get_nth_parent); +/** + * fwnode_is_ancestor_of - Test if @test_ancestor is ancestor of @test_child + * @test_ancestor: Firmware which is tested for being an ancestor + * @test_child: Firmware which is tested for being the child + * + * A node is considered an ancestor of itself too. + * + * Returns true if @test_ancestor is an ancestor of @test_child. + * Otherwise, returns false. + */ +bool fwnode_is_ancestor_of(struct fwnode_handle *test_ancestor, + struct fwnode_handle *test_child) +{ + if (!test_ancestor) + return false; + + fwnode_handle_get(test_child); + while (test_child) { + if (test_child == test_ancestor) { + fwnode_handle_put(test_child); + return true; + } + test_child = fwnode_get_next_parent(test_child); + } + return false; +} + /** * fwnode_get_next_child_node - Return the next child node handle for a node * @fwnode: Firmware node to find the next child node for. diff --git a/include/linux/property.h b/include/linux/property.h index 2d4542629d80..0a9001fe7aea 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -85,9 +85,12 @@ const char *fwnode_get_name_prefix(const struct fwnode_handle *fwnode); struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode); struct fwnode_handle *fwnode_get_next_parent( struct fwnode_handle *fwnode); +struct device *fwnode_get_next_parent_dev(struct fwnode_handle *fwnode); unsigned int fwnode_count_parents(const struct fwnode_handle *fwn); struct fwnode_handle *fwnode_get_nth_parent(struct fwnode_handle *fwn, unsigned int depth); +bool fwnode_is_ancestor_of(struct fwnode_handle *test_ancestor, + struct fwnode_handle *test_child); struct fwnode_handle *fwnode_get_next_child_node( const struct fwnode_handle *fwnode, struct fwnode_handle *child); struct fwnode_handle *fwnode_get_next_available_child_node( From 04f63c213b671d89db35f4239f57fa1edeb736a8 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:26 -0800 Subject: [PATCH 18/39] driver core: Redefine the meaning of fwnode_operations.add_links() Change the meaning of fwnode_operations.add_links() to just create fwnode links by parsing the properties of a given fwnode. This patch doesn't actually make any code changes. To keeps things more digestable, the actual functional changes come in later patches in this series. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-12-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/firmware/efi/efi-init.c | 2 +- include/linux/fwnode.h | 42 +++------------------------------ 2 files changed, 4 insertions(+), 40 deletions(-) diff --git a/drivers/firmware/efi/efi-init.c b/drivers/firmware/efi/efi-init.c index b148f1459fb3..65bb97c391b0 100644 --- a/drivers/firmware/efi/efi-init.c +++ b/drivers/firmware/efi/efi-init.c @@ -316,7 +316,7 @@ static struct device_node *find_pci_overlap_node(void) * resource reservation conflict on the memory window that the efifb * framebuffer steals from the PCIe host bridge. */ -static int efifb_add_links(const struct fwnode_handle *fwnode, +static int efifb_add_links(struct fwnode_handle *fwnode, struct device *dev) { struct device_node *sup_np; diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h index b88365187347..942a6bb18201 100644 --- a/include/linux/fwnode.h +++ b/include/linux/fwnode.h @@ -78,44 +78,8 @@ struct fwnode_reference_args { * endpoint node. * @graph_get_port_parent: Return the parent node of a port node. * @graph_parse_endpoint: Parse endpoint for port and endpoint id. - * @add_links: Called after the device corresponding to the fwnode is added - * using device_add(). The function is expected to create device - * links to all the suppliers of the device that are available at - * the time this function is called. The function must NOT stop - * at the first failed device link if other unlinked supplier - * devices are present in the system. This is necessary for the - * driver/bus sync_state() callbacks to work correctly. - * - * For example, say Device-C depends on suppliers Device-S1 and - * Device-S2 and the dependency is listed in that order in the - * firmware. Say, S1 gets populated from the firmware after - * late_initcall_sync(). Say S2 is populated and probed way - * before that in device_initcall(). When C is populated, if this - * add_links() function doesn't continue past a "failed linking to - * S1" and continue linking C to S2, then S2 will get a - * sync_state() callback before C is probed. This is because from - * the perspective of S2, C was never a consumer when its - * sync_state() evaluation is done. To avoid this, the add_links() - * function has to go through all available suppliers of the - * device (that corresponds to this fwnode) and link to them - * before returning. - * - * If some suppliers are not yet available (indicated by an error - * return value), this function will be called again when other - * devices are added to allow creating device links to any newly - * available suppliers. - * - * Return 0 if device links have been successfully created to all - * the known suppliers of this device or if the supplier - * information is not known. - * - * Return -ENODEV if the suppliers needed for probing this device - * have not been registered yet (because device links can only be - * created to devices registered with the driver core). - * - * Return -EAGAIN if some of the suppliers of this device have not - * been registered yet, but none of those suppliers are necessary - * for probing the device. + * @add_links: Create fwnode links to all the suppliers of the fwnode. Return + * zero on success, a negative error code otherwise. */ struct fwnode_operations { struct fwnode_handle *(*get)(struct fwnode_handle *fwnode); @@ -155,7 +119,7 @@ struct fwnode_operations { (*graph_get_port_parent)(struct fwnode_handle *fwnode); int (*graph_parse_endpoint)(const struct fwnode_handle *fwnode, struct fwnode_endpoint *endpoint); - int (*add_links)(const struct fwnode_handle *fwnode, + int (*add_links)(struct fwnode_handle *fwnode, struct device *dev); }; From c2c724c868c42c5166bf7aa644dd0a0c8d30b47a Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:27 -0800 Subject: [PATCH 19/39] driver core: Add fw_devlink_parse_fwtree() This function is a wrapper around fwnode_operations.add_links(). This function parses each node in a fwnode tree and create fwnode links for each of those nodes. The information for creating the fwnode links (the supplier and consumer fwnode) is obtained by parsing the properties in each of the fwnodes. This function also ensures that no fwnode is parsed more than once by marking the fwnodes as parsed. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-13-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 19 +++++++++++++++++++ include/linux/fwnode.h | 8 ++++++++ 2 files changed, 27 insertions(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index edde79fc3d33..92a2dc355d13 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1542,6 +1542,25 @@ static bool fw_devlink_is_permissive(void) return fw_devlink_flags == DL_FLAG_SYNC_STATE_ONLY; } +static void fw_devlink_parse_fwnode(struct fwnode_handle *fwnode) +{ + if (fwnode->flags & FWNODE_FLAG_LINKS_ADDED) + return; + + fwnode_call_int_op(fwnode, add_links, NULL); + fwnode->flags |= FWNODE_FLAG_LINKS_ADDED; +} + +static void fw_devlink_parse_fwtree(struct fwnode_handle *fwnode) +{ + struct fwnode_handle *child = NULL; + + fw_devlink_parse_fwnode(fwnode); + + while ((child = fwnode_get_next_available_child_node(fwnode, child))) + fw_devlink_parse_fwtree(child); +} + static void fw_devlink_link_device(struct device *dev) { int fw_ret; diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h index 942a6bb18201..ffa9129182a6 100644 --- a/include/linux/fwnode.h +++ b/include/linux/fwnode.h @@ -15,12 +15,20 @@ struct fwnode_operations; struct device; +/* + * fwnode link flags + * + * LINKS_ADDED: The fwnode has already be parsed to add fwnode links. + */ +#define FWNODE_FLAG_LINKS_ADDED BIT(0) + struct fwnode_handle { struct fwnode_handle *secondary; const struct fwnode_operations *ops; struct device *dev; struct list_head suppliers; struct list_head consumers; + u8 flags; }; struct fwnode_link { From 25ac86c6dbe62fba9b97e997fa648cdbe2d40173 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:28 -0800 Subject: [PATCH 20/39] driver core: Use device's fwnode to check if it is waiting for suppliers To check if a device is still waiting for its supplier devices to be added, we used to check if the devices is in a global waiting_for_suppliers list. Since the global list will be deleted in subsequent patches, this patch stops using this check. Instead, this patch uses a more device specific check. It checks if the device's fwnode has any fwnode links that haven't been converted to device links yet. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-14-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 92a2dc355d13..8a973d9601d3 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -51,6 +51,7 @@ static DEFINE_MUTEX(wfs_lock); static LIST_HEAD(deferred_sync); static unsigned int defer_sync_state_count = 1; static DEFINE_MUTEX(fwnode_link_lock); +static bool fw_devlink_is_permissive(void); /** * fwnode_link_add - Create a link between two fwnode_handles. @@ -995,13 +996,13 @@ int device_links_check_suppliers(struct device *dev) * Device waiting for supplier to become available is not allowed to * probe. */ - mutex_lock(&wfs_lock); - if (!list_empty(&dev->links.needs_suppliers) && - dev->links.need_for_probe) { - mutex_unlock(&wfs_lock); + mutex_lock(&fwnode_link_lock); + if (dev->fwnode && !list_empty(&dev->fwnode->suppliers) && + !fw_devlink_is_permissive()) { + mutex_unlock(&fwnode_link_lock); return -EPROBE_DEFER; } - mutex_unlock(&wfs_lock); + mutex_unlock(&fwnode_link_lock); device_links_write_lock(); @@ -1167,8 +1168,7 @@ static ssize_t waiting_for_supplier_show(struct device *dev, bool val; device_lock(dev); - val = !list_empty(&dev->links.needs_suppliers) - && dev->links.need_for_probe; + val = !list_empty(&dev->fwnode->suppliers); device_unlock(dev); return sysfs_emit(buf, "%u\n", val); } @@ -2200,7 +2200,7 @@ static int device_add_attrs(struct device *dev) goto err_remove_dev_groups; } - if (fw_devlink_flags && !fw_devlink_is_permissive()) { + if (fw_devlink_flags && !fw_devlink_is_permissive() && dev->fwnode) { error = device_create_file(dev, &dev_attr_waiting_for_supplier); if (error) goto err_remove_dev_online; From 8a06d1ea061739dd2e60aff3d64a58892e4031cf Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:29 -0800 Subject: [PATCH 21/39] of: property: Update implementation of add_links() to create fwnode links The semantics of add_links() has changed from creating device link between devices to creating fwnode links between fwnodes. So, update the implementation of add_links() to match the new semantics. Acked-by: Rob Herring Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-15-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/of/property.c | 154 ++++++++++++------------------------------ 1 file changed, 43 insertions(+), 111 deletions(-) diff --git a/drivers/of/property.c b/drivers/of/property.c index 408a7b5f06a9..620d29fdace8 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1038,33 +1038,9 @@ static bool of_is_ancestor_of(struct device_node *test_ancestor, } /** - * of_get_next_parent_dev - Add device link to supplier from supplier phandle - * @np: device tree node - * - * Given a device tree node (@np), this function finds its closest ancestor - * device tree node that has a corresponding struct device. - * - * The caller of this function is expected to call put_device() on the returned - * device when they are done. - */ -static struct device *of_get_next_parent_dev(struct device_node *np) -{ - struct device *dev = NULL; - - of_node_get(np); - do { - np = of_get_next_parent(np); - if (np) - dev = get_dev_from_fwnode(&np->fwnode); - } while (np && !dev); - of_node_put(np); - return dev; -} - -/** - * of_link_to_phandle - Add device link to supplier from supplier phandle - * @dev: consumer device - * @sup_np: phandle to supplier device tree node + * of_link_to_phandle - Add fwnode link to supplier from supplier phandle + * @con_np: consumer device tree node + * @sup_np: supplier device tree node * * Given a phandle to a supplier device tree node (@sup_np), this function * finds the device that owns the supplier device tree node and creates a @@ -1074,16 +1050,14 @@ static struct device *of_get_next_parent_dev(struct device_node *np) * cases, it returns an error. * * Returns: - * - 0 if link successfully created to supplier - * - -EAGAIN if linking to the supplier should be reattempted + * - 0 if fwnode link successfully created to supplier * - -EINVAL if the supplier link is invalid and should not be created - * - -ENODEV if there is no device that corresponds to the supplier phandle + * - -ENODEV if struct device will never be create for supplier */ -static int of_link_to_phandle(struct device *dev, struct device_node *sup_np, - u32 dl_flags) +static int of_link_to_phandle(struct device_node *con_np, + struct device_node *sup_np) { - struct device *sup_dev, *sup_par_dev; - int ret = 0; + struct device *sup_dev; struct device_node *tmp_np = sup_np; of_node_get(sup_np); @@ -1106,7 +1080,8 @@ static int of_link_to_phandle(struct device *dev, struct device_node *sup_np, } if (!sup_np) { - dev_dbg(dev, "Not linking to %pOFP - No device\n", tmp_np); + pr_debug("Not linking %pOFP to %pOFP - No device\n", + con_np, tmp_np); return -ENODEV; } @@ -1115,53 +1090,30 @@ static int of_link_to_phandle(struct device *dev, struct device_node *sup_np, * descendant nodes. By definition, a child node can't be a functional * dependency for the parent node. */ - if (of_is_ancestor_of(dev->of_node, sup_np)) { - dev_dbg(dev, "Not linking to %pOFP - is descendant\n", sup_np); + if (of_is_ancestor_of(con_np, sup_np)) { + pr_debug("Not linking %pOFP to %pOFP - is descendant\n", + con_np, sup_np); of_node_put(sup_np); return -EINVAL; } + + /* + * Don't create links to "early devices" that won't have struct devices + * created for them. + */ sup_dev = get_dev_from_fwnode(&sup_np->fwnode); if (!sup_dev && of_node_check_flag(sup_np, OF_POPULATED)) { - /* Early device without struct device. */ - dev_dbg(dev, "Not linking to %pOFP - No struct device\n", - sup_np); + pr_debug("Not linking %pOFP to %pOFP - No struct device\n", + con_np, sup_np); of_node_put(sup_np); return -ENODEV; - } else if (!sup_dev) { - /* - * DL_FLAG_SYNC_STATE_ONLY doesn't block probing and supports - * cycles. So cycle detection isn't necessary and shouldn't be - * done. - */ - if (dl_flags & DL_FLAG_SYNC_STATE_ONLY) { - of_node_put(sup_np); - return -EAGAIN; - } - - sup_par_dev = of_get_next_parent_dev(sup_np); - - if (sup_par_dev && device_is_dependent(dev, sup_par_dev)) { - /* Cyclic dependency detected, don't try to link */ - dev_dbg(dev, "Not linking to %pOFP - cycle detected\n", - sup_np); - ret = -EINVAL; - } else { - /* - * Can't check for cycles or no cycles. So let's try - * again later. - */ - ret = -EAGAIN; - } - - of_node_put(sup_np); - put_device(sup_par_dev); - return ret; } - of_node_put(sup_np); - if (!device_link_add(dev, sup_dev, dl_flags)) - ret = -EINVAL; put_device(sup_dev); - return ret; + + fwnode_link_add(of_fwnode_handle(con_np), of_fwnode_handle(sup_np)); + of_node_put(sup_np); + + return 0; } /** @@ -1361,37 +1313,29 @@ static const struct supplier_bindings of_supplier_bindings[] = { * that list phandles to suppliers. If @prop_name isn't one, this function * doesn't do anything. * - * If @prop_name is one, this function attempts to create device links from the - * consumer device @dev to all the devices of the suppliers listed in - * @prop_name. + * If @prop_name is one, this function attempts to create fwnode links from the + * consumer device tree node @con_np to all the suppliers device tree nodes + * listed in @prop_name. * - * Any failed attempt to create a device link will NOT result in an immediate + * Any failed attempt to create a fwnode link will NOT result in an immediate * return. of_link_property() must create links to all the available supplier - * devices even when attempts to create a link to one or more suppliers fail. + * device tree nodes even when attempts to create a link to one or more + * suppliers fail. */ -static int of_link_property(struct device *dev, struct device_node *con_np, - const char *prop_name) +static int of_link_property(struct device_node *con_np, const char *prop_name) { struct device_node *phandle; const struct supplier_bindings *s = of_supplier_bindings; unsigned int i = 0; bool matched = false; int ret = 0; - u32 dl_flags; - - if (dev->of_node == con_np) - dl_flags = fw_devlink_get_flags(); - else - dl_flags = DL_FLAG_SYNC_STATE_ONLY; /* Do not stop at first failed link, link all available suppliers. */ while (!matched && s->parse_prop) { while ((phandle = s->parse_prop(con_np, prop_name, i))) { matched = true; i++; - if (of_link_to_phandle(dev, phandle, dl_flags) - == -EAGAIN) - ret = -EAGAIN; + of_link_to_phandle(con_np, phandle); of_node_put(phandle); } s++; @@ -1399,31 +1343,19 @@ static int of_link_property(struct device *dev, struct device_node *con_np, return ret; } -static int of_link_to_suppliers(struct device *dev, - struct device_node *con_np) -{ - struct device_node *child; - struct property *p; - int ret = 0; - - for_each_property_of_node(con_np, p) - if (of_link_property(dev, con_np, p->name)) - ret = -ENODEV; - - for_each_available_child_of_node(con_np, child) - if (of_link_to_suppliers(dev, child) && !ret) - ret = -EAGAIN; - - return ret; -} - -static int of_fwnode_add_links(const struct fwnode_handle *fwnode, +static int of_fwnode_add_links(struct fwnode_handle *fwnode, struct device *dev) { - if (unlikely(!is_of_node(fwnode))) - return 0; + struct property *p; + struct device_node *con_np = to_of_node(fwnode); - return of_link_to_suppliers(dev, to_of_node(fwnode)); + if (!con_np) + return -EINVAL; + + for_each_property_of_node(con_np, p) + of_link_property(con_np, p->name); + + return 0; } const struct fwnode_operations of_fwnode_ops = { From e82a840cb1c1c83d01a9b81bb63b6cf1c09239d7 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:30 -0800 Subject: [PATCH 22/39] efi: Update implementation of add_links() to create fwnode links The semantics of add_links() has changed from creating device link between devices to creating fwnode links between fwnodes. So, update the implementation of add_links() to match the new semantics. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-16-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/firmware/efi/efi-init.c | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/drivers/firmware/efi/efi-init.c b/drivers/firmware/efi/efi-init.c index 65bb97c391b0..c0c3d4c3837a 100644 --- a/drivers/firmware/efi/efi-init.c +++ b/drivers/firmware/efi/efi-init.c @@ -320,7 +320,6 @@ static int efifb_add_links(struct fwnode_handle *fwnode, struct device *dev) { struct device_node *sup_np; - struct device *sup_dev; sup_np = find_pci_overlap_node(); @@ -331,27 +330,9 @@ static int efifb_add_links(struct fwnode_handle *fwnode, if (!sup_np) return 0; - sup_dev = get_dev_from_fwnode(&sup_np->fwnode); + fwnode_link_add(fwnode, of_fwnode_handle(sup_np)); of_node_put(sup_np); - /* - * Return -ENODEV if the PCI graphics controller device hasn't been - * registered yet. This ensures that efifb isn't allowed to probe - * and this function is retried again when new devices are - * registered. - */ - if (!sup_dev) - return -ENODEV; - - /* - * If this fails, retrying this function at a later point won't - * change anything. So, don't return an error after this. - */ - if (!device_link_add(dev, sup_dev, fw_devlink_get_flags())) - dev_warn(dev, "device_link_add() failed\n"); - - put_device(sup_dev); - return 0; } From f9aa460672c9c56896cdc12a521159e3e67000ba Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:31 -0800 Subject: [PATCH 23/39] driver core: Refactor fw_devlink feature The current implementation of fw_devlink is very inefficient because it tries to get away without creating fwnode links in the name of saving memory usage. Past attempts to optimize runtime at the cost of memory usage were blocked with request for data showing that the optimization made significant improvement for real world scenarios. We have those scenarios now. There have been several reports of boot time increase in the order of seconds in this thread [1]. Several OEMs and SoC manufacturers have also privately reported significant (350-400ms) increase in boot time due to all the parsing done by fw_devlink. So this patch uses all the setup done by the previous patches in this series to refactor fw_devlink to be more efficient. Most of the code has been moved out of firmware specific (DT mostly) code into driver core. This brings the following benefits: - Instead of parsing the device tree multiple times during bootup, fw_devlink parses each fwnode node/property only once and creates fwnode links. The rest of the fw_devlink code then just looks at these fwnode links to do rest of the work. - Makes it much easier to debug probe issue due to fw_devlink in the future. fw_devlink=on blocks the probing of devices if they depend on a device that hasn't been added yet. With this refactor, it'll be very easy to tell what that device is because we now have a reference to the fwnode of the device. - Much easier to add fw_devlink support to ACPI and other firmware types. A refactor to move the common bits from DT specific code to driver core was in my TODO list as a prerequisite to adding ACPI support to fw_devlink. This series gets that done. [1] - https://lore.kernel.org/linux-omap/ea02f57e-871d-cd16-4418-c1da4bbc4696@ti.com/ Tested-by: Laurent Pinchart Tested-by: Grygorii Strashko Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-17-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 325 ++++++++++++++++++++++++++++++----------- include/linux/device.h | 5 - 2 files changed, 238 insertions(+), 92 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 8a973d9601d3..9edf9084fc98 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -46,8 +46,6 @@ early_param("sysfs.deprecated", sysfs_deprecated_setup); #endif /* Device links support. */ -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 DEFINE_MUTEX(fwnode_link_lock); @@ -803,74 +801,6 @@ out: } EXPORT_SYMBOL_GPL(device_link_add); -/** - * device_link_wait_for_supplier - Add device to wait_for_suppliers list - * @consumer: Consumer device - * - * Marks the @consumer device as waiting for suppliers to become available by - * adding it to the wait_for_suppliers list. The consumer device will never be - * probed until it's removed from the wait_for_suppliers list. - * - * The caller is responsible for adding the links to the supplier devices once - * they are available and removing the @consumer device from the - * wait_for_suppliers list once links to all the suppliers have been created. - * - * This function is NOT meant to be called from the probe function of the - * consumer but rather from code that creates/adds the consumer device. - */ -static void device_link_wait_for_supplier(struct device *consumer, - bool need_for_probe) -{ - mutex_lock(&wfs_lock); - list_add_tail(&consumer->links.needs_suppliers, &wait_for_suppliers); - consumer->links.need_for_probe = need_for_probe; - mutex_unlock(&wfs_lock); -} - -static void device_link_wait_for_mandatory_supplier(struct device *consumer) -{ - device_link_wait_for_supplier(consumer, true); -} - -static void device_link_wait_for_optional_supplier(struct device *consumer) -{ - device_link_wait_for_supplier(consumer, false); -} - -/** - * device_link_add_missing_supplier_links - Add links from consumer devices to - * supplier devices, leaving any - * consumer with inactive suppliers on - * the wait_for_suppliers list - * - * Loops through all consumers waiting on suppliers and tries to add all their - * supplier links. If that succeeds, the consumer device is removed from - * wait_for_suppliers list. Otherwise, they are left in the wait_for_suppliers - * list. Devices left on the wait_for_suppliers list will not be probed. - * - * The fwnode add_links callback is expected to return 0 if it has found and - * added all the supplier links for the consumer device. It should return an - * error if it isn't able to do so. - * - * The caller of device_link_wait_for_supplier() is expected to call this once - * it's aware of potential suppliers becoming available. - */ -static void device_link_add_missing_supplier_links(void) -{ - struct device *dev, *tmp; - - mutex_lock(&wfs_lock); - list_for_each_entry_safe(dev, tmp, &wait_for_suppliers, - links.needs_suppliers) { - int ret = fwnode_call_int_op(dev->fwnode, add_links, dev); - if (!ret) - list_del_init(&dev->links.needs_suppliers); - else if (ret != -ENODEV) - dev->links.need_for_probe = false; - } - mutex_unlock(&wfs_lock); -} - #ifdef CONFIG_SRCU static void __device_link_del(struct kref *kref) { @@ -1195,9 +1125,8 @@ void device_links_driver_bound(struct device *dev) * the device links it needs to or make new device links as it needs * them. So, it no longer needs to wait on any suppliers. */ - mutex_lock(&wfs_lock); - list_del_init(&dev->links.needs_suppliers); - mutex_unlock(&wfs_lock); + if (dev->fwnode && dev->fwnode->dev == dev) + fwnode_links_purge_suppliers(dev->fwnode); device_remove_file(dev, &dev_attr_waiting_for_supplier); device_links_write_lock(); @@ -1488,10 +1417,6 @@ static void device_links_purge(struct device *dev) if (dev->class == &devlink_class) return; - mutex_lock(&wfs_lock); - list_del_init(&dev->links.needs_suppliers); - mutex_unlock(&wfs_lock); - /* * Delete all of the remaining links from this device to any other * devices (either consumers or suppliers). @@ -1561,19 +1486,246 @@ static void fw_devlink_parse_fwtree(struct fwnode_handle *fwnode) fw_devlink_parse_fwtree(child); } +/** + * fw_devlink_create_devlink - Create a device link from a consumer to fwnode + * @con - Consumer device for the device link + * @sup_handle - fwnode handle of supplier + * + * This function will try to create a device link between the consumer device + * @con and the supplier device represented by @sup_handle. + * + * The supplier has to be provided as a fwnode because incorrect cycles in + * fwnode links can sometimes cause the supplier device to never be created. + * This function detects such cases and returns an error if it cannot create a + * device link from the consumer to a missing supplier. + * + * Returns, + * 0 on successfully creating a device link + * -EINVAL if the device link cannot be created as expected + * -EAGAIN if the device link cannot be created right now, but it may be + * possible to do that in the future + */ +static int fw_devlink_create_devlink(struct device *con, + struct fwnode_handle *sup_handle, u32 flags) +{ + struct device *sup_dev; + int ret = 0; + + sup_dev = get_dev_from_fwnode(sup_handle); + if (sup_dev) { + /* + * If this fails, it is due to cycles in device links. Just + * give up on this link and treat it as invalid. + */ + if (!device_link_add(con, sup_dev, flags)) + ret = -EINVAL; + + goto out; + } + + /* + * DL_FLAG_SYNC_STATE_ONLY doesn't block probing and supports + * cycles. So cycle detection isn't necessary and shouldn't be + * done. + */ + if (flags & DL_FLAG_SYNC_STATE_ONLY) + return -EAGAIN; + + /* + * If we can't find the supplier device from its fwnode, it might be + * due to a cyclic dependency between fwnodes. Some of these cycles can + * be broken by applying logic. Check for these types of cycles and + * break them so that devices in the cycle probe properly. + * + * If the supplier's parent is dependent on the consumer, then + * the consumer-supplier dependency is a false dependency. So, + * treat it as an invalid link. + */ + sup_dev = fwnode_get_next_parent_dev(sup_handle); + if (sup_dev && device_is_dependent(con, sup_dev)) { + dev_dbg(con, "Not linking to %pfwP - False link\n", + sup_handle); + ret = -EINVAL; + } else { + /* + * Can't check for cycles or no cycles. So let's try + * again later. + */ + ret = -EAGAIN; + } + +out: + put_device(sup_dev); + return ret; +} + +/** + * __fw_devlink_link_to_consumers - Create device links to consumers of a device + * @dev - Device that needs to be linked to its consumers + * + * This function looks at all the consumer fwnodes of @dev and creates device + * links between the consumer device and @dev (supplier). + * + * If the consumer device has not been added yet, then this function creates a + * SYNC_STATE_ONLY link between @dev (supplier) and the closest ancestor device + * of the consumer fwnode. This is necessary to make sure @dev doesn't get a + * sync_state() callback before the real consumer device gets to be added and + * then probed. + * + * Once device links are created from the real consumer to @dev (supplier), the + * fwnode links are deleted. + */ +static void __fw_devlink_link_to_consumers(struct device *dev) +{ + struct fwnode_handle *fwnode = dev->fwnode; + struct fwnode_link *link, *tmp; + + list_for_each_entry_safe(link, tmp, &fwnode->consumers, s_hook) { + u32 dl_flags = fw_devlink_get_flags(); + struct device *con_dev; + bool own_link = true; + int ret; + + con_dev = get_dev_from_fwnode(link->consumer); + /* + * If consumer device is not available yet, make a "proxy" + * SYNC_STATE_ONLY link from the consumer's parent device to + * the supplier device. This is necessary to make sure the + * supplier doesn't get a sync_state() callback before the real + * consumer can create a device link to the supplier. + * + * This proxy link step is needed to handle the case where the + * consumer's parent device is added before the supplier. + */ + if (!con_dev) { + con_dev = fwnode_get_next_parent_dev(link->consumer); + /* + * However, if the consumer's parent device is also the + * parent of the supplier, don't create a + * consumer-supplier link from the parent to its child + * device. Such a dependency is impossible. + */ + if (con_dev && + fwnode_is_ancestor_of(con_dev->fwnode, fwnode)) { + put_device(con_dev); + con_dev = NULL; + } else { + own_link = false; + dl_flags = DL_FLAG_SYNC_STATE_ONLY; + } + } + + if (!con_dev) + continue; + + ret = fw_devlink_create_devlink(con_dev, fwnode, dl_flags); + put_device(con_dev); + if (!own_link || ret == -EAGAIN) + continue; + + list_del(&link->s_hook); + list_del(&link->c_hook); + kfree(link); + } +} + +/** + * __fw_devlink_link_to_suppliers - Create device links to suppliers of a device + * @dev - The consumer device that needs to be linked to its suppliers + * @fwnode - Root of the fwnode tree that is used to create device links + * + * This function looks at all the supplier fwnodes of fwnode tree rooted at + * @fwnode and creates device links between @dev (consumer) and all the + * supplier devices of the entire fwnode tree at @fwnode. + * + * The function creates normal (non-SYNC_STATE_ONLY) device links between @dev + * and the real suppliers of @dev. Once these device links are created, the + * fwnode links are deleted. When such device links are successfully created, + * this function is called recursively on those supplier devices. This is + * needed to detect and break some invalid cycles in fwnode links. See + * fw_devlink_create_devlink() for more details. + * + * In addition, it also looks at all the suppliers of the entire fwnode tree + * because some of the child devices of @dev that have not been added yet + * (because @dev hasn't probed) might already have their suppliers added to + * driver core. So, this function creates SYNC_STATE_ONLY device links between + * @dev (consumer) and these suppliers to make sure they don't execute their + * sync_state() callbacks before these child devices have a chance to create + * their device links. The fwnode links that correspond to the child devices + * aren't delete because they are needed later to create the device links + * between the real consumer and supplier devices. + */ +static void __fw_devlink_link_to_suppliers(struct device *dev, + struct fwnode_handle *fwnode) +{ + bool own_link = (dev->fwnode == fwnode); + struct fwnode_link *link, *tmp; + struct fwnode_handle *child = NULL; + u32 dl_flags; + + if (own_link) + dl_flags = fw_devlink_get_flags(); + else + dl_flags = DL_FLAG_SYNC_STATE_ONLY; + + list_for_each_entry_safe(link, tmp, &fwnode->suppliers, c_hook) { + int ret; + struct device *sup_dev; + struct fwnode_handle *sup = link->supplier; + + ret = fw_devlink_create_devlink(dev, sup, dl_flags); + if (!own_link || ret == -EAGAIN) + continue; + + list_del(&link->s_hook); + list_del(&link->c_hook); + kfree(link); + + /* If no device link was created, nothing more to do. */ + if (ret) + continue; + + /* + * If a device link was successfully created to a supplier, we + * now need to try and link the supplier to all its suppliers. + * + * This is needed to detect and delete false dependencies in + * fwnode links that haven't been converted to a device link + * yet. See comments in fw_devlink_create_devlink() for more + * details on the false dependency. + * + * Without deleting these false dependencies, some devices will + * never probe because they'll keep waiting for their false + * dependency fwnode links to be converted to device links. + */ + sup_dev = get_dev_from_fwnode(sup); + __fw_devlink_link_to_suppliers(sup_dev, sup_dev->fwnode); + put_device(sup_dev); + } + + /* + * Make "proxy" SYNC_STATE_ONLY device links to represent the needs of + * all the descendants. This proxy link step is needed to handle the + * case where the supplier is added before the consumer's parent device + * (@dev). + */ + while ((child = fwnode_get_next_available_child_node(fwnode, child))) + __fw_devlink_link_to_suppliers(dev, child); +} + static void fw_devlink_link_device(struct device *dev) { - int fw_ret; + struct fwnode_handle *fwnode = dev->fwnode; - device_link_add_missing_supplier_links(); + if (!fw_devlink_flags) + return; - if (fw_devlink_flags && fwnode_has_op(dev->fwnode, add_links)) { - 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); - } + fw_devlink_parse_fwtree(fwnode); + + mutex_lock(&fwnode_link_lock); + __fw_devlink_link_to_consumers(dev); + __fw_devlink_link_to_suppliers(dev, fwnode); + mutex_unlock(&fwnode_link_lock); } /* Device links support end. */ @@ -2431,7 +2583,6 @@ void device_initialize(struct device *dev) #endif INIT_LIST_HEAD(&dev->links.consumers); INIT_LIST_HEAD(&dev->links.suppliers); - INIT_LIST_HEAD(&dev->links.needs_suppliers); INIT_LIST_HEAD(&dev->links.defer_sync); dev->links.status = DL_DEV_NO_DRIVER; } diff --git a/include/linux/device.h b/include/linux/device.h index 1e771ea4dca6..89bb8b84173e 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -351,18 +351,13 @@ enum dl_dev_state { * struct dev_links_info - Device data related to device links. * @suppliers: List of links to supplier devices. * @consumers: List of links to consumer devices. - * @needs_suppliers: Hook to global list of devices waiting for suppliers. * @defer_sync: Hook to global list of devices that have deferred sync_state. - * @need_for_probe: If needs_suppliers is on a list, this indicates if the - * suppliers are needed for probe or not. * @status: Driver status information. */ struct dev_links_info { struct list_head suppliers; struct list_head consumers; - struct list_head needs_suppliers; struct list_head defer_sync; - bool need_for_probe; enum dl_dev_state status; }; From 2d09e6eb4a6f20273959f4905ccf009da8c64c7a Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 20 Nov 2020 18:02:32 -0800 Subject: [PATCH 24/39] driver core: Delete pointless parameter in fwnode_operations.add_links The struct device input to add_links() is not used for anything. So delete it. Acked-by: Rob Herring Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201121020232.908850-18-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 2 +- drivers/firmware/efi/efi-init.c | 3 +-- drivers/of/property.c | 3 +-- include/linux/fwnode.h | 3 +-- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 9edf9084fc98..63edb8bd9d7d 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1472,7 +1472,7 @@ static void fw_devlink_parse_fwnode(struct fwnode_handle *fwnode) if (fwnode->flags & FWNODE_FLAG_LINKS_ADDED) return; - fwnode_call_int_op(fwnode, add_links, NULL); + fwnode_call_int_op(fwnode, add_links); fwnode->flags |= FWNODE_FLAG_LINKS_ADDED; } diff --git a/drivers/firmware/efi/efi-init.c b/drivers/firmware/efi/efi-init.c index c0c3d4c3837a..a552a08a1741 100644 --- a/drivers/firmware/efi/efi-init.c +++ b/drivers/firmware/efi/efi-init.c @@ -316,8 +316,7 @@ static struct device_node *find_pci_overlap_node(void) * resource reservation conflict on the memory window that the efifb * framebuffer steals from the PCIe host bridge. */ -static int efifb_add_links(struct fwnode_handle *fwnode, - struct device *dev) +static int efifb_add_links(struct fwnode_handle *fwnode) { struct device_node *sup_np; diff --git a/drivers/of/property.c b/drivers/of/property.c index 620d29fdace8..5f9eed79a8aa 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1343,8 +1343,7 @@ static int of_link_property(struct device_node *con_np, const char *prop_name) return ret; } -static int of_fwnode_add_links(struct fwnode_handle *fwnode, - struct device *dev) +static int of_fwnode_add_links(struct fwnode_handle *fwnode) { struct property *p; struct device_node *con_np = to_of_node(fwnode); diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h index ffa9129182a6..fde4ad97564c 100644 --- a/include/linux/fwnode.h +++ b/include/linux/fwnode.h @@ -127,8 +127,7 @@ struct fwnode_operations { (*graph_get_port_parent)(struct fwnode_handle *fwnode); int (*graph_parse_endpoint)(const struct fwnode_handle *fwnode, struct fwnode_endpoint *endpoint); - int (*add_links)(struct fwnode_handle *fwnode, - struct device *dev); + int (*add_links)(struct fwnode_handle *fwnode); }; #define fwnode_has_op(fwnode, op) \ From 5b6164d3465fcc13b5679c860c452963443172a7 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Thu, 3 Dec 2020 18:57:56 +0100 Subject: [PATCH 25/39] driver core: Reorder devices on successful probe Device drivers usually depend on the fact that the devices that they control are suspended in the same order that they were probed in. In most cases this is already guaranteed via deferred probe. However, there's one case where this can still break: if a device is instantiated before a dependency (for example if it appears before the dependency in device tree) but gets probed only after the dependency is probed. Instantiation order would cause the dependency to get probed later, in which case probe of the original device would be deferred and the suspend/resume queue would get reordered properly. However, if the dependency is provided by a built-in driver and the device depending on that driver is controlled by a loadable module, which may only get loaded after the root filesystem has become available, we can be faced with a situation where the probe order ends up being different from the suspend/resume order. One example where this happens is on Tegra186, where the ACONNECT is listed very early in device tree (sorted by unit-address) and depends on BPMP (listed very late because it has no unit-address) for power domains and clocks/resets. If the ACONNECT driver is built-in, there is no problem because it will be probed before BPMP, causing a probe deferral and that in turn reorders the suspend/resume queue. However, if built as a module, it will end up being probed after BPMP, and therefore not result in a probe deferral, and therefore the suspend/resume queue will stay in the instantiation order. This in turn causes problems because ACONNECT will be resumed before BPMP, which will result in a hang because the ACONNECT's power domain cannot be powered on as long as the BPMP is still suspended. Fix this by always reordering devices on successful probe. This ensures that the suspend/resume queue is always in probe order and hence meets the natural expectations of drivers vs. their dependencies. Reported-by: Jonathan Hunter Acked-by: Rafael. J. Wysocki Signed-off-by: Thierry Reding Link: https://lore.kernel.org/r/20201203175756.1405564-1-thierry.reding@gmail.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 148e81969e04..cfc079e738bb 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -370,6 +370,13 @@ static void driver_bound(struct device *dev) device_pm_check_callbacks(dev); + /* + * Reorder successfully probed devices to the end of the device list. + * This ensures that suspend/resume order matches probe order, which + * is usually what drivers rely on. + */ + device_pm_move_to_tail(dev); + /* * Make sure the device is no longer in one of the deferred lists and * kick off retrying all pending devices From d475f8ea98a039e51d27f5557dc17333cf8a52f6 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Fri, 27 Nov 2020 11:46:30 +0100 Subject: [PATCH 26/39] driver core: Fix a couple of typos These were just some minor typos that have crept in recently and are easily fixed. Reviewed-by: Rafael J. Wysocki Signed-off-by: Thierry Reding Link: https://lore.kernel.org/r/20201127104630.1839171-1-thierry.reding@gmail.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 63edb8bd9d7d..25e08e5f40bd 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -561,7 +561,7 @@ postcore_initcall(devlink_class_init); * with runtime PM. First, setting the DL_FLAG_PM_RUNTIME flag will cause the * runtime PM framework to take the link into account. Second, if the * DL_FLAG_RPM_ACTIVE flag is set in addition to it, the supplier devices will - * be forced into the active metastate and reference-counted upon the creation + * be forced into the active meta state and reference-counted upon the creation * of the link. If DL_FLAG_PM_RUNTIME is not set, DL_FLAG_RPM_ACTIVE will be * ignored. * @@ -584,7 +584,7 @@ postcore_initcall(devlink_class_init); * Also, if DL_FLAG_STATELESS, DL_FLAG_AUTOREMOVE_CONSUMER and * DL_FLAG_AUTOREMOVE_SUPPLIER are not set in @flags (that is, a persistent * managed device link is being added), the DL_FLAG_AUTOPROBE_CONSUMER flag can - * be used to request the driver core to automaticall probe for a consmer + * be used to request the driver core to automatically probe for a consumer * driver after successfully binding a driver to the supplier device. * * The combination of DL_FLAG_STATELESS and one of DL_FLAG_AUTOREMOVE_CONSUMER, From 2c3dc6432f3316e97c10eced6b535046a80f73b0 Mon Sep 17 00:00:00 2001 From: Julian Wiedmann Date: Mon, 23 Nov 2020 12:19:38 +0100 Subject: [PATCH 27/39] driver core: make driver_probe_device() static It's only used inside drivers/base/dd.c Signed-off-by: Julian Wiedmann Link: https://lore.kernel.org/r/20201123111938.18968-1-jwi@linux.ibm.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 1 - drivers/base/dd.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index 91cfb8405abd..f5600a83124f 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -133,7 +133,6 @@ extern void device_release_driver_internal(struct device *dev, struct device *parent); extern void driver_detach(struct device_driver *drv); -extern int driver_probe_device(struct device_driver *drv, struct device *dev); extern void driver_deferred_probe_del(struct device *dev); extern void device_set_deferred_probe_reason(const struct device *dev, struct va_format *vaf); diff --git a/drivers/base/dd.c b/drivers/base/dd.c index cfc079e738bb..2f32f38a11ed 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -724,7 +724,7 @@ EXPORT_SYMBOL_GPL(wait_for_device_probe); * * If the device has a parent, runtime-resume the parent before driver probing. */ -int driver_probe_device(struct device_driver *drv, struct device *dev) +static int driver_probe_device(struct device_driver *drv, struct device *dev) { int ret = 0; From e21d740a3fe5ad2db7b5f5c2331fe2b713b1edba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Thu, 19 Nov 2020 13:46:09 +0100 Subject: [PATCH 28/39] driver core: platform: reorder functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This way all callbacks and structures used to initialize platform_bus_type are defined just before platform_bus_type and in the same order. Also move platform_drv_probe_fail just before it's only user. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20201119124611.2573057-1-u.kleine-koenig@pengutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 333 ++++++++++++++++++++-------------------- 1 file changed, 167 insertions(+), 166 deletions(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 88aef93eb4dd..b36ab5831cb6 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -772,11 +772,6 @@ out: return ret; } -static int platform_drv_probe_fail(struct device *_dev) -{ - return -ENXIO; -} - static int platform_drv_remove(struct device *_dev) { struct platform_driver *drv = to_platform_driver(_dev->driver); @@ -827,6 +822,11 @@ void platform_driver_unregister(struct platform_driver *drv) } EXPORT_SYMBOL_GPL(platform_driver_unregister); +static int platform_drv_probe_fail(struct device *_dev) +{ + return -ENXIO; +} + /** * __platform_driver_probe - register driver for non-hotpluggable device * @drv: platform driver structure @@ -1017,129 +1017,6 @@ void platform_unregister_drivers(struct platform_driver * const *drivers, } EXPORT_SYMBOL_GPL(platform_unregister_drivers); -/* modalias support enables more hands-off userspace setup: - * (a) environment variable lets new-style hotplug events work once system is - * fully running: "modprobe $MODALIAS" - * (b) sysfs attribute lets new-style coldplug recover from hotplug events - * mishandled before system is fully running: "modprobe $(cat modalias)" - */ -static ssize_t modalias_show(struct device *dev, - struct device_attribute *attr, char *buf) -{ - struct platform_device *pdev = to_platform_device(dev); - int len; - - len = of_device_modalias(dev, buf, PAGE_SIZE); - if (len != -ENODEV) - return len; - - len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1); - if (len != -ENODEV) - return len; - - return sysfs_emit(buf, "platform:%s\n", pdev->name); -} -static DEVICE_ATTR_RO(modalias); - -static ssize_t driver_override_store(struct device *dev, - struct device_attribute *attr, - const char *buf, size_t count) -{ - struct platform_device *pdev = to_platform_device(dev); - char *driver_override, *old, *cp; - - /* We need to keep extra room for a newline */ - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(dev); - old = pdev->driver_override; - if (strlen(driver_override)) { - pdev->driver_override = driver_override; - } else { - kfree(driver_override); - pdev->driver_override = NULL; - } - device_unlock(dev); - - kfree(old); - - return count; -} - -static ssize_t driver_override_show(struct device *dev, - struct device_attribute *attr, char *buf) -{ - struct platform_device *pdev = to_platform_device(dev); - ssize_t len; - - device_lock(dev); - len = sysfs_emit(buf, "%s\n", pdev->driver_override); - device_unlock(dev); - - return len; -} -static DEVICE_ATTR_RW(driver_override); - -static ssize_t numa_node_show(struct device *dev, - struct device_attribute *attr, char *buf) -{ - return sysfs_emit(buf, "%d\n", dev_to_node(dev)); -} -static DEVICE_ATTR_RO(numa_node); - -static umode_t platform_dev_attrs_visible(struct kobject *kobj, struct attribute *a, - int n) -{ - struct device *dev = container_of(kobj, typeof(*dev), kobj); - - if (a == &dev_attr_numa_node.attr && - dev_to_node(dev) == NUMA_NO_NODE) - return 0; - - return a->mode; -} - -static struct attribute *platform_dev_attrs[] = { - &dev_attr_modalias.attr, - &dev_attr_numa_node.attr, - &dev_attr_driver_override.attr, - NULL, -}; - -static struct attribute_group platform_dev_group = { - .attrs = platform_dev_attrs, - .is_visible = platform_dev_attrs_visible, -}; -__ATTRIBUTE_GROUPS(platform_dev); - -static int platform_uevent(struct device *dev, struct kobj_uevent_env *env) -{ - struct platform_device *pdev = to_platform_device(dev); - int rc; - - /* Some devices have extra OF data and an OF-style MODALIAS */ - rc = of_device_uevent_modalias(dev, env); - if (rc != -ENODEV) - return rc; - - rc = acpi_device_uevent_modalias(dev, env); - if (rc != -ENODEV) - return rc; - - add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX, - pdev->name); - return 0; -} - static const struct platform_device_id *platform_match_id( const struct platform_device_id *id, struct platform_device *pdev) @@ -1154,44 +1031,6 @@ static const struct platform_device_id *platform_match_id( return NULL; } -/** - * platform_match - bind platform device to platform driver. - * @dev: device. - * @drv: driver. - * - * Platform device IDs are assumed to be encoded like this: - * "", where is a short description of the type of - * device, like "pci" or "floppy", and is the enumerated - * instance of the device, like '0' or '42'. Driver IDs are simply - * "". So, extract the from the platform_device structure, - * and compare it against the name of the driver. Return whether they match - * or not. - */ -static int platform_match(struct device *dev, struct device_driver *drv) -{ - struct platform_device *pdev = to_platform_device(dev); - struct platform_driver *pdrv = to_platform_driver(drv); - - /* When driver_override is set, only bind to the matching driver */ - if (pdev->driver_override) - return !strcmp(pdev->driver_override, drv->name); - - /* Attempt an OF style match first */ - if (of_driver_match_device(dev, drv)) - return 1; - - /* Then try ACPI style match */ - if (acpi_driver_match_device(dev, drv)) - return 1; - - /* Then try to match against the id table */ - if (pdrv->id_table) - return platform_match_id(pdrv->id_table, pdev) != NULL; - - /* fall-back to driver name match */ - return (strcmp(pdev->name, drv->name) == 0); -} - #ifdef CONFIG_PM_SLEEP static int platform_legacy_suspend(struct device *dev, pm_message_t mesg) @@ -1336,6 +1175,168 @@ int platform_pm_restore(struct device *dev) #endif /* CONFIG_HIBERNATE_CALLBACKS */ +/* modalias support enables more hands-off userspace setup: + * (a) environment variable lets new-style hotplug events work once system is + * fully running: "modprobe $MODALIAS" + * (b) sysfs attribute lets new-style coldplug recover from hotplug events + * mishandled before system is fully running: "modprobe $(cat modalias)" + */ +static ssize_t modalias_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct platform_device *pdev = to_platform_device(dev); + int len; + + len = of_device_modalias(dev, buf, PAGE_SIZE); + if (len != -ENODEV) + return len; + + len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1); + if (len != -ENODEV) + return len; + + return sysfs_emit(buf, "platform:%s\n", pdev->name); +} +static DEVICE_ATTR_RO(modalias); + +static ssize_t numa_node_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sysfs_emit(buf, "%d\n", dev_to_node(dev)); +} +static DEVICE_ATTR_RO(numa_node); + +static ssize_t driver_override_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct platform_device *pdev = to_platform_device(dev); + ssize_t len; + + device_lock(dev); + len = sysfs_emit(buf, "%s\n", pdev->driver_override); + device_unlock(dev); + + return len; +} + +static ssize_t driver_override_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct platform_device *pdev = to_platform_device(dev); + char *driver_override, *old, *cp; + + /* We need to keep extra room for a newline */ + if (count >= (PAGE_SIZE - 1)) + return -EINVAL; + + driver_override = kstrndup(buf, count, GFP_KERNEL); + if (!driver_override) + return -ENOMEM; + + cp = strchr(driver_override, '\n'); + if (cp) + *cp = '\0'; + + device_lock(dev); + old = pdev->driver_override; + if (strlen(driver_override)) { + pdev->driver_override = driver_override; + } else { + kfree(driver_override); + pdev->driver_override = NULL; + } + device_unlock(dev); + + kfree(old); + + return count; +} +static DEVICE_ATTR_RW(driver_override); + +static struct attribute *platform_dev_attrs[] = { + &dev_attr_modalias.attr, + &dev_attr_numa_node.attr, + &dev_attr_driver_override.attr, + NULL, +}; + +static umode_t platform_dev_attrs_visible(struct kobject *kobj, struct attribute *a, + int n) +{ + struct device *dev = container_of(kobj, typeof(*dev), kobj); + + if (a == &dev_attr_numa_node.attr && + dev_to_node(dev) == NUMA_NO_NODE) + return 0; + + return a->mode; +} + +static struct attribute_group platform_dev_group = { + .attrs = platform_dev_attrs, + .is_visible = platform_dev_attrs_visible, +}; +__ATTRIBUTE_GROUPS(platform_dev); + + +/** + * platform_match - bind platform device to platform driver. + * @dev: device. + * @drv: driver. + * + * Platform device IDs are assumed to be encoded like this: + * "", where is a short description of the type of + * device, like "pci" or "floppy", and is the enumerated + * instance of the device, like '0' or '42'. Driver IDs are simply + * "". So, extract the from the platform_device structure, + * and compare it against the name of the driver. Return whether they match + * or not. + */ +static int platform_match(struct device *dev, struct device_driver *drv) +{ + struct platform_device *pdev = to_platform_device(dev); + struct platform_driver *pdrv = to_platform_driver(drv); + + /* When driver_override is set, only bind to the matching driver */ + if (pdev->driver_override) + return !strcmp(pdev->driver_override, drv->name); + + /* Attempt an OF style match first */ + if (of_driver_match_device(dev, drv)) + return 1; + + /* Then try ACPI style match */ + if (acpi_driver_match_device(dev, drv)) + return 1; + + /* Then try to match against the id table */ + if (pdrv->id_table) + return platform_match_id(pdrv->id_table, pdev) != NULL; + + /* fall-back to driver name match */ + return (strcmp(pdev->name, drv->name) == 0); +} + +static int platform_uevent(struct device *dev, struct kobj_uevent_env *env) +{ + struct platform_device *pdev = to_platform_device(dev); + int rc; + + /* Some devices have extra OF data and an OF-style MODALIAS */ + rc = of_device_uevent_modalias(dev, env); + if (rc != -ENODEV) + return rc; + + rc = acpi_device_uevent_modalias(dev, env); + if (rc != -ENODEV) + return rc; + + add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX, + pdev->name); + return 0; +} + int platform_dma_configure(struct device *dev) { enum dev_dma_attr attr; From 16085668eacdc56c46652d0f3bfef81ecace57de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Thu, 19 Nov 2020 13:46:10 +0100 Subject: [PATCH 29/39] driver core: platform: change logic implementing platform_driver_probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of overwriting the core driver's probe function handle probing devices for drivers loaded by platform_driver_probe() in the platform driver probe function. The intended goal is to not have to change the probe function to simplify converting the platform bus to use bus functions. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20201119124611.2573057-2-u.kleine-koenig@pengutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index b36ab5831cb6..b847f5f8f992 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -743,12 +743,25 @@ err: } EXPORT_SYMBOL_GPL(platform_device_register_full); +static int platform_probe_fail(struct platform_device *pdev); + static int platform_drv_probe(struct device *_dev) { struct platform_driver *drv = to_platform_driver(_dev->driver); struct platform_device *dev = to_platform_device(_dev); int ret; + /* + * A driver registered using platform_driver_probe() cannot be bound + * again later because the probe function usually lives in __init code + * and so is gone. For these drivers .probe is set to + * platform_probe_fail in __platform_driver_probe(). Don't even + * prepare clocks and PM domains for these to match the traditional + * behaviour. + */ + if (unlikely(drv->probe == platform_probe_fail)) + return -ENXIO; + ret = of_clk_set_defaults(_dev->of_node, false); if (ret < 0) return ret; @@ -822,7 +835,7 @@ void platform_driver_unregister(struct platform_driver *drv) } EXPORT_SYMBOL_GPL(platform_driver_unregister); -static int platform_drv_probe_fail(struct device *_dev) +static int platform_probe_fail(struct platform_device *pdev) { return -ENXIO; } @@ -887,10 +900,9 @@ int __init_or_module __platform_driver_probe(struct platform_driver *drv, * new devices fail. */ spin_lock(&drv->driver.bus->p->klist_drivers.k_lock); - drv->probe = NULL; + drv->probe = platform_probe_fail; if (code == 0 && list_empty(&drv->driver.p->klist_devices.k_list)) retval = -ENODEV; - drv->driver.probe = platform_drv_probe_fail; spin_unlock(&drv->driver.bus->p->klist_drivers.k_lock); if (code != retval) From 9c30921fe7994907e0b3e0637b2c8c0fc4b5171f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Thu, 19 Nov 2020 13:46:11 +0100 Subject: [PATCH 30/39] driver core: platform: use bus_type functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This works towards the goal mentioned in 2006 in commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown methods."). The functions are moved to where the other bus_type functions are defined and renamed to match the already established naming scheme. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20201119124611.2573057-3-u.kleine-koenig@pengutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 132 ++++++++++++++++++++-------------------- 1 file changed, 65 insertions(+), 67 deletions(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index b847f5f8f992..8ad06daa2eaa 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -743,70 +743,6 @@ err: } EXPORT_SYMBOL_GPL(platform_device_register_full); -static int platform_probe_fail(struct platform_device *pdev); - -static int platform_drv_probe(struct device *_dev) -{ - struct platform_driver *drv = to_platform_driver(_dev->driver); - struct platform_device *dev = to_platform_device(_dev); - int ret; - - /* - * A driver registered using platform_driver_probe() cannot be bound - * again later because the probe function usually lives in __init code - * and so is gone. For these drivers .probe is set to - * platform_probe_fail in __platform_driver_probe(). Don't even - * prepare clocks and PM domains for these to match the traditional - * behaviour. - */ - if (unlikely(drv->probe == platform_probe_fail)) - return -ENXIO; - - ret = of_clk_set_defaults(_dev->of_node, false); - if (ret < 0) - return ret; - - ret = dev_pm_domain_attach(_dev, true); - if (ret) - goto out; - - if (drv->probe) { - ret = drv->probe(dev); - if (ret) - dev_pm_domain_detach(_dev, true); - } - -out: - if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) { - dev_warn(_dev, "probe deferral not supported\n"); - ret = -ENXIO; - } - - return ret; -} - -static int platform_drv_remove(struct device *_dev) -{ - struct platform_driver *drv = to_platform_driver(_dev->driver); - struct platform_device *dev = to_platform_device(_dev); - int ret = 0; - - if (drv->remove) - ret = drv->remove(dev); - dev_pm_domain_detach(_dev, true); - - return ret; -} - -static void platform_drv_shutdown(struct device *_dev) -{ - struct platform_driver *drv = to_platform_driver(_dev->driver); - struct platform_device *dev = to_platform_device(_dev); - - if (drv->shutdown) - drv->shutdown(dev); -} - /** * __platform_driver_register - register a driver for platform-level devices * @drv: platform driver structure @@ -817,9 +753,6 @@ int __platform_driver_register(struct platform_driver *drv, { drv->driver.owner = owner; drv->driver.bus = &platform_bus_type; - drv->driver.probe = platform_drv_probe; - drv->driver.remove = platform_drv_remove; - drv->driver.shutdown = platform_drv_shutdown; return driver_register(&drv->driver); } @@ -1349,6 +1282,68 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env) return 0; } +static int platform_probe(struct device *_dev) +{ + struct platform_driver *drv = to_platform_driver(_dev->driver); + struct platform_device *dev = to_platform_device(_dev); + int ret; + + /* + * A driver registered using platform_driver_probe() cannot be bound + * again later because the probe function usually lives in __init code + * and so is gone. For these drivers .probe is set to + * platform_probe_fail in __platform_driver_probe(). Don't even prepare + * clocks and PM domains for these to match the traditional behaviour. + */ + if (unlikely(drv->probe == platform_probe_fail)) + return -ENXIO; + + ret = of_clk_set_defaults(_dev->of_node, false); + if (ret < 0) + return ret; + + ret = dev_pm_domain_attach(_dev, true); + if (ret) + goto out; + + if (drv->probe) { + ret = drv->probe(dev); + if (ret) + dev_pm_domain_detach(_dev, true); + } + +out: + if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) { + dev_warn(_dev, "probe deferral not supported\n"); + ret = -ENXIO; + } + + return ret; +} + +static int platform_remove(struct device *_dev) +{ + struct platform_driver *drv = to_platform_driver(_dev->driver); + struct platform_device *dev = to_platform_device(_dev); + int ret = 0; + + if (drv->remove) + ret = drv->remove(dev); + dev_pm_domain_detach(_dev, true); + + return ret; +} + +static void platform_shutdown(struct device *_dev) +{ + struct platform_driver *drv = to_platform_driver(_dev->driver); + struct platform_device *dev = to_platform_device(_dev); + + if (drv->shutdown) + drv->shutdown(dev); +} + + int platform_dma_configure(struct device *dev) { enum dev_dma_attr attr; @@ -1375,6 +1370,9 @@ struct bus_type platform_bus_type = { .dev_groups = platform_dev_groups, .match = platform_match, .uevent = platform_uevent, + .probe = platform_probe, + .remove = platform_remove, + .shutdown = platform_shutdown, .dma_configure = platform_dma_configure, .pm = &platform_dev_pm_ops, }; From 0b81144a4ac20f0757b7c965d2fa57de367c7db5 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Mon, 7 Dec 2020 19:59:52 +0100 Subject: [PATCH 31/39] soc: fix comment for freeing soc_dev_attr The soc_dev_attr is stored soc_dev->attr during soc_device_register() so it could be used till the cleanup call: soc_device_unregister(). Therefore this memory should not be freed prior, but after unregistering soc device. Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20201207185952.261697-1-krzk@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/base/soc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/soc.c b/drivers/base/soc.c index d34609bb7386..0af5363a582c 100644 --- a/drivers/base/soc.c +++ b/drivers/base/soc.c @@ -168,7 +168,7 @@ out1: } EXPORT_SYMBOL_GPL(soc_device_register); -/* Ensure soc_dev->attr is freed prior to calling soc_device_unregister. */ +/* Ensure soc_dev->attr is freed after calling soc_device_unregister. */ void soc_device_unregister(struct soc_device *soc_dev) { device_unregister(&soc_dev->dev); From 3577afb0052fca65e67efdfc8e0859bb7bac87a6 Mon Sep 17 00:00:00 2001 From: Jim Cromie Date: Wed, 9 Dec 2020 11:36:25 -0700 Subject: [PATCH 32/39] dyndbg: fix use before null check In commit a2d375eda771 ("dyndbg: refine export, rename to dynamic_debug_exec_queries()"), a string is copied before checking it isn't NULL. Fix this, report a usage/interface error, and return the proper error code. Fixes: a2d375eda771 ("dyndbg: refine export, rename to dynamic_debug_exec_queries()") Cc: stable@vger.kernel.org -- -v2 drop comment tweak, improve commit message Signed-off-by: Jim Cromie Link: https://lore.kernel.org/r/20201209183625.2432329-1-jim.cromie@gmail.com Signed-off-by: Greg Kroah-Hartman --- lib/dynamic_debug.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index bd7b3aaa93c3..c70d6347afa2 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -561,9 +561,14 @@ static int ddebug_exec_queries(char *query, const char *modname) int dynamic_debug_exec_queries(const char *query, const char *modname) { int rc; - char *qry = kstrndup(query, PAGE_SIZE, GFP_KERNEL); + char *qry; /* writable copy of query */ - if (!query) + if (!query) { + pr_err("non-null query/command string expected\n"); + return -EINVAL; + } + qry = kstrndup(query, PAGE_SIZE, GFP_KERNEL); + if (!qry) return -ENOMEM; rc = ddebug_exec_queries(qry, modname); From 0aec2da436623abe19b80b21dd9fc5ec9300a152 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 9 Dec 2020 22:36:38 +0200 Subject: [PATCH 33/39] driver core: platform: Introduce platform_get_mem_or_io() There are at least few existing users of the proposed API which retrieves either MEM or IO resource from platform device. Make it common to utilize in the existing and new users. Cc: Eric Auger Cc: Alex Williamson Cc: kvm@vger.kernel.org Cc: linux-usb@vger.kernel.org Cc: Peng Hao Cc: Arnd Bergmann Reviewed-by: Cornelia Huck Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20201209203642.27648-1-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 15 +++++++++++++++ include/linux/platform_device.h | 3 +++ 2 files changed, 18 insertions(+) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 8ad06daa2eaa..0358dc3ea3ad 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -63,6 +63,21 @@ struct resource *platform_get_resource(struct platform_device *dev, } EXPORT_SYMBOL_GPL(platform_get_resource); +struct resource *platform_get_mem_or_io(struct platform_device *dev, + unsigned int num) +{ + u32 i; + + for (i = 0; i < dev->num_resources; i++) { + struct resource *r = &dev->resource[i]; + + if ((resource_type(r) & (IORESOURCE_MEM|IORESOURCE_IO)) && num-- == 0) + return r; + } + return NULL; +} +EXPORT_SYMBOL_GPL(platform_get_mem_or_io); + #ifdef CONFIG_HAS_IOMEM /** * devm_platform_get_and_ioremap_resource - call devm_ioremap_resource() for a diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 77a2aada106d..ee6a9f10c2c7 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -52,6 +52,9 @@ extern struct device platform_bus; extern struct resource *platform_get_resource(struct platform_device *, unsigned int, unsigned int); +extern struct resource *platform_get_mem_or_io(struct platform_device *, + unsigned int); + extern struct device * platform_find_device_by_driver(struct device *start, const struct device_driver *drv); From feaba5932b6f4bfc875c874a3b7a28c7f05f5a77 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 9 Dec 2020 22:36:39 +0200 Subject: [PATCH 34/39] vfio: platform: Switch to use platform_get_mem_or_io() Switch to use new platform_get_mem_or_io() instead of home grown analogue. Cc: Alex Williamson Cc: Cornelia Huck Cc: kvm@vger.kernel.org Acked-by: Eric Auger Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20201209203642.27648-2-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/vfio/platform/vfio_platform.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c index 1e2769010089..9fb6818cea12 100644 --- a/drivers/vfio/platform/vfio_platform.c +++ b/drivers/vfio/platform/vfio_platform.c @@ -25,19 +25,8 @@ static struct resource *get_platform_resource(struct vfio_platform_device *vdev, int num) { struct platform_device *dev = (struct platform_device *) vdev->opaque; - int i; - for (i = 0; i < dev->num_resources; i++) { - struct resource *r = &dev->resource[i]; - - if (resource_type(r) & (IORESOURCE_MEM|IORESOURCE_IO)) { - if (!num) - return r; - - num--; - } - } - return NULL; + return platform_get_mem_or_io(dev, num); } static int get_platform_irq(struct vfio_platform_device *vdev, int i) From ebee0cde1960a41e40840eaa8fe0185aa20e3eb5 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 9 Dec 2020 22:36:40 +0200 Subject: [PATCH 35/39] usb: host: sl811: Switch to use platform_get_mem_or_io() Switch to use new platform_get_mem_or_io() instead of home grown analogue. Note, the code has been moved upper in the function to allow farther cleanups, such as resource sanity check. Cc: linux-usb@vger.kernel.org Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20201209203642.27648-3-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/sl811-hcd.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/drivers/usb/host/sl811-hcd.c b/drivers/usb/host/sl811-hcd.c index adaf4063690a..115ced0d93e1 100644 --- a/drivers/usb/host/sl811-hcd.c +++ b/drivers/usb/host/sl811-hcd.c @@ -1614,12 +1614,18 @@ sl811h_probe(struct platform_device *dev) void __iomem *addr_reg; void __iomem *data_reg; int retval; - u8 tmp, ioaddr = 0; + u8 tmp, ioaddr; unsigned long irqflags; if (usb_disabled()) return -ENODEV; + /* the chip may be wired for either kind of addressing */ + addr = platform_get_mem_or_io(dev, 0); + data = platform_get_mem_or_io(dev, 1); + if (!addr || !data || resource_type(addr) != resource_type(data)) + return -ENODEV; + /* basic sanity checks first. board-specific init logic should * have initialized these three resources and probably board * specific platform_data. we don't probe for IRQs, and do only @@ -1632,16 +1638,8 @@ sl811h_probe(struct platform_device *dev) irq = ires->start; irqflags = ires->flags & IRQF_TRIGGER_MASK; - /* the chip may be wired for either kind of addressing */ - addr = platform_get_resource(dev, IORESOURCE_MEM, 0); - data = platform_get_resource(dev, IORESOURCE_MEM, 1); - retval = -EBUSY; - if (!addr || !data) { - addr = platform_get_resource(dev, IORESOURCE_IO, 0); - data = platform_get_resource(dev, IORESOURCE_IO, 1); - if (!addr || !data) - return -ENODEV; - ioaddr = 1; + ioaddr = resource_type(addr) == IORESOURCE_IO; + if (ioaddr) { /* * NOTE: 64-bit resource->start is getting truncated * to avoid compiler warning, assuming that ->start From f104060813fe79350565b68c1098fcea050a6504 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 9 Dec 2020 22:36:41 +0200 Subject: [PATCH 36/39] misc: pvpanic: Combine ACPI and platform drivers There is nothing special in the driver that requires to have a special ACPI driver for it. Combine both into simple platform driver. Cc: Peng Hao Cc: Arnd Bergmann Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20201209203642.27648-4-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/misc/pvpanic.c | 130 ++++++----------------------------------- 1 file changed, 17 insertions(+), 113 deletions(-) diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c index e16a5e51006e..fcab2efd0c45 100644 --- a/drivers/misc/pvpanic.c +++ b/drivers/misc/pvpanic.c @@ -8,7 +8,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt -#include +#include #include #include #include @@ -49,101 +49,16 @@ static struct notifier_block pvpanic_panic_nb = { .priority = 1, /* let this called before broken drm_fb_helper */ }; -#ifdef CONFIG_ACPI -static int pvpanic_add(struct acpi_device *device); -static int pvpanic_remove(struct acpi_device *device); - -static const struct acpi_device_id pvpanic_device_ids[] = { - { "QEMU0001", 0 }, - { "", 0 } -}; -MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids); - -static struct acpi_driver pvpanic_driver = { - .name = "pvpanic", - .class = "QEMU", - .ids = pvpanic_device_ids, - .ops = { - .add = pvpanic_add, - .remove = pvpanic_remove, - }, - .owner = THIS_MODULE, -}; - -static acpi_status -pvpanic_walk_resources(struct acpi_resource *res, void *context) -{ - struct resource r; - - if (acpi_dev_resource_io(res, &r)) { -#ifdef CONFIG_HAS_IOPORT_MAP - base = ioport_map(r.start, resource_size(&r)); - return AE_OK; -#else - return AE_ERROR; -#endif - } else if (acpi_dev_resource_memory(res, &r)) { - base = ioremap(r.start, resource_size(&r)); - return AE_OK; - } - - return AE_ERROR; -} - -static int pvpanic_add(struct acpi_device *device) -{ - int ret; - - ret = acpi_bus_get_status(device); - if (ret < 0) - return ret; - - if (!device->status.enabled || !device->status.functional) - return -ENODEV; - - acpi_walk_resources(device->handle, METHOD_NAME__CRS, - pvpanic_walk_resources, NULL); - - if (!base) - return -ENODEV; - - atomic_notifier_chain_register(&panic_notifier_list, - &pvpanic_panic_nb); - - return 0; -} - -static int pvpanic_remove(struct acpi_device *device) -{ - - atomic_notifier_chain_unregister(&panic_notifier_list, - &pvpanic_panic_nb); - iounmap(base); - - return 0; -} - -static int pvpanic_register_acpi_driver(void) -{ - return acpi_bus_register_driver(&pvpanic_driver); -} - -static void pvpanic_unregister_acpi_driver(void) -{ - acpi_bus_unregister_driver(&pvpanic_driver); -} -#else -static int pvpanic_register_acpi_driver(void) -{ - return -ENODEV; -} - -static void pvpanic_unregister_acpi_driver(void) {} -#endif - static int pvpanic_mmio_probe(struct platform_device *pdev) { - base = devm_platform_ioremap_resource(pdev, 0); + struct device *dev = &pdev->dev; + struct resource *res; + + res = platform_get_mem_or_io(pdev, 0); + if (res && resource_type(res) == IORESOURCE_IO) + base = devm_ioport_map(dev, res->start, resource_size(res)); + else + base = devm_ioremap_resource(dev, res); if (IS_ERR(base)) return PTR_ERR(base); @@ -167,30 +82,19 @@ static const struct of_device_id pvpanic_mmio_match[] = { {} }; +static const struct acpi_device_id pvpanic_device_ids[] = { + { "QEMU0001", 0 }, + { "", 0 } +}; +MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids); + static struct platform_driver pvpanic_mmio_driver = { .driver = { .name = "pvpanic-mmio", .of_match_table = pvpanic_mmio_match, + .acpi_match_table = pvpanic_device_ids, }, .probe = pvpanic_mmio_probe, .remove = pvpanic_mmio_remove, }; - -static int __init pvpanic_mmio_init(void) -{ - if (acpi_disabled) - return platform_driver_register(&pvpanic_mmio_driver); - else - return pvpanic_register_acpi_driver(); -} - -static void __exit pvpanic_mmio_exit(void) -{ - if (acpi_disabled) - platform_driver_unregister(&pvpanic_mmio_driver); - else - pvpanic_unregister_acpi_driver(); -} - -module_init(pvpanic_mmio_init); -module_exit(pvpanic_mmio_exit); +module_platform_driver(pvpanic_mmio_driver); From 2a0387e8128ae0c3de9ff976bc25afaae3d4a916 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 9 Dec 2020 22:36:42 +0200 Subject: [PATCH 37/39] misc: pvpanic: Replace OF headers by mod_devicetable.h There is no use for OF headers in the driver, but mod_devicetable.h must be included. Update driver accordingly. Cc: Peng Hao Cc: Arnd Bergmann Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20201209203642.27648-5-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/misc/pvpanic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c index fcab2efd0c45..951b37da5e3c 100644 --- a/drivers/misc/pvpanic.c +++ b/drivers/misc/pvpanic.c @@ -11,11 +11,11 @@ #include #include #include +#include #include -#include -#include #include #include + #include static void __iomem *base; From 2ab695aa8eb8f3226f68a2b91fc6103b56fcb57d Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Fri, 11 Dec 2020 12:26:29 -0800 Subject: [PATCH 38/39] ACPI: Use fwnode_init() to set up fwnode Commit 01bb86b380a3 ("driver core: Add fwnode_init()") was supposed to fix up all instances of fwnode creation to use fwnode_init(). But looks like this instance was missed. This causes a NULL pointer dereference during device_add() [1]. So, fix it. [ 60.792324][ T1] Call trace: [ 60.795495][ T1] device_add+0xf60/0x16b0 __fw_devlink_link_to_consumers at drivers/base/core.c:1583 (inlined by) fw_devlink_link_device at drivers/base/core.c:1726 (inlined by) device_add at drivers/base/core.c:3088 [ 60.799813][ T1] platform_device_add+0x274/0x628 [ 60.804833][ T1] acpi_iort_init+0x9d8/0xc50 [ 60.809415][ T1] acpi_init+0x45c/0x4e8 [ 60.813556][ T1] do_one_initcall+0x170/0xb70 [ 60.818224][ T1] kernel_init_freeable+0x6a8/0x734 [ 60.823332][ T1] kernel_init+0x18/0x12c [ 60.827566][ T1] ret_from_fork+0x10/0x1c [ 60.838756][ T1] ---[ end trace fa5c8ce17a226d83 ]--- [1] - https://lore.kernel.org/linux-arm-kernel/02e7047071f0b54b046ac472adeeb3fafabc643c.camel@redhat.com/ Fixes: 01bb86b380a3 ("driver core: Add fwnode_init()") Reported-by: Qian Cai Suggested-by: Robin Murphy Tested-by: Marc Zyngier Acked-by: Rafael J. Wysocki Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20201211202629.2164655-1-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- include/linux/acpi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 39263c6b52e1..2630c2e953f7 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -55,7 +55,7 @@ static inline struct fwnode_handle *acpi_alloc_fwnode_static(void) if (!fwnode) return NULL; - fwnode->ops = &acpi_static_fwnode_ops; + fwnode_init(fwnode, &acpi_static_fwnode_ops); return fwnode; } From 46e85af0cc53f35584e00bb5db7db6893d0e16e5 Mon Sep 17 00:00:00 2001 From: Dmitry Baryshkov Date: Sun, 13 Dec 2020 02:55:33 +0300 Subject: [PATCH 39/39] driver core: platform: don't oops in platform_shutdown() on unbound devices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On shutdown the driver core calls the bus' shutdown callback also for unbound devices. A driver's shutdown callback however is only called for devices bound to this driver. Commit 9c30921fe799 ("driver core: platform: use bus_type functions") changed the platform bus from driver callbacks to bus callbacks, so the shutdown function must be prepared to be called without a driver. Add the corresponding check in the shutdown function. Fixes: 9c30921fe799 ("driver core: platform: use bus_type functions") Tested-by: Guenter Roeck Reviewed-by: Uwe Kleine-König Signed-off-by: Dmitry Baryshkov Link: https://lore.kernel.org/r/20201212235533.247537-1-dmitry.baryshkov@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 0358dc3ea3ad..e9477e0bbca5 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -1351,9 +1351,13 @@ static int platform_remove(struct device *_dev) static void platform_shutdown(struct device *_dev) { - struct platform_driver *drv = to_platform_driver(_dev->driver); struct platform_device *dev = to_platform_device(_dev); + struct platform_driver *drv; + if (!_dev->driver) + return; + + drv = to_platform_driver(_dev->driver); if (drv->shutdown) drv->shutdown(dev); }