Received-SPF: pass (google.com: domain of linux-kernel+bounces-191023-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Mon, 27 May 2024 15:57:17 +0100
From: Jonathan Cameron <jic23@kernel.org>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Javier Carrasco <javier.carrasco.cruz@gmail.com>, Greg Kroah-Hartman
 <gregkh@linuxfoundation.org>, "Rafael J. Wysocki" <rafael@kernel.org>, Andy
 Shevchenko <andriy.shevchenko@linux.intel.com>, Daniel Scally
 <djrscally@gmail.com>, Heikki Krogerus <heikki.krogerus@linux.intel.com>,
 Sakari Ailus <sakari.ailus@linux.intel.com>, Jean Delvare
 <jdelvare@suse.com>, Guenter Roeck <linux@roeck-us.net>, Antoniu Miclaus
 <antoniu.miclaus@analog.com>, linux-acpi@vger.kernel.org,
 linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org, Rob Herring
 <robh@kernel.org>, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 3/3] hwmon: (ltc2992) Use
 fwnode_for_each_available_child_node_scoped()
Message-ID: <20240527155717.58292509@jic23-huawei>
In-Reply-To: <ZlSY8tjYm5g9bEJ_@surfacebook.localdomain>
References: <20240523-fwnode_for_each_available_child_node_scoped-v2-0-701f3a03f2fb@gmail.com>
	<20240523-fwnode_for_each_available_child_node_scoped-v2-3-701f3a03f2fb@gmail.com>
	<20240526144851.493dd3f2@jic23-huawei>
	<ZlSY8tjYm5g9bEJ_@surfacebook.localdomain>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Mon, 27 May 2024 17:30:10 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> Sun, May 26, 2024 at 02:48:51PM +0100, Jonathan Cameron kirjoitti:
> > On Thu, 23 May 2024 17:47:16 +0200
> > Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> >   
> > > The scoped version of the fwnode_for_each_available_child_node() macro
> > > automates object recfount decrement, avoiding possible memory leaks
> > > in new error paths inside the loop like it happened when
> > > commit '10b029020487 ("hwmon: (ltc2992) Avoid division by zero")'
> > > was added.
> > > 
> > > The new macro removes the need to manually call fwnode_handle_put() in
> > > the existing error paths and in any future addition. It also removes the
> > > need for the current child node declaration as well, as it is internally
> > > declared.
> > > 
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>  
> > 
> > This looks like another instances of the lack of clarify about 
> > what device_for_each_child_node[_scoped]() guarantees about node availability.
> > On DT it guarantees the node is available as ultimately calls
> > of_get_next_available_child()
> > 
> > On ACPI it doesn't (I think).
> > For swnode, there isn't an obvious concept of available.
> > 
> > It would be much better if we reached some agreement on this and
> > hence could avoid using the fwnode variants just to get the _available_ form
> > as done here.  
> 
> > Or just add the device_for_each_available_child_node[_scoped]()
> > and call that in almost all cases.  
> 
> device_for_each*() _implies_ availability. You need to talk to Rob about all
> this. The design of the device_for_each*() was exactly done in accordance with
> his suggestions...
> 

Does it imply that for ACPI? I can't find a query of _STA in the callbacks
(which is there for the for fwnode_*available calls.
Mind you it wouldn't be the first time I've missed something in the ACPI parsing
code, so maybe it is there indirectly.

I know from previous discussions that the DT version was intentional, but
I'm nervous that the same assumptions don't apply to ACPI.

> > In generic code, do we ever want to walk unavailable child nodes?  
> 
> ...which are most likely like your question here, i.e. why we ever need to
> traverse over unavailable nodes.
> 

Jonathan