From patchwork Mon Feb 15 17:38:08 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Quan Nguyen X-Patchwork-Id: 8317511 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id F19C29F72C for ; Mon, 15 Feb 2016 17:48:03 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id EF3272037C for ; Mon, 15 Feb 2016 17:48:02 +0000 (UTC) Received: from bombadil.infradead.org (unknown [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D274B20382 for ; Mon, 15 Feb 2016 17:48:01 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aVN6Y-0003Xs-I4; Mon, 15 Feb 2016 17:38:34 +0000 Received: from mail-vk0-x22d.google.com ([2607:f8b0:400c:c05::22d]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1aVN6U-0003LV-72 for linux-arm-kernel@lists.infradead.org; Mon, 15 Feb 2016 17:38:31 +0000 Received: by mail-vk0-x22d.google.com with SMTP id e6so112810254vkh.2 for ; Mon, 15 Feb 2016 09:38:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apm.com; s=apm; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=ppxJz5TXN9Catx8wPR86201IPHd5B1oDbB3c0I2exPE=; b=jeHxjAPdD+rMjNOxwzEeC3Ag7OhHXL0ZXrN36vMqOXrkk9mRfSzG1Cd90ulxEcfx74 hHmAzLqakYV0b0vrOMvjRLuD2ZJ7fHCHTNgdA0XtPXNP5aet1AhfJAWRqR3et/aiDq0d fy6kcd3WWPRPhRh56pvh/YNla9MzkY6ISJ9r4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=ppxJz5TXN9Catx8wPR86201IPHd5B1oDbB3c0I2exPE=; b=bMRGKZoAdJj8u3iQOft0ZlBYpIpSJ4SIkStKyRfO3k3NQ+/bjmOuAxBRBSaNSNBUXA HZzrYKpinKSMHzkBaFFNoSyXqKxnQpi7Z9SfK0OpuLiAfgxflX/K5d7xrsD01feMNFrv bAAmjuT/FeYcLdlXarGRrIJnENMWuGt0dtW52LqM/nFFTq2Sj0N+MKoDpz22DyG8u6OM ZY6dk6n5AoLZiA3z+xH3uqdtJb/VP7GEQ9SyRdh/CEdqWkSjWhb/oeHHotnsoj6EYcIP eWwRY1ZUhO9/BfNvtPSwZ34BGa57S+Eb4fubnJ9qoswoqzmiGUA4SlbfEkKcCN4taYQa gMoA== X-Gm-Message-State: AG10YORYOiYM3CLDPBcHmiX69x1+9cfNUMqkiiihuCXb8iqo34tLtpkohoNa1H36YQVE9b4SVLS84QxRD3zVVW8i MIME-Version: 1.0 X-Received: by 10.31.167.75 with SMTP id q72mr14503482vke.71.1455557888955; Mon, 15 Feb 2016 09:38:08 -0800 (PST) Received: by 10.159.39.170 with HTTP; Mon, 15 Feb 2016 09:38:08 -0800 (PST) In-Reply-To: <56C1F7EC.3020701@arm.com> References: <1455512263-18183-1-git-send-email-qnguyen@apm.com> <1455512263-18183-3-git-send-email-qnguyen@apm.com> <56C1CA6E.1060606@arm.com> <56C1F7EC.3020701@arm.com> Date: Tue, 16 Feb 2016 00:38:08 +0700 Message-ID: Subject: Re: [PATCH v6 2/3] Documentation: gpio: Update description for X-Gene standby GPIO controller DTS binding From: Quan Nguyen To: Marc Zyngier X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160215_093830_435930_87EF29FB X-CRM114-Status: GOOD ( 20.81 ) X-Spam-Score: -2.7 (--) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, Jason Cooper , Phong Vo , Linus Walleij , Duc Dang , patches , linux-gpio@vger.kernel.org, Feng Kan , Y Vo , Thomas Gleixner , linux-arm-kernel@lists.infradead.org, Loc Ho Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-3.3 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RDNS_NONE,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Feb 15, 2016 at 11:08 PM, Marc Zyngier wrote: > On 15/02/16 15:31, Quan Nguyen wrote: >> On Mon, Feb 15, 2016 at 7:54 PM, Marc Zyngier 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 >>>> Signed-off-by: Quan Nguyen >>>> --- >>>> .../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 ? Thanks, -- Quan Nguyen --- 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