linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] driver core: Managed device links rework and "consumer autoprobe" flag
@ 2019-01-28 23:03 Rafael J. Wysocki
  2019-01-28 23:05 ` [PATCH 1/4] IOMMU: Make dwo drivers use stateless device links Rafael J. Wysocki
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2019-01-28 23:03 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: LKML, Linux PM, Ulf Hansson, Daniel Vetter, Lukas Wunner,
	Andrzej Hajda, Russell King - ARM Linux, Lucas Stach,
	Linus Walleij, Thierry Reding, Laurent Pinchart, Joerg Roedel

Hi Greg at al,

This is on top of the series I posted on Thursday last week:

https://lore.kernel.org/lkml/2493187.oiOpCWJBV7@aspire.rjw.lan/

and the purpose here is to (1) clean up the remaining device link management
issues mentioned in the cover letter above and (2) make device links support
the "composite device" use case mentioned during the recent discussion on
possibly using device links in the DRM subsystem (see for example
https://marc.info/?l=linux-pm&m=154832771905309&w=2).

Patch [1/4] simply makes two IOMMU drivers pass DL_FLAG_STATELESS to
device_link_add(), because the cleanup part of their device links
handling will not work any more after patch [2/4] and there is no
reason why those links cannot be stateless AFAICS.

Patch [2/4] can be regarded as a fix as it cleans up some link management
issues related to device links reference counting and stateful (managed)
device links.

Patch [3/4] add a "consumer autoprobe" flag causing the driver core to
probe consumer drivers automatically after binding the supplier driver
for persistent managed device links.

Patch [4/4] is just another fix on top of the previous series of device
links patches and it doesn't depend on patches [1-3/4].

For easier testing I have created a git branch containing these patches
along with the previous series (including the v2 of the 5th patch) which
can be pulled from:

 git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
 device-links

(not that this branch may be rebased in the future).

Cheers,
Rafael



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/4] IOMMU: Make dwo drivers use stateless device links
  2019-01-28 23:03 [PATCH 0/4] driver core: Managed device links rework and "consumer autoprobe" flag Rafael J. Wysocki
@ 2019-01-28 23:05 ` Rafael J. Wysocki
  2019-01-29  8:55   ` Marek Szyprowski
  2019-01-29  9:17   ` Joerg Roedel
  2019-01-28 23:06 ` [PATCH 2/4] driver core: Make driver core own stateful " Rafael J. Wysocki
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2019-01-28 23:05 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Joerg Roedel
  Cc: LKML, Linux PM, Ulf Hansson, Daniel Vetter, Lukas Wunner,
	Andrzej Hajda, Russell King - ARM Linux, Lucas Stach,
	Linus Walleij, Thierry Reding, Laurent Pinchart,
	Marek Szyprowski, Jeffy Chen

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The device links used by rockchip-iommu and exynos-iommu are
completely managed by these drivers within the IOMMU framework,
so there is no reason to involve the driver core in the management
of these links.

For this reason, make rockchip-iommu and exynos-iommu pass
DL_FLAG_STATELESS in flags to device_link_add(), so that the device
links used by them are stateless.

[Note that this change is requisite for a subsequent one that will
 rework the management of stateful device links in the driver core
 and it will not be compatible with the two drivers in question any
 more.]

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/iommu/exynos-iommu.c   |    1 +
 drivers/iommu/rockchip-iommu.c |    3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/iommu/rockchip-iommu.c
===================================================================
--- linux-pm.orig/drivers/iommu/rockchip-iommu.c
+++ linux-pm/drivers/iommu/rockchip-iommu.c
@@ -1071,7 +1071,8 @@ static int rk_iommu_add_device(struct de
 	iommu_group_put(group);
 
 	iommu_device_link(&iommu->iommu, dev);
-	data->link = device_link_add(dev, iommu->dev, DL_FLAG_PM_RUNTIME);
+	data->link = device_link_add(dev, iommu->dev,
+				     DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
 
 	return 0;
 }
Index: linux-pm/drivers/iommu/exynos-iommu.c
===================================================================
--- linux-pm.orig/drivers/iommu/exynos-iommu.c
+++ linux-pm/drivers/iommu/exynos-iommu.c
@@ -1260,6 +1260,7 @@ static int exynos_iommu_add_device(struc
 		 * direct calls to pm_runtime_get/put in this driver.
 		 */
 		data->link = device_link_add(dev, data->sysmmu,
+					     DL_FLAG_STATELESS |
 					     DL_FLAG_PM_RUNTIME);
 	}
 	iommu_group_put(group);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/4] driver core: Make driver core own stateful device links
  2019-01-28 23:03 [PATCH 0/4] driver core: Managed device links rework and "consumer autoprobe" flag Rafael J. Wysocki
  2019-01-28 23:05 ` [PATCH 1/4] IOMMU: Make dwo drivers use stateless device links Rafael J. Wysocki
