[PATCH] can: mcp251x: Use dmam_alloc_coherent

[PATCH] can: mcp251x: Use dmam_alloc_coherent

[Himangi Saraogi]

This patch moves the data allocated using dma_alloc_coherent to the
corresponding managed interface and does away with the calls to free the
allocated memory in the probe and remove functions.

Signed-off-by: Himangi Saraogi <himangi774@gmail.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
---
 drivers/net/can/spi/mcp251x.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index 5df239e..c66d699 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -1107,10 +1107,10 @@ static int mcp251x_can_probe(struct spi_device *spi)
 		 * Minimum coherent DMA allocation is PAGE_SIZE, so allocate
 		 * that much and share it between Tx and Rx DMA buffers.
 		 */
-		priv->spi_tx_buf = dma_alloc_coherent(&spi->dev,
-						      PAGE_SIZE,
-						      &priv->spi_tx_dma,
-						      GFP_DMA);
+		priv->spi_tx_buf = dmam_alloc_coherent(&spi->dev,
+						       PAGE_SIZE,
+						       &priv->spi_tx_dma,
+						       GFP_DMA);
 
 		if (priv->spi_tx_buf) {
 			priv->spi_rx_buf = (priv->spi_tx_buf + (PAGE_SIZE / 2));
@@ -1156,9 +1156,6 @@ static int mcp251x_can_probe(struct spi_device *spi)
 	return 0;
 
 error_probe:
-	if (mcp251x_enable_dma)
-		dma_free_coherent(&spi->dev, PAGE_SIZE,
-				  priv->spi_tx_buf, priv->spi_tx_dma);
 	mcp251x_power_enable(priv->power, 0);
 
 out_clk:
@@ -1178,11 +1175,6 @@ static int mcp251x_can_remove(struct spi_device *spi)
 
 	unregister_candev(net);
 
-	if (mcp251x_enable_dma) {
-		dma_free_coherent(&spi->dev, PAGE_SIZE,
-				  priv->spi_tx_buf, priv->spi_tx_dma);
-	}
-
 	mcp251x_power_enable(priv->power, 0);
 
 	if (!IS_ERR(priv->clk))
-- 
1.9.1

Re: [PATCH] can: mcp251x: Use dmam_alloc_coherent

[Alexander Shiyan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 582 bytes --]

Sat, 2 Aug 2014 19:22:04 +0530 от Himangi Saraogi <himangi774@gmail.com>:
> This patch moves the data allocated using dma_alloc_coherent to the
> corresponding managed interface and does away with the calls to free the
> allocated memory in the probe and remove functions.

Just for information: On my opinion, all SPI DMA stuff should be going into SPI layer.
It is not a good idea to keep this into this driver.

---

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] can: mcp251x: Use dmam_alloc_coherent

[Stefano Babic]

Hi Himangi,

On 02/08/2014 16:07, Alexander Shiyan wrote:
> Sat, 2 Aug 2014 19:22:04 +0530 от Himangi Saraogi <himangi774@gmail.com>:
>> This patch moves the data allocated using dma_alloc_coherent to the
>> corresponding managed interface and does away with the calls to free the
>> allocated memory in the probe and remove functions.
> 
> Just for information: On my opinion, all SPI DMA stuff should be going into SPI layer.
> It is not a good idea to keep this into this driver.

If I can add my two cents, I agree with Alexander. An underlying SPI
(master) controller driver can or cannot support DMA transfer, and this
should be transparent to the CAN layer. The mcp251x driver works with
both of them, taking advantage when DMA is available.

Best regards,
Stefano Babic


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
=====================================================================

Re: [PATCH] can: mcp251x: Use dmam_alloc_coherent

[christian pellegrin]

Hi all,

BTW the reason that use of DMA is selectable by a parameter is that on
some architectures (for example the venerable S3C24x0) the cost for
its setup is so high that it doesn't make sense for smaller transfers.
I absolutely agree with Stefano and Alexander, the SPI master driver
is in the best position to decide if DMA is worth or not.


On Tue, Aug 5, 2014 at 10:23 AM, Stefano Babic <sbabic@denx.de> wrote:
>> Just for information: On my opinion, all SPI DMA stuff should be going into SPI layer.
>> It is not a good idea to keep this into this driver.
>
> If I can add my two cents, I agree with Alexander. An underlying SPI
> (master) controller driver can or cannot support DMA transfer, and this
> should be transparent to the CAN layer. The mcp251x driver works with
> both of them, taking advantage when DMA is available.




-- 
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."

Re: [PATCH] can: mcp251x: Use dmam_alloc_coherent

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

On 08/02/2014 03:52 PM, Himangi Saraogi wrote:
> This patch moves the data allocated using dma_alloc_coherent to the
> corresponding managed interface and does away with the calls to free the
> allocated memory in the probe and remove functions.
> 
> Signed-off-by: Himangi Saraogi <himangi774@gmail.com>
> Acked-by: Julia Lawall <julia.lawall@lip6.fr>

Applied to can-next.

Thanks,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]