From mboxrd@z Thu Jan 1 00:00:00 1970 From: H Hartley Sweeten Subject: RE: [PATCH v2 5/5] net: ep93xx_eth: fix DMA API violations Date: Thu, 9 Jun 2011 16:25:02 -0500 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: "ryan@bluewatersys.com" , "kernel@wantstofly.org" , "linux-arm-kernel@lists.infradead.org" To: Mika Westerberg , "netdev@vger.kernel.org" Return-path: In-Reply-To: Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org List-Id: netdev.vger.kernel.org On Thursday, June 02, 2011 12:00 PM, Mika Westerberg wrote: > Russell King said: >> >> So, to summarize what its doing: >> >> 1. It allocates buffers for rx and tx. >> 2. It maps them with dma_map_single(). >> This transfers ownership of the buffer to the DMA device. >> 3. In ep93xx_xmit, >> 3a. It copies the data into the buffer with skb_copy_and_csum_dev() >> This violates the DMA buffer ownership rules - the CPU should >> not be writing to this buffer while it is (in principle) owned >> by the DMA device. >> 3b. It then calls dma_sync_single_for_cpu() for the buffer. >> This transfers ownership of the buffer to the CPU, which surely >> is the wrong direction. >> 4. In ep93xx_rx, >> 4a. It calls dma_sync_single_for_cpu() for the buffer. >> This at least transfers the DMA buffer ownership to the CPU >> before the CPU reads the buffer >> 4b. It then uses skb_copy_to_linear_data() to copy the data out. >> At no point does it transfer ownership back to the DMA device. >> 5. When the driver is removed, it dma_unmap_single()'s the buffer. >> This transfers ownership of the buffer to the CPU. >> 6. It frees the buffer. >> >> While it may work on ep93xx, it's not respecting the DMA API rules, >> and with DMA debugging enabled it will probably encounter quite a few >> warnings. > > This patch fixes these violations. > > Signed-off-by: Mika Westerberg > Acked-by: Russell King Well... I'm not going to even pretend to actually understand the DMA API at this point. But, this patch seems to follow what DMA-API-HOWTO.txt describes as the proper usage of the DMA API. Also, with this patch and the others in your series my test systems are still booting and working properly. Acked-by: H Hartley Sweeten From mboxrd@z Thu Jan 1 00:00:00 1970 From: hartleys@visionengravers.com (H Hartley Sweeten) Date: Thu, 9 Jun 2011 16:25:02 -0500 Subject: [PATCH v2 5/5] net: ep93xx_eth: fix DMA API violations In-Reply-To: References: Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thursday, June 02, 2011 12:00 PM, Mika Westerberg wrote: > Russell King said: >> >> So, to summarize what its doing: >> >> 1. It allocates buffers for rx and tx. >> 2. It maps them with dma_map_single(). >> This transfers ownership of the buffer to the DMA device. >> 3. In ep93xx_xmit, >> 3a. It copies the data into the buffer with skb_copy_and_csum_dev() >> This violates the DMA buffer ownership rules - the CPU should >> not be writing to this buffer while it is (in principle) owned >> by the DMA device. >> 3b. It then calls dma_sync_single_for_cpu() for the buffer. >> This transfers ownership of the buffer to the CPU, which surely >> is the wrong direction. >> 4. In ep93xx_rx, >> 4a. It calls dma_sync_single_for_cpu() for the buffer. >> This at least transfers the DMA buffer ownership to the CPU >> before the CPU reads the buffer >> 4b. It then uses skb_copy_to_linear_data() to copy the data out. >> At no point does it transfer ownership back to the DMA device. >> 5. When the driver is removed, it dma_unmap_single()'s the buffer. >> This transfers ownership of the buffer to the CPU. >> 6. It frees the buffer. >> >> While it may work on ep93xx, it's not respecting the DMA API rules, >> and with DMA debugging enabled it will probably encounter quite a few >> warnings. > > This patch fixes these violations. > > Signed-off-by: Mika Westerberg > Acked-by: Russell King Well... I'm not going to even pretend to actually understand the DMA API at this point. But, this patch seems to follow what DMA-API-HOWTO.txt describes as the proper usage of the DMA API. Also, with this patch and the others in your series my test systems are still booting and working properly. Acked-by: H Hartley Sweeten