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

[Matti Vaittinen <mazziesaccount@gmail.com>]

On 6/3/24 19:38, Thomas Gleixner wrote:
> Matti!
> 
> On Mon, Jun 03 2024 at 15:19, Matti Vaittinen wrote:
>> On 6/3/24 13:20, Thomas Gleixner wrote:
>>> On Fri, May 24 2024 at 11:18, Matti Vaittinen wrote:
>>> Now you start talking about parent interrupts. Can you please make your
>>> mind up and concisely explain what this is about?
>>
>> I hope I can explain what I am after here. I am also very happy when
>> incorrect terminology is pointed out! So, it'd be great to know if I
>> should use 'parent' or 'physical IRQ' here after I explain this further.
>>
>> What I am dealing with is an I2C device (PMIC) which provides two GPIO
>> IRQ lines for SOC. This is what I meant by "physical IRQs".
>>
>> ----------------	INTB IRQ	-----------------
>> |		|-----------------------|		|
>> |	PMIC	|			|    SOC	|
>> |		|-----------------------|		|
>> -----------------	ERRB IRQ	-----------------
>>
>>
>> Both the INTB and ERRB can report various events, and correct event can
>> be further read from the PMIC registers when IRQ line is asserted. I
>> think this is business as usual.
>>
>> I'd like to use the regmap_irq for representing these events as separate
>> 'IRQs' (which can be handled using handlers registered with
>> request_threaded_irq() as usual).
>>
>> Here, when talking about 'parent IRQ', I was referring to ERRB or INTB
>> as 'parent IRQ'. My thinking was that, the regmap IRQ instance uses INTB
>> or ERRB as it's parent IRQ, which it splits (demuxes) to separate "child
>> IRQs" for the rest of the PMIC drivers to use. I'd be grateful if better
>> terms were suggested so that readers can stay on same page with me.
>>
>> After talking with Mark:
>>
>> we both thought it'd be cleaner to have separate regmap IRQ instances
>> for the ERRB and INTB lines. This makes sense (to me) because a lot of
>> (almost all of?) the regmap IRQ internal data describe the IRQ line
>> related things like registers related to the IRQ line, IRQ line polarity
>> etc. Hence, making single regmap-IRQ instance to support more than one
>> <please, add what is the correct term for INTB / ERRB like line> would
>> require duplicating a plenty of the regmap data. This would make
>> registering an regmap-IRQ entity much more complex and additionally it'd
>> also complicate the internals of the regmap-IRQ. It'd be a bit like
>> trying to fill and drink a six-pack of lemonade at once, instead of
>> going a bottle by bottle :)
> 
> This makes a lot more sense now. So what I read out of this in change
> log style is:
> 
>    Devices can have subfunctions where each has its own interrupt
>    line. Each interrupt line acts as demultiplex interrupt for
>    subfunction specific interrupts and has its distinct set of registers.
> 
>    regmap can support such a setup, but the interrupt domain code ends up
>    to assign the same device name when creating the underlying per
>    subfunction interrupt domains. This causes name space collision in the
>    debugfs code and also leads to confusion in other places, e.g.
>    /proc/interrupts would show two distinct interrupts with the same
>    domain name and hardware interrupt number.
> 
>    Instead of working around this in the driver or the regmap code, allow
>    to provide a name suffix for the domain name when creating the
>    interrupt domains.
> 
>    Add the infrastructure to __irq_domain_add() and expose it via
>    irq_domain_create_linear_named() which is the only function required
>    by the regmap code to support such setups.
> 
> Did I get it halfways right?

I think you got it perfectly right. :) And, I really appreciate the 
extra mile you went when spelling it out in this way. I will send a new 
version where the legacy domain function is dropped, and the commit 
message will use what you wrote here  :)

Thanks a ton!

Yours,
	-- Matti

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

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