linux-kernel - Re: [Patch v2 2/2] ASoC: dt-bindings: lpc32xx: Add lpc32xx i2s DT binding

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] [thread-next>] [day] [month] [year] [list]

Date: Tue, 11 Jun 2024 12:18:45 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Piotr Wojtaszczyk <piotr.wojtaszczyk@...esys.com>
Cc: 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>, Vladimir Zapolskiy <vz@...ia.com>,
 Russell King <linux@...linux.org.uk>, Jaroslav Kysela <perex@...ex.cz>,
 Takashi Iwai <tiwai@...e.com>, Arnd Bergmann <arnd@...db.de>,
 Chancel Liu <chancel.liu@....com>, Michael Ellerman <mpe@...erman.id.au>,
 linux-sound@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 alsa-devel@...a-project.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [Patch v2 2/2] ASoC: dt-bindings: lpc32xx: Add lpc32xx i2s DT
 binding

On 11/06/2024 11:47, Piotr Wojtaszczyk wrote:
> Add nxp,lpc3220-i2s DT binding documentation.
> 
> Signed-off-by: Piotr Wojtaszczyk <piotr.wojtaszczyk@...esys.com>
> ---

> +
> +maintainers:
> +  - Piotr Wojtaszczyk <piotr.wojtaszczyk@...esys.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nxp,lpc3220-i2s
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: input clock of the peripheral.
> +

I do not see my comment about DAI being addressed.

<form letter>
This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.
</<form letter>

> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names

Drop

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/lpc32xx-clock.h>
> +
> +    i2s0: i2s@...94000 {

Drop label, not used.

> +      compatible = "nxp,lpc3220-i2s";
> +      reg = <0x20094000 0x1000>;
> +      clocks = <&clk LPC32XX_CLK_I2S0>;
> +      clock-names = "i2s_clk";

Not tested. Drop.

> +      status = "disabled";

Then what is the point of example? Drop.

Your DTS was also not tested.

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

Best regards,
Krzysztof