[PATCH dt-schema] Fix interrupt controllers with interrupt-map

[PATCH dt-schema] Fix interrupt controllers with interrupt-map

[Geert Uytterhoeven]

When an interrupt controller has an "interrupt-map" property, an "is
valid under each of" error is triggered.

Fix this by allowing "interrupt-controller" and "interrupt-map" to
coexist, in both the interrrupts meta-schema and the
interrupt-controller schema.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 meta-schemas/interrupts.yaml      | 2 +-
 schemas/interrupt-controller.yaml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta-schemas/interrupts.yaml b/meta-schemas/interrupts.yaml
index 7073063486d44dab..93d8e4cafca679d3 100644
--- a/meta-schemas/interrupts.yaml
+++ b/meta-schemas/interrupts.yaml
@@ -26,7 +26,7 @@ properties:
 dependencies:
   interrupt-map: ['#interrupt-cells', 'interrupt-map-mask']
   '#interrupt-cells':
-    oneOf:
+    anyOf:
       - required:
           - interrupt-controller
       - required:
diff --git a/schemas/interrupt-controller.yaml b/schemas/interrupt-controller.yaml
index f65d5f6d2f1ac524..5dce87d839959489 100644
--- a/schemas/interrupt-controller.yaml
+++ b/schemas/interrupt-controller.yaml
@@ -30,7 +30,7 @@ dependencies:
   interrupt-controller: ['#interrupt-cells']
   interrupt-map: ['#interrupt-cells', 'interrupt-map-mask']
   "#interrupt-cells":
-    oneOf:
+    anyOf:
       - required:
           - interrupt-controller
       - required:
-- 
2.17.1

Re: [PATCH dt-schema] Fix interrupt controllers with interrupt-map

[Rob Herring]

On Thu, May 28, 2020 at 7:23 AM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> When an interrupt controller has an "interrupt-map" property, an "is
> valid under each of" error is triggered.
>
> Fix this by allowing "interrupt-controller" and "interrupt-map" to
> coexist, in both the interrrupts meta-schema and the
> interrupt-controller schema.

But both should not be present. If 'interrupt-controller' is present,
the Linux irq parsing code will ignore 'interrupt-map'. Seems like
that's backwards, but this parsing code is older than dirt and we'd
probably break some 1990s machine changing it.

Rob

Re: [PATCH dt-schema] Fix interrupt controllers with interrupt-map

[Geert Uytterhoeven]

Hi Rob,

On Fri, May 29, 2020 at 5:54 PM Rob Herring <robh+dt@kernel.org> wrote:
> On Thu, May 28, 2020 at 7:23 AM Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> > When an interrupt controller has an "interrupt-map" property, an "is
> > valid under each of" error is triggered.
> >
> > Fix this by allowing "interrupt-controller" and "interrupt-map" to
> > coexist, in both the interrrupts meta-schema and the
> > interrupt-controller schema.
>
> But both should not be present. If 'interrupt-controller' is present,

Why not?

> the Linux irq parsing code will ignore 'interrupt-map'. Seems like
> that's backwards, but this parsing code is older than dirt and we'd
> probably break some 1990s machine changing it.

That's fine.  rza1_irqc_parse_map() parses the interrupt-map itself,
to map from downstream to upstream interrupts.

Cfr. the original bindings at
https://lore.kernel.org/linux-devicetree/20190502123220.3016-2-geert+renesas@glider.be/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH dt-schema] Fix interrupt controllers with interrupt-map

[Rob Herring]

On Fri, May 29, 2020 at 10:02 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Rob,
>
> On Fri, May 29, 2020 at 5:54 PM Rob Herring <robh+dt@kernel.org> wrote:
> > On Thu, May 28, 2020 at 7:23 AM Geert Uytterhoeven
> > <geert+renesas@glider.be> wrote:
> > > When an interrupt controller has an "interrupt-map" property, an "is
> > > valid under each of" error is triggered.
> > >
> > > Fix this by allowing "interrupt-controller" and "interrupt-map" to
> > > coexist, in both the interrrupts meta-schema and the
> > > interrupt-controller schema.
> >
> > But both should not be present. If 'interrupt-controller' is present,
>
> Why not?

