[PATCH 0/4] staging: comedi: addi_apci_3120: redo DMA buffer allocation

[PATCH 0/4] staging: comedi: addi_apci_3120: redo DMA buffer allocation

[Ian Abbott]

"addi_apci_3120" allocates more pages of DMA buffer than it uses, may
allocate half a double-buffer it does not use because it's the "wrong"
half that it managed to allocate (unlikely), and relies on virt_to_bus()
to treat generic kernel memory from get_free_pages() as coherent DMA
memory.  Correct the issues, using dma_alloc_coherent() to allocate the
DMA buffers.

1) staging: comedi: addi_apci_3120: don't allocate 2nd DMA buffer on
   failure
2) staging: comedi: addi_apci_3120: don't overallocate DMA buffer
3) staging: comedi: addi_apci_3120: use dma_alloc_coherent()
4) staging: comedi: addi_apci_3120: simplify setting of
   devpriv->us_UseDma

 drivers/staging/comedi/Kconfig                     |  2 +-
 .../staging/comedi/drivers/addi-data/addi_common.h |  3 +-
 drivers/staging/comedi/drivers/addi_apci_3120.c    | 44 ++++++++++------------
 3 files changed, 22 insertions(+), 27 deletions(-)

[PATCH 0/4] staging: comedi: addi_apci_3120: redo DMA buffer allocation

[Ian Abbott]

"addi_apci_3120" allocates more pages of DMA buffer than it uses, may
allocate half a double-buffer it does not use because it's the "wrong"
half that it managed to allocate (unlikely), and relies on virt_to_bus()
to treat generic kernel memory from get_free_pages() as coherent DMA
memory.  Correct the issues, using dma_alloc_coherent() to allocate the
DMA buffers.

1) staging: comedi: addi_apci_3120: don't allocate 2nd DMA buffer on
   failure
2) staging: comedi: addi_apci_3120: don't overallocate DMA buffer
3) staging: comedi: addi_apci_3120: use dma_alloc_coherent()
4) staging: comedi: addi_apci_3120: simplify setting of
   devpriv->us_UseDma

 drivers/staging/comedi/Kconfig                     |  2 +-
 .../staging/comedi/drivers/addi-data/addi_common.h |  3 +-
 drivers/staging/comedi/drivers/addi_apci_3120.c    | 44 ++++++++++------------
 3 files changed, 22 insertions(+), 27 deletions(-)
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/4] staging: comedi: addi_apci_3120: don't allocate 2nd DMA buffer on failure

[Ian Abbott]

`apci3120_auto_attach()` tries to allocate two DMA buffers but may
allocate a single buffer or none at all.  If it fails to allocate the
first buffer, it still tries to allocate the second buffer, even though
it won't be used.  Change it to not bother trying to allocate the second
buffer if the first one fails.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/addi_apci_3120.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_3120.c b/drivers/staging/comedi/drivers/addi_apci_3120.c
index 84501a3..2ac95ba 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3120.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3120.c
@@ -100,13 +100,12 @@ static int apci3120_auto_attach(struct comedi_device *dev,
 			if (devpriv->ul_DmaBufferVirtual[i])
 				break;
 		}
-		if (devpriv->ul_DmaBufferVirtual[i]) {
-			devpriv->ui_DmaBufferPages[i] = pages;
-			devpriv->ui_DmaBufferSize[i] = PAGE_SIZE * pages;
-			devpriv->ul_DmaBufferHw[i] =
-				virt_to_bus((void *)devpriv->
-				ul_DmaBufferVirtual[i]);
-		}
+		if (!devpriv->ul_DmaBufferVirtual[i])
+			break;
+		devpriv->ui_DmaBufferPages[i] = pages;
+		devpriv->ui_DmaBufferSize[i] = PAGE_SIZE * pages;
+		devpriv->ul_DmaBufferHw[i] =
+		    virt_to_bus(devpriv->ul_DmaBufferVirtual[i]);
 	}
 	if (!devpriv->ul_DmaBufferVirtual[0])
 		devpriv->us_UseDma = 0;
