diff mbox

[-mm] ssb: open-code dma_alloc_coherent

Message ID 20100215142111R.fujita.tomonori@lab.ntt.co.jp
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

FUJITA Tomonori Feb. 15, 2010, 5:21 a.m. UTC
This patch is against -mm since it depends on the DMA API changes in
-mm.

=
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
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 <fujita.tomonori@lab.ntt.co.jp>
Cc: Michael Buesch <mb@bu3sch.de>
Cc: Gary Zambrano <zambrano@broadcom.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Stefano Brivio <stefano.brivio@polimi.it>
Cc: Larry Finger <Larry.Finger@lwfinger.net>
---
 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(-)

Comments

David Miller Feb. 15, 2010, 6:24 a.m. UTC | #1
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Date: Mon, 15 Feb 2010 14:21:58 +0900

> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> 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 <fujita.tomonori@lab.ntt.co.jp>

I'm fine with this:

Acked-by: David S. Miller <davem@davemloft.net>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Larry Finger Feb. 15, 2010, 6:49 a.m. UTC | #2
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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
FUJITA Tomonori Feb. 15, 2010, 7:25 a.m. UTC | #3
On Mon, 15 Feb 2010 00:49:14 -0600
Larry Finger <Larry.Finger@lwfinger.net> 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!
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Buesch Feb. 15, 2010, 11:03 a.m. UTC | #4
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).
FUJITA Tomonori Feb. 15, 2010, 11:18 a.m. UTC | #5
On Mon, 15 Feb 2010 12:03:41 +0100
Michael Buesch <mb@bu3sch.de> 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,
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Buesch Feb. 15, 2010, 11:21 a.m. UTC | #6
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*
Larry Finger Feb. 16, 2010, 12:09 a.m. UTC | #7
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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
FUJITA Tomonori Feb. 16, 2010, 2:05 a.m. UTC | #8
On Mon, 15 Feb 2010 18:09:53 -0600
Larry Finger <Larry.Finger@lwfinger.net> 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,
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Larry Finger Feb. 16, 2010, 7:12 p.m. UTC | #9
On 02/15/2010 08:05 PM, FUJITA Tomonori wrote:
> On Mon, 15 Feb 2010 18:09:53 -0600
> Larry Finger <Larry.Finger@lwfinger.net> 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 <Larry.Finger@lwfinger.net>

Larry

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;