@ 2019-01-28 23:06 ` Rafael J. Wysocki
  2019-01-28 23:07 ` [PATCH 3/4] driver core: Add device link flag DL_FLAG_AUTOPROBE_CONSUMER Rafael J. Wysocki
  2019-01-28 23:08 ` [PATCH 4/4] driver core: Do not call rpm_put_suppliers() in pm_runtime_drop_link() Rafael J. Wysocki
  3 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2019-01-28 23:06 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: LKML, Linux PM, Ulf Hansson, Daniel Vetter, Lukas Wunner,
	Andrzej Hajda, Russell King - ARM Linux, Lucas Stach,
	Linus Walleij, Thierry Reding, Laurent Pinchart, Joerg Roedel

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Even though stateful device links are managed by the driver core in
principle, their creators are allowed and sometimes even expected
to drop references to them via device_link_del() or
device_link_remove(), but that doesn't really play well with the
DL_FLAG_AUTOREMOVE_CONSUMER and DL_FLAG_AUTOREMOVE_SUPPLIER device
link flags and with the "persistent" link concept.

For instance, if device_link_add() is called from two different
places for the same consumer-supplier pair and each time
DL_FLAG_AUTOREMOVE_CONSUMER is passed in the flags to it, each
of the two callers of it may expect the link to be removed
automatically and they will not drop their references to the
link, but the core will only drop one reference to it when
unbinding the consumer driver from its device.  The link will
then stay around, which may be unexpected.

Moreover, if "persistent" device links are created from driver
probe callbacks, device_link_add() called to do that will take a
new reference to the link each time the callback runs and those
references will never be dropped, which kind of isn't nice.

These issues arise because of the link reference counting carried
out by device_link_add() for existing links, but that is only done to
avoid deleting device links that may still be necessary, which
shouldn't be a concern for stateful links.  These device links are
managed by the driver core and whoever creates one of them will
need it at least as long as until the consumer driver is detached
from its device and deleting it may be left to the driver core just
fine.

For this reason, rework device_link_add() to apply the reference
counting to stateless links only and make device_link_del() and
device_link_remove() drop references to stateless links only too.
After this change, if called to add a stateful device link for
a consumer-supplier pair for which a stateful device link is
present already, device_link_add() will return the existing link
without incrementing its reference counter.  Accordingly,
device_link_del() and device_link_remove() will WARN() and do
nothing when called to drop a reference to a stateful link.  Thus,
effectively, all stateful device links will be owned by the driver
core.

In addition, clean up the handling of the link management flags,
DL_FLAG_AUTOREMOVE_CONSUMER and DL_FLAG_AUTOREMOVE_SUPPLIER, so that
(a) they are never set at the same time and (b) if device_link_add()
is called for a consumer-supplier pair with an existing stateful link
between them, the flags of that link will be combined with the flags
passed to device_link_add() to ensure that the life time of the link
is sufficient for all of the callers of device_link_add() for the
same consumer-supplier pair.

Update the device_link_add() kerneldoc comment to reflect the
above changes.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/driver-api/device_link.rst |   42 ++++++++++-------
 drivers/base/core.c                      |   74 +++++++++++++++++++++++--------
 2 files changed, 81 insertions(+), 35 deletions(-)

Index: linux-pm/drivers/base/core.c
===================================================================
--- linux-pm.orig/drivers/base/core.c
+++ linux-pm/drivers/base/core.c
@@ -192,11 +192,21 @@ static void device_link_rpm_prepare(stru
  * of the link.  If DL_FLAG_PM_RUNTIME is not set, DL_FLAG_RPM_ACTIVE will be
  * ignored.
  *
- * If the DL_FLAG_AUTOREMOVE_CONSUMER flag is set, the reference to the link
- * acquired by this function will be dropped automatically when the consumer
- * device driver unbinds from it.  Analogously, if DL_FLAG_AUTOREMOVE_SUPPLIER
- * is set in @flags, the reference to the link acquired by this function will
- * be dropped automatically when the supplier device driver unbinds from it.
+ * If DL_FLAG_STATELESS is set in @flags, the link is not going to be managed by
+ * the driver core and, in particular, the caller of this function is expected
+ * to drop the reference to the link acquired by it directly.
+ *
+ * If that flag is not set, however, the caller of this function is handing the
+ * management of the link over to the driver core entirely and its return value
+ * can only be used to check whether or not the link is present.  In that case,
+ * the DL_FLAG_AUTOREMOVE_CONSUMER and DL_FLAG_AUTOREMOVE_SUPPLIER device link
+ * flags can be used to indicate to the driver core when the link can be safely
+ * deleted.  Namely, setting one of them in @flags indicates to the driver core
+ * that the link is not going to be used (by the given caller of this function)
+ * after unbinding the consumer or supplier driver, respectively, from its
+ * device, so the link can be deleted at that point.  If none of them is set,
+ * the link will be maintained until one of the devices pointed to by it (either
+ * the consumer or the supplier) is unregistered.
  *
  * The combination of DL_FLAG_STATELESS and either DL_FLAG_AUTOREMOVE_CONSUMER
  * or DL_FLAG_AUTOREMOVE_SUPPLIER set in @flags at the same time is invalid and
@@ -242,6 +252,14 @@ struct device_link *device_link_add(stru
 		goto out;
 	}
 
+	/*
+	 * DL_FLAG_AUTOREMOVE_SUPPLIER indicates that the link will be needed
+	 * longer than for DL_FLAG_AUTOREMOVE_CONSUMER and setting them both
+	 * together doesn't make sense, so prefer DL_FLAG_AUTOREMOVE_SUPPLIER.
+	 */
+	if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER)
+		flags &= ~DL_FLAG_AUTOREMOVE_CONSUMER;
+
 	list_for_each_entry(link, &supplier->links.consumers, s_node) {
 		if (link->consumer != consumer)
 			continue;
@@ -255,12 +273,6 @@ struct device_link *device_link_add(stru
 			goto out;
 		}
 
-		if (flags & DL_FLAG_AUTOREMOVE_CONSUMER)
-			link->flags |= DL_FLAG_AUTOREMOVE_CONSUMER;
-
-		if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER)
-			link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER;
-
 		if (flags & DL_FLAG_PM_RUNTIME) {
 			if (!(link->flags & DL_FLAG_PM_RUNTIME)) {
 				device_link_rpm_prepare(consumer, supplier);
@@ -270,7 +282,25 @@ struct device_link *device_link_add(stru
 				refcount_inc(&link->rpm_active);
 		}
 
-		kref_get(&link->kref);
+		if (flags & DL_FLAG_STATELESS) {
+			kref_get(&link->kref);
+			goto out;
+		}
+
+		/*
+		 * If the life time of the link following from the new flags is
+		 * longer than indicated by the flags of the existing link,
+		 * update the existing link to stay around longer.
+		 */
+		if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) {
+			if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) {
+				link->flags &= ~DL_FLAG_AUTOREMOVE_CONSUMER;
+				link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER;
+			}
+		} else if (!(flags & DL_FLAG_AUTOREMOVE_CONSUMER)) {
+			link->flags &= ~(DL_FLAG_AUTOREMOVE_CONSUMER |
+					 DL_FLAG_AUTOREMOVE_SUPPLIER);
+		}
 		goto out;
 	}
 
@@ -420,8 +450,16 @@ static void __device_link_del(struct kre
 }
 #endif /* !CONFIG_SRCU */
 
+static void device_link_put_kref(struct device_link *link)
+{
+	if (link->flags & DL_FLAG_STATELESS)
+		kref_put(&link->kref, __device_link_del);
+	else
+		WARN(1, "Unable to drop a managed device link reference\n");
+}
+
 /**
- * device_link_del - Delete a link between two devices.
+ * device_link_del - Delete a stateless link between two devices.
  * @link: Device link to delete.
  *
  * The caller must ensure proper synchronization of this function with runtime
@@ -433,14 +471,14 @@ void device_link_del(struct device_link
 {
 	device_links_write_lock();
 	device_pm_lock();
-	kref_put(&link->kref, __device_link_del);
+	device_link_put_kref(link);
 	device_pm_unlock();
 	device_links_write_unlock();
 }
 EXPORT_SYMBOL_GPL(device_link_del);
 
 /**
- * device_link_remove - remove a link between two devices.
+ * device_link_remove - Delete a stateless link between two devices.
  * @consumer: Consumer end of the link.
  * @supplier: Supplier end of the link.
  *
@@ -459,7 +497,7 @@ void device_link_remove(void *consumer,
 
 	list_for_each_entry(link, &supplier->links.consumers, s_node) {
 		if (link->consumer == consumer) {
-			kref_put(&link->kref, __device_link_del);
+			device_link_put_kref(link);
 			break;
 		}
 	}
@@ -591,7 +629,7 @@ static void __device_links_no_driver(str
 			WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
 
 		if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER)
-			kref_put(&link->kref, __device_link_del);
+			__device_link_del(&link->kref);
 	}
 
 	dev->links.status = DL_DEV_NO_DRIVER;
@@ -660,7 +698,7 @@ void device_links_driver_cleanup(struct
 		WRITE_ONCE(link->status, DL_STATE_DORMANT);
 
 		if (link->flags & DL_FLAG_AUTOREMOVE_SUPPLIER)
-			kref_put(&link->kref, __device_link_del);
+			__device_link_del(&link->kref);
 	}
 
 	__device_links_no_driver(dev);
Index: linux-pm/Documentation/driver-api/device_link.rst
===================================================================
--- linux-pm.orig/Documentation/driver-api/device_link.rst
+++ linux-pm/Documentation/driver-api/device_link.rst
@@ -25,8 +25,8 @@ suspend/resume and shutdown ordering.
 
 Device links allow representation of such dependencies in the driver core.
 
-In its standard form, a device link combines *both* dependency types:
-It guarantees correct suspend/resume and shutdown ordering between a
+In its standard or *managed* form, a device link combines *both* dependency
+types:  It guarantees correct suspend/resume and shutdown ordering between a
 "supplier" device and its "consumer" devices, and it guarantees driver
 presence on the supplier.  The consumer devices are not probed before the
 supplier is bound to a driver, and they're unbound before the supplier
@@ -69,12 +69,14 @@ know that the supplier is functional alr
 the case, for instance, if the consumer has just acquired some resources that
 would not have been available had the supplier not been functional then).]
 
-If a device link is added in the ``->probe`` callback of the supplier or
-consumer driver, it is 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.
+If a device link with ``DL_FLAG_STATELESS`` set (i.e. a stateless device link)
+is added in the ``->probe`` callback of the supplier or consumer driver, it is
+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.
 
 Several flags may be specified on device link addition, two of which
 have already been mentioned above:  ``DL_FLAG_STATELESS`` to express that no
@@ -87,8 +89,6 @@ link is added from the consumer's ``->pr
 can be specified to runtime resume the supplier upon addition of the
 device link.  ``DL_FLAG_AUTOREMOVE_CONSUMER`` causes the device link to be
 automatically purged when the consumer fails to probe or later unbinds.
-This obviates the need to explicitly delete the link in the ``->remove``
-callback or in the error path of the ``->probe`` callback.
 
 Similarly, when the device link is added from supplier's ``->probe`` callback,
 ``DL_FLAG_AUTOREMOVE_SUPPLIER`` causes the device link to be automatically
@@ -97,12 +97,20 @@ purged when the supplier fails to probe
 Limitations
 ===========
 
-Driver authors should be aware that a driver presence dependency (i.e. when
-``DL_FLAG_STATELESS`` is not specified on link addition) may cause probing of
-the consumer to be deferred indefinitely.  This can become a problem if the
-consumer is required to probe before a certain initcall level is reached.
-Worse, if the supplier driver is blacklisted or missing, the consumer will
-never be probed.
+Driver authors should be aware that a driver presence dependency for managed
+device links (i.e. when ``DL_FLAG_STATELESS`` is not specified on link addition)
+may cause probing of the consumer to be deferred indefinitely.  This can become
+a problem if the consumer is required to probe before a certain initcall level
+is reached.  Worse, if the supplier driver is blacklisted or missing, the
+consumer will never be probed.
+
+Moreover, managed device links cannot be deleted directly.  They are deleted
+by the driver core when they are not necessary any more in accordance with the
+``DL_FLAG_AUTOREMOVE_CONSUMER`` and ``DL_FLAG_AUTOREMOVE_SUPPLIER`` flags.
+However, stateless device links (i.e. device links with ``DL_FLAG_STATELESS``
+set) are expected to be removed by whoever called :c:func:`device_link_add()`
+to add them with the help of either :c:func:`device_link_del()` or
+:c:func:`device_link_remove()`.
 
 Sometimes drivers depend on optional resources.  They are able to operate
 in a degraded mode (reduced feature set or performance) when those resources
@@ -286,4 +294,4 @@ API
 ===
 
 .. kernel-doc:: drivers/base/core.c
-   :functions: device_link_add device_link_del
+   :functions: device_link_add device_link_del device_link_remove


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/4] driver core: Add device link flag DL_FLAG_AUTOPROBE_CONSUMER
  2019-01-28 23:03 [PATCH 0/4] driver core: Managed device links rework and "consumer autoprobe" flag Rafael J. Wysocki
  2019-01-28 23:05 ` [PATCH 1/4] IOMMU: Make dwo drivers use stateless device links Rafael J. Wysocki
  2019-01-28 23:06 ` [PATCH 2/4] driver core: Make driver core own stateful " Rafael J. Wysocki
@ 2019-01-28 23:07 ` Rafael J. Wysocki
  2019-01-28 23:08 ` [PATCH 4/4] driver core: Do not call rpm_put_suppliers() in pm_runtime_drop_link() Rafael J. Wysocki
  3 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2019-01-28 23:07 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: LKML, Linux PM, Ulf Hansson, Daniel Vetter, Lukas Wunner,
	Andrzej Hajda, Russell King - ARM Linux, Lucas Stach,
	Linus Walleij, Thierry Reding, Laurent Pinchart, Joerg Roedel

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add a new device link flag, DL_FLAG_AUTOPROBE_CONSUMER, to request the
driver core to probe for a consumer driver automatically after binding
a driver to the supplier device on a persistent managed device link.

As unbinding the supplier driver on a managed device link causes the
consumer driver to be detached from its device automatically, this
flag provides a complementary mechanism which is needed to address
some "composite device" use cases.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/driver-api/device_link.rst |    9 +++++++++
 drivers/base/core.c                      |   16 +++++++++++++++-
 drivers/base/dd.c                        |    2 +-
 include/linux/device.h                   |    3 +++
 4 files changed, 28 insertions(+), 2 deletions(-)

Index: linux-pm/include/linux/device.h
===================================================================
--- linux-pm.orig/include/linux/device.h
+++ linux-pm/include/linux/device.h
@@ -341,6 +341,7 @@ struct device *driver_find_device(struct
 				  struct device *start, void *data,
 				  int (*match)(struct device *dev, void *data));
 
+void driver_deferred_probe_add(struct device *dev);
 int driver_deferred_probe_check_state(struct device *dev);
 
 /**
@@ -827,12 +828,14 @@ enum device_link_state {
  * 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.
  */
 #define DL_FLAG_STATELESS		BIT(0)
 #define DL_FLAG_AUTOREMOVE_CONSUMER	BIT(1)
 #define DL_FLAG_PM_RUNTIME		BIT(2)
 #define DL_FLAG_RPM_ACTIVE		BIT(3)
 #define DL_FLAG_AUTOREMOVE_SUPPLIER	BIT(4)
+#define DL_FLAG_AUTOPROBE_CONSUMER	BIT(5)
 
 /**
  * struct device_link - Device link representation.
Index: linux-pm/drivers/base/core.c
===================================================================
--- linux-pm.orig/drivers/base/core.c
+++ linux-pm/drivers/base/core.c
@@ -208,6 +208,12 @@ static void device_link_rpm_prepare(stru
  * the link will be maintained until one of the devices pointed to by it (either
  * the consumer or the supplier) is unregistered.
  *
+ * Also, if DL_FLAG_STATELESS, DL_FLAG_AUTOREMOVE_CONSUMER and
+ * DL_FLAG_AUTOREMOVE_SUPPLIER are not set in @flags (that is, a persistent
+ * managed device link is being added), the DL_FLAG_AUTOPROBE_CONSUMER flag can
+ * be used to request the driver core to automaticall probe for a consmer
+ * driver after successfully binding a driver to the supplier device.
+ *
  * The combination of DL_FLAG_STATELESS and either DL_FLAG_AUTOREMOVE_CONSUMER
  * or DL_FLAG_AUTOREMOVE_SUPPLIER set in @flags at the same time is invalid and
  * will cause NULL to be returned upfront.
@@ -228,7 +234,12 @@ struct device_link *device_link_add(stru
 
 	if (!consumer || !supplier ||
 	    (flags & DL_FLAG_STATELESS &&
-	     flags & (DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_AUTOREMOVE_SUPPLIER)))
+	     flags & (DL_FLAG_AUTOREMOVE_CONSUMER |
+		      DL_FLAG_AUTOREMOVE_SUPPLIER |
+		      DL_FLAG_AUTOPROBE_CONSUMER)) ||
+	    (flags & DL_FLAG_AUTOPROBE_CONSUMER &&
+	     flags & (DL_FLAG_AUTOREMOVE_CONSUMER |
+		      DL_FLAG_AUTOREMOVE_SUPPLIER)))
 		return NULL;
 
 	if (flags & DL_FLAG_PM_RUNTIME && flags & DL_FLAG_RPM_ACTIVE) {
@@ -589,6 +600,9 @@ void device_links_driver_bound(struct de
 
 		WARN_ON(link->status != DL_STATE_DORMANT);
 		WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
+
+		if (link->flags & DL_FLAG_AUTOPROBE_CONSUMER)
+			driver_deferred_probe_add(link->consumer);
 	}
 
 	list_for_each_entry(link, &dev->links.suppliers, c_node) {
Index: linux-pm/drivers/base/dd.c
===================================================================
--- linux-pm.orig/drivers/base/dd.c
+++ linux-pm/drivers/base/dd.c
@@ -116,7 +116,7 @@ static void deferred_probe_work_func(str
 }
 static DECLARE_WORK(deferred_probe_work, deferred_probe_work_func);
 
-static void driver_deferred_probe_add(struct device *dev)
+void driver_deferred_probe_add(struct device *dev)
 {
 	mutex_lock(&deferred_probe_mutex);
 	if (list_empty(&dev->p->deferred_probe)) {
Index: linux-pm/Documentation/driver-api/device_link.rst
===================================================================
--- linux-pm.orig/Documentation/driver-api/device_link.rst
+++ linux-pm/Documentation/driver-api/device_link.rst
@@ -94,6 +94,15 @@ Similarly, when the device link is added
 ``DL_FLAG_AUTOREMOVE_SUPPLIER`` causes the device link to be automatically
 purged when the supplier fails to probe or later unbinds.
 
+If neither ``DL_FLAG_AUTOREMOVE_CONSUMER`` nor ``DL_FLAG_AUTOREMOVE_SUPPLIER``
+is set, ``DL_FLAG_AUTOPROBE_CONSUMER`` can be used to request the driver core
+to probe for a driver for the consumer driver on the link automatically after
+a driver has been bound to the supplier device.
+
+Note, however, that any combinations of ``DL_FLAG_AUTOREMOVE_CONSUMER``,
+``DL_FLAG_AUTOREMOVE_SUPPLIER`` or ``DL_FLAG_AUTOPROBE_CONSUMER`` with
+``DL_FLAG_STATELESS`` are invalid and cannot be used.
+
 Limitations
 ===========
 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 4/4] driver core: Do not call rpm_put_suppliers() in pm_runtime_drop_link()
  2019-01-28 23:03 [PATCH 0/4] driver core: Managed device links rework and "consumer autoprobe" flag Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2019-01-28 23:07 ` [PATCH 3/4] driver core: Add device link flag DL_FLAG_AUTOPROBE_CONSUMER Rafael J. Wysocki
