2010-02-15 05:22:57

by FUJITA Tomonori

[permalink] [raw]
Subject: [PATCH -mm] ssb: open-code dma_alloc_coherent

This patch is against -mm since it depends on the DMA API changes in
-mm.

=
From: FUJITA Tomonori <[email protected]>
Subject: [PATCH -mm] ssb: open-code dma_alloc_coherent

This removes ssb_dma_alloc_consistent, a wrapper for
dma_alloc_coherent and pci_alloc_consistent. dma_alloc_coherent
internally calls pci_alloc_consistent against pci devices so we always
use dma_alloc_coherent.

I prefer to use the DMA API directly instead of creating the ssb_dma_
API. We have been removing any bus specific DMA APIs.

One disadvantage of this approach that we lost the debugging ability
of checking the buggy usage of the DMA API against non DMA-capable ssb
buses. But I guess that the ssb drivers are mature enough and can cope
with such change?

I plan to replace all the ssb_dma API if this approach is fine.

Signed-off-by: FUJITA Tomonori <[email protected]>
Cc: Michael Buesch <[email protected]>
Cc: Gary Zambrano <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Stefano Brivio <[email protected]>
Cc: Larry Finger <[email protected]>
---
drivers/net/b44.c | 4 ++--
drivers/net/wireless/b43/dma.c | 6 +++---
drivers/net/wireless/b43legacy/dma.c | 8 ++++----
drivers/ssb/main.c | 25 ++-----------------------
include/linux/ssb/ssb.h | 2 +-
5 files changed, 12 insertions(+), 33 deletions(-)

diff --git a/drivers/net/b44.c b/drivers/net/b44.c
index 16e760b..dbe7480 100644
--- a/drivers/net/b44.c
+++ b/drivers/net/b44.c
@@ -1182,7 +1182,7 @@ static int b44_alloc_consistent(struct b44 *bp, gfp_t gfp)
goto out_err;

