Re: [PATCH 1/8] MIPS: BCM63XX: move flash registration out of board_bcm963xx.c

[Florian Fainelli <florian@openwrt.org>]

Hi Ralf,

On Wednesday 13 June 2012 14:48:01 Ralf Baechle wrote:
> On Tue, Jun 12, 2012 at 10:23:38AM +0200, Jonas Gorski wrote:
> 
> > board_bcm963xx.c is already large enough.
> 
> And the grand cure for that sort of issue is FDT - we by now have built
> big deserts of code just registering platform devices like this..  See
> John Crispin's Lantiq work or David's Cavium code for FDT examples.

I have some patches to convert bcm63xx to FDT but that is still work in 
progress, and I don't want them to hold support for newer BCM63xx CPUs.

> 
> > +int __init bcm63xx_flash_register(void)
> > +{
> > +	u32 val;
> > +
> > +	/* read base address of boot chip select (0) */
> > +	val = bcm_mpi_readl(MPI_CSBASE_REG(0));
> > +	val &= MPI_CSBASE_BASE_MASK;
> > +
> > +	mtd_resources[0].start = val;
> > +	mtd_resources[0].end = 0x1FFFFFFF;
> > +
> > +	return platform_device_register(&mtd_dev);
> > +}
> > diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_dev_flash.h 
b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_dev_flash.h
> > new file mode 100644
> > index 0000000..8dcb541
> > --- /dev/null
> > +++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_dev_flash.h
> > @@ -0,0 +1,6 @@
> > +#ifndef __BCM63XX_FLASH_H
> > +#define __BCM63XX_FLASH_H
> > +
> > +int __init bcm63xx_flash_register(void);
> 
> Don't use __init in declarations.  It doesn't make any difference to the
> compiler but it may cause build errors if <linux/init.h> has not been
> included before which this file doesn't.
> 
> > +#endif /* __BCM63XX_FLASH_H */
> 
> I suggest to make bcm63xx_flash_register an arch_initcall.  It already is
> being called indirectly from an bcm63xx_flash_register() so this would
> allow making the function static, get rid of bcm63xx_dev_flash.h which
> only exists to silence checkpatch warnings and make board_register_devices
> a little cleaner.

Well, yes, this makes it easier, but this is not robust, because you rely on 
the function alphabetical name to make sure that everything gets registered in 
the right order. Plus, the big advantage of letting this code separate and 
explicitely called, is to let out-of-tree boards use it as they wish too.

> 
>   Ralf
> 
> PS: Don't forget about FDT :-)  Eventually, not necessarily now.
-- 
Florian