| Message ID | 20210804132912.30685-2-kishon@ti.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: Add legacy interrupt support in pci-j721e | expand |
On Wed, 04 Aug 2021 14:29:10 +0100, Kishon Vijay Abraham I <kishon@ti.com> wrote: > > Add bindings to specify interrupt controller for legacy interrupts. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > .../bindings/pci/ti,j721e-pci-host.yaml | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml > index cc900202df29..f461d7b4c0cc 100644 > --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml > +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml > @@ -74,6 +74,11 @@ properties: > > msi-map: true > > +patternProperties: > + "interrupt-controller": > + type: object > + description: interrupt controller to handle legacy interrupts. > + > required: > - compatible > - reg > @@ -97,6 +102,8 @@ unevaluatedProperties: false > > examples: > - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/soc/ti,sci_pm_domain.h> > #include <dt-bindings/gpio/gpio.h> > > @@ -131,5 +138,13 @@ examples: > ranges = <0x01000000 0x0 0x10001000 0x00 0x10001000 0x0 0x0010000>, > <0x02000000 0x0 0x10011000 0x00 0x10011000 0x0 0x7fef000>; > dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x10000 0x0>; > + > + > + pcie0_intc: interrupt-controller { > + interrupt-controller; > + #interrupt-cells = <1>; > + interrupt-parent = <&gic500>; > + interrupts = <GIC_SPI 312 IRQ_TYPE_EDGE_RISING>; Are you sure about the edge signalling? How is the interrupt retriggered when the input is still high, which could well be the case for shared INTx? Thanks, M.
Hi Marc, On 04/08/21 8:35 pm, Marc Zyngier wrote: > On Wed, 04 Aug 2021 14:29:10 +0100, > Kishon Vijay Abraham I <kishon@ti.com> wrote: >> >> Add bindings to specify interrupt controller for legacy interrupts. >> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> .../bindings/pci/ti,j721e-pci-host.yaml | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml >> index cc900202df29..f461d7b4c0cc 100644 >> --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml >> +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml >> @@ -74,6 +74,11 @@ properties: >> >> msi-map: true >> >> +patternProperties: >> + "interrupt-controller": >> + type: object >> + description: interrupt controller to handle legacy interrupts. >> + >> required: >> - compatible >> - reg >> @@ -97,6 +102,8 @@ unevaluatedProperties: false >> >> examples: >> - | >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + #include <dt-bindings/interrupt-controller/irq.h> >> #include <dt-bindings/soc/ti,sci_pm_domain.h> >> #include <dt-bindings/gpio/gpio.h> >> >> @@ -131,5 +138,13 @@ examples: >> ranges = <0x01000000 0x0 0x10001000 0x00 0x10001000 0x0 0x0010000>, >> <0x02000000 0x0 0x10011000 0x00 0x10011000 0x0 0x7fef000>; >> dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x10000 0x0>; >> + >> + >> + pcie0_intc: interrupt-controller { >> + interrupt-controller; >> + #interrupt-cells = <1>; >> + interrupt-parent = <&gic500>; >> + interrupts = <GIC_SPI 312 IRQ_TYPE_EDGE_RISING>; > > Are you sure about the edge signalling? How is the interrupt > retriggered when the input is still high, which could well be the case > for shared INTx? There is a EOI register which is used for re-triggering the interrupt. That functionality is broken in J721E but is fixed in J7200 (the following two patches in the series deals with that). Thanks, Kishon
On Wed, Aug 04, 2021 at 06:59:10PM +0530, Kishon Vijay Abraham I wrote: > Add bindings to specify interrupt controller for legacy interrupts. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > .../bindings/pci/ti,j721e-pci-host.yaml | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml > index cc900202df29..f461d7b4c0cc 100644 > --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml > +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml > @@ -74,6 +74,11 @@ properties: > > msi-map: true > > +patternProperties: > + "interrupt-controller": Not a pattern unless you meant for foo-interrupt-controller-bar to be valid. Anything is allowed in the node? > + type: object > + description: interrupt controller to handle legacy interrupts. > + > required: > - compatible > - reg > @@ -97,6 +102,8 @@ unevaluatedProperties: false > > examples: > - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/soc/ti,sci_pm_domain.h> > #include <dt-bindings/gpio/gpio.h> > > @@ -131,5 +138,13 @@ examples: > ranges = <0x01000000 0x0 0x10001000 0x00 0x10001000 0x0 0x0010000>, > <0x02000000 0x0 0x10011000 0x00 0x10011000 0x0 0x7fef000>; > dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x10000 0x0>; > + > + > + pcie0_intc: interrupt-controller { > + interrupt-controller; > + #interrupt-cells = <1>; > + interrupt-parent = <&gic500>; > + interrupts = <GIC_SPI 312 IRQ_TYPE_EDGE_RISING>; > + }; > }; > }; > -- > 2.17.1 > >
Hi Rob, On 13/08/21 10:47 pm, Rob Herring wrote: > On Wed, Aug 04, 2021 at 06:59:10PM +0530, Kishon Vijay Abraham I wrote: >> Add bindings to specify interrupt controller for legacy interrupts. >> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> .../bindings/pci/ti,j721e-pci-host.yaml | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml >> index cc900202df29..f461d7b4c0cc 100644 >> --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml >> +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml >> @@ -74,6 +74,11 @@ properties: >> >> msi-map: true >> >> +patternProperties: >> + "interrupt-controller": > > Not a pattern unless you meant for foo-interrupt-controller-bar to be > valid. > > Anything is allowed in the node? It's same as whatever is defined in schemas/interrupt-controller.yaml, just that it should be a subnode of pcie@. Should I add whatever is present in schemas/interrupt-controller.yaml here? Thanks Kishon > >> + type: object >> + description: interrupt controller to handle legacy interrupts. >> + >> required: >> - compatible >> - reg >> @@ -97,6 +102,8 @@ unevaluatedProperties: false >> >> examples: >> - | >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + #include <dt-bindings/interrupt-controller/irq.h> >> #include <dt-bindings/soc/ti,sci_pm_domain.h> >> #include <dt-bindings/gpio/gpio.h> >> >> @@ -131,5 +138,13 @@ examples: >> ranges = <0x01000000 0x0 0x10001000 0x00 0x10001000 0x0 0x0010000>, >> <0x02000000 0x0 0x10011000 0x00 0x10011000 0x0 0x7fef000>; >> dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x10000 0x0>; >> + >> + >> + pcie0_intc: interrupt-controller { >> + interrupt-controller; >> + #interrupt-cells = <1>; >> + interrupt-parent = <&gic500>; >> + interrupts = <GIC_SPI 312 IRQ_TYPE_EDGE_RISING>; >> + }; >> }; >> }; >> -- >> 2.17.1 >> >>
Hi Rob, On 18/08/21 7:28 pm, Kishon Vijay Abraham I wrote: > Hi Rob, > > On 13/08/21 10:47 pm, Rob Herring wrote: >> On Wed, Aug 04, 2021 at 06:59:10PM +0530, Kishon Vijay Abraham I wrote: >>> Add bindings to specify interrupt controller for legacy interrupts. >>> >>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >>> --- >>> .../bindings/pci/ti,j721e-pci-host.yaml | 15 +++++++++++++++ >>> 1 file changed, 15 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml >>> index cc900202df29..f461d7b4c0cc 100644 >>> --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml >>> +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml >>> @@ -74,6 +74,11 @@ properties: >>> >>> msi-map: true >>> >>> +patternProperties: >>> + "interrupt-controller": >> >> Not a pattern unless you meant for foo-interrupt-controller-bar to be >> valid. >> >> Anything is allowed in the node? > > It's same as whatever is defined in schemas/interrupt-controller.yaml, > just that it should be a subnode of pcie@. Should I add whatever is > present in schemas/interrupt-controller.yaml here? Can you suggest how to include this? Thanks, Kishon > > Thanks > Kishon >> >>> + type: object >>> + description: interrupt controller to handle legacy interrupts. >>> + >>> required: >>> - compatible >>> - reg >>> @@ -97,6 +102,8 @@ unevaluatedProperties: false >>> >>> examples: >>> - | >>> + #include <dt-bindings/interrupt-controller/arm-gic.h> >>> + #include <dt-bindings/interrupt-controller/irq.h> >>> #include <dt-bindings/soc/ti,sci_pm_domain.h> >>> #include <dt-bindings/gpio/gpio.h> >>> >>> @@ -131,5 +138,13 @@ examples: >>> ranges = <0x01000000 0x0 0x10001000 0x00 0x10001000 0x0 0x0010000>, >>> <0x02000000 0x0 0x10011000 0x00 0x10011000 0x0 0x7fef000>; >>> dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x10000 0x0>; >>> + >>> + >>> + pcie0_intc: interrupt-controller { >>> + interrupt-controller; >>> + #interrupt-cells = <1>; >>> + interrupt-parent = <&gic500>; >>> + interrupts = <GIC_SPI 312 IRQ_TYPE_EDGE_RISING>; >>> + }; >>> }; >>> }; >>> -- >>> 2.17.1 >>> >>>
On Wed, Sep 22, 2021 at 11:33 PM Kishon Vijay Abraham I <kishon@ti.com> wrote: > > Hi Rob, > > On 18/08/21 7:28 pm, Kishon Vijay Abraham I wrote: > > Hi Rob, > > > > On 13/08/21 10:47 pm, Rob Herring wrote: > >> On Wed, Aug 04, 2021 at 06:59:10PM +0530, Kishon Vijay Abraham I wrote: > >>> Add bindings to specify interrupt controller for legacy interrupts. > >>> > >>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > >>> --- > >>> .../bindings/pci/ti,j721e-pci-host.yaml | 15 +++++++++++++++ > >>> 1 file changed, 15 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml > >>> index cc900202df29..f461d7b4c0cc 100644 > >>> --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml > >>> +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml > >>> @@ -74,6 +74,11 @@ properties: > >>> > >>> msi-map: true > >>> > >>> +patternProperties: > >>> + "interrupt-controller": > >> > >> Not a pattern unless you meant for foo-interrupt-controller-bar to be > >> valid. > >> > >> Anything is allowed in the node? > > > > It's same as whatever is defined in schemas/interrupt-controller.yaml, > > just that it should be a subnode of pcie@. Should I add whatever is > > present in schemas/interrupt-controller.yaml here? > > Can you suggest how to include this? You don't. List the properties you are using. You need to define the #interrupt-cells value for example. I'm sure there's already examples in the tree doing this same child node. Rob
diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml index cc900202df29..f461d7b4c0cc 100644 --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml @@ -74,6 +74,11 @@ properties: msi-map: true +patternProperties: + "interrupt-controller": + type: object + description: interrupt controller to handle legacy interrupts. + required: - compatible - reg @@ -97,6 +102,8 @@ unevaluatedProperties: false examples: - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/soc/ti,sci_pm_domain.h> #include <dt-bindings/gpio/gpio.h> @@ -131,5 +138,13 @@ examples: ranges = <0x01000000 0x0 0x10001000 0x00 0x10001000 0x0 0x0010000>, <0x02000000 0x0 0x10011000 0x00 0x10011000 0x0 0x7fef000>; dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x10000 0x0>; + + + pcie0_intc: interrupt-controller { + interrupt-controller; + #interrupt-cells = <1>; + interrupt-parent = <&gic500>; + interrupts = <GIC_SPI 312 IRQ_TYPE_EDGE_RISING>; + }; }; };
Add bindings to specify interrupt controller for legacy interrupts. Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> --- .../bindings/pci/ti,j721e-pci-host.yaml | 15 +++++++++++++++ 1 file changed, 15 insertions(+)