linux-kernel - Re: [PATCH] ASoC: dt-bindings: tegra20-ac97: convert to dt schema

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]

Date: Sun, 21 Apr 2024 20:25:03 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Mohammad Shehar Yaar Tausif <sheharyaar48@...il.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>, Thierry Reding
 <thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>
Cc: Thierry Reding <treding@...dia.com>, linux-sound@...r.kernel.org,
 devicetree@...r.kernel.org, linux-tegra@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ASoC: dt-bindings: tegra20-ac97: convert to dt schema

On 21/04/2024 13:09, Mohammad Shehar Yaar Tausif wrote:
> Convert NVIDIA Tegra20 AC97 binding to DT schema.
> 
> Signed-off-by: Mohammad Shehar Yaar Tausif <sheharyaar48@...il.com>

Thank you for your patch. There is something to discuss/improve.

> -};
> diff --git a/Documentation/devicetree/bindings/sound/nvidia,tegra20-ac97.yaml b/Documentation/devicetree/bindings/sound/nvidia,tegra20-ac97.yaml
> new file mode 100644
> index 000000000000..e7ffa93eec30
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/nvidia,tegra20-ac97.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/nvidia,tegra20-ac97.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#

All my other comments apply. Please confirm that you tested DTS.

> +
> +title: NVIDIA Tegra20 AC97 controller
> +
> +maintainers:
> +  - Thierry Reding <treding@...dia.com>
> +  - Jon Hunter <jonathanh@...dia.com>
> +
> +properties:
> +  compatible:
> +    const: nvidia,tegra20-ac97
> +
> +  reg:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    const: ac97
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 1

Nope, this must be maxItems instead.

> +
> +  dmas:
> +    minItems: 2

No, maxItems instead.


> +
> +  dma-names:
> +    items:
> +      - const: rx
> +      - const: tx
> +
> +  nvidia,codec-reset-gpio:


-gpios instead and mention this in commit msg.

> +    description: |

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

> +      The Tegra GPIO controller's phandle and the number
> +      of the GPIO used to reset the external AC97 codec
> +    maxItems: 1
> +
> +  nvidia,codec-sync-gpio:
> +    description: |

Ditto

> +      The Tegra GPIO controller's phandle and the number
> +      of the GPIO corresponding with the AC97 DAP _FS line
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - resets
> +  - reset-names
> +  - interrupts
> +  - clocks
> +  - dmas
> +  - dma-names
> +  - nvidia,codec-reset-gpio
> +  - nvidia,codec-sync-gpio
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    ac97@...02000 {
> +            compatible = "nvidia,tegra20-ac97";
> +            reg = <0x70002000 0x200>;
> +            resets = <&tegra_car 3>;

All previous comments apply...

> +            reset-names = "ac97";
> +            interrupts = <0 81 0x04>;

Use proper defines.

> +            clocks = <&tegra_car 3>;
> +            dmas = <&apbdma 12>, <&apbdma 12>;
> +            dma-names = "rx", "tx";
> +            nvidia,codec-reset-gpio = <&gpio 170 0>;

Include gpio header and use proper defines.



Best regards,
Krzysztof