[coreboot] [PATCH] SECC Pentium 2/3 users are gonna love this

[Stefan Reinauer]

>
> ping... Is this too much? :-)
>   

Please don't gzip patches, it lowers the chance of someone reviewing
them to zero ;-)

There is 65x microcode in the model_67x directory, and it's not split up
into single files as for the other CPUs.

+ if (cpuid_res.ebx != 0x756e6547 || cpuid_res.edx != 0x49656e69 ||
cpuid_res.ecx != 0x6c65746e) {
+ printk(BIOS_INFO, "Not 'GenuineIntel' Processor");
+ return 0;
+ }

This is not necessary. When model_6**_init() is executed, it's clear
already that this is an intel cpu.

+/* if (signature & 0x1000) {
+ printk(BIOS_DEBUG,"Overdrive chip no L2 cache configuration\n");
+ return 0;
+ }
+
+ if (signature < 0x630 || signature >= 0x680) {
+ printk(BIOS_DEBUG,"L2 cache on CPUID %x does not require
configuration\n", signature);
+ return 0;
+ }*/

I think this code should just be dropped

Is it generally possible to move p6_configure_l2_cache to a generic
place, maybe to cpu/x86/cache ?

+/*--- End L2 init code from corebbot v1 ---*/

I think this kind of comment is not needed.

+static inline void strcpy(char *dst, char *src)
+{
+ while (*src) *dst++ = *src++;
+}

I know I am the idiot who introduced this in a model_xxx_init.c file,
but maybe we should move it to lib/strcpy.c ?

+static struct cpu_device_id cpu_table[] = {
+ { X86_VENDOR_INTEL, 0x0650 },
+ { X86_VENDOR_INTEL, 0x0651 },
+ { X86_VENDOR_INTEL, 0x0652 },
+ { X86_VENDOR_INTEL, 0x0653 },
+ { X86_VENDOR_INTEL, 0x0671 },
+ { X86_VENDOR_INTEL, 0x0672 },
+ { X86_VENDOR_INTEL, 0x0673 },
+ { 0, 0 },
+};

Instead of using a common driver, can two drivers use a common set of
shared functions instead?

+ifeq ($(CONFIG_USE_DCACHE_RAM),y)
+cpu_incs += $(src)/cpu/intel/car/cache_as_ram.inc
+endif

This will break all targets except the ones you worked on.


Stefan



> On Thu, May 13, 2010 at 10:03 PM, Keith Hui <buurin at gmail.com> wrote:
>   
>> Hi all,
>>
>> This is it. The 72oz steak. ;-) It is ~180k seasoned with a few other
>> related changes (below), so it is gzipped. I may have committed a few
>> "crimes" here, but anyway...
>>     
>   
>> This patch:
>> 1. Brings back L2 initialization from coreboot v1 for family 63x,65x
>> and 67x CPUs. Need someone with a Mendocino Celeron to see if the
>> entire 128k of L2 is still enabled.
>> 2. Split model_67x/65x and model_63x from model_6xx. model_67x also
>> serves model 65x because they share too much code. Also included are
>> Intel microcode for all CPUs in these families. There's just one file
>> for all microcodes in one family.
>> 3. In Slot 1 Makefile.inc, conditionally bring in sources in models
>> 63x/67x/6bx only when the proper config has been selected in Kconfig.
>> Also, only include cache_as_ram.inc if USE_DCACHE_RAM (ie. CAR) has
>> been selected.
>> 4. Remove USE_DCACHE_RAM from Slot 1 Kconfig. They should be in the
>> mainboards. Add CPU_INTEL_MODEL_6xX Kconfigs needed for (3) above.
>> 5. Blocked out some apparently unused #includes from model_6xx_init.c.
>> Once we're sure nothing really are using it, then remove them.
>>
>> Bootlog with a PIII 600MHz can be found here:
>> http://coreboot.pastebin.com/PNUzJXZT
>>     
>   


-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866