Re: [PATCH v3] gpio: pl061: Support implementations without GPIOINTR line

[Linus Walleij]

On Fri, Mar 19, 2021 at 4:32 PM Alexander Sverdlin
<alexander.sverdlin@xxxxxxxxx> wrote:
> [Andy]

> > I'm wondering if the GPIO library support for IRQ hierarchy is what
> > you are looking for.

It is indeed what should be used.

> do you have a better suggestion? That's why I reference the below discussion from 2017, where
> Linus Walleij suggested to use it. Well, the initial patch was just 11-liner and PL061 is
> rather counterexample and doesn't fit well into the existing hierarchical infrastructure.
>
> >> Link: https://lore.kernel.org/linux-gpio/CACRpkdZpYzpMDWqJobSYH=JHgB74HbCQihOtexs+sVyo6SRJdA@xxxxxxxxxxxxxx/

Don't trust that guy. He's inexperienced with the new API and talks a lot
of crap. ;)

We now have a proper API for hierarchical IRQs on GPIO controllers,
so we need to somehow detect that this is the case and act accordingly.

1. In Kconfig
    select IRQ_DOMAIN_HIERARCHY if ARCH_NOKIA_FOO

2. Look at other hierarchical GPIO IRQ drivers such as
    drivers/gpio/gpio-ixp4xx.c

3. Detect that we have a hierarchical situation. The hierarchical
  IRQ is determined by the chip integration so I would go and
  check the SoC compatible in the top-level DT, e.g.:
  if (of_machine_is_compatible("nokia,rock-n-roll-soc")) {
     /* Initialize the interrupt as hiearchical */
     girq->fwnode =...
     girq->parent_domain = ...
     girq->child_to_parent_hwirq = pl061_child_to_parent_nokia_rock_n_roll;
  } else {
     girq->parent_handler = pl061_irq_handler;
     girq->num_parents = 1;
     girq->parents = devm_kcalloc(dev, 1, sizeof(*girq->parents),
                                     GFP_KERNEL);
     if (!girq->parents)
            return -ENOMEM;
      girq->parents[0] = irq;
  }

  This might need some #ifdef or IS_ENABLED() for systems without
  hierarchical IRQs.

4. Implement pl061_child_to_parent_nokia_rock_n_roll()
   Just use hardcoded hardware IRQ offsets like other drivers such as
   the ixp4xx does. Do not attempt to read any parent IRQs from
   the device tree, and assign no IRQ in the device tree.

This is a side effect of the essentially per-soc pecularities around
interrupts. The interrupt is not cascaded so it need special
handling.

I think it can be done with quite little code.

Yours,
Linus Walleij