linux-kernel - Re: [PATCH v3 4/7] dt-bindings: pinctrl: add bindings for Mediatek MT8365 SoC

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]

Message-ID: <CAP2ifjMQAx23xc0p_LZ9Dj79Hx1cyLZ-tx8HrGUbtDR-SmdECw@mail.gmail.com>
Date:   Sun, 20 Nov 2022 15:38:51 +0100
From:   Bernhard Rosenkränzer <bero@...libre.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Kevin Hilman <khilman@...libre.com>,
        linux-mediatek@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, matthias.bgg@...il.com,
        "angelogioacchino.delregno@...labora.com" 
        <angelogioacchino.delregno@...labora.com>
Subject: Re: [PATCH v3 4/7] dt-bindings: pinctrl: add bindings for Mediatek
 MT8365 SoC

Hi Krzysztof,

On Sun, Nov 20, 2022 at 11:40 AM Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org> wrote:
> >>> +  pins-are-numbered:
> >
> > Yeah, having this as a flag kind of implies that this could be present
> > for some boards but not others.  But in practice, the driver requires it
> > to be present or just fails[1].  What's the right way to describe that?
> > We're just trying to add a binding that reflects the existing driver.
>
> Uh, what an interesting property. What's the point of it then? Why
> failing to probe on a missing property which does nothing else?

I couldn't find any other use of it in the kernel, and also checked
u-boot (where the property also appears in some devicetree files, but
isn't used anywhere).
Both the MTK and STM drivers use it just to refuse it if it isn't there.

Unfortunately "git blame" only shows pins-are-numbered being added as
part of a larger commit ("add the driver"), so there's no "add check
for pins-are-numbered because xyz" commit message.

I can think of 3 possible explanations, but none of them are good:
1. It's something that had a purpose at some point, but doesn't
anymore (but that would likely leave some trace in "git blame"...)
2. It's something that was added in preparation for another patch (but
I can't find any queued/suggested patches that make use of it)
3. It's for the sake of userland -- check if pins-are-numbered is set
(which will be true for MTK and STM because these drivers enforce it,
false for anything else because the schemas don't mention it) and then
do different things. But this seems unlikely as well, the usual
suspects (libgpiod and friends) don't do any such lookup, and there
are ways to look up pins without that property - and I'd expect pins
are numbered on many controllers outside of MTK and STM, so looking up
that property would give a false response there.

> I would like to understand why do we need this property and what is
> described by it.

Same here...

> Because if it's purpose is only to fail or not fail
> driver probe, then we should just drop it everywhere.

Agreed (and I think more likely than not, that is the only purpose),
but probably a "add support for another board" patchset isn't the
right context for that.

I can prepare a "remove pins-are-numbered" patchset, but given it will
likely take time to track down someone who knows why this was added in
the first place, I don't think it should block the MT8365 patchset.

Best regards
bero