Re: [POWERPC] merge iSeries i/o operations with the rest

[linas@austin.ibm.com (Linas Vepstas)]

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