Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

2017-09-26 Thread Christoph Hellwig 
On Tue, Sep 26, 2017 at 09:48:01AM +0800, 陈华才 wrote:
> Hi, Christoph,
> 
> Can I put the declaration in asm/dma-coherence.h?

Generally something not expose to the rest of the kernel (that is not
in arch/mips/include/) would be preferred, but in the end the architecture
maintainer will have to decide.

> And, last time you said it is OK to pass a NULL to dma_get_cache_alignment() 
> and cc all driver maintainers. I have do so.

No, I asked you to converted everything to pass the struct device and
cc the driver maintainers.

Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

2017-09-26 Thread Christoph Hellwig 
On Tue, Sep 26, 2017 at 09:48:01AM +0800, 陈华才 wrote:
> Hi, Christoph,
> 
> Can I put the declaration in asm/dma-coherence.h?

Generally something not expose to the rest of the kernel (that is not
in arch/mips/include/) would be preferred, but in the end the architecture
maintainer will have to decide.

> And, last time you said it is OK to pass a NULL to dma_get_cache_alignment() 
> and cc all driver maintainers. I have do so.

No, I asked you to converted everything to pass the struct device and
cc the driver maintainers.

Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

2017-09-25 Thread 陈华才 
Hi, Christoph,

Can I put the declaration in asm/dma-coherence.h?

And, last time you said it is OK to pass a NULL to dma_get_cache_alignment() 
and cc all driver maintainers. I have do so.

Huacai

-- Original --
From:  "Christoph Hellwig"<h...@lst.de>;
Date:  Mon, Sep 25, 2017 08:51 PM
To:  "Huacai Chen"<che...@lemote.com>; 
Cc:  "Christoph Hellwig"<h...@lst.de>; "Marek 
Szyprowski"<m.szyprow...@samsung.com>; "Robin Murphy"<robin.mur...@arm.com>; 
"Andrew Morton"<a...@linux-foundation.org>; "Fuxin Zhang"<zhan...@lemote.com>; 
"linux-kernel"<linux-kernel@vger.kernel.org>; "Ralf 
Baechle"<r...@linux-mips.org>; "James Hogan"<james.ho...@imgtec.com>; 
"linux-mips"<linux-m...@linux-mips.org>; "James E . J . 
Bottomley"<j...@linux.vnet.ibm.com>; "Martin K . 
Petersen"<martin.peter...@oracle.com>; 
"linux-scsi"<linux-s...@vger.kernel.org>; "Roland Dreier"<rola...@cisco.com>; 
"Pawel Osciak"<pa...@osciak.com>; "Kyungmin Park"<kyungmin.p...@samsung.com>; 
"Michael Chan"<michael.c...@broadcom.com>; "Benjamin 
Herrenschmidt"<b...@kernel.crashing.org>; "Ivan Mikhaylov"<i...@ru.ibm.com>; 
"Tariq Toukan"<tar...@mellanox.com>; "Andy Gross"<agr...@codeaurora.org>; "Mark 
A . Greer"<mgr...@mvista.com>; "Robert Baldyga"<r.bald...@samsung.com>; 
"stable"<sta...@vger.kernel.org>; 
Subject:  Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

 
> index aba7138..e2c5d9e 100644
> --- a/arch/mips/include/asm/dma-mapping.h
> +++ b/arch/mips/include/asm/dma-mapping.h
> @@ -39,4 +39,6 @@ static inline void arch_setup_dma_ops(struct device *dev, 
> u64 dma_base,
>  #endif
>  }
>  
> +int mips_get_cache_alignment(struct device *dev);

All the other mips generic dma helpers are prefixed mips_dma_*
so it might make sense to follow that.

Also please don't add arch-local helpers to asm/dma-mapping.h - this
is a header used by linux/dma-mapping.h and should not contain
implementation details if avoidable.

> +dma_get_cache_alignment(NULL)) / 
> mdev->limits.mtt_seg_size;

As said before - please don't pass NULL to this function but the proper
device, which would be >pdev->dev in this case for example.

Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

2017-09-25 Thread 陈华才 
Hi, Christoph,

Can I put the declaration in asm/dma-coherence.h?

And, last time you said it is OK to pass a NULL to dma_get_cache_alignment() 
and cc all driver maintainers. I have do so.

Huacai

-- Original --
From:  "Christoph Hellwig";
Date:  Mon, Sep 25, 2017 08:51 PM
To:  "Huacai Chen"; 
Cc:  "Christoph Hellwig"; "Marek 
Szyprowski"; "Robin Murphy"; 
"Andrew Morton"; "Fuxin Zhang"; 
"linux-kernel"; "Ralf 
Baechle"; "James Hogan"; 
"linux-mips"; "James E . J . 
Bottomley"; "Martin K . 
Petersen"; 
"linux-scsi"; "Roland Dreier"; 
"Pawel Osciak"; "Kyungmin Park"; 
"Michael Chan"; "Benjamin 
Herrenschmidt"; "Ivan Mikhaylov"; 
"Tariq Toukan"; "Andy Gross"; "Mark 
A . Greer"; "Robert Baldyga"; 
"stable"; 
Subject:  Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

 
> index aba7138..e2c5d9e 100644
> --- a/arch/mips/include/asm/dma-mapping.h
> +++ b/arch/mips/include/asm/dma-mapping.h
> @@ -39,4 +39,6 @@ static inline void arch_setup_dma_ops(struct device *dev, 
> u64 dma_base,
>  #endif
>  }
>  
> +int mips_get_cache_alignment(struct device *dev);

All the other mips generic dma helpers are prefixed mips_dma_*
so it might make sense to follow that.

Also please don't add arch-local helpers to asm/dma-mapping.h - this
is a header used by linux/dma-mapping.h and should not contain
implementation details if avoidable.

> +dma_get_cache_alignment(NULL)) / 
> mdev->limits.mtt_seg_size;

As said before - please don't pass NULL to this function but the proper
device, which would be >pdev->dev in this case for example.

Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

2017-09-25 Thread 陈华才 
Hi, Robin,

Can ARM/ARM64 use the same implementation as MIPS? Or I just do MIPS things is 
OK?
 
Huacai
 
-- Original --
From:  "Robin Murphy"<robin.mur...@arm.com>;
Date:  Mon, Sep 25, 2017 08:57 PM
To:  "Huacai Chen"<che...@lemote.com>; "Christoph Hellwig"<h...@lst.de>; 
Cc:  "Marek Szyprowski"<m.szyprow...@samsung.com>; "Andrew 
Morton"<a...@linux-foundation.org>; "Fuxin Zhang"<zhan...@lemote.com>; 
"linux-kernel"<linux-kernel@vger.kernel.org>; "Ralf 
Baechle"<r...@linux-mips.org>; "James Hogan"<james.ho...@imgtec.com>; 
"linux-mips"<linux-m...@linux-mips.org>; "James E . J . 
Bottomley"<j...@linux.vnet.ibm.com>; "Martin K . 
Petersen"<martin.peter...@oracle.com>; 
"linux-scsi"<linux-s...@vger.kernel.org>; "Roland Dreier"<rola...@cisco.com>; 
"Pawel Osciak"<pa...@osciak.com>; "Kyungmin Park"<kyungmin.p...@samsung.com>; 
"Michael Chan"<michael.c...@broadcom.com>; "Benjamin 
Herrenschmidt"<b...@kernel.crashing.org>; "Ivan Mikhaylov"<i...@ru.ibm.com>; 
"Tariq Toukan"<tar...@mellanox.com>; "Andy Gross"<agr...@codeaurora.org>; "Mark 
A . Greer"<mgr...@mvista.com>; "Robert Baldyga"<r.bald...@samsung.com>; 
"stable"<sta...@vger.kernel.org>; 
Subject:  Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

 
On 25/09/17 10:46, Huacai Chen wrote:
> Make dma_get_cache_alignment() to accept a 'dev' argument. As a result,
> it can return different alignments due to different devices' I/O cache
> coherency.
> 
> Currently, MIPS is the only architecture which support coherent & non-
> coherent devices co-exist. This may be changed in the future, so add a
> new get_cache_alignment() function pointer in 'struct dma_map_ops' as a
> generic solution.

FWIW, ARM and arm64 have also supported per-device coherency for quite
some time.

> For compatibility (always return ARCH_DMA_MINALIGN), make all existing
> callers pass a NULL dev argument to dma_get_cache_alignment().
> 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Huacai Chen <che...@lemote.co> ---
>  arch/mips/cavium-octeon/dma-octeon.c   |  3 ++-
>  arch/mips/include/asm/dma-mapping.h|  2 ++
>  arch/mips/loongson64/common/dma-swiotlb.c  |  1 +
>  arch/mips/mm/dma-default.c | 11 ++-
>  arch/mips/netlogic/common/nlm-dma.c|  3 ++-
>  drivers/infiniband/hw/mthca/mthca_main.c   |  2 +-
>  drivers/media/v4l2-core/videobuf2-dma-contig.c |  2 +-
>  drivers/net/ethernet/broadcom/b44.c|  2 +-
>  drivers/net/ethernet/ibm/emac/core.h   |  2 +-
>  drivers/net/ethernet/mellanox/mlx4/main.c  |  2 +-
>  drivers/spi/spi-qup.c  |  4 ++--
>  drivers/tty/serial/mpsc.c  | 16 
>  drivers/tty/serial/samsung.c   | 14 +++---
>  include/linux/dma-mapping.h| 17 -
>  14 files changed, 51 insertions(+), 30 deletions(-)

I think it might be neater to split this into two patches - one making
the treewide prototype change, then introducing the .get_cache_alignemnt
callback separately - but that's only my personal preference.

Otherwise (and modulo Christoph's comments), I'd say we're nearly there.

Thanks,
Robin.

> diff --git a/arch/mips/cavium-octeon/dma-octeon.c 
> b/arch/mips/cavium-octeon/dma-octeon.c
> index c64bd87..7978237 100644
> --- a/arch/mips/cavium-octeon/dma-octeon.c
> +++ b/arch/mips/cavium-octeon/dma-octeon.c
> @@ -324,7 +324,8 @@ static struct octeon_dma_map_ops _octeon_pci_dma_map_ops 
> = {
>   .sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
>   .sync_sg_for_device = octeon_dma_sync_sg_for_device,
>   .mapping_error = swiotlb_dma_mapping_error,
> - .dma_supported = swiotlb_dma_supported
> + .dma_supported = swiotlb_dma_supported,
> + .get_cache_alignment = mips_get_cache_alignment
>   },
>  };
>  
> diff --git a/arch/mips/include/asm/dma-mapping.h 
> b/arch/mips/include/asm/dma-mapping.h
> index aba7138..e2c5d9e 100644
> --- a/arch/mips/include/asm/dma-mapping.h
> +++ b/arch/mips/include/asm/dma-mapping.h
> @@ -39,4 +39,6 @@ static inline void arch_setup_dma_ops(struct device *dev, 
> u64 dma_base,
>  #endif
>  }
>  
> +int mips_get_cache_alignment(struct device *dev);
> +
>  #endif /* _ASM_DMA_MAPPING_H */
> diff --git a/arch/mips/loongson64/common/dma-swiotlb.c 
> b/arch/mips/loongson64/c

Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

