Re: powerpc_flash_init(), wtf!?

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:

quoted

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]

quoted

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