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

[Krzysztof Kozlowski]

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@xxxxxxxxxx/
> 
> 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