WSA881X also supports analog mode when device is configured via i2c
only. Document it, add properties, new compatibles and example.
Cc: Srinivas Kandagatla <srini@kernel.org>
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.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
reg:
maxItems: 1
@@ -35,17 +37,51 @@ properties:
'#sound-dai-cells':
const: 0
+ clocks:
+ maxItems: 1
+
+ mclk-gpios:
+ description: GPIO spec for control signal to the clock gating circuit
+ maxItems: 1
+
required:
- compatible
- reg
- powerdown-gpios
- - "#thermal-sensor-cells"
- - "#sound-dai-cells"
+ - '#thermal-sensor-cells'
+ - '#sound-dai-cells'
+
+allOf:
+ - $ref: dai-common.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,wsa8810
+ - qcom,wsa8815
+ then:
+ properties:
+ reg:
+ description:
+ In case of analog mode this should be I2C address of the digital
+ part of the device. The I2C address of analog part of an amplifier
+ is expected to be located at the fixed offset.
+ maxItems: 1
+ items:
+ minimum: 0x0e
+ maximum: 0x0f
+ required:
+ - clocks
+ - mclk-gpios
unevaluatedProperties: false
examples:
- |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/sound/qcom,q6afe.h>
+
soundwire@c2d0000 {
#address-cells = <2>;
#size-cells = <0>;
@@ -68,4 +104,18 @@ examples:
};
};
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ amplifier@e {
+ compatible = "qcom,wsa8810";
+ reg = <0x0e>;
+ clocks = <&q6afecc LPASS_CLK_ID_MCLK_3 LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
+ powerdown-gpios = <&lpass_tlmm 16 GPIO_ACTIVE_LOW>;
+ mclk-gpios = <&lpass_tlmm 18 GPIO_ACTIVE_HIGH>;
+ #sound-dai-cells = <0>;
+ #thermal-sensor-cells = <0>;
+ };
+ };
...
--
2.47.2On 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@kernel.org> > Signed-off-by: Alexey Klimov <alexey.klimov@linaro.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. Best regards, Krzysztof
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@kernel.org> >> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.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
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@kernel.org> >>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.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
© 2016 - 2025 Red Hat, Inc.