Re: [PATCH v3 07/10] irqdomain: Allow giving name suffix for domain

[Matti Vaittinen <mazziesaccount@gmail.com>]

Hello Thomas, Herve.

On 6/6/24 21:59, Thomas Gleixner wrote:
> Matti!
> 
> On Tue, Jun 04 2024 at 10:55, Matti Vaittinen wrote:
>>   struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int size,
>>   				    irq_hw_number_t hwirq_max, int direct_max,
>>   				    const struct irq_domain_ops *ops,
>> -				    void *host_data);
>> +				    void *host_data, const char *name_suffix);
>>   struct irq_domain *irq_domain_create_simple(struct fwnode_handle *fwnode,
>>   					    unsigned int size,
>>   					    unsigned int first_irq,
>> @@ -350,7 +350,8 @@ static inline struct irq_domain *irq_domain_add_linear(struct device_node *of_no
>>   					 const struct irq_domain_ops *ops,
>>   					 void *host_data)
>>   {
>> -	return __irq_domain_add(of_node_to_fwnode(of_node), size, size, 0, ops, host_data);
>> +	return __irq_domain_add(of_node_to_fwnode(of_node), size, size, 0, ops,
>> +				host_data, NULL);
> 
> ....
> 
> Looking at the resulting amount of churn to add that argument, I'm not
> really enthused. There is some other unrelated change required in this
> area:
> 
>    https://lore.kernel.org/all/8734pr5yq1.ffs@tglx
> 
> My suggestion to convert all of this mess into a template based
> mechanism would nicely solve your problem too.

I am not entirely sure what you mean by template based in this context. 
My brains are somehow fixed to start thinking of C++ templates, or C 
macro magic with typeof() and I just can't get past that.

Anyways, what I picked from discussion between you and Herve, is using 
an initialization structure (struct irq_domain_info) for the new domain 
creation function (irq_domain_instantiate()) instead of adding bunch of 
functions with quite a few separate arguments. So, I assume you're 
referring to a possibility to add the name-suffix in this initialization 
structure? I hope I got this right.

I assume there is no intention to change the existing public 
irq_domain_creat_foo() APIs to use the new irq_domain_info - and change 
all the callers(?) But I think changing the internal 
__irq_domain_create() to use this new info struct should be very much 
doable - although, in my opinion, making existing callers of the 
__irq_domain_create() to assign their parameters to this struct so they 
can pass it to __irq_domain_create() does not seem so nice.

So, even though I am not really happy about the delay (I secretly hoped 
to get the series merged before my summer vacations ;) ) - I admit your 
suggested change looks cleaner (again, at least to me).

Herve, do you have any idea when you plan to do further sketching on 
this? Do you want me to try seeing if I can add the struct 
irq_domain_info and maybe use that in the __irq_domain_add() to get the 
name-suffix added? I might be able to send one version out during next 
week - but then I plan to be offline for couple of weeks ... so it may 
be I am not much of a help here.

> Can you please have a look and eventually team up with Herve (CC'ed) to
> sort this out? I'm happy to help and give guidance.

I appreciate the guidance! Thanks Thomas.

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~