[PATCH net-next v2 0/9] dpll: do not serialize driver callbacks under global dpll_lock

[PATCH net-next v2 0/9] dpll: do not serialize driver callbacks under global dpll_lock

[Ivan Vecera]

dpll_lock protects topology and object lifetime, but it also serializes
all driver callbacks in the system, so one slow device stalls every other
DPLL device, whichever driver it belongs to. mlx5 callbacks issue firmware
commands with a 60 s default timeout, and zl3073x callbacks can wait
seconds on a driver lock held across ToD writes and phase steps.

Make dpll_lock an rwsem for topology and lifetime and add a mutex per
domain for callbacks. A domain is all objects created with one (module,
clock_id), in practice one chip or card, so channels of one chip stay
serialized as drivers expect, and they still block each other. Device
callbacks run under the domain of the device, pin callbacks under the
domain of the pin, whichever driver registered them. Only one domain lock
is ever held, so domains have no lock order. A lock per device would not
work, because pins are N:M with devices and callbacks notify sibling
channels.

The driver API does not change, but a callback can now notify only
objects of its own domain. Patch 1 changes the one in-tree callback that
does not follow this.

Registrations across domains exist in the tree only on E825, where ice
registers fwnode pins of an external DPLL with its TXC and TSPLL DPLLs.
These callbacks are no longer serialized with TXC and TSPLL, they rely on
pf->dplls.lock, and requests about these pins wait while the external
DPLL is busy. Intel folks, please check that this is fine.

Each complex of a dual complex E825 has its own DSN and so its own
domain, but both use the CGU of complex 0. The lock for the registers
they share is in a fix sent to net [1], which this series needs. Without
it a test lost updates of them.

Patch 4 is the only patch that lets callbacks run concurrently. Patch 5
adds KUnit tests. I also tested each commit with a mock driver and YNL in
QEMU, with lockdep and KASAN, and the tip with lockdep on two systems:

- A board with ZL30732A: every notification the driver sends from a
  callback, concurrent traffic with PTP, driver reload and rebind.
- GNR-D with dual complex E825 and zl3073x: callbacks of the pins shared
  by both drivers, TX clock switches, RCLK pins of both complexes, a port
  going down and up, and both drivers unloaded and loaded in either order
  under load.

mlx5 is build tested only.

On GNR-D, with ToD writes running on the zl3073x and requests waiting for
them, requests to an ice DPLL took 1448 ms in median and 3.9 s at most
before this series, 0.2 ms and 0.3 ms with it, the same as when idle.
Requests about the pins shared by both drivers still wait for the zl3073x.

Changes in v2:
- Dropped "ice: dpll: serialize changes of the SyncE CGU registers", sent
  to net with a fix of the SyncE divider [1]
- Addressed the Sashiko review, see the patches for details
- Added Reviewed-by tags from Petr

v1: https://lore.kernel.org/netdev/20261106121005.1327603-1-ivecera@redhat.com/

[1] https://lore.kernel.org/netdev/20261110094350.184185-1-ivecera@redhat.com/

Ivan Vecera (9):
  ice: dpll: notify TSPLL lock status from the periodic work only
  dpll: add domain object
  dpll: take domain locks around driver callbacks
  dpll: convert dpll_lock to rwsem
  dpll: add KUnit tests for core locking
  dpll: reject measured_freq_get for foreign and on-pin registrations
  dpll: zl3073x: update locking comments
  ice: dpll: update locking comments
  dpll: document locking rules

 Documentation/driver-api/dpll.rst         |   48 +
 drivers/dpll/.kunitconfig                 |    5 +
 drivers/dpll/Kconfig                      |   13 +
 drivers/dpll/Makefile                     |    1 +
 drivers/dpll/dpll_core.c                  |  204 ++-
 drivers/dpll/dpll_core.h                  |   34 +-
 drivers/dpll/dpll_core_kunit.c            | 1580 +++++++++++++++++++++
 drivers/dpll/dpll_netlink.c               |  117 +-
 drivers/dpll/zl3073x/dpll.c               |    9 +-
 drivers/net/ethernet/intel/ice/ice_dpll.c |   24 +-
 drivers/net/ethernet/intel/ice/ice_dpll.h |    5 +
 11 files changed, 1919 insertions(+), 121 deletions(-)
 create mode 100644 drivers/dpll/.kunitconfig
 create mode 100644 drivers/dpll/dpll_core_kunit.c


base-commit: 368fbf5d82da9ebce309449c74fc0b128f7d0219
-- 
2.56.0

[PATCH net-next v2 1/9] ice: dpll: notify TSPLL lock status from the periodic work only

[Ivan Vecera]

ice_dpll_tspll_state_on_dpll_set() notifies the TSPLL device from a
callback of the TIME_REF pin. This pin belongs to an external DPLL and the
DPLL core is going to run its callbacks under a lock of the pin's owner,
which does not cover the TSPLL device.

Only update dpll_state in the callback, so a device get reports the new
state right away, and leave prev_dpll_state to
ice_dpll_periodic_work_e825(), which then sends the notification. It now
arrives up to 500 ms later and after the reply to the request.

Refresh dpll_state on every run of the work, as the state written by the
callback can be transient and the lock status can return to
prev_dpll_state before the next run. No notification is sent in this case.

Reviewed-by: Petr Oros <poros@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
Changes in v2:
- Merged the two comments in ice_dpll_tspll_state_on_dpll_set() (Sashiko)
---
 drivers/net/ethernet/intel/ice/ice_dpll.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index ade1fb3dd06743..0877827a6521cc 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -2914,7 +2914,6 @@ ice_dpll_tspll_state_on_dpll_set(const struct dpll_pin *pin, void *pin_priv,
 	struct ice_dpll_pin *p = pin_priv;
 	struct ice_pf *pf = p->pf;
 	enum ice_clk_src clk_src;
-	bool changed = false;
 	struct ice_dpll *tp;
 	int ret;
 
@@ -2939,23 +2938,16 @@ ice_dpll_tspll_state_on_dpll_set(const struct dpll_pin *pin, void *pin_priv,
 
 		new_state = ice_dpll_tspll_lock_status_get(pf, false);
 
-		/* Sync the cached lock state to what we just observed from HW
-		 * so ice_dpll_periodic_work_e825() (cached path) does not emit
-		 * a spurious UNLOCKED notification between now and the next
-		 * PTP monitor tick.
+		/* Sync the cached lock state to what we just observed from HW.
+		 * ice_dpll_periodic_work_e825() reads it (cached path),
+		 * compares it with prev_dpll_state and sends the notification.
+		 * dpll_state lets a device get report it right away.
 		 */
 		WRITE_ONCE(pf->ptp.tspll_locked,
 			   new_state == DPLL_LOCK_STATUS_LOCKED);
-
-		if (tp->prev_dpll_state != new_state) {
-			tp->dpll_state = new_state;
-			tp->prev_dpll_state = new_state;
-			changed = true;
-		}
+		tp->dpll_state = new_state;
 	}
 	mutex_unlock(&pf->dplls.lock);
-	if (changed)
-		__dpll_device_change_ntf(tp->dpll);
 	/* TSPLL reconfiguration may complete before lock is reacquired.
 	 * PTP periodic monitoring tracks and restores the final lock state.
 	 */
@@ -3372,8 +3364,8 @@ static void ice_dpll_periodic_work_e825(struct ice_pf *pf)
 	mutex_lock(&pf->dplls.lock);
 	new_state = ice_dpll_tspll_lock_status_get(pf, true);
 
+	tp->dpll_state = new_state;
 	if (tp->prev_dpll_state != new_state) {
-		tp->dpll_state = new_state;
 		tp->prev_dpll_state = new_state;
 		changed = true;
 	}
-- 
2.56.0

[PATCH net-next v2 2/9] dpll: add domain object

[Ivan Vecera]

All dpll device and pin callbacks are serialized by the global dpll_lock,
so a slow device stalls all others, whichever driver they belong to.

Add struct dpll_domain as a preparation for serializing callbacks per
driver instance. A domain groups all devices and pins created with the
same (module, clock_id) pair, which is in practice one chip or card. The
same pair already identifies the owner of a pin in
dpll_pin_own_dpll_ref_first(), which now compares the domains instead.

Domains are kept in an xarray, like devices and pins, and they live only as
long as their objects.

No functional change.

Reviewed-by: Petr Oros <poros@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
Changes in v2:
- Moved the mutex to the next patch, which first takes it (Sashiko)
- dpll_pin_own_dpll_ref_first() compares the domains
---
 drivers/dpll/dpll_core.c | 71 ++++++++++++++++++++++++++++++++++++----
 drivers/dpll/dpll_core.h | 21 ++++++++++++
 2 files changed, 85 insertions(+), 7 deletions(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index 8389983f425695..c1b54e6ba430bf 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -24,6 +24,7 @@ DEFINE_MUTEX(dpll_lock);
 
 DEFINE_XARRAY_FLAGS(dpll_device_xa, XA_FLAGS_ALLOC);
 DEFINE_XARRAY_FLAGS(dpll_pin_xa, XA_FLAGS_ALLOC);
+static DEFINE_XARRAY_FLAGS(dpll_domain_xa, XA_FLAGS_ALLOC);
 
 static RAW_NOTIFIER_HEAD(dpll_notifier_chain);
 static DEFINE_IDA(dpll_pin_idx_ida);
@@ -88,6 +89,47 @@ void dpll_pin_notify(struct dpll_pin *pin, u64 src_clock_id,
 	call_dpll_notifiers(action, &info);
 }
 
+static struct dpll_domain *dpll_domain_get(u64 clock_id, struct module *module)
+{
+	struct dpll_domain *domain;
+	unsigned long i;
+	int ret;
+
+	lockdep_assert_held(&dpll_lock);
+
+	xa_for_each(&dpll_domain_xa, i, domain) {
+		if (domain->clock_id == clock_id && domain->module == module) {
+			refcount_inc(&domain->refcount);
+			return domain;
+		}
+	}
+
+	domain = kzalloc_obj(*domain);
+	if (!domain)
+		return ERR_PTR(-ENOMEM);
+	domain->clock_id = clock_id;
+	domain->module = module;
+	refcount_set(&domain->refcount, 1);
+	ret = xa_alloc(&dpll_domain_xa, &domain->id, domain, xa_limit_32b,
+		       GFP_KERNEL);
+	if (ret) {
+		kfree(domain);
+		return ERR_PTR(ret);
+	}
+
+	return domain;
+}
+
+static void dpll_domain_put(struct dpll_domain *domain)
+{
+	lockdep_assert_held(&dpll_lock);
+
+	if (refcount_dec_and_test(&domain->refcount)) {
+		xa_erase(&dpll_domain_xa, domain->id);
+		kfree(domain);
+	}
+}
+
 static void dpll_device_tracker_alloc(struct dpll_device *dpll,
 				      dpll_tracker *tracker)
 {
@@ -120,6 +162,7 @@ static void __dpll_device_put(struct dpll_device *dpll, dpll_tracker *tracker)
 		xa_erase(&dpll_device_xa, dpll->id);
 		WARN_ON(!list_empty(&dpll->registration_list));
 		ref_tracker_dir_exit(&dpll->refcnt_tracker);
+		dpll_domain_put(dpll->domain);
 		kfree(dpll);
 	}
 }
@@ -159,6 +202,7 @@ static void __dpll_pin_put(struct dpll_pin *pin, dpll_tracker *tracker)
 		fwnode_handle_put(pin->fwnode);
 		dpll_pin_idx_free(pin->pin_idx);
 		ref_tracker_dir_exit(&pin->refcnt_tracker);
+		dpll_domain_put(pin->domain);
 		kfree_rcu(pin, rcu);
 	}
 }
@@ -366,6 +410,12 @@ dpll_device_alloc(const u64 clock_id, u32 device_idx, struct module *module)
 	dpll = kzalloc_obj(*dpll);
 	if (!dpll)
 		return ERR_PTR(-ENOMEM);
+	dpll->domain = dpll_domain_get(clock_id, module);
+	if (IS_ERR(dpll->domain)) {
+		ret = PTR_ERR(dpll->domain);
+		kfree(dpll);
+		return ERR_PTR(ret);
+	}
 	refcount_set(&dpll->refcount, 1);
 	INIT_LIST_HEAD(&dpll->registration_list);
 	dpll->device_idx = device_idx;
@@ -374,6 +424,7 @@ dpll_device_alloc(const u64 clock_id, u32 device_idx, struct module *module)
 	ret = xa_alloc_cyclic(&dpll_device_xa, &dpll->id, dpll, xa_limit_32b,
 			      &dpll_device_xa_id, GFP_KERNEL);
 	if (ret < 0) {
+		dpll_domain_put(dpll->domain);
 		kfree(dpll);
 		return ERR_PTR(ret);
 	}
@@ -664,6 +715,11 @@ dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct module *module,
 	ret = dpll_pin_prop_dup(prop, &pin->prop);
 	if (ret)
 		goto err_pin_prop;
+	pin->domain = dpll_domain_get(clock_id, module);
+	if (IS_ERR(pin->domain)) {
+		ret = PTR_ERR(pin->domain);
+		goto err_domain;
+	}
 	refcount_set(&pin->refcount, 1);
 	xa_init_flags(&pin->dpll_refs, XA_FLAGS_ALLOC);
 	xa_init_flags(&pin->parent_refs, XA_FLAGS_ALLOC);
@@ -678,6 +734,8 @@ dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct module *module,
 	xa_destroy(&pin->dpll_refs);
 	xa_destroy(&pin->parent_refs);
 	xa_destroy(&pin->ref_sync_pins);
+	dpll_domain_put(pin->domain);
+err_domain:
 	dpll_pin_prop_free(&pin->prop);
 err_pin_prop:
 	kfree(pin);
