2017-08-26 07:21:39

by Christoph Hellwig

[permalink] [raw]
Subject: remove dma_alloc_noncoherent V2

For many years we've had the dma_alloc_attrs API that is more flexible
than dma_alloc_noncoherent. This series moves the remaining users over
to the attrs API.

After half of the series went in for the last merge window I'd really
like to merge the remainer. Any chance to get some ACKs/reviews for
the net drivers?


2017-08-26 07:21:40

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 1/4] sgiseeq: switch to dma_alloc_attrs

Use dma_alloc_attrs directly instead of the dma_alloc_noncoherent wrapper.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/net/ethernet/seeq/sgiseeq.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/seeq/sgiseeq.c b/drivers/net/ethernet/seeq/sgiseeq.c
index 70347720fdf9..573691bc3b71 100644
--- a/drivers/net/ethernet/seeq/sgiseeq.c
+++ b/drivers/net/ethernet/seeq/sgiseeq.c
@@ -737,8 +737,8 @@ static int sgiseeq_probe(struct platform_device *pdev)
sp = netdev_priv(dev);

/* Make private data page aligned */
- sr = dma_alloc_noncoherent(&pdev->dev, sizeof(*sp->srings),
- &sp->srings_dma, GFP_KERNEL);
+ sr = dma_alloc_attrs(&pdev->dev, sizeof(*sp->srings), &sp->srings_dma,
+ GFP_KERNEL, DMA_ATTR_NON_CONSISTENT);
if (!sr) {
printk(KERN_ERR "Sgiseeq: Page alloc failed, aborting.\n");
err = -ENOMEM;
@@ -813,8 +813,8 @@ static int sgiseeq_remove(struct platform_device *pdev)
struct sgiseeq_private *sp = netdev_priv(dev);

unregister_netdev(dev);
- dma_free_noncoherent(&pdev->dev, sizeof(*sp->srings), sp->srings,
- sp->srings_dma);
+ dma_free_attrs(&pdev->dev, sizeof(*sp->srings), sp->srings,
+ sp->srings_dma, DMA_ATTR_NON_CONSISTENT);
free_netdev(dev);

return 0;
--
2.11.0

2017-08-26 07:21:44

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 2/4] au1000_eth: switch to dma_alloc_attrs

Use dma_alloc_attrs directly instead of the dma_alloc_noncoherent wrapper.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/net/ethernet/amd/au1000_eth.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/amd/au1000_eth.c b/drivers/net/ethernet/amd/au1000_eth.c
index a3c90fe5de00..73ca8879ada7 100644
--- a/drivers/net/ethernet/amd/au1000_eth.c
+++ b/drivers/net/ethernet/amd/au1000_eth.c
@@ -1180,9 +1180,10 @@ static int au1000_probe(struct platform_device *pdev)
/* Allocate the data buffers
* Snooping works fine with eth on all au1xxx
*/
- aup->vaddr = (u32)dma_alloc_noncoherent(NULL, MAX_BUF_SIZE *
- (NUM_TX_BUFFS + NUM_RX_BUFFS),
- &aup->dma_addr, 0);
+ aup->vaddr = (u32)dma_alloc_attrs(NULL, MAX_BUF_SIZE *
+ (NUM_TX_BUFFS + NUM_RX_BUFFS),
+ &aup->dma_addr, 0,
+ DMA_ATTR_NON_CONSISTENT);
if (!aup->vaddr) {
dev_err(&pdev->dev, "failed to allocate data buffers\n");
err = -ENOMEM;
@@ -1361,8 +1362,9 @@ static int au1000_probe(struct platform_device *pdev)
err_remap2:
iounmap(aup->mac);
err_remap1:
- dma_free_noncoherent(NULL, MAX_BUF_SIZE * (NUM_TX_BUFFS + NUM_RX_BUFFS),
- (void *)aup->vaddr, aup->dma_addr);
+ dma_free_attrs(NULL, MAX_BUF_SIZE * (NUM_TX_BUFFS + NUM_RX_BUFFS),
+ (void *)aup->vaddr, aup->dma_addr,
+ DMA_ATTR_NON_CONSISTENT);
err_vaddr:
free_netdev(dev);
err_alloc:
@@ -1394,9 +1396,9 @@ static int au1000_remove(struct platform_device *pdev)
if (aup->tx_db_inuse[i])
au1000_ReleaseDB(aup, aup->tx_db_inuse[i]);

- dma_free_noncoherent(NULL, MAX_BUF_SIZE *
- (NUM_TX_BUFFS + NUM_RX_BUFFS),
- (void *)aup->vaddr, aup->dma_addr);
+ dma_free_attrs(NULL, MAX_BUF_SIZE * (NUM_TX_BUFFS + NUM_RX_BUFFS),
+ (void *)aup->vaddr, aup->dma_addr,
+ DMA_ATTR_NON_CONSISTENT);

iounmap(aup->macdma);
iounmap(aup->mac);
--
2.11.0

2017-08-26 07:21:51

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 3/4] i825xx: switch to switch to dma_alloc_attrs