2017-09-25 Thread 陈华才 
Hi, Robin,

Can ARM/ARM64 use the same implementation as MIPS? Or I just do MIPS things is 
OK?
 
Huacai
 
-- Original --
From:  "Robin Murphy";
Date:  Mon, Sep 25, 2017 08:57 PM
To:  "Huacai Chen"; "Christoph Hellwig"; 
Cc:  "Marek Szyprowski"; "Andrew 
Morton"; "Fuxin Zhang"; 
"linux-kernel"; "Ralf 
Baechle"; "James Hogan"; 
"linux-mips"; "James E . J . 
Bottomley"; "Martin K . 
Petersen"; 
"linux-scsi"; "Roland Dreier"; 
"Pawel Osciak"; "Kyungmin Park"; 
"Michael Chan"; "Benjamin 
Herrenschmidt"; "Ivan Mikhaylov"; 
"Tariq Toukan"; "Andy Gross"; "Mark 
A . Greer"; "Robert Baldyga"; 
"stable"; 
Subject:  Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

 
On 25/09/17 10:46, Huacai Chen wrote:
> Make dma_get_cache_alignment() to accept a 'dev' argument. As a result,
> it can return different alignments due to different devices' I/O cache
> coherency.
> 
> Currently, MIPS is the only architecture which support coherent & non-
> coherent devices co-exist. This may be changed in the future, so add a
> new get_cache_alignment() function pointer in 'struct dma_map_ops' as a
> generic solution.

FWIW, ARM and arm64 have also supported per-device coherency for quite
some time.

> For compatibility (always return ARCH_DMA_MINALIGN), make all existing
> callers pass a NULL dev argument to dma_get_cache_alignment().
> 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Huacai Chen  ---
>  arch/mips/cavium-octeon/dma-octeon.c   |  3 ++-
>  arch/mips/include/asm/dma-mapping.h|  2 ++
>  arch/mips/loongson64/common/dma-swiotlb.c  |  1 +
>  arch/mips/mm/dma-default.c | 11 ++-
>  arch/mips/netlogic/common/nlm-dma.c|  3 ++-
>  drivers/infiniband/hw/mthca/mthca_main.c   |  2 +-
>  drivers/media/v4l2-core/videobuf2-dma-contig.c |  2 +-
>  drivers/net/ethernet/broadcom/b44.c|  2 +-
>  drivers/net/ethernet/ibm/emac/core.h   |  2 +-
>  drivers/net/ethernet/mellanox/mlx4/main.c  |  2 +-
>  drivers/spi/spi-qup.c  |  4 ++--
>  drivers/tty/serial/mpsc.c  | 16 
>  drivers/tty/serial/samsung.c   | 14 +++---
>  include/linux/dma-mapping.h| 17 -
>  14 files changed, 51 insertions(+), 30 deletions(-)

I think it might be neater to split this into two patches - one making
the treewide prototype change, then introducing the .get_cache_alignemnt
callback separately - but that's only my personal preference.

Otherwise (and modulo Christoph's comments), I'd say we're nearly there.

Thanks,
Robin.

> diff --git a/arch/mips/cavium-octeon/dma-octeon.c 
> b/arch/mips/cavium-octeon/dma-octeon.c
> index c64bd87..7978237 100644
> --- a/arch/mips/cavium-octeon/dma-octeon.c
> +++ b/arch/mips/cavium-octeon/dma-octeon.c
> @@ -324,7 +324,8 @@ static struct octeon_dma_map_ops _octeon_pci_dma_map_ops 
> = {
>   .sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
>   .sync_sg_for_device = octeon_dma_sync_sg_for_device,
>   .mapping_error = swiotlb_dma_mapping_error,
> - .dma_supported = swiotlb_dma_supported
> + .dma_supported = swiotlb_dma_supported,
> + .get_cache_alignment = mips_get_cache_alignment
>   },
>  };
>  
> diff --git a/arch/mips/include/asm/dma-mapping.h 
> b/arch/mips/include/asm/dma-mapping.h
> index aba7138..e2c5d9e 100644
> --- a/arch/mips/include/asm/dma-mapping.h
> +++ b/arch/mips/include/asm/dma-mapping.h
> @@ -39,4 +39,6 @@ static inline void arch_setup_dma_ops(struct device *dev, 
> u64 dma_base,
>  #endif
>  }
>  
> +int mips_get_cache_alignment(struct device *dev);
> +
>  #endif /* _ASM_DMA_MAPPING_H */
> diff --git a/arch/mips/loongson64/common/dma-swiotlb.c 
> b/arch/mips/loongson64/common/dma-swiotlb.c
> index 34486c1..09cb8a4 100644
> --- a/arch/mips/loongson64/common/dma-swiotlb.c
> +++ b/arch/mips/loongson64/common/dma-swiotlb.c
> @@ -119,6 +119,7 @@ static const struct dma_map_ops loongson_dma_map_ops = {
>   .sync_sg_for_device = loongson_dma_sync_sg_for_device,
>   .mapping_error = swiotlb_dma_mapping_error,
>   .dma_supported = loongson_dma_supported,
> + .get_cache_alignment = mips_get_cache_alignment
>  };
>  
>  void __init plat_swiotlb_setup(void)
> diff --git a/arch/mips/mm/dma-default.c b/arch/mips/mm/dma-default.c
> index c01bd20..c49987e 100644
> --- a/arch/mips/mm/dma-default.c
> +++ b/arch/mips/mm/dma-default.c
&

Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

