[PATCH net-next 00/10] dpll: do not serialize driver callbacks under global dpll_lock

[PATCH net-next 00/10] 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. Patch 2 adds a lock for the
registers they share. Without it a test lost updates of them.

Patch 5 is the only patch that lets callbacks run concurrently. Patch 6
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.

Ivan Vecera (10):
  ice: dpll: notify TSPLL lock status from the periodic work only
  ice: dpll: serialize changes of the SyncE CGU registers
  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            |   44 +
 drivers/dpll/.kunitconfig                    |    5 +
 drivers/dpll/Kconfig                         |   13 +
 drivers/dpll/Makefile                        |    1 +
 drivers/dpll/dpll_core.c                     |  187 ++-
 drivers/dpll/dpll_core.h                     |   34 +-
 drivers/dpll/dpll_core_kunit.c               | 1573 ++++++++++++++++++
 drivers/dpll/dpll_netlink.c                  |  117 +-
 drivers/dpll/zl3073x/dpll.c                  |    9 +-
 drivers/net/ethernet/intel/ice/ice_adapter.c |    2 +
 drivers/net/ethernet/intel/ice/ice_adapter.h |    4 +
 drivers/net/ethernet/intel/ice/ice_dpll.c    |   29 +-
 drivers/net/ethernet/intel/ice/ice_dpll.h    |    7 +
 drivers/net/ethernet/intel/ice/ice_ptp.c     |    2 +
 drivers/net/ethernet/intel/ice/ice_tspll.c   |    4 +-
 15 files changed, 1915 insertions(+), 116 deletions(-)
 create mode 100644 drivers/dpll/.kunitconfig
 create mode 100644 drivers/dpll/dpll_core_kunit.c


base-commit: 6584acd42820478910a771efb28a355a707e9cb0
-- 
2.56.0

[PATCH net-next 01/10] 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.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/ethernet/intel/ice/ice_dpll.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index ade1fb3dd06743..bdbcf7af296eb4 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;
 
@@ -2947,15 +2946,12 @@ ice_dpll_tspll_state_on_dpll_set(const struct dpll_pin *pin, void *pin_priv,
 		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;
-		}
+		/* ice_dpll_periodic_work_e825() compares the state with
+		 * prev_dpll_state and sends the notification.
+		 */
+		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 +3368,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 02/10] ice: dpll: serialize changes of the SyncE CGU registers

[Ivan Vecera]

On dual complex E825 only complex 0 has a functional CGU and the second
complex reaches it through cgu_peer, so PFs of both complexes share the
CGU registers R10 and R11. They read, modify and write them when the
state of an RCLK pin changes, and ice_ptp_link_change() rewrites R10 when
a link changes, under the pf->dplls.lock of their own PF only.

ice_ptp_link_change() does not hold dpll_lock, so it can already overwrite
an update of another PF. RCLK callbacks are serialized by dpll_lock, but
the DPLL core is going to serialize callbacks per (module, clock_id), and
each complex has its own DSN and so its own clock_id. With that change, a
test toggling RCLK pins of both complexes lost an update.

Add a mutex to struct ice_adapter, which all E825C PFs share, and hold it
around these sequences.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/ethernet/intel/ice/ice_adapter.c |  2 ++
 drivers/net/ethernet/intel/ice/ice_adapter.h |  4 ++++
 drivers/net/ethernet/intel/ice/ice_dpll.c    | 11 ++++++-----
 drivers/net/ethernet/intel/ice/ice_dpll.h    |  4 ++++
 drivers/net/ethernet/intel/ice/ice_ptp.c     |  2 ++
 drivers/net/ethernet/intel/ice/ice_tspll.c   |  4 ++--
 6 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c b/drivers/net/ethernet/intel/ice/ice_adapter.c
