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

[tglx <tglx@linutronix.de>]

Jiri!

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.

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?

Thanks,

        tglx