Re: [PATCH RFC] dt-bindings: sound: Convert cirrus,cs35l36 to DT schema

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Low] The `$id` field in the YAML schema omits the `sound/` subdirectory component, which breaks the schema cross-reference system.
- [Low] The `dt_binding_check` validation will fail because the example uses properties that are not defined in the schema.
- [Low] The default value for `cirrus,classh-wk-fet-delay` is set to `100`, which violates the property's own enum constraints.
- [Low] An incorrect and completely unrelated maintainer (Bjorn Helgaas) is listed for this Cirrus Logic audio codec binding.
--

commit 6ae17828fc1b1804817814b5209c4fcb2dc8b463
Author: David Heidelberg <david@ixit.cz>

dt-bindings: sound: Convert cirrus,cs35l36 to DT schema

Converts the CS35L36 Speaker Amplifier from a text binding to a DT schema.
It updates the maintainer to the generic Cirrus email, makes VP-supply
optional based on datasheet behavior, and incorporates dai-common.yaml.

> diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs35l36.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs35l36.yaml
> new file mode 100644
> index 0000000000000..af0acaaefb68e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/cirrus,cs35l36.yaml
> @@ -0,0 +1,224 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/cirrus,cs35l36.yaml#

[Severity: Low]
Does this `$id` path need to include the `sound/` subdirectory? Without it,
the schema cross-reference system might break.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cirrus Logic CS35L36 Speaker Amplifier
> +
> +maintainers:
> +  - patches@opensource.cirrus.com
> +  - Bjorn Helgaas <bhelgaas@google.com>

[Severity: Low]
Is Bjorn Helgaas the correct maintainer to list here for this Cirrus Logic
audio codec? Since he typically maintains PCI, listing him here might
misdirect queries and patches.

[ ... ]

> +  cirrus,classh-wk-fet-delay:
> +    description: Weak-FET entry delay in ms
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 100

[Severity: Low]
Will this default value of 100 cause schema validation to fail? The enum
below only allows values from 0 to 7, where 4 seems to map to 100ms.

> +    enum:
> +      - 0 # 0
> +      - 1 # 5
> +      - 2 # 10
> +      - 3 # 50
> +      - 4 # 100
> +      - 5 # 200
> +      - 6 # 500
> +      - 7 # 1000

[ ... ]

> +            cirrus,boost-ind-nanohenry = <1000>;
> +            cirrus,boost-ctl-millivolt = <10000>;
> +            cirrus,boost-peak-milliamp = <4500>;
> +            cirrus,boost-ctl-select = <0x00>;
> +            cirrus,weak-fet-delay = <4>;
> +            cirrus,weak-fet-thld = <0x01>;

[Severity: Low]
Could this example cause `dt_binding_check` to fail? It uses
`cirrus,weak-fet-delay` and `cirrus,weak-fet-thld`, but the schema actually
defines these properties as `cirrus,classh-wk-fet-delay` and
`cirrus,classh-weak-fet-thld-millivolt` respectively. With
`unevaluatedProperties: false` set, undocumented properties will trigger a
fatal schema error.

> +            cirrus,temp-warn-threshold = <1>;
> +            cirrus,multi-amp-mode;
> +            cirrus,irq-drive-select = <1>;
> +            cirrus,irq-gpio-select = <0x01>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-dt-cirrus-cs35l36-v1-1-1a43515666ad@ixit.cz?part=1