diff mbox series

New yaml file: tas2781

Message ID 20221220144114.2137-1-luminlong@139.com
State New
Headers show
Series New yaml file: tas2781 | expand

Commit Message

Kevin Lu Dec. 20, 2022, 2:41 p.m. UTC
Add DTS discription for tas2781 driver code

Signed-off-by: Kevin Lu <luminlong@139.com>
---
 .../devicetree/bindings/sound/tas2781.yaml    | 90 +++++++++++++++++++
 1 file changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/tas2781.yaml

Comments

Krzysztof Kozlowski Dec. 20, 2022, 4:04 p.m. UTC | #1
On 20/12/2022 15:41, Kevin Lu wrote:
> Add DTS discription for tas2781 driver code

1. Use subject prefixes matching the subsystem (git log --oneline -- ...).

2. Anyway that's not correct subject. Look at existing commits.

3. Run spell check.

4. Missing full stop.

> 
> Signed-off-by: Kevin Lu <luminlong@139.com>
> ---
>  .../devicetree/bindings/sound/tas2781.yaml    | 90 +++++++++++++++++++
>  1 file changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/tas2781.yaml

Filename with vendor prefix, just like compatible.

> 
> diff --git a/Documentation/devicetree/bindings/sound/tas2781.yaml b/Documentation/devicetree/bindings/sound/tas2781.yaml
> new file mode 100644
> index 000000000..96fa45bf6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/tas2781.yaml
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 Texas Instruments Incorporated
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/sound/tas2781.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"

Drop quotes from both.

> +
> +title: Texas Instruments TAS2781 Smart PA
> +
> +maintainers:
> +  - Shenghao Ding <shenghao-ding@ti.com>
> +  - Kevin Lu <kevin-lu@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.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,tas2781
> +      - ti,audev

That's not a correct (real) compatible. Drop.

> +    description: |
> +        ti,audev will disable the irq of tas2781.

Drop description.

Missing blank line.

> +  reg:
> +    maxItems: 1
> +    description: |
> +       I2C address of the device can be between 0x38 to 0x40.
> +
> +  reset-gpioN:

Nope, use existing property. See gpio-consumer-common

> +    maxItems: 4
> +    description: GPIO used to reset the device.

This does not work like that. You did not even test it, right?

> +
> +  ti,topleft-channel:
> +    maxItems: 1
> +    description: I2C Address for each specific device.

No clue what's this. You need to explain in details in description.

> +
> +  ti,topright-channel:
> +    maxItems: 1

Ditto and in all other places.

> +
> +  ti,bottomleft-channel:
> +    maxItems: 1
> +
> +  ti,bottomright-channel:
> +    maxItems: 1
> +
> +  ti,global-address:
> +    maxItems: 1
> +    description: This item is not mandatory. if the device support gloabel mode, this item should be active.

I have no clue what is gloabel mode but the field looks incorrect. Drop
or properly describe.

Also wrong line wrapping.

> +
> +  ti,irq-gpio:
> +    maxItems: 1
> +    description: GPIO used to interrupt the device.

No. Use interrupts.

