Re: [PATCH 3/8] crypto4xx_core: don't abuse __dma_sync_page
From: Christian Lamparter <hidden>
Date: 2018-12-16 18:31:05
On Sunday, December 16, 2018 6:19:46 PM CET Christoph Hellwig wrote:
quoted hunk ↗ jump to hunk
This function is internal to the DMA API implementation. Instead use the DMA API to properly unmap. Note that the DMA API usage in this driver is a disaster and urgently needs some work - it is missing all the unmaps, seems to do a secondary map where it looks like it should to a unmap in one place to work around cache coherency and the directions passed in seem to be partially wrong. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/crypto/amcc/crypto4xx_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/drivers/crypto/amcc/crypto4xx_core.c b/drivers/crypto/amcc/crypto4xx_core.c index 6eaec9ba0f68..63cb6956c948 100644 --- a/drivers/crypto/amcc/crypto4xx_core.c +++ b/drivers/crypto/amcc/crypto4xx_core.c@@ -596,7 +596,7 @@ static void crypto4xx_aead_done(struct crypto4xx_device *dev, pd->pd_ctl_len.bf.pkt_len, dst); } else { - __dma_sync_page(sg_page(dst), dst->offset, dst->length, + dma_unmap_page(dev->core_dev->device, pd->dest, dst->length, DMA_FROM_DEVICE); }
Yeap, the crypto4xx is a real piece of work. However, ibm emac network driver is even worse: |/* | * Lack of dma_unmap_???? calls is intentional. | * | * API-correct usage requires additional support state information to be | * maintained for every RX and TX buffer descriptor (BD). Unfortunately, due to | * EMAC design (e.g. TX buffer passed from network stack can be split into | * several BDs, dma_map_single/dma_map_page can be used to map particular BD), | * maintaining such information will add additional overhead. | * Current DMA API implementation for 4xx processors only ensures cache coherency | * and dma_unmap_???? routines are empty and are likely to stay this way. | * I decided to omit dma_unmap_??? calls because I don't want to add additional | * complexity just for the sake of following some abstract API, when it doesn't | * add any real benefit to the driver. I understand that this decision maybe | * controversial, but I really tried to make code API-correct and efficient | * at the same time and didn't come up with code I liked :(. --ebs | */ <https://elixir.bootlin.com/linux/v4.20-rc6/source/drivers/net/ethernet/ibm/emac/core.c#L58> Problem is, I can't really enable the DMA_DEBUG because every PPC4XX/APM82181 device I have is some sort of network appliance. So a proper test would require to fix the emac driver first since DMA_DEBUG will disable itself. Regards, Christian PS: Since it looks like you are located in Germany as well: If you are interested (PM me), I could just mail you a "MyBook Live" NAS Kit (PCB, Shell, Screws). All you would need: a SATA 3,5" HDD and a standard 12V PSU with a 5.5mm plug. I maintain a active OpenWrt port for the device: http://downloads.openwrt.org/snapshots/targets/apm821xx/sata/