[0/5] Add support for Texas Instruments MCRC64 engine

  Add support for MCRC64 engine to calculate 64-bit CRC in Full-CPU mode

MCRC64 engine calculates 64-bit cyclic redundancy checks (CRC)
according to the ISO 3309 standard.

Generator polynomial: x^64 + x^4 + x^3 + x + 1
Polynomial value: 0x000000000000001b

Tested with

and tcrypt,
sudo modprobe tcrypt mode=329 sec=1

Signed-off-by: Kamlesh Gurudasani <kamlesh@ti.com>
---
Kamlesh Gurudasani (5):
      crypto: crc64 - add crc64-iso test vectors
      dt-bindings: crypto: Add binding for TI MCRC64 driver
      crypto: ti - add driver for MCRC64 engine
      arm64: dts: ti: k3-am62: Add dt node, cbass_main ranges for MCRC64
      arm64: defconfig: enable MCRC module

 Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml |  42 +++++++
 MAINTAINERS                                             |   7 ++
 arch/arm64/boot/dts/ti/k3-am62-main.dtsi                |   7 ++
 arch/arm64/boot/dts/ti/k3-am62.dtsi                     |   1 +
 arch/arm64/configs/defconfig                            |   2 +
 crypto/tcrypt.c                                         |   5 +
 crypto/testmgr.c                                        |   7 ++
 crypto/testmgr.h                                        | 401 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/crypto/Kconfig                                  |   1 +
 drivers/crypto/Makefile                                 |   1 +
 drivers/crypto/ti/Kconfig                               |  10 ++
 drivers/crypto/ti/Makefile                              |   2 +
 drivers/crypto/ti/mcrc64.c                              | 360 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 13 files changed, 846 insertions(+)
---
base-commit: d7b3af5a77e8d8da28f435f313e069aea5bcf172
change-id: 20230719-mcrc-upstream-7ae9a75cab37

Best regards,
  

On 30/07/2023 20:55, Kamlesh Gurudasani wrote:
> Add binding for Texas Instruments MCRC64 driver

A nit, subject: drop second/last, redundant "binding for". The
"dt-bindings" prefix is already stating that these are bindings.

Here and subject: drop driver. Bindings are for hardware.

Neither commit nor bindings in description: field explain what is this
hardware.

> 
> Signed-off-by: Kamlesh Gurudasani <kamlesh@ti.com>
> ---
>  Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml | 42 ++++++++++++++++++++++++++++++++++++++++++
>  MAINTAINERS                                             |  5 +++++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml b/Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml
> new file mode 100644
> index 000000000000..1d1e3f87638c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/crypto/ti,mcrc64.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments MCRC64 bindings

Drop bindings. If you tested your code, you would see a warning, so:

It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.

> +
> +maintainers:
> +  - Kamlesh Gurudasani <kamlesh@ti.com>
> +
> +properties:
> +  compatible:
> +    const: ti,mcrc64

What's this? Part of SoC? Then the compatible is not correct.

> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - power-domains
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    mcrc64: mcrc64@30300000 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +      compatible = "ti,mcrc64";
> +      reg = <0x00 0x30300000 0x00 0x1000>;
> +      clocks = <&k3_clks 116 0>;
> +      power-domains = <&k3_pds 116 TI_SCI_PD_EXCLUSIVE>;
> +      };

Indentation is messed up.
> 

Best regards,
Krzysztof
  

On Sun, 30 Jul 2023 at 20:56, Kamlesh Gurudasani <kamlesh@ti.com> wrote:
>
> Add support for MCRC64 engine to calculate 64-bit CRC in Full-CPU mode
>
> MCRC64 engine calculates 64-bit cyclic redundancy checks (CRC)
> according to the ISO 3309 standard.
>
> Generator polynomial: x^64 + x^4 + x^3 + x + 1
> Polynomial value: 0x000000000000001b
>