> +
> +  '#sound-dai-cells':
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +   #include <dt-bindings/gpio/gpio.h>
> +   i2c0 {

i2c

> +     #address-cells = <1>;
> +     #size-cells = <0>;
> +     codec: codec@38 {
> +       compatible = "ti,tas2781";
> +       reg = <0x38>;
> +       #sound-dai-cells = <1>;
> +       ti,topleft-channel = <0x38>;
> +       ti,topright-channel = <0x39>;
> +       ti,bottomright-channel = <0x3a>;
> +       ti,bottomright-channel = <0x3b>;
> +       ti,global-address = <0x40>;
> +       ti,reset-gpio0 = <&gpio1 10 GPIO_ACTIVE_HIGH>;
> +       ti,reset-gpio1 = <&gpio1 11 GPIO_ACTIVE_HIGH>;
> +       ti,reset-gpio2 = <&gpio1 12 GPIO_ACTIVE_HIGH>;
> +       ti,reset-gpio3 = <&gpio1 13 GPIO_ACTIVE_HIGH>;


Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).


Best regards,
Krzysztof
Rob Herring Dec. 20, 2022, 4:40 p.m. UTC | #2
On Tue, 20 Dec 2022 22:41:14 +0800, Kevin Lu wrote:
> Add DTS discription for tas2781 driver code
> 
> Signed-off-by: Kevin Lu <luminlong@139.com>
> ---
>  .../devicetree/bindings/sound/tas2781.yaml    | 90 +++++++++++++++++++
>  1 file changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/tas2781.yaml
> 

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/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,bottomright-channel: 'oneOf' conditional failed, one must be fixed:
	'type' is a required property
		hint: A vendor boolean property can use "type: boolean"
	'description' is a required property
		hint: A vendor boolean property can use "type: boolean"
	Additional properties are not allowed ('maxItems' was unexpected)
		hint: A vendor boolean property can use "type: boolean"
	/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,bottomright-channel: 'oneOf' conditional failed, one must be fixed:
		'enum' is a required property
		'const' is a required property
		hint: A vendor string property with exact values has an implicit type
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,bottomright-channel: 'oneOf' conditional failed, one must be fixed:
		'$ref' is a required property
		'allOf' is a required property
		hint: A vendor property needs a $ref to types.yaml
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,topleft-channel: 'oneOf' conditional failed, one must be fixed:
	'type' is a required property
		hint: A vendor boolean property can use "type: boolean"
	Additional properties are not allowed ('maxItems' was unexpected)
		hint: A vendor boolean property can use "type: boolean"
	/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,topleft-channel: 'oneOf' conditional failed, one must be fixed:
		'enum' is a required property
		'const' is a required property
		hint: A vendor string property with exact values has an implicit type
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,topleft-channel: 'oneOf' conditional failed, one must be fixed:
		'$ref' is a required property
		'allOf' is a required property
		hint: A vendor property needs a $ref to types.yaml
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,bottomleft-channel: 'oneOf' conditional failed, one must be fixed:
	'type' is a required property
		hint: A vendor boolean property can use "type: boolean"
	'description' is a required property
		hint: A vendor boolean property can use "type: boolean"
	Additional properties are not allowed ('maxItems' was unexpected)
		hint: A vendor boolean property can use "type: boolean"
	/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,bottomleft-channel: 'oneOf' conditional failed, one must be fixed:
		'enum' is a required property
		'const' is a required property
		hint: A vendor string property with exact values has an implicit type
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,bottomleft-channel: 'oneOf' conditional failed, one must be fixed:
		'$ref' is a required property
		'allOf' is a required property
		hint: A vendor property needs a $ref to types.yaml
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,topright-channel: 'oneOf' conditional failed, one must be fixed:
	'type' is a required property
		hint: A vendor boolean property can use "type: boolean"
	'description' is a required property
		hint: A vendor boolean property can use "type: boolean"
	Additional properties are not allowed ('maxItems' was unexpected)
		hint: A vendor boolean property can use "type: boolean"
	/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,topright-channel: 'oneOf' conditional failed, one must be fixed:
		'enum' is a required property
		'const' is a required property
		hint: A vendor string property with exact values has an implicit type
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,topright-channel: 'oneOf' conditional failed, one must be fixed:
		'$ref' is a required property
		'allOf' is a required property
		hint: A vendor property needs a $ref to types.yaml
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,global-address: 'oneOf' conditional failed, one must be fixed:
	'type' is a required property
		hint: A vendor boolean property can use "type: boolean"
	Additional properties are not allowed ('maxItems' was unexpected)
		hint: A vendor boolean property can use "type: boolean"
	/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,global-address: 'oneOf' conditional failed, one must be fixed:
		'enum' is a required property
		'const' is a required property
		hint: A vendor string property with exact values has an implicit type
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/tas2781.yaml: properties:ti,global-address: 'oneOf' conditional failed, one must be fixed:
		'$ref' is a required property
		'allOf' is a required property
		hint: A vendor property needs a $ref to types.yaml
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
Documentation/devicetree/bindings/sound/tas2781.example.dts:28.13-45: ERROR (duplicate_property_names): /example-0/i2c0/codec@38:ti,bottomright-channel: Duplicate property name
ERROR: Input tree has errors, aborting (use -f to force output)
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/sound/tas2781.example.dtb] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221220144114.2137-1-luminlong@139.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/tas2781.yaml b/Documentation/devicetree/bindings/sound/tas2781.yaml
new file mode 100644
index 000000000..96fa45bf6
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/tas2781.yaml
@@ -0,0 +1,90 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 Texas Instruments Incorporated
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/sound/tas2781.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Texas Instruments TAS2781 Smart PA
+
+maintainers:
+  - Shenghao Ding <shenghao-ding@ti.com>
+  - Kevin Lu <kevin-lu@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.
+
+properties:
+  compatible:
+    enum:
+      - ti,tas2781
+      - ti,audev
+    description: |
+        ti,audev will disable the irq of tas2781.
+  reg:
+    maxItems: 1
+    description: |
+       I2C address of the device can be between 0x38 to 0x40.
+
+  reset-gpioN:
+    maxItems: 4
+    description: GPIO used to reset the device.
+
+  ti,topleft-channel:
+    maxItems: 1
+    description: I2C Address for each specific device.
+
+  ti,topright-channel:
+    maxItems: 1
+
+  ti,bottomleft-channel:
+    maxItems: 1
+
+  ti,bottomright-channel:
+    maxItems: 1
+
+  ti,global-address:
+    maxItems: 1
+    description: This item is not mandatory. if the device support gloabel mode, this item should be active.
+
+  ti,irq-gpio:
+    maxItems: 1
+    description: GPIO used to interrupt the device.
+
+  '#sound-dai-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+   #include <dt-bindings/gpio/gpio.h>
+   i2c0 {
+     #address-cells = <1>;
+     #size-cells = <0>;
+     codec: codec@38 {
+       compatible = "ti,tas2781";
+       reg = <0x38>;
+       #sound-dai-cells = <1>;
+       ti,topleft-channel = <0x38>;
+       ti,topright-channel = <0x39>;
+       ti,bottomright-channel = <0x3a>;
+       ti,bottomright-channel = <0x3b>;
+       ti,global-address = <0x40>;
+       ti,reset-gpio0 = <&gpio1 10 GPIO_ACTIVE_HIGH>;
+       ti,reset-gpio1 = <&gpio1 11 GPIO_ACTIVE_HIGH>;
+       ti,reset-gpio2 = <&gpio1 12 GPIO_ACTIVE_HIGH>;
+       ti,reset-gpio3 = <&gpio1 13 GPIO_ACTIVE_HIGH>;
+       ti,irq-gpio = <&gpio1 15 0>;
+     };
+   };
+...