Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751359Ab3HTM6F (ORCPT ); Tue, 20 Aug 2013 08:58:05 -0400 Received: from bear.ext.ti.com ([192.94.94.41]:43629 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751156Ab3HTM6C (ORCPT ); Tue, 20 Aug 2013 08:58:02 -0400 Message-ID: <521367B4.2020203@ti.com> Date: Tue, 20 Aug 2013 18:27:24 +0530 From: Lokesh Vutla User-Agent: Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20120912 Thunderbird/15.0.1 MIME-Version: 1.0 To: Joel Fernandes CC: Herbert Xu , "David S. Miller" , Mark Greer , Tony Lindgren , Santosh Shilimkar , Rajendra Nayak , Linux OMAP List , Linux ARM Kernel List , Linux Kernel Mailing List , Linux Crypto Mailing List Subject: Re: [PATCH v2 04/14] crypto: omap-aes: Simplify DMA usage by using direct SGs References: <1376793755-30478-1-git-send-email-joelf@ti.com> <1376793755-30478-5-git-send-email-joelf@ti.com> In-Reply-To: <1376793755-30478-5-git-send-email-joelf@ti.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8382 Lines: 258 Hi Joel, On Sunday 18 August 2013 08:12 AM, Joel Fernandes wrote: > In early version of this driver, assumptions were made such as DMA layer > requires contiguous buffers etc. Due to this, new buffers were allocated, > mapped and used for DMA. These assumptions are no longer true and DMAEngine > scatter-gather DMA doesn't have such requirements. We simply the DMA operations > by directly using the scatter-gather buffers provided by the crypto layer > instead of creating our own. > > Lot of logic that handled DMA'ing only X number of bytes of the total, or as > much as fitted into a 3rd party buffer is removed and is no longer required. > > Also, good performance improvement of atleast ~20% seen with encrypting a > buffer size of 8K (1800 ops/sec vs 1400 ops/sec). Improvement will be higher > for much larger blocks though such benchmarking is left as an exercise for the > reader. Also DMA usage is much more simplified and coherent with rest of the > code. > > Signed-off-by: Joel Fernandes > --- > drivers/crypto/omap-aes.c | 147 ++++++++------------------------------------- > 1 file changed, 25 insertions(+), 122 deletions(-) > > diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c > index e369e6e..64dd5c1 100644 > --- a/drivers/crypto/omap-aes.c > +++ b/drivers/crypto/omap-aes.c > @@ -480,22 +480,14 @@ static int sg_copy(struct scatterlist **sg, size_t *offset, void *buf, > } > > static int omap_aes_crypt_dma(struct crypto_tfm *tfm, > - struct scatterlist *in_sg, struct scatterlist *out_sg) > + struct scatterlist *in_sg, struct scatterlist *out_sg, > + int in_sg_len, int out_sg_len) > { > struct omap_aes_ctx *ctx = crypto_tfm_ctx(tfm); > struct omap_aes_dev *dd = ctx->dd; > struct dma_async_tx_descriptor *tx_in, *tx_out; > struct dma_slave_config cfg; > - dma_addr_t dma_addr_in = sg_dma_address(in_sg); > - int ret, length = sg_dma_len(in_sg); > - > - pr_debug("len: %d\n", length); > - > - dd->dma_size = length; > - > - if (!(dd->flags & FLAGS_FAST)) > - dma_sync_single_for_device(dd->dev, dma_addr_in, length, > - DMA_TO_DEVICE); > + int ret; By this change FLAGS_FAST is unsed, it can be cleaned right? or Am I missing something? Thanks and regards, Lokesh > > memset(&cfg, 0, sizeof(cfg)); > > @@ -514,7 +506,7 @@ static int omap_aes_crypt_dma(struct crypto_tfm *tfm, > return ret; > } > > - tx_in = dmaengine_prep_slave_sg(dd->dma_lch_in, in_sg, 1, > + tx_in = dmaengine_prep_slave_sg(dd->dma_lch_in, in_sg, in_sg_len, > DMA_MEM_TO_DEV, > DMA_PREP_INTERRUPT | DMA_CTRL_ACK); > if (!tx_in) { > @@ -533,7 +525,7 @@ static int omap_aes_crypt_dma(struct crypto_tfm *tfm, > return ret; > } > > - tx_out = dmaengine_prep_slave_sg(dd->dma_lch_out, out_sg, 1, > + tx_out = dmaengine_prep_slave_sg(dd->dma_lch_out, out_sg, out_sg_len, > DMA_DEV_TO_MEM, > DMA_PREP_INTERRUPT | DMA_CTRL_ACK); > if (!tx_out) { > @@ -551,7 +543,7 @@ static int omap_aes_crypt_dma(struct crypto_tfm *tfm, > dma_async_issue_pending(dd->dma_lch_out); > > /* start DMA */ > - dd->pdata->trigger(dd, length); > + dd->pdata->trigger(dd, dd->total); > > return 0; > } > @@ -560,93 +552,28 @@ static int omap_aes_crypt_dma_start(struct omap_aes_dev *dd) > { > struct crypto_tfm *tfm = crypto_ablkcipher_tfm( > crypto_ablkcipher_reqtfm(dd->req)); > - int err, fast = 0, in, out; > - size_t count; > - dma_addr_t addr_in, addr_out; > - struct scatterlist *in_sg, *out_sg; > - int len32; > + int err; > > pr_debug("total: %d\n", dd->total); > > - if (sg_is_last(dd->in_sg) && sg_is_last(dd->out_sg)) { > - /* check for alignment */ > - in = IS_ALIGNED((u32)dd->in_sg->offset, sizeof(u32)); > - out = IS_ALIGNED((u32)dd->out_sg->offset, sizeof(u32)); > - > - fast = in && out; > + err = dma_map_sg(dd->dev, dd->in_sg, dd->in_sg_len, DMA_TO_DEVICE); > + if (!err) { > + dev_err(dd->dev, "dma_map_sg() error\n"); > + return -EINVAL; > } > > - if (fast) { > - count = min(dd->total, sg_dma_len(dd->in_sg)); > - count = min(count, sg_dma_len(dd->out_sg)); > - > - if (count != dd->total) { > - pr_err("request length != buffer length\n"); > - return -EINVAL; > - } > - > - pr_debug("fast\n"); > - > - err = dma_map_sg(dd->dev, dd->in_sg, 1, DMA_TO_DEVICE); > - if (!err) { > - dev_err(dd->dev, "dma_map_sg() error\n"); > - return -EINVAL; > - } > - > - err = dma_map_sg(dd->dev, dd->out_sg, 1, DMA_FROM_DEVICE); > - if (!err) { > - dev_err(dd->dev, "dma_map_sg() error\n"); > - dma_unmap_sg(dd->dev, dd->in_sg, 1, DMA_TO_DEVICE); > - return -EINVAL; > - } > - > - addr_in = sg_dma_address(dd->in_sg); > - addr_out = sg_dma_address(dd->out_sg); > - > - in_sg = dd->in_sg; > - out_sg = dd->out_sg; > - > - dd->flags |= FLAGS_FAST; > - > - } else { > - /* use cache buffers */ > - count = sg_copy(&dd->in_sg, &dd->in_offset, dd->buf_in, > - dd->buflen, dd->total, 0); > - > - len32 = DIV_ROUND_UP(count, DMA_MIN) * DMA_MIN; > - > - /* > - * The data going into the AES module has been copied > - * to a local buffer and the data coming out will go > - * into a local buffer so set up local SG entries for > - * both. > - */ > - sg_init_table(&dd->in_sgl, 1); > - dd->in_sgl.offset = dd->in_offset; > - sg_dma_len(&dd->in_sgl) = len32; > - sg_dma_address(&dd->in_sgl) = dd->dma_addr_in; > - > - sg_init_table(&dd->out_sgl, 1); > - dd->out_sgl.offset = dd->out_offset; > - sg_dma_len(&dd->out_sgl) = len32; > - sg_dma_address(&dd->out_sgl) = dd->dma_addr_out; > - > - in_sg = &dd->in_sgl; > - out_sg = &dd->out_sgl; > - > - addr_in = dd->dma_addr_in; > - addr_out = dd->dma_addr_out; > - > - dd->flags &= ~FLAGS_FAST; > - > + err = dma_map_sg(dd->dev, dd->out_sg, dd->out_sg_len, DMA_FROM_DEVICE); > + if (!err) { > + dev_err(dd->dev, "dma_map_sg() error\n"); > + return -EINVAL; > } > > - dd->total -= count; > - > - err = omap_aes_crypt_dma(tfm, in_sg, out_sg); > + err = omap_aes_crypt_dma(tfm, dd->in_sg, dd->out_sg, dd->in_sg_len, > + dd->out_sg_len); > if (err) { > - dma_unmap_sg(dd->dev, dd->in_sg, 1, DMA_TO_DEVICE); > - dma_unmap_sg(dd->dev, dd->out_sg, 1, DMA_TO_DEVICE); > + dma_unmap_sg(dd->dev, dd->in_sg, dd->in_sg_len, DMA_TO_DEVICE); > + dma_unmap_sg(dd->dev, dd->out_sg, dd->out_sg_len, > + DMA_FROM_DEVICE); > } > > return err; > @@ -667,7 +594,6 @@ static void omap_aes_finish_req(struct omap_aes_dev *dd, int err) > static int omap_aes_crypt_dma_stop(struct omap_aes_dev *dd) > { > int err = 0; > - size_t count; > > pr_debug("total: %d\n", dd->total); > > @@ -676,21 +602,8 @@ static int omap_aes_crypt_dma_stop(struct omap_aes_dev *dd) > dmaengine_terminate_all(dd->dma_lch_in); > dmaengine_terminate_all(dd->dma_lch_out); > > - if (dd->flags & FLAGS_FAST) { > - dma_unmap_sg(dd->dev, dd->out_sg, 1, DMA_FROM_DEVICE); > - dma_unmap_sg(dd->dev, dd->in_sg, 1, DMA_TO_DEVICE); > - } else { > - dma_sync_single_for_device(dd->dev, dd->dma_addr_out, > - dd->dma_size, DMA_FROM_DEVICE); > - > - /* copy data */ > - count = sg_copy(&dd->out_sg, &dd->out_offset, dd->buf_out, > - dd->buflen, dd->dma_size, 1); > - if (count != dd->dma_size) { > - err = -EINVAL; > - pr_err("not all data converted: %u\n", count); > - } > - } > + dma_unmap_sg(dd->dev, dd->in_sg, dd->in_sg_len, DMA_TO_DEVICE); > + dma_unmap_sg(dd->dev, dd->out_sg, dd->out_sg_len, DMA_FROM_DEVICE); > > return err; > } > @@ -760,21 +673,11 @@ static int omap_aes_handle_queue(struct omap_aes_dev *dd, > static void omap_aes_done_task(unsigned long data) > { > struct omap_aes_dev *dd = (struct omap_aes_dev *)data; > - int err; > - > - pr_debug("enter\n"); > > - err = omap_aes_crypt_dma_stop(dd); > - > - err = dd->err ? : err; > - > - if (dd->total && !err) { > - err = omap_aes_crypt_dma_start(dd); > - if (!err) > - return; /* DMA started. Not fininishing. */ > - } > + pr_debug("enter done_task\n"); > > - omap_aes_finish_req(dd, err); > + omap_aes_crypt_dma_stop(dd); > + omap_aes_finish_req(dd, 0); > omap_aes_handle_queue(dd, NULL); > > pr_debug("exit\n"); > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/