How will this code be used? WIthout a user of the crc64-iso crypto API
algorithm, there is no point in having a driver that implements it.

Also, *if* such a user exists, we'd need to have a generic C
implementation as well - we don't add new algorithms unless they can
be enabled on all platforms and architectures.


> Tested with
>
> and tcrypt,
> sudo modprobe tcrypt mode=329 sec=1
>
> Signed-off-by: Kamlesh Gurudasani <kamlesh@ti.com>
> ---
> Kamlesh Gurudasani (5):
>       crypto: crc64 - add crc64-iso test vectors
>       dt-bindings: crypto: Add binding for TI MCRC64 driver
>       crypto: ti - add driver for MCRC64 engine
>       arm64: dts: ti: k3-am62: Add dt node, cbass_main ranges for MCRC64
>       arm64: defconfig: enable MCRC module
>
>  Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml |  42 +++++++
>  MAINTAINERS                                             |   7 ++
>  arch/arm64/boot/dts/ti/k3-am62-main.dtsi                |   7 ++
>  arch/arm64/boot/dts/ti/k3-am62.dtsi                     |   1 +
>  arch/arm64/configs/defconfig                            |   2 +
>  crypto/tcrypt.c                                         |   5 +
>  crypto/testmgr.c                                        |   7 ++
>  crypto/testmgr.h                                        | 401 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/crypto/Kconfig                                  |   1 +
>  drivers/crypto/Makefile                                 |   1 +
>  drivers/crypto/ti/Kconfig                               |  10 ++
>  drivers/crypto/ti/Makefile                              |   2 +
>  drivers/crypto/ti/mcrc64.c                              | 360 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  13 files changed, 846 insertions(+)
> ---
> base-commit: d7b3af5a77e8d8da28f435f313e069aea5bcf172
> change-id: 20230719-mcrc-upstream-7ae9a75cab37
>
> Best regards,
> --
> Kamlesh Gurudasani <kamlesh@ti.com>
>
  

Ard Biesheuvel <ardb@kernel.org> writes:

> On Sun, 30 Jul 2023 at 20:56, Kamlesh Gurudasani <kamlesh@ti.com> wrote:
>>
>> Add support for MCRC64 engine to calculate 64-bit CRC in Full-CPU mode
>>
>> MCRC64 engine calculates 64-bit cyclic redundancy checks (CRC)
>> according to the ISO 3309 standard.
>>
>> Generator polynomial: x^64 + x^4 + x^3 + x + 1
>> Polynomial value: 0x000000000000001b
>>
>
> How will this code be used? WIthout a user of the crc64-iso crypto API
> algorithm, there is no point in having a driver that implements it.

Thanks for the review.

MCRC64 will be used to check the integrity of memory blocks.
On TI K3 SOCs, users can use MCRC64 to accelerate the integrity check.

>
> Also, *if* such a user exists, we'd need to have a generic C
> implementation as well - we don't add new algorithms unless they can
> be enabled on all platforms and architectures.

If it is must, will implement generic C code.

>
>
>> Tested with
>>
>> and tcrypt,
>> sudo modprobe tcrypt mode=329 sec=1
>>
>> Signed-off-by: Kamlesh Gurudasani <kamlesh@ti.com>
>> ---
>> Kamlesh Gurudasani (5):
>>       crypto: crc64 - add crc64-iso test vectors
>>       dt-bindings: crypto: Add binding for TI MCRC64 driver
>>       crypto: ti - add driver for MCRC64 engine
>>       arm64: dts: ti: k3-am62: Add dt node, cbass_main ranges for MCRC64
>>       arm64: defconfig: enable MCRC module
>>
>>  Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml |  42 +++++++
>>  MAINTAINERS                                             |   7 ++
>>  arch/arm64/boot/dts/ti/k3-am62-main.dtsi                |   7 ++
>>  arch/arm64/boot/dts/ti/k3-am62.dtsi                     |   1 +
>>  arch/arm64/configs/defconfig                            |   2 +
>>  crypto/tcrypt.c                                         |   5 +
>>  crypto/testmgr.c                                        |   7 ++
>>  crypto/testmgr.h                                        | 401 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/crypto/Kconfig                                  |   1 +
>>  drivers/crypto/Makefile                                 |   1 +
>>  drivers/crypto/ti/Kconfig                               |  10 ++
>>  drivers/crypto/ti/Makefile                              |   2 +
>>  drivers/crypto/ti/mcrc64.c                              | 360 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  13 files changed, 846 insertions(+)
>> ---
>> base-commit: d7b3af5a77e8d8da28f435f313e069aea5bcf172
>> change-id: 20230719-mcrc-upstream-7ae9a75cab37
>>
>> Best regards,
>> --
>> Kamlesh Gurudasani <kamlesh@ti.com>
>>
  

