diff mbox series

[v6,08/10] ASoC: dt-bindings: Add SC7280 lpass cpu bindings

Message ID 1637928282-2819-9-git-send-email-srivasam@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series Add support for audio on SC7280 based targets | expand

Commit Message

Srinivasa Rao Mandadapu Nov. 26, 2021, 12:04 p.m. UTC 
Add bindings for sc7280 lpass cpu driver which supports
audio over i2s based speaker, soundwire based headset, msm dmics
and HDMI Port.

Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
Co-developed-by: Venkata Prasad Potturu <potturu@codeaurora.org>
Signed-off-by: Venkata Prasad Potturu <potturu@codeaurora.org>
---
 .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  | 69 +++++++++++++++++++---
 1 file changed, 61 insertions(+), 8 deletions(-)

Comments

Rob Herring Nov. 27, 2021, 11:13 p.m. UTC | #1 
On Fri, 26 Nov 2021 17:34:40 +0530, Srinivasa Rao Mandadapu wrote:
> Add bindings for sc7280 lpass cpu driver which supports
> audio over i2s based speaker, soundwire based headset, msm dmics
> and HDMI Port.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> Co-developed-by: Venkata Prasad Potturu <potturu@codeaurora.org>
> Signed-off-by: Venkata Prasad Potturu <potturu@codeaurora.org>
> ---
>  .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  | 69 +++++++++++++++++++---
>  1 file changed, 61 insertions(+), 8 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.example.dt.yaml: lpass@62d80000: reg: [[0, 1658351616, 0, 425984], [0, 1659895808, 0, 167936]] is too short
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.example.dt.yaml: lpass@62d80000: reg-names: ['lpass-hdmiif', 'lpass-lpaif'] is too short
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.example.dt.yaml: lpass@62d80000: interrupts: [[0, 160, 1], [0, 268, 1]] is too short
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.example.dt.yaml: lpass@62d80000: interrupt-names: ['lpass-irq-lpaif', 'lpass-irq-hdmi'] is too short
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.example.dt.yaml: lpass@62d80000: iommus: [[4294967295, 4128, 0], [4294967295, 4146, 0]] is too short
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1560102

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Rob Herring Nov. 28, 2021, 4:53 p.m. UTC | #2 
On Fri, Nov 26, 2021 at 05:34:40PM +0530, Srinivasa Rao Mandadapu wrote:
> Add bindings for sc7280 lpass cpu driver which supports
> audio over i2s based speaker, soundwire based headset, msm dmics
> and HDMI Port.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> Co-developed-by: Venkata Prasad Potturu <potturu@codeaurora.org>
> Signed-off-by: Venkata Prasad Potturu <potturu@codeaurora.org>
> ---
>  .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  | 69 +++++++++++++++++++---
>  1 file changed, 61 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
> index 1e23c0e..0f5a57c 100644
> --- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
> @@ -22,35 +22,36 @@ properties:
>        - qcom,lpass-cpu
>        - qcom,apq8016-lpass-cpu
>        - qcom,sc7180-lpass-cpu
> +      - qcom,sc7280-lpass-cpu
>  
>    reg:
> -    maxItems: 2
> +    maxItems: 5
>      description: LPAIF core registers
>  
>    reg-names:
> -    maxItems: 2
> +    maxItems: 5
>  
>    clocks:
>      minItems: 3
> -    maxItems: 6
> +    maxItems: 7
>  
>    clock-names:
>      minItems: 3
> -    maxItems: 6
> +    maxItems: 7
>  
>    interrupts:
> -    maxItems: 2
> +    maxItems: 4
>      description: LPAIF DMA buffer interrupt
>  
>    interrupt-names:
> -    maxItems: 2
> +    maxItems: 4
>  
>    qcom,adsp:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description: Phandle for the audio DSP node
>  
>    iommus:
> -    maxItems: 2
> +    maxItems: 3
>      description: Phandle to apps_smmu node with sid mask
>  
>    power-domains:
> @@ -69,7 +70,7 @@ patternProperties:
>    "^dai-link@[0-9a-f]$":
>      type: object
>      description: |
> -      LPASS CPU dai node for each I2S device. Bindings of each node
> +      LPASS CPU dai node for each I2S device or Soundwire device. Bindings of each node
>        depends on the specific driver providing the functionality and
>        properties.
>      properties:
> @@ -174,6 +175,58 @@ allOf:
>          - iommus
>          - power-domains
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,sc7280-lpass-cpu
> +
> +    then:
> +      properties:
> +        clock-names:
> +          oneOf:
> +            - items:   #for I2S
> +                - const: lpass_aon_cc_audio_hm_h_clk
> +                - const: lpass_core_cc_sysnoc_mport_core_clk
> +                - const: lpass_core_cc_ext_if1_ibit_clk
> +            - items:   #for Soundwire
> +                - const: lpass_aon_cc_audio_hm_h_clk
> +                - const: lpass_audio_cc_codec_mem0_clk
> +                - const: lpass_audio_cc_codec_mem1_clk
> +                - const: lpass_audio_cc_codec_mem2_clk
> +            - items:   #for HDMI
> +                - const: lpass_aon_cc_audio_hm_h_clk

