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
|