Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201008130515.2385825-1-lars.povlsen@microchip.com>
 <20201008130515.2385825-2-lars.povlsen@microchip.com> <CACRpkdaFYoXFUuWow5s9TitrRDhMW=wiaxgfMcY6sQkYYgC-Lw@mail.gmail.com>
 <87d01ryb04.fsf@soft-dev15.microsemi.net>
In-Reply-To: <87d01ryb04.fsf@soft-dev15.microsemi.net>
From:   Linus Walleij <linus.walleij@linaro.org>
Date:   Tue, 13 Oct 2020 15:36:30 +0200
Message-ID: <CACRpkdZbxohoxg3bN-XpLXwBUgTDByoP9sNEocUt10R6t9FgXQ@mail.gmail.com>
Subject: Re: [PATCH v5 1/3] dt-bindings: pinctrl: Add bindings for
 pinctrl-microchip-sgpio driver
To:     Lars Povlsen <lars.povlsen@microchip.com>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Oct 9, 2020 at 12:00 PM Lars Povlsen <lars.povlsen@microchip.com> wrote:

> > So here reg = 0 and the out port has reg 1. Isn't that what you also put
> > in the second cell of the GPIO phandle? Then why? The driver
> > can very well just parse its own reg property and fill that in.
>
> NO! The second cell is the second dimension - NOT the direction. As I
> wrote previously, the direction is now inherent from the handle, ie. the
> "reg" value of the handle.

OK I get it ... I think :)

> The hardware describe a "port" and a "bit index" addressing, where the
> second cell in
>
>   gpios = <&sgpio_in2 11 0 GPIO_OUT_LOW>;
>
> is the "bit index" - not the "reg" from the phandle.

As long as the bindings specify exactly what is meant by bit index
and the tupe (port, bit_index) is what uniquely addresses a certain
GPIO line then it is fine I suppose.

> In the example above, note
>
>   ngpios = <96>;
>
> As the "port" is [0; 31], this defines "bit index" to be [0; 2], so the
> (input) GPIO cells will be:
>
> p0b0, p0b1, p0b2
> ...
> p31b0, p31b1, p31b2
>
> being identical to
>
> <&sgpio_inX 0 0 GPIO_OUT_LOW>
> <&sgpio_inX 0 1 GPIO_OUT_LOW>
> <&sgpio_inX 0 2 GPIO_OUT_LOW>
> ...
> <&sgpio_inX 31 0 GPIO_OUT_LOW>
> <&sgpio_inX 31 1 GPIO_OUT_LOW>
> <&sgpio_inX 31 2 GPIO_OUT_LOW>
>
> ('X' being the SGPIO controller instance).

So 32 possible ports with 3 possible bit indexes on each?
This constraint should go into the bindings as well so it becomes
impossible to put in illegal port numbers or bit indices.

(Use the YAML min/max constraints, I suppose?)

> So no, there *really* is a need for a 3-cell GPIO specifier (or whatever
> its called).

If that is the natural way to address the hardware lines
and what is used in the documentation then it's fine, it's just so
unorthodox that I have to push back on it a bit you know.

Yours,
Linus Walleij