Re: [PATCH v6 2/3] Documentation: gpio: Update description for X-Gene standby GPIO controller DTS binding

[Quan Nguyen <qnguyen@xxxxxxx>]

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."

Thanks,
-- Quan Nguyen
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html