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@xxxxxxxxxxxxx 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@xxxxxxxxxxxxx>
>>>> ---
>>>>  .../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@xxxxxxxxxxxxx>
>>>> +
>>>> +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