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

[Wolfgang Denk]

Dear Poonam Aggrwal,

In message <1251864186-11732-1-git-send-email-poonam.aggrwal at freescale.com> 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?

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