On Sun, Dec 24, 2017 at 9:55 AM, christophe leroy <christophe.leroy@xxxxxx> wrote: > Le 23/12/2017 à 16:57, Guenter Roeck a écrit : >> >> On 12/23/2017 05:48 AM, Greg KH wrote: >>> >>> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote: >>>> >>>> Hi all, >>>> >>>> When I tried to use devm_ioremap function and review related code, I >>>> found >>>> devm_ioremap and devm_ioremap_nocache is almost the same with each >>>> other, >>>> except one use ioremap while the other use ioremap_nocache. >>> >>> >>> For all arches? Really? Look at MIPS, and x86, they have different >>> functions. >>> >> >> Both mips and x86 end up mapping the same function, but other arches >> don't. >> mn10300 is one where ioremap and ioremap_nocache are definitely different. > > > alpha: identical > arc: identical > arm: identical > arm64: identical > cris: different <== > frv: identical > hexagone: identical > ia64: different <== > m32r: identical > m68k: identical > metag: identical > microblaze: identical > mips: identical > mn10300: different <== > nios: identical > openrisc: different <== > parisc: identical > riscv: identical > s390: identical > sh: identical > sparc: identical > tile: identical > um: rely on asm/generic > unicore32: identical > x86: identical > asm/generic (no mmu): identical > > So 4 among all arches seems to have ioremap() and ioremap_nocache() being > different. > > Could we have a define set by the 4 arches on which ioremap() and > ioremap_nocache() are different, something like > HAVE_DIFFERENT_IOREMAP_NOCACHE ? I wonder if those are actually correct or not. What I found looking at those architectures: - openrisc only has one driver using ioremap (drivers/net/ethernet/ethoc.c) and that calls ioremap_nocache(). Presumably the authors went with the implementation for ioremap that made sense (using default attributes) rather than the one that actually works (using uncached). - On ia64, ioremap() checks the attributes for the physical address based on firmware tables and then picks either cached or uncached mappings. ioremap_nocache() does the same but returns NULL instead of a cached mapping for anything that is not an MMIO address. Presumably it would just work to always call ioremap(). - mn10300 appears to be wrong, broken by David Howells in commit 83c2dc15ce82 ("MN10300: Handle cacheable PCI regions in pci_iomap()") for any driver calling ioremap() by to get uncached memory, if I understand the comment for commit 34f1bdee1910 ("mn10300: switch to GENERIC_PCI_IOMAP") correctly: it seems that PCI addresses include the 'uncached' bit by default to get the right behavior, but dropping that bit breaks it. - cris seems similar to mn10300 in hardware, using an phys address bit for uncached access. There are two callers in arch code that appear to rely on the cachable output of ioremap() arch/cris/arch-v32/kernel/signal.c: __ioremap_prot(virt_to_phys(data), PAGE_SIZE, PAGE_SIGNAL_TRAMPOLINE); arch/cris/arch-v32/mm/intmem.c: intmem_virtual = ioremap(MEM_INTMEM_START + RESERVED_SIZE, It's unclear whether ioremap_nocache() actually has any users on cris, or whether it was only added for compile-time testing, and calling plain ioremap() would always work too (assuming we pass the phys address with the uncached-bit set). Arnd