linux-kernel - Re: [PATCH 1/2] ASoC: dt-bindings: Add cmx655 codec

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e52e6257-1091-4cf8-a757-c2ddbe348a77@kernel.org>
Date: Mon, 3 Feb 2025 21:31:46 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Nikola Jelic <nikola.jelic83@...il.com>, lgirdwood@...il.com,
 broonie@...nel.org, perex@...ex.cz, tiwai@...e.com, robh@...nel.org,
 krzk+dt@...nel.org, conor+dt@...nel.org, linux-sound@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: rwalton@...micro.com
Subject: Re: [PATCH 1/2] ASoC: dt-bindings: Add cmx655 codec

On 03/02/2025 18:01, Nikola Jelic wrote:
> +
> +title: CML Micro CMX655D codec
> +
> +maintainers:
> +  - Richard Walton <rwalton@...micro.com>
> +  - Nikola Jelic <nikola.jelic83@...il.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The CMX655D is an ultra-low power voice codec.
> +
> +allOf:
> +  - $ref: dai-common.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - cml,cmx655d
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#sound-dai-cells":
> +    const: 0
> +
> +  reset-gpios:
> +    description: GPIO used for codec reset, negative logic
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-names:
> +    maxItems: 1

Describe the names or drop. Look how other bindings do it - there is
never syntax like that alone.

> +
> +  pinctrl-names:
> +    maxItems: 1

Drop property, not needed here.

> +
> +  pinctr-0:
> +    maxItems: 1

Drop property, not needed and not correct.

> +

> +  cmx655,classd-oc-reset-attempts:
> +    description: Maximum number of times to reset CMX655 class-D
> +      following a overcurrent event.
> +      Default = 5, >10000 = disable limit.

Missing constraints, min/max. Drop also redundant part - schema already
says default 5.

But more important, I am not sure why this is board-configurable.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 5
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        status = "okay";

Drop

> +        codec: cmx655 {

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


audio-codec, codec etc.

> +            reg = <0x54>;
> +            #sound-dai-cells = <0>;
> +            compatible = "cml,cmx655d";

Follow DTS coding style. compatible must be first.

> +            reset-gpios = <&gpio 24 1>;

Use defines for standard flags.

> +            interrupt-parent = <&gpio>;
> +            interrupts = <25 0x2>;

Use defines for standard flags.

> +            interrupt-names = "irq";

Drop

> +            pinctrl-names = "default";
> +            pinctrl-0 = <&ev6550DHAT_pins>;



Best regards,
Krzysztof