Re: [PATCH 3/4] omap: move detection of NAND CS to common-board-devices

[Tony Lindgren <tony@atomide.com>]

* Mike Rapoport <mike@compulab.co.il> [110504 10:13]:
> On 05/04/11 09:46, Tony Lindgren wrote:
> > * Mike Rapoport <mike@compulab.co.il> [110504 09:35]:
> >> On 05/04/11 07:10, Oleg Drokin wrote:
> >>> Ok, so here's a simple patch to save everyone trouble, I guess.
> >>>
> >>> Though on the other hand I can imagine that perhaps including this generic common-board-devices.c
> >>> might not be desirable for people that don't use anything from that file.
> >>
> >> Since the common-board-devices.c has TWL initialization I doubt there would a
> >> board that does not use it at all...
> >>
> >>> Would it be a better idea to split it to a file-per-feature?
> >>
> >> Splitting the common-board-devices into a file-per-feature will diminish its
> >> added value, IMO.
> >> We can either continue to use #ifdef CONFIG_SOMETHING in both
> >> common-board-devices.[ch] as your fix proposes or just drop #ifdefs and inlines
> >> from the header.
> >> Tony, what is your preference?
> > 
> > We should consider the code size too.. Maybe see if you can make them
> > weak instead of the ifdefs?
> 
> Unless I completely misunderstand how weak works, we'll still have ifdefs in .c
> file, i.e.
> 
> common-board-devices.h:
> 
> void __omap_nand_flash_init(int opts, struct mtd_partition *parts, int n_parts)
> {
> }
> 
> void omap_nand_flash_init(int opts, struct mtd_partition *parts, int n_parts)
> __attribute__((weak, alias("__omap_nand_flash_init")));
> 
> common-board-devices.c:
> #if defined(CONFIG_MTD_NAND_OMAP2) || defined(CONFIG_MTD_NAND_OMAP2_MODULE)
> void omap_nand_flash_init(int opts, struct mtd_partition *parts, int n_parts)
> {
> ...
> }
> #endif
> 
> Yet, we can keep the ifdefs only in common-board-devices.c and get rid if
> inlines in the header.
> Also, all the code in common-board-devices.c is __init, so it's eventually
> dropped in runtime

Sounds OK to me for the simple platform init functions used for most
common devices.

For anything more complex, we can have a separate file with ifdef in the
header and Makefile compiling it in only as selected.

Regards,

Tony