Hi Rob, Marc,
On Fri, 8 Jun 2018 16:46:23 +0200, Miquel Raynal
<miquel.ray...@bootlin.com> wrote:
> Hi Rob, Marc,
>
> On Tue, 5 Jun 2018 14:51:21 -0600, Rob Herring <r...@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 <ha...@marvell.com>
> > > Signed-off-by: Miquel Raynal <miquel.ray...@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