@ 2019-01-28 23:08 ` Rafael J. Wysocki
  3 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2019-01-28 23:08 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: LKML, Linux PM, Ulf Hansson, Daniel Vetter, Lukas Wunner,
	Andrzej Hajda, Russell King - ARM Linux, Lucas Stach,
	Linus Walleij, Thierry Reding, Laurent Pinchart, Joerg Roedel

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Calling rpm_put_suppliers() from pm_runtime_drop_link() is excessive
as it affects all suppliers of the consumer device and not just the
one pointed to by the device link being dropped.  Worst case it may
cause the consumer device to stop working unexpectedly.  Moreover, in
principle it is racy with respect to runtime PM of the consumer
device.

To avoid these problems drop runtime PM references on the particular
supplier pointed to by the link in question only and do that after
the link has been dropped from the consumer device's list of links to
suppliers, which is in device_link_free().

Fixes: a0504aecba76 ("PM / runtime: Drop usage count for suppliers at device link removal")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/core.c          |    3 +++
 drivers/base/power/runtime.c |    2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/base/core.c
===================================================================
--- linux-pm.orig/drivers/base/core.c
+++ linux-pm/drivers/base/core.c
@@ -375,6 +375,9 @@ EXPORT_SYMBOL_GPL(device_link_add);
 
 static void device_link_free(struct device_link *link)
 {
+	while (refcount_dec_not_one(&link->rpm_active))
+		pm_runtime_put(link->supplier);
+
 	put_device(link->consumer);
 	put_device(link->supplier);
 	kfree(link);
Index: linux-pm/drivers/base/power/runtime.c
===================================================================
--- linux-pm.orig/drivers/base/power/runtime.c
+++ linux-pm/drivers/base/power/runtime.c
@@ -1623,8 +1623,6 @@ void pm_runtime_new_link(struct device *
 
 void pm_runtime_drop_link(struct device *dev)
 {
-	rpm_put_suppliers(dev);
-
 	spin_lock_irq(&dev->power.lock);
 	WARN_ON(dev->power.links_count == 0);
 	dev->power.links_count--;


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/4] IOMMU: Make dwo drivers use stateless device links
  2019-01-28 23:05 ` [PATCH 1/4] IOMMU: Make dwo drivers use stateless device links Rafael J. Wysocki
