From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932614AbcGKS5X (ORCPT ); Mon, 11 Jul 2016 14:57:23 -0400 Received: from smtprelay0109.hostedemail.com ([216.40.44.109]:34015 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932515AbcGKS5V (ORCPT ); Mon, 11 Jul 2016 14:57:21 -0400 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,joe@perches.com,:::::::::::::::::::,RULES_HIT:41:355:379:541:599:960:966:973:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1381:1437:1515:1516:1518:1534:1543:1593:1594:1711:1730:1747:1777:1792:1981:2194:2196:2199:2200:2393:2559:2562:2828:2895:2898:2914:3138:3139:3140:3141:3142:3354:3622:3865:3866:3867:3868:3871:3872:4321:4385:5007:6119:7875:7904:10004:10400:10848:11026:11232:11473:11658:11783:11914:12043:12048:12296:12438:12517:12519:12740:13439:13894:13972:14181:14659:14721:21080:30012:30029:30054:30070:30075:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:1,LUA_SUMMARY:none X-HE-Tag: soup70_3fc632ff72a18 X-Filterd-Recvd-Size: 4065 Message-ID: <1468263436.8360.154.camel@perches.com> Subject: Re: [PATCH 2/3] chcr: Support for Chelsio's Crypto Hardware From: Joe Perches To: Yeshaswi M R Gowda , hariprasad@chelsio.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, herbert@gondor.apana.org.au, davem@davemloft.net, linux-crypto@vger.kernel.org, jlulla@chelsio.com, atul.gupta@chelsio.com, harsh@chelsio.com Date: Mon, 11 Jul 2016 11:57:16 -0700 In-Reply-To: <1468261688-24525-3-git-send-email-yeshaswi@chelsio.com> References: <1468261688-24525-1-git-send-email-yeshaswi@chelsio.com> <1468261688-24525-3-git-send-email-yeshaswi@chelsio.com> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.18.5.2-0ubuntu3 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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_ 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 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.