From: Quan Nguyen <qnguyen@apm.com>
To: Marc Zyngier <marc.zyngier@arm.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
Thomas Gleixner <tglx@linutronix.de>,
Jason Cooper <jason@lakedaemon.net>, Y Vo <yvo@apm.com>,
Phong Vo <pvo@apm.com>, Loc Ho <lho@apm.com>,
Feng Kan <fkan@apm.com>, Duc Dang <dhdang@apm.com>,
patches <patches@apm.com>
Subject: Re: [PATCH v6 2/3] Documentation: gpio: Update description for X-Gene standby GPIO controller DTS binding
Date: Tue, 16 Feb 2016 00:38:08 +0700 [thread overview]
Message-ID: <CAFopSPSh6AApYgZ7vmQgGYWekpAB8ktywC9T4xdfCiJbbSCQ8Q@mail.gmail.com> (raw)
In-Reply-To: <56C1F7EC.3020701@arm.com>
On Mon, Feb 15, 2016 at 11:08 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 15/02/16 15:31, Quan Nguyen wrote:
>> On Mon, Feb 15, 2016 at 7:54 PM, Marc Zyngier <marc.zyngier@arm.com> 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@apm.com>
>>>> Signed-off-by: Quan Nguyen <qnguyen@apm.com>
>>>> ---
>>>> .../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.
>
Sorry, Marc, I was misunderstood.
It is relative number, EXT_INT_0 corresponding with SPI40 and so on.
So, how about adding more specific description into the diagram as below ?
--- a/Documentation/devicetree/bindings/gpio/gpio-xgene-sb.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-xgene-sb.txt
@@ -3,18 +3,18 @@ APM X-Gene Standby GPIO controller bindings
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
-+------------+ +-----------------+
+ +-----------------+
+ | X-Gene standby |
+ | GPIO controller +------ GPIO_0
++------------+ | | ...
+| Parent IRQ | EXT_INT_0 | +------ GPIO_8/EXT_INT_0
+| controller | (SPI40) | | ...
+| (GICv2) +--------------+ +------ GPIO_[N+8]/EXT_INT_N
+| | ... | |
+| | EXT_INT_N | +------ GPIO_[N+9]
+| | (SPI[40 + N])| | ...
+| +--------------+ +------ GPIO_MAX
++------------+ +-----------------+
Required properties:
- compatible: "apm,xgene-gpio-sb" for the X-Gene Standby GPIO controller
Thanks,
-- Quan Nguyen
next prev parent reply other threads:[~2016-02-15 17:38 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-15 4:57 [PATCH v6 0/3] Enable X-Gene standby GPIO as interrupt controller Quan Nguyen
2016-02-15 4:57 ` [PATCH v6 1/3] gpio: xgene: " Quan Nguyen
2016-02-15 12:55 ` Marc Zyngier
2016-02-15 4:57 ` [PATCH v6 2/3] Documentation: gpio: Update description for X-Gene standby GPIO controller DTS binding Quan Nguyen
2016-02-15 12:54 ` Marc Zyngier
2016-02-15 15:31 ` Quan Nguyen
2016-02-15 16:08 ` Marc Zyngier
2016-02-15 17:38 ` Quan Nguyen [this message]
2016-02-15 4:57 ` [PATCH v6 3/3] arm64: dts: Update X-Gene standby GPIO controller DTS entries Quan Nguyen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFopSPSh6AApYgZ7vmQgGYWekpAB8ktywC9T4xdfCiJbbSCQ8Q@mail.gmail.com \
--to=qnguyen@apm.com \
--cc=devicetree@vger.kernel.org \
--cc=dhdang@apm.com \
--cc=fkan@apm.com \
--cc=jason@lakedaemon.net \
--cc=lho@apm.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=patches@apm.com \
--cc=pvo@apm.com \
--cc=tglx@linutronix.de \
--cc=yvo@apm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).