'lpass_' and '_clk' are redundant.

> +
> +        reg-names:
> +          anyOf:
> +            - items:   #for I2S
> +                - const: lpass-lpaif
> +            - items:   #for I2S and HDMI
> +                - const: lpass-hdmiif
> +                - const: lpass-lpaif

Doesn't this apply to other SoCs?

> +            - items:   #for I2S, soundwire and HDMI
> +                - const: lpass-cdc-lpm
> +                - const: lpass-rxtx-lpaif
> +                - const: lpass-va-lpaif
> +                - const: lpass-hdmiif
> +                - const: lpass-lpaif

'lpass-' is redundant too, but consistency across SoCs is better.

hdmiif and lpaif should be first. (Add new resources on the end.)

> +        interrupt-names:
> +          anyOf:
> +            - items:   #for I2S
> +                - const: lpass-irq-lpaif
> +            - items:   #for I2S and HDMI
> +                - const: lpass-irq-lpaif
> +                - const: lpass-irq-hdmi
> +            - items:   #for I2S, soundwire and HDMI
> +                - const: lpass-irq-lpaif
> +                - const: lpass-irq-vaif
> +                - const: lpass-irq-rxtxif
> +                - const: lpass-irq-hdmi

Again, add new entries to the end.

> +
> +      required:
> +        - iommus
> +        - power-domains
> +
>  examples:
>    - |
>      #include <dt-bindings/sound/sc7180-lpass.h>
> -- 
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
> is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
> 
>
Srinivasa Rao Mandadapu Nov. 29, 2021, 10:37 a.m. UTC | #3 
On 11/28/2021 10:23 PM, Rob Herring wrote:
Thanks for Your Time Rob!!!
> On Fri, Nov 26, 2021 at 05:34:40PM +0530, Srinivasa Rao Mandadapu wrote:
>> Add bindings for sc7280 lpass cpu driver which supports
>> audio over i2s based speaker, soundwire based headset, msm dmics
>> and HDMI Port.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
>> Co-developed-by: Venkata Prasad Potturu <potturu@codeaurora.org>
>> Signed-off-by: Venkata Prasad Potturu <potturu@codeaurora.org>
>> ---
>>   .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  | 69 +++++++++++++++++++---
>>   1 file changed, 61 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
>> index 1e23c0e..0f5a57c 100644
>> --- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
>> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
>> @@ -22,35 +22,36 @@ properties:
>>         - qcom,lpass-cpu
>>         - qcom,apq8016-lpass-cpu
>>         - qcom,sc7180-lpass-cpu
>> +      - qcom,sc7280-lpass-cpu
>>   
>>     reg:
>> -    maxItems: 2
>> +    maxItems: 5
>>       description: LPAIF core registers
>>   
>>     reg-names:
>> -    maxItems: 2
>> +    maxItems: 5
>>   
>>     clocks:
>>       minItems: 3
>> -    maxItems: 6
>> +    maxItems: 7
>>   
>>     clock-names:
>>       minItems: 3
>> -    maxItems: 6
>> +    maxItems: 7
>>   
>>     interrupts:
>> -    maxItems: 2
>> +    maxItems: 4
>>       description: LPAIF DMA buffer interrupt
>>   
>>     interrupt-names:
>> -    maxItems: 2
>> +    maxItems: 4
>>   
>>     qcom,adsp:
>>       $ref: /schemas/types.yaml#/definitions/phandle
>>       description: Phandle for the audio DSP node
>>   
>>     iommus:
>> -    maxItems: 2
>> +    maxItems: 3
>>       description: Phandle to apps_smmu node with sid mask
>>   
>>     power-domains:
>> @@ -69,7 +70,7 @@ patternProperties:
>>     "^dai-link@[0-9a-f]$":
>>       type: object
>>       description: |
>> -      LPASS CPU dai node for each I2S device. Bindings of each node
>> +      LPASS CPU dai node for each I2S device or Soundwire device. Bindings of each node
>>         depends on the specific driver providing the functionality and
>>         properties.
>>       properties:
>> @@ -174,6 +175,58 @@ allOf:
>>           - iommus
>>           - power-domains
>>   
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: qcom,sc7280-lpass-cpu
>> +
>> +    then:
>> +      properties:
>> +        clock-names:
>> +          oneOf:
>> +            - items:   #for I2S
>> +                - const: lpass_aon_cc_audio_hm_h_clk
>> +                - const: lpass_core_cc_sysnoc_mport_core_clk
>> +                - const: lpass_core_cc_ext_if1_ibit_clk
>> +            - items:   #for Soundwire
>> +                - const: lpass_aon_cc_audio_hm_h_clk
>> +                - const: lpass_audio_cc_codec_mem0_clk
>> +                - const: lpass_audio_cc_codec_mem1_clk
>> +                - const: lpass_audio_cc_codec_mem2_clk
>> +            - items:   #for HDMI
>> +                - const: lpass_aon_cc_audio_hm_h_clk
> 'lpass_' and '_clk' are redundant.
Yes. but these clock names are defined by HW design team. clock drivers 
fallowed the same,  hence in audio drivers.
>
>> +
>> +        reg-names:
>> +          anyOf:
>> +            - items:   #for I2S
>> +                - const: lpass-lpaif
>> +            - items:   #for I2S and HDMI
>> +                - const: lpass-hdmiif
>> +                - const: lpass-lpaif
> Doesn't this apply to other SoCs?
>
>> +            - items:   #for I2S, soundwire and HDMI
>> +                - const: lpass-cdc-lpm
>> +                - const: lpass-rxtx-lpaif
>> +                - const: lpass-va-lpaif
>> +                - const: lpass-hdmiif
>> +                - const: lpass-lpaif
> 'lpass-' is redundant too, but consistency across SoCs is better.
>
> hdmiif and lpaif should be first. (Add new resources on the end.)
Okay.. order is maintained as per register addresses. if it's okay, even 
address range is out of order, will change accordingly.
>
>> +        interrupt-names:
>> +          anyOf:
>> +            - items:   #for I2S
>> +                - const: lpass-irq-lpaif
>> +            - items:   #for I2S and HDMI
>> +                - const: lpass-irq-lpaif
>> +                - const: lpass-irq-hdmi
>> +            - items:   #for I2S, soundwire and HDMI
>> +                - const: lpass-irq-lpaif
>> +                - const: lpass-irq-vaif
>> +                - const: lpass-irq-rxtxif
>> +                - const: lpass-irq-hdmi
> Again, add new entries to the end.
  Replied above for the same.
