diff mbox series

dt-bindings: PCI: snps,dw-pcie: Drop "#interrupt-cells" from example

Message ID 20241105213217.442809-1-robh@kernel.org (mailing list archive)
State Accepted
Delegated to: Krzysztof Wilczyński
Headers show
Series dt-bindings: PCI: snps,dw-pcie: Drop "#interrupt-cells" from example | expand

Commit Message

Rob Herring (Arm) Nov. 5, 2024, 9:32 p.m. UTC
"#interrupt-cells" is not valid without a corresponding "interrupt-map"
or "interrupt-controller" property. As the example has neither, drop
"#interrupt-cells". This fixes a dtc interrupt_provider warning.

Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml | 1 -
 1 file changed, 1 deletion(-)

Comments

Krzysztof Wilczyński Nov. 5, 2024, 10:12 p.m. UTC | #1
Hello,

> "#interrupt-cells" is not valid without a corresponding "interrupt-map"
> or "interrupt-controller" property. As the example has neither, drop
> "#interrupt-cells". This fixes a dtc interrupt_provider warning.

Applied to dt-bindings, thank you!

[01/01] dt-bindings: PCI: snps,dw-pcie: Drop "#interrupt-cells" from example
        https://git.kernel.org/pci/pci/c/718c157a0b94

	Krzysztof
Manivannan Sadhasivam Nov. 15, 2024, 7:26 a.m. UTC | #2
On Tue, Nov 05, 2024 at 03:32:16PM -0600, Rob Herring (Arm) wrote:
> "#interrupt-cells" is not valid without a corresponding "interrupt-map"
> or "interrupt-controller" property. As the example has neither, drop
> "#interrupt-cells". This fixes a dtc interrupt_provider warning.
> 

But the DWC controllers have an in-built MSI controller. Shouldn't we add
'interrrupt-controller' property then?

- Mani

> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
> index 548f59d76ef2..205326fb2d75 100644
> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
> @@ -230,7 +230,6 @@ examples:
>  
>        interrupts = <25>, <24>;
>        interrupt-names = "msi", "hp";
> -      #interrupt-cells = <1>;
>  
>        reset-gpios = <&port0 0 1>;
>  
> -- 
> 2.45.2
>
Rob Herring (Arm) Nov. 15, 2024, 2:07 p.m. UTC | #3
On Fri, Nov 15, 2024 at 1:26 AM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Tue, Nov 05, 2024 at 03:32:16PM -0600, Rob Herring (Arm) wrote:
> > "#interrupt-cells" is not valid without a corresponding "interrupt-map"
> > or "interrupt-controller" property. As the example has neither, drop
> > "#interrupt-cells". This fixes a dtc interrupt_provider warning.
> >
>
> But the DWC controllers have an in-built MSI controller. Shouldn't we add
> 'interrrupt-controller' property then?

Why? Is that needed for the MSI controller to function? I don't think so.

Now we do have "interrupt-controller" present for a number of MSI
providers. I suspect that's there to get OF_DECLARE to work, but I
doubt we really need MSI controllers initialized early.

Rob
Manivannan Sadhasivam Nov. 19, 2024, 5:04 p.m. UTC | #4
On Fri, Nov 15, 2024 at 08:07:07AM -0600, Rob Herring wrote:
> On Fri, Nov 15, 2024 at 1:26 AM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Tue, Nov 05, 2024 at 03:32:16PM -0600, Rob Herring (Arm) wrote:
> > > "#interrupt-cells" is not valid without a corresponding "interrupt-map"
> > > or "interrupt-controller" property. As the example has neither, drop
> > > "#interrupt-cells". This fixes a dtc interrupt_provider warning.
> > >
> >
> > But the DWC controllers have an in-built MSI controller. Shouldn't we add
> > 'interrrupt-controller' property then?
> 
> Why? Is that needed for the MSI controller to function? I don't think so.
> 

No. I was asking from bindings perspective.