@@ -1156,11 +1214,11 @@ void *dpll_pin_on_pin_priv(struct dpll_pin *parent,
  * dpll_pin_own_dpll_ref_first - find the first owner dpll ref of a pin
  * @pin: pointer to a dpll pin
  *
- * Search pin's dpll_refs for a ref whose dpll matches the pin's
- * (module, clock_id) tuple, i.e. the dpll registered by the driver
- * that created the pin. This ensures pin-level attributes are
- * reported and modified using the owner's ops even when the pin is
- * also registered with dplls from other drivers.
+ * Search pin's dpll_refs for a ref whose dpll belongs to the domain of
+ * the pin, i.e. the dpll registered by the driver that created the pin.
+ * This ensures pin-level attributes are reported and modified using the
+ * owner's ops even when the pin is also registered with dplls from other
+ * drivers.
  *
  * Return: pointer to the owner's dpll_pin_ref, or NULL if no
  * owner ref is found.
@@ -1171,8 +1229,7 @@ struct dpll_pin_ref *dpll_pin_own_dpll_ref_first(struct dpll_pin *pin)
 	unsigned long i;
 
 	xa_for_each(&pin->dpll_refs, i, ref) {
-		if (ref->dpll->module == pin->module &&
-		    ref->dpll->clock_id == pin->clock_id)
+		if (ref->dpll->domain == pin->domain)
 			return ref;
 	}
 
diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
index da8a369556ed11..6edf7edf7a9539 100644
--- a/drivers/dpll/dpll_core.h
+++ b/drivers/dpll/dpll_core.h
@@ -15,12 +15,30 @@
 
 #define DPLL_REGISTERED		XA_MARK_1
 
+/**
+ * struct dpll_domain - group of dpll objects
+ * @id:			index of the domain in dpll_domain_xa
+ * @clock_id:		clock_id of the objects in the domain
+ * @module:		module of the objects in the domain
+ * @refcount:		number of dpll devices and pins in the domain
+ *
+ * All dpll devices and pins created with the same (module, clock_id) pair
+ * belong to one domain, in practice to one chip or card.
+ **/
+struct dpll_domain {
+	u32 id;
+	u64 clock_id;
+	struct module *module;
+	refcount_t refcount;
+};
+
 /**
  * struct dpll_device - stores DPLL device internal data
  * @id:			unique id number for device given by dpll subsystem
  * @device_idx:		id given by dev driver
  * @clock_id:		unique identifier (clock_id) of a dpll
  * @module:		module of creator
+ * @domain:		domain the device belongs to
  * @type:		type of a dpll
  * @pin_refs:		stores pins registered within a dpll
  * @refcount:		refcount
@@ -32,6 +50,7 @@ struct dpll_device {
 	u32 device_idx;
 	u64 clock_id;
 	struct module *module;
+	struct dpll_domain *domain;
 	enum dpll_type type;
 	struct xarray pin_refs;
 	refcount_t refcount;
@@ -46,6 +65,7 @@ struct dpll_device {
  * @clock_id:		clock_id of creator
  * @module:		module of creator
  * @module_name:	module name of creator
+ * @domain:		domain the pin belongs to
  * @fwnode:		optional reference to firmware node
  * @dpll_refs:		hold referencees to dplls pin was registered with
  * @parent_refs:	hold references to parent pins pin was registered with
@@ -61,6 +81,7 @@ struct dpll_pin {
 	u64 clock_id;
 	struct module *module;
 	char module_name[MODULE_NAME_LEN];
+	struct dpll_domain *domain;
 	struct fwnode_handle *fwnode;
 	struct xarray dpll_refs;
 	struct xarray parent_refs;
-- 
2.56.0

[PATCH net-next v2 3/9] dpll: take domain locks around driver callbacks

[Ivan Vecera]

Add a mutex to struct dpll_domain. Hold the domain lock of the addressed
object wherever driver callbacks can run:

- netlink requests addressing a device or a pin, from pre_doit to post_doit
- dumps, for each object
- notifications, while the message is being built and sent

Callbacks of a device run under the domain of the device. Callbacks of a
pin run under the domain of the pin, also when another driver registered
the pin with its dpll or under its parent pin. All callbacks a message
about a pin needs are thus covered by one lock, and only one domain lock
is ever held. Domains have no lock order, so registrations can connect
them in any way.

A request is atomic against other requests addressing objects of that
domain, and messages of one object are sent in the order they were built.

Requests resolving an id run no callbacks, so they do not take the lock.

__dpll_{device,pin}_change_ntf() now also require the domain lock of the
object. A callback can therefore only notify objects of the domain it runs
under.

Callbacks stay serialized by dpll_lock for now.

Reviewed-by: Petr Oros <poros@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
Changes in v2:
- Added the domain mutex here, moved from the previous patch (Sashiko)
---
 drivers/dpll/dpll_core.c    | 21 ++++++----
 drivers/dpll/dpll_core.h    | 12 +++++-
 drivers/dpll/dpll_netlink.c | 78 ++++++++++++++++++++++++++++---------
 3 files changed, 85 insertions(+), 26 deletions(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index c1b54e6ba430bf..b4b2b2dc449aec 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -110,9 +110,11 @@ static struct dpll_domain *dpll_domain_get(u64 clock_id, struct module *module)
 	domain->clock_id = clock_id;
 	domain->module = module;
 	refcount_set(&domain->refcount, 1);
+	mutex_init(&domain->lock);
 	ret = xa_alloc(&dpll_domain_xa, &domain->id, domain, xa_limit_32b,
 		       GFP_KERNEL);
 	if (ret) {
+		mutex_destroy(&domain->lock);
 		kfree(domain);
 		return ERR_PTR(ret);
 	}
@@ -126,6 +128,7 @@ static void dpll_domain_put(struct dpll_domain *domain)
 
 	if (refcount_dec_and_test(&domain->refcount)) {
 		xa_erase(&dpll_domain_xa, domain->id);
+		mutex_destroy(&domain->lock);
 		kfree(domain);
 	}
 }
@@ -517,7 +520,7 @@ dpll_device_registration_find(struct dpll_device *dpll,
  *
  * Make dpll device available for user space.
  *
- * Context: Acquires a lock (dpll_lock)
+ * Context: Acquires dpll_lock and the domain lock of @dpll
  * Return:
  * * 0 on success
  * * negative - error value
@@ -576,7 +579,7 @@ EXPORT_SYMBOL_GPL(dpll_device_register);
  *
  * Unregister device, make it unavailable for userspace.
  * Note: It does not free the memory
- * Context: Acquires a lock (dpll_lock)
+ * Context: Acquires dpll_lock and the domain lock of @dpll
  */
 void dpll_device_unregister(struct dpll_device *dpll,
 			    const struct dpll_device_ops *ops, void *priv)
@@ -925,7 +928,7 @@ __dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
  * @ops: ops for a dpll pin ops
  * @priv: pointer to private information of owner
  *
- * Context: Acquires a lock (dpll_lock)
+ * Context: Acquires dpll_lock and domain locks
  * Return:
  * * 0 on success
  * * negative - error value
@@ -986,7 +989,9 @@ static void dpll_pin_ref_sync_pair_del(u32 ref_sync_pin_id)
 		ref_sync_pin = xa_load(&pin->ref_sync_pins, ref_sync_pin_id);
 		if (ref_sync_pin) {
 			xa_erase(&pin->ref_sync_pins, ref_sync_pin_id);
+			mutex_lock(&pin->domain->lock);
 			__dpll_pin_change_ntf(pin);
+			mutex_unlock(&pin->domain->lock);
 		}
 	}
 }