index 536923b6ae979d..aa2d8addae5bef 100644
--- a/drivers/net/ethernet/intel/ice/ice_adapter.c
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
@@ -72,6 +72,7 @@ static struct ice_adapter *ice_adapter_new(struct pci_dev *pdev)
 	spin_lock_init(&adapter->txq_ctx_lock);
 	for (int i = 0; i < ARRAY_SIZE(adapter->cpi_phy_lock); i++)
 		mutex_init(&adapter->cpi_phy_lock[i]);
+	mutex_init(&adapter->cgu_synce_lock);
 	refcount_set(&adapter->refcount, 1);
 
 	spin_lock_init(&adapter->ports.lock);
@@ -85,6 +86,7 @@ static void ice_adapter_free(struct ice_adapter *adapter)
 	WARN_ON(!list_empty(&adapter->ports.list));
 	for (int i = 0; i < ARRAY_SIZE(adapter->cpi_phy_lock); i++)
 		mutex_destroy(&adapter->cpi_phy_lock[i]);
+	mutex_destroy(&adapter->cgu_synce_lock);
 
 	cleanup_srcu_struct(&adapter->ports.srcu);
 
diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.h b/drivers/net/ethernet/intel/ice/ice_adapter.h
index c61bd645e460de..fd5585005d91ba 100644
--- a/drivers/net/ethernet/intel/ice/ice_adapter.h
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.h
@@ -41,6 +41,8 @@ struct ice_port_list {
  * @txq_ctx_lock: Spinlock protecting access to the GLCOMM_QTX_CNTX_CTL register
  * @cpi_phy_lock: Per-PHY mutex serializing CPI REQ/ACK transactions.
  *               Index 0 = PHY0, index 1 = PHY1. Used on E825C devices.
+ * @cgu_synce_lock: Mutex serializing changes of the CGU registers R10 and R11,
+ *                  which all PFs reach. Used on E825C devices.
  * @ctrl_pf: Control PF of the adapter
  * @ports: Ports list
  * @index: 64-bit index cached for collision detection on 32bit systems
@@ -53,6 +55,8 @@ struct ice_adapter {
 	spinlock_t txq_ctx_lock;
 	/* Serialize CPI REQ/ACK transactions per PHY (E825C only) */
 	struct mutex cpi_phy_lock[ICE_E825_MAX_PHYS];
+	/* Serialize changes of the SyncE CGU registers (E825C only) */
+	struct mutex cgu_synce_lock;
 
 	struct ice_pf *ctrl_pf;
 	struct ice_port_list ports;
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index bdbcf7af296eb4..d6302409a2dacf 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -2027,7 +2027,7 @@ ice_dpll_phase_offset_get(const struct dpll_pin *pin, void *pin_priv,
  *
  * DPLL subsystem callback. Set proper signals to recover clock from port.
  *
- * Context: Called under pf->dplls.lock
+ * Context: Called under pf->dplls.lock, acquires adapter->cgu_synce_lock
  * Return:
  * * 0 - success
  * * negative - error
@@ -2035,14 +2035,15 @@ ice_dpll_phase_offset_get(const struct dpll_pin *pin, void *pin_priv,
 static int ice_dpll_synce_update_e825c(struct ice_hw *hw, bool ena,
 				       u32 port_num, enum ice_synce_clk output)
 {
+	struct ice_pf *pf = hw->back;
 	int err;
 
+	mutex_lock(&pf->adapter->cgu_synce_lock);
 	/* configure the mux to deliver proper signal to DPLL from the MUX */
 	err = ice_tspll_cfg_bypass_mux_e825c(hw, ena, port_num, output);
-	if (err)
-		return err;
-
-	err = ice_tspll_cfg_synce_ethdiv_e825c(hw, output);
+	if (!err)
+		err = ice_tspll_cfg_synce_ethdiv_e825c(hw, output);
+	mutex_unlock(&pf->adapter->cgu_synce_lock);
 	if (err)
 		return err;
 
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h
index bce0bea9a664bc..4ba76e2e2e0b01 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
@@ -158,6 +158,7 @@ struct ice_dpll {
  *     txclk_notify_rwsem (read)
  *       -> pf->dplls.lock
  *         -> ctrl_pf->dplls.lock
+ *         -> adapter->cgu_synce_lock
  *
  *   - @lock serializes all DPLL state mutations on this PF. When the
  *     controlling PF's lock must also be taken (e.g. updating the shared
@@ -169,6 +170,9 @@ struct ice_dpll {
  *     dpll_*_change_ntf() calls. ice_dpll_deinit() takes the write side
  *     standalone (not nested under any other lock) to drain in-flight
  *     readers before pins and the TXC DPLL device are freed.
+ *   - adapter->cgu_synce_lock serializes changes of the CGU registers R10 and
+ *     R11. All PFs of a dual complex E825 reach one CGU, so @lock does not
+ *     cover them.
  */
 struct ice_dplls {
 	struct kthread_worker *kworker;
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 3df08714510da5..4d4b6aaaf30c29 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1395,7 +1395,9 @@ void ice_ptp_link_change(struct ice_pf *pf, bool linkup)
 				break;
 			}
 
+			mutex_lock(&pf->adapter->cgu_synce_lock);
 			err = ice_tspll_cfg_synce_ethdiv_e825c(hw, clk_pin);
+			mutex_unlock(&pf->adapter->cgu_synce_lock);
 			if (active && err) {
 				dev_err_once(ice_pf_to_dev(pf),
 					     "Failed to configure SyncE ETH divider for pin %d, err %d\n",
diff --git a/drivers/net/ethernet/intel/ice/ice_tspll.c b/drivers/net/ethernet/intel/ice/ice_tspll.c
index 85bacb7c3a7903..b99e02cfe559bd 100644
--- a/drivers/net/ethernet/intel/ice/ice_tspll.c
+++ b/drivers/net/ethernet/intel/ice/ice_tspll.c
@@ -792,7 +792,7 @@ int ice_tspll_bypass_mux_active_e825c(struct ice_hw *hw, u8 port, bool *active,
  *
  * Set reference clock source and output clock selection.
  *
- * Context: Called under pf->dplls.lock
+ * Context: Called under pf->dplls.lock and adapter->cgu_synce_lock
  * Return:
  * * 0 - success
  * * negative - error
@@ -898,7 +898,7 @@ static int ice_tspll_get_div_e825c(u16 link_speed, unsigned int *divider)
  *
  * Set the correct CGU divider for RCLKA or RCLKB.
  *
- * Context: Called under pf->dplls.lock
+ * Context: Called under pf->dplls.lock and adapter->cgu_synce_lock
  * Return:
  * * 0 - success
  * * negative - error
-- 
2.56.0

[PATCH net-next 03/10] 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().

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

No functional change.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/dpll_core.c | 61 ++++++++++++++++++++++++++++++++++++++++
 drivers/dpll/dpll_core.h | 27 ++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index 8389983f425695..d39c4905172f2d 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,50 @@ 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);
+	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);
+	}
+
+	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);
+		mutex_destroy(&domain->lock);
+		kfree(domain);
+	}
+}
+
 static void dpll_device_tracker_alloc(struct dpll_device *dpll,
 				      dpll_tracker *tracker)
 {
@@ -120,6 +165,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 +205,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 +413,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 +427,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 +718,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 +737,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);
diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
index da8a369556ed11..6705e81bc11de9 100644
--- a/drivers/dpll/dpll_core.h
+++ b/drivers/dpll/dpll_core.h
@@ -9,18 +9,42 @@
 
 #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"
 
 #define DPLL_REGISTERED		XA_MARK_1
 
+/**
+ * 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:		lock 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.
+ **/
+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;
+};
+
 /**
  * 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 +56,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 +71,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 +87,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 04/10] dpll: take domain locks around driver callbacks

[Ivan Vecera]

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.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/dpll_core.c    | 18 +++++----
 drivers/dpll/dpll_core.h    |  6 ++-
 drivers/dpll/dpll_netlink.c | 78 ++++++++++++++++++++++++++++---------
 3 files changed, 76 insertions(+), 26 deletions(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index d39c4905172f2d..dd09f528170994 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -520,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
@@ -579,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)
@@ -928,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
@@ -989,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);
 		}
 	}
 }
@@ -1016,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)
@@ -1042,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
@@ -1099,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,
@@ -1129,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
@@ -1142,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 6705e81bc11de9..630c5a1ca572c2 100644
--- a/drivers/dpll/dpll_core.h
+++ b/drivers/dpll/dpll_core.h
@@ -22,10 +22,14 @@
  * @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:		lock of 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;
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 05/10] 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.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/dpll_core.c    | 93 +++++++++++++++++--------------------
 drivers/dpll/dpll_core.h    |  3 +-
 drivers/dpll/dpll_netlink.c | 32 ++++++-------
 3 files changed, 61 insertions(+), 67 deletions(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index dd09f528170994..8eb74682343156 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);
@@ -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;
 }
@@ -1254,23 +1258,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 06/10] 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

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/.kunitconfig      |    5 +
 drivers/dpll/Kconfig           |   13 +
 drivers/dpll/Makefile          |    1 +
 drivers/dpll/dpll_core_kunit.c | 1471 ++++++++++++++++++++++++++++++++
 drivers/dpll/dpll_netlink.c    |    7 +
 5 files changed, 1497 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..a58800eda5ff51
--- /dev/null
+++ b/drivers/dpll/dpll_core_kunit.c
@@ -0,0 +1,1471 @@
+// 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/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/property.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);
+}
+
+static void dt_fwnode_release(void *data)
+{
+	fwnode_remove_software_node(data);
+}
+
+/* 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 = fwnode_create_software_node(NULL, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fwnode);
+	KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, dt_fwnode_release,
+							fwnode), 0);
+	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);
+}
+
+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,
+	.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 07/10] 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.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 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 8eb74682343156..84377f8ba16036 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 a58800eda5ff51..84c405f855d8cb 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);
@@ -1432,6 +1479,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),
@@ -1456,6 +1556,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 08/10] 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.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 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 09/10] ice: dpll: update locking comments

[Ivan Vecera]

Callbacks run under the DPLL core domain locks instead of dpll_lock. Note
that callbacks of pins of an external DPLL run under the domain of that
DPLL, so only pf->dplls.lock serializes them with the rest of the driver.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/ethernet/intel/ice/ice_dpll.c | 4 ++--
 drivers/net/ethernet/intel/ice/ice_dpll.h | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index d6302409a2dacf..07cbd4cf0160ca 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 4ba76e2e2e0b01..39e041a30090d0 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
@@ -165,6 +165,9 @@ 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.
+ *   - Callbacks of the SYNCE and TIME_REF pins of an external DPLL run under
+ *     the DPLL core domain lock of that DPLL, not of the TXC and TSPLL DPLLs,
+ *     so only @lock serializes them with the callbacks of these DPLLs.
  *   - @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 10/10] 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.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 Documentation/driver-api/dpll.rst | 44 +++++++++++++++++++++++++++++++
 drivers/dpll/dpll_core.c          |  9 +++++++
 2 files changed, 53 insertions(+)

diff --git a/Documentation/driver-api/dpll.rst b/Documentation/driver-api/dpll.rst
index 7c117ae37cc1bc..40d5037ff3bb0f 100644
--- a/Documentation/driver-api/dpll.rst
+++ b/Documentation/driver-api/dpll.rst
@@ -675,6 +675,50 @@ 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``.
+
+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.
+- 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 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 84377f8ba16036..6ad40fb9eee855 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
-- 
2.56.0