> Now we do have "interrupt-controller" present for a number of MSI
> providers. I suspect that's there to get OF_DECLARE to work, but I
> doubt we really need MSI controllers initialized early.
> 

Again no, for this case. I was under the assumption that all interrupt
providers should have the 'interrupt-controller' property in their nodes.

- Mani
Rob Herring (Arm) Nov. 19, 2024, 5:26 p.m. UTC | #5
On Tue, Nov 19, 2024 at 11:04 AM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Fri, Nov 15, 2024 at 08:07:07AM -0600, Rob Herring wrote:
> > On Fri, Nov 15, 2024 at 1:26 AM Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > >
> > > On Tue, Nov 05, 2024 at 03:32:16PM -0600, Rob Herring (Arm) wrote:
> > > > "#interrupt-cells" is not valid without a corresponding "interrupt-map"
> > > > or "interrupt-controller" property. As the example has neither, drop
> > > > "#interrupt-cells". This fixes a dtc interrupt_provider warning.
> > > >
> > >
> > > But the DWC controllers have an in-built MSI controller. Shouldn't we add
> > > 'interrrupt-controller' property then?
> >
> > Why? Is that needed for the MSI controller to function? I don't think so.
> >
>
> No. I was asking from bindings perspective.
>
> > Now we do have "interrupt-controller" present for a number of MSI
> > providers. I suspect that's there to get OF_DECLARE to work, but I
> > doubt we really need MSI controllers initialized early.
> >
>
> Again no, for this case. I was under the assumption that all interrupt
> providers should have the 'interrupt-controller' property in their nodes.

Yes. What interrupts is the DW controller providing? Only the PCI
legacy interrupts which are optional. An msi-controller and an
interrupt-controller are 2 distinct providers. An MSI provider is not
an interrupt provider, but an interrupt consumer. Some bindings define
both, but I think many of those cases are probably wrong.

Rob
Manivannan Sadhasivam Nov. 19, 2024, 5:31 p.m. UTC | #6
On Tue, Nov 19, 2024 at 11:26:57AM -0600, Rob Herring wrote:
> On Tue, Nov 19, 2024 at 11:04 AM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Fri, Nov 15, 2024 at 08:07:07AM -0600, Rob Herring wrote:
> > > On Fri, Nov 15, 2024 at 1:26 AM Manivannan Sadhasivam
> > > <manivannan.sadhasivam@linaro.org> wrote:
> > > >
> > > > On Tue, Nov 05, 2024 at 03:32:16PM -0600, Rob Herring (Arm) wrote:
> > > > > "#interrupt-cells" is not valid without a corresponding "interrupt-map"
> > > > > or "interrupt-controller" property. As the example has neither, drop
> > > > > "#interrupt-cells". This fixes a dtc interrupt_provider warning.
> > > > >
> > > >
> > > > But the DWC controllers have an in-built MSI controller. Shouldn't we add
> > > > 'interrrupt-controller' property then?
> > >
> > > Why? Is that needed for the MSI controller to function? I don't think so.
> > >
> >
> > No. I was asking from bindings perspective.
> >
> > > Now we do have "interrupt-controller" present for a number of MSI
> > > providers. I suspect that's there to get OF_DECLARE to work, but I
> > > doubt we really need MSI controllers initialized early.
> > >
> >
> > Again no, for this case. I was under the assumption that all interrupt
> > providers should have the 'interrupt-controller' property in their nodes.
> 
> Yes. What interrupts is the DW controller providing? Only the PCI
> legacy interrupts which are optional. An msi-controller and an
> interrupt-controller are 2 distinct providers. An MSI provider is not
> an interrupt provider, but an interrupt consumer. Some bindings define
> both, but I think many of those cases are probably wrong.
> 

Ok, thanks for clarifying.

- Mani
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index 548f59d76ef2..205326fb2d75 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -230,7 +230,6 @@  examples:
 
       interrupts = <25>, <24>;
       interrupt-names = "msi", "hp";
-      #interrupt-cells = <1>;
 
       reset-gpios = <&port0 0 1>;