[U-Boot] [PATCH][v1] ppc/85xx, 86xx: Handling Unknown SOC version

[Aggrwal Poonam-B10812]

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Wednesday, September 02, 2009 1:03 PM
> To: Aggrwal Poonam-B10812
> Cc: u-boot at lists.denx.de; Kumar Gala
> Subject: Re: [U-Boot] [PATCH][v1] ppc/85xx,86xx: Handling 
> Unknown SOC version
> 
> Dear Poonam Aggrwal,
> 
> In message 
> <1251864186-11732-1-git-send-email-poonam.aggrwal at freescale.co
m> you wrote:
> > Incase the system is detected with Unknown SVR, let the system boot 
> > with a default value and a proper message.
> > Now with dynamic detection of SOC properties from SVR, this is 
> > necessary to prevent a crash.
> > 
> > Signed-off-by: Poonam Aggrwal <poonam.aggrwal at freescale.com>
> > Signed-off-by: Kumar Gala <galak at kernel.crashing.org>
> > ---
> ...
> > --- a/cpu/mpc8xxx/cpu.c
> > +++ b/cpu/mpc8xxx/cpu.c
> > @@ -77,6 +77,7 @@ struct cpu_type cpu_type_list [] = {
> >  	CPU_TYPE_ENTRY(8641, 8641, 2),
> >  	CPU_TYPE_ENTRY(8641D, 8641D, 2),
> >  #endif
> > +	CPU_TYPE_ENTRY(Unknown, Unknown, 1),
> >  };
> >  
> >  struct cpu_type *identify_cpu(u32 ver) @@ -86,8 +87,7 @@ struct 
> > cpu_type *identify_cpu(u32 ver)
> >  		if (cpu_type_list[i].soc_ver == ver)
> >  			return &cpu_type_list[i];
> >  	}
> > -
> > -	return NULL;
> > +	return &cpu_type_list[i-1];
> 
> This looks unlogical to me. First, with this change you 
> should change the 'for' loop into
> 
> 	for (i = 0; i < ARRAY_SIZE(cpu_type_list) - 1; i++) {
> 		...
> 	}
> 
> But all this makes the code just harder to read and understand.
> 
> Why not add a separate
> 
> 	struct cpu_type cpu_type_unknown = 
> CPU_TYPE_ENTRY(Unknown, Unknown, 1);
> 
> and then do a simple
> 
> 	return &cpu_type_unknown;
> 
> ?
> 
> > diff --git a/include/asm-ppc/processor.h 
> b/include/asm-ppc/processor.h 
> > index dcaf8c0..9b27634 100644
> > --- a/include/asm-ppc/processor.h
> > +++ b/include/asm-ppc/processor.h
> > @@ -1022,6 +1022,8 @@
> >  #define SVR_8641	0x809000
> >  #define SVR_8641D	0x809001
> >  
> > +#define SVR_Unknown	0x000000
> 
> Is this a good and reliable choice? I don't know the rules 
> for SVR contents... Maybe 0xFFFFFF would be better?
> 
Thanks for the comments , let me incorporate them and resend.
Regards
Poonam
> Best regards,
> 
> Wolfgang Denk
> 
> -- 
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> "Intelligence without character is a dangerous thing."   - G. Steinem
> 
>