Re: [PATCH 1/2] dt-bindings: irq: add binding for Freescale IRQSTEER multiplexer

[Lucas Stach]

Hi Rob,

Am Donnerstag, den 25.10.2018, 09:35 -0500 schrieb Rob Herring:
> On Tue, Oct 16, 2018 at 06:42:17PM +0200, Lucas Stach wrote:
> > This adds the DT binding for the Freescale IRQSTEER interrupt
> > multiplexer found in the i.MX8 familiy SoCs.
> > 
> > > > Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> > ---
> > Â.../interrupt-controller/fsl,irqsteer.txtÂÂÂÂÂ| 39 +++++++++++++++++++
> > Â1 file changed, 39 insertions(+)
> > Âcreate mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt
> > new file mode 100644
> > index 000000000000..ed2b18165591
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt
> > @@ -0,0 +1,39 @@
> > +Freescale IRQSTEER Interrupt multiplexer
> > +
> > +Required properties:
> > +
> > +- compatible: should be:
> > > > +	- "fsl,imx8m-irqsteer"
> > > > +	- "fsl,imx-irqsteer"
> > +- reg: Physical base address and size of registers.
> > +- interrupts: Should contain the parent interrupt line used to multiplex the
> > +ÂÂinput interrupts.
> > +- clocks: Should contain one clock for entry in clock-names
> > +ÂÂsee Documentation/devicetree/bindings/clock/clock-bindings.txt
> > +- clock-names:
> > +ÂÂÂ- "ipg": main logic clock
> > +- interrupt-controller: Identifies the node as an interrupt controller.
> > +- #interrupt-cells: Specifies the number of cells needed to encode an
> > +ÂÂinterrupt source. The value must be 2.
> > +
> > +Optional properties:
> > +- fsl,channel: Number of channels managed by this controller. Each channel
> > +ÂÂcontains up to 32 interrupt sources. If absent defaults to 1.
> 
> What's a channel? Why isn't this implied by the compatible?

The documentation calls the mux outputs a channel. Basically the
irqsteer IP block can mux up to 512 input interrupts onto up to 8
output interrupt lines (just noticed that the 32 in the description is
wrong and should be 64).

So the channel count defines the number of input IRQs in steps of 64
and the output IRQs in steps of 1. The register layout/offsets don't
change with different number of channels, as those are always laid out
as if the controller supported the maximum count of 8 channels. It's
just the actual implemented number of registers that change.

Do you prefer to encode this in the compatible, or is it okay to keep
this a a parameter?

> > +- fsl,endian:
> 
> The standard property for endianness doesn't work for you?

It does, I'll switch to that.

Regards,
Lucas