>From: Ivan Vecera <[email protected]> >Sent: Tuesday, February 3, 2026 6:40 PM > >Refactor the reference counting mechanism for DPLL devices and pins to >improve consistency and prevent potential lifetime issues. > >Introduce internal helpers __dpll_{device,pin}_{hold,put}() to >centralize reference management. > >Update the internal XArray reference helpers (dpll_xa_ref_*) to >automatically grab a reference to the target object when it is added to >a list, and release it when removed. This ensures that objects linked >internally (e.g., pins referenced by parent pins) are properly kept >alive without relying on the caller to manually manage the count. > >Consequently, remove the now redundant manual `refcount_inc/dec` calls >in dpll_pin_on_pin_{,un}register()`, as ownership is now correctly handled >by the dpll_xa_ref_* functions. > >Additionally, ensure that dpll_device_{,un}register()` takes/releases >a reference to the device, ensuring the device object remains valid for >the duration of its registration. > >Reviewed-by: Aleksandr Loktionov <[email protected]>
LGTM, Reviewed-by: Arkadiusz Kubalewski <[email protected]> >Signed-off-by: Ivan Vecera <[email protected]> >--- > drivers/dpll/dpll_core.c | 74 +++++++++++++++++++++++++++------------- > 1 file changed, 50 insertions(+), 24 deletions(-) > >diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c >index 59081cf2c73ae..f6ab4f0cad84d 100644 >--- a/drivers/dpll/dpll_core.c >+++ b/drivers/dpll/dpll_core.c >@@ -83,6 +83,45 @@ void dpll_pin_notify(struct dpll_pin *pin, unsigned >long action) > call_dpll_notifiers(action, &info); > } > >+static void __dpll_device_hold(struct dpll_device *dpll) >+{ >+ refcount_inc(&dpll->refcount); >+} >+ >+static void __dpll_device_put(struct dpll_device *dpll) >+{ >+ if (refcount_dec_and_test(&dpll->refcount)) { >+ ASSERT_DPLL_NOT_REGISTERED(dpll); >+ WARN_ON_ONCE(!xa_empty(&dpll->pin_refs)); >+ xa_destroy(&dpll->pin_refs); >+ xa_erase(&dpll_device_xa, dpll->id); >+ WARN_ON(!list_empty(&dpll->registration_list)); >+ kfree(dpll); >+ } >+} >+ >+static void __dpll_pin_hold(struct dpll_pin *pin) >+{ >+ refcount_inc(&pin->refcount); >+} >+ >+static void dpll_pin_idx_free(u32 pin_idx); >+static void dpll_pin_prop_free(struct dpll_pin_properties *prop); >+ >+static void __dpll_pin_put(struct dpll_pin *pin) >+{ >+ if (refcount_dec_and_test(&pin->refcount)) { >+ xa_erase(&dpll_pin_xa, pin->id); >+ xa_destroy(&pin->dpll_refs); >+ xa_destroy(&pin->parent_refs); >+ xa_destroy(&pin->ref_sync_pins); >+ dpll_pin_prop_free(&pin->prop); >+ fwnode_handle_put(pin->fwnode); >+ dpll_pin_idx_free(pin->pin_idx); >+ kfree_rcu(pin, rcu); >+ } >+} >+ > struct dpll_device *dpll_device_get_by_id(int id) > { > if (xa_get_mark(&dpll_device_xa, id, DPLL_REGISTERED)) >@@ -152,6 +191,7 @@ dpll_xa_ref_pin_add(struct xarray *xa_pins, struct >dpll_pin *pin, > reg->ops = ops; > reg->priv = priv; > reg->cookie = cookie; >+ __dpll_pin_hold(pin); > if (ref_exists) > refcount_inc(&ref->refcount); > list_add_tail(®->list, &ref->registration_list); >@@ -174,6 +214,7 @@ static int dpll_xa_ref_pin_del(struct xarray *xa_pins, >struct dpll_pin *pin, > if (WARN_ON(!reg)) > return -EINVAL; > list_del(®->list); >+ __dpll_pin_put(pin); > kfree(reg); > if (refcount_dec_and_test(&ref->refcount)) { > xa_erase(xa_pins, i); >@@ -231,6 +272,7 @@ dpll_xa_ref_dpll_add(struct xarray *xa_dplls, struct >dpll_device *dpll, > reg->ops = ops; > reg->priv = priv; > reg->cookie = cookie; >+ __dpll_device_hold(dpll); > if (ref_exists) > refcount_inc(&ref->refcount); > list_add_tail(®->list, &ref->registration_list); >@@ -253,6 +295,7 @@ dpll_xa_ref_dpll_del(struct xarray *xa_dplls, struct >dpll_device *dpll, > if (WARN_ON(!reg)) > return; > list_del(®->list); >+ __dpll_device_put(dpll); > kfree(reg); > if (refcount_dec_and_test(&ref->refcount)) { > xa_erase(xa_dplls, i); >@@ -323,8 +366,8 @@ dpll_device_get(u64 clock_id, u32 device_idx, struct >module *module) > if (dpll->clock_id == clock_id && > dpll->device_idx == device_idx && > dpll->module == module) { >+ __dpll_device_hold(dpll); > ret = dpll; >- refcount_inc(&ret->refcount); > break; > } > } >@@ -347,14 +390,7 @@ EXPORT_SYMBOL_GPL(dpll_device_get); > void dpll_device_put(struct dpll_device *dpll) > { > mutex_lock(&dpll_lock); >- if (refcount_dec_and_test(&dpll->refcount)) { >- ASSERT_DPLL_NOT_REGISTERED(dpll); >- WARN_ON_ONCE(!xa_empty(&dpll->pin_refs)); >- xa_destroy(&dpll->pin_refs); >- xa_erase(&dpll_device_xa, dpll->id); >- WARN_ON(!list_empty(&dpll->registration_list)); >- kfree(dpll); >- } >+ __dpll_device_put(dpll); > mutex_unlock(&dpll_lock); > } > EXPORT_SYMBOL_GPL(dpll_device_put); >@@ -416,6 +452,7 @@ int dpll_device_register(struct dpll_device *dpll, >enum dpll_type type, > reg->ops = ops; > reg->priv = priv; > dpll->type = type; >+ __dpll_device_hold(dpll); > first_registration = list_empty(&dpll->registration_list); > list_add_tail(®->list, &dpll->registration_list); > if (!first_registration) { >@@ -455,6 +492,7 @@ void dpll_device_unregister(struct dpll_device *dpll, > return; > } > list_del(®->list); >+ __dpll_device_put(dpll); > kfree(reg); > > if (!list_empty(&dpll->registration_list)) { >@@ -666,8 +704,8 @@ dpll_pin_get(u64 clock_id, u32 pin_idx, struct module >*module, > if (pos->clock_id == clock_id && > pos->pin_idx == pin_idx && > pos->module == module) { >+ __dpll_pin_hold(pos); > ret = pos; >- refcount_inc(&ret->refcount); > break; > } > } >@@ -690,16 +728,7 @@ EXPORT_SYMBOL_GPL(dpll_pin_get); > void dpll_pin_put(struct dpll_pin *pin) > { > mutex_lock(&dpll_lock); >- if (refcount_dec_and_test(&pin->refcount)) { >- xa_erase(&dpll_pin_xa, pin->id); >- xa_destroy(&pin->dpll_refs); >- xa_destroy(&pin->parent_refs); >- xa_destroy(&pin->ref_sync_pins); >- dpll_pin_prop_free(&pin->prop); >- fwnode_handle_put(pin->fwnode); >- dpll_pin_idx_free(pin->pin_idx); >- kfree_rcu(pin, rcu); >- } >+ __dpll_pin_put(pin); > mutex_unlock(&dpll_lock); > } > EXPORT_SYMBOL_GPL(dpll_pin_put); >@@ -740,8 +769,8 @@ struct dpll_pin *fwnode_dpll_pin_find(struct >fwnode_handle *fwnode) > mutex_lock(&dpll_lock); > xa_for_each(&dpll_pin_xa, index, pin) { > if (pin->fwnode == fwnode) { >+ __dpll_pin_hold(pin); > ret = pin; >- refcount_inc(&ret->refcount); > break; > } > } >@@ -893,7 +922,6 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, >struct dpll_pin *pin, > ret = dpll_xa_ref_pin_add(&pin->parent_refs, parent, ops, priv, >pin); > if (ret) > goto unlock; >- refcount_inc(&pin->refcount); > xa_for_each(&parent->dpll_refs, i, ref) { > ret = __dpll_pin_register(ref->dpll, pin, ops, priv, parent); > if (ret) { >@@ -913,7 +941,6 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, >struct dpll_pin *pin, > parent); > dpll_pin_delete_ntf(pin); > } >- refcount_dec(&pin->refcount); > dpll_xa_ref_pin_del(&pin->parent_refs, parent, ops, priv, pin); > unlock: > mutex_unlock(&dpll_lock); >@@ -940,7 +967,6 @@ void dpll_pin_on_pin_unregister(struct dpll_pin >*parent, struct dpll_pin *pin, > mutex_lock(&dpll_lock); > dpll_pin_delete_ntf(pin); > dpll_xa_ref_pin_del(&pin->parent_refs, parent, ops, priv, pin); >- refcount_dec(&pin->refcount); > xa_for_each(&pin->dpll_refs, i, ref) > __dpll_pin_unregister(ref->dpll, pin, ops, priv, parent); > mutex_unlock(&dpll_lock); >-- >2.52.0
