diff mbox series

ASoC: dt-bindings: qcom,apq8016-sbc-sndcard: move to separate binding

Message ID 20240723083300.35605-1-krzysztof.kozlowski@linaro.org
State New
Headers show
Series ASoC: dt-bindings: qcom,apq8016-sbc-sndcard: move to separate binding | expand

Commit Message

Krzysztof Kozlowski July 23, 2024, 8:33 a.m. UTC 
The APQ8016 SBC and MSM8916 QDSP6 sound cards are a bit different from
others: they have additional IO muxing address space and pin control.
Move them to separate schema, so the original qcom,sm8250.yaml will be
easier to manage.  New schema is going to grow for other platforms
having more of IO muxing address spaces.

Cc: Adam Skladowski <a39.skl@gmail.com>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../sound/qcom,apq8016-sbc-sndcard.yaml       | 205 ++++++++++++++++++
 .../bindings/sound/qcom,sm8250.yaml           | 137 ------------
 2 files changed, 205 insertions(+), 137 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/qcom,apq8016-sbc-sndcard.yaml

Comments

Stephan Gerhold July 27, 2024, 8:23 p.m. UTC | #1 
On Tue, Jul 23, 2024 at 10:33:00AM +0200, Krzysztof Kozlowski wrote:
>The APQ8016 SBC and MSM8916 QDSP6 sound cards are a bit different from
>others: they have additional IO muxing address space and pin control.
>Move them to separate schema, so the original qcom,sm8250.yaml will be
>easier to manage.  New schema is going to grow for other platforms
>having more of IO muxing address spaces.
>
>Cc: Adam Skladowski <a39.skl@gmail.com>
>Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>---
> .../sound/qcom,apq8016-sbc-sndcard.yaml       | 205 ++++++++++++++++++
> .../bindings/sound/qcom,sm8250.yaml           | 137 ------------
> 2 files changed, 205 insertions(+), 137 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/sound/qcom,apq8016-sbc-sndcard.yaml
>
>diff --git a/Documentation/devicetree/bindings/sound/qcom,apq8016-sbc-sndcard.yaml b/Documentation/devicetree/bindings/sound/qcom,apq8016-sbc-sndcard.yaml
>new file mode 100644
>index 000000000000..6ad451549036
> [...]
>diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>index c9076dcd44c1..1d3acdc0c733 100644
>--- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>+++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>@@ -27,9 +27,7 @@ properties:
>               - qcom,sm8650-sndcard
>           - const: qcom,sm8450-sndcard
>       - enum:
>-          - qcom,apq8016-sbc-sndcard
>           - qcom,apq8096-sndcard
>-          - qcom,msm8916-qdsp6-sndcard
>           - qcom,qcm6490-idp-sndcard
>           - qcom,qcs6490-rb3gen2-sndcard
>           - qcom,qrb5165-rb5-sndcard
>@@ -58,18 +56,6 @@ properties:
>     $ref: /schemas/types.yaml#/definitions/string
>     description: User visible long sound card name
>
>-  pin-switches:
>-    description: List of widget names for which pin switches should be created.
>-    $ref: /schemas/types.yaml#/definitions/string-array
>-
>-  widgets:
>-    description: User specified audio sound widgets.
>-    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>-

These two properties are also valid and supported on all newer
platforms, please keep them here! There are certain use cases where
these are needed independent of the platform, e.g. to control an analog
switch or mux connected to speaker or headphone outputs.

I agree that it is cleaner to move the IO muxing to a new schema though.
Perhaps we could define something like a shared qcom,sndcard-common.yaml
schema to avoid duplication for these generic properties? In the Linux
driver, these are handled for all platforms in sound/soc/qcom/common.c.

