> -----Original Message----- > From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] > Sent: Tuesday, January 20, 2009 9:07 PM > To: Premi, Sanjeev > Cc: linux-omap@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 2/3v4] Runtime check for OMAP35x > > "Premi, Sanjeev" <premi@xxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
