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