Thanks,
Stephan
Stephan Gerhold July 30, 2024, 10:30 a.m. UTC | #2 
On Sun, Jul 28, 2024 at 12:30:12PM +0200, Krzysztof Kozlowski wrote:
> On 27/07/2024 22:23, Stephan Gerhold wrote:
> > On Tue, Jul 23, 2024 at 10:33:00AM +0200, Krzysztof Kozlowski wrote:
> >> The APQ8016 SBC and MSM8916 QDSP6 sound cards are a bit different from
> >> others: they have additional IO muxing address space and pin control.
> >> Move them to separate schema, so the original qcom,sm8250.yaml will be
> >> easier to manage.  New schema is going to grow for other platforms
> >> having more of IO muxing address spaces.
> >>
> >> Cc: Adam Skladowski <a39.skl@gmail.com>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> ---
> >> .../sound/qcom,apq8016-sbc-sndcard.yaml       | 205 ++++++++++++++++++
> >> .../bindings/sound/qcom,sm8250.yaml           | 137 ------------
> >> 2 files changed, 205 insertions(+), 137 deletions(-)
> >> create mode 100644 Documentation/devicetree/bindings/sound/qcom,apq8016-sbc-sndcard.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/sound/qcom,apq8016-sbc-sndcard.yaml b/Documentation/devicetree/bindings/sound/qcom,apq8016-sbc-sndcard.yaml
> >> new file mode 100644
> >> index 000000000000..6ad451549036
> >> [...]
> >> diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
> >> index c9076dcd44c1..1d3acdc0c733 100644
> >> --- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
> >> +++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
> >> @@ -27,9 +27,7 @@ properties:
> >>               - qcom,sm8650-sndcard
> >>           - const: qcom,sm8450-sndcard
> >>       - enum:
> >> -          - qcom,apq8016-sbc-sndcard
> >>           - qcom,apq8096-sndcard
> >> -          - qcom,msm8916-qdsp6-sndcard
> >>           - qcom,qcm6490-idp-sndcard
> >>           - qcom,qcs6490-rb3gen2-sndcard
> >>           - qcom,qrb5165-rb5-sndcard
> >> @@ -58,18 +56,6 @@ properties:
> >>     $ref: /schemas/types.yaml#/definitions/string
> >>     description: User visible long sound card name
> >>
> >> -  pin-switches:
> >> -    description: List of widget names for which pin switches should be created.
> >> -    $ref: /schemas/types.yaml#/definitions/string-array
> >> -
> >> -  widgets:
> >> -    description: User specified audio sound widgets.
> >> -    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> >> -
> > 
> > These two properties are also valid and supported on all newer
> > platforms, please keep them here! There are certain use cases where
> > these are needed independent of the platform, e.g. to control an analog
> > switch or mux connected to speaker or headphone outputs.
> > 
> > I agree that it is cleaner to move the IO muxing to a new schema though.
> > Perhaps we could define something like a shared qcom,sndcard-common.yaml
> > schema to avoid duplication for these generic properties? In the Linux
> > driver, these are handled for all platforms in sound/soc/qcom/common.c.
> 
> This was added to the common driver code but it does not mean it is
> reasonable binding. I don't understand why for example we even accept
> here aux-devs, instead of putting them into one of DAI links.
> 

The auxiliary devices (typically analog audio components) are not
necessarily related to one particular digital audio interface link. It
is typically the case (e.g. an analog speaker amplifier connected in
parallel to the headphone output of one of the codecs), but I don't
think we can assume that as a general rule. There are often multiple DAI
links that go to one codec and then it might be tricky to decide which
of the DAI links the aux-dev belongs to.

> The pin-switches and widgets could be used, but are they? The only valid
> argument to keep them is that you added them to common driver code.

These go hand in hand with the aux-devs property. If you have multiple
analog audio components connected to a codec output (e.g. an analog
speaker amplifier connected to the codec headphone output) then the
pin-switches/widgets describe that the output paths (speaker and
headphones) should be separately controllable.

The alternative for pin-switches and widgets in the DT is to hardcode
them in the ASoC board/machine driver with a separate board-specific
compatible. Personally, I would prefer to keep the machine driver
specific to the SoC instead of having definitions for each and every
board.