This way we can always pass DMA_ATTR_NON_CONSISTENT, the SNI mips version
will simply ignore the flag.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/net/ethernet/i825xx/lasi_82596.c | 6 ++----
drivers/net/ethernet/i825xx/lib82596.c | 9 +++++----
drivers/net/ethernet/i825xx/sni_82596.c | 6 ++----
3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/i825xx/lasi_82596.c b/drivers/net/ethernet/i825xx/lasi_82596.c
index d787fdd5db7b..d5b5021aa759 100644
--- a/drivers/net/ethernet/i825xx/lasi_82596.c
+++ b/drivers/net/ethernet/i825xx/lasi_82596.c
@@ -96,8 +96,6 @@

#define OPT_SWAP_PORT 0x0001 /* Need to wordswp on the MPU port */

-#define DMA_ALLOC dma_alloc_noncoherent
-#define DMA_FREE dma_free_noncoherent
#define DMA_WBACK(ndev, addr, len) \
do { dma_cache_sync((ndev)->dev.parent, (void *)addr, len, DMA_TO_DEVICE); } while (0)

@@ -200,8 +198,8 @@ static int lan_remove_chip(struct parisc_device *pdev)
struct i596_private *lp = netdev_priv(dev);

unregister_netdev (dev);
- DMA_FREE(&pdev->dev, sizeof(struct i596_private),
- (void *)lp->dma, lp->dma_addr);
+ dma_free_attrs(&pdev->dev, sizeof(struct i596_private), lp->dma,
+ lp->dma_addr, DMA_ATTR_NON_CONSISTENT);
free_netdev (dev);
return 0;
}
diff --git a/drivers/net/ethernet/i825xx/lib82596.c b/drivers/net/ethernet/i825xx/lib82596.c
index 8449c58f01fd..f00a1dc2128c 100644
--- a/drivers/net/ethernet/i825xx/lib82596.c
+++ b/drivers/net/ethernet/i825xx/lib82596.c
@@ -1063,8 +1063,9 @@ static int i82596_probe(struct net_device *dev)
if (!dev->base_addr || !dev->irq)
return -ENODEV;

- dma = (struct i596_dma *) DMA_ALLOC(dev->dev.parent,
- sizeof(struct i596_dma), &lp->dma_addr, GFP_KERNEL);
+ dma = dma_alloc_attrs(dev->dev.parent, sizeof(struct i596_dma),
+ &lp->dma_addr, GFP_KERNEL,
+ DMA_ATTR_NON_CONSISTENT);
if (!dma) {
printk(KERN_ERR "%s: Couldn't get shared memory\n", __FILE__);
return -ENOMEM;
@@ -1085,8 +1086,8 @@ static int i82596_probe(struct net_device *dev)

i = register_netdev(dev);
if (i) {
- DMA_FREE(dev->dev.parent, sizeof(struct i596_dma),
- (void *)dma, lp->dma_addr);
+ dma_free_attrs(dev->dev.parent, sizeof(struct i596_dma),
+ dma, lp->dma_addr, DMA_ATTR_NON_CONSISTENT);
return i;
}

diff --git a/drivers/net/ethernet/i825xx/sni_82596.c b/drivers/net/ethernet/i825xx/sni_82596.c
index 2af7f77345fb..b2c04a789744 100644
--- a/drivers/net/ethernet/i825xx/sni_82596.c
+++ b/drivers/net/ethernet/i825xx/sni_82596.c
@@ -23,8 +23,6 @@

static const char sni_82596_string[] = "snirm_82596";

-#define DMA_ALLOC dma_alloc_coherent
-#define DMA_FREE dma_free_coherent
#define DMA_WBACK(priv, addr, len) do { } while (0)
#define DMA_INV(priv, addr, len) do { } while (0)
#define DMA_WBACK_INV(priv, addr, len) do { } while (0)
@@ -152,8 +150,8 @@ static int sni_82596_driver_remove(struct platform_device *pdev)
struct i596_private *lp = netdev_priv(dev);

unregister_netdev(dev);
- DMA_FREE(dev->dev.parent, sizeof(struct i596_private),
- lp->dma, lp->dma_addr);
+ dma_free_attrs(dev->dev.parent, sizeof(struct i596_private), lp->dma,
+ lp->dma_addr, DMA_ATTR_NON_CONSISTENT);
iounmap(lp->ca);
iounmap(lp->mpu_port);
free_netdev (dev);
--
2.11.0

2017-08-26 07:21:58

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 4/4] dma-mapping: remove dma_alloc_noncoherent and dma_free_noncoherent