>> +
>> +      required:
>> +        - iommus
>> +        - power-domains
>> +
>>   examples:
>>     - |
>>       #include <dt-bindings/sound/sc7180-lpass.h>
>> -- 
>> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
>> is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
>>
>>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
index 1e23c0e..0f5a57c 100644
--- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
@@ -22,35 +22,36 @@  properties:
       - qcom,lpass-cpu
       - qcom,apq8016-lpass-cpu
       - qcom,sc7180-lpass-cpu
+      - qcom,sc7280-lpass-cpu
 
   reg:
-    maxItems: 2
+    maxItems: 5
     description: LPAIF core registers
 
   reg-names:
-    maxItems: 2
+    maxItems: 5
 
   clocks:
     minItems: 3
-    maxItems: 6
+    maxItems: 7
 
   clock-names:
     minItems: 3
-    maxItems: 6
+    maxItems: 7
 
   interrupts:
-    maxItems: 2
+    maxItems: 4
     description: LPAIF DMA buffer interrupt
 
   interrupt-names:
-    maxItems: 2
+    maxItems: 4
 
   qcom,adsp:
     $ref: /schemas/types.yaml#/definitions/phandle
     description: Phandle for the audio DSP node
 
   iommus:
-    maxItems: 2
+    maxItems: 3
     description: Phandle to apps_smmu node with sid mask
 
   power-domains:
@@ -69,7 +70,7 @@  patternProperties:
   "^dai-link@[0-9a-f]$":
     type: object
     description: |
-      LPASS CPU dai node for each I2S device. Bindings of each node
+      LPASS CPU dai node for each I2S device or Soundwire device. Bindings of each node
       depends on the specific driver providing the functionality and
       properties.
     properties:
@@ -174,6 +175,58 @@  allOf:
         - iommus
         - power-domains
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,sc7280-lpass-cpu
+
+    then:
+      properties:
+        clock-names:
+          oneOf:
+            - items:   #for I2S
+                - const: lpass_aon_cc_audio_hm_h_clk
+                - const: lpass_core_cc_sysnoc_mport_core_clk
+                - const: lpass_core_cc_ext_if1_ibit_clk
+            - items:   #for Soundwire
+                - const: lpass_aon_cc_audio_hm_h_clk
+                - const: lpass_audio_cc_codec_mem0_clk
+                - const: lpass_audio_cc_codec_mem1_clk
+                - const: lpass_audio_cc_codec_mem2_clk
+            - items:   #for HDMI
+                - const: lpass_aon_cc_audio_hm_h_clk
+
+        reg-names:
+          anyOf:
+            - items:   #for I2S
+                - const: lpass-lpaif
+            - items:   #for I2S and HDMI
+                - const: lpass-hdmiif
+                - const: lpass-lpaif
+            - items:   #for I2S, soundwire and HDMI
+                - const: lpass-cdc-lpm
+                - const: lpass-rxtx-lpaif
+                - const: lpass-va-lpaif
+                - const: lpass-hdmiif
+                - const: lpass-lpaif
+        interrupt-names:
+          anyOf:
+            - items:   #for I2S
+                - const: lpass-irq-lpaif
+            - items:   #for I2S and HDMI
+                - const: lpass-irq-lpaif
+                - const: lpass-irq-hdmi
+            - items:   #for I2S, soundwire and HDMI
+                - const: lpass-irq-lpaif
+                - const: lpass-irq-vaif
+                - const: lpass-irq-rxtxif
+                - const: lpass-irq-hdmi
+
+      required:
+        - iommus
+        - power-domains
+
 examples:
   - |
     #include <dt-bindings/sound/sc7180-lpass.h>