Re: [PATCH v2 1/3] dt-bindings: rng: add MediaTek MT7622 Hardware Random Generator bindings

[Matthias Brugger]

On 12/06/17 17:56, sean.w...@mediatek.com wrote:

From: Sean Wang <sean.w...@mediatek.com>

Document the bindings used by MediaTek MT7622 SoC hardware random number
generator.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
  Documentation/devicetree/bindings/rng/mtk-rng.txt | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)



Reviewed-by: Matthias Brugger <matthias@gmail.com>


diff --git a/Documentation/devicetree/bindings/rng/mtk-rng.txt 
b/Documentation/devicetree/bindings/rng/mtk-rng.txt
index a6d62a2..366b99b 100644
--- a/Documentation/devicetree/bindings/rng/mtk-rng.txt
+++ b/Documentation/devicetree/bindings/rng/mtk-rng.txt
@@ -2,7 +2,9 @@ Device-Tree bindings for Mediatek random number generator
  found in Mediatek SoC family
  
  Required properties:

-- compatible   : Should be "mediatek,mt7623-rng"
+- compatible   : Should be
+   "mediatek,mt7622-rng","mediatek,mt7623-rng" : 
for MT7622
+   "mediatek,mt7623-rng" : for MT7623
  - clocks  : list of clock specifiers, corresponding to
  entries in clock-names property;
  - clock-names : Should contain "rng" entries;

Re: [PATCH 1/4] dt-bindings: rng: add generic bindings for MediaTek SoCs

[Matthias Brugger]

On 07/06/17 15:20, Sean Wang wrote:

On Tue, 2017-06-06 at 13:07 +0200, Matthias Brugger wrote:


On 31/05/17 20:44, sean.w...@mediatek.com wrote:

From: Sean Wang <sean.w...@mediatek.com>

Add the generic binding for allowing the support of RNG on MediaTek SoCs
such as MT7622.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
   Documentation/devicetree/bindings/rng/mtk-rng.txt | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/rng/mtk-rng.txt 
b/Documentation/devicetree/bindings/rng/mtk-rng.txt
index a6d62a2..0772913 100644
--- a/Documentation/devicetree/bindings/rng/mtk-rng.txt
+++ b/Documentation/devicetree/bindings/rng/mtk-rng.txt
@@ -2,7 +2,8 @@ Device-Tree bindings for Mediatek random number generator
   found in Mediatek SoC family
   
   Required properties:

-- compatible   : Should be "mediatek,mt7623-rng"
+- compatible   : Should be "mediatek,generic-rng" or
+   "mediatek,mt7623-rng".


What does generic-rng mean. Is it for all mt7xxx, or also for mt6xxx and
mt8xxx based SoCs? I think we should stick with SoC specific bindings,
as we don't know if Mediatek won't publish a new IP block next year
which is differnet.



Yes, what I mean is generic-rng can be applied to all
platform MediaTek provides.



Just in case we should add a binding for the actual SoC + a fallback.
For example.
- compatible " Should be
"mediatek,mt7622-rng","mediatek,mt7623-rng" for SoC mt7622
"mediatek,mt7623-rng" for SoC mt7623

This will also eliminate the need of adding mt6722-rng to the driver, as
it will use mt7623-rng as fallback. If in the future we realize that
mt7622-rng has a extra feature/bug, we can still work around it, without
breaking the bindings.



I knew the fallback rules you said here because I saw them being used in
many drivers such as sysirq and uart driver, such kind of basic drivers.

These drivers are basic enough, various following chipsets almost fall
back into the oldest one. So the clues let me think the hardware
interface shouldn't have too much differences among them.

If there is string used like generic-uart or generic-sysirq, it can
stop we blindly add new string into the binding document when a new
platform is introduced.

And they easily allows users unfamiliar MediaTek platform (they didn't
know what the oldest MediaTek chipset is) pick up the right compatible
string to start bring up the new platform.

The specific one can be added after new feature required is added or
critical hardware bug is found. Otherwise the generic one can fit
all generic needs for those.

