[PATCH v2 1/4 RESEND] staging: et131x: simplify rx dma code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The original code allocate rx dma memory in several dma_alloc_coherent calls,
which causes some problems:
1. since dma_alloc_coherent allocate at least one page memory, it wastes some
   memory when allocation size is smaller than one page.
2. it causes et131x_rx_dma_memory_free as complex as et131x_rx_dma_memory_alloc

Instead, allocate all rx dma memory in one dma_alloc_coherent call makes less code,
makes it easy to handle dma allocation error, and makes et131x_rx_dma_memory_free
as simple as it could be.

Also add error check to kmalloc.

Signed-off-by: ZHAO Gang <gamerh2o@xxxxxxxxx>
---
 drivers/staging/et131x/et131x.c | 202 ++++++++++++----------------------------
 1 file changed, 59 insertions(+), 143 deletions(-)

diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index 27da1db..a9ae1f3 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -304,6 +304,7 @@ struct rx_ring {
 	u32 num_ready_recv;
 
 	u32 num_rfd;
+	u32 dma_size;
 
 	bool unfinished_receives;
 };
@@ -2186,21 +2187,15 @@ static inline u32 bump_free_buff_ring(u32 *free_buff_ring, u32 limit)
 	return tmp_free_buff_ring;
 }
 
-/* et131x_rx_dma_memory_alloc
- * @adapter: pointer to our private adapter structure
- *
- * Returns 0 on success and errno on failure (as defined in errno.h)
- *
- * Allocates Free buffer ring 1 for sure, free buffer ring 0 if required,
- * and the Packet Status Ring.
- */
 static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
 {
 	u8 id;
 	u32 i, j;
-	u32 bufsize;
-	u32 pktstat_ringsize;
-	u32 fbr_chunksize;
+	u32 dma_size;
+	u32 fbr_size;
+	u32 pktstat_ring_size;
+	dma_addr_t dma_addr;
+	void *virt_addr;
 	struct rx_ring *rx_ring;
 	struct fbr_lookup *fbr;
 
@@ -2209,7 +2204,14 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
 
 	/* Alloc memory for the lookup table */
 	rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL);
+	if (!rx_ring->fbr[0])
+		return -ENOMEM;
+
 	rx_ring->fbr[1] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL);
+	if (!rx_ring->fbr[1]) {
+		kfree(rx_ring->fbr[0]);
+		return -ENOMEM;
+	}
 
 	/* The first thing we will do is configure the sizes of the buffer
 	 * rings. These will change based on jumbo packet support.  Larger
@@ -2246,48 +2248,45 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
 		rx_ring->fbr[1]->num_entries = 128;
 	}
 
-	adapter->rx_ring.psr_num_entries =
-				adapter->rx_ring.fbr[0]->num_entries +
-				adapter->rx_ring.fbr[1]->num_entries;
+	rx_ring->psr_num_entries =
+		rx_ring->fbr[0]->num_entries + rx_ring->fbr[1]->num_entries;
+	pktstat_ring_size =
+		sizeof(struct pkt_stat_desc) * rx_ring->psr_num_entries;
+
+	dma_size = sizeof(struct fbr_desc) * rx_ring->psr_num_entries;
+	dma_size += pktstat_ring_size;
+	dma_size += sizeof(struct rx_status_block);
 
 	for (id = 0; id < NUM_FBRS; id++) {
-		fbr = rx_ring->fbr[id];
+		dma_size += FBR_CHUNKS * rx_ring->fbr[id]->buffsize;
+	}
 
-		/* Allocate an area of memory for Free Buffer Ring */
-		bufsize = sizeof(struct fbr_desc) * fbr->num_entries;
-		fbr->ring_virtaddr = dma_alloc_coherent(&adapter->pdev->dev,
-							bufsize,
-							&fbr->ring_physaddr,
-							GFP_KERNEL);
+	rx_ring->dma_size = dma_size;
 
-		if (!fbr->ring_virtaddr) {
-			dev_err(&adapter->pdev->dev,
-			   "Cannot alloc memory for Free Buffer Ring %d\n", id);
-			return -ENOMEM;
-		}
+	virt_addr = dma_alloc_coherent(&adapter->pdev->dev,
+				       dma_size,
+				       &dma_addr,
+				       GFP_KERNEL);
+	if (!virt_addr) {
+		kfree(rx_ring->fbr[0]);
+		kfree(rx_ring->fbr[1]);
+		dev_err(&adapter->pdev->dev,
+			"Cannot allocate memory for Rx ring\n");
+		return -ENOMEM;
 	}
 
+	/* Now we distribute the pie */
 	for (id = 0; id < NUM_FBRS; id++) {
 		fbr = rx_ring->fbr[id];
-		fbr_chunksize = (FBR_CHUNKS * fbr->buffsize);
+		fbr->ring_virtaddr = virt_addr;
+		fbr->ring_physaddr = dma_addr;
+		fbr_size = sizeof(struct fbr_desc) * fbr->num_entries;
+		virt_addr += fbr_size;
+		dma_addr += fbr_size;
 
 		for (i = 0; i < (fbr->num_entries / FBR_CHUNKS); i++) {
-			dma_addr_t fbr_tmp_physaddr;
-
-			fbr->mem_virtaddrs[i] =
-				dma_alloc_coherent(&adapter->pdev->dev,
-						   fbr_chunksize,
-						   &fbr->mem_physaddrs[i],
-						   GFP_KERNEL);
-
-			if (!fbr->mem_virtaddrs[i]) {
-				dev_err(&adapter->pdev->dev,
-					"Could not alloc memory\n");
-				return -ENOMEM;
-			}
-
-			/* See NOTE in "Save Physical Address" comment above */
-			fbr_tmp_physaddr = fbr->mem_physaddrs[i];
+			fbr->mem_virtaddrs[i] = virt_addr;
+			fbr->mem_physaddrs[i] = dma_addr;
 
 			for (j = 0; j < FBR_CHUNKS; j++) {
 				u32 index = (i * FBR_CHUNKS) + j;
@@ -2301,52 +2300,28 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
 				/* now store the physical address in the
 				 * descriptor so the device can access it
 				 */
-				fbr->bus_high[index] =
-						upper_32_bits(fbr_tmp_physaddr);
-				fbr->bus_low[index] =
-						lower_32_bits(fbr_tmp_physaddr);
-
-				fbr_tmp_physaddr += fbr->buffsize;
+				fbr->bus_high[index] = upper_32_bits(dma_addr);
+				fbr->bus_low[index] = lower_32_bits(dma_addr);
+				dma_addr += fbr->buffsize;
+				virt_addr += fbr->buffsize;
 			}
 		}
 	}
 
-	/* Allocate an area of memory for FIFO of Packet Status ring entries */
-	pktstat_ringsize =
-	    sizeof(struct pkt_stat_desc) * adapter->rx_ring.psr_num_entries;
+	rx_ring->ps_ring_virtaddr = virt_addr;
+	rx_ring->ps_ring_physaddr = dma_addr;
+	virt_addr += pktstat_ring_size;
+	dma_addr += pktstat_ring_size;
 
-	rx_ring->ps_ring_virtaddr = dma_alloc_coherent(&adapter->pdev->dev,
-						  pktstat_ringsize,
-						  &rx_ring->ps_ring_physaddr,
-						  GFP_KERNEL);
+	rx_ring->rx_status_block = virt_addr;
+	rx_ring->rx_status_bus = dma_addr;
 
-	if (!rx_ring->ps_ring_virtaddr) {
-		dev_err(&adapter->pdev->dev,
-			"Cannot alloc memory for Packet Status Ring\n");
-		return -ENOMEM;
-	}
 	pr_info("Packet Status Ring %llx\n",
 		(unsigned long long) rx_ring->ps_ring_physaddr);
+	pr_info("Receive Status Ring %llx\n",
+		(unsigned long long)rx_ring->rx_status_bus);
 
-	/* NOTE : dma_alloc_coherent(), used above to alloc DMA regions,
-	 * ALWAYS returns SAC (32-bit) addresses. If DAC (64-bit) addresses
-	 * are ever returned, make sure the high part is retrieved here before
-	 * storing the adjusted address.
-	 */
-
-	/* Allocate an area of memory for writeback of status information */
-	rx_ring->rx_status_block = dma_alloc_coherent(&adapter->pdev->dev,
-					    sizeof(struct rx_status_block),
-					    &rx_ring->rx_status_bus,
-					    GFP_KERNEL);
-	if (!rx_ring->rx_status_block) {
-		dev_err(&adapter->pdev->dev,
-			"Cannot alloc memory for Status Block\n");
-		return -ENOMEM;
-	}
 	rx_ring->num_rfd = NIC_DEFAULT_NUM_RFD;
-	pr_info("PRS %llx\n", (unsigned long long)rx_ring->rx_status_bus);
-
 	/* The RFDs are going to be put on lists later on, so initialize the
 	 * lists now.
 	 */
@@ -2354,18 +2329,10 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
 	return 0;
 }
 
-/* et131x_rx_dma_memory_free - Free all memory allocated within this module.
- * @adapter: pointer to our private adapter structure
- */
 static void et131x_rx_dma_memory_free(struct et131x_adapter *adapter)
 {
-	u8 id;
-	u32 index;
-	u32 bufsize;
-	u32 pktstat_ringsize;
 	struct rfd *rfd;
 	struct rx_ring *rx_ring;
-	struct fbr_lookup *fbr;
 
 	/* Setup some convenience pointers */
 	rx_ring = &adapter->rx_ring;
@@ -2382,67 +2349,16 @@ static void et131x_rx_dma_memory_free(struct et131x_adapter *adapter)
 		kfree(rfd);
 	}
 
-	/* Free Free Buffer Rings */
-	for (id = 0; id < NUM_FBRS; id++) {
-		fbr = rx_ring->fbr[id];
-
-		if (!fbr->ring_virtaddr)
-			continue;
-
-		/* First the packet memory */
-		for (index = 0; index < (fbr->num_entries / FBR_CHUNKS);
-		     index++) {
-			if (fbr->mem_virtaddrs[index]) {
-				bufsize = fbr->buffsize * FBR_CHUNKS;
-
-				dma_free_coherent(&adapter->pdev->dev,
-						  bufsize,
-						  fbr->mem_virtaddrs[index],
-						  fbr->mem_physaddrs[index]);
-
-				fbr->mem_virtaddrs[index] = NULL;
-			}
-		}
-
-		bufsize = sizeof(struct fbr_desc) * fbr->num_entries;
-
-		dma_free_coherent(&adapter->pdev->dev,
-				  bufsize,
-				  fbr->ring_virtaddr,
-				  fbr->ring_physaddr);
-
-		fbr->ring_virtaddr = NULL;
-	}
-
-	/* Free Packet Status Ring */
-	if (rx_ring->ps_ring_virtaddr) {
-		pktstat_ringsize = sizeof(struct pkt_stat_desc) *
-					adapter->rx_ring.psr_num_entries;
-
+	if (rx_ring->fbr[0]->ring_virtaddr)
 		dma_free_coherent(&adapter->pdev->dev,
-				  pktstat_ringsize,
-				  rx_ring->ps_ring_virtaddr,
-				  rx_ring->ps_ring_physaddr);
-
-		rx_ring->ps_ring_virtaddr = NULL;
-	}
-
-	/* Free area of memory for the writeback of status information */
-	if (rx_ring->rx_status_block) {
-		dma_free_coherent(&adapter->pdev->dev,
-				  sizeof(struct rx_status_block),
-				  rx_ring->rx_status_block,
-				  rx_ring->rx_status_bus);
-
-		rx_ring->rx_status_block = NULL;
-	}
+				  rx_ring->dma_size,
+				  rx_ring->fbr[0]->ring_virtaddr,
+				  rx_ring->fbr[0]->ring_physaddr);
 
-	/* Free the FBR Lookup Table */
 	kfree(rx_ring->fbr[0]);
 	kfree(rx_ring->fbr[1]);
 
-	/* Reset Counters */
-	rx_ring->num_ready_recv = 0;
+	memset(rx_ring, 0, sizeof(struct rx_ring));
 }
 
 /* et131x_init_recv - Initialize receive data structures.
-- 
1.8.3.1

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




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux