[v2,1/5] dt-bindings: crypto: Add Tegra Security Engine

  Add DT binding document for Tegra Security Engine.
The AES and HASH algorithms are handled independently by separate
engines within the Security Engine. These engines are registered
as two separate crypto engine drivers.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 .../crypto/nvidia,tegra234-se-aes.yaml        | 53 +++++++++++++++++++
 .../crypto/nvidia,tegra234-se-hash.yaml       | 53 +++++++++++++++++++
 2 files changed, 106 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-aes.yaml
 create mode 100644 Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-hash.yaml
  

On 19/12/2023 13:56, Akhil R wrote:
> Add DT binding document for Tegra Security Engine.
> The AES and HASH algorithms are handled independently by separate
> engines within the Security Engine. These engines are registered
> as two separate crypto engine drivers.
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>  .../crypto/nvidia,tegra234-se-aes.yaml        | 53 +++++++++++++++++++
>  .../crypto/nvidia,tegra234-se-hash.yaml       | 53 +++++++++++++++++++
>  2 files changed, 106 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-aes.yaml
>  create mode 100644 Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-hash.yaml
> 
> diff --git a/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-aes.yaml b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-aes.yaml
> new file mode 100644
> index 000000000000..35c2e701bd42
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-aes.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/crypto/nvidia,tegra234-se-aes.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVIDIA Tegra Security Engine for AES algorithms
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The Tegra Security Engine accelerates the following AES encryption/decryption
> +  algorithms.

s/./:/ and join lines? Wasn't that your intention here?

> +  AES-ECB, AES-CBC, AES-OFB, AES-XTS, AES-CTR, AES-GCM, AES-CCM, AES-CMAC
> +
> +maintainers:
> +  - Akhil R <akhilrajeev@nvidia.com>
> +
> +properties:
> +  compatible:
> +    const: nvidia,tegra234-se2-aes

Why "se2"?

Anyway, filename like compatible.


> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  iommus:
> +    maxItems: 1
> +
> +  dma-coherent: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - iommus
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/memory/tegra234-mc.h>
> +    #include <dt-bindings/clock/tegra234-clock.h>
> +
> +    crypto@15820000 {
> +        compatible = "nvidia,tegra234-se2-aes";
> +        reg = <0x15820000 0x10000>;
> +        clocks = <&bpmp TEGRA234_CLK_SE>;
> +        iommus = <&smmu TEGRA234_SID_SES_SE1>;
> +        dma-coherent;
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-hash.yaml b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-hash.yaml
> new file mode 100644
> index 000000000000..e3848e9a53b5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-hash.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/crypto/nvidia,tegra234-se-hash.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVIDIA Tegra Security Engine for HASH algorithms
> +
> +description: |
> +  The Tegra Security HASH Engine accelerates the following HASH functions.

Similar comment

> +  SHA1, SHA224, SHA256, SHA384, SHA512, SHA3-224, SHA3-256, SHA3-384, SHA3-512
> +  HMAC(SHA224), HMAC(SHA256), HMAC(SHA384), HMAC(SHA512)
> +

> +maintainers:
> +  - Akhil R <akhilrajeev@nvidia.com>
> +
> +properties:
> +  compatible:
> +    const: nvidia,tegra234-se4-hash

What is se4?

Anyway, filename like compatible.



Best regards,
Krzysztof
  

> On 19/12/2023 13:56, Akhil R wrote:
> > Add DT binding document for Tegra Security Engine.
> > The AES and HASH algorithms are handled independently by separate
> > engines within the Security Engine. These engines are registered
> > as two separate crypto engine drivers.
> >
> > Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> > ---
> >  .../crypto/nvidia,tegra234-se-aes.yaml        | 53 +++++++++++++++++++
> >  .../crypto/nvidia,tegra234-se-hash.yaml       | 53 +++++++++++++++++++
> >  2 files changed, 106 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-aes.yaml
> >  create mode 100644
> Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-hash.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-
> aes.yaml b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-
> aes.yaml
> > new file mode 100644
> > index 000000000000..35c2e701bd42
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-aes.yaml
> > @@ -0,0 +1,53 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/crypto/nvidia,tegra234-se-aes.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NVIDIA Tegra Security Engine for AES algorithms
> > +
> > +description: |
> 
> Do not need '|' unless you need to preserve formatting.
Okay.

> 
> > +  The Tegra Security Engine accelerates the following AES
> encryption/decryption
> > +  algorithms.
> 
> s/./:/ and join lines? Wasn't that your intention here?
Yes. Correct. 

> 
> > +  AES-ECB, AES-CBC, AES-OFB, AES-XTS, AES-CTR, AES-GCM, AES-CCM, AES-
> CMAC
> > +
> > +maintainers:
> > +  - Akhil R <akhilrajeev@nvidia.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: nvidia,tegra234-se2-aes
> 
> Why "se2"?
> 
> Anyway, filename like compatible.
The instance is named SE2 in the hardware. Thought to align the compatible as well.

There is one more instance with a slightly different capability, which we aren't using currently.
In case it needs to be supported, we would need a different compatible, but the driver will need
only minor changes. I suppose the current compatible can help to maintain the ABI with DT then.

> 
> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  iommus:
> > +    maxItems: 1
> > +
> > +  dma-coherent: true
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - iommus
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/memory/tegra234-mc.h>
> > +    #include <dt-bindings/clock/tegra234-clock.h>
> > +
> > +    crypto@15820000 {
> > +        compatible = "nvidia,tegra234-se2-aes";
> > +        reg = <0x15820000 0x10000>;
> > +        clocks = <&bpmp TEGRA234_CLK_SE>;
> > +        iommus = <&smmu TEGRA234_SID_SES_SE1>;
> > +        dma-coherent;
> > +    };
> > +...
> > diff --git a/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-
> hash.yaml b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-
> hash.yaml
> > new file mode 100644
> > index 000000000000..e3848e9a53b5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-
> hash.yaml
> > @@ -0,0 +1,53 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/crypto/nvidia,tegra234-se-hash.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NVIDIA Tegra Security Engine for HASH algorithms
> > +
> > +description: |
> > +  The Tegra Security HASH Engine accelerates the following HASH functions.
> 
> Similar comment
> 
> > +  SHA1, SHA224, SHA256, SHA384, SHA512, SHA3-224, SHA3-256, SHA3-384,
> SHA3-512
> > +  HMAC(SHA224), HMAC(SHA256), HMAC(SHA384), HMAC(SHA512)
> > +
> 
> > +maintainers:
> > +  - Akhil R <akhilrajeev@nvidia.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: nvidia,tegra234-se4-hash
> 
> What is se4?
> 
> Anyway, filename like compatible.
Similar to the above, the hardware name is SE4.

nvidia,tegra234-se-aes and nvidia,tegra234-se-hash does look good to me. But I am a bit concerned
about the ABI breakage in case, we need a different compatible for the remaining instance.

Thanks and Regards,
Akhil
  

On 28/12/2023 10:33, Akhil R wrote:
>>> +properties:
>>> +  compatible:
>>> +    const: nvidia,tegra234-se4-hash
>>
>> What is se4?
>>
>> Anyway, filename like compatible.
> Similar to the above, the hardware name is SE4.
> 
> nvidia,tegra234-se-aes and nvidia,tegra234-se-hash does look good to me. But I am a bit concerned
> about the ABI breakage in case, we need a different compatible for the remaining instance.

Isn't this a new device? What ABI breakage? What would be affected?

Best regards,
Krzysztof
  

> On 28/12/2023 10:33, Akhil R wrote:
> >>> +properties:
> >>> +  compatible:
> >>> +    const: nvidia,tegra234-se4-hash
> >>
> >> What is se4?
> >>
> >> Anyway, filename like compatible.
> > Similar to the above, the hardware name is SE4.
> >
> > nvidia,tegra234-se-aes and nvidia,tegra234-se-hash does look good to
> > me. But I am a bit concerned about the ABI breakage in case, we need a
> different compatible for the remaining instance.
> 
> Isn't this a new device? What ABI breakage? What would be affected?

I meant a scenario where we need to support SE1 instance as well.

There is one more SE instance in Tegra, which is very similar to SE2 AES Engine.
But right now, it does not have a good use case in Linux. Now if we add 
nvidia,tegra234-se-aes and nvidia,tegra234-se-hash, when SE1 needs to be
supported, I guess it would be confusing to find the right compatible for it.

Regards,
Akhil
  

On 29/12/2023 08:11, Akhil R wrote:
>> On 28/12/2023 10:33, Akhil R wrote:
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    const: nvidia,tegra234-se4-hash
>>>>
>>>> What is se4?
>>>>
>>>> Anyway, filename like compatible.
>>> Similar to the above, the hardware name is SE4.
>>>
>>> nvidia,tegra234-se-aes and nvidia,tegra234-se-hash does look good to
>>> me. But I am a bit concerned about the ABI breakage in case, we need a
>> different compatible for the remaining instance.
>>
>> Isn't this a new device? What ABI breakage? What would be affected?
> 
> I meant a scenario where we need to support SE1 instance as well.
> 
> There is one more SE instance in Tegra, which is very similar to SE2 AES Engine.
> But right now, it does not have a good use case in Linux. Now if we add 
> nvidia,tegra234-se-aes and nvidia,tegra234-se-hash, when SE1 needs to be
> supported, I guess it would be confusing to find the right compatible for it.

Hm, I still do not see possibility of breaking of ABI, but sure, se4
makes sense if instances are really different. Otherwise could be one
compatible with some property. It kind of depends on the differences.

Anyway, name the file based on the compatible.

Best regards,
Krzysztof
  

> On 29/12/2023 08:11, Akhil R wrote:
> >> On 28/12/2023 10:33, Akhil R wrote:
> >>>>> +properties:
> >>>>> +  compatible:
> >>>>> +    const: nvidia,tegra234-se4-hash
> >>>>
> >>>> What is se4?
> >>>>
> >>>> Anyway, filename like compatible.
> >>> Similar to the above, the hardware name is SE4.
> >>>
> >>> nvidia,tegra234-se-aes and nvidia,tegra234-se-hash does look good to
> >>> me. But I am a bit concerned about the ABI breakage in case, we need a
> >> different compatible for the remaining instance.
> >>
> >> Isn't this a new device? What ABI breakage? What would be affected?
> >
> > I meant a scenario where we need to support SE1 instance as well.
> >
> > There is one more SE instance in Tegra, which is very similar to SE2 AES Engine.
> > But right now, it does not have a good use case in Linux. Now if we add
> > nvidia,tegra234-se-aes and nvidia,tegra234-se-hash, when SE1 needs to be
> > supported, I guess it would be confusing to find the right compatible for it.
> 
> Hm, I still do not see possibility of breaking of ABI, but sure, se4
> makes sense if instances are really different. Otherwise could be one
> compatible with some property. It kind of depends on the differences.
> 
> Anyway, name the file based on the compatible.
 
One compatible with some property looks to be a good approach to me.
Instances aren't totally different.
So, I will update the compatible to nvidia,tegra234-se-aes and
nvidia,tegra234-se-hash in the next revision.

Thanks for the comments and inputs.

Regards,
Akhil