2022-03-21 22:20:52

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH v3 21/26] dt-bindings: crypto: convert rockchip-crypto to yaml

Convert rockchip-crypto to yaml

Signed-off-by: Corentin Labbe <[email protected]>
---
.../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 <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - rockchip,rk3288-crypto
+ - rockchip,rk3328-crypto
+ - rockchip,rk3399-crypto
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ minItems: 4
+
+ clock-names:
+ minItems: 4
+
+ resets:
+ maxItems: 1
+
+if:
+ properties:
+ compatible:
+ const: rockchip,rk3399-crypto
+then:
+ properties:
+ reg:
+ minItems: 2
+ interrupts:
+ minItems: 2
+ clocks:
+ minItems: 6
+ clock-names:
+ minItems: 6
+ resets:
+ minItems: 6
+else:
+ if:
+ properties:
+ compatible:
+ const: rockchip,rk3328-crypto
+ then:
+ properties:
+ clocks:
+ minItems: 3
+ clock-names:
+ minItems: 3
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - resets
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/rk3288-cru.h>
+ crypto@ff8a0000 {
+ compatible = "rockchip,rk3288-crypto";
+ reg = <0xff8a0000 0x4000>;
+ interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru ACLK_CRYPTO>, <&cru HCLK_CRYPTO>,
+ <&cru SCLK_CRYPTO>, <&cru ACLK_DMAC1>;
+ clock-names = "aclk", "hclk", "sclk", "apb_pclk";
+ resets = <&cru SRST_CRYPTO>;
+ };
diff --git a/Documentation/devicetree/bindings/crypto/rockchip-crypto.txt b/Documentation/devicetree/bindings/crypto/rockchip-crypto.txt
deleted file mode 100644
index 5e2ba385b8c9..000000000000
--- a/Documentation/devicetree/bindings/crypto/rockchip-crypto.txt
+++ /dev/null
@@ -1,28 +0,0 @@
-Rockchip Electronics And Security Accelerator
-
-Required properties:
-- compatible: Should be "rockchip,rk3288-crypto"
-- reg: Base physical address of the engine and length of memory mapped
- region
-- interrupts: Interrupt number
-- clocks: Reference to the clocks about crypto
-- clock-names: "aclk" used to clock data
- "hclk" used to clock data
- "sclk" used to clock crypto accelerator
- "apb_pclk" used to clock dma
-- resets: Must contain an entry for each entry in reset-names.
- See ../reset/reset.txt for details.
-- reset-names: Must include the name "crypto-rst".
-
-Examples:
-
- crypto: cypto-controller@ff8a0000 {
- compatible = "rockchip,rk3288-crypto";
- reg = <0xff8a0000 0x4000>;
- interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&cru ACLK_CRYPTO>, <&cru HCLK_CRYPTO>,
- <&cru SCLK_CRYPTO>, <&cru ACLK_DMAC1>;
- clock-names = "aclk", "hclk", "sclk", "apb_pclk";
- resets = <&cru SRST_CRYPTO>;
- reset-names = "crypto-rst";
- };
--
2.34.1


2022-03-22 02:18:03

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 21/26] dt-bindings: crypto: convert rockchip-crypto to yaml

On Mon, 21 Mar 2022 20:07:34 +0000, Corentin Labbe wrote:
> Convert rockchip-crypto to yaml
>
> Signed-off-by: Corentin Labbe <[email protected]>
> ---
> .../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
>

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/1607887