2017-09-25 Thread Robin Murphy 
On 25/09/17 10:46, Huacai Chen wrote:
> Make dma_get_cache_alignment() to accept a 'dev' argument. As a result,
> it can return different alignments due to different devices' I/O cache
> coherency.
> 
> Currently, MIPS is the only architecture which support coherent & non-
> coherent devices co-exist. This may be changed in the future, so add a
> new get_cache_alignment() function pointer in 'struct dma_map_ops' as a
> generic solution.

FWIW, ARM and arm64 have also supported per-device coherency for quite
some time.

> For compatibility (always return ARCH_DMA_MINALIGN), make all existing
> callers pass a NULL dev argument to dma_get_cache_alignment().
> 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Huacai Chen  ---
>  arch/mips/cavium-octeon/dma-octeon.c   |  3 ++-
>  arch/mips/include/asm/dma-mapping.h|  2 ++
>  arch/mips/loongson64/common/dma-swiotlb.c  |  1 +
>  arch/mips/mm/dma-default.c | 11 ++-
>  arch/mips/netlogic/common/nlm-dma.c|  3 ++-
>  drivers/infiniband/hw/mthca/mthca_main.c   |  2 +-
>  drivers/media/v4l2-core/videobuf2-dma-contig.c |  2 +-
>  drivers/net/ethernet/broadcom/b44.c|  2 +-
>  drivers/net/ethernet/ibm/emac/core.h   |  2 +-
>  drivers/net/ethernet/mellanox/mlx4/main.c  |  2 +-
>  drivers/spi/spi-qup.c  |  4 ++--
>  drivers/tty/serial/mpsc.c  | 16 
>  drivers/tty/serial/samsung.c   | 14 +++---
>  include/linux/dma-mapping.h| 17 -
>  14 files changed, 51 insertions(+), 30 deletions(-)

I think it might be neater to split this into two patches - one making
the treewide prototype change, then introducing the .get_cache_alignemnt
callback separately - but that's only my personal preference.

