On Tue, Aug 06, 2019 at 09:39:06PM +0200, Shawn Anastasio wrote: >> -#ifdef CONFIG_ARCH_HAS_DMA_MMAP_PGPROT >> pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot, >> unsigned long attrs); >> -#else >> -# define arch_dma_mmap_pgprot(dev, prot, attrs) pgprot_noncached(prot) >> -#endif > > Nit, but maybe the prototype should still be ifdef'd here? It at least > could prevent a reader from incorrectly thinking that the function is > always present.
Actually it is typical modern Linux style to just provide a prototype and then use "if (IS_ENABLED(CONFIG_FOO))" to guard the call(s) to it. > > Also, like Will mentioned earlier, the function name isn't entirely > accurate anymore. I second the suggestion of using something like > arch_dma_noncoherent_pgprot(). As mentioned I plan to remove arch_dma_mmap_pgprot for 5.4, so I'd rather avoid churn for the short period of time. > As for your idea of defining > pgprot_dmacoherent for all architectures as > > #ifndef pgprot_dmacoherent > #define pgprot_dmacoherent pgprot_noncached > #endif > > I think that the name here is kind of misleading too, since this > definition will only be used when there is no support for proper > DMA coherency. Do you have a suggestion for a better name? I'm pretty bad at naming, so just reusing the arm name seemed like a good way to avoid having to make naming decisions myself.