Thanks,
Stephan
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/qcom,apq8016-sbc-sndcard.yaml b/Documentation/devicetree/bindings/sound/qcom,apq8016-sbc-sndcard.yaml
new file mode 100644
index 000000000000..6ad451549036
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/qcom,apq8016-sbc-sndcard.yaml
@@ -0,0 +1,205 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/qcom,apq8016-sbc-sndcard.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm APQ8016 and similar sound cards
+
+maintainers:
+  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
+  - Stephan Gerhold <stephan@gerhold.net>
+
+properties:
+  compatible:
+    enum:
+      - qcom,apq8016-sbc-sndcard
+      - qcom,msm8916-qdsp6-sndcard
+
+  reg:
+    items:
+      - description: Microphone I/O mux register address
+      - description: Speaker I/O mux register address
+
+  reg-names:
+    items:
+      - const: mic-iomux
+      - const: spkr-iomux
+
+  audio-routing:
+    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+    description:
+      A list of the connections between audio components. Each entry is a
+      pair of strings, the first being the connection's sink, the second
+      being the connection's source. Valid names could be power supplies,
+      MicBias of codec and the jacks on the board.
+
+  aux-devs:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: |
+      List of phandles pointing to auxiliary devices, such
+      as amplifiers, to be added to the sound card.
+
+  model:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: User visible long sound card name
+
+  pin-switches:
+    description: List of widget names for which pin switches should be created.
+    $ref: /schemas/types.yaml#/definitions/string-array
+
+  widgets:
+    description: User specified audio sound widgets.
+    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+
+patternProperties:
+  ".*-dai-link$":
+    description:
+      Each subnode represents a dai link. Subnodes of each dai links would be
+      cpu/codec dais.
+
+    type: object
+
+    properties:
+      link-name:
+        description: Indicates dai-link name and PCM stream name.
+        $ref: /schemas/types.yaml#/definitions/string
+        maxItems: 1
+
+      cpu:
+        description: Holds subnode which indicates cpu dai.
+        type: object
+        additionalProperties: false
+
+        properties:
+          sound-dai:
+            maxItems: 1
+
+      platform:
+        description: Holds subnode which indicates platform dai.
+        type: object
+        additionalProperties: false
+
+        properties:
+          sound-dai:
+            maxItems: 1
+
+      codec:
+        description: Holds subnode which indicates codec dai.
+        type: object
+        additionalProperties: false
+
+        properties:
+          sound-dai:
+            minItems: 1
+            maxItems: 8
+
+    required:
+      - link-name
+      - cpu
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - model
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/sound/qcom,lpass.h>
+    sound@7702000 {
+        compatible = "qcom,apq8016-sbc-sndcard";
+        reg = <0x07702000 0x4>, <0x07702004 0x4>;
+        reg-names = "mic-iomux", "spkr-iomux";
+
+        model = "DB410c";
+        audio-routing =
+            "AMIC2", "MIC BIAS Internal2",
+            "AMIC3", "MIC BIAS External1";
+
+        pinctrl-0 = <&cdc_pdm_lines_act &ext_sec_tlmm_lines_act &ext_mclk_tlmm_lines_act>;
+        pinctrl-1 = <&cdc_pdm_lines_sus &ext_sec_tlmm_lines_sus &ext_mclk_tlmm_lines_sus>;
+        pinctrl-names = "default", "sleep";
+
+        quaternary-dai-link {
+            link-name = "ADV7533";
+            cpu {
+                sound-dai = <&lpass MI2S_QUATERNARY>;
+            };
+            codec {
+                sound-dai = <&adv_bridge 0>;
+            };
+        };
+
+        primary-dai-link {
+            link-name = "WCD";
+            cpu {
+                sound-dai = <&lpass MI2S_PRIMARY>;
+            };
+            codec {
+                sound-dai = <&lpass_codec 0>, <&wcd_codec 0>;
+            };
+        };
+
+        tertiary-dai-link {
+            link-name = "WCD-Capture";
+            cpu {
+                sound-dai = <&lpass MI2S_TERTIARY>;
+            };
+            codec {
+                sound-dai = <&lpass_codec 1>, <&wcd_codec 1>;
+            };
+        };
+    };
+
+  - |
+    #include <dt-bindings/sound/qcom,q6afe.h>
+    #include <dt-bindings/sound/qcom,q6asm.h>
+    sound@7702000 {
+        compatible = "qcom,msm8916-qdsp6-sndcard";
+        reg = <0x07702000 0x4>, <0x07702004 0x4>;
+        reg-names = "mic-iomux", "spkr-iomux";
+
+        model = "msm8916";
+        widgets =
+            "Speaker", "Speaker",
+            "Headphone", "Headphones";
+        pin-switches = "Speaker";
+        audio-routing =
+            "Speaker", "Speaker Amp OUT",
+            "Speaker Amp IN", "HPH_R",
+            "Headphones", "HPH_L",
+            "Headphones", "HPH_R",
+            "AMIC1", "MIC BIAS Internal1",
+            "AMIC2", "MIC BIAS Internal2",
+            "AMIC3", "MIC BIAS Internal3";
+        aux-devs = <&speaker_amp>;
+
+        pinctrl-names = "default", "sleep";
+        pinctrl-0 = <&cdc_pdm_lines_act>;
+        pinctrl-1 = <&cdc_pdm_lines_sus>;
+
+        mm1-dai-link {
+            link-name = "MultiMedia1";
+            cpu {
+                sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA1>;
+            };
+        };
+
+        primary-dai-link {
+            link-name = "Primary MI2S";
+            cpu {
+                sound-dai = <&q6afedai PRIMARY_MI2S_RX>;
+            };
+            platform {
+                sound-dai = <&q6routing>;
+            };
+            codec {
+                sound-dai = <&lpass_codec 0>, <&wcd_codec 0>;
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
index c9076dcd44c1..1d3acdc0c733 100644
--- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
@@ -27,9 +27,7 @@  properties:
               - qcom,sm8650-sndcard
           - const: qcom,sm8450-sndcard
       - enum:
-          - qcom,apq8016-sbc-sndcard
           - qcom,apq8096-sndcard
-          - qcom,msm8916-qdsp6-sndcard
           - qcom,qcm6490-idp-sndcard
           - qcom,qcs6490-rb3gen2-sndcard
           - qcom,qrb5165-rb5-sndcard
@@ -58,18 +56,6 @@  properties:
     $ref: /schemas/types.yaml#/definitions/string
     description: User visible long sound card name
 
-  pin-switches:
-    description: List of widget names for which pin switches should be created.
-    $ref: /schemas/types.yaml#/definitions/string-array
-
-  widgets:
-    description: User specified audio sound widgets.
-    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
-
-  # Only valid for some compatibles (see allOf if below)
-  reg: true
-  reg-names: true
-
 patternProperties:
   ".*-dai-link$":
     description:
@@ -122,34 +108,6 @@  required:
   - compatible
   - model
 
-allOf:
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - qcom,apq8016-sbc-sndcard
-              - qcom,msm8916-qdsp6-sndcard
-    then:
-      properties:
-        reg:
-          items:
-            - description: Microphone I/O mux register address
-            - description: Speaker I/O mux register address
-        reg-names:
-          items:
-            - const: mic-iomux
-            - const: spkr-iomux
-      required:
-        - compatible
-        - model
-        - reg
-        - reg-names
-    else:
-      properties:
-        reg: false
-        reg-names: false
-
 additionalProperties: false
 
 examples:
@@ -231,98 +189,3 @@  examples:
             };
         };
     };
