[PATCH 0/3] dt-bindings: crypto: Convert atmel-{aes,tdes,sha} to YAML

[PATCH 0/3] dt-bindings: crypto: Convert atmel-{aes,tdes,sha} to YAML

[Tudor Ambarus]

Convert Atmel AES, TDES and SHA documentation to yaml format. There is one
binding defined per file. Keeping all bindings under the same yaml does
not make sense, as these are individual IPs. With the conversion the clock
and clock-names properties are made mandatory, to reflect how the drivers
treat them: when these properties are not provided, the drivers return
error.

Tudor Ambarus (3):
  dt-bindings: crypto: Convert Atmel AES to yaml
  dt-bindings: crypto: Convert Atmel TDES to yaml
  dt-bindings: crypto: Convert Atmel SHA to yaml

 .../devicetree/bindings/crypto/atmel,aes.yaml | 65 ++++++++++++++++++
 .../devicetree/bindings/crypto/atmel,sha.yaml | 59 ++++++++++++++++
 .../bindings/crypto/atmel,tdes.yaml           | 63 +++++++++++++++++
 .../bindings/crypto/atmel-crypto.txt          | 68 -------------------
 4 files changed, 187 insertions(+), 68 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/crypto/atmel,aes.yaml
 create mode 100644 Documentation/devicetree/bindings/crypto/atmel,sha.yaml
 create mode 100644 Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
 delete mode 100644 Documentation/devicetree/bindings/crypto/atmel-crypto.txt

-- 
2.25.1

[PATCH 1/3] dt-bindings: crypto: Convert Atmel AES to yaml

[Tudor Ambarus]

Convert Atmel AES documentation to yaml format. With the conversion the
clock and clock-names properties are made mandatory. The driver returns
-EINVAL if "aes_clk" is not found, reflect that in the bindings and make
the clock and clock-names properties mandatory. Update the example to
better describe how one should define the dt node.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 .../devicetree/bindings/crypto/atmel,aes.yaml | 65 +++++++++++++++++++
 .../bindings/crypto/atmel-crypto.txt          | 20 ------
 2 files changed, 65 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/crypto/atmel,aes.yaml

diff --git a/Documentation/devicetree/bindings/crypto/atmel,aes.yaml b/Documentation/devicetree/bindings/crypto/atmel,aes.yaml
new file mode 100644
index 000000000000..f77ec04dbabe
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/atmel,aes.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/crypto/atmel,aes.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel Advanced Encryption Standard (AES) HW cryptographic accelerator
+
+maintainers:
+  - Tudor Ambarus <tudor.ambarus@microchip.com>
+
+properties:
+  compatible:
+    const: atmel,at91sam9g46-aes
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: aes_clk
+
+  dmas:
+    items:
+      - description: TX DMA Channel
+      - description: RX DMA Channel
+
+  dma-names:
+    items:
+      - const: tx
+      - const: rx
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - dmas
+  - dma-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/at91.h>
+    #include <dt-bindings/dma/at91.h>
+    aes: aes@f8038000 {
+      compatible = "atmel,at91sam9g46-aes";
+      reg = <0xe1810000 0x100>;
+      interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&pmc PMC_TYPE_PERIPHERAL 27>;
+      clock-names = "aes_clk";
+      dmas = <&dma0 AT91_XDMAC_DT_PERID(1)>,
+             <&dma0 AT91_XDMAC_DT_PERID(2)>;
+      dma-names = "tx", "rx";
+      status= "okay";
+    };
diff --git a/Documentation/devicetree/bindings/crypto/atmel-crypto.txt b/Documentation/devicetree/bindings/crypto/atmel-crypto.txt
index f2aab3dc2b52..1353ebd0dcaa 100644
--- a/Documentation/devicetree/bindings/crypto/atmel-crypto.txt
+++ b/Documentation/devicetree/bindings/crypto/atmel-crypto.txt
@@ -2,26 +2,6 @@
 
 These are the HW cryptographic accelerators found on some Atmel products.
 