-- 
2.1.0

[PATCH 1/4] staging: comedi: addi_apci_3120: don't allocate 2nd DMA buffer on failure

[Ian Abbott]

`apci3120_auto_attach()` tries to allocate two DMA buffers but may
allocate a single buffer or none at all.  If it fails to allocate the
first buffer, it still tries to allocate the second buffer, even though
it won't be used.  Change it to not bother trying to allocate the second
buffer if the first one fails.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 drivers/staging/comedi/drivers/addi_apci_3120.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_3120.c b/drivers/staging/comedi/drivers/addi_apci_3120.c
index 84501a3..2ac95ba 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3120.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3120.c
@@ -100,13 +100,12 @@ static int apci3120_auto_attach(struct comedi_device *dev,
 			if (devpriv->ul_DmaBufferVirtual[i])
 				break;
 		}
-		if (devpriv->ul_DmaBufferVirtual[i]) {
-			devpriv->ui_DmaBufferPages[i] = pages;
-			devpriv->ui_DmaBufferSize[i] = PAGE_SIZE * pages;
-			devpriv->ul_DmaBufferHw[i] =
-				virt_to_bus((void *)devpriv->
-				ul_DmaBufferVirtual[i]);
-		}
+		if (!devpriv->ul_DmaBufferVirtual[i])
+			break;
+		devpriv->ui_DmaBufferPages[i] = pages;
+		devpriv->ui_DmaBufferSize[i] = PAGE_SIZE * pages;
+		devpriv->ul_DmaBufferHw[i] =
+		    virt_to_bus(devpriv->ul_DmaBufferVirtual[i]);
 	}
 	if (!devpriv->ul_DmaBufferVirtual[0])
 		devpriv->us_UseDma = 0;
-- 
2.1.0

[PATCH 2/4] staging: comedi: addi_apci_3120: don't overallocate DMA buffer

[Ian Abbott]

The last parameter of `__get_free_pages()` is log2 (the 'order') of the
number of pages to be allocated.  This driver seems to think it is the
linear number of pages, so `apci3120_auto_attach()` first tries to allocate
16 pages, but only uses 4 of them, setting the buffer size to PAGE_SIZE
multiplied by the 'order'.  If the allocation fails, it tries
progressively smaller orders, down to 0.  If the allocation at order 0
succeeds, the buffer size is set to 0, which is likely to cause
problems.

Set the buffer size to `PAGE_SIZE` shifted left by the allocation order.
Since the maximum buffer size previously used was 4, start with an
allocation order of 2 instead of 4.  Rename the `ui_DmaBufferPages` member of
`struct addi_private` to `ui_DmaBufferPageOrder` and rename the `pages`
local variable to `order` to make it clearer what it is.

Note: `struct addi_private` is used by some other ADDI-DATA drivers as
well, but this is the only one using the affected members.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/addi-data/addi_common.h |  2 +-
 drivers/staging/comedi/drivers/addi_apci_3120.c        | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/addi_common.h b/drivers/staging/comedi/drivers/addi-data/addi_common.h
