linux-kernel - Re: [PATCH v3 21/26] dt-bindings: crypto: convert rockchip-crypto to yaml

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, 22 Mar 2022 19:04:43 +0100
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Corentin Labbe <clabbe@...libre.com>, heiko@...ech.de,
        herbert@...dor.apana.org.au, krzk+dt@...nel.org,
        mturquette@...libre.com, robh+dt@...nel.org, sboyd@...nel.org
Cc:     devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-clk@...r.kernel.org, linux-crypto@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH v3 21/26] dt-bindings: crypto: convert rockchip-crypto to
 yaml

On 21/03/2022 21:07, Corentin Labbe wrote:
> Convert rockchip-crypto to yaml
> 
> Signed-off-by: Corentin Labbe <clabbe@...libre.com>
> ---
>  .../crypto/rockchip,rk3288-crypto.yaml        | 84 +++++++++++++++++++
>  .../bindings/crypto/rockchip-crypto.txt       | 28 -------
>  2 files changed, 84 insertions(+), 28 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/crypto/rockchip,rk3288-crypto.yaml
>  delete mode 100644 Documentation/devicetree/bindings/crypto/rockchip-crypto.txt
> 
> diff --git a/Documentation/devicetree/bindings/crypto/rockchip,rk3288-crypto.yaml b/Documentation/devicetree/bindings/crypto/rockchip,rk3288-crypto.yaml
> new file mode 100644
> index 000000000000..a6be89a1c890
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/rockchip,rk3288-crypto.yaml
> @@ -0,0 +1,84 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/crypto/rockchip,rk3288-crypto.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip Electronics And Security Accelerator
> +
> +maintainers:
> +  - Heiko Stuebner <heiko@...ech.de>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - rockchip,rk3288-crypto
> +      - rockchip,rk3328-crypto
> +      - rockchip,rk3399-crypto

Waaaait, what? Only rockchip,rk3288-crypto is in original bindings.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 4
> +
> +  clock-names:
> +    minItems: 4
> +
> +  resets:
> +    maxItems: 1

You missed reset-names.

This patch is quite different than previous, in unexpected way. What
happened here?

> +
> +if:

Please define it after "allOf:", so it could be easily extended without
changing indentation.

> +  properties:
> +    compatible:
> +      const: rockchip,rk3399-crypto
> +then:
> +  properties:
> +    reg:
> +      minItems: 2
> +    interrupts:
> +      minItems: 2

List interrupts. This is really different than your v1. It also looks
different than original bindings and you did not mention any differences
here, nor in the commit msg. Either explain in commit msg all
differences (and why) or move them to separate commit.

You seem to change the bindings a lot (new properties, different
constraints, new compatibles), so this should all go to separate commit.
Now it is just confusing.

> +    clocks:
> +      minItems: 6

You need maxItems. Everywhere.

> +    clock-names:
> +      minItems: 6

List all items.

> +    resets:
> +      minItems: 6
> +else:
> +  if:
> +    properties:
> +      compatible:
> +        const: rockchip,rk3328-crypto
> +  then:
> +    properties:
> +      clocks:
> +        minItems: 3
> +      clock-names:
> +        minItems: 3
> +

Best regards,
Krzysztof