On Tue, 1 Aug 2023 at 08:22, Kamlesh Gurudasani <kamlesh@ti.com> wrote:
>
> Ard Biesheuvel <ardb@kernel.org> writes:
>
> > On Sun, 30 Jul 2023 at 20:56, Kamlesh Gurudasani <kamlesh@ti.com> wrote:
> >>
> >> Add support for MCRC64 engine to calculate 64-bit CRC in Full-CPU mode
> >>
> >> MCRC64 engine calculates 64-bit cyclic redundancy checks (CRC)
> >> according to the ISO 3309 standard.
> >>
> >> Generator polynomial: x^64 + x^4 + x^3 + x + 1
> >> Polynomial value: 0x000000000000001b
> >>
> >
> > How will this code be used? WIthout a user of the crc64-iso crypto API
> > algorithm, there is no point in having a driver that implements it.
>
> Thanks for the review.
>
> MCRC64 will be used to check the integrity of memory blocks.
> On TI K3 SOCs, users can use MCRC64 to accelerate the integrity check.
>

Where is the implementation of this logic? Does it use the shash/ahash API?

If so, it should be part of this series.


> >
> > Also, *if* such a user exists, we'd need to have a generic C
> > implementation as well - we don't add new algorithms unless they can
> > be enabled on all platforms and architectures.
>
> If it is must, will implement generic C code.
>
> >
> >
> >> Tested with
> >>
> >> and tcrypt,
> >> sudo modprobe tcrypt mode=329 sec=1
> >>
> >> Signed-off-by: Kamlesh Gurudasani <kamlesh@ti.com>
> >> ---
> >> Kamlesh Gurudasani (5):
> >>       crypto: crc64 - add crc64-iso test vectors
> >>       dt-bindings: crypto: Add binding for TI MCRC64 driver
> >>       crypto: ti - add driver for MCRC64 engine
> >>       arm64: dts: ti: k3-am62: Add dt node, cbass_main ranges for MCRC64
> >>       arm64: defconfig: enable MCRC module
> >>
> >>  Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml |  42 +++++++
> >>  MAINTAINERS                                             |   7 ++
> >>  arch/arm64/boot/dts/ti/k3-am62-main.dtsi                |   7 ++
> >>  arch/arm64/boot/dts/ti/k3-am62.dtsi                     |   1 +
> >>  arch/arm64/configs/defconfig                            |   2 +
> >>  crypto/tcrypt.c                                         |   5 +
> >>  crypto/testmgr.c                                        |   7 ++
> >>  crypto/testmgr.h                                        | 401 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  drivers/crypto/Kconfig                                  |   1 +
> >>  drivers/crypto/Makefile                                 |   1 +
> >>  drivers/crypto/ti/Kconfig                               |  10 ++
> >>  drivers/crypto/ti/Makefile                              |   2 +
> >>  drivers/crypto/ti/mcrc64.c                              | 360 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  13 files changed, 846 insertions(+)
> >> ---
> >> base-commit: d7b3af5a77e8d8da28f435f313e069aea5bcf172
> >> change-id: 20230719-mcrc-upstream-7ae9a75cab37
> >>
> >> Best regards,
> >> --
> >> Kamlesh Gurudasani <kamlesh@ti.com>
> >>
  

