linux-kernel - Re: [PATCH 1/6] ASoC: wcd938x: switch to using gpiod API

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: <a1d797bc-609c-4cb5-a531-9f840f3cf2b7@sirena.org.uk>
Date:   Thu, 20 Apr 2023 19:19:59 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Vinod Koul <vkoul@...nel.org>,
        Bard Liao <yung-chuan.liao@...ux.intel.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Sanyog Kale <sanyog.r.kale@...el.com>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, alsa-devel@...a-project.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Patrick Lai <quic_plai@...cinc.com>
Subject: Re: [PATCH 1/6] ASoC: wcd938x: switch to using gpiod API

On Thu, Apr 20, 2023 at 07:51:27PM +0200, Krzysztof Kozlowski wrote:
> On 20/04/2023 18:28, Mark Brown wrote:
> > On Thu, Apr 20, 2023 at 04:16:59PM +0200, Krzysztof Kozlowski wrote:

> >> Life of downstream. We all know the drill: merge your DTS or suffer. The

> > No, the DT is supposed to be an ABI.  

> No, the DT bindings are the ABI. We are supposed not to break
> user-space, but out-of-tree users of drivers are not ABI by itself.
> Bindings are. If out-of-tree users make mistakes in their DTS and do not
> want to upstream it, it's their choice but it does not come for free.

This is absolutely not the case, users should be able to ship DTs
without upstreaming them and run multiple operating systems on top of a
single DT - ideally boards would ship with DTs in firmware and people
would be able to install generic OSs onto them with just off the shelf
install media.  This is even a thing that people have actually done,
both non-FDT systems like SPARC and the PowerPC systems from Apple and a
few FDT ones like Synquacer.

The enormous costs of DT would hardly be worth it if it were purely an
in tree thing.

> > The point in having a domain
> > specific language with a compiler is to allow device trees to be
> > distributed independently of the kernel.

> When it is written incorrectly - wrong flag used for GPIO - there is no
> requirement to support it.

If it worked was it ever really wrong (and note that the bindings may
not always be super clear...)?  While there is a point at which things
never worked, can be fixed and we don't need to care about it or where
we know the userbase well enough to know there won't be any issue those
shouldn't be the default and should generally be avoided.  Where there
is a good reason to break compatibility it should be something we're
actively deciding to do for a clear reason having considered the
tradeoffs, not something that gets done on a whim without even
mentioning it.

> > It's not just this individual transition, it's the whole thing with
> > encoding the polarity of the signal at all - it's a layer of abstraction
> > that feels like it introduces at least as many problems as it solves,
> > and requiring configuration on every single system integration doesn't
> > feel like the right choice in general.

> Choosing appropriate flag for GPIO in DTS is not difficult. It was
> skipped because we rarely cared in the drivers, but it should have been
> chosen correctly. The same about interrupt flags. We had many DTS for
> many times marking all possible interrupts as IRQ_TYPE_NONE. Did it
> matter for many drivers and setups? No, was perfectly "fine". Is it
> correct from DTS point of view. Also no.

There's no natural definition of "correct" here though - it's just
picking something in a binding.  If someone for example flips the label
on a signal from reset to enable (perhaps during review) that ends up
changing active high to active low, and really I'm not sure how much
we're really winning compared to just having code in the end consumer
which just directly says what value it wants the physical signal to
have.

My point is not that we haven't defined things such that the user has to
specify if something is active high or active low, it's that it feels
like it's more trouble han it's worth.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)