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. Signed-off-by: ZHAO Gang <gamerh2o@xxxxxxxxx> --- drivers/staging/et131x/et131x.c | 219 +++++++++++++--------------------------- 1 file changed, 72 insertions(+), 147 deletions(-) diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c index 27da1db..409949f 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,16 @@ 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; + u32 fbr_chunk_size; + dma_addr_t dma_addr; + void *virt_addr; struct rx_ring *rx_ring; struct fbr_lookup *fbr; @@ -2208,8 +2204,11 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) rx_ring = &adapter->rx_ring; /* Alloc memory for the lookup table */ - rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); - rx_ring->fbr[1] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL); + rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup) * 2, GFP_KERNEL); + if (!rx_ring->fbr[0]) + return -ENOMEM; + + rx_ring->fbr[1] = rx_ring->fbr[0] + 1; /* 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 +2245,53 @@ 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; + + dma_size = sizeof(struct fbr_desc) * rx_ring->psr_num_entries; + + pktstat_ring_size = + sizeof(struct pkt_stat_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]; + fbr_chunk_size = FBR_CHUNKS * rx_ring->fbr[id]->buffsize; + dma_size += fbr_chunk_size; + } - /* 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]); + 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; + /* Update the pointer */ + 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; + /* Update the pointer */ + fbr_chunk_size = FBR_CHUNKS * fbr->buffsize; + virt_addr += fbr_chunk_size; + /* dma_addr is updated below */ for (j = 0; j < FBR_CHUNKS; j++) { u32 index = (i * FBR_CHUNKS) + j; @@ -2302,51 +2306,31 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) * descriptor so the device can access it */ fbr->bus_high[index] = - upper_32_bits(fbr_tmp_physaddr); + upper_32_bits(dma_addr); fbr->bus_low[index] = - lower_32_bits(fbr_tmp_physaddr); - - fbr_tmp_physaddr += fbr->buffsize; + lower_32_bits(dma_addr); + /* Update the pointer */ + dma_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; + /* Update the pointer */ + 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; + /* Finally, don't need to update pointer here :-) */ - 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); + (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 +2338,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; @@ -2374,75 +2350,24 @@ static void et131x_rx_dma_memory_free(struct et131x_adapter *adapter) WARN_ON(rx_ring->num_ready_recv != rx_ring->num_rfd); while (!list_empty(&rx_ring->recv_list)) { - rfd = (struct rfd *) list_entry(rx_ring->recv_list.next, - struct rfd, list_node); + rfd = list_entry(rx_ring->recv_list.next, + struct rfd, + list_node); list_del(&rfd->list_node); rfd->skb = NULL; 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