Re: [PATCH 1/2] Add crypto driver support for some MediaTek chips
From: Corentin Labbe <hidden>
Date: 2016-12-02 08:18:42
Also in:
linux-arm-kernel, linux-crypto, linux-mediatek, lkml
Hello I have some minor comment inline On Fri, Dec 02, 2016 at 11:26:44AM +0800, Ryder Lee wrote:
This adds support for the MediaTek hardware accelerator on mt7623/mt2701/mt8521p SoC. This driver currently implement: - SHA1 and SHA2 family(HMAC) hash alogrithms. - AES block cipher in CBC/ECB mode with 128/196/256 bits keys.
I see also a PRNG but is seems not really used.
quoted hunk ↗ jump to hunk
Signed-off-by: Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> --- drivers/crypto/Kconfig | 17 + drivers/crypto/Makefile | 1 + drivers/crypto/mediatek/Makefile | 2 + drivers/crypto/mediatek/mtk-aes.c | 734 +++++++++++++++++ drivers/crypto/mediatek/mtk-platform.c | 575 +++++++++++++ drivers/crypto/mediatek/mtk-platform.h | 230 ++++++ drivers/crypto/mediatek/mtk-regs.h | 194 +++++ drivers/crypto/mediatek/mtk-sha.c | 1384 ++++++++++++++++++++++++++++++++ 8 files changed, 3137 insertions(+) create mode 100644 drivers/crypto/mediatek/Makefile create mode 100644 drivers/crypto/mediatek/mtk-aes.c create mode 100644 drivers/crypto/mediatek/mtk-platform.c create mode 100644 drivers/crypto/mediatek/mtk-platform.h create mode 100644 drivers/crypto/mediatek/mtk-regs.h create mode 100644 drivers/crypto/mediatek/mtk-sha.cdiff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 4d2b81f..5d9c803 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig@@ -553,6 +553,23 @@ config CRYPTO_DEV_ROCKCHIP This driver interfaces with the hardware crypto accelerator. Supporting cbc/ecb chainmode, and aes/des/des3_ede cipher mode. +config CRYPTO_DEV_MEDIATEK + tristate "MediaTek's Cryptographic Engine driver" + depends on ARM && ARCH_MEDIATEK + select NEON + select KERNEL_MODE_NEON + select ARM_CRYPTO + select CRYPTO_AES + select CRYPTO_BLKCIPHER + select CRYPTO_SHA1_ARM_NEON + select CRYPTO_SHA256_ARM + select CRYPTO_SHA512_ARM + select CRYPTO_HMAC
Why do you select accelerated algos ? Adding COMPILE_TEST could be helpfull also. [...]
+ +#include <linux/dma-mapping.h> +#include <linux/scatterlist.h> +#include <crypto/scatterwalk.h> +#include <crypto/algapi.h> +#include <crypto/aes.h> +#include "mtk-platform.h" +#include "mtk-regs.h" +
Sort headers in alphabetical order [...]
+ + mtk_aes_unregister_algs(); + mtk_aes_record_free(cryp); +} +EXPORT_SYMBOL(mtk_cipher_alg_release);
Why not EXPORT_SYMBOL_GPL ? Furthermore do you really need it to be exported ? [...]
+ +#include <linux/init.h> +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/mfd/syscon.h> +#include <linux/pm_runtime.h> +#include <linux/clk.h> +#include <linux/platform_device.h> +#include "mtk-platform.h" +#include "mtk-regs.h" +
Sort headers in alphabetical order [...]
+
+static void mtk_prng_reseed(struct mtk_cryp *cryp)
+{
+ /* 8 words to seed the PRNG to provide IVs */
+ void __iomem *base = cryp->base;
+ const u32 prng_key[8] = {0x48c24cfd, 0x6c07f742,
+ 0xaee75681, 0x0f27c239,
+ 0x79947198, 0xe2991275,
+ 0x21ac3c7c, 0xd008c4b4};Why do you seed with thoses constant ? [...]
+
+static int mtk_accelerator_init(struct mtk_cryp *cryp)
+{
+ int i, err;
+
+ /* Initialize advanced interrupt controller(AIC) */
+ for (i = 0; i < 5; i++) {I see this 5 for interrupt away, so perhaps a define could be used [...] here
+ for (i = 0; i < 5; i++) {
+ cryp->irq[i] = platform_get_irq(pdev, i);
+ if (cryp->irq[i] < 0) {
+ dev_err(cryp->dev, "no IRQ:%d resource info\n", i);
+ return -ENXIO;
+ }
+ }[...]
+#ifndef __MTK_PLATFORM_H_ +#define __MTK_PLATFORM_H_ + +#include <linux/crypto.h> +#include <crypto/internal/hash.h> +#include <linux/interrupt.h>
Sort headers in alphabetical order [...]
+#define MTK_DESC_FIRST BIT(23) +#define MTK_DESC_BUF_LEN(x) ((x) & 0x1ffff) +#define MTK_DESC_CT_LEN(x) (((x) & 0xff) << 24) + +#define WORD(x) ((x) >> 2)
dangerous and ambigous define [...]
+ +#include <linux/dma-mapping.h> +#include <linux/scatterlist.h> +#include <linux/crypto.h> +#include <crypto/scatterwalk.h> +#include <crypto/algapi.h> +#include <crypto/sha.h> +#include <crypto/internal/hash.h>
Sort headers in alphabetical order [...] Generally more function comment could be helpfull. Regards -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html