linux-kernel - Re: [PATCH v5 01/10] pinctrl: generic: Add bi-directional and output-enable

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:   Mon, 29 May 2017 19:42:29 +0900
From:   jmondi <jacopo@...ndi.org>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Dong Aisheng <dongas86@...il.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Chris Brandt <Chris.Brandt@...esas.com>,
        Jacopo Mondi <jacopo+renesas@...ndi.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 01/10] pinctrl: generic: Add bi-directional and
 output-enable

Hi Linus,

On Mon, May 29, 2017 at 10:45:44AM +0200, Linus Walleij wrote:
> On Tue, May 23, 2017 at 8:37 PM, jmondi <jacopo@...ndi.org> wrote:
>
> >> I did not follow too much.
> >> But it seems IMX7ULP/Vybrid to be also a fan of generic
> >> output-enable/input-enable
> >> property.
> >>
> >> See:
> >> Figure 5-2. GPIO PAD in Page 241
> >> http://www.nxp.com/assets/documents/data/en/reference-manuals/VFXXXRM.pdf
> >>
> >> It has separate register bits to control input buffer enable and
> >> output buffer enable
> >> and we need set it property for GPIO function.
> >
> > As it seems we have another user for 'output-enable' here, what if we just
> > add that one to the generic bindings properties list, and we keep
> > 'bi-directional' (which seems to be the most debated property we have
> > added) out of generic properties?
> >
> > We can handle 'bi-directional' pins with static tables in our pin
> > controller driver and not have it anywhere in DT.
>
> This sounds like a viable approach.
>
> I just want to know if "output-enable" is the right name?
> "output-buffer-enable"?

Great! Thanks!

On naming: if we need "output-buffer-enable" should we add
"input-buffer-enable" as well?

Currently we are using "input-enable" to pair with "output-enable",
but as you said, just "output-enable" when "output-high" and
"output-low" are there already seems a bit confusing.
At the same time "input-buffer-enable" seems to actually be just
electrically equivalent to "input-enable", so adding it is a bit of a
waste as well.

I see three options here:

1) Add "output-buffer-enable" and "input-buffer-enable"
we end up with
"output-high"
"output-low"
"input-enable"
"output-buffer-enable"
"input-buffer-enable"

2) Add "output-buffer-enable" only
we end up with
"output-high"
"output-low"
"input-enable"
"output-buffer-enable"

Binding may be confusing as in one case we use "output-buffer-enable"
while in the other "input-enable"

3) Add "output-enable" only
"output-high"
"output-low"
"input-enable"
"output-enable"

As you, I don't like "output-enable" that much but it pairs better with
"input-enable".

I'll let you and DT people decide on this, as it's really an ABI definition
problem and you have better judgment there.

>
> > I see commit 42d5a11200d0[1] has not been reverted yet as Andy asked
> > in some previous email.
>
> I'm just overloaded. I sent that revert to Torvalds today.

Thank you. Didn't want to put pressure ;)
>
> > I can send another version of that patch with
> > only 'output-enable' if you wish.
>
> That's what we want.
>
> > Once we reach consesus, I can then send v6 of our pin controller driver
> > based on that.
>
> OK sounds like a plan.
>
> Sorry for the mess, I'm just trying to get this right :/

Not a mess, and thanks for your effort in maintaining  all of this

Thanks
   j
>
> Yours,
> Linus Walleij