No users left, everyone switched to the _attrs versions.

Signed-off-by: Christoph Hellwig <[email protected]>
---
Documentation/DMA-API.txt | 30 ++++++++++++++++--------------
arch/metag/include/asm/dma-mapping.h | 2 +-
arch/nios2/include/asm/dma-mapping.h | 2 +-
arch/tile/include/asm/dma-mapping.h | 4 ++--
include/linux/dma-mapping.h | 14 --------------
5 files changed, 20 insertions(+), 32 deletions(-)

diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index 45b29326d719..ef3a04fcad65 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -515,14 +515,15 @@ API at all.
::

void *
- dma_alloc_noncoherent(struct device *dev, size_t size,
- dma_addr_t *dma_handle, gfp_t flag)
+ dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
+ gfp_t flag, unsigned long attrs)

-Identical to dma_alloc_coherent() except that the platform will
-choose to return either consistent or non-consistent memory as it sees
-fit. By using this API, you are guaranteeing to the platform that you
-have all the correct and necessary sync points for this memory in the
-driver should it choose to return non-consistent memory.
+Identical to dma_alloc_coherent() except that when the
+DMA_ATTR_NON_CONSISTENT flags is passed in the attrs argument, the
+platform will choose to return either consistent or non-consistent memory
+as it sees fit. By using this API, you are guaranteeing to the platform
+that you have all the correct and necessary sync points for this memory
+in the driver should it choose to return non-consistent memory.

Note: where the platform can return consistent memory, it will
guarantee that the sync points become nops.
@@ -535,12 +536,13 @@ that simply cannot make consistent memory.
::

void
- dma_free_noncoherent(struct device *dev, size_t size, void *cpu_addr,
- dma_addr_t dma_handle)
+ dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
+ dma_addr_t dma_handle, unsigned long attrs)

-Free memory allocated by the nonconsistent API. All parameters must
-be identical to those passed in (and returned by
-dma_alloc_noncoherent()).
+Free memory allocated by the dma_alloc_attrs(). All parameters common
+parameters must identical to those otherwise passed to dma_fre_coherent,
+and the attrs argument must be identical to the attrs passed to
+dma_alloc_attrs().

::

@@ -564,8 +566,8 @@ memory or doing partial flushes.
dma_cache_sync(struct device *dev, void *vaddr, size_t size,
enum dma_data_direction direction)

-Do a partial sync of memory that was allocated by
-dma_alloc_noncoherent(), starting at virtual address vaddr and
+Do a partial sync of memory that was allocated by dma_alloc_attrs() with
+the DMA_ATTR_NON_CONSISTENT flag starting at virtual address vaddr and
continuing on for size. Again, you *must* observe the cache line
boundaries when doing this.

diff --git a/arch/metag/include/asm/dma-mapping.h b/arch/metag/include/asm/dma-mapping.h
index fad3dc3cb210..ea573be2b6d0 100644
--- a/arch/metag/include/asm/dma-mapping.h
+++ b/arch/metag/include/asm/dma-mapping.h
@@ -9,7 +9,7 @@ static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
}

/*
- * dma_alloc_noncoherent() returns non-cacheable memory, so there's no need to
+ * dma_alloc_attrs() always returns non-cacheable memory, so there's no need to
* do any flushing here.
*/
static inline void
diff --git a/arch/nios2/include/asm/dma-mapping.h b/arch/nios2/include/asm/dma-mapping.h
index 7b3c6f280293..f8dc62222741 100644
--- a/arch/nios2/include/asm/dma-mapping.h
+++ b/arch/nios2/include/asm/dma-mapping.h
@@ -18,7 +18,7 @@ static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
}