@@ -1013,7 +1018,7 @@ __dpll_pin_unregister(struct dpll_device *dpll, struct dpll_pin *pin,
  * @priv: pointer to private information of owner
  *
  * Note: It does not free the memory
- * Context: Acquires a lock (dpll_lock)
+ * Context: Acquires dpll_lock and domain locks
  */
 void dpll_pin_unregister(struct dpll_device *dpll, struct dpll_pin *pin,
 			 const struct dpll_pin_ops *ops, void *priv)
@@ -1039,7 +1044,7 @@ EXPORT_SYMBOL_GPL(dpll_pin_unregister);
  * Register a pin with a parent pin, create references between them and
  * between newly registered pin and dplls connected with a parent pin.
  *
- * Context: Acquires a lock (dpll_lock)
+ * Context: Acquires dpll_lock and domain locks
  * Return:
  * * 0 on success
  * * negative - error value
@@ -1096,7 +1101,7 @@ EXPORT_SYMBOL_GPL(dpll_pin_on_pin_register);
  * @ops: ops for a dpll pin
  * @priv: pointer to private information of owner
  *
- * Context: Acquires a lock (dpll_lock)
+ * Context: Acquires dpll_lock and domain locks
  * Note: It does not free the memory
  */
 void dpll_pin_on_pin_unregister(struct dpll_pin *parent, struct dpll_pin *pin,
@@ -1126,7 +1131,7 @@ EXPORT_SYMBOL_GPL(dpll_pin_on_pin_unregister);
  *
  * Once pins are paired, the user-space configuration of reference sync pair
  * is possible.
- * Context: Acquires a lock (dpll_lock)
+ * Context: Acquires dpll_lock and domain locks
  * Return:
  * * 0 on success
  * * negative - error value
@@ -1139,7 +1144,9 @@ int dpll_pin_ref_sync_pair_add(struct dpll_pin *pin,
 	mutex_lock(&dpll_lock);
 	ret = xa_insert(&pin->ref_sync_pins, ref_sync_pin->id,
 			ref_sync_pin, GFP_KERNEL);
+	mutex_lock(&pin->domain->lock);
 	__dpll_pin_change_ntf(pin);
+	mutex_unlock(&pin->domain->lock);
 	mutex_unlock(&dpll_lock);
 
 	return ret;
diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
index 6edf7edf7a9539..630c5a1ca572c2 100644
--- a/drivers/dpll/dpll_core.h
+++ b/drivers/dpll/dpll_core.h
@@ -9,6 +9,7 @@
 
 #include <linux/dpll.h>
 #include <linux/list.h>
+#include <linux/mutex.h>
 #include <linux/refcount.h>
 #include <linux/ref_tracker.h>
 #include "dpll_nl.h"
@@ -16,20 +17,29 @@
 #define DPLL_REGISTERED		XA_MARK_1
 
 /**
- * struct dpll_domain - group of dpll objects
+ * struct dpll_domain - group of dpll objects sharing a lock
  * @id:			index of the domain in dpll_domain_xa
  * @clock_id:		clock_id of the objects in the domain
  * @module:		module of the objects in the domain
  * @refcount:		number of dpll devices and pins in the domain
+ * @lock:		serializes driver callbacks of the domain
  *
  * All dpll devices and pins created with the same (module, clock_id) pair
  * belong to one domain, in practice to one chip or card.
+ *
+ * Callbacks of a device run under the domain of the device and callbacks of a
+ * pin under the domain of the pin, whichever driver registered them. Only one
+ * domain lock is held at a time, so domains have no lock order.
  **/
 struct dpll_domain {
 	u32 id;
 	u64 clock_id;
 	struct module *module;
 	refcount_t refcount;
+	/* Serializes the driver callbacks of all DPLL devices and pins
+	 * that share one (module, clock_id) pair.
+	 */
+	struct mutex lock;
 };
 
 /**
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index e5380d95f238b3..1b4e3a0166a744 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -861,27 +861,39 @@ dpll_device_event_send(enum dpll_cmd event, struct dpll_device *dpll)
 
 int dpll_device_create_ntf(struct dpll_device *dpll)
 {
+	int ret;
+
+	mutex_lock(&dpll->domain->lock);
 	dpll_device_notify(dpll, DPLL_DEVICE_CREATED);
-	return dpll_device_event_send(DPLL_CMD_DEVICE_CREATE_NTF, dpll);
+	ret = dpll_device_event_send(DPLL_CMD_DEVICE_CREATE_NTF, dpll);
+	mutex_unlock(&dpll->domain->lock);
+
+	return ret;
 }
 
 int dpll_device_delete_ntf(struct dpll_device *dpll)
 {
+	int ret;
+
+	mutex_lock(&dpll->domain->lock);
 	dpll_device_notify(dpll, DPLL_DEVICE_DELETED);
-	return dpll_device_event_send(DPLL_CMD_DEVICE_DELETE_NTF, dpll);
+	ret = dpll_device_event_send(DPLL_CMD_DEVICE_DELETE_NTF, dpll);
+	mutex_unlock(&dpll->domain->lock);
+
+	return ret;
 }
 
 /**
  * __dpll_device_change_ntf - notify that the dpll device has been changed
  * @dpll: registered dpll pointer
  *
- * Context: caller must hold dpll_lock. Suitable for use inside device
- *          callbacks which are already invoked under dpll_lock.
+ * Context: caller must hold dpll_lock and the domain lock of @dpll.
+ *          Suitable for use inside callbacks running under these locks.
  * Return: 0 if succeeds, error code otherwise.
  */
 int __dpll_device_change_ntf(struct dpll_device *dpll)
 {
-	lockdep_assert_held(&dpll_lock);
+	lockdep_assert_held(&dpll->domain->lock);
 	dpll_device_notify(dpll, DPLL_DEVICE_CHANGED);
 	return dpll_device_event_send(DPLL_CMD_DEVICE_CHANGE_NTF, dpll);
 }
@@ -891,7 +903,7 @@ EXPORT_SYMBOL_GPL(__dpll_device_change_ntf);
  * dpll_device_change_ntf - notify that the dpll device has been changed
  * @dpll: registered dpll pointer
  *
- * Context: acquires and holds a dpll_lock.
+ * Context: acquires and holds dpll_lock and the domain lock of @dpll.
  * Return: 0 if succeeds, error code otherwise.
  */
 int dpll_device_change_ntf(struct dpll_device *dpll)
@@ -899,7 +911,9 @@ int dpll_device_change_ntf(struct dpll_device *dpll)
 	int ret;
 
 	mutex_lock(&dpll_lock);
+	mutex_lock(&dpll->domain->lock);
 	ret = __dpll_device_change_ntf(dpll);
+	mutex_unlock(&dpll->domain->lock);
 	mutex_unlock(&dpll_lock);
 
 	return ret;
@@ -941,27 +955,39 @@ dpll_pin_event_send(enum dpll_cmd event, struct dpll_pin *pin)
 
 int dpll_pin_create_ntf(struct dpll_pin *pin, u64 src_clock_id)
 {
+	int ret;
+
+	mutex_lock(&pin->domain->lock);
 	dpll_pin_notify(pin, src_clock_id, DPLL_PIN_CREATED);
-	return dpll_pin_event_send(DPLL_CMD_PIN_CREATE_NTF, pin);
+	ret = dpll_pin_event_send(DPLL_CMD_PIN_CREATE_NTF, pin);
+	mutex_unlock(&pin->domain->lock);
+
+	return ret;
 }
 
 int dpll_pin_delete_ntf(struct dpll_pin *pin, u64 src_clock_id)
 {
+	int ret;
+
+	mutex_lock(&pin->domain->lock);
 	dpll_pin_notify(pin, src_clock_id, DPLL_PIN_DELETED);
-	return dpll_pin_event_send(DPLL_CMD_PIN_DELETE_NTF, pin);
+	ret = dpll_pin_event_send(DPLL_CMD_PIN_DELETE_NTF, pin);
+	mutex_unlock(&pin->domain->lock);
+
+	return ret;
 }
 
 /**
  * __dpll_pin_change_ntf - notify that the pin has been changed
  * @pin: registered pin pointer
  *
- * Context: caller must hold dpll_lock. Suitable for use inside pin
- *          callbacks which are already invoked under dpll_lock.
+ * Context: caller must hold dpll_lock and the domain lock of @pin.
+ *          Suitable for use inside callbacks running under these locks.
  * Return: 0 if succeeds, error code otherwise.
  */
 int __dpll_pin_change_ntf(struct dpll_pin *pin)
 {
-	lockdep_assert_held(&dpll_lock);
+	lockdep_assert_held(&pin->domain->lock);
 	dpll_pin_notify(pin, pin->clock_id, DPLL_PIN_CHANGED);
 	return dpll_pin_event_send(DPLL_CMD_PIN_CHANGE_NTF, pin);
 }
@@ -971,7 +997,7 @@ EXPORT_SYMBOL_GPL(__dpll_pin_change_ntf);
  * dpll_pin_change_ntf - notify that the pin has been changed
  * @pin: registered pin pointer
  *
- * Context: acquires and holds a dpll_lock.
+ * Context: acquires and holds dpll_lock and the domain lock of @pin.
  * Return: 0 if succeeds, error code otherwise.
  */
 int dpll_pin_change_ntf(struct dpll_pin *pin)
@@ -979,7 +1005,9 @@ int dpll_pin_change_ntf(struct dpll_pin *pin)
 	int ret;
 
 	mutex_lock(&dpll_lock);
+	mutex_lock(&pin->domain->lock);
 	ret = __dpll_pin_change_ntf(pin);
+	mutex_unlock(&pin->domain->lock);
 	mutex_unlock(&dpll_lock);
 
 	return ret;
@@ -1780,7 +1808,9 @@ int dpll_nl_pin_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 			ret = -EMSGSIZE;
 			break;
 		}
+		mutex_lock(&pin->domain->lock);
 		ret = dpll_cmd_pin_get_one(skb, pin, cb->extack);
+		mutex_unlock(&pin->domain->lock);
 		if (ret) {
 			genlmsg_cancel(skb, hdr);
 			if (ret == -ENODEV) {
@@ -1999,7 +2029,9 @@ int dpll_nl_device_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 			ret = -EMSGSIZE;
 			break;
 		}
+		mutex_lock(&dpll->domain->lock);
 		ret = dpll_device_get_one(dpll, skb, cb->extack);
+		mutex_unlock(&dpll->domain->lock);
 		if (ret) {
 			genlmsg_cancel(skb, hdr);
 			break;
@@ -2018,6 +2050,7 @@ int dpll_nl_device_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 int dpll_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 		  struct genl_info *info)
 {
+	struct dpll_device *dpll;
 	u32 id;
 
 	if (GENL_REQ_ATTR_CHECK(info, DPLL_A_ID))
@@ -2025,11 +2058,13 @@ int dpll_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 
 	mutex_lock(&dpll_lock);
 	id = nla_get_u32(info->attrs[DPLL_A_ID]);
-	info->user_ptr[0] = dpll_device_get_by_id(id);
-	if (!info->user_ptr[0]) {
+	dpll = dpll_device_get_by_id(id);
+	if (!dpll) {
 		NL_SET_ERR_MSG(info->extack, "device not found");
 		goto unlock;
 	}
+	info->user_ptr[0] = dpll;
+	mutex_lock(&dpll->domain->lock);
 	return 0;
 unlock:
 	mutex_unlock(&dpll_lock);
@@ -2039,6 +2074,9 @@ int dpll_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 void dpll_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 		    struct genl_info *info)
 {
+	struct dpll_device *dpll = info->user_ptr[0];
+
+	mutex_unlock(&dpll->domain->lock);
 	mutex_unlock(&dpll_lock);
 }
 
@@ -2061,6 +2099,7 @@ dpll_unlock_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 int dpll_pin_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 		      struct genl_info *info)
 {
+	struct dpll_pin *pin;
 	int ret;
 
 	mutex_lock(&dpll_lock);
@@ -2068,14 +2107,14 @@ int dpll_pin_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 		ret = -EINVAL;
 		goto unlock_dev;
 	}
-	info->user_ptr[0] = xa_load(&dpll_pin_xa,
-				    nla_get_u32(info->attrs[DPLL_A_PIN_ID]));
-	if (!info->user_ptr[0] ||
-	    !dpll_pin_available(info->user_ptr[0])) {
+	pin = xa_load(&dpll_pin_xa, nla_get_u32(info->attrs[DPLL_A_PIN_ID]));
+	if (!pin || !dpll_pin_available(pin)) {
 		NL_SET_ERR_MSG(info->extack, "pin not found");
 		ret = -ENODEV;
 		goto unlock_dev;
 	}
+	info->user_ptr[0] = pin;
+	mutex_lock(&pin->domain->lock);
 
 	return 0;
 
@@ -2087,5 +2126,8 @@ int dpll_pin_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 void dpll_pin_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 			struct genl_info *info)
 {
+	struct dpll_pin *pin = info->user_ptr[0];
+
+	mutex_unlock(&pin->domain->lock);
 	mutex_unlock(&dpll_lock);
 }
-- 
2.56.0

[PATCH net-next v2 4/9] dpll: convert dpll_lock to rwsem

[Ivan Vecera]

Driver callbacks are now serialized by domain locks, so dpll_lock only has
to protect the topology and lifetime of devices and pins.

Convert it to an rwsem. Core functions changing the topology take it for
write. Netlink requests, each part of a dump and change notifications take
it for read, for their whole duration, so objects found by a lookup stay
valid. This is the patch that lets callbacks of different domains run
concurrently.

A callback running for long still blocks topology changes, and a waiting
writer blocks new readers, so a stuck callback stalls the subsystem while
a device is being probed or removed. This is no worse than the global
serialization it replaces.

Reviewed-by: Petr Oros <poros@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
Changes in v2:
- lockdep_assert_held_write() in dpll_domain_get() and
  dpll_domain_put() (Sashiko)
---
 drivers/dpll/dpll_core.c    | 97 +++++++++++++++++--------------------
 drivers/dpll/dpll_core.h    |  3 +-
 drivers/dpll/dpll_netlink.c | 32 ++++++------
 3 files changed, 63 insertions(+), 69 deletions(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index b4b2b2dc449aec..2b1dfe2929647e 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -19,8 +19,12 @@
 #include "dpll_core.h"
 #include "dpll_netlink.h"
 
-/* Mutex lock to protect DPLL subsystem devices and pins */
-DEFINE_MUTEX(dpll_lock);
+/* Protects the topology and lifetime of DPLL devices and pins. Taken for write
+ * to change them and for read for the whole netlink request, part of a dump or
+ * change notification that uses them. Driver callbacks are serialized by domain
+ * locks, which are only taken under this lock.
+ */
+DECLARE_RWSEM(dpll_lock);
 
 DEFINE_XARRAY_FLAGS(dpll_device_xa, XA_FLAGS_ALLOC);
 DEFINE_XARRAY_FLAGS(dpll_pin_xa, XA_FLAGS_ALLOC);
@@ -95,7 +99,7 @@ static struct dpll_domain *dpll_domain_get(u64 clock_id, struct module *module)
 	unsigned long i;
 	int ret;
 
-	lockdep_assert_held(&dpll_lock);
+	lockdep_assert_held_write(&dpll_lock);
 
 	xa_for_each(&dpll_domain_xa, i, domain) {
 		if (domain->clock_id == clock_id && domain->module == module) {
@@ -124,7 +128,7 @@ static struct dpll_domain *dpll_domain_get(u64 clock_id, struct module *module)
 
 static void dpll_domain_put(struct dpll_domain *domain)
 {
-	lockdep_assert_held(&dpll_lock);
+	lockdep_assert_held_write(&dpll_lock);
 
 	if (refcount_dec_and_test(&domain->refcount)) {
 		xa_erase(&dpll_domain_xa, domain->id);
@@ -459,7 +463,7 @@ dpll_device_get(u64 clock_id, u32 device_idx, struct module *module,
 	struct dpll_device *dpll, *ret = NULL;
 	unsigned long index;
 
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	xa_for_each(&dpll_device_xa, index, dpll) {
 		if (dpll->clock_id == clock_id &&
 		    dpll->device_idx == device_idx &&
@@ -475,7 +479,7 @@ dpll_device_get(u64 clock_id, u32 device_idx, struct module *module,
 			dpll_device_tracker_alloc(ret, tracker);
 	}
 
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 
 	return ret;
 }
@@ -492,9 +496,9 @@ EXPORT_SYMBOL_GPL(dpll_device_get);
  */
 void dpll_device_put(struct dpll_device *dpll, dpll_tracker *tracker)
 {
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	__dpll_device_put(dpll, tracker);
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 }
 EXPORT_SYMBOL_GPL(dpll_device_put);
 
@@ -540,16 +544,16 @@ int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
 	if (WARN_ON(type < DPLL_TYPE_PPS || type > DPLL_TYPE_MAX))
 		return -EINVAL;
 
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	reg = dpll_device_registration_find(dpll, ops, priv);
 	if (reg) {
-		mutex_unlock(&dpll_lock);
+		up_write(&dpll_lock);
 		return -EEXIST;
 	}
 
 	reg = kzalloc_obj(*reg);
 	if (!reg) {
-		mutex_unlock(&dpll_lock);
+		up_write(&dpll_lock);
 		return -ENOMEM;
 	}
 	reg->ops = ops;
@@ -559,13 +563,13 @@ int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
 	first_registration = list_empty(&dpll->registration_list);
 	list_add_tail(&reg->list, &dpll->registration_list);
 	if (!first_registration) {
-		mutex_unlock(&dpll_lock);
+		up_write(&dpll_lock);
 		return 0;
 	}
 
 	xa_set_mark(&dpll_device_xa, dpll->id, DPLL_REGISTERED);
 	dpll_device_create_ntf(dpll);
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 
 	return 0;
 }
@@ -586,12 +590,12 @@ void dpll_device_unregister(struct dpll_device *dpll,
 {
 	struct dpll_device_registration *reg;
 
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	ASSERT_DPLL_REGISTERED(dpll);
 	dpll_device_delete_ntf(dpll);
 	reg = dpll_device_registration_find(dpll, ops, priv);
 	if (WARN_ON(!reg)) {
-		mutex_unlock(&dpll_lock);
+		up_write(&dpll_lock);
 		return;
 	}
 	list_del(&reg->list);
@@ -599,11 +603,11 @@ void dpll_device_unregister(struct dpll_device *dpll,
 	kfree(reg);
 
 	if (!list_empty(&dpll->registration_list)) {
-		mutex_unlock(&dpll_lock);
+		up_write(&dpll_lock);
 		return;
 	}
 	xa_clear_mark(&dpll_device_xa, dpll->id, DPLL_REGISTERED);
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 }
 EXPORT_SYMBOL_GPL(dpll_device_unregister);
 
@@ -771,9 +775,9 @@ int register_dpll_notifier(struct notifier_block *nb)
 {
 	int ret;
 
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	ret = raw_notifier_chain_register(&dpll_notifier_chain, nb);
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(register_dpll_notifier);
@@ -782,9 +786,9 @@ int unregister_dpll_notifier(struct notifier_block *nb)
 {
 	int ret;
 
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	ret = raw_notifier_chain_unregister(&dpll_notifier_chain, nb);
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(unregister_dpll_notifier);
@@ -812,7 +816,7 @@ dpll_pin_get(u64 clock_id, u32 pin_idx, struct module *module,
 	struct dpll_pin *pos, *ret = NULL;
 	unsigned long i;
 
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	xa_for_each(&dpll_pin_xa, i, pos) {
 		if (pos->clock_id == clock_id &&
 		    pos->pin_idx == pin_idx &&
@@ -827,7 +831,7 @@ dpll_pin_get(u64 clock_id, u32 pin_idx, struct module *module,
 		if (!IS_ERR(ret))
 			dpll_pin_tracker_alloc(ret, tracker);
 	}
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 
 	return ret;
 }
@@ -844,9 +848,9 @@ EXPORT_SYMBOL_GPL(dpll_pin_get);
  */
 void dpll_pin_put(struct dpll_pin *pin, dpll_tracker *tracker)
 {
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	__dpll_pin_put(pin, tracker);
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 }
 EXPORT_SYMBOL_GPL(dpll_pin_put);
 
@@ -859,10 +863,10 @@ EXPORT_SYMBOL_GPL(dpll_pin_put);
  */
 void dpll_pin_fwnode_set(struct dpll_pin *pin, struct fwnode_handle *fwnode)
 {
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	fwnode_handle_put(pin->fwnode); /* Drop fwnode previously set */
 	pin->fwnode = fwnode_handle_get(fwnode);
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 }
 EXPORT_SYMBOL_GPL(dpll_pin_fwnode_set);
 
@@ -885,7 +889,7 @@ struct dpll_pin *fwnode_dpll_pin_find(struct fwnode_handle *fwnode,
 	struct dpll_pin *pin, *ret = NULL;
 	unsigned long index;
 
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	xa_for_each(&dpll_pin_xa, index, pin) {
 		if (pin->fwnode == fwnode) {
 			__dpll_pin_hold(pin, tracker);
@@ -893,7 +897,7 @@ struct dpll_pin *fwnode_dpll_pin_find(struct fwnode_handle *fwnode,
 			break;
 		}
 	}
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 
 	return ret;
 }
@@ -950,7 +954,7 @@ dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
 		      DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE)))
 		return -EINVAL;
 
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 
 	dev_ops = dpll_device_ops(dpll);
 	if (WARN_ON(ops->measured_freq_get &&
@@ -974,7 +978,7 @@ dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
 
 	ret = __dpll_pin_register(dpll, pin, ops, priv, NULL);
 out_unlock:
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 
 	return ret;
 }
@@ -1028,9 +1032,9 @@ void dpll_pin_unregister(struct dpll_device *dpll, struct dpll_pin *pin,
 	if (WARN_ON(!xa_empty(&pin->parent_refs)))
 		return;
 
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	__dpll_pin_unregister(dpll, pin, ops, priv, NULL);
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 }
 EXPORT_SYMBOL_GPL(dpll_pin_unregister);
 
@@ -1064,7 +1068,7 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, struct dpll_pin *pin,
 	    WARN_ON(!ops->direction_get))
 		return -EINVAL;
 
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	ret = dpll_xa_ref_pin_add(&pin->parent_refs, parent, ops, priv, pin);
 	if (ret)
 		goto unlock;
@@ -1076,7 +1080,7 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, struct dpll_pin *pin,
 		}
 		dpll_pin_create_ntf(pin, parent->clock_id);
 	}
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 
 	return ret;
 
@@ -1089,7 +1093,7 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, struct dpll_pin *pin,
 		}
 	dpll_xa_ref_pin_del(&pin->parent_refs, parent, ops, priv, pin);
 unlock:
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(dpll_pin_on_pin_register);
@@ -1111,7 +1115,7 @@ void dpll_pin_on_pin_unregister(struct dpll_pin *parent, struct dpll_pin *pin,
 	struct dpll_pin_ref *ref;
 	unsigned long i;
 
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	xa_for_each(&pin->dpll_refs, i, ref) {
 		reg = dpll_pin_registration_find(ref, ops, priv, parent);
 		if (!reg)
@@ -1120,7 +1124,7 @@ void dpll_pin_on_pin_unregister(struct dpll_pin *parent, struct dpll_pin *pin,
 		__dpll_pin_unregister(ref->dpll, pin, ops, priv, parent);
 	}
 	dpll_xa_ref_pin_del(&pin->parent_refs, parent, ops, priv, pin);
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 }
 EXPORT_SYMBOL_GPL(dpll_pin_on_pin_unregister);
 
@@ -1141,13 +1145,13 @@ int dpll_pin_ref_sync_pair_add(struct dpll_pin *pin,
 {
 	int ret;
 
-	mutex_lock(&dpll_lock);
+	down_write(&dpll_lock);
 	ret = xa_insert(&pin->ref_sync_pins, ref_sync_pin->id,
 			ref_sync_pin, GFP_KERNEL);
 	mutex_lock(&pin->domain->lock);
 	__dpll_pin_change_ntf(pin);
 	mutex_unlock(&pin->domain->lock);
-	mutex_unlock(&dpll_lock);
+	up_write(&dpll_lock);
 
 	return ret;
 }
@@ -1253,23 +1257,12 @@ const struct dpll_pin_ops *dpll_pin_ops(struct dpll_pin_ref *ref)
 
 static int __init dpll_init(void)
 {
-	int ret;
-
-	ret = genl_register_family(&dpll_nl_family);
-	if (ret)
-		goto error;
-
-	return 0;
-
-error:
-	mutex_destroy(&dpll_lock);
-	return ret;
+	return genl_register_family(&dpll_nl_family);
 }
 
 static void __exit dpll_exit(void)
 {
 	genl_unregister_family(&dpll_nl_family);
-	mutex_destroy(&dpll_lock);
 }
 
 subsys_initcall(dpll_init);
diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
index 630c5a1ca572c2..e212e53528e33b 100644
--- a/drivers/dpll/dpll_core.h
+++ b/drivers/dpll/dpll_core.h
@@ -12,6 +12,7 @@
 #include <linux/mutex.h>
 #include <linux/refcount.h>
 #include <linux/ref_tracker.h>
+#include <linux/rwsem.h>
 #include "dpll_nl.h"
 
 #define DPLL_REGISTERED		XA_MARK_1
@@ -129,7 +130,7 @@ const struct dpll_pin_ops *dpll_pin_ops(struct dpll_pin_ref *ref);
 struct dpll_pin_ref *dpll_xa_ref_dpll_first(struct xarray *xa_refs);
 extern struct xarray dpll_device_xa;
 extern struct xarray dpll_pin_xa;
-extern struct mutex dpll_lock;
+extern struct rw_semaphore dpll_lock;
 
 void dpll_device_notify(struct dpll_device *dpll, unsigned long action);
 void dpll_pin_notify(struct dpll_pin *pin, u64 src_clock_id,
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index 1b4e3a0166a744..9d8f5d1ce97248 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -910,11 +910,11 @@ int dpll_device_change_ntf(struct dpll_device *dpll)
 {
 	int ret;
 
-	mutex_lock(&dpll_lock);
+	down_read(&dpll_lock);
 	mutex_lock(&dpll->domain->lock);
 	ret = __dpll_device_change_ntf(dpll);
 	mutex_unlock(&dpll->domain->lock);
-	mutex_unlock(&dpll_lock);
+	up_read(&dpll_lock);
 
 	return ret;
 }
@@ -1004,11 +1004,11 @@ int dpll_pin_change_ntf(struct dpll_pin *pin)
 {
 	int ret;
 
-	mutex_lock(&dpll_lock);
+	down_read(&dpll_lock);
 	mutex_lock(&pin->domain->lock);
 	ret = __dpll_pin_change_ntf(pin);
 	mutex_unlock(&pin->domain->lock);
-	mutex_unlock(&dpll_lock);
+	up_read(&dpll_lock);
 
 	return ret;
 }
@@ -1795,7 +1795,7 @@ int dpll_nl_pin_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 	unsigned long i;
 	int ret = 0;
 
-	mutex_lock(&dpll_lock);
+	down_read(&dpll_lock);
 	xa_for_each_marked_start(&dpll_pin_xa, i, pin, DPLL_REGISTERED,
 				 ctx->idx) {
 		if (!dpll_pin_available(pin))
@@ -1821,7 +1821,7 @@ int dpll_nl_pin_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 		}
 		genlmsg_end(skb, hdr);
 	}
-	mutex_unlock(&dpll_lock);
+	up_read(&dpll_lock);
 
 	if (ret == -EMSGSIZE) {
 		ctx->idx = i;
@@ -2019,7 +2019,7 @@ int dpll_nl_device_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 	unsigned long i;
 	int ret = 0;
 
-	mutex_lock(&dpll_lock);
+	down_read(&dpll_lock);
 	xa_for_each_marked_start(&dpll_device_xa, i, dpll, DPLL_REGISTERED,
 				 ctx->idx) {
 		hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid,
@@ -2038,7 +2038,7 @@ int dpll_nl_device_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 		}
 		genlmsg_end(skb, hdr);
 	}
-	mutex_unlock(&dpll_lock);
+	up_read(&dpll_lock);
 
 	if (ret == -EMSGSIZE) {
 		ctx->idx = i;
@@ -2056,7 +2056,7 @@ int dpll_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 	if (GENL_REQ_ATTR_CHECK(info, DPLL_A_ID))
 		return -EINVAL;
 
-	mutex_lock(&dpll_lock);
+	down_read(&dpll_lock);
 	id = nla_get_u32(info->attrs[DPLL_A_ID]);
 	dpll = dpll_device_get_by_id(id);
 	if (!dpll) {
@@ -2067,7 +2067,7 @@ int dpll_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 	mutex_lock(&dpll->domain->lock);
 	return 0;
 unlock:
-	mutex_unlock(&dpll_lock);
+	up_read(&dpll_lock);
 	return -ENODEV;
 }
 
@@ -2077,14 +2077,14 @@ void dpll_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 	struct dpll_device *dpll = info->user_ptr[0];
 
 	mutex_unlock(&dpll->domain->lock);
-	mutex_unlock(&dpll_lock);
+	up_read(&dpll_lock);
 }
 
 int
 dpll_lock_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 	       struct genl_info *info)
 {
-	mutex_lock(&dpll_lock);
+	down_read(&dpll_lock);
 
 	return 0;
 }
@@ -2093,7 +2093,7 @@ void
 dpll_unlock_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 		 struct genl_info *info)
 {
-	mutex_unlock(&dpll_lock);
+	up_read(&dpll_lock);
 }
 
 int dpll_pin_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
@@ -2102,7 +2102,7 @@ int dpll_pin_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 	struct dpll_pin *pin;
 	int ret;
 
-	mutex_lock(&dpll_lock);
+	down_read(&dpll_lock);
 	if (GENL_REQ_ATTR_CHECK(info, DPLL_A_PIN_ID)) {
 		ret = -EINVAL;
 		goto unlock_dev;
@@ -2119,7 +2119,7 @@ int dpll_pin_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 	return 0;
 
 unlock_dev:
-	mutex_unlock(&dpll_lock);
+	up_read(&dpll_lock);
 	return ret;
 }
 
@@ -2129,5 +2129,5 @@ void dpll_pin_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 	struct dpll_pin *pin = info->user_ptr[0];
 
 	mutex_unlock(&pin->domain->lock);
-	mutex_unlock(&dpll_lock);
+	up_read(&dpll_lock);
 }
-- 
2.56.0

[PATCH net-next v2 5/9] dpll: add KUnit tests for core locking

[Ivan Vecera]

Add KUnit tests using a mock driver with unrelated domains, a pin of one
domain registered with a dpll of another and a pin of one domain under a
MUX pin of another. They check that:

- objects with the same (module, clock_id) share a refcounted domain
- callbacks of a device run under its domain and callbacks of a pin under
  the domain of the pin alone, whichever driver registered them
- a callback blocked in one domain does not stop unrelated domains
- callbacks of one domain are serialized, also across its devices
- a stuck callback of a foreign registration stops only the domain of the
  pin, and a stuck foreign device or domain of the pin does not stop the
  other side
- a pin and its parent pin of another domain do not block each other
- registrations can connect two domains in both directions
- a topology change waits until running requests finish
- a request setting several attributes of a pin is atomic against other
  requests about the pin, also when it spans two domains
- notifications from callbacks keep their order, also when a callback
  fails

Requests that can block run in kthreads, so a test can report a locking
bug instead of blocking itself. A real deadlock cannot be undone, the
stuck request keeps its locks, so the teardown then hangs until the run
times out. The handlers are called directly with a hand-built message,
so they are exported for KUnit.

Only lockdep can tell which locks a callback holds. Without it the checks
of held locks cannot fail, so the test that checks nothing else is
skipped. The tests of what blocks what do not depend on it.

The tests do not cover what needs a netlink socket, for example the order
of notifications against the ACK or the content of messages.

Run them with:

  tools/testing/kunit/kunit.py run --kunitconfig=drivers/dpll

Reviewed-by: Petr Oros <poros@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
Changes in v2:
- Used kunit_fwnode_create_software_node() (Sashiko)
- Released blocked callbacks in the exit callback of the suite, so a
  test aborted before its request is set up cannot hang the
  teardown (Sashiko)
---
 drivers/dpll/.kunitconfig      |    5 +
 drivers/dpll/Kconfig           |   13 +
 drivers/dpll/Makefile          |    1 +
 drivers/dpll/dpll_core_kunit.c | 1478 ++++++++++++++++++++++++++++++++
 drivers/dpll/dpll_netlink.c    |    7 +
 5 files changed, 1504 insertions(+)
 create mode 100644 drivers/dpll/.kunitconfig
 create mode 100644 drivers/dpll/dpll_core_kunit.c

diff --git a/drivers/dpll/.kunitconfig b/drivers/dpll/.kunitconfig
new file mode 100644
index 00000000000000..017c9d5b593a4f
--- /dev/null
+++ b/drivers/dpll/.kunitconfig
@@ -0,0 +1,5 @@
+CONFIG_KUNIT=y
+CONFIG_NET=y
+CONFIG_DPLL_KUNIT_TEST=y
+CONFIG_DEBUG_KERNEL=y
+CONFIG_PROVE_LOCKING=y
diff --git a/drivers/dpll/Kconfig b/drivers/dpll/Kconfig
index be98969f040ab6..12bcf706a15e14 100644
--- a/drivers/dpll/Kconfig
+++ b/drivers/dpll/Kconfig
@@ -23,6 +23,19 @@ config DPLL_REFCNT_TRACKER
 
 	  If unsure, say N.
 
+config DPLL_KUNIT_TEST
+	tristate "KUnit tests for DPLL core" if !KUNIT_ALL_TESTS
+	depends on KUNIT && NET
+	select DPLL
+	default KUNIT_ALL_TESTS
+	help
+	  KUnit tests for locking in the DPLL core. They use a mock driver
+	  to check that callbacks run under the locks of their domains and
+	  that unrelated domains do not block each other. The checks of held
+	  locks need PROVE_LOCKING.
+
+	  If unsure, say N.
+
 source "drivers/dpll/zl3073x/Kconfig"
 
 endmenu
diff --git a/drivers/dpll/Makefile b/drivers/dpll/Makefile
index 9e7a3a3e592e81..75ee1608c76a93 100644
--- a/drivers/dpll/Makefile
+++ b/drivers/dpll/Makefile
@@ -8,4 +8,5 @@ dpll-y                  += dpll_core.o
 dpll-y                  += dpll_netlink.o
 dpll-y                  += dpll_nl.o
 
+obj-$(CONFIG_DPLL_KUNIT_TEST)	+= dpll_core_kunit.o
 obj-$(CONFIG_ZL3073X)	+= zl3073x/
diff --git a/drivers/dpll/dpll_core_kunit.c b/drivers/dpll/dpll_core_kunit.c
new file mode 100644
index 00000000000000..21f941a5fdec4e
--- /dev/null
+++ b/drivers/dpll/dpll_core_kunit.c
@@ -0,0 +1,1478 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for locking in the DPLL core.
+ *
+ * Each test starts with two unrelated domains:
+ *
+ * Domain A: devices A0 and A1
+ *   pin P on A0 and A1, setting its state on A0 notifies pin S
+ *   pin S on A0 and A1
+ * Domain B: device B0
+ *   pin BP on B0
+ *
+ * Tests of objects connected by a pin add these domains:
+ *
+ * Domain E: device E0, pin X on E0 (the owner), pin M (MUX) on E0
+ * Domain F: device F0, pin X from E is also registered with F0
+ * Domain G: pin C registered under pin M from E
+ *
+ * Requests that can block run in kthreads, so a test can report a locking bug
+ * instead of blocking itself. A real deadlock cannot be undone, the stuck
+ * request keeps its locks, so the teardown then hangs until the run times out.
+ */
+
+#include <kunit/fwnode.h>
+#include <kunit/test.h>
+#include <linux/completion.h>
+#include <linux/debug_locks.h>
+#include <linux/dpll.h>
+#include <linux/kthread.h>
+#include <linux/lockdep.h>
+#include <linux/notifier.h>
+#include <linux/wait.h>
+#include <net/genetlink.h>
+#include <net/netlink.h>
+
+#include "dpll_core.h"
+
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
+
+#define DT_CLOCK_ID_A		0xd0110a
+#define DT_CLOCK_ID_B		0xd0110b
+#define DT_CLOCK_ID_E		0xd0110e
+#define DT_CLOCK_ID_F		0xd0110f
+#define DT_CLOCK_ID_G		0xd01107
+
+#define DT_PIN_IDX_P		0
+#define DT_PIN_IDX_S		1
+#define DT_PIN_IDX_BP		2
+#define DT_PIN_IDX_M		3
+#define DT_PIN_IDX_X		4
+#define DT_PIN_IDX_C		5
+#define DT_PIN_IDX_Y		6
+#define DT_PIN_IDX_EXTRA	7
+
+#define DT_MAX_EVENTS		8
+#define DT_TIMEOUT		msecs_to_jiffies(5000)
+#define DT_SHORT_TIMEOUT	msecs_to_jiffies(100)
+
+struct dt_ctx;
+
+/* Counters and blocking of the callbacks of one driver instance */
+struct dt_dom {
+	struct dt_ctx *ctx;
+	struct completion entered;
+	atomic_t cur;
+	atomic_t peak;
+	bool block;
+};
+
+struct dt_dev {
+	struct dt_dom *dom;
+	struct dpll_device *dpll;
+	dpll_tracker tracker;
+	struct dt_dev *notify;
+	enum dpll_mode mode;
+	bool block;
+};
+
+struct dt_pin {
+	struct dpll_pin *pin;
+	dpll_tracker tracker;
+};
+
+struct dt_reg {
+	struct dt_dom *dom;
+	struct dt_pin *pin;
+	struct dt_dev *dev;
+	struct dt_pin *parent;
+	struct dt_pin *notify;
+	struct dpll_domain *forbid;
+	enum dpll_pin_state state;
+	enum dpll_pin_direction direction;
+	atomic_t freq_gets;
+	u32 prio;
+	bool block_prio_set;
+};
+
+struct dt_ctx {
+	struct kunit *test;
+	struct dt_dom dom_a, dom_b, dom_e, dom_f, dom_g;
+	struct dt_dev a0, a1, b0, e0, f0;
+	struct dt_pin p, s, bp, x, y, m, c, extra;
+	struct dt_reg reg_p_a0, reg_p_a1, reg_s_a0, reg_s_a1, reg_bp_b0;
+	struct dt_reg reg_x_e0, reg_x_f0, reg_y_e0, reg_y_f0, reg_m_e0, reg_c_m;
+	struct dt_reg *atomic_regs[2];
+	struct notifier_block nb;
+	wait_queue_head_t wq;
+	atomic_t cur_total;
+	atomic_t peak_total;
+	atomic_t wrong_lock;
+	atomic_t bad_ntf;
+	atomic_t torn;
+	struct task_struct *set_task;
+	void *events[DT_MAX_EVENTS];
+	int num_events;
+	bool record;
+	bool fail_after_ntf;
+	bool debug_locks_at_start;
+};
+
+struct dt_req {
+	struct dt_ctx *ctx;
+	int (*fn)(void *arg);
+	void *arg;
+	int ret;
+	struct completion done;
+	bool finished;
+};
+
+/* A nested attribute of a pin set request */
+struct dt_nest {
+	int type;
+	u32 parent_id;
+	int attr;
+	u32 val;
+};
+
+/* Allocated with kunit_kzalloc(), a failed assertion must not free them */
+struct dt_pin_set_args {
+	struct dt_ctx *ctx;
+	struct dpll_pin *pin;
+	struct dt_nest nests[2];
+	int num;
+};
+
+struct dt_mode_set_args {
+	struct dpll_device *dpll;
+	enum dpll_mode mode;
+};
+
+/*
+ * The lock may be held by another task, so mutex_is_locked() is not enough.
+ * Without lockdep no check of a held lock can fail.
+ */
+static bool dt_held(struct dpll_domain *domain)
+{
+#ifdef CONFIG_LOCKDEP
+	return lockdep_is_held(&domain->lock) != 0;
+#else
+	return true;
+#endif
+}
+
+static bool dt_surely_held(struct dpll_domain *domain)
+{
+#ifdef CONFIG_LOCKDEP
+	return lockdep_is_held(&domain->lock) > 0;
+#else
+	return false;
+#endif
+}
+
+static void dt_peak(atomic_t *peak, int val)
+{
+	int old = atomic_read(peak);
+
+	while (val > old && !atomic_try_cmpxchg(peak, &old, val))
+		;
+}
+
+static void dt_block(struct dt_dom *dom, bool *block)
+{
+	complete(&dom->entered);
+	wait_event(dom->ctx->wq, !READ_ONCE(*block));
+}
+
+static void dt_enter(struct dt_dom *dom)
+{
+	struct dt_ctx *ctx = dom->ctx;
+
+	dt_peak(&ctx->peak_total, atomic_inc_return(&ctx->cur_total));
+	dt_peak(&dom->peak, atomic_inc_return(&dom->cur));
+
+	if (READ_ONCE(dom->block))
+		dt_block(dom, &dom->block);
+}
+
+static void dt_leave(struct dt_dom *dom)
+{
+	atomic_dec(&dom->cur);
+	atomic_dec(&dom->ctx->cur_total);
+}
+
+static void dt_dev_enter(struct dt_dev *dev, const struct dpll_device *dpll)
+{
+	if (!dt_held(dpll->domain))
+		atomic_inc(&dev->dom->ctx->wrong_lock);
+	dt_enter(dev->dom);
+	if (READ_ONCE(dev->block))
+		dt_block(dev->dom, &dev->block);
+}
+
+/* A callback of a pin runs under the domain of the pin alone */
+static void dt_pin_enter(struct dt_reg *reg, const struct dpll_pin *pin)
+{
+	struct dt_ctx *ctx = reg->dom->ctx;
+
+	if (!dt_held(pin->domain) ||
+	    (reg->forbid && dt_surely_held(reg->forbid)))
+		atomic_inc(&ctx->wrong_lock);
+	dt_enter(reg->dom);
+}
+
+static int dt_mode_get(const struct dpll_device *dpll, void *priv,
+		       enum dpll_mode *mode, struct netlink_ext_ack *extack)
+{
+	struct dt_dev *dev = priv;
+
+	dt_dev_enter(dev, dpll);
+	*mode = dev->mode;
+	dt_leave(dev->dom);
+
+	return 0;
+}
+
+static int dt_mode_set(const struct dpll_device *dpll, void *priv,
+		       enum dpll_mode mode, struct netlink_ext_ack *extack)
+{
+	struct dt_dev *dev = priv;
+
+	dt_dev_enter(dev, dpll);
+	dev->mode = mode;
+	dt_leave(dev->dom);
+
+	if (dev->notify)
+		__dpll_device_change_ntf(dev->notify->dpll);
+
+	return 0;
+}
+
+static int dt_supported_modes_get(const struct dpll_device *dpll, void *priv,
+				  unsigned long *modes,
+				  struct netlink_ext_ack *extack)
+{
+	struct dt_dev *dev = priv;
+
+	dt_dev_enter(dev, dpll);
+	__set_bit(DPLL_MODE_AUTOMATIC, modes);
+	__set_bit(DPLL_MODE_MANUAL, modes);
+	dt_leave(dev->dom);
+
+	return 0;
+}
+
+static int dt_lock_status_get(const struct dpll_device *dpll, void *priv,
+			      enum dpll_lock_status *status,
+			      enum dpll_lock_status_error *status_error,
+			      struct netlink_ext_ack *extack)
+{
+	struct dt_dev *dev = priv;
+
+	dt_dev_enter(dev, dpll);
+	*status = DPLL_LOCK_STATUS_LOCKED;
+	dt_leave(dev->dom);
+
+	return 0;
+}
+
+static const struct dpll_device_ops dt_dev_ops = {
+	.mode_get = dt_mode_get,
+	.mode_set = dt_mode_set,
+	.supported_modes_get = dt_supported_modes_get,
+	.lock_status_get = dt_lock_status_get,
+};
+
+static int dt_frequency_get(const struct dpll_pin *pin, void *pin_priv,
+			    const struct dpll_device *dpll, void *dpll_priv,
+			    u64 *frequency, struct netlink_ext_ack *extack)
+{
+	struct dt_reg *reg = pin_priv;
+
+	dt_pin_enter(reg, pin);
+	atomic_inc(&reg->freq_gets);
+	*frequency = 1;
+	dt_leave(reg->dom);
+
+	return 0;
+}
+
+static int dt_state_on_dpll_get(const struct dpll_pin *pin, void *pin_priv,
+				const struct dpll_device *dpll, void *dpll_priv,
+				enum dpll_pin_state *state,
+				struct netlink_ext_ack *extack)
+{
+	struct dt_reg *reg = pin_priv;
+
+	dt_pin_enter(reg, pin);
+	*state = reg->state;
+	dt_leave(reg->dom);
+
+	return 0;
+}
+
+static int dt_state_on_dpll_set(const struct dpll_pin *pin, void *pin_priv,
+				const struct dpll_device *dpll, void *dpll_priv,
+				const enum dpll_pin_state state,
+				struct netlink_ext_ack *extack)
+{
+	struct dt_reg *reg = pin_priv;
+
+	dt_pin_enter(reg, pin);
+	reg->state = state;
+	dt_leave(reg->dom);
+
+	if (reg->notify)
+		__dpll_pin_change_ntf(reg->notify->pin);
+
+	return reg->dom->ctx->fail_after_ntf ? -EIO : 0;
+}
+
+static int dt_state_on_pin_get(const struct dpll_pin *pin, void *pin_priv,
+			       const struct dpll_pin *parent_pin,
+			       void *parent_pin_priv,
+			       enum dpll_pin_state *state,
+			       struct netlink_ext_ack *extack)
+{
+	struct dt_reg *reg = pin_priv;
+
+	dt_pin_enter(reg, pin);
+	*state = reg->state;
+	dt_leave(reg->dom);
+
+	return 0;
+}
+
+static int dt_state_on_pin_set(const struct dpll_pin *pin, void *pin_priv,
+			       const struct dpll_pin *parent_pin,
+			       void *parent_pin_priv,
+			       const enum dpll_pin_state state,
+			       struct netlink_ext_ack *extack)
+{
+	struct dt_reg *reg = pin_priv;
+
+	dt_pin_enter(reg, pin);
+	reg->state = state;
+	dt_leave(reg->dom);
+
+	return 0;
+}
+
+static int dt_prio_get(const struct dpll_pin *pin, void *pin_priv,
+		       const struct dpll_device *dpll, void *dpll_priv,
+		       u32 *prio, struct netlink_ext_ack *extack)
+{
+	struct dt_reg *reg = pin_priv;
+	struct dt_ctx *ctx = reg->dom->ctx;
+
+	dt_pin_enter(reg, pin);
+	/* Only the request doing the set may see it half done */
+	if (READ_ONCE(ctx->set_task) && current != ctx->set_task &&
+	    ctx->atomic_regs[0]->prio != ctx->atomic_regs[1]->prio)
+		atomic_inc(&ctx->torn);
+	*prio = reg->prio;
+	dt_leave(reg->dom);
+
+	return 0;
+}
+
+static int dt_prio_set(const struct dpll_pin *pin, void *pin_priv,
+		       const struct dpll_device *dpll, void *dpll_priv,
+		       const u32 prio, struct netlink_ext_ack *extack)
+{
+	struct dt_reg *reg = pin_priv;
+
+	dt_pin_enter(reg, pin);
+	if (READ_ONCE(reg->block_prio_set))
+		dt_block(reg->dom, &reg->block_prio_set);
+	reg->prio = prio;
+	dt_leave(reg->dom);
+
+	return 0;
+}
+
+static int dt_direction_get(const struct dpll_pin *pin, void *pin_priv,
+			    const struct dpll_device *dpll, void *dpll_priv,
+			    enum dpll_pin_direction *direction,
+			    struct netlink_ext_ack *extack)
+{
+	struct dt_reg *reg = pin_priv;
+
+	dt_pin_enter(reg, pin);
+	*direction = reg->direction;
+	dt_leave(reg->dom);
+
+	return 0;
+}
+
+static int dt_direction_set(const struct dpll_pin *pin, void *pin_priv,
+			    const struct dpll_device *dpll, void *dpll_priv,
+			    const enum dpll_pin_direction direction,
+			    struct netlink_ext_ack *extack)
+{
+	struct dt_reg *reg = pin_priv;
+
+	dt_pin_enter(reg, pin);
+	reg->direction = direction;
+	dt_leave(reg->dom);
+
+	return 0;
+}
+
+static int dt_ref_sync_get(const struct dpll_pin *pin, void *pin_priv,
+			   const struct dpll_pin *ref_sync_pin,
+			   void *ref_sync_pin_priv, enum dpll_pin_state *state,
+			   struct netlink_ext_ack *extack)
+{
+	struct dt_reg *reg = pin_priv;
+
+	dt_pin_enter(reg, pin);
+	*state = DPLL_PIN_STATE_DISCONNECTED;
+	dt_leave(reg->dom);
+
+	return 0;
+}
+
+static const struct dpll_pin_ops dt_pin_ops = {
+	.frequency_get = dt_frequency_get,
+	.state_on_dpll_get = dt_state_on_dpll_get,
+	.state_on_dpll_set = dt_state_on_dpll_set,
+	.state_on_pin_get = dt_state_on_pin_get,
+	.state_on_pin_set = dt_state_on_pin_set,
+	.prio_get = dt_prio_get,
+	.prio_set = dt_prio_set,
+	.direction_get = dt_direction_get,
+	.direction_set = dt_direction_set,
+	.ref_sync_get = dt_ref_sync_get,
+};
+
+static const struct dpll_pin_properties dt_prop = {
+	.type = DPLL_PIN_TYPE_EXT,
+	.capabilities = DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE |
+			DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE |
+			DPLL_PIN_CAPABILITIES_DIRECTION_CAN_CHANGE,
+};
+
+static const struct dpll_pin_properties dt_prop_mux = {
+	.type = DPLL_PIN_TYPE_MUX,
+	.capabilities = DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE,
+};
+
+/* Other dpll drivers in the system can send notifications at any time */
+static bool dt_own_clock_id(u64 clock_id)
+{
+	switch (clock_id) {
+	case DT_CLOCK_ID_A:
+	case DT_CLOCK_ID_B:
+	case DT_CLOCK_ID_E:
+	case DT_CLOCK_ID_F:
+	case DT_CLOCK_ID_G:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static int dt_notifier(struct notifier_block *nb, unsigned long action,
+		       void *data)
+{
+	struct dt_ctx *ctx = container_of(nb, struct dt_ctx, nb);
+	struct dpll_device_notifier_info *dev_info = data;
+	struct dpll_pin_notifier_info *pin_info = data;
+	struct dpll_domain *domain;
+	u64 clock_id;
+	void *obj;
+
+	switch (action) {
+	case DPLL_DEVICE_CHANGED:
+		clock_id = dev_info->clock_id;
+		obj = dev_info->dpll;
+		domain = dev_info->dpll->domain;
+		break;
+	case DPLL_PIN_CHANGED:
+		clock_id = pin_info->clock_id;
+		obj = pin_info->pin;
+		domain = pin_info->pin->domain;
+		break;
+	default:
+		return NOTIFY_DONE;
+	}
+
+	if (!dt_own_clock_id(clock_id) || !ctx->record)
+		return NOTIFY_DONE;
+
+	if (!dt_held(domain))
+		atomic_inc(&ctx->bad_ntf);
+	if (ctx->num_events < DT_MAX_EVENTS)
+		ctx->events[ctx->num_events] = obj;
+	ctx->num_events++;
+
+	return NOTIFY_OK;
+}
+
+static void dt_final_check(void *data)
+{
+	struct dt_ctx *ctx = data;
+
+	KUNIT_EXPECT_EQ(ctx->test, atomic_read(&ctx->wrong_lock), 0);
+	if (ctx->debug_locks_at_start)
+		KUNIT_EXPECT_TRUE(ctx->test, debug_locks);
+}
+
+static void dt_dev_release(void *data)
+{
+	struct dt_dev *dev = data;
+
+	dpll_device_unregister(dev->dpll, &dt_dev_ops, dev);
+	dpll_device_put(dev->dpll, &dev->tracker);
+}
+
+static void dt_dev_add(struct kunit *test, struct dt_dev *dev,
+		       struct dt_dom *dom, u64 clock_id, u32 idx,
+		       enum dpll_type type)
+{
+	int ret;
+
+	dev->dom = dom;
+	dev->mode = DPLL_MODE_AUTOMATIC;
+	dev->dpll = dpll_device_get(clock_id, idx, THIS_MODULE, &dev->tracker);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev->dpll);
+
+	ret = dpll_device_register(dev->dpll, type, &dt_dev_ops, dev);
+	if (ret)
+		dpll_device_put(dev->dpll, &dev->tracker);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+	KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, dt_dev_release,
+							dev), 0);
+}
+
+static void dt_pin_release(void *data)
+{
+	struct dt_pin *pin = data;
+
+	if (!IS_ERR_OR_NULL(pin->pin))
+		dpll_pin_put(pin->pin, &pin->tracker);
+}
+
+static void dt_pin_add(struct kunit *test, struct dt_pin *pin, u64 clock_id,
+		       u32 idx, const struct dpll_pin_properties *prop)
+{
+	pin->pin = dpll_pin_get(clock_id, idx, THIS_MODULE, prop,
+				&pin->tracker);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pin->pin);
+	KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, dt_pin_release,
+							pin), 0);
+}
+
+static void dt_reg_release(void *data)
+{
+	struct dt_reg *reg = data;
+
+	if (reg->parent)
+		dpll_pin_on_pin_unregister(reg->parent->pin, reg->pin->pin,
+					   &dt_pin_ops, reg);
+	else
+		dpll_pin_unregister(reg->dev->dpll, reg->pin->pin, &dt_pin_ops,
+				    reg);
+}
+
+static void dt_reg_add(struct kunit *test, struct dt_reg *reg,
+		       struct dt_dom *dom, struct dt_pin *pin,
+		       struct dt_dev *dev, struct dt_pin *parent)
+{
+	int ret;
+
+	reg->dom = dom;
+	reg->pin = pin;
+	reg->dev = dev;
+	reg->parent = parent;
+	reg->state = DPLL_PIN_STATE_SELECTABLE;
+	reg->direction = DPLL_PIN_DIRECTION_INPUT;
+
+	if (parent)
+		ret = dpll_pin_on_pin_register(parent->pin, pin->pin,
+					       &dt_pin_ops, reg);
+	else
+		ret = dpll_pin_register(dev->dpll, pin->pin, &dt_pin_ops, reg);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+	KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, dt_reg_release,
+							reg), 0);
+}
+
+/* Creates a pin with a fwnode so that it can be registered with another dpll */
+static void dt_fwnode_pin_add(struct kunit *test, struct dt_pin *pin,
+			      u64 clock_id, u32 idx)
+{
+	struct fwnode_handle *fwnode;
+
+	fwnode = kunit_fwnode_create_software_node(test, NULL, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fwnode);
+	dt_pin_add(test, pin, clock_id, idx, &dt_prop);
+	dpll_pin_fwnode_set(pin->pin, fwnode);
+}
+
+/* Builds domains E and F connected by pin X */
+static void dt_foreign_pin_add(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+
+	dt_dev_add(test, &ctx->e0, &ctx->dom_e, DT_CLOCK_ID_E, 0,
+		   DPLL_TYPE_PPS);
+	dt_dev_add(test, &ctx->f0, &ctx->dom_f, DT_CLOCK_ID_F, 0,
+		   DPLL_TYPE_PPS);
+	dt_fwnode_pin_add(test, &ctx->x, DT_CLOCK_ID_E, DT_PIN_IDX_X);
+	dt_reg_add(test, &ctx->reg_x_e0, &ctx->dom_e, &ctx->x, &ctx->e0, NULL);
+	dt_reg_add(test, &ctx->reg_x_f0, &ctx->dom_f, &ctx->x, &ctx->f0, NULL);
+
+	/* No callback of the pin takes the lock of the other domain */
+	ctx->reg_x_e0.forbid = ctx->f0.dpll->domain;
+	ctx->reg_x_f0.forbid = ctx->f0.dpll->domain;
+}
+
+/*
+ * Builds domains E and G connected by pin C under pin M. The pins are added
+ * first, so that domain G is freed only after the registrations are gone.
+ */
+static void dt_on_pin_add(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+
+	dt_dev_add(test, &ctx->e0, &ctx->dom_e, DT_CLOCK_ID_E, 0,
+		   DPLL_TYPE_PPS);
+	dt_pin_add(test, &ctx->m, DT_CLOCK_ID_E, DT_PIN_IDX_M, &dt_prop_mux);
+	dt_pin_add(test, &ctx->c, DT_CLOCK_ID_G, DT_PIN_IDX_C, &dt_prop);
+	dt_reg_add(test, &ctx->reg_m_e0, &ctx->dom_e, &ctx->m, &ctx->e0, NULL);
+	dt_reg_add(test, &ctx->reg_c_m, &ctx->dom_g, &ctx->c, NULL, &ctx->m);
+
+	ctx->reg_c_m.forbid = ctx->e0.dpll->domain;
+	ctx->reg_m_e0.forbid = ctx->c.pin->domain;
+}
+
+static void dt_notifier_release(void *data)
+{
+	struct dt_ctx *ctx = data;
+
+	unregister_dpll_notifier(&ctx->nb);
+}
+
+static void dt_dom_init(struct dt_dom *dom, struct dt_ctx *ctx)
+{
+	dom->ctx = ctx;
+	init_completion(&dom->entered);
+}
+
+static int dt_init(struct kunit *test)
+{
+	struct dt_ctx *ctx;
+
+	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, ctx);
+	test->priv = ctx;
+
+	ctx->test = test;
+	dt_dom_init(&ctx->dom_a, ctx);
+	dt_dom_init(&ctx->dom_b, ctx);
+	dt_dom_init(&ctx->dom_e, ctx);
+	dt_dom_init(&ctx->dom_f, ctx);
+	dt_dom_init(&ctx->dom_g, ctx);
+	init_waitqueue_head(&ctx->wq);
+	ctx->debug_locks_at_start = debug_locks;
+
+	/* Runs last, after the deletion of all objects */
+	KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, dt_final_check,
+							ctx), 0);
+
+	dt_dev_add(test, &ctx->a0, &ctx->dom_a, DT_CLOCK_ID_A, 0,
+		   DPLL_TYPE_PPS);
+	dt_dev_add(test, &ctx->a1, &ctx->dom_a, DT_CLOCK_ID_A, 1,
+		   DPLL_TYPE_EEC);
+	dt_dev_add(test, &ctx->b0, &ctx->dom_b, DT_CLOCK_ID_B, 0,
+		   DPLL_TYPE_PPS);
+	ctx->a0.notify = &ctx->a1;
+
+	dt_pin_add(test, &ctx->p, DT_CLOCK_ID_A, DT_PIN_IDX_P, &dt_prop);
+	dt_reg_add(test, &ctx->reg_p_a0, &ctx->dom_a, &ctx->p, &ctx->a0, NULL);
+	dt_reg_add(test, &ctx->reg_p_a1, &ctx->dom_a, &ctx->p, &ctx->a1, NULL);
+	ctx->reg_p_a0.notify = &ctx->s;
+
+	dt_pin_add(test, &ctx->s, DT_CLOCK_ID_A, DT_PIN_IDX_S, &dt_prop);
+	dt_reg_add(test, &ctx->reg_s_a0, &ctx->dom_a, &ctx->s, &ctx->a0, NULL);
+	dt_reg_add(test, &ctx->reg_s_a1, &ctx->dom_a, &ctx->s, &ctx->a1, NULL);
+
+	dt_pin_add(test, &ctx->bp, DT_CLOCK_ID_B, DT_PIN_IDX_BP, &dt_prop);
+	dt_reg_add(test, &ctx->reg_bp_b0, &ctx->dom_b, &ctx->bp, &ctx->b0,
+		   NULL);
+
+	ctx->nb.notifier_call = dt_notifier;
+	KUNIT_ASSERT_EQ(test, register_dpll_notifier(&ctx->nb), 0);
+	KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test,
+							dt_notifier_release,
+							ctx), 0);
+
+	atomic_set(&ctx->peak_total, 0);
+	atomic_set(&ctx->dom_a.peak, 0);
+	atomic_set(&ctx->dom_b.peak, 0);
+
+	return 0;
+}
+
+static int dt_device_ntf(void *arg)
+{
+	struct dt_dev *dev = arg;
+
+	return dpll_device_change_ntf(dev->dpll);
+}
+
+static int dt_pin_ntf(void *arg)
+{
+	struct dt_pin *pin = arg;
+
+	return dpll_pin_change_ntf(pin->pin);
+}
+
+static int dt_extra_pin_get(void *arg)
+{
+	struct dt_pin *pin = arg;
+
+	pin->pin = dpll_pin_get(DT_CLOCK_ID_B, DT_PIN_IDX_EXTRA, THIS_MODULE,
+				&dt_prop, &pin->tracker);
+
+	return PTR_ERR_OR_ZERO(pin->pin);
+}
+
+static void dt_unblock(struct dt_ctx *ctx)
+{
+	WRITE_ONCE(ctx->dom_a.block, false);
+	WRITE_ONCE(ctx->dom_b.block, false);
+	WRITE_ONCE(ctx->dom_e.block, false);
+	WRITE_ONCE(ctx->dom_f.block, false);
+	WRITE_ONCE(ctx->dom_g.block, false);
+	WRITE_ONCE(ctx->f0.block, false);
+	WRITE_ONCE(ctx->reg_p_a0.block_prio_set, false);
+	WRITE_ONCE(ctx->reg_x_e0.block_prio_set, false);
+	WRITE_ONCE(ctx->reg_x_f0.block_prio_set, false);
+	wake_up_all(&ctx->wq);
+}
+
+/*
+ * Runs before the cleanup actions, also when a test aborts, so that no release
+ * of an object waits for a blocked callback. test->priv is NULL if dt_init()
+ * failed to allocate the context.
+ */
+static void dt_exit(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+
+	if (ctx)
+		dt_unblock(ctx);
+}
+
+static int dt_req_thread(void *data)
+{
+	struct dt_req *req = data;
+
+	req->ret = req->fn(req->arg);
+	kthread_complete_and_exit(&req->done, 0);
+}
+
+/* Runs before the objects used by the request are released */
+static void dt_req_release(void *data)
+{
+	struct dt_req *req = data;
+
+	dt_unblock(req->ctx);
+	if (!req->finished)
+		wait_for_completion(&req->done);
+}
+
+static struct dt_req *dt_req_start(struct kunit *test, int (*fn)(void *arg),
+				   void *arg)
+{
+	struct task_struct *task;
+	struct dt_req *req;
+
+	req = kunit_kzalloc(test, sizeof(*req), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, req);
+
+	req->ctx = test->priv;
+	req->fn = fn;
+	req->arg = arg;
+	init_completion(&req->done);
+
+	task = kthread_run(dt_req_thread, req, "dpll-test");
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, task);
+	KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, dt_req_release,
+							req), 0);
+
+	return req;
+}
+
+/*
+ * Consumes the completion, so it must not be waited for again. Assert it before
+ * reading what the request wrote.
+ */
+static bool dt_done(struct dt_req *req, unsigned long timeout)
+{
+	if (!req->finished && wait_for_completion_timeout(&req->done, timeout))
+		req->finished = true;
+
+	return req->finished;
+}
+
+/* Blocks a request of @dom in a callback, which holds the domain lock */
+static struct dt_req *dt_block_in(struct kunit *test, struct dt_dom *dom,
+				  int (*fn)(void *arg), void *arg)
+{
+	struct dt_req *req;
+
+	WRITE_ONCE(dom->block, true);
+	req = dt_req_start(test, fn, arg);
+	KUNIT_ASSERT_TRUE(test, wait_for_completion_timeout(&dom->entered,
+							    DT_TIMEOUT));
+
+	return req;
+}
+
+/* Build a set request and pass it to the handlers genetlink would call */
+static int dt_pin_set_fn(void *arg)
+{
+	struct dt_pin_set_args *args = arg;
+	struct nlattr *attrs[DPLL_A_PIN_MAX + 1] = {};
+	struct genl_info info = {};
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+	struct nlattr *nest;
+	int i, ret;
+
+	if (args->ctx)
+		WRITE_ONCE(args->ctx->set_task, current);
+
+	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+	ret = -EMSGSIZE;
+	nlh = nlmsg_put(skb, 0, 0, NLMSG_MIN_TYPE, GENL_HDRLEN, 0);
+	if (!nlh || nla_put_u32(skb, DPLL_A_PIN_ID, args->pin->id))
+		goto out;
+	for (i = 0; i < args->num; i++) {
+		const struct dt_nest *n = &args->nests[i];
+
+		nest = nla_nest_start(skb, n->type);
+		if (!nest ||
+		    nla_put_u32(skb, DPLL_A_PIN_PARENT_ID, n->parent_id) ||
+		    nla_put_u32(skb, n->attr, n->val))
+			goto out;
+		nla_nest_end(skb, nest);
+	}
+	nlmsg_end(skb, nlh);
+
+	info.genlhdr = nlmsg_data(nlh);
+	info.attrs = attrs;
+	attrs[DPLL_A_PIN_ID] = nla_find(genlmsg_data(info.genlhdr),
+					genlmsg_len(info.genlhdr),
+					DPLL_A_PIN_ID);
+
+	ret = dpll_pin_pre_doit(NULL, skb, &info);
+	if (!ret) {
+		ret = dpll_nl_pin_set_doit(skb, &info);
+		dpll_pin_post_doit(NULL, skb, &info);
+	}
+out:
+	nlmsg_free(skb);
+
+	return ret;
+}
+
+static int dt_mode_set_fn(void *arg)
+{
+	struct dt_mode_set_args *args = arg;
+	struct nlattr *attrs[DPLL_A_MAX + 1] = {};
+	struct genl_info info = {};
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+	int ret;
+
+	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+	ret = -EMSGSIZE;
+	nlh = nlmsg_put(skb, 0, 0, NLMSG_MIN_TYPE, GENL_HDRLEN, 0);
+	if (!nlh || nla_put_u32(skb, DPLL_A_ID, args->dpll->id) ||
+	    nla_put_u32(skb, DPLL_A_MODE, args->mode))
+		goto out;
+	nlmsg_end(skb, nlh);
+
+	info.genlhdr = nlmsg_data(nlh);
+	info.attrs = attrs;
+	attrs[DPLL_A_ID] = nla_find(genlmsg_data(info.genlhdr),
+				    genlmsg_len(info.genlhdr), DPLL_A_ID);
+
+	ret = dpll_pre_doit(NULL, skb, &info);
+	if (!ret) {
+		ret = dpll_nl_device_set_doit(skb, &info);
+		dpll_post_doit(NULL, skb, &info);
+	}
+out:
+	nlmsg_free(skb);
+
+	return ret;
+}
+
+/* Runs a request that should not block in a kthread and returns its result */
+static int dt_run(struct kunit *test, int (*fn)(void *arg), void *arg)
+{
+	struct dt_req *req = dt_req_start(test, fn, arg);
+
+	KUNIT_ASSERT_TRUE(test, dt_done(req, DT_TIMEOUT));
+
+	return req->ret;
+}
+
+static struct dt_pin_set_args *dt_pin_set_args_alloc(struct kunit *test,
+						     struct dpll_pin *pin,
+						     int num)
+{
+	struct dt_pin_set_args *args;
+
+	args = kunit_kzalloc(test, sizeof(*args), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, args);
+	args->pin = pin;
+	args->num = num;
+
+	return args;
+}
+
+static struct dt_pin_set_args *dt_pin_set_args_one(struct kunit *test,
+						   struct dpll_pin *pin,
+						   int type, u32 parent_id,
+						   int attr, u32 val)
+{
+	struct dt_pin_set_args *args = dt_pin_set_args_alloc(test, pin, 1);
+
+	args->nests[0].type = type;
+	args->nests[0].parent_id = parent_id;
+	args->nests[0].attr = attr;
+	args->nests[0].val = val;
+
+	return args;
+}
+
+static int dt_pin_set_one(struct kunit *test, struct dpll_pin *pin, int type,
+			  u32 parent_id, int attr, u32 val)
+{
+	return dt_run(test, dt_pin_set_fn,
+		      dt_pin_set_args_one(test, pin, type, parent_id, attr,
+					  val));
+}
+
+static void dt_expect_events(struct kunit *test, void * const *expected,
+			     int num)
+{
+	struct dt_ctx *ctx = test->priv;
+	int i;
+
+	KUNIT_ASSERT_EQ(test, ctx->num_events, num);
+	for (i = 0; i < num; i++)
+		KUNIT_EXPECT_PTR_EQ(test, ctx->events[i], expected[i]);
+	KUNIT_EXPECT_EQ(test, atomic_read(&ctx->bad_ntf), 0);
+}
+
+/* Objects with the same (module, clock_id) share a refcounted domain */
+static void dt_test_domain_sharing(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	struct dpll_domain *a = ctx->a0.dpll->domain;
+	struct dpll_domain *b = ctx->b0.dpll->domain;
+
+	KUNIT_EXPECT_PTR_EQ(test, ctx->a1.dpll->domain, a);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->p.pin->domain, a);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->bp.pin->domain, b);
+	KUNIT_EXPECT_PTR_NE(test, a, b);
+
+	KUNIT_EXPECT_EQ(test, refcount_read(&a->refcount), 4);
+	KUNIT_EXPECT_EQ(test, refcount_read(&b->refcount), 2);
+
+	KUNIT_ASSERT_EQ(test, dt_extra_pin_get(&ctx->extra), 0);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->extra.pin->domain, b);
+	KUNIT_EXPECT_EQ(test, refcount_read(&b->refcount), 3);
+	dpll_pin_put(ctx->extra.pin, &ctx->extra.tracker);
+	KUNIT_EXPECT_EQ(test, refcount_read(&b->refcount), 2);
+}
+
+/* The same clock_id used by another module means another domain */
+static void dt_test_domain_per_module(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	static char other_module;
+	struct dpll_device *dpll;
+	dpll_tracker tracker;
+
+	/* Never registered, so the core does not dereference the module */
+	dpll = dpll_device_get(DT_CLOCK_ID_A, 0, (struct module *)&other_module,
+			       &tracker);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dpll);
+	KUNIT_EXPECT_PTR_NE(test, dpll, ctx->a0.dpll);
+	KUNIT_EXPECT_PTR_NE(test, dpll->domain, ctx->a0.dpll->domain);
+	dpll_device_put(dpll, &tracker);
+}
+
+/* A pin on a dpll of another domain stays in its domain */
+static void dt_test_foreign_registration_domain(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	struct dpll_domain *e, *f;
+
+	dt_foreign_pin_add(test);
+	e = ctx->e0.dpll->domain;
+	f = ctx->f0.dpll->domain;
+
+	KUNIT_EXPECT_PTR_NE(test, e, f);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->x.pin->domain, e);
+	KUNIT_EXPECT_EQ(test, refcount_read(&e->refcount), 2);
+	KUNIT_EXPECT_EQ(test, refcount_read(&f->refcount), 1);
+}
+
+/* A pin under a parent pin of another domain stays in its domain */
+static void dt_test_on_pin_registration_domain(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	struct dpll_domain *e, *g;
+
+	dt_on_pin_add(test);
+	e = ctx->e0.dpll->domain;
+	g = ctx->c.pin->domain;
+
+	KUNIT_EXPECT_PTR_NE(test, e, g);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->m.pin->domain, e);
+	KUNIT_EXPECT_EQ(test, refcount_read(&e->refcount), 2);
+	KUNIT_EXPECT_EQ(test, refcount_read(&g->refcount), 1);
+}
+
+/* A callback blocked in one domain does not stop other domains */
+static void dt_test_domains_run_in_parallel(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	struct dt_req *blocked, *other;
+
+	blocked = dt_block_in(test, &ctx->dom_a, dt_device_ntf, &ctx->a0);
+
+	other = dt_req_start(test, dt_device_ntf, &ctx->b0);
+	KUNIT_EXPECT_TRUE(test, dt_done(other, DT_TIMEOUT));
+	KUNIT_EXPECT_FALSE(test, completion_done(&blocked->done));
+	KUNIT_EXPECT_EQ(test, atomic_read(&ctx->peak_total), 2);
+
+	dt_unblock(ctx);
+	KUNIT_EXPECT_TRUE(test, dt_done(blocked, DT_TIMEOUT));
+}
+
+/* Callbacks of one domain are serialized, across its devices too */
+static void dt_test_domain_is_serialized(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	struct dt_req *first, *second;
+
+	first = dt_block_in(test, &ctx->dom_a, dt_device_ntf, &ctx->a0);
+
+	second = dt_req_start(test, dt_device_ntf, &ctx->a1);
+	KUNIT_EXPECT_FALSE(test, dt_done(second, DT_SHORT_TIMEOUT));
+
+	dt_unblock(ctx);
+	KUNIT_EXPECT_TRUE(test, dt_done(first, DT_TIMEOUT));
+	KUNIT_EXPECT_TRUE(test, dt_done(second, DT_TIMEOUT));
+	KUNIT_EXPECT_EQ(test, atomic_read(&ctx->dom_a.peak), 1);
+}
+
+/* Callbacks of all refs of a pin run under the domain of the pin alone */
+static void dt_test_foreign_get_locks(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+
+	if (!IS_ENABLED(CONFIG_LOCKDEP) || !debug_locks)
+		kunit_skip(test, "needs lockdep");
+
+	dt_foreign_pin_add(test);
+
+	KUNIT_EXPECT_EQ(test, dt_run(test, dt_pin_ntf, &ctx->x), 0);
+	KUNIT_EXPECT_EQ(test, atomic_read(&ctx->wrong_lock), 0);
+}
+
+/* Setters of a foreign ref run under the domain of the pin alone */
+static void dt_test_foreign_set_locks(struct kunit *test)
+{
+	static const struct {
+		int attr;
+		u32 val;
+	} sets[] = {
+		{ DPLL_A_PIN_STATE, DPLL_PIN_STATE_CONNECTED },
+		{ DPLL_A_PIN_PRIO, 7 },
+		{ DPLL_A_PIN_DIRECTION, DPLL_PIN_DIRECTION_OUTPUT },
+	};
+	struct dt_ctx *ctx = test->priv;
+	int i;
+
+	dt_foreign_pin_add(test);
+
+	for (i = 0; i < ARRAY_SIZE(sets); i++) {
+		int ret;
+
+		ctx->num_events = 0;
+		ctx->record = true;
+		ret = dt_pin_set_one(test, ctx->x.pin, DPLL_A_PIN_PARENT_DEVICE,
+				     ctx->f0.dpll->id, sets[i].attr,
+				     sets[i].val);
+		ctx->record = false;
+
+		KUNIT_EXPECT_EQ(test, ret, 0);
+		dt_expect_events(test, (void *[]){ ctx->x.pin }, 1);
+	}
+	KUNIT_EXPECT_EQ(test, ctx->reg_x_f0.state, DPLL_PIN_STATE_CONNECTED);
+	KUNIT_EXPECT_EQ(test, ctx->reg_x_f0.prio, 7);
+	KUNIT_EXPECT_EQ(test, ctx->reg_x_f0.direction,
+			DPLL_PIN_DIRECTION_OUTPUT);
+	KUNIT_EXPECT_EQ(test, ctx->reg_x_e0.prio, 0);
+}
+
+/* The callback of a foreign ref can notify objects of the domain of the pin */
+static void dt_test_foreign_callback_notifies_pin(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	int ret;
+
+	dt_foreign_pin_add(test);
+	ctx->reg_x_f0.notify = &ctx->x;
+
+	ctx->record = true;
+	ret = dt_pin_set_one(test, ctx->x.pin, DPLL_A_PIN_PARENT_DEVICE,
+			     ctx->f0.dpll->id, DPLL_A_PIN_STATE,
+			     DPLL_PIN_STATE_CONNECTED);
+	ctx->record = false;
+
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	dt_expect_events(test, (void *[]){ ctx->x.pin, ctx->x.pin }, 2);
+}
+
+/* Without an owner the pin-level attributes come from the foreign ref */
+static void dt_test_pin_without_owner(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+
+	dt_dev_add(test, &ctx->f0, &ctx->dom_f, DT_CLOCK_ID_F, 0,
+		   DPLL_TYPE_PPS);
+	dt_fwnode_pin_add(test, &ctx->x, DT_CLOCK_ID_E, DT_PIN_IDX_X);
+	dt_reg_add(test, &ctx->reg_x_f0, &ctx->dom_f, &ctx->x, &ctx->f0, NULL);
+	KUNIT_ASSERT_PTR_NE(test, ctx->x.pin->domain, ctx->f0.dpll->domain);
+	ctx->reg_x_f0.forbid = ctx->f0.dpll->domain;
+
+	atomic_set(&ctx->reg_x_f0.freq_gets, 0);
+	KUNIT_EXPECT_EQ(test, dt_run(test, dt_pin_ntf, &ctx->x), 0);
+	KUNIT_EXPECT_GT(test, atomic_read(&ctx->reg_x_f0.freq_gets), 0);
+	KUNIT_EXPECT_EQ(test, atomic_read(&ctx->wrong_lock), 0);
+}
+
+/* A stuck callback of a foreign ref stops only the domain of the pin */
+static void dt_test_stuck_foreign_callback(struct kunit *test)
+{
+	struct dt_req *blocked, *owner, *foreign, *other;
+	struct dt_ctx *ctx = test->priv;
+
+	dt_foreign_pin_add(test);
+
+	WRITE_ONCE(ctx->reg_x_f0.block_prio_set, true);
+	blocked = dt_req_start(test, dt_pin_set_fn,
+			       dt_pin_set_args_one(test, ctx->x.pin,
+						   DPLL_A_PIN_PARENT_DEVICE,
+						   ctx->f0.dpll->id,
+						   DPLL_A_PIN_PRIO, 7));
+	KUNIT_ASSERT_TRUE(test, wait_for_completion_timeout(&ctx->dom_f.entered,
+							    DT_TIMEOUT));
+
+	owner = dt_req_start(test, dt_device_ntf, &ctx->e0);
+	KUNIT_EXPECT_FALSE(test, dt_done(owner, DT_SHORT_TIMEOUT));
+	foreign = dt_req_start(test, dt_device_ntf, &ctx->f0);
+	KUNIT_EXPECT_TRUE(test, dt_done(foreign, DT_TIMEOUT));
+	other = dt_req_start(test, dt_device_ntf, &ctx->b0);
+	KUNIT_EXPECT_TRUE(test, dt_done(other, DT_TIMEOUT));
+
+	dt_unblock(ctx);
+	KUNIT_EXPECT_TRUE(test, dt_done(blocked, DT_TIMEOUT));
+	KUNIT_EXPECT_TRUE(test, dt_done(owner, DT_TIMEOUT));
+}
+
+/* A stuck device callback does not stop a foreign pin registered with it */
+static void dt_test_stuck_foreign_device(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	struct dt_req *blocked, *pin;
+
+	dt_foreign_pin_add(test);
+
+	WRITE_ONCE(ctx->f0.block, true);
+	blocked = dt_req_start(test, dt_device_ntf, &ctx->f0);
+	KUNIT_ASSERT_TRUE(test, wait_for_completion_timeout(&ctx->dom_f.entered,
+							    DT_TIMEOUT));
+
+	pin = dt_req_start(test, dt_pin_ntf, &ctx->x);
+	KUNIT_EXPECT_TRUE(test, dt_done(pin, DT_TIMEOUT));
+
+	dt_unblock(ctx);
+	KUNIT_EXPECT_TRUE(test, dt_done(blocked, DT_TIMEOUT));
+}
+
+/* A stuck owner stops requests for its pins, but not the foreign device */
+static void dt_test_stuck_owner_domain(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	struct dt_req *blocked, *foreign, *pin;
+
+	dt_foreign_pin_add(test);
+
+	blocked = dt_block_in(test, &ctx->dom_e, dt_device_ntf, &ctx->e0);
+
+	foreign = dt_req_start(test, dt_device_ntf, &ctx->f0);
+	KUNIT_EXPECT_TRUE(test, dt_done(foreign, DT_TIMEOUT));
+	pin = dt_req_start(test, dt_pin_ntf, &ctx->x);
+	KUNIT_EXPECT_FALSE(test, dt_done(pin, DT_SHORT_TIMEOUT));
+
+	dt_unblock(ctx);
+	KUNIT_EXPECT_TRUE(test, dt_done(blocked, DT_TIMEOUT));
+	KUNIT_EXPECT_TRUE(test, dt_done(pin, DT_TIMEOUT));
+}
+
+/* A pin and its parent pin of another domain do not block each other */
+static void dt_test_on_pin_domains_are_independent(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	struct dt_req *blocked, *other;
+
+	dt_on_pin_add(test);
+
+	blocked = dt_block_in(test, &ctx->dom_e, dt_device_ntf, &ctx->e0);
+	other = dt_req_start(test, dt_pin_ntf, &ctx->c);
+	KUNIT_EXPECT_TRUE(test, dt_done(other, DT_TIMEOUT));
+	dt_unblock(ctx);
+	KUNIT_EXPECT_TRUE(test, dt_done(blocked, DT_TIMEOUT));
+
+	blocked = dt_block_in(test, &ctx->dom_g, dt_pin_ntf, &ctx->c);
+	other = dt_req_start(test, dt_pin_ntf, &ctx->m);
+	KUNIT_EXPECT_TRUE(test, dt_done(other, DT_TIMEOUT));
+	dt_unblock(ctx);
+	KUNIT_EXPECT_TRUE(test, dt_done(blocked, DT_TIMEOUT));
+}
+
+/* Registrations in both directions between two domains are fine */
+static void dt_test_registrations_both_directions(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+
+	dt_foreign_pin_add(test);
+	dt_fwnode_pin_add(test, &ctx->y, DT_CLOCK_ID_F, DT_PIN_IDX_Y);
+	dt_reg_add(test, &ctx->reg_y_f0, &ctx->dom_f, &ctx->y, &ctx->f0, NULL);
+	dt_reg_add(test, &ctx->reg_y_e0, &ctx->dom_e, &ctx->y, &ctx->e0, NULL);
+	ctx->reg_y_f0.forbid = ctx->e0.dpll->domain;
+	ctx->reg_y_e0.forbid = ctx->e0.dpll->domain;
+
+	KUNIT_EXPECT_EQ(test, dt_run(test, dt_pin_ntf, &ctx->x), 0);
+	KUNIT_EXPECT_EQ(test, dt_run(test, dt_pin_ntf, &ctx->y), 0);
+}
+
+/* A topology change waits until running requests finish */
+static void dt_test_topology_change_waits(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	struct dt_req *blocked, *writer;
+
+	blocked = dt_block_in(test, &ctx->dom_a, dt_device_ntf, &ctx->a0);
+
+	/* Added before the request starts, so it runs after waiting for it */
+	KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, dt_pin_release,
+							&ctx->extra), 0);
+	writer = dt_req_start(test, dt_extra_pin_get, &ctx->extra);
+	KUNIT_EXPECT_FALSE(test, dt_done(writer, DT_SHORT_TIMEOUT));
+
+	dt_unblock(ctx);
+	KUNIT_EXPECT_TRUE(test, dt_done(blocked, DT_TIMEOUT));
+	KUNIT_ASSERT_TRUE(test, dt_done(writer, DT_TIMEOUT));
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, ctx->extra.pin);
+}
+
+/* Other requests about a pin cannot see a set of two attributes half done */
+static void dt_check_request_atomic(struct kunit *test, struct dt_pin *pin,
+				    struct dt_dev *dev0, struct dt_reg *reg0,
+				    struct dt_dev *dev1, struct dt_reg *reg1)
+{
+	struct dt_pin_set_args *args = dt_pin_set_args_alloc(test, pin->pin, 2);
+	struct dt_ctx *ctx = test->priv;
+	struct dt_req *set, *other;
+	int i;
+
+	args->ctx = ctx;
+	for (i = 0; i < args->num; i++) {
+		args->nests[i].type = DPLL_A_PIN_PARENT_DEVICE;
+		args->nests[i].parent_id = i ? dev1->dpll->id : dev0->dpll->id;
+		args->nests[i].attr = DPLL_A_PIN_PRIO;
+		args->nests[i].val = 11;
+	}
+
+	ctx->atomic_regs[0] = reg0;
+	ctx->atomic_regs[1] = reg1;
+
+	/* The other request starts to wait before the first attribute is set */
+	WRITE_ONCE(reg0->block_prio_set, true);
+	set = dt_req_start(test, dt_pin_set_fn, args);
+	KUNIT_ASSERT_TRUE(test, wait_for_completion_timeout(&reg0->dom->entered,
+							    DT_TIMEOUT));
+	other = dt_req_start(test, dt_pin_ntf, pin);
+	KUNIT_EXPECT_FALSE(test, dt_done(other, DT_SHORT_TIMEOUT));
+
+	dt_unblock(ctx);
+	KUNIT_ASSERT_TRUE(test, dt_done(set, DT_TIMEOUT));
+	KUNIT_ASSERT_TRUE(test, dt_done(other, DT_TIMEOUT));
+	KUNIT_EXPECT_EQ(test, set->ret, 0);
+	KUNIT_EXPECT_EQ(test, reg0->prio, 11);
+	KUNIT_EXPECT_EQ(test, reg1->prio, 11);
+	KUNIT_EXPECT_EQ(test, atomic_read(&ctx->torn), 0);
+}
+
+static void dt_test_request_is_atomic(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+
+	dt_check_request_atomic(test, &ctx->p, &ctx->a0, &ctx->reg_p_a0,
+				&ctx->a1, &ctx->reg_p_a1);
+}
+
+static void dt_test_spanning_request_is_atomic(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+
+	dt_foreign_pin_add(test);
+	dt_check_request_atomic(test, &ctx->x, &ctx->e0, &ctx->reg_x_e0,
+				&ctx->f0, &ctx->reg_x_f0);
+}
+
+/* Notifications from a callback come first, under the domain lock */
+static void dt_test_ntf_order(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	int ret;
+
+	ctx->record = true;
+	ret = dt_pin_set_one(test, ctx->p.pin, DPLL_A_PIN_PARENT_DEVICE,
+			     ctx->a0.dpll->id, DPLL_A_PIN_STATE,
+			     DPLL_PIN_STATE_CONNECTED);
+	ctx->record = false;
+
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	KUNIT_EXPECT_EQ(test, ctx->reg_p_a0.state, DPLL_PIN_STATE_CONNECTED);
+	dt_expect_events(test, (void *[]){ ctx->s.pin, ctx->p.pin }, 2);
+}
+
+/* A notification from a callback that fails is still sent */
+static void dt_test_ntf_on_failure(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	int ret;
+
+	ctx->fail_after_ntf = true;
+	ctx->record = true;
+	ret = dt_pin_set_one(test, ctx->p.pin, DPLL_A_PIN_PARENT_DEVICE,
+			     ctx->a0.dpll->id, DPLL_A_PIN_STATE,
+			     DPLL_PIN_STATE_CONNECTED);
+	ctx->record = false;
+
+	KUNIT_EXPECT_EQ(test, ret, -EIO);
+	dt_expect_events(test, (void *[]){ ctx->s.pin }, 1);
+}
+
+/* A set on a pin under a parent of another domain needs only its domain */
+static void dt_test_set_on_pin(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	int ret;
+
+	dt_on_pin_add(test);
+
+	ctx->record = true;
+	ret = dt_pin_set_one(test, ctx->c.pin, DPLL_A_PIN_PARENT_PIN,
+			     ctx->m.pin->id, DPLL_A_PIN_STATE,
+			     DPLL_PIN_STATE_CONNECTED);
+	ctx->record = false;
+
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	KUNIT_EXPECT_EQ(test, ctx->reg_c_m.state, DPLL_PIN_STATE_CONNECTED);
+	dt_expect_events(test, (void *[]){ ctx->c.pin }, 1);
+}
+
+/* A device callback notifies a sibling device of its domain */
+static void dt_test_device_set_ntf(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	struct dt_mode_set_args *args;
+	int ret;
+
+	args = kunit_kzalloc(test, sizeof(*args), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, args);
+	args->dpll = ctx->a0.dpll;
+	args->mode = DPLL_MODE_MANUAL;
+
+	ctx->record = true;
+	ret = dt_run(test, dt_mode_set_fn, args);
+	ctx->record = false;
+
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	KUNIT_EXPECT_EQ(test, ctx->a0.mode, DPLL_MODE_MANUAL);
+	dt_expect_events(test, (void *[]){ ctx->a1.dpll }, 1);
+}
+
+/* A change of a reference sync pair notifies under the domain of the pin */
+static void dt_test_ref_sync_pair(struct kunit *test)
+{
+	struct dt_ctx *ctx = test->priv;
+	int ret;
+
+	ctx->record = true;
+	ret = dpll_pin_ref_sync_pair_add(ctx->p.pin, ctx->s.pin);
+	ctx->record = false;
+
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	dt_expect_events(test, (void *[]){ ctx->p.pin }, 1);
+}
+
+static struct kunit_case dt_cases[] = {
+	KUNIT_CASE(dt_test_domain_sharing),
+	KUNIT_CASE(dt_test_domain_per_module),
+	KUNIT_CASE(dt_test_foreign_registration_domain),
+	KUNIT_CASE(dt_test_on_pin_registration_domain),
+	KUNIT_CASE(dt_test_domains_run_in_parallel),
+	KUNIT_CASE(dt_test_domain_is_serialized),
+	KUNIT_CASE(dt_test_foreign_get_locks),
+	KUNIT_CASE(dt_test_foreign_set_locks),
+	KUNIT_CASE(dt_test_foreign_callback_notifies_pin),
+	KUNIT_CASE(dt_test_pin_without_owner),
+	KUNIT_CASE(dt_test_stuck_foreign_callback),
+	KUNIT_CASE(dt_test_stuck_foreign_device),
+	KUNIT_CASE(dt_test_stuck_owner_domain),
+	KUNIT_CASE(dt_test_on_pin_domains_are_independent),
+	KUNIT_CASE(dt_test_registrations_both_directions),
+	KUNIT_CASE(dt_test_topology_change_waits),
+	KUNIT_CASE(dt_test_request_is_atomic),
+	KUNIT_CASE(dt_test_spanning_request_is_atomic),
+	KUNIT_CASE(dt_test_ntf_order),
+	KUNIT_CASE(dt_test_ntf_on_failure),
+	KUNIT_CASE(dt_test_set_on_pin),
+	KUNIT_CASE(dt_test_device_set_ntf),
+	KUNIT_CASE(dt_test_ref_sync_pair),
+	{}
+};
+
+static struct kunit_suite dt_suite = {
+	.name = "dpll_core",
+	.init = dt_init,
+	.exit = dt_exit,
+	.test_cases = dt_cases,
+};
+
+kunit_test_suite(dt_suite);
+
+MODULE_DESCRIPTION("KUnit tests for locking in the DPLL core");
+MODULE_LICENSE("GPL");
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index 9d8f5d1ce97248..223cd1de905c1a 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -6,6 +6,7 @@
  *  Copyright (c) 2023 Intel and affiliates
  *
  */
+#include <kunit/visibility.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
@@ -1836,6 +1837,7 @@ int dpll_nl_pin_set_doit(struct sk_buff *skb, struct genl_info *info)
 
 	return dpll_pin_set_from_nlattr(pin, info);
 }
+EXPORT_SYMBOL_IF_KUNIT(dpll_nl_pin_set_doit);
 
 static struct dpll_device *
 dpll_device_find(u64 clock_id, struct nlattr *mod_name_attr,
@@ -2010,6 +2012,7 @@ int dpll_nl_device_set_doit(struct sk_buff *skb, struct genl_info *info)
 
 	return dpll_set_from_nlattr(dpll, info);
 }
+EXPORT_SYMBOL_IF_KUNIT(dpll_nl_device_set_doit);
 
 int dpll_nl_device_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 {
@@ -2070,6 +2073,7 @@ int dpll_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 	up_read(&dpll_lock);
 	return -ENODEV;
 }
+EXPORT_SYMBOL_IF_KUNIT(dpll_pre_doit);
 
 void dpll_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 		    struct genl_info *info)
@@ -2079,6 +2083,7 @@ void dpll_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 	mutex_unlock(&dpll->domain->lock);
 	up_read(&dpll_lock);
 }
+EXPORT_SYMBOL_IF_KUNIT(dpll_post_doit);
 
 int
 dpll_lock_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
