On Fri, 7 Jan 2005, Atsushi Nemoto wrote: > 1. How about adding 'volatile' and '__iomem' to *read*()/*write*() ? > While some archs use 'volatile void __iomem *' and some are not, I > think *read*()/*write*()/ioremap()/iounmap() should use same type. > This will remove some compiler warnings. Hmm, what's the semantics of "volatile void *"? I can't imagine any, so I don't think it would be useful. OTOH, I do agree the types used should be the same for all the mentioned functions and I'm inclined to make it "void __iomem *". For *read*(), "const" would be included as well. > 2. How about using 'const void *' for outs*()? This will remove some > compiler warnings too. Agreed. > 3. In *in*()/*out*(), it would be better to call __swizzle_addr*() > AFTER adding mips_io_port_base. This unifies the meaning of the > argument of __swizzle_addr*() (always virtual address). Then, > mach-specific __swizzle_addr*() can to every evil thing based on > the argument. Well, it shouldn't really matter as mips_io_port_base is expected to be page-aligned anyway. But I have no objections either. > 4. How about enclosing all *ioswab*() by '#ifndef' ? Also how about > passing virtual address to *ioswab*() ? I mean something like: > > # ifndef ioswabw > # define ioswabw(a,x) le16_to_cpu(x) > # endif > # ifndef __raw_ioswabw > # define __raw_ioswabw(a,x) (x) > # endif > ... > __val = pfx##ioswab##bwlq(__mem, val); \ > > Then we can provide mach-specific *ioswab*() in mach-*/mangle-port.h > and can do every evil thing based on its argument. It is usefull on > machines which have regions with defirrent endian conversion scheme. > Also, this can clean up CONFIG_SGI_IP22 from io.h > (mach-ip22/mangle-port.h can provide its own *ioswabw*()). Instead of using "#ifndef" the generic versions could be moved to <asm-mips/mach-generic/mangle-port.h>. Otherwise it sounds reasonable. Note that __mem is a virtual address, though, so you'd have to perform a physical address lookup before deciding on a swapping strategy -- would we really have a gain on any system from using regions with different swapping properties? Thanks for your feedback. Maciej
