2015-05-01 06:47:41

by Akinobu Mita

[permalink] [raw]
Subject: [PATCH 1/5] m68k: use for_each_sg()

Since m68k doesn't select ARCH_HAS_SG_CHAIN, it is not necessary to
use for_each_sg() in order to loop over each sg element. But this can
help find problems with drivers that do not properly initialize their
sg tables when CONFIG_DEBUG_SG is enabled.

Signed-off-by: Akinobu Mita <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/m68k/kernel/dma.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/m68k/kernel/dma.c b/arch/m68k/kernel/dma.c
index e546a55..564665f 100644
--- a/arch/m68k/kernel/dma.c
+++ b/arch/m68k/kernel/dma.c
@@ -120,13 +120,16 @@ void dma_sync_single_for_device(struct device *dev, dma_addr_t handle,
}
EXPORT_SYMBOL(dma_sync_single_for_device);

-void dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg, int nents,
- enum dma_data_direction dir)
+void dma_sync_sg_for_device(struct device *dev, struct scatterlist *sglist,
+ int nents, enum dma_data_direction dir)
{
int i;
+ struct scatterlist *sg;

- for (i = 0; i < nents; sg++, i++)
- dma_sync_single_for_device(dev, sg->dma_address, sg->length, dir);
+ for_each_sg(sglist, sg, nents, i) {
+ dma_sync_single_for_device(dev, sg->dma_address, sg->length,
+ dir);
+ }
}
EXPORT_SYMBOL(dma_sync_sg_for_device);

@@ -151,14 +154,16 @@ dma_addr_t dma_map_page(struct device *dev, struct page *page,
}
EXPORT_SYMBOL(dma_map_page);

-int dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
+int dma_map_sg(struct device *dev, struct scatterlist *sglist, int nents,
enum dma_data_direction dir)
{
int i;
+ struct scatterlist *sg;

- for (i = 0; i < nents; sg++, i++) {
+ for_each_sg(sglist, sg, nents, i) {
sg->dma_address = sg_phys(sg);
- dma_sync_single_for_device(dev, sg->dma_address, sg->length, dir);
+ dma_sync_single_for_device(dev, sg->dma_address, sg->length,
+ dir);
}
return nents;
}
--
1.9.1


2015-05-01 06:47:50

by Akinobu Mita

[permalink] [raw]
Subject: [PATCH 2/5] arc: use for_each_sg()

Since arc doesn't select ARCH_HAS_SG_CHAIN, it is not necessary to
use for_each_sg() in order to loop over each sg element. But this can
help find problems with drivers that do not properly initialize their
sg tables when CONFIG_DEBUG_SG is enabled.

Signed-off-by: Akinobu Mita <[email protected]>
Cc: Vineet Gupta <[email protected]>
Cc: [email protected]
---
arch/arc/include/asm/dma-mapping.h | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/arc/include/asm/dma-mapping.h b/arch/arc/include/asm/dma-mapping.h
index 45b8e0c..f787894 100644
--- a/arch/arc/include/asm/dma-mapping.h
+++ b/arch/arc/include/asm/dma-mapping.h
@@ -178,22 +178,24 @@ dma_sync_single_range_for_device(struct device *dev, dma_addr_t dma_handle,
}

static inline void
-dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg, int nelems,
+dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sglist, int nelems,
enum dma_data_direction dir)
{
int i;
+ struct scatterlist *sg;

- for (i = 0; i < nelems; i++, sg++)
+ for_each_sg(sglist, sg, nelems, i)
_dma_cache_sync((unsigned int)sg_virt(sg), sg->length, dir);
}

static inline void
-dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg, int nelems,
- enum dma_data_direction dir)
+dma_sync_sg_for_device(struct device *dev, struct scatterlist *sglist,
+ int nelems, enum dma_data_direction dir)
{
int i;
+ struct scatterlist *sg;

- for (i = 0; i < nelems; i++, sg++)
+ for_each_sg(sglist, sg, nelems, i)
_dma_cache_sync((unsigned int)sg_virt(sg), sg->length, dir);
}

