| Message ID | 1444390262-15804-1-git-send-email-thomas@wytron.com.tw |
|---|---|
| State | Superseded |
| Delegated to: | Thomas Chou |
| Headers | show |
On Friday, October 09, 2015 at 01:31:02 PM, Thomas Chou wrote: > Convert dma_alloc_coherent to use memalign. > > Signed-off-by: Thomas Chou <thomas@wytron.com.tw> > --- > v2 > use memalign. > v3 > check memalign() return for out of memory. > > arch/nios2/include/asm/dma-mapping.h | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) > > diff --git a/arch/nios2/include/asm/dma-mapping.h > b/arch/nios2/include/asm/dma-mapping.h index 1350e3b..0618dc2 100644 > --- a/arch/nios2/include/asm/dma-mapping.h > +++ b/arch/nios2/include/asm/dma-mapping.h > @@ -1,23 +1,20 @@ > #ifndef __ASM_NIOS2_DMA_MAPPING_H > #define __ASM_NIOS2_DMA_MAPPING_H > > -/* dma_alloc_coherent() return cache-line aligned allocation which is > mapped +#include <asm/io.h> > + > +/* > + * dma_alloc_coherent() return cache-line aligned allocation which is > mapped * to uncached io region. > - * > - * IO_REGION_BASE should be defined in board config header file > - * 0x80000000 for nommu, 0xe0000000 for mmu > */ > - > static inline void *dma_alloc_coherent(size_t len, unsigned long *handle) > { > - void *addr = malloc(len + CONFIG_SYS_DCACHELINE_SIZE); > - if (!addr) > - return 0; > - flush_dcache((unsigned long)addr, len + CONFIG_SYS_DCACHELINE_SIZE); > - *handle = ((unsigned long)addr + > - (CONFIG_SYS_DCACHELINE_SIZE - 1)) & > - ~(CONFIG_SYS_DCACHELINE_SIZE - 1) & ~(IO_REGION_BASE); > - return (void *)(*handle | IO_REGION_BASE); > + *handle = (unsigned long)memalign(ARCH_DMA_MINALIGN, len); This still modifies the handle in both cases (failure and success). We really want to modify external variables in case of failure, no? > + if (!*handle) > + return NULL; > + flush_dcache_range(*handle, *handle + len); > + > + return ioremap(*handle, len); > } > > #endif /* __ASM_NIOS2_DMA_MAPPING_H */ Best regards, Marek Vasut
Hi Marek, On 10/09/2015 10:49 PM, Marek Vasut wrote: >> + *handle = (unsigned long)memalign(ARCH_DMA_MINALIGN, len); > > This still modifies the handle in both cases (failure and success). > We really want to modify external variables in case of failure, no? The *handle return a DMA address, and the function itself return a virtual address. Both of them should be set to NULL in case of failure. It might be wrong to keep DMA address looks alive but actually dead. Best regards, Thomas Chou
On Saturday, October 10, 2015 at 07:33:06 AM, Thomas Chou wrote: > Hi Marek, Hi! > On 10/09/2015 10:49 PM, Marek Vasut wrote: > >> + *handle = (unsigned long)memalign(ARCH_DMA_MINALIGN, len); > > > > This still modifies the handle in both cases (failure and success). > > We really want to modify external variables in case of failure, no? > > The *handle return a DMA address, and the function itself return a > virtual address. Both of them should be set to NULL in case of failure. So you depend on this property of memalign(), ok, I see. Thanks for clarifying! > It might be wrong to keep DMA address looks alive but actually dead. I don't quite understand this sentence, can you please rephrase it ? Best regards, Marek Vasut
Hi Marek, On 10/11/2015 02:19 AM, Marek Vasut wrote: > It might be wrong to keep DMA address looks alive but actually dead. > > I don't quite understand this sentence, can you please rephrase it ? Sorry. I meant the *handle, which is the DMA address, should be invalidated to NULL when the allocation failed. Best regards, Thomas
On Sunday, October 11, 2015 at 03:48:58 AM, Thomas Chou wrote: > Hi Marek, Hi, > On 10/11/2015 02:19 AM, Marek Vasut wrote: > > It might be wrong to keep DMA address looks alive but actually dead. > > > > I don't quite understand this sentence, can you please rephrase it ? > > Sorry. I meant the *handle, which is the DMA address, should be > invalidated to NULL when the allocation failed. OK Best regards, Marek Vasut
diff --git a/arch/nios2/include/asm/dma-mapping.h b/arch/nios2/include/asm/dma-mapping.h index 1350e3b..0618dc2 100644 --- a/arch/nios2/include/asm/dma-mapping.h +++ b/arch/nios2/include/asm/dma-mapping.h @@ -1,23 +1,20 @@ #ifndef __ASM_NIOS2_DMA_MAPPING_H #define __ASM_NIOS2_DMA_MAPPING_H -/* dma_alloc_coherent() return cache-line aligned allocation which is mapped +#include <asm/io.h> + +/* + * dma_alloc_coherent() return cache-line aligned allocation which is mapped * to uncached io region. - * - * IO_REGION_BASE should be defined in board config header file - * 0x80000000 for nommu, 0xe0000000 for mmu */ - static inline void *dma_alloc_coherent(size_t len, unsigned long *handle) { - void *addr = malloc(len + CONFIG_SYS_DCACHELINE_SIZE); - if (!addr) - return 0; - flush_dcache((unsigned long)addr, len + CONFIG_SYS_DCACHELINE_SIZE); - *handle = ((unsigned long)addr + - (CONFIG_SYS_DCACHELINE_SIZE - 1)) & - ~(CONFIG_SYS_DCACHELINE_SIZE - 1) & ~(IO_REGION_BASE); - return (void *)(*handle | IO_REGION_BASE); + *handle = (unsigned long)memalign(ARCH_DMA_MINALIGN, len); + if (!*handle) + return NULL; + flush_dcache_range(*handle, *handle + len); + + return ioremap(*handle, len); } #endif /* __ASM_NIOS2_DMA_MAPPING_H */
Convert dma_alloc_coherent to use memalign. Signed-off-by: Thomas Chou <thomas@wytron.com.tw> --- v2 use memalign. v3 check memalign() return for out of memory. arch/nios2/include/asm/dma-mapping.h | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-)