Re: [PATCH v4 6/6] gpio: uniphier: add UniPhier GPIO controller driver

[David Daney]

On 09/12/2017 07:03 AM, Linus Walleij wrote:
> On Thu, Sep 7, 2017 at 1:42 PM, Masahiro Yamada
> <yamada.masahiro@xxxxxxxxxxxxx> wrote:
>
> > This GPIO controller device is used on UniPhier SoCs.
> >
> > It also serves as an interrupt controller, but interrupt signals are
> > just delivered to the parent irqchip without any latching or OR'ing.
> > This is implemented by using hierarchy IRQ domain.
> >
> > Implementation note:
> > Unfortunately, the IRQ mapping from this controller to the parent is
> > random. (48, 49, ..., 63, 154, 155, ...)
> > If "interrupts" property is used, IRQ resources may be statically
> > allocated when platform devices are populated from DT.  This can be
> > a problem for the hierarchy IRQ domain because IRQ allocation must
> > happen from the outer-most domain up to the root domain in order to
> > build up the stacked IRQ.  (https://lkml.org/lkml/2017/7/6/758)
> > Solutions to work around it could be to hard-code parent hwirqs or
> > to invent a driver-specific DT property.
> >
> > Here, the new API irq_domain_push_irq() was merged by v4.14-rc1.
> > It allows to add irq_data to the existing hierarchy.  It will help
> > to make this driver work whether the parent has already initialized
> > the hierarchy or not.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> > ---
> >
> > Changes in v4:
> >    - Add COMPILE_TEST and select IRQ_DOMAIN_HIERARCHY
> >    - Reimplement irqchip part by using irq_domain_push_irq()
>
> Awesome improvement. There was a build error and I also
> would like David Daney to have a look at this so we know we
> use things the right way,

It looks correct to me.

I haven't verified it, but I think the OF device-tree probing code for 
the platform devices will automatically xlat-and-map all those irqs, so 
that the  irq_domain_push_irq() is required to get the domain hierarchy 
properly configured.  It would be similar to the PCI case where we 
configure all the MSI-X and then do the irq_domain_push_irq() in the 
Cavium ThunderX driver.

If interrupt handling has been verified to work with this driver, I 
would say that we are probably using things "the right way".

David.



> but overall I am happy after this
> so I hope I will be able to apply next version.
>
> Yours,
> Linus Walleij