@@ -2122,6 +2127,7 @@ int dpll_pin_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 	up_read(&dpll_lock);
 	return ret;
 }
+EXPORT_SYMBOL_IF_KUNIT(dpll_pin_pre_doit);
 
 void dpll_pin_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 			struct genl_info *info)
@@ -2131,3 +2137,4 @@ void dpll_pin_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 	mutex_unlock(&pin->domain->lock);
 	up_read(&dpll_lock);
 }
+EXPORT_SYMBOL_IF_KUNIT(dpll_pin_post_doit);
-- 
2.56.0

[PATCH net-next v2 6/9] dpll: reject measured_freq_get for foreign and on-pin registrations

[Ivan Vecera]

A message about a pin with .measured_freq_get calls .freq_monitor_get of
a dpll of the pin. For a pin of another domain, this device callback
would run outside the domain of the dpll, so dpll_pin_register() now
rejects such ops.

dpll_pin_on_pin_register() checks nothing about them. The message would
call .freq_monitor_get with no NULL check, on a dpll of the parent pin
that can belong to another driver, so reject them there as well.

No in-tree driver passes such ops. Add KUnit tests: the ops work on a dpll
of the domain of the pin, and are rejected on a dpll of another domain and
under a parent pin.

Reviewed-by: Petr Oros <poros@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
No changes since v1
---
 drivers/dpll/dpll_core.c       |   6 +-
 drivers/dpll/dpll_core_kunit.c | 112 +++++++++++++++++++++++++++++++--
 2 files changed, 111 insertions(+), 7 deletions(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index 2b1dfe2929647e..a73abb0772cc6b 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -958,7 +958,8 @@ dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
 
 	dev_ops = dpll_device_ops(dpll);
 	if (WARN_ON(ops->measured_freq_get &&
-		    (!dev_ops || !dev_ops->freq_monitor_get ||
+		    (pin->domain != dpll->domain || !dev_ops ||
+		     !dev_ops->freq_monitor_get ||
 		     !dev_ops->freq_monitor_set))) {
 		ret = -EINVAL;
 		goto out_unlock;
@@ -1065,7 +1066,8 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, struct dpll_pin *pin,
 
 	if (WARN_ON(!ops) ||
 	    WARN_ON(!ops->state_on_pin_get) ||
-	    WARN_ON(!ops->direction_get))
+	    WARN_ON(!ops->direction_get) ||
+	    WARN_ON(ops->measured_freq_get))
 		return -EINVAL;
 
 	down_write(&dpll_lock);
diff --git a/drivers/dpll/dpll_core_kunit.c b/drivers/dpll/dpll_core_kunit.c
index 21f941a5fdec4e..d575eed160700f 100644
--- a/drivers/dpll/dpll_core_kunit.c
+++ b/drivers/dpll/dpll_core_kunit.c
@@ -87,6 +87,7 @@ struct dt_reg {
 	struct dt_dev *dev;
 	struct dt_pin *parent;
 	struct dt_pin *notify;
+	const struct dpll_pin_ops *ops;
 	struct dpll_domain *forbid;
 	enum dpll_pin_state state;
 	enum dpll_pin_direction direction;
@@ -276,11 +277,33 @@ static int dt_lock_status_get(const struct dpll_device *dpll, void *priv,
 	return 0;
 }
 
+static int dt_freq_monitor_set(const struct dpll_device *dpll, void *priv,
+			       enum dpll_feature_state state,
+			       struct netlink_ext_ack *extack)
+{
+	return 0;
+}
+
+static int dt_freq_monitor_get(const struct dpll_device *dpll, void *priv,
+			       enum dpll_feature_state *state,
+			       struct netlink_ext_ack *extack)
+{
+	struct dt_dev *dev = priv;
+
+	dt_dev_enter(dev, dpll);
+	*state = DPLL_FEATURE_STATE_ENABLE;
+	dt_leave(dev->dom);
+
+	return 0;
+}
+
 static const struct dpll_device_ops dt_dev_ops = {
 	.mode_get = dt_mode_get,
 	.mode_set = dt_mode_set,
 	.supported_modes_get = dt_supported_modes_get,
 	.lock_status_get = dt_lock_status_get,
+	.freq_monitor_set = dt_freq_monitor_set,
+	.freq_monitor_get = dt_freq_monitor_get,
 };
 
 static int dt_frequency_get(const struct dpll_pin *pin, void *pin_priv,
@@ -433,6 +456,21 @@ static int dt_ref_sync_get(const struct dpll_pin *pin, void *pin_priv,
 	return 0;
 }
 
+static int dt_measured_freq_get(const struct dpll_pin *pin, void *pin_priv,
+				const struct dpll_device *dpll, void *dpll_priv,
+				u64 *measured_freq,
+				struct netlink_ext_ack *extack)
+{
+	struct dt_reg *reg = pin_priv;
+
+	dt_pin_enter(reg, pin);
+	atomic_inc(&reg->freq_gets);
+	*measured_freq = 1;
+	dt_leave(reg->dom);
+
+	return 0;
+}
+
 static const struct dpll_pin_ops dt_pin_ops = {
 	.frequency_get = dt_frequency_get,
 	.state_on_dpll_get = dt_state_on_dpll_get,
@@ -446,6 +484,13 @@ static const struct dpll_pin_ops dt_pin_ops = {
 	.ref_sync_get = dt_ref_sync_get,
 };
 
+static const struct dpll_pin_ops dt_pin_ops_measured = {
+	.measured_freq_get = dt_measured_freq_get,
+	.state_on_dpll_get = dt_state_on_dpll_get,
+	.state_on_pin_get = dt_state_on_pin_get,
+	.direction_get = dt_direction_get,
+};
+
 static const struct dpll_pin_properties dt_prop = {
 	.type = DPLL_PIN_TYPE_EXT,
 	.capabilities = DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE |
@@ -570,9 +615,9 @@ static void dt_reg_release(void *data)
 
 	if (reg->parent)
 		dpll_pin_on_pin_unregister(reg->parent->pin, reg->pin->pin,
-					   &dt_pin_ops, reg);
+					   reg->ops, reg);
 	else
-		dpll_pin_unregister(reg->dev->dpll, reg->pin->pin, &dt_pin_ops,
+		dpll_pin_unregister(reg->dev->dpll, reg->pin->pin, reg->ops,
 				    reg);
 }
 
@@ -588,12 +633,14 @@ static void dt_reg_add(struct kunit *test, struct dt_reg *reg,
 	reg->parent = parent;
 	reg->state = DPLL_PIN_STATE_SELECTABLE;
 	reg->direction = DPLL_PIN_DIRECTION_INPUT;
+	if (!reg->ops)
+		reg->ops = &dt_pin_ops;
 
 	if (parent)
-		ret = dpll_pin_on_pin_register(parent->pin, pin->pin,
-					       &dt_pin_ops, reg);
+		ret = dpll_pin_on_pin_register(parent->pin, pin->pin, reg->ops,
+					       reg);
 	else
-		ret = dpll_pin_register(dev->dpll, pin->pin, &dt_pin_ops, reg);
+		ret = dpll_pin_register(dev->dpll, pin->pin, reg->ops, reg);
 	KUNIT_ASSERT_EQ(test, ret, 0);
 	KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, dt_reg_release,
 							reg), 0);
@@ -1438,6 +1485,59 @@ static void dt_test_ref_sync_pair(struct kunit *test)
 	dt_expect_events(test, (void *[]){ ctx->p.pin }, 1);
 }
 
+/* A pin with a measured frequency must be in the domain of its dpll */
+static void dt_test_measured_freq_foreign(struct kunit *test)
+{
+	struct kunit_suppressed_warning *w;
+	struct dt_ctx *ctx = test->priv;
+	struct dt_reg *reg;
+	int ret;
+
+	dt_dev_add(test, &ctx->e0, &ctx->dom_e, DT_CLOCK_ID_E, 0,
+		   DPLL_TYPE_PPS);
+	dt_dev_add(test, &ctx->f0, &ctx->dom_f, DT_CLOCK_ID_F, 0,
+		   DPLL_TYPE_PPS);
+	dt_fwnode_pin_add(test, &ctx->x, DT_CLOCK_ID_E, DT_PIN_IDX_X);
+
+	reg = &ctx->reg_x_e0;
+	reg->ops = &dt_pin_ops_measured;
+	dt_reg_add(test, reg, &ctx->dom_e, &ctx->x, &ctx->e0, NULL);
+	atomic_set(&reg->freq_gets, 0);
+	KUNIT_EXPECT_EQ(test, dt_run(test, dt_pin_ntf, &ctx->x), 0);
+	KUNIT_EXPECT_GT(test, atomic_read(&reg->freq_gets), 0);
+
+	w = kunit_start_suppress_warning(test);
+	ret = dpll_pin_register(ctx->f0.dpll, ctx->x.pin, reg->ops, reg);
+	kunit_end_suppress_warning(test, w);
+
+	KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+	if (!ret)
+		dpll_pin_unregister(ctx->f0.dpll, ctx->x.pin, reg->ops, reg);
+}
+
+/* A pin under a parent pin cannot have a measured frequency */
+static void dt_test_measured_freq_on_pin(struct kunit *test)
+{
+	struct kunit_suppressed_warning *w;
+	struct dt_ctx *ctx = test->priv;
+	struct dt_reg *reg;
+	int ret;
+
+	dt_on_pin_add(test);
+	dt_pin_add(test, &ctx->y, DT_CLOCK_ID_G, DT_PIN_IDX_Y, &dt_prop);
+	reg = &ctx->reg_c_m;
+
+	w = kunit_start_suppress_warning(test);
+	ret = dpll_pin_on_pin_register(ctx->m.pin, ctx->y.pin,
+				       &dt_pin_ops_measured, reg);
+	kunit_end_suppress_warning(test, w);
+
+	KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+	if (!ret)
+		dpll_pin_on_pin_unregister(ctx->m.pin, ctx->y.pin,
+					   &dt_pin_ops_measured, reg);
+}
+
 static struct kunit_case dt_cases[] = {
 	KUNIT_CASE(dt_test_domain_sharing),
 	KUNIT_CASE(dt_test_domain_per_module),
@@ -1462,6 +1562,8 @@ static struct kunit_case dt_cases[] = {
 	KUNIT_CASE(dt_test_set_on_pin),
 	KUNIT_CASE(dt_test_device_set_ntf),
 	KUNIT_CASE(dt_test_ref_sync_pair),
+	KUNIT_CASE(dt_test_measured_freq_foreign),
+	KUNIT_CASE(dt_test_measured_freq_on_pin),
 	{}
 };
 
-- 
2.56.0

[PATCH net-next v2 7/9] dpll: zl3073x: update locking comments

[Ivan Vecera]

Callbacks are serialized by the DPLL core domain lock instead of dpll_lock.
All channels of a chip belong to one domain, so what the comments rely on
still holds.

Reviewed-by: Petr Oros <poros@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
No changes since v1
---
 drivers/dpll/zl3073x/dpll.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index 1cdadb8e46fb2d..6e5e2bb41f55d6 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -349,9 +349,10 @@ zl3073x_dpll_input_pin_ref_sync_set(const struct dpll_pin *dpll_pin,
 	if (rc)
 		goto unlock;
 
-	/* All code paths accessing per-channel reference priorities are
-	 * serialized by the subsystem dpll_lock, so it is safe to release
-	 * our lock here before iterating over the other channels.
+	/* All callbacks accessing per-channel reference priorities are
+	 * serialized by the DPLL core domain lock shared by all channels of
+	 * the chip, so it is safe to release our lock here before iterating
+	 * over the other channels.
 	 */
 	mutex_unlock(&zldpll->lock);
 
@@ -2401,7 +2402,7 @@ zl3073x_dpll_changes_check(struct zl3073x_dpll *zldpll)
 	mutex_unlock(&zldpll->lock);
 
 	/* Send notifications outside the lock to avoid ABBA deadlock
-	 * with dpll_lock taken by notification functions.
+	 * with the DPLL core domain lock taken by notification functions.
 	 */
 	if (dev_changed)
 		dpll_device_change_ntf(zldpll->dpll_dev);
-- 
2.56.0

[PATCH net-next v2 8/9] ice: dpll: update locking comments

[Ivan Vecera]

Callbacks run under the DPLL core domain locks instead of dpll_lock. On
E825 the TXC DPLL, the TSPLL DPLL and the other objects of a PF are in
three domains, and callbacks of pins of an external DPLL run under the
domain of that DPLL, so only pf->dplls.lock serializes them with each
other.

Reviewed-by: Petr Oros <poros@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
Changes in v2:
- Described the three domains of an E825 PF (Sashiko)
---
 drivers/net/ethernet/intel/ice/ice_dpll.c | 4 ++--
 drivers/net/ethernet/intel/ice/ice_dpll.h | 5 +++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index 0877827a6521cc..5d0a52d86f03e8 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -1178,8 +1178,8 @@ ice_dpll_input_state_get(const struct dpll_pin *pin, void *pin_priv,
  * peer via dpll netlink learn about the update.
  *
  * Context: Called from dpll_pin_ops callbacks after pf->dplls.lock is
- *          released.  Uses __dpll_pin_change_ntf() because dpll_lock is
- *          still held by the dpll netlink layer.
+ *          released.  Uses __dpll_pin_change_ntf() because the domain lock
+ *          is still held by the dpll core.
  */
 static void ice_dpll_sw_pin_notify_peer(struct ice_dplls *d,
 					struct ice_dpll_pin *changed)
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h
index bce0bea9a664bc..c522c5fcbcce52 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
@@ -164,6 +164,11 @@ struct ice_dpll {
  *     tx_refclks usage map), acquire pf->dplls.lock first, then
  *     ctrl_pf->dplls.lock. Skip the second acquire when pf == ctrl_pf
  *     to avoid recursive locking.
+ *   - The DPLL core serializes callbacks per (module, clock_id) domain. On
+ *     E825 the TXC DPLL, the TSPLL DPLL and the other objects of a PF, such
+ *     as the RCLK pin, use three different clock_ids, and callbacks of the
+ *     SYNCE and TIME_REF pins of an external DPLL run under the domain of
+ *     that DPLL, so only @lock serializes all of them with each other.
  *   - @txclk_notify_rwsem is held for read across
  *     ice_txclk_update_and_notify(), including the out-of-lock
  *     dpll_*_change_ntf() calls. ice_dpll_deinit() takes the write side
-- 
2.56.0

[PATCH net-next v2 9/9] dpll: document locking rules

[Ivan Vecera]

Describe the lock levels of the subsystem, which domain a callback runs
under and the rules device drivers have to follow.

Reviewed-by: Petr Oros <poros@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
Changes in v2:
- Added a rule for reference sync pairs (Sashiko)
- Rules 1 and 3 also forbid waiting for a work item that notifies
  (Sashiko)
---
 Documentation/driver-api/dpll.rst | 48 +++++++++++++++++++++++++++++++
 drivers/dpll/dpll_core.c          | 13 +++++++++
 2 files changed, 61 insertions(+)

diff --git a/Documentation/driver-api/dpll.rst b/Documentation/driver-api/dpll.rst
index 7c117ae37cc1bc..9cc6a912c58913 100644
--- a/Documentation/driver-api/dpll.rst
+++ b/Documentation/driver-api/dpll.rst
@@ -675,6 +675,54 @@ In the error path we have to rewind every allocation in the reverse order:
 
 More complex example can be found in Intel's ICE driver or nVidia's mlx5 driver.
 
+Locking
+=======
+
+The subsystem uses two levels of locks, always taken in this order:
+
+- ``dpll_lock`` protects the topology of dpll devices and pins and their
+  lifetime. Functions changing the topology, for example dpll_pin_register(),
+  take it for write and send the create and delete notifications under it.
+  Netlink requests, each part of a dump and change notifications take it for
+  read for their whole duration.
+- A domain lock serializes driver callbacks. All dpll devices and pins created
+  with the same module and ``clock_id`` belong to one domain, in practice to
+  one chip or card.
+
+A netlink request addressing a device or a pin by its ID holds the domain lock
+of that object from start to end. A notification holds it while its message is
+built and sent, so messages of one object are sent in the order they were
+built. Only one domain lock is held at a time, so domains have no lock order,
+and callbacks of different domains can run concurrently.
+
+Callbacks of a dpll device run under the domain of the device. Callbacks of a
+pin run under the domain of the pin, whichever driver registered them:
+
+- A driver registering a pin of another driver with its dpll device has these
+  callbacks serialized with the driver that created the pin, not with the other
+  callbacks of its device, so it has to protect the state they share by itself.
+  A callback stuck in such a registration stops the domain of the pin.
+- The parent pin of a pin registered with dpll_pin_on_pin_register() and its
+  dpll devices can belong to another driver, so the callback must not
+  dereference their private data.
+- The frequency monitor belongs to the dpll device, so only callbacks of a pin
+  of the same domain registered with dpll_pin_register() can include
+  ``.measured_freq_get``.
+- The reference sync callbacks of a pin get the private data of the other pin
+  of the pair, so both pins of a pair added by dpll_pin_ref_sync_pair_add()
+  have to belong to one domain.
+
+Device drivers have to follow these rules:
+
+- A callback must not call dpll_device_change_ntf(), dpll_pin_change_ntf() or
+  any function changing the topology, nor wait for a work item that calls them.
+- A callback notifies about objects of the domain it runs under with
+  __dpll_device_change_ntf() or __dpll_pin_change_ntf(). Other objects have to
+  be notified from a context that holds no domain lock, for example a work item.
+- A driver must not call dpll_device_change_ntf(), dpll_pin_change_ntf() or any
+  function changing the topology, nor wait for a work item that calls them,
+  while holding a lock that its callbacks take.
+
 SyncE enablement
 ================
 For SyncE enablement it is required to allow control over dpll device
diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index a73abb0772cc6b..9a0843bba184af 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -932,6 +932,10 @@ __dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
  * @ops: ops for a dpll pin ops
  * @priv: pointer to private information of owner
  *
+ * The callbacks run under the domain of @pin. If @pin belongs to another domain
+ * than @dpll, they are not serialized with the callbacks of @dpll and @ops must
+ * not include .measured_freq_get.
+ *
  * Context: Acquires dpll_lock and domain locks
  * Return:
  * * 0 on success
@@ -1049,6 +1053,11 @@ EXPORT_SYMBOL_GPL(dpll_pin_unregister);
  * Register a pin with a parent pin, create references between them and
  * between newly registered pin and dplls connected with a parent pin.
  *
+ * The callbacks run under the domain of @pin. The parent pin and its dplls can
+ * belong to another driver, so @ops must not dereference their private data.
+ * The frequency monitor belongs to a dpll, so @ops must not include
+ * .measured_freq_get.
+ *
  * Context: Acquires dpll_lock and domain locks
  * Return:
  * * 0 on success
@@ -1137,6 +1146,10 @@ EXPORT_SYMBOL_GPL(dpll_pin_on_pin_unregister);
  *
  * Once pins are paired, the user-space configuration of reference sync pair
  * is possible.
+ *
+ * The reference sync callbacks of each pin get the private data of the other
+ * one, so both pins must belong to one domain.
+ *
  * Context: Acquires dpll_lock and domain locks
  * Return:
  * * 0 on success
-- 
2.56.0