linux-kernel - Re: [PATCH v5 1/3] dt-bindings: pinctrl: Add bindings for pinctrl-microchip-sgpio driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 13 Oct 2020 15:36:30 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Lars Povlsen <lars.povlsen@...rochip.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>
Subject: Re: [PATCH v5 1/3] dt-bindings: pinctrl: Add bindings for
 pinctrl-microchip-sgpio driver

On Fri, Oct 9, 2020 at 12:00 PM Lars Povlsen <lars.povlsen@...rochip.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