>typedef struct {
>u8 vendor;
>u8 eff_family;
>u8 eff_model;
>vrm_t vrm_type;
>} vrm_models_t;
>#define ANY 0xFF
What is 0xFF is ever a valid value for eff_model?
>vrm_models_t vrm_models[] = {
> {X86_VENDOR_AMD,0x6,ANY,VRM_INTEL_9x}, //athlon duron etc
> {X86_VENDOR_AMD,0xF,4,VRM_INTEL_9x}, //Athlon 64
> {X86_VENDOR_AMD,0xF,5,VRM_AMD_OPTERON},
> {X86_VENDOR_INTEL,0x6,0x9,VRM_INTEL_85}, /* 0.13um too */
> {X86_VENDOR_INTEL,0x6,0xB,VRM_INTEL_85}, /* 0xB Tualatin */
> {X86_VENDOR_INTEL,0x6,ANY,VRM_INTEL_8x}, /* any P6 */
> {X86_VENDOR_INTEL,0x7,ANY,VRM_UNKNOWN}, /* Itanium */
> {X86_VENDOR_INTEL,0xF,0x3,VRM_INTEL_10x}, /* P4 Prescott */
> {X86_VENDOR_INTEL,0xF,ANY,VRM_INTEL_9x}, /* P4 before
>Prescott*/
> {X86_VENDOR_INTEL,0x10,ANY,VRM_INTEL_ITANIUM2} /*Itanium 2*/
> };
>#define num_models (sizeof(vrm_models) / sizeof(vrm_models_t) )
I think that the usual practice for this kind of lookup tables is to have
an "empty" final line and rely on it to detect the end of the table.
Your method looks OK too, but maybe there's a reason why kernel folks
seem to prefer the empty last line?
>vrm_t find_vrm(u8 ef,u8 em,u8 vendor) {
>int i;
>
>for (i=0;i<num_models;i++) {
> if (vrm_models[i].vendor==vendor)
> if
>((vrm_models[i].eff_family==ef)&&((vrm_models[i].eff_model==ef)|| \
^^
em
> ((vrm_models[i].eff_model==ANY)))) return vrm_models[i].vrm_type;
> }
>return VRM_UNKNOWN;
>}
>static vrm_t whichVRM(void) {
>struct cpuinfo_x86 *c = cpu_data;
>
>u32 eax;
>u8 f,m;
Please use longer variable names in the final code. Short varible names
make the code harder to read.
> eax = cpuid_eax(1);
> f = ((eax & 0x00000F00)>>8);
> m = ((eax & 0x000000F0)>>4);
> if (f == 0xF) {
> f+=((eax & 0x00F00000)>>20);
No <<4 here?
> m+=((eax & 0x000F0000)>>16)<<4;
> }
>return find_vrm(f,m,c->x86_vendor);
Looks overwise really good, I'll test this on as many systems as
possible soon.
Thanks,
Jean Delvare
