| Message ID | 1612853965-67777-1-git-send-email-chenxiang66@hisilicon.com |
|---|---|
| Headers | show |
| Series | Fix the parameter of dma_map_sg() | expand |
Ping... 在 2021/2/9 14:59, chenxiang 写道: > From: Xiang Chen <chenxiang66@hisilicon.com> > > According to Documentation/core-api/dma-api-howto.rst, the parameters > of dma_unmap_sg() must be the same as those which are passed in to the > scatter/gather mapping API. > But for some drivers under crypto, the <nents> parameter of dma_unmap_sg() > is number of elements after mapping. So fix them. > > Part of the document is as follows: > > To unmap a scatterlist, just call:: > > dma_unmap_sg(dev, sglist, nents, direction); > > Again, make sure DMA activity has already finished. > > .. note:: > > The 'nents' argument to the dma_unmap_sg call must be > the _same_ one you passed into the dma_map_sg call, > it should _NOT_ be the 'count' value _returned_ from the > dma_map_sg call. > > chenxiang (4): > crypto: amlogic - Fix the parameter of dma_unmap_sg() > crypto: cavium - Fix the parameter of dma_unmap_sg() > crypto: ux500 - Fix the parameter of dma_unmap_sg() > crypto: allwinner - Fix the parameter of dma_unmap_sg() > > drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c | 9 ++++++--- > drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c | 3 ++- > drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c | 9 ++++++--- > drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c | 3 ++- > drivers/crypto/amlogic/amlogic-gxl-cipher.c | 6 +++--- > drivers/crypto/cavium/nitrox/nitrox_reqmgr.c | 8 ++++---- > drivers/crypto/ux500/cryp/cryp_core.c | 4 ++-- > drivers/crypto/ux500/hash/hash_core.c | 2 +- > 8 files changed, 26 insertions(+), 18 deletions(-) >
On Sat, Feb 20, 2021 at 09:51:17AM +0800, chenxiang (M) wrote:
> Ping...
Please be patient.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Tue, Feb 09, 2021 at 02:59:23PM +0800, chenxiang wrote: > From: Xiang Chen <chenxiang66@hisilicon.com> > > For function dma_unmap_sg(), the <nents> parameter should be number of > elements in the scatterlist prior to the mapping, not after the mapping. > So fix this usage. > > Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com> > --- > drivers/crypto/cavium/nitrox/nitrox_reqmgr.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/crypto/cavium/nitrox/nitrox_reqmgr.c b/drivers/crypto/cavium/nitrox/nitrox_reqmgr.c > index 53ef067..1263194 100644 > --- a/drivers/crypto/cavium/nitrox/nitrox_reqmgr.c > +++ b/drivers/crypto/cavium/nitrox/nitrox_reqmgr.c > @@ -170,7 +170,7 @@ static int dma_map_inbufs(struct nitrox_softreq *sr, > sr->in.total_bytes += sg_dma_len(sg); > > sr->in.sg = req->src; > - sr->in.sgmap_cnt = nents; > + sr->in.sgmap_cnt = sg_nents(req->src); > ret = create_sg_component(sr, &sr->in, sr->in.sgmap_cnt); So you're changing the count passed to create_sg_component. Are you sure that's correct? Even if it is correct you should change your patch description to document this change. Thanks, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Hi Herbert, 在 2021/3/3 16:59, Herbert Xu 写道: > On Tue, Feb 09, 2021 at 02:59:23PM +0800, chenxiang wrote: >> From: Xiang Chen <chenxiang66@hisilicon.com> >> >> For function dma_unmap_sg(), the <nents> parameter should be number of >> elements in the scatterlist prior to the mapping, not after the mapping. >> So fix this usage. >> >> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com> >> --- >> drivers/crypto/cavium/nitrox/nitrox_reqmgr.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/crypto/cavium/nitrox/nitrox_reqmgr.c b/drivers/crypto/cavium/nitrox/nitrox_reqmgr.c >> index 53ef067..1263194 100644 >> --- a/drivers/crypto/cavium/nitrox/nitrox_reqmgr.c >> +++ b/drivers/crypto/cavium/nitrox/nitrox_reqmgr.c >> @@ -170,7 +170,7 @@ static int dma_map_inbufs(struct nitrox_softreq *sr, >> sr->in.total_bytes += sg_dma_len(sg); >> >> sr->in.sg = req->src; >> - sr->in.sgmap_cnt = nents; >> + sr->in.sgmap_cnt = sg_nents(req->src); >> ret = create_sg_component(sr, &sr->in, sr->in.sgmap_cnt); > So you're changing the count passed to create_sg_component. Are you > sure that's correct? Even if it is correct you should change your > patch description to document this change. Thank you for reminding me about this. I didn't notice that it changes the count passed to create_sg_component. I have a change on this patch as follows, and please have a check on it: --- a/drivers/crypto/cavium/nitrox/nitrox_reqmgr.c +++ b/drivers/crypto/cavium/nitrox/nitrox_reqmgr.c @@ -58,14 +58,14 @@ static void softreq_unmap_sgbufs(struct nitrox_softreq *sr) struct device *dev = DEV(ndev); - dma_unmap_sg(dev, sr->in.sg, sr->in.sgmap_cnt, DMA_BIDIRECTIONAL); + dma_unmap_sg(dev, sr->in.sg, sg_nents(sr->in.sg), DMA_BIDIRECTIONAL); dma_unmap_single(dev, sr->in.sgcomp_dma, sr->in.sgcomp_len, DMA_TO_DEVICE); kfree(sr->in.sgcomp); sr->in.sg = NULL; sr->in.sgmap_cnt = 0; - dma_unmap_sg(dev, sr->out.sg, sr->out.sgmap_cnt, + dma_unmap_sg(dev, sr->out.sg, sg_nents(sr->out.sg), DMA_BIDIRECTIONAL); dma_unmap_single(dev, sr->out.sgcomp_dma, sr->out.sgcomp_len, DMA_TO_DEVICE); @@ -178,7 +178,8 @@ static int dma_map_inbufs(struct nitrox_softreq *sr, return 0; incomp_err: - dma_unmap_sg(dev, req->src, nents, DMA_BIDIRECTIONAL); + dma_unmap_sg(dev, req->src, sg_nents(req->src), + DMA_BIDIRECTIONAL); sr->in.sgmap_cnt = 0; return ret; } @@ -203,7 +204,8 @@ static int dma_map_outbufs(struct nitrox_softreq *sr, return 0; outcomp_map_err: - dma_unmap_sg(dev, req->dst, nents, DMA_BIDIRECTIONAL); + dma_unmap_sg(dev, req->dst, sg_nents(req->dst), + DMA_BIDIRECTIONAL); sr->out.sgmap_cnt = 0; sr->out.sg = NULL; return ret;
On Thu, Mar 04, 2021 at 10:18:51AM +0800, chenxiang (M) wrote: > > Thank you for reminding me about this. I didn't notice that it changes the > count passed to create_sg_component. > I have a change on this patch as follows, and please have a check on it: Yes this looks fine. Thanks, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
From: Xiang Chen <chenxiang66@hisilicon.com> According to Documentation/core-api/dma-api-howto.rst, the parameters of dma_unmap_sg() must be the same as those which are passed in to the scatter/gather mapping API. But for some drivers under crypto, the <nents> parameter of dma_unmap_sg() is number of elements after mapping. So fix them. Part of the document is as follows: To unmap a scatterlist, just call:: dma_unmap_sg(dev, sglist, nents, direction); Again, make sure DMA activity has already finished. .. note:: The 'nents' argument to the dma_unmap_sg call must be the _same_ one you passed into the dma_map_sg call, it should _NOT_ be the 'count' value _returned_ from the dma_map_sg call. chenxiang (4): crypto: amlogic - Fix the parameter of dma_unmap_sg() crypto: cavium - Fix the parameter of dma_unmap_sg() crypto: ux500 - Fix the parameter of dma_unmap_sg() crypto: allwinner - Fix the parameter of dma_unmap_sg() drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c | 9 ++++++--- drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c | 3 ++- drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c | 9 ++++++--- drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c | 3 ++- drivers/crypto/amlogic/amlogic-gxl-cipher.c | 6 +++--- drivers/crypto/cavium/nitrox/nitrox_reqmgr.c | 8 ++++---- drivers/crypto/ux500/cryp/cryp_core.c | 4 ++-- drivers/crypto/ux500/hash/hash_core.c | 2 +- 8 files changed, 26 insertions(+), 18 deletions(-)