On Mon, 2016-07-11 at 11:28 -0700, Yeshaswi M R Gowda wrote:
> The Chelsio's Crypto Hardware can perform the following operations:
> SHA1, SHA224, SHA256, SHA384 and SHA512, HMAC(SHA1), HMAC(SHA224),
> HMAC(SHA256), HMAC(SHA384), HAMC(SHA512), AES-128-CBC, AES-192-CBC,
> AES-256-CBC, AES-128-XTS, AES-256-XTS
>
> This patch implements the driver for above mentioned features.
trivial notes:
> diff --git a/drivers/crypto/chelsio/chcr_algo.c
> b/drivers/crypto/chelsio/chcr_algo.c
[]
> +int chcr_handle_resp(struct crypto_async_request *req, unsigned char *input,
> + int error_status)
> +{
[]
> + case CRYPTO_ALG_TYPE_BLKCIPHER:
> + ctx_req.req.ablk_req = (struct ablkcipher_request *)req;
> + ctx_req.ctx.ablk_ctx =
> + ablkcipher_request_ctx(ctx_req.req.ablk_req);
> + if (error_status)
> + goto dma_unmap_blkcipher;
> + fw6_pld = (struct cpl_fw6_pld *)input;
> + memcpy(ctx_req.req.ablk_req->info, &fw6_pld->data[2],
> + AES_BLOCK_SIZE);
> +dma_unmap_blkcipher:
> + dma_unmap_sg(&u_ctx->lldi.pdev->dev, ctx_req.req.ablk_req->dst,
> + ABLK_CTX(ctx)->dst_nents, DMA_FROM_DEVICE);
> + if (ctx_req.ctx.ablk_ctx->skb) {
> + kfree_skb(ctx_req.ctx.ablk_ctx->skb);
> + ctx_req.ctx.ablk_ctx->skb = NULL;
> + }
> + break;
This case label is only used here right?
This would be better without the goto
[]
> + if (IS_ERR(base_hash)) {
> + pr_err("Can not allocate sha-generic algo.\n");
> + return (void *)base_hash;
> + }
Please add
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
before any #include to prefix any pr_<level> uses.
[]
> +/*
> + * chcr_register_alg - Register crypto algorithms with kernel framework.
> + */
> +static int chcr_register_alg(void)
> +{
> + struct crypto_alg ai;
> + int err = 0, i;
> + char *name = NULL;
> +
> + for (i = 0; i < ARRAY_SIZE(driver_algs); i++) {
> + if (driver_algs[i].is_registered)
> + continue;
> + switch (driver_algs[i].type & CRYPTO_ALG_TYPE_MASK) {
> + case CRYPTO_ALG_TYPE_ABLKCIPHER:
> + err = crypto_register_alg(&driver_algs[i].alg.crypto);
> + name = driver_algs[i].alg.crypto.cra_driver_name;
> + break;
> + case CRYPTO_ALG_TYPE_AHASH:
This could be clearer with a temporary for
driver_algs[i].alg.hash
<whatever type *> hash = &driver_algs[i].alg.hash;
> + driver_algs[i].alg.hash.update = chcr_ahash_update;
hash->update = chcr_ahash_update;
etc...
> + driver_algs[i].alg.hash.final = chcr_ahash_final;
> + driver_algs[i].alg.hash.finup = chcr_ahash_finup;
> + driver_algs[i].alg.hash.digest = chcr_ahash_digest;
> + driver_algs[i].alg.hash.export = chcr_ahash_export;
> + driver_algs[i].alg.hash.import = chcr_ahash_import;
> + driver_algs[i].alg.hash.halg.statesize =
> + sizeof(struct chcr_ahash_req_ctx);
Even with this sort of change, a lot of barely >80 column lines
are split making the code a bit less readable.
It might be better to avoid splitting these long lines and
ignore the >80 column limits occasionally.