@ 2019-01-29  8:55   ` Marek Szyprowski
  2019-01-29  9:17   ` Joerg Roedel
  1 sibling, 0 replies; 7+ messages in thread
From: Marek Szyprowski @ 2019-01-29  8:55 UTC (permalink / raw)
  To: Rafael J. Wysocki, Greg Kroah-Hartman, Joerg Roedel
  Cc: LKML, Linux PM, Ulf Hansson, Daniel Vetter, Lukas Wunner,
	Andrzej Hajda, Russell King - ARM Linux, Lucas Stach,
	Linus Walleij, Thierry Reding, Laurent Pinchart, Jeffy Chen

Hi Rafael,

On 2019-01-29 00:05, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The device links used by rockchip-iommu and exynos-iommu are
> completely managed by these drivers within the IOMMU framework,
> so there is no reason to involve the driver core in the management
> of these links.
>
> For this reason, make rockchip-iommu and exynos-iommu pass
> DL_FLAG_STATELESS in flags to device_link_add(), so that the device
> links used by them are stateless.
>
> [Note that this change is requisite for a subsequent one that will
>  rework the management of stateful device links in the driver core
>  and it will not be compatible with the two drivers in question any
>  more.]
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Indeed there is no need to use driver-state tracking feature of device
links in this case.