Those are only opinions, if you don't like it, I still can accept the
original way as you suggest :)



I can see your reasoning, but the device tree maintainers prefer to have 
the bindings updated for a new SoC. As I mentioned before, just imagine 
next year Mediatek changes the IP block and from now on, it uses the new 
device in all SoCs. In 5 years we would have a binding which states 
'generic' although it is not compatible with any SoC of the last So


So please keep with the bindings as done up to now.

Best regards,
Matthias




Makes sense?

Regards,
Matthias


   - clocks : list of clock specifiers, corresponding to
  entries in clock-names property;
   - clock-names: Should contain "rng" entries;

Re: [PATCH 1/4] dt-bindings: rng: add generic bindings for MediaTek SoCs

[Matthias Brugger]

On 31/05/17 20:44, sean.w...@mediatek.com wrote:

From: Sean Wang 

Add the generic binding for allowing the support of RNG on MediaTek SoCs
such as MT7622.

Signed-off-by: Sean Wang 
---
  Documentation/devicetree/bindings/rng/mtk-rng.txt | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/rng/mtk-rng.txt 
b/Documentation/devicetree/bindings/rng/mtk-rng.txt
index a6d62a2..0772913 100644
--- a/Documentation/devicetree/bindings/rng/mtk-rng.txt
+++ b/Documentation/devicetree/bindings/rng/mtk-rng.txt
@@ -2,7 +2,8 @@ Device-Tree bindings for Mediatek random number generator
  found in Mediatek SoC family
  
  Required properties:

-- compatible   : Should be "mediatek,mt7623-rng"
+- compatible   : Should be "mediatek,generic-rng" or
+   "mediatek,mt7623-rng".


What does generic-rng mean. Is it for all mt7xxx, or also for mt6xxx and 
mt8xxx based SoCs? I think we should stick with SoC specific bindings, 
as we don't know if Mediatek won't publish a new IP block next year 
which is differnet.


Just in case we should add a binding for the actual SoC + a fallback. 
For example.

- compatible " Should be
"mediatek,mt7622-rng","mediatek,mt7623-rng" for SoC mt7622
"mediatek,mt7623-rng" for SoC mt7623

This will also eliminate the need of adding mt6722-rng to the driver, as 
it will use mt7623-rng as fallback. If in the future we realize that 
mt7622-rng has a extra feature/bug, we can still work around it, without 
breaking the bindings.


Makes sense?

Regards,
Matthias


  - clocks  : list of clock specifiers, corresponding to
  entries in clock-names property;
  - clock-names : Should contain "rng" entries;

Re: [PATCH 2/2] crypto: mediatek - update DT binding documentation

[Matthias Brugger]

On 26/05/17 11:43, Ryder Lee wrote:

This patch removes unnecessary clock in binding file.

Signed-off-by: Ryder Lee <ryder@mediatek.com>
---


In the driver clocks are get by name, so this change does not break 
backwards compatibility.


Reviewed-by: Matthias Brugger <matthias@gmail.com>


  Documentation/devicetree/bindings/crypto/mediatek-crypto.txt | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt 
b/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
index c204725..450da36 100644
--- a/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
+++ b/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
@@ -6,8 +6,7 @@ Required properties:
  - 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"
+- clock-names: Must contain "cryp".
  - power-domains: Must contain a reference to the PM domain.
  
  
@@ -20,8 +19,7 @@ Example:

 ,
 ,
 ;
-   clocks = < CLK_TOP_ETHIF_SEL>,
-< CLK_ETHSYS_CRYPTO>;
-   clock-names = "ethif","cryp";
+   clocks = < CLK_ETHSYS_CRYPTO>;
+   clock-names = "cryp";
power-domains = < MT2701_POWER_DOMAIN_ETH>;
};

Re: [PATCH v2 8/9] crypto: mediatek - Use IPAD/OPAD constant

[Matthias Brugger]

On 19/05/17 08:53, Corentin Labbe wrote:

This patch simply replace all occurrence of HMAC IPAD/OPAD value by their
define.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>
---


