[Intel-gfx] [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries

[Sakari Ailus]

Hi Andy, Hans,

On Wed, May 05, 2021 at 12:17:14PM +0300, Andy Shevchenko wrote:
> On Wed, May 5, 2021 at 12:07 PM Hans de Goede <hdegoede at redhat.com> wrote:
> > On 5/4/21 9:52 AM, Andy Shevchenko wrote:
> > > On Monday, May 3, 2021, Hans de Goede <hdegoede at redhat.com <mailto:hdegoede at redhat.com>> wrote:
> 
> ...
> 
> > >     +               fwnode = device_get_next_child_node(kdev, fwnode);
> 
> > > Who is dropping reference counting on fwnode ?
> >
> > We are dealing with ACPI fwnode-s here and those are not ref-counted, they
> > are embedded inside a struct acpi_device and their lifetime is tied to
> > that struct. They should probably still be ref-counted (with the count
> > never dropping to 0) so that the generic fwnode functions behave the same
> > anywhere but atm the ACPI nodes are not refcounted, see: acpi_get_next_subnode()
> > in drivers/acpi/property.c which is the get_next_child_node() implementation
> > for ACPI fwnode-s.
> 
> Yes, ACPI currently is exceptional, but fwnode API is not.
> If you may guarantee that this case won't ever be outside of ACPI and
> even though if ACPI won't ever gain a reference counting for fwnodes,
> we can leave it as is.
> 
> > > I’m in the middle of a pile of fixes for fwnode refcounting when
> > > for_each_child or get_next_child is used. So, please double check you
> > > drop a reference.
> >
> > The kdoc comments on device_get_next_child_node() / fwnode_get_next_child_node()
> > do not mention anything about these functions returning a reference.
> 
> It's possible. I dunno if it had to be done earlier. Sakari?

The fwnode API has worked with references for a long time, looks like from
the very beginning of it, as in patch
8a0662d9ed2968e1186208336a8e1fab3fdfea63 .

If you're expecting an fwnode family of function returning another node,
then that function has to have taken a reference to that node before
returning it. Otherwise there's no guarantee that node is still there when
it is returned.

It may be this is not documented for every function doing so. That should
probably be added.

-- 
Kind regards,

Sakari Ailus