From 515db266a9dace92b0cbaed9a6044dd5304b8ca9 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 16 Jul 2019 17:21:06 +0200 Subject: [PATCH 01/49] driver core: Remove device link creation limitation If device_link_add() is called for a consumer/supplier pair with an existing device link between them and the existing link's type is not in agreement with the flags passed to that function by its caller, NULL will be returned. That is seriously inconvenient, because it forces the callers of device_link_add() to worry about what others may or may not do even if that is not relevant to them for any other reasons. It turns out, however, that this limitation can be made go away relatively easily. The underlying observation is that if DL_FLAG_STATELESS has been passed to device_link_add() in flags for the given consumer/supplier pair at least once, calling either device_link_del() or device_link_remove() to release the link returned by it should work, but there are no other requirements associated with that flag. In turn, if at least one of the callers of device_link_add() for the given consumer/supplier pair has not passed DL_FLAG_STATELESS to it in flags, the driver core should track the status of the link and act on it as appropriate (ie. the link should be treated as "managed"). This means that DL_FLAG_STATELESS needs to be set for managed device links and it should be valid to call device_link_del() or device_link_remove() to drop references to them in certain sutiations. To allow that to happen, introduce a new (internal) device link flag called DL_FLAG_MANAGED and make device_link_add() set it automatically whenever DL_FLAG_STATELESS is not passed to it. Also make it take additional references to existing device links that were previously stateless (that is, with DL_FLAG_STATELESS set and DL_FLAG_MANAGED unset) and will need to be managed going forward and initialize their status (which has been DL_STATE_NONE so far). Accordingly, when a managed device link is dropped automatically by the driver core, make it clear DL_FLAG_MANAGED, reset the link's status back to DL_STATE_NONE and drop the reference to it associated with DL_FLAG_MANAGED instead of just deleting it right away (to allow it to stay around in case it still needs to be released explicitly by someone). With that, since setting DL_FLAG_STATELESS doesn't mean that the device link in question is not managed any more, replace all of the status-tracking checks against DL_FLAG_STATELESS with analogous checks against DL_FLAG_MANAGED and update the documentation to reflect these changes. While at it, make device_link_add() reject flags that it does not recognize, including DL_FLAG_MANAGED. Signed-off-by: Rafael J. Wysocki Reviewed-by: Saravana Kannan Tested-by: Marek Szyprowski Review-by: Saravana Kannan Link: https://lore.kernel.org/r/2305283.AStDPdUUnE@kreacher Signed-off-by: Greg Kroah-Hartman --- Documentation/driver-api/device_link.rst | 4 +- drivers/base/core.c | 176 +++++++++++++---------- drivers/base/power/runtime.c | 4 +- include/linux/device.h | 4 +- 4 files changed, 106 insertions(+), 82 deletions(-) diff --git a/Documentation/driver-api/device_link.rst b/Documentation/driver-api/device_link.rst index ae1e3d0394b0..1b5020ec6517 100644 --- a/Documentation/driver-api/device_link.rst +++ b/Documentation/driver-api/device_link.rst @@ -78,8 +78,8 @@ typically deleted in its ``->remove`` callback for symmetry. That way, if the driver is compiled as a module, the device link is added on module load and orderly deleted on unload. The same restrictions that apply to device link addition (e.g. exclusion of a parallel suspend/resume transition) apply equally -to deletion. Device links with ``DL_FLAG_STATELESS`` unset (i.e. managed -device links) are deleted automatically by the driver core. +to deletion. Device links managed by the driver core are deleted automatically +by it. Several flags may be specified on device link addition, two of which have already been mentioned above: ``DL_FLAG_STATELESS`` to express that no diff --git a/drivers/base/core.c b/drivers/base/core.c index da84a73f2ba6..21cd08162219 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -124,6 +124,50 @@ static int device_is_dependent(struct device *dev, void *target) return ret; } +static void device_link_init_status(struct device_link *link, + struct device *consumer, + struct device *supplier) +{ + switch (supplier->links.status) { + case DL_DEV_PROBING: + switch (consumer->links.status) { + case DL_DEV_PROBING: + /* + * 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). + */ + 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: + link->status = DL_STATE_ACTIVE; + break; + default: + link->status = DL_STATE_AVAILABLE; + break; + } + break; + case DL_DEV_UNBINDING: + link->status = DL_STATE_SUPPLIER_UNBIND; + break; + default: + link->status = DL_STATE_DORMANT; + break; + } +} + static int device_reorder_to_tail(struct device *dev, void *not_used) { struct device_link *link; @@ -165,6 +209,10 @@ void device_pm_move_to_tail(struct device *dev) device_links_read_unlock(idx); } +#define DL_MANAGED_LINK_FLAGS (DL_FLAG_AUTOREMOVE_CONSUMER | \ + DL_FLAG_AUTOREMOVE_SUPPLIER | \ + DL_FLAG_AUTOPROBE_CONSUMER) + /** * device_link_add - Create a link between two devices. * @consumer: Consumer end of the link. @@ -179,9 +227,9 @@ 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 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 DL_FLAG_STATELESS is set in @flags, the caller of this function is + * expected to release the link returned by it directly with the help of either + * device_link_del() or device_link_remove(). * * 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 @@ -201,9 +249,16 @@ void device_pm_move_to_tail(struct device *dev) * 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. + * The combination of DL_FLAG_STATELESS and one of DL_FLAG_AUTOREMOVE_CONSUMER, + * DL_FLAG_AUTOREMOVE_SUPPLIER, or DL_FLAG_AUTOPROBE_CONSUMER set in @flags at + * the same time is invalid and will cause NULL to be returned upfront. + * However, if a device link between the given @consumer and @supplier pair + * exists already when this function is called for them, the existing link will + * be returned regardless of its current type and status (the link's flags may + * be modified then). The caller of this function is then expected to treat + * the link as though it has just been created, so (in particular) if + * DL_FLAG_STATELESS was passed in @flags, the link needs to be released + * explicitly when not needed any more (as stated above). * * 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 @@ -220,10 +275,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 | - DL_FLAG_AUTOREMOVE_SUPPLIER | - DL_FLAG_AUTOPROBE_CONSUMER)) || + (flags & ~(DL_FLAG_STATELESS | DL_MANAGED_LINK_FLAGS)) || + (flags & DL_FLAG_STATELESS && flags & DL_MANAGED_LINK_FLAGS) || (flags & DL_FLAG_AUTOPROBE_CONSUMER && flags & (DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_AUTOREMOVE_SUPPLIER))) @@ -236,6 +289,9 @@ struct device_link *device_link_add(struct device *consumer, } } + if (!(flags & DL_FLAG_STATELESS)) + flags |= DL_FLAG_MANAGED; + device_links_write_lock(); device_pm_lock(); @@ -262,15 +318,6 @@ struct device_link *device_link_add(struct device *consumer, 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_PM_RUNTIME) { if (!(link->flags & DL_FLAG_PM_RUNTIME)) { pm_runtime_new_link(consumer); @@ -281,6 +328,7 @@ struct device_link *device_link_add(struct device *consumer, } if (flags & DL_FLAG_STATELESS) { + link->flags |= DL_FLAG_STATELESS; kref_get(&link->kref); goto out; } @@ -299,6 +347,11 @@ struct device_link *device_link_add(struct device *consumer, link->flags &= ~(DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_AUTOREMOVE_SUPPLIER); } + if (!(link->flags & DL_FLAG_MANAGED)) { + kref_get(&link->kref); + link->flags |= DL_FLAG_MANAGED; + device_link_init_status(link, consumer, supplier); + } goto out; } @@ -325,48 +378,10 @@ struct device_link *device_link_add(struct device *consumer, kref_init(&link->kref); /* Determine the initial link state. */ - if (flags & DL_FLAG_STATELESS) { + if (flags & DL_FLAG_STATELESS) link->status = DL_STATE_NONE; - } else { - switch (supplier->links.status) { - case DL_DEV_PROBING: - switch (consumer->links.status) { - case DL_DEV_PROBING: - /* - * 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). - */ - 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: - link->status = DL_STATE_ACTIVE; - break; - default: - link->status = DL_STATE_AVAILABLE; - break; - } - break; - case DL_DEV_UNBINDING: - link->status = DL_STATE_SUPPLIER_UNBIND; - break; - default: - link->status = DL_STATE_DORMANT; - break; - } - } + else + device_link_init_status(link, consumer, supplier); /* * Some callers expect the link creation during consumer driver probe to @@ -528,7 +543,7 @@ static void device_links_missing_supplier(struct device *dev) * mark the link as "consumer probe in progress" to make the supplier removal * wait for us to complete (or bad things may happen). * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ int device_links_check_suppliers(struct device *dev) { @@ -538,7 +553,7 @@ int device_links_check_suppliers(struct device *dev) device_links_write_lock(); list_for_each_entry(link, &dev->links.suppliers, c_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; if (link->status != DL_STATE_AVAILABLE) { @@ -563,7 +578,7 @@ int device_links_check_suppliers(struct device *dev) * * Also change the status of @dev's links to suppliers to "active". * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ void device_links_driver_bound(struct device *dev) { @@ -572,7 +587,7 @@ void device_links_driver_bound(struct device *dev) device_links_write_lock(); list_for_each_entry(link, &dev->links.consumers, s_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; /* @@ -593,7 +608,7 @@ void device_links_driver_bound(struct device *dev) } list_for_each_entry(link, &dev->links.suppliers, c_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; WARN_ON(link->status != DL_STATE_CONSUMER_PROBE); @@ -605,6 +620,13 @@ void device_links_driver_bound(struct device *dev) device_links_write_unlock(); } +static void device_link_drop_managed(struct device_link *link) +{ + link->flags &= ~DL_FLAG_MANAGED; + WRITE_ONCE(link->status, DL_STATE_NONE); + kref_put(&link->kref, __device_link_del); +} + /** * __device_links_no_driver - Update links of a device without a driver. * @dev: Device without a drvier. @@ -615,18 +637,18 @@ void device_links_driver_bound(struct device *dev) * unless they already are in the "supplier unbind in progress" state in which * case they need not be updated. * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ static void __device_links_no_driver(struct device *dev) { struct device_link *link, *ln; list_for_each_entry_safe_reverse(link, ln, &dev->links.suppliers, c_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) - __device_link_del(&link->kref); + device_link_drop_managed(link); else if (link->status == DL_STATE_CONSUMER_PROBE || link->status == DL_STATE_ACTIVE) WRITE_ONCE(link->status, DL_STATE_AVAILABLE); @@ -643,7 +665,7 @@ static void __device_links_no_driver(struct device *dev) * %__device_links_no_driver() to update links to suppliers for it as * appropriate. * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ void device_links_no_driver(struct device *dev) { @@ -652,7 +674,7 @@ void device_links_no_driver(struct device *dev) device_links_write_lock(); list_for_each_entry(link, &dev->links.consumers, s_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; /* @@ -680,7 +702,7 @@ void device_links_no_driver(struct device *dev) * invoke %__device_links_no_driver() to update links to suppliers for it as * appropriate. * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ void device_links_driver_cleanup(struct device *dev) { @@ -689,7 +711,7 @@ void device_links_driver_cleanup(struct device *dev) device_links_write_lock(); list_for_each_entry_safe(link, ln, &dev->links.consumers, s_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; WARN_ON(link->flags & DL_FLAG_AUTOREMOVE_CONSUMER); @@ -702,7 +724,7 @@ void device_links_driver_cleanup(struct device *dev) */ if (link->status == DL_STATE_SUPPLIER_UNBIND && link->flags & DL_FLAG_AUTOREMOVE_SUPPLIER) - __device_link_del(&link->kref); + device_link_drop_managed(link); WRITE_ONCE(link->status, DL_STATE_DORMANT); } @@ -724,7 +746,7 @@ void device_links_driver_cleanup(struct device *dev) * * Return 'false' if there are no probing or active consumers. * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ bool device_links_busy(struct device *dev) { @@ -734,7 +756,7 @@ bool device_links_busy(struct device *dev) device_links_write_lock(); list_for_each_entry(link, &dev->links.consumers, s_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; if (link->status == DL_STATE_CONSUMER_PROBE @@ -764,7 +786,7 @@ bool device_links_busy(struct device *dev) * driver to unbind and start over (the consumer will not re-probe as we have * changed the state of the link already). * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ void device_links_unbind_consumers(struct device *dev) { @@ -776,7 +798,7 @@ void device_links_unbind_consumers(struct device *dev) list_for_each_entry(link, &dev->links.consumers, s_node) { enum device_link_state status; - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; status = link->status; diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index b75335508d2c..45a8fbe6987a 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1624,7 +1624,7 @@ void pm_runtime_remove(struct device *dev) * runtime PM references to the device, drop the usage counter of the device * (as many times as needed). * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links with the DL_FLAG_MANAGED flag unset are ignored. * * Since the device is guaranteed to be runtime-active at the point this is * called, nothing else needs to be done here. @@ -1641,7 +1641,7 @@ void pm_runtime_clean_up_links(struct device *dev) idx = device_links_read_lock(); list_for_each_entry_rcu(link, &dev->links.consumers, s_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; while (refcount_dec_not_one(&link->rpm_active)) diff --git a/include/linux/device.h b/include/linux/device.h index c330b75c6c57..c1c489921e4c 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -833,12 +833,13 @@ enum device_link_state { /* * Device link flags. * - * STATELESS: The core won't track the presence of supplier/consumer drivers. + * STATELESS: The core will not remove this link automatically. * AUTOREMOVE_CONSUMER: Remove the link automatically on consumer driver unbind. * PM_RUNTIME: If set, the runtime PM framework will use this link. * RPM_ACTIVE: Run pm_runtime_get_sync() on the supplier during link creation. * AUTOREMOVE_SUPPLIER: Remove the link automatically on supplier driver unbind. * AUTOPROBE_CONSUMER: Probe consumer driver automatically after supplier binds. + * MANAGED: The core tracks presence of supplier/consumer drivers (internal). */ #define DL_FLAG_STATELESS BIT(0) #define DL_FLAG_AUTOREMOVE_CONSUMER BIT(1) @@ -846,6 +847,7 @@ enum device_link_state { #define DL_FLAG_RPM_ACTIVE BIT(3) #define DL_FLAG_AUTOREMOVE_SUPPLIER BIT(4) #define DL_FLAG_AUTOPROBE_CONSUMER BIT(5) +#define DL_FLAG_MANAGED BIT(6) /** * struct device_link - Device link representation. From 2fd60da46da76948b8cc561ae8cc902209686253 Mon Sep 17 00:00:00 2001 From: Peng Wang Date: Mon, 8 Jul 2019 23:16:11 +0800 Subject: [PATCH 02/49] kernfs: fix potential null pointer dereference Get root safely after kn is ensureed to be not null. Signed-off-by: Peng Wang Acked-by: Tejun Heo Link: https://lore.kernel.org/r/20190708151611.13242-1-rocking@whu.edu.cn Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/dir.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index a387534c9577..ddf537923a0a 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -430,7 +430,6 @@ struct kernfs_node *kernfs_get_active(struct kernfs_node *kn) */ void kernfs_put_active(struct kernfs_node *kn) { - struct kernfs_root *root = kernfs_root(kn); int v; if (unlikely(!kn)) @@ -442,7 +441,7 @@ void kernfs_put_active(struct kernfs_node *kn) if (likely(v != KN_DEACTIVATED_BIAS)) return; - wake_up_all(&root->deactivate_waitq); + wake_up_all(&kernfs_root(kn)->deactivate_waitq); } /** From bbe70e4e421116db0af3acf84cbce3c3f6cac53e Mon Sep 17 00:00:00 2001 From: Jia-Ju Bai Date: Wed, 24 Jul 2019 10:22:42 +0800 Subject: [PATCH 03/49] fs: kernfs: Fix possible null-pointer dereferences in kernfs_path_from_node_locked() In kernfs_path_from_node_locked(), there is an if statement on line 147 to check whether buf is NULL: if (buf) When buf is NULL, it is used on line 151: len += strlcpy(buf + len, parent_str, ...) and line 158: len += strlcpy(buf + len, "/", ...) and line 160: len += strlcpy(buf + len, kn->name, ...) Thus, possible null-pointer dereferences may occur. To fix these possible bugs, buf is checked before being used. If it is NULL, -EINVAL is returned. These bugs are found by a static analysis tool STCheck written by us. Signed-off-by: Jia-Ju Bai Link: https://lore.kernel.org/r/20190724022242.27505-1-baijiaju1990@gmail.com Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/dir.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index ddf537923a0a..6ebae6bbe6a5 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -137,6 +137,9 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to, if (kn_from == kn_to) return strlcpy(buf, "/", buflen); + if (!buf) + return -EINVAL; + common = kernfs_common_ancestor(kn_from, kn_to); if (WARN_ON(!common)) return -EINVAL; @@ -144,8 +147,7 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to, depth_to = kernfs_depth(common, kn_to); depth_from = kernfs_depth(common, kn_from); - if (buf) - buf[0] = '\0'; + buf[0] = '\0'; for (i = 0; i < depth_from; i++) len += strlcpy(buf + len, parent_str, From fb583c8eeeb1fd57e24ef41ed94c9112067aeac9 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 30 Jul 2019 11:28:57 +0200 Subject: [PATCH 04/49] driver core: Fix creation of device links with PM-runtime flags After commit 515db266a9da ("driver core: Remove device link creation limitation"), if PM-runtime flags are passed to device_link_add(), it will fail (returning NULL) due to an overly restrictive flags check introduced by that commit. Fix this issue by extending the check in question to cover the PM-runtime flags too. Fixes: 515db266a9da ("driver core: Remove device link creation limitation") Reported-by: Dmitry Osipenko Tested-by: Jon Hunter Signed-off-by: Rafael J. Wysocki Tested-by: Dmitry Osipenko Tested-by: Marek Szyprowski Link: https://lore.kernel.org/r/7674989.cD04D8YV3U@kreacher Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 21cd08162219..08eecab2497b 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -213,6 +213,9 @@ void device_pm_move_to_tail(struct device *dev) DL_FLAG_AUTOREMOVE_SUPPLIER | \ DL_FLAG_AUTOPROBE_CONSUMER) +#define DL_ADD_VALID_FLAGS (DL_MANAGED_LINK_FLAGS | DL_FLAG_STATELESS | \ + DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE) + /** * device_link_add - Create a link between two devices. * @consumer: Consumer end of the link. @@ -274,8 +277,7 @@ struct device_link *device_link_add(struct device *consumer, { struct device_link *link; - if (!consumer || !supplier || - (flags & ~(DL_FLAG_STATELESS | DL_MANAGED_LINK_FLAGS)) || + if (!consumer || !supplier || flags & ~DL_ADD_VALID_FLAGS || (flags & DL_FLAG_STATELESS && flags & DL_MANAGED_LINK_FLAGS) || (flags & DL_FLAG_AUTOPROBE_CONSUMER && flags & (DL_FLAG_AUTOREMOVE_CONSUMER | From 7723f4c5ecdb8d832f049f8483beb0d1081cedf6 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Mon, 29 Jul 2019 22:38:43 -0700 Subject: [PATCH 05/49] driver core: platform: Add an error message to platform_get_irq*() A grep of the kernel shows that many drivers print an error message if they fail to get the irq they're looking for. Furthermore, those drivers all decide to print the device name, or not, and the irq they were requesting, or not, etc. Let's consolidate all these error messages into the API itself, allowing us to get rid of the error messages in each driver. Cc: Rob Herring Cc: Bartlomiej Zolnierkiewicz Cc: Javier Martinez Canillas Cc: Andrzej Hajda Cc: Mark Brown Cc: Russell King - ARM Linux Cc: Marek Szyprowski Cc: Markus Elfring Reviewed-by: Rafael J. Wysocki Reviewed-by: Andy Shevchenko Signed-off-by: Stephen Boyd Link: https://lore.kernel.org/r/20190730053845.126834-2-swboyd@chromium.org Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 42 +++++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index a174ce5ea17c..9426736551b5 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -99,12 +99,7 @@ void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev, EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource); #endif /* CONFIG_HAS_IOMEM */ -/** - * platform_get_irq - get an IRQ for a device - * @dev: platform device - * @num: IRQ number index - */ -int platform_get_irq(struct platform_device *dev, unsigned int num) +static int __platform_get_irq(struct platform_device *dev, unsigned int num) { #ifdef CONFIG_SPARC /* sparc does not have irqs represented as IORESOURCE_IRQ resources */ @@ -163,6 +158,33 @@ int platform_get_irq(struct platform_device *dev, unsigned int num) return -ENXIO; #endif } + +/** + * platform_get_irq - get an IRQ for a device + * @dev: platform device + * @num: IRQ number index + * + * Gets an IRQ for a platform device and prints an error message if finding the + * IRQ fails. Device drivers should check the return value for errors so as to + * not pass a negative integer value to the request_irq() APIs. + * + * Example: + * int irq = platform_get_irq(pdev, 0); + * if (irq < 0) + * return irq; + * + * Return: IRQ number on success, negative error number on failure. + */ +int platform_get_irq(struct platform_device *dev, unsigned int num) +{ + int ret; + + ret = __platform_get_irq(dev, num); + if (ret < 0 && ret != -EPROBE_DEFER) + dev_err(&dev->dev, "IRQ index %u not found\n", num); + + return ret; +} EXPORT_SYMBOL_GPL(platform_get_irq); /** @@ -175,7 +197,7 @@ int platform_irq_count(struct platform_device *dev) { int ret, nr = 0; - while ((ret = platform_get_irq(dev, nr)) >= 0) + while ((ret = __platform_get_irq(dev, nr)) >= 0) nr++; if (ret == -EPROBE_DEFER) @@ -228,7 +250,11 @@ int platform_get_irq_byname(struct platform_device *dev, const char *name) } r = platform_get_resource_byname(dev, IORESOURCE_IRQ, name); - return r ? r->start : -ENXIO; + if (r) + return r->start; + + dev_err(&dev->dev, "IRQ %s not found\n", name); + return -ENXIO; } EXPORT_SYMBOL_GPL(platform_get_irq_byname); From 98051ba2b28be8a8ec5e22c01f464cff2d6d5f7c Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Mon, 29 Jul 2019 22:38:45 -0700 Subject: [PATCH 06/49] coccinelle: Add script to check for platform_get_irq() excessive prints Add a coccinelle script to check for the usage of dev_err() after a call to platform_get_irq{,_byname}() as it's redundant now that the function already prints an error when it fails. Cc: Greg Kroah-Hartman Cc: Rob Herring Cc: Bartlomiej Zolnierkiewicz Cc: Javier Martinez Canillas Cc: Andrzej Hajda Cc: Mark Brown Cc: Russell King - ARM Linux Cc: Marek Szyprowski Cc: Rafael J. Wysocki Cc: Andy Shevchenko Cc: Markus Elfring Signed-off-by: Stephen Boyd Link: https://lore.kernel.org/r/20190730053845.126834-4-swboyd@chromium.org Signed-off-by: Greg Kroah-Hartman --- scripts/coccinelle/api/platform_get_irq.cocci | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 scripts/coccinelle/api/platform_get_irq.cocci diff --git a/scripts/coccinelle/api/platform_get_irq.cocci b/scripts/coccinelle/api/platform_get_irq.cocci new file mode 100644 index 000000000000..f6e1afc08c0b --- /dev/null +++ b/scripts/coccinelle/api/platform_get_irq.cocci @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0 +/// Remove dev_err() messages after platform_get_irq*() failures +// +// Confidence: Medium +// Options: --include-headers + +virtual patch +virtual context +virtual org +virtual report + +@depends on context@ +expression ret; +struct platform_device *E; +@@ + +ret = +( +platform_get_irq +| +platform_get_irq_byname +)(E, ...); + +if ( ret \( < \| <= \) 0 ) +{ +( +if (ret != -EPROBE_DEFER) +{ ... +*dev_err(...); +... } +| +... +*dev_err(...); +) +... +} + +@depends on patch@ +expression ret; +struct platform_device *E; +@@ + +ret = +( +platform_get_irq +| +platform_get_irq_byname +)(E, ...); + +if ( ret \( < \| <= \) 0 ) +{ +( +-if (ret != -EPROBE_DEFER) +-{ ... +-dev_err(...); +-... } +| +... +-dev_err(...); +) +... +} + +@r depends on org || report@ +position p1; +expression ret; +struct platform_device *E; +@@ + +ret = +( +platform_get_irq +| +platform_get_irq_byname +)(E, ...); + +if ( ret \( < \| <= \) 0 ) +{ +( +if (ret != -EPROBE_DEFER) +{ ... +dev_err@p1(...); +... } +| +... +dev_err@p1(...); +) +... +} + +@script:python depends on org@ +p1 << r.p1; +@@ + +cocci.print_main(p1) + +@script:python depends on report@ +p1 << r.p1; +@@ + +msg = "line %s is redundant because platform_get_irq() already prints an error" % (p1[0].line) +coccilib.report.print_report(p1[0],msg) From 45640a383adc83331250f6fc7bf7e2c04f3e7879 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Sat, 6 Jul 2019 18:47:20 +0200 Subject: [PATCH 07/49] mfd: ab3100: no need to check return value of debugfs_create functions When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this. Cc: Linus Walleij Cc: Lee Jones Cc: linux-arm-kernel@lists.infradead.org Link: https://lore.kernel.org/r/20190706164722.18766-1-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- drivers/mfd/ab3100-core.c | 45 ++++++--------------------------------- drivers/mfd/ab3100-otp.c | 21 ++++++------------ 2 files changed, 13 insertions(+), 53 deletions(-) diff --git a/drivers/mfd/ab3100-core.c b/drivers/mfd/ab3100-core.c index e350ab64238e..9f3dbc31d3e9 100644 --- a/drivers/mfd/ab3100-core.c +++ b/drivers/mfd/ab3100-core.c @@ -575,58 +575,27 @@ static const struct file_operations ab3100_get_set_reg_fops = { .llseek = noop_llseek, }; -static struct dentry *ab3100_dir; -static struct dentry *ab3100_reg_file; static struct ab3100_get_set_reg_priv ab3100_get_priv; -static struct dentry *ab3100_get_reg_file; static struct ab3100_get_set_reg_priv ab3100_set_priv; -static struct dentry *ab3100_set_reg_file; static void ab3100_setup_debugfs(struct ab3100 *ab3100) { - int err; + struct dentry *ab3100_dir; ab3100_dir = debugfs_create_dir("ab3100", NULL); - if (!ab3100_dir) - goto exit_no_debugfs; - ab3100_reg_file = debugfs_create_file("registers", - S_IRUGO, ab3100_dir, ab3100, - &ab3100_registers_fops); - if (!ab3100_reg_file) { - err = -ENOMEM; - goto exit_destroy_dir; - } + debugfs_create_file("registers", S_IRUGO, ab3100_dir, ab3100, + &ab3100_registers_fops); ab3100_get_priv.ab3100 = ab3100; ab3100_get_priv.mode = false; - ab3100_get_reg_file = debugfs_create_file("get_reg", - S_IWUSR, ab3100_dir, &ab3100_get_priv, - &ab3100_get_set_reg_fops); - if (!ab3100_get_reg_file) { - err = -ENOMEM; - goto exit_destroy_reg; - } + debugfs_create_file("get_reg", S_IWUSR, ab3100_dir, &ab3100_get_priv, + &ab3100_get_set_reg_fops); ab3100_set_priv.ab3100 = ab3100; ab3100_set_priv.mode = true; - ab3100_set_reg_file = debugfs_create_file("set_reg", - S_IWUSR, ab3100_dir, &ab3100_set_priv, - &ab3100_get_set_reg_fops); - if (!ab3100_set_reg_file) { - err = -ENOMEM; - goto exit_destroy_get_reg; - } - return; - - exit_destroy_get_reg: - debugfs_remove(ab3100_get_reg_file); - exit_destroy_reg: - debugfs_remove(ab3100_reg_file); - exit_destroy_dir: - debugfs_remove(ab3100_dir); - exit_no_debugfs: - return; + debugfs_create_file("set_reg", S_IWUSR, ab3100_dir, &ab3100_set_priv, + &ab3100_get_set_reg_fops); } #else static inline void ab3100_setup_debugfs(struct ab3100 *ab3100) diff --git a/drivers/mfd/ab3100-otp.c b/drivers/mfd/ab3100-otp.c index b3f8d359f409..c4751fb9bc22 100644 --- a/drivers/mfd/ab3100-otp.c +++ b/drivers/mfd/ab3100-otp.c @@ -122,17 +122,11 @@ static const struct file_operations ab3100_otp_operations = { .release = single_release, }; -static int __init ab3100_otp_init_debugfs(struct device *dev, - struct ab3100_otp *otp) +static void __init ab3100_otp_init_debugfs(struct device *dev, + struct ab3100_otp *otp) { otp->debugfs = debugfs_create_file("ab3100_otp", S_IFREG | S_IRUGO, - NULL, otp, - &ab3100_otp_operations); - if (!otp->debugfs) { - dev_err(dev, "AB3100 debugfs OTP file registration failed!\n"); - return -ENOENT; - } - return 0; + NULL, otp, &ab3100_otp_operations); } static void __exit ab3100_otp_exit_debugfs(struct ab3100_otp *otp) @@ -141,10 +135,9 @@ static void __exit ab3100_otp_exit_debugfs(struct ab3100_otp *otp) } #else /* Compile this out if debugfs not selected */ -static inline int __init ab3100_otp_init_debugfs(struct device *dev, - struct ab3100_otp *otp) +static inline void __init ab3100_otp_init_debugfs(struct device *dev, + struct ab3100_otp *otp) { - return 0; } static inline void __exit ab3100_otp_exit_debugfs(struct ab3100_otp *otp) @@ -211,9 +204,7 @@ static int __init ab3100_otp_probe(struct platform_device *pdev) } /* debugfs entries */ - err = ab3100_otp_init_debugfs(&pdev->dev, otp); - if (err) - goto err; + ab3100_otp_init_debugfs(&pdev->dev, otp); return 0; From b11f75d032d8706bd39390a63107828985a61de3 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Sat, 6 Jul 2019 18:47:21 +0200 Subject: [PATCH 08/49] mfd: ab8500: no need to check return value of debugfs_create functions When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this. Cc: Linus Walleij Cc: Lee Jones Cc: linux-arm-kernel@lists.infradead.org Link: https://lore.kernel.org/r/20190706164722.18766-2-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- drivers/mfd/ab8500-debugfs.c | 322 +++++++++++------------------------ 1 file changed, 97 insertions(+), 225 deletions(-) diff --git a/drivers/mfd/ab8500-debugfs.c b/drivers/mfd/ab8500-debugfs.c index d24c6ecccb88..567a34b073dd 100644 --- a/drivers/mfd/ab8500-debugfs.c +++ b/drivers/mfd/ab8500-debugfs.c @@ -2644,12 +2644,10 @@ static const struct file_operations ab8500_hwreg_fops = { .owner = THIS_MODULE, }; -static struct dentry *ab8500_dir; -static struct dentry *ab8500_gpadc_dir; - static int ab8500_debug_probe(struct platform_device *plf) { - struct dentry *file; + struct dentry *ab8500_dir; + struct dentry *ab8500_gpadc_dir; struct ab8500 *ab8500; struct resource *res; @@ -2694,47 +2692,22 @@ static int ab8500_debug_probe(struct platform_device *plf) } ab8500_dir = debugfs_create_dir(AB8500_NAME_STRING, NULL); - if (!ab8500_dir) - goto err; ab8500_gpadc_dir = debugfs_create_dir(AB8500_ADC_NAME_STRING, ab8500_dir); - if (!ab8500_gpadc_dir) - goto err; - file = debugfs_create_file("all-bank-registers", S_IRUGO, ab8500_dir, - &plf->dev, &ab8500_bank_registers_fops); - if (!file) - goto err; - - file = debugfs_create_file("all-banks", S_IRUGO, ab8500_dir, - &plf->dev, &ab8500_all_banks_fops); - if (!file) - goto err; - - file = debugfs_create_file("register-bank", - (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_dir, &plf->dev, &ab8500_bank_fops); - if (!file) - goto err; - - file = debugfs_create_file("register-address", - (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_dir, &plf->dev, &ab8500_address_fops); - if (!file) - goto err; - - file = debugfs_create_file("register-value", - (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_dir, &plf->dev, &ab8500_val_fops); - if (!file) - goto err; - - file = debugfs_create_file("irq-subscribe", - (S_IRUGO | S_IWUSR | S_IWGRP), ab8500_dir, - &plf->dev, &ab8500_subscribe_fops); - if (!file) - goto err; + debugfs_create_file("all-bank-registers", S_IRUGO, ab8500_dir, + &plf->dev, &ab8500_bank_registers_fops); + debugfs_create_file("all-banks", S_IRUGO, ab8500_dir, + &plf->dev, &ab8500_all_banks_fops); + debugfs_create_file("register-bank", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_dir, &plf->dev, &ab8500_bank_fops); + debugfs_create_file("register-address", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_dir, &plf->dev, &ab8500_address_fops); + debugfs_create_file("register-value", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_dir, &plf->dev, &ab8500_val_fops); + debugfs_create_file("irq-subscribe", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_dir, &plf->dev, &ab8500_subscribe_fops); if (is_ab8500(ab8500)) { debug_ranges = ab8500_debug_ranges; @@ -2750,194 +2723,93 @@ static int ab8500_debug_probe(struct platform_device *plf) num_interrupt_lines = AB8540_NR_IRQS; } - file = debugfs_create_file("interrupts", (S_IRUGO), ab8500_dir, - &plf->dev, &ab8500_interrupts_fops); - if (!file) - goto err; - - file = debugfs_create_file("irq-unsubscribe", - (S_IRUGO | S_IWUSR | S_IWGRP), ab8500_dir, - &plf->dev, &ab8500_unsubscribe_fops); - if (!file) - goto err; - - file = debugfs_create_file("hwreg", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_dir, &plf->dev, &ab8500_hwreg_fops); - if (!file) - goto err; - - file = debugfs_create_file("all-modem-registers", - (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_dir, &plf->dev, &ab8500_modem_fops); - if (!file) - goto err; - - file = debugfs_create_file("bat_ctrl", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_bat_ctrl_fops); - if (!file) - goto err; - - file = debugfs_create_file("btemp_ball", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, - &plf->dev, &ab8500_gpadc_btemp_ball_fops); - if (!file) - goto err; - - file = debugfs_create_file("main_charger_v", - (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_main_charger_v_fops); - if (!file) - goto err; - - file = debugfs_create_file("acc_detect1", - (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_acc_detect1_fops); - if (!file) - goto err; - - file = debugfs_create_file("acc_detect2", - (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_acc_detect2_fops); - if (!file) - goto err; - - file = debugfs_create_file("adc_aux1", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_aux1_fops); - if (!file) - goto err; - - file = debugfs_create_file("adc_aux2", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_aux2_fops); - if (!file) - goto err; - - file = debugfs_create_file("main_bat_v", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_main_bat_v_fops); - if (!file) - goto err; - - file = debugfs_create_file("vbus_v", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_vbus_v_fops); - if (!file) - goto err; - - file = debugfs_create_file("main_charger_c", - (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_main_charger_c_fops); - if (!file) - goto err; - - file = debugfs_create_file("usb_charger_c", - (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, - &plf->dev, &ab8500_gpadc_usb_charger_c_fops); - if (!file) - goto err; - - file = debugfs_create_file("bk_bat_v", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_bk_bat_v_fops); - if (!file) - goto err; - - file = debugfs_create_file("die_temp", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_die_temp_fops); - if (!file) - goto err; - - file = debugfs_create_file("usb_id", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_usb_id_fops); - if (!file) - goto err; - + debugfs_create_file("interrupts", (S_IRUGO), ab8500_dir, &plf->dev, + &ab8500_interrupts_fops); + debugfs_create_file("irq-unsubscribe", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_dir, &plf->dev, &ab8500_unsubscribe_fops); + debugfs_create_file("hwreg", (S_IRUGO | S_IWUSR | S_IWGRP), ab8500_dir, + &plf->dev, &ab8500_hwreg_fops); + debugfs_create_file("all-modem-registers", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_dir, &plf->dev, &ab8500_modem_fops); + debugfs_create_file("bat_ctrl", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_bat_ctrl_fops); + debugfs_create_file("btemp_ball", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_btemp_ball_fops); + debugfs_create_file("main_charger_v", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_main_charger_v_fops); + debugfs_create_file("acc_detect1", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_acc_detect1_fops); + debugfs_create_file("acc_detect2", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_acc_detect2_fops); + debugfs_create_file("adc_aux1", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_aux1_fops); + debugfs_create_file("adc_aux2", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_aux2_fops); + debugfs_create_file("main_bat_v", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_main_bat_v_fops); + debugfs_create_file("vbus_v", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_vbus_v_fops); + debugfs_create_file("main_charger_c", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_main_charger_c_fops); + debugfs_create_file("usb_charger_c", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_usb_charger_c_fops); + debugfs_create_file("bk_bat_v", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_bk_bat_v_fops); + debugfs_create_file("die_temp", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_die_temp_fops); + debugfs_create_file("usb_id", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_usb_id_fops); if (is_ab8540(ab8500)) { - file = debugfs_create_file("xtal_temp", - (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8540_gpadc_xtal_temp_fops); - if (!file) - goto err; - file = debugfs_create_file("vbattruemeas", - (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8540_gpadc_vbat_true_meas_fops); - if (!file) - goto err; - file = debugfs_create_file("batctrl_and_ibat", - (S_IRUGO | S_IWUGO), - ab8500_gpadc_dir, - &plf->dev, - &ab8540_gpadc_bat_ctrl_and_ibat_fops); - if (!file) - goto err; - file = debugfs_create_file("vbatmeas_and_ibat", - (S_IRUGO | S_IWUGO), - ab8500_gpadc_dir, &plf->dev, - &ab8540_gpadc_vbat_meas_and_ibat_fops); - if (!file) - goto err; - file = debugfs_create_file("vbattruemeas_and_ibat", - (S_IRUGO | S_IWUGO), - ab8500_gpadc_dir, - &plf->dev, - &ab8540_gpadc_vbat_true_meas_and_ibat_fops); - if (!file) - goto err; - file = debugfs_create_file("battemp_and_ibat", - (S_IRUGO | S_IWUGO), - ab8500_gpadc_dir, - &plf->dev, &ab8540_gpadc_bat_temp_and_ibat_fops); - if (!file) - goto err; - file = debugfs_create_file("otp_calib", - (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, - &plf->dev, &ab8540_gpadc_otp_calib_fops); - if (!file) - goto err; + debugfs_create_file("xtal_temp", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8540_gpadc_xtal_temp_fops); + debugfs_create_file("vbattruemeas", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8540_gpadc_vbat_true_meas_fops); + debugfs_create_file("batctrl_and_ibat", (S_IRUGO | S_IWUGO), + ab8500_gpadc_dir, &plf->dev, + &ab8540_gpadc_bat_ctrl_and_ibat_fops); + debugfs_create_file("vbatmeas_and_ibat", (S_IRUGO | S_IWUGO), + ab8500_gpadc_dir, &plf->dev, + &ab8540_gpadc_vbat_meas_and_ibat_fops); + debugfs_create_file("vbattruemeas_and_ibat", (S_IRUGO | S_IWUGO), + ab8500_gpadc_dir, &plf->dev, + &ab8540_gpadc_vbat_true_meas_and_ibat_fops); + debugfs_create_file("battemp_and_ibat", (S_IRUGO | S_IWUGO), + ab8500_gpadc_dir, &plf->dev, + &ab8540_gpadc_bat_temp_and_ibat_fops); + debugfs_create_file("otp_calib", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8540_gpadc_otp_calib_fops); } - file = debugfs_create_file("avg_sample", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_avg_sample_fops); - if (!file) - goto err; - - file = debugfs_create_file("trig_edge", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_trig_edge_fops); - if (!file) - goto err; - - file = debugfs_create_file("trig_timer", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_trig_timer_fops); - if (!file) - goto err; - - file = debugfs_create_file("conv_type", (S_IRUGO | S_IWUSR | S_IWGRP), - ab8500_gpadc_dir, &plf->dev, - &ab8500_gpadc_conv_type_fops); - if (!file) - goto err; + debugfs_create_file("avg_sample", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_avg_sample_fops); + debugfs_create_file("trig_edge", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_trig_edge_fops); + debugfs_create_file("trig_timer", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_trig_timer_fops); + debugfs_create_file("conv_type", (S_IRUGO | S_IWUSR | S_IWGRP), + ab8500_gpadc_dir, &plf->dev, + &ab8500_gpadc_conv_type_fops); return 0; - -err: - debugfs_remove_recursive(ab8500_dir); - dev_err(&plf->dev, "failed to create debugfs entries.\n"); - - return -ENOMEM; } static struct platform_driver ab8500_debug_driver = { From dc607f6bbafd2d8bf1d518c346560ba9577e402f Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Sat, 6 Jul 2019 18:47:22 +0200 Subject: [PATCH 09/49] mfd: aat2870: no need to check return value of debugfs_create functions When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this. Cc: Lee Jones Link: https://lore.kernel.org/r/20190706164722.18766-3-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- drivers/mfd/aat2870-core.c | 13 ++----------- include/linux/mfd/aat2870.h | 1 - 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/drivers/mfd/aat2870-core.c b/drivers/mfd/aat2870-core.c index 9f58cb2d3789..78ee4b28fca2 100644 --- a/drivers/mfd/aat2870-core.c +++ b/drivers/mfd/aat2870-core.c @@ -321,18 +321,9 @@ static const struct file_operations aat2870_reg_fops = { static void aat2870_init_debugfs(struct aat2870_data *aat2870) { aat2870->dentry_root = debugfs_create_dir("aat2870", NULL); - if (!aat2870->dentry_root) { - dev_warn(aat2870->dev, - "Failed to create debugfs root directory\n"); - return; - } - aat2870->dentry_reg = debugfs_create_file("regs", 0644, - aat2870->dentry_root, - aat2870, &aat2870_reg_fops); - if (!aat2870->dentry_reg) - dev_warn(aat2870->dev, - "Failed to create debugfs register file\n"); + debugfs_create_file("regs", 0644, aat2870->dentry_root, aat2870, + &aat2870_reg_fops); } #else diff --git a/include/linux/mfd/aat2870.h b/include/linux/mfd/aat2870.h index af7267c480ee..2445842d482d 100644 --- a/include/linux/mfd/aat2870.h +++ b/include/linux/mfd/aat2870.h @@ -136,7 +136,6 @@ struct aat2870_data { /* for debugfs */ struct dentry *dentry_root; - struct dentry *dentry_reg; }; struct aat2870_subdev_info { From 644bf60088955421051e716ab9c8fe7fb7997fd7 Mon Sep 17 00:00:00 2001 From: Suzuki K Poulose Date: Thu, 1 Aug 2019 11:20:24 +0100 Subject: [PATCH 10/49] i2c: Revert incorrect conversion to use generic helper The patch "drivers: Introduce device lookup variants by ACPI_COMPANION device" converted an incorrect instance in i2c driver to a new helper. Revert this change. Reported-by: Stephen Rothwell Cc: Mika Westerberg Cc: Wolfram Sang Fixes: 00500147cbd3 ("drivers: Introduce device lookup variants by ACPI_COMPANION device") Signed-off-by: Suzuki K Poulose Link: https://lore.kernel.org/r/20190801102026.27312-1-suzuki.poulose@arm.com Signed-off-by: Greg Kroah-Hartman --- drivers/i2c/i2c-core-acpi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c index bc80aafb521f..f60f7a95d48e 100644 --- a/drivers/i2c/i2c-core-acpi.c +++ b/drivers/i2c/i2c-core-acpi.c @@ -357,7 +357,10 @@ static int i2c_acpi_find_match_adapter(struct device *dev, const void *data) struct i2c_adapter *i2c_acpi_find_adapter_by_handle(acpi_handle handle) { - struct device *dev = bus_find_device_by_acpi_dev(&i2c_bus_type, handle); + struct device *dev; + + dev = bus_find_device(&i2c_bus_type, NULL, handle, + i2c_acpi_find_match_adapter); return dev ? i2c_verify_adapter(dev) : NULL; } From b9515ecbf6caef2ea911ca59801eff84d034fa48 Mon Sep 17 00:00:00 2001 From: Suzuki K Poulose Date: Thu, 1 Aug 2019 11:20:25 +0100 Subject: [PATCH 11/49] drivers: Fix typo in parameter description for driver_find_device_by_acpi_dev Fix a typo in the comment describing the parameters for the new API, which triggers the following warning for htmldocs: include/linux/device.h:479: warning: Function parameter or member 'drv' not described in 'driver_find_device_by_acpi_dev' Reported-by: kbuild test robot Signed-off-by: Suzuki K Poulose Link: https://lore.kernel.org/r/20190801102026.27312-2-suzuki.poulose@arm.com Signed-off-by: Greg Kroah-Hartman --- include/linux/device.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/device.h b/include/linux/device.h index 23ef6eba7213..8e1b2ead5d96 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -467,7 +467,7 @@ driver_find_device_by_fwnode(struct device_driver *drv, /** * driver_find_device_by_devt- device iterator for locating a particular device * by devt. - * @driver: the driver we're iterating + * @drv: the driver we're iterating * @devt: devt pointer to match. */ static inline struct device *driver_find_device_by_devt(struct device_driver *drv, From 313b46d831189f593840c625d7972092cb0088fc Mon Sep 17 00:00:00 2001 From: Suzuki K Poulose Date: Thu, 1 Aug 2019 11:20:26 +0100 Subject: [PATCH 12/49] drivers: Fix htmldocs warnings with bus_find_next_device() Document the parameters for bus_find_next_device() to avoid htmldocs build warnings as reported below : include/linux/device.h:236: warning: Function parameter or member 'bus' not described in 'bus_find_next_device' include/linux/device.h:236: warning: Function parameter or member 'cur' not described in 'bus_find_next_device' Reported-by: kbuild test robot Signed-off-by: Suzuki K Poulose Link: https://lore.kernel.org/r/20190801102026.27312-3-suzuki.poulose@arm.com Signed-off-by: Greg Kroah-Hartman --- include/linux/device.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/linux/device.h b/include/linux/device.h index 8e1b2ead5d96..bff46ce3bc3b 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -229,6 +229,8 @@ static inline struct device *bus_find_device_by_devt(struct bus_type *bus, /** * bus_find_next_device - Find the next device after a given device in a * given bus. + * @bus: bus type + * @cur: device to begin the search with. */ static inline struct device * bus_find_next_device(struct bus_type *bus,struct device *cur) From 5302dd7dd0b6d04c63cdce51d1e9fda9ef0be886 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Wed, 31 Jul 2019 15:17:14 -0700 Subject: [PATCH 13/49] driver core: Add support for linking devices during device addition When devices are added, the bus might want to create device links to track functional dependencies between supplier and consumer devices. This tracking of supplier-consumer relationship allows optimizing device probe order and tracking whether all consumers of a supplier are active. The add_links bus callback is added to support this. However, when consumer devices are added, they might not have a supplier device to link to despite needing mandatory resources/functionality from one or more suppliers. A waiting_for_suppliers list is created to track such consumers and retry linking them when new devices get added. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20190731221721.187713-2-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 83 ++++++++++++++++++++++++++++++++++++++++++ include/linux/device.h | 14 +++++++ 2 files changed, 97 insertions(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index 90f2dd4661f5..18e04ca1de13 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -44,6 +44,8 @@ early_param("sysfs.deprecated", sysfs_deprecated_setup); #endif /* Device links support. */ +static LIST_HEAD(wait_for_suppliers); +static DEFINE_MUTEX(wfs_lock); #ifdef CONFIG_SRCU static DEFINE_MUTEX(device_links_lock); @@ -418,6 +420,51 @@ struct device_link *device_link_add(struct device *consumer, } EXPORT_SYMBOL_GPL(device_link_add); +/** + * device_link_wait_for_supplier - Mark device as waiting for supplier + * @consumer: Consumer device + * + * Marks the consumer device as waiting for suppliers to become available. The + * consumer device will never be probed until it's unmarked as waiting for + * suppliers. The caller is responsible for adding the link to the supplier + * once the supplier device is present. + * + * This function is NOT meant to be called from the probe function of the + * consumer but rather from code that creates/adds the consumer device. + */ +static void device_link_wait_for_supplier(struct device *consumer) +{ + mutex_lock(&wfs_lock); + list_add_tail(&consumer->links.needs_suppliers, &wait_for_suppliers); + mutex_unlock(&wfs_lock); +} + +/** + * device_link_check_waiting_consumers - Try to unmark waiting consumers + * + * Loops through all consumers waiting on suppliers and tries to add all their + * supplier links. If that succeeds, the consumer device is unmarked as waiting + * for suppliers. Otherwise, they are left marked as waiting on suppliers, + * + * The add_links bus callback is expected to return 0 if it has found and added + * all the supplier links for the consumer device. It should return an error if + * it isn't able to do so. + * + * The caller of device_link_wait_for_supplier() is expected to call this once + * it's aware of potential suppliers becoming available. + */ +static void device_link_check_waiting_consumers(void) +{ + struct device *dev, *tmp; + + mutex_lock(&wfs_lock); + list_for_each_entry_safe(dev, tmp, &wait_for_suppliers, + links.needs_suppliers) + if (!dev->bus->add_links(dev)) + list_del_init(&dev->links.needs_suppliers); + mutex_unlock(&wfs_lock); +} + static void device_link_free(struct device_link *link) { while (refcount_dec_not_one(&link->rpm_active)) @@ -552,6 +599,19 @@ int device_links_check_suppliers(struct device *dev) struct device_link *link; int ret = 0; + /* + * If a device is waiting for one or more suppliers (in + * wait_for_suppliers list), it is not ready to probe yet. So just + * return -EPROBE_DEFER without having to check the links with existing + * suppliers. + */ + mutex_lock(&wfs_lock); + if (!list_empty(&dev->links.needs_suppliers)) { + mutex_unlock(&wfs_lock); + return -EPROBE_DEFER; + } + mutex_unlock(&wfs_lock); + device_links_write_lock(); list_for_each_entry(link, &dev->links.suppliers, c_node) { @@ -836,6 +896,10 @@ static void device_links_purge(struct device *dev) { struct device_link *link, *ln; + mutex_lock(&wfs_lock); + list_del(&dev->links.needs_suppliers); + mutex_unlock(&wfs_lock); + /* * Delete all of the remaining links from this device to any other * devices (either consumers or suppliers). @@ -1697,6 +1761,7 @@ void device_initialize(struct device *dev) #endif INIT_LIST_HEAD(&dev->links.consumers); INIT_LIST_HEAD(&dev->links.suppliers); + INIT_LIST_HEAD(&dev->links.needs_suppliers); dev->links.status = DL_DEV_NO_DRIVER; } EXPORT_SYMBOL_GPL(device_initialize); @@ -2132,6 +2197,24 @@ int device_add(struct device *dev) BUS_NOTIFY_ADD_DEVICE, dev); kobject_uevent(&dev->kobj, KOBJ_ADD); + + /* + * Check if any of the other devices (consumers) have been waiting for + * this device (supplier) to be added so that they can create a device + * link to it. + * + * This needs to happen after device_pm_add() because device_link_add() + * requires the supplier be registered before it's called. + * + * But this also needs to happe before bus_probe_device() to make sure + * waiting consumers can link to it before the driver is bound to the + * device and the driver sync_state callback is called for this device. + */ + device_link_check_waiting_consumers(); + + if (dev->bus && dev->bus->add_links && dev->bus->add_links(dev)) + device_link_wait_for_supplier(dev); + bus_probe_device(dev); if (parent) klist_add_tail(&dev->p->knode_parent, diff --git a/include/linux/device.h b/include/linux/device.h index bff46ce3bc3b..1e05911325f0 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -78,6 +78,17 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *); * -EPROBE_DEFER it will queue the device for deferred probing. * @uevent: Called when a device is added, removed, or a few other things * that generate uevents to add the environment variables. + * @add_links: Called, perhaps multiple times per device, after a device is + * added to this bus. The function is expected to create device + * links to all the suppliers of the input device that are + * available at the time this function is called. As in, the + * function should NOT stop at the first failed device link if + * other unlinked supplier devices are present in the system. + * + * Return 0 if device links have been successfully created to all + * the suppliers of this device. Return an error if some of the + * suppliers are not yet available and this function needs to be + * reattempted in the future. * @probe: Called when a new device or driver add to this bus, and callback * the specific driver's probe to initial the matched device. * @remove: Called when a device removed from this bus. @@ -122,6 +133,7 @@ struct bus_type { int (*match)(struct device *dev, struct device_driver *drv); int (*uevent)(struct device *dev, struct kobj_uevent_env *env); + int (*add_links)(struct device *dev); int (*probe)(struct device *dev); int (*remove)(struct device *dev); void (*shutdown)(struct device *dev); @@ -1128,11 +1140,13 @@ enum dl_dev_state { * struct dev_links_info - Device data related to device links. * @suppliers: List of links to supplier devices. * @consumers: List of links to consumer devices. + * @needs_suppliers: Hook to global list of devices waiting for suppliers. * @status: Driver status information. */ struct dev_links_info { struct list_head suppliers; struct list_head consumers; + struct list_head needs_suppliers; enum dl_dev_state status; }; From 134b23eec9e3a3c795a6ceb0efe2fa63e87983b2 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Wed, 31 Jul 2019 15:17:15 -0700 Subject: [PATCH 14/49] driver core: Add edit_links() callback for drivers The driver core/bus adding supplier-consumer dependencies by default enables functional dependencies to be tracked correctly even when the consumer devices haven't had their drivers registered or loaded (if they are modules). However, when the bus incorrectly adds dependencies that it shouldn't have added, the devices might never probe. For example, if device-C is a consumer of device-S and they have phandles to each other in DT, the following could happen: 1. Device-S get added first. 2. The bus add_links() callback will (incorrectly) try to link it as a consumer of device-C. 3. Since device-C isn't present, device-S will be put in "waiting-for-supplier" list. 4. Device-C gets added next. 5. All devices in "waiting-for-supplier" list are retried for linking. 6. Device-S gets linked as consumer to Device-C. 7. The bus add_links() callback will (correctly) try to link it as a consumer of device-S. 8. This isn't allowed because it would create a cyclic device links. Neither devices will get probed since the supplier is marked as dependent on the consumer. And the consumer will never probe because the consumer can't get resources from the supplier. Without this patch, things stay in this broken state. However, with this patch, the execution will continue like this: 9. Device-C's driver is loaded. 10. Device-C's driver removes Device-S as a consumer of Device-C. 11. Device-C's driver adds Device-C as a consumer of Device-S. 12. Device-S probes. 14. Device-C probes. kbuild test robot reported missing documentation for device.has_edit_links Reported-by: kbuild test robot Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20190731221721.187713-3-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 24 ++++++++++++++++++++++-- drivers/base/dd.c | 29 +++++++++++++++++++++++++++++ include/linux/device.h | 20 ++++++++++++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 18e04ca1de13..feec8dee1e91 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -439,6 +439,19 @@ static void device_link_wait_for_supplier(struct device *consumer) mutex_unlock(&wfs_lock); } +/** + * device_link_remove_from_wfs - Unmark device as waiting for supplier + * @consumer: Consumer device + * + * Unmark the consumer device as waiting for suppliers to become available. + */ +void device_link_remove_from_wfs(struct device *consumer) +{ + mutex_lock(&wfs_lock); + list_del_init(&consumer->links.needs_suppliers); + mutex_unlock(&wfs_lock); +} + /** * device_link_check_waiting_consumers - Try to unmark waiting consumers * @@ -456,12 +469,19 @@ static void device_link_wait_for_supplier(struct device *consumer) static void device_link_check_waiting_consumers(void) { struct device *dev, *tmp; + int ret; mutex_lock(&wfs_lock); list_for_each_entry_safe(dev, tmp, &wait_for_suppliers, - links.needs_suppliers) - if (!dev->bus->add_links(dev)) + links.needs_suppliers) { + ret = 0; + if (dev->has_edit_links) + ret = driver_edit_links(dev); + else if (dev->bus->add_links) + ret = dev->bus->add_links(dev); + if (!ret) list_del_init(&dev->links.needs_suppliers); + } mutex_unlock(&wfs_lock); } diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 994a90747420..5e7041ede0d7 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -698,6 +698,12 @@ int driver_probe_device(struct device_driver *drv, struct device *dev) pr_debug("bus: '%s': %s: matched device %s with driver %s\n", drv->bus->name, __func__, dev_name(dev), drv->name); + if (drv->edit_links) { + if (drv->edit_links(dev)) + dev->has_edit_links = true; + else + device_link_remove_from_wfs(dev); + } pm_runtime_get_suppliers(dev); if (dev->parent) pm_runtime_get_sync(dev->parent); @@ -786,6 +792,29 @@ struct device_attach_data { bool have_async; }; +static int __driver_edit_links(struct device_driver *drv, void *data) +{ + struct device *dev = data; + + if (!drv->edit_links) + return 0; + + if (driver_match_device(drv, dev) <= 0) + return 0; + + return drv->edit_links(dev); +} + +int driver_edit_links(struct device *dev) +{ + int ret; + + device_lock(dev); + ret = bus_for_each_drv(dev->bus, NULL, dev, __driver_edit_links); + device_unlock(dev); + return ret; +} + static int __device_attach_driver(struct device_driver *drv, void *_data) { struct device_attach_data *data = _data; diff --git a/include/linux/device.h b/include/linux/device.h index 1e05911325f0..d3991810f39d 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -349,6 +349,20 @@ enum probe_type { * @probe_type: Type of the probe (synchronous or asynchronous) to use. * @of_match_table: The open firmware table. * @acpi_match_table: The ACPI match table. + * @edit_links: Called to allow a matched driver to edit the device links the + * bus might have added incorrectly. This will be useful to handle + * cases where the bus incorrectly adds functional dependencies + * that aren't true or tries to create cyclic dependencies. But + * doesn't correctly handle functional dependencies that are + * missed by the bus as the supplier's sync_state might get to + * execute before the driver for a missing consumer is loaded and + * gets to edit the device links for the consumer. + * + * This function might be called multiple times after a new device + * is added. The function is expected to create all the device + * links for the new device and return 0 if it was completed + * successfully or return an error if it needs to be reattempted + * in the future. * @probe: Called to query the existence of a specific device, * whether this driver can work with it, and bind the driver * to a specific device. @@ -388,6 +402,7 @@ struct device_driver { const struct of_device_id *of_match_table; const struct acpi_device_id *acpi_match_table; + int (*edit_links)(struct device *dev); int (*probe) (struct device *dev); int (*remove) (struct device *dev); void (*shutdown) (struct device *dev); @@ -1220,6 +1235,8 @@ struct dev_links_info { * @offline: Set after successful invocation of bus type's .offline(). * @of_node_reused: Set if the device-tree node is shared with an ancestor * device. + * @has_edit_links: This device has a driver than is capable of + * editing the device links created by driver core. * @dma_coherent: this particular device is dma coherent, even if the * architecture supports non-coherent devices. * @@ -1313,6 +1330,7 @@ struct device { bool offline_disabled:1; bool offline:1; bool of_node_reused:1; + bool has_edit_links:1; #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \ defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \ defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) @@ -1564,6 +1582,7 @@ extern int __must_check device_attach(struct device *dev); extern int __must_check driver_attach(struct device_driver *drv); extern void device_initial_probe(struct device *dev); extern int __must_check device_reprobe(struct device *dev); +extern int driver_edit_links(struct device *dev); extern bool device_is_bound(struct device *dev); @@ -1654,6 +1673,7 @@ struct device_link *device_link_add(struct device *consumer, struct device *supplier, u32 flags); void device_link_del(struct device_link *link); void device_link_remove(void *consumer, struct device *supplier); +void device_link_remove_from_wfs(struct device *consumer); #ifndef dev_fmt #define dev_fmt(fmt) fmt From 690ff7881b2681afca1c3c063f4a5cb7c71d9d8b Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Wed, 31 Jul 2019 15:17:16 -0700 Subject: [PATCH 15/49] of/platform: Add functional dependency link from DT bindings Add device-links after the devices are created (but before they are probed) by looking at common DT bindings like clocks and interconnects. Automatically adding device-links for functional dependencies at the framework level provides the following benefits: - Optimizes device probe order and avoids the useless work of attempting probes of devices that will not probe successfully (because their suppliers aren't present or haven't probed yet). For example, in a commonly available mobile SoC, registering just one consumer device's driver at an initcall level earlier than the supplier device's driver causes 11 failed probe attempts before the consumer device probes successfully. This was with a kernel with all the drivers statically compiled in. This problem gets a lot worse if all the drivers are loaded as modules without direct symbol dependencies. - Supplier devices like clock providers, interconnect providers, etc need to keep the resources they provide active and at a particular state(s) during boot up even if their current set of consumers don't request the resource to be active. This is because the rest of the consumers might not have probed yet and turning off the resource before all the consumers have probed could lead to a hang or undesired user experience. Some frameworks (Eg: regulator) handle this today by turning off "unused" resources at late_initcall_sync and hoping all the devices have probed by then. This is not a valid assumption for systems with loadable modules. Other frameworks (Eg: clock) just don't handle this due to the lack of a clear signal for when they can turn off resources. This leads to downstream hacks to handle cases like this that can easily be solved in the upstream kernel. By linking devices before they are probed, we give suppliers a clear count of the number of dependent consumers. Once all of the consumers are active, the suppliers can turn off the unused resources without making assumptions about the number of consumers. By default we just add device-links to track "driver presence" (probe succeeded) of the supplier device. If any other functionality provided by device-links are needed, it is left to the consumer/supplier devices to change the link when they probe. kbuild test robot reported clang error about missing const Reported-by: kbuild test robot Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20190731221721.187713-4-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- .../admin-guide/kernel-parameters.txt | 5 + drivers/of/platform.c | 165 ++++++++++++++++++ 2 files changed, 170 insertions(+) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 46b826fcb5ad..12937349d79d 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -3170,6 +3170,11 @@ This can be set from sysctl after boot. See Documentation/admin-guide/sysctl/vm.rst for details. + of_devlink [KNL] Make device links from common DT bindings. Useful + for optimizing probe order and making sure resources + aren't turned off before the consumer devices have + probed. + ohci1394_dma=early [HW] enable debugging via the ohci1394 driver. See Documentation/debugging-via-ohci1394.txt for more info. diff --git a/drivers/of/platform.c b/drivers/of/platform.c index b47a2292fe8e..c1c433333124 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -503,6 +503,170 @@ int of_platform_default_populate(struct device_node *root, } EXPORT_SYMBOL_GPL(of_platform_default_populate); +bool of_link_is_valid(struct device_node *con, struct device_node *sup) +{ + of_node_get(sup); + /* + * Don't allow linking a device node as a consumer of one of its + * descendant nodes. By definition, a child node can't be a functional + * dependency for the parent node. + */ + while (sup) { + if (sup == con) { + of_node_put(sup); + return false; + } + sup = of_get_next_parent(sup); + } + return true; +} + +static int of_link_to_phandle(struct device *dev, struct device_node *sup_np) +{ + struct platform_device *sup_dev; + u32 dl_flags = DL_FLAG_AUTOPROBE_CONSUMER; + int ret = 0; + + /* + * Since we are trying to create device links, we need to find + * the actual device node that owns this supplier phandle. + * Often times it's the same node, but sometimes it can be one + * of the parents. So walk up the parent till you find a + * device. + */ + while (sup_np && !of_find_property(sup_np, "compatible", NULL)) + sup_np = of_get_next_parent(sup_np); + if (!sup_np) + return 0; + + if (!of_link_is_valid(dev->of_node, sup_np)) { + of_node_put(sup_np); + return 0; + } + sup_dev = of_find_device_by_node(sup_np); + of_node_put(sup_np); + if (!sup_dev) + return -ENODEV; + if (!device_link_add(dev, &sup_dev->dev, dl_flags)) + ret = -ENODEV; + put_device(&sup_dev->dev); + return ret; +} + +static struct device_node *parse_prop_cells(struct device_node *np, + const char *prop, int index, + const char *binding, + const char *cell) +{ + struct of_phandle_args sup_args; + + /* Don't need to check property name for every index. */ + if (!index && strcmp(prop, binding)) + return NULL; + + if (of_parse_phandle_with_args(np, binding, cell, index, &sup_args)) + return NULL; + + return sup_args.np; +} + +static struct device_node *parse_clocks(struct device_node *np, + const char *prop, int index) +{ + return parse_prop_cells(np, prop, index, "clocks", "#clock-cells"); +} + +static struct device_node *parse_interconnects(struct device_node *np, + const char *prop, int index) +{ + return parse_prop_cells(np, prop, index, "interconnects", + "#interconnect-cells"); +} + +static int strcmp_suffix(const char *str, const char *suffix) +{ + unsigned int len, suffix_len; + + len = strlen(str); + suffix_len = strlen(suffix); + if (len <= suffix_len) + return -1; + return strcmp(str + len - suffix_len, suffix); +} + +static struct device_node *parse_regulators(struct device_node *np, + const char *prop, int index) +{ + if (index || strcmp_suffix(prop, "-supply")) + return NULL; + + return of_parse_phandle(np, prop, 0); +} + +/** + * struct supplier_bindings - Information for parsing supplier DT binding + * + * @parse_prop: If the function cannot parse the property, return NULL. + * Otherwise, return the phandle listed in the property + * that corresponds to the index. + */ +struct supplier_bindings { + struct device_node *(*parse_prop)(struct device_node *np, + const char *name, int index); +}; + +static const struct supplier_bindings bindings[] = { + { .parse_prop = parse_clocks, }, + { .parse_prop = parse_interconnects, }, + { .parse_prop = parse_regulators, }, + { }, +}; + +static bool of_link_property(struct device *dev, struct device_node *con_np, + const char *prop) +{ + struct device_node *phandle; + const struct supplier_bindings *s = bindings; + unsigned int i = 0; + bool done = true, matched = false; + + while (!matched && s->parse_prop) { + while ((phandle = s->parse_prop(con_np, prop, i))) { + matched = true; + i++; + if (of_link_to_phandle(dev, phandle)) + /* + * Don't stop at the first failure. See + * Documentation for bus_type.add_links for + * more details. + */ + done = false; + } + s++; + } + return done ? 0 : -ENODEV; +} + +static bool of_devlink; +core_param(of_devlink, of_devlink, bool, 0); + +static int of_link_to_suppliers(struct device *dev) +{ + struct property *p; + bool done = true; + + if (!of_devlink) + return 0; + if (unlikely(!dev->of_node)) + return 0; + + for_each_property_of_node(dev->of_node, p) + if (of_link_property(dev, dev->of_node, p->name)) + done = false; + + return done ? 0 : -ENODEV; +} + #ifndef CONFIG_PPC static const struct of_device_id reserved_mem_matches[] = { { .compatible = "qcom,rmtfs-mem" }, @@ -518,6 +682,7 @@ static int __init of_platform_default_populate_init(void) if (!of_have_populated_dt()) return -ENODEV; + platform_bus_type.add_links = of_link_to_suppliers; /* * Handle certain compatibles explicitly, since we don't want to create * platform_devices for every node in /reserved-memory with a From 8f8184d6bf676a8680d6f441e40317d166b46f73 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Wed, 31 Jul 2019 15:17:17 -0700 Subject: [PATCH 16/49] driver core: Add sync_state driver/bus callback This sync_state driver/bus callback is called once all the consumers of a supplier have probed successfully. This allows the supplier device's driver/bus to sync the supplier device's state to the software state with the guarantee that all the consumers are actively managing the resources provided by the supplier device. To maintain backwards compatibility and ease transition from existing frameworks and resource cleanup schemes, late_initcall_sync is the earliest when the sync_state callback might be called. There is no upper bound on the time by which the sync_state callback has to be called. This is because if a consumer device never probes, the supplier has to maintain its resources in the state left by the bootloader. For example, if the bootloader leaves the display backlight at a fixed voltage and the backlight driver is never probed, you don't want the backlight to ever be turned off after boot up. Also, when multiple devices are added after kernel init, some suppliers could be added before their consumer devices get added. In these instances, the supplier devices could get their sync_state callback called right after they probe because the consumers devices haven't had a chance to create device links to the suppliers. To handle this correctly, this change also provides APIs to pause/resume sync state callbacks so that when multiple devices are added, their sync_state callback evaluation can be postponed to happen after all of them are added. kbuild test robot reported missing documentation for device.state_synced Reported-by: kbuild test robot Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20190731221721.187713-5-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 65 ++++++++++++++++++++++++++++++++++++++++++ include/linux/device.h | 26 +++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index feec8dee1e91..5b1795cd116a 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -46,6 +46,8 @@ early_param("sysfs.deprecated", sysfs_deprecated_setup); /* Device links support. */ static LIST_HEAD(wait_for_suppliers); static DEFINE_MUTEX(wfs_lock); +static LIST_HEAD(deferred_sync); +static unsigned int supplier_sync_state_disabled; #ifdef CONFIG_SRCU static DEFINE_MUTEX(device_links_lock); @@ -651,6 +653,62 @@ int device_links_check_suppliers(struct device *dev) return ret; } +static void __device_links_supplier_sync_state(struct device *dev) +{ + struct device_link *link; + + if (dev->state_synced) + return; + + list_for_each_entry(link, &dev->links.consumers, s_node) { + if (!(link->flags & DL_FLAG_MANAGED)) + continue; + if (link->status != DL_STATE_ACTIVE) + return; + } + + if (dev->bus->sync_state) + dev->bus->sync_state(dev); + else if (dev->driver && dev->driver->sync_state) + dev->driver->sync_state(dev); + + dev->state_synced = true; +} + +void device_links_supplier_sync_state_pause(void) +{ + device_links_write_lock(); + supplier_sync_state_disabled++; + device_links_write_unlock(); +} + +void device_links_supplier_sync_state_resume(void) +{ + struct device *dev, *tmp; + + device_links_write_lock(); + if (!supplier_sync_state_disabled) { + WARN(true, "Unmatched sync_state pause/resume!"); + goto out; + } + supplier_sync_state_disabled--; + if (supplier_sync_state_disabled) + goto out; + + list_for_each_entry_safe(dev, tmp, &deferred_sync, links.defer_sync) { + __device_links_supplier_sync_state(dev); + list_del_init(&dev->links.defer_sync); + } +out: + device_links_write_unlock(); +} + +static void __device_links_supplier_defer_sync(struct device *sup) +{ + if (list_empty(&sup->links.defer_sync)) + list_add_tail(&sup->links.defer_sync, &deferred_sync); +} + /** * device_links_driver_bound - Update device links after probing its driver. * @dev: Device to update the links for. @@ -695,6 +753,11 @@ void device_links_driver_bound(struct device *dev) WARN_ON(link->status != DL_STATE_CONSUMER_PROBE); WRITE_ONCE(link->status, DL_STATE_ACTIVE); + + if (supplier_sync_state_disabled) + __device_links_supplier_defer_sync(link->supplier); + else + __device_links_supplier_sync_state(link->supplier); } dev->links.status = DL_DEV_DRIVER_BOUND; @@ -811,6 +874,7 @@ void device_links_driver_cleanup(struct device *dev) WRITE_ONCE(link->status, DL_STATE_DORMANT); } + list_del_init(&dev->links.defer_sync); __device_links_no_driver(dev); device_links_write_unlock(); @@ -1782,6 +1846,7 @@ void device_initialize(struct device *dev) INIT_LIST_HEAD(&dev->links.consumers); INIT_LIST_HEAD(&dev->links.suppliers); INIT_LIST_HEAD(&dev->links.needs_suppliers); + INIT_LIST_HEAD(&dev->links.defer_sync); dev->links.status = DL_DEV_NO_DRIVER; } EXPORT_SYMBOL_GPL(device_initialize); diff --git a/include/linux/device.h b/include/linux/device.h index d3991810f39d..63a3aafabcd6 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -84,6 +84,8 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *); * available at the time this function is called. As in, the * function should NOT stop at the first failed device link if * other unlinked supplier devices are present in the system. + * This is necessary for the sync_state() callback to work + * correctly. * * Return 0 if device links have been successfully created to all * the suppliers of this device. Return an error if some of the @@ -91,6 +93,13 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *); * reattempted in the future. * @probe: Called when a new device or driver add to this bus, and callback * the specific driver's probe to initial the matched device. + * @sync_state: Called to sync device state to software state after all the + * state tracking consumers linked to this device (present at + * the time of late_initcall) have successfully bound to a + * driver. If the device has no consumers, this function will + * be called at late_initcall_sync level. If the device has + * consumers that are never bound to a driver, this function + * will never get called until they do. * @remove: Called when a device removed from this bus. * @shutdown: Called at shut-down time to quiesce the device. * @@ -135,6 +144,7 @@ struct bus_type { int (*uevent)(struct device *dev, struct kobj_uevent_env *env); int (*add_links)(struct device *dev); int (*probe)(struct device *dev); + void (*sync_state)(struct device *dev); int (*remove)(struct device *dev); void (*shutdown)(struct device *dev); @@ -366,6 +376,13 @@ enum probe_type { * @probe: Called to query the existence of a specific device, * whether this driver can work with it, and bind the driver * to a specific device. + * @sync_state: Called to sync device state to software state after all the + * state tracking consumers linked to this device (present at + * the time of late_initcall) have successfully bound to a + * driver. If the device has no consumers, this function will + * be called at late_initcall_sync level. If the device has + * consumers that are never bound to a driver, this function + * will never get called until they do. * @remove: Called when the device is removed from the system to * unbind a device from this driver. * @shutdown: Called at shut-down time to quiesce the device. @@ -404,6 +421,7 @@ struct device_driver { int (*edit_links)(struct device *dev); int (*probe) (struct device *dev); + void (*sync_state)(struct device *dev); int (*remove) (struct device *dev); void (*shutdown) (struct device *dev); int (*suspend) (struct device *dev, pm_message_t state); @@ -1156,12 +1174,14 @@ enum dl_dev_state { * @suppliers: List of links to supplier devices. * @consumers: List of links to consumer devices. * @needs_suppliers: Hook to global list of devices waiting for suppliers. + * @defer_sync: Hook to global list of devices that have deferred sync_state. * @status: Driver status information. */ struct dev_links_info { struct list_head suppliers; struct list_head consumers; struct list_head needs_suppliers; + struct list_head defer_sync; enum dl_dev_state status; }; @@ -1237,6 +1257,9 @@ struct dev_links_info { * device. * @has_edit_links: This device has a driver than is capable of * editing the device links created by driver core. + * @state_synced: The hardware state of this device has been synced to match + * the software state of this device by calling the driver/bus + * sync_state() callback. * @dma_coherent: this particular device is dma coherent, even if the * architecture supports non-coherent devices. * @@ -1331,6 +1354,7 @@ struct device { bool offline:1; bool of_node_reused:1; bool has_edit_links:1; + bool state_synced:1; #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \ defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \ defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) @@ -1674,6 +1698,8 @@ struct device_link *device_link_add(struct device *consumer, void device_link_del(struct device_link *link); void device_link_remove(void *consumer, struct device *supplier); void device_link_remove_from_wfs(struct device *consumer); +void device_links_supplier_sync_state_pause(void); +void device_links_supplier_sync_state_resume(void); #ifndef dev_fmt #define dev_fmt(fmt) fmt From 21871a99b34c65c56a24193c277a4981529c306f Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Wed, 31 Jul 2019 15:17:18 -0700 Subject: [PATCH 17/49] of/platform: Pause/resume sync state during init and of_platform_populate() When all the top level devices are populated from DT during kernel init, the supplier devices could be added and probed before the consumer devices are added and linked to the suppliers. To avoid the sync_state() callback from being called prematurely, pause the sync_state() callbacks before populating the devices and resume them at late_initcall_sync(). Similarly, when children devices are populated after kernel init using of_platform_populate(), there could be supplier-consumer dependencies between the children devices that are populated. To avoid the same problem with sync_state() being called prematurely, pause and resume sync_state() callbacks across of_platform_populate(). Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20190731221721.187713-6-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index c1c433333124..ae85a88ba53e 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -480,6 +480,7 @@ int of_platform_populate(struct device_node *root, pr_debug("%s()\n", __func__); pr_debug(" starting at: %pOF\n", root); + device_links_supplier_sync_state_pause(); for_each_child_of_node(root, child) { rc = of_platform_bus_create(child, matches, lookup, parent, true); if (rc) { @@ -487,6 +488,8 @@ int of_platform_populate(struct device_node *root, break; } } + device_links_supplier_sync_state_resume(); + of_node_set_flag(root, OF_POPULATED_BUS); of_node_put(root); @@ -683,6 +686,7 @@ static int __init of_platform_default_populate_init(void) return -ENODEV; platform_bus_type.add_links = of_link_to_suppliers; + device_links_supplier_sync_state_pause(); /* * Handle certain compatibles explicitly, since we don't want to create * platform_devices for every node in /reserved-memory with a @@ -703,6 +707,13 @@ static int __init of_platform_default_populate_init(void) return 0; } arch_initcall_sync(of_platform_default_populate_init); + +static int __init of_platform_sync_state_init(void) +{ + device_links_supplier_sync_state_resume(); + return 0; +} +late_initcall_sync(of_platform_sync_state_init); #endif int of_platform_device_destroy(struct device *dev, void *data) From 709fb8297358f08a125b770d1518a95d03b541db Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Wed, 31 Jul 2019 15:17:19 -0700 Subject: [PATCH 18/49] of/platform: Create device links for all child-supplier depencencies A parent device can have child devices that it adds when it probes. But this probing of the parent device can happen way after kernel init is done -- for example, when the parent device's driver is loaded as a module. In such cases, if the child devices depend on a supplier in the system, we need to make sure the supplier gets the sync_state() callback only after these child devices are added and probed. To achieve this, when creating device links for a device by looking at its DT node, don't just look at DT references at the top node level. Look at DT references in all the descendant nodes too and create device links from the ancestor device to all these supplier devices. This way, when the parent device probes and adds child devices, the child devices can then create their own device links to the suppliers and further delay the supplier's sync_state() callback to after the child devices are probed. Example: In this illustration, -> denotes DT references and indentation represents child status. Device node A Device node B -> D Device node C -> B, D Device node D Assume all these devices have their drivers loaded as modules. Without this patch, this is the sequence of events: 1. D is added. 2. A is added. 3. Device D probes. 4. Device D gets its sync_state() callback. 5. Device B and C might malfunction because their resources got altered/turned off before they can make active requests for them. With this patch, this is the sequence of events: 1. D is added. 2. A is added and creates device links to D. 3. Device link from A to B is not added because A is a parent of B. 4. Device D probes. 5. Device D does not get it's sync_state() callback because consumer A hasn't probed yet. 5. Device A probes. 5. a. Devices B and C are added. 5. b. Device links from B and C to D are added. 5. c. Device A's probe completes. 6. Device D does not get it's sync_state() callback because consumer A has probed but consumers B and C haven't probed yet. 7. Device B and C probe. 8. Device D gets it's sync_state() callback because all its consumers have probed. 9. None of the devices malfunction. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20190731221721.187713-7-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index ae85a88ba53e..52590fb20422 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -650,24 +650,35 @@ static bool of_link_property(struct device *dev, struct device_node *con_np, return done ? 0 : -ENODEV; } +static int __of_link_to_suppliers(struct device *dev, + struct device_node *con_np) +{ + struct device_node *child; + struct property *p; + bool done = true; + + for_each_property_of_node(con_np, p) + if (of_link_property(dev, con_np, p->name)) + done = false; + + for_each_child_of_node(con_np, child) + if (__of_link_to_suppliers(dev, child)) + done = false; + + return done ? 0 : -ENODEV; +} + static bool of_devlink; core_param(of_devlink, of_devlink, bool, 0); static int of_link_to_suppliers(struct device *dev) { - struct property *p; - bool done = true; - if (!of_devlink) return 0; if (unlikely(!dev->of_node)) return 0; - for_each_property_of_node(dev->of_node, p) - if (of_link_property(dev, dev->of_node, p->name)) - done = false; - - return done ? 0 : -ENODEV; + return __of_link_to_suppliers(dev, dev->of_node); } #ifndef CONFIG_PPC From b3173c2292fbaf24ff7062d366830b012ed04269 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Wed, 31 Jul 2019 15:17:20 -0700 Subject: [PATCH 19/49] of/platform: Don't create device links for default busses Default busses also have devices created for them. But there's no point in creating device links for them. It's especially wasteful as it'll cause the traversal of the entire device tree and also spend a lot of time checking and figuring out that creating those links isn't allowed. So check for default busses and skip trying to create device links for them. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20190731221721.187713-8-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 52590fb20422..21838226d68a 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -677,6 +677,8 @@ static int of_link_to_suppliers(struct device *dev) return 0; if (unlikely(!dev->of_node)) return 0; + if (of_match_node(of_default_bus_match_table, dev->of_node)) + return 0; return __of_link_to_suppliers(dev, dev->of_node); } From 23b6904442d08b7dbed7622ed33b236d41a3aa8b Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Wed, 31 Jul 2019 14:43:40 +0200 Subject: [PATCH 20/49] driver core: add dev_groups to all drivers Add the ability for the driver core to create and remove a list of attribute groups automatically when the device is bound/unbound from a specific driver. Signed-off-by: Dmitry Torokhov Tested-by: Richard Gong Link: https://lore.kernel.org/r/20190731124349.4474-2-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 14 ++++++++++++++ include/linux/device.h | 3 +++ 2 files changed, 17 insertions(+) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 994a90747420..d811e60610d3 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -554,9 +554,16 @@ re_probe: goto probe_failed; } + if (device_add_groups(dev, drv->dev_groups)) { + dev_err(dev, "device_add_groups() failed\n"); + goto dev_groups_failed; + } + if (test_remove) { test_remove = false; + device_remove_groups(dev, drv->dev_groups); + if (dev->bus->remove) dev->bus->remove(dev); else if (drv->remove) @@ -584,6 +591,11 @@ re_probe: drv->bus->name, __func__, dev_name(dev), drv->name); goto done; +dev_groups_failed: + if (dev->bus->remove) + dev->bus->remove(dev); + else if (drv->remove) + drv->remove(dev); probe_failed: if (dev->bus) blocking_notifier_call_chain(&dev->bus->p->bus_notifier, @@ -1114,6 +1126,8 @@ static void __device_release_driver(struct device *dev, struct device *parent) pm_runtime_put_sync(dev); + device_remove_groups(dev, drv->dev_groups); + if (dev->bus && dev->bus->remove) dev->bus->remove(dev); else if (drv->remove) diff --git a/include/linux/device.h b/include/linux/device.h index c330b75c6c57..98c00b71b598 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -262,6 +262,8 @@ enum probe_type { * @resume: Called to bring a device from sleep mode. * @groups: Default attributes that get created by the driver core * automatically. + * @dev_groups: Additional attributes attached to device instance once the + * it is bound to the driver. * @pm: Power management operations of the device which matched * this driver. * @coredump: Called when sysfs entry is written to. The device driver @@ -296,6 +298,7 @@ struct device_driver { int (*suspend) (struct device *dev, pm_message_t state); int (*resume) (struct device *dev); const struct attribute_group **groups; + const struct attribute_group **dev_groups; const struct dev_pm_ops *pm; void (*coredump) (struct device *dev); From 0682e005d1705a740a36860cc75199a411f46025 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 31 Jul 2019 14:43:41 +0200 Subject: [PATCH 21/49] uio: uio_fsl_elbc_gpcm: convert platform driver to use dev_groups Platform drivers now have the option to have the platform core create and remove any needed sysfs attribute files. So take advantage of that and do not register "by hand" a sysfs group of attributes. Link: https://lore.kernel.org/r/20190731124349.4474-3-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- drivers/uio/uio_fsl_elbc_gpcm.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/drivers/uio/uio_fsl_elbc_gpcm.c b/drivers/uio/uio_fsl_elbc_gpcm.c index 450e2f5c9b43..be8a6905f507 100644 --- a/drivers/uio/uio_fsl_elbc_gpcm.c +++ b/drivers/uio/uio_fsl_elbc_gpcm.c @@ -71,6 +71,13 @@ static ssize_t reg_store(struct device *dev, struct device_attribute *attr, static DEVICE_ATTR(reg_br, 0664, reg_show, reg_store); static DEVICE_ATTR(reg_or, 0664, reg_show, reg_store); +static struct attribute *uio_fsl_elbc_gpcm_attrs[] = { + &dev_attr_reg_br.attr, + &dev_attr_reg_or.attr, + NULL, +}; +ATTRIBUTE_GROUPS(uio_fsl_elbc_gpcm); + static ssize_t reg_show(struct device *dev, struct device_attribute *attr, char *buf) { @@ -411,25 +418,12 @@ static int uio_fsl_elbc_gpcm_probe(struct platform_device *pdev) /* store private data */ platform_set_drvdata(pdev, info); - /* create sysfs files */ - ret = device_create_file(priv->dev, &dev_attr_reg_br); - if (ret) - goto out_err3; - ret = device_create_file(priv->dev, &dev_attr_reg_or); - if (ret) - goto out_err4; - dev_info(priv->dev, "eLBC/GPCM device (%s) at 0x%llx, bank %d, irq=%d\n", priv->name, (unsigned long long)res.start, priv->bank, irq != NO_IRQ ? irq : -1); return 0; -out_err4: - device_remove_file(priv->dev, &dev_attr_reg_br); -out_err3: - platform_set_drvdata(pdev, NULL); - uio_unregister_device(info); out_err2: if (priv->shutdown) priv->shutdown(info, true); @@ -448,8 +442,6 @@ static int uio_fsl_elbc_gpcm_remove(struct platform_device *pdev) struct uio_info *info = platform_get_drvdata(pdev); struct fsl_elbc_gpcm *priv = info->priv; - device_remove_file(priv->dev, &dev_attr_reg_or); - device_remove_file(priv->dev, &dev_attr_reg_br); platform_set_drvdata(pdev, NULL); uio_unregister_device(info); if (priv->shutdown) @@ -474,6 +466,7 @@ static struct platform_driver uio_fsl_elbc_gpcm_driver = { .driver = { .name = "fsl,elbc-gpcm-uio", .of_match_table = uio_fsl_elbc_gpcm_match, + .dev_groups = uio_fsl_elbc_gpcm_groups, }, .probe = uio_fsl_elbc_gpcm_probe, .remove = uio_fsl_elbc_gpcm_remove, From 43b9ac937be6f2afb2f0cf060d40cea365c09d07 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 31 Jul 2019 14:43:44 +0200 Subject: [PATCH 22/49] firmware: arm_scpi: convert platform driver to use dev_groups Platform drivers now have the option to have the platform core create and remove any needed sysfs attribute files. So take advantage of that and do not register "by hand" a sysfs group of attributes. Acked-by: Sudeep Holla Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20190731124349.4474-6-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- drivers/firmware/arm_scpi.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c index 725164b83242..a80c331c3a6e 100644 --- a/drivers/firmware/arm_scpi.c +++ b/drivers/firmware/arm_scpi.c @@ -1011,10 +1011,6 @@ static int scpi_probe(struct platform_device *pdev) scpi_info->firmware_version)); scpi_info->scpi_ops = &scpi_ops; - ret = devm_device_add_groups(dev, versions_groups); - if (ret) - dev_err(dev, "unable to create sysfs version group\n"); - return devm_of_platform_populate(dev); } @@ -1030,6 +1026,7 @@ static struct platform_driver scpi_driver = { .driver = { .name = "scpi_protocol", .of_match_table = scpi_of_match, + .dev_groups = versions_groups, }, .probe = scpi_probe, .remove = scpi_remove, From 0abd02ede7dfd0f3ea7a6ac68e00ca6946827889 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 31 Jul 2019 14:43:47 +0200 Subject: [PATCH 23/49] video: fbdev: wm8505fb: convert platform driver to use dev_groups Platform drivers now have the option to have the platform core create and remove any needed sysfs attribute files. So take advantage of that and do not register "by hand" a sysfs file. Cc: Tony Prisk Cc: Bartlomiej Zolnierkiewicz Cc: linux-arm-kernel@lists.infradead.org Cc: dri-devel@lists.freedesktop.org Cc: linux-fbdev@vger.kernel.org Acked-by: Bartlomiej Zolnierkiewicz Signed-off-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20190731124349.4474-9-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- drivers/video/fbdev/wm8505fb.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/video/fbdev/wm8505fb.c b/drivers/video/fbdev/wm8505fb.c index ff752635a31c..17c780315ca5 100644 --- a/drivers/video/fbdev/wm8505fb.c +++ b/drivers/video/fbdev/wm8505fb.c @@ -176,6 +176,12 @@ static ssize_t contrast_store(struct device *dev, static DEVICE_ATTR_RW(contrast); +static struct attribute *wm8505fb_attrs[] = { + &dev_attr_contrast.attr, + NULL, +}; +ATTRIBUTE_GROUPS(wm8505fb); + static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf) { chan &= 0xffff; @@ -361,10 +367,6 @@ static int wm8505fb_probe(struct platform_device *pdev) return ret; } - ret = device_create_file(&pdev->dev, &dev_attr_contrast); - if (ret < 0) - fb_warn(&fbi->fb, "failed to register attributes (%d)\n", ret); - fb_info(&fbi->fb, "%s frame buffer at 0x%lx-0x%lx\n", fbi->fb.fix.id, fbi->fb.fix.smem_start, fbi->fb.fix.smem_start + fbi->fb.fix.smem_len - 1); @@ -376,8 +378,6 @@ static int wm8505fb_remove(struct platform_device *pdev) { struct wm8505fb_info *fbi = platform_get_drvdata(pdev); - device_remove_file(&pdev->dev, &dev_attr_contrast); - unregister_framebuffer(&fbi->fb); writel(0, fbi->regbase); @@ -399,6 +399,7 @@ static struct platform_driver wm8505fb_driver = { .driver = { .name = DRIVER_NAME, .of_match_table = wmt_dt_ids, + .dev_groups = wm8505fb_groups, }, }; From e14018cc34d633a55e8fabd2085f1da51d6c60fb Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 31 Jul 2019 14:43:48 +0200 Subject: [PATCH 24/49] video: fbdev: w100fb: convert platform driver to use dev_groups Platform drivers now have the option to have the platform core create and remove any needed sysfs attribute files. So take advantage of that and do not register "by hand" a bunch of sysfs files. Cc: Tony Prisk Cc: linux-arm-kernel@lists.infradead.org Cc: dri-devel@lists.freedesktop.org Cc: linux-fbdev@vger.kernel.org Acked-by: Bartlomiej Zolnierkiewicz Signed-off-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20190731124349.4474-10-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- drivers/video/fbdev/w100fb.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/drivers/video/fbdev/w100fb.c b/drivers/video/fbdev/w100fb.c index 597ffaa13cd2..3be07807edcd 100644 --- a/drivers/video/fbdev/w100fb.c +++ b/drivers/video/fbdev/w100fb.c @@ -164,6 +164,15 @@ static ssize_t fastpllclk_store(struct device *dev, struct device_attribute *att static DEVICE_ATTR_RW(fastpllclk); +static struct attribute *w100fb_attrs[] = { + &dev_attr_fastpllclk.attr, + &dev_attr_reg_read.attr, + &dev_attr_reg_write.attr, + &dev_attr_flip.attr, + NULL, +}; +ATTRIBUTE_GROUPS(w100fb); + /* * Some touchscreens need hsync information from the video driver to * function correctly. We export it here. @@ -752,14 +761,6 @@ int w100fb_probe(struct platform_device *pdev) goto out; } - err = device_create_file(&pdev->dev, &dev_attr_fastpllclk); - err |= device_create_file(&pdev->dev, &dev_attr_reg_read); - err |= device_create_file(&pdev->dev, &dev_attr_reg_write); - err |= device_create_file(&pdev->dev, &dev_attr_flip); - - if (err != 0) - fb_warn(info, "failed to register attributes (%d)\n", err); - fb_info(info, "%s frame buffer device\n", info->fix.id); return 0; out: @@ -784,11 +785,6 @@ static int w100fb_remove(struct platform_device *pdev) struct fb_info *info = platform_get_drvdata(pdev); struct w100fb_par *par=info->par; - device_remove_file(&pdev->dev, &dev_attr_fastpllclk); - device_remove_file(&pdev->dev, &dev_attr_reg_read); - device_remove_file(&pdev->dev, &dev_attr_reg_write); - device_remove_file(&pdev->dev, &dev_attr_flip); - unregister_framebuffer(info); vfree(par->saved_intmem); @@ -1625,6 +1621,7 @@ static struct platform_driver w100fb_driver = { .resume = w100fb_resume, .driver = { .name = "w100fb", + .dev_groups = w100fb_groups, }, }; From de3dacf0347e3dc6f63cd5abe944bc60077c5abf Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 31 Jul 2019 14:43:49 +0200 Subject: [PATCH 25/49] video: fbdev: sm501fb: convert platform driver to use dev_groups Platform drivers now have the option to have the platform core create and remove any needed sysfs attribute files. So take advantage of that and do not register "by hand" a bunch of sysfs files. Cc: dri-devel@lists.freedesktop.org Cc: linux-fbdev@vger.kernel.org Acked-by: Bartlomiej Zolnierkiewicz Signed-off-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20190731124349.4474-11-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- drivers/video/fbdev/sm501fb.c | 37 +++++++++-------------------------- 1 file changed, 9 insertions(+), 28 deletions(-) diff --git a/drivers/video/fbdev/sm501fb.c b/drivers/video/fbdev/sm501fb.c index 6edb4492e675..3dd1b1d76e98 100644 --- a/drivers/video/fbdev/sm501fb.c +++ b/drivers/video/fbdev/sm501fb.c @@ -1271,6 +1271,14 @@ static ssize_t sm501fb_debug_show_pnl(struct device *dev, static DEVICE_ATTR(fbregs_pnl, 0444, sm501fb_debug_show_pnl, NULL); +static struct attribute *sm501fb_attrs[] = { + &dev_attr_crt_src.attr, + &dev_attr_fbregs_pnl.attr, + &dev_attr_fbregs_crt.attr, + NULL, +}; +ATTRIBUTE_GROUPS(sm501fb); + /* acceleration operations */ static int sm501fb_sync(struct fb_info *info) { @@ -2011,33 +2019,9 @@ static int sm501fb_probe(struct platform_device *pdev) goto err_started_crt; } - /* create device files */ - - ret = device_create_file(dev, &dev_attr_crt_src); - if (ret) - goto err_started_panel; - - ret = device_create_file(dev, &dev_attr_fbregs_pnl); - if (ret) - goto err_attached_crtsrc_file; - - ret = device_create_file(dev, &dev_attr_fbregs_crt); - if (ret) - goto err_attached_pnlregs_file; - /* we registered, return ok */ return 0; -err_attached_pnlregs_file: - device_remove_file(dev, &dev_attr_fbregs_pnl); - -err_attached_crtsrc_file: - device_remove_file(dev, &dev_attr_crt_src); - -err_started_panel: - unregister_framebuffer(info->fb[HEAD_PANEL]); - sm501_free_init_fb(info, HEAD_PANEL); - err_started_crt: unregister_framebuffer(info->fb[HEAD_CRT]); sm501_free_init_fb(info, HEAD_CRT); @@ -2067,10 +2051,6 @@ static int sm501fb_remove(struct platform_device *pdev) struct fb_info *fbinfo_crt = info->fb[0]; struct fb_info *fbinfo_pnl = info->fb[1]; - device_remove_file(&pdev->dev, &dev_attr_fbregs_crt); - device_remove_file(&pdev->dev, &dev_attr_fbregs_pnl); - device_remove_file(&pdev->dev, &dev_attr_crt_src); - sm501_free_init_fb(info, HEAD_CRT); sm501_free_init_fb(info, HEAD_PANEL); @@ -2234,6 +2214,7 @@ static struct platform_driver sm501fb_driver = { .resume = sm501fb_resume, .driver = { .name = "sm501-fb", + .dev_groups = sm501fb_groups, }, }; From 5bd08a4ae3d06cf598b91444c2d727aa25045bf0 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 31 Jul 2019 14:43:46 +0200 Subject: [PATCH 26/49] platform: x86: hp-wmi: convert platform driver to use dev_groups Platform drivers now have the option to have the platform core create and remove any needed sysfs attribute files. So take advantage of that and do not register "by hand" a bunch of sysfs files. Cc: Darren Hart Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: "H. Peter Anvin" Cc: x86@kernel.org Cc: platform-driver-x86@vger.kernel.org Acked-by: Andy Shevchenko Signed-off-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20190731124349.4474-8-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- drivers/platform/x86/hp-wmi.c | 47 +++++++++-------------------------- 1 file changed, 12 insertions(+), 35 deletions(-) diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index 2521e45280b8..6bcbbb375401 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -502,6 +502,17 @@ static DEVICE_ATTR_RO(dock); static DEVICE_ATTR_RO(tablet); static DEVICE_ATTR_RW(postcode); +static struct attribute *hp_wmi_attrs[] = { + &dev_attr_display.attr, + &dev_attr_hddtemp.attr, + &dev_attr_als.attr, + &dev_attr_dock.attr, + &dev_attr_tablet.attr, + &dev_attr_postcode.attr, + NULL, +}; +ATTRIBUTE_GROUPS(hp_wmi); + static void hp_wmi_notify(u32 value, void *context) { struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL }; @@ -678,16 +689,6 @@ static void hp_wmi_input_destroy(void) input_unregister_device(hp_wmi_input_dev); } -static void cleanup_sysfs(struct platform_device *device) -{ - device_remove_file(&device->dev, &dev_attr_display); - device_remove_file(&device->dev, &dev_attr_hddtemp); - device_remove_file(&device->dev, &dev_attr_als); - device_remove_file(&device->dev, &dev_attr_dock); - device_remove_file(&device->dev, &dev_attr_tablet); - device_remove_file(&device->dev, &dev_attr_postcode); -} - static int __init hp_wmi_rfkill_setup(struct platform_device *device) { int err, wireless; @@ -858,8 +859,6 @@ fail: static int __init hp_wmi_bios_setup(struct platform_device *device) { - int err; - /* clear detected rfkill devices */ wifi_rfkill = NULL; bluetooth_rfkill = NULL; @@ -869,35 +868,12 @@ static int __init hp_wmi_bios_setup(struct platform_device *device) if (hp_wmi_rfkill_setup(device)) hp_wmi_rfkill2_setup(device); - err = device_create_file(&device->dev, &dev_attr_display); - if (err) - goto add_sysfs_error; - err = device_create_file(&device->dev, &dev_attr_hddtemp); - if (err) - goto add_sysfs_error; - err = device_create_file(&device->dev, &dev_attr_als); - if (err) - goto add_sysfs_error; - err = device_create_file(&device->dev, &dev_attr_dock); - if (err) - goto add_sysfs_error; - err = device_create_file(&device->dev, &dev_attr_tablet); - if (err) - goto add_sysfs_error; - err = device_create_file(&device->dev, &dev_attr_postcode); - if (err) - goto add_sysfs_error; return 0; - -add_sysfs_error: - cleanup_sysfs(device); - return err; } static int __exit hp_wmi_bios_remove(struct platform_device *device) { int i; - cleanup_sysfs(device); for (i = 0; i < rfkill2_count; i++) { rfkill_unregister(rfkill2[i].rfkill); @@ -966,6 +942,7 @@ static struct platform_driver hp_wmi_driver = { .driver = { .name = "hp-wmi", .pm = &hp_wmi_pm_ops, + .dev_groups = hp_wmi_groups, }, .remove = __exit_p(hp_wmi_bios_remove), }; From 0fbb93fce79b938643889862e74ee30213e1d5fe Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 31 Jul 2019 14:43:45 +0200 Subject: [PATCH 27/49] olpc: x01: convert platform driver to use dev_groups Platform drivers now have the option to have the platform core create and remove any needed sysfs attribute files. So take advantage of that and do not register "by hand" a lid sysfs file. Cc: Darren Hart Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: "H. Peter Anvin" Cc: x86@kernel.org Cc: platform-driver-x86@vger.kernel.org Acked-by: Andy Shevchenko Signed-off-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20190731124349.4474-7-gregkh@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- arch/x86/platform/olpc/olpc-xo1-sci.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/arch/x86/platform/olpc/olpc-xo1-sci.c b/arch/x86/platform/olpc/olpc-xo1-sci.c index 25ce1b3b0732..99a28ce2244c 100644 --- a/arch/x86/platform/olpc/olpc-xo1-sci.c +++ b/arch/x86/platform/olpc/olpc-xo1-sci.c @@ -157,6 +157,12 @@ static ssize_t lid_wake_mode_set(struct device *dev, static DEVICE_ATTR(lid_wake_mode, S_IWUSR | S_IRUGO, lid_wake_mode_show, lid_wake_mode_set); +static struct attribute *lid_attrs[] = { + &dev_attr_lid_wake_mode.attr, + NULL, +}; +ATTRIBUTE_GROUPS(lid); + /* * Process all items in the EC's SCI queue. * @@ -510,17 +516,8 @@ static int setup_lid_switch(struct platform_device *pdev) goto err_register; } - r = device_create_file(&lid_switch_idev->dev, &dev_attr_lid_wake_mode); - if (r) { - dev_err(&pdev->dev, "failed to create wake mode attr: %d\n", r); - goto err_create_attr; - } - return 0; -err_create_attr: - input_unregister_device(lid_switch_idev); - lid_switch_idev = NULL; err_register: input_free_device(lid_switch_idev); return r; @@ -528,7 +525,6 @@ err_register: static void free_lid_switch(void) { - device_remove_file(&lid_switch_idev->dev, &dev_attr_lid_wake_mode); input_unregister_device(lid_switch_idev); } @@ -624,6 +620,7 @@ static int xo1_sci_remove(struct platform_device *pdev) static struct platform_driver xo1_sci_driver = { .driver = { .name = "olpc-xo1-sci-acpi", + .dev_groups = lid_groups, }, .probe = xo1_sci_probe, .remove = xo1_sci_remove, From 0ad0bdd8ba6489d05044aa5ca6c6f7491be7743c Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Tue, 6 Aug 2019 12:26:53 -0700 Subject: [PATCH 28/49] of/platform: Fix fn definitons for of_link_is_valid() and of_link_property() of_link_is_valid() can be static since it's not used anywhere else. of_link_property() return type should have been int instead of bool. Reported-by: Stephen Rothwell Signed-off-by: Saravana Kannan Acked-by: Rob Herring Link: https://lore.kernel.org/r/20190806192654.138605-1-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 21838226d68a..f68de5c4aeff 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -506,7 +506,7 @@ int of_platform_default_populate(struct device_node *root, } EXPORT_SYMBOL_GPL(of_platform_default_populate); -bool of_link_is_valid(struct device_node *con, struct device_node *sup) +static bool of_link_is_valid(struct device_node *con, struct device_node *sup) { of_node_get(sup); /* @@ -625,7 +625,7 @@ static const struct supplier_bindings bindings[] = { { }, }; -static bool of_link_property(struct device *dev, struct device_node *con_np, +static int of_link_property(struct device *dev, struct device_node *con_np, const char *prop) { struct device_node *phandle; From 5adf5781019dd21233d6d13ab0d78bf03a13d4f1 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Tue, 6 Aug 2019 19:05:58 -0700 Subject: [PATCH 29/49] of/platform: Fix device_links_supplier_sync_state_resume() warning In platforms/devices which have CONFIG_OF turned on but don't have a populated DT, the calls to device_links_supplier_sync_state_pause() and device_links_supplier_sync_state_resume() can get mismatched. This will cause a warning during boot. Fix the warning by making sure the calls are matched even in that case. Reported-by: Qian Cai Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20190807020559.74458-1-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index f68de5c4aeff..a3cd69772264 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -723,7 +723,8 @@ arch_initcall_sync(of_platform_default_populate_init); static int __init of_platform_sync_state_init(void) { - device_links_supplier_sync_state_resume(); + if (of_have_populated_dt()) + device_links_supplier_sync_state_resume(); return 0; } late_initcall_sync(of_platform_sync_state_init); From 1f573cce48a2ebb35953034062ef01056b7d6a58 Mon Sep 17 00:00:00 2001 From: Suzuki K Poulose Date: Fri, 9 Aug 2019 11:20:33 +0100 Subject: [PATCH 30/49] device.h: Fix warnings for mismatched parameter names in comments Fix the warnings for parameter named as "driver" instead of the actual "drv" in the comments as reported by the kbuild robot. Reported-by: kbuild test robot Cc: Greg Kroah-Hartman Signed-off-by: Suzuki K Poulose Link: https://lore.kernel.org/r/20190809102033.28463-1-suzuki.poulose@arm.com Signed-off-by: Greg Kroah-Hartman --- include/linux/device.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/linux/device.h b/include/linux/device.h index 23efaff5f10c..e32038f4ef56 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -479,7 +479,7 @@ struct device *driver_find_device(struct device_driver *drv, /** * driver_find_device_by_name - device iterator for locating a particular device * of a specific name. - * @driver: the driver we're iterating + * @drv: the driver we're iterating * @name: name of the device to match */ static inline struct device *driver_find_device_by_name(struct device_driver *drv, @@ -491,7 +491,7 @@ static inline struct device *driver_find_device_by_name(struct device_driver *dr /** * driver_find_device_by_of_node- device iterator for locating a particular device * by of_node pointer. - * @driver: the driver we're iterating + * @drv: the driver we're iterating * @np: of_node pointer to match. */ static inline struct device * @@ -504,7 +504,7 @@ driver_find_device_by_of_node(struct device_driver *drv, /** * driver_find_device_by_fwnode- device iterator for locating a particular device * by fwnode pointer. - * @driver: the driver we're iterating + * @drv: the driver we're iterating * @fwnode: fwnode pointer to match. */ static inline struct device * @@ -536,7 +536,7 @@ static inline struct device *driver_find_next_device(struct device_driver *drv, /** * driver_find_device_by_acpi_dev : device iterator for locating a particular * device matching the ACPI_COMPANION device. - * @driver: the driver we're iterating + * @drv: the driver we're iterating * @adev: ACPI_COMPANION device to match. */ static inline struct device * From 9c1c5e0bc5ec032e78bf15c302f3b20152f865ad Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Tue, 6 Aug 2019 12:26:54 -0700 Subject: [PATCH 31/49] of/platform: Disable generic device linking code for PowerPC PowerPC platforms don't use the generic of/platform code to populate the devices from DT. Therefore the generic device linking code is never used in PowerPC. Compile it out to avoid warning about unused functions. If a specific PowerPC platform wants to use this code in the future, bringing this back for PowerPC would be trivial. We'll just need to export of_link_to_suppliers() and then let the machine specific files do the linking as they populate the devices from DT. Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20190806192654.138605-2-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index a3cd69772264..e5f7e40df439 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -506,6 +506,7 @@ int of_platform_default_populate(struct device_node *root, } EXPORT_SYMBOL_GPL(of_platform_default_populate); +#ifndef CONFIG_PPC static bool of_link_is_valid(struct device_node *con, struct device_node *sup) { of_node_get(sup); @@ -683,7 +684,6 @@ static int of_link_to_suppliers(struct device *dev) return __of_link_to_suppliers(dev, dev->of_node); } -#ifndef CONFIG_PPC static const struct of_device_id reserved_mem_matches[] = { { .compatible = "qcom,rmtfs-mem" }, { .compatible = "qcom,cmd-db" }, From ce684d957c5672f3bb55e6b0872932b5b4c39c56 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Wed, 31 Jul 2019 12:00:06 +0200 Subject: [PATCH 32/49] devcoredump: use memory_read_from_buffer Use memory_read_from_buffer() to simplify devcd_readv(). Reviewed-by: Chaitanya Kulkarni Reviewed-by: Johannes Berg Signed-off-by: Akinobu Mita Link: https://lore.kernel.org/r/1564243146-5681-2-git-send-email-akinobu.mita@gmail.com Signed-off-by: Johannes Berg Link: https://lore.kernel.org/r/20190731100007.32684-1-johannes@sipsolutions.net Signed-off-by: Greg Kroah-Hartman --- drivers/base/devcoredump.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c index f1a3353f3494..3c960a63062f 100644 --- a/drivers/base/devcoredump.c +++ b/drivers/base/devcoredump.c @@ -164,16 +164,7 @@ static struct class devcd_class = { static ssize_t devcd_readv(char *buffer, loff_t offset, size_t count, void *data, size_t datalen) { - if (offset > datalen) - return -EINVAL; - - if (offset + count > datalen) - count = datalen - offset; - - if (count) - memcpy(buffer, ((u8 *)data) + offset, count); - - return count; + return memory_read_from_buffer(buffer, count, &offset, data, datalen); } static void devcd_freev(void *data) From 2a77eec0d3ca20481a340d4543930a704b3230f6 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Wed, 31 Jul 2019 12:00:07 +0200 Subject: [PATCH 33/49] devcoredump: fix typo in comment s/dev_coredumpmsg/dev_coredumpsg/ in the kernel-doc Reviewed-by: Chaitanya Kulkarni Signed-off-by: Akinobu Mita Link: https://lore.kernel.org/r/1564243146-5681-3-git-send-email-akinobu.mita@gmail.com Signed-off-by: Johannes Berg Link: https://lore.kernel.org/r/20190731100007.32684-2-johannes@sipsolutions.net Signed-off-by: Greg Kroah-Hartman --- drivers/base/devcoredump.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c index 3c960a63062f..e42d0b514384 100644 --- a/drivers/base/devcoredump.c +++ b/drivers/base/devcoredump.c @@ -314,7 +314,7 @@ void dev_coredumpm(struct device *dev, struct module *owner, EXPORT_SYMBOL_GPL(dev_coredumpm); /** - * dev_coredumpmsg - create device coredump that uses scatterlist as data + * dev_coredumpsg - create device coredump that uses scatterlist as data * parameter * @dev: the struct device for the crashed device * @table: the dump data From caec05b6688284bd9745804d54aab5ef62b7ac78 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 27 Aug 2019 21:39:43 +0200 Subject: [PATCH 34/49] Revert "of/platform: Disable generic device linking code for PowerPC" This reverts commit 9c1c5e0bc5ec032e78bf15c302f3b20152f865ad. Based on a lot of email and in-person discussions, this patch series is being reworked to address a number of issues that were pointed out that needed to be taken care of before it should be merged. It will be resubmitted with those changes hopefully soon. Cc: Frank Rowand Cc: Saravana Kannan Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index e5f7e40df439..a3cd69772264 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -506,7 +506,6 @@ int of_platform_default_populate(struct device_node *root, } EXPORT_SYMBOL_GPL(of_platform_default_populate); -#ifndef CONFIG_PPC static bool of_link_is_valid(struct device_node *con, struct device_node *sup) { of_node_get(sup); @@ -684,6 +683,7 @@ static int of_link_to_suppliers(struct device *dev) return __of_link_to_suppliers(dev, dev->of_node); } +#ifndef CONFIG_PPC static const struct of_device_id reserved_mem_matches[] = { { .compatible = "qcom,rmtfs-mem" }, { .compatible = "qcom,cmd-db" }, From 8ef4df5ae1091af6c8aa65938a80f9ad9fa4460f Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 27 Aug 2019 21:40:00 +0200 Subject: [PATCH 35/49] Revert "of/platform: Fix device_links_supplier_sync_state_resume() warning" This reverts commit 5adf5781019dd21233d6d13ab0d78bf03a13d4f1. Based on a lot of email and in-person discussions, this patch series is being reworked to address a number of issues that were pointed out that needed to be taken care of before it should be merged. It will be resubmitted with those changes hopefully soon. Cc: Frank Rowand Cc: Saravana Kannan Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index a3cd69772264..f68de5c4aeff 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -723,8 +723,7 @@ arch_initcall_sync(of_platform_default_populate_init); static int __init of_platform_sync_state_init(void) { - if (of_have_populated_dt()) - device_links_supplier_sync_state_resume(); + device_links_supplier_sync_state_resume(); return 0; } late_initcall_sync(of_platform_sync_state_init); From f6680a66a3ca3ed41a5870ead7591a27827d5632 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 27 Aug 2019 21:40:08 +0200 Subject: [PATCH 36/49] Revert "of/platform: Fix fn definitons for of_link_is_valid() and of_link_property()" This reverts commit 0ad0bdd8ba6489d05044aa5ca6c6f7491be7743c. Based on a lot of email and in-person discussions, this patch series is being reworked to address a number of issues that were pointed out that needed to be taken care of before it should be merged. It will be resubmitted with those changes hopefully soon. Cc: Frank Rowand Cc: Saravana Kannan Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index f68de5c4aeff..21838226d68a 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -506,7 +506,7 @@ int of_platform_default_populate(struct device_node *root, } EXPORT_SYMBOL_GPL(of_platform_default_populate); -static bool of_link_is_valid(struct device_node *con, struct device_node *sup) +bool of_link_is_valid(struct device_node *con, struct device_node *sup) { of_node_get(sup); /* @@ -625,7 +625,7 @@ static const struct supplier_bindings bindings[] = { { }, }; -static int of_link_property(struct device *dev, struct device_node *con_np, +static bool of_link_property(struct device *dev, struct device_node *con_np, const char *prop) { struct device_node *phandle; From 40e05e6acdc07f1a513f206c1a8c5fce16ae5a81 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 27 Aug 2019 21:40:17 +0200 Subject: [PATCH 37/49] Revert "of/platform: Don't create device links for default busses" This reverts commit b3173c2292fbaf24ff7062d366830b012ed04269. Based on a lot of email and in-person discussions, this patch series is being reworked to address a number of issues that were pointed out that needed to be taken care of before it should be merged. It will be resubmitted with those changes hopefully soon. Cc: Frank Rowand Cc: Saravana Kannan Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 21838226d68a..52590fb20422 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -677,8 +677,6 @@ static int of_link_to_suppliers(struct device *dev) return 0; if (unlikely(!dev->of_node)) return 0; - if (of_match_node(of_default_bus_match_table, dev->of_node)) - return 0; return __of_link_to_suppliers(dev, dev->of_node); } From b1c46e118b21dfaa4112a897ead12d7b141cb83b Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 27 Aug 2019 21:40:35 +0200 Subject: [PATCH 38/49] Revert "of/platform: Create device links for all child-supplier depencencies" This reverts commit 709fb8297358f08a125b770d1518a95d03b541db. Based on a lot of email and in-person discussions, this patch series is being reworked to address a number of issues that were pointed out that needed to be taken care of before it should be merged. It will be resubmitted with those changes hopefully soon. Cc: Frank Rowand Cc: Saravana Kannan Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 52590fb20422..ae85a88ba53e 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -650,35 +650,24 @@ static bool of_link_property(struct device *dev, struct device_node *con_np, return done ? 0 : -ENODEV; } -static int __of_link_to_suppliers(struct device *dev, - struct device_node *con_np) -{ - struct device_node *child; - struct property *p; - bool done = true; - - for_each_property_of_node(con_np, p) - if (of_link_property(dev, con_np, p->name)) - done = false; - - for_each_child_of_node(con_np, child) - if (__of_link_to_suppliers(dev, child)) - done = false; - - return done ? 0 : -ENODEV; -} - static bool of_devlink; core_param(of_devlink, of_devlink, bool, 0); static int of_link_to_suppliers(struct device *dev) { + struct property *p; + bool done = true; + if (!of_devlink) return 0; if (unlikely(!dev->of_node)) return 0; - return __of_link_to_suppliers(dev, dev->of_node); + for_each_property_of_node(dev->of_node, p) + if (of_link_property(dev, dev->of_node, p->name)) + done = false; + + return done ? 0 : -ENODEV; } #ifndef CONFIG_PPC From 77df8e790026d6f1a60c83e5fe822bd35d54a002 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 27 Aug 2019 21:40:42 +0200 Subject: [PATCH 39/49] Revert "of/platform: Pause/resume sync state during init and of_platform_populate()" This reverts commit 21871a99b34c65c56a24193c277a4981529c306f. Based on a lot of email and in-person discussions, this patch series is being reworked to address a number of issues that were pointed out that needed to be taken care of before it should be merged. It will be resubmitted with those changes hopefully soon. Cc: Frank Rowand Cc: Saravana Kannan Signed-off-by: Greg Kroah-Hartman --- drivers/of/platform.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index ae85a88ba53e..c1c433333124 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -480,7 +480,6 @@ int of_platform_populate(struct device_node *root, pr_debug("%s()\n", __func__); pr_debug(" starting at: %pOF\n", root); - device_links_supplier_sync_state_pause(); for_each_child_of_node(root, child) { rc = of_platform_bus_create(child, matches, lookup, parent, true); if (rc) { @@ -488,8 +487,6 @@ int of_platform_populate(struct device_node *root, break; } } - device_links_supplier_sync_state_resume(); - of_node_set_flag(root, OF_POPULATED_BUS); of_node_put(root); @@ -686,7 +683,6 @@ static int __init of_platform_default_populate_init(void) return -ENODEV; platform_bus_type.add_links = of_link_to_suppliers; - device_links_supplier_sync_state_pause(); /* * Handle certain compatibles explicitly, since we don't want to create * platform_devices for every node in /reserved-memory with a @@ -707,13 +703,6 @@ static int __init of_platform_default_populate_init(void) return 0; } arch_initcall_sync(of_platform_default_populate_init); - -static int __init of_platform_sync_state_init(void) -{ - device_links_supplier_sync_state_resume(); - return 0; -} -late_initcall_sync(of_platform_sync_state_init); #endif int of_platform_device_destroy(struct device *dev, void *data) From bcca686c11cdad3b4566ed1818cd2688e7901def Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 27 Aug 2019 21:40:50 +0200 Subject: [PATCH 40/49] Revert "driver core: Add sync_state driver/bus callback" This reverts commit 8f8184d6bf676a8680d6f441e40317d166b46f73. Based on a lot of email and in-person discussions, this patch series is being reworked to address a number of issues that were pointed out that needed to be taken care of before it should be merged. It will be resubmitted with those changes hopefully soon. Cc: Frank Rowand Cc: Saravana Kannan Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 65 ------------------------------------------ include/linux/device.h | 26 ----------------- 2 files changed, 91 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 75fd59360940..de775c7a4d7c 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -46,8 +46,6 @@ early_param("sysfs.deprecated", sysfs_deprecated_setup); /* Device links support. */ static LIST_HEAD(wait_for_suppliers); static DEFINE_MUTEX(wfs_lock); -static LIST_HEAD(deferred_sync); -static unsigned int supplier_sync_state_disabled; #ifdef CONFIG_SRCU static DEFINE_MUTEX(device_links_lock); @@ -653,62 +651,6 @@ int device_links_check_suppliers(struct device *dev) return ret; } -static void __device_links_supplier_sync_state(struct device *dev) -{ - struct device_link *link; - - if (dev->state_synced) - return; - - list_for_each_entry(link, &dev->links.consumers, s_node) { - if (!(link->flags & DL_FLAG_MANAGED)) - continue; - if (link->status != DL_STATE_ACTIVE) - return; - } - - if (dev->bus->sync_state) - dev->bus->sync_state(dev); - else if (dev->driver && dev->driver->sync_state) - dev->driver->sync_state(dev); - - dev->state_synced = true; -} - -void device_links_supplier_sync_state_pause(void) -{ - device_links_write_lock(); - supplier_sync_state_disabled++; - device_links_write_unlock(); -} - -void device_links_supplier_sync_state_resume(void) -{ - struct device *dev, *tmp; - - device_links_write_lock(); - if (!supplier_sync_state_disabled) { - WARN(true, "Unmatched sync_state pause/resume!"); - goto out; - } - supplier_sync_state_disabled--; - if (supplier_sync_state_disabled) - goto out; - - list_for_each_entry_safe(dev, tmp, &deferred_sync, links.defer_sync) { - __device_links_supplier_sync_state(dev); - list_del_init(&dev->links.defer_sync); - } -out: - device_links_write_unlock(); -} - -static void __device_links_supplier_defer_sync(struct device *sup) -{ - if (list_empty(&sup->links.defer_sync)) - list_add_tail(&sup->links.defer_sync, &deferred_sync); -} - /** * device_links_driver_bound - Update device links after probing its driver. * @dev: Device to update the links for. @@ -753,11 +695,6 @@ void device_links_driver_bound(struct device *dev) WARN_ON(link->status != DL_STATE_CONSUMER_PROBE); WRITE_ONCE(link->status, DL_STATE_ACTIVE); - - if (supplier_sync_state_disabled) - __device_links_supplier_defer_sync(link->supplier); - else - __device_links_supplier_sync_state(link->supplier); } dev->links.status = DL_DEV_DRIVER_BOUND; @@ -874,7 +811,6 @@ void device_links_driver_cleanup(struct device *dev) WRITE_ONCE(link->status, DL_STATE_DORMANT); } - list_del_init(&dev->links.defer_sync); __device_links_no_driver(dev); device_links_write_unlock(); @@ -1849,7 +1785,6 @@ void device_initialize(struct device *dev) INIT_LIST_HEAD(&dev->links.consumers); INIT_LIST_HEAD(&dev->links.suppliers); INIT_LIST_HEAD(&dev->links.needs_suppliers); - INIT_LIST_HEAD(&dev->links.defer_sync); dev->links.status = DL_DEV_NO_DRIVER; } EXPORT_SYMBOL_GPL(device_initialize); diff --git a/include/linux/device.h b/include/linux/device.h index 76496497e753..90142ec9ce84 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -84,8 +84,6 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *); * available at the time this function is called. As in, the * function should NOT stop at the first failed device link if * other unlinked supplier devices are present in the system. - * This is necessary for the sync_state() callback to work - * correctly. * * Return 0 if device links have been successfully created to all * the suppliers of this device. Return an error if some of the @@ -93,13 +91,6 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *); * reattempted in the future. * @probe: Called when a new device or driver add to this bus, and callback * the specific driver's probe to initial the matched device. - * @sync_state: Called to sync device state to software state after all the - * state tracking consumers linked to this device (present at - * the time of late_initcall) have successfully bound to a - * driver. If the device has no consumers, this function will - * be called at late_initcall_sync level. If the device has - * consumers that are never bound to a driver, this function - * will never get called until they do. * @remove: Called when a device removed from this bus. * @shutdown: Called at shut-down time to quiesce the device. * @@ -144,7 +135,6 @@ struct bus_type { int (*uevent)(struct device *dev, struct kobj_uevent_env *env); int (*add_links)(struct device *dev); int (*probe)(struct device *dev); - void (*sync_state)(struct device *dev); int (*remove)(struct device *dev); void (*shutdown)(struct device *dev); @@ -376,13 +366,6 @@ enum probe_type { * @probe: Called to query the existence of a specific device, * whether this driver can work with it, and bind the driver * to a specific device. - * @sync_state: Called to sync device state to software state after all the - * state tracking consumers linked to this device (present at - * the time of late_initcall) have successfully bound to a - * driver. If the device has no consumers, this function will - * be called at late_initcall_sync level. If the device has - * consumers that are never bound to a driver, this function - * will never get called until they do. * @remove: Called when the device is removed from the system to * unbind a device from this driver. * @shutdown: Called at shut-down time to quiesce the device. @@ -423,7 +406,6 @@ struct device_driver { int (*edit_links)(struct device *dev); int (*probe) (struct device *dev); - void (*sync_state)(struct device *dev); int (*remove) (struct device *dev); void (*shutdown) (struct device *dev); int (*suspend) (struct device *dev, pm_message_t state); @@ -1177,14 +1159,12 @@ enum dl_dev_state { * @suppliers: List of links to supplier devices. * @consumers: List of links to consumer devices. * @needs_suppliers: Hook to global list of devices waiting for suppliers. - * @defer_sync: Hook to global list of devices that have deferred sync_state. * @status: Driver status information. */ struct dev_links_info { struct list_head suppliers; struct list_head consumers; struct list_head needs_suppliers; - struct list_head defer_sync; enum dl_dev_state status; }; @@ -1262,9 +1242,6 @@ struct dev_links_info { * device. * @has_edit_links: This device has a driver than is capable of * editing the device links created by driver core. - * @state_synced: The hardware state of this device has been synced to match - * the software state of this device by calling the driver/bus - * sync_state() callback. * @dma_coherent: this particular device is dma coherent, even if the * architecture supports non-coherent devices. * @@ -1362,7 +1339,6 @@ struct device { bool offline:1; bool of_node_reused:1; bool has_edit_links:1; - bool state_synced:1; #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \ defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \ defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) @@ -1707,8 +1683,6 @@ struct device_link *device_link_add(struct device *consumer, void device_link_del(struct device_link *link); void device_link_remove(void *consumer, struct device *supplier); void device_link_remove_from_wfs(struct device *consumer); -void device_links_supplier_sync_state_pause(void); -void device_links_supplier_sync_state_resume(void); #ifndef dev_fmt #define dev_fmt(fmt) fmt From d77b3f07a1d7127f1ecaf82a95a36b8a3e8fc689 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 27 Aug 2019 21:40:58 +0200 Subject: [PATCH 41/49] Revert "of/platform: Add functional dependency link from DT bindings" This reverts commit 690ff7881b2681afca1c3c063f4a5cb7c71d9d8b. Based on a lot of email and in-person discussions, this patch series is being reworked to address a number of issues that were pointed out that needed to be taken care of before it should be merged. It will be resubmitted with those changes hopefully soon. Cc: Frank Rowand Cc: Saravana Kannan Signed-off-by: Greg Kroah-Hartman --- .../admin-guide/kernel-parameters.txt | 5 - drivers/of/platform.c | 165 ------------------ 2 files changed, 170 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 794b7fd69361..47d981a86e2f 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -3170,11 +3170,6 @@ This can be set from sysctl after boot. See Documentation/admin-guide/sysctl/vm.rst for details. - of_devlink [KNL] Make device links from common DT bindings. Useful - for optimizing probe order and making sure resources - aren't turned off before the consumer devices have - probed. - ohci1394_dma=early [HW] enable debugging via the ohci1394 driver. See Documentation/debugging-via-ohci1394.txt for more info. diff --git a/drivers/of/platform.c b/drivers/of/platform.c index c1c433333124..b47a2292fe8e 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -503,170 +503,6 @@ int of_platform_default_populate(struct device_node *root, } EXPORT_SYMBOL_GPL(of_platform_default_populate); -bool of_link_is_valid(struct device_node *con, struct device_node *sup) -{ - of_node_get(sup); - /* - * Don't allow linking a device node as a consumer of one of its - * descendant nodes. By definition, a child node can't be a functional - * dependency for the parent node. - */ - while (sup) { - if (sup == con) { - of_node_put(sup); - return false; - } - sup = of_get_next_parent(sup); - } - return true; -} - -static int of_link_to_phandle(struct device *dev, struct device_node *sup_np) -{ - struct platform_device *sup_dev; - u32 dl_flags = DL_FLAG_AUTOPROBE_CONSUMER; - int ret = 0; - - /* - * Since we are trying to create device links, we need to find - * the actual device node that owns this supplier phandle. - * Often times it's the same node, but sometimes it can be one - * of the parents. So walk up the parent till you find a - * device. - */ - while (sup_np && !of_find_property(sup_np, "compatible", NULL)) - sup_np = of_get_next_parent(sup_np); - if (!sup_np) - return 0; - - if (!of_link_is_valid(dev->of_node, sup_np)) { - of_node_put(sup_np); - return 0; - } - sup_dev = of_find_device_by_node(sup_np); - of_node_put(sup_np); - if (!sup_dev) - return -ENODEV; - if (!device_link_add(dev, &sup_dev->dev, dl_flags)) - ret = -ENODEV; - put_device(&sup_dev->dev); - return ret; -} - -static struct device_node *parse_prop_cells(struct device_node *np, - const char *prop, int index, - const char *binding, - const char *cell) -{ - struct of_phandle_args sup_args; - - /* Don't need to check property name for every index. */ - if (!index && strcmp(prop, binding)) - return NULL; - - if (of_parse_phandle_with_args(np, binding, cell, index, &sup_args)) - return NULL; - - return sup_args.np; -} - -static struct device_node *parse_clocks(struct device_node *np, - const char *prop, int index) -{ - return parse_prop_cells(np, prop, index, "clocks", "#clock-cells"); -} - -static struct device_node *parse_interconnects(struct device_node *np, - const char *prop, int index) -{ - return parse_prop_cells(np, prop, index, "interconnects", - "#interconnect-cells"); -} - -static int strcmp_suffix(const char *str, const char *suffix) -{ - unsigned int len, suffix_len; - - len = strlen(str); - suffix_len = strlen(suffix); - if (len <= suffix_len) - return -1; - return strcmp(str + len - suffix_len, suffix); -} - -static struct device_node *parse_regulators(struct device_node *np, - const char *prop, int index) -{ - if (index || strcmp_suffix(prop, "-supply")) - return NULL; - - return of_parse_phandle(np, prop, 0); -} - -/** - * struct supplier_bindings - Information for parsing supplier DT binding - * - * @parse_prop: If the function cannot parse the property, return NULL. - * Otherwise, return the phandle listed in the property - * that corresponds to the index. - */ -struct supplier_bindings { - struct device_node *(*parse_prop)(struct device_node *np, - const char *name, int index); -}; - -static const struct supplier_bindings bindings[] = { - { .parse_prop = parse_clocks, }, - { .parse_prop = parse_interconnects, }, - { .parse_prop = parse_regulators, }, - { }, -}; - -static bool of_link_property(struct device *dev, struct device_node *con_np, - const char *prop) -{ - struct device_node *phandle; - const struct supplier_bindings *s = bindings; - unsigned int i = 0; - bool done = true, matched = false; - - while (!matched && s->parse_prop) { - while ((phandle = s->parse_prop(con_np, prop, i))) { - matched = true; - i++; - if (of_link_to_phandle(dev, phandle)) - /* - * Don't stop at the first failure. See - * Documentation for bus_type.add_links for - * more details. - */ - done = false; - } - s++; - } - return done ? 0 : -ENODEV; -} - -static bool of_devlink; -core_param(of_devlink, of_devlink, bool, 0); - -static int of_link_to_suppliers(struct device *dev) -{ - struct property *p; - bool done = true; - - if (!of_devlink) - return 0; - if (unlikely(!dev->of_node)) - return 0; - - for_each_property_of_node(dev->of_node, p) - if (of_link_property(dev, dev->of_node, p->name)) - done = false; - - return done ? 0 : -ENODEV; -} - #ifndef CONFIG_PPC static const struct of_device_id reserved_mem_matches[] = { { .compatible = "qcom,rmtfs-mem" }, @@ -682,7 +518,6 @@ static int __init of_platform_default_populate_init(void) if (!of_have_populated_dt()) return -ENODEV; - platform_bus_type.add_links = of_link_to_suppliers; /* * Handle certain compatibles explicitly, since we don't want to create * platform_devices for every node in /reserved-memory with a From 33cbfe54499338af08ab906a99afac247ea533f6 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 27 Aug 2019 21:41:06 +0200 Subject: [PATCH 42/49] Revert "driver core: Add edit_links() callback for drivers" This reverts commit 134b23eec9e3a3c795a6ceb0efe2fa63e87983b2. Based on a lot of email and in-person discussions, this patch series is being reworked to address a number of issues that were pointed out that needed to be taken care of before it should be merged. It will be resubmitted with those changes hopefully soon. Cc: Frank Rowand Cc: Saravana Kannan Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 24 ++---------------------- drivers/base/dd.c | 29 ----------------------------- include/linux/device.h | 20 -------------------- 3 files changed, 2 insertions(+), 71 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index de775c7a4d7c..605905de0cca 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -439,19 +439,6 @@ static void device_link_wait_for_supplier(struct device *consumer) mutex_unlock(&wfs_lock); } -/** - * device_link_remove_from_wfs - Unmark device as waiting for supplier - * @consumer: Consumer device - * - * Unmark the consumer device as waiting for suppliers to become available. - */ -void device_link_remove_from_wfs(struct device *consumer) -{ - mutex_lock(&wfs_lock); - list_del_init(&consumer->links.needs_suppliers); - mutex_unlock(&wfs_lock); -} - /** * device_link_check_waiting_consumers - Try to unmark waiting consumers * @@ -469,19 +456,12 @@ void device_link_remove_from_wfs(struct device *consumer) static void device_link_check_waiting_consumers(void) { struct device *dev, *tmp; - int ret; mutex_lock(&wfs_lock); list_for_each_entry_safe(dev, tmp, &wait_for_suppliers, - links.needs_suppliers) { - ret = 0; - if (dev->has_edit_links) - ret = driver_edit_links(dev); - else if (dev->bus->add_links) - ret = dev->bus->add_links(dev); - if (!ret) + links.needs_suppliers) + if (!dev->bus->add_links(dev)) list_del_init(&dev->links.needs_suppliers); - } mutex_unlock(&wfs_lock); } diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 55fbc2467b37..d811e60610d3 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -710,12 +710,6 @@ int driver_probe_device(struct device_driver *drv, struct device *dev) pr_debug("bus: '%s': %s: matched device %s with driver %s\n", drv->bus->name, __func__, dev_name(dev), drv->name); - if (drv->edit_links) { - if (drv->edit_links(dev)) - dev->has_edit_links = true; - else - device_link_remove_from_wfs(dev); - } pm_runtime_get_suppliers(dev); if (dev->parent) pm_runtime_get_sync(dev->parent); @@ -804,29 +798,6 @@ struct device_attach_data { bool have_async; }; -static int __driver_edit_links(struct device_driver *drv, void *data) -{ - struct device *dev = data; - - if (!drv->edit_links) - return 0; - - if (driver_match_device(drv, dev) <= 0) - return 0; - - return drv->edit_links(dev); -} - -int driver_edit_links(struct device *dev) -{ - int ret; - - device_lock(dev); - ret = bus_for_each_drv(dev->bus, NULL, dev, __driver_edit_links); - device_unlock(dev); - return ret; -} - static int __device_attach_driver(struct device_driver *drv, void *_data) { struct device_attach_data *data = _data; diff --git a/include/linux/device.h b/include/linux/device.h index 90142ec9ce84..73210745cc6b 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -349,20 +349,6 @@ enum probe_type { * @probe_type: Type of the probe (synchronous or asynchronous) to use. * @of_match_table: The open firmware table. * @acpi_match_table: The ACPI match table. - * @edit_links: Called to allow a matched driver to edit the device links the - * bus might have added incorrectly. This will be useful to handle - * cases where the bus incorrectly adds functional dependencies - * that aren't true or tries to create cyclic dependencies. But - * doesn't correctly handle functional dependencies that are - * missed by the bus as the supplier's sync_state might get to - * execute before the driver for a missing consumer is loaded and - * gets to edit the device links for the consumer. - * - * This function might be called multiple times after a new device - * is added. The function is expected to create all the device - * links for the new device and return 0 if it was completed - * successfully or return an error if it needs to be reattempted - * in the future. * @probe: Called to query the existence of a specific device, * whether this driver can work with it, and bind the driver * to a specific device. @@ -404,7 +390,6 @@ struct device_driver { const struct of_device_id *of_match_table; const struct acpi_device_id *acpi_match_table; - int (*edit_links)(struct device *dev); int (*probe) (struct device *dev); int (*remove) (struct device *dev); void (*shutdown) (struct device *dev); @@ -1240,8 +1225,6 @@ struct dev_links_info { * @offline: Set after successful invocation of bus type's .offline(). * @of_node_reused: Set if the device-tree node is shared with an ancestor * device. - * @has_edit_links: This device has a driver than is capable of - * editing the device links created by driver core. * @dma_coherent: this particular device is dma coherent, even if the * architecture supports non-coherent devices. * @@ -1338,7 +1321,6 @@ struct device { bool offline_disabled:1; bool offline:1; bool of_node_reused:1; - bool has_edit_links:1; #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \ defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \ defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) @@ -1590,7 +1572,6 @@ extern int __must_check device_attach(struct device *dev); extern int __must_check driver_attach(struct device_driver *drv); extern void device_initial_probe(struct device *dev); extern int __must_check device_reprobe(struct device *dev); -extern int driver_edit_links(struct device *dev); extern bool device_is_bound(struct device *dev); @@ -1682,7 +1663,6 @@ struct device_link *device_link_add(struct device *consumer, struct device *supplier, u32 flags); void device_link_del(struct device_link *link); void device_link_remove(void *consumer, struct device *supplier); -void device_link_remove_from_wfs(struct device *consumer); #ifndef dev_fmt #define dev_fmt(fmt) fmt From bfb3943bed670a0655aa2f9dcf82222ff09d6dd1 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 27 Aug 2019 21:41:16 +0200 Subject: [PATCH 43/49] Revert "driver core: Add support for linking devices during device addition" This reverts commit 5302dd7dd0b6d04c63cdce51d1e9fda9ef0be886. Based on a lot of email and in-person discussions, this patch series is being reworked to address a number of issues that were pointed out that needed to be taken care of before it should be merged. It will be resubmitted with those changes hopefully soon. Cc: Frank Rowand Cc: Saravana Kannan Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 83 ------------------------------------------ include/linux/device.h | 14 ------- 2 files changed, 97 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 605905de0cca..066badad988c 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -44,8 +44,6 @@ early_param("sysfs.deprecated", sysfs_deprecated_setup); #endif /* Device links support. */ -static LIST_HEAD(wait_for_suppliers); -static DEFINE_MUTEX(wfs_lock); #ifdef CONFIG_SRCU static DEFINE_MUTEX(device_links_lock); @@ -420,51 +418,6 @@ struct device_link *device_link_add(struct device *consumer, } EXPORT_SYMBOL_GPL(device_link_add); -/** - * device_link_wait_for_supplier - Mark device as waiting for supplier - * @consumer: Consumer device - * - * Marks the consumer device as waiting for suppliers to become available. The - * consumer device will never be probed until it's unmarked as waiting for - * suppliers. The caller is responsible for adding the link to the supplier - * once the supplier device is present. - * - * This function is NOT meant to be called from the probe function of the - * consumer but rather from code that creates/adds the consumer device. - */ -static void device_link_wait_for_supplier(struct device *consumer) -{ - mutex_lock(&wfs_lock); - list_add_tail(&consumer->links.needs_suppliers, &wait_for_suppliers); - mutex_unlock(&wfs_lock); -} - -/** - * device_link_check_waiting_consumers - Try to unmark waiting consumers - * - * Loops through all consumers waiting on suppliers and tries to add all their - * supplier links. If that succeeds, the consumer device is unmarked as waiting - * for suppliers. Otherwise, they are left marked as waiting on suppliers, - * - * The add_links bus callback is expected to return 0 if it has found and added - * all the supplier links for the consumer device. It should return an error if - * it isn't able to do so. - * - * The caller of device_link_wait_for_supplier() is expected to call this once - * it's aware of potential suppliers becoming available. - */ -static void device_link_check_waiting_consumers(void) -{ - struct device *dev, *tmp; - - mutex_lock(&wfs_lock); - list_for_each_entry_safe(dev, tmp, &wait_for_suppliers, - links.needs_suppliers) - if (!dev->bus->add_links(dev)) - list_del_init(&dev->links.needs_suppliers); - mutex_unlock(&wfs_lock); -} - static void device_link_free(struct device_link *link) { while (refcount_dec_not_one(&link->rpm_active)) @@ -599,19 +552,6 @@ int device_links_check_suppliers(struct device *dev) struct device_link *link; int ret = 0; - /* - * If a device is waiting for one or more suppliers (in - * wait_for_suppliers list), it is not ready to probe yet. So just - * return -EPROBE_DEFER without having to check the links with existing - * suppliers. - */ - mutex_lock(&wfs_lock); - if (!list_empty(&dev->links.needs_suppliers)) { - mutex_unlock(&wfs_lock); - return -EPROBE_DEFER; - } - mutex_unlock(&wfs_lock); - device_links_write_lock(); list_for_each_entry(link, &dev->links.suppliers, c_node) { @@ -896,10 +836,6 @@ static void device_links_purge(struct device *dev) { struct device_link *link, *ln; - mutex_lock(&wfs_lock); - list_del(&dev->links.needs_suppliers); - mutex_unlock(&wfs_lock); - /* * Delete all of the remaining links from this device to any other * devices (either consumers or suppliers). @@ -1764,7 +1700,6 @@ void device_initialize(struct device *dev) #endif INIT_LIST_HEAD(&dev->links.consumers); INIT_LIST_HEAD(&dev->links.suppliers); - INIT_LIST_HEAD(&dev->links.needs_suppliers); dev->links.status = DL_DEV_NO_DRIVER; } EXPORT_SYMBOL_GPL(device_initialize); @@ -2251,24 +2186,6 @@ int device_add(struct device *dev) BUS_NOTIFY_ADD_DEVICE, dev); kobject_uevent(&dev->kobj, KOBJ_ADD); - - /* - * Check if any of the other devices (consumers) have been waiting for - * this device (supplier) to be added so that they can create a device - * link to it. - * - * This needs to happen after device_pm_add() because device_link_add() - * requires the supplier be registered before it's called. - * - * But this also needs to happe before bus_probe_device() to make sure - * waiting consumers can link to it before the driver is bound to the - * device and the driver sync_state callback is called for this device. - */ - device_link_check_waiting_consumers(); - - if (dev->bus && dev->bus->add_links && dev->bus->add_links(dev)) - device_link_wait_for_supplier(dev); - bus_probe_device(dev); if (parent) klist_add_tail(&dev->p->knode_parent, diff --git a/include/linux/device.h b/include/linux/device.h index 73210745cc6b..ec598ede9455 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -78,17 +78,6 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *); * -EPROBE_DEFER it will queue the device for deferred probing. * @uevent: Called when a device is added, removed, or a few other things * that generate uevents to add the environment variables. - * @add_links: Called, perhaps multiple times per device, after a device is - * added to this bus. The function is expected to create device - * links to all the suppliers of the input device that are - * available at the time this function is called. As in, the - * function should NOT stop at the first failed device link if - * other unlinked supplier devices are present in the system. - * - * Return 0 if device links have been successfully created to all - * the suppliers of this device. Return an error if some of the - * suppliers are not yet available and this function needs to be - * reattempted in the future. * @probe: Called when a new device or driver add to this bus, and callback * the specific driver's probe to initial the matched device. * @remove: Called when a device removed from this bus. @@ -133,7 +122,6 @@ struct bus_type { int (*match)(struct device *dev, struct device_driver *drv); int (*uevent)(struct device *dev, struct kobj_uevent_env *env); - int (*add_links)(struct device *dev); int (*probe)(struct device *dev); int (*remove)(struct device *dev); void (*shutdown)(struct device *dev); @@ -1143,13 +1131,11 @@ enum dl_dev_state { * struct dev_links_info - Device data related to device links. * @suppliers: List of links to supplier devices. * @consumers: List of links to consumer devices. - * @needs_suppliers: Hook to global list of devices waiting for suppliers. * @status: Driver status information. */ struct dev_links_info { struct list_head suppliers; struct list_head consumers; - struct list_head needs_suppliers; enum dl_dev_state status; }; From 8973ea47901c81a1912bd05f1577bed9b5b52506 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Wed, 28 Aug 2019 10:34:10 +0200 Subject: [PATCH 44/49] driver core: platform: Introduce platform_get_irq_optional() In some cases the interrupt line of a device is optional. Introduce a new platform_get_irq_optional() that works much like platform_get_irq() but does not output an error on failure to find the interrupt. Signed-off-by: Thierry Reding Reviewed-by: Stephen Boyd Link: https://lore.kernel.org/r/20190828083411.2496-1-thierry.reding@gmail.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 22 ++++++++++++++++++++++ include/linux/platform_device.h | 1 + 2 files changed, 23 insertions(+) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 8ad701068c11..0dda6ade50fd 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -192,6 +192,28 @@ int platform_get_irq(struct platform_device *dev, unsigned int num) } EXPORT_SYMBOL_GPL(platform_get_irq); +/** + * platform_get_irq_optional - get an optional IRQ for a device + * @dev: platform device + * @num: IRQ number index + * + * Gets an IRQ for a platform device. Device drivers should check the return + * value for errors so as to not pass a negative integer value to the + * request_irq() APIs. This is the same as platform_get_irq(), except that it + * does not print an error message if an IRQ can not be obtained. + * + * Example: + * int irq = platform_get_irq_optional(pdev, 0); + * if (irq < 0) + * return irq; + * + * Return: IRQ number on success, negative error number on failure. + */ +int platform_get_irq_optional(struct platform_device *dev, unsigned int num) +{ + return __platform_get_irq(dev, num); +} + /** * platform_irq_count - Count the number of IRQs a platform device uses * @dev: platform device diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 37e15a935a42..35bc4355a9df 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -58,6 +58,7 @@ extern void __iomem * devm_platform_ioremap_resource(struct platform_device *pdev, unsigned int index); extern int platform_get_irq(struct platform_device *, unsigned int); +extern int platform_get_irq_optional(struct platform_device *, unsigned int); extern int platform_irq_count(struct platform_device *); extern struct resource *platform_get_resource_byname(struct platform_device *, unsigned int, From 6e7e5c7fbc1c84e4ac657e0cb5bf9ca24492cc8f Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Wed, 28 Aug 2019 10:34:11 +0200 Subject: [PATCH 45/49] hwmon: pwm-fan: Use platform_get_irq_optional() The PWM fan interrupt is optional, so we don't want an error message in the kernel log if it wasn't specified. Signed-off-by: Thierry Reding Reviewed-by: Stephen Boyd Acked-by: Guenter Roeck Acked-by: Bartlomiej Zolnierkiewicz Link: https://lore.kernel.org/r/20190828083411.2496-2-thierry.reding@gmail.com Signed-off-by: Greg Kroah-Hartman --- drivers/hwmon/pwm-fan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c index 54c0ff00d67f..42ffd2e5182d 100644 --- a/drivers/hwmon/pwm-fan.c +++ b/drivers/hwmon/pwm-fan.c @@ -304,7 +304,7 @@ static int pwm_fan_probe(struct platform_device *pdev) platform_set_drvdata(pdev, ctx); - ctx->irq = platform_get_irq(pdev, 0); + ctx->irq = platform_get_irq_optional(pdev, 0); if (ctx->irq == -EPROBE_DEFER) return ctx->irq; From d9430f96c051ede4e4a3d5a20e68518181e9cd3c Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Thu, 29 Aug 2019 09:29:32 +0200 Subject: [PATCH 46/49] driver core: platform: Export platform_get_irq_optional() This function can be used by modules, so it needs to be exported. Reported-by: kbuild test robot Reported-by: Stephen Rothwell Signed-off-by: Thierry Reding Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 0dda6ade50fd..11c6e56ccc22 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -213,6 +213,7 @@ int platform_get_irq_optional(struct platform_device *dev, unsigned int num) { return __platform_get_irq(dev, num); } +EXPORT_SYMBOL_GPL(platform_get_irq_optional); /** * platform_irq_count - Count the number of IRQs a platform device uses From 7f905761e15a870761a7f9ba14bbb9e132ab8481 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 26 Aug 2019 17:01:53 +0200 Subject: [PATCH 47/49] sysfs: add BIN_ATTR_WO() macro This variant was missing from sysfs.h, I guess no one noticed it before. Turns out the powerpc secure variable code can use it, so add it to the tree for it, and potentially others to take advantage of, instead of open-coding it. Reported-by: Nayna Jain Signed-off-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20190826150153.GD18418@kroah.com Signed-off-by: Greg Kroah-Hartman --- include/linux/sysfs.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index 965236795750..5420817ed317 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -196,6 +196,12 @@ struct bin_attribute { .size = _size, \ } +#define __BIN_ATTR_WO(_name) { \ + .attr = { .name = __stringify(_name), .mode = 0200 }, \ + .store = _name##_store, \ + .size = _size, \ +} + #define __BIN_ATTR_RW(_name, _size) \ __BIN_ATTR(_name, 0644, _name##_read, _name##_write, _size) @@ -208,6 +214,9 @@ struct bin_attribute bin_attr_##_name = __BIN_ATTR(_name, _mode, _read, \ #define BIN_ATTR_RO(_name, _size) \ struct bin_attribute bin_attr_##_name = __BIN_ATTR_RO(_name, _size) +#define BIN_ATTR_WO(_name, _size) \ +struct bin_attribute bin_attr_##_name = __BIN_ATTR_WO(_name, _size) + #define BIN_ATTR_RW(_name, _size) \ struct bin_attribute bin_attr_##_name = __BIN_ATTR_RW(_name, _size) From 1d5a013f9c0f5b4ef3cf64ac61e9c1f386c7b750 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 19 Aug 2019 16:16:06 +0900 Subject: [PATCH 48/49] driver-core: add include guard to linux/container.h Add a header include guard just in case. Signed-off-by: Masahiro Yamada Reviewed-by: Rafael J. Wysocki Link: https://lore.kernel.org/r/20190819071606.10965-1-yamada.masahiro@socionext.com Signed-off-by: Greg Kroah-Hartman --- include/linux/container.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/linux/container.h b/include/linux/container.h index 0cc2ee91905c..2566a1baa736 100644 --- a/include/linux/container.h +++ b/include/linux/container.h @@ -6,6 +6,9 @@ * Author: Rafael J. Wysocki */ +#ifndef _LINUX_CONTAINER_H +#define _LINUX_CONTAINER_H + #include /* drivers/base/power/container.c */ @@ -20,3 +23,5 @@ static inline struct container_dev *to_container_dev(struct device *dev) { return container_of(dev, struct container_dev, dev); } + +#endif /* _LINUX_CONTAINER_H */ From ca7ce5a2710ad2a57bf7d0c4c712590bb69a5e1c Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Fri, 6 Sep 2019 11:30:06 +0800 Subject: [PATCH 49/49] coccinelle: platform_get_irq: Fix parse error When do coccicheck, I get this error: spatch -D report --no-show-diff --very-quiet --cocci-file ./scripts/coccinelle/api/platform_get_irq.cocci --include-headers --dir . -I ./arch/x86/include -I ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi -I ./arch/x86/include/generated/uapi -I ./include/uapi -I ./include/generated/uapi --include ./include/linux/kconfig.h --jobs 192 --chunksize 1 minus: parse error: File "./scripts/coccinelle/api/platform_get_irq.cocci", line 24, column 9, charpos = 355 around = '\(', whole content = if ( ret \( < \| <= \) 0 ) In commit e56476897448 ("fpga: Remove dev_err() usage after platform_get_irq()") log, I found the semantic patch, it fix this issue. Fixes: 98051ba2b28b ("coccinelle: Add script to check for platform_get_irq() excessive prints") Signed-off-by: YueHaibing Reviewed-by: Stephen Boyd Acked-by: Julia Lawall Link: https://lore.kernel.org/r/20190906033006.17616-1-yuehaibing@huawei.com Signed-off-by: Greg Kroah-Hartman --- scripts/coccinelle/api/platform_get_irq.cocci | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/coccinelle/api/platform_get_irq.cocci b/scripts/coccinelle/api/platform_get_irq.cocci index f6e1afc08c0b..06b6a95e2bfc 100644 --- a/scripts/coccinelle/api/platform_get_irq.cocci +++ b/scripts/coccinelle/api/platform_get_irq.cocci @@ -21,7 +21,7 @@ platform_get_irq platform_get_irq_byname )(E, ...); -if ( ret \( < \| <= \) 0 ) +if ( \( ret < 0 \| ret <= 0 \) ) { ( if (ret != -EPROBE_DEFER) @@ -47,7 +47,7 @@ platform_get_irq platform_get_irq_byname )(E, ...); -if ( ret \( < \| <= \) 0 ) +if ( \( ret < 0 \| ret <= 0 \) ) { ( -if (ret != -EPROBE_DEFER) @@ -74,7 +74,7 @@ platform_get_irq platform_get_irq_byname )(E, ...); -if ( ret \( < \| <= \) 0 ) +if ( \( ret < 0 \| ret <= 0 \) ) { ( if (ret != -EPROBE_DEFER)