Reviewed-by: Matthias Brugger <matthias@gmail.com>


  drivers/crypto/mediatek/mtk-sha.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/mediatek/mtk-sha.c 
b/drivers/crypto/mediatek/mtk-sha.c
index 2226f12d1c7a..5f4f845adbb8 100644
--- a/drivers/crypto/mediatek/mtk-sha.c
+++ b/drivers/crypto/mediatek/mtk-sha.c
@@ -12,6 +12,7 @@
   * Some ideas are from atmel-sha.c and omap-sham.c drivers.
   */
  
+#include 

  #include 
  #include "mtk-platform.h"
  
@@ -825,8 +826,8 @@ static int mtk_sha_setkey(struct crypto_ahash *tfm, const u8 *key,

memcpy(bctx->opad, bctx->ipad, bs);
  
  	for (i = 0; i < bs; i++) {

-   bctx->ipad[i] ^= 0x36;
-   bctx->opad[i] ^= 0x5c;
+   bctx->ipad[i] ^= HMAC_IPAD_VALUE;
+   bctx->opad[i] ^= HMAC_OPAD_VALUE;
}
  
  	return 0;

Re: [PATCH] crypto: arm64/crc32 - merge CRC32 and PMULL instruction based drivers

[Matthias Brugger]

On 02/01/2017 04:35 PM, Ard Biesheuvel wrote:

The PMULL based CRC32 implementation already contains code based on the
separate, optional CRC32 instructions to fallback to when operating on
small quantities of data. We can expose these routines directly on systems
that lack the 64x64 PMULL instructions but do implement the CRC32 ones,
which makes the driver that is based solely on those CRC32 instructions
redundant. So remove it.

Note that this aligns arm64 with ARM, whose accelerated CRC32 driver
also combines the CRC32 extension based and the PMULL based versions.

Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
---

This is a meaningful patch by itself imho, but also fixes the issue reported
by Matthias where their v4.8 based GCC does not understand the -mcpu=generic+crc
command line option, resulting in failed builds.



This patch fixes the issue, thanks.
Tested-by: Matthias Brugger <mbrug...@suse.com>


 arch/arm64/configs/defconfig  |   1 -
 arch/arm64/crypto/Kconfig |   9 +-
 arch/arm64/crypto/Makefile|   4 -
 arch/arm64/crypto/crc32-arm64.c   | 290 
 arch/arm64/crypto/crc32-ce-glue.c |  49 +++-
 5 files changed, 41 insertions(+), 312 deletions(-)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 33b744d54739..6fc6f5a2a6e5 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -516,4 +516,3 @@ CONFIG_CRYPTO_GHASH_ARM64_CE=y
 CONFIG_CRYPTO_AES_ARM64_CE_CCM=y
 CONFIG_CRYPTO_AES_ARM64_CE_BLK=y
 # CONFIG_CRYPTO_AES_ARM64_NEON_BLK is not set
-CONFIG_CRYPTO_CRC32_ARM64=y
diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
index bed7feddfeed..d92293747d63 100644
--- a/arch/arm64/crypto/Kconfig
+++ b/arch/arm64/crypto/Kconfig
@@ -37,8 +37,8 @@ config CRYPTO_CRCT10DIF_ARM64_CE
select CRYPTO_HASH

 config CRYPTO_CRC32_ARM64_CE
-   tristate "CRC32 and CRC32C digest algorithms using PMULL instructions"
-   depends on KERNEL_MODE_NEON && CRC32
+   tristate "CRC32 and CRC32C digest algorithms using ARMv8 extensions"
+   depends on CRC32
select CRYPTO_HASH

 config CRYPTO_AES_ARM64
@@ -71,11 +71,6 @@ config CRYPTO_AES_ARM64_NEON_BLK
select CRYPTO_AES
select CRYPTO_SIMD

-config CRYPTO_CRC32_ARM64
-   tristate "CRC32 and CRC32C using optional ARMv8 instructions"
-   depends on ARM64
-   select CRYPTO_HASH
-
 config CRYPTO_CHACHA20_NEON
tristate "NEON accelerated ChaCha20 symmetric cipher"
depends on KERNEL_MODE_NEON
diff --git a/arch/arm64/crypto/Makefile b/arch/arm64/crypto/Makefile
index d1ae1b9cbe70..b5edc5918c28 100644
--- a/arch/arm64/crypto/Makefile
+++ b/arch/arm64/crypto/Makefile
@@ -55,10 +55,6 @@ AFLAGS_aes-neon.o:= -DINTERLEAVE=4

 CFLAGS_aes-glue-ce.o   := -DUSE_V8_CRYPTO_EXTENSIONS

-obj-$(CONFIG_CRYPTO_CRC32_ARM64) += crc32-arm64.o
-
-CFLAGS_crc32-arm64.o   := -mcpu=generic+crc
-
 $(obj)/aes-glue-%.o: $(src)/aes-glue.c FORCE
$(call if_changed_rule,cc_o_c)

diff --git a/arch/arm64/crypto/crc32-arm64.c b/arch/arm64/crypto/crc32-arm64.c
deleted file mode 100644
index 6a37c3c6b11d..
--- a/arch/arm64/crypto/crc32-arm64.c
+++ /dev/null
@@ -1,290 +0,0 @@
-/*
- * crc32-arm64.c - CRC32 and CRC32C using optional ARMv8 instructions
- *
- * Module based on crypto/crc32c_generic.c
- *
- * CRC32 loop taken from Ed Nevill's Hadoop CRC patch
- * 
http://mail-archives.apache.org/mod_mbox/hadoop-common-dev/201406.mbox/%3C1403687030.3355.19.camel%40localhost.localdomain%3E
- *
- * Using inline assembly instead of intrinsics in order to be backwards
- * compatible with older compilers.
- *
- * Copyright (C) 2014 Linaro Ltd <yazen.ghan...@linaro.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-
-MODULE_AUTHOR("Yazen Ghannam <yazen.ghan...@linaro.org>");
-MODULE_DESCRIPTION("CRC32 and CRC32C using optional ARMv8 instructions");
-MODULE_LICENSE("GPL v2");
-
-#define CRC32X(crc, value) __asm__("crc32x %w[c], %w[c], 
%x[v]":[c]"+r"(crc):[v]"r"(value))
-#define CRC32W(crc, value) __asm__("crc32w %w[c], %w[c], 
%w[v]":[c]"+r"(crc):[v]"r"(value))
-#define CRC32H(crc, value) __asm__("crc32h %w[c], %w[c], 
%w[v]":[c]"+r"(crc):[v]"r"(value))
-#define CRC32B(crc, value) __asm__("crc32b %w[c], %w[c], 
%w[v]":[c]"+r"(crc):[v]"r"(value))
-#define CRC32CX(crc, value) __asm__("crc32cx %w[c], %w[c], 
%x[v]":[c]"+r"(crc):[v]"r"(value))
-#define CRC32CW(crc, value) __asm__("crc32cw %w[c], %w[c], 
%w[v]":[c]"+r"(crc):[

Re: [PATCH v1 2/2] crypto: mediatek - add DT bindings documentation

[Matthias Brugger]

On 05/12/16 08:01, Ryder Lee wrote:

Add DT bindings documentation for the crypto driver

Signed-off-by: Ryder Lee 
---
 .../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 000..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"


Do you know how big the difference is between the crypto engine for 
mt7623/mt2701/mt8521p in comparison, let's say mt8173 or mt6797?
Do this SoCs have a crypot engine? If so and they are quite similar, we 
might think of adding a mtk-crypto binding and add soc specific bindings.


Regards,
Matthias


+- 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@1b24 {
+   compatible = "mediatek,mt7623-crypto";
+   reg = <0 0x1b24 0 0x2>;
+   interrupts = ,
+,
+,
+,
+;
+   clocks = < CLK_TOP_ETHIF_SEL>,
+< CLK_ETHSYS_CRYPTO>;
+   clock-names = "ethif","cryp";
+   power-domains = < MT2701_POWER_DOMAIN_ETH>;
+   };


--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html