linux-kernel - Re: [PATCH v3 03/12] ASoC: dt-bindings: qcom,wsa881x: extend description to analog mode

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: <8609abe9-8aac-42a2-a2a1-2ccd6eafb171@kernel.org>
Date: Wed, 28 May 2025 07:58:32 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Alexey Klimov <alexey.klimov@...aro.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Srinivas Kandagatla <srini@...nel.org>, Mark Brown <broonie@...nel.org>,
 linux-sound@...r.kernel.org, Liam Girdwood <lgirdwood@...il.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Bjorn Andersson <andersson@...nel.org>,
 Dmitry Baryshkov <lumag@...nel.org>, Konrad Dybcio <konradybcio@...nel.org>,
 Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
 Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
 linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org
Subject: Re: [PATCH v3 03/12] ASoC: dt-bindings: qcom,wsa881x: extend
 description to analog mode

On 27/05/2025 22:34, Alexey Klimov wrote:
> On Thu May 22, 2025 at 6:45 PM BST, Krzysztof Kozlowski wrote:
>> On 22/05/2025 19:40, Alexey Klimov wrote:
>>> WSA881X also supports analog mode when device is configured via i2c
>>> only. Document it, add properties, new compatibles and example.
>>>
>>> Cc: Srinivas Kandagatla <srini@...nel.org>
>>> Signed-off-by: Alexey Klimov <alexey.klimov@...aro.org>
>>> ---
>>>  .../devicetree/bindings/sound/qcom,wsa881x.yaml    | 66 +++++++++++++++++++---
>>>  1 file changed, 58 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/qcom,wsa881x.yaml b/Documentation/devicetree/bindings/sound/qcom,wsa881x.yaml
>>> index ac03672ebf6de1df862ce282f955ac91bdd9167d..a33e2754ec6159dbcaf5b6fcacf89eb2a6056899 100644
>>> --- a/Documentation/devicetree/bindings/sound/qcom,wsa881x.yaml
>>> +++ b/Documentation/devicetree/bindings/sound/qcom,wsa881x.yaml
>>> @@ -12,15 +12,17 @@ maintainers:
>>>  description: |
>>>    WSA8810 is a class-D smart speaker amplifier and WSA8815
>>>    is a high-output power class-D smart speaker amplifier.
>>> -  Their primary operating mode uses a SoundWire digital audio
>>> -  interface. This binding is for SoundWire interface.
>>> -
>>> -allOf:
>>> -  - $ref: dai-common.yaml#
>>> +  This family of amplifiers support two operating modes:
>>> +  SoundWire digital audio interface which is a primary mode
>>> +  and analog mode when device is configured via i2c only.
>>> +  This binding describes both modes.
>>>  
>>>  properties:
>>>    compatible:
>>> -    const: sdw10217201000
>>> +    enum:
>>> +      - qcom,wsa8810
>>> +      - qcom,wsa8815
>>> +      - sdw10217201000
>>
>> You never responded to my comments, never implemented them. Same problem
>> as before.
> 
> You don't respond to emails sometimes and, while I want to move this forward,
> I am not taking any chances replying to few months old thread, so if it okay
> I'll respond here. Sorry for doing this.
> 
> Previous comment:
> 
>> You implement only one compatible, so does it mean they are compatible?
>> If so, make them compatible.
> 
> There are two compatibles in wsa881x-i2c.c.
> By looking at downstream sources and current code I think there is no diff
> between wsa8810 and wsa8815 and it is handled by reading hw registers if
> needed. So I am thinking that maybe it makes sense to reduce it to
> "qcom,wsa881x".

No, you need specific compatibles. That's the standard DT rule.
Compatibility is expressed with list and fallback (see example-schema or
any other qcom binding, really 95% of them have fallbacks).

WSA usually have version registers so even if there are differences,
they are fully detectable, thus one more argument for compatibility.

> 
> Previous comment:
>> Do not repeat property name as description. Say something useful. "GPIO
>> spec for" is redundant, it cannot be anything else, so basically your
>> description saod "mclk" which is the same as in property name.
> 
>> Usually clocks are not GPIOs, so description could explain that.
> 
> Should the "GPIO spec for control signal to the clock gating circuit" be
> changed to "control signal to the clock gating circuit"?

I don't see previous code, cannot even reference it via reply-to
link/header. That way of communication is not effective.


Best regards,
Krzysztof