Ard Biesheuvel <ardb@kernel.org> writes:

> On Tue, 1 Aug 2023 at 08:22, Kamlesh Gurudasani <kamlesh@ti.com> wrote:
>>
>> Ard Biesheuvel <ardb@kernel.org> writes:
>>
>> > On Sun, 30 Jul 2023 at 20:56, Kamlesh Gurudasani <kamlesh@ti.com> wrote:
>> >>
>> >> Add support for MCRC64 engine to calculate 64-bit CRC in Full-CPU mode
>> >>
>> >> MCRC64 engine calculates 64-bit cyclic redundancy checks (CRC)
>> >> according to the ISO 3309 standard.
>> >>
>> >> Generator polynomial: x^64 + x^4 + x^3 + x + 1
>> >> Polynomial value: 0x000000000000001b
>> >>
>> >
>> > How will this code be used? WIthout a user of the crc64-iso crypto API
>> > algorithm, there is no point in having a driver that implements it.
>>
>> Thanks for the review.
>>
>> MCRC64 will be used to check the integrity of memory blocks.
>> On TI K3 SOCs, users can use MCRC64 to accelerate the integrity check.
>>
>
> Where is the implementation of this logic? Does it use the shash/ahash API?
>
> If so, it should be part of this series.

We have implemented user-space application to utilize crc64-iso using below kernel module

algif_hash: User-space interface for hash algorithms 

https://gist.github.com/ti-kamlesh/73abfcc1a33318bb3b199d36b6209e59

>
>
>> >
>> > Also, *if* such a user exists, we'd need to have a generic C
>> > implementation as well - we don't add new algorithms unless they can
>> > be enabled on all platforms and architectures.
>>
>> If it is must, will implement generic C code.
>>
>> >
>> >
>> >> Tested with
>> >>
>> >> and tcrypt,
>> >> sudo modprobe tcrypt mode=329 sec=1
>> >>
>> >> Signed-off-by: Kamlesh Gurudasani <kamlesh@ti.com>
>> >> ---
>> >> Kamlesh Gurudasani (5):
>> >>       crypto: crc64 - add crc64-iso test vectors
>> >>       dt-bindings: crypto: Add binding for TI MCRC64 driver
>> >>       crypto: ti - add driver for MCRC64 engine
>> >>       arm64: dts: ti: k3-am62: Add dt node, cbass_main ranges for MCRC64
>> >>       arm64: defconfig: enable MCRC module
>> >>
>> >>  Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml |  42 +++++++
>> >>  MAINTAINERS                                             |   7 ++
>> >>  arch/arm64/boot/dts/ti/k3-am62-main.dtsi                |   7 ++
>> >>  arch/arm64/boot/dts/ti/k3-am62.dtsi                     |   1 +
>> >>  arch/arm64/configs/defconfig                            |   2 +
>> >>  crypto/tcrypt.c                                         |   5 +
>> >>  crypto/testmgr.c                                        |   7 ++
>> >>  crypto/testmgr.h                                        | 401 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  drivers/crypto/Kconfig                                  |   1 +
>> >>  drivers/crypto/Makefile                                 |   1 +
>> >>  drivers/crypto/ti/Kconfig                               |  10 ++
>> >>  drivers/crypto/ti/Makefile                              |   2 +
>> >>  drivers/crypto/ti/mcrc64.c                              | 360 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  13 files changed, 846 insertions(+)
>> >> ---
>> >> base-commit: d7b3af5a77e8d8da28f435f313e069aea5bcf172
>> >> change-id: 20230719-mcrc-upstream-7ae9a75cab37
>> >>
>> >> Best regards,
>> >> --
>> >> Kamlesh Gurudasani <kamlesh@ti.com>
>> >>