For the Exynos IOMMU driver:

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>  drivers/iommu/exynos-iommu.c   |    1 +
>  drivers/iommu/rockchip-iommu.c |    3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/iommu/rockchip-iommu.c
> ===================================================================
> --- linux-pm.orig/drivers/iommu/rockchip-iommu.c
> +++ linux-pm/drivers/iommu/rockchip-iommu.c
> @@ -1071,7 +1071,8 @@ static int rk_iommu_add_device(struct de
>  	iommu_group_put(group);
>  
>  	iommu_device_link(&iommu->iommu, dev);
> -	data->link = device_link_add(dev, iommu->dev, DL_FLAG_PM_RUNTIME);
> +	data->link = device_link_add(dev, iommu->dev,
> +				     DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
>  
>  	return 0;
>  }
> Index: linux-pm/drivers/iommu/exynos-iommu.c
> ===================================================================
> --- linux-pm.orig/drivers/iommu/exynos-iommu.c
> +++ linux-pm/drivers/iommu/exynos-iommu.c
> @@ -1260,6 +1260,7 @@ static int exynos_iommu_add_device(struc
>  		 * direct calls to pm_runtime_get/put in this driver.
>  		 */
>  		data->link = device_link_add(dev, data->sysmmu,
> +					     DL_FLAG_STATELESS |
>  					     DL_FLAG_PM_RUNTIME);
>  	}
>  	iommu_group_put(group);
>
>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/4] IOMMU: Make dwo drivers use stateless device links
  2019-01-28 23:05 ` [PATCH 1/4] IOMMU: Make dwo drivers use stateless device links Rafael J. Wysocki
  2019-01-29  8:55   ` Marek Szyprowski
@ 2019-01-29  9:17   ` Joerg Roedel
  1 sibling, 0 replies; 7+ messages in thread
From: Joerg Roedel @ 2019-01-29  9:17 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Greg Kroah-Hartman, LKML, Linux PM, Ulf Hansson, Daniel Vetter,
	Lukas Wunner, Andrzej Hajda, Russell King - ARM Linux,
	Lucas Stach, Linus Walleij, Thierry Reding, Laurent Pinchart,
	Marek Szyprowski, Jeffy Chen

On Tue, Jan 29, 2019 at 12:05:21AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The device links used by rockchip-iommu and exynos-iommu are
> completely managed by these drivers within the IOMMU framework,
> so there is no reason to involve the driver core in the management
> of these links.
> 
> For this reason, make rockchip-iommu and exynos-iommu pass
> DL_FLAG_STATELESS in flags to device_link_add(), so that the device
> links used by them are stateless.
> 
> [Note that this change is requisite for a subsequent one that will
>  rework the management of stateful device links in the driver core
>  and it will not be compatible with the two drivers in question any
>  more.]
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/iommu/exynos-iommu.c   |    1 +
>  drivers/iommu/rockchip-iommu.c |    3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)

Acked-by: Joerg Roedel <jroedel@suse.de>

> 
> Index: linux-pm/drivers/iommu/rockchip-iommu.c
> ===================================================================
> --- linux-pm.orig/drivers/iommu/rockchip-iommu.c
> +++ linux-pm/drivers/iommu/rockchip-iommu.c
> @@ -1071,7 +1071,8 @@ static int rk_iommu_add_device(struct de
>  	iommu_group_put(group);
>  
>  	iommu_device_link(&iommu->iommu, dev);
> -	data->link = device_link_add(dev, iommu->dev, DL_FLAG_PM_RUNTIME);
> +	data->link = device_link_add(dev, iommu->dev,
> +				     DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
>  
>  	return 0;
>  }
> Index: linux-pm/drivers/iommu/exynos-iommu.c
> ===================================================================
> --- linux-pm.orig/drivers/iommu/exynos-iommu.c
> +++ linux-pm/drivers/iommu/exynos-iommu.c
> @@ -1260,6 +1260,7 @@ static int exynos_iommu_add_device(struc
>  		 * direct calls to pm_runtime_get/put in this driver.
>  		 */
>  		data->link = device_link_add(dev, data->sysmmu,
> +					     DL_FLAG_STATELESS |
>  					     DL_FLAG_PM_RUNTIME);
>  	}
>  	iommu_group_put(group);

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-01-29  9:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-28 23:03 [PATCH 0/4] driver core: Managed device links rework and "consumer autoprobe" flag Rafael J. Wysocki
2019-01-28 23:05 ` [PATCH 1/4] IOMMU: Make dwo drivers use stateless device links Rafael J. Wysocki
2019-01-29  8:55   ` Marek Szyprowski
2019-01-29  9:17   ` Joerg Roedel
2019-01-28 23:06 ` [PATCH 2/4] driver core: Make driver core own stateful " Rafael J. Wysocki
2019-01-28 23:07 ` [PATCH 3/4] driver core: Add device link flag DL_FLAG_AUTOPROBE_CONSUMER Rafael J. Wysocki
2019-01-28 23:08 ` [PATCH 4/4] driver core: Do not call rpm_put_suppliers() in pm_runtime_drop_link() Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).