Re: [PATCH 6/6] crypto: rockchip: add rk3588 driver

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 07/11/2023 17:42, Krzysztof Kozlowski wrote:
> On 07/11/2023 17:35, Krzysztof Kozlowski wrote:
>> On 07/11/2023 16:55, Corentin Labbe wrote:
>>> RK3588 have a new crypto IP, this patch adds basic support for it.
>>> Only hashes and cipher are handled for the moment.
>>>
>>> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
>>> ---
>>>  drivers/crypto/Kconfig                        |  29 +
>>>  drivers/crypto/rockchip/Makefile              |   5 +
>>>  drivers/crypto/rockchip/rk2_crypto.c          | 739 ++++++++++++++++++
>>>  drivers/crypto/rockchip/rk2_crypto.h          | 246 ++++++
>>>  drivers/crypto/rockchip/rk2_crypto_ahash.c    | 344 ++++++++
>>>  drivers/crypto/rockchip/rk2_crypto_skcipher.c | 576 ++++++++++++++
>>>  6 files changed, 1939 insertions(+)
>>>  create mode 100644 drivers/crypto/rockchip/rk2_crypto.c
>>>  create mode 100644 drivers/crypto/rockchip/rk2_crypto.h
>>>  create mode 100644 drivers/crypto/rockchip/rk2_crypto_ahash.c
>>>  create mode 100644 drivers/crypto/rockchip/rk2_crypto_skcipher.c
>>>
>>> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
>>> index 79c3bb9c99c3..b6a2027b1f9a 100644
>>> --- a/drivers/crypto/Kconfig
>>> +++ b/drivers/crypto/Kconfig
>>> @@ -660,6 +660,35 @@ config CRYPTO_DEV_ROCKCHIP_DEBUG
>>>  	  the number of requests per algorithm and other internal stats.
>>>  
>>>  
>>> +config CRYPTO_DEV_ROCKCHIP2
>>> +	tristate "Rockchip's cryptographic offloader V2"
>>> +	depends on OF && ARCH_ROCKCHIP
>>> +	depends on PM
>>> +	select CRYPTO_ECB
>>> +	select CRYPTO_CBC
>>> +	select CRYPTO_AES
>>> +	select CRYPTO_MD5
>>> +	select CRYPTO_SHA1
>>> +	select CRYPTO_SHA256
>>> +	select CRYPTO_SHA512
>>> +	select CRYPTO_SM3_GENERIC
>>> +	select CRYPTO_HASH
>>> +	select CRYPTO_SKCIPHER
>>> +	select CRYPTO_ENGINE
>>> +
>>> +	help
>>> +	  This driver interfaces with the hardware crypto offloader present
>>> +	  on RK3566, RK3568 and RK3588.
>>> +
>>> +config CRYPTO_DEV_ROCKCHIP2_DEBUG
>>> +	bool "Enable Rockchip V2 crypto stats"
>>> +	depends on CRYPTO_DEV_ROCKCHIP2
>>> +	depends on DEBUG_FS
>>> +	help
>>> +	  Say y to enable Rockchip crypto debug stats.
>>> +	  This will create /sys/kernel/debug/rk3588_crypto/stats for displaying
>>> +	  the number of requests per algorithm and other internal stats.
>>> +
>>>  config CRYPTO_DEV_ZYNQMP_AES
>>>  	tristate "Support for Xilinx ZynqMP AES hw accelerator"
>>>  	depends on ZYNQMP_FIRMWARE || COMPILE_TEST
>>> diff --git a/drivers/crypto/rockchip/Makefile b/drivers/crypto/rockchip/Makefile
>>> index 785277aca71e..452a12ff6538 100644
>>> --- a/drivers/crypto/rockchip/Makefile
>>> +++ b/drivers/crypto/rockchip/Makefile
>>> @@ -3,3 +3,8 @@ obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP) += rk_crypto.o
>>>  rk_crypto-objs := rk3288_crypto.o \
>>>  		  rk3288_crypto_skcipher.o \
>>>  		  rk3288_crypto_ahash.o
>>> +
>>> +obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP2) += rk_crypto2.o
>>> +rk_crypto2-objs := rk2_crypto.o \
>>> +		  rk2_crypto_skcipher.o \
>>> +		  rk2_crypto_ahash.o
>>> diff --git a/drivers/crypto/rockchip/rk2_crypto.c b/drivers/crypto/rockchip/rk2_crypto.c
>>> new file mode 100644
>>> index 000000000000..f3b8d27924da
>>> --- /dev/null
>>> +++ b/drivers/crypto/rockchip/rk2_crypto.c
>>> @@ -0,0 +1,739 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * hardware cryptographic offloader for RK3568/RK3588 SoC
>>> + *
>>> + * Copyright (c) 2022-2023, Corentin Labbe <clabbe@baylibre.com>
>>> + */
>>> +
>>> +#include "rk2_crypto.h"
>>> +#include <linux/clk.h>
>>> +#include <linux/crypto.h>
>>> +#include <linux/dma-mapping.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/reset.h>
>>> +
>>> +static struct rockchip_ip rocklist = {
>>> +	.dev_list = LIST_HEAD_INIT(rocklist.dev_list),
>>> +	.lock = __SPIN_LOCK_UNLOCKED(rocklist.lock),
>>
>> Drop it, not needed.
> 
> To clarify: I mean entire structure.

... and I think I was wrong - skcipher_engine_alg and other parts still
do not handle device context, so indeed you might need global list.

Best regards,
Krzysztof