cypto-controller@ff8a0000: 'reset-names' does not match any of the regexes: 'pinctrl-[0-9]+'
arch/arm/boot/dts/rk3288-evb-act8846.dt.yaml
arch/arm/boot/dts/rk3288-evb-rk808.dt.yaml
arch/arm/boot/dts/rk3288-firefly-beta.dt.yaml
arch/arm/boot/dts/rk3288-firefly.dt.yaml
arch/arm/boot/dts/rk3288-firefly-reload.dt.yaml
arch/arm/boot/dts/rk3288-miqi.dt.yaml
arch/arm/boot/dts/rk3288-phycore-rdk.dt.yaml
arch/arm/boot/dts/rk3288-popmetal.dt.yaml
arch/arm/boot/dts/rk3288-r89.dt.yaml
arch/arm/boot/dts/rk3288-rock2-square.dt.yaml
arch/arm/boot/dts/rk3288-rock-pi-n8.dt.yaml
arch/arm/boot/dts/rk3288-tinker.dt.yaml
arch/arm/boot/dts/rk3288-tinker-s.dt.yaml
arch/arm/boot/dts/rk3288-veyron-brain.dt.yaml
arch/arm/boot/dts/rk3288-veyron-fievel.dt.yaml
arch/arm/boot/dts/rk3288-veyron-jaq.dt.yaml
arch/arm/boot/dts/rk3288-veyron-jerry.dt.yaml
arch/arm/boot/dts/rk3288-veyron-mickey.dt.yaml
arch/arm/boot/dts/rk3288-veyron-mighty.dt.yaml
arch/arm/boot/dts/rk3288-veyron-minnie.dt.yaml
arch/arm/boot/dts/rk3288-veyron-pinky.dt.yaml
arch/arm/boot/dts/rk3288-veyron-speedy.dt.yaml
arch/arm/boot/dts/rk3288-veyron-tiger.dt.yaml
arch/arm/boot/dts/rk3288-vyasa.dt.yaml

2022-03-22 09:47:16

by Corentin LABBE

[permalink] [raw]
Subject: Re: [PATCH v3 21/26] dt-bindings: crypto: convert rockchip-crypto to yaml

Le Mon, Mar 21, 2022 at 08:50:51PM -0500, Rob Herring a ?crit :
> On Mon, 21 Mar 2022 20:07:34 +0000, Corentin Labbe wrote:
> > Convert rockchip-crypto to yaml
> >
> > Signed-off-by: Corentin Labbe <[email protected]>
> > ---
> > .../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
> >
>
> Running 'make dtbs_check' with the schema in this patch gives the
> following warnings. Consider if they are expected or the schema is
> incorrect. These may not be new warnings.
>
> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> This will change in the future.
>
> Full log is available here: https://patchwork.ozlabs.org/patch/1607887
>
>
> cypto-controller@ff8a0000: 'reset-names' does not match any of the regexes: 'pinctrl-[0-9]+'
> arch/arm/boot/dts/rk3288-evb-act8846.dt.yaml
> arch/arm/boot/dts/rk3288-evb-rk808.dt.yaml
> arch/arm/boot/dts/rk3288-firefly-beta.dt.yaml
> arch/arm/boot/dts/rk3288-firefly.dt.yaml
> arch/arm/boot/dts/rk3288-firefly-reload.dt.yaml
> arch/arm/boot/dts/rk3288-miqi.dt.yaml
> arch/arm/boot/dts/rk3288-phycore-rdk.dt.yaml
> arch/arm/boot/dts/rk3288-popmetal.dt.yaml
> arch/arm/boot/dts/rk3288-r89.dt.yaml
> arch/arm/boot/dts/rk3288-rock2-square.dt.yaml
> arch/arm/boot/dts/rk3288-rock-pi-n8.dt.yaml
> arch/arm/boot/dts/rk3288-tinker.dt.yaml
> arch/arm/boot/dts/rk3288-tinker-s.dt.yaml
> arch/arm/boot/dts/rk3288-veyron-brain.dt.yaml
> arch/arm/boot/dts/rk3288-veyron-fievel.dt.yaml
> arch/arm/boot/dts/rk3288-veyron-jaq.dt.yaml
> arch/arm/boot/dts/rk3288-veyron-jerry.dt.yaml
> arch/arm/boot/dts/rk3288-veyron-mickey.dt.yaml
> arch/arm/boot/dts/rk3288-veyron-mighty.dt.yaml
> arch/arm/boot/dts/rk3288-veyron-minnie.dt.yaml
> arch/arm/boot/dts/rk3288-veyron-pinky.dt.yaml
> arch/arm/boot/dts/rk3288-veyron-speedy.dt.yaml
> arch/arm/boot/dts/rk3288-veyron-tiger.dt.yaml
> arch/arm/boot/dts/rk3288-vyasa.dt.yaml
>

Hello

This should not happen since patch 20 remove it.

Regards

2022-03-22 18:39:31

by Krzysztof Kozlowski

[permalink] [raw]
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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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

2022-03-22 23:28:27

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 21/26] dt-bindings: crypto: convert rockchip-crypto to yaml

On 22/03/2022 10:12, LABBE Corentin wrote:
> Le Mon, Mar 21, 2022 at 08:50:51PM -0500, Rob Herring a écrit :
>> On Mon, 21 Mar 2022 20:07:34 +0000, Corentin Labbe wrote:
>>> Convert rockchip-crypto to yaml
>>>
>>> Signed-off-by: Corentin Labbe <[email protected]>
>>> ---
>>> .../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
>>>
>>
>> Running 'make dtbs_check' with the schema in this patch gives the
>> following warnings. Consider if they are expected or the schema is
>> incorrect. These may not be new warnings.
>>
>> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
>> This will change in the future.
>>
>> Full log is available here: https://patchwork.ozlabs.org/patch/1607887
>>
>>
>> cypto-controller@ff8a0000: 'reset-names' does not match any of the regexes: 'pinctrl-[0-9]+'
>> arch/arm/boot/dts/rk3288-evb-act8846.dt.yaml
>> arch/arm/boot/dts/rk3288-evb-rk808.dt.yaml
>> arch/arm/boot/dts/rk3288-firefly-beta.dt.yaml
>> arch/arm/boot/dts/rk3288-firefly.dt.yaml
>> arch/arm/boot/dts/rk3288-firefly-reload.dt.yaml
>> arch/arm/boot/dts/rk3288-miqi.dt.yaml
>> arch/arm/boot/dts/rk3288-phycore-rdk.dt.yaml
>> arch/arm/boot/dts/rk3288-popmetal.dt.yaml
>> arch/arm/boot/dts/rk3288-r89.dt.yaml
>> arch/arm/boot/dts/rk3288-rock2-square.dt.yaml
>> arch/arm/boot/dts/rk3288-rock-pi-n8.dt.yaml
>> arch/arm/boot/dts/rk3288-tinker.dt.yaml
>> arch/arm/boot/dts/rk3288-tinker-s.dt.yaml
>> arch/arm/boot/dts/rk3288-veyron-brain.dt.yaml
>> arch/arm/boot/dts/rk3288-veyron-fievel.dt.yaml
>> arch/arm/boot/dts/rk3288-veyron-jaq.dt.yaml
>> arch/arm/boot/dts/rk3288-veyron-jerry.dt.yaml
>> arch/arm/boot/dts/rk3288-veyron-mickey.dt.yaml
>> arch/arm/boot/dts/rk3288-veyron-mighty.dt.yaml
>> arch/arm/boot/dts/rk3288-veyron-minnie.dt.yaml
>> arch/arm/boot/dts/rk3288-veyron-pinky.dt.yaml
>> arch/arm/boot/dts/rk3288-veyron-speedy.dt.yaml
>> arch/arm/boot/dts/rk3288-veyron-tiger.dt.yaml
>> arch/arm/boot/dts/rk3288-vyasa.dt.yaml
>>
>
> Hello
>
> This should not happen since patch 20 remove it.


From all DTBS in the world? If you want to deprecate a property, add a
"deprecated: true".


Best regards,
Krzysztof

2022-03-24 17:50:05

by Corentin LABBE

[permalink] [raw]
Subject: Re: [PATCH v3 21/26] dt-bindings: crypto: convert rockchip-crypto to yaml

Le Tue, Mar 22, 2022 at 07:04:43PM +0100, Krzysztof Kozlowski a ?crit :
> On 21/03/2022 21:07, Corentin Labbe wrote:
> > Convert rockchip-crypto to yaml
> >
> > Signed-off-by: Corentin Labbe <[email protected]>
> > ---
> > .../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 <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - rockchip,rk3288-crypto
> > + - rockchip,rk3328-crypto
> > + - rockchip,rk3399-crypto
>
> Waaaait, what? Only rockchip,rk3288-crypto is in original bindings.

Hello

Yes, my way is an error.
Next time, I will split my patch in 2, first a 1 to 1 conversion, then a binding update.

>
> > +
> > + 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
> > +
>

I have create a binding update patch (https://github.com/montjoie/linux/commit/da05ef9bb488c16cfd15a47054f5b1161829b6bf)
But I have lot of problem, DT are not validating.
Example: Documentation/devicetree/bindings/crypto/rockchip,rk3288-crypto.example.dtb: crypto@ff8a0000: resets: [[4294967295, 174]] is too short

I have tried also to set default resets/maxItems to 3 and setting it to 4 via an if. But I still got error like maxItems cannot be update after initial set.

Any idea on why my new binding update patch is failling ?

Regards