index a7400a2..88295e4 100644
--- a/drivers/staging/comedi/drivers/addi-data/addi_common.h
+++ b/drivers/staging/comedi/drivers/addi-data/addi_common.h
@@ -110,7 +110,7 @@ struct addi_private {
 	unsigned int ul_DmaBufferHw[2];	/*  hw address of DMA buff */
 	unsigned int ui_DmaBufferSize[2];	/*  size of dma buffer in bytes */
 	unsigned int ui_DmaBufferUsesize[2];	/*  which size we may now used for transfer */
-	unsigned int ui_DmaBufferPages[2];	/*  number of pages in buffer */
+	unsigned int ui_DmaBufferPageOrder[2];	/*  log2 of pages in buffer */
 	unsigned char b_DigitalOutputRegister;	/*  Digital Output Register */
 	unsigned char b_OutputMemoryStatus;
 	unsigned char b_TimerSelectMode;	/*  Contain data written at iobase + 0C */
diff --git a/drivers/staging/comedi/drivers/addi_apci_3120.c b/drivers/staging/comedi/drivers/addi_apci_3120.c
index 2ac95ba..57c36ed 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3120.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3120.c
@@ -57,7 +57,7 @@ static int apci3120_auto_attach(struct comedi_device *dev,
 	const struct addi_board *this_board = NULL;
 	struct addi_private *devpriv;
 	struct comedi_subdevice *s;
-	int ret, pages, i;
+	int ret, order, i;
 
 	if (context < ARRAY_SIZE(apci3120_boardtypes))
 		this_board = &apci3120_boardtypes[context];
@@ -93,17 +93,17 @@ static int apci3120_auto_attach(struct comedi_device *dev,
 	/* Allocate DMA buffers */
 	devpriv->b_DmaDoubleBuffer = 0;
 	for (i = 0; i < 2; i++) {
-		for (pages = 4; pages >= 0; pages--) {
+		for (order = 2; order >= 0; order--) {
 			devpriv->ul_DmaBufferVirtual[i] =
-				(void *) __get_free_pages(GFP_KERNEL, pages);
+			    (void *)__get_free_pages(GFP_KERNEL, order);
 
 			if (devpriv->ul_DmaBufferVirtual[i])
 				break;
 		}
 		if (!devpriv->ul_DmaBufferVirtual[i])
 			break;
-		devpriv->ui_DmaBufferPages[i] = pages;
-		devpriv->ui_DmaBufferSize[i] = PAGE_SIZE * pages;
+		devpriv->ui_DmaBufferPageOrder[i] = order;
+		devpriv->ui_DmaBufferSize[i] = PAGE_SIZE << order;
 		devpriv->ul_DmaBufferHw[i] =
 		    virt_to_bus(devpriv->ul_DmaBufferVirtual[i]);
 	}
@@ -201,12 +201,12 @@ static void apci3120_detach(struct comedi_device *dev)
 		if (devpriv->ul_DmaBufferVirtual[0]) {
 			free_pages((unsigned long)devpriv->
 				ul_DmaBufferVirtual[0],
-				devpriv->ui_DmaBufferPages[0]);
+				devpriv->ui_DmaBufferPageOrder[0]);
 		}
 		if (devpriv->ul_DmaBufferVirtual[1]) {
 			free_pages((unsigned long)devpriv->
 				ul_DmaBufferVirtual[1],
-				devpriv->ui_DmaBufferPages[1]);
+				devpriv->ui_DmaBufferPageOrder[1]);
 		}
 	}
 }
-- 
2.1.0

[PATCH 2/4] staging: comedi: addi_apci_3120: don't overallocate DMA buffer

[Ian Abbott]

The last parameter of `__get_free_pages()` is log2 (the 'order') of the
number of pages to be allocated.  This driver seems to think it is the
linear number of pages, so `apci3120_auto_attach()` first tries to allocate
16 pages, but only uses 4 of them, setting the buffer size to PAGE_SIZE
multiplied by the 'order'.  If the allocation fails, it tries
progressively smaller orders, down to 0.  If the allocation at order 0
succeeds, the buffer size is set to 0, which is likely to cause
problems.

Set the buffer size to `PAGE_SIZE` shifted left by the allocation order.
Since the maximum buffer size previously used was 4, start with an
allocation order of 2 instead of 4.  Rename the `ui_DmaBufferPages` member of
`struct addi_private` to `ui_DmaBufferPageOrder` and rename the `pages`
local variable to `order` to make it clearer what it is.

Note: `struct addi_private` is used by some other ADDI-DATA drivers as
well, but this is the only one using the affected members.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 drivers/staging/comedi/drivers/addi-data/addi_common.h |  2 +-
 drivers/staging/comedi/drivers/addi_apci_3120.c        | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/addi_common.h b/drivers/staging/comedi/drivers/addi-data/addi_common.h
