Re: [PATCH 2/2] regmap: Allow setting IRQ domain name suffix

From: Thomas Gleixner
Date: Wed Aug 07 2024 - 09:02:32 EST


Matti!

On Tue, Aug 06 2024 at 14:51, Matti Vaittinen wrote:
> On 7/13/24 15:22, Thomas Gleixner wrote:
>> Something like the untested below should work. That would make your
>> info:
>>
>> struct irq_domain_info info = {
>> .fwnode = fwnode,
>> .size = chip->num_irqs,
>
> Based on my code reading, the .size is used for allocating the "revmap".
> Looking at the info struct for existing implementation of the
> irq_domain_create_legacy(), the .size is set as:
>
> .size = first_hwirq + size,
>
>> .hwirq_max = chip->num_irqs,
>
> Also, the irq_domain_create_legacy() sets hwirq_max as:
>
> .hwirq_max = first_hwirq + size.
>
> see:
>
> > @@ -476,18 +486,14 @@ struct irq_domain *irq_domain_create_leg
> > .fwnode = fwnode,
> > .size = first_hwirq + size,
> > .hwirq_max = first_hwirq + size,
> > + .hwirq_base = first_hwirq,
> > + .virq_base = first_irq,
> > .ops = ops,
> > .host_data = host_data,
> > };
> > - struct irq_domain *domain;
> > + struct irq_domain *domain = irq_domain_instantiate(&info);
> >
> > - domain = irq_domain_instantiate(&info);
> > - if (IS_ERR(domain))
> > - return NULL;
> > -
> > - irq_domain_associate_many(domain, first_irq, first_hwirq, size);
>
> Lookin at this, the existing code calls irq_domain_associate_many() with
> the given size parameter (without the + first_hwirq which is assigned to
> .size).

Indeed.

> I think this is not aligned with what the patch below results (and yes,
> I know Thomas told it's untested).

:)

> I'd better admit I am not 100% sure how the legacy domains work and that
> I don't (any more) fully trust on my ability to flawlessly interpret the
> code ;)

You definitely did better than me :)

> Hence I'd rather learn from a small explanation (what is the
> expected .size) than by fixing this after I see regression reports from
> real users of the irq_domain_create_legacy() :)

So the size of the domain is sum of the parameters @size and
@first_hwirq. That's so that the hardware interrupt is zero indexed for
an array based lookup.

The association obviously wants only the @size parameter because that's
what the caller wants interrupts for as it obviously can't provide
interrupts below @first_hwirq.

> So, any guidance as to what the revmap allocation size should be (the
> info->size), and what should be the size for the
> irq_domain_associate_many()?

So that associate should be:

irq_domain_associate_many(domain, info->virq_base, info->hwirq_base,
info->size - info->hwirq_base);

Thanks,

tglx