* [PATCH] arm/dma-mapping: use for_each_sg
@ 2017-03-24 14:12 Geliang Tang
2017-03-24 14:48 ` Russell King - ARM Linux
0 siblings, 1 reply; 2+ messages in thread
From: Geliang Tang @ 2017-03-24 14:12 UTC (permalink / raw)
To: linux-arm-kernel
Use for_each_sg() instead of open-coding it.
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
arch/arm/mm/dma-mapping.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 63eabb0..e551351 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1720,7 +1720,7 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
if (iova == DMA_ERROR_CODE)
return -ENOMEM;
- for (count = 0, s = sg; count < (size >> PAGE_SHIFT); s = sg_next(s)) {
+ for_each_sg(sg, s, size >> PAGE_SHIFT, count) {
phys_addr_t phys = page_to_phys(sg_page(s));
unsigned int len = PAGE_ALIGN(s->offset + s->length);
--
2.9.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH] arm/dma-mapping: use for_each_sg
2017-03-24 14:12 [PATCH] arm/dma-mapping: use for_each_sg Geliang Tang
@ 2017-03-24 14:48 ` Russell King - ARM Linux
0 siblings, 0 replies; 2+ messages in thread
From: Russell King - ARM Linux @ 2017-03-24 14:48 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Mar 24, 2017 at 10:12:23PM +0800, Geliang Tang wrote:
> Use for_each_sg() instead of open-coding it.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
No.
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 63eabb0..e551351 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -1720,7 +1720,7 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
> if (iova == DMA_ERROR_CODE)
> return -ENOMEM;
>
> - for (count = 0, s = sg; count < (size >> PAGE_SHIFT); s = sg_next(s)) {
> + for_each_sg(sg, s, size >> PAGE_SHIFT, count) {
This is _not_ equivalent.
#define for_each_sg(sglist, sg, nr, __i) \
for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
Putting the arguments into that gives:
for (count = 0, s = sg; count < (size >> PAGE_SHIFT); count++, s = sg_next(s))
whereas the old code is:
for (count = 0, s = sg; count < (size >> PAGE_SHIFT); s = sg_next(s))
Spot the difference? "count++".
This is because "size >> PAGE_SHIFT" is not the number of scatterlist
entries, it's the number of pages to map.
Please be more careful in the future to ensure that, when cleaning up
code, that the code is indeed equivalent. Subtle errors like this are
sometimes incredibly difficult to spot.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-03-24 14:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-24 14:12 [PATCH] arm/dma-mapping: use for_each_sg Geliang Tang
2017-03-24 14:48 ` Russell King - ARM Linux
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).