[PATCH 2/9] 8xx: Infrastructure code cleanup.

[David Gibson]

On Thu, Sep 13, 2007 at 12:16:40PM +0400, Vitaly Bordug wrote:
[snip]
> > This looks bogus.  You're replacing the old crap immr_map() functions,
> > which ioremap()ed the registers every time, with a much simpler
> > version which uses an established-once mapping of the register
> > region.  AFAICT, anywah.
> > 
> > So far, so good - but your immr_unmap() still does an iounmap() which
> > is surely wrong - it should now be a no-op, leaving the mpc8xx_immr
> > mapping intact.  You probably get away with it by accident, because I
> > imagine attempting to unmap an unaligned chunk of the region will just
> > fail.
> >
> 
> yes, it should do nop instead of iounmap. 
> > In fact, with this patch in place, I'd like to see another patch which
> > removes all calls to immr_map() and immr_unmap(), simply accessing the
> > common mapping directly.
> > 
> Sorry, but originally, that stuff was created to get rid of BSP
> ifdefs in drivers. For PQ family, it is a common practice to have
> single driver handling all 3 CPU families, which use the same logic,
> but immr structure differs a little bit.
> 
> At this point it's clear case-by-case ioremapping does not have firm
> benefit, but getting back to the way it was is useless either.  In
> ideal world, we'd have all those stuff put into dts and have
> specific drivers be a shim layer between core hw and IO drivers.

Err.. I don't understand what you're getting at.  As the code stands
after Scott's cleanup, the map() and unmap() calls can certainly be
trivially removed, regardless of the history for them.

And, yes, the drivers should certainly uses addresses from the device
tree, rather than that revolting structure covering all the inbuilt
device retgisters.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson