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

[Thomas Gleixner]

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?

Thanks,

        tglx