--
1.9.1

2015-05-01 06:48:37

by Akinobu Mita

[permalink] [raw]
Subject: [PATCH 3/5] metag: use for_each_sg()

Since metag doesn't select ARCH_HAS_SG_CHAIN, it is not necessary to
use for_each_sg() in order to loop over each sg element. But this can
help find problems with drivers that do not properly initialize their
sg tables when CONFIG_DEBUG_SG is enabled.

Signed-off-by: Akinobu Mita <[email protected]>
Cc: James Hogan <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/metag/include/asm/dma-mapping.h | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/metag/include/asm/dma-mapping.h b/arch/metag/include/asm/dma-mapping.h
index 14b23ef..eb5cdec 100644
--- a/arch/metag/include/asm/dma-mapping.h
+++ b/arch/metag/include/asm/dma-mapping.h
@@ -134,20 +134,24 @@ dma_sync_single_range_for_device(struct device *dev, dma_addr_t dma_handle,
}

static inline void
-dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg, int nelems,
+dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sglist, int nelems,
enum dma_data_direction direction)
{
int i;
- for (i = 0; i < nelems; i++, sg++)
+ struct scatterlist *sg;
+
+ for_each_sg(sglist, sg, nelems, i)
dma_sync_for_cpu(sg_virt(sg), sg->length, direction);
}

static inline void
-dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg, int nelems,
- enum dma_data_direction direction)
+dma_sync_sg_for_device(struct device *dev, struct scatterlist *sglist,
+ int nelems, enum dma_data_direction direction)
{
int i;
- for (i = 0; i < nelems; i++, sg++)
+ struct scatterlist *sg;
+
+ for_each_sg(sglist, sg, nelems, i)
dma_sync_for_device(sg_virt(sg), sg->length, direction);
}

--
1.9.1

2015-05-01 06:47:55

by Akinobu Mita

[permalink] [raw]
Subject: [PATCH 4/5] xtensa: use for_each_sg()

Since xtensa doesn't select ARCH_HAS_SG_CHAIN, it is not necessary to
use for_each_sg() in order to loop over each sg element. But this can
help find problems with drivers that do not properly initialize their
sg tables when CONFIG_DEBUG_SG is enabled.

Signed-off-by: Akinobu Mita <[email protected]>
Cc: Chris Zankel <[email protected]>
Cc: Max Filippov <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/xtensa/include/asm/dma-mapping.h | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/xtensa/include/asm/dma-mapping.h b/arch/xtensa/include/asm/dma-mapping.h
index 172a02a..54d2b22 100644
--- a/arch/xtensa/include/asm/dma-mapping.h
+++ b/arch/xtensa/include/asm/dma-mapping.h
@@ -52,14 +52,15 @@ dma_unmap_single(struct device *dev, dma_addr_t dma_addr, size_t size,
}