index a7400a2..88295e4 100644
--- a/drivers/staging/comedi/drivers/addi-data/addi_common.h
+++ b/drivers/staging/comedi/drivers/addi-data/addi_common.h
@@ -110,7 +110,7 @@ struct addi_private {
 	unsigned int ul_DmaBufferHw[2];	/*  hw address of DMA buff */
 	unsigned int ui_DmaBufferSize[2];	/*  size of dma buffer in bytes */
 	unsigned int ui_DmaBufferUsesize[2];	/*  which size we may now used for transfer */
-	unsigned int ui_DmaBufferPages[2];	/*  number of pages in buffer */
+	unsigned int ui_DmaBufferPageOrder[2];	/*  log2 of pages in buffer */
 	unsigned char b_DigitalOutputRegister;	/*  Digital Output Register */
 	unsigned char b_OutputMemoryStatus;
 	unsigned char b_TimerSelectMode;	/*  Contain data written at iobase + 0C */
diff --git a/drivers/staging/comedi/drivers/addi_apci_3120.c b/drivers/staging/comedi/drivers/addi_apci_3120.c
index 2ac95ba..57c36ed 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3120.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3120.c
@@ -57,7 +57,7 @@ static int apci3120_auto_attach(struct comedi_device *dev,
 	const struct addi_board *this_board = NULL;
 	struct addi_private *devpriv;
 	struct comedi_subdevice *s;
-	int ret, pages, i;
+	int ret, order, i;
 
 	if (context < ARRAY_SIZE(apci3120_boardtypes))
 		this_board = &apci3120_boardtypes[context];
@@ -93,17 +93,17 @@ static int apci3120_auto_attach(struct comedi_device *dev,
 	/* Allocate DMA buffers */
 	devpriv->b_DmaDoubleBuffer = 0;
 	for (i = 0; i < 2; i++) {
-		for (pages = 4; pages >= 0; pages--) {
+		for (order = 2; order >= 0; order--) {
 			devpriv->ul_DmaBufferVirtual[i] =
-				(void *) __get_free_pages(GFP_KERNEL, pages);
+			    (void *)__get_free_pages(GFP_KERNEL, order);
 
 			if (devpriv->ul_DmaBufferVirtual[i])
 				break;
 		}
 		if (!devpriv->ul_DmaBufferVirtual[i])
 			break;
-		devpriv->ui_DmaBufferPages[i] = pages;
-		devpriv->ui_DmaBufferSize[i] = PAGE_SIZE * pages;
+		devpriv->ui_DmaBufferPageOrder[i] = order;
+		devpriv->ui_DmaBufferSize[i] = PAGE_SIZE << order;
 		devpriv->ul_DmaBufferHw[i] =
 		    virt_to_bus(devpriv->ul_DmaBufferVirtual[i]);
 	}
@@ -201,12 +201,12 @@ static void apci3120_detach(struct comedi_device *dev)
 		if (devpriv->ul_DmaBufferVirtual[0]) {
 			free_pages((unsigned long)devpriv->
 				ul_DmaBufferVirtual[0],
-				devpriv->ui_DmaBufferPages[0]);
+				devpriv->ui_DmaBufferPageOrder[0]);
 		}
 		if (devpriv->ul_DmaBufferVirtual[1]) {
 			free_pages((unsigned long)devpriv->
 				ul_DmaBufferVirtual[1],
-				devpriv->ui_DmaBufferPages[1]);
+				devpriv->ui_DmaBufferPageOrder[1]);
 		}
 	}
 }
-- 
2.1.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/4] staging: comedi: addi_apci_3120: use dma_alloc_coherent()

[Ian Abbott]

Use `dma_alloc_coherent()` to allocate the DMA buffers instead of
using `__get_free_pages()` to allocate and `virt_to_bus()` to get the
hardware address.  The coherent buffers are fairly small - at most 4
pages (although there are two of them).  Use of `virt_to_bus()` is
discouraged.

