> -----Original Message-----
> From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
> Sent: Tuesday, January 20, 2009 9:07 PM
> To: Premi, Sanjeev
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH 2/3v4] Runtime check for OMAP35x
>
> "Premi, Sanjeev" <pr...@ti.com> writes:
>
> > <snip>--<snip>
> >
> >> > +#ifdef CONFIG_ARCH_OMAP35XX
> >> > +static struct omap_globals omap35xx_globals = {
> >> > + .class = OMAP35XX_CLASS,
> >> > + .tap = OMAP2_IO_ADDRESS(0x4830A000),
> >> > + .sdrc = OMAP2_IO_ADDRESS(OMAP343X_SDRC_BASE),
> >> > + .sms = OMAP2_IO_ADDRESS(OMAP343X_SMS_BASE),
> >> > + .ctrl = OMAP2_IO_ADDRESS(OMAP343X_CTRL_BASE),
> >> > + .prm = OMAP2_IO_ADDRESS(OMAP3430_PRM_BASE),
> >> > + .cm = OMAP2_IO_ADDRESS(OMAP3430_CM_BASE),
> >> > +};
> >>
> >> This is exactly the same as omap34xx_globals. Why is this needed?
> >
>
> > [sp] I have tried to add minimal support for OMAP35x. Since OMAP34x
> > and OMAP35x seem to be compatible today, this structure
> seems same. As
> > more code for specific OMAP35x variants comes in, I expect this to
> > change.
> >
> > The key difference here (as against OMAP34x) is use of
> OMAP35XX_CLASS;
> > which helps in identifying the different OMAP variants. We
> could have
> > 're-used' OMAP35XX_CLASS, it I wouldn't be right as this
> definition is
> > used to print the CPU name and Si version in id.c.
> >
> > With this patch, boot log with show (for example) "OMAP3530 ES2.1"
> > on the OMAP3530 EVM - which can be considered same as
> OMAP3430 ES2.1;
> > but with 3503, 3515 and 3525 the print would read 3403, 3415,
> > 3425 resp; this definitley would not be right.
>
> I was not asking about the patch as a whole, I was commenting
> only on the addition of the omap35xx_globals variable. You
> added a new struct which is exactly the same as the 35xx
> struct instead of just re-using the old one with a new name
> as I suggested.
>
> Kevin
[sp] This would mean adding another #ifdef to get the right _CLASS. From
earlier discussion, I understood that we wanted to remove #ifdefs so that same
image can work for OMAP35x and OMAP34x.
>
> >>
> >> > +
> >> > +void __init omap2_set_globals_35xx(void) {
> >> > + omap2_globals = &omap35xx_globals;
> >> > +
> >> > + __omap2_set_globals();
> >> > +}
> >> > +#endif /* CONFIG_ARCH_OMAP35XX */
> >> > +
> >>
> >> What is the problem of the init code just leaving
> >> omap2_set_globals_343x()
> >>
> >> If you really think this will confuse folks, then how about just
> >> changing the #ifdef of the 343x defines to:
> >>
> >> #if defined(CONFIG_ARCH_OMAP3430) || defined(CONFIG_ARCH_OMAP35XX)
> >>
> >> and then adding this:
> >>
> >> #define omap2_set_globals_35xx omap2_set_globals_343x()
> >>
> >> Kevin
> >>
<snip>--<snip>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
