| Message ID | 20190906184551.17858-4-clabbe.montjoie@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | crypto: add sun8i-ce driver for Allwinner crypto engine | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
On Fri, Sep 06, 2019 at 08:45:45PM +0200, Corentin Labbe wrote: > This patch adds documentation for Device-Tree bindings for the > Crypto Engine cryptographic accelerator driver. > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > --- > .../bindings/crypto/allwinner,sun8i-ce.yaml | 84 +++++++++++++++++++ > 1 file changed, 84 insertions(+) > create mode 100644 Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml > > diff --git a/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml b/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml > new file mode 100644 > index 000000000000..bd8ccedd6059 > --- /dev/null > +++ b/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml So, usually we're using the first compatible supported here as the name. > @@ -0,0 +1,84 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/crypto/allwinner,sun8i-ce.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Allwinner Crypto Engine driver > + > +maintainers: > + - Corentin Labbe <clabbe@baylibre.com> > + > +properties: > + compatible: > + oneOf: > + - const: allwinner,sun8i-h3-crypto > + - const: allwinner,sun8i-r40-crypto > + - const: allwinner,sun50i-a64-crypto > + - const: allwinner,sun50i-h5-crypto > + - const: allwinner,sun50i-h6-crypto An enum would be better here, it provides a more obvious error message. > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > +if: > + properties: > + compatible: > + contains: > + const: allwinner,sun50i-h6-crypto > +then: > + clocks: > + items: > + - description: Bus clock > + - description: Module clock > + - description: MBus clock > + > + clock-names: > + items: > + - const: ahb > + - const: mod > + - const: mbus It looks like there's a reset line on the H6 as well for that controller (register 0x68c of the CCU, "CE_BGR_REG"). > +else: > + clocks: > + items: > + - description: Bus clock > + - description: Module clock > + > + clock-names: > + items: > + - const: ahb > + - const: mod > + > + resets: > + maxItems: 1 > + > + reset-names: > + const: ahb This prevents the usage of the additionalProperties property, which you should really use. What you can do instead is moving the clocks and clock-names description under properties, with a minItems of 2 and a maxItems of 3. Then you can restrict the length of that property to either 2 or 3 depending on the case here. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Sat, Sep 07, 2019 at 07:01:16AM +0300, Maxime Ripard wrote: > On Fri, Sep 06, 2019 at 08:45:45PM +0200, Corentin Labbe wrote: > > This patch adds documentation for Device-Tree bindings for the > > Crypto Engine cryptographic accelerator driver. > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > --- > > .../bindings/crypto/allwinner,sun8i-ce.yaml | 84 +++++++++++++++++++ > > 1 file changed, 84 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml > > > > diff --git a/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml b/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml [...] > > +else: > > + clocks: > > + items: > > + - description: Bus clock > > + - description: Module clock > > + > > + clock-names: > > + items: > > + - const: ahb > > + - const: mod > > + > > + resets: > > + maxItems: 1 > > + > > + reset-names: > > + const: ahb > > This prevents the usage of the additionalProperties property, which > you should really use. > > What you can do instead is moving the clocks and clock-names > description under properties, with a minItems of 2 and a maxItems of > 3. Then you can restrict the length of that property to either 2 or 3 > depending on the case here. > Hello I fail to do this. I do the following (keeped only clock stuff) properties: clocks: items: - description: Bus clock - description: Module clock - description: MBus clock clock-names: items: - const: ahb - const: mod - const: mbus if: properties: compatible: items: const: allwinner,sun50i-h6-crypto then: properties: clocks: minItems: 3 maxItems: 3 clock-names: minItems: 3 maxItems: 3 else: properties: clocks: minItems: 2 maxItems: 2 clock-names: minItems: 2 maxItems: 2 With this, the dtb_check keep complain that a64 have two short clocks. Regards
Hi Corentin, On Wed, Sep 11, 2019 at 08:31:58PM +0200, Corentin Labbe wrote: > On Sat, Sep 07, 2019 at 07:01:16AM +0300, Maxime Ripard wrote: > > On Fri, Sep 06, 2019 at 08:45:45PM +0200, Corentin Labbe wrote: > > > This patch adds documentation for Device-Tree bindings for the > > > Crypto Engine cryptographic accelerator driver. > > > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > --- > > > .../bindings/crypto/allwinner,sun8i-ce.yaml | 84 +++++++++++++++++++ > > > 1 file changed, 84 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml b/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml > [...] > > > +else: > > > + clocks: > > > + items: > > > + - description: Bus clock > > > + - description: Module clock > > > + > > > + clock-names: > > > + items: > > > + - const: ahb > > > + - const: mod > > > + > > > + resets: > > > + maxItems: 1 > > > + > > > + reset-names: > > > + const: ahb > > > > This prevents the usage of the additionalProperties property, which > > you should really use. > > > > What you can do instead is moving the clocks and clock-names > > description under properties, with a minItems of 2 and a maxItems of > > 3. Then you can restrict the length of that property to either 2 or 3 > > depending on the case here. > > > > Hello > > I fail to do this. > I do the following (keeped only clock stuff) > properties: > > clocks: > items: > - description: Bus clock > - description: Module clock > - description: MBus clock Add minItems: 2 and maxItems: 3 at the same level than items > > clock-names: > items: > - const: ahb > - const: mod > - const: mbus And here as well Something I missed earlier though was that we've tried to unify as much as possible the ahb / apb / axi clocks around the bus name, it would be great if you could do it. > > if: > properties: > compatible: > items: > const: allwinner,sun50i-h6-crypto > then: > properties: > clocks: > minItems: 3 > maxItems: 3 > clock-names: > minItems: 3 > maxItems: 3 You don't need to duplicate the min and maxItems here Maxime
On Thu, Sep 12, 2019 at 10:37 AM Maxime Ripard <mripard@kernel.org> wrote: > > Hi Corentin, > > On Wed, Sep 11, 2019 at 08:31:58PM +0200, Corentin Labbe wrote: > > On Sat, Sep 07, 2019 at 07:01:16AM +0300, Maxime Ripard wrote: > > > On Fri, Sep 06, 2019 at 08:45:45PM +0200, Corentin Labbe wrote: > > > > This patch adds documentation for Device-Tree bindings for the > > > > Crypto Engine cryptographic accelerator driver. > > > > > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > > --- > > > > .../bindings/crypto/allwinner,sun8i-ce.yaml | 84 +++++++++++++++++++ > > > > 1 file changed, 84 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml b/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml > > [...] > > > > +else: > > > > + clocks: > > > > + items: > > > > + - description: Bus clock > > > > + - description: Module clock > > > > + > > > > + clock-names: > > > > + items: > > > > + - const: ahb > > > > + - const: mod > > > > + > > > > + resets: > > > > + maxItems: 1 > > > > + > > > > + reset-names: > > > > + const: ahb > > > > > > This prevents the usage of the additionalProperties property, which > > > you should really use. > > > > > > What you can do instead is moving the clocks and clock-names > > > description under properties, with a minItems of 2 and a maxItems of > > > 3. Then you can restrict the length of that property to either 2 or 3 > > > depending on the case here. > > > > > > > Hello > > > > I fail to do this. > > I do the following (keeped only clock stuff) > > properties: > > > > clocks: > > items: > > - description: Bus clock > > - description: Module clock > > - description: MBus clock > > Add minItems: 2 and maxItems: 3 at the same level than items > > > > > clock-names: > > items: > > - const: ahb > > - const: mod > > - const: mbus > > And here as well > > Something I missed earlier though was that we've tried to unify as > much as possible the ahb / apb / axi clocks around the bus name, it > would be great if you could do it. I think we also want to standardize "mbus" as "dram"? ChenYu > > > > if: > > properties: > > compatible: > > items: > > const: allwinner,sun50i-h6-crypto > > then: > > properties: > > clocks: > > minItems: 3 > > maxItems: 3 > > clock-names: > > minItems: 3 > > maxItems: 3 > > You don't need to duplicate the min and maxItems here > > Maxime
On Thu, Sep 12, 2019 at 09:26:27PM +0100, Chen-Yu Tsai wrote: > > > > > > clock-names: > > > items: > > > - const: ahb > > > - const: mod > > > - const: mbus > > > > And here as well > > > > Something I missed earlier though was that we've tried to unify as > > much as possible the ahb / apb / axi clocks around the bus name, it > > would be great if you could do it. > > I think we also want to standardize "mbus" as "dram"? Do we? The only user so far seems to be sun9i-de, while mbus has more users. I don't really care though, both mbus and dram are pretty generic to me. What makes you prefer dram over mbus? Maxime
On Thu, Sep 12, 2019 at 9:33 PM Maxime Ripard <mripard@kernel.org> wrote: > > On Thu, Sep 12, 2019 at 09:26:27PM +0100, Chen-Yu Tsai wrote: > > > > > > > > clock-names: > > > > items: > > > > - const: ahb > > > > - const: mod > > > > - const: mbus > > > > > > And here as well > > > > > > Something I missed earlier though was that we've tried to unify as > > > much as possible the ahb / apb / axi clocks around the bus name, it > > > would be great if you could do it. > > > > I think we also want to standardize "mbus" as "dram"? > > Do we? The only user so far seems to be sun9i-de, while mbus has more > users. I don't really care though, both mbus and dram are pretty > generic to me. What makes you prefer dram over mbus? Argh... it's actually "ram" we use the most. Both "dram" and "mbus" have only one instance each. ChenYu
On Thu, Sep 12, 2019 at 09:37:17PM +0100, Chen-Yu Tsai wrote: > On Thu, Sep 12, 2019 at 9:33 PM Maxime Ripard <mripard@kernel.org> wrote: > > On Thu, Sep 12, 2019 at 09:26:27PM +0100, Chen-Yu Tsai wrote: > > > > > > > > > > clock-names: > > > > > items: > > > > > - const: ahb > > > > > - const: mod > > > > > - const: mbus > > > > > > > > And here as well > > > > > > > > Something I missed earlier though was that we've tried to unify as > > > > much as possible the ahb / apb / axi clocks around the bus name, it > > > > would be great if you could do it. > > > > > > I think we also want to standardize "mbus" as "dram"? > > > > Do we? The only user so far seems to be sun9i-de, while mbus has more > > users. I don't really care though, both mbus and dram are pretty > > generic to me. What makes you prefer dram over mbus? > > Argh... it's actually "ram" we use the most. Both "dram" and "mbus" > have only one instance each. Let's use ram then :) Maxime
diff --git a/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml b/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml new file mode 100644 index 000000000000..bd8ccedd6059 --- /dev/null +++ b/Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml @@ -0,0 +1,84 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/crypto/allwinner,sun8i-ce.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Allwinner Crypto Engine driver + +maintainers: + - Corentin Labbe <clabbe@baylibre.com> + +properties: + compatible: + oneOf: + - const: allwinner,sun8i-h3-crypto + - const: allwinner,sun8i-r40-crypto + - const: allwinner,sun50i-a64-crypto + - const: allwinner,sun50i-h5-crypto + - const: allwinner,sun50i-h6-crypto + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + +if: + properties: + compatible: + contains: + const: allwinner,sun50i-h6-crypto +then: + clocks: + items: + - description: Bus clock + - description: Module clock + - description: MBus clock + + clock-names: + items: + - const: ahb + - const: mod + - const: mbus +else: + clocks: + items: + - description: Bus clock + - description: Module clock + + clock-names: + items: + - const: ahb + - const: mod + + resets: + maxItems: 1 + + reset-names: + const: ahb + + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/sun50i-a64-ccu.h> + #include <dt-bindings/reset/sun50i-a64-ccu.h> + + crypto: crypto@1c15000 { + compatible = "allwinner,sun8i-h3-crypto"; + reg = <0x01c15000 0x1000>; + interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&ccu CLK_BUS_CE>, <&ccu CLK_CE>; + clock-names = "ahb", "mod"; + resets = <&ccu RST_BUS_CE>; + reset-names = "ahb"; + }; +
This patch adds documentation for Device-Tree bindings for the Crypto Engine cryptographic accelerator driver. Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> --- .../bindings/crypto/allwinner,sun8i-ce.yaml | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.yaml