diff mbox

[v2,13/16] dt-bindings/interrupt-controller: add documentation for Marvell SEI controller

Message ID 20180522094042.24770-14-miquel.raynal@bootlin.com
State New, archived
Headers show

Commit Message

Miquel Raynal May 22, 2018, 9:40 a.m. UTC
Describe the System Error Interrupt (SEI) controller. It aggregates two
types of interrupts, wired and MSIs from respectively the AP and the
CPs, into a single SPI interrupt.

Suggested-by: Haim Boot <hayim@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/interrupt-controller/marvell,sei.txt  | 50 ++++++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt

Comments

Rob Herring June 5, 2018, 8:51 p.m. UTC | #1
On Tue, May 22, 2018 at 11:40:39AM +0200, Miquel Raynal wrote:
> Describe the System Error Interrupt (SEI) controller. It aggregates two
> types of interrupts, wired and MSIs from respectively the AP and the
> CPs, into a single SPI interrupt.
> 
> Suggested-by: Haim Boot <hayim@marvell.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../bindings/interrupt-controller/marvell,sei.txt  | 50 ++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> new file mode 100644
> index 000000000000..689981036c30
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> @@ -0,0 +1,50 @@
> +Marvell SEI (System Error Interrupt) Controller
> +-----------------------------------------------
> +
> +Marvell SEI (System Error Interrupt) controller is an interrupt
> +aggregator. It receives interrupts from several sources and aggregates
> +them to a single interrupt line (an SPI) on the parent interrupt
> +controller.
> +
> +This interrupt controller can handle up to 64 SEIs, a set comes from the
> +AP and is wired while a second set comes from the CPs by the mean of
> +MSIs. Each 'domain' is represented as a subnode.
> +
> +Required properties:
> +
> +- compatible: should be "marvell,armada-8k-sei".
> +- reg: SEI registers location and length.
> +- interrupts: identifies the parent IRQ that will be triggered.
> +
> +Child node 'sei-wired-controller' required properties:
> +
> +- marvell,sei-ranges: ranges of wired interrupts.
> +- #interrupt-cells: number of cells to define an SEI wired interrupt
> +                    coming from the AP, should be 1. The cell is the IRQ
> +                    number.
> +- interrupt-controller: identifies the node as an interrupt controller.
> +
> +Child node 'sei-msi-controller' required properties:
> +
> +- marvell,sei-ranges: ranges of non-wired interrupts triggered by way of
> +                      MSIs.
> +- msi-controller: identifies the node as an MSI controller.
> +
> +Example:
> +
> +        sei: sei@3f0200 {
> +               compatible = "marvell,armada-8k-sei";
> +               reg = <0x3f0200 0x40>;
> +               interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
> +
> +               sei_wired_controller: sei-wired-controller@0 {
> +                       marvell,sei-ranges = <0 21>;
> +                       #interrupt-cells = <1>;
> +                       interrupt-controller;
> +               };
> +
> +               sei_msi_controller: sei-msi-controller@21 {
> +                       marvell,sei-ranges = <21 43>;
> +                       msi-controller;
> +               };

I still think this should just be all one node. There's several examples 
in the tree of nodes which are both interrupt-controller and 
msi-controller. Marvell MPIC is one example.

Rob
Miquel Raynal June 8, 2018, 2:46 p.m. UTC | #2
Hi Rob, Marc,

On Tue, 5 Jun 2018 14:51:21 -0600, Rob Herring <robh@kernel.org> wrote:

> On Tue, May 22, 2018 at 11:40:39AM +0200, Miquel Raynal wrote:
> > Describe the System Error Interrupt (SEI) controller. It aggregates two
> > types of interrupts, wired and MSIs from respectively the AP and the
> > CPs, into a single SPI interrupt.
> > 
> > Suggested-by: Haim Boot <hayim@marvell.com>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  .../bindings/interrupt-controller/marvell,sei.txt  | 50 ++++++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> > new file mode 100644
> > index 000000000000..689981036c30
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> > @@ -0,0 +1,50 @@
> > +Marvell SEI (System Error Interrupt) Controller
> > +-----------------------------------------------
> > +
> > +Marvell SEI (System Error Interrupt) controller is an interrupt
> > +aggregator. It receives interrupts from several sources and aggregates
> > +them to a single interrupt line (an SPI) on the parent interrupt
> > +controller.
> > +
> > +This interrupt controller can handle up to 64 SEIs, a set comes from the
> > +AP and is wired while a second set comes from the CPs by the mean of
> > +MSIs. Each 'domain' is represented as a subnode.
> > +
> > +Required properties:
> > +
> > +- compatible: should be "marvell,armada-8k-sei".
> > +- reg: SEI registers location and length.
> > +- interrupts: identifies the parent IRQ that will be triggered.
> > +
> > +Child node 'sei-wired-controller' required properties:
> > +
> > +- marvell,sei-ranges: ranges of wired interrupts.
> > +- #interrupt-cells: number of cells to define an SEI wired interrupt
> > +                    coming from the AP, should be 1. The cell is the IRQ
> > +                    number.
> > +- interrupt-controller: identifies the node as an interrupt controller.
> > +
> > +Child node 'sei-msi-controller' required properties:
> > +
> > +- marvell,sei-ranges: ranges of non-wired interrupts triggered by way of
> > +                      MSIs.
> > +- msi-controller: identifies the node as an MSI controller.
> > +
> > +Example:
> > +
> > +        sei: sei@3f0200 {
> > +               compatible = "marvell,armada-8k-sei";
> > +               reg = <0x3f0200 0x40>;
> > +               interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > +               sei_wired_controller: sei-wired-controller@0 {
> > +                       marvell,sei-ranges = <0 21>;
> > +                       #interrupt-cells = <1>;
> > +                       interrupt-controller;
> > +               };
> > +
> > +               sei_msi_controller: sei-msi-controller@21 {
> > +                       marvell,sei-ranges = <21 43>;
> > +                       msi-controller;
> > +               };  
> 
> I still think this should just be all one node. There's several examples 
> in the tree of nodes which are both interrupt-controller and 
> msi-controller. Marvell MPIC is one example.

I checked Marvell MPIC example (armada 370 XP), it does not use
hierarchy domains, so I totally understand your point but I'm not sure
how I could get inspired by this driver (I'm looking for others).

Here I'm stuck. I know from a pure DT point of view the following is
not a valid argument. But from Linux, there is no easy way to handle
this situation without two different device nodes due to the internals
of the irqchip subsystem. There is simply no easy solution and having
only one node would require consequent changes in the core.

Maybe Marc will have an idea, but I think we already gave up on this
topic :/

Regards,
Miquèl
Miquel Raynal June 22, 2018, 2:57 p.m. UTC | #3
Hi Rob, Marc,

On Fri, 8 Jun 2018 16:46:23 +0200, Miquel Raynal
<miquel.raynal@bootlin.com> wrote:

> Hi Rob, Marc,
> 
> On Tue, 5 Jun 2018 14:51:21 -0600, Rob Herring <robh@kernel.org> wrote:
> 
> > On Tue, May 22, 2018 at 11:40:39AM +0200, Miquel Raynal wrote:  
> > > Describe the System Error Interrupt (SEI) controller. It aggregates two
> > > types of interrupts, wired and MSIs from respectively the AP and the
> > > CPs, into a single SPI interrupt.
> > > 
> > > Suggested-by: Haim Boot <hayim@marvell.com>
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  .../bindings/interrupt-controller/marvell,sei.txt  | 50 ++++++++++++++++++++++
> > >  1 file changed, 50 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> > > new file mode 100644
> > > index 000000000000..689981036c30
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> > > @@ -0,0 +1,50 @@
> > > +Marvell SEI (System Error Interrupt) Controller
> > > +-----------------------------------------------
> > > +
> > > +Marvell SEI (System Error Interrupt) controller is an interrupt
> > > +aggregator. It receives interrupts from several sources and aggregates
> > > +them to a single interrupt line (an SPI) on the parent interrupt
> > > +controller.
> > > +
> > > +This interrupt controller can handle up to 64 SEIs, a set comes from the
> > > +AP and is wired while a second set comes from the CPs by the mean of
> > > +MSIs. Each 'domain' is represented as a subnode.
> > > +
> > > +Required properties:
> > > +
> > > +- compatible: should be "marvell,armada-8k-sei".
> > > +- reg: SEI registers location and length.
> > > +- interrupts: identifies the parent IRQ that will be triggered.
> > > +
> > > +Child node 'sei-wired-controller' required properties:
> > > +
> > > +- marvell,sei-ranges: ranges of wired interrupts.
> > > +- #interrupt-cells: number of cells to define an SEI wired interrupt
> > > +                    coming from the AP, should be 1. The cell is the IRQ
> > > +                    number.
> > > +- interrupt-controller: identifies the node as an interrupt controller.
> > > +
> > > +Child node 'sei-msi-controller' required properties:
> > > +
> > > +- marvell,sei-ranges: ranges of non-wired interrupts triggered by way of
> > > +                      MSIs.
> > > +- msi-controller: identifies the node as an MSI controller.
> > > +
> > > +Example:
> > > +
> > > +        sei: sei@3f0200 {
> > > +               compatible = "marvell,armada-8k-sei";
> > > +               reg = <0x3f0200 0x40>;
> > > +               interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
> > > +
> > > +               sei_wired_controller: sei-wired-controller@0 {
> > > +                       marvell,sei-ranges = <0 21>;
> > > +                       #interrupt-cells = <1>;
> > > +                       interrupt-controller;
> > > +               };
> > > +
> > > +               sei_msi_controller: sei-msi-controller@21 {
> > > +                       marvell,sei-ranges = <21 43>;
> > > +                       msi-controller;
> > > +               };    
> > 
> > I still think this should just be all one node. There's several examples 
> > in the tree of nodes which are both interrupt-controller and 
> > msi-controller. Marvell MPIC is one example.  
> 
> I checked Marvell MPIC example (armada 370 XP), it does not use
> hierarchy domains, so I totally understand your point but I'm not sure
> how I could get inspired by this driver (I'm looking for others).
> 
> Here I'm stuck. I know from a pure DT point of view the following is
> not a valid argument. But from Linux, there is no easy way to handle
> this situation without two different device nodes due to the internals
> of the irqchip subsystem. There is simply no easy solution and having
> only one node would require consequent changes in the core.
> 
> Maybe Marc will have an idea, but I think we already gave up on this
> topic :/

I double checked the MPIC driver and I still don't understand how we
can allocate both MSIs and wired interrupts with the current driver.

Anyway, I finally managed to merge 'sei_wired_controller' and
'sei_msi_controller' and have only one 'sei' node.

I had to do not use the fwnode of the 'sei' node in at least one IRQ
domain (I choose the wired one) and instead implement for that domain
the ->match() hook. I hope this is the right way to do it.

Please have a look at the v3 coming soon.

Thanks,
Miquèl
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
new file mode 100644
index 000000000000..689981036c30
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
@@ -0,0 +1,50 @@ 
+Marvell SEI (System Error Interrupt) Controller
+-----------------------------------------------
+
+Marvell SEI (System Error Interrupt) controller is an interrupt
+aggregator. It receives interrupts from several sources and aggregates
+them to a single interrupt line (an SPI) on the parent interrupt
+controller.
+
+This interrupt controller can handle up to 64 SEIs, a set comes from the
+AP and is wired while a second set comes from the CPs by the mean of
+MSIs. Each 'domain' is represented as a subnode.
+
+Required properties:
+
+- compatible: should be "marvell,armada-8k-sei".
+- reg: SEI registers location and length.
+- interrupts: identifies the parent IRQ that will be triggered.
+
+Child node 'sei-wired-controller' required properties:
+
+- marvell,sei-ranges: ranges of wired interrupts.
+- #interrupt-cells: number of cells to define an SEI wired interrupt
+                    coming from the AP, should be 1. The cell is the IRQ
+                    number.
+- interrupt-controller: identifies the node as an interrupt controller.
+
+Child node 'sei-msi-controller' required properties:
+
+- marvell,sei-ranges: ranges of non-wired interrupts triggered by way of
+                      MSIs.
+- msi-controller: identifies the node as an MSI controller.
+
+Example:
+
+        sei: sei@3f0200 {
+               compatible = "marvell,armada-8k-sei";
+               reg = <0x3f0200 0x40>;
+               interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+
+               sei_wired_controller: sei-wired-controller@0 {
+                       marvell,sei-ranges = <0 21>;
+                       #interrupt-cells = <1>;
+                       interrupt-controller;
+               };
+
+               sei_msi_controller: sei-msi-controller@21 {
+                       marvell,sei-ranges = <21 43>;
+                       msi-controller;
+               };
+       };