From c2a737eb2ea5682ffe63bc08003965496d6dc088 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 29 Jun 2017 09:39:02 +0530 Subject: [PATCH 01/28] debugfs: Add dummy implementation of few helpers This adds (missing) dummy implementations of debugfs_create_file_unsafe() and debugfs_create_ulong(). Signed-off-by: Viresh Kumar Signed-off-by: Greg Kroah-Hartman --- include/linux/debugfs.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index aa86e6d8c1aa..b93efc8feecd 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -196,6 +196,14 @@ static inline struct dentry *debugfs_create_file(const char *name, umode_t mode, return ERR_PTR(-ENODEV); } +static inline struct dentry *debugfs_create_file_unsafe(const char *name, + umode_t mode, struct dentry *parent, + void *data, + const struct file_operations *fops) +{ + return ERR_PTR(-ENODEV); +} + static inline struct dentry *debugfs_create_file_size(const char *name, umode_t mode, struct dentry *parent, void *data, const struct file_operations *fops, @@ -289,6 +297,14 @@ static inline struct dentry *debugfs_create_u64(const char *name, umode_t mode, return ERR_PTR(-ENODEV); } +static inline struct dentry *debugfs_create_ulong(const char *name, + umode_t mode, + struct dentry *parent, + unsigned long *value) +{ + return ERR_PTR(-ENODEV); +} + static inline struct dentry *debugfs_create_x8(const char *name, umode_t mode, struct dentry *parent, u8 *value) From 3eeba1a28e0df150adec37d67b567de653cf285c Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Fri, 23 Jun 2017 14:55:30 +0530 Subject: [PATCH 02/28] arch_topology: Don't break lines unnecessarily There is no need of line break at few places, avoid them. Signed-off-by: Viresh Kumar Reviewed-by: Juri Lelli Tested-by: Juri Lelli Signed-off-by: Greg Kroah-Hartman --- drivers/base/arch_topology.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index d1c33a85059e..0ad79b5cd56d 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -41,8 +41,7 @@ static ssize_t cpu_capacity_show(struct device *dev, { struct cpu *cpu = container_of(dev, struct cpu, dev); - return sprintf(buf, "%lu\n", - topology_get_cpu_scale(NULL, cpu->dev.id)); + return sprintf(buf, "%lu\n", topology_get_cpu_scale(NULL, cpu->dev.id)); } static ssize_t cpu_capacity_store(struct device *dev, @@ -128,8 +127,7 @@ int __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) if (cap_parsing_failed) return !ret; - ret = of_property_read_u32(cpu_node, - "capacity-dmips-mhz", + ret = of_property_read_u32(cpu_node, "capacity-dmips-mhz", &cpu_capacity); if (!ret) { if (!raw_capacity) { @@ -181,8 +179,7 @@ init_cpu_capacity_callback(struct notifier_block *nb, pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n", cpumask_pr_args(policy->related_cpus), cpumask_pr_args(cpus_to_visit)); - cpumask_andnot(cpus_to_visit, - cpus_to_visit, + cpumask_andnot(cpus_to_visit, cpus_to_visit, policy->related_cpus); for_each_cpu(cpu, policy->related_cpus) { raw_capacity[cpu] = topology_get_cpu_scale(NULL, cpu) * From 93a57081d20c1f93c209fec0f247f5bed936cc34 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Fri, 23 Jun 2017 14:55:31 +0530 Subject: [PATCH 03/28] arch_topology: Convert switch block to if block We only need to take care of one case here (CPUFREQ_NOTIFY) and there is no need to add an extra level of indentation to the case specific code by using a switch block. Use an if block instead. Also add some blank lines to make the code look better. Signed-off-by: Viresh Kumar Reviewed-by: Juri Lelli Tested-by: Juri Lelli Signed-off-by: Greg Kroah-Hartman --- drivers/base/arch_topology.c | 41 +++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index 0ad79b5cd56d..a3cd7c869c3e 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -174,26 +174,29 @@ init_cpu_capacity_callback(struct notifier_block *nb, if (cap_parsing_failed || cap_parsing_done) return 0; - switch (val) { - case CPUFREQ_NOTIFY: - pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n", - cpumask_pr_args(policy->related_cpus), - cpumask_pr_args(cpus_to_visit)); - cpumask_andnot(cpus_to_visit, cpus_to_visit, - policy->related_cpus); - for_each_cpu(cpu, policy->related_cpus) { - raw_capacity[cpu] = topology_get_cpu_scale(NULL, cpu) * - policy->cpuinfo.max_freq / 1000UL; - capacity_scale = max(raw_capacity[cpu], capacity_scale); - } - if (cpumask_empty(cpus_to_visit)) { - topology_normalize_cpu_scale(); - kfree(raw_capacity); - pr_debug("cpu_capacity: parsing done\n"); - cap_parsing_done = true; - schedule_work(&parsing_done_work); - } + if (val != CPUFREQ_NOTIFY) + return 0; + + pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n", + cpumask_pr_args(policy->related_cpus), + cpumask_pr_args(cpus_to_visit)); + + cpumask_andnot(cpus_to_visit, cpus_to_visit, policy->related_cpus); + + for_each_cpu(cpu, policy->related_cpus) { + raw_capacity[cpu] = topology_get_cpu_scale(NULL, cpu) * + policy->cpuinfo.max_freq / 1000UL; + capacity_scale = max(raw_capacity[cpu], capacity_scale); } + + if (cpumask_empty(cpus_to_visit)) { + topology_normalize_cpu_scale(); + kfree(raw_capacity); + pr_debug("cpu_capacity: parsing done\n"); + cap_parsing_done = true; + schedule_work(&parsing_done_work); + } + return 0; } From 805df2966f67a6b1a228c8e580e230b6c849b41e Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Fri, 23 Jun 2017 14:55:32 +0530 Subject: [PATCH 04/28] arch_topology: Change return type of topology_parse_cpu_capacity() to bool topology_parse_cpu_capacity() returns 1 on success and 0 on errors. Make it return bool instead of int as that suits the purpose better. Signed-off-by: Viresh Kumar Reviewed-by: Juri Lelli Tested-by: Juri Lelli Signed-off-by: Greg Kroah-Hartman --- drivers/base/arch_topology.c | 8 ++++---- include/linux/arch_topology.h | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index a3cd7c869c3e..5728e2fbb765 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -119,13 +119,13 @@ void topology_normalize_cpu_scale(void) mutex_unlock(&cpu_scale_mutex); } -int __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) +bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) { - int ret = 1; + int ret; u32 cpu_capacity; if (cap_parsing_failed) - return !ret; + return false; ret = of_property_read_u32(cpu_node, "capacity-dmips-mhz", &cpu_capacity); @@ -137,7 +137,7 @@ int __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) if (!raw_capacity) { pr_err("cpu_capacity: failed to allocate memory for raw capacities\n"); cap_parsing_failed = true; - return 0; + return false; } } capacity_scale = max(cpu_capacity, capacity_scale); diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h index 9af3c174c03a..716ce587247e 100644 --- a/include/linux/arch_topology.h +++ b/include/linux/arch_topology.h @@ -4,10 +4,12 @@ #ifndef _LINUX_ARCH_TOPOLOGY_H_ #define _LINUX_ARCH_TOPOLOGY_H_ +#include + void topology_normalize_cpu_scale(void); struct device_node; -int topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu); +bool topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu); struct sched_domain; unsigned long topology_get_cpu_scale(struct sched_domain *sd, int cpu); From 62de1161e220bc6ded7806ef0d149560f06152b3 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Fri, 23 Jun 2017 14:55:33 +0530 Subject: [PATCH 05/28] arch_topology: Localize cap_parsing_failed to topology_parse_cpu_capacity() cap_parsing_failed is only required in topology_parse_cpu_capacity() to know if we have already tried to allocate raw_capacity and failed, or if at least one of the cpu_node didn't had the required "capacity-dmips-mhz" property. All other users can use raw_capacity instead of cap_parsing_failed. Make sure we set raw_capacity to NULL after we free it. Signed-off-by: Viresh Kumar Reviewed-by: Juri Lelli Tested-by: Juri Lelli Signed-off-by: Greg Kroah-Hartman --- drivers/base/arch_topology.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index 5728e2fbb765..9e4d2107f4fa 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -95,14 +95,21 @@ subsys_initcall(register_cpu_capacity_sysctl); static u32 capacity_scale; static u32 *raw_capacity; -static bool cap_parsing_failed; + +static int __init free_raw_capacity(void) +{ + kfree(raw_capacity); + raw_capacity = NULL; + + return 0; +} void topology_normalize_cpu_scale(void) { u64 capacity; int cpu; - if (!raw_capacity || cap_parsing_failed) + if (!raw_capacity) return; pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale); @@ -121,6 +128,7 @@ void topology_normalize_cpu_scale(void) bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) { + static bool cap_parsing_failed; int ret; u32 cpu_capacity; @@ -151,7 +159,7 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n"); } cap_parsing_failed = true; - kfree(raw_capacity); + free_raw_capacity(); } return !ret; @@ -171,7 +179,7 @@ init_cpu_capacity_callback(struct notifier_block *nb, struct cpufreq_policy *policy = data; int cpu; - if (cap_parsing_failed || cap_parsing_done) + if (!raw_capacity || cap_parsing_done) return 0; if (val != CPUFREQ_NOTIFY) @@ -191,7 +199,7 @@ init_cpu_capacity_callback(struct notifier_block *nb, if (cpumask_empty(cpus_to_visit)) { topology_normalize_cpu_scale(); - kfree(raw_capacity); + free_raw_capacity(); pr_debug("cpu_capacity: parsing done\n"); cap_parsing_done = true; schedule_work(&parsing_done_work); @@ -233,11 +241,5 @@ static void parsing_done_workfn(struct work_struct *work) } #else -static int __init free_raw_capacity(void) -{ - kfree(raw_capacity); - - return 0; -} core_initcall(free_raw_capacity); #endif From d8bcf4db9244e2b85597c680f4e1c3a837b067fe Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Fri, 23 Jun 2017 14:55:34 +0530 Subject: [PATCH 06/28] arch_topology: Get rid of cap_parsing_done There is no need to check for cap_parsing_done flag anymore as !raw_capacity flag alone is enough for us. Remove the (now) useless flag cap_parsing_done. Signed-off-by: Viresh Kumar Reviewed-by: Juri Lelli Tested-by: Juri Lelli Signed-off-by: Greg Kroah-Hartman --- drivers/base/arch_topology.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index 9e4d2107f4fa..74043ead9da1 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -167,7 +167,6 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) #ifdef CONFIG_CPU_FREQ static cpumask_var_t cpus_to_visit; -static bool cap_parsing_done; static void parsing_done_workfn(struct work_struct *work); static DECLARE_WORK(parsing_done_work, parsing_done_workfn); @@ -179,7 +178,7 @@ init_cpu_capacity_callback(struct notifier_block *nb, struct cpufreq_policy *policy = data; int cpu; - if (!raw_capacity || cap_parsing_done) + if (!raw_capacity) return 0; if (val != CPUFREQ_NOTIFY) @@ -201,7 +200,6 @@ init_cpu_capacity_callback(struct notifier_block *nb, topology_normalize_cpu_scale(); free_raw_capacity(); pr_debug("cpu_capacity: parsing done\n"); - cap_parsing_done = true; schedule_work(&parsing_done_work); } From 1455cf8dbfd06aa7651dcfccbadb7a093944ca65 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 19 Jul 2017 17:24:30 -0700 Subject: [PATCH 07/28] driver core: emit uevents when device is bound to a driver There are certain touch controllers that may come up in either normal (application) or boot mode, depending on whether firmware/configuration is corrupted when they are powered on. In boot mode the kernel does not create input device instance (because it does not necessarily know the characteristics of the input device in question). Another number of controllers does not store firmware in a non-volatile memory, and they similarly need to have firmware loaded before input device instance is created. There are also other types of devices with similar behavior. There is a desire to be able to trigger firmware loading via udev, but it has to happen only when driver is bound to a physical device (i2c or spi). These udev actions can not use ADD events, as those happen too early, so we are introducing BIND and UNBIND events that are emitted at the right moment. Also, many drivers create additional driver-specific device attributes when binding to the device, to provide userspace with additional controls. The new events allow userspace to adjust these driver-specific attributes without worrying that they are not there yet. Signed-off-by: Dmitry Torokhov Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 4 ++++ include/linux/kobject.h | 2 ++ lib/kobject_uevent.c | 2 ++ 3 files changed, 8 insertions(+) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 4882f06d12df..c17fefc77345 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -259,6 +259,8 @@ static void driver_bound(struct device *dev) if (dev->bus) blocking_notifier_call_chain(&dev->bus->p->bus_notifier, BUS_NOTIFY_BOUND_DRIVER, dev); + + kobject_uevent(&dev->kobj, KOBJ_BIND); } static int driver_sysfs_add(struct device *dev) @@ -848,6 +850,8 @@ static void __device_release_driver(struct device *dev, struct device *parent) blocking_notifier_call_chain(&dev->bus->p->bus_notifier, BUS_NOTIFY_UNBOUND_DRIVER, dev); + + kobject_uevent(&dev->kobj, KOBJ_UNBIND); } } diff --git a/include/linux/kobject.h b/include/linux/kobject.h index ca85cb80e99a..12f5ddccb97c 100644 --- a/include/linux/kobject.h +++ b/include/linux/kobject.h @@ -57,6 +57,8 @@ enum kobject_action { KOBJ_MOVE, KOBJ_ONLINE, KOBJ_OFFLINE, + KOBJ_BIND, + KOBJ_UNBIND, KOBJ_MAX }; diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c index 9a2b811966eb..4682e8545b5c 100644 --- a/lib/kobject_uevent.c +++ b/lib/kobject_uevent.c @@ -50,6 +50,8 @@ static const char *kobject_actions[] = { [KOBJ_MOVE] = "move", [KOBJ_ONLINE] = "online", [KOBJ_OFFLINE] = "offline", + [KOBJ_BIND] = "bind", + [KOBJ_UNBIND] = "unbind", }; /** From a7670d425b75f9e44b7d4d0aea04f4a6d5f34291 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 19 Jul 2017 17:24:31 -0700 Subject: [PATCH 08/28] driver core: make device_{add|remove}_groups() public Many drivers create additional driver-specific device attributes when binding to the device. To avoid them calling SYSFS API directly, let's export these helpers. Signed-off-by: Dmitry Torokhov Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 5 ----- drivers/base/core.c | 2 ++ include/linux/device.h | 5 +++++ 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index e19b1008e5fb..539432a14b5c 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -126,11 +126,6 @@ extern int driver_add_groups(struct device_driver *drv, extern void driver_remove_groups(struct device_driver *drv, const struct attribute_group **groups); -extern int device_add_groups(struct device *dev, - const struct attribute_group **groups); -extern void device_remove_groups(struct device *dev, - const struct attribute_group **groups); - extern char *make_class_name(const char *name, struct kobject *kobj); extern int devres_release_all(struct device *dev); diff --git a/drivers/base/core.c b/drivers/base/core.c index bbecaf9293be..14f8cf5c8b05 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1026,12 +1026,14 @@ int device_add_groups(struct device *dev, const struct attribute_group **groups) { return sysfs_create_groups(&dev->kobj, groups); } +EXPORT_SYMBOL_GPL(device_add_groups); void device_remove_groups(struct device *dev, const struct attribute_group **groups) { sysfs_remove_groups(&dev->kobj, groups); } +EXPORT_SYMBOL_GPL(device_remove_groups); static int device_add_attrs(struct device *dev) { diff --git a/include/linux/device.h b/include/linux/device.h index 9ef518af5515..10cf209a4e82 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -1200,6 +1200,11 @@ struct device *device_create_with_groups(struct class *cls, const char *fmt, ...); extern void device_destroy(struct class *cls, dev_t devt); +extern int __must_check device_add_groups(struct device *dev, + const struct attribute_group **groups); +extern void device_remove_groups(struct device *dev, + const struct attribute_group **groups); + /* * Platform "fixup" functions - allow the platform to have their say * about devices and actions that the general device layer doesn't From e323b2dddc1ce7ea7f032c986c012a04d5977dc8 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 19 Jul 2017 17:24:32 -0700 Subject: [PATCH 09/28] driver core: add device_{add|remove}_group() helpers We have helpers that work with NULL terminated array of groups, but many drivers only create a single supplemental group, and do not want to declare a group array. Let's provide them with helpers working with a single group. Signed-off-by: Dmitry Torokhov Signed-off-by: Greg Kroah-Hartman --- include/linux/device.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/include/linux/device.h b/include/linux/device.h index 10cf209a4e82..7698a513b35e 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -1205,6 +1205,22 @@ extern int __must_check device_add_groups(struct device *dev, extern void device_remove_groups(struct device *dev, const struct attribute_group **groups); +static inline int __must_check device_add_group(struct device *dev, + const struct attribute_group *grp) +{ + const struct attribute_group *groups[] = { grp, NULL }; + + return device_add_groups(dev, groups); +} + +static inline void device_remove_group(struct device *dev, + const struct attribute_group *grp) +{ + const struct attribute_group *groups[] = { grp, NULL }; + + return device_remove_groups(dev, groups); +} + /* * Platform "fixup" functions - allow the platform to have their say * about devices and actions that the general device layer doesn't From 57b8ff070f9897b22e4f80fda775a489fc797008 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 19 Jul 2017 17:24:33 -0700 Subject: [PATCH 10/28] driver core: add devm_device_add_group() and friends Many drivers create additional driver-specific device attributes when binding to the device, and providing managed version of device_create_group() will simplify unbinding and error handling in probe path for such drivers. Without managed version driver writers either have to mix manual and managed resources, which is prone to errors, or open-code this function by providing a wrapper to device_add_group() and use it with devm_add_action() or devm_add_action_or_reset(). Signed-off-by: Dmitry Torokhov Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 130 +++++++++++++++++++++++++++++++++++++++++ include/linux/device.h | 9 +++ 2 files changed, 139 insertions(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index 14f8cf5c8b05..09723532725d 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1035,6 +1035,136 @@ void device_remove_groups(struct device *dev, } EXPORT_SYMBOL_GPL(device_remove_groups); +union device_attr_group_devres { + const struct attribute_group *group; + const struct attribute_group **groups; +}; + +static int devm_attr_group_match(struct device *dev, void *res, void *data) +{ + return ((union device_attr_group_devres *)res)->group == data; +} + +static void devm_attr_group_remove(struct device *dev, void *res) +{ + union device_attr_group_devres *devres = res; + const struct attribute_group *group = devres->group; + + dev_dbg(dev, "%s: removing group %p\n", __func__, group); + sysfs_remove_group(&dev->kobj, group); +} + +static void devm_attr_groups_remove(struct device *dev, void *res) +{ + union device_attr_group_devres *devres = res; + const struct attribute_group **groups = devres->groups; + + dev_dbg(dev, "%s: removing groups %p\n", __func__, groups); + sysfs_remove_groups(&dev->kobj, groups); +} + +/** + * devm_device_add_group - given a device, create a managed attribute group + * @dev: The device to create the group for + * @grp: The attribute group to create + * + * This function creates a group for the first time. It will explicitly + * warn and error if any of the attribute files being created already exist. + * + * Returns 0 on success or error code on failure. + */ +int devm_device_add_group(struct device *dev, const struct attribute_group *grp) +{ + union device_attr_group_devres *devres; + int error; + + devres = devres_alloc(devm_attr_group_remove, + sizeof(*devres), GFP_KERNEL); + if (!devres) + return -ENOMEM; + + error = sysfs_create_group(&dev->kobj, grp); + if (error) { + devres_free(devres); + return error; + } + + devres->group = grp; + devres_add(dev, devres); + return 0; +} +EXPORT_SYMBOL_GPL(devm_device_add_group); + +/** + * devm_device_remove_group: remove a managed group from a device + * @dev: device to remove the group from + * @grp: group to remove + * + * This function removes a group of attributes from a device. The attributes + * previously have to have been created for this group, otherwise it will fail. + */ +void devm_device_remove_group(struct device *dev, + const struct attribute_group *grp) +{ + WARN_ON(devres_release(dev, devm_attr_group_remove, + devm_attr_group_match, + /* cast away const */ (void *)grp)); +} +EXPORT_SYMBOL_GPL(devm_device_remove_group); + +/** + * devm_device_add_groups - create a bunch of managed attribute groups + * @dev: The device to create the group for + * @groups: The attribute groups to create, NULL terminated + * + * This function creates a bunch of managed attribute groups. If an error + * occurs when creating a group, all previously created groups will be + * removed, unwinding everything back to the original state when this + * function was called. It will explicitly warn and error if any of the + * attribute files being created already exist. + * + * Returns 0 on success or error code from sysfs_create_group on failure. + */ +int devm_device_add_groups(struct device *dev, + const struct attribute_group **groups) +{ + union device_attr_group_devres *devres; + int error; + + devres = devres_alloc(devm_attr_groups_remove, + sizeof(*devres), GFP_KERNEL); + if (!devres) + return -ENOMEM; + + error = sysfs_create_groups(&dev->kobj, groups); + if (error) { + devres_free(devres); + return error; + } + + devres->groups = groups; + devres_add(dev, devres); + return 0; +} +EXPORT_SYMBOL_GPL(devm_device_add_groups); + +/** + * devm_device_remove_groups - remove a list of managed groups + * + * @dev: The device for the groups to be removed from + * @groups: NULL terminated list of groups to be removed + * + * If groups is not NULL, remove the specified groups from the device. + */ +void devm_device_remove_groups(struct device *dev, + const struct attribute_group **groups) +{ + WARN_ON(devres_release(dev, devm_attr_groups_remove, + devm_attr_group_match, + /* cast away const */ (void *)groups)); +} +EXPORT_SYMBOL_GPL(devm_device_remove_groups); + static int device_add_attrs(struct device *dev) { struct class *class = dev->class; diff --git a/include/linux/device.h b/include/linux/device.h index 7698a513b35e..f52288c24734 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -1221,6 +1221,15 @@ static inline void device_remove_group(struct device *dev, return device_remove_groups(dev, groups); } +extern int __must_check devm_device_add_groups(struct device *dev, + const struct attribute_group **groups); +extern void devm_device_remove_groups(struct device *dev, + const struct attribute_group **groups); +extern int __must_check devm_device_add_group(struct device *dev, + const struct attribute_group *grp); +extern void devm_device_remove_group(struct device *dev, + const struct attribute_group *grp); + /* * Platform "fixup" functions - allow the platform to have their say * about devices and actions that the general device layer doesn't From 3184125ee2da70c5189cdf03c86cfeac7dd7bba9 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 19 Jul 2017 17:24:34 -0700 Subject: [PATCH 11/28] Input: gpio_keys - use devm_device_add_group() for attributes Now that we have proper managed API to create device attributes, let's start using it instead of the manual unwinding. Signed-off-by: Dmitry Torokhov Reviewed-by: Guenter Roeck Signed-off-by: Greg Kroah-Hartman --- drivers/input/keyboard/gpio_keys.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index da3d362f21b1..5a08bbc4a8d8 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -814,7 +814,7 @@ static int gpio_keys_probe(struct platform_device *pdev) fwnode_handle_put(child); - error = sysfs_create_group(&dev->kobj, &gpio_keys_attr_group); + error = devm_device_add_group(dev, &gpio_keys_attr_group); if (error) { dev_err(dev, "Unable to export keys/switches, error: %d\n", error); @@ -825,22 +825,11 @@ static int gpio_keys_probe(struct platform_device *pdev) if (error) { dev_err(dev, "Unable to register input device, error: %d\n", error); - goto err_remove_group; + return error; } device_init_wakeup(dev, wakeup); - return 0; - -err_remove_group: - sysfs_remove_group(&dev->kobj, &gpio_keys_attr_group); - return error; -} - -static int gpio_keys_remove(struct platform_device *pdev) -{ - sysfs_remove_group(&pdev->dev.kobj, &gpio_keys_attr_group); - return 0; } @@ -897,7 +886,6 @@ static SIMPLE_DEV_PM_OPS(gpio_keys_pm_ops, gpio_keys_suspend, gpio_keys_resume); static struct platform_driver gpio_keys_device_driver = { .probe = gpio_keys_probe, - .remove = gpio_keys_remove, .driver = { .name = "gpio-keys", .pm = &gpio_keys_pm_ops, From 36a44af5c176d619552d99697433261141dd1296 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 19 Jul 2017 17:24:35 -0700 Subject: [PATCH 12/28] Input: synaptics_rmi4 - use devm_device_add_group() for attributes in F01 Now that we have proper managed API to create device attributes, let's start using it instead of the manual unwinding. Signed-off-by: Dmitry Torokhov Reviewed-by: Guenter Roeck Signed-off-by: Greg Kroah-Hartman --- drivers/input/rmi4/rmi_f01.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c index 7f7e9176f7ea..6dca3c0fbb4a 100644 --- a/drivers/input/rmi4/rmi_f01.c +++ b/drivers/input/rmi4/rmi_f01.c @@ -570,18 +570,14 @@ static int rmi_f01_probe(struct rmi_function *fn) dev_set_drvdata(&fn->dev, f01); - error = sysfs_create_group(&fn->rmi_dev->dev.kobj, &rmi_f01_attr_group); + error = devm_device_add_group(&fn->rmi_dev->dev, &rmi_f01_attr_group); if (error) - dev_warn(&fn->dev, "Failed to create sysfs group: %d\n", error); + dev_warn(&fn->dev, + "Failed to create attribute group: %d\n", error); return 0; } -static void rmi_f01_remove(struct rmi_function *fn) -{ - sysfs_remove_group(&fn->rmi_dev->dev.kobj, &rmi_f01_attr_group); -} - static int rmi_f01_config(struct rmi_function *fn) { struct f01_data *f01 = dev_get_drvdata(&fn->dev); @@ -721,7 +717,6 @@ struct rmi_function_handler rmi_f01_handler = { }, .func = 0x01, .probe = rmi_f01_probe, - .remove = rmi_f01_remove, .config = rmi_f01_config, .attention = rmi_f01_attention, .suspend = rmi_f01_suspend, From 072a7852338af900c302490474939e089f4bd4c4 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 19 Jul 2017 17:24:36 -0700 Subject: [PATCH 13/28] Input: axp20x-pek - switch to using devm_device_add_group() Now that we have proper managed API to create device attributes, let's use it instead of installing a custom devm action. Signed-off-by: Dmitry Torokhov Reviewed-by: Guenter Roeck Signed-off-by: Greg Kroah-Hartman --- drivers/input/misc/axp20x-pek.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c index 400869e61a06..0932d3e56199 100644 --- a/drivers/input/misc/axp20x-pek.c +++ b/drivers/input/misc/axp20x-pek.c @@ -182,13 +182,6 @@ static irqreturn_t axp20x_pek_irq(int irq, void *pwr) return IRQ_HANDLED; } -static void axp20x_remove_sysfs_group(void *_data) -{ - struct device *dev = _data; - - sysfs_remove_group(&dev->kobj, &axp20x_attribute_group); -} - static int axp20x_pek_probe_input_device(struct axp20x_pek *axp20x_pek, struct platform_device *pdev) { @@ -310,22 +303,13 @@ static int axp20x_pek_probe(struct platform_device *pdev) return error; } - error = sysfs_create_group(&pdev->dev.kobj, &axp20x_attribute_group); + error = devm_device_add_group(&pdev->dev, &axp20x_attribute_group); if (error) { dev_err(&pdev->dev, "Failed to create sysfs attributes: %d\n", error); return error; } - error = devm_add_action(&pdev->dev, - axp20x_remove_sysfs_group, &pdev->dev); - if (error) { - axp20x_remove_sysfs_group(&pdev->dev); - dev_err(&pdev->dev, "Failed to add sysfs cleanup action: %d\n", - error); - return error; - } - platform_set_drvdata(pdev, axp20x_pek); return 0; From 1f5000bd8afab0ceed58c67f673250b864e5a9c9 Mon Sep 17 00:00:00 2001 From: Todd Poynor Date: Tue, 25 Jul 2017 16:31:59 -0700 Subject: [PATCH 14/28] initcall_debug: add deferred probe times initcall_debug attributes all deferred device probe retries for the late_initcall level to function deferred_probe_initcall. Add logs of the individual device probe routines called, to identify which drivers are executing for how long during the initcall path. Deferred probes that occur after initcall processing are not shown. Example log messages added: [ 0.505119] deferred probe my-sound-device @ 6 [ 0.517656] deferred probe my-sound-device returned after 1227 usecs Signed-off-by: Todd Poynor Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index c17fefc77345..ad44b40fe284 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -53,6 +54,7 @@ static DEFINE_MUTEX(deferred_probe_mutex); static LIST_HEAD(deferred_probe_pending_list); static LIST_HEAD(deferred_probe_active_list); static atomic_t deferred_trigger_count = ATOMIC_INIT(0); +static bool initcalls_done; /* * In some cases, like suspend to RAM or hibernation, It might be reasonable @@ -61,6 +63,26 @@ static atomic_t deferred_trigger_count = ATOMIC_INIT(0); */ static bool defer_all_probes; +/* + * For initcall_debug, show the deferred probes executed in late_initcall + * processing. + */ +static void deferred_probe_debug(struct device *dev) +{ + ktime_t calltime, delta, rettime; + unsigned long long duration; + + printk(KERN_DEBUG "deferred probe %s @ %i\n", dev_name(dev), + task_pid_nr(current)); + calltime = ktime_get(); + bus_probe_device(dev); + rettime = ktime_get(); + delta = ktime_sub(rettime, calltime); + duration = (unsigned long long) ktime_to_ns(delta) >> 10; + printk(KERN_DEBUG "deferred probe %s returned after %lld usecs\n", + dev_name(dev), duration); +} + /* * deferred_probe_work_func() - Retry probing devices in the active list. */ @@ -106,7 +128,10 @@ static void deferred_probe_work_func(struct work_struct *work) device_pm_unlock(); dev_dbg(dev, "Retrying from deferred list\n"); - bus_probe_device(dev); + if (initcall_debug && !initcalls_done) + deferred_probe_debug(dev); + else + bus_probe_device(dev); mutex_lock(&deferred_probe_mutex); @@ -215,6 +240,7 @@ static int deferred_probe_initcall(void) driver_deferred_probe_trigger(); /* Sort as many dependencies as possible before exiting initcalls */ flush_work(&deferred_probe_work); + initcalls_done = true; return 0; } late_initcall(deferred_probe_initcall); From 0d1f417eee8ad0687afb90eab282614eecce1a13 Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 20 Jul 2017 13:13:38 -0700 Subject: [PATCH 15/28] test_firmware: add test case for SIGCHLD on sync fallback It has been reported that SIGCHLD will trigger an immediate abort on sync firmware requests which rely on the sysfs interface for a trigger. This is unexpected behaviour, this reproduces this issue. This test case currenty fails. Reported-by: Martin Fuzzey Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- .../testing/selftests/firmware/fw_fallback.sh | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tools/testing/selftests/firmware/fw_fallback.sh b/tools/testing/selftests/firmware/fw_fallback.sh index 2e4c22d5abf7..8f511035f783 100755 --- a/tools/testing/selftests/firmware/fw_fallback.sh +++ b/tools/testing/selftests/firmware/fw_fallback.sh @@ -134,6 +134,27 @@ load_fw_custom_cancel() wait } +load_fw_fallback_with_child() +{ + local name="$1" + local file="$2" + + # This is the value already set but we want to be explicit + echo 4 >/sys/class/firmware/timeout + + sleep 1 & + SECONDS_BEFORE=$(date +%s) + echo -n "$name" >"$DIR"/trigger_request 2>/dev/null + SECONDS_AFTER=$(date +%s) + SECONDS_DELTA=$(($SECONDS_AFTER - $SECONDS_BEFORE)) + if [ "$SECONDS_DELTA" -lt 4 ]; then + RET=1 + else + RET=0 + fi + wait + return $RET +} trap "test_finish" EXIT @@ -221,4 +242,14 @@ else echo "$0: cancelling custom fallback mechanism works" fi +set +e +load_fw_fallback_with_child "nope-signal-$NAME" "$FW" +if [ "$?" -eq 0 ]; then + echo "$0: SIGCHLD on sync ignored as expected" >&2 +else + echo "$0: error - sync firmware request cancelled due to SIGCHLD" >&2 + exit 1 +fi +set -e + exit 0 From 76098b36b5db1a509e5af94128b08f950692c7f8 Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 20 Jul 2017 13:13:39 -0700 Subject: [PATCH 16/28] firmware: send -EINTR on signal abort on fallback mechanism Right now we send -EAGAIN to a syfs write which got interrupted. Userspace can't tell what happened though, send -EINTR if we were killed due to a signal so userspace can tell things apart. This is only applicable to the fallback mechanism. Reported-by: Martin Fuzzey Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_class.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index b9f907eedbf7..ac7e32fac8fc 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -1089,9 +1089,12 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, mutex_unlock(&fw_lock); } - if (fw_state_is_aborted(&buf->fw_st)) - retval = -EAGAIN; - else if (buf->is_paged_buf && !buf->data) + if (fw_state_is_aborted(&buf->fw_st)) { + if (retval == -ERESTARTSYS) + retval = -EINTR; + else + retval = -EAGAIN; + } else if (buf->is_paged_buf && !buf->data) retval = -ENOMEM; device_del(f_dev); From 73da4b4b77661ba3a9a50e5fbb412c9766149911 Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 20 Jul 2017 13:13:40 -0700 Subject: [PATCH 17/28] firmware: define pr_fmt For some reason we have always forgotten this. Without this we don't get a nice prefix on our pr_debug() / pr_*() messages. Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_class.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index ac7e32fac8fc..ceab749b1790 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -7,6 +7,8 @@ * */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include From 30172bead80e089072c6f2d00e68831c51ebbc19 Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 20 Jul 2017 13:13:41 -0700 Subject: [PATCH 18/28] firmware: enable a debug print for batched requests Otherwise there is no easy way this actually happened. Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_class.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index ceab749b1790..68726c8e8306 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -337,6 +337,7 @@ static struct firmware_buf *__fw_lookup_buf(const char *fw_name) return NULL; } +/* Returns 1 for batching firmware requests with the same name */ static int fw_lookup_and_allocate_buf(const char *fw_name, struct firmware_cache *fwc, struct firmware_buf **buf, void *dbuf, @@ -350,6 +351,7 @@ static int fw_lookup_and_allocate_buf(const char *fw_name, kref_get(&tmp->ref); spin_unlock(&fwc->lock); *buf = tmp; + pr_debug("batched request - sharing the same struct firmware_buf and lookup for multiple requests\n"); return 1; } tmp = __allocate_fw_buf(fw_name, fwc, dbuf, size); From c92316bf8e94830a0225f2e904cbdbd173768419 Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 20 Jul 2017 13:13:42 -0700 Subject: [PATCH 19/28] test_firmware: add batched firmware tests The firmware API has a feature to enable batching requests for the same fil e under one worker, so only one lookup is done. This only triggers if we so happen to schedule two lookups for same file around the same time, or if release_firmware() has not been called for a successful firmware call. This can happen for instance if you happen to have multiple devices and one device driver for certain drivers where the stars line up scheduling wise. This adds a new sync and async test trigger. Instead of adding a new trigger for each new test type we make the tests a bit configurable so that we could configure the tests in userspace and just kick a test through a few basic triggers. With this, for instance the two types of sync requests: o request_firmware() and o request_firmware_direct() can be modified with a knob. Likewise the two type of async requests: o request_firmware_nowait(uevent=true) and o request_firmware_nowait(uevent=false) can be configured with another knob. The call request_firmware_into_buf() has no users... yet. The old tests are left in place as-is given they serve a few other purposes which we are currently not interested in also testing yet. This will change later as we will be able to just consolidate all tests under a few basic triggers with just one general configuration setup. We perform two types of tests, one for where the file is present and one for where the file is not present. All test tests go tested and they now pass for the following 3 kernel builds possible for the firmware API: 0. Most distro setup: CONFIG_FW_LOADER_USER_HELPER_FALLBACK=n CONFIG_FW_LOADER_USER_HELPER=y 1. Android: CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y CONFIG_FW_LOADER_USER_HELPER=y 2. Rare build: CONFIG_FW_LOADER_USER_HELPER_FALLBACK=n CONFIG_FW_LOADER_USER_HELPER=n Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- lib/test_firmware.c | 710 ++++++++++++++++++ .../selftests/firmware/fw_filesystem.sh | 241 +++++- 2 files changed, 949 insertions(+), 2 deletions(-) diff --git a/lib/test_firmware.c b/lib/test_firmware.c index 09371b0a9baf..64a4c76cba2b 100644 --- a/lib/test_firmware.c +++ b/lib/test_firmware.c @@ -19,10 +19,85 @@ #include #include #include +#include +#include + +#define TEST_FIRMWARE_NAME "test-firmware.bin" +#define TEST_FIRMWARE_NUM_REQS 4 static DEFINE_MUTEX(test_fw_mutex); static const struct firmware *test_firmware; +struct test_batched_req { + u8 idx; + int rc; + bool sent; + const struct firmware *fw; + const char *name; + struct completion completion; + struct task_struct *task; + struct device *dev; +}; + +/** + * test_config - represents configuration for the test for different triggers + * + * @name: the name of the firmware file to look for + * @sync_direct: when the sync trigger is used if this is true + * request_firmware_direct() will be used instead. + * @send_uevent: whether or not to send a uevent for async requests + * @num_requests: number of requests to try per test case. This is trigger + * specific. + * @reqs: stores all requests information + * @read_fw_idx: index of thread from which we want to read firmware results + * from through the read_fw trigger. + * @test_result: a test may use this to collect the result from the call + * of the request_firmware*() calls used in their tests. In order of + * priority we always keep first any setup error. If no setup errors were + * found then we move on to the first error encountered while running the + * API. Note that for async calls this typically will be a successful + * result (0) unless of course you've used bogus parameters, or the system + * is out of memory. In the async case the callback is expected to do a + * bit more homework to figure out what happened, unfortunately the only + * information passed today on error is the fact that no firmware was + * found so we can only assume -ENOENT on async calls if the firmware is + * NULL. + * + * Errors you can expect: + * + * API specific: + * + * 0: success for sync, for async it means request was sent + * -EINVAL: invalid parameters or request + * -ENOENT: files not found + * + * System environment: + * + * -ENOMEM: memory pressure on system + * -ENODEV: out of number of devices to test + * -EINVAL: an unexpected error has occurred + * @req_firmware: if @sync_direct is true this is set to + * request_firmware_direct(), otherwise request_firmware() + */ +struct test_config { + char *name; + bool sync_direct; + bool send_uevent; + u8 num_requests; + u8 read_fw_idx; + + /* + * These below don't belong her but we'll move them once we create + * a struct fw_test_device and stuff the misc_dev under there later. + */ + struct test_batched_req *reqs; + int test_result; + int (*req_firmware)(const struct firmware **fw, const char *name, + struct device *device); +}; + +struct test_config *test_fw_config; + static ssize_t test_fw_misc_read(struct file *f, char __user *buf, size_t size, loff_t *offset) { @@ -42,6 +117,338 @@ static const struct file_operations test_fw_fops = { .read = test_fw_misc_read, }; +static void __test_release_all_firmware(void) +{ + struct test_batched_req *req; + u8 i; + + if (!test_fw_config->reqs) + return; + + for (i = 0; i < test_fw_config->num_requests; i++) { + req = &test_fw_config->reqs[i]; + if (req->fw) + release_firmware(req->fw); + } + + vfree(test_fw_config->reqs); + test_fw_config->reqs = NULL; +} + +static void test_release_all_firmware(void) +{ + mutex_lock(&test_fw_mutex); + __test_release_all_firmware(); + mutex_unlock(&test_fw_mutex); +} + + +static void __test_firmware_config_free(void) +{ + __test_release_all_firmware(); + kfree_const(test_fw_config->name); + test_fw_config->name = NULL; +} + +/* + * XXX: move to kstrncpy() once merged. + * + * Users should use kfree_const() when freeing these. + */ +static int __kstrncpy(char **dst, const char *name, size_t count, gfp_t gfp) +{ + *dst = kstrndup(name, count, gfp); + if (!*dst) + return -ENOSPC; + return count; +} + +static int __test_firmware_config_init(void) +{ + int ret; + + ret = __kstrncpy(&test_fw_config->name, TEST_FIRMWARE_NAME, + strlen(TEST_FIRMWARE_NAME), GFP_KERNEL); + if (ret < 0) + goto out; + + test_fw_config->num_requests = TEST_FIRMWARE_NUM_REQS; + test_fw_config->send_uevent = true; + test_fw_config->sync_direct = false; + test_fw_config->req_firmware = request_firmware; + test_fw_config->test_result = 0; + test_fw_config->reqs = NULL; + + return 0; + +out: + __test_firmware_config_free(); + return ret; +} + +static ssize_t reset_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + int ret; + + mutex_lock(&test_fw_mutex); + + __test_firmware_config_free(); + + ret = __test_firmware_config_init(); + if (ret < 0) { + ret = -ENOMEM; + pr_err("could not alloc settings for config trigger: %d\n", + ret); + goto out; + } + + pr_info("reset\n"); + ret = count; + +out: + mutex_unlock(&test_fw_mutex); + + return ret; +} +static DEVICE_ATTR_WO(reset); + +static ssize_t config_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + int len = 0; + + mutex_lock(&test_fw_mutex); + + len += snprintf(buf, PAGE_SIZE, + "Custom trigger configuration for: %s\n", + dev_name(dev)); + + if (test_fw_config->name) + len += snprintf(buf+len, PAGE_SIZE, + "name:\t%s\n", + test_fw_config->name); + else + len += snprintf(buf+len, PAGE_SIZE, + "name:\tEMTPY\n"); + + len += snprintf(buf+len, PAGE_SIZE, + "num_requests:\t%u\n", test_fw_config->num_requests); + + len += snprintf(buf+len, PAGE_SIZE, + "send_uevent:\t\t%s\n", + test_fw_config->send_uevent ? + "FW_ACTION_HOTPLUG" : + "FW_ACTION_NOHOTPLUG"); + len += snprintf(buf+len, PAGE_SIZE, + "sync_direct:\t\t%s\n", + test_fw_config->sync_direct ? "true" : "false"); + len += snprintf(buf+len, PAGE_SIZE, + "read_fw_idx:\t%u\n", test_fw_config->read_fw_idx); + + mutex_unlock(&test_fw_mutex); + + return len; +} +static DEVICE_ATTR_RO(config); + +static ssize_t config_name_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + int ret; + + mutex_lock(&test_fw_mutex); + kfree_const(test_fw_config->name); + ret = __kstrncpy(&test_fw_config->name, buf, count, GFP_KERNEL); + mutex_unlock(&test_fw_mutex); + + return ret; +} + +/* + * As per sysfs_kf_seq_show() the buf is max PAGE_SIZE. + */ +static ssize_t config_test_show_str(char *dst, + char *src) +{ + int len; + + mutex_lock(&test_fw_mutex); + len = snprintf(dst, PAGE_SIZE, "%s\n", src); + mutex_unlock(&test_fw_mutex); + + return len; +} + +static int test_dev_config_update_bool(const char *buf, size_t size, + bool *cfg) +{ + int ret; + + mutex_lock(&test_fw_mutex); + if (strtobool(buf, cfg) < 0) + ret = -EINVAL; + else + ret = size; + mutex_unlock(&test_fw_mutex); + + return ret; +} + +static ssize_t +test_dev_config_show_bool(char *buf, + bool config) +{ + bool val; + + mutex_lock(&test_fw_mutex); + val = config; + mutex_unlock(&test_fw_mutex); + + return snprintf(buf, PAGE_SIZE, "%d\n", val); +} + +static ssize_t test_dev_config_show_int(char *buf, int cfg) +{ + int val; + + mutex_lock(&test_fw_mutex); + val = cfg; + mutex_unlock(&test_fw_mutex); + + return snprintf(buf, PAGE_SIZE, "%d\n", val); +} + +static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) +{ + int ret; + long new; + + ret = kstrtol(buf, 10, &new); + if (ret) + return ret; + + if (new > U8_MAX) + return -EINVAL; + + mutex_lock(&test_fw_mutex); + *(u8 *)cfg = new; + mutex_unlock(&test_fw_mutex); + + /* Always return full write size even if we didn't consume all */ + return size; +} + +static ssize_t test_dev_config_show_u8(char *buf, u8 cfg) +{ + u8 val; + + mutex_lock(&test_fw_mutex); + val = cfg; + mutex_unlock(&test_fw_mutex); + + return snprintf(buf, PAGE_SIZE, "%u\n", val); +} + +static ssize_t config_name_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return config_test_show_str(buf, test_fw_config->name); +} +static DEVICE_ATTR(config_name, 0644, config_name_show, config_name_store); + +static ssize_t config_num_requests_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + int rc; + + mutex_lock(&test_fw_mutex); + if (test_fw_config->reqs) { + pr_err("Must call release_all_firmware prior to changing config\n"); + rc = -EINVAL; + goto out; + } + mutex_unlock(&test_fw_mutex); + + rc = test_dev_config_update_u8(buf, count, + &test_fw_config->num_requests); + +out: + return rc; +} + +static ssize_t config_num_requests_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return test_dev_config_show_u8(buf, test_fw_config->num_requests); +} +static DEVICE_ATTR(config_num_requests, 0644, config_num_requests_show, + config_num_requests_store); + +static ssize_t config_sync_direct_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + int rc = test_dev_config_update_bool(buf, count, + &test_fw_config->sync_direct); + + if (rc == count) + test_fw_config->req_firmware = test_fw_config->sync_direct ? + request_firmware_direct : + request_firmware; + return rc; +} + +static ssize_t config_sync_direct_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return test_dev_config_show_bool(buf, test_fw_config->sync_direct); +} +static DEVICE_ATTR(config_sync_direct, 0644, config_sync_direct_show, + config_sync_direct_store); + +static ssize_t config_send_uevent_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + return test_dev_config_update_bool(buf, count, + &test_fw_config->send_uevent); +} + +static ssize_t config_send_uevent_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return test_dev_config_show_bool(buf, test_fw_config->send_uevent); +} +static DEVICE_ATTR(config_send_uevent, 0644, config_send_uevent_show, + config_send_uevent_store); + +static ssize_t config_read_fw_idx_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + return test_dev_config_update_u8(buf, count, + &test_fw_config->read_fw_idx); +} + +static ssize_t config_read_fw_idx_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return test_dev_config_show_u8(buf, test_fw_config->read_fw_idx); +} +static DEVICE_ATTR(config_read_fw_idx, 0644, config_read_fw_idx_show, + config_read_fw_idx_store); + + static ssize_t trigger_request_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) @@ -170,12 +577,301 @@ static ssize_t trigger_custom_fallback_store(struct device *dev, } static DEVICE_ATTR_WO(trigger_custom_fallback); +static int test_fw_run_batch_request(void *data) +{ + struct test_batched_req *req = data; + + if (!req) { + test_fw_config->test_result = -EINVAL; + return -EINVAL; + } + + req->rc = test_fw_config->req_firmware(&req->fw, req->name, req->dev); + if (req->rc) { + pr_info("#%u: batched sync load failed: %d\n", + req->idx, req->rc); + if (!test_fw_config->test_result) + test_fw_config->test_result = req->rc; + } else if (req->fw) { + req->sent = true; + pr_info("#%u: batched sync loaded %zu\n", + req->idx, req->fw->size); + } + complete(&req->completion); + + req->task = NULL; + + return 0; +} + +/* + * We use a kthread as otherwise the kernel serializes all our sync requests + * and we would not be able to mimic batched requests on a sync call. Batched + * requests on a sync call can for instance happen on a device driver when + * multiple cards are used and firmware loading happens outside of probe. + */ +static ssize_t trigger_batched_requests_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct test_batched_req *req; + int rc; + u8 i; + + mutex_lock(&test_fw_mutex); + + test_fw_config->reqs = vzalloc(sizeof(struct test_batched_req) * + test_fw_config->num_requests * 2); + if (!test_fw_config->reqs) { + rc = -ENOMEM; + goto out_unlock; + } + + pr_info("batched sync firmware loading '%s' %u times\n", + test_fw_config->name, test_fw_config->num_requests); + + for (i = 0; i < test_fw_config->num_requests; i++) { + req = &test_fw_config->reqs[i]; + if (!req) { + WARN_ON(1); + rc = -ENOMEM; + goto out_bail; + } + req->fw = NULL; + req->idx = i; + req->name = test_fw_config->name; + req->dev = dev; + init_completion(&req->completion); + req->task = kthread_run(test_fw_run_batch_request, req, + "%s-%u", KBUILD_MODNAME, req->idx); + if (!req->task || IS_ERR(req->task)) { + pr_err("Setting up thread %u failed\n", req->idx); + req->task = NULL; + rc = -ENOMEM; + goto out_bail; + } + } + + rc = count; + + /* + * We require an explicit release to enable more time and delay of + * calling release_firmware() to improve our chances of forcing a + * batched request. If we instead called release_firmware() right away + * then we might miss on an opportunity of having a successful firmware + * request pass on the opportunity to be come a batched request. + */ + +out_bail: + for (i = 0; i < test_fw_config->num_requests; i++) { + req = &test_fw_config->reqs[i]; + if (req->task || req->sent) + wait_for_completion(&req->completion); + } + + /* Override any worker error if we had a general setup error */ + if (rc < 0) + test_fw_config->test_result = rc; + +out_unlock: + mutex_unlock(&test_fw_mutex); + + return rc; +} +static DEVICE_ATTR_WO(trigger_batched_requests); + +/* + * We wait for each callback to return with the lock held, no need to lock here + */ +static void trigger_batched_cb(const struct firmware *fw, void *context) +{ + struct test_batched_req *req = context; + + if (!req) { + test_fw_config->test_result = -EINVAL; + return; + } + + /* forces *some* batched requests to queue up */ + if (!req->idx) + ssleep(2); + + req->fw = fw; + + /* + * Unfortunately the firmware API gives us nothing other than a null FW + * if the firmware was not found on async requests. Best we can do is + * just assume -ENOENT. A better API would pass the actual return + * value to the callback. + */ + if (!fw && !test_fw_config->test_result) + test_fw_config->test_result = -ENOENT; + + complete(&req->completion); +} + +static +ssize_t trigger_batched_requests_async_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct test_batched_req *req; + bool send_uevent; + int rc; + u8 i; + + mutex_lock(&test_fw_mutex); + + test_fw_config->reqs = vzalloc(sizeof(struct test_batched_req) * + test_fw_config->num_requests * 2); + if (!test_fw_config->reqs) { + rc = -ENOMEM; + goto out; + } + + pr_info("batched loading '%s' custom fallback mechanism %u times\n", + test_fw_config->name, test_fw_config->num_requests); + + send_uevent = test_fw_config->send_uevent ? FW_ACTION_HOTPLUG : + FW_ACTION_NOHOTPLUG; + + for (i = 0; i < test_fw_config->num_requests; i++) { + req = &test_fw_config->reqs[i]; + if (!req) { + WARN_ON(1); + goto out_bail; + } + req->name = test_fw_config->name; + req->fw = NULL; + req->idx = i; + init_completion(&req->completion); + rc = request_firmware_nowait(THIS_MODULE, send_uevent, + req->name, + dev, GFP_KERNEL, req, + trigger_batched_cb); + if (rc) { + pr_info("#%u: batched async load failed setup: %d\n", + i, rc); + req->rc = rc; + goto out_bail; + } else + req->sent = true; + } + + rc = count; + +out_bail: + + /* + * We require an explicit release to enable more time and delay of + * calling release_firmware() to improve our chances of forcing a + * batched request. If we instead called release_firmware() right away + * then we might miss on an opportunity of having a successful firmware + * request pass on the opportunity to be come a batched request. + */ + + for (i = 0; i < test_fw_config->num_requests; i++) { + req = &test_fw_config->reqs[i]; + if (req->sent) + wait_for_completion(&req->completion); + } + + /* Override any worker error if we had a general setup error */ + if (rc < 0) + test_fw_config->test_result = rc; + +out: + mutex_unlock(&test_fw_mutex); + + return rc; +} +static DEVICE_ATTR_WO(trigger_batched_requests_async); + +static ssize_t test_result_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return test_dev_config_show_int(buf, test_fw_config->test_result); +} +static DEVICE_ATTR_RO(test_result); + +static ssize_t release_all_firmware_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + test_release_all_firmware(); + return count; +} +static DEVICE_ATTR_WO(release_all_firmware); + +static ssize_t read_firmware_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct test_batched_req *req; + u8 idx; + ssize_t rc = 0; + + mutex_lock(&test_fw_mutex); + + idx = test_fw_config->read_fw_idx; + if (idx >= test_fw_config->num_requests) { + rc = -ERANGE; + goto out; + } + + if (!test_fw_config->reqs) { + rc = -EINVAL; + goto out; + } + + req = &test_fw_config->reqs[idx]; + if (!req->fw) { + pr_err("#%u: failed to async load firmware\n", idx); + rc = -ENOENT; + goto out; + } + + pr_info("#%u: loaded %zu\n", idx, req->fw->size); + + if (req->fw->size > PAGE_SIZE) { + pr_err("Testing interface must use PAGE_SIZE firmware for now\n"); + rc = -EINVAL; + } + memcpy(buf, req->fw->data, req->fw->size); + + rc = req->fw->size; +out: + mutex_unlock(&test_fw_mutex); + + return rc; +} +static DEVICE_ATTR_RO(read_firmware); + #define TEST_FW_DEV_ATTR(name) &dev_attr_##name.attr static struct attribute *test_dev_attrs[] = { + TEST_FW_DEV_ATTR(reset), + + TEST_FW_DEV_ATTR(config), + TEST_FW_DEV_ATTR(config_name), + TEST_FW_DEV_ATTR(config_num_requests), + TEST_FW_DEV_ATTR(config_sync_direct), + TEST_FW_DEV_ATTR(config_send_uevent), + TEST_FW_DEV_ATTR(config_read_fw_idx), + + /* These don't use the config at all - they could be ported! */ TEST_FW_DEV_ATTR(trigger_request), TEST_FW_DEV_ATTR(trigger_async_request), TEST_FW_DEV_ATTR(trigger_custom_fallback), + + /* These use the config and can use the test_result */ + TEST_FW_DEV_ATTR(trigger_batched_requests), + TEST_FW_DEV_ATTR(trigger_batched_requests_async), + + TEST_FW_DEV_ATTR(release_all_firmware), + TEST_FW_DEV_ATTR(test_result), + TEST_FW_DEV_ATTR(read_firmware), NULL, }; @@ -192,8 +888,17 @@ static int __init test_firmware_init(void) { int rc; + test_fw_config = kzalloc(sizeof(struct test_config), GFP_KERNEL); + if (!test_fw_config) + return -ENOMEM; + + rc = __test_firmware_config_init(); + if (rc) + return rc; + rc = misc_register(&test_fw_misc_device); if (rc) { + kfree(test_fw_config); pr_err("could not register misc device: %d\n", rc); return rc; } @@ -207,8 +912,13 @@ module_init(test_firmware_init); static void __exit test_firmware_exit(void) { + mutex_lock(&test_fw_mutex); release_firmware(test_firmware); misc_deregister(&test_fw_misc_device); + __test_firmware_config_free(); + kfree(test_fw_config); + mutex_unlock(&test_fw_mutex); + pr_warn("removed interface\n"); } diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh index e35691239350..7d8fd2e3695a 100755 --- a/tools/testing/selftests/firmware/fw_filesystem.sh +++ b/tools/testing/selftests/firmware/fw_filesystem.sh @@ -25,8 +25,9 @@ if [ ! -d $DIR ]; then fi # CONFIG_FW_LOADER_USER_HELPER has a sysfs class under /sys/class/firmware/ -# These days no one enables CONFIG_FW_LOADER_USER_HELPER so check for that -# as an indicator for CONFIG_FW_LOADER_USER_HELPER. +# These days most distros enable CONFIG_FW_LOADER_USER_HELPER but disable +# CONFIG_FW_LOADER_USER_HELPER_FALLBACK. We use /sys/class/firmware/ as an +# indicator for CONFIG_FW_LOADER_USER_HELPER. HAS_FW_LOADER_USER_HELPER=$(if [ -d /sys/class/firmware/ ]; then echo yes; else echo no; fi) if [ "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then @@ -116,4 +117,240 @@ else echo "$0: async filesystem loading works" fi +### Batched requests tests +test_config_present() +{ + if [ ! -f $DIR/reset ]; then + echo "Configuration triggers not present, ignoring test" + exit 0 + fi +} + +# Defaults : +# +# send_uevent: 1 +# sync_direct: 0 +# name: test-firmware.bin +# num_requests: 4 +config_reset() +{ + echo 1 > $DIR/reset +} + +release_all_firmware() +{ + echo 1 > $DIR/release_all_firmware +} + +config_set_name() +{ + echo -n $1 > $DIR/config_name +} + +config_set_sync_direct() +{ + echo 1 > $DIR/config_sync_direct +} + +config_unset_sync_direct() +{ + echo 0 > $DIR/config_sync_direct +} + +config_set_uevent() +{ + echo 1 > $DIR/config_send_uevent +} + +config_unset_uevent() +{ + echo 0 > $DIR/config_send_uevent +} + +config_trigger_sync() +{ + echo -n 1 > $DIR/trigger_batched_requests 2>/dev/null +} + +config_trigger_async() +{ + echo -n 1 > $DIR/trigger_batched_requests_async 2> /dev/null +} + +config_set_read_fw_idx() +{ + echo -n $1 > $DIR/config_read_fw_idx 2> /dev/null +} + +read_firmwares() +{ + for i in $(seq 0 3); do + config_set_read_fw_idx $i + # Verify the contents are what we expect. + # -Z required for now -- check for yourself, md5sum + # on $FW and DIR/read_firmware will yield the same. Even + # cmp agrees, so something is off. + if ! diff -q -Z "$FW" $DIR/read_firmware 2>/dev/null ; then + echo "request #$i: firmware was not loaded" >&2 + exit 1 + fi + done +} + +read_firmwares_expect_nofile() +{ + for i in $(seq 0 3); do + config_set_read_fw_idx $i + # Ensures contents differ + if diff -q -Z "$FW" $DIR/read_firmware 2>/dev/null ; then + echo "request $i: file was not expected to match" >&2 + exit 1 + fi + done +} + +test_batched_request_firmware_nofile() +{ + echo -n "Batched request_firmware() nofile try #$1: " + config_reset + config_set_name nope-test-firmware.bin + config_trigger_sync + read_firmwares_expect_nofile + release_all_firmware + echo "OK" +} + +test_batched_request_firmware_direct_nofile() +{ + echo -n "Batched request_firmware_direct() nofile try #$1: " + config_reset + config_set_name nope-test-firmware.bin + config_set_sync_direct + config_trigger_sync + release_all_firmware + echo "OK" +} + +test_request_firmware_nowait_uevent_nofile() +{ + echo -n "Batched request_firmware_nowait(uevent=true) nofile try #$1: " + config_reset + config_set_name nope-test-firmware.bin + config_trigger_async + release_all_firmware + echo "OK" +} + +test_wait_and_cancel_custom_load() +{ + if [ "$HAS_FW_LOADER_USER_HELPER" != "yes" ]; then + return + fi + local timeout=10 + name=$1 + while [ ! -e "$DIR"/"$name"/loading ]; do + sleep 0.1 + timeout=$(( $timeout - 1 )) + if [ "$timeout" -eq 0 ]; then + echo "firmware interface never appeared:" >&2 + echo "$DIR/$name/loading" >&2 + exit 1 + fi + done + echo -1 >"$DIR"/"$name"/loading +} + +test_request_firmware_nowait_custom_nofile() +{ + echo -n "Batched request_firmware_nowait(uevent=false) nofile try #$1: " + config_unset_uevent + config_set_name nope-test-firmware.bin + config_trigger_async & + test_wait_and_cancel_custom_load nope-test-firmware.bin + wait + release_all_firmware + echo "OK" +} + +test_batched_request_firmware() +{ + echo -n "Batched request_firmware() try #$1: " + config_reset + config_trigger_sync + read_firmwares + release_all_firmware + echo "OK" +} + +test_batched_request_firmware_direct() +{ + echo -n "Batched request_firmware_direct() try #$1: " + config_reset + config_set_sync_direct + config_trigger_sync + release_all_firmware + echo "OK" +} + +test_request_firmware_nowait_uevent() +{ + echo -n "Batched request_firmware_nowait(uevent=true) try #$1: " + config_reset + config_trigger_async + release_all_firmware + echo "OK" +} + +test_request_firmware_nowait_custom() +{ + echo -n "Batched request_firmware_nowait(uevent=false) try #$1: " + config_unset_uevent + config_trigger_async + release_all_firmware + echo "OK" +} + +# Only continue if batched request triggers are present on the +# test-firmware driver +test_config_present + +# test with the file present +echo +echo "Testing with the file present..." +for i in $(seq 1 5); do + test_batched_request_firmware $i +done + +for i in $(seq 1 5); do + test_batched_request_firmware_direct $i +done + +for i in $(seq 1 5); do + test_request_firmware_nowait_uevent $i +done + +for i in $(seq 1 5); do + test_request_firmware_nowait_custom $i +done + +# Test for file not found, errors are expected, the failure would be +# a hung task, which would require a hard reset. +echo +echo "Testing with the file missing..." +for i in $(seq 1 5); do + test_batched_request_firmware_nofile $i +done + +for i in $(seq 1 5); do + test_batched_request_firmware_direct_nofile $i +done + +for i in $(seq 1 5); do + test_request_firmware_nowait_uevent_nofile $i +done + +for i in $(seq 1 5); do + test_request_firmware_nowait_custom_nofile $i +done + exit 0 From 00ee84159020728cbdbfc569b42c036ef20c6e55 Mon Sep 17 00:00:00 2001 From: sayli karnik Date: Fri, 16 Jun 2017 15:39:38 +0530 Subject: [PATCH 20/28] mod_devicetable: Remove excess description from structured comment Remove excess member description to fix following warnings in sphinx build: Excess struct/union/enum/typedef member 'ver_major' description in 'fsl_mc_device_id' Excess struct/union/enum/typedef member 'ver_minor' description in 'fsl_mc_device_id' Signed-off-by: sayli karnik CC: Stuart Yoder Signed-off-by: Jonathan Corbet Signed-off-by: Greg Kroah-Hartman --- include/linux/mod_devicetable.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index 3f74ef2281e8..694cebb50f72 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -674,8 +674,6 @@ struct ulpi_device_id { * struct fsl_mc_device_id - MC object device identifier * @vendor: vendor ID * @obj_type: MC object type - * @ver_major: MC object version major number - * @ver_minor: MC object version minor number * * Type of entries in the "device Id" table for MC object devices supported by * a MC object device driver. The last entry of the table has vendor set to 0x0 From 6a7a81761bee2a6694c0a5fb1a16b263c96241c1 Mon Sep 17 00:00:00 2001 From: Jonathan Corbet Date: Thu, 24 Aug 2017 16:09:10 -0600 Subject: [PATCH 21/28] driver core: Document struct device:dma_ops Commit 5657933dbb6e (treewide: Move dma_ops from struct dev_archdata into struct device) added the dma_ops field to struct device, but did not update the kerneldoc comment, yielding this warning: ./include/linux/device.h:969: warning: No description found for parameter 'dma_ops' Add a description and bring a little peace to the world. Signed-off-by: Jonathan Corbet Reviewed-by: Bart Van Assche Signed-off-by: Greg Kroah-Hartman --- include/linux/device.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/device.h b/include/linux/device.h index 5db26b9c0fcc..d5bcc5f109eb 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -847,6 +847,7 @@ struct dev_links_info { * @msi_list: Hosts MSI descriptors * @msi_domain: The generic MSI domain this device is using. * @numa_node: NUMA node this device is close to. + * @dma_ops: DMA mapping operations for this device. * @dma_mask: Dma mask (if dma'ble device). * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all * hardware supports 64-bit addresses for consistent allocations From 0538dcb0171aad2155db405a5416d9df563e1187 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 19 Jul 2017 16:43:44 +0200 Subject: [PATCH 22/28] xen: xen-pciback: remove DRIVER_ATTR() usage It's better to be explicit and use the DRIVER_ATTR_RW() and DRIVER_ATTR_RO() macros when defining a driver's sysfs file. Bonus is this fixes up a checkpatch.pl warning. This is part of a series to drop DRIVER_ATTR() from the tree entirely. Reviewed-by: Juergen Gross Signed-off-by: Greg Kroah-Hartman --- drivers/xen/xen-pciback/pci_stub.c | 44 ++++++++++++++---------------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c index 6331a95691a4..9e480fdebe1f 100644 --- a/drivers/xen/xen-pciback/pci_stub.c +++ b/drivers/xen/xen-pciback/pci_stub.c @@ -1172,8 +1172,8 @@ static int pcistub_reg_add(int domain, int bus, int slot, int func, return err; } -static ssize_t pcistub_slot_add(struct device_driver *drv, const char *buf, - size_t count) +static ssize_t new_slot_store(struct device_driver *drv, const char *buf, + size_t count) { int domain, bus, slot, func; int err; @@ -1189,10 +1189,10 @@ static ssize_t pcistub_slot_add(struct device_driver *drv, const char *buf, err = count; return err; } -static DRIVER_ATTR(new_slot, S_IWUSR, NULL, pcistub_slot_add); +static DRIVER_ATTR_WO(new_slot); -static ssize_t pcistub_slot_remove(struct device_driver *drv, const char *buf, - size_t count) +static ssize_t remove_slot_store(struct device_driver *drv, const char *buf, + size_t count) { int domain, bus, slot, func; int err; @@ -1208,9 +1208,9 @@ static ssize_t pcistub_slot_remove(struct device_driver *drv, const char *buf, err = count; return err; } -static DRIVER_ATTR(remove_slot, S_IWUSR, NULL, pcistub_slot_remove); +static DRIVER_ATTR_WO(remove_slot); -static ssize_t pcistub_slot_show(struct device_driver *drv, char *buf) +static ssize_t slots_show(struct device_driver *drv, char *buf) { struct pcistub_device_id *pci_dev_id; size_t count = 0; @@ -1231,9 +1231,9 @@ static ssize_t pcistub_slot_show(struct device_driver *drv, char *buf) return count; } -static DRIVER_ATTR(slots, S_IRUSR, pcistub_slot_show, NULL); +static DRIVER_ATTR_RO(slots); -static ssize_t pcistub_irq_handler_show(struct device_driver *drv, char *buf) +static ssize_t irq_handlers_show(struct device_driver *drv, char *buf) { struct pcistub_device *psdev; struct xen_pcibk_dev_data *dev_data; @@ -1260,11 +1260,10 @@ static ssize_t pcistub_irq_handler_show(struct device_driver *drv, char *buf) spin_unlock_irqrestore(&pcistub_devices_lock, flags); return count; } -static DRIVER_ATTR(irq_handlers, S_IRUSR, pcistub_irq_handler_show, NULL); +static DRIVER_ATTR_RO(irq_handlers); -static ssize_t pcistub_irq_handler_switch(struct device_driver *drv, - const char *buf, - size_t count) +static ssize_t irq_handler_state_store(struct device_driver *drv, + const char *buf, size_t count) { struct pcistub_device *psdev; struct xen_pcibk_dev_data *dev_data; @@ -1301,11 +1300,10 @@ static ssize_t pcistub_irq_handler_switch(struct device_driver *drv, err = count; return err; } -static DRIVER_ATTR(irq_handler_state, S_IWUSR, NULL, - pcistub_irq_handler_switch); +static DRIVER_ATTR_WO(irq_handler_state); -static ssize_t pcistub_quirk_add(struct device_driver *drv, const char *buf, - size_t count) +static ssize_t quirks_store(struct device_driver *drv, const char *buf, + size_t count) { int domain, bus, slot, func, reg, size, mask; int err; @@ -1323,7 +1321,7 @@ static ssize_t pcistub_quirk_add(struct device_driver *drv, const char *buf, return err; } -static ssize_t pcistub_quirk_show(struct device_driver *drv, char *buf) +static ssize_t quirks_show(struct device_driver *drv, char *buf) { int count = 0; unsigned long flags; @@ -1366,11 +1364,10 @@ static ssize_t pcistub_quirk_show(struct device_driver *drv, char *buf) return count; } -static DRIVER_ATTR(quirks, S_IRUSR | S_IWUSR, pcistub_quirk_show, - pcistub_quirk_add); +static DRIVER_ATTR_RW(quirks); -static ssize_t permissive_add(struct device_driver *drv, const char *buf, - size_t count) +static ssize_t permissive_store(struct device_driver *drv, const char *buf, + size_t count) { int domain, bus, slot, func; int err; @@ -1431,8 +1428,7 @@ static ssize_t permissive_show(struct device_driver *drv, char *buf) spin_unlock_irqrestore(&pcistub_devices_lock, flags); return count; } -static DRIVER_ATTR(permissive, S_IRUSR | S_IWUSR, permissive_show, - permissive_add); +static DRIVER_ATTR_RW(permissive); static void pcistub_exit(void) { From b5f0dc7b4f30d08cb56c3a14f555083846ebd2fc Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 19 Jul 2017 14:51:19 +0200 Subject: [PATCH 23/28] fbdev: uvesafb: remove DRIVER_ATTR() usage It's better to be explicit and use the DRIVER_ATTR_RW() macro when defining a driver's sysfs file. Bonus is this fixes up a checkpatch.pl warning. This is part of a series to drop DRIVER_ATTR() from the tree entirely. Acked-by: Bartlomiej Zolnierkiewicz Signed-off-by: Greg Kroah-Hartman --- drivers/video/fbdev/uvesafb.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/video/fbdev/uvesafb.c b/drivers/video/fbdev/uvesafb.c index dc0e8d90d9cc..6f8c0b9fc558 100644 --- a/drivers/video/fbdev/uvesafb.c +++ b/drivers/video/fbdev/uvesafb.c @@ -1860,19 +1860,18 @@ static int uvesafb_setup(char *options) } #endif /* !MODULE */ -static ssize_t show_v86d(struct device_driver *dev, char *buf) +static ssize_t v86d_show(struct device_driver *dev, char *buf) { return snprintf(buf, PAGE_SIZE, "%s\n", v86d_path); } -static ssize_t store_v86d(struct device_driver *dev, const char *buf, +static ssize_t v86d_store(struct device_driver *dev, const char *buf, size_t count) { strncpy(v86d_path, buf, PATH_MAX); return count; } - -static DRIVER_ATTR(v86d, S_IRUGO | S_IWUSR, show_v86d, store_v86d); +static DRIVER_ATTR_RW(v86d); static int uvesafb_init(void) { From 39bf04db6b2b13eeb2aa565930034dc9d4198d93 Mon Sep 17 00:00:00 2001 From: Waiman Long Date: Thu, 17 Aug 2017 11:49:56 -0400 Subject: [PATCH 24/28] kernfs: Clarify lockdep name for kn->count The reference count in kernfs_node structure is treated like a rwsem by using lockdep instrumentation code. The lockdep name, however, is still "s_active" which is carried over from the old sysfs code. As s_active is no longer the variable name, its use may confuse users on where the lock is when it is reported by lockdep. So it is changed to "kn->count" which is how this variable is normally referenced in kernfs code. Signed-off-by: Waiman Long Acked-by: Tejun Heo Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c index ac2dfe0c5a9c..e6c8954a4e89 100644 --- a/fs/kernfs/file.c +++ b/fs/kernfs/file.c @@ -997,7 +997,7 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent, #ifdef CONFIG_DEBUG_LOCK_ALLOC if (key) { - lockdep_init_map(&kn->dep_map, "s_active", key, 0); + lockdep_init_map(&kn->dep_map, "kn->count", key, 0); kn->flags |= KERNFS_LOCKDEP; } #endif From 6ef2541f268f56a4b93acb0f921c8fc4b74bfc9f Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Tue, 18 Jul 2017 16:42:49 -0500 Subject: [PATCH 25/28] base: Convert to using %pOF instead of full_name Now that we have a custom printf format specifier, convert users of full_name to use %pOF instead. This is preparation to remove storing of the full path string for each node. Signed-off-by: Rob Herring Cc: Greg Kroah-Hartman Signed-off-by: Greg Kroah-Hartman --- drivers/base/arch_topology.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index 74043ead9da1..41be9ff7d70a 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -150,12 +150,12 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) } capacity_scale = max(cpu_capacity, capacity_scale); raw_capacity[cpu] = cpu_capacity; - pr_debug("cpu_capacity: %s cpu_capacity=%u (raw)\n", - cpu_node->full_name, raw_capacity[cpu]); + pr_debug("cpu_capacity: %pOF cpu_capacity=%u (raw)\n", + cpu_node, raw_capacity[cpu]); } else { if (raw_capacity) { - pr_err("cpu_capacity: missing %s raw capacity\n", - cpu_node->full_name); + pr_err("cpu_capacity: missing %pOF raw capacity\n", + cpu_node); pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n"); } cap_parsing_failed = true; From df44d30d28cb7dd4f3dc105d843a4a01f5dfc860 Mon Sep 17 00:00:00 2001 From: Arvind Yadav Date: Wed, 2 Aug 2017 19:36:20 +0530 Subject: [PATCH 26/28] base: topology: constify attribute_group structures. attribute_group are not supposed to change at runtime. All functions working with attribute_group provided by work with const attribute_group. So mark the non-const structs as const. Signed-off-by: Arvind Yadav Signed-off-by: Greg Kroah-Hartman --- drivers/base/topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/topology.c b/drivers/base/topology.c index d6ec1c546f5b..d936fcf9f1fb 100644 --- a/drivers/base/topology.c +++ b/drivers/base/topology.c @@ -105,7 +105,7 @@ static struct attribute *default_attrs[] = { NULL }; -static struct attribute_group topology_attr_group = { +static const struct attribute_group topology_attr_group = { .attrs = default_attrs, .name = "topology" }; From 7521621e600aeefe5ffcc1f90ae26a42fc20c452 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Fri, 11 Aug 2017 15:44:43 +0200 Subject: [PATCH 27/28] Do not disable driver and bus shutdown hook when class shutdown hook is set. As seen from the implementation of the single class shutdown hook this is not very sound design. Rename the class shutdown hook to shutdown_pre to make it clear it runs before the driver shutdown hook. Signed-off-by: Michal Suchanek Reviewed-by: Jarkko Sakkinen Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 9 +++++---- drivers/char/tpm/tpm-chip.c | 11 ++--------- include/linux/device.h | 4 ++-- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index cb4b5b1f1b09..12ebd055724c 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2796,11 +2796,12 @@ void device_shutdown(void) pm_runtime_get_noresume(dev); pm_runtime_barrier(dev); - if (dev->class && dev->class->shutdown) { + if (dev->class && dev->class->shutdown_pre) { if (initcall_debug) - dev_info(dev, "shutdown\n"); - dev->class->shutdown(dev); - } else if (dev->bus && dev->bus->shutdown) { + dev_info(dev, "shutdown_pre\n"); + dev->class->shutdown_pre(dev); + } + if (dev->bus && dev->bus->shutdown) { if (initcall_debug) dev_info(dev, "shutdown\n"); dev->bus->shutdown(dev); diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index 67ec9d3d04f5..0eca20c5a80c 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -164,14 +164,7 @@ static int tpm_class_shutdown(struct device *dev) chip->ops = NULL; up_write(&chip->ops_sem); } - /* Allow bus- and device-specific code to run. Note: since chip->ops - * is NULL, more-specific shutdown code will not be able to issue TPM - * commands. - */ - if (dev->bus && dev->bus->shutdown) - dev->bus->shutdown(dev); - else if (dev->driver && dev->driver->shutdown) - dev->driver->shutdown(dev); + return 0; } @@ -214,7 +207,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev, device_initialize(&chip->devs); chip->dev.class = tpm_class; - chip->dev.class->shutdown = tpm_class_shutdown; + chip->dev.class->shutdown_pre = tpm_class_shutdown; chip->dev.release = tpm_dev_release; chip->dev.parent = pdev; chip->dev.groups = chip->groups; diff --git a/include/linux/device.h b/include/linux/device.h index d5bcc5f109eb..c6f27207dbe8 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -375,7 +375,7 @@ int subsys_virtual_register(struct bus_type *subsys, * @suspend: Used to put the device to sleep mode, usually to a low power * state. * @resume: Used to bring the device from the sleep mode. - * @shutdown: Called at shut-down time to quiesce the device. + * @shutdown_pre: Called at shut-down time before driver shutdown. * @ns_type: Callbacks so sysfs can detemine namespaces. * @namespace: Namespace of the device belongs to this class. * @pm: The default device power management operations of this class. @@ -404,7 +404,7 @@ struct class { int (*suspend)(struct device *dev, pm_message_t state); int (*resume)(struct device *dev); - int (*shutdown)(struct device *dev); + int (*shutdown_pre)(struct device *dev); const struct kobj_ns_type_operations *ns_type; const void *(*namespace)(struct device *dev); From 0f9b011d3321ca1079c7a46c18cb1956fbdb7bcb Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Tue, 29 Aug 2017 21:23:49 +0200 Subject: [PATCH 28/28] driver core: bus: Fix a potential double free The .release function of driver_ktype is 'driver_release()'. This function frees the container_of this kobject. So, this memory must not be freed explicitly in the error handling path of 'bus_add_driver()'. Otherwise a double free will occur. Signed-off-by: Christophe JAILLET Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/base/bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/bus.c b/drivers/base/bus.c index e162c9a789ba..22a64fd3309b 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -698,7 +698,7 @@ int bus_add_driver(struct device_driver *drv) out_unregister: kobject_put(&priv->kobj); - kfree(drv->p); + /* drv->p is freed in driver_release() */ drv->p = NULL; out_put_bus: bus_put(bus);