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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b3ba90ed-dbc1-413a-bef5-e49adb9153e0@kernel.org>
Date: Sat, 14 Dec 2024 11:24:50 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Vincenzo Frascino <vincenzo.frascino@....com>,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: Maruthi Srinivas Bayyavarapu <maruthi.srinivas.bayyavarapu@...inx.com>,
 Sudeep Holla <sudeep.holla@....com>, Rob Herring <robh@...nel.org>
Subject: Re: [PATCH 1/3] xlnx: dt-bindings: xlnx,i2s: Convert to json-schema

On 13/12/2024 17:52, Vincenzo Frascino wrote:
> Convert the Xilinx I2S device tree binding documentation to json-schema.
> 
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@....com>

<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument, so you will
not CC people just because they made one commit years ago). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.
</form letter>

> ---
>  .../devicetree/bindings/sound/xlnx,i2s.txt    | 28 -------
>  .../devicetree/bindings/sound/xlnx,i2s.yaml   | 79 +++++++++++++++++++
>  2 files changed, 79 insertions(+), 28 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/sound/xlnx,i2s.txt
>  create mode 100644 Documentation/devicetree/bindings/sound/xlnx,i2s.yaml


Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

> 
> diff --git a/Documentation/devicetree/bindings/sound/xlnx,i2s.txt b/Documentation/devicetree/bindings/sound/xlnx,i2s.txt
> deleted file mode 100644
> index 5e7c7d5bb60a..000000000000
> --- a/Documentation/devicetree/bindings/sound/xlnx,i2s.txt
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -Device-Tree bindings for Xilinx I2S PL block
> -
> -The IP supports I2S based playback/capture audio
> -
> -Required property:
> - - compatible: "xlnx,i2s-transmitter-1.0" for playback and
> -	       "xlnx,i2s-receiver-1.0" for capture
> -
> -Required property common to both I2S playback and capture:
> - - reg: Base address and size of the IP core instance.
> - - xlnx,dwidth: sample data width. Can be any of 16, 24.
> - - xlnx,num-channels: Number of I2S streams. Can be any of 1, 2, 3, 4.
> -		      supported channels = 2 * xlnx,num-channels
> -
> -Example:
> -
> -	i2s_receiver@...80000 {
> -		compatible = "xlnx,i2s-receiver-1.0";
> -		reg = <0x0 0xa0080000 0x0 0x10000>;
> -		xlnx,dwidth = <0x18>;
> -		xlnx,num-channels = <1>;
> -	};
> -	i2s_transmitter@...90000 {
> -		compatible = "xlnx,i2s-transmitter-1.0";
> -		reg = <0x0 0xa0090000 0x0 0x10000>;
> -		xlnx,dwidth = <0x18>;
> -		xlnx,num-channels = <1>;
> -	};
> diff --git a/Documentation/devicetree/bindings/sound/xlnx,i2s.yaml b/Documentation/devicetree/bindings/sound/xlnx,i2s.yaml
> new file mode 100644
> index 000000000000..b8bda7c28d37
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/xlnx,i2s.yaml
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/xlnx,i2s.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Device-Tree bindings for Xilinx I2S PL block

There is never "Device-tree bindings for" in the title. Please start
bindings from some existing sources or example-schema.

It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint. Don't rely on
distro packages for dtschema and be sure you are using the latest
released dtschema.


> +
> +description: |
> +  The IP supports I2S based playback/capture audio.
> +
> +maintainers:
> +  - Vincenzo Frascino <vincenzo.frascino@....com>
> +  - Maruthi Srinivas Bayyavarapu <maruthi.srinivas.bayyavarapu@...inx.com>
> +
> +allOf:
> +  - $ref: dai-common.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: xlnx,i2s-receiver-1.0
> +      - const: xlnx,i2s-transmitter-1.0
> +
> +  reg:
> +    maxItems: 1
> +    description: |
> +      Base address and size of the IP core instance.
> +
> +  "#sound-dai-cells":
> +    const: 0
> +
> +  "#address-cells":
> +    maxItems: 1

no, drop or explain. There was nothing like that in old bindingg.

> +
> +  "#size-cells":
> +    maxItems: 1

Drop


> +
> +  xlnx,dwidth:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    maxItems: 1

Drop


> +    oneOf:
> +      - const: 16
> +      - const: 24

That's just enum.

> +    description: |
> +      Sample data width. Can be any of 16, 24.
> +
> +  xlnx,num-channels:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    maxItems: 1

Drop

> +    description: |

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

> +      Number of I2S streams.
> +    minimum: 1
> +    maximum: 4
> +
> +required:
> +  - compatible
> +  - reg
> +  - xlnx,dwidth
> +  - xlnx,num-channels
> +
> +additionalProperties: false

unevaluatedProperties instead

> +
> +examples:
> +  - |
> +    i2s_receiver@...80000 {

i2s@

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


> +      compatible = "xlnx,i2s-receiver-1.0";
> +      reg = <0x0 0xa0080000 0x0 0x10000>;
> +      xlnx,dwidth = <0x18>;
> +      xlnx,num-channels = <1>;
> +    };
> +    i2s_transmitter@...90000 {
> +      compatible = "xlnx,i2s-transmitter-1.0";

Drop the node, one example is enough since they do not differ.


Best regards,
Krzysztof

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