diff mbox series

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

Message ID 20220207032405.70733-3-tudor.ambarus@microchip.com (mailing list archive)
State New, archived
Headers show
Series dt-bindings: crypto: Convert atmel-{aes, tdes, sha} to YAML | expand

Commit Message

Tudor Ambarus Feb. 7, 2022, 3:24 a.m. UTC 
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

Comments

Krzysztof Kozlowski Feb. 7, 2022, 4:04 p.m. UTC | #1 
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
Tudor Ambarus Feb. 8, 2022, 4:04 a.m. UTC | #2 
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
Krzysztof Kozlowski Feb. 8, 2022, 9:01 a.m. UTC | #3 
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
Tudor Ambarus Feb. 8, 2022, 9:19 a.m. UTC | #4 
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
diff mbox series

Patch

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: