Skip to contentSkip to conversation

can: mcp251x: Use dmam_alloc_coherent

PATCH Applied Himangi Saraogi 5 participants 5 messages Aug 2, 2014 – Aug 15, 2014 CAN network drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 can: mcp251x: Use dmam_alloc_coherent

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.

---
 drivers/net/can/spi/mcp251x.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)
drivers/net/can/spi/mcp251x.c +4 -12
index 5df239e..c66d699 100644
@@ -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))
Signature · 1 line
1.9.1
#1 lore raw

Sat, 2 Aug 2014 19:22:04 +0530 от Himangi Saraogi <[email protected]>:

Quoted 3 lines

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�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�

#2 lore raw

Hi Himangi,

Quoted 7 lines · Alexander Shiyan

Sat, 2 Aug 2014 19:22:04 +0530 от Himangi Saraogi <[email protected]>:

Quoted 3 lines

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

Signature · 5 lines
=====================================================================
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: [email protected]
=====================================================================
#3 lore raw

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.

Quoted 7 lines · Stefano Babic
Quoted 2 lines

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.

Signature · 5 lines
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."
maintainer Applied
#4 lore raw
In reply to Himangi Saraogi
Quoted 6 lines · 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.

Applied to can-next.

Thanks,
Marc

Signature · 4 lines
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   |