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

Matti Vaittinen
2024-06-03T17:38:15Z

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! ~~