On Thu, Feb 27, 2025 at 10:01:49AM +0200, Matti Vaittinen wrote:

On 26/02/2025 16:11, Andy Shevchenko wrote:

quoted

On Wed, Feb 26, 2025 at 04:04:02PM +0200, Matti Vaittinen wrote:

quoted

On 25/02/2025 15:59, Andy Shevchenko wrote:

quoted

On Tue, Feb 25, 2025 at 03:29:17PM +0200, Matti Vaittinen wrote:

quoted

On 25/02/2025 12:39, Andy Shevchenko wrote:

quoted

On Tue, Feb 25, 2025 at 12:29:31PM +0200, Matti Vaittinen wrote:

quoted

On 25/02/2025 12:21, Andy Shevchenko wrote:

quoted

On Tue, Feb 25, 2025 at 11:40:16AM +0200, Heikki Krogerus wrote:

...

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

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.

???

The loops are working on

	struct device_node *np = pdev->dev.np;

which is the equivalent to

	struct device_node *np = dev_of_node(&pdev->dev);

which takes device pointer.

(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.

Are you forbidding yourself to change the function prototype to take a device
pointer instead of device_node one? :-)

quoted

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 :)

Make gfar_of_group_count() to take device pointer. As simple as that.

quoted

quoted

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.

quoted

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.

Without doing the proposal we will never know what they will think of all
this...

-- 
With Best Regards,
Andy Shevchenko