Re: powerpc_flash_init(), wtf!?

[David Gibson <david@gibson.dropbear.id.au>]

On Thu, May 03, 2007 at 10:35:34AM +0400, Vitaly Bordug wrote:
> On Tue, 1 May 2007 15:18:04 +1000
> David Gibson wrote:
> 
> > powerpc_flash_init(), the only function in arch/powerpc/sysdev/rom.c,
> > goes through the device tree finding anything with device_type=="rom"
> > and creating of_platform devices for them, which will be picked up by
> > the physmap_of mtd driver.  This has two serious conceptual errors and
> > one bad implementation error which is quite an accomplishment for 15
> > lines of code.
> > 
> It should be rewritten then - the way it does init is obsoleted by
> the of_platform_bus_probe() now and is confusion-prone. Have to
> admit I missed this part while reviewing that rom of_device patch.

Well, I don't see that it needs rewriting as a unit at all.  The
correct place for probing is in the platform code, no extra rom.c
necessary.

> [snip]
> 
> > 
> > Unless someone who actually knows how this code was intended to be
> > used can suggest a more polite way of fixing it.
> > 
> I guess, the idea was for this stuff to be updated once one of the
> dts inside boot/ would have physmap nodes added. I have
> rom/physmap[dts] rehaul in my TODO list, but it has (so far at
> least) little chance to happen during this merge window. Yet, if
> someone has suggestions and/or some interest for this to be cured,
> it will gain priority. Otherwise, I'll replace actual erroneous code
> with kind of rant that it's up to BSP code to take care of
> of_devices to be registered, using of_platform_bus_probe() or other
> way.

I'm having some trouble parsing that paragraph.  At this stage I don't
see any reason to hold off on tearing out arch/powerpc/sysdev/rom.c,
any necessary changes to replace it will go in the platform code or
other places.

-- 
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