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

On Thu, Feb 16, 2023 at 01:02:40PM -0600, Daniel Kaehn wrote:

On Sat, Feb 11, 2023 at 6:10 AM Andy Shevchenko
[off-list ref] wrote:

quoted

On Fri, Feb 10, 2023 at 04:36:38PM -0600, Danny Kaehn wrote:

...

quoted

quoted

+     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