Re: [PATCH v5 3/3] HID: cp2112: Fwnode Support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux