Re: [PATCH 09/18] irqdomain: Rename _add functions to _add_*_of_node

[Jiri Slaby]

Thomas,

sorry for the delay, I drowned in tty.

On 06. 02. 25, 17:22, tglx wrote:
> On Wed, Jan 15 2025 at 09:53, Jiri Slaby (SUSE) wrote:
> > For readers, it is very confusing that:
> > * irq_domain_add_*() functions are dedicated to of_nodes,
> > * irq_domain_create_*() ones to fwnodes, and
> > * irq_domain_instantiate() to the universal struct irq_domain_info.
> >
> > Neither _create, nor _add designate any of those nodes. Despite the
> > naming, the functionality of them is the same: add an irq domain (by
> > generic irq_domain_instantiate()). So the source of the confusion is the
> > naming proper -- making the distinction based on _create, _add, and
> > _instantiate.
> >
> > Therefore, here an "_of_node" suffix is added to all "_add" functions
> > (of_node ones). In the next patch, "_create" (fwnode ones) are switched
> > to "_add_fwnode". And finally, "_instantiate" is renamed to "_add".
> >
> > So when all are applied, the interface is much easier to follow:
> > * dom = irq_domain_add_linear_of_node(of_node, ...)
> > * dom = irq_domain_add_linear_fwnode(fwnode, ...)
> > * dom = irq_domain_add(info)
> > * irq_domain_remove(dom)
>
> I'm not convinced that this _of_node() _fwnode() churn is actually
> valuable. I rather go and consolidate the code so that the core
> functions take a fwnode argument, i.e.
>
>     - irq_domain_add_xxx(node, ...)
>     + irq_domain_add_xxx(of_fwnode_handle(node), ....)
>
> It's not asked too much from the developer to use of_fwnode_handle() at
> the call site and the resulting treewide churn is pretty much the same
> as in any case all call sites need to be touched.

OK, NP. I am only confused by your "I rather go". Does it mean you are 
already on it? Or should I translate that as "I'd rather go", ie. /me 
doing the work -- I expect this case and can indeed do the job. I just 
don't want to duplicate the work.

> But that brings me to the question of logistics for this overhaul. As
> this is a treewide change, there is quite some potential to create
> conflicts all over the place.
>
> So the obvious solution is to consolidate on the existing
> irq_domain_create_*() API, which is not the worst naming once everything
> is unified, i.e.
>
>     - irq_domain_add_xxx(node, ...)
>     + irq_domain_create_xxx(of_fwnode_handle(node), ....)
>
> It allows to distribute these changes (except for the _nomap() oddity,
> which is OF only) right now to the relevant subsystems and I can collect
> the ignored changes in the irq tree. The final removal of the _add*()
> interfaces can then be done towards the end of the merge window.
>
> Thoughts?

Sounds good to me.

thanks,
--
js
suse labs