size = DMA_TABLE_BYTES;
- bp->rx_ring = ssb_dma_alloc_consistent(bp->sdev, size, &bp->rx_ring_dma, gfp);
+ bp->rx_ring = dma_alloc_coherent(bp->sdev->dma_dev, size, &bp->rx_ring_dma, gfp);
if (!bp->rx_ring) {
/* Allocation may have failed due to pci_alloc_consistent
insisting on use of GFP_DMA, which is more restrictive
@@ -1209,7 +1209,7 @@ static int b44_alloc_consistent(struct b44 *bp, gfp_t gfp)
bp->flags |= B44_FLAG_RX_RING_HACK;
}

- bp->tx_ring = ssb_dma_alloc_consistent(bp->sdev, size, &bp->tx_ring_dma, gfp);
+ bp->tx_ring = dma_alloc_coherent(bp->sdev->dma_dev, size, &bp->tx_ring_dma, gfp);
if (!bp->tx_ring) {
/* Allocation may have failed due to ssb_dma_alloc_consistent
insisting on use of GFP_DMA, which is more restrictive
diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
index be7abf8..b5e00dd 100644
--- a/drivers/net/wireless/b43/dma.c
+++ b/drivers/net/wireless/b43/dma.c
@@ -400,9 +400,9 @@ static int alloc_ringmemory(struct b43_dmaring *ring)
*/
if (ring->type == B43_DMA_64BIT)
flags |= GFP_DMA;
- ring->descbase = ssb_dma_alloc_consistent(ring->dev->dev,
- B43_DMA_RINGMEMSIZE,
- &(ring->dmabase), flags);
+ ring->descbase = dma_alloc_coherent(ring->dev->dev->dma_dev,
+ B43_DMA_RINGMEMSIZE,
+ &(ring->dmabase), flags);
if (!ring->descbase) {
b43err(ring->dev->wl, "DMA ringmemory allocation failed\n");
return -ENOMEM;
diff --git a/drivers/net/wireless/b43legacy/dma.c b/drivers/net/wireless/b43legacy/dma.c
index 8b9387c..4942f10 100644
--- a/drivers/net/wireless/b43legacy/dma.c
+++ b/drivers/net/wireless/b43legacy/dma.c
@@ -459,10 +459,10 @@ void free_descriptor_buffer(struct b43legacy_dmaring *ring,
static int alloc_ringmemory(struct b43legacy_dmaring *ring)
{
/* GFP flags must match the flags in free_ringmemory()! */
- ring->descbase = ssb_dma_alloc_consistent(ring->dev->dev,
- B43legacy_DMA_RINGMEMSIZE,
- &(ring->dmabase),
- GFP_KERNEL);
+ ring->descbase = dma_alloc_coherent(ring->dev->dev->dma_dev,
+ B43legacy_DMA_RINGMEMSIZE,
+ &(ring->dmabase),
+ GFP_KERNEL);
if (!ring->descbase) {
b43legacyerr(ring->dev->wl, "DMA ringmemory allocation"
" failed\n");
diff --git a/drivers/ssb/main.c b/drivers/ssb/main.c
index 03dfd27..d823041 100644
--- a/drivers/ssb/main.c
+++ b/drivers/ssb/main.c
@@ -485,6 +485,7 @@ static int ssb_devices_register(struct ssb_bus *bus)
#ifdef CONFIG_SSB_PCIHOST
sdev->irq = bus->host_pci->irq;
dev->parent = &bus->host_pci->dev;
+ sdev->dma_dev = dev->parent;
#endif
break;
case SSB_BUSTYPE_PCMCIA:
@@ -500,6 +501,7 @@ static int ssb_devices_register(struct ssb_bus *bus)
break;
case SSB_BUSTYPE_SSB:
dev->dma_mask = &dev->coherent_dma_mask;
+ sdev->dma_dev = dev;
break;
}

@@ -1246,29 +1248,6 @@ int ssb_dma_set_mask(struct ssb_device *dev, u64 mask)
}
EXPORT_SYMBOL(ssb_dma_set_mask);

-void * ssb_dma_alloc_consistent(struct ssb_device *dev, size_t size,
- dma_addr_t *dma_handle, gfp_t gfp_flags)
-{
- switch (dev->bus->bustype) {
- case SSB_BUSTYPE_PCI:
-#ifdef CONFIG_SSB_PCIHOST
- if (gfp_flags & GFP_DMA) {
- /* Workaround: The PCI API does not support passing
- * a GFP flag. */
- return dma_alloc_coherent(&dev->bus->host_pci->dev,
- size, dma_handle, gfp_flags);
- }
- return pci_alloc_consistent(dev->bus->host_pci, size, dma_handle);
-#endif
- case SSB_BUSTYPE_SSB:
- return dma_alloc_coherent(dev->dev, size, dma_handle, gfp_flags);
- default:
- __ssb_dma_not_implemented(dev);
- }
- return NULL;
-}
-EXPORT_SYMBOL(ssb_dma_alloc_consistent);
-
void ssb_dma_free_consistent(struct ssb_device *dev, size_t size,
void *vaddr, dma_addr_t dma_handle,
gfp_t gfp_flags)
diff --git a/include/linux/ssb/ssb.h b/include/linux/ssb/ssb.h
index e99c5d3..4604ee5 100644
--- a/include/linux/ssb/ssb.h
+++ b/include/linux/ssb/ssb.h
@@ -167,7 +167,7 @@ struct ssb_device {
* is an optimization. */
const struct ssb_bus_ops *ops;

- struct device *dev;
+ struct device *dev, *dma_dev;

struct ssb_bus *bus;
struct ssb_device_id id;
--
1.6.5


2010-02-15 06:24:33

by David Miller

[permalink] [raw]
Subject: Re: [PATCH -mm] ssb: open-code dma_alloc_coherent

From: FUJITA Tomonori <[email protected]>
Date: Mon, 15 Feb 2010 14:21:58 +0900

> From: FUJITA Tomonori <[email protected]>
> Subject: [PATCH -mm] ssb: open-code dma_alloc_coherent
>
> This removes ssb_dma_alloc_consistent, a wrapper for
> dma_alloc_coherent and pci_alloc_consistent. dma_alloc_coherent
> internally calls pci_alloc_consistent against pci devices so we always
> use dma_alloc_coherent.
>
> I prefer to use the DMA API directly instead of creating the ssb_dma_
> API. We have been removing any bus specific DMA APIs.
>
> One disadvantage of this approach that we lost the debugging ability
> of checking the buggy usage of the DMA API against non DMA-capable ssb
> buses. But I guess that the ssb drivers are mature enough and can cope
> with such change?
>
> I plan to replace all the ssb_dma API if this approach is fine.
>
> Signed-off-by: FUJITA Tomonori <[email protected]>

I'm fine with this:

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

2010-02-15 06:49:20

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH -mm] ssb: open-code dma_alloc_coherent

On 02/14/2010 11:21 PM, FUJITA Tomonori wrote:
> This patch is against -mm since it depends on the DMA API changes in
> -mm.

Are the patches that implement these DMA API changes available for testing? I
don't keep an -mm tree.

Larry

2010-02-15 07:26:01

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH -mm] ssb: open-code dma_alloc_coherent

On Mon, 15 Feb 2010 00:49:14 -0600
Larry Finger <[email protected]> wrote:

> On 02/14/2010 11:21 PM, FUJITA Tomonori wrote:
> > This patch is against -mm since it depends on the DMA API changes in
> > -mm.
>
> Are the patches that implement these DMA API changes available for testing? I
> don't keep an -mm tree.

All the patches are in -mm and I've just uploaded the patches against
2.6.33-rc8:

git://git.kernel.org/pub/scm/linux/kernel/git/tomo/linux-2.6-misc.git pending

The ssb patch depends on the latest eight patches in the tree:

http://marc.info/?l=linux-kernel&m=126596737604808&w=2



Thanks!

2010-02-15 10:09:50

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH -mm] ssb: open-code dma_alloc_coherent

On Mon, 15 Feb 2010 16:25:14 +0900
FUJITA Tomonori <[email protected]> wrote:

> On Mon, 15 Feb 2010 00:49:14 -0600
> Larry Finger <[email protected]> wrote:
>
> > On 02/14/2010 11:21 PM, FUJITA Tomonori wrote:
> > > This patch is against -mm since it depends on the DMA API changes in
> > > -mm.
> >
> > Are the patches that implement these DMA API changes available for testing? I
> > don't keep an -mm tree.
>
> All the patches are in -mm and I've just uploaded the patches against
> 2.6.33-rc8:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tomo/linux-2.6-misc.git pending
>
> The ssb patch depends on the latest eight patches in the tree:
>
> http://marc.info/?l=linux-kernel&m=126596737604808&w=2

This patch replaces the rest of the ssb_dma_* API in b43legacy with
the DMA API (it depends on the previous ssb patch).

=
From: FUJITA Tomonori <[email protected]>
Subject: [PATCH] b43legacy: replace the ssb_dma_* API with the DMA API

Replaces all the ssb_dma_* API with the DMA API.

Signed-off-by: FUJITA Tomonori <[email protected]>
---
drivers/net/wireless/b43legacy/dma.c | 34 +++++++++++++++++-----------------
1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/b43legacy/dma.c b/drivers/net/wireless/b43legacy/dma.c
index e7c0812..df85d93 100644
--- a/drivers/net/wireless/b43legacy/dma.c
+++ b/drivers/net/wireless/b43legacy/dma.c
@@ -393,11 +393,11 @@ dma_addr_t map_descbuffer(struct b43legacy_dmaring *ring,
dma_addr_t dmaaddr;

if (tx)
- dmaaddr = ssb_dma_map_single(ring->dev->dev,
+ dmaaddr = dma_map_single(ring->dev->dev->dma_dev,
buf, len,
DMA_TO_DEVICE);
else
- dmaaddr = ssb_dma_map_single(ring->dev->dev,
+ dmaaddr = dma_map_single(ring->dev->dev->dma_dev,
buf, len,
DMA_FROM_DEVICE);

@@ -411,11 +411,11 @@ void unmap_descbuffer(struct b43legacy_dmaring *ring,
int tx)
{
if (tx)
- ssb_dma_unmap_single(ring->dev->dev,
+ dma_unmap_single(ring->dev->dev->dma_dev,
addr, len,
DMA_TO_DEVICE);
else
- ssb_dma_unmap_single(ring->dev->dev,
+ dma_unmap_single(ring->dev->dev->dma_dev,
addr, len,
DMA_FROM_DEVICE);
}
@@ -427,8 +427,8 @@ void sync_descbuffer_for_cpu(struct b43legacy_dmaring *ring,
{
B43legacy_WARN_ON(ring->tx);

- ssb_dma_sync_single_for_cpu(ring->dev->dev,
- addr, len, DMA_FROM_DEVICE);
+ dma_sync_single_for_cpu(ring->dev->dev->dma_dev,
+ addr, len, DMA_FROM_DEVICE);
}

static inline
@@ -438,8 +438,8 @@ void sync_descbuffer_for_device(struct b43legacy_dmaring *ring,
{
B43legacy_WARN_ON(ring->tx);

- ssb_dma_sync_single_for_device(ring->dev->dev,
- addr, len, DMA_FROM_DEVICE);
+ dma_sync_single_for_device(ring->dev->dev->dma_dev,
+ addr, len, DMA_FROM_DEVICE);
}

static inline
@@ -475,8 +475,8 @@ static int alloc_ringmemory(struct b43legacy_dmaring *ring)

static void free_ringmemory(struct b43legacy_dmaring *ring)
{
- ssb_dma_free_consistent(ring->dev->dev, B43legacy_DMA_RINGMEMSIZE,
- ring->descbase, ring->dmabase, GFP_KERNEL);
+ dma_free_coherent(ring->dev->dev->dma_dev, B43legacy_DMA_RINGMEMSIZE,
+ ring->descbase, ring->dmabase);
}

/* Reset the RX DMA channel */
@@ -588,7 +588,7 @@ static bool b43legacy_dma_mapping_error(struct b43legacy_dmaring *ring,
size_t buffersize,
bool dma_to_device)
{
- if (unlikely(ssb_dma_mapping_error(ring->dev->dev, addr)))
+ if (unlikely(dma_mapping_error(ring->dev->dev->dma_dev, addr)))
return 1;

switch (ring->type) {
@@ -905,7 +905,7 @@ struct b43legacy_dmaring *b43legacy_setup_dmaring(struct b43legacy_wldev *dev,
goto err_kfree_meta;

/* test for ability to dma to txhdr_cache */
- dma_test = ssb_dma_map_single(dev->dev, ring->txhdr_cache,
+ dma_test = dma_map_single(dev->dev->dma_dev, ring->txhdr_cache,
sizeof(struct b43legacy_txhdr_fw3),
DMA_TO_DEVICE);

@@ -919,7 +919,7 @@ struct b43legacy_dmaring *b43legacy_setup_dmaring(struct b43legacy_wldev *dev,
if (!ring->txhdr_cache)
goto err_kfree_meta;

- dma_test = ssb_dma_map_single(dev->dev,
+ dma_test = dma_map_single(dev->dev->dma_dev,
ring->txhdr_cache,
sizeof(struct b43legacy_txhdr_fw3),
DMA_TO_DEVICE);
@@ -929,9 +929,9 @@ struct b43legacy_dmaring *b43legacy_setup_dmaring(struct b43legacy_wldev *dev,
goto err_kfree_txhdr_cache;
}

- ssb_dma_unmap_single(dev->dev, dma_test,
- sizeof(struct b43legacy_txhdr_fw3),
- DMA_TO_DEVICE);
+ dma_unmap_single(dev->dev->dma_dev, dma_test,
+ sizeof(struct b43legacy_txhdr_fw3),
+ DMA_TO_DEVICE);
}

ring->nr_slots = nr_slots;
@@ -1039,7 +1039,7 @@ static int b43legacy_dma_set_mask(struct b43legacy_wldev *dev, u64 mask)
/* Try to set the DMA mask. If it fails, try falling back to a
* lower mask, as we can always also support a lower one. */
while (1) {
- err = ssb_dma_set_mask(dev->dev, mask);
+ err = dma_set_mask(dev->dev->dma_dev, mask);
if (!err)
break;
if (mask == DMA_BIT_MASK(64)) {
--
1.6.5

2010-02-15 11:04:00

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH -mm] ssb: open-code dma_alloc_coherent

On Monday 15 February 2010 06:21:58 FUJITA Tomonori wrote:
> This patch is against -mm since it depends on the DMA API changes in

I appreciate your efforts to remove bus-specific APIs.
However, always keep in mind that a SSB bus might be running on a system
where there's no PCI bus at all.
In the past we used the generic dma_... API for DMA operations on all
SSB devices. This worked 99.9% of the time. _But_ it broke in really obscure
cases where the architecture implementations _slightly_ differed between
the generic dma and the pci variants.

So, I'm all for it. But simply keep in mind that this has to run on lots of
architectures (i386, x86_64, ppc, MIPS, probably more) and it has to run on
machines without a PCI bus (where SSB is the main system bus).

--
Greetings, Michael.

2010-02-15 11:19:31

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH -mm] ssb: open-code dma_alloc_coherent

On Mon, 15 Feb 2010 12:03:41 +0100
Michael Buesch <[email protected]> wrote:

> On Monday 15 February 2010 06:21:58 FUJITA Tomonori wrote:
> > This patch is against -mm since it depends on the DMA API changes in
>
> I appreciate your efforts to remove bus-specific APIs.
> However, always keep in mind that a SSB bus might be running on a system
> where there's no PCI bus at all.

Yup, that's why we introduced the generic device model and the generic
DMA API is better.


> In the past we used the generic dma_... API for DMA operations on all
> SSB devices. This worked 99.9% of the time. _But_ it broke in really obscure
> cases where the architecture implementations _slightly_ differed between
> the generic dma and the pci variants.

Yeah, I know. There are some differences between the PCI DMA API and
the generic DMA API on some architectures.

My patchset in -mm fixes the above problem:

http://marc.info/?l=linux-kernel&m=126596737604808&w=2

http://git.kernel.org/?p=linux/kernel/git/tomo/linux-2.6-misc.git;a=blob;f=include/asm-generic/pci-dma-compat.h;h=ddfa9c5309229d171af63dcaf3872288fb4fa845;hb=refs/heads/pending

The PCI DMA API simply calls the generic DMA API. And all the
architectures that supports pci uses asm-generic/pci-dma-compat.h.

You can use the generic DMA API with any buses. I'll remove the PCI
DMA API in the long term.


Thanks,

2010-02-15 11:21:12

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH -mm] ssb: open-code dma_alloc_coherent

On Monday 15 February 2010 12:18:41 FUJITA Tomonori wrote:
> You can use the generic DMA API with any buses. I'll remove the PCI
> DMA API in the long term.

Cool. Thanks a lot for cleaning up the mess. *thumbs up*

--
Greetings, Michael.

2010-02-16 00:09:58

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH -mm] ssb: open-code dma_alloc_coherent

On 02/15/2010 01:25 AM, FUJITA Tomonori wrote:
> 2.6.33-rc8:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tomo/linux-2.6-misc.git pending
>
> The ssb patch depends on the latest eight patches in the tree:
>
> http://marc.info/?l=linux-kernel&m=126596737604808&w=2

The DMA changes work on i386 architecture with wireless devices in Cardbus (PCI)
format. Both b43legacy and b43 were tested.

Larry

2010-02-16 02:05:53

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH -mm] ssb: open-code dma_alloc_coherent

On Mon, 15 Feb 2010 18:09:53 -0600
Larry Finger <[email protected]> wrote:

> On 02/15/2010 01:25 AM, FUJITA Tomonori wrote:
> > 2.6.33-rc8:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/tomo/linux-2.6-misc.git pending
> >
> > The ssb patch depends on the latest eight patches in the tree:
> >
> > http://marc.info/?l=linux-kernel&m=126596737604808&w=2
>
> The DMA changes work on i386 architecture with wireless devices in Cardbus (PCI)
> format. Both b43legacy and b43 were tested.

Great, thanks a lot!

I've posted the second version:

http://marc.info/?l=linux-kernel&m=126624677226865&w=2

It's also available via the git tree:

git://git.kernel.org/pub/scm/linux/kernel/git/tomo/linux-2.6-misc.git pending

Can I get your ack on b43 stuff?


Thanks,

2010-02-16 19:12:18

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH -mm] ssb: open-code dma_alloc_coherent

On 02/15/2010 08:05 PM, FUJITA Tomonori wrote:
> On Mon, 15 Feb 2010 18:09:53 -0600
> Larry Finger <[email protected]> wrote:
>
>> On 02/15/2010 01:25 AM, FUJITA Tomonori wrote:
>>> 2.6.33-rc8:
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/tomo/linux-2.6-misc.git pending
>>>
>>> The ssb patch depends on the latest eight patches in the tree:
>>>
>>> http://marc.info/?l=linux-kernel&m=126596737604808&w=2
>>
>> The DMA changes work on i386 architecture with wireless devices in Cardbus (PCI)
>> format. Both b43legacy and b43 were tested.
>
> Great, thanks a lot!
>
> I've posted the second version:
>
> http://marc.info/?l=linux-kernel&m=126624677226865&w=2
>
> It's also available via the git tree:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tomo/linux-2.6-misc.git pending
>
> Can I get your ack on b43 stuff?
>

Both b43 and b43legacy changes are

ACKed-by: Larry Finger <[email protected]>

Larry