[POWERPC] merge iSeries i/o operations with the rest
Linas Vepstas
linas at austin.ibm.com
Wed Sep 6 02:41:46 EST 2006
On Tue, Sep 05, 2006 at 12:08:17PM +1000, Stephen Rothwell wrote:
> @@ -273,25 +234,30 @@ #define iobarrier_w() eieio()
> * These routines do not perform EEH-related I/O address translation,
> * and should not be used directly by device drivers. Use inb/readb
> * instead.
> + *
> + * For some of these, we force the target/source register to be
> + * r0 to ease decoding on iSeries.
???
Why?
> static inline int in_8(const volatile unsigned char __iomem *addr)
> {
> - int ret;
> + register unsigned int ret __asm__("r0");
Such as here .. why is this being forced ??
> - __asm__ __volatile__("lbz%U1%X1 %0,%1; twi 0,%0,0; isync"
> - : "=r" (ret) : "m" (*addr));
> + __asm__ __volatile__("lbzx %0,0,%1; twi 0,%0,0; isync"
> + : "=r" (ret) : "r" (addr));
Why make this change? The old code allows the compiler to optimize
better than the new code. One might argue that, in the grand scheme of
things, i/o is very slow, so a few cycles here doesn't matter much.
But still, I don't understand why this change is needed.
> +static inline void memset_io(volatile void __iomem *addr, int c,
> + unsigned long n)
> +{
> + if (firmware_has_feature(FW_FEATURE_ISERIES))
> + iSeries_memset_io(addr, c, n);
> + else
> + eeh_memset_io(addr, c, n);
> +}
!! I think it would be much better to have this be a compile-time
check rather than a run-time check. No only does it save cycles,
but it makes the iSeries kernel smaller (by not needing eeh code
in it) and the p-series code smaller (by not compiling iseries
code into it.
--linas
More information about the Linuxppc-dev
mailing list