[v2,2/3] mfd: da9062: add support for the DA9062 GPIOs in the core

Currently the da9062 GPIO's aren't available. The patch adds the support
to make these available by adding a gpio device with the corresponding
irq resources. Furthermore the patch fixes a minor style issue for the
onkey device.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/mfd/da9062-core.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

On Wed, Nov 27, 2019 at 12:56 PM Marco Felsch <m.felsch@pengutronix.de> wrote:

> Currently the da9062 GPIO's aren't available. The patch adds the support
> to make these available by adding a gpio device with the corresponding
> irq resources. Furthermore the patch fixes a minor style issue for the
> onkey device.
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

This is a regmap irqchip so I guess not much to say about it.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

HOWEVER: this looks very much hierarchical does it not?

I can clearly see that regmap's irqchip does not support
hierarchical interrupt domains, so we should just make a
mental note somewhere that "oh yeah and then one day
we should make regmap irqchips play well with hierarchical
interrupts".

Yours,
Linus Walleij

Hi Linus,

thanks for your feedback.

On 19-11-27 14:35, Linus Walleij wrote:
> On Wed, Nov 27, 2019 at 12:56 PM Marco Felsch <m.felsch@pengutronix.de> wrote:
> 
> > Currently the da9062 GPIO's aren't available. The patch adds the support
> > to make these available by adding a gpio device with the corresponding
> > irq resources. Furthermore the patch fixes a minor style issue for the
> > onkey device.
> >
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> 
> This is a regmap irqchip so I guess not much to say about it.
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> HOWEVER: this looks very much hierarchical does it not?

Yes it that's right and I converted it upon Bartosz comments.

> I can clearly see that regmap's irqchip does not support
> hierarchical interrupt domains, so we should just make a
> mental note somewhere that "oh yeah and then one day
> we should make regmap irqchips play well with hierarchical
> interrupts".

That's right, should I add this somewhere and if the answer is yes then
where?

Regards,
  Marco

> 
> Yours,
> Linus Walleij
>

On Wed, Nov 27, 2019 at 02:35:33PM +0100, Linus Walleij wrote:

> I can clearly see that regmap's irqchip does not support
> hierarchical interrupt domains, so we should just make a
> mental note somewhere that "oh yeah and then one day
> we should make regmap irqchips play well with hierarchical
> interrupts".

Why, what do we need to do?  We're just doing all the default stuff,
it's not something we've opted out of, and the whole point with using
the frameworks should be that we should get software features like this
for free :(

On Wed, Nov 27, 2019 at 4:19 PM Mark Brown <broonie@kernel.org> wrote:
> On Wed, Nov 27, 2019 at 02:35:33PM +0100, Linus Walleij wrote:
>
> > I can clearly see that regmap's irqchip does not support
> > hierarchical interrupt domains, so we should just make a
> > mental note somewhere that "oh yeah and then one day
> > we should make regmap irqchips play well with hierarchical
> > interrupts".
>
> Why, what do we need to do?  We're just doing all the default stuff,
> it's not something we've opted out of, and the whole point with using
> the frameworks should be that we should get software features like this
> for free :(

I guess it is a bit about moving targets.

The regmap irq thing was covering all reasonable cases until
the hierarchical interrupts were introduced some years ago.

The hallmark of these are that the irq_domain_ops implement
.translate() .alloc() and .free() rather than .map() and .xlate()
as the irqdomain in reqmap-irq currently does.

The problem with hierarchical domains is that the system using
them need to be hierarchical "all the way up" to the overarching
top-level irqchip. Currently only the ARM GIC and the IXP4xx
irq top-level irq controllers supports this, ruling out some
obvious users like all non-ARM systems (for now).

I think the assumption in hierarchical irq is that you have
a few hardware-specific irchips and you know exactly which
irqchip that goes on top of another one, as well as which
hardware irq line is connected to which hardware irq line
on the parent.

Since we know the specific hardware (from a compatible
string or so) we can hardcode the parent-to-child mappings
of interrupt lines in the driver. These mappings are not
in the device tree for example.

Therefore supporting hierarchical and nonhierarchical alike
in a very generic plug-in irqchip like the regmap-irq is a bit
of a challenge as it has to support both hierarchical and
non-hierarchical, because it is not possible to just
convert this to hierarchical callbacks: it has to check what
its parent is doing and adapt, essentially implement both
hierarchical and non-hierarchical at this time.

Also we need to pass mappings between parent and child
somehow. In the gpiolib we did this with a callback to the
GPIO-chip-specific driver. How to do it for something
generic like regmap-irq is an open question.

So it is a bit complex.

(Marc may correct me here.)

Yours,
Linus Walleij

On Fri, Nov 29, 2019 at 09:49:56AM +0100, Linus Walleij wrote:
> On Wed, Nov 27, 2019 at 4:19 PM Mark Brown <broonie@kernel.org> wrote:

> > Why, what do we need to do?  We're just doing all the default stuff,
> > it's not something we've opted out of, and the whole point with using
> > the frameworks should be that we should get software features like this
> > for free :(

> I guess it is a bit about moving targets.

> The regmap irq thing was covering all reasonable cases until
> the hierarchical interrupts were introduced some years ago.

> The hallmark of these are that the irq_domain_ops implement
> .translate() .alloc() and .free() rather than .map() and .xlate()
> as the irqdomain in reqmap-irq currently does.

So there's two completely different APIs.  Are all the drivers supposed
to be being updated to implement both?  It looks like the second API is
ifdefed out when not in use so I guess so but...

> The problem with hierarchical domains is that the system using
> them need to be hierarchical "all the way up" to the overarching
> top-level irqchip. Currently only the ARM GIC and the IXP4xx
> irq top-level irq controllers supports this, ruling out some
> obvious users like all non-ARM systems (for now).

Are you sure?  It looks like the API was introduced by Intel and io_apic
appears to be using the new interfaces.

> I think the assumption in hierarchical irq is that you have
> a few hardware-specific irchips and you know exactly which
> irqchip that goes on top of another one, as well as which
> hardware irq line is connected to which hardware irq line
> on the parent.

The documentation says that this is for systems where "there may be
multiple interrupt controllers involved in delivering an interrupt from
the device to the target CPU" which describes more or less every single
regmap-irq user, though the threading might mean it doesn't quite map
onto what was being thought of there.  But basically everything I can
find suggests that regmap-irq should be a hierarchical controller apart
from how we figure out the primary IRQ.

> Since we know the specific hardware (from a compatible
> string or so) we can hardcode the parent-to-child mappings
> of interrupt lines in the driver. These mappings are not
> in the device tree for example.

That seems to be part of it, yes, which seems unfortunate.

> Therefore supporting hierarchical and nonhierarchical alike
> in a very generic plug-in irqchip like the regmap-irq is a bit
> of a challenge as it has to support both hierarchical and
> non-hierarchical, because it is not possible to just
> convert this to hierarchical callbacks: it has to check what
> its parent is doing and adapt, essentially implement both
> hierarchical and non-hierarchical at this time.

It looks that way, yes.

> Also we need to pass mappings between parent and child
> somehow. In the gpiolib we did this with a callback to the
> GPIO-chip-specific driver. How to do it for something
> generic like regmap-irq is an open question.

Currently regmap-irq just gets a parent interrupt from whatever is using
it so yeah, this doesn't map on at all well especially since the user is
likely to be using just a normal interrupt binding for this which is
apparently explicitly not allowed[1].  I'm not sure what to do here.

[1] https://elinux.org/images/8/8c/Zyngier.pdf