On Thu, Feb 16, 2023 at 01:02:40PM -0600, Daniel Kaehn wrote: > On Sat, Feb 11, 2023 at 6:10 AM Andy Shevchenko > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > On Fri, Feb 10, 2023 at 04:36:38PM -0600, Danny Kaehn wrote: ... > > > + dev->gc.fwnode = device_get_named_child_node(&hdev->dev, "gpio"); > > > > Using like this bumps a reference count IIRC, so one need to drop it after use. > > But please double check this. > > > > Thanks for bringing this up -- I should have explicitly called this > out as something I was looking for feedback on, as I was unsure on > this. > > I noticed that many of the users of device_get_named_child_node didn't > explicitly call fwnode_handle_put, and was unsure about the mechanics > of when this is needed. > > The underlying call to device_get_named_child_node for an of_node is > of_fwnode_get_named_child_node, which does call > for_each_available_child_of_node and returns from within the loop, so > I _think_ you're right that the return will have its refcount > incremented (of_get_next_available_child calls of_node_get on the next > node, and doesn't call put until the next iteration). > > However, I also noticed that many other functions in > drivers/base/property.c contain a message like the following in their > header comment: > "The caller is responsible for calling fwnode_handle_put() for the > returned node." > and this isn't present for device_get_named_child_node, which is > defined in that same file, which made me suspicious that this is > somehow done elsewhere internally (although I should know better than > to trust documenting comments :) ). Good catch! > I'll wait a while longer to see if someone with a better grasp than me > on dynamic DT/firmware weighs in, otherwise, I'll assume I'll need to > call fwnode_handle_put both on error paths in _probe as well as in > _remove, since that appeared to be the case with the DT-specific > of_get_child_by_name path. Patch to update the kernel documentation has been just sent. -- With Best Regards, Andy Shevchenko
