Re: [PATCH 2/2] RFC: gpio: ixp4xx: Convert to hieararchical GPIOLIB_IRQCHIP

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Mon, Jun 3, 2019 at 11:16 AM Thierry Reding <thierry.reding@xxxxxxxxx> wrote:
> Me

> > +const struct gpiochip_hierarchy_map ixp4xx_gpiomap[] = {
> > +     { .hwirq = 0, .parent_hwirq = 6, .parent_type = IRQ_TYPE_LEVEL_HIGH },
> > +     { .hwirq = 1, .parent_hwirq = 7, .parent_type = IRQ_TYPE_LEVEL_HIGH },
> > +     { .hwirq = 2, .parent_hwirq = 19, .parent_type = IRQ_TYPE_LEVEL_HIGH },
> > +     { .hwirq = 3, .parent_hwirq = 20, .parent_type = IRQ_TYPE_LEVEL_HIGH },
> > +     { .hwirq = 4, .parent_hwirq = 21, .parent_type = IRQ_TYPE_LEVEL_HIGH },
> > +     { .hwirq = 5, .parent_hwirq = 22, .parent_type = IRQ_TYPE_LEVEL_HIGH },
> > +     { .hwirq = 6, .parent_hwirq = 23, .parent_type = IRQ_TYPE_LEVEL_HIGH },
> > +     { .hwirq = 7, .parent_hwirq = 24, .parent_type = IRQ_TYPE_LEVEL_HIGH },
> > +     { .hwirq = 8, .parent_hwirq = 25, .parent_type = IRQ_TYPE_LEVEL_HIGH },
> > +     { .hwirq = 9, .parent_hwirq = 26, .parent_type = IRQ_TYPE_LEVEL_HIGH },
> > +     { .hwirq = 10, .parent_hwirq = 27, .parent_type = IRQ_TYPE_LEVEL_HIGH },
> > +     { .hwirq = 11, .parent_hwirq = 28, .parent_type = IRQ_TYPE_LEVEL_HIGH },
> > +     { .hwirq = 12, .parent_hwirq = 29, .parent_type = IRQ_TYPE_LEVEL_HIGH },
>
> I think this is a good example of what I was referring to. There's no
> easy way to represent this mapping in code, other than basically writing
> up the table as a switch statement, or a couple of if/else clauses. But
> if you have a large number of pins in the GPIO controller and they all
> can be easily described by just a handful of lines of code, I think it'd
> be nice to avoid all that extra data.

I agree, but we can add those as we go along. I wonder if such code expressions
of the mapping are really that magical? Isn't it simply at best ranges of hwirqs
so a n_hwirqs need to be added to the map and just collapse the above to:

{ .hwirq = 0, .parent_hwirq = 6, .nr_hwirqs = 13, .parent_type =
IRQ_TYPE_LEVEL_HIGH }

I'm up for that. Maybe as a separate patch though.

Yours,
Linus Walleij