Re: [PATCH] fsldma: add support to 36-bit physical address
On Tue, Sep 21, 2010 at 10:41 PM, Kumar Gala ga...@kernel.crashing.org wrote: On Sep 21, 2010, at 5:34 PM, Timur Tabi wrote: On Tue, Sep 21, 2010 at 5:17 PM, Scott Wood scottw...@freescale.com wrote: It needs to be the actual device that is performing the DMA -- the platform may need to do things such as IOMMU manipulation where knowing the device matters. Ok, this all makes sense. So it appears that the patch is valid, at least in theory. I would like to see some testing of it, but I realize that may be too difficult. There's no easy way to force an allocation above 4GB. I think the patch is pretty safe w/o testing. However I agree we need a better solution to testing 36-bit addressing. I'll take that as an acked-by, but I'll wait for the next version of the patch with the completed changelog before acting on it. -- Dan ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] fsldma: add support to 36-bit physical address
Signed-off-by: Li Yang le...@freescale.com --- drivers/dma/fsldma.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c index cea08be..9163552 100644 --- a/drivers/dma/fsldma.c +++ b/drivers/dma/fsldma.c @@ -1,7 +1,7 @@ /* * Freescale MPC85xx, MPC83xx DMA Engine support * - * Copyright (C) 2007 Freescale Semiconductor, Inc. All rights reserved. + * Copyright (C) 2007-2009 Freescale Semiconductor, Inc. All rights reserved. * * Author: * Zhang Wei wei.zh...@freescale.com, Jul 2007 @@ -1338,6 +1338,7 @@ static int __devinit fsldma_of_probe(struct platform_device *op, fdev-common.device_control = fsl_dma_device_control; fdev-common.dev = op-dev; + dma_set_mask(op-dev, DMA_BIT_MASK(36)); dev_set_drvdata(op-dev, fdev); /* -- 1.6.6-rc1.GIT ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] fsldma: add support to 36-bit physical address
On Sep 21, 2010, at 5:57 AM, Li Yang wrote: Signed-off-by: Li Yang le...@freescale.com --- We really should have a sentence about how or why this works to address 36-bit addressing. drivers/dma/fsldma.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c index cea08be..9163552 100644 --- a/drivers/dma/fsldma.c +++ b/drivers/dma/fsldma.c @@ -1,7 +1,7 @@ /* * Freescale MPC85xx, MPC83xx DMA Engine support * - * Copyright (C) 2007 Freescale Semiconductor, Inc. All rights reserved. + * Copyright (C) 2007-2009 Freescale Semiconductor, Inc. All rights reserved. * * Author: * Zhang Wei wei.zh...@freescale.com, Jul 2007 @@ -1338,6 +1338,7 @@ static int __devinit fsldma_of_probe(struct platform_device *op, fdev-common.device_control = fsl_dma_device_control; fdev-common.dev = op-dev; + dma_set_mask(op-dev, DMA_BIT_MASK(36)); dev_set_drvdata(op-dev, fdev); /* -- 1.6.6-rc1.GIT ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] fsldma: add support to 36-bit physical address
On Tue, 21 Sep 2010 16:24:10 -0500 Timur Tabi timur.t...@gmail.com wrote: On Tue, Sep 21, 2010 at 7:55 AM, Kumar Gala ga...@kernel.crashing.org wrote: On Sep 21, 2010, at 5:57 AM, Li Yang wrote: Signed-off-by: Li Yang le...@freescale.com --- We really should have a sentence about how or why this works to address 36-bit addressing. For example, I would like to know which memory is going to be allocated above 4GB. I don't know much about the kernel's async library, but my understanding is that fsldma does not allocate any of the memory buffers that it copies data to/from. This doesn't control allocation (it probably should with dma_alloc_coherent, though I don't see it in the code), it controls whether swiotlb will create a bounce buffer -- defeating the point of using DMA to accelerate a memcpy. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] fsldma: add support to 36-bit physical address
On Tue, Sep 21, 2010 at 4:34 PM, Scott Wood scottw...@freescale.com wrote: This doesn't control allocation (it probably should with dma_alloc_coherent, though I don't see it in the code), it controls whether swiotlb will create a bounce buffer -- defeating the point of using DMA to accelerate a memcpy. But it would do that only for the 'dev' used in the dma_set_mask() call. That dev is only used here: chan-desc_pool = dma_pool_create(fsl_dma_engine_desc_pool, chan-dev, sizeof(struct fsl_desc_sw), __alignof__(struct fsl_desc_sw), 0); Since we don't DMA the descriptors themselves, I just don't see how this patch does anything. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] fsldma: add support to 36-bit physical address
On Tue, 21 Sep 2010 16:43:12 -0500 Timur Tabi timur.t...@gmail.com wrote: On Tue, Sep 21, 2010 at 4:34 PM, Scott Wood scottw...@freescale.com wrote: This doesn't control allocation (it probably should with dma_alloc_coherent, though I don't see it in the code), it controls whether swiotlb will create a bounce buffer -- defeating the point of using DMA to accelerate a memcpy. But it would do that only for the 'dev' used in the dma_set_mask() call. That dev is only used here: chan-desc_pool = dma_pool_create(fsl_dma_engine_desc_pool, chan-dev, sizeof(struct fsl_desc_sw), __alignof__(struct fsl_desc_sw), 0); Since we don't DMA the descriptors themselves, I just don't see how this patch does anything. Look in dmaengine.c, there are calls to dma_map_single() and dma_map_page(), using what I assume is that same device pointer -- unless there's confusion between the channel and the controller. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] fsldma: add support to 36-bit physical address
On Tue, Sep 21, 2010 at 4:49 PM, Scott Wood scottw...@freescale.com wrote: Look in dmaengine.c, there are calls to dma_map_single() and dma_map_page(), using what I assume is that same device pointer -- unless there's confusion between the channel and the controller. You're right. I missed this line in the driver: fdev-common.dev = op-dev; Also, the driver does something stupid. Sometimes chan-dev refers to dma_chan.chan, and sometimes it refers to fsldma_chan.chan. I could have sworn I saw a patch that fixes that, though. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] fsldma: add support to 36-bit physical address
On Tue, Sep 21, 2010 at 5:05 PM, Dan Malek ppc6...@digitaldans.com wrote: The DMA descriptors are accessed using DMA by the controller itself. Yes and no. Technically, it is DMA, but it's not something that SWIOTLB could ever know about. We just pass the physical address to the DMA controller, and it does a memory read to obtain the data. That's not the kind of DMA that SWIOTLB would deal with, I think. The APIs need to ensure proper coherency between the CPU and the DMA controller for the descriptor access. The underlying implementation of the API will depend upon the hardware capabilities that ensure this coherency. I think that's already covered. The dma_set_mask() call is supposed to only affect the dma_map_single() calls that dmaengine makes, as Scott pointed out. My question is, should dmaengine be using the same 'dev' that fsldma uses to allocate the DMA descriptors? I wonder if the 'dev' should be allocated internally by dmaengine, or provided by the client drivers. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] fsldma: add support to 36-bit physical address
On Sep 21, 2010, at 2:49 PM, Scott Wood wrote: On Tue, 21 Sep 2010 16:43:12 -0500 Timur Tabi timur.t...@gmail.com wrote: Since we don't DMA the descriptors themselves, I just don't see how this patch does anything. Look in dmaengine.c, there are calls to dma_map_single() and dma_map_page(), using what I assume is that same device pointer -- unless there's confusion between the channel and the controller. The DMA descriptors are accessed using DMA by the controller itself. The APIs need to ensure proper coherency between the CPU and the DMA controller for the descriptor access. The underlying implementation of the API will depend upon the hardware capabilities that ensure this coherency. -- Dan ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] fsldma: add support to 36-bit physical address
On Tue, 21 Sep 2010 17:08:54 -0500 Timur Tabi timur.t...@gmail.com wrote: On Tue, Sep 21, 2010 at 5:05 PM, Dan Malek ppc6...@digitaldans.com wrote: The DMA descriptors are accessed using DMA by the controller itself. Yes and no. Technically, it is DMA, but it's not something that SWIOTLB could ever know about. We just pass the physical address to the DMA controller, and it does a memory read to obtain the data. That's not the kind of DMA that SWIOTLB would deal with, I think. SWIOTLB should see that the address is directly DMAable and do nothing, but you don't want to skip the DMA API altogether. You still need to flush caches if DMA is noncoherent, etc. My question is, should dmaengine be using the same 'dev' that fsldma uses to allocate the DMA descriptors? I wonder if the 'dev' should be allocated internally by dmaengine, or provided by the client drivers. It needs to be the actual device that is performing the DMA -- the platform may need to do things such as IOMMU manipulation where knowing the device matters. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] fsldma: add support to 36-bit physical address
On Tue, Sep 21, 2010 at 5:17 PM, Scott Wood scottw...@freescale.com wrote: It needs to be the actual device that is performing the DMA -- the platform may need to do things such as IOMMU manipulation where knowing the device matters. Ok, this all makes sense. So it appears that the patch is valid, at least in theory. I would like to see some testing of it, but I realize that may be too difficult. There's no easy way to force an allocation above 4GB. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] fsldma: add support to 36-bit physical address
On Sep 21, 2010, at 5:34 PM, Timur Tabi wrote: On Tue, Sep 21, 2010 at 5:17 PM, Scott Wood scottw...@freescale.com wrote: It needs to be the actual device that is performing the DMA -- the platform may need to do things such as IOMMU manipulation where knowing the device matters. Ok, this all makes sense. So it appears that the patch is valid, at least in theory. I would like to see some testing of it, but I realize that may be too difficult. There's no easy way to force an allocation above 4GB. I think the patch is pretty safe w/o testing. However I agree we need a better solution to testing 36-bit addressing. - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
