>From: Jakub Kicinski <kuba@xxxxxxxxxx> >Sent: Tuesday, June 13, 2023 1:45 AM > >On Fri, 9 Jun 2023 14:18:46 +0200 Arkadiusz Kubalewski wrote: >> + xa_for_each(xa_pins, i, ref) { >> + if (ref->pin != pin) >> + continue; >> + reg = dpll_pin_registration_find(ref, ops, priv); >> + if (reg) { >> + refcount_inc(&ref->refcount); >> + return 0; >> + } >> + ref_exists = true; >> + break; >> + } >> + >> + if (!ref_exists) { >> + ref = kzalloc(sizeof(*ref), GFP_KERNEL); >> + if (!ref) >> + return -ENOMEM; >> + ref->pin = pin; >> + INIT_LIST_HEAD(&ref->registration_list); >> + ret = xa_insert(xa_pins, pin->pin_idx, ref, GFP_KERNEL); >> + if (ret) { >> + kfree(ref); >> + return ret; >> + } >> + refcount_set(&ref->refcount, 1); >> + } >> + >> + reg = kzalloc(sizeof(*reg), GFP_KERNEL); > >Why do we have two structures - ref and reg? > Thank to Jiri and reg struct we solved a pin/dpll association with multiple device drivers.. I.e. for pin: struct dpll_pin_registration { struct list_head list; const struct dpll_pin_ops *ops; void *priv; }; struct dpll_pin_ref { union { struct dpll_device *dpll; struct dpll_pin *pin; }; struct list_head registration_list; refcount_t refcount; }; struct dpll_pin { u32 id; u32 pin_idx; u64 clock_id; struct module *module; struct xarray dpll_refs; struct xarray parent_refs; const struct dpll_pin_properties *prop; char *rclk_dev_name; refcount_t refcount; }; Basically, a pin or a device can be registered from multiple drivers, where each driver has own priv and ops. A single dpll_pin has references to dplls or pins (dpll_refs/parent_refs) it is connected with, and thanks to registration list single reference can have multiple drivers being attached with a particular dpll/pin. The same scheme is for a dpll_device struct and associated pins. >> + if (!reg) { >> + if (!ref_exists) >> + kfree(ref); > >ref has already been inserted into xa_pins > True, seems like a bug, will fix it. Thank you, Arkadiusz >> + return -ENOMEM;