Message-ID: <d7982b76-3da7-47ff-b2b2-f964610af1f7@gmail.com>
Date: Thu, 27 Feb 2025 10:01:49 +0200
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
 Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
 Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Daniel Scally <djrscally@...il.com>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 "Rafael J. Wysocki" <rafael@...nel.org>, Danilo Krummrich <dakr@...nel.org>,
 Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
 Chen-Yu Tsai <wens@...e.org>, Jernej Skrabec <jernej.skrabec@...il.com>,
 Samuel Holland <samuel@...lland.org>,
 Hugo Villeneuve <hvilleneuve@...onoff.com>, Nuno Sa <nuno.sa@...log.com>,
 David Lechner <dlechner@...libre.com>,
 Javier Carrasco <javier.carrasco.cruz@...il.com>,
 Guillaume Stols <gstols@...libre.com>,
 Olivier Moysan <olivier.moysan@...s.st.com>,
 Dumitru Ceclan <mitrutzceclan@...il.com>,
 Trevor Gamblin <tgamblin@...libre.com>,
 Matteo Martelli <matteomartelli3@...il.com>,
 Alisa-Dariana Roman <alisadariana@...il.com>,
 Ramona Alexandra Nechita <ramona.nechita@...log.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
 linux-renesas-soc@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-sunxi@...ts.linux.dev
Subject: Re: [PATCH v4 02/10] property: Add
 device_get_child_node_count_named()

On 26/02/2025 16:11, Andy Shevchenko wrote:
> On Wed, Feb 26, 2025 at 04:04:02PM +0200, Matti Vaittinen wrote:
>> On 25/02/2025 15:59, Andy Shevchenko wrote:
>>> On Tue, Feb 25, 2025 at 03:29:17PM +0200, Matti Vaittinen wrote:
>>>> On 25/02/2025 12:39, Andy Shevchenko wrote:
>>>>> On Tue, Feb 25, 2025 at 12:29:31PM +0200, Matti Vaittinen wrote:
>>>>>> On 25/02/2025 12:21, Andy Shevchenko wrote:
>>>>>>> On Tue, Feb 25, 2025 at 11:40:16AM +0200, Heikki Krogerus wrote:
> 
> ...
> 
>>>>>>>>
>>>>>>>> I did not check how many users are you proposing for this, but if
>>>>>>>> there's only one, then IMO this should not be a global function yet.
>>>>>>>> It just feels to special case to me. But let's see what the others
>>>>>>>> think.
>>>>>>>
>>>>>>> The problem is that if somebody hides it, we might potentially see
>>>>>>> a duplication in the future. So I _slightly_ prefer to publish and
>>>>>>> then drop that after a few cycles if no users appear.
>>>>>>
>>>>>> After taking a very quick grep I spotted one other existing place where we
>>>>>> might be able to do direct conversion to use this function.
>>>>>>
>>>>>> drivers/net/ethernet/freescale/gianfar.c
>>>>>>
>>>>>> That'd be 2 users.
>>>>>
>>>>> I haven't checked myself, I believe your judgement,
>>>>
>>>> I took a better look and you obviously shouldn't believe :) The gianfar used
>>>> of_node instead of the fwnode. So, it'd be a single caller at starters.
>>>
>>> ...which is the same as dev_of_node(), which means that you can use your
>>> function there.
>>
>> I'm unsure what you mean. The proposed function
>> device_get_child_node_count_named() takes device pointer. I don't see how
>> dev_of_node() helps converting node to device?
> 
> dev_of_node() takes the device pointer and dev_fwnode() takes that as well,
> it means that there is no difference which one to use OF-centric or fwnode

The proposed device_get_child_node_count_named() takes a device pointer. 
I don't see how dev_of_node() helps if there is just of_node and no 
device pointer available in the calling code. (Well, as I wrote below, I 
could alter the gianfar code by dropping the gfar_of_group_count(), so 
that I have the device pointer in caller). Anyways, I don't see how 
dev_of_node() should help unless you're proposing I add a 
of_get_child_node_count_named() or somesuch - which I don't think makes 
sense.

> API in this particular case. Just make sure that the function (and there
> is also a second loop AFAICS) takes struct device *dev instead of struct
> device_node *np as a parameter.

I think I lost the track here :)

>> I think I could actually kill the whole gfar_of_group_count() function and
>> replace it with a direct call to the device_get_child_node_count_named() -
>> but I am not at all convinced that'd be worth including the property.h to a
>> file which is currently using only of_* -stuff. Well, I suppose it can be
>> asked from netdev peeps but I am not convinced they see it as a great idea.
>>
>> If I misunderstood your meaning - please elaborate.
> 
> The driver is quite old

I remember having to modify this driver somewhere around 2010 or so. :) 
Time flies.

> and has a lot of room to improve. Briefly looking it
> may be almost fully converted to fwnode, but it's not your call (only if you
> wish). Nevertheless, using agnostic APIs if they reduce code base is fine.
> We have drivers that do OF and fwnode mixed approach (for various reasons,
> one of which is the new API that is absent in OF realm.

Well, we can propose this to netdev people but I wouldn't be surprized 
if they requested full of_node => fwnode rewrite instead of removing 
simple looking loop and bringing mixture of fwnode and of_node in driver.

Yours,
	-- Matti

Powered by blists - more mailing lists