Note: `struct addi_private` is used by some other ADDI-DATA drivers as
well, but this is the only one using the affected members.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/Kconfig                     |  2 +-
 .../staging/comedi/drivers/addi-data/addi_common.h |  3 +--
 drivers/staging/comedi/drivers/addi_apci_3120.c    | 26 +++++++++++-----------
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/comedi/Kconfig b/drivers/staging/comedi/Kconfig
index fd5f939..f21bf6c 100644
--- a/drivers/staging/comedi/Kconfig
+++ b/drivers/staging/comedi/Kconfig
@@ -677,7 +677,7 @@ config COMEDI_ADDI_APCI_2200
 
 config COMEDI_ADDI_APCI_3120
 	tristate "ADDI-DATA APCI_3120/3001 support"
-	depends on VIRT_TO_BUS
+	depends on HAS_DMA
 	select COMEDI_FC
 	---help---
 	  Enable support for ADDI-DATA APCI_3120/3001 cards
diff --git a/drivers/staging/comedi/drivers/addi-data/addi_common.h b/drivers/staging/comedi/drivers/addi-data/addi_common.h
index 88295e4..e2a3ffe 100644
--- a/drivers/staging/comedi/drivers/addi-data/addi_common.h
+++ b/drivers/staging/comedi/drivers/addi-data/addi_common.h
@@ -107,10 +107,9 @@ struct addi_private {
 	unsigned char b_DmaDoubleBuffer;	/*  we can use double buffering */
 	unsigned int ui_DmaActualBuffer;	/*  which buffer is used now */
 	unsigned short *ul_DmaBufferVirtual[2];	/*  pointers to DMA buffer */
-	unsigned int ul_DmaBufferHw[2];	/*  hw address of DMA buff */
+	dma_addr_t ul_DmaBufferHw[2];		/*  hw address of DMA buff */
 	unsigned int ui_DmaBufferSize[2];	/*  size of dma buffer in bytes */
 	unsigned int ui_DmaBufferUsesize[2];	/*  which size we may now used for transfer */
-	unsigned int ui_DmaBufferPageOrder[2];	/*  log2 of pages in buffer */
 	unsigned char b_DigitalOutputRegister;	/*  Digital Output Register */
 	unsigned char b_OutputMemoryStatus;
 	unsigned char b_TimerSelectMode;	/*  Contain data written at iobase + 0C */
diff --git a/drivers/staging/comedi/drivers/addi_apci_3120.c b/drivers/staging/comedi/drivers/addi_apci_3120.c
index 57c36ed..1025541 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3120.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3120.c
@@ -95,17 +95,16 @@ static int apci3120_auto_attach(struct comedi_device *dev,
 	for (i = 0; i < 2; i++) {
 		for (order = 2; order >= 0; order--) {
 			devpriv->ul_DmaBufferVirtual[i] =
-			    (void *)__get_free_pages(GFP_KERNEL, order);
+			    dma_alloc_coherent(dev->hw_dev, PAGE_SIZE << order,
+					       &devpriv->ul_DmaBufferHw[i],
+					       GFP_KERNEL);
 
 			if (devpriv->ul_DmaBufferVirtual[i])
 				break;
 		}
 		if (!devpriv->ul_DmaBufferVirtual[i])
 			break;
-		devpriv->ui_DmaBufferPageOrder[i] = order;
 		devpriv->ui_DmaBufferSize[i] = PAGE_SIZE << order;
-		devpriv->ul_DmaBufferHw[i] =
-		    virt_to_bus(devpriv->ul_DmaBufferVirtual[i]);
 	}
 	if (!devpriv->ul_DmaBufferVirtual[0])
 		devpriv->us_UseDma = 0;
@@ -198,15 +197,16 @@ static void apci3120_detach(struct comedi_device *dev)
 		apci3120_reset(dev);
 	comedi_pci_detach(dev);
 	if (devpriv) {
-		if (devpriv->ul_DmaBufferVirtual[0]) {
-			free_pages((unsigned long)devpriv->
-				ul_DmaBufferVirtual[0],
-				devpriv->ui_DmaBufferPageOrder[0]);
-		}
-		if (devpriv->ul_DmaBufferVirtual[1]) {
-			free_pages((unsigned long)devpriv->
-				ul_DmaBufferVirtual[1],
-				devpriv->ui_DmaBufferPageOrder[1]);
+		unsigned int i;
+
+		for (i = 0; i < 2; i++) {
+			if (devpriv->ul_DmaBufferVirtual[i]) {
+				dma_free_coherent(dev->hw_dev,
+						  devpriv->ui_DmaBufferSize[i],
+						  devpriv->
+						  ul_DmaBufferVirtual[i],
+						  devpriv->ul_DmaBufferHw[i]);
+			}
 		}
 	}
 }
-- 
2.1.0

[PATCH 3/4] staging: comedi: addi_apci_3120: use dma_alloc_coherent()

[Ian Abbott]

Use `dma_alloc_coherent()` to allocate the DMA buffers instead of
using `__get_free_pages()` to allocate and `virt_to_bus()` to get the
hardware address.  The coherent buffers are fairly small - at most 4
pages (although there are two of them).  Use of `virt_to_bus()` is
discouraged.

Note: `struct addi_private` is used by some other ADDI-DATA drivers as
well, but this is the only one using the affected members.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 drivers/staging/comedi/Kconfig                     |  2 +-
 .../staging/comedi/drivers/addi-data/addi_common.h |  3 +--
 drivers/staging/comedi/drivers/addi_apci_3120.c    | 26 +++++++++++-----------
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/comedi/Kconfig b/drivers/staging/comedi/Kconfig
index fd5f939..f21bf6c 100644
--- a/drivers/staging/comedi/Kconfig
+++ b/drivers/staging/comedi/Kconfig
@@ -677,7 +677,7 @@ config COMEDI_ADDI_APCI_2200
 
 config COMEDI_ADDI_APCI_3120
 	tristate "ADDI-DATA APCI_3120/3001 support"
-	depends on VIRT_TO_BUS
+	depends on HAS_DMA
 	select COMEDI_FC
 	---help---
 	  Enable support for ADDI-DATA APCI_3120/3001 cards
diff --git a/drivers/staging/comedi/drivers/addi-data/addi_common.h b/drivers/staging/comedi/drivers/addi-data/addi_common.h
index 88295e4..e2a3ffe 100644
--- a/drivers/staging/comedi/drivers/addi-data/addi_common.h
+++ b/drivers/staging/comedi/drivers/addi-data/addi_common.h
@@ -107,10 +107,9 @@ struct addi_private {
 	unsigned char b_DmaDoubleBuffer;	/*  we can use double buffering */
 	unsigned int ui_DmaActualBuffer;	/*  which buffer is used now */
 	unsigned short *ul_DmaBufferVirtual[2];	/*  pointers to DMA buffer */
-	unsigned int ul_DmaBufferHw[2];	/*  hw address of DMA buff */
+	dma_addr_t ul_DmaBufferHw[2];		/*  hw address of DMA buff */
 	unsigned int ui_DmaBufferSize[2];	/*  size of dma buffer in bytes */
 	unsigned int ui_DmaBufferUsesize[2];	/*  which size we may now used for transfer */
-	unsigned int ui_DmaBufferPageOrder[2];	/*  log2 of pages in buffer */
 	unsigned char b_DigitalOutputRegister;	/*  Digital Output Register */
 	unsigned char b_OutputMemoryStatus;
 	unsigned char b_TimerSelectMode;	/*  Contain data written at iobase + 0C */
diff --git a/drivers/staging/comedi/drivers/addi_apci_3120.c b/drivers/staging/comedi/drivers/addi_apci_3120.c
index 57c36ed..1025541 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3120.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3120.c
@@ -95,17 +95,16 @@ static int apci3120_auto_attach(struct comedi_device *dev,
 	for (i = 0; i < 2; i++) {
 		for (order = 2; order >= 0; order--) {
 			devpriv->ul_DmaBufferVirtual[i] =
-			    (void *)__get_free_pages(GFP_KERNEL, order);
+			    dma_alloc_coherent(dev->hw_dev, PAGE_SIZE << order,
+					       &devpriv->ul_DmaBufferHw[i],
+					       GFP_KERNEL);
 
 			if (devpriv->ul_DmaBufferVirtual[i])
 				break;
 		}
 		if (!devpriv->ul_DmaBufferVirtual[i])
 			break;
-		devpriv->ui_DmaBufferPageOrder[i] = order;
 		devpriv->ui_DmaBufferSize[i] = PAGE_SIZE << order;
-		devpriv->ul_DmaBufferHw[i] =
-		    virt_to_bus(devpriv->ul_DmaBufferVirtual[i]);
 	}
 	if (!devpriv->ul_DmaBufferVirtual[0])
 		devpriv->us_UseDma = 0;
@@ -198,15 +197,16 @@ static void apci3120_detach(struct comedi_device *dev)
 		apci3120_reset(dev);
 	comedi_pci_detach(dev);
 	if (devpriv) {
-		if (devpriv->ul_DmaBufferVirtual[0]) {
-			free_pages((unsigned long)devpriv->
-				ul_DmaBufferVirtual[0],
-				devpriv->ui_DmaBufferPageOrder[0]);
-		}
-		if (devpriv->ul_DmaBufferVirtual[1]) {
-			free_pages((unsigned long)devpriv->
-				ul_DmaBufferVirtual[1],
-				devpriv->ui_DmaBufferPageOrder[1]);
+		unsigned int i;
+
+		for (i = 0; i < 2; i++) {
+			if (devpriv->ul_DmaBufferVirtual[i]) {
+				dma_free_coherent(dev->hw_dev,
+						  devpriv->ui_DmaBufferSize[i],
+						  devpriv->
+						  ul_DmaBufferVirtual[i],
+						  devpriv->ul_DmaBufferHw[i]);
+			}
 		}
 	}
 }
-- 
2.1.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 4/4] staging: comedi: addi_apci_3120: simplify setting of devpriv->us_UseDma

[Ian Abbott]

`apci3120_auto_attach()` first sets `devpriv->us_UseDma` to 1, then sets
it back to 0 if it fails to allocate the DMA buffer.  Since `*devpriv`
is initially zeroed out by `comedi_alloc_devpriv()`, change it to only
set `devpriv->us_UseDma` to 1 if the allocation succeeds.  Also, don't
bother explicitly initializing `devpriv->b_DmaDoubleBuffer` to 0 as it
is already zeroed out.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/addi_apci_3120.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_3120.c b/drivers/staging/comedi/drivers/addi_apci_3120.c
index 1025541..ba71e24 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3120.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3120.c
@@ -88,10 +88,7 @@ static int apci3120_auto_attach(struct comedi_device *dev,
 			dev->irq = pcidev->irq;
 	}
 
-	devpriv->us_UseDma = 1;
-
 	/* Allocate DMA buffers */
-	devpriv->b_DmaDoubleBuffer = 0;
 	for (i = 0; i < 2; i++) {
 		for (order = 2; order >= 0; order--) {
 			devpriv->ul_DmaBufferVirtual[i] =
@@ -106,8 +103,8 @@ static int apci3120_auto_attach(struct comedi_device *dev,
 			break;
 		devpriv->ui_DmaBufferSize[i] = PAGE_SIZE << order;
 	}
-	if (!devpriv->ul_DmaBufferVirtual[0])
-		devpriv->us_UseDma = 0;
+	if (devpriv->ul_DmaBufferVirtual[0])
+		devpriv->us_UseDma = 1;
 
 	if (devpriv->ul_DmaBufferVirtual[1])
 		devpriv->b_DmaDoubleBuffer = 1;
-- 
2.1.0

[PATCH 4/4] staging: comedi: addi_apci_3120: simplify setting of devpriv->us_UseDma

[Ian Abbott]

`apci3120_auto_attach()` first sets `devpriv->us_UseDma` to 1, then sets
it back to 0 if it fails to allocate the DMA buffer.  Since `*devpriv`
is initially zeroed out by `comedi_alloc_devpriv()`, change it to only
set `devpriv->us_UseDma` to 1 if the allocation succeeds.  Also, don't
bother explicitly initializing `devpriv->b_DmaDoubleBuffer` to 0 as it
is already zeroed out.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 drivers/staging/comedi/drivers/addi_apci_3120.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_3120.c b/drivers/staging/comedi/drivers/addi_apci_3120.c
index 1025541..ba71e24 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3120.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3120.c
@@ -88,10 +88,7 @@ static int apci3120_auto_attach(struct comedi_device *dev,
 			dev->irq = pcidev->irq;
 	}
 
-	devpriv->us_UseDma = 1;
-
 	/* Allocate DMA buffers */
-	devpriv->b_DmaDoubleBuffer = 0;
 	for (i = 0; i < 2; i++) {
 		for (order = 2; order >= 0; order--) {
 			devpriv->ul_DmaBufferVirtual[i] =
@@ -106,8 +103,8 @@ static int apci3120_auto_attach(struct comedi_device *dev,
 			break;
 		devpriv->ui_DmaBufferSize[i] = PAGE_SIZE << order;
 	}
-	if (!devpriv->ul_DmaBufferVirtual[0])
-		devpriv->us_UseDma = 0;
+	if (devpriv->ul_DmaBufferVirtual[0])
+		devpriv->us_UseDma = 1;
 
 	if (devpriv->ul_DmaBufferVirtual[1])
 		devpriv->b_DmaDoubleBuffer = 1;
-- 
2.1.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 0/4] staging: comedi: addi_apci_3120: redo DMA buffer allocation

[Hartley Sweeten]

On Friday, September 12, 2014 4:20 AM, Ian Abbott wrote:
> "addi_apci_3120" allocates more pages of DMA buffer than it uses, may
> allocate half a double-buffer it does not use because it's the "wrong"
> half that it managed to allocate (unlikely), and relies on virt_to_bus()
> to treat generic kernel memory from get_free_pages() as coherent DMA
> memory.  Correct the issues, using dma_alloc_coherent() to allocate the
> DMA buffers.
>
> 1) staging: comedi: addi_apci_3120: don't allocate 2nd DMA buffer on failure
> 2) staging: comedi: addi_apci_3120: don't overallocate DMA buffer
> 3) staging: comedi: addi_apci_3120: use dma_alloc_coherent()
> 4) staging: comedi: addi_apci_3120: simplify setting of devpriv->us_UseDma
>
>  drivers/staging/comedi/Kconfig                     |  2 +-
>  .../staging/comedi/drivers/addi-data/addi_common.h |  3 +-
>  drivers/staging/comedi/drivers/addi_apci_3120.c    | 44 ++++++++++------------
>  3 files changed, 22 insertions(+), 27 deletions(-)

Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>

RE: [PATCH 0/4] staging: comedi: addi_apci_3120: redo DMA buffer allocation

[Hartley Sweeten]

On Friday, September 12, 2014 4:20 AM, Ian Abbott wrote:
> "addi_apci_3120" allocates more pages of DMA buffer than it uses, may
> allocate half a double-buffer it does not use because it's the "wrong"
> half that it managed to allocate (unlikely), and relies on virt_to_bus()
> to treat generic kernel memory from get_free_pages() as coherent DMA
> memory.  Correct the issues, using dma_alloc_coherent() to allocate the
> DMA buffers.
>
> 1) staging: comedi: addi_apci_3120: don't allocate 2nd DMA buffer on failure
> 2) staging: comedi: addi_apci_3120: don't overallocate DMA buffer
> 3) staging: comedi: addi_apci_3120: use dma_alloc_coherent()
> 4) staging: comedi: addi_apci_3120: simplify setting of devpriv->us_UseDma
>
>  drivers/staging/comedi/Kconfig                     |  2 +-
>  .../staging/comedi/drivers/addi-data/addi_common.h |  3 +-
>  drivers/staging/comedi/drivers/addi_apci_3120.c    | 44 ++++++++++------------
>  3 files changed, 22 insertions(+), 27 deletions(-)

Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel