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: <94bbd608-a90b-605d-a61c-6d6769b60445@linaro.org>
Date:   Thu, 20 Apr 2023 19:51:27 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Mark Brown <broonie@...nel.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 20/04/2023 18:28, Mark Brown wrote:
> On Thu, Apr 20, 2023 at 04:16:59PM +0200, Krzysztof Kozlowski wrote:
>> On 20/04/2023 15:00, Mark Brown wrote:
> 
>>> That's just upstream, what about any downstream users?
> 
>> 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.

> 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.

>> I could rework this patch to provide backwards compatible solution like
>> I did for WSA:
>> https://lore.kernel.org/all/20230102114152.297305-4-krzysztof.kozlowski@linaro.org/
> 
> There we go...
> 
>>> We could handle inversions through an explicit property if that were
>>> needed, that would be a less problematic transition and clearer in the
>>> consumer code.
> 
>> I am not sure if it is worth. The DTS is supposed to describe hardware,
>> so even if reset pin flag was not effective, it is a mistake to describe
>> it as ACTIVE_HIGH. Do we care about keeping broken code happy? If yes,
>> then property is the way to go. If partially, then I can add
>> backwards-compatible approach like I mentioned above.
> 
> 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.

Best regards,
Krzysztof