On 15/02/16 15:31, Quan Nguyen wrote: > On Mon, Feb 15, 2016 at 7:54 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: >> On 15/02/16 04:57, Quan Nguyen wrote: >>> Update description for X-Gene standby GPIO controller DTS binding to >>> support GPIO line configuration as input, output or external IRQ pin. >>> >>> Signed-off-by: Y Vo <yvo@xxxxxxx> >>> Signed-off-by: Quan Nguyen <qnguyen@xxxxxxx> >>> --- >>> .../devicetree/bindings/gpio/gpio-xgene-sb.txt | 47 ++++++++++++++++++---- >>> 1 file changed, 40 insertions(+), 7 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-xgene-sb.txt b/Documentation/devicetree/bindings/gpio/gpio-xgene-sb.txt >>> index dae1300..7b8b4cb 100644 >>> --- a/Documentation/devicetree/bindings/gpio/gpio-xgene-sb.txt >>> +++ b/Documentation/devicetree/bindings/gpio/gpio-xgene-sb.txt >>> @@ -1,10 +1,20 @@ >>> APM X-Gene Standby GPIO controller bindings >>> >>> -This is a gpio controller in the standby domain. >>> - >>> -There are 20 GPIO pins from 0..21. There is no GPIO_DS14 or GPIO_DS15, >>> -only GPIO_DS8..GPIO_DS13 support interrupts. The IRQ mapping >>> -is currently 1-to-1 on interrupts 0x28 thru 0x2d. >>> +This is a gpio controller in the standby domain. It also supports interrupt in >>> +some particular pins which are sourced to its parent interrupt controller >>> +as diagram below: >>> + +-----------------+ >>> + | X-Gene standby | >>> + | GPIO controller +--------- GPIO_0 >>> ++------------+ | | ... >>> +| Parent IRQ | | +--------- GPIO_8/EXT_INT_0 >>> +| controller | EXT_INT_0 | | ... >>> +| (GICv2) +-------------+ +--------- GPIO_[N+8]/EXT_INT_N >>> +| | ... | | >>> +| | EXT_INT_N | +--------- GPIO_[N+9] >>> +| +-------------+ | ... >>> +| | | +--------- GPIO_MAX >>> ++------------+ +-----------------+ >>> >>> Required properties: >>> - compatible: "apm,xgene-gpio-sb" for the X-Gene Standby GPIO controller >>> @@ -15,10 +25,18 @@ Required properties: >>> 0 = active high >>> 1 = active low >>> - gpio-controller: Marks the device node as a GPIO controller. >>> -- interrupts: Shall contain exactly 6 interrupts. >>> +- interrupts: The EXT_INT_0 parent interrupt resource must be listed first. >>> +- interrupt-parent: Phandle of the parent interrupt controller. >>> +- interrupt-cells: Should be two. >>> + - first cell is 0-N coresponding for EXT_INT_0 to EXT_INT_N. >>> + - second cell is used to specify flags. >>> +- interrupt-controller: Marks the device node as an interrupt controller. >>> +- apm,nr-gpios: Optional, specify number of gpios pin. >>> +- apm,nr-irqs: Optional, specify number of interrupt pins. >>> +- apm,irq-start: Optional, specify lowest gpio pin support interrupt. >> >> This is quite ambiguous. Is that relative to the GIC? Assuming this is >> the case, you should then document it, specify the type of interrupt >> (SPI?), and whether this is 0- or 32-based (the code seems to indicate >> that is is 0-based). >> > > Hi Marc, Let me explain by diagram below. > As you can also see in diagram above, the lowest gpio pin that > supports interrupt is GPIO_8 (EXT_INT_0), > and hence, apm,irq-start=<8> as default. > > +-----------------+ > | X-Gene standby | > | GPIO controller +--------- GPIO_0 > +------------+ | | ... > | Parent IRQ | | > +--------- GPIO_[apm,irq-start]/EXT_INT_0 > | controller | EXT_INT_0 | | ... > | (GICv2) +-----------------------------+ > +--------- GPIO_[apm,nr-irqs + apm,irq-start - 1]/EXT_INT_[apm,nr-irqs > - 1] > | | ... | | > | | EXT_INT_[apm,nr-irqs - 1] | > +--------- GPIO_[apm,nr-irqs + apm,irq-start] > | +-----------------------------+ | ... > | | | +--------- GPIO_MAX > +------------+ +-----------------+ > > To fix this ambiguity, I'm thinking to change it to: > "apm,irq-start: Optional, specify index of first gpio pin > corresponding to EXT_INT_0, default is 8." I think you are missing the point I'm trying to make. There are two ways to refer to an SPI: either by its absolute number (INT32 for example) or by its relative number (SPI0). SPI0 and INT32 are the same interrupt. Just with a different base. My question is: which base are you using? By looking at the code, I think you're using the absolute version (INTx). And as your controller seems to be very GIC specific, you should add a pointer to the GIC documentation. Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
