Re: [PATCH 2/3v4] Runtime check for OMAP35x

[Kevin Hilman <khilman@deeprootsystems.com>]

"Premi, Sanjeev" <premi@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.

Indeed, I hadn't noticed the new class definition.  This looks ok to
me.

Kevin