Re: [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Hans de Goede <hdegoede@redhat.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@linux.ie>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"platform-driver-x86@vger.kernel.org" 
	<platform-driver-x86@vger.kernel.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Subject: Re: [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries
Date: Wed, 5 May 2021 12:17:14 +0300	[thread overview]
Message-ID: <CAHp75VcfkcaVAu2-8-5he7PN=W_tRHiHAgXYn04gRnLehDVsyQ@mail.gmail.com> (raw)
In-Reply-To: <ee230261-423d-0e2f-16b0-852d264afa2b@redhat.com>

On Wed, May 5, 2021 at 12:07 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 5/4/21 9:52 AM, Andy Shevchenko wrote:
> > On Monday, May 3, 2021, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@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?

> So I think we need to first make up our mind here how we want this all to
> work and then fix the actual implementation and docs before fixing callers.

We have already issues, so I prefer not to wait for a documentation
update, because for old kernels it will still be an issue.

In any case most of my fixes are against LED subsystem (drivers) and
they are valid due to use in the OF environment.

-- 
With Best Regards,
Andy Shevchenko