diff mbox series

[v5,4/4] ASoC: dt-bindings: Add tas2781 amplifier

Message ID 20230610141821.576926-1-13916275206@139.com (mailing list archive)
State Superseded
Headers show
Series [v5,1/4] ASoC: tas2781: Add Header file for tas2781 driver | expand

Commit Message

Shenghao Ding June 10, 2023, 2:18 p.m. UTC
Create tas2781.yaml for tas2781 driver.

Signed-off-by: Shenghao Ding <13916275206@139.com>

---
Changes in v5:
 - remove ti,broadcast-addr
 - remove address-cells
 - remove size-cells
 - put compatible item first in properties
 - change the maxItems of reg from 4 to 8
 - remove white space around <>
 - correct the reg format to <0x38>, <0x3a> etc
 - remove '\t' in the file
 - correct a comment in the example
---
 .../devicetree/bindings/sound/ti,tas2781.yaml | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/ti,tas2781.yaml

Comments

Krzysztof Kozlowski June 10, 2023, 4:33 p.m. UTC | #1
On 10/06/2023 16:18, Shenghao Ding wrote:
> Create tas2781.yaml for tas2781 driver.
> 
> Signed-off-by: Shenghao Ding <13916275206@139.com>

We keep telling you that your email threading is broken. It still is
makes reviewers life more difficult. Maintainer as well as applying is
not straightforward.

Why this is not improved?

> 
> ---
> Changes in v5:
>  - remove ti,broadcast-addr
>  - remove address-cells
>  - remove size-cells
>  - put compatible item first in properties
>  - change the maxItems of reg from 4 to 8
>  - remove white space around <>
>  - correct the reg format to <0x38>, <0x3a> etc
>  - remove '\t' in the file
>  - correct a comment in the example
> ---
>  .../devicetree/bindings/sound/ti,tas2781.yaml | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/ti,tas2781.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/ti,tas2781.yaml b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml
> new file mode 100644
> index 000000000000..61db14a39630
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 - 2023 Texas Instruments Incorporated
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/ti,tas2781.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments TAS2781 SmartAMP
> +
> +maintainers:
> +  - Shenghao Ding <shenghao-ding@ti.com>
> +
> +description:
> +  The TAS2781 is a mono, digital input Class-D audio amplifier
> +  optimized for efficiently driving high peak power into small
> +  loudspeakers. Integrated an on-chip DSP supports Texas Instruments
> +  Smart Amp speaker protection algorithm. The integrated speaker
> +  voltage and current sense provides for real time
> +  monitoring of loudspeaker behavior.
> +
> +allOf:
> +  - $ref: dai-common.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,tas2781
> +
> +  reg:
> +    description:
> +      I2C address, in multiple tas2781s case, all the i2c address
> +      aggreate as one Audio Device to support multiple audio slots.
> +    maxItems: 8
> +    items:
> +      minimum: 0x38
> +      maximum: 0x3f

It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).

You miss here minItems.


Best regards,
Krzysztof
Ding, Shenghao June 12, 2023, 8:57 p.m. UTC | #2
> 在 2023年6月11日,00:33,Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 写道:
> 
> On 10/06/2023 16:18, Shenghao Ding wrote:
>> Create tas2781.yaml for tas2781 driver.
>> 
>> Signed-off-by: Shenghao Ding <13916275206@139.com>
> 
> We keep telling you that your email threading is broken. It still is
> makes reviewers life more difficult. Maintainer as well as applying is
> not straightforward.
> 
> Why this is not improved?
> 
Sorry for causing difficulties. Would you be so kind and tell me how to avoid threading broken? I get the threading broken issue every time, but I really have no idea of how to fix it.
>> 
>> ---
>> Changes in v5:
>> - remove ti,broadcast-addr
>> - remove address-cells
>> - remove size-cells
>> - put compatible item first in properties
>> - change the maxItems of reg from 4 to 8
>> - remove white space around <>
>> - correct the reg format to <0x38>, <0x3a> etc
>> - remove '\t' in the file
>> - correct a comment in the example
>> ---
>> .../devicetree/bindings/sound/ti,tas2781.yaml | 73 +++++++++++++++++++
>> 1 file changed, 73 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/sound/ti,tas2781.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/sound/ti,tas2781.yaml b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml
>> new file mode 100644
>> index 000000000000..61db14a39630
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml
>> @@ -0,0 +1,73 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright (C) 2022 - 2023 Texas Instruments Incorporated
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/sound/ti,tas2781.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Texas Instruments TAS2781 SmartAMP
>> +
>> +maintainers:
>> +  - Shenghao Ding <shenghao-ding@ti.com>
>> +
>> +description:
>> +  The TAS2781 is a mono, digital input Class-D audio amplifier
>> +  optimized for efficiently driving high peak power into small
>> +  loudspeakers. Integrated an on-chip DSP supports Texas Instruments
>> +  Smart Amp speaker protection algorithm. The integrated speaker
>> +  voltage and current sense provides for real time
>> +  monitoring of loudspeaker behavior.
>> +
>> +allOf:
>> +  - $ref: dai-common.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - ti,tas2781
>> +
>> +  reg:
>> +    description:
>> +      I2C address, in multiple tas2781s case, all the i2c address
>> +      aggreate as one Audio Device to support multiple audio slots.
>> +    maxItems: 8
>> +    items:
>> +      minimum: 0x38
>> +      maximum: 0x3f
> 
> It does not look like you tested the bindings, at least after quick
> look. Please run `make dt_binding_check` (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).
> 
Kindly point my mistake, I run the commands from the Documentation/devicetree/bindings/writing-schema.rst before submission.
For dt_binding_check, command as follow, no issue reported,
make DT_CHECKER_FLAGS=-m O= O=build_dir/x86 dt_binding_check -j $(expr $(nproc) - 1) 2>&1 | tee $OUTPUT_DIR1/$OUTPUT_DIR2/dt_binding_check.log
……………
  DTEX    Documentation/devicetree/bindings/sound/samsung,tm2.example.dts
  DTEX    Documentation/devicetree/bindings/sound/ti,tas2781.example.dts
  DTEX    Documentation/devicetree/bindings/sound/amlogic,t9015.example.dts
