linux-kernel - Re: [PATCH] ASoC: dt-bindings: cirrus,cs4271: Convert to dtschema

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: <62edecf4-6fc6-4aff-bd42-4fac171f5093@kernel.org>
Date: Mon, 26 Aug 2024 17:51:37 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: nikita.shubin@...uefel.me, David Rhodes <david.rhodes@...rus.com>,
 Richard Fitzgerald <rf@...nsource.cirrus.com>,
 Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Alexander Sverdlin <alexander.sverdlin@...il.com>
Cc: linux-sound@...r.kernel.org, patches@...nsource.cirrus.com,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ASoC: dt-bindings: cirrus,cs4271: Convert to dtschema

On 26/08/2024 11:53, Nikita Shubin via B4 Relay wrote:
> From: Nikita Shubin <nikita.shubin@...uefel.me>
> 
> Convert the Cirrus Logic CS4271 audio CODEC bindings to DT schema.
> 
> Cc: Alexander Sverdlin <alexander.sverdlin@...il.com>
> Signed-off-by: Nikita Shubin <nikita.shubin@...uefel.me>
> Link: https://lore.kernel.org/all/20240715-ep93xx-v11-0-4e924efda795@maquefel.me

I think Link goes above your SoB. Your Sob must be the last tag from you.

> ---
> This is complementary patch to ep93xx DT conversion series.

...

> +
> +title: Cirrus Logic CS4271 audio CODEC
> +
> +maintainers:
> +  - Alexander Sverdlin <alexander.sverdlin@...il.com>
> +  - Nikita Shubin <nikita.shubin@...uefel.me>
> +
> +description:
> +  The CS4271 is a stereo audio codec. This driver supports both the I2C
> +  and the SPI bus.

Drop references to driver. You can say that device supports
configuration over I2C and SPI.

> +
> +allOf:
> +  - $ref: dai-common.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: cirrus,cs4271
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-cpha: true
> +
> +  spi-cpol: true
> +
> +  '#sound-dai-cells':
> +    const: 0

None of these three were present in original binding.

> +
> +  reset-gpio:

If it bases on cs4270, look there - there is no "gpio", but "gpios".
Mention changes in commit msg.


> +    description:
> +      This pin will be deasserted before communication to the codec starts.
> +    maxItems: 1
> +
> +  va-supply:
> +    description: Analog power supply.
> +
> +  vd-supply:
> +    description: Digital power supply.
> +
> +  vl-supply:
> +    description: Serial Control Port power supply.
> +
> +  port:
> +    $ref: audio-graph-port.yaml#
> +    unevaluatedProperties: false

This wasn't present in original binding and nothing in commit msg
explained changes from pure conversion. Explain them in commit msg.

> +
> +  cirrus,amuteb-eq-bmutec:
> +    description:
> +      When given, the Codec's AMUTEB=BMUTEC flag is enabled.
> +    type: boolean
> +
> +  cirrus,enable-soft-reset:
> +    description: |
> +      The CS4271 requires its LRCLK and MCLK to be stable before its RESET
> +      line is de-asserted. That also means that clocks cannot be changed
> +      without putting the chip back into hardware reset, which also requires
> +      a complete re-initialization of all registers.
> +
> +      One (undocumented) workaround is to assert and de-assert the PDN bit
> +      in the MODE2 register. This workaround can be enabled with this DT
> +      property.
> +
> +      Note that this is not needed in case the clocks are stable
> +      throughout the entire runtime of the codec.
> +    type: boolean
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        cs4271: codec@0 {

Drop unused label.

> +            compatible = "cirrus,cs4271";
> +            reg = <0>;
> +            reset-gpio = <&gpio0 1 0>;

Use defines for flags.

Also, make the example complete. You  miss many properties.

> +        };
> +    };
> +
> +...
> \ No newline at end of file

This needs to be always fixed before posting.



Best regards,
Krzysztof