* Re: [PATCH] dt-bindings: Drop required 'interrupt-parent'
2022-01-07 3:19 [PATCH] dt-bindings: Drop required 'interrupt-parent' Rob Herring
@ 2022-01-07 9:31 ` Bartosz Golaszewski
2022-01-07 14:26 ` Suman Anna
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2022-01-07 9:31 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Nobuhiro Iwamatsu, Jassi Brar, Charles Keepax,
Richard Fitzgerald, Lee Jones, David S. Miller, Jakub Kicinski,
Paul Walmsley, Greentime Hu, Bjorn Helgaas, Palmer Dabbelt,
Michal Simek, Suman Anna, -,
John Crispin, Hauke Mehrtens, Kumar Gogada,
open list:GPIO SUBSYSTEM, devicetree, Linux ARM,
Linux Kernel Mailing List, Linux-ALSA, netdev, linux-pci,
linux-riscv
On Fri, Jan 7, 2022 at 4:19 AM Rob Herring <robh@kernel.org> wrote:
>
> 'interrupt-parent' is never required as it can be in a parent node or a
> parent node itself can be an interrupt provider. Where exactly it lives is
> outside the scope of a binding schema.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> .../devicetree/bindings/gpio/toshiba,gpio-visconti.yaml | 1 -
> .../devicetree/bindings/mailbox/ti,omap-mailbox.yaml | 9 ---------
> Documentation/devicetree/bindings/mfd/cirrus,madera.yaml | 1 -
> .../devicetree/bindings/net/lantiq,etop-xway.yaml | 1 -
> .../devicetree/bindings/net/lantiq,xrx200-net.yaml | 1 -
> .../devicetree/bindings/pci/sifive,fu740-pcie.yaml | 1 -
> .../devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 -
> 7 files changed, 15 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml b/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
> index 9ad470e01953..b085450b527f 100644
> --- a/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
> +++ b/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
> @@ -43,7 +43,6 @@ required:
> - gpio-controller
> - interrupt-controller
> - "#interrupt-cells"
> - - interrupt-parent
>
> additionalProperties: false
>
> diff --git a/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
> index e864d798168d..d433e496ec6e 100644
> --- a/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
> +++ b/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
> @@ -175,15 +175,6 @@ required:
> - ti,mbox-num-fifos
>
> allOf:
> - - if:
> - properties:
> - compatible:
> - enum:
> - - ti,am654-mailbox
> - then:
> - required:
> - - interrupt-parent
> -
> - if:
> properties:
> compatible:
> diff --git a/Documentation/devicetree/bindings/mfd/cirrus,madera.yaml b/Documentation/devicetree/bindings/mfd/cirrus,madera.yaml
> index 499c62c04daa..5dce62a7eff2 100644
> --- a/Documentation/devicetree/bindings/mfd/cirrus,madera.yaml
> +++ b/Documentation/devicetree/bindings/mfd/cirrus,madera.yaml
> @@ -221,7 +221,6 @@ required:
> - '#gpio-cells'
> - interrupt-controller
> - '#interrupt-cells'
> - - interrupt-parent
> - interrupts
> - AVDD-supply
> - DBVDD1-supply
> diff --git a/Documentation/devicetree/bindings/net/lantiq,etop-xway.yaml b/Documentation/devicetree/bindings/net/lantiq,etop-xway.yaml
> index 437502c5ca96..3ce9f9a16baf 100644
> --- a/Documentation/devicetree/bindings/net/lantiq,etop-xway.yaml
> +++ b/Documentation/devicetree/bindings/net/lantiq,etop-xway.yaml
> @@ -46,7 +46,6 @@ properties:
> required:
> - compatible
> - reg
> - - interrupt-parent
> - interrupts
> - interrupt-names
> - lantiq,tx-burst-length
> diff --git a/Documentation/devicetree/bindings/net/lantiq,xrx200-net.yaml b/Documentation/devicetree/bindings/net/lantiq,xrx200-net.yaml
> index 7bc074a42369..5bc1a21ca579 100644
> --- a/Documentation/devicetree/bindings/net/lantiq,xrx200-net.yaml
> +++ b/Documentation/devicetree/bindings/net/lantiq,xrx200-net.yaml
> @@ -38,7 +38,6 @@ properties:
> required:
> - compatible
> - reg
> - - interrupt-parent
> - interrupts
> - interrupt-names
> - "#address-cells"
> diff --git a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
> index 2b9d1d6fc661..72c78f4ec269 100644
> --- a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
> @@ -61,7 +61,6 @@ required:
> - num-lanes
> - interrupts
> - interrupt-names
> - - interrupt-parent
> - interrupt-map-mask
> - interrupt-map
> - clock-names
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> index a2bbc0eb7220..32f4641085bc 100644
> --- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> @@ -55,7 +55,6 @@ required:
> - reg-names
> - "#interrupt-cells"
> - interrupts
> - - interrupt-parent
> - interrupt-map
> - interrupt-map-mask
> - bus-range
> --
> 2.32.0
>
For GPIO:
Acked-by: Bartosz Golaszewski <brgl@bgdev.pl>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] dt-bindings: Drop required 'interrupt-parent'
2022-01-07 3:19 [PATCH] dt-bindings: Drop required 'interrupt-parent' Rob Herring
2022-01-07 9:31 ` Bartosz Golaszewski
@ 2022-01-07 14:26 ` Suman Anna
2022-01-07 15:20 ` Rob Herring
2022-01-10 10:33 ` Lee Jones
2022-01-11 17:51 ` Rob Herring
3 siblings, 1 reply; 8+ messages in thread
From: Suman Anna @ 2022-01-07 14:26 UTC (permalink / raw)
To: Rob Herring, Linus Walleij, Bartosz Golaszewski,
Nobuhiro Iwamatsu, Jassi Brar, Charles Keepax,
Richard Fitzgerald, Lee Jones, David S. Miller, Jakub Kicinski,
Paul Walmsley, Greentime Hu, Bjorn Helgaas, Palmer Dabbelt,
Michal Simek, -,
John Crispin, Hauke Mehrtens, Kumar Gogada
Cc: linux-gpio, devicetree, linux-arm-kernel, linux-kernel,
alsa-devel, netdev, linux-pci, linux-riscv, Nagalla, Hari
Hi Rob,
On 1/6/22 9:19 PM, Rob Herring wrote:
> 'interrupt-parent' is never required as it can be in a parent node or a
> parent node itself can be an interrupt provider. Where exactly it lives is
> outside the scope of a binding schema.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> .../devicetree/bindings/gpio/toshiba,gpio-visconti.yaml | 1 -
> .../devicetree/bindings/mailbox/ti,omap-mailbox.yaml | 9 ---------
> Documentation/devicetree/bindings/mfd/cirrus,madera.yaml | 1 -
> .../devicetree/bindings/net/lantiq,etop-xway.yaml | 1 -
> .../devicetree/bindings/net/lantiq,xrx200-net.yaml | 1 -
> .../devicetree/bindings/pci/sifive,fu740-pcie.yaml | 1 -
> .../devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 -
> 7 files changed, 15 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml b/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
> index 9ad470e01953..b085450b527f 100644
> --- a/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
> +++ b/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
> @@ -43,7 +43,6 @@ required:
> - gpio-controller
> - interrupt-controller
> - "#interrupt-cells"
> - - interrupt-parent
>
> additionalProperties: false
>
> diff --git a/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
> index e864d798168d..d433e496ec6e 100644
> --- a/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
> +++ b/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
> @@ -175,15 +175,6 @@ required:
> - ti,mbox-num-fifos
>
> allOf:
> - - if:
> - properties:
> - compatible:
> - enum:
> - - ti,am654-mailbox
> - then:
> - required:
> - - interrupt-parent
> -
There are multiple interrupt controllers on TI K3 devices, and we need this
property to be defined _specifically_ to point to the relevant interrupt router
parent node.
While what you state in general is true, I cannot have a node not define this on
K3 devices, and end up using the wrong interrupt parent (GIC
interrupt-controller). That's why the conditional compatible check.
regards
Suman
> - if:
> properties:
> compatible:
> diff --git a/Documentation/devicetree/bindings/mfd/cirrus,madera.yaml b/Documentation/devicetree/bindings/mfd/cirrus,madera.yaml
> index 499c62c04daa..5dce62a7eff2 100644
> --- a/Documentation/devicetree/bindings/mfd/cirrus,madera.yaml
> +++ b/Documentation/devicetree/bindings/mfd/cirrus,madera.yaml
> @@ -221,7 +221,6 @@ required:
> - '#gpio-cells'
> - interrupt-controller
> - '#interrupt-cells'
> - - interrupt-parent
> - interrupts
> - AVDD-supply
> - DBVDD1-supply
> diff --git a/Documentation/devicetree/bindings/net/lantiq,etop-xway.yaml b/Documentation/devicetree/bindings/net/lantiq,etop-xway.yaml
> index 437502c5ca96..3ce9f9a16baf 100644
> --- a/Documentation/devicetree/bindings/net/lantiq,etop-xway.yaml
> +++ b/Documentation/devicetree/bindings/net/lantiq,etop-xway.yaml
> @@ -46,7 +46,6 @@ properties:
> required:
> - compatible
> - reg
> - - interrupt-parent
> - interrupts
> - interrupt-names
> - lantiq,tx-burst-length
> diff --git a/Documentation/devicetree/bindings/net/lantiq,xrx200-net.yaml b/Documentation/devicetree/bindings/net/lantiq,xrx200-net.yaml
> index 7bc074a42369..5bc1a21ca579 100644
> --- a/Documentation/devicetree/bindings/net/lantiq,xrx200-net.yaml
> +++ b/Documentation/devicetree/bindings/net/lantiq,xrx200-net.yaml
> @@ -38,7 +38,6 @@ properties:
> required:
> - compatible
> - reg
> - - interrupt-parent
> - interrupts
> - interrupt-names
> - "#address-cells"
> diff --git a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
> index 2b9d1d6fc661..72c78f4ec269 100644
> --- a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
> @@ -61,7 +61,6 @@ required:
> - num-lanes
> - interrupts
> - interrupt-names
> - - interrupt-parent
> - interrupt-map-mask
> - interrupt-map
> - clock-names
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> index a2bbc0eb7220..32f4641085bc 100644
> --- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> @@ -55,7 +55,6 @@ required:
> - reg-names
> - "#interrupt-cells"
> - interrupts
> - - interrupt-parent
> - interrupt-map
> - interrupt-map-mask
> - bus-range
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: Drop required 'interrupt-parent'
2022-01-07 14:26 ` Suman Anna
@ 2022-01-07 15:20 ` Rob Herring
2022-01-07 16:29 ` Suman Anna
0 siblings, 1 reply; 8+ messages in thread
From: Rob Herring @ 2022-01-07 15:20 UTC (permalink / raw)
To: Suman Anna
Cc: Linus Walleij, Bartosz Golaszewski, Nobuhiro Iwamatsu,
Jassi Brar, Charles Keepax, Richard Fitzgerald, Lee Jones,
David S. Miller, Jakub Kicinski, Paul Walmsley, Greentime Hu,
Bjorn Helgaas, Palmer Dabbelt, Michal Simek, -,
John Crispin, Hauke Mehrtens, Kumar Gogada,
open list:GPIO SUBSYSTEM, devicetree, linux-arm-kernel,
linux-kernel, Linux-ALSA, netdev, PCI, linux-riscv, Nagalla,
Hari
On Fri, Jan 7, 2022 at 8:27 AM Suman Anna <s-anna@ti.com> wrote:
>
> Hi Rob,
>
> On 1/6/22 9:19 PM, Rob Herring wrote:
> > 'interrupt-parent' is never required as it can be in a parent node or a
> > parent node itself can be an interrupt provider. Where exactly it lives is
> > outside the scope of a binding schema.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > .../devicetree/bindings/gpio/toshiba,gpio-visconti.yaml | 1 -
> > .../devicetree/bindings/mailbox/ti,omap-mailbox.yaml | 9 ---------
> > Documentation/devicetree/bindings/mfd/cirrus,madera.yaml | 1 -
> > .../devicetree/bindings/net/lantiq,etop-xway.yaml | 1 -
> > .../devicetree/bindings/net/lantiq,xrx200-net.yaml | 1 -
> > .../devicetree/bindings/pci/sifive,fu740-pcie.yaml | 1 -
> > .../devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 -
> > 7 files changed, 15 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml b/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
> > index 9ad470e01953..b085450b527f 100644
> > --- a/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
> > +++ b/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
> > @@ -43,7 +43,6 @@ required:
> > - gpio-controller
> > - interrupt-controller
> > - "#interrupt-cells"
> > - - interrupt-parent
> >
> > additionalProperties: false
> >
> > diff --git a/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
> > index e864d798168d..d433e496ec6e 100644
> > --- a/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
> > +++ b/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
> > @@ -175,15 +175,6 @@ required:
> > - ti,mbox-num-fifos
> >
> > allOf:
> > - - if:
> > - properties:
> > - compatible:
> > - enum:
> > - - ti,am654-mailbox
> > - then:
> > - required:
> > - - interrupt-parent
> > -
>
> There are multiple interrupt controllers on TI K3 devices, and we need this
> property to be defined _specifically_ to point to the relevant interrupt router
> parent node.
>
> While what you state in general is true, I cannot have a node not define this on
> K3 devices, and end up using the wrong interrupt parent (GIC
> interrupt-controller). That's why the conditional compatible check.
But you could.
The parent node can have a default interrupt-parent and child nodes
can override that. It doesn't matter which one is the default though
typically you would want the one used the most to be the default.
Looking at your dts files, it looks like you all did the opposite. The
only way that wouldn't work is if the parent node is if the parent
node has its own 'interrupts' or you are just abusing
'interrupt-parent' where the standard parsing doesn't work.
You are also free to use 'interrupts-extended' anywhere 'interrupts'
is used and then interrupt-parent being present is an error. How you
structure all this is outside the scope of binding schemas which only
need to define how many interrupts and what are they. Ensuring parents
and cell sizes are correct is mostly done by dtc.
Rob
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: Drop required 'interrupt-parent'
2022-01-07 15:20 ` Rob Herring
@ 2022-01-07 16:29 ` Suman Anna
2022-01-07 17:03 ` Rob Herring
0 siblings, 1 reply; 8+ messages in thread
From: Suman Anna @ 2022-01-07 16:29 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Bartosz Golaszewski, Nobuhiro Iwamatsu,
Jassi Brar, Charles Keepax, Richard Fitzgerald, Lee Jones,
David S. Miller, Jakub Kicinski, Paul Walmsley, Greentime Hu,
Bjorn Helgaas, Palmer Dabbelt, Michal Simek, -,
John Crispin, Hauke Mehrtens, Kumar Gogada,
open list:GPIO SUBSYSTEM, devicetree, linux-arm-kernel,
linux-kernel, Linux-ALSA, netdev, PCI, linux-riscv, Nagalla,
Hari, Menon, Nishanth, Vignesh R
Hi Rob,
On 1/7/22 9:20 AM, Rob Herring wrote:
> On Fri, Jan 7, 2022 at 8:27 AM Suman Anna <s-anna@ti.com> wrote:
>>
>> Hi Rob,
>>
>> On 1/6/22 9:19 PM, Rob Herring wrote:
>>> 'interrupt-parent' is never required as it can be in a parent node or a
>>> parent node itself can be an interrupt provider. Where exactly it lives is
>>> outside the scope of a binding schema.
>>>
>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>> ---
>>> .../devicetree/bindings/gpio/toshiba,gpio-visconti.yaml | 1 -
>>> .../devicetree/bindings/mailbox/ti,omap-mailbox.yaml | 9 ---------
>>> Documentation/devicetree/bindings/mfd/cirrus,madera.yaml | 1 -
>>> .../devicetree/bindings/net/lantiq,etop-xway.yaml | 1 -
>>> .../devicetree/bindings/net/lantiq,xrx200-net.yaml | 1 -
>>> .../devicetree/bindings/pci/sifive,fu740-pcie.yaml | 1 -
>>> .../devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 -
>>> 7 files changed, 15 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml b/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
>>> index 9ad470e01953..b085450b527f 100644
>>> --- a/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
>>> +++ b/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
>>> @@ -43,7 +43,6 @@ required:
>>> - gpio-controller
>>> - interrupt-controller
>>> - "#interrupt-cells"
>>> - - interrupt-parent
>>>
>>> additionalProperties: false
>>>
>>> diff --git a/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
>>> index e864d798168d..d433e496ec6e 100644
>>> --- a/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
>>> +++ b/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
>>> @@ -175,15 +175,6 @@ required:
>>> - ti,mbox-num-fifos
>>>
>>> allOf:
>>> - - if:
>>> - properties:
>>> - compatible:
>>> - enum:
>>> - - ti,am654-mailbox
>>> - then:
>>> - required:
>>> - - interrupt-parent
>>> -
>>
>> There are multiple interrupt controllers on TI K3 devices, and we need this
>> property to be defined _specifically_ to point to the relevant interrupt router
>> parent node.
>>
>> While what you state in general is true, I cannot have a node not define this on
>> K3 devices, and end up using the wrong interrupt parent (GIC
>> interrupt-controller). That's why the conditional compatible check.
>
> But you could.
>
> The parent node can have a default interrupt-parent and child nodes
> can override that. It doesn't matter which one is the default though
> typically you would want the one used the most to be the default.
> Looking at your dts files, it looks like you all did the opposite.
Hmm, I am not sure I understood your last comment. Can you point out the
specific usage?
All our K3 dts files have the interrupt-parent = <&gic500> defined at the
root-node, which is the default ARM GIC.
Let us know if we need to fix something in our dts files.
The
> only way that wouldn't work is if the parent node is if the parent
> node has its own 'interrupts' or you are just abusing
> 'interrupt-parent' where the standard parsing doesn't work.
All our K3 gic500 nodes does have an 'interrupts' property.
>
> You are also free to use 'interrupts-extended' anywhere 'interrupts'
> is used and then interrupt-parent being present is an error.
Yes, this is understood. The OMAP Mailbox binding is reused between multiple SoC
families, some of which do not use an Interrupt Router in between.
So, whats the best way to enforce this in the specific schema? I have used the
common 'interrupts' property that applies to all SoCs, and enforced the
conditional 'interrupt-parent' only on relevant compatibles.
regards
Suman
How you
> structure all this is outside the scope of binding schemas which only
> need to define how many interrupts and what are they. Ensuring parents
> and cell sizes are correct is mostly done by dtc.
>
> Rob
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: Drop required 'interrupt-parent'
2022-01-07 16:29 ` Suman Anna
@ 2022-01-07 17:03 ` Rob Herring
0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2022-01-07 17:03 UTC (permalink / raw)
To: Suman Anna
Cc: Linus Walleij, Bartosz Golaszewski, Nobuhiro Iwamatsu,
Jassi Brar, Charles Keepax, Richard Fitzgerald, Lee Jones,
David S. Miller, Jakub Kicinski, Paul Walmsley, Greentime Hu,
Bjorn Helgaas, Palmer Dabbelt, Michal Simek, -,
John Crispin, Hauke Mehrtens, Kumar Gogada,
open list:GPIO SUBSYSTEM, devicetree, linux-arm-kernel,
linux-kernel, Linux-ALSA, netdev, PCI, linux-riscv, Nagalla,
Hari, Menon, Nishanth, Vignesh R
On Fri, Jan 7, 2022 at 10:29 AM Suman Anna <s-anna@ti.com> wrote:
>
> Hi Rob,
>
> On 1/7/22 9:20 AM, Rob Herring wrote:
> > On Fri, Jan 7, 2022 at 8:27 AM Suman Anna <s-anna@ti.com> wrote:
> >>
> >> Hi Rob,
> >>
> >> On 1/6/22 9:19 PM, Rob Herring wrote:
> >>> 'interrupt-parent' is never required as it can be in a parent node or a
> >>> parent node itself can be an interrupt provider. Where exactly it lives is
> >>> outside the scope of a binding schema.
> >>>
> >>> Signed-off-by: Rob Herring <robh@kernel.org>
> >>> ---
> >>> .../devicetree/bindings/gpio/toshiba,gpio-visconti.yaml | 1 -
> >>> .../devicetree/bindings/mailbox/ti,omap-mailbox.yaml | 9 ---------
> >>> Documentation/devicetree/bindings/mfd/cirrus,madera.yaml | 1 -
> >>> .../devicetree/bindings/net/lantiq,etop-xway.yaml | 1 -
> >>> .../devicetree/bindings/net/lantiq,xrx200-net.yaml | 1 -
> >>> .../devicetree/bindings/pci/sifive,fu740-pcie.yaml | 1 -
> >>> .../devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 -
> >>> 7 files changed, 15 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml b/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
> >>> index 9ad470e01953..b085450b527f 100644
> >>> --- a/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
> >>> +++ b/Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
> >>> @@ -43,7 +43,6 @@ required:
> >>> - gpio-controller
> >>> - interrupt-controller
> >>> - "#interrupt-cells"
> >>> - - interrupt-parent
> >>>
> >>> additionalProperties: false
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
> >>> index e864d798168d..d433e496ec6e 100644
> >>> --- a/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
> >>> +++ b/Documentation/devicetree/bindings/mailbox/ti,omap-mailbox.yaml
> >>> @@ -175,15 +175,6 @@ required:
> >>> - ti,mbox-num-fifos
> >>>
> >>> allOf:
> >>> - - if:
> >>> - properties:
> >>> - compatible:
> >>> - enum:
> >>> - - ti,am654-mailbox
> >>> - then:
> >>> - required:
> >>> - - interrupt-parent
> >>> -
> >>
> >> There are multiple interrupt controllers on TI K3 devices, and we need this
> >> property to be defined _specifically_ to point to the relevant interrupt router
> >> parent node.
> >>
> >> While what you state in general is true, I cannot have a node not define this on
> >> K3 devices, and end up using the wrong interrupt parent (GIC
> >> interrupt-controller). That's why the conditional compatible check.
> >
> > But you could.
> >
> > The parent node can have a default interrupt-parent and child nodes
> > can override that. It doesn't matter which one is the default though
> > typically you would want the one used the most to be the default.
> > Looking at your dts files, it looks like you all did the opposite.
>
> Hmm, I am not sure I understood your last comment. Can you point out the
> specific usage?
Perhaps an example. These are all equivalent:
parent {
child1 {
interrupt-parent = <&intc1>;
interrupts = <1>;
};
child2 {
interrupt-parent = <&intc2>;
interrupts = <2>;
};
};
parent {
interrupt-parent = <&intc1>; // Or in the parent's parent...
child1 {
interrupts = <1>;
};
child2 {
interrupt-parent = <&intc2>;
interrupts = <2>;
};
};
parent {
interrupt-parent = <&intc2>;
child1 {
interrupt-parent = <&intc1>;
interrupts = <1>;
};
child2 {
interrupts = <2>;
};
};
You could structure main_navss and child nodes in any of these 3 ways.
>
> All our K3 dts files have the interrupt-parent = <&gic500> defined at the
> root-node, which is the default ARM GIC.
>
> Let us know if we need to fix something in our dts files.
No! I'm just saying there are multiple correct ways to write the dts files.
> The
> > only way that wouldn't work is if the parent node is if the parent
> > node has its own 'interrupts' or you are just abusing
> > 'interrupt-parent' where the standard parsing doesn't work.
>
> All our K3 gic500 nodes does have an 'interrupts' property.
I said the parent node, not the 'interrupt-parent'. In this case, the
parent is 'main_navss: bus@30800000'. It doesn't have 'interrupts' in
your case, so only the 2nd case is a possibility.
> > You are also free to use 'interrupts-extended' anywhere 'interrupts'
> > is used and then interrupt-parent being present is an error.
>
> Yes, this is understood. The OMAP Mailbox binding is reused between multiple SoC
> families, some of which do not use an Interrupt Router in between.
>
> So, whats the best way to enforce this in the specific schema? I have used the
> common 'interrupts' property that applies to all SoCs, and enforced the
> conditional 'interrupt-parent' only on relevant compatibles.
You can't. There is no way a schema can ensure you connect the right
interrupt controller just as it can't ensure you used the right
interrupt number and flags or used the right addresses. Well, you
could technically, but then at that point we could just generate the
dts from the schema.
Rob
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: Drop required 'interrupt-parent'
2022-01-07 3:19 [PATCH] dt-bindings: Drop required 'interrupt-parent' Rob Herring
2022-01-07 9:31 ` Bartosz Golaszewski
2022-01-07 14:26 ` Suman Anna
@ 2022-01-10 10:33 ` Lee Jones
2022-01-11 17:51 ` Rob Herring
3 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2022-01-10 10:33 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Bartosz Golaszewski, Nobuhiro Iwamatsu,
Jassi Brar, Charles Keepax, Richard Fitzgerald, David S. Miller,
Jakub Kicinski, Paul Walmsley, Greentime Hu, Bjorn Helgaas,
Palmer Dabbelt, Michal Simek, Suman Anna, -,
John Crispin, Hauke Mehrtens, Kumar Gogada, linux-gpio,
devicetree, linux-arm-kernel, linux-kernel, alsa-devel, netdev,
linux-pci, linux-riscv
On Thu, 06 Jan 2022, Rob Herring wrote:
> 'interrupt-parent' is never required as it can be in a parent node or a
> parent node itself can be an interrupt provider. Where exactly it lives is
> outside the scope of a binding schema.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> .../devicetree/bindings/gpio/toshiba,gpio-visconti.yaml | 1 -
> .../devicetree/bindings/mailbox/ti,omap-mailbox.yaml | 9 ---------
> Documentation/devicetree/bindings/mfd/cirrus,madera.yaml | 1 -
Acked-by: Lee Jones <lee.jones@linaro.org>
> .../devicetree/bindings/net/lantiq,etop-xway.yaml | 1 -
> .../devicetree/bindings/net/lantiq,xrx200-net.yaml | 1 -
> .../devicetree/bindings/pci/sifive,fu740-pcie.yaml | 1 -
> .../devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 -
> 7 files changed, 15 deletions(-)
--
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dt-bindings: Drop required 'interrupt-parent'
2022-01-07 3:19 [PATCH] dt-bindings: Drop required 'interrupt-parent' Rob Herring
` (2 preceding siblings ...)
2022-01-10 10:33 ` Lee Jones
@ 2022-01-11 17:51 ` Rob Herring
3 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2022-01-11 17:51 UTC (permalink / raw)
To: Rob Herring
Cc: linux-gpio, Jassi Brar, -,
linux-kernel, Kumar Gogada, Lee Jones, Bartosz Golaszewski,
John Crispin, linux-arm-kernel, Greentime Hu, Jakub Kicinski,
Paul Walmsley, Bjorn Helgaas, Charles Keepax, devicetree,
alsa-devel, linux-riscv, Richard Fitzgerald, linux-pci,
Hauke Mehrtens, Linus Walleij, Nobuhiro Iwamatsu, Suman Anna,
netdev, Palmer Dabbelt, Michal Simek, David S. Miller
On Thu, 06 Jan 2022 21:19:04 -0600, Rob Herring wrote:
> 'interrupt-parent' is never required as it can be in a parent node or a
> parent node itself can be an interrupt provider. Where exactly it lives is
> outside the scope of a binding schema.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> .../devicetree/bindings/gpio/toshiba,gpio-visconti.yaml | 1 -
> .../devicetree/bindings/mailbox/ti,omap-mailbox.yaml | 9 ---------
> Documentation/devicetree/bindings/mfd/cirrus,madera.yaml | 1 -
> .../devicetree/bindings/net/lantiq,etop-xway.yaml | 1 -
> .../devicetree/bindings/net/lantiq,xrx200-net.yaml | 1 -
> .../devicetree/bindings/pci/sifive,fu740-pcie.yaml | 1 -
> .../devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 -
> 7 files changed, 15 deletions(-)
>
Applied, thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread