From ab7c1e163b525316a870a494dd4ea196e7a6c455 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 15 Apr 2020 18:45:00 +0200 Subject: [PATCH 01/23] firmware: Drop unused pages field from struct firmware The struct firmware contains a page table pointer that was used only internally in the past. Since the actual page tables are referred from struct fw_priv and should be never from struct firmware, we can drop this unused field gracefully. Signed-off-by: Takashi Iwai Acked-by: Luis Chamberlain Link: https://lore.kernel.org/r/20200415164500.28749-1-tiwai@suse.de Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/main.c | 3 --- include/linux/firmware.h | 1 - 2 files changed, 4 deletions(-) diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 76f79913916d..5296aaca35cf 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -548,9 +548,6 @@ static void firmware_free_data(const struct firmware *fw) static void fw_set_page_data(struct fw_priv *fw_priv, struct firmware *fw) { fw->priv = fw_priv; -#ifdef CONFIG_FW_LOADER_USER_HELPER - fw->pages = fw_priv->pages; -#endif fw->size = fw_priv->size; fw->data = fw_priv->data; diff --git a/include/linux/firmware.h b/include/linux/firmware.h index 4bbd0afd91b7..cb3e2c06ed8a 100644 --- a/include/linux/firmware.h +++ b/include/linux/firmware.h @@ -12,7 +12,6 @@ struct firmware { size_t size; const u8 *data; - struct page **pages; /* firmware loader private fields */ void *priv; From 55623260bb33e2ab849af76edf2253bc04cb241f Mon Sep 17 00:00:00 2001 From: Scott Branden Date: Tue, 14 Apr 2020 17:25:17 -0700 Subject: [PATCH 02/23] test_firmware: remove unnecessary test_fw_mutex in test_dev_config_show_xxx Remove unnecessary use of test_fw_mutex in test_dev_config_show_xxx functions that show simple bool, int, and u8. Signed-off-by: Scott Branden Reviewed-by: Kees Cook Reviewed-by: Luis Chamberlain Signed-off-by: Andrew Morton Link: https://lore.kernel.org/r/20200415002517.4328-1-scott.branden@broadcom.com Signed-off-by: Greg Kroah-Hartman --- lib/test_firmware.c | 26 +++----------------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/lib/test_firmware.c b/lib/test_firmware.c index 0c7fbcf07ac5..9fee2b93a8d1 100644 --- a/lib/test_firmware.c +++ b/lib/test_firmware.c @@ -310,27 +310,13 @@ static int test_dev_config_update_bool(const char *buf, size_t size, return ret; } -static ssize_t -test_dev_config_show_bool(char *buf, - bool config) +static ssize_t test_dev_config_show_bool(char *buf, bool val) { - bool val; - - mutex_lock(&test_fw_mutex); - val = config; - mutex_unlock(&test_fw_mutex); - return snprintf(buf, PAGE_SIZE, "%d\n", val); } -static ssize_t test_dev_config_show_int(char *buf, int cfg) +static ssize_t test_dev_config_show_int(char *buf, int val) { - int val; - - mutex_lock(&test_fw_mutex); - val = cfg; - mutex_unlock(&test_fw_mutex); - return snprintf(buf, PAGE_SIZE, "%d\n", val); } @@ -354,14 +340,8 @@ static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) return size; } -static ssize_t test_dev_config_show_u8(char *buf, u8 cfg) +static ssize_t test_dev_config_show_u8(char *buf, u8 val) { - u8 val; - - mutex_lock(&test_fw_mutex); - val = cfg; - mutex_unlock(&test_fw_mutex); - return snprintf(buf, PAGE_SIZE, "%u\n", val); } From 0f605db5bdd42edfbfcac36acaf8f72cfe9ce774 Mon Sep 17 00:00:00 2001 From: Waiman Long Date: Thu, 2 Apr 2020 13:10:56 -0400 Subject: [PATCH 03/23] kernfs: Change kernfs_node lockdep name to "kn->active" The kernfs_node lockdep tracking is being done on kn->active, the active reference count. The other reference count (kn->count) is not tracked by lockdep. So change the lockdep name to reflect what it is tracking. Signed-off-by: Waiman Long Acked-by: Tejun Heo Link: https://lore.kernel.org/r/20200402171056.27871-1-longman@redhat.com Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c index 34366db3620d..fd6ddfe4cd94 100644 --- a/fs/kernfs/file.c +++ b/fs/kernfs/file.c @@ -1010,7 +1010,7 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent, #ifdef CONFIG_DEBUG_LOCK_ALLOC if (key) { - lockdep_init_map(&kn->dep_map, "kn->count", key, 0); + lockdep_init_map(&kn->dep_map, "kn->active", key, 0); kn->flags |= KERNFS_LOCKDEP; } #endif From 69b07ee33eb12a505d55e3e716fc7452496b9041 Mon Sep 17 00:00:00 2001 From: Nishad Kamdar Date: Sun, 19 Apr 2020 20:18:55 +0530 Subject: [PATCH 04/23] debugfs: Use the correct style for SPDX License Identifier This patch corrects the SPDX License Identifier style in header file related to debugfs File System support. For C header files Documentation/process/license-rules.rst mandates C-like comments (opposed to C source files where C++ style should be used). Changes made by using a script provided by Joe Perches here: https://lkml.org/lkml/2019/2/7/46. Suggested-by: Joe Perches Signed-off-by: Nishad Kamdar Link: https://lore.kernel.org/r/20200419144852.GA9206@nishad Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h index f0d73d86cc1a..034e6973cead 100644 --- a/fs/debugfs/internal.h +++ b/fs/debugfs/internal.h @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 +/* SPDX-License-Identifier: GPL-2.0 */ /* * internal.h - declarations internal to debugfs * From 45bb08de65b418959313593f527c619e102c2d57 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Thu, 2 Apr 2020 12:13:41 +0100 Subject: [PATCH 05/23] driver core: platform: remove redundant assignment to variable ret The variable ret is being initialized with a value that is never read and it is being updated later with a new value. The initialization is redundant and can be removed. Addresses-Coverity: ("Unused value") Signed-off-by: Colin Ian King Link: https://lore.kernel.org/r/20200402111341.511801-1-colin.king@canonical.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 5255550b7c34..2e454c18b3a9 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -668,7 +668,7 @@ EXPORT_SYMBOL_GPL(platform_device_unregister); struct platform_device *platform_device_register_full( const struct platform_device_info *pdevinfo) { - int ret = -ENOMEM; + int ret; struct platform_device *pdev; pdev = platform_device_alloc(pdevinfo->name, pdevinfo->id); From fbc35b45f9f6a971341b9462c6e94c257e779fb5 Mon Sep 17 00:00:00 2001 From: Grant Likely Date: Fri, 27 Mar 2020 17:01:32 +0000 Subject: [PATCH 06/23] Add documentation on meaning of -EPROBE_DEFER Add a bit of documentation on what it means when a driver .probe() hook returns the -EPROBE_DEFER error code, including the limitation that -EPROBE_DEFER should be returned as early as possible, before the driver starts to register child devices. Also: minor markup fixes in the same file Cc: Greg Kroah-Hartman Cc: Jonathan Corbet Cc: Saravana Kannan Cc: Andy Shevchenko Reviewed-by: Andy Shevchenko Signed-off-by: Grant Likely Link: https://lore.kernel.org/r/20200327170132.17275-1-grant.likely@arm.com Signed-off-by: Greg Kroah-Hartman --- .../driver-api/driver-model/driver.rst | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/Documentation/driver-api/driver-model/driver.rst b/Documentation/driver-api/driver-model/driver.rst index 63887b813005..7d5040f6a3d8 100644 --- a/Documentation/driver-api/driver-model/driver.rst +++ b/Documentation/driver-api/driver-model/driver.rst @@ -4,7 +4,6 @@ Device Drivers See the kerneldoc for the struct device_driver. - Allocation ~~~~~~~~~~ @@ -167,9 +166,26 @@ the driver to that device. A driver's probe() may return a negative errno value to indicate that the driver did not bind to this device, in which case it should have -released all resources it allocated:: +released all resources it allocated. - void (*sync_state)(struct device *dev); +Optionally, probe() may return -EPROBE_DEFER if the driver depends on +resources that are not yet available (e.g., supplied by a driver that +hasn't initialized yet). The driver core will put the device onto the +deferred probe list and will try to call it again later. If a driver +must defer, it should return -EPROBE_DEFER as early as possible to +reduce the amount of time spent on setup work that will need to be +unwound and reexecuted at a later time. + +.. warning:: + -EPROBE_DEFER must not be returned if probe() has already created + child devices, even if those child devices are removed again + in a cleanup path. If -EPROBE_DEFER is returned after a child + device has been registered, it may result in an infinite loop of + .probe() calls to the same driver. + +:: + + void (*sync_state) (struct device *dev); sync_state is called only once for a device. It's called when all the consumer devices of the device have successfully probed. The list of consumers of the @@ -212,6 +228,8 @@ over management of devices from the bootloader, the usage of sync_state() is not restricted to that. Use it whenever it makes sense to take an action after all the consumers of a device have probed:: +:: + int (*remove) (struct device *dev); remove is called to unbind a driver from a device. This may be @@ -224,11 +242,15 @@ not. It should free any resources allocated specifically for the device; i.e. anything in the device's driver_data field. If the device is still present, it should quiesce the device and place -it into a supported low-power state:: +it into a supported low-power state. + +:: int (*suspend) (struct device *dev, pm_message_t state); -suspend is called to put the device in a low power state:: +suspend is called to put the device in a low power state. + +:: int (*resume) (struct device *dev); From baf1d9c182935e88aab08701b0a0b22871117fe0 Mon Sep 17 00:00:00 2001 From: zhouchuangao Date: Sun, 26 Apr 2020 13:13:51 +0800 Subject: [PATCH 07/23] driver/base/soc: Use kobj_to_dev() API Use kobj_to_dev() API instead of container_of(). Signed-off-by: zhouchuangao Link: https://lore.kernel.org/r/1587878031-16591-1-git-send-email-zhouchuangao@xiaomi.com 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 4af11a423475..a5bae551167d 100644 --- a/drivers/base/soc.c +++ b/drivers/base/soc.c @@ -46,7 +46,7 @@ static umode_t soc_attribute_mode(struct kobject *kobj, struct attribute *attr, int index) { - struct device *dev = container_of(kobj, struct device, kobj); + struct device *dev = kobj_to_dev(kobj); struct soc_device *soc_dev = container_of(dev, struct soc_device, dev); if ((attr == &dev_attr_machine.attr) From eb7fbc9fb1185a7f89adeb2de724c2c96ff608e9 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Sat, 11 Apr 2020 15:31:58 +0200 Subject: [PATCH 08/23] driver core: Add missing '\n' in log messages Message logged by 'dev_xxx()' or 'pr_xxx()' should end with a '\n'. While at it, convert some "printk(KERN_" into equivalent but less verbose (pr|dev)_xxx functions. Signed-off-by: Christophe JAILLET Link: https://lore.kernel.org/r/20200411133158.27390-1-christophe.jaillet@wanadoo.fr Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 06ec0e851fa1..efe6df5bff26 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -262,12 +262,12 @@ __setup("deferred_probe_timeout=", deferred_probe_timeout_setup); int driver_deferred_probe_check_state(struct device *dev) { if (!IS_ENABLED(CONFIG_MODULES) && initcalls_done) { - dev_warn(dev, "ignoring dependency for device, assuming no driver"); + dev_warn(dev, "ignoring dependency for device, assuming no driver\n"); return -ENODEV; } if (!driver_deferred_probe_timeout) { - dev_WARN(dev, "deferred probe timeout, ignoring dependency"); + dev_WARN(dev, "deferred probe timeout, ignoring dependency\n"); return -ETIMEDOUT; } @@ -283,7 +283,7 @@ static void deferred_probe_timeout_work_func(struct work_struct *work) flush_work(&deferred_probe_work); list_for_each_entry_safe(private, p, &deferred_probe_pending_list, deferred_probe) - dev_info(private->device, "deferred probe pending"); + dev_info(private->device, "deferred probe pending\n"); } static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_work_func); @@ -343,7 +343,7 @@ bool device_is_bound(struct device *dev) static void driver_bound(struct device *dev) { if (device_is_bound(dev)) { - printk(KERN_WARNING "%s: device %s already bound\n", + pr_warn("%s: device %s already bound\n", __func__, kobject_name(&dev->kobj)); return; } @@ -512,8 +512,8 @@ re_probe: } if (driver_sysfs_add(dev)) { - printk(KERN_ERR "%s: driver_sysfs_add(%s) failed\n", - __func__, dev_name(dev)); + pr_err("%s: driver_sysfs_add(%s) failed\n", + __func__, dev_name(dev)); goto probe_failed; } @@ -604,9 +604,8 @@ pinctrl_bind_failed: break; default: /* driver matched but the probe failed */ - printk(KERN_WARNING - "%s: probe of %s failed with error %d\n", - drv->name, dev_name(dev), ret); + pr_warn("%s: probe of %s failed with error %d\n", + drv->name, dev_name(dev), ret); } /* * Ignore errors returned by ->probe so that the next driver can try @@ -631,8 +630,8 @@ static int really_probe_debug(struct device *dev, struct device_driver *drv) ret = really_probe(dev, drv); rettime = ktime_get(); delta = ktime_sub(rettime, calltime); - printk(KERN_DEBUG "probe of %s returned %d after %lld usecs\n", - dev_name(dev), ret, (s64) ktime_to_us(delta)); + pr_debug("probe of %s returned %d after %lld usecs\n", + dev_name(dev), ret, (s64) ktime_to_us(delta)); return ret; } @@ -717,8 +716,7 @@ static inline bool cmdline_requested_async_probing(const char *drv_name) static int __init save_async_options(char *buf) { if (strlen(buf) >= ASYNC_DRV_NAMES_MAX_LEN) - printk(KERN_WARNING - "Too long list of driver names for 'driver_async_probe'!\n"); + pr_warn("Too long list of driver names for 'driver_async_probe'!\n"); strlcpy(async_probe_drv_names, buf, ASYNC_DRV_NAMES_MAX_LEN); return 0; @@ -793,7 +791,7 @@ static int __device_attach_driver(struct device_driver *drv, void *_data) dev_dbg(dev, "Device match requests probe deferral\n"); driver_deferred_probe_add(dev); } else if (ret < 0) { - dev_dbg(dev, "Bus failed to match device: %d", ret); + dev_dbg(dev, "Bus failed to match device: %d\n", ret); return ret; } /* ret > 0 means positive match */ @@ -1026,7 +1024,7 @@ static int __driver_attach(struct device *dev, void *data) dev_dbg(dev, "Device match requests probe deferral\n"); driver_deferred_probe_add(dev); } else if (ret < 0) { - dev_dbg(dev, "Bus failed to match device: %d", ret); + dev_dbg(dev, "Bus failed to match device: %d\n", ret); return ret; } /* ret > 0 means positive match */ From 48ebea5026d692c5ab0a7d303f0fe1f8ba046e0f Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Fri, 24 Apr 2020 18:49:16 +0000 Subject: [PATCH 09/23] firmware_loader: move fw_fallback_config to a private kernel symbol namespace Take advantage of the new kernel symbol namespacing functionality, and export the fw_fallback_config symbol only to a new private firmware loader namespace. This would prevent misuses from other drivers and makes it clear the goal is to keep this private to the firmware loader only. It should also make it clearer for folks git grep'ing for users of the symbol that this exported symbol is private, and prevent future accidental removals of the exported symbol. Signed-off-by: Luis Chamberlain Link: https://lore.kernel.org/r/20200424184916.22843-2-mcgrof@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/fallback.c | 3 +++ drivers/base/firmware_loader/fallback_table.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c index 1e9c96e3ed63..d9ac7296205e 100644 --- a/drivers/base/firmware_loader/fallback.c +++ b/drivers/base/firmware_loader/fallback.c @@ -9,6 +9,7 @@ #include #include #include +#include #include "fallback.h" #include "firmware.h" @@ -17,6 +18,8 @@ * firmware fallback mechanism */ +MODULE_IMPORT_NS(FIRMWARE_LOADER_PRIVATE); + extern struct firmware_fallback_config fw_fallback_config; /* These getters are vetted to use int properly */ diff --git a/drivers/base/firmware_loader/fallback_table.c b/drivers/base/firmware_loader/fallback_table.c index a182e318bd09..46a731dede6f 100644 --- a/drivers/base/firmware_loader/fallback_table.c +++ b/drivers/base/firmware_loader/fallback_table.c @@ -21,7 +21,7 @@ struct firmware_fallback_config fw_fallback_config = { .loading_timeout = 60, .old_timeout = 60, }; -EXPORT_SYMBOL_GPL(fw_fallback_config); +EXPORT_SYMBOL_NS_GPL(fw_fallback_config, FIRMWARE_LOADER_PRIVATE); #ifdef CONFIG_SYSCTL struct ctl_table firmware_config_table[] = { From 388bcc6ecc609fca1b4920de7dc3806c98ec535e Mon Sep 17 00:00:00 2001 From: Kuppuswamy Sathyanarayanan Date: Wed, 8 Apr 2020 14:40:03 -0700 Subject: [PATCH 10/23] drivers: base: Fix NULL pointer exception in __platform_driver_probe() if a driver developer is foolish If platform bus driver registration is failed then, accessing platform bus spin lock (&drv->driver.bus->p->klist_drivers.k_lock) in __platform_driver_probe() without verifying the return value __platform_driver_register() can lead to NULL pointer exception. So check the return value before attempting the spin lock. One such example is below: For a custom usecase, I have intentionally failed the platform bus registration and I expected all the platform device/driver registrations to fail gracefully. But I came across this panic issue. [ 1.331067] BUG: kernel NULL pointer dereference, address: 00000000000000c8 [ 1.331118] #PF: supervisor write access in kernel mode [ 1.331163] #PF: error_code(0x0002) - not-present page [ 1.331208] PGD 0 P4D 0 [ 1.331233] Oops: 0002 [#1] PREEMPT SMP [ 1.331268] CPU: 3 PID: 1 Comm: swapper/0 Tainted: G W 5.6.0-00049-g670d35fb0144 #165 [ 1.331341] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 [ 1.331406] RIP: 0010:_raw_spin_lock+0x15/0x30 [ 1.331588] RSP: 0000:ffffc9000001be70 EFLAGS: 00010246 [ 1.331632] RAX: 0000000000000000 RBX: 00000000000000c8 RCX: 0000000000000001 [ 1.331696] RDX: 0000000000000001 RSI: 0000000000000092 RDI: 0000000000000000 [ 1.331754] RBP: 00000000ffffffed R08: 0000000000000501 R09: 0000000000000001 [ 1.331817] R10: ffff88817abcc520 R11: 0000000000000670 R12: 00000000ffffffed [ 1.331881] R13: ffffffff82dbc268 R14: ffffffff832f070a R15: 0000000000000000 [ 1.331945] FS: 0000000000000000(0000) GS:ffff88817bd80000(0000) knlGS:0000000000000000 [ 1.332008] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1.332062] CR2: 00000000000000c8 CR3: 000000000681e001 CR4: 00000000003606e0 [ 1.332126] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 1.332189] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 1.332252] Call Trace: [ 1.332281] __platform_driver_probe+0x92/0xee [ 1.332323] ? rtc_dev_init+0x2b/0x2b [ 1.332358] cmos_init+0x37/0x67 [ 1.332396] do_one_initcall+0x7d/0x168 [ 1.332428] kernel_init_freeable+0x16c/0x1c9 [ 1.332473] ? rest_init+0xc0/0xc0 [ 1.332508] kernel_init+0x5/0x100 [ 1.332543] ret_from_fork+0x1f/0x30 [ 1.332579] CR2: 00000000000000c8 [ 1.332616] ---[ end trace 3bd87f12e9010b87 ]--- [ 1.333549] note: swapper/0[1] exited with preempt_count 1 [ 1.333592] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009 [ 1.333736] Kernel Offset: disabled Note, this can only be triggered if a driver errors out from this call, which should never happen. If it does, the driver needs to be fixed. Signed-off-by: Kuppuswamy Sathyanarayanan Link: https://lore.kernel.org/r/20200408214003.3356-1-sathyanarayanan.kuppuswamy@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 2e454c18b3a9..2c1812d75762 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -849,6 +849,8 @@ int __init_or_module __platform_driver_probe(struct platform_driver *drv, /* temporary section violation during probe() */ drv->probe = probe; retval = code = __platform_driver_register(drv, module); + if (retval) + return retval; /* * Fixup that section violation, being paranoid about code scanning From c78c31b374a68be79cb4a03ef5b6c187f034e903 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Tue, 28 Apr 2020 12:20:05 -0700 Subject: [PATCH 11/23] Revert "Revert "driver core: Set fw_devlink to "permissive" behavior by default"" This reverts commit 18555cb6db2373b9a5ec1f7572773fd58c77f9ba. The reason[1] for the original revert has now been fixed by commit 00b247557858 ("driver core: Fix handling of fw_devlink=permissive"). So, this patch reverts the revert. Marek has also tested this patch with the fix mentioned above and confirmed that the issue has been fixed. [1] - https://lore.kernel.org/lkml/CAGETcx8nbz-J1gLvoEKE_HgCcVGyV2o8rZeq_USFKM6=s7WmNg@mail.gmail.com/T/#m12dfb5dfd23805b84c49f4bb2238a8cce436c2f7 [2] - https://lore.kernel.org/lkml/CAGETcx8nbz-J1gLvoEKE_HgCcVGyV2o8rZeq_USFKM6=s7WmNg@mail.gmail.com/T/#m2408a6ce098b2ebf583ca8534329695923ae57fe Tested-by: Marek Szyprowski Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20200428192006.109006-1-saravanak@google.com 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 139cdf7e7327..db103e32489b 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2345,7 +2345,7 @@ static int device_private_init(struct device *dev) return 0; } -static u32 fw_devlink_flags; +static u32 fw_devlink_flags = DL_FLAG_SYNC_STATE_ONLY; static int __init fw_devlink_setup(char *arg) { if (!arg) From 42926ac3cd50937346c23c0005817264af4357a7 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Thu, 14 May 2020 22:34:57 -0700 Subject: [PATCH 12/23] driver core: Move code to the right part of the file This commit just moves around code to match the general organization of the file. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20200515053500.215929-2-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 60 ++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index c9045521596f..2b454aae64b5 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1143,6 +1143,36 @@ static void device_links_purge(struct device *dev) device_links_write_unlock(); } +static u32 fw_devlink_flags = DL_FLAG_SYNC_STATE_ONLY; +static int __init fw_devlink_setup(char *arg) +{ + if (!arg) + return -EINVAL; + + if (strcmp(arg, "off") == 0) { + fw_devlink_flags = 0; + } else if (strcmp(arg, "permissive") == 0) { + fw_devlink_flags = DL_FLAG_SYNC_STATE_ONLY; + } else if (strcmp(arg, "on") == 0) { + fw_devlink_flags = DL_FLAG_AUTOPROBE_CONSUMER; + } else if (strcmp(arg, "rpm") == 0) { + fw_devlink_flags = DL_FLAG_AUTOPROBE_CONSUMER | + DL_FLAG_PM_RUNTIME; + } + return 0; +} +early_param("fw_devlink", fw_devlink_setup); + +u32 fw_devlink_get_flags(void) +{ + return fw_devlink_flags; +} + +static bool fw_devlink_is_permissive(void) +{ + return fw_devlink_flags == DL_FLAG_SYNC_STATE_ONLY; +} + /* Device links support end. */ int (*platform_notify)(struct device *dev) = NULL; @@ -2345,36 +2375,6 @@ static int device_private_init(struct device *dev) return 0; } -static u32 fw_devlink_flags = DL_FLAG_SYNC_STATE_ONLY; -static int __init fw_devlink_setup(char *arg) -{ - if (!arg) - return -EINVAL; - - if (strcmp(arg, "off") == 0) { - fw_devlink_flags = 0; - } else if (strcmp(arg, "permissive") == 0) { - fw_devlink_flags = DL_FLAG_SYNC_STATE_ONLY; - } else if (strcmp(arg, "on") == 0) { - fw_devlink_flags = DL_FLAG_AUTOPROBE_CONSUMER; - } else if (strcmp(arg, "rpm") == 0) { - fw_devlink_flags = DL_FLAG_AUTOPROBE_CONSUMER | - DL_FLAG_PM_RUNTIME; - } - return 0; -} -early_param("fw_devlink", fw_devlink_setup); - -u32 fw_devlink_get_flags(void) -{ - return fw_devlink_flags; -} - -static bool fw_devlink_is_permissive(void) -{ - return fw_devlink_flags == DL_FLAG_SYNC_STATE_ONLY; -} - /** * device_add - add device to device hierarchy. * @dev: device. From 5f5377eaddfc24e5d7562e588d0ff84f9264d7c1 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Thu, 14 May 2020 22:34:58 -0700 Subject: [PATCH 13/23] driver core: Look for waiting consumers only for a fwnode's primary device Commit 4dbe191c046e ("driver core: Add device links from fwnode only for the primary device") skipped linking a fwnode's secondary device to the suppliers listed in its fwnode. However, a fwnode's secondary device can't be found using get_dev_from_fwnode(). So, there's no point in trying to see if devices waiting for suppliers might want to link to a fwnode's secondary device. This commit removes that unnecessary step for devices that aren't a fwnode's primary device and also moves the code to a more appropriate part of the file. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20200515053500.215929-3-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 2b454aae64b5..f585d92e09d0 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1173,6 +1173,21 @@ static bool fw_devlink_is_permissive(void) return fw_devlink_flags == DL_FLAG_SYNC_STATE_ONLY; } +static void fw_devlink_link_device(struct device *dev) +{ + int fw_ret; + + device_link_add_missing_supplier_links(); + + 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; @@ -2407,7 +2422,7 @@ int device_add(struct device *dev) struct device *parent; struct kobject *kobj; struct class_interface *class_intf; - int error = -EINVAL, fw_ret; + int error = -EINVAL; struct kobject *glue_dir = NULL; bool is_fwnode_dev = false; @@ -2524,16 +2539,8 @@ int device_add(struct device *dev) * waiting consumers can link to it before the driver is bound to the * device and the driver sync_state callback is called for this device. */ - device_link_add_missing_supplier_links(); - - if (fw_devlink_flags && is_fwnode_dev && - 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); - } + if (is_fwnode_dev) + fw_devlink_link_device(dev); bus_probe_device(dev); if (parent) From 716a7a25969003d82ab738179c3f1068a120ed11 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Thu, 14 May 2020 22:34:59 -0700 Subject: [PATCH 14/23] driver core: fw_devlink: Add support for batching fwnode parsing The amount of time spent parsing fwnodes of devices can become really high if the devices are added in an non-ideal order. Worst case can be O(N^2) when N devices are added. But this can be optimized to O(N) by adding all the devices and then parsing all their fwnodes in one batch. This commit adds fw_devlink_pause() and fw_devlink_resume() to allow doing this. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20200515053500.215929-4-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 1 + drivers/base/core.c | 116 ++++++++++++++++++++++++++++++++++++++--- drivers/base/dd.c | 8 +++ include/linux/fwnode.h | 2 + 4 files changed, 120 insertions(+), 7 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index 40fb069a8a7e..95c22c0f9036 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -153,6 +153,7 @@ extern char *make_class_name(const char *name, struct kobject *kobj); extern int devres_release_all(struct device *dev); extern void device_block_probing(void); extern void device_unblock_probing(void); +extern void driver_deferred_probe_force_trigger(void); /* /sys/devices directory */ extern struct kset *devices_kset; diff --git a/drivers/base/core.c b/drivers/base/core.c index f585d92e09d0..84c569726d75 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -49,6 +49,9 @@ static LIST_HEAD(wait_for_suppliers); static DEFINE_MUTEX(wfs_lock); static LIST_HEAD(deferred_sync); static unsigned int defer_sync_state_count = 1; +static unsigned int defer_fw_devlink_count; +static DEFINE_MUTEX(defer_fw_devlink_lock); +static bool fw_devlink_is_permissive(void); #ifdef CONFIG_SRCU static DEFINE_MUTEX(device_links_lock); @@ -527,7 +530,7 @@ static void device_link_add_missing_supplier_links(void) int ret = fwnode_call_int_op(dev->fwnode, add_links, dev); if (!ret) list_del_init(&dev->links.needs_suppliers); - else if (ret != -ENODEV) + else if (ret != -ENODEV || fw_devlink_is_permissive()) dev->links.need_for_probe = false; } mutex_unlock(&wfs_lock); @@ -1177,17 +1180,116 @@ static void fw_devlink_link_device(struct device *dev) { int fw_ret; - device_link_add_missing_supplier_links(); + if (!fw_devlink_flags) + return; - if (fw_devlink_flags && fwnode_has_op(dev->fwnode, add_links)) { + 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()) - device_link_wait_for_mandatory_supplier(dev); - else if (fw_ret) - device_link_wait_for_optional_supplier(dev); + if (fw_ret == -ENODEV && fw_devlink_is_permissive()) + fw_ret = -EAGAIN; + } else { + fw_ret = -ENODEV; } + + if (fw_ret == -ENODEV) + device_link_wait_for_mandatory_supplier(dev); + else if (fw_ret) + device_link_wait_for_optional_supplier(dev); + +out: + mutex_unlock(&defer_fw_devlink_lock); } +/** + * fw_devlink_pause - Pause parsing of fwnode to create device links + * + * Calling this function defers any fwnode parsing to create device links until + * fw_devlink_resume() is called. Both these functions are ref counted and the + * caller needs to match the calls. + * + * While fw_devlink is paused: + * - Any device that is added won't have its fwnode parsed to create device + * links. + * - The probe of the device will also be deferred during this period. + * - Any devices that were already added, but waiting for suppliers won't be + * able to link to newly added devices. + * + * Once fw_devlink_resume(): + * - All the fwnodes that was not parsed will be parsed. + * - All the devices that were deferred probing will be reattempted if they + * aren't waiting for any more suppliers. + * + * This pair of functions, is mainly meant to optimize the parsing of fwnodes + * when a lot of devices that need to link to each other are added in a short + * interval of time. For example, adding all the top level devices in a system. + * + * For example, if N devices are added and: + * - All the consumers are added before their suppliers + * - All the suppliers of the N devices are part of the N devices + * + * Then: + * + * - With the use of fw_devlink_pause() and fw_devlink_resume(), each device + * will only need one parsing of its fwnode because it is guaranteed to find + * all the supplier devices already registered and ready to link to. It won't + * have to do another pass later to find one or more suppliers it couldn't + * find in the first parse of the fwnode. So, we'll only need O(N) fwnode + * parses. + * + * - Without the use of fw_devlink_pause() and fw_devlink_resume(), we would + * end up doing O(N^2) parses of fwnodes because every device that's added is + * guaranteed to trigger a parse of the fwnode of every device added before + * it. This O(N^2) parse is made worse by the fact that when a fwnode of a + * device is parsed, all it descendant devices might need to have their + * fwnodes parsed too (even if the devices themselves aren't added). + */ +void fw_devlink_pause(void) +{ + mutex_lock(&defer_fw_devlink_lock); + defer_fw_devlink_count++; + mutex_unlock(&defer_fw_devlink_lock); +} + +/** fw_devlink_resume - Resume parsing of fwnode to create device links + * + * This function is used in conjunction with fw_devlink_pause() and is ref + * counted. See documentation for fw_devlink_pause() for more details. + */ +void fw_devlink_resume(void) +{ + mutex_lock(&defer_fw_devlink_lock); + if (!defer_fw_devlink_count) { + WARN(true, "Unmatched fw_devlink pause/resume!"); + goto out; + } + + defer_fw_devlink_count--; + if (defer_fw_devlink_count) + goto out; + + device_link_add_missing_supplier_links(); + driver_deferred_probe_force_trigger(); +out: + mutex_unlock(&defer_fw_devlink_lock); +} /* Device links support end. */ int (*platform_notify)(struct device *dev) = NULL; diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 48ca81cb8ebc..63991d97adcc 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -160,6 +160,14 @@ static bool driver_deferred_probe_enable = false; * again. */ static void driver_deferred_probe_trigger(void) +{ + if (!driver_deferred_probe_enable) + return; + + driver_deferred_probe_force_trigger(); +} + +void driver_deferred_probe_force_trigger(void) { if (!driver_deferred_probe_enable) return; diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h index e0abafbb17f8..9506f8ec0974 100644 --- a/include/linux/fwnode.h +++ b/include/linux/fwnode.h @@ -171,5 +171,7 @@ struct fwnode_operations { #define get_dev_from_fwnode(fwnode) get_device((fwnode)->dev) extern u32 fw_devlink_get_flags(void); +void fw_devlink_pause(void); +void fw_devlink_resume(void); #endif From 93d2e4322aa74c1ad1e8c2160608eb9a960d69ff Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Thu, 14 May 2020 22:35:00 -0700 Subject: [PATCH 15/23] of: platform: Batch fwnode parsing when adding all top level devices The fw_devlink_pause() and fw_devlink_resume() APIs allow batching the parsing of the device tree nodes when a lot of devices are added. This will significantly cut down parsing time (as much a 1 second on some systems). So, use them when adding devices for all the top level device tree nodes in a system. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20200515053500.215929-5-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 3371e4a06248..55d719347810 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -538,7 +538,9 @@ 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 fefcfc968723caf93318613a08e1f3ad07a6154f Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Sun, 17 May 2020 10:34:53 -0700 Subject: [PATCH 16/23] driver core: Remove check in driver_deferred_probe_force_trigger() The whole point behind adding driver_deferred_probe_force_trigger() in commit 716a7a259690 ("driver core: fw_devlink: Add support for batching fwnode parsing") was to skip the check for driver_deferred_probe_enable. Otherwise, it's identical to driver_deferred_probe_trigger(). Delete the check in driver_deferred_probe_force_trigger() so that fw_devlink_pause() and fw_devlink_resume() can kick off deferred probe as intended. Without doing this forced deferred probe trigger, some platforms seem to be crashing during boot because they assume probe order of devices. Fixes: 716a7a259690 ("driver core: fw_devlink: Add support for batching fwnode parsing") Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20200517173453.157703-1-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 63991d97adcc..9a1d940342ac 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -169,9 +169,6 @@ 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 c82c83c330654c5639960ebc3dabbae53c43f79e Mon Sep 17 00:00:00 2001 From: Tang Bin Date: Wed, 20 May 2020 22:12:02 +0800 Subject: [PATCH 17/23] driver core: platform: Fix spelling errors in platform.c There is a word spelling mistake of 'Unegisters', thus it should be fixed. Signed-off-by: Tang Bin Link: https://lore.kernel.org/r/20200520141202.19568-1-tangbin@cmss.chinamobile.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 4ed3dc38f45c..fe08ef8ffd5c 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -977,7 +977,7 @@ EXPORT_SYMBOL_GPL(__platform_register_drivers); * @drivers: an array of drivers to unregister * @count: the number of drivers to unregister * - * Unegisters platform drivers specified by an array. This is typically used + * Unregisters platform drivers specified by an array. This is typically used * to complement an earlier call to platform_register_drivers(). Drivers are * unregistered in the reverse order in which they were registered. */ From 114dbb4fa7c4053a51964d112e2851e818e085c6 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 20 May 2020 13:29:59 +0300 Subject: [PATCH 18/23] drivers property: When no children in primary, try secondary Software firmware nodes can provide a child node to its parent. Since software node can be secondary, we need a mechanism to access the children. The idea is to list children of the primary node first and when they are finished, continue with secondary node if available. Signed-off-by: Andy Shevchenko Reviewed-by: Rafael J. Wysocki Acked-by: Sakari Ailus Reviewed-by: Heikki Krogerus Link: https://lore.kernel.org/r/20200520102959.34812-1-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/property.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/base/property.c b/drivers/base/property.c index 5f35c0ccf5e0..1e6d75e65938 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -708,14 +708,23 @@ struct fwnode_handle *device_get_next_child_node(struct device *dev, struct fwnode_handle *child) { struct acpi_device *adev = ACPI_COMPANION(dev); - struct fwnode_handle *fwnode = NULL; + struct fwnode_handle *fwnode = NULL, *next; if (dev->of_node) fwnode = &dev->of_node->fwnode; else if (adev) fwnode = acpi_fwnode_handle(adev); - return fwnode_get_next_child_node(fwnode, child); + /* Try to find a child in primary fwnode */ + next = fwnode_get_next_child_node(fwnode, child); + if (next) + return next; + + /* When no more children in primary, continue with secondary */ + if (!IS_ERR_OR_NULL(fwnode->secondary)) + next = fwnode_get_next_child_node(fwnode->secondary, child); + + return next; } EXPORT_SYMBOL_GPL(device_get_next_child_node); From 2cd38fd15e4ebcfe917a443734820269f8b5ba2b Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Tue, 19 May 2020 20:48:21 -0700 Subject: [PATCH 19/23] driver core: Remove unnecessary is_fwnode_dev variable in device_add() That variable is no longer necessary. Remove it and also fix a minor typo in comments. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20200520034824.79049-2-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 84c569726d75..55310c97a8b7 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2526,7 +2526,6 @@ int device_add(struct device *dev) struct class_interface *class_intf; int error = -EINVAL; struct kobject *glue_dir = NULL; - bool is_fwnode_dev = false; dev = get_device(dev); if (!dev) @@ -2624,11 +2623,6 @@ int device_add(struct device *dev) kobject_uevent(&dev->kobj, KOBJ_ADD); - if (dev->fwnode && !dev->fwnode->dev) { - dev->fwnode->dev = dev; - is_fwnode_dev = true; - } - /* * Check if any of the other devices (consumers) have been waiting for * this device (supplier) to be added so that they can create a device @@ -2637,12 +2631,14 @@ int device_add(struct device *dev) * This needs to happen after device_pm_add() because device_link_add() * requires the supplier be registered before it's called. * - * But this also needs to happe before bus_probe_device() to make sure + * But this also needs to happen before bus_probe_device() to make sure * waiting consumers can link to it before the driver is bound to the * device and the driver sync_state callback is called for this device. */ - if (is_fwnode_dev) + if (dev->fwnode && !dev->fwnode->dev) { + dev->fwnode->dev = dev; fw_devlink_link_device(dev); + } bus_probe_device(dev); if (parent) From 0e5596c54aa2fbfd22842096d3ebb6753b2a220a Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Sun, 24 May 2020 17:30:41 +0200 Subject: [PATCH 20/23] kobject: send KOBJ_REMOVE uevent when the object is removed from sysfs It is possible for a KOBJ_REMOVE uevent to be sent to userspace way after the files are actually gone from sysfs, due to how reference counting for kobjects work. This should not be a problem, but it would be good to properly send the information when things are going away, not at some later point in time in the future. Before this move, if a kobject's parent was torn down before the child, when the call to kobject_uevent() happened, the parent walk to try to reconstruct the full path of the kobject could be a total mess and cause crashes. It's not good to try to tear down a kobject tree from top down, but let's at least try to not to crash if a user does so. Reviewed-by: Rafael J. Wysocki Link: https://lore.kernel.org/r/20200524153041.2361-2-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- lib/kobject.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/kobject.c b/lib/kobject.c index 83198cb37d8d..2509e22ad74a 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -620,6 +620,13 @@ void kobject_del(struct kobject *kobj) if (ktype) sysfs_remove_groups(kobj, ktype->default_groups); + /* send "remove" if the caller did not do it but sent "add" */ + if (kobj->state_add_uevent_sent && !kobj->state_remove_uevent_sent) { + pr_debug("kobject: '%s' (%p): auto cleanup 'remove' event\n", + kobject_name(kobj), kobj); + kobject_uevent(kobj, KOBJ_REMOVE); + } + sysfs_remove_dir(kobj); sysfs_put(sd); @@ -673,13 +680,6 @@ static void kobject_cleanup(struct kobject *kobj) pr_debug("kobject: '%s' (%p): does not have a release() function, it is broken and must be fixed. See Documentation/kobject.txt.\n", kobject_name(kobj), kobj); - /* send "remove" if the caller did not do it but sent "add" */ - if (kobj->state_add_uevent_sent && !kobj->state_remove_uevent_sent) { - pr_debug("kobject: '%s' (%p): auto cleanup 'remove' event\n", - kobject_name(kobj), kobj); - kobject_uevent(kobj, KOBJ_REMOVE); - } - /* remove from sysfs if the caller did not do it */ if (kobj->state_in_sysfs) { pr_debug("kobject: '%s' (%p): auto cleanup kobject_del\n", From 46d26819a5056f4831649c5887ad5c71a16d86f7 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Sun, 24 May 2020 17:30:40 +0200 Subject: [PATCH 21/23] software node: implement software_node_unregister() Sometimes it is better to unregister individual nodes instead of trying to do them all at once with software_node_unregister_nodes(), so create software_node_unregister() so that you can unregister them one at a time. This is especially important when creating nodes in a hierarchy, with parent -> children representations. Children always need to be removed before a parent is, as the swnode logic assumes this is going to be the case. Fix up the lib/test_printf.c fwnode_pointer() test which to use this new function as it had the problem of tearing things down in the backwards order. Fixes: f1ce39df508d ("lib/test_printf: Add tests for %pfw printk modifier") Cc: stable Cc: Andy Shevchenko Cc: Brendan Higgins Cc: Dmitry Torokhov Cc: Petr Mladek Cc: Rafael J. Wysocki Cc: Rasmus Villemoes Cc: Sakari Ailus Cc: Sergey Senozhatsky Cc: Steven Rostedt Reported-by: Naresh Kamboju Reported-by: kernel test robot Reported-by: Randy Dunlap Tested-by: Petr Mladek Tested-by: Randy Dunlap Tested-by: Guenter Roeck Reviewed-by: Heikki Krogerus Acked-by: Randy Dunlap Link: https://lore.kernel.org/r/20200524153041.2361-1-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- drivers/base/swnode.c | 27 +++++++++++++++++++++------ include/linux/property.h | 1 + lib/test_printf.c | 4 +++- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c index de8d3543e8fe..770b1f47a625 100644 --- a/drivers/base/swnode.c +++ b/drivers/base/swnode.c @@ -712,17 +712,18 @@ EXPORT_SYMBOL_GPL(software_node_register_nodes); * @nodes: Zero terminated array of software nodes to be unregistered * * Unregister multiple software nodes at once. + * + * NOTE: Be careful using this call if the nodes had parent pointers set up in + * them before registering. If so, it is wiser to remove the nodes + * individually, in the correct order (child before parent) instead of relying + * on the sequential order of the list of nodes in the array. */ void software_node_unregister_nodes(const struct software_node *nodes) { - struct swnode *swnode; int i; - for (i = 0; nodes[i].name; i++) { - swnode = software_node_to_swnode(&nodes[i]); - if (swnode) - fwnode_remove_software_node(&swnode->fwnode); - } + for (i = 0; nodes[i].name; i++) + software_node_unregister(&nodes[i]); } EXPORT_SYMBOL_GPL(software_node_unregister_nodes); @@ -741,6 +742,20 @@ int software_node_register(const struct software_node *node) } EXPORT_SYMBOL_GPL(software_node_register); +/** + * software_node_unregister - Unregister static software node + * @node: The software node to be unregistered + */ +void software_node_unregister(const struct software_node *node) +{ + struct swnode *swnode; + + swnode = software_node_to_swnode(node); + if (swnode) + fwnode_remove_software_node(&swnode->fwnode); +} +EXPORT_SYMBOL_GPL(software_node_unregister); + struct fwnode_handle * fwnode_create_software_node(const struct property_entry *properties, const struct fwnode_handle *parent) diff --git a/include/linux/property.h b/include/linux/property.h index d86de017c689..0d4099b4ce1f 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -441,6 +441,7 @@ int software_node_register_nodes(const struct software_node *nodes); void software_node_unregister_nodes(const struct software_node *nodes); int software_node_register(const struct software_node *node); +void software_node_unregister(const struct software_node *node); int software_node_notify(struct device *dev, unsigned long action); diff --git a/lib/test_printf.c b/lib/test_printf.c index 6b1622f4d7c2..fc63b8959d42 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -637,7 +637,9 @@ static void __init fwnode_pointer(void) test(second_name, "%pfwP", software_node_fwnode(&softnodes[1])); test(third_name, "%pfwP", software_node_fwnode(&softnodes[2])); - software_node_unregister_nodes(softnodes); + software_node_unregister(&softnodes[2]); + software_node_unregister(&softnodes[1]); + software_node_unregister(&softnodes[0]); } static void __init From c2c076166b5880eabe068ce1cab30bf6edeeea1a Mon Sep 17 00:00:00 2001 From: Scott Branden Date: Fri, 22 May 2020 16:12:02 -0700 Subject: [PATCH 22/23] firmware_loader: change enum fw_opt to u32 "enum fw_opt" is not used as an enum. Change fw_opt to u32 as FW_OPT_* values are OR'd together. Signed-off-by: Scott Branden Acked-by: Luis Chamberlain Link: https://lore.kernel.org/r/20200522231202.13681-1-scott.branden@broadcom.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/fallback.c | 12 ++++++------ drivers/base/firmware_loader/fallback.h | 8 ++++---- drivers/base/firmware_loader/fallback_platform.c | 2 +- drivers/base/firmware_loader/firmware.h | 3 +-- drivers/base/firmware_loader/main.c | 11 +++++------ 5 files changed, 17 insertions(+), 19 deletions(-) diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c index d9ac7296205e..5327bfc6ba71 100644 --- a/drivers/base/firmware_loader/fallback.c +++ b/drivers/base/firmware_loader/fallback.c @@ -463,7 +463,7 @@ static const struct attribute_group *fw_dev_attr_groups[] = { static struct fw_sysfs * fw_create_instance(struct firmware *firmware, const char *fw_name, - struct device *device, enum fw_opt opt_flags) + struct device *device, u32 opt_flags) { struct fw_sysfs *fw_sysfs; struct device *f_dev; @@ -496,7 +496,7 @@ exit: * In charge of constructing a sysfs fallback interface for firmware loading. **/ static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs, - enum fw_opt opt_flags, long timeout) + u32 opt_flags, long timeout) { int retval = 0; struct device *f_dev = &fw_sysfs->dev; @@ -550,7 +550,7 @@ err_put_dev: static int fw_load_from_user_helper(struct firmware *firmware, const char *name, struct device *device, - enum fw_opt opt_flags) + u32 opt_flags) { struct fw_sysfs *fw_sysfs; long timeout; @@ -591,7 +591,7 @@ out_unlock: return ret; } -static bool fw_force_sysfs_fallback(enum fw_opt opt_flags) +static bool fw_force_sysfs_fallback(u32 opt_flags) { if (fw_fallback_config.force_sysfs_fallback) return true; @@ -600,7 +600,7 @@ static bool fw_force_sysfs_fallback(enum fw_opt opt_flags) return true; } -static bool fw_run_sysfs_fallback(enum fw_opt opt_flags) +static bool fw_run_sysfs_fallback(u32 opt_flags) { int ret; @@ -643,7 +643,7 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags) **/ int firmware_fallback_sysfs(struct firmware *fw, const char *name, struct device *device, - enum fw_opt opt_flags, + u32 opt_flags, int ret) { if (!fw_run_sysfs_fallback(opt_flags)) diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h index 06f4577733a8..2afdb6adb23f 100644 --- a/drivers/base/firmware_loader/fallback.h +++ b/drivers/base/firmware_loader/fallback.h @@ -33,7 +33,7 @@ struct firmware_fallback_config { #ifdef CONFIG_FW_LOADER_USER_HELPER int firmware_fallback_sysfs(struct firmware *fw, const char *name, struct device *device, - enum fw_opt opt_flags, + u32 opt_flags, int ret); void kill_pending_fw_fallback_reqs(bool only_kill_custom); @@ -45,7 +45,7 @@ void unregister_sysfs_loader(void); #else /* CONFIG_FW_LOADER_USER_HELPER */ static inline int firmware_fallback_sysfs(struct firmware *fw, const char *name, struct device *device, - enum fw_opt opt_flags, + u32 opt_flags, int ret) { /* Keep carrying over the same error */ @@ -67,10 +67,10 @@ static inline void unregister_sysfs_loader(void) #endif /* CONFIG_FW_LOADER_USER_HELPER */ #ifdef CONFIG_EFI_EMBEDDED_FIRMWARE -int firmware_fallback_platform(struct fw_priv *fw_priv, enum fw_opt opt_flags); +int firmware_fallback_platform(struct fw_priv *fw_priv, u32 opt_flags); #else static inline int firmware_fallback_platform(struct fw_priv *fw_priv, - enum fw_opt opt_flags) + u32 opt_flags) { return -ENOENT; } diff --git a/drivers/base/firmware_loader/fallback_platform.c b/drivers/base/firmware_loader/fallback_platform.c index c88c745590fe..cdd2c9a9f38a 100644 --- a/drivers/base/firmware_loader/fallback_platform.c +++ b/drivers/base/firmware_loader/fallback_platform.c @@ -8,7 +8,7 @@ #include "fallback.h" #include "firmware.h" -int firmware_fallback_platform(struct fw_priv *fw_priv, enum fw_opt opt_flags) +int firmware_fallback_platform(struct fw_priv *fw_priv, u32 opt_flags) { const u8 *data; size_t size; diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h index 25836a6afc9f..933e2192fbe8 100644 --- a/drivers/base/firmware_loader/firmware.h +++ b/drivers/base/firmware_loader/firmware.h @@ -136,8 +136,7 @@ static inline void fw_state_done(struct fw_priv *fw_priv) __fw_state_set(fw_priv, FW_STATUS_DONE); } -int assign_fw(struct firmware *fw, struct device *device, - enum fw_opt opt_flags); +int assign_fw(struct firmware *fw, struct device *device, u32 opt_flags); #ifdef CONFIG_FW_LOADER_PAGED_BUF void fw_free_paged_buf(struct fw_priv *fw_priv); diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 5296aaca35cf..ca871b13524e 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -210,7 +210,7 @@ static struct fw_priv *__lookup_fw_priv(const char *fw_name) static int alloc_lookup_fw_priv(const char *fw_name, struct firmware_cache *fwc, struct fw_priv **fw_priv, void *dbuf, - size_t size, enum fw_opt opt_flags) + size_t size, u32 opt_flags) { struct fw_priv *tmp; @@ -632,8 +632,7 @@ static int fw_add_devm_name(struct device *dev, const char *name) } #endif -int assign_fw(struct firmware *fw, struct device *device, - enum fw_opt opt_flags) +int assign_fw(struct firmware *fw, struct device *device, u32 opt_flags) { struct fw_priv *fw_priv = fw->priv; int ret; @@ -684,7 +683,7 @@ int assign_fw(struct firmware *fw, struct device *device, static int _request_firmware_prepare(struct firmware **firmware_p, const char *name, struct device *device, void *dbuf, size_t size, - enum fw_opt opt_flags) + u32 opt_flags) { struct firmware *firmware; struct fw_priv *fw_priv; @@ -750,7 +749,7 @@ static void fw_abort_batch_reqs(struct firmware *fw) static int _request_firmware(const struct firmware **firmware_p, const char *name, struct device *device, void *buf, size_t size, - enum fw_opt opt_flags) + u32 opt_flags) { struct firmware *fw = NULL; int ret; @@ -987,7 +986,7 @@ struct firmware_work { struct device *device; void *context; void (*cont)(const struct firmware *fw, void *context); - enum fw_opt opt_flags; + u32 opt_flags; }; static void request_firmware_work_func(struct work_struct *work) From 8c3e315d4296421cd26b3300ee0ac117f0877f20 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Tue, 26 May 2020 15:09:27 -0700 Subject: [PATCH 23/23] driver core: Update device link status correctly for SYNC_STATE_ONLY links When SYNC_STATE_ONLY support was added in commit 05ef983e0d65 ("driver core: Add device link support for SYNC_STATE_ONLY flag"), SYNC_STATE_ONLY links were treated similar to STATELESS links in terms of not blocking consumer probe if the supplier hasn't probed yet. That caused a SYNC_STATE_ONLY device link's status to not get updated. Since SYNC_STATE_ONLY device link is no longer useful once the consumer probes, commit 21c27f06587d ("driver core: Fix SYNC_STATE_ONLY device link implementation") addresses the status update issue by deleting the SYNC_STATE_ONLY device link instead of complicating the status update code. However, there are still some cases where we need to update the status of a SYNC_STATE_ONLY device link. This is because a SYNC_STATE_ONLY device link can later get converted into a normal MANAGED device link when a normal MANAGED device link is created between a supplier and consumer that already have a SYNC_STATE_ONLY device link between them. If a SYNC_STATE_ONLY device link's status isn't maintained correctly till it's converted to a normal MANAGED device link, then the normal MANAGED device link will end up with a wrong link status. This can cause a warning stack trace[1] when the consumer device probes successfully. This commit fixes the SYNC_STATE_ONLY device link status update issue where it wouldn't transition correctly from DL_STATE_DORMANT or DL_STATE_AVAILABLE to DL_STATE_CONSUMER_PROBE. It also resets the status back to DL_STATE_DORMANT or DL_STATE_AVAILABLE if the consumer probe fails. [1] - https://lore.kernel.org/lkml/20200522204120.3b3c9ed6@apollo/ Fixes: 05ef983e0d65 ("driver core: Add device link support for SYNC_STATE_ONLY flag") Fixes: 21c27f06587d ("driver core: Fix SYNC_STATE_ONLY device link implementation") Reported-by: Michael Walle Tested-by: Michael Walle Signed-off-by: Saravana Kannan Reviewed-by: Rafael J. Wysocki Link: https://lore.kernel.org/r/20200526220928.49939-1-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 791b7530599f..9a76dd44cb37 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -646,9 +646,17 @@ static void device_links_missing_supplier(struct device *dev) { struct device_link *link; - list_for_each_entry(link, &dev->links.suppliers, c_node) - if (link->status == DL_STATE_CONSUMER_PROBE) + list_for_each_entry(link, &dev->links.suppliers, c_node) { + if (link->status != DL_STATE_CONSUMER_PROBE) + continue; + + if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) { WRITE_ONCE(link->status, DL_STATE_AVAILABLE); + } else { + WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY)); + WRITE_ONCE(link->status, DL_STATE_DORMANT); + } + } } /** @@ -687,11 +695,11 @@ int device_links_check_suppliers(struct device *dev) device_links_write_lock(); list_for_each_entry(link, &dev->links.suppliers, c_node) { - if (!(link->flags & DL_FLAG_MANAGED) || - link->flags & DL_FLAG_SYNC_STATE_ONLY) + if (!(link->flags & DL_FLAG_MANAGED)) continue; - if (link->status != DL_STATE_AVAILABLE) { + if (link->status != DL_STATE_AVAILABLE && + !(link->flags & DL_FLAG_SYNC_STATE_ONLY)) { device_links_missing_supplier(dev); ret = -EPROBE_DEFER; break; @@ -952,11 +960,21 @@ static void __device_links_no_driver(struct device *dev) if (!(link->flags & DL_FLAG_MANAGED)) continue; - if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) + if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) { device_link_drop_managed(link); - else if (link->status == DL_STATE_CONSUMER_PROBE || - link->status == DL_STATE_ACTIVE) + continue; + } + + if (link->status != DL_STATE_CONSUMER_PROBE && + link->status != DL_STATE_ACTIVE) + continue; + + if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) { WRITE_ONCE(link->status, DL_STATE_AVAILABLE); + } else { + WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY)); + WRITE_ONCE(link->status, DL_STATE_DORMANT); + } } dev->links.status = DL_DEV_NO_DRIVER;