Otherwise (and modulo Christoph's comments), I'd say we're nearly there.

Thanks,
Robin.

> diff --git a/arch/mips/cavium-octeon/dma-octeon.c 
> b/arch/mips/cavium-octeon/dma-octeon.c
> index c64bd87..7978237 100644
> --- a/arch/mips/cavium-octeon/dma-octeon.c
> +++ b/arch/mips/cavium-octeon/dma-octeon.c
> @@ -324,7 +324,8 @@ static struct octeon_dma_map_ops _octeon_pci_dma_map_ops 
> = {
>   .sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
>   .sync_sg_for_device = octeon_dma_sync_sg_for_device,
>   .mapping_error = swiotlb_dma_mapping_error,
> - .dma_supported = swiotlb_dma_supported
> + .dma_supported = swiotlb_dma_supported,
> + .get_cache_alignment = mips_get_cache_alignment
>   },
>  };
>  
> diff --git a/arch/mips/include/asm/dma-mapping.h 
> b/arch/mips/include/asm/dma-mapping.h
> index aba7138..e2c5d9e 100644
> --- a/arch/mips/include/asm/dma-mapping.h
> +++ b/arch/mips/include/asm/dma-mapping.h
> @@ -39,4 +39,6 @@ static inline void arch_setup_dma_ops(struct device *dev, 
> u64 dma_base,
>  #endif
>  }
>  
> +int mips_get_cache_alignment(struct device *dev);
> +
>  #endif /* _ASM_DMA_MAPPING_H */
> diff --git a/arch/mips/loongson64/common/dma-swiotlb.c 
> b/arch/mips/loongson64/common/dma-swiotlb.c
> index 34486c1..09cb8a4 100644
> --- a/arch/mips/loongson64/common/dma-swiotlb.c
> +++ b/arch/mips/loongson64/common/dma-swiotlb.c
> @@ -119,6 +119,7 @@ static const struct dma_map_ops loongson_dma_map_ops = {
>   .sync_sg_for_device = loongson_dma_sync_sg_for_device,
>   .mapping_error = swiotlb_dma_mapping_error,
>   .dma_supported = loongson_dma_supported,
> + .get_cache_alignment = mips_get_cache_alignment
>  };
>  
>  void __init plat_swiotlb_setup(void)
> diff --git a/arch/mips/mm/dma-default.c b/arch/mips/mm/dma-default.c
> index c01bd20..c49987e 100644
> --- a/arch/mips/mm/dma-default.c
> +++ b/arch/mips/mm/dma-default.c
> @@ -394,6 +394,14 @@ void dma_cache_sync(struct device *dev, void *vaddr, 
> size_t size,
>  
>  EXPORT_SYMBOL(dma_cache_sync);
>  
> +int mips_get_cache_alignment(struct device *dev)
> +{
> + if (plat_device_is_coherent(dev))
> + return 1;
> + else
> + return ARCH_DMA_MINALIGN;
> +}
> +
>  static const struct dma_map_ops mips_default_dma_map_ops = {
>   .alloc = mips_dma_alloc_coherent,
>   .free = mips_dma_free_coherent,
> @@ -407,7 +415,8 @@ static const struct dma_map_ops mips_default_dma_map_ops 
> = {
>   .sync_sg_for_cpu = mips_dma_sync_sg_for_cpu,
>   .sync_sg_for_device = mips_dma_sync_sg_for_device,
>   .mapping_error = mips_dma_mapping_error,
> - .dma_supported = mips_dma_supported
> + .dma_supported = mips_dma_supported,
> + .get_cache_alignment = mips_get_cache_alignment
>  };
>  
>  const struct dma_map_ops *mips_dma_map_ops = _default_dma_map_ops;
> diff --git a/arch/mips/netlogic/common/nlm-dma.c 
> b/arch/mips/netlogic/common/nlm-dma.c
> index 0ec9d9d..1e107ac 100644
> --- a/arch/mips/netlogic/common/nlm-dma.c
> +++ b/arch/mips/netlogic/common/nlm-dma.c
> @@ -79,7 +79,8

Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

2017-09-25 Thread Robin Murphy 
On 25/09/17 10:46, Huacai Chen wrote:
> Make dma_get_cache_alignment() to accept a 'dev' argument. As a result,
> it can return different alignments due to different devices' I/O cache
> coherency.
> 
> Currently, MIPS is the only architecture which support coherent & non-
> coherent devices co-exist. This may be changed in the future, so add a
> new get_cache_alignment() function pointer in 'struct dma_map_ops' as a
> generic solution.

FWIW, ARM and arm64 have also supported per-device coherency for quite
some time.

> For compatibility (always return ARCH_DMA_MINALIGN), make all existing
> callers pass a NULL dev argument to dma_get_cache_alignment().
> 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Huacai Chen  ---
>  arch/mips/cavium-octeon/dma-octeon.c   |  3 ++-
>  arch/mips/include/asm/dma-mapping.h|  2 ++
>  arch/mips/loongson64/common/dma-swiotlb.c  |  1 +
>  arch/mips/mm/dma-default.c | 11 ++-
>  arch/mips/netlogic/common/nlm-dma.c|  3 ++-
>  drivers/infiniband/hw/mthca/mthca_main.c   |  2 +-
>  drivers/media/v4l2-core/videobuf2-dma-contig.c |  2 +-
>  drivers/net/ethernet/broadcom/b44.c|  2 +-
>  drivers/net/ethernet/ibm/emac/core.h   |  2 +-
>  drivers/net/ethernet/mellanox/mlx4/main.c  |  2 +-
>  drivers/spi/spi-qup.c  |  4 ++--
>  drivers/tty/serial/mpsc.c  | 16 
>  drivers/tty/serial/samsung.c   | 14 +++---
>  include/linux/dma-mapping.h| 17 -
>  14 files changed, 51 insertions(+), 30 deletions(-)

I think it might be neater to split this into two patches - one making
the treewide prototype change, then introducing the .get_cache_alignemnt
callback separately - but that's only my personal preference.

Otherwise (and modulo Christoph's comments), I'd say we're nearly there.

Thanks,
Robin.

> diff --git a/arch/mips/cavium-octeon/dma-octeon.c 
> b/arch/mips/cavium-octeon/dma-octeon.c
> index c64bd87..7978237 100644
> --- a/arch/mips/cavium-octeon/dma-octeon.c
> +++ b/arch/mips/cavium-octeon/dma-octeon.c
> @@ -324,7 +324,8 @@ static struct octeon_dma_map_ops _octeon_pci_dma_map_ops 
> = {
>   .sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
>   .sync_sg_for_device = octeon_dma_sync_sg_for_device,
>   .mapping_error = swiotlb_dma_mapping_error,
> - .dma_supported = swiotlb_dma_supported
> + .dma_supported = swiotlb_dma_supported,
> + .get_cache_alignment = mips_get_cache_alignment
>   },
>  };
>  
> diff --git a/arch/mips/include/asm/dma-mapping.h 
> b/arch/mips/include/asm/dma-mapping.h
> index aba7138..e2c5d9e 100644
> --- a/arch/mips/include/asm/dma-mapping.h
> +++ b/arch/mips/include/asm/dma-mapping.h
> @@ -39,4 +39,6 @@ static inline void arch_setup_dma_ops(struct device *dev, 
> u64 dma_base,
>  #endif
>  }
>  
> +int mips_get_cache_alignment(struct device *dev);
> +
>  #endif /* _ASM_DMA_MAPPING_H */
> diff --git a/arch/mips/loongson64/common/dma-swiotlb.c 
> b/arch/mips/loongson64/common/dma-swiotlb.c
> index 34486c1..09cb8a4 100644
> --- a/arch/mips/loongson64/common/dma-swiotlb.c
> +++ b/arch/mips/loongson64/common/dma-swiotlb.c
> @@ -119,6 +119,7 @@ static const struct dma_map_ops loongson_dma_map_ops = {
>   .sync_sg_for_device = loongson_dma_sync_sg_for_device,
>   .mapping_error = swiotlb_dma_mapping_error,
>   .dma_supported = loongson_dma_supported,
> + .get_cache_alignment = mips_get_cache_alignment
>  };
>  
>  void __init plat_swiotlb_setup(void)
> diff --git a/arch/mips/mm/dma-default.c b/arch/mips/mm/dma-default.c
> index c01bd20..c49987e 100644
> --- a/arch/mips/mm/dma-default.c
> +++ b/arch/mips/mm/dma-default.c
> @@ -394,6 +394,14 @@ void dma_cache_sync(struct device *dev, void *vaddr, 
> size_t size,
>  
>  EXPORT_SYMBOL(dma_cache_sync);
>  
> +int mips_get_cache_alignment(struct device *dev)
> +{
> + if (plat_device_is_coherent(dev))
> + return 1;
> + else
> + return ARCH_DMA_MINALIGN;
> +}
> +
>  static const struct dma_map_ops mips_default_dma_map_ops = {
>   .alloc = mips_dma_alloc_coherent,
>   .free = mips_dma_free_coherent,
> @@ -407,7 +415,8 @@ static const struct dma_map_ops mips_default_dma_map_ops 
> = {
>   .sync_sg_for_cpu = mips_dma_sync_sg_for_cpu,
>   .sync_sg_for_device = mips_dma_sync_sg_for_device,
>   .mapping_error = mips_dma_mapping_error,
> - .dma_supported = mips_dma_supported
> + .dma_supported = mips_dma_supported,
> + .get_cache_alignment = mips_get_cache_alignment
>  };
>  
>  const struct dma_map_ops *mips_dma_map_ops = _default_dma_map_ops;
> diff --git a/arch/mips/netlogic/common/nlm-dma.c 
> b/arch/mips/netlogic/common/nlm-dma.c
> index 0ec9d9d..1e107ac 100644
> --- a/arch/mips/netlogic/common/nlm-dma.c
> +++ b/arch/mips/netlogic/common/nlm-dma.c
> @@ -79,7 +79,8 @@ const struct

Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

2017-09-25 Thread Christoph Hellwig 
> index aba7138..e2c5d9e 100644
> --- a/arch/mips/include/asm/dma-mapping.h
> +++ b/arch/mips/include/asm/dma-mapping.h
> @@ -39,4 +39,6 @@ static inline void arch_setup_dma_ops(struct device *dev, 
> u64 dma_base,
>  #endif
>  }
>  
> +int mips_get_cache_alignment(struct device *dev);

All the other mips generic dma helpers are prefixed mips_dma_*
so it might make sense to follow that.

Also please don't add arch-local helpers to asm/dma-mapping.h - this
is a header used by linux/dma-mapping.h and should not contain
implementation details if avoidable.

> +dma_get_cache_alignment(NULL)) / 
> mdev->limits.mtt_seg_size;

As said before - please don't pass NULL to this function but the proper
device, which would be >pdev->dev in this case for example.

Re: [PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

2017-09-25 Thread Christoph Hellwig 
> index aba7138..e2c5d9e 100644
> --- a/arch/mips/include/asm/dma-mapping.h
> +++ b/arch/mips/include/asm/dma-mapping.h
> @@ -39,4 +39,6 @@ static inline void arch_setup_dma_ops(struct device *dev, 
> u64 dma_base,
>  #endif
>  }
>  
> +int mips_get_cache_alignment(struct device *dev);

All the other mips generic dma helpers are prefixed mips_dma_*
so it might make sense to follow that.

Also please don't add arch-local helpers to asm/dma-mapping.h - this
is a header used by linux/dma-mapping.h and should not contain
implementation details if avoidable.

> +dma_get_cache_alignment(NULL)) / 
> mdev->limits.mtt_seg_size;

As said before - please don't pass NULL to this function but the proper
device, which would be >pdev->dev in this case for example.

[PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

2017-09-25 Thread Huacai Chen 
Make dma_get_cache_alignment() to accept a 'dev' argument. As a result,
it can return different alignments due to different devices' I/O cache
coherency.

Currently, MIPS is the only architecture which support coherent & non-
coherent devices co-exist. This may be changed in the future, so add a
new get_cache_alignment() function pointer in 'struct dma_map_ops' as a
generic solution.

For compatibility (always return ARCH_DMA_MINALIGN), make all existing
callers pass a NULL dev argument to dma_get_cache_alignment().

Cc: sta...@vger.kernel.org
Signed-off-by: Huacai Chen 
---
 arch/mips/cavium-octeon/dma-octeon.c   |  3 ++-
 arch/mips/include/asm/dma-mapping.h|  2 ++
 arch/mips/loongson64/common/dma-swiotlb.c  |  1 +
 arch/mips/mm/dma-default.c | 11 ++-
 arch/mips/netlogic/common/nlm-dma.c|  3 ++-
 drivers/infiniband/hw/mthca/mthca_main.c   |  2 +-
 drivers/media/v4l2-core/videobuf2-dma-contig.c |  2 +-
 drivers/net/ethernet/broadcom/b44.c|  2 +-
 drivers/net/ethernet/ibm/emac/core.h   |  2 +-
 drivers/net/ethernet/mellanox/mlx4/main.c  |  2 +-
 drivers/spi/spi-qup.c  |  4 ++--
 drivers/tty/serial/mpsc.c  | 16 
 drivers/tty/serial/samsung.c   | 14 +++---
 include/linux/dma-mapping.h| 17 -
 14 files changed, 51 insertions(+), 30 deletions(-)

diff --git a/arch/mips/cavium-octeon/dma-octeon.c 
b/arch/mips/cavium-octeon/dma-octeon.c
index c64bd87..7978237 100644
--- a/arch/mips/cavium-octeon/dma-octeon.c
+++ b/arch/mips/cavium-octeon/dma-octeon.c
@@ -324,7 +324,8 @@ static struct octeon_dma_map_ops _octeon_pci_dma_map_ops = {
.sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
.sync_sg_for_device = octeon_dma_sync_sg_for_device,
.mapping_error = swiotlb_dma_mapping_error,
-   .dma_supported = swiotlb_dma_supported
+   .dma_supported = swiotlb_dma_supported,
+   .get_cache_alignment = mips_get_cache_alignment
},
 };
 
diff --git a/arch/mips/include/asm/dma-mapping.h 
b/arch/mips/include/asm/dma-mapping.h
index aba7138..e2c5d9e 100644
--- a/arch/mips/include/asm/dma-mapping.h
+++ b/arch/mips/include/asm/dma-mapping.h
@@ -39,4 +39,6 @@ static inline void arch_setup_dma_ops(struct device *dev, u64 
dma_base,
 #endif
 }
 
+int mips_get_cache_alignment(struct device *dev);
+
 #endif /* _ASM_DMA_MAPPING_H */
diff --git a/arch/mips/loongson64/common/dma-swiotlb.c 
b/arch/mips/loongson64/common/dma-swiotlb.c
index 34486c1..09cb8a4 100644
--- a/arch/mips/loongson64/common/dma-swiotlb.c
+++ b/arch/mips/loongson64/common/dma-swiotlb.c
@@ -119,6 +119,7 @@ static const struct dma_map_ops loongson_dma_map_ops = {
.sync_sg_for_device = loongson_dma_sync_sg_for_device,
.mapping_error = swiotlb_dma_mapping_error,
.dma_supported = loongson_dma_supported,
+   .get_cache_alignment = mips_get_cache_alignment
 };
 
 void __init plat_swiotlb_setup(void)
diff --git a/arch/mips/mm/dma-default.c b/arch/mips/mm/dma-default.c
index c01bd20..c49987e 100644
--- a/arch/mips/mm/dma-default.c
+++ b/arch/mips/mm/dma-default.c
@@ -394,6 +394,14 @@ void dma_cache_sync(struct device *dev, void *vaddr, 
size_t size,
 
 EXPORT_SYMBOL(dma_cache_sync);
 
+int mips_get_cache_alignment(struct device *dev)
+{
+   if (plat_device_is_coherent(dev))
+   return 1;
+   else
+   return ARCH_DMA_MINALIGN;
+}
+
 static const struct dma_map_ops mips_default_dma_map_ops = {
.alloc = mips_dma_alloc_coherent,
.free = mips_dma_free_coherent,
@@ -407,7 +415,8 @@ static const struct dma_map_ops mips_default_dma_map_ops = {
.sync_sg_for_cpu = mips_dma_sync_sg_for_cpu,
.sync_sg_for_device = mips_dma_sync_sg_for_device,
.mapping_error = mips_dma_mapping_error,
-   .dma_supported = mips_dma_supported
+   .dma_supported = mips_dma_supported,
+   .get_cache_alignment = mips_get_cache_alignment
 };
 
 const struct dma_map_ops *mips_dma_map_ops = _default_dma_map_ops;
diff --git a/arch/mips/netlogic/common/nlm-dma.c 
b/arch/mips/netlogic/common/nlm-dma.c
index 0ec9d9d..1e107ac 100644
--- a/arch/mips/netlogic/common/nlm-dma.c
+++ b/arch/mips/netlogic/common/nlm-dma.c
@@ -79,7 +79,8 @@ const struct dma_map_ops nlm_swiotlb_dma_ops = {
.sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
.sync_sg_for_device = swiotlb_sync_sg_for_device,
.mapping_error = swiotlb_dma_mapping_error,
-   .dma_supported = swiotlb_dma_supported
+   .dma_supported = swiotlb_dma_supported,
+   .get_cache_alignment = mips_get_cache_alignment
 };
 
 void __init plat_swiotlb_setup(void)
diff --git a/drivers/infiniband/hw/mthca/mthca_main.c 
b/drivers/infiniband/hw/mthca/mthca_main.c
index e36a9bc..cac5fac 100644
--- a/drivers/infiniband/hw/mthca/mthca_main.c
+++

[PATCH V7 1/2] dma-mapping: Rework dma_get_cache_alignment()

2017-09-25 Thread Huacai Chen 
Make dma_get_cache_alignment() to accept a 'dev' argument. As a result,
it can return different alignments due to different devices' I/O cache
coherency.

Currently, MIPS is the only architecture which support coherent & non-
coherent devices co-exist. This may be changed in the future, so add a
new get_cache_alignment() function pointer in 'struct dma_map_ops' as a
generic solution.

For compatibility (always return ARCH_DMA_MINALIGN), make all existing
callers pass a NULL dev argument to dma_get_cache_alignment().

Cc: sta...@vger.kernel.org
Signed-off-by: Huacai Chen 
---
 arch/mips/cavium-octeon/dma-octeon.c   |  3 ++-
 arch/mips/include/asm/dma-mapping.h|  2 ++
 arch/mips/loongson64/common/dma-swiotlb.c  |  1 +
 arch/mips/mm/dma-default.c | 11 ++-
 arch/mips/netlogic/common/nlm-dma.c|  3 ++-
 drivers/infiniband/hw/mthca/mthca_main.c   |  2 +-
 drivers/media/v4l2-core/videobuf2-dma-contig.c |  2 +-
 drivers/net/ethernet/broadcom/b44.c|  2 +-
 drivers/net/ethernet/ibm/emac/core.h   |  2 +-
 drivers/net/ethernet/mellanox/mlx4/main.c  |  2 +-
 drivers/spi/spi-qup.c  |  4 ++--
 drivers/tty/serial/mpsc.c  | 16 
 drivers/tty/serial/samsung.c   | 14 +++---
 include/linux/dma-mapping.h| 17 -
 14 files changed, 51 insertions(+), 30 deletions(-)

diff --git a/arch/mips/cavium-octeon/dma-octeon.c 
b/arch/mips/cavium-octeon/dma-octeon.c
index c64bd87..7978237 100644
--- a/arch/mips/cavium-octeon/dma-octeon.c
+++ b/arch/mips/cavium-octeon/dma-octeon.c
@@ -324,7 +324,8 @@ static struct octeon_dma_map_ops _octeon_pci_dma_map_ops = {
.sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
.sync_sg_for_device = octeon_dma_sync_sg_for_device,
.mapping_error = swiotlb_dma_mapping_error,
-   .dma_supported = swiotlb_dma_supported
+   .dma_supported = swiotlb_dma_supported,
+   .get_cache_alignment = mips_get_cache_alignment
},
 };
 
diff --git a/arch/mips/include/asm/dma-mapping.h 
b/arch/mips/include/asm/dma-mapping.h
index aba7138..e2c5d9e 100644
--- a/arch/mips/include/asm/dma-mapping.h
+++ b/arch/mips/include/asm/dma-mapping.h
@@ -39,4 +39,6 @@ static inline void arch_setup_dma_ops(struct device *dev, u64 
dma_base,
 #endif
 }
 
+int mips_get_cache_alignment(struct device *dev);
+
 #endif /* _ASM_DMA_MAPPING_H */
diff --git a/arch/mips/loongson64/common/dma-swiotlb.c 
b/arch/mips/loongson64/common/dma-swiotlb.c
index 34486c1..09cb8a4 100644
--- a/arch/mips/loongson64/common/dma-swiotlb.c
+++ b/arch/mips/loongson64/common/dma-swiotlb.c
@@ -119,6 +119,7 @@ static const struct dma_map_ops loongson_dma_map_ops = {
.sync_sg_for_device = loongson_dma_sync_sg_for_device,
.mapping_error = swiotlb_dma_mapping_error,
.dma_supported = loongson_dma_supported,
+   .get_cache_alignment = mips_get_cache_alignment
 };
 
 void __init plat_swiotlb_setup(void)
diff --git a/arch/mips/mm/dma-default.c b/arch/mips/mm/dma-default.c
index c01bd20..c49987e 100644
--- a/arch/mips/mm/dma-default.c
+++ b/arch/mips/mm/dma-default.c
@@ -394,6 +394,14 @@ void dma_cache_sync(struct device *dev, void *vaddr, 
size_t size,
 
 EXPORT_SYMBOL(dma_cache_sync);
 
+int mips_get_cache_alignment(struct device *dev)
+{
+   if (plat_device_is_coherent(dev))
+   return 1;
+   else
+   return ARCH_DMA_MINALIGN;
+}
+
 static const struct dma_map_ops mips_default_dma_map_ops = {
.alloc = mips_dma_alloc_coherent,
.free = mips_dma_free_coherent,
@@ -407,7 +415,8 @@ static const struct dma_map_ops mips_default_dma_map_ops = {
.sync_sg_for_cpu = mips_dma_sync_sg_for_cpu,
.sync_sg_for_device = mips_dma_sync_sg_for_device,
.mapping_error = mips_dma_mapping_error,
-   .dma_supported = mips_dma_supported
+   .dma_supported = mips_dma_supported,
+   .get_cache_alignment = mips_get_cache_alignment
 };
 
 const struct dma_map_ops *mips_dma_map_ops = _default_dma_map_ops;
diff --git a/arch/mips/netlogic/common/nlm-dma.c 
b/arch/mips/netlogic/common/nlm-dma.c
index 0ec9d9d..1e107ac 100644
--- a/arch/mips/netlogic/common/nlm-dma.c
+++ b/arch/mips/netlogic/common/nlm-dma.c
@@ -79,7 +79,8 @@ const struct dma_map_ops nlm_swiotlb_dma_ops = {
.sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
.sync_sg_for_device = swiotlb_sync_sg_for_device,
.mapping_error = swiotlb_dma_mapping_error,
-   .dma_supported = swiotlb_dma_supported
+   .dma_supported = swiotlb_dma_supported,
+   .get_cache_alignment = mips_get_cache_alignment
 };
 
 void __init plat_swiotlb_setup(void)
diff --git a/drivers/infiniband/hw/mthca/mthca_main.c 
b/drivers/infiniband/hw/mthca/mthca_main.c
index e36a9bc..cac5fac 100644
--- a/drivers/infiniband/hw/mthca/mthca_main.c
+++