-
-  - |
-    #include <dt-bindings/sound/qcom,lpass.h>
-    sound@7702000 {
-        compatible = "qcom,apq8016-sbc-sndcard";
-        reg = <0x07702000 0x4>, <0x07702004 0x4>;
-        reg-names = "mic-iomux", "spkr-iomux";
-
-        model = "DB410c";
-        audio-routing =
-            "AMIC2", "MIC BIAS Internal2",
-            "AMIC3", "MIC BIAS External1";
-
-        pinctrl-0 = <&cdc_pdm_lines_act &ext_sec_tlmm_lines_act &ext_mclk_tlmm_lines_act>;
-        pinctrl-1 = <&cdc_pdm_lines_sus &ext_sec_tlmm_lines_sus &ext_mclk_tlmm_lines_sus>;
-        pinctrl-names = "default", "sleep";
-
-        quaternary-dai-link {
-            link-name = "ADV7533";
-            cpu {
-                sound-dai = <&lpass MI2S_QUATERNARY>;
-            };
-            codec {
-                sound-dai = <&adv_bridge 0>;
-            };
-        };
-
-        primary-dai-link {
-            link-name = "WCD";
-            cpu {
-                sound-dai = <&lpass MI2S_PRIMARY>;
-            };
-            codec {
-                sound-dai = <&lpass_codec 0>, <&wcd_codec 0>;
-            };
-        };
-
-        tertiary-dai-link {
-            link-name = "WCD-Capture";
-            cpu {
-                sound-dai = <&lpass MI2S_TERTIARY>;
-            };
-            codec {
-                sound-dai = <&lpass_codec 1>, <&wcd_codec 1>;
-            };
-        };
-    };
-
-  - |
-    #include <dt-bindings/sound/qcom,q6afe.h>
-    #include <dt-bindings/sound/qcom,q6asm.h>
-    sound@7702000 {
-        compatible = "qcom,msm8916-qdsp6-sndcard";
-        reg = <0x07702000 0x4>, <0x07702004 0x4>;
-        reg-names = "mic-iomux", "spkr-iomux";
-
-        model = "msm8916";
-        widgets =
-            "Speaker", "Speaker",
-            "Headphone", "Headphones";
-        pin-switches = "Speaker";
-        audio-routing =
-            "Speaker", "Speaker Amp OUT",
-            "Speaker Amp IN", "HPH_R",
-            "Headphones", "HPH_L",
-            "Headphones", "HPH_R",
-            "AMIC1", "MIC BIAS Internal1",
-            "AMIC2", "MIC BIAS Internal2",
-            "AMIC3", "MIC BIAS Internal3";
-        aux-devs = <&speaker_amp>;
-
-        pinctrl-names = "default", "sleep";
-        pinctrl-0 = <&cdc_pdm_lines_act>;
-        pinctrl-1 = <&cdc_pdm_lines_sus>;
-
-        mm1-dai-link {
-            link-name = "MultiMedia1";
-            cpu {
-                sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA1>;
-            };
-        };
-
-        primary-dai-link {
-            link-name = "Primary MI2S";
-            cpu {
-                sound-dai = <&q6afedai PRIMARY_MI2S_RX>;
-            };
-            platform {
-                sound-dai = <&q6routing>;
-            };
-            codec {
-                sound-dai = <&lpass_codec 0>, <&wcd_codec 0>;
-            };
-        };
-    };