| Message ID | 20180522094042.24770-14-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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 --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; + }; + };
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