/*
- * dma_alloc_noncoherent() returns non-cacheable memory, so there's no need to
+ * dma_alloc_attrs() always returns non-cacheable memory, so there's no need to
* do any flushing here.
*/
static inline void dma_cache_sync(struct device *dev, void *vaddr, size_t size,
diff --git a/arch/tile/include/asm/dma-mapping.h b/arch/tile/include/asm/dma-mapping.h
index bbc71a29b2c6..7061dc8af43a 100644
--- a/arch/tile/include/asm/dma-mapping.h
+++ b/arch/tile/include/asm/dma-mapping.h
@@ -68,8 +68,8 @@ static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
int dma_set_mask(struct device *dev, u64 mask);

/*
- * dma_alloc_noncoherent() is #defined to return coherent memory,
- * so there's no need to do any flushing here.
+ * dma_alloc_attrs() always returns non-cacheable memory, so there's no need to
+ * do any flushing here.
*/
static inline void dma_cache_sync(struct device *dev, void *vaddr, size_t size,
enum dma_data_direction direction)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 66d8ea68f40b..4c98cc96971f 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -549,20 +549,6 @@ static inline void dma_free_coherent(struct device *dev, size_t size,
return dma_free_attrs(dev, size, cpu_addr, dma_handle, 0);
}

-static inline void *dma_alloc_noncoherent(struct device *dev, size_t size,
- dma_addr_t *dma_handle, gfp_t gfp)
-{
- return dma_alloc_attrs(dev, size, dma_handle, gfp,
- DMA_ATTR_NON_CONSISTENT);
-}
-
-static inline void dma_free_noncoherent(struct device *dev, size_t size,
- void *cpu_addr, dma_addr_t dma_handle)
-{
- dma_free_attrs(dev, size, cpu_addr, dma_handle,
- DMA_ATTR_NON_CONSISTENT);
-}
-
static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
{
const struct dma_map_ops *ops = get_dma_ops(dev);
--
2.11.0

2017-08-26 13:07:16

by Ralf Baechle

[permalink] [raw]
Subject: Re: [PATCH 1/4] sgiseeq: switch to dma_alloc_attrs

On Sat, Aug 26, 2017 at 09:21:22AM +0200, Christoph Hellwig wrote:

Looks good,

Acked-by: Ralf Baechle <[email protected]>

Ralf

2017-08-26 13:10:48

by Ralf Baechle

[permalink] [raw]
Subject: Re: [PATCH 3/4] i825xx: switch to switch to dma_alloc_attrs

On Sat, Aug 26, 2017 at 09:21:24AM +0200, Christoph Hellwig wrote:

Adding Thomas Bogendoerfer <[email protected]>, the author of
sni_82596.c to cc.

> This way we can always pass DMA_ATTR_NON_CONSISTENT, the SNI mips version
> will simply ignore the flag.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> drivers/net/ethernet/i825xx/lasi_82596.c | 6 ++----
> drivers/net/ethernet/i825xx/lib82596.c | 9 +++++----
> drivers/net/ethernet/i825xx/sni_82596.c | 6 ++----
> 3 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/i825xx/lasi_82596.c b/drivers/net/ethernet/i825xx/lasi_82596.c
> index d787fdd5db7b..d5b5021aa759 100644
> --- a/drivers/net/ethernet/i825xx/lasi_82596.c
> +++ b/drivers/net/ethernet/i825xx/lasi_82596.c
> @@ -96,8 +96,6 @@
>
> #define OPT_SWAP_PORT 0x0001 /* Need to wordswp on the MPU port */
>
> -#define DMA_ALLOC dma_alloc_noncoherent
> -#define DMA_FREE dma_free_noncoherent
> #define DMA_WBACK(ndev, addr, len) \
> do { dma_cache_sync((ndev)->dev.parent, (void *)addr, len, DMA_TO_DEVICE); } while (0)
>
> @@ -200,8 +198,8 @@ static int lan_remove_chip(struct parisc_device *pdev)
> struct i596_private *lp = netdev_priv(dev);
>
> unregister_netdev (dev);
> - DMA_FREE(&pdev->dev, sizeof(struct i596_private),
> - (void *)lp->dma, lp->dma_addr);
> + dma_free_attrs(&pdev->dev, sizeof(struct i596_private), lp->dma,
> + lp->dma_addr, DMA_ATTR_NON_CONSISTENT);
> free_netdev (dev);
> return 0;
> }
> diff --git a/drivers/net/ethernet/i825xx/lib82596.c b/drivers/net/ethernet/i825xx/lib82596.c
> index 8449c58f01fd..f00a1dc2128c 100644
> --- a/drivers/net/ethernet/i825xx/lib82596.c
> +++ b/drivers/net/ethernet/i825xx/lib82596.c
> @@ -1063,8 +1063,9 @@ static int i82596_probe(struct net_device *dev)
> if (!dev->base_addr || !dev->irq)
> return -ENODEV;
>
> - dma = (struct i596_dma *) DMA_ALLOC(dev->dev.parent,
> - sizeof(struct i596_dma), &lp->dma_addr, GFP_KERNEL);
> + dma = dma_alloc_attrs(dev->dev.parent, sizeof(struct i596_dma),
> + &lp->dma_addr, GFP_KERNEL,
> + DMA_ATTR_NON_CONSISTENT);
> if (!dma) {
> printk(KERN_ERR "%s: Couldn't get shared memory\n", __FILE__);
> return -ENOMEM;
> @@ -1085,8 +1086,8 @@ static int i82596_probe(struct net_device *dev)
>
> i = register_netdev(dev);
> if (i) {
> - DMA_FREE(dev->dev.parent, sizeof(struct i596_dma),
> - (void *)dma, lp->dma_addr);
> + dma_free_attrs(dev->dev.parent, sizeof(struct i596_dma),
> + dma, lp->dma_addr, DMA_ATTR_NON_CONSISTENT);
> return i;
> }
>
> diff --git a/drivers/net/ethernet/i825xx/sni_82596.c b/drivers/net/ethernet/i825xx/sni_82596.c
> index 2af7f77345fb..b2c04a789744 100644
> --- a/drivers/net/ethernet/i825xx/sni_82596.c
> +++ b/drivers/net/ethernet/i825xx/sni_82596.c
> @@ -23,8 +23,6 @@
>
> static const char sni_82596_string[] = "snirm_82596";
>
> -#define DMA_ALLOC dma_alloc_coherent
> -#define DMA_FREE dma_free_coherent
> #define DMA_WBACK(priv, addr, len) do { } while (0)
> #define DMA_INV(priv, addr, len) do { } while (0)
> #define DMA_WBACK_INV(priv, addr, len) do { } while (0)
> @@ -152,8 +150,8 @@ static int sni_82596_driver_remove(struct platform_device *pdev)
> struct i596_private *lp = netdev_priv(dev);
>
> unregister_netdev(dev);
> - DMA_FREE(dev->dev.parent, sizeof(struct i596_private),
> - lp->dma, lp->dma_addr);
> + dma_free_attrs(dev->dev.parent, sizeof(struct i596_private), lp->dma,
> + lp->dma_addr, DMA_ATTR_NON_CONSISTENT);
> iounmap(lp->ca);
> iounmap(lp->mpu_port);
> free_netdev (dev);
> --
> 2.11.0

2017-08-28 22:41:54

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 1/4] sgiseeq: switch to dma_alloc_attrs

From: Christoph Hellwig <[email protected]>
Date: Sat, 26 Aug 2017 09:21:22 +0200

> Use dma_alloc_attrs directly instead of the dma_alloc_noncoherent wrapper.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Acked-by: David S. Miller <[email protected]>

2017-08-28 22:42:07

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 2/4] au1000_eth: switch to dma_alloc_attrs

From: Christoph Hellwig <[email protected]>
Date: Sat, 26 Aug 2017 09:21:23 +0200

> Use dma_alloc_attrs directly instead of the dma_alloc_noncoherent wrapper.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Acked-by: David S. Miller <[email protected]>

2017-08-28 22:42:24

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 3/4] i825xx: switch to switch to dma_alloc_attrs

From: Christoph Hellwig <[email protected]>
Date: Sat, 26 Aug 2017 09:21:24 +0200

> This way we can always pass DMA_ATTR_NON_CONSISTENT, the SNI mips version
> will simply ignore the flag.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Acked-by: David S. Miller <[email protected]>

2017-08-29 08:02:49

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/4] sgiseeq: switch to dma_alloc_attrs

On Mon, Aug 28, 2017 at 03:41:51PM -0700, David Miller wrote:
> From: Christoph Hellwig <[email protected]>
> Date: Sat, 26 Aug 2017 09:21:22 +0200
>
> > Use dma_alloc_attrs directly instead of the dma_alloc_noncoherent wrapper.
> >
> > Signed-off-by: Christoph Hellwig <[email protected]>
>
> Acked-by: David S. Miller <[email protected]>

I take the Acks as an ok to merges these patches through the dma-mapping
tree. Thanks Dave!