……………..
  DTC_CHK Documentation/devicetree/bindings/sound/samsung,tm2.example.dtb
  DTC_CHK Documentation/devicetree/bindings/sound/ti,tas2781.example.dtb
  DTC_CHK Documentation/devicetree/bindings/sound/amlogic,t9015.example.dtb
……………..

As to “make dtbs_checktest”, it can’t work in default compiling system(x86) at all. In order to test the bindings, it was integrated into the BeagleBoneBlack compiling system.
Command as follow:
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- DT_CHECKER_FLAGS=-m KCONFIG_CONFIG=config_dir/bbb_dt_config O=build_dir/dt dtbs_check -j $(expr $(nproc) - 1) 2>&1 | tee build_dir/dt/dtbs_check.log
Only two lines have something to do with the tas2781 from dtbs_check, it is platform issue, not tas2781 bindings
  DTC_CHK arch/arm/boot/dts/am335x-evm.dtb
  DTC_CHK arch/arm/boot/dts/am335x-evmsk.dtb
  DTC_CHK arch/arm/boot/dts/am335x-guardian.dtb
/usr/local/src/linux-dt/arch/arm/boot/dts/am33xx-l4.dtsi:1467.5-28: Warning (interrupts_property): /ocp/interconnect@48000000/segment@100000/target-module@9c000/i2c@0/tas2781@3a:#interrupt-cells: size is (4), expected multiple of 8
/usr/local/src/linux-dt/arch/arm/boot/dts/am33xx-l4.dtsi:1467.5-28: Warning (interrupts_property): /ocp/interconnect@48000000/segment@100000/target-module@9c000/i2c@0/tasdevice@3a:#interrupt-cells: size is (4), expected multiple of 8
/usr/local/src/linux-dt/build_dir/dt/arch/arm/boot/dts/am335x-boneblue.dtb: /: fixedregulator@2: 'anyOf' conditional fail

> You miss here minItems.
> 
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski June 12, 2023, 9:14 p.m. UTC | #3
On 12/06/2023 22:57, Ding, Shenghao wrote:
> 
> 
>> 在 2023年6月11日,00:33,Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 写道:
>>
>> On 10/06/2023 16:18, Shenghao Ding wrote:
>>> Create tas2781.yaml for tas2781 driver.
>>>
>>> Signed-off-by: Shenghao Ding <13916275206@139.com>
>>
>> We keep telling you that your email threading is broken. It still is
>> makes reviewers life more difficult. Maintainer as well as applying is
>> not straightforward.
>>
>> Why this is not improved?
>>
> Sorry for causing difficulties. Would you be so kind and tell me how to avoid threading broken? I get the threading broken issue every time, but I really have no idea of how to fix it.

git format-patch -v6 -4
git send-email v6*

You keep sending them as separate emails thus no proper threading.
Everything is explained:
https://elixir.bootlin.com/linux/v6.4-rc6/source/Documentation/process/5.Posting.rst
https://elixir.bootlin.com/linux/v6.4-rc6/source/Documentation/process/submitting-patches.rst

