Re: [PATCH v2 09/10] genirq/irqdomain: fall back to default domain when creating hierarchy domain

[Thomas Bogendoerfer <tbogendoerfer@suse.de>]

On Tue, 19 Feb 2019 16:27:16 +0000
Marc Zyngier <marc.zyngier@arm.com> wrote:

> On Tue, 19 Feb 2019 16:57:23 +0100
> Thomas Bogendoerfer <tbogendoerfer@suse.de> wrote:
> 
> Hi Thomas,
> 
> > When creating hierarchy domains use irq_default_domain as parent, if no
> > parent was given by the caller. This avoids adding helper code for
> > querying the underlying platform irq domain.
> > 
> > Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> > ---
> >  kernel/irq/irqdomain.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> > index 8b0be4bd6565..617c482d0778 100644
> > --- a/kernel/irq/irqdomain.c
> > +++ b/kernel/irq/irqdomain.c
> > @@ -1021,7 +1021,10 @@ struct irq_domain *irq_domain_create_hierarchy(struct irq_domain *parent,
> >  	else
> >  		domain = irq_domain_create_tree(fwnode, ops, host_data);
> >  	if (domain) {
> > -		domain->parent = parent;
> > +		if (parent)
> > +			domain->parent = parent;
> > +		else
> > +			domain->parent = irq_default_domain;
> >  		domain->flags |= flags;
> >  	}
> >  
> 
> I'm really not keen on this. The whole "default domain" made sense at a
> distant point in time (when irqdomains were new and platform code was
> blissfully ignoring it), but it really looks like a sore spot in the
> hierarchy code, which assumes that you always know what you're building
> your hierarchy on top of.
> 
> It also create a small issue in the sense that you can create a root
> domain using irq_domain_create_hierarchy() by passing NULL as the
> parent. With this patch, the new domain now points to the default one,
> with unexpected consequences.
> 
> So let's come back to first principles: How comes you can't obtain the
> parent domain at creation time? Because I'd rather give you a way to
> retrieve it instead if this.

the bridge irq domain could be stacked on different underlying irq domains
for different platforms (HUB is IP27, HEART for IP30 and BEDROCK for IP35).
And my idea was to set a irq default domain in the IP27/IP30/IP35 platform
code so that bridge code will pick up the correct underlying irq domain.
As there is no device tree I haven't found an already implemented other way.
Right now I have two idea to solve my problem without this patch:

- implement a SGI specific helper for getting the underlying irq domain
- use a helper function to read irq_default_domain

What do you prefer ? Or do you see something else ?

Thomas.

-- 
SUSE Linux GmbH
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)