Hello
I have some minor comments below.
On Fri, Dec 11, 2015 at 09:58:23AM +0800, Zain Wang wrote:
> Add md5 sha1 sha256 support for crypto engine in rk3288.
> This patch can't support multiple updatings because of limited of IC,
> as result, it can't support import and export too.
>
> Signed-off-by: Zain Wang <zain.wang at rock-chips.com>
> ---
>
> Changes in V3:
> - add switch instead of multiple if.
>
> Changes in V2:
> - add some comments to code.
> - fix some issues about zero message process.
>
> drivers/crypto/rockchip/Makefile | 1 +
> drivers/crypto/rockchip/rk3288_crypto.c | 33 +-
> drivers/crypto/rockchip/rk3288_crypto.h | 50 ++-
> drivers/crypto/rockchip/rk3288_crypto_ablkcipher.c | 20 +-
> drivers/crypto/rockchip/rk3288_crypto_ahash.c | 394 +++++++++++++++++++++
> 5 files changed, 480 insertions(+), 18 deletions(-)
> create mode 100644 drivers/crypto/rockchip/rk3288_crypto_ahash.c
>
> diff --git a/drivers/crypto/rockchip/rk3288_crypto.h b/drivers/crypto/rockchip/rk3288_crypto.h
> index 604ffe7..01d8299 100644
> --- a/drivers/crypto/rockchip/rk3288_crypto.h
> +++ b/drivers/crypto/rockchip/rk3288_crypto.h
> @@ -6,6 +6,10 @@
> #include <crypto/algapi.h>
> #include <linux/interrupt.h>
> #include <linux/delay.h>
> +#include <crypto/internal/hash.h>
> +
> +#include "crypto/md5.h"
> +#include "crypto/sha.h"
It is proper to include with <> not ""
> diff --git a/drivers/crypto/rockchip/rk3288_crypto_ahash.c b/drivers/crypto/rockchip/rk3288_crypto_ahash.c
> new file mode 100644
> index 0000000..a5795f6
> --- /dev/null
> +++ b/drivers/crypto/rockchip/rk3288_crypto_ahash.c
> @@ -0,0 +1,394 @@
> +/*
> + * Crypto acceleration support for Rockchip RK3288
> + *
> + * Copyright (c) 2015, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * Author: Zain Wang <zain.wang at rock-chips.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * Some ideas are from marvell/cesa.c and s5p-sss.c driver.
> + */
> +#include "rk3288_crypto.h"
> +
> +/*
> + * IC can not process zero message hash,
> + * so we put the fixed hash out when met zero message.
> + */
> +const static u8 *sha1_zero_message_hash = {
> + "\xda\x39\xa3\xee\x5e\x6b\x4b\x0d\x32\x55"
> + "\xbf\xef\x95\x60\x18\x90\xaf\xd8\x07\x09"
> +};
> +
> +const static u8 *sha256_zero_message_hash = {
> + "\xe3\xb0\xc4\x42\x98\xfc\x1c\x14"
> + "\x9a\xfb\xf4\xc8\x99\x6f\xb9\x24"
> + "\x27\xae\x41\xe4\x64\x9b\x93\x4c"
> + "\xa4\x95\x99\x1b\x78\x52\xb8\x55"
> +};
> +
> +const static u8 *md5_zero_message_hash = {
> + "\xd4\x1d\x8c\xd9\x8f\x00\xb2\x04"
> + "\xe9\x80\x09\x98\xec\xf8\x42\x7e"
> +};
> +
> +
> +static int zero_message_process(struct ahash_request *req)
> +{
> + struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> +
> + switch (crypto_ahash_digestsize(tfm)) {
> + case SHA1_DIGEST_SIZE:
> + memcpy(req->result, sha1_zero_message_hash, SHA1_DIGEST_SIZE);
> + break;
> + case SHA256_DIGEST_SIZE:
> + memcpy(req->result, sha256_zero_message_hash,
> + SHA256_DIGEST_SIZE);
> + break;
> + case MD5_DIGEST_SIZE:
> + memcpy(req->result, md5_zero_message_hash, MD5_DIGEST_SIZE);
> + break;
> + default:
> + return -EINVAL;
> + break;
> + }
You could store crypto_ahash_digestsize result and use it in memcpy.
You could also store the name of xxx_zero_message_hash in crypto_alg like n2 do.
Perhaps a combination of the two could produce more generic code.
> +struct rk_crypto_tmp rk_ahash_sha256 = {
> + .type = ALG_TYPE_HASH,
> + .alg.hash = {
> + .init = rk_ahash_init,
> + .update = rk_ahash_update,
> + .final = rk_ahash_final,
> + .finup = rk_ahash_finup,
> + .digest = rk_ahash_digest,
> + .halg = {
> + .digestsize = SHA256_DIGEST_SIZE,
> + .statesize = sizeof(struct sha256_state),
> + .base = {
> + .cra_name = "sha256",
> + .cra_driver_name = "rk-sha256",
> + .cra_priority = 300,
> + .cra_flags = CRYPTO_ALG_ASYNC |
> + CRYPTO_ALG_NEED_FALLBACK,
> + .cra_blocksize = SHA256_BLOCK_SIZE,
> + .cra_ctxsize = sizeof(struct rk_ahash_ctx),
> + .cra_alignmask = 0,
> + .cra_init = rk_cra_hash_init,
> + .cra_exit = rk_cra_hash_exit,
> + .cra_module = THIS_MODULE,
> + }
> + }
> + }
> +};
> +
> +struct rk_crypto_tmp rk_ahash_md5 = {
> + .type = ALG_TYPE_HASH,
> + .alg.hash = {
> + .init = rk_ahash_init,
> + .update = rk_ahash_update,
> + .final = rk_ahash_final,
> + .finup = rk_ahash_finup,
> + .digest = rk_ahash_digest,
> + .halg = {
> + .digestsize = MD5_DIGEST_SIZE,
> + .statesize = sizeof(struct md5_state),
> + .base = {
> + .cra_name = "md5",
> + .cra_driver_name = "rk-md5",
> + .cra_priority = 300,
> + .cra_flags = CRYPTO_ALG_ASYNC |
> + CRYPTO_ALG_NEED_FALLBACK,
You said that the driver need a fallback, but I do not see any use of it.
Regards