static inline int
-dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
+dma_map_sg(struct device *dev, struct scatterlist *sglist, int nents,
enum dma_data_direction direction)
{
int i;
+ struct scatterlist *sg;

BUG_ON(direction == DMA_NONE);

- for (i = 0; i < nents; i++, sg++ ) {
+ for_each_sg(sglist, sg, nents, i) {
BUG_ON(!sg_page(sg));

sg->dma_address = sg_phys(sg);
@@ -124,20 +125,24 @@ dma_sync_single_range_for_device(struct device *dev, dma_addr_t dma_handle,
consistent_sync((void *)bus_to_virt(dma_handle)+offset,size,direction);
}
static inline void
-dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg, int nelems,
+dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sglist, int nelems,
enum dma_data_direction dir)
{
int i;
- for (i = 0; i < nelems; i++, sg++)
+ struct scatterlist *sg;
+
+ for_each_sg(sglist, sg, nelems, i)
consistent_sync(sg_virt(sg), sg->length, dir);
}

static inline void
-dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg, int nelems,
- enum dma_data_direction dir)
+dma_sync_sg_for_device(struct device *dev, struct scatterlist *sglist,
+ int nelems, enum dma_data_direction dir)
{
int i;
- for (i = 0; i < nelems; i++, sg++)
+ struct scatterlist *sg;
+
+ for_each_sg(sglist, sg, nelems, i)
consistent_sync(sg_virt(sg), sg->length, dir);
}
static inline int
--
1.9.1

2015-05-01 06:48:14

by Akinobu Mita

[permalink] [raw]
Subject: [PATCH 5/5] mips: use for_each_sg()

Since mips doesn't select ARCH_HAS_SG_CHAIN, it is not necessary to
use for_each_sg() in order to loop over each sg element. But this can
help find problems with drivers that do not properly initialize their
sg tables when CONFIG_DEBUG_SG is enabled.

Signed-off-by: Akinobu Mita <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/mm/dma-default.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/arch/mips/mm/dma-default.c b/arch/mips/mm/dma-default.c
index 609d124..eeaf024 100644
--- a/arch/mips/mm/dma-default.c
+++ b/arch/mips/mm/dma-default.c
@@ -262,12 +262,13 @@ static void mips_dma_unmap_page(struct device *dev, dma_addr_t dma_addr,
plat_unmap_dma_mem(dev, dma_addr, size, direction);
}

-static int mips_dma_map_sg(struct device *dev, struct scatterlist *sg,
+static int mips_dma_map_sg(struct device *dev, struct scatterlist *sglist,
int nents, enum dma_data_direction direction, struct dma_attrs *attrs)
{
int i;
+ struct scatterlist *sg;

- for (i = 0; i < nents; i++, sg++) {
+ for_each_sg(sglist, sg, nents, i) {
if (!plat_device_is_coherent(dev))
__dma_sync(sg_page(sg), sg->offset, sg->length,
direction);
@@ -291,13 +292,14 @@ static dma_addr_t mips_dma_map_page(struct device *dev, struct page *page,
return plat_map_dma_mem_page(dev, page) + offset;
}

-static void mips_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
+static void mips_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
int nhwentries, enum dma_data_direction direction,
struct dma_attrs *attrs)
{
int i;
+ struct scatterlist *sg;

- for (i = 0; i < nhwentries; i++, sg++) {
+ for_each_sg(sglist, sg, nhwentries, i) {
if (!plat_device_is_coherent(dev) &&
direction != DMA_TO_DEVICE)
__dma_sync(sg_page(sg), sg->offset, sg->length,
@@ -324,26 +326,34 @@ static void mips_dma_sync_single_for_device(struct device *dev,
}

static void mips_dma_sync_sg_for_cpu(struct device *dev,
- struct scatterlist *sg, int nelems, enum dma_data_direction direction)
+ struct scatterlist *sglist, int nelems,
+ enum dma_data_direction direction)
{
int i;
+ struct scatterlist *sg;

- if (cpu_needs_post_dma_flush(dev))
- for (i = 0; i < nelems; i++, sg++)
+ if (cpu_needs_post_dma_flush(dev)) {
+ for_each_sg(sglist, sg, nelems, i) {
__dma_sync(sg_page(sg), sg->offset, sg->length,
direction);
+ }
+ }
plat_post_dma_flush(dev);
}

static void mips_dma_sync_sg_for_device(struct device *dev,
- struct scatterlist *sg, int nelems, enum dma_data_direction direction)
+ struct scatterlist *sglist, int nelems,
+ enum dma_data_direction direction)
{
int i;
+ struct scatterlist *sg;

- if (!plat_device_is_coherent(dev))
- for (i = 0; i < nelems; i++, sg++)
+ if (!plat_device_is_coherent(dev)) {
+ for_each_sg(sglist, sg, nelems, i) {
__dma_sync(sg_page(sg), sg->offset, sg->length,
direction);
+ }
+ }
}

int mips_dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
--
1.9.1

2015-05-01 08:40:27

by Vineet Gupta

[permalink] [raw]
Subject: Re: [PATCH 2/5] arc: use for_each_sg()

On Friday 01 May 2015 12:17 PM, Akinobu Mita wrote:
> Since arc doesn't select ARCH_HAS_SG_CHAIN, it is not necessary to
> use for_each_sg() in order to loop over each sg element. But this can
> help find problems with drivers that do not properly initialize their
> sg tables when CONFIG_DEBUG_SG is enabled.
>
> Signed-off-by: Akinobu Mita <[email protected]>
> Cc: Vineet Gupta <[email protected]>
> Cc: [email protected]

Looks fine to me. However it must be noted (perhaps add to change log) that this
will lead to different generated code as sg_next() is a function call etc. So this
change is strictly not equivalent to what we had before.

Acked-by: Vineet Gupta <[email protected]>

Thx,
-Vineet


> ---
> arch/arc/include/asm/dma-mapping.h | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arc/include/asm/dma-mapping.h b/arch/arc/include/asm/dma-mapping.h
> index 45b8e0c..f787894 100644
> --- a/arch/arc/include/asm/dma-mapping.h
> +++ b/arch/arc/include/asm/dma-mapping.h
> @@ -178,22 +178,24 @@ dma_sync_single_range_for_device(struct device *dev, dma_addr_t dma_handle,
> }
>
> static inline void
> -dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg, int nelems,
> +dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sglist, int nelems,
> enum dma_data_direction dir)
> {
> int i;
> + struct scatterlist *sg;
>
> - for (i = 0; i < nelems; i++, sg++)
> + for_each_sg(sglist, sg, nelems, i)
> _dma_cache_sync((unsigned int)sg_virt(sg), sg->length, dir);
> }
>
> static inline void
> -dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg, int nelems,
> - enum dma_data_direction dir)
> +dma_sync_sg_for_device(struct device *dev, struct scatterlist *sglist,
> + int nelems, enum dma_data_direction dir)
> {
> int i;
> + struct scatterlist *sg;
>
> - for (i = 0; i < nelems; i++, sg++)
> + for_each_sg(sglist, sg, nelems, i)
> _dma_cache_sync((unsigned int)sg_virt(sg), sg->length, dir);
> }
>
>

2015-05-01 09:07:35

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 1/5] m68k: use for_each_sg()

Hi Mita-san,

On Fri, May 1, 2015 at 8:47 AM, Akinobu Mita <[email protected]> wrote:
> Since m68k doesn't select ARCH_HAS_SG_CHAIN, it is not necessary to
> use for_each_sg() in order to loop over each sg element. But this can
> help find problems with drivers that do not properly initialize their
> sg tables when CONFIG_DEBUG_SG is enabled.
>
> Signed-off-by: Akinobu Mita <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: [email protected]
> Cc: [email protected]

Acked-by: Geert Uytterhoeven <[email protected]>

Do you want me to queue this up for v4.2, or do you want to handle these
changes yourself, together with "[PATCH] scatterlist: enable sg chaining
for all architectures"?

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2015-05-01 10:30:20

by Akinobu Mita

[permalink] [raw]
Subject: Re: [PATCH 1/5] m68k: use for_each_sg()

2015-05-01 18:07 GMT+09:00 Geert Uytterhoeven <[email protected]>:
> Hi Mita-san,
>
> On Fri, May 1, 2015 at 8:47 AM, Akinobu Mita <[email protected]> wrote:
>> Since m68k doesn't select ARCH_HAS_SG_CHAIN, it is not necessary to
>> use for_each_sg() in order to loop over each sg element. But this can
>> help find problems with drivers that do not properly initialize their
>> sg tables when CONFIG_DEBUG_SG is enabled.
>>
>> Signed-off-by: Akinobu Mita <[email protected]>
>> Cc: Geert Uytterhoeven <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>
> Acked-by: Geert Uytterhoeven <[email protected]>
>
> Do you want me to queue this up for v4.2, or do you want to handle these
> changes yourself, together with "[PATCH] scatterlist: enable sg chaining
> for all architectures"?

Hi Geert,

Please queue it up to your tree. As it turned out that enabling
sg chaining for all architectures requires more survey, I've just
started trivial works like this.