Well, maybe I'm wrong. If you have more than just transparent
remapping (i.e. mask/unmask/clear), then perhaps both are appropriate
because you want get the irq domain for the first irq parent.

> > the Linux irq parsing code will ignore 'interrupt-map'. Seems like
> > that's backwards, but this parsing code is older than dirt and we'd
> > probably break some 1990s machine changing it.
>
> That's fine.  rza1_irqc_parse_map() parses the interrupt-map itself,
> to map from downstream to upstream interrupts.

You shouldn't really be parsing interrupt-map yourself. The code there
doesn't account for #address-cells which can be a factor for
interrupt-map. dtc is gaining some checks for 'interrupt-map', so
let's hope you have it right.

Rob

Re: [PATCH dt-schema] Fix interrupt controllers with interrupt-map

[Geert Uytterhoeven]

Hi Rob,

Cc Rasmus

On Fri, May 29, 2020 at 11:44 PM Rob Herring <robh+dt@kernel.org> wrote:
> On Fri, May 29, 2020 at 10:02 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Fri, May 29, 2020 at 5:54 PM Rob Herring <robh+dt@kernel.org> wrote:
> > > On Thu, May 28, 2020 at 7:23 AM Geert Uytterhoeven
> > > <geert+renesas@glider.be> wrote:
> > > > When an interrupt controller has an "interrupt-map" property, an "is
> > > > valid under each of" error is triggered.
> > > >
> > > > Fix this by allowing "interrupt-controller" and "interrupt-map" to
> > > > coexist, in both the interrrupts meta-schema and the
> > > > interrupt-controller schema.
> > >
> > > But both should not be present. If 'interrupt-controller' is present,
> >
> > Why not?
>
> Well, maybe I'm wrong. If you have more than just transparent
> remapping (i.e. mask/unmask/clear), then perhaps both are appropriate
> because you want get the irq domain for the first irq parent.

Indeed, that's the case for rza1-irqc.

> > > the Linux irq parsing code will ignore 'interrupt-map'. Seems like
> > > that's backwards, but this parsing code is older than dirt and we'd
> > > probably break some 1990s machine changing it.
> >
> > That's fine.  rza1_irqc_parse_map() parses the interrupt-map itself,
> > to map from downstream to upstream interrupts.
>
> You shouldn't really be parsing interrupt-map yourself. The code there

Of course I had tried that before[1];

|    I also considered extracting the parsing code in of_irq_parse_raw() in a
|    new public helper function:
|
|        int of_irq_parse_map(struct device_node **ipar, u32 *addrsize,
|                             u32 *intsize, const __be32 **match_array,
|                             struct of_phandle_args *out_irq);
|
|    However, that API is a bit ugly due to many output parameters
|    (of_irq_parse_raw() needs to iterate the interrupt hierarchy).
|    In addition, it's less efficient for irq-renesas-rza1.c, as the
|    interrupt-map must be parsed 8 times.

Or is there a better way?

In the mean time, drivers/irqchip/irq-ls-extirq.c has landed, which was
mimiced after irq-renesas-rza1.c.

> doesn't account for #address-cells which can be a factor for
> interrupt-map. dtc is gaining some checks for 'interrupt-map', so
> let's hope you have it right.

The driver code indeed doesn't account for #address-cells, but the
binding says it must be zero anyway.

[1] "[PATCH v4 2/2] irqchip: Add Renesas RZ/A1 Interrupt Controller driver"
    http://lore.kernel.org/r/20190527121711.5138-3-geert+renesas@glider.be

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH dt-schema] Fix interrupt controllers with interrupt-map

[Rob Herring]

On Thu, May 28, 2020 at 03:23:23PM +0200, Geert Uytterhoeven wrote:
> When an interrupt controller has an "interrupt-map" property, an "is
> valid under each of" error is triggered.
> 
> Fix this by allowing "interrupt-controller" and "interrupt-map" to
> coexist, in both the interrrupts meta-schema and the
> interrupt-controller schema.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  meta-schemas/interrupts.yaml      | 2 +-
>  schemas/interrupt-controller.yaml | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Applied.

I'll queue the schema up for 5.9. That will mean updating dt-schema is 
required, but that's generally been true frequently anyways.

Rob