Re: [PATCH] crypto: hisilicon/hpre - delete redundant log

[tanghui20]

On 2021/4/2 15:19, Herbert Xu wrote:

On Sat, Mar 27, 2021 at 04:32:26PM +0800, Hui Tang wrote:

'hpre_cfg_by_dsm' has checked and printed error path, so it is not
necessary at all.

Signed-off-by: Hui Tang 
---
 drivers/crypto/hisilicon/hpre/hpre_main.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/crypto/hisilicon/hpre/hpre_main.c 
b/drivers/crypto/hisilicon/hpre/hpre_main.c
index 8aae921..73ee997 100644
--- a/drivers/crypto/hisilicon/hpre/hpre_main.c
+++ b/drivers/crypto/hisilicon/hpre/hpre_main.c
@@ -384,10 +384,7 @@ static int hpre_set_user_domain_and_cache(struct hisi_qm 
*qm)

/* This setting is only needed by Kunpeng 920. */
if (qm->ver == QM_HW_V2) {
-   ret = hpre_cfg_by_dsm(qm);
-   if (ret)
-   dev_err(dev, "acpi_evaluate_dsm err.\n");
-
+   hpre_cfg_by_dsm(qm);
disable_flr_of_bme(qm);
}


If the return value is non-zero you've just changed what this code
does from returning non-zero to returning zero.  Are you sure about
this?


I am sorry, it should return error immediately when return value of 
'hpre_cfg_by_dsm' is non-zero,
and I will fix it in the next version.

Re: [PATCH] crypto: hisilicon - check if debugfs opened

[tanghui20]

On 2021/3/30 20:23, Greg KH wrote:

On Tue, Mar 30, 2021 at 08:09:46PM +0800, tanghui20 wrote:



On 2021/3/28 23:09, Greg KH wrote:

On Sat, Mar 27, 2021 at 04:33:00PM +0800, Hui Tang wrote:

'xx_debugfs_init' check if debugfs opened.

Signed-off-by: Hui Tang 
---
 drivers/crypto/hisilicon/hpre/hpre_main.c | 5 -
 drivers/crypto/hisilicon/qm.c | 3 +++
 drivers/crypto/hisilicon/sec2/sec_main.c  | 5 -
 drivers/crypto/hisilicon/zip/zip_main.c   | 3 +++
 4 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/hisilicon/hpre/hpre_main.c 
b/drivers/crypto/hisilicon/hpre/hpre_main.c
index c7ab06d..f2605c4 100644
--- a/drivers/crypto/hisilicon/hpre/hpre_main.c
+++ b/drivers/crypto/hisilicon/hpre/hpre_main.c
@@ -779,6 +779,9 @@ static int hpre_debugfs_init(struct hisi_qm *qm)
struct device *dev = >pdev->dev;
int ret;

+   if (!debugfs_initialized())
+   return -ENOENT;


Why?  What does this help with?  Why does the code care if debugfs is
running or not?


When !CONFIG_DEBUG_FS, there is no problem if debugfs is not checked,
but if checking debugfs, a series of stub functions of debugfs can be
skipped and 'xx_debugfs_init' will be return immediately.


And have you measured an actual speed difference for that?  I would be
amazed if you could...



I think what you said makes sense.
I am confused when to use 'debugfs_initialized'.

Thanks

Re: [PATCH] crypto: hisilicon - check if debugfs opened

[tanghui20]

On 2021/3/28 23:09, Greg KH wrote:

On Sat, Mar 27, 2021 at 04:33:00PM +0800, Hui Tang wrote:

'xx_debugfs_init' check if debugfs opened.

Signed-off-by: Hui Tang 
---
 drivers/crypto/hisilicon/hpre/hpre_main.c | 5 -
 drivers/crypto/hisilicon/qm.c | 3 +++
 drivers/crypto/hisilicon/sec2/sec_main.c  | 5 -
 drivers/crypto/hisilicon/zip/zip_main.c   | 3 +++
 4 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/hisilicon/hpre/hpre_main.c 
b/drivers/crypto/hisilicon/hpre/hpre_main.c
index c7ab06d..f2605c4 100644
--- a/drivers/crypto/hisilicon/hpre/hpre_main.c
+++ b/drivers/crypto/hisilicon/hpre/hpre_main.c
@@ -779,6 +779,9 @@ static int hpre_debugfs_init(struct hisi_qm *qm)
struct device *dev = >pdev->dev;
int ret;

+   if (!debugfs_initialized())
+   return -ENOENT;


Why?  What does this help with?  Why does the code care if debugfs is
running or not?


When !CONFIG_DEBUG_FS, there is no problem if debugfs is not checked,
but if checking debugfs, a series of stub functions of debugfs can be
skipped and 'xx_debugfs_init' will be return immediately.

Thanks.

Re: [PATCH -next] crypto: hisilicon/hpre: fix link error

[tanghui20]

There is a patch similar to yours that was send in advance, as follows:
https://www.spinics.net/lists/linux-crypto/msg54238.html

On 2021/3/23 15:27, Yang Yingliang wrote:

Fix the follow link error by select config CRYPTO_ECC and CRYPTO_ECDH.

ERROR: modpost: "ecc_get_curve25519" 
[drivers/crypto/hisilicon/hpre/hisi_hpre.ko] undefined!
ERROR: modpost: "ecc_get_curve" [drivers/crypto/hisilicon/hpre/hisi_hpre.ko] 
undefined!
ERROR: modpost: "crypto_ecdh_decode_key" 
[drivers/crypto/hisilicon/hpre/hisi_hpre.ko] undefined!

Fixes: 90274769cf79 ("crypto: hisilicon/hpre - add 'CURVE25519' algorithm")
Fixes: 05e7b906aa7c ("crypto: hisilicon/hpre - add 'ECDH' algorithm")
Signed-off-by: Yang Yingliang 
---
 drivers/crypto/hisilicon/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/hisilicon/Kconfig b/drivers/crypto/hisilicon/Kconfig
index c45adb15ce8d..d87c89af2a7f 100644
--- a/drivers/crypto/hisilicon/Kconfig
+++ b/drivers/crypto/hisilicon/Kconfig
@@ -69,6 +69,8 @@ config CRYPTO_DEV_HISI_HPRE
select CRYPTO_DEV_HISI_QM
select CRYPTO_DH
select CRYPTO_RSA
+   select CRYPTO_ECC
+   select CRYPTO_ECDH
help
  Support for HiSilicon HPRE(High Performance RSA Engine)
  accelerator, which can accelerate RSA and DH algorithms.