From 2e7189b6c774adc6b848808286b14d0825dc9d1a Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Thu, 12 Jul 2018 09:33:17 +0200 Subject: driver core: bus: convert to use BUS_ATTR_WO and RW We are trying to get rid of BUS_ATTR() and the usage of that in bus.c can be trivially converted to use BUS_ATTR_WO and RW, so use those macros instead. Cc: "Rafael J. Wysocki" Signed-off-by: Greg Kroah-Hartman --- drivers/base/bus.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/bus.c b/drivers/base/bus.c index e06a57936cc9..a56e865cdbfa 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -236,12 +236,12 @@ static ssize_t bind_store(struct device_driver *drv, const char *buf, } static DRIVER_ATTR_IGNORE_LOCKDEP(bind, S_IWUSR, NULL, bind_store); -static ssize_t show_drivers_autoprobe(struct bus_type *bus, char *buf) +static ssize_t drivers_autoprobe_show(struct bus_type *bus, char *buf) { return sprintf(buf, "%d\n", bus->p->drivers_autoprobe); } -static ssize_t store_drivers_autoprobe(struct bus_type *bus, +static ssize_t drivers_autoprobe_store(struct bus_type *bus, const char *buf, size_t count) { if (buf[0] == '0') @@ -251,7 +251,7 @@ static ssize_t store_drivers_autoprobe(struct bus_type *bus, return count; } -static ssize_t store_drivers_probe(struct bus_type *bus, +static ssize_t drivers_probe_store(struct bus_type *bus, const char *buf, size_t count) { struct device *dev; @@ -586,9 +586,8 @@ static void remove_bind_files(struct device_driver *drv) driver_remove_file(drv, &driver_attr_unbind); } -static BUS_ATTR(drivers_probe, S_IWUSR, NULL, store_drivers_probe); -static BUS_ATTR(drivers_autoprobe, S_IWUSR | S_IRUGO, - show_drivers_autoprobe, store_drivers_autoprobe); +static BUS_ATTR_WO(drivers_probe); +static BUS_ATTR_RW(drivers_autoprobe); static int add_probe_files(struct bus_type *bus) { -- cgit v1.2.3 From a4723041857eaa35f189d237da769c4c63235544 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 29 Oct 2018 16:31:26 +0100 Subject: driver core: drop use of BUS_ATTR() We are trying to get rid of BUS_ATTR() so drop the last user of it from the tree. We had to "open code" it in order to prevent a function name conflict due to the use of DEVICE_ATTR_WO() earlier in the file :( Cc: "Rafael J. Wysocki" Signed-off-by: Greg Kroah-Hartman --- drivers/base/bus.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/bus.c b/drivers/base/bus.c index a56e865cdbfa..3ab45843da39 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -837,7 +837,14 @@ static ssize_t bus_uevent_store(struct bus_type *bus, rc = kobject_synth_uevent(&bus->p->subsys.kobj, buf, count); return rc ? rc : count; } -static BUS_ATTR(uevent, S_IWUSR, NULL, bus_uevent_store); +/* + * "open code" the old BUS_ATTR() macro here. We want to use BUS_ATTR_WO() + * here, but can not use it as earlier in the file we have + * DEVICE_ATTR_WO(uevent), which would cause a clash with the with the store + * function name. + */ +static struct bus_attribute bus_attr_uevent = __ATTR(uevent, S_IWUSR, NULL, + bus_uevent_store); /** * bus_register - register a driver-core subsystem -- cgit v1.2.3 From 570d0200123fb4f809aa2f6226e93a458d664d70 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Fri, 18 Jan 2019 10:34:59 +0800 Subject: driver core: move device->knode_class to device_private As the description of struct device_private says, it stores data which is private to driver core. And it already has similar fields like: knode_parent, knode_driver, knode_driver and knode_bus. This look it is more proper to put knode_class together with those fields to make it private to driver core. This patch move device->knode_class to device_private to make it comply with code convention. Signed-off-by: Wei Yang Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 4 ++++ drivers/base/class.c | 14 ++++++++++---- drivers/base/core.c | 4 ++-- 3 files changed, 16 insertions(+), 6 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/base.h b/drivers/base/base.h index 7a419a7a6235..37329a668935 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -60,6 +60,7 @@ struct driver_private { * @knode_parent - node in sibling list * @knode_driver - node in driver list * @knode_bus - node in bus list + * @knode_class - node in class list * @deferred_probe - entry in deferred_probe_list which is used to retry the * binding of drivers which were unable to get all the resources needed by * the device; typically because it depends on another driver getting @@ -74,6 +75,7 @@ struct device_private { struct klist_node knode_parent; struct klist_node knode_driver; struct klist_node knode_bus; + struct klist_node knode_class; struct list_head deferred_probe; struct device *device; }; @@ -83,6 +85,8 @@ struct device_private { container_of(obj, struct device_private, knode_driver) #define to_device_private_bus(obj) \ container_of(obj, struct device_private, knode_bus) +#define to_device_private_class(obj) \ + container_of(obj, struct device_private, knode_class) /* initialisation functions */ extern int devices_init(void); diff --git a/drivers/base/class.c b/drivers/base/class.c index 54def4e02f00..d8a6a5864c2e 100644 --- a/drivers/base/class.c +++ b/drivers/base/class.c @@ -117,16 +117,22 @@ static void class_put(struct class *cls) kset_put(&cls->p->subsys); } +static struct device *klist_class_to_dev(struct klist_node *n) +{ + struct device_private *p = to_device_private_class(n); + return p->device; +} + static void klist_class_dev_get(struct klist_node *n) { - struct device *dev = container_of(n, struct device, knode_class); + struct device *dev = klist_class_to_dev(n); get_device(dev); } static void klist_class_dev_put(struct klist_node *n) { - struct device *dev = container_of(n, struct device, knode_class); + struct device *dev = klist_class_to_dev(n); put_device(dev); } @@ -277,7 +283,7 @@ void class_dev_iter_init(struct class_dev_iter *iter, struct class *class, struct klist_node *start_knode = NULL; if (start) - start_knode = &start->knode_class; + start_knode = &start->p->knode_class; klist_iter_init_node(&class->p->klist_devices, &iter->ki, start_knode); iter->type = type; } @@ -304,7 +310,7 @@ struct device *class_dev_iter_next(struct class_dev_iter *iter) knode = klist_next(&iter->ki); if (!knode) return NULL; - dev = container_of(knode, struct device, knode_class); + dev = klist_class_to_dev(knode); if (!iter->type || iter->type == dev->type) return dev; } diff --git a/drivers/base/core.c b/drivers/base/core.c index 0073b09bb99f..4a4b6f8cbc4f 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1966,7 +1966,7 @@ int device_add(struct device *dev) if (dev->class) { mutex_lock(&dev->class->p->mutex); /* tie the class to the device */ - klist_add_tail(&dev->knode_class, + klist_add_tail(&dev->p->knode_class, &dev->class->p->klist_devices); /* notify any interfaces that the device is here */ @@ -2105,7 +2105,7 @@ void device_del(struct device *dev) if (class_intf->remove_dev) class_intf->remove_dev(dev, class_intf); /* remove the device from the class list */ - klist_del(&dev->knode_class); + klist_del(&dev->p->knode_class); mutex_unlock(&dev->class->p->mutex); } device_remove_file(dev, &dev_attr_uevent); -- cgit v1.2.3 From 91f382a46822c7a2913e6fabb2b882cc0f23144a Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 11 Jan 2019 18:51:32 +0900 Subject: firmware_loader: move CONFIG_FW_LOADER_USER_HELPER switch to Makefile The whole code of fallback_table.c is surrounded by #ifdef of CONFIG_FW_LOADER_USER_HELPER. Move the CONFIG_FW_LOADER_USER_HELPER switch to Makefile so that it is not compiled at all when this CONFIG option is disabled. I also removed the confusing comment, "Module or buit-in [sic]". CONFIG_FW_LOADER_USER_HELPER is a boolean option. (If it were a module, CONFIG_FW_LOADER_USER_HELPER_MODULE would be defined instead.) Signed-off-by: Masahiro Yamada Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/Makefile | 2 +- drivers/base/firmware_loader/fallback_table.c | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/firmware_loader/Makefile b/drivers/base/firmware_loader/Makefile index a97eeb0be1d8..5c7f422f7528 100644 --- a/drivers/base/firmware_loader/Makefile +++ b/drivers/base/firmware_loader/Makefile @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 # Makefile for the Linux firmware loader -obj-y := fallback_table.o +obj-$(CONFIG_FW_LOADER_USER_HELPER) += fallback_table.o obj-$(CONFIG_FW_LOADER) += firmware_class.o firmware_class-objs := main.o firmware_class-$(CONFIG_FW_LOADER_USER_HELPER) += fallback.o diff --git a/drivers/base/firmware_loader/fallback_table.c b/drivers/base/firmware_loader/fallback_table.c index 7428659d8df9..776dd69cf5be 100644 --- a/drivers/base/firmware_loader/fallback_table.c +++ b/drivers/base/firmware_loader/fallback_table.c @@ -16,9 +16,6 @@ * firmware fallback configuration table */ -/* Module or buit-in */ -#ifdef CONFIG_FW_LOADER_USER_HELPER - static unsigned int zero; static unsigned int one = 1; @@ -51,5 +48,3 @@ struct ctl_table firmware_config_table[] = { { } }; EXPORT_SYMBOL_GPL(firmware_config_table); - -#endif -- cgit v1.2.3 From f96182e959a41e35df0adae9ae09a49ff8a618a8 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 11 Jan 2019 18:52:00 +0900 Subject: firmware_loader: move firmware/ to drivers/base/firmware_loader/builtin/ Currently, the 'firmware' directory only contains a single Makefile to embed extra firmware into the kernel. Move it to the more relevant place. Signed-off-by: Masahiro Yamada Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/Makefile | 2 ++ drivers/base/firmware_loader/builtin/.gitignore | 1 + drivers/base/firmware_loader/builtin/Makefile | 40 +++++++++++++++++++++++++ 3 files changed, 43 insertions(+) create mode 100644 drivers/base/firmware_loader/builtin/.gitignore create mode 100644 drivers/base/firmware_loader/builtin/Makefile (limited to 'drivers/base') diff --git a/drivers/base/firmware_loader/Makefile b/drivers/base/firmware_loader/Makefile index 5c7f422f7528..0b2dfa6259c9 100644 --- a/drivers/base/firmware_loader/Makefile +++ b/drivers/base/firmware_loader/Makefile @@ -5,3 +5,5 @@ obj-$(CONFIG_FW_LOADER_USER_HELPER) += fallback_table.o obj-$(CONFIG_FW_LOADER) += firmware_class.o firmware_class-objs := main.o firmware_class-$(CONFIG_FW_LOADER_USER_HELPER) += fallback.o + +obj-y += builtin/ diff --git a/drivers/base/firmware_loader/builtin/.gitignore b/drivers/base/firmware_loader/builtin/.gitignore new file mode 100644 index 000000000000..9c8bdb9fdcc3 --- /dev/null +++ b/drivers/base/firmware_loader/builtin/.gitignore @@ -0,0 +1 @@ +*.gen.S diff --git a/drivers/base/firmware_loader/builtin/Makefile b/drivers/base/firmware_loader/builtin/Makefile new file mode 100644 index 000000000000..37e5ae387400 --- /dev/null +++ b/drivers/base/firmware_loader/builtin/Makefile @@ -0,0 +1,40 @@ +# SPDX-License-Identifier: GPL-2.0 + +# Create $(fwdir) from $(CONFIG_EXTRA_FIRMWARE_DIR) -- if it doesn't have a +# leading /, it's relative to $(srctree). +fwdir := $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE_DIR)) +fwdir := $(addprefix $(srctree)/,$(filter-out /%,$(fwdir)))$(filter /%,$(fwdir)) + +obj-y := $(addsuffix .gen.o, $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE))) + +FWNAME = $(patsubst $(obj)/%.gen.S,%,$@) +FWSTR = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME)))) +ASM_WORD = $(if $(CONFIG_64BIT),.quad,.long) +ASM_ALIGN = $(if $(CONFIG_64BIT),3,2) +PROGBITS = $(if $(CONFIG_ARM),%,@)progbits + +filechk_fwbin = \ + echo "/* Generated by $(src)/Makefile */" ;\ + echo " .section .rodata" ;\ + echo " .p2align $(ASM_ALIGN)" ;\ + echo "_fw_$(FWSTR)_bin:" ;\ + echo " .incbin \"$(fwdir)/$(FWNAME)\"" ;\ + echo "_fw_end:" ;\ + echo " .section .rodata.str,\"aMS\",$(PROGBITS),1" ;\ + echo " .p2align $(ASM_ALIGN)" ;\ + echo "_fw_$(FWSTR)_name:" ;\ + echo " .string \"$(FWNAME)\"" ;\ + echo " .section .builtin_fw,\"a\",$(PROGBITS)" ;\ + echo " .p2align $(ASM_ALIGN)" ;\ + echo " $(ASM_WORD) _fw_$(FWSTR)_name" ;\ + echo " $(ASM_WORD) _fw_$(FWSTR)_bin" ;\ + echo " $(ASM_WORD) _fw_end - _fw_$(FWSTR)_bin" + +$(obj)/%.gen.S: FORCE + $(call filechk,fwbin) + +# The .o files depend on the binaries directly; the .S files don't. +$(addprefix $(obj)/, $(obj-y)): $(obj)/%.gen.o: $(fwdir)/% + +targets := $(patsubst $(obj)/%,%, \ + $(shell find $(obj) -name \*.gen.S 2>/dev/null)) -- cgit v1.2.3 From 8a4b32691120b8fd6b029f5ad37d742a97ec82c1 Mon Sep 17 00:00:00 2001 From: Jerome Brunet Date: Fri, 21 Dec 2018 17:23:41 +0100 Subject: driver core: silence device link messages unless debugging On platforms making a fair use of regulators, the dev_info() messages coming from the device link function are a bit too verbose. The amount of message will increase further with the clock framework joining the device link party. These messages looks valuable for people debugging device link related issues, so dev_dbg() looks more appropriate than dev_info(). Signed-off-by: Jerome Brunet Acked-by: Kevin Hilman Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index 4a4b6f8cbc4f..4e57e1632f87 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -302,7 +302,7 @@ struct device_link *device_link_add(struct device *consumer, list_add_tail_rcu(&link->s_node, &supplier->links.consumers); list_add_tail_rcu(&link->c_node, &consumer->links.suppliers); - dev_info(consumer, "Linked as a consumer to %s\n", dev_name(supplier)); + dev_dbg(consumer, "Linked as a consumer to %s\n", dev_name(supplier)); out: device_pm_unlock(); @@ -328,8 +328,8 @@ static void __device_link_del(struct kref *kref) { struct device_link *link = container_of(kref, struct device_link, kref); - dev_info(link->consumer, "Dropping the link to %s\n", - dev_name(link->supplier)); + dev_dbg(link->consumer, "Dropping the link to %s\n", + dev_name(link->supplier)); if (link->flags & DL_FLAG_PM_RUNTIME) pm_runtime_drop_link(link->consumer); -- cgit v1.2.3 From 0fe6f7874d467456da6f6a221dd92499a3ab1780 Mon Sep 17 00:00:00 2001 From: Yong Wu Date: Tue, 1 Jan 2019 12:51:05 +0800 Subject: driver core: Remove the link if there is no driver with AUTO flag DL_FLAG_AUTOREMOVE_CONSUMER/SUPPLIER means "Remove the link automatically on consumer/supplier driver unbind", that means we should remove whole the device_link when there is no this driver no matter what the ref_count of the link is. CC: Greg Kroah-Hartman Signed-off-by: Yong Wu Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index 4e57e1632f87..26f8cd6dd0b3 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -512,7 +512,7 @@ static void __device_links_no_driver(struct device *dev) continue; if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) - kref_put(&link->kref, __device_link_del); + __device_link_del(&link->kref); else if (link->status != DL_STATE_SUPPLIER_UNBIND) WRITE_ONCE(link->status, DL_STATE_AVAILABLE); } @@ -557,7 +557,7 @@ void device_links_driver_cleanup(struct device *dev) */ if (link->status == DL_STATE_SUPPLIER_UNBIND && link->flags & DL_FLAG_AUTOREMOVE_SUPPLIER) - kref_put(&link->kref, __device_link_del); + __device_link_del(&link->kref); WRITE_ONCE(link->status, DL_STATE_DORMANT); } -- cgit v1.2.3 From 3451a495ef244a88ed6317a035299d835554d579 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:10 -0800 Subject: driver core: Establish order of operations for device_add and device_del via bitflag Add an additional bit flag to the device_private struct named "dead". This additional flag provides a guarantee that when a device_del is executed on a given interface an async worker will not attempt to attach the driver following the earlier device_del call. Previously this guarantee was not present and could result in the device_del call attempting to remove a driver from an interface only to have the async worker attempt to probe the driver later when it finally completes the asynchronous probe call. One additional change added was that I pulled the check for dev->driver out of the __device_attach_driver call and instead placed it in the __device_attach_async_helper call. This was motivated by the fact that the only other caller of this, __device_attach, had already taken the device_lock() and checked for dev->driver. Instead of testing for this twice in this path it makes more sense to just consolidate the dev->dead and dev->driver checks together into one set of checks. Reviewed-by: Dan Williams Reviewed-by: Rafael J. Wysocki Signed-off-by: Alexander Duyck Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 4 ++++ drivers/base/core.c | 11 +++++++++++ drivers/base/dd.c | 22 +++++++++++----------- 3 files changed, 26 insertions(+), 11 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/base.h b/drivers/base/base.h index 37329a668935..7ca475af8953 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -67,6 +67,9 @@ struct driver_private { * probed first. * @device - pointer back to the struct device that this structure is * associated with. + * @dead - This device is currently either in the process of or has been + * removed from the system. Any asynchronous events scheduled for this + * device should exit without taking any action. * * Nothing outside of the driver core should ever touch these fields. */ @@ -78,6 +81,7 @@ struct device_private { struct klist_node knode_class; struct list_head deferred_probe; struct device *device; + u8 dead:1; }; #define to_device_private_parent(obj) \ container_of(obj, struct device_private, knode_parent) diff --git a/drivers/base/core.c b/drivers/base/core.c index 26f8cd6dd0b3..1febfd2a353a 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2080,6 +2080,17 @@ void device_del(struct device *dev) struct kobject *glue_dir = NULL; struct class_interface *class_intf; + /* + * Hold the device lock and set the "dead" flag to guarantee that + * the update behavior is consistent with the other bitfields near + * it and that we cannot have an asynchronous probe routine trying + * to run while we are tearing out the bus/class/sysfs from + * underneath the device. + */ + device_lock(dev); + dev->p->dead = true; + device_unlock(dev); + /* Notify clients of device removal. This call must come * before dpm_sysfs_remove(). */ diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 8ac10af17c00..636cd16b1b62 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -731,15 +731,6 @@ static int __device_attach_driver(struct device_driver *drv, void *_data) bool async_allowed; int ret; - /* - * Check if device has already been claimed. This may - * happen with driver loading, device discovery/registration, - * and deferred probe processing happens all at once with - * multiple threads. - */ - if (dev->driver) - return -EBUSY; - ret = driver_match_device(drv, dev); if (ret == 0) { /* no match */ @@ -774,6 +765,15 @@ static void __device_attach_async_helper(void *_dev, async_cookie_t cookie) device_lock(dev); + /* + * Check if device has already been removed or claimed. This may + * happen with driver loading, device discovery/registration, + * and deferred probe processing happens all at once with + * multiple threads. + */ + if (dev->p->dead || dev->driver) + goto out_unlock; + if (dev->parent) pm_runtime_get_sync(dev->parent); @@ -784,7 +784,7 @@ static void __device_attach_async_helper(void *_dev, async_cookie_t cookie) if (dev->parent) pm_runtime_put(dev->parent); - +out_unlock: device_unlock(dev); put_device(dev); @@ -897,7 +897,7 @@ static int __driver_attach(struct device *dev, void *data) if (dev->parent && dev->bus->need_parent_lock) device_lock(dev->parent); device_lock(dev); - if (!dev->driver) + if (!dev->p->dead && !dev->driver) driver_probe_device(drv, dev); device_unlock(dev); if (dev->parent && dev->bus->need_parent_lock) -- cgit v1.2.3 From ed88747c6c4a2fc2f961a36d4c50cb0868c30229 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:16 -0800 Subject: device core: Consolidate locking and unlocking of parent and device Try to consolidate all of the locking and unlocking of both the parent and device when attaching or removing a driver from a given device. To do that I first consolidated the lock pattern into two functions __device_driver_lock and __device_driver_unlock. After doing that I then created functions specific to attaching and detaching the driver while acquiring these locks. By doing this I was able to reduce the number of spots where we touch need_parent_lock from 12 down to 4. This patch should produce no functional changes, it is meant to be a code clean-up/consolidation only. Reviewed-by: Luis Chamberlain Reviewed-by: Bart Van Assche Reviewed-by: Dan Williams Reviewed-by: Rafael J. Wysocki Signed-off-by: Alexander Duyck Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 2 ++ drivers/base/bus.c | 23 +++---------- drivers/base/dd.c | 95 ++++++++++++++++++++++++++++++++++++++++++----------- 3 files changed, 81 insertions(+), 39 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/base.h b/drivers/base/base.h index 7ca475af8953..b6966511b0ca 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -132,6 +132,8 @@ extern int driver_add_groups(struct device_driver *drv, const struct attribute_group **groups); extern void driver_remove_groups(struct device_driver *drv, const struct attribute_group **groups); +int device_driver_attach(struct device_driver *drv, struct device *dev); +void device_driver_detach(struct device *dev); extern char *make_class_name(const char *name, struct kobject *kobj); diff --git a/drivers/base/bus.c b/drivers/base/bus.c index 3ab45843da39..cfa5b63b5582 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -187,11 +187,7 @@ static ssize_t unbind_store(struct device_driver *drv, const char *buf, dev = bus_find_device_by_name(bus, NULL, buf); if (dev && dev->driver == drv) { - if (dev->parent && dev->bus->need_parent_lock) - device_lock(dev->parent); - device_release_driver(dev); - if (dev->parent && dev->bus->need_parent_lock) - device_unlock(dev->parent); + device_driver_detach(dev); err = count; } put_device(dev); @@ -214,13 +210,7 @@ static ssize_t bind_store(struct device_driver *drv, const char *buf, dev = bus_find_device_by_name(bus, NULL, buf); if (dev && dev->driver == NULL && driver_match_device(drv, dev)) { - if (dev->parent && bus->need_parent_lock) - device_lock(dev->parent); - device_lock(dev); - err = driver_probe_device(drv, dev); - device_unlock(dev); - if (dev->parent && bus->need_parent_lock) - device_unlock(dev->parent); + err = device_driver_attach(drv, dev); if (err > 0) { /* success */ @@ -773,13 +763,8 @@ EXPORT_SYMBOL_GPL(bus_rescan_devices); */ int device_reprobe(struct device *dev) { - if (dev->driver) { - if (dev->parent && dev->bus->need_parent_lock) - device_lock(dev->parent); - device_release_driver(dev); - if (dev->parent && dev->bus->need_parent_lock) - device_unlock(dev->parent); - } + if (dev->driver) + device_driver_detach(dev); return bus_rescan_devices_helper(dev, NULL); } EXPORT_SYMBOL_GPL(device_reprobe); diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 636cd16b1b62..34556359c7da 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -867,6 +867,64 @@ void device_initial_probe(struct device *dev) __device_attach(dev, true); } +/* + * __device_driver_lock - acquire locks needed to manipulate dev->drv + * @dev: Device we will update driver info for + * @parent: Parent device. Needed if the bus requires parent lock + * + * This function will take the required locks for manipulating dev->drv. + * Normally this will just be the @dev lock, but when called for a USB + * interface, @parent lock will be held as well. + */ +static void __device_driver_lock(struct device *dev, struct device *parent) +{ + if (parent && dev->bus->need_parent_lock) + device_lock(parent); + device_lock(dev); +} + +/* + * __device_driver_unlock - release locks needed to manipulate dev->drv + * @dev: Device we will update driver info for + * @parent: Parent device. Needed if the bus requires parent lock + * + * This function will release the required locks for manipulating dev->drv. + * Normally this will just be the the @dev lock, but when called for a + * USB interface, @parent lock will be released as well. + */ +static void __device_driver_unlock(struct device *dev, struct device *parent) +{ + device_unlock(dev); + if (parent && dev->bus->need_parent_lock) + device_unlock(parent); +} + +/** + * device_driver_attach - attach a specific driver to a specific device + * @drv: Driver to attach + * @dev: Device to attach it to + * + * Manually attach driver to a device. Will acquire both @dev lock and + * @dev->parent lock if needed. + */ +int device_driver_attach(struct device_driver *drv, struct device *dev) +{ + int ret = 0; + + __device_driver_lock(dev, dev->parent); + + /* + * If device has been removed or someone has already successfully + * bound a driver before us just skip the driver probe call. + */ + if (!dev->p->dead && !dev->driver) + ret = driver_probe_device(drv, dev); + + __device_driver_unlock(dev, dev->parent); + + return ret; +} + static int __driver_attach(struct device *dev, void *data) { struct device_driver *drv = data; @@ -894,14 +952,7 @@ static int __driver_attach(struct device *dev, void *data) return ret; } /* ret > 0 means positive match */ - if (dev->parent && dev->bus->need_parent_lock) - device_lock(dev->parent); - device_lock(dev); - if (!dev->p->dead && !dev->driver) - driver_probe_device(drv, dev); - device_unlock(dev); - if (dev->parent && dev->bus->need_parent_lock) - device_unlock(dev->parent); + device_driver_attach(drv, dev); return 0; } @@ -932,15 +983,11 @@ static void __device_release_driver(struct device *dev, struct device *parent) drv = dev->driver; if (drv) { while (device_links_busy(dev)) { - device_unlock(dev); - if (parent && dev->bus->need_parent_lock) - device_unlock(parent); + __device_driver_unlock(dev, parent); device_links_unbind_consumers(dev); - if (parent && dev->bus->need_parent_lock) - device_lock(parent); - device_lock(dev); + __device_driver_lock(dev, parent); /* * A concurrent invocation of the same function might * have released the driver successfully while this one @@ -993,16 +1040,12 @@ void device_release_driver_internal(struct device *dev, struct device_driver *drv, struct device *parent) { - if (parent && dev->bus->need_parent_lock) - device_lock(parent); + __device_driver_lock(dev, parent); - device_lock(dev); if (!drv || drv == dev->driver) __device_release_driver(dev, parent); - device_unlock(dev); - if (parent && dev->bus->need_parent_lock) - device_unlock(parent); + __device_driver_unlock(dev, parent); } /** @@ -1027,6 +1070,18 @@ void device_release_driver(struct device *dev) } EXPORT_SYMBOL_GPL(device_release_driver); +/** + * device_driver_detach - detach driver from a specific device + * @dev: device to detach driver from + * + * Detach driver from device. Will acquire both @dev lock and @dev->parent + * lock if needed. + */ +void device_driver_detach(struct device *dev) +{ + device_release_driver_internal(dev, NULL, dev->parent); +} + /** * driver_detach - detach driver from all devices it controls. * @drv: driver. -- cgit v1.2.3 From ef0ff68351be4fd83bec2d797f0efdc0174a55a4 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:21 -0800 Subject: driver core: Probe devices asynchronously instead of the driver Probe devices asynchronously instead of the driver. This results in us seeing the same behavior if the device is registered before the driver or after. This way we can avoid serializing the initialization should the driver not be loaded until after the devices have already been added. The motivation behind this is that if we have a set of devices that take a significant amount of time to load we can greatly reduce the time to load by processing them in parallel instead of one at a time. In addition, each device can exist on a different node so placing a single thread on one CPU to initialize all of the devices for a given driver can result in poor performance on a system with multiple nodes. This approach can reduce the time needed to scan SCSI LUNs significantly. The only way to realize that speedup is by enabling more concurrency which is what is achieved with this patch. To achieve this it was necessary to add a new member "async_driver" to the device_private structure to store the driver pointer while we wait on the deferred probe call. Reviewed-by: Bart Van Assche Reviewed-by: Dan Williams Signed-off-by: Alexander Duyck Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 2 ++ drivers/base/bus.c | 23 +++-------------------- drivers/base/dd.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 20 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/base.h b/drivers/base/base.h index b6966511b0ca..b405436ee28e 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -65,6 +65,7 @@ struct driver_private { * binding of drivers which were unable to get all the resources needed by * the device; typically because it depends on another driver getting * probed first. + * @async_driver - pointer to device driver awaiting probe via async_probe * @device - pointer back to the struct device that this structure is * associated with. * @dead - This device is currently either in the process of or has been @@ -80,6 +81,7 @@ struct device_private { struct klist_node knode_bus; struct klist_node knode_class; struct list_head deferred_probe; + struct device_driver *async_driver; struct device *device; u8 dead:1; }; diff --git a/drivers/base/bus.c b/drivers/base/bus.c index cfa5b63b5582..0a58e969f8b7 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -610,17 +610,6 @@ static ssize_t uevent_store(struct device_driver *drv, const char *buf, } static DRIVER_ATTR_WO(uevent); -static void driver_attach_async(void *_drv, async_cookie_t cookie) -{ - struct device_driver *drv = _drv; - int ret; - - ret = driver_attach(drv); - - pr_debug("bus: '%s': driver %s async attach completed: %d\n", - drv->bus->name, drv->name, ret); -} - /** * bus_add_driver - Add a driver to the bus. * @drv: driver. @@ -653,15 +642,9 @@ int bus_add_driver(struct device_driver *drv) klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers); if (drv->bus->p->drivers_autoprobe) { - if (driver_allows_async_probing(drv)) { - pr_debug("bus: '%s': probing driver %s asynchronously\n", - drv->bus->name, drv->name); - async_schedule(driver_attach_async, drv); - } else { - error = driver_attach(drv); - if (error) - goto out_unregister; - } + error = driver_attach(drv); + if (error) + goto out_unregister; } module_add_driver(drv->owner, drv); diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 34556359c7da..627ad05064e0 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -925,6 +925,30 @@ int device_driver_attach(struct device_driver *drv, struct device *dev) return ret; } +static void __driver_attach_async_helper(void *_dev, async_cookie_t cookie) +{ + struct device *dev = _dev; + struct device_driver *drv; + int ret = 0; + + __device_driver_lock(dev, dev->parent); + + drv = dev->p->async_driver; + + /* + * If device has been removed or someone has already successfully + * bound a driver before us just skip the driver probe call. + */ + if (!dev->p->dead && !dev->driver) + ret = driver_probe_device(drv, dev); + + __device_driver_unlock(dev, dev->parent); + + dev_dbg(dev, "driver %s async attach completed: %d\n", drv->name, ret); + + put_device(dev); +} + static int __driver_attach(struct device *dev, void *data) { struct device_driver *drv = data; @@ -952,6 +976,25 @@ static int __driver_attach(struct device *dev, void *data) return ret; } /* ret > 0 means positive match */ + if (driver_allows_async_probing(drv)) { + /* + * Instead of probing the device synchronously we will + * probe it asynchronously to allow for more parallelism. + * + * We only take the device lock here in order to guarantee + * that the dev->driver and async_driver fields are protected + */ + dev_dbg(dev, "probing driver %s asynchronously\n", drv->name); + device_lock(dev); + if (!dev->driver) { + get_device(dev); + dev->p->async_driver = drv; + async_schedule(__driver_attach_async_helper, dev); + } + device_unlock(dev); + return 0; + } + device_driver_attach(drv, dev); return 0; -- cgit v1.2.3 From c37e20eaf4b21125898fd454f3ea6b212865d0a6 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:37 -0800 Subject: driver core: Attach devices on CPU local to device node Call the asynchronous probe routines on a CPU local to the device node. By doing this we should be able to improve our initialization time significantly as we can avoid having to access the device from a remote node which may introduce higher latency. For example, in the case of initializing memory for NVDIMM this can have a significant impact as initialing 3TB on remote node can take up to 39 seconds while initialing it on a local node only takes 23 seconds. It is situations like this where we will see the biggest improvement. Reviewed-by: Dan Williams Reviewed-by: Bart Van Assche Signed-off-by: Alexander Duyck Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 627ad05064e0..aa6a9c613595 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -829,7 +829,7 @@ static int __device_attach(struct device *dev, bool allow_async) */ dev_dbg(dev, "scheduling asynchronous probe\n"); get_device(dev); - async_schedule(__device_attach_async_helper, dev); + async_schedule_dev(__device_attach_async_helper, dev); } else { pm_request_idle(dev); } @@ -989,7 +989,7 @@ static int __driver_attach(struct device *dev, void *data) if (!dev->driver) { get_device(dev); dev->p->async_driver = drv; - async_schedule(__driver_attach_async_helper, dev); + async_schedule_dev(__driver_attach_async_helper, dev); } device_unlock(dev); return 0; -- cgit v1.2.3 From 8b9ec6b732775849f506aa6c2649e626e82a297c Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:42 -0800 Subject: PM core: Use new async_schedule_dev command Use the device specific version of the async_schedule commands to defer various tasks related to power management. By doing this we should see a slight improvement in performance as any device that is sensitive to latency/locality in the setup will now be initializing on the node closest to the device. Reviewed-by: Dan Williams Reviewed-by: Bart Van Assche Reviewed-by: Rafael J. Wysocki Signed-off-by: Alexander Duyck Signed-off-by: Greg Kroah-Hartman --- drivers/base/power/main.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 0992e67e862b..93ddbcebcece 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -727,7 +727,7 @@ void dpm_noirq_resume_devices(pm_message_t state) reinit_completion(&dev->power.completion); if (is_async(dev)) { get_device(dev); - async_schedule(async_resume_noirq, dev); + async_schedule_dev(async_resume_noirq, dev); } } @@ -884,7 +884,7 @@ void dpm_resume_early(pm_message_t state) reinit_completion(&dev->power.completion); if (is_async(dev)) { get_device(dev); - async_schedule(async_resume_early, dev); + async_schedule_dev(async_resume_early, dev); } } @@ -1048,7 +1048,7 @@ void dpm_resume(pm_message_t state) reinit_completion(&dev->power.completion); if (is_async(dev)) { get_device(dev); - async_schedule(async_resume, dev); + async_schedule_dev(async_resume, dev); } } @@ -1368,7 +1368,7 @@ static int device_suspend_noirq(struct device *dev) if (is_async(dev)) { get_device(dev); - async_schedule(async_suspend_noirq, dev); + async_schedule_dev(async_suspend_noirq, dev); return 0; } return __device_suspend_noirq(dev, pm_transition, false); @@ -1571,7 +1571,7 @@ static int device_suspend_late(struct device *dev) if (is_async(dev)) { get_device(dev); - async_schedule(async_suspend_late, dev); + async_schedule_dev(async_suspend_late, dev); return 0; } @@ -1835,7 +1835,7 @@ static int device_suspend(struct device *dev) if (is_async(dev)) { get_device(dev); - async_schedule(async_suspend, dev); + async_schedule_dev(async_suspend, dev); return 0; } -- cgit v1.2.3 From 57ea974fb8717864e8b7ec679363c5a3298a165e Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 22 Jan 2019 10:39:52 -0800 Subject: driver core: Rewrite test_async_driver_probe to cover serialization and NUMA affinity The current async_probe test code is only testing one device allocated prior to driver load and only loading one device afterwards. Instead of doing things this way it makes much more sense to load one device per CPU in order to actually stress the async infrastructure. By doing this we should see delays significantly increase in the event of devices being serialized. In addition I have updated the test to verify that we are trying to place the work on the correct NUMA node when we are running in async mode. By doing this we can verify the best possible outcome for device and driver load times. I have added a timeout value that is used to disable the sleep and instead cause the probe routine to report an error indicating it timed out. By doing this we limit the maximum runtime for the test to 20 seconds or less. The last major change in this set is that I have gone through and tuned it for handling the massive number of possible events that will be scheduled. Instead of reporting the sleep for each individual device it is moved to only being displayed if we enable debugging. With this patch applied below are what a failing test and a passing test should look like. I elided a few hundred lines in the failing test that were duplicated since the system I was testing on had a massive number of CPU cores: -- Failing -- [ 243.524697] test_async_driver_probe: registering first set of asynchronous devices... [ 243.535625] test_async_driver_probe: registering asynchronous driver... [ 243.543038] test_async_driver_probe: registration took 0 msecs [ 243.549559] test_async_driver_probe: registering second set of asynchronous devices... [ 243.568350] platform test_async_driver.447: registration took 9 msecs [ 243.575544] test_async_driver_probe: registering first synchronous device... [ 243.583454] test_async_driver_probe: registering synchronous driver... [ 248.825920] test_async_driver_probe: registration took 5235 msecs [ 248.825922] test_async_driver_probe: registering second synchronous device... [ 248.825928] test_async_driver test_async_driver.443: NUMA node mismatch 3 != 1 [ 248.825932] test_async_driver test_async_driver.445: NUMA node mismatch 3 != 1 [ 248.825935] test_async_driver test_async_driver.446: NUMA node mismatch 3 != 1 [ 248.825939] test_async_driver test_async_driver.440: NUMA node mismatch 3 != 1 [ 248.825943] test_async_driver test_async_driver.441: NUMA node mismatch 3 != 1 ... [ 248.827150] test_async_driver test_async_driver.229: NUMA node mismatch 0 != 1 [ 248.827158] test_async_driver test_async_driver.228: NUMA node mismatch 0 != 1 [ 248.827220] test_async_driver test_async_driver.281: NUMA node mismatch 2 != 1 [ 248.827229] test_async_driver test_async_driver.282: NUMA node mismatch 2 != 1 [ 248.827240] test_async_driver test_async_driver.280: NUMA node mismatch 2 != 1 [ 253.945834] test_async_driver test_async_driver.1: NUMA node mismatch 0 != 1 [ 253.945878] test_sync_driver test_sync_driver.1: registration took 5119 msecs [ 253.961693] test_async_driver_probe: async events still pending, forcing timeout and synchronize [ 259.065839] test_async_driver test_async_driver.2: NUMA node mismatch 0 != 1 [ 259.073786] test_async_driver test_async_driver.3: async probe took too long [ 259.081669] test_async_driver test_async_driver.3: NUMA node mismatch 0 != 1 [ 259.089569] test_async_driver test_async_driver.4: async probe took too long [ 259.097451] test_async_driver test_async_driver.4: NUMA node mismatch 0 != 1 [ 259.105338] test_async_driver test_async_driver.5: async probe took too long [ 259.113204] test_async_driver test_async_driver.5: NUMA node mismatch 0 != 1 [ 259.121089] test_async_driver test_async_driver.6: async probe took too long [ 259.128961] test_async_driver test_async_driver.6: NUMA node mismatch 0 != 1 [ 259.136850] test_async_driver test_async_driver.7: async probe took too long ... [ 262.124062] test_async_driver test_async_driver.221: async probe took too long [ 262.132130] test_async_driver test_async_driver.221: NUMA node mismatch 3 != 1 [ 262.140206] test_async_driver test_async_driver.222: async probe took too long [ 262.148277] test_async_driver test_async_driver.222: NUMA node mismatch 3 != 1 [ 262.156351] test_async_driver test_async_driver.223: async probe took too long [ 262.164419] test_async_driver test_async_driver.223: NUMA node mismatch 3 != 1 [ 262.172630] test_async_driver_probe: Test failed with 222 errors and 336 warnings -- Passing -- [ 105.419247] test_async_driver_probe: registering first set of asynchronous devices... [ 105.432040] test_async_driver_probe: registering asynchronous driver... [ 105.439718] test_async_driver_probe: registration took 0 msecs [ 105.446239] test_async_driver_probe: registering second set of asynchronous devices... [ 105.477986] platform test_async_driver.447: registration took 22 msecs [ 105.485276] test_async_driver_probe: registering first synchronous device... [ 105.493169] test_async_driver_probe: registering synchronous driver... [ 110.597981] test_async_driver_probe: registration took 5097 msecs [ 110.604806] test_async_driver_probe: registering second synchronous device... [ 115.707490] test_sync_driver test_sync_driver.1: registration took 5094 msecs [ 115.715478] test_async_driver_probe: completed successfully Signed-off-by: Alexander Duyck Signed-off-by: Greg Kroah-Hartman --- drivers/base/test/test_async_driver_probe.c | 261 +++++++++++++++++++++------- 1 file changed, 201 insertions(+), 60 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/test/test_async_driver_probe.c b/drivers/base/test/test_async_driver_probe.c index e7f145d662f0..f4b1d8e54daf 100644 --- a/drivers/base/test/test_async_driver_probe.c +++ b/drivers/base/test/test_async_driver_probe.c @@ -11,16 +11,47 @@ #include #include #include +#include +#include +#include #define TEST_PROBE_DELAY (5 * 1000) /* 5 sec */ #define TEST_PROBE_THRESHOLD (TEST_PROBE_DELAY / 2) +static atomic_t warnings, errors, timeout, async_completed; + static int test_probe(struct platform_device *pdev) { - dev_info(&pdev->dev, "sleeping for %d msecs in probe\n", - TEST_PROBE_DELAY); - msleep(TEST_PROBE_DELAY); - dev_info(&pdev->dev, "done sleeping\n"); + struct device *dev = &pdev->dev; + + /* + * Determine if we have hit the "timeout" limit for the test if we + * have then report it as an error, otherwise we wil sleep for the + * required amount of time and then report completion. + */ + if (atomic_read(&timeout)) { + dev_err(dev, "async probe took too long\n"); + atomic_inc(&errors); + } else { + dev_dbg(&pdev->dev, "sleeping for %d msecs in probe\n", + TEST_PROBE_DELAY); + msleep(TEST_PROBE_DELAY); + dev_dbg(&pdev->dev, "done sleeping\n"); + } + + /* + * Report NUMA mismatch if device node is set and we are not + * performing an async init on that node. + */ + if (dev->driver->probe_type == PROBE_PREFER_ASYNCHRONOUS) { + if (dev_to_node(dev) != numa_node_id()) { + dev_warn(dev, "NUMA node mismatch %d != %d\n", + dev_to_node(dev), numa_node_id()); + atomic_inc(&warnings); + } + + atomic_inc(&async_completed); + } return 0; } @@ -41,31 +72,64 @@ static struct platform_driver sync_driver = { .probe = test_probe, }; -static struct platform_device *async_dev_1, *async_dev_2; -static struct platform_device *sync_dev_1; +static struct platform_device *async_dev[NR_CPUS * 2]; +static struct platform_device *sync_dev[2]; + +static struct platform_device * +test_platform_device_register_node(char *name, int id, int nid) +{ + struct platform_device *pdev; + int ret; + + pdev = platform_device_alloc(name, id); + if (!pdev) + return NULL; + + if (nid != NUMA_NO_NODE) + set_dev_node(&pdev->dev, nid); + + ret = platform_device_add(pdev); + if (ret) { + platform_device_put(pdev); + return ERR_PTR(ret); + } + + return pdev; + +} static int __init test_async_probe_init(void) { - ktime_t calltime, delta; + struct platform_device **pdev = NULL; + int async_id = 0, sync_id = 0; unsigned long long duration; - int error; + ktime_t calltime, delta; + int err, nid, cpu; + + pr_info("registering first set of asynchronous devices...\n"); - pr_info("registering first asynchronous device...\n"); + for_each_online_cpu(cpu) { + nid = cpu_to_node(cpu); + pdev = &async_dev[async_id]; + *pdev = test_platform_device_register_node("test_async_driver", + async_id, + nid); + if (IS_ERR(*pdev)) { + err = PTR_ERR(*pdev); + *pdev = NULL; + pr_err("failed to create async_dev: %d\n", err); + goto err_unregister_async_devs; + } - async_dev_1 = platform_device_register_simple("test_async_driver", 1, - NULL, 0); - if (IS_ERR(async_dev_1)) { - error = PTR_ERR(async_dev_1); - pr_err("failed to create async_dev_1: %d\n", error); - return error; + async_id++; } pr_info("registering asynchronous driver...\n"); calltime = ktime_get(); - error = platform_driver_register(&async_driver); - if (error) { - pr_err("Failed to register async_driver: %d\n", error); - goto err_unregister_async_dev_1; + err = platform_driver_register(&async_driver); + if (err) { + pr_err("Failed to register async_driver: %d\n", err); + goto err_unregister_async_devs; } delta = ktime_sub(ktime_get(), calltime); @@ -73,86 +137,163 @@ static int __init test_async_probe_init(void) pr_info("registration took %lld msecs\n", duration); if (duration > TEST_PROBE_THRESHOLD) { pr_err("test failed: probe took too long\n"); - error = -ETIMEDOUT; + err = -ETIMEDOUT; goto err_unregister_async_driver; } - pr_info("registering second asynchronous device...\n"); + pr_info("registering second set of asynchronous devices...\n"); calltime = ktime_get(); - async_dev_2 = platform_device_register_simple("test_async_driver", 2, - NULL, 0); - if (IS_ERR(async_dev_2)) { - error = PTR_ERR(async_dev_2); - pr_err("failed to create async_dev_2: %d\n", error); - goto err_unregister_async_driver; + for_each_online_cpu(cpu) { + nid = cpu_to_node(cpu); + pdev = &sync_dev[sync_id]; + + *pdev = test_platform_device_register_node("test_async_driver", + async_id, + nid); + if (IS_ERR(*pdev)) { + err = PTR_ERR(*pdev); + *pdev = NULL; + pr_err("failed to create async_dev: %d\n", err); + goto err_unregister_async_driver; + } + + async_id++; } delta = ktime_sub(ktime_get(), calltime); duration = (unsigned long long) ktime_to_ms(delta); - pr_info("registration took %lld msecs\n", duration); + dev_info(&(*pdev)->dev, + "registration took %lld msecs\n", duration); if (duration > TEST_PROBE_THRESHOLD) { - pr_err("test failed: probe took too long\n"); - error = -ETIMEDOUT; - goto err_unregister_async_dev_2; + dev_err(&(*pdev)->dev, + "test failed: probe took too long\n"); + err = -ETIMEDOUT; + goto err_unregister_async_driver; } - pr_info("registering synchronous driver...\n"); - error = platform_driver_register(&sync_driver); - if (error) { - pr_err("Failed to register async_driver: %d\n", error); - goto err_unregister_async_dev_2; + pr_info("registering first synchronous device...\n"); + nid = cpu_to_node(cpu); + pdev = &sync_dev[sync_id]; + + *pdev = test_platform_device_register_node("test_sync_driver", + sync_id, + NUMA_NO_NODE); + if (IS_ERR(*pdev)) { + err = PTR_ERR(*pdev); + *pdev = NULL; + pr_err("failed to create sync_dev: %d\n", err); + goto err_unregister_async_driver; } - pr_info("registering synchronous device...\n"); + sync_id++; + + pr_info("registering synchronous driver...\n"); calltime = ktime_get(); - sync_dev_1 = platform_device_register_simple("test_sync_driver", 1, - NULL, 0); - if (IS_ERR(sync_dev_1)) { - error = PTR_ERR(sync_dev_1); - pr_err("failed to create sync_dev_1: %d\n", error); - goto err_unregister_sync_driver; + err = platform_driver_register(&sync_driver); + if (err) { + pr_err("Failed to register async_driver: %d\n", err); + goto err_unregister_sync_devs; } delta = ktime_sub(ktime_get(), calltime); duration = (unsigned long long) ktime_to_ms(delta); pr_info("registration took %lld msecs\n", duration); if (duration < TEST_PROBE_THRESHOLD) { - pr_err("test failed: probe was too quick\n"); - error = -ETIMEDOUT; - goto err_unregister_sync_dev_1; + dev_err(&(*pdev)->dev, + "test failed: probe was too quick\n"); + err = -ETIMEDOUT; + goto err_unregister_sync_driver; } - pr_info("completed successfully"); + pr_info("registering second synchronous device...\n"); + pdev = &sync_dev[sync_id]; + calltime = ktime_get(); - return 0; + *pdev = test_platform_device_register_node("test_sync_driver", + sync_id, + NUMA_NO_NODE); + if (IS_ERR(*pdev)) { + err = PTR_ERR(*pdev); + *pdev = NULL; + pr_err("failed to create sync_dev: %d\n", err); + goto err_unregister_sync_driver; + } -err_unregister_sync_dev_1: - platform_device_unregister(sync_dev_1); + sync_id++; -err_unregister_sync_driver: - platform_driver_unregister(&sync_driver); + delta = ktime_sub(ktime_get(), calltime); + duration = (unsigned long long) ktime_to_ms(delta); + dev_info(&(*pdev)->dev, + "registration took %lld msecs\n", duration); + if (duration < TEST_PROBE_THRESHOLD) { + dev_err(&(*pdev)->dev, + "test failed: probe was too quick\n"); + err = -ETIMEDOUT; + goto err_unregister_sync_driver; + } -err_unregister_async_dev_2: - platform_device_unregister(async_dev_2); + /* + * The async events should have completed while we were taking care + * of the synchronous events. We will now terminate any outstanding + * asynchronous probe calls remaining by forcing timeout and remove + * the driver before we return which should force the flush of the + * pending asynchronous probe calls. + * + * Otherwise if they completed without errors or warnings then + * report successful completion. + */ + if (atomic_read(&async_completed) != async_id) { + pr_err("async events still pending, forcing timeout\n"); + atomic_inc(&timeout); + err = -ETIMEDOUT; + } else if (!atomic_read(&errors) && !atomic_read(&warnings)) { + pr_info("completed successfully\n"); + return 0; + } +err_unregister_sync_driver: + platform_driver_unregister(&sync_driver); +err_unregister_sync_devs: + while (sync_id--) + platform_device_unregister(sync_dev[sync_id]); err_unregister_async_driver: platform_driver_unregister(&async_driver); +err_unregister_async_devs: + while (async_id--) + platform_device_unregister(async_dev[async_id]); + + /* + * If err is already set then count that as an additional error for + * the test. Otherwise we will report an invalid argument error and + * not count that as we should have reached here as a result of + * errors or warnings being reported by the probe routine. + */ + if (err) + atomic_inc(&errors); + else + err = -EINVAL; -err_unregister_async_dev_1: - platform_device_unregister(async_dev_1); + pr_err("Test failed with %d errors and %d warnings\n", + atomic_read(&errors), atomic_read(&warnings)); - return error; + return err; } module_init(test_async_probe_init); static void __exit test_async_probe_exit(void) { + int id = 2; + platform_driver_unregister(&async_driver); platform_driver_unregister(&sync_driver); - platform_device_unregister(async_dev_1); - platform_device_unregister(async_dev_2); - platform_device_unregister(sync_dev_1); + + while (id--) + platform_device_unregister(sync_dev[id]); + + id = NR_CPUS * 2; + while (id--) + platform_device_unregister(async_dev[id]); } module_exit(test_async_probe_exit); -- cgit v1.2.3 From fa548d79d87fd1ad2a2c4e26f829984a106c6ca5 Mon Sep 17 00:00:00 2001 From: Mathieu Malaterre Date: Wed, 23 Jan 2019 20:42:00 +0100 Subject: drivers: base: Use __printf markup to silence compiler Silence warnings (triggered at W=1) by adding relevant __printf attributes. drivers/base/cpu.c:432:2: warning: function '__cpu_device_create' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format] Signed-off-by: Mathieu Malaterre Signed-off-by: Greg Kroah-Hartman --- drivers/base/cpu.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/base') diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c index eb9443d5bae1..de74d7d93558 100644 --- a/drivers/base/cpu.c +++ b/drivers/base/cpu.c @@ -409,6 +409,7 @@ static void device_create_release(struct device *dev) kfree(dev); } +__printf(4, 0) static struct device * __cpu_device_create(struct device *parent, void *drvdata, const struct attribute_group **groups, -- cgit v1.2.3 From c8d50986da5d74ddfc233b13b91d0a13369fa164 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:45:55 +0100 Subject: driver core: Fix DL_FLAG_AUTOREMOVE_SUPPLIER device link flag handling Change the list walk in device_links_driver_cleanup() to a safe one to avoid use-after-free when dropping a link from the list during the walk. Also, while at it, fix device_link_add() to refuse to create stateless device links with DL_FLAG_AUTOREMOVE_SUPPLIER set, which is an invalid combination (setting that flag means that the driver core should manage the link, so it cannot be stateless), and extend the kerneldoc comment of device_link_add() to cover the DL_FLAG_AUTOREMOVE_SUPPLIER flag properly too. Fixes: 1689cac5b32a ("driver core: Add flag to autoremove device link on supplier unbind") Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index 1febfd2a353a..f24eb2daad00 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -179,10 +179,14 @@ void device_pm_move_to_tail(struct device *dev) * of the link. If DL_FLAG_PM_RUNTIME is not set, DL_FLAG_RPM_ACTIVE will be * ignored. * - * If the DL_FLAG_AUTOREMOVE_CONSUMER is set, the link will be removed - * automatically when the consumer device driver unbinds from it. - * The combination of both DL_FLAG_AUTOREMOVE_CONSUMER and DL_FLAG_STATELESS - * set is invalid and will cause NULL to be returned. + * If the DL_FLAG_AUTOREMOVE_CONSUMER flag is set, the link will be removed + * automatically when the consumer device driver unbinds from it. Analogously, + * if DL_FLAG_AUTOREMOVE_SUPPLIER is set in @flags, the link will be removed + * automatically when the supplier device driver unbinds from it. + * + * The combination of DL_FLAG_STATELESS and either DL_FLAG_AUTOREMOVE_CONSUMER + * or DL_FLAG_AUTOREMOVE_SUPPLIER set in @flags at the same time is invalid and + * will cause NULL to be returned upfront. * * A side effect of the link creation is re-ordering of dpm_list and the * devices_kset list by moving the consumer device and all devices depending @@ -199,8 +203,8 @@ struct device_link *device_link_add(struct device *consumer, struct device_link *link; if (!consumer || !supplier || - ((flags & DL_FLAG_STATELESS) && - (flags & DL_FLAG_AUTOREMOVE_CONSUMER))) + (flags & DL_FLAG_STATELESS && + flags & (DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_AUTOREMOVE_SUPPLIER))) return NULL; device_links_write_lock(); @@ -539,11 +543,11 @@ void device_links_no_driver(struct device *dev) */ void device_links_driver_cleanup(struct device *dev) { - struct device_link *link; + struct device_link *link, *ln; device_links_write_lock(); - list_for_each_entry(link, &dev->links.consumers, s_node) { + list_for_each_entry_safe(link, ln, &dev->links.consumers, s_node) { if (link->flags & DL_FLAG_STATELESS) continue; -- cgit v1.2.3 From f265df550a4350dce0a4d721a77c52e4b847ea40 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:46:54 +0100 Subject: driver core: Avoid careless re-use of existing device links After commit ead18c23c263 ("driver core: Introduce device links reference counting"), if there is a link between the given supplier and the given consumer already, device_link_add() will refcount it and return it unconditionally. However, if the flags passed to it on the second (or any subsequent) attempt to create a device link between the same consumer-supplier pair are not compatible with the existing link's flags, that is incorrect. First off, if the existing link is stateless and the next caller of device_link_add() for the same consumer-supplier pair wants a stateful one, or the other way around, the existing link cannot be returned, because it will not match the expected behavior, so make device_link_add() dump the stack and return NULL in that case. Moreover, if the DL_FLAG_AUTOREMOVE_CONSUMER flag is passed to device_link_add(), its caller will expect its reference to the link to be dropped automatically on consumer driver removal, which will not happen if that flag is not set in the link's flags (and analogously for DL_FLAG_AUTOREMOVE_SUPPLIER). For this reason, make device_link_add() update the existing link's flags accordingly before returning it to the caller. Fixes: ead18c23c263 ("driver core: Introduce device links reference counting") Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index f24eb2daad00..a4d6f9b93088 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -221,12 +221,29 @@ struct device_link *device_link_add(struct device *consumer, goto out; } - list_for_each_entry(link, &supplier->links.consumers, s_node) - if (link->consumer == consumer) { - kref_get(&link->kref); + list_for_each_entry(link, &supplier->links.consumers, s_node) { + if (link->consumer != consumer) + continue; + + /* + * Don't return a stateless link if the caller wants a stateful + * one and vice versa. + */ + if (WARN_ON((flags & DL_FLAG_STATELESS) != (link->flags & DL_FLAG_STATELESS))) { + link = NULL; goto out; } + if (flags & DL_FLAG_AUTOREMOVE_CONSUMER) + link->flags |= DL_FLAG_AUTOREMOVE_CONSUMER; + + if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) + link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER; + + kref_get(&link->kref); + goto out; + } + link = kzalloc(sizeof(*link), GFP_KERNEL); if (!link) goto out; -- cgit v1.2.3 From 5db25c9eb893df8f6b93c1d97b8006d768e1b6f5 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:47:53 +0100 Subject: driver core: Do not resume suppliers under device_links_write_lock() It is incorrect to call pm_runtime_get_sync() under device_links_write_lock(), because it may end up trying to take device_links_read_lock() while resuming the target device and that will deadlock in the non-SRCU case, so avoid that by resuming the supplier device in device_link_add() before calling device_links_write_lock(). Fixes: 21d5c57b3726 ("PM / runtime: Use device links") Fixes: baa8809f6097 ("PM / runtime: Optimize the use of device links") Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index a4d6f9b93088..50610cd87e71 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -201,12 +201,21 @@ struct device_link *device_link_add(struct device *consumer, struct device *supplier, u32 flags) { struct device_link *link; + bool rpm_put_supplier = false; if (!consumer || !supplier || (flags & DL_FLAG_STATELESS && flags & (DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_AUTOREMOVE_SUPPLIER))) return NULL; + if (flags & DL_FLAG_PM_RUNTIME && flags & DL_FLAG_RPM_ACTIVE) { + if (pm_runtime_get_sync(supplier) < 0) { + pm_runtime_put_noidle(supplier); + return NULL; + } + rpm_put_supplier = true; + } + device_links_write_lock(); device_pm_lock(); @@ -250,13 +259,8 @@ struct device_link *device_link_add(struct device *consumer, if (flags & DL_FLAG_PM_RUNTIME) { if (flags & DL_FLAG_RPM_ACTIVE) { - if (pm_runtime_get_sync(supplier) < 0) { - pm_runtime_put_noidle(supplier); - kfree(link); - link = NULL; - goto out; - } link->rpm_active = true; + rpm_put_supplier = false; } pm_runtime_new_link(consumer); /* @@ -328,6 +332,10 @@ struct device_link *device_link_add(struct device *consumer, out: device_pm_unlock(); device_links_write_unlock(); + + if (rpm_put_supplier) + pm_runtime_put(supplier); + return link; } EXPORT_SYMBOL_GPL(device_link_add); -- cgit v1.2.3 From e2f3cd831a280fc226118d9369bf3f77aab58c56 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:49:14 +0100 Subject: driver core: Fix handling of runtime PM flags in device_link_add() After commit ead18c23c263 ("driver core: Introduce device links reference counting"), if there is a link between the given supplier and the given consumer already, device_link_add() will refcount it and return it unconditionally without updating its flags. It is possible, however, that the second (or any subsequent) caller of device_link_add() for the same consumer-supplier pair will pass DL_FLAG_PM_RUNTIME, possibly along with DL_FLAG_RPM_ACTIVE, in flags to it and the existing link may not behave as expected then. First, if DL_FLAG_PM_RUNTIME is not set in the existing link's flags at all, it needs to be set like during the original initialization of the link. Second, if DL_FLAG_RPM_ACTIVE is passed to device_link_add() in flags (in addition to DL_FLAG_PM_RUNTIME), the existing link should to be updated to reflect the "active" runtime PM configuration of the consumer-supplier pair and extra care must be taken here to avoid possible destructive races with runtime PM of the consumer. To that end, redefine the rpm_active field in struct device_link as a refcount, initialize it to 1 and make rpm_resume() (for the consumer) and device_link_add() increment it whenever they acquire a runtime PM reference on the supplier device. Accordingly, make rpm_suspend() (for the consumer) and pm_runtime_clean_up_links() decrement it and drop runtime PM references to the supplier device in a loop until rpm_active becones 1 again. Fixes: ead18c23c263 ("driver core: Introduce device links reference counting") Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 45 +++++++++++++++++++++++++++++--------------- drivers/base/power/runtime.c | 26 +++++++++++-------------- 2 files changed, 41 insertions(+), 30 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index 50610cd87e71..8611385e44b5 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -165,6 +165,19 @@ void device_pm_move_to_tail(struct device *dev) device_links_read_unlock(idx); } +static void device_link_rpm_prepare(struct device *consumer, + struct device *supplier) +{ + pm_runtime_new_link(consumer); + /* + * If the link is being added by the consumer driver at probe time, + * balance the decrementation of the supplier's runtime PM usage counter + * after consumer probe in driver_probe_device(). + */ + if (consumer->links.status == DL_DEV_PROBING) + pm_runtime_get_noresume(supplier); +} + /** * device_link_add - Create a link between two devices. * @consumer: Consumer end of the link. @@ -201,7 +214,6 @@ struct device_link *device_link_add(struct device *consumer, struct device *supplier, u32 flags) { struct device_link *link; - bool rpm_put_supplier = false; if (!consumer || !supplier || (flags & DL_FLAG_STATELESS && @@ -213,7 +225,6 @@ struct device_link *device_link_add(struct device *consumer, pm_runtime_put_noidle(supplier); return NULL; } - rpm_put_supplier = true; } device_links_write_lock(); @@ -249,6 +260,15 @@ struct device_link *device_link_add(struct device *consumer, if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER; + if (flags & DL_FLAG_PM_RUNTIME) { + if (!(link->flags & DL_FLAG_PM_RUNTIME)) { + device_link_rpm_prepare(consumer, supplier); + link->flags |= DL_FLAG_PM_RUNTIME; + } + if (flags & DL_FLAG_RPM_ACTIVE) + refcount_inc(&link->rpm_active); + } + kref_get(&link->kref); goto out; } @@ -257,20 +277,15 @@ struct device_link *device_link_add(struct device *consumer, if (!link) goto out; + refcount_set(&link->rpm_active, 1); + if (flags & DL_FLAG_PM_RUNTIME) { - if (flags & DL_FLAG_RPM_ACTIVE) { - link->rpm_active = true; - rpm_put_supplier = false; - } - pm_runtime_new_link(consumer); - /* - * If the link is being added by the consumer driver at probe - * time, balance the decrementation of the supplier's runtime PM - * usage counter after consumer probe in driver_probe_device(). - */ - if (consumer->links.status == DL_DEV_PROBING) - pm_runtime_get_noresume(supplier); + if (flags & DL_FLAG_RPM_ACTIVE) + refcount_inc(&link->rpm_active); + + device_link_rpm_prepare(consumer, supplier); } + get_device(supplier); link->supplier = supplier; INIT_LIST_HEAD(&link->s_node); @@ -333,7 +348,7 @@ struct device_link *device_link_add(struct device *consumer, device_pm_unlock(); device_links_write_unlock(); - if (rpm_put_supplier) + if ((flags & DL_FLAG_PM_RUNTIME && flags & DL_FLAG_RPM_ACTIVE) && !link) pm_runtime_put(supplier); return link; diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 457be03b744d..8bc9a432de70 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -259,11 +259,8 @@ static int rpm_get_suppliers(struct device *dev) list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) { int retval; - if (!(link->flags & DL_FLAG_PM_RUNTIME)) - continue; - - if (READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND || - link->rpm_active) + if (!(link->flags & DL_FLAG_PM_RUNTIME) || + READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND) continue; retval = pm_runtime_get_sync(link->supplier); @@ -272,7 +269,7 @@ static int rpm_get_suppliers(struct device *dev) pm_runtime_put_noidle(link->supplier); return retval; } - link->rpm_active = true; + refcount_inc(&link->rpm_active); } return 0; } @@ -281,12 +278,13 @@ static void rpm_put_suppliers(struct device *dev) { struct device_link *link; - list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) - if (link->rpm_active && - READ_ONCE(link->status) != DL_STATE_SUPPLIER_UNBIND) { + list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) { + if (READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND) + continue; + + while (refcount_dec_not_one(&link->rpm_active)) pm_runtime_put(link->supplier); - link->rpm_active = false; - } + } } /** @@ -1539,7 +1537,7 @@ void pm_runtime_remove(struct device *dev) * * Check links from this device to any consumers and if any of them have active * runtime PM references to the device, drop the usage counter of the device - * (once per link). + * (as many times as needed). * * Links with the DL_FLAG_STATELESS flag set are ignored. * @@ -1561,10 +1559,8 @@ void pm_runtime_clean_up_links(struct device *dev) if (link->flags & DL_FLAG_STATELESS) continue; - if (link->rpm_active) { + while (refcount_dec_not_one(&link->rpm_active)) pm_runtime_put_noidle(dev); - link->rpm_active = false; - } } device_links_read_unlock(idx); -- cgit v1.2.3 From 15cfb094160385cc0b303c4cda483caa102af654 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:50:39 +0100 Subject: driver core: Fix adding device links to probing suppliers Currently, it is not valid to add a device link from a consumer driver ->probe callback to a supplier that is still probing too, but generally this is a valid use case. For example, if the consumer has just acquired a resource that can only be available if the supplier is functional, adding a device link to that supplier right away should be safe (and even desirable arguably), but device_link_add() doesn't handle that case correctly and the initial state of the link created by it is wrong then. To address this problem, change the initial state of device links added between a probing supplier and a probing consumer to DL_STATE_CONSUMER_PROBE and update device_links_driver_bound() to skip such links on the supplier side. With this change, if the supplier probe completes first, device_links_driver_bound() called for it will skip the link state update and when it is called for the consumer, the link state will be updated to "active". In turn, if the consumer probe completes first, device_links_driver_bound() called for it will change the state of the link to "active" and when it is called for the supplier, the link status update will be skipped. However, in principle the supplier or consumer probe may still fail after the link has been added, so modify device_links_no_driver() to change device links in the "active" or "consumer probe" state to "dormant" on the supplier side and update __device_links_no_driver() to change the link state to "available" only if it is "consumer probe" or "active". Then, if the supplier probe fails first, the leftover link to the probing consumer will become "dormant" and device_links_no_driver() called for the consumer (when its probe fails) will clean it up. In turn, if the consumer probe fails first, it will either drop the link, or change its state to "available" and, in the latter case, when device_links_no_driver() is called for the supplier, it will update the link state to "dormant". [If the supplier probe fails, but the consumer probe succeeds, which should not happen as long as the consumer driver is correct, the link still will be around, but it will be "dormant" until the supplier is probed again.] Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 66 insertions(+), 8 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index 8611385e44b5..d58ea075f807 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -300,17 +300,26 @@ struct device_link *device_link_add(struct device *consumer, link->status = DL_STATE_NONE; } else { switch (supplier->links.status) { - case DL_DEV_DRIVER_BOUND: + case DL_DEV_PROBING: switch (consumer->links.status) { case DL_DEV_PROBING: /* - * Some callers expect the link creation during - * consumer driver probe to resume the supplier - * even without DL_FLAG_RPM_ACTIVE. + * A consumer driver can create a link to a + * supplier that has not completed its probing + * yet as long as it knows that the supplier is + * already functional (for example, it has just + * acquired some resources from the supplier). */ - if (flags & DL_FLAG_PM_RUNTIME) - pm_runtime_resume(supplier); - + link->status = DL_STATE_CONSUMER_PROBE; + break; + default: + link->status = DL_STATE_DORMANT; + break; + } + break; + case DL_DEV_DRIVER_BOUND: + switch (consumer->links.status) { + case DL_DEV_PROBING: link->status = DL_STATE_CONSUMER_PROBE; break; case DL_DEV_DRIVER_BOUND: @@ -330,6 +339,14 @@ struct device_link *device_link_add(struct device *consumer, } } + /* + * Some callers expect the link creation during consumer driver probe to + * resume the supplier even without DL_FLAG_RPM_ACTIVE. + */ + if (link->status == DL_STATE_CONSUMER_PROBE && + flags & DL_FLAG_PM_RUNTIME) + pm_runtime_resume(supplier); + /* * Move the consumer and all of the devices depending on it to the end * of dpm_list and the devices_kset list. @@ -518,6 +535,16 @@ void device_links_driver_bound(struct device *dev) if (link->flags & DL_FLAG_STATELESS) continue; + /* + * Links created during consumer probe may be in the "consumer + * probe" state to start with if the supplier is still probing + * when they are created and they may become "active" if the + * consumer probe returns first. Skip them here. + */ + if (link->status == DL_STATE_CONSUMER_PROBE || + link->status == DL_STATE_ACTIVE) + continue; + WARN_ON(link->status != DL_STATE_DORMANT); WRITE_ONCE(link->status, DL_STATE_AVAILABLE); } @@ -557,17 +584,48 @@ static void __device_links_no_driver(struct device *dev) if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) __device_link_del(&link->kref); - else if (link->status != DL_STATE_SUPPLIER_UNBIND) + else if (link->status == DL_STATE_CONSUMER_PROBE || + link->status == DL_STATE_ACTIVE) WRITE_ONCE(link->status, DL_STATE_AVAILABLE); } dev->links.status = DL_DEV_NO_DRIVER; } +/** + * device_links_no_driver - Update links after failing driver probe. + * @dev: Device whose driver has just failed to probe. + * + * Clean up leftover links to consumers for @dev and invoke + * %__device_links_no_driver() to update links to suppliers for it as + * appropriate. + * + * Links with the DL_FLAG_STATELESS flag set are ignored. + */ void device_links_no_driver(struct device *dev) { + struct device_link *link; + device_links_write_lock(); + + list_for_each_entry(link, &dev->links.consumers, s_node) { + if (link->flags & DL_FLAG_STATELESS) + continue; + + /* + * The probe has failed, so if the status of the link is + * "consumer probe" or "active", it must have been added by + * a probing consumer while this device was still probing. + * Change its state to "dormant", as it represents a valid + * relationship, but it is not functionally meaningful. + */ + if (link->status == DL_STATE_CONSUMER_PROBE || + link->status == DL_STATE_ACTIVE) + WRITE_ONCE(link->status, DL_STATE_DORMANT); + } + __device_links_no_driver(dev); + device_links_write_unlock(); } -- cgit v1.2.3 From a1fdbfbb1da2063ba98a12eb6f1bdd07451c7145 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:52:45 +0100 Subject: driver core: Do not call rpm_put_suppliers() in pm_runtime_drop_link() Calling rpm_put_suppliers() from pm_runtime_drop_link() is excessive as it affects all suppliers of the consumer device and not just the one pointed to by the device link being dropped. Worst case it may cause the consumer device to stop working unexpectedly. Moreover, in principle it is racy with respect to runtime PM of the consumer device. To avoid these problems drop runtime PM references on the particular supplier pointed to by the link in question only and do that after the link has been dropped from the consumer device's list of links to suppliers, which is in device_link_free(). Fixes: a0504aecba76 ("PM / runtime: Drop usage count for suppliers at device link removal") Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 3 +++ drivers/base/power/runtime.c | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index d58ea075f807..8c7327d45406 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -374,6 +374,9 @@ EXPORT_SYMBOL_GPL(device_link_add); static void device_link_free(struct device_link *link) { + while (refcount_dec_not_one(&link->rpm_active)) + pm_runtime_put(link->supplier); + put_device(link->consumer); put_device(link->supplier); kfree(link); diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 8bc9a432de70..fd5c4a7b96f0 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1611,8 +1611,6 @@ void pm_runtime_new_link(struct device *dev) void pm_runtime_drop_link(struct device *dev) { - rpm_put_suppliers(dev); - spin_lock_irq(&dev->power.lock); WARN_ON(dev->power.links_count == 0); dev->power.links_count--; -- cgit v1.2.3 From 72175d4ea4c442d95cf690c3e968eeee90fd43ca Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:58:33 +0100 Subject: driver core: Make driver core own stateful device links Even though stateful device links are managed by the driver core in principle, their creators are allowed and sometimes even expected to drop references to them via device_link_del() or device_link_remove(), but that doesn't really play well with the "persistent" link concept. If "persistent" managed device links are created from driver probe callbacks, device_link_add() called to do that will take a new reference on the link each time the callback runs and those references will never be dropped, which kind of isn't nice. This issues arises because of the link reference counting carried out by device_link_add() for existing links, but that is only done to avoid deleting device links that may still be necessary, which shouldn't be a concern for managed (stateful) links. These device links are managed by the driver core and whoever creates one of them will need it at least as long as until the consumer driver is detached from its device and deleting it may be left to the driver core just fine. For this reason, rework device_link_add() to apply the reference counting to stateless links only and make device_link_del() and device_link_remove() drop references to stateless links only too. After this change, if called to add a stateful device link for a consumer-supplier pair for which a stateful device link is present already, device_link_add() will return the existing link without incrementing its reference counter. Accordingly, device_link_del() and device_link_remove() will WARN() and do nothing when called to drop a reference to a stateful link. Thus, effectively, all stateful device links will be owned by the driver core. In addition, clean up the handling of the link management flags, DL_FLAG_AUTOREMOVE_CONSUMER and DL_FLAG_AUTOREMOVE_SUPPLIER, so that (a) they are never set at the same time and (b) if device_link_add() is called for a consumer-supplier pair with an existing stateful link between them, the flags of that link will be combined with the flags passed to device_link_add() to ensure that the life time of the link is sufficient for all of the callers of device_link_add() for the same consumer-supplier pair. Update the device_link_add() kerneldoc comment to reflect the above changes. Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 69 +++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 15 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index 8c7327d45406..9d49b461b1d9 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -192,10 +192,21 @@ static void device_link_rpm_prepare(struct device *consumer, * of the link. If DL_FLAG_PM_RUNTIME is not set, DL_FLAG_RPM_ACTIVE will be * ignored. * - * If the DL_FLAG_AUTOREMOVE_CONSUMER flag is set, the link will be removed - * automatically when the consumer device driver unbinds from it. Analogously, - * if DL_FLAG_AUTOREMOVE_SUPPLIER is set in @flags, the link will be removed - * automatically when the supplier device driver unbinds from it. + * If DL_FLAG_STATELESS is set in @flags, the link is not going to be managed by + * the driver core and, in particular, the caller of this function is expected + * to drop the reference to the link acquired by it directly. + * + * If that flag is not set, however, the caller of this function is handing the + * management of the link over to the driver core entirely and its return value + * can only be used to check whether or not the link is present. In that case, + * the DL_FLAG_AUTOREMOVE_CONSUMER and DL_FLAG_AUTOREMOVE_SUPPLIER device link + * flags can be used to indicate to the driver core when the link can be safely + * deleted. Namely, setting one of them in @flags indicates to the driver core + * that the link is not going to be used (by the given caller of this function) + * after unbinding the consumer or supplier driver, respectively, from its + * device, so the link can be deleted at that point. If none of them is set, + * the link will be maintained until one of the devices pointed to by it (either + * the consumer or the supplier) is unregistered. * * The combination of DL_FLAG_STATELESS and either DL_FLAG_AUTOREMOVE_CONSUMER * or DL_FLAG_AUTOREMOVE_SUPPLIER set in @flags at the same time is invalid and @@ -241,6 +252,14 @@ struct device_link *device_link_add(struct device *consumer, goto out; } + /* + * DL_FLAG_AUTOREMOVE_SUPPLIER indicates that the link will be needed + * longer than for DL_FLAG_AUTOREMOVE_CONSUMER and setting them both + * together doesn't make sense, so prefer DL_FLAG_AUTOREMOVE_SUPPLIER. + */ + if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) + flags &= ~DL_FLAG_AUTOREMOVE_CONSUMER; + list_for_each_entry(link, &supplier->links.consumers, s_node) { if (link->consumer != consumer) continue; @@ -254,12 +273,6 @@ struct device_link *device_link_add(struct device *consumer, goto out; } - if (flags & DL_FLAG_AUTOREMOVE_CONSUMER) - link->flags |= DL_FLAG_AUTOREMOVE_CONSUMER; - - if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) - link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER; - if (flags & DL_FLAG_PM_RUNTIME) { if (!(link->flags & DL_FLAG_PM_RUNTIME)) { device_link_rpm_prepare(consumer, supplier); @@ -269,7 +282,25 @@ struct device_link *device_link_add(struct device *consumer, refcount_inc(&link->rpm_active); } - kref_get(&link->kref); + if (flags & DL_FLAG_STATELESS) { + kref_get(&link->kref); + goto out; + } + + /* + * If the life time of the link following from the new flags is + * longer than indicated by the flags of the existing link, + * update the existing link to stay around longer. + */ + if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) { + if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) { + link->flags &= ~DL_FLAG_AUTOREMOVE_CONSUMER; + link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER; + } + } else if (!(flags & DL_FLAG_AUTOREMOVE_CONSUMER)) { + link->flags &= ~(DL_FLAG_AUTOREMOVE_CONSUMER | + DL_FLAG_AUTOREMOVE_SUPPLIER); + } goto out; } @@ -419,8 +450,16 @@ static void __device_link_del(struct kref *kref) } #endif /* !CONFIG_SRCU */ +static void device_link_put_kref(struct device_link *link) +{ + if (link->flags & DL_FLAG_STATELESS) + kref_put(&link->kref, __device_link_del); + else + WARN(1, "Unable to drop a managed device link reference\n"); +} + /** - * device_link_del - Delete a link between two devices. + * device_link_del - Delete a stateless link between two devices. * @link: Device link to delete. * * The caller must ensure proper synchronization of this function with runtime @@ -432,14 +471,14 @@ void device_link_del(struct device_link *link) { device_links_write_lock(); device_pm_lock(); - kref_put(&link->kref, __device_link_del); + device_link_put_kref(link); device_pm_unlock(); device_links_write_unlock(); } EXPORT_SYMBOL_GPL(device_link_del); /** - * device_link_remove - remove a link between two devices. + * device_link_remove - Delete a stateless link between two devices. * @consumer: Consumer end of the link. * @supplier: Supplier end of the link. * @@ -458,7 +497,7 @@ void device_link_remove(void *consumer, struct device *supplier) list_for_each_entry(link, &supplier->links.consumers, s_node) { if (link->consumer == consumer) { - kref_put(&link->kref, __device_link_del); + device_link_put_kref(link); break; } } -- cgit v1.2.3 From e7dd40105aac9ba051e44ad711123bc53a5e4c71 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 1 Feb 2019 01:59:42 +0100 Subject: driver core: Add device link flag DL_FLAG_AUTOPROBE_CONSUMER Add a new device link flag, DL_FLAG_AUTOPROBE_CONSUMER, to request the driver core to probe for a consumer driver automatically after binding a driver to the supplier device on a persistent managed device link. As unbinding the supplier driver on a managed device link causes the consumer driver to be detached from its device automatically, this flag provides a complementary mechanism which is needed to address some "composite device" use cases. Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 16 +++++++++++++++- drivers/base/dd.c | 2 +- 2 files changed, 16 insertions(+), 2 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index 9d49b461b1d9..abfce4f613f8 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -208,6 +208,12 @@ static void device_link_rpm_prepare(struct device *consumer, * the link will be maintained until one of the devices pointed to by it (either * the consumer or the supplier) is unregistered. * + * 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 + * driver after successfully binding a driver to the supplier device. + * * The combination of DL_FLAG_STATELESS and either DL_FLAG_AUTOREMOVE_CONSUMER * or DL_FLAG_AUTOREMOVE_SUPPLIER set in @flags at the same time is invalid and * will cause NULL to be returned upfront. @@ -228,7 +234,12 @@ struct device_link *device_link_add(struct device *consumer, if (!consumer || !supplier || (flags & DL_FLAG_STATELESS && - flags & (DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_AUTOREMOVE_SUPPLIER))) + flags & (DL_FLAG_AUTOREMOVE_CONSUMER | + DL_FLAG_AUTOREMOVE_SUPPLIER | + DL_FLAG_AUTOPROBE_CONSUMER)) || + (flags & DL_FLAG_AUTOPROBE_CONSUMER && + flags & (DL_FLAG_AUTOREMOVE_CONSUMER | + DL_FLAG_AUTOREMOVE_SUPPLIER))) return NULL; if (flags & DL_FLAG_PM_RUNTIME && flags & DL_FLAG_RPM_ACTIVE) { @@ -589,6 +600,9 @@ void device_links_driver_bound(struct device *dev) WARN_ON(link->status != DL_STATE_DORMANT); WRITE_ONCE(link->status, DL_STATE_AVAILABLE); + + if (link->flags & DL_FLAG_AUTOPROBE_CONSUMER) + driver_deferred_probe_add(link->consumer); } list_for_each_entry(link, &dev->links.suppliers, c_node) { diff --git a/drivers/base/dd.c b/drivers/base/dd.c index aa6a9c613595..2e898cbba79b 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -116,7 +116,7 @@ static void deferred_probe_work_func(struct work_struct *work) } static DECLARE_WORK(deferred_probe_work, deferred_probe_work_func); -static void driver_deferred_probe_add(struct device *dev) +void driver_deferred_probe_add(struct device *dev) { mutex_lock(&deferred_probe_mutex); if (list_empty(&dev->p->deferred_probe)) { -- cgit v1.2.3 From 4080ab083000a1e9656b0d1607e238e7001e0c84 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 7 Feb 2019 19:38:56 +0100 Subject: PM-runtime: Take suppliers into account in __pm_runtime_set_status() If the target device has any suppliers, as reflected by device links to them, __pm_runtime_set_status() does not take them into account, which is not consistent with the other parts of the PM-runtime framework and may lead to programming mistakes. Modify __pm_runtime_set_status() to take suppliers into account by activating them upfront if the new status is RPM_ACTIVE and deactivating them on exit if the new status is RPM_SUSPENDED. If the activation of one of the suppliers fails, the new status will be RPM_SUSPENDED and the (remaining) suppliers will be deactivated on exit (the child count of the device's parent will be dropped too then). Of course, adding device links locking to __pm_runtime_set_status() means that it cannot be run fron interrupt context, so make it use spin_lock_irq() and spin_unlock_irq() instead of spin_lock_irqsave() and spin_unlock_irqrestore(), respectively. Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/power/runtime.c | 45 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index fd5c4a7b96f0..a0e55065817b 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1089,20 +1089,43 @@ EXPORT_SYMBOL_GPL(pm_runtime_get_if_in_use); * and the device parent's counter of unsuspended children is modified to * reflect the new status. If the new status is RPM_SUSPENDED, an idle * notification request for the parent is submitted. + * + * If @dev has any suppliers (as reflected by device links to them), and @status + * is RPM_ACTIVE, they will be activated upfront and if the activation of one + * of them fails, the status of @dev will be changed to RPM_SUSPENDED (instead + * of the @status value) and the suppliers will be deacticated on exit. The + * error returned by the failing supplier activation will be returned in that + * case. */ int __pm_runtime_set_status(struct device *dev, unsigned int status) { struct device *parent = dev->parent; - unsigned long flags; bool notify_parent = false; int error = 0; if (status != RPM_ACTIVE && status != RPM_SUSPENDED) return -EINVAL; - spin_lock_irqsave(&dev->power.lock, flags); + /* + * If the new status is RPM_ACTIVE, the suppliers can be activated + * upfront regardless of the current status, because next time + * rpm_put_suppliers() runs, the rpm_active refcounts of the links + * involved will be dropped down to one anyway. + */ + if (status == RPM_ACTIVE) { + int idx = device_links_read_lock(); + + error = rpm_get_suppliers(dev); + if (error) + status = RPM_SUSPENDED; + + device_links_read_unlock(idx); + } + + spin_lock_irq(&dev->power.lock); if (!dev->power.runtime_error && !dev->power.disable_depth) { + status = dev->power.runtime_status; error = -EAGAIN; goto out; } @@ -1134,19 +1157,31 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) spin_unlock(&parent->power.lock); - if (error) + if (error) { + status = RPM_SUSPENDED; goto out; + } } out_set: __update_runtime_status(dev, status); - dev->power.runtime_error = 0; + if (!error) + dev->power.runtime_error = 0; + out: - spin_unlock_irqrestore(&dev->power.lock, flags); + spin_unlock_irq(&dev->power.lock); if (notify_parent) pm_request_idle(parent); + if (status == RPM_SUSPENDED) { + int idx = device_links_read_lock(); + + rpm_put_suppliers(dev); + + device_links_read_unlock(idx); + } + return error; } EXPORT_SYMBOL_GPL(__pm_runtime_set_status); -- cgit v1.2.3 From 376991db4b6464e906d699ef07681e2ffa8ab08c Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Thu, 7 Feb 2019 20:36:53 +0100 Subject: driver core: Postpone DMA tear-down until after devres release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When unbinding the (IOMMU-enabled) R-Car SATA device on Salvator-XS (R-Car H3 ES2.0), in preparation of rebinding against vfio-platform for device pass-through for virtualization:     echo ee300000.sata > /sys/bus/platform/drivers/sata_rcar/unbind the kernel crashes with:     Unable to handle kernel paging request at virtual address ffffffbf029ffffc     Mem abort info:       ESR = 0x96000006       Exception class = DABT (current EL), IL = 32 bits       SET = 0, FnV = 0       EA = 0, S1PTW = 0     Data abort info:       ISV = 0, ISS = 0x00000006       CM = 0, WnR = 0     swapper pgtable: 4k pages, 39-bit VAs, pgdp = 000000007e8c586c     [ffffffbf029ffffc] pgd=000000073bfc6003, pud=000000073bfc6003, pmd=0000000000000000     Internal error: Oops: 96000006 [#1] SMP     Modules linked in:     CPU: 0 PID: 1098 Comm: bash Not tainted 5.0.0-rc5-salvator-x-00452-g37596f884f4318ef #287     Hardware name: Renesas Salvator-X 2nd version board based on r8a7795 ES2.0+ (DT)     pstate: 60400005 (nZCv daif +PAN -UAO)     pc : __free_pages+0x8/0x58     lr : __dma_direct_free_pages+0x50/0x5c     sp : ffffff801268baa0     x29: ffffff801268baa0 x28: 0000000000000000     x27: ffffffc6f9c60bf0 x26: ffffffc6f9c60bf0     x25: ffffffc6f9c60810 x24: 0000000000000000     x23: 00000000fffff000 x22: ffffff8012145000     x21: 0000000000000800 x20: ffffffbf029fffc8     x19: 0000000000000000 x18: ffffffc6f86c42c8     x17: 0000000000000000 x16: 0000000000000070     x15: 0000000000000003 x14: 0000000000000000     x13: ffffff801103d7f8 x12: 0000000000000028     x11: ffffff8011117604 x10: 0000000000009ad8     x9 : ffffff80110126d0 x8 : ffffffc6f7563000     x7 : 6b6b6b6b6b6b6b6b x6 : 0000000000000018     x5 : ffffff8011cf3cc8 x4 : 0000000000004000     x3 : 0000000000080000 x2 : 0000000000000001     x1 : 0000000000000000 x0 : ffffffbf029fffc8     Process bash (pid: 1098, stack limit = 0x00000000c38e3e32)     Call trace:      __free_pages+0x8/0x58      __dma_direct_free_pages+0x50/0x5c      arch_dma_free+0x1c/0x98      dma_direct_free+0x14/0x24      dma_free_attrs+0x9c/0xdc      dmam_release+0x18/0x20      release_nodes+0x25c/0x28c      devres_release_all+0x48/0x4c      device_release_driver_internal+0x184/0x1f0      device_release_driver+0x14/0x1c      unbind_store+0x70/0xb8      drv_attr_store+0x24/0x34      sysfs_kf_write+0x4c/0x64      kernfs_fop_write+0x154/0x1c4      __vfs_write+0x34/0x164      vfs_write+0xb4/0x16c      ksys_write+0x5c/0xbc      __arm64_sys_write+0x14/0x1c      el0_svc_common+0x98/0x114      el0_svc_handler+0x1c/0x24      el0_svc+0x8/0xc     Code: d51b4234 17fffffa a9bf7bfd 910003fd (b9403404)     ---[ end trace 8c564cdd3a1a840f ]--- While I've bisected this to commit e8e683ae9a736407 ("iommu/of: Fix probe-deferral"), and reverting that commit on post-v5.0-rc4 kernels does fix the problem, this turned out to be a red herring. On arm64, arch_teardown_dma_ops() resets dev->dma_ops to NULL. Hence if a driver has used a managed DMA allocation API, the allocated DMA memory will be freed using the direct DMA ops, while it may have been allocated using a custom DMA ops (iommu_dma_ops in this case). Fix this by reversing the order of the calls to devres_release_all() and arch_teardown_dma_ops(). Signed-off-by: Geert Uytterhoeven Acked-by: Christoph Hellwig Reviewed-by: Rafael J. Wysocki Cc: stable Reviewed-by: Robin Murphy Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 2e898cbba79b..348fc4695d4d 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -1058,9 +1058,9 @@ static void __device_release_driver(struct device *dev, struct device *parent) drv->remove(dev); device_links_driver_cleanup(dev); - arch_teardown_dma_ops(dev); devres_release_all(dev); + arch_teardown_dma_ops(dev); dev->driver = NULL; dev_set_drvdata(dev, NULL); if (dev->pm_domain && dev->pm_domain->dismiss) -- cgit v1.2.3 From daaef255dc96834aaaad627d3271504cba3ac2dc Mon Sep 17 00:00:00 2001 From: Enrico Granata Date: Mon, 11 Feb 2019 11:01:12 -0800 Subject: driver: platform: Support parsing GpioInt 0 in platform_get_irq() ACPI 5 added support for GpioInt resources as a way to provide information about interrupts mediated via a GPIO controller. Several device buses (e.g. SPI, I2C) have support for retrieving an IRQ specified via this type of resource, and providing it directly to the driver as an IRQ number. This is not currently done for the platform drivers, as platform_get_irq() does not try to parse GpioInt() resources. This requires drivers to either have to support only one possible IRQ resource, or to have code in place to try both as a failsafe. While there is a possibility of ambiguity for devices that exposes multiple IRQs, it is easy and feasible to support the common case of devices that only expose one IRQ which would be of either type depending on the underlying system's architecture. This commit adds support for parsing a GpioInt resource in order to fulfill a request for the index 0 IRQ for a platform device. Signed-off-by: Enrico Granata Reviewed-by: Dmitry Torokhov Acked-by: Hans de Goede Reviewed-by: Mika Westerberg Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 1c958eb33ef4..0d3611cd1b3b 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -127,7 +127,20 @@ int platform_get_irq(struct platform_device *dev, unsigned int num) irqd_set_trigger_type(irqd, r->flags & IORESOURCE_BITS); } - return r ? r->start : -ENXIO; + if (r) + return r->start; + + /* + * For the index 0 interrupt, allow falling back to GpioInt + * resources. While a device could have both Interrupt and GpioInt + * resources, making this fallback ambiguous, in many common cases + * the device will only expose one IRQ, and this fallback + * allows a common code path across either kind of resource. + */ + if (num == 0 && has_acpi_companion(&dev->dev)) + return acpi_dev_gpio_irq_get(ACPI_COMPANION(&dev->dev), num); + + return -ENXIO; #endif } EXPORT_SYMBOL_GPL(platform_get_irq); -- cgit v1.2.3 From c1567f813a9992a64f8d0f6cfb912c3922812c35 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 12 Feb 2019 13:04:12 +0100 Subject: PM-runtime: Fix __pm_runtime_set_status() race with runtime resume Commit 4080ab083000 ("PM-runtime: Take suppliers into account in __pm_runtime_set_status()") introduced a race condition that may trigger if __pm_runtime_set_status() is used incorrectly (that is, if it is called when PM-runtime is enabled for the target device and working). In that case, if the original PM-runtime status of the device is RPM_SUSPENDED, a runtime resume of the device may occur after __pm_runtime_set_status() has dropped its power.lock spinlock and before deactivating its suppliers, so the suppliers may be deactivated while the device is PM-runtime-active which may lead to functional issues. To avoid that, modify __pm_runtime_set_status() to check whether or not PM-runtime is enabled for the device before activating its suppliers (if the new status is RPM_ACTIVE) and either return an error if that's the case or increment the device's disable_depth counter to prevent PM-runtime from being enabled for it while the remaining part of the function is running (disable_depth is then decremented on the way out). Fixes: 4080ab083000 ("PM-runtime: Take suppliers into account in __pm_runtime_set_status()") Signed-off-by: Rafael J. Wysocki Reviewed-by: Ulf Hansson Signed-off-by: Greg Kroah-Hartman --- drivers/base/power/runtime.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index ac54f65b6d63..af23eb327f57 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1106,6 +1106,22 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) if (status != RPM_ACTIVE && status != RPM_SUSPENDED) return -EINVAL; + spin_lock_irq(&dev->power.lock); + + /* + * Prevent PM-runtime from being enabled for the device or return an + * error if it is enabled already and working. + */ + if (dev->power.runtime_error || dev->power.disable_depth) + dev->power.disable_depth++; + else + error = -EAGAIN; + + spin_unlock_irq(&dev->power.lock); + + if (error) + return error; + /* * If the new status is RPM_ACTIVE, the suppliers can be activated * upfront regardless of the current status, because next time @@ -1124,12 +1140,6 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) spin_lock_irq(&dev->power.lock); - if (!dev->power.runtime_error && !dev->power.disable_depth) { - status = dev->power.runtime_status; - error = -EAGAIN; - goto out; - } - if (dev->power.runtime_status == status || !parent) goto out_set; @@ -1182,6 +1192,8 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) device_links_read_unlock(idx); } + pm_runtime_enable(dev); + return error; } EXPORT_SYMBOL_GPL(__pm_runtime_set_status); -- cgit v1.2.3 From 4c06c4e6cf63d7f3d5dfe62593a073253d750a59 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 12 Feb 2019 13:08:10 +0100 Subject: driver core: Fix possible supplier PM-usage counter imbalance If a stateless device link to a certain supplier with DL_FLAG_PM_RUNTIME set in the flags is added and then removed by the consumer driver's probe callback, the supplier's PM-runtime usage counter will be nonzero after that which effectively causes the supplier to remain "always on" going forward. Namely, device_link_add() called to add the link invokes device_link_rpm_prepare() which notices that the consumer driver is probing, so it increments the supplier's PM-runtime usage counter with the assumption that the link will stay around until pm_runtime_put_suppliers() is called by driver_probe_device(), but if the link goes away before that point, the supplier's PM-runtime usage counter will remain nonzero. To prevent that from happening, first rework pm_runtime_get_suppliers() and pm_runtime_put_suppliers() to use the rpm_active refounts of device links and make the latter only drop rpm_active and the supplier's PM-runtime usage counter for each link by one, unless rpm_active is one already for it. Next, modify device_link_add() to bump up the new link's rpm_active refcount and the suppliers PM-runtime usage counter by two, to prevent pm_runtime_put_suppliers(), if it is called subsequently, from suspending the supplier prematurely (in case its PM-runtime usage counter goes down to 0 in there). Due to the way rpm_put_suppliers() works, this change does not affect runtime suspend of the consumer ends of new device links (or, generally, device links for which DL_FLAG_PM_RUNTIME has just been set). Fixes: e2f3cd831a28 ("driver core: Fix handling of runtime PM flags in device_link_add()") Reported-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki Reviewed-by: Ulf Hansson Tested-by: Ulf Hansson Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 21 ++++----------------- drivers/base/power/runtime.c | 27 +++++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 19 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index abfce4f613f8..787190238753 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -165,19 +165,6 @@ void device_pm_move_to_tail(struct device *dev) device_links_read_unlock(idx); } -static void device_link_rpm_prepare(struct device *consumer, - struct device *supplier) -{ - pm_runtime_new_link(consumer); - /* - * If the link is being added by the consumer driver at probe time, - * balance the decrementation of the supplier's runtime PM usage counter - * after consumer probe in driver_probe_device(). - */ - if (consumer->links.status == DL_DEV_PROBING) - pm_runtime_get_noresume(supplier); -} - /** * device_link_add - Create a link between two devices. * @consumer: Consumer end of the link. @@ -286,11 +273,11 @@ struct device_link *device_link_add(struct device *consumer, if (flags & DL_FLAG_PM_RUNTIME) { if (!(link->flags & DL_FLAG_PM_RUNTIME)) { - device_link_rpm_prepare(consumer, supplier); + pm_runtime_new_link(consumer); link->flags |= DL_FLAG_PM_RUNTIME; } if (flags & DL_FLAG_RPM_ACTIVE) - refcount_inc(&link->rpm_active); + pm_runtime_active_link(link, supplier); } if (flags & DL_FLAG_STATELESS) { @@ -323,9 +310,9 @@ struct device_link *device_link_add(struct device *consumer, if (flags & DL_FLAG_PM_RUNTIME) { if (flags & DL_FLAG_RPM_ACTIVE) - refcount_inc(&link->rpm_active); + pm_runtime_active_link(link, supplier); - device_link_rpm_prepare(consumer, supplier); + pm_runtime_new_link(consumer); } get_device(supplier); diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index af23eb327f57..6b8aa6bed064 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1625,8 +1625,10 @@ void pm_runtime_get_suppliers(struct device *dev) idx = device_links_read_lock(); list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) - if (link->flags & DL_FLAG_PM_RUNTIME) + if (link->flags & DL_FLAG_PM_RUNTIME) { + refcount_inc(&link->rpm_active); pm_runtime_get_sync(link->supplier); + } device_links_read_unlock(idx); } @@ -1643,7 +1645,8 @@ void pm_runtime_put_suppliers(struct device *dev) idx = device_links_read_lock(); list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) - if (link->flags & DL_FLAG_PM_RUNTIME) + if (link->flags & DL_FLAG_PM_RUNTIME && + refcount_dec_not_one(&link->rpm_active)) pm_runtime_put(link->supplier); device_links_read_unlock(idx); @@ -1656,6 +1659,26 @@ void pm_runtime_new_link(struct device *dev) spin_unlock_irq(&dev->power.lock); } +/** + * pm_runtime_active_link - Set up new device link as active for PM-runtime. + * @link: Device link to be set up as active. + * @supplier: Supplier end of the link. + * + * Add 2 to the rpm_active refcount of @link and increment the PM-runtime + * usage counter of @supplier once more in case the link is being added while + * the consumer driver is probing and pm_runtime_put_suppliers() will be called + * subsequently. + * + * Note that this doesn't prevent rpm_put_suppliers() from decreasing the link's + * rpm_active refcount down to one, so runtime suspend of the consumer end of + * @link is not affected. + */ +void pm_runtime_active_link(struct device_link *link, struct device *supplier) +{ + refcount_add(2, &link->rpm_active); + pm_runtime_get_noresume(supplier); +} + void pm_runtime_drop_link(struct device *dev) { spin_lock_irq(&dev->power.lock); -- cgit v1.2.3 From 1ea61b68d0f8685775c897c2de040c73b8d1c56a Mon Sep 17 00:00:00 2001 From: Feng Tang Date: Wed, 13 Feb 2019 15:47:36 +0800 Subject: async: Add cmdline option to specify drivers to be async probed Asynchronous driver probing can help much on kernel fastboot, and this option can provide a flexible way to optimize and quickly verify async driver probe. Also it will help in below cases: * Some driver actually covers several families of HWs, some of which could use async probing while others don't. So we can't simply turn on the PROBE_PREFER_ASYNCHRONOUS flag in driver, but use this cmdline option, like igb driver async patch discussed at https://www.spinics.net/lists/netdev/msg545986.html * For SOC (System on Chip) with multiple spi or i2c controllers, most of the slave spi/i2c devices will be assigned with fixed controller number, while async probing may make those controllers get different index for each boot, which prevents those controller drivers to be async probed. For platforms not using these spi/i2c slave devices, they can use this cmdline option to benefit from the async probing. Suggested-by: Alexander Duyck Cc: Randy Dunlap Signed-off-by: Feng Tang Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'drivers/base') diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 348fc4695d4d..a823f469e53f 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -57,6 +57,10 @@ static atomic_t deferred_trigger_count = ATOMIC_INIT(0); static struct dentry *deferred_devices; static bool initcalls_done; +/* Save the async probe drivers' name from kernel cmdline */ +#define ASYNC_DRV_NAMES_MAX_LEN 256 +static char async_probe_drv_names[ASYNC_DRV_NAMES_MAX_LEN]; + /* * In some cases, like suspend to RAM or hibernation, It might be reasonable * to prohibit probing of devices as it could be unsafe. @@ -674,6 +678,23 @@ int driver_probe_device(struct device_driver *drv, struct device *dev) return ret; } +static inline bool cmdline_requested_async_probing(const char *drv_name) +{ + return parse_option_str(async_probe_drv_names, drv_name); +} + +/* The option format is "driver_async_probe=drv_name1,drv_name2,..." */ +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"); + + strlcpy(async_probe_drv_names, buf, ASYNC_DRV_NAMES_MAX_LEN); + return 0; +} +__setup("driver_async_probe=", save_async_options); + bool driver_allows_async_probing(struct device_driver *drv) { switch (drv->probe_type) { @@ -684,6 +705,9 @@ bool driver_allows_async_probing(struct device_driver *drv) return false; default: + if (cmdline_requested_async_probing(drv->name)) + return true; + if (module_requested_async_probing(drv->owner)) return true; -- cgit v1.2.3 From e4246b05507fc6102008bac0aee848f207bd96de Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 18 Feb 2019 17:36:48 +0100 Subject: drivers/component: kerneldoc polish Polish the kerneldoc a bit with suggestions from Randy. v2: Randy found another typo: s/compent/component/ Signed-off-by: Daniel Vetter Cc: "Rafael J. Wysocki" Cc: Daniel Vetter Cc: Ramalingam C Acked-by: Randy Dunlap Signed-off-by: Greg Kroah-Hartman --- drivers/base/component.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/component.c b/drivers/base/component.c index 7dbc41cccd58..532a3a5d8f63 100644 --- a/drivers/base/component.c +++ b/drivers/base/component.c @@ -27,7 +27,7 @@ * helper fills the niche of aggregate drivers for specific hardware, where * further standardization into a subsystem would not be practical. The common * example is when a logical device (e.g. a DRM display driver) is spread around - * the SoC on various component (scanout engines, blending blocks, transcoders + * the SoC on various components (scanout engines, blending blocks, transcoders * for various outputs and so on). * * The component helper also doesn't solve runtime dependencies, e.g. for system @@ -378,7 +378,7 @@ static void __component_match_add(struct device *master, } /** - * component_match_add_release - add a component match with release callback + * component_match_add_release - add a component match entry with release callback * @master: device with the aggregate driver * @matchptr: pointer to the list of component matches * @release: release function for @compare_data @@ -408,7 +408,7 @@ void component_match_add_release(struct device *master, EXPORT_SYMBOL(component_match_add_release); /** - * component_match_add_typed - add a compent match for a typed component + * component_match_add_typed - add a component match entry for a typed component * @master: device with the aggregate driver * @matchptr: pointer to the list of component matches * @compare_typed: compare function to match against all typed components @@ -537,11 +537,11 @@ static void component_unbind(struct component *component, } /** - * component_unbind_all - unbind all component to an aggregate driver + * component_unbind_all - unbind all components of an aggregate driver * @master_dev: device with the aggregate driver * @data: opaque pointer, passed to all components * - * Unbinds all components to the aggregate @dev by passing @data to their + * Unbinds all components of the aggregate @dev by passing @data to their * &component_ops.unbind functions. Should be called from * &component_master_ops.unbind. */ @@ -619,11 +619,11 @@ static int component_bind(struct component *component, struct master *master, } /** - * component_bind_all - bind all component to an aggregate driver + * component_bind_all - bind all components of an aggregate driver * @master_dev: device with the aggregate driver * @data: opaque pointer, passed to all components * - * Binds all components to the aggregate @dev by passing @data to their + * Binds all components of the aggregate @dev by passing @data to their * &component_ops.bind functions. Should be called from * &component_master_ops.bind. */ -- cgit v1.2.3 From 36003d4cf57ca431fb3f94d317bcca426a2394d6 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 19 Feb 2019 17:53:26 +0100 Subject: driver core: Fix PM-runtime for links added during consumer probe Commit 4c06c4e6cf63 ("driver core: Fix possible supplier PM-usage counter imbalance") introduced a regression that causes suppliers to be suspended prematurely for device links added during consumer driver probe if the initial PM-runtime status of the consumer is "suspended" and the consumer is resumed after adding the link and before pm_runtime_put_suppliers() is called. In that case, pm_runtime_put_suppliers() will drop the rpm_active refcount for the link by one and (since rpm_active is equal to two after the preceding consumer resume) the supplier's PM-runtime usage counter will be decremented, which may cause the supplier to suspend even though the consumer's PM-runtime status is "active". For this reason, partially revert commit 4c06c4e6cf63 as the problem it tried to fix needs to be addressed somewhat differently, and change pm_runtime_get_suppliers() and pm_runtime_put_suppliers() so that the latter only drops rpm_active references acquired by the former. [This requires adding a new field to struct device_link, but I coulnd't find a cleaner way to address the issue that would work in all cases.] This causes pm_runtime_put_suppliers() to effectively ignore device links added during consumer probe, so device_link_add() doesn't need to worry about ensuring that suppliers will remain active after pm_runtime_put_suppliers() for links created with DL_FLAG_RPM_ACTIVE set and it only needs to bump up rpm_active by one for those links, so pm_runtime_active_link() is not necessary any more. Fixes: 4c06c4e6cf63 ("driver core: Fix possible supplier PM-usage counter imbalance") Reported-by: Jon Hunter Tested-by: Jon Hunter Tested-by: Ulf Hansson Reviewed-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki Tested-by: Thierry Reding Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 4 ++-- drivers/base/power/runtime.c | 29 ++++++----------------------- 2 files changed, 8 insertions(+), 25 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index 787190238753..4aeaa0c92bda 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -277,7 +277,7 @@ struct device_link *device_link_add(struct device *consumer, link->flags |= DL_FLAG_PM_RUNTIME; } if (flags & DL_FLAG_RPM_ACTIVE) - pm_runtime_active_link(link, supplier); + refcount_inc(&link->rpm_active); } if (flags & DL_FLAG_STATELESS) { @@ -310,7 +310,7 @@ struct device_link *device_link_add(struct device *consumer, if (flags & DL_FLAG_PM_RUNTIME) { if (flags & DL_FLAG_RPM_ACTIVE) - pm_runtime_active_link(link, supplier); + refcount_inc(&link->rpm_active); pm_runtime_new_link(consumer); } diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 6b8aa6bed064..70d2cb188601 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1626,6 +1626,7 @@ void pm_runtime_get_suppliers(struct device *dev) list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) if (link->flags & DL_FLAG_PM_RUNTIME) { + link->supplier_preactivated = true; refcount_inc(&link->rpm_active); pm_runtime_get_sync(link->supplier); } @@ -1645,9 +1646,11 @@ void pm_runtime_put_suppliers(struct device *dev) idx = device_links_read_lock(); list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) - if (link->flags & DL_FLAG_PM_RUNTIME && - refcount_dec_not_one(&link->rpm_active)) - pm_runtime_put(link->supplier); + if (link->supplier_preactivated) { + link->supplier_preactivated = false; + if (refcount_dec_not_one(&link->rpm_active)) + pm_runtime_put(link->supplier); + } device_links_read_unlock(idx); } @@ -1659,26 +1662,6 @@ void pm_runtime_new_link(struct device *dev) spin_unlock_irq(&dev->power.lock); } -/** - * pm_runtime_active_link - Set up new device link as active for PM-runtime. - * @link: Device link to be set up as active. - * @supplier: Supplier end of the link. - * - * Add 2 to the rpm_active refcount of @link and increment the PM-runtime - * usage counter of @supplier once more in case the link is being added while - * the consumer driver is probing and pm_runtime_put_suppliers() will be called - * subsequently. - * - * Note that this doesn't prevent rpm_put_suppliers() from decreasing the link's - * rpm_active refcount down to one, so runtime suspend of the consumer end of - * @link is not affected. - */ -void pm_runtime_active_link(struct device_link *link, struct device *supplier) -{ - refcount_add(2, &link->rpm_active); - pm_runtime_get_noresume(supplier); -} - void pm_runtime_drop_link(struct device *dev) { spin_lock_irq(&dev->power.lock); -- cgit v1.2.3 From eac473bce4b73a089880fb164b8badef81f57fbc Mon Sep 17 00:00:00 2001 From: John Zhao Date: Wed, 20 Feb 2019 10:39:04 +0800 Subject: firmware: hardcode the debug message for -ENOENT When no file /path was found, the error code of -ENOENT enumerated in errno-base.h, is returned. Stating clearly that the file was not found is much more useful for debugging, So let's be explicit about that. Signed-off-by: John Zhao Acked-by: Luis Chamberlain Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/main.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 8e9213b36e31..7eaaf5ee5ba6 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -328,12 +328,12 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv) rc = kernel_read_file_from_path(path, &fw_priv->data, &size, msize, id); if (rc) { - if (rc == -ENOENT) - dev_dbg(device, "loading %s failed with error %d\n", - path, rc); - else + if (rc != -ENOENT) dev_warn(device, "loading %s failed with error %d\n", path, rc); + else + dev_dbg(device, "loading %s failed for no such file or directory.\n", + path); continue; } dev_dbg(device, "direct-loading %s\n", fw_priv->fw_name); -- cgit v1.2.3 From 2c1ea6abde8884208a9b94254740ae4597c62000 Mon Sep 17 00:00:00 2001 From: Mans Rullgard Date: Thu, 21 Feb 2019 11:29:35 +0000 Subject: platform: set of_node in platform_device_register_full() If the provided fwnode is an OF node, set dev.of_node as well. Also add an of_node_reused flag to struct platform_device_info and copy this to the new device. This is needed to avoid pinctrl settings being requested twice. See 4e75e1d7dac9 ("driver core: add helper to reuse a device-tree node") for a longer explanation. Some drivers are just shims that create extra "glue" devices with the DT device as parent and have the real driver bind to these. In these cases, the glue device needs to get a reference to the original DT node in order for the main driver to access properties and child nodes. For example, the sunxi-musb driver creates such a glue device using platform_device_register_full(). Consequently, devices attached to this USB interface don't get associated with DT nodes, if present, the way they do with EHCI. This change will allow sunxi-musb and similar drivers to easily propagate the DT node to child devices as required. Signed-off-by: Mans Rullgard Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/base') diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 0d3611cd1b3b..fc67a325beaa 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -525,6 +525,8 @@ struct platform_device *platform_device_register_full( pdev->dev.parent = pdevinfo->parent; pdev->dev.fwnode = pdevinfo->fwnode; + pdev->dev.of_node = of_node_get(to_of_node(pdev->dev.fwnode)); + pdev->dev.of_node_reused = pdevinfo->of_node_reused; if (pdevinfo->dma_mask) { /* -- cgit v1.2.3 From 36cf3b1363f464c40f6ce647d3ac0ae9617d5fbc Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Fri, 1 Mar 2019 13:24:47 +0100 Subject: driver core: platform: remove misleading err_alloc label In platform_device_register_full() the err_alloc label is misleading, we only ever jump to it if the pdev is NULL, but it then proceeds to free it, which is a no-op. Remove the label and simply exit the function immediately. Signed-off-by: Johannes Berg Reviewed-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/platform.c b/drivers/base/platform.c index fc67a325beaa..4e45ac21d672 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -521,7 +521,7 @@ struct platform_device *platform_device_register_full( pdev = platform_device_alloc(pdevinfo->name, pdevinfo->id); if (!pdev) - goto err_alloc; + return ERR_PTR(-ENOMEM); pdev->dev.parent = pdevinfo->parent; pdev->dev.fwnode = pdevinfo->fwnode; @@ -568,8 +568,6 @@ struct platform_device *platform_device_register_full( err: ACPI_COMPANION_SET(&pdev->dev, NULL); kfree(pdev->dev.dma_mask); - -err_alloc: platform_device_put(pdev); return ERR_PTR(ret); } -- cgit v1.2.3