Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1534646 > unrolled thread

[PATCH 0/2] Add MediaTek crypto acclelrator driver

Started byRyder Lee <ryder.lee@mediatek.com>
First post2016-12-02 04:30 +0100
Last post2016-12-02 13:20 +0100
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] Add MediaTek crypto acclelrator driver Ryder Lee <ryder.lee@mediatek.com> - 2016-12-02 04:30 +0100
    [PATCH 2/2] crypto: mediatek - add DT bindings documentation Ryder Lee <ryder.lee@mediatek.com> - 2016-12-02 04:30 +0100
    Re: [PATCH 1/2] Add crypto driver support for some MediaTek chips Corentin Labbe <clabbe.montjoie@gmail.com> - 2016-12-02 09:20 +0100
      Re: [PATCH 1/2] Add crypto driver support for some MediaTek chips mtk09577 <ryder.lee@mediatek.com> - 2016-12-02 12:50 +0100
      Re: [PATCH 1/2] Add crypto driver support for some MediaTek chips Ryder Lee <ryder.lee@mediatek.com> - 2016-12-02 13:20 +0100

#1534646 — [PATCH 0/2] Add MediaTek crypto acclelrator driver

FromRyder Lee <ryder.lee@mediatek.com>
Date2016-12-02 04:30 +0100
Subject[PATCH 0/2] Add MediaTek crypto acclelrator driver
Message-ID<sJMHT-6s9-3@gated-at.bofh.it>
This adds support for the MediaTek hardware accelerator on 
mt7623 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.

Ryder Lee (2):
  Add crypto driver support for some MediaTek chips
  crypto: mediatek - add DT bindings documentation

 .../devicetree/bindings/crypto/mediatek-crypto.txt |   32 +
 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 ++++++++++++++++++++
 9 files changed, 3169 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
 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.c

-- 
1.9.1

[toc] | [next] | [standalone]

#1534647 — [PATCH 2/2] crypto: mediatek - add DT bindings documentation

FromRyder Lee <ryder.lee@mediatek.com>
Date2016-12-02 04:30 +0100
Subject[PATCH 2/2] crypto: mediatek - add DT bindings documentation
Message-ID<sJMHT-6s9-5@gated-at.bofh.it>
In reply to#1534646
Add DT bindings documentation for the crypto driver

Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
---
 .../devicetree/bindings/crypto/mediatek-crypto.txt | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/mediatek-crypto.txt

diff --git a/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt b/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
new file mode 100644
index 0000000..8b1db08
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
@@ -0,0 +1,32 @@
+MediaTek cryptographic accelerators
+
+Required properties:
+- compatible: Should be "mediatek,mt7623-crypto"
+- reg: Address and length of the register set for the device
+- interrupts: Should contain the five crypto engines interrupts in numeric
+	order. These are global system and four descriptor rings.
+- clocks: the clock used by the core
+- clock-names: the names of the clock listed in the clocks property. These are
+	"ethif", "cryp"
+- power-domains: Must contain a reference to the PM domain.
+
+
+Optional properties:
+- interrupt-parent: Should be the phandle for the interrupt controller
+  that services interrupts for this device
+
+
+Example:
+	crypto: crypto@1b240000 {
+		compatible = "mediatek,mt7623-crypto";
+		reg = <0 0x1b240000 0 0x20000>;
+		interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 83 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 84 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 97 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&topckgen CLK_TOP_ETHIF_SEL>,
+			 <&ethsys CLK_ETHSYS_CRYPTO>;
+		clock-names = "ethif","cryp";
+		power-domains = <&scpsys MT2701_POWER_DOMAIN_ETH>;
+	};
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1534726 — Re: [PATCH 1/2] Add crypto driver support for some MediaTek chips

FromCorentin Labbe <clabbe.montjoie@gmail.com>
Date2016-12-02 09:20 +0100
SubjectRe: [PATCH 1/2] Add crypto driver support for some MediaTek chips
Message-ID<sJRex-16S-3@gated-at.bofh.it>
In reply to#1534646
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.

> 
> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
>  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.c
> 
> diff --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

[toc] | [prev] | [next] | [standalone]

#1534868 — Re: [PATCH 1/2] Add crypto driver support for some MediaTek chips

Frommtk09577 <ryder.lee@mediatek.com>
Date2016-12-02 12:50 +0100
SubjectRe: [PATCH 1/2] Add crypto driver support for some MediaTek chips
Message-ID<sJUvM-3hJ-27@gated-at.bofh.it>
In reply to#1534726
Hello,

On Fri, 2016-12-02 at 09:18 +0100, Corentin Labbe wrote:
> 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.

Yes, PRNG is not implemented yet, i will remove it temporarily.

> > 
> > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> > ---
> >  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.c
> > 
> > diff --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.

Our Hardware has complex procedure on calculate HMAC, and it get a bad
performance.... So i decide to use ARM NEON instruction as fallback to
speedup it.
I will add COMPILE_TEST.

> [...]
> > +
> > +#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 ?

My mistake. I will remove it.

> [...]
> > +
> > +#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

I will define a IRQ_NUM and modify ambiguous definition.

> [...]
> > +
> > +#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.

I will sort all header and add more function comment.
Thanks for your review.

> Regards

[toc] | [prev] | [next] | [standalone]

#1534880 — Re: [PATCH 1/2] Add crypto driver support for some MediaTek chips

FromRyder Lee <ryder.lee@mediatek.com>
Date2016-12-02 13:20 +0100
SubjectRe: [PATCH 1/2] Add crypto driver support for some MediaTek chips
Message-ID<sJUYN-3L6-17@gated-at.bofh.it>
In reply to#1534726
Hello,

On Fri, 2016-12-02 at 09:18 +0100, Corentin Labbe wrote:
> 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.

Yes, PRNG is not implemented yet, i will remove it temporarily.

> > 
> > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> > ---
> >  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.c
> > 
> > diff --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.

Our Hardware has complex procedure on calculate HMAC, and it get a bad
performance.... So i decide to use ARM NEON instruction as fallback to
speedup it.
I will add COMPILE_TEST.

> [...]
> > +
> > +#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 ?

My mistake. I will remove it.

> [...]
> > +
> > +#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

I will define a IRQ_NUM and modify ambiguous definition.

> [...]
> > +
> > +#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.

I will sort all header and add more function comment.
Thanks for your review.

> Regards

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web