[coreboot] [v3] VT8237R support

Corey Osgood corey.osgood at gmail.com
Mon Aug 11 23:50:23 CEST 2008


On Sun, Aug 10, 2008 at 3:30 PM, Carl-Daniel Hailfinger
<c-d.hailfinger.devel.2006 at gmx.net> wrote:
> Hi Corey,
>
> this seems to have slipped through the cracks.
>
> On 24.03.2008 07:08, Corey Osgood wrote:
>> Add preliminary, as yet untested support for the vt8237r, based on the code in
>> coreboot v2. This will need some fixes, but at the very least it compiles and
>> should do smbus operations.
>>
>> Signed-off-by: Corey Osgood <corey.osgood at gmail.com>
>>
>
> Can you repost an updated patch? V3 has changed quite a bit in the last
> few months, especially considering PCI access functions and other stuff.
>

I haven't been following v3 development lately, been too busy. I'll
try to bring this up as soon as I get a chance.

---snip---
>> +     l = (unsigned long *)ioapic_base;
>> +
>> +     /* Set APIC to FSB message bus. */
>> +     l[0] = 0x3;
>>
>
> Magic values...
>
>> +     val = l[4];
>> +     l[4] = (val & 0xFFFFFE) | 1;
>>
>
> The line above is equivalent to
> l[4] = (val & 0xffffff) | 1;
>
> You decide which one is clearer. Anyway, the 0xFFFFFE is magic.
>
>> +
>> +     /* Set APIC ADDR - this will be VT8237R_APIC_ID. */
>> +     l[0] = 0;
>> +     val = l[4];
>> +     l[4] = (val & 0xF0FFFF) | (VT8237R_APIC_ID << 24);
>> +
>> +     for (i = 0; i < ARRAY_SIZE(ioapic_table); i++) {
>> +             l[0] = (ioapic_table[i].reg * 2) + 0x10;
>> +             l[4] = ioapic_table[i].value_low;
>> +             value_low = l[4];
>> +             l[0] = (ioapic_table[i].reg * 2) + 0x11;
>> +             l[4] = ioapic_table[i].value_high;
>> +             value_high = l[4];
>> +
>> +             if ((i == 0) && (value_low == 0xffffffff)) {
>> +                     printk_warning("IO APIC not responding.\n");
>> +                     return;
>> +             }
>> +     }
>>
>
> Reading the code sequence above hurts. You set l[0] and l[4] always in
> combination. What about helper functions like set_apic() and get_apic()?

That would be Rudolf's migraine :p I frankly don't really understand
APIC, so I honestly have no idea what's going on there, that was
pulled right out of v2. I'll try to fix everything else up though.

-Corey

PS: sorry if this gets sent more then once, gmail is acting up today :(




More information about the coreboot mailing list