ASoC: dt-bindings: cirrus,cs4270: Convert to dtschema

Commit Message

Animesh Agarwal July 7, 2024, 6:26 a.m. UTC

Convert the Cirrus Logic CS4270 audio CODEC bindings to DT schema. Add
missing va-supply, vd-supply and vlc-supply properties, because they
are already being used in the DTS and the driver for this device.

Cc: Daniel Baluta <[e-mail]>
Signed-off-by: Animesh Agarwal <[e-mail]>
---
 .../bindings/sound/cirrus,cs4270.yaml         | 53 +++++++++++++++++++
 .../devicetree/bindings/sound/cs4270.txt      | 21 --------
 2 files changed, 53 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/cirrus,cs4270.yaml
 delete mode 100644 Documentation/devicetree/bindings/sound/cs4270.txt

Comments

Krzysztof Kozlowski July 8, 2024, 7:37 a.m. UTC | #1

On 07/07/2024 08:26, Animesh Agarwal wrote:
> +
> +  reg:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    description:
> +      This pin will be deasserted before communication to the codec starts.
> +    maxItems: 1
> +
> +  va-supply:
> +    description: Voltage regulator phandle for the VA supply.

Your description is redundant: you did not say anything more than it is
already said in the property name. This could be in such case just:
": true"

Please keep this feedback for any future work (I feel I already said it
once...).

> +
> +  vd-supply:
> +    description: Voltage regulator phandle for the VD supply.
> +
> +  vlc-supply:
> +    description: Voltage regulator phandle for the VLC supply.
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false

This won't validate DTS... test the DTS. You miss dai-common.

> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      codec: cs4270@48 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
e.g. audio-codec or codec

And drop unused label "codec:"


Best regards,
Krzysztof

Animesh Agarwal July 9, 2024, 6:13 p.m. UTC | #2

xOn Mon, Jul 8, 2024 at 1:08 PM Krzysztof Kozlowski <[e-mail]> wrote:
>
> On 07/07/2024 08:26, Animesh Agarwal wrote:
> > +  va-supply:
> > +    description: Voltage regulator phandle for the VA supply.
>
> Your description is redundant: you did not say anything more than it is
> already said in the property name. This could be in such case just:
> ": true"
>
> Please keep this feedback for any future work (I feel I already said it
> once...).
>

I'll add proper descriptions for each of these instead, thanks for the heads-up.

> > +
> > +  vd-supply:
> > +    description: Voltage regulator phandle for the VD supply.
> > +
> > +  vlc-supply:
> > +    description: Voltage regulator phandle for the VLC supply.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
>
> This won't validate DTS... test the DTS. You miss dai-common.
>

Weirdly, this wasn't giving any errors upon running the dtbs_check,
however I should fix this.

> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +
> > +      codec: cs4270@48 {
>
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> e.g. audio-codec or codec
>
> And drop unused label "codec:"
>

I can see some other cirrus bindings with this mistake, maybe because
they were written/converted several years ago. I shall fix it here.

>
> Best regards,
> Krzysztof
>

Patch

diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs4270.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs4270.yaml
new file mode 100644
index 000000000000..824d799ee5ff
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/cirrus,cs4270.yaml
@@ -0,0 +1,53 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/cirrus,cs4270.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus Logic CS4270 audio CODEC
+
+maintainers:
+  - [e-mail]
+
+description:
+  The CS4270 is a stereo audio codec. The driver for this device currently only
+  supports I2C.
+
+properties:
+  compatible:
+    const: cirrus,cs4270
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    description:
+      This pin will be deasserted before communication to the codec starts.
+    maxItems: 1
+
+  va-supply:
+    description: Voltage regulator phandle for the VA supply.
+
+  vd-supply:
+    description: Voltage regulator phandle for the VD supply.
+
+  vlc-supply:
+    description: Voltage regulator phandle for the VLC supply.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      codec: cs4270@48 {
+          compatible = "cirrus,cs4270";
+          reg = <0x48>;
+      };
+    };
diff --git a/Documentation/devicetree/bindings/sound/cs4270.txt b/Documentation/devicetree/bindings/sound/cs4270.txt
deleted file mode 100644
index c33770ec4c3c..000000000000
--- a/Documentation/devicetree/bindings/sound/cs4270.txt
+++ /dev/null
@@ -1,21 +0,0 @@ 
-CS4270 audio CODEC
-
-The driver for this device currently only supports I2C.
-
-Required properties:
-
-  - compatible : "cirrus,cs4270"
-
-  - reg : the I2C address of the device for I2C
-
-Optional properties:
-
-  - reset-gpios : a GPIO spec for the reset pin. If specified, it will be
-		  deasserted before communication to the codec starts.
-
-Example:
-
-codec: cs4270@48 {
-	compatible = "cirrus,cs4270";
-	reg = <0x48>;
-};