>>>
>>> ---
>>> Changes in v5:
>>> - remove ti,broadcast-addr
>>> - remove address-cells
>>> - remove size-cells
>>> - put compatible item first in properties
>>> - change the maxItems of reg from 4 to 8
>>> - remove white space around <>
>>> - correct the reg format to <0x38>, <0x3a> etc
>>> - remove '\t' in the file
>>> - correct a comment in the example
>>> ---
>>> .../devicetree/bindings/sound/ti,tas2781.yaml | 73 +++++++++++++++++++
>>> 1 file changed, 73 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/sound/ti,tas2781.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/ti,tas2781.yaml b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml
>>> new file mode 100644
>>> index 000000000000..61db14a39630
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml
>>> @@ -0,0 +1,73 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +# Copyright (C) 2022 - 2023 Texas Instruments Incorporated
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/sound/ti,tas2781.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Texas Instruments TAS2781 SmartAMP
>>> +
>>> +maintainers:
>>> +  - Shenghao Ding <shenghao-ding@ti.com>
>>> +
>>> +description:
>>> +  The TAS2781 is a mono, digital input Class-D audio amplifier
>>> +  optimized for efficiently driving high peak power into small
>>> +  loudspeakers. Integrated an on-chip DSP supports Texas Instruments
>>> +  Smart Amp speaker protection algorithm. The integrated speaker
>>> +  voltage and current sense provides for real time
>>> +  monitoring of loudspeaker behavior.
>>> +
>>> +allOf:
>>> +  - $ref: dai-common.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - ti,tas2781
>>> +
>>> +  reg:
>>> +    description:
>>> +      I2C address, in multiple tas2781s case, all the i2c address
>>> +      aggreate as one Audio Device to support multiple audio slots.
>>> +    maxItems: 8
>>> +    items:
>>> +      minimum: 0x38
>>> +      maximum: 0x3f
>>
>> It does not look like you tested the bindings, at least after quick
>> look. Please run `make dt_binding_check` (see
>> Documentation/devicetree/bindings/writing-schema.rst for instructions).
>>
> Kindly point my mistake, I run the commands from the Documentation/devicetree/bindings/writing-schema.rst before submission.
> For dt_binding_check, command as follow, no issue reported,
> make DT_CHECKER_FLAGS=-m O= O=build_dir/x86 dt_binding_check -j $(expr $(nproc) - 1) 2>&1 | tee $OUTPUT_DIR1/$OUTPUT_DIR2/dt_binding_check.log
> ……………
>   DTEX    Documentation/devicetree/bindings/sound/samsung,tm2.example.dts
>   DTEX    Documentation/devicetree/bindings/sound/ti,tas2781.example.dts
>   DTEX    Documentation/devicetree/bindings/sound/amlogic,t9015.example.dts

Hm, you are right, it passes the checks. There must be a bug in dtschema
around minimal items if "items" is present. You miss minItems: X, next
to maxItems.

> ……………..
>   DTC_CHK Documentation/devicetree/bindings/sound/samsung,tm2.example.dtb
>   DTC_CHK Documentation/devicetree/bindings/sound/ti,tas2781.example.dtb
>   DTC_CHK Documentation/devicetree/bindings/sound/amlogic,t9015.example.dtb
> ……………..
> 
> As to “make dtbs_checktest”, it can’t work in default compiling system(x86) at all. In order to test the bindings, it was integrated into the BeagleBoneBlack compiling system.

There is no problem running it on x86 system. We all run it there. Just
install basic cross compiler. Every distro has it...


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/ti,tas2781.yaml b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml
new file mode 100644
index 000000000000..61db14a39630
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml
@@ -0,0 +1,73 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 - 2023 Texas Instruments Incorporated
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/ti,tas2781.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments TAS2781 SmartAMP
+
+maintainers:
+  - Shenghao Ding <shenghao-ding@ti.com>
+
+description:
+  The TAS2781 is a mono, digital input Class-D audio amplifier
+  optimized for efficiently driving high peak power into small
+  loudspeakers. Integrated an on-chip DSP supports Texas Instruments
+  Smart Amp speaker protection algorithm. The integrated speaker
+  voltage and current sense provides for real time
+  monitoring of loudspeaker behavior.
+
+allOf:
+  - $ref: dai-common.yaml#
+
+properties:
+  compatible:
+    enum:
+      - ti,tas2781
+
+  reg:
+    description:
+      I2C address, in multiple tas2781s case, all the i2c address
+      aggreate as one Audio Device to support multiple audio slots.
+    maxItems: 8
+    items:
+      minimum: 0x38
+      maximum: 0x3f
+
+  reset-gpios:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  '#sound-dai-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+   #include <dt-bindings/gpio/gpio.h>
+   i2c {
+     /* example with quad tas2781s, such as tablet or pad device */
+     #address-cells = <1>;
+     #size-cells = <0>;
+     quad: codec@38 {
+       compatible = "ti,tas2781";
+       reg = <0x38>, /* Audio slot 0 */
+             <0x3a>, /* Audio slot 1 */
+             <0x39>, /* Audio slot 2 */
+             <0x3b>; /* Audio slot 3 */
+
+       #sound-dai-cells = <1>;
+       reset-gpios = <&gpio1 10 GPIO_ACTIVE_HIGH>;
+       interrupt-parent = <&gpio1>;
+       interrupts = <15>;
+     };
+   };
+...