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: <DA78AT6VV956.3FZVIIIM3ZTFZ@linaro.org>
Date: Tue, 27 May 2025 21:34:30 +0100
From: "Alexey Klimov" <alexey.klimov@...aro.org>
To: "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 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".

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"?

Previous comment:
>That's not a valid syntax. Either enum or const.

>This was never tested.

>Why are you repeating the if?

These parts are no longer present.

Also i2c0 was changed to i2c in the example.

Best regards,
Alexey