| Message ID | 20220321200739.3572792-22-clabbe@baylibre.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: rockchip: permit to pass self-tests | expand |
On Mon, 21 Mar 2022 20:07:34 +0000, Corentin Labbe wrote: > Convert rockchip-crypto to yaml > > Signed-off-by: Corentin Labbe <clabbe@baylibre.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 > 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
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 <clabbe@baylibre.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 > > > > 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
On 21/03/2022 21:07, Corentin Labbe wrote: > Convert rockchip-crypto to yaml > > Signed-off-by: Corentin Labbe <clabbe@baylibre.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@sntech.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
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 <clabbe@baylibre.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 >>> >> >> 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
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 <clabbe@baylibre.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@sntech.de> > > + > > +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
On 24/03/2022 17:20, LABBE Corentin wrote: (...) >> >>> + 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 ? For such case one way to solve is to: 1. Define the most relaxed min/maxItems in properties. 2. Narrow the min/maxItems in allOf for each flavor. Something like in clocks for: Documentation/devicetree/bindings/clock/samsung,exynos7885-clock.yaml Best regards, Krzysztof
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@sntech.de> + +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"; - };
Convert rockchip-crypto to yaml Signed-off-by: Corentin Labbe <clabbe@baylibre.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