On Thu, Feb 26, 2009 at 09:10:12AM -0800, Kevin Hilman wrote:
> "Mark A. Greer" <[email protected]> writes:
> 
> > From: Mark A. Greer <[email protected]>
> >
> > Factor out the common SoC init code and create some infrastructure
> > to allow easy adding of hooks to SoC-specific data in future patches.
> >
> > Signed-off-by: Mark A. Greer <[email protected]>
> 
> Hi Mark,
> 
> Overall, I think this approach looks good.  A minor comment below.
> 
> Also, this will require some slight rework on the clock front after I
> merge the clkdev patch.

OK

> Kevin
> 
> 
> [ ... ]
> 
> > +++ b/arch/arm/mach-davinci/common.c
> > @@ -0,0 +1,56 @@
> > +/*
> > + * Code commons to all DaVinci SoCs.
> > + *
> > + * Author: Mark A. Greer <[email protected]>
> > + *
> > + * 2009 (c) MontaVista Software, Inc. This file is licensed under
> > + * the terms of the GNU General Public License version 2. This program
> > + * is licensed "as is" without any warranty of any kind, whether express
> > + * or implied.
> > + */
> > +#include <linux/bootmem.h>
> > +
> > +#include <mach/clock.h>
> > +#include <mach/mux.h>
> > +#include <mach/common.h>
> > +
> > +struct davinci_soc_info *davinci_soc_info;
> > +
> 
> This current approach will not quite work for current modules that use
> cpu_is_* because the davinci_soc_info symbol is not exported to
> modules.  This causes it not to build for DaVinci git but it actually
> exposes a problem that needs to be fixed.  In general, drivers
> shouldn't be using cpu_is_* in the first place.  Ideally, they should
> be using platform_data for this.
> 
> This currently only affects MMC and MUSB. 
> 
> I'll submit a patch to fixup MMC by adding more platform_data options.
> That only leaves MUSB to cleanup.

OK

> [ ... ]
> 
> > +DAVINCI_SOC_START(dm644x)
> > +   .cpu_clks       = dm644x_clks,
> > +   .mux_pins       = dm644x_pins,
> > +   .mux_pins_num   = ARRAY_SIZE(dm644x_pins),
> > +DAVINCI_SOC_END
> 
> I agree with Dave on this one.   Just use standard struct decls here.

OK

> You should probably drop the __initdata too and then you don't need to
> alloc bootmem and copy the initdata.  I don't see the gain in using
> initdata if it just copied to bootmem later.

I did that as a first step towards copying the non-initdata for the
platform that the kernel is running on.  Once its all copied, the
original data can be made __initdata (since we have a copy) and we'll
magically get rid of all the data for the other platforms that's just
wasting memory.

Its not needed so I can get rid of it right now and make a separate
series of patches after the merge is done.

Mark
--

_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to