On Wed, Nov 27, 2024 at 09:39:34PM -0800, Dmitry Torokhov wrote: > The documentation to various API calls that locate children for a given > fwnode (such as fwnode_get_next_available_child_node() or > device_get_next_child_node()) states that the reference to the node > passed in "child" argument is dropped unconditionally, however the > change that added checks for the main node to be NULL or error pointer > broke this promise. This commit message doesn't explain a use case. Hence it might be just a documentation issue, please elaborate. > Add missing fwnode_handle_put() calls to restore the documented > behavior. ... While at it, please fix the kernel-doc (missing Return section). > { > + if (IS_ERR_OR_NULL(fwnode) || Unneeded check as fwnode_has_op() has it already. > + !fwnode_has_op(fwnode, get_next_child_node)) { > + fwnode_handle_put(child); > + return NULL; > + } > return fwnode_call_ptr_op(fwnode, get_next_child_node, child); Now it's useless to call the macro, you can simply take the direct call. > } ... > @@ struct fwnode_handle *device_get_next_child_node(const struct device *dev, > const struct fwnode_handle *fwnode = dev_fwnode(dev); > struct fwnode_handle *next; > - if (IS_ERR_OR_NULL(fwnode)) > + if (IS_ERR_OR_NULL(fwnode)) { > + fwnode_handle_put(child); > return NULL; > + } > /* Try to find a child in primary fwnode */ > next = fwnode_get_next_child_node(fwnode, child); So, why not just moving the original check (w/o dropping the reference) here? Wouldn't it have the same effect w/o explicit call to the fwnode_handle_put()? -- With Best Regards, Andy Shevchenko