-* Advanced Encryption Standard (AES)
-
-Required properties:
-- compatible : Should be "atmel,at91sam9g46-aes".
-- reg: Should contain AES registers location and length.
-- interrupts: Should contain the IRQ line for the AES.
-- dmas: List of two DMA specifiers as described in
-        atmel-dma.txt and dma.txt files.
-- dma-names: Contains one identifier string for each DMA specifier
-             in the dmas property.
-
-Example:
-aes@f8038000 {
-	compatible = "atmel,at91sam9g46-aes";
-	reg = <0xf8038000 0x100>;
-	interrupts = <43 4 0>;
-	dmas = <&dma1 2 18>,
-	       <&dma1 2 19>;
-	dma-names = "tx", "rx";
-
 * Triple Data Encryption Standard (Triple DES)
 
 Required properties:
-- 
2.25.1

[PATCH 2/3] dt-bindings: crypto: Convert Atmel TDES to yaml

[Tudor Ambarus]

Convert Atmel TDES documentation to yaml format. With the conversion the
clock and clock-names properties are made mandatory. The driver returns
-EINVAL if "tdes_clk" is not found, reflect that in the bindings and make
the clock and clock-names properties mandatory. Update the example to
better describe how one should define the dt node.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 .../bindings/crypto/atmel,tdes.yaml           | 63 +++++++++++++++++++
 .../bindings/crypto/atmel-crypto.txt          | 23 -------
 2 files changed, 63 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/crypto/atmel,tdes.yaml

diff --git a/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml b/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
new file mode 100644
index 000000000000..7efa5e4acaa1
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/crypto/atmel,tdes.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel Triple Data Encryption Standard (TDES) HW cryptographic accelerator
+
+maintainers:
+  - Tudor Ambarus <tudor.ambarus@microchip.com>
+
+properties:
+  compatible:
+    const: atmel,at91sam9g46-tdes
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: tdes_clk
+
+  dmas:
+    items:
+      - description: TX DMA Channel
+      - description: RX DMA Channel
+
+  dma-names:
+    items:
+      - const: tx
+      - const: rx
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/at91.h>
+    #include <dt-bindings/dma/at91.h>
+    tdes: tdes@e2014000 {
+      compatible = "atmel,at91sam9g46-tdes";
+      reg = <0xe2014000 0x100>;
+      interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&pmc PMC_TYPE_PERIPHERAL 96>;
+      clock-names = "tdes_clk";
+      dmas = <&dma0 AT91_XDMAC_DT_PERID(54)>,
+             <&dma0 AT91_XDMAC_DT_PERID(53)>;
+      dma-names = "tx", "rx";
+      status = "okay";
+    };
diff --git a/Documentation/devicetree/bindings/crypto/atmel-crypto.txt b/Documentation/devicetree/bindings/crypto/atmel-crypto.txt
index 1353ebd0dcaa..5c6541cfcc4a 100644
--- a/Documentation/devicetree/bindings/crypto/atmel-crypto.txt
+++ b/Documentation/devicetree/bindings/crypto/atmel-crypto.txt
@@ -2,29 +2,6 @@
 
 These are the HW cryptographic accelerators found on some Atmel products.
 
-* Triple Data Encryption Standard (Triple DES)
-
-Required properties:
-- compatible : Should be "atmel,at91sam9g46-tdes".
-- reg: Should contain TDES registers location and length.
-- interrupts: Should contain the IRQ line for the TDES.
-
-Optional properties:
-- dmas: List of two DMA specifiers as described in
-        atmel-dma.txt and dma.txt files.
-- dma-names: Contains one identifier string for each DMA specifier
-             in the dmas property.
-
-Example:
-tdes@f803c000 {
-	compatible = "atmel,at91sam9g46-tdes";
-	reg = <0xf803c000 0x100>;
-	interrupts = <44 4 0>;
-	dmas = <&dma1 2 20>,
-	       <&dma1 2 21>;
-	dma-names = "tx", "rx";
-};
-
 * Secure Hash Algorithm (SHA)
 
 Required properties:
-- 
2.25.1

[PATCH 3/3] dt-bindings: crypto: Convert Atmel SHA to yaml

[Tudor Ambarus]

Convert Atmel SHA documentation to yaml format. With the conversion the
clock and clock-names properties are made mandatory. The driver returns
-EINVAL if "sha_clk" is not found, reflect that in the bindings and make
the clock and clock-names properties mandatory. Update the example to
better describe how one should define the dt node.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 .../devicetree/bindings/crypto/atmel,sha.yaml | 59 +++++++++++++++++++
 .../bindings/crypto/atmel-crypto.txt          | 25 --------
 2 files changed, 59 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/crypto/atmel,sha.yaml
 delete mode 100644 Documentation/devicetree/bindings/crypto/atmel-crypto.txt

diff --git a/Documentation/devicetree/bindings/crypto/atmel,sha.yaml b/Documentation/devicetree/bindings/crypto/atmel,sha.yaml
new file mode 100644
index 000000000000..ccba6d36ee68
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/atmel,sha.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/crypto/atmel,sha.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel Secure Hash Algorithm (SHA) HW cryptographic accelerator
+
+maintainers:
+  - Tudor Ambarus <tudor.ambarus@microchip.com>
+
+properties:
+  compatible:
+    const: atmel,at91sam9g46-sha
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: sha_clk
+
+  dmas:
+    maxItems: 1
+    description: TX DMA Channel
+
+  dma-names:
+    const: tx
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/at91.h>
+    #include <dt-bindings/dma/at91.h>
+    sha: sha@e1814000 {
+      compatible = "atmel,at91sam9g46-sha";
+      reg = <0xe1814000 0x100>;
+      interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&pmc PMC_TYPE_PERIPHERAL 83>;
+      clock-names = "sha_clk";
+      dmas = <&dma0 AT91_XDMAC_DT_PERID(48)>;
+      dma-names = "tx";
+      status = "okay";
+    };
diff --git a/Documentation/devicetree/bindings/crypto/atmel-crypto.txt b/Documentation/devicetree/bindings/crypto/atmel-crypto.txt
deleted file mode 100644
index 5c6541cfcc4a..000000000000
--- a/Documentation/devicetree/bindings/crypto/atmel-crypto.txt
+++ /dev/null
@@ -1,25 +0,0 @@
-* Atmel HW cryptographic accelerators
-
-These are the HW cryptographic accelerators found on some Atmel products.
-
-* Secure Hash Algorithm (SHA)
-
-Required properties:
-- compatible : Should be "atmel,at91sam9g46-sha".
-- reg: Should contain SHA registers location and length.
-- interrupts: Should contain the IRQ line for the SHA.
-
-Optional properties:
-- dmas: One DMA specifiers as described in
-        atmel-dma.txt and dma.txt files.
-- dma-names: Contains one identifier string for each DMA specifier
-             in the dmas property. Only one "tx" string needed.
-
-Example:
-sha@f8034000 {
-	compatible = "atmel,at91sam9g46-sha";
-	reg = <0xf8034000 0x100>;
-	interrupts = <42 4 0>;
-	dmas = <&dma1 2 17>;
-	dma-names = "tx";
-};
-- 
2.25.1

Re: [PATCH 1/3] dt-bindings: crypto: Convert Atmel AES to yaml

[Krzysztof Kozlowski]

On 07/02/2022 04:24, Tudor Ambarus wrote:
> Convert Atmel AES documentation to yaml format. With the conversion the
> clock and clock-names properties are made mandatory. The driver returns
> -EINVAL if "aes_clk" is not found, reflect that in the bindings and make
> the clock and clock-names properties mandatory. Update the example to
> better describe how one should define the dt node.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  .../devicetree/bindings/crypto/atmel,aes.yaml | 65 +++++++++++++++++++
>  .../bindings/crypto/atmel-crypto.txt          | 20 ------
>  2 files changed, 65 insertions(+), 20 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/crypto/atmel,aes.yaml
> 
> diff --git a/Documentation/devicetree/bindings/crypto/atmel,aes.yaml b/Documentation/devicetree/bindings/crypto/atmel,aes.yaml
> new file mode 100644
> index 000000000000..f77ec04dbabe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/atmel,aes.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/crypto/atmel,aes.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel Advanced Encryption Standard (AES) HW cryptographic accelerator
> +
> +maintainers:
> +  - Tudor Ambarus <tudor.ambarus@microchip.com>
> +
> +properties:
> +  compatible:
> +    const: atmel,at91sam9g46-aes
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: aes_clk
> +
> +  dmas:
> +    items:
> +      - description: TX DMA Channel
> +      - description: RX DMA Channel
> +
> +  dma-names:
> +    items:
> +      - const: tx
> +      - const: rx
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - dmas
> +  - dma-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/at91.h>
> +    #include <dt-bindings/dma/at91.h>

One empty line for readability.

> +    aes: aes@f8038000 {

Generic node name, so "crypto".

> +      compatible = "atmel,at91sam9g46-aes";
> +      reg = <0xe1810000 0x100>;
> +      interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
> +      clocks = <&pmc PMC_TYPE_PERIPHERAL 27>;
> +      clock-names = "aes_clk";
> +      dmas = <&dma0 AT91_XDMAC_DT_PERID(1)>,
> +             <&dma0 AT91_XDMAC_DT_PERID(2)>;
> +      dma-names = "tx", "rx";
> +      status= "okay";
> +    };

Drop the status property.

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: crypto: Convert Atmel TDES to yaml

[Krzysztof Kozlowski]

On 07/02/2022 04:24, Tudor Ambarus wrote:
> Convert Atmel TDES documentation to yaml format. With the conversion the
> clock and clock-names properties are made mandatory. The driver returns
> -EINVAL if "tdes_clk" is not found, reflect that in the bindings and make
> the clock and clock-names properties mandatory. Update the example to
> better describe how one should define the dt node.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  .../bindings/crypto/atmel,tdes.yaml           | 63 +++++++++++++++++++
>  .../bindings/crypto/atmel-crypto.txt          | 23 -------
>  2 files changed, 63 insertions(+), 23 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
> 
> diff --git a/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml b/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
> new file mode 100644
> index 000000000000..7efa5e4acaa1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/crypto/atmel,tdes.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel Triple Data Encryption Standard (TDES) HW cryptographic accelerator
> +
> +maintainers:
> +  - Tudor Ambarus <tudor.ambarus@microchip.com>
> +
> +properties:
> +  compatible:
> +    const: atmel,at91sam9g46-tdes
> +

Same comments as for patch 1 plus one new (also applying to previous
one). You named the file quite generic "atmel,tdes" or "atmel,aes", but
what if something newer comes for at91? Maybe name it instead
"atmel,at91sam9-aes"?


Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: crypto: Convert Atmel TDES to yaml

[Tudor.Ambarus]

Hi, Krzysztof,

On 2/7/22 18:04, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 07/02/2022 04:24, Tudor Ambarus wrote:
>> Convert Atmel TDES documentation to yaml format. With the conversion the
>> clock and clock-names properties are made mandatory. The driver returns
>> -EINVAL if "tdes_clk" is not found, reflect that in the bindings and make
>> the clock and clock-names properties mandatory. Update the example to
>> better describe how one should define the dt node.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  .../bindings/crypto/atmel,tdes.yaml           | 63 +++++++++++++++++++
>>  .../bindings/crypto/atmel-crypto.txt          | 23 -------
>>  2 files changed, 63 insertions(+), 23 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml b/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
>> new file mode 100644
>> index 000000000000..7efa5e4acaa1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
>> @@ -0,0 +1,63 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/crypto/atmel,tdes.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atmel Triple Data Encryption Standard (TDES) HW cryptographic accelerator
>> +
>> +maintainers:
>> +  - Tudor Ambarus <tudor.ambarus@microchip.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: atmel,at91sam9g46-tdes
>> +
> 
> Same comments as for patch 1 plus one new (also applying to previous
> one). You named the file quite generic "atmel,tdes" or "atmel,aes", but
> what if something newer comes for at91? Maybe name it instead
> "atmel,at91sam9-aes"?
> 

For historical reasons, the atmel-{aes,tdes,sha} drivers use their own
fixed compatible. The differentiation between the versions of the same IP
and their capabilities is done at run-time, by interrogating a version
register. Thus I expect that no new compatible will be added for neither of
these IPs.

Cheers,
ta

> Best regards,
> Krzysztof

Re: [PATCH 1/3] dt-bindings: crypto: Convert Atmel AES to yaml

[Tudor.Ambarus]

Hi, Krzysztof,

On 2/7/22 17:56, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 07/02/2022 04:24, Tudor Ambarus wrote:
>> Convert Atmel AES documentation to yaml format. With the conversion the
>> clock and clock-names properties are made mandatory. The driver returns
>> -EINVAL if "aes_clk" is not found, reflect that in the bindings and make
>> the clock and clock-names properties mandatory. Update the example to
>> better describe how one should define the dt node.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  .../devicetree/bindings/crypto/atmel,aes.yaml | 65 +++++++++++++++++++
>>  .../bindings/crypto/atmel-crypto.txt          | 20 ------
>>  2 files changed, 65 insertions(+), 20 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/crypto/atmel,aes.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/atmel,aes.yaml b/Documentation/devicetree/bindings/crypto/atmel,aes.yaml
>> new file mode 100644
>> index 000000000000..f77ec04dbabe
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/crypto/atmel,aes.yaml
>> @@ -0,0 +1,65 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/crypto/atmel,aes.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atmel Advanced Encryption Standard (AES) HW cryptographic accelerator
>> +
>> +maintainers:
>> +  - Tudor Ambarus <tudor.ambarus@microchip.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: atmel,at91sam9g46-aes
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  clock-names:
>> +    const: aes_clk
>> +
>> +  dmas:
>> +    items:
>> +      - description: TX DMA Channel
>> +      - description: RX DMA Channel
>> +
>> +  dma-names:
>> +    items:
>> +      - const: tx
>> +      - const: rx
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - clocks
>> +  - clock-names
>> +  - dmas
>> +  - dma-names
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/clock/at91.h>
>> +    #include <dt-bindings/dma/at91.h>
> 
> One empty line for readability.

Ok.

> 
>> +    aes: aes@f8038000 {
> 
> Generic node name, so "crypto".

Hm, I'm not convinced why, would you please give more details about this
requirement? This IP is capable of doing just AES operations, I find it
generic enough. We use the "aes" name on all our SoCs that have a version
of this IP, that would be quite a change. So I would prefer to keep the
"aes" name if possible.

> 
>> +      compatible = "atmel,at91sam9g46-aes";
>> +      reg = <0xe1810000 0x100>;
>> +      interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
>> +      clocks = <&pmc PMC_TYPE_PERIPHERAL 27>;
>> +      clock-names = "aes_clk";
>> +      dmas = <&dma0 AT91_XDMAC_DT_PERID(1)>,
>> +             <&dma0 AT91_XDMAC_DT_PERID(2)>;
>> +      dma-names = "tx", "rx";
>> +      status= "okay";
>> +    };
> 
> Drop the status property.

Ok. Thanks for reviewing.
ta

> 
> Best regards,
> Krzysztof

Re: [PATCH 1/3] dt-bindings: crypto: Convert Atmel AES to yaml

[Krzysztof Kozlowski]

On 08/02/2022 05:10, Tudor.Ambarus@microchip.com wrote:
> Hi, Krzysztof,
> 
> On 2/7/22 17:56, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 07/02/2022 04:24, Tudor Ambarus wrote:
>>> Convert Atmel AES documentation to yaml format. With the conversion the
>>> clock and clock-names properties are made mandatory. The driver returns
>>> -EINVAL if "aes_clk" is not found, reflect that in the bindings and make
>>> the clock and clock-names properties mandatory. Update the example to
>>> better describe how one should define the dt node.
>>>
>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>> ---
>>>  .../devicetree/bindings/crypto/atmel,aes.yaml | 65 +++++++++++++++++++
>>>  .../bindings/crypto/atmel-crypto.txt          | 20 ------
>>>  2 files changed, 65 insertions(+), 20 deletions(-)
>>>  create mode 100644 Documentation/devicetree/bindings/crypto/atmel,aes.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/crypto/atmel,aes.yaml b/Documentation/devicetree/bindings/crypto/atmel,aes.yaml
>>> new file mode 100644
>>> index 000000000000..f77ec04dbabe
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/crypto/atmel,aes.yaml
>>> @@ -0,0 +1,65 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/crypto/atmel,aes.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Atmel Advanced Encryption Standard (AES) HW cryptographic accelerator
>>> +
>>> +maintainers:
>>> +  - Tudor Ambarus <tudor.ambarus@microchip.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: atmel,at91sam9g46-aes
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  clock-names:
>>> +    const: aes_clk
>>> +
>>> +  dmas:
>>> +    items:
>>> +      - description: TX DMA Channel
>>> +      - description: RX DMA Channel
>>> +
>>> +  dma-names:
>>> +    items:
>>> +      - const: tx
>>> +      - const: rx
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - interrupts
>>> +  - clocks
>>> +  - clock-names
>>> +  - dmas
>>> +  - dma-names
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/irq.h>
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +    #include <dt-bindings/clock/at91.h>
>>> +    #include <dt-bindings/dma/at91.h>
>>
>> One empty line for readability.
> 
> Ok.
> 
>>
>>> +    aes: aes@f8038000 {
>>
>> Generic node name, so "crypto".
> 
> Hm, I'm not convinced why, would you please give more details about this
> requirement? This IP is capable of doing just AES operations, I find it
> generic enough. We use the "aes" name on all our SoCs that have a version
> of this IP, that would be quite a change. So I would prefer to keep the
> "aes" name if possible.
> 

The requirement comes from DT specification.
"The name of a node should be somewhat generic, reflecting the function
of the device and not its precise programming
 model. If appropriate, the name should be one of the following choice"
AES is not generic. AES is specific crypto operation. The spec gives
example - "crypto", so use this one just like others are using. Atmel is
not special in that matter.


Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: crypto: Convert Atmel TDES to yaml

[Krzysztof Kozlowski]

On 08/02/2022 05:04, Tudor.Ambarus@microchip.com wrote:
> Hi, Krzysztof,
> 
> On 2/7/22 18:04, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 07/02/2022 04:24, Tudor Ambarus wrote:
>>> Convert Atmel TDES documentation to yaml format. With the conversion the
>>> clock and clock-names properties are made mandatory. The driver returns
>>> -EINVAL if "tdes_clk" is not found, reflect that in the bindings and make
>>> the clock and clock-names properties mandatory. Update the example to
>>> better describe how one should define the dt node.
>>>
>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>> ---
>>>  .../bindings/crypto/atmel,tdes.yaml           | 63 +++++++++++++++++++
>>>  .../bindings/crypto/atmel-crypto.txt          | 23 -------
>>>  2 files changed, 63 insertions(+), 23 deletions(-)
>>>  create mode 100644 Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml b/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
>>> new file mode 100644
>>> index 000000000000..7efa5e4acaa1
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
>>> @@ -0,0 +1,63 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/crypto/atmel,tdes.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Atmel Triple Data Encryption Standard (TDES) HW cryptographic accelerator
>>> +
>>> +maintainers:
>>> +  - Tudor Ambarus <tudor.ambarus@microchip.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: atmel,at91sam9g46-tdes
>>> +
>>
>> Same comments as for patch 1 plus one new (also applying to previous
>> one). You named the file quite generic "atmel,tdes" or "atmel,aes", but
>> what if something newer comes for at91? Maybe name it instead
>> "atmel,at91sam9-aes"?
>>
> 
> For historical reasons, the atmel-{aes,tdes,sha} drivers use their own
> fixed compatible. The differentiation between the versions of the same IP
> and their capabilities is done at run-time, by interrogating a version
> register. Thus I expect that no new compatible will be added for neither of
> these IPs.

I was not talking about compatibles. I was talking about file name. You
called it "atmel,tdes" which is quite generic. If Microchip (not
Atmel...) comes with a new type of AES/TDES/SHA block for new line of
architectures, how are you going to name the bindings?

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: crypto: Convert Atmel TDES to yaml

[Tudor.Ambarus]

On 2/8/22 11:01, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 08/02/2022 05:04, Tudor.Ambarus@microchip.com wrote:
>> Hi, Krzysztof,
>>
>> On 2/7/22 18:04, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 07/02/2022 04:24, Tudor Ambarus wrote:
>>>> Convert Atmel TDES documentation to yaml format. With the conversion the
>>>> clock and clock-names properties are made mandatory. The driver returns
>>>> -EINVAL if "tdes_clk" is not found, reflect that in the bindings and make
>>>> the clock and clock-names properties mandatory. Update the example to
>>>> better describe how one should define the dt node.
>>>>
>>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>> ---
>>>>  .../bindings/crypto/atmel,tdes.yaml           | 63 +++++++++++++++++++
>>>>  .../bindings/crypto/atmel-crypto.txt          | 23 -------
>>>>  2 files changed, 63 insertions(+), 23 deletions(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml b/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
>>>> new file mode 100644
>>>> index 000000000000..7efa5e4acaa1
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/crypto/atmel,tdes.yaml
>>>> @@ -0,0 +1,63 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/crypto/atmel,tdes.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Atmel Triple Data Encryption Standard (TDES) HW cryptographic accelerator
>>>> +
>>>> +maintainers:
>>>> +  - Tudor Ambarus <tudor.ambarus@microchip.com>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: atmel,at91sam9g46-tdes
>>>> +
>>>
>>> Same comments as for patch 1 plus one new (also applying to previous
>>> one). You named the file quite generic "atmel,tdes" or "atmel,aes", but
>>> what if something newer comes for at91? Maybe name it instead
>>> "atmel,at91sam9-aes"?
>>>
>>
>> For historical reasons, the atmel-{aes,tdes,sha} drivers use their own
>> fixed compatible. The differentiation between the versions of the same IP
>> and their capabilities is done at run-time, by interrogating a version
>> register. Thus I expect that no new compatible will be added for neither of
>> these IPs.
> 
> I was not talking about compatibles. I was talking about file name. You
> called it "atmel,tdes" which is quite generic. If Microchip (not

oh yes, I misread your previous email.

> Atmel...) comes with a new type of AES/TDES/SHA block for new line of
> architectures, how are you going to name the bindings?
> 

"atmel,at91sam9g46-tdes" file name should be fine, thanks for the suggestion.

ta

Re: [PATCH 1/3] dt-bindings: crypto: Convert Atmel AES to yaml

[Tudor.Ambarus]

On 2/8/22 10:59, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 08/02/2022 05:10, Tudor.Ambarus@microchip.com wrote:
>> Hi, Krzysztof,
>>
>> On 2/7/22 17:56, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 07/02/2022 04:24, Tudor Ambarus wrote:
>>>> Convert Atmel AES documentation to yaml format. With the conversion the
>>>> clock and clock-names properties are made mandatory. The driver returns
>>>> -EINVAL if "aes_clk" is not found, reflect that in the bindings and make
>>>> the clock and clock-names properties mandatory. Update the example to
>>>> better describe how one should define the dt node.
>>>>
>>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>> ---
>>>>  .../devicetree/bindings/crypto/atmel,aes.yaml | 65 +++++++++++++++++++
>>>>  .../bindings/crypto/atmel-crypto.txt          | 20 ------
>>>>  2 files changed, 65 insertions(+), 20 deletions(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/crypto/atmel,aes.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/crypto/atmel,aes.yaml b/Documentation/devicetree/bindings/crypto/atmel,aes.yaml
>>>> new file mode 100644
>>>> index 000000000000..f77ec04dbabe
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/crypto/atmel,aes.yaml
>>>> @@ -0,0 +1,65 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/crypto/atmel,aes.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Atmel Advanced Encryption Standard (AES) HW cryptographic accelerator
>>>> +
>>>> +maintainers:
>>>> +  - Tudor Ambarus <tudor.ambarus@microchip.com>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: atmel,at91sam9g46-aes
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  interrupts:
>>>> +    maxItems: 1
>>>> +
>>>> +  clocks:
>>>> +    maxItems: 1
>>>> +
>>>> +  clock-names:
>>>> +    const: aes_clk
>>>> +
>>>> +  dmas:
>>>> +    items:
>>>> +      - description: TX DMA Channel
>>>> +      - description: RX DMA Channel
>>>> +
>>>> +  dma-names:
>>>> +    items:
>>>> +      - const: tx
>>>> +      - const: rx
>>>> +
>>>> +required:
>>>> +  - compatible
>>>> +  - reg
>>>> +  - interrupts
>>>> +  - clocks
>>>> +  - clock-names
>>>> +  - dmas
>>>> +  - dma-names
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> +  - |
>>>> +    #include <dt-bindings/interrupt-controller/irq.h>
>>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>> +    #include <dt-bindings/clock/at91.h>
>>>> +    #include <dt-bindings/dma/at91.h>
>>>
>>> One empty line for readability.
>>
>> Ok.
>>
>>>
>>>> +    aes: aes@f8038000 {
>>>
>>> Generic node name, so "crypto".
>>
>> Hm, I'm not convinced why, would you please give more details about this
>> requirement? This IP is capable of doing just AES operations, I find it
>> generic enough. We use the "aes" name on all our SoCs that have a version
>> of this IP, that would be quite a change. So I would prefer to keep the
>> "aes" name if possible.
>>
> 
> The requirement comes from DT specification.
> "The name of a node should be somewhat generic, reflecting the function
> of the device and not its precise programming
>  model. If appropriate, the name should be one of the following choice"
> AES is not generic. AES is specific crypto operation. The spec gives
> example - "crypto", so use this one just like others are using. Atmel is
> not special in that matter.
> 
I see, thanks for the explanation. I will put the node name as "crypto", and add
a label to it as "aes":
aes: crypto@f8038000 {

Cheers,
ta

Re: [PATCH 1/3] dt-bindings: crypto: Convert Atmel AES to yaml

[Krzysztof Kozlowski]

On 08/02/2022 10:49, Tudor.Ambarus@microchip.com wrote:
> On 2/8/22 10:59, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 08/02/2022 05:10, Tudor.Ambarus@microchip.com wrote:
>>> Hi, Krzysztof,
>>>
>>> On 2/7/22 17:56, Krzysztof Kozlowski wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> On 07/02/2022 04:24, Tudor Ambarus wrote:
>>>>> Convert Atmel AES documentation to yaml format. With the conversion the
>>>>> clock and clock-names properties are made mandatory. The driver returns
>>>>> -EINVAL if "aes_clk" is not found, reflect that in the bindings and make
>>>>> the clock and clock-names properties mandatory. Update the example to
>>>>> better describe how one should define the dt node.
>>>>>
>>>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>>> ---
>>>>>  .../devicetree/bindings/crypto/atmel,aes.yaml | 65 +++++++++++++++++++
>>>>>  .../bindings/crypto/atmel-crypto.txt          | 20 ------
>>>>>  2 files changed, 65 insertions(+), 20 deletions(-)
>>>>>  create mode 100644 Documentation/devicetree/bindings/crypto/atmel,aes.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/crypto/atmel,aes.yaml b/Documentation/devicetree/bindings/crypto/atmel,aes.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..f77ec04dbabe
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/crypto/atmel,aes.yaml
>>>>> @@ -0,0 +1,65 @@
>>>>> +# SPDX-License-Identifier: GPL-2.0-only
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/crypto/atmel,aes.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Atmel Advanced Encryption Standard (AES) HW cryptographic accelerator
>>>>> +
>>>>> +maintainers:
>>>>> +  - Tudor Ambarus <tudor.ambarus@microchip.com>
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    const: atmel,at91sam9g46-aes
>>>>> +
>>>>> +  reg:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  interrupts:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  clocks:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  clock-names:
>>>>> +    const: aes_clk
>>>>> +
>>>>> +  dmas:
>>>>> +    items:
>>>>> +      - description: TX DMA Channel
>>>>> +      - description: RX DMA Channel
>>>>> +
>>>>> +  dma-names:
>>>>> +    items:
>>>>> +      - const: tx
>>>>> +      - const: rx
>>>>> +
>>>>> +required:
>>>>> +  - compatible
>>>>> +  - reg
>>>>> +  - interrupts
>>>>> +  - clocks
>>>>> +  - clock-names
>>>>> +  - dmas
>>>>> +  - dma-names
>>>>> +
>>>>> +additionalProperties: false
>>>>> +
>>>>> +examples:
>>>>> +  - |
>>>>> +    #include <dt-bindings/interrupt-controller/irq.h>
>>>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>> +    #include <dt-bindings/clock/at91.h>
>>>>> +    #include <dt-bindings/dma/at91.h>
>>>>
>>>> One empty line for readability.
>>>
>>> Ok.
>>>
>>>>
>>>>> +    aes: aes@f8038000 {
>>>>
>>>> Generic node name, so "crypto".
>>>
>>> Hm, I'm not convinced why, would you please give more details about this
>>> requirement? This IP is capable of doing just AES operations, I find it
>>> generic enough. We use the "aes" name on all our SoCs that have a version
>>> of this IP, that would be quite a change. So I would prefer to keep the
>>> "aes" name if possible.
>>>
>>
>> The requirement comes from DT specification.
>> "The name of a node should be somewhat generic, reflecting the function
>> of the device and not its precise programming
>>  model. If appropriate, the name should be one of the following choice"
>> AES is not generic. AES is specific crypto operation. The spec gives
>> example - "crypto", so use this one just like others are using. Atmel is
>> not special in that matter.
>>
> I see, thanks for the explanation. I will put the node name as "crypto", and add
> a label to it as "aes":
> aes: crypto@f8038000 {
> 

That's fine (label naming doesn't matter). You don't have to change all
your DTS files with the new node name, although at some point it could
be required.

For some other bindings in different subsystems, the node name pattern
is now enforced by dtschema (git grep -C 1 -E
'(nodename|patternProperties)' -- Documentation/devicetree/bindings/).

Best regards,
Krzysztof