[coreboot] [patch] fix intel 82810 onboard VGA and SDRAM functions

Joseph Smith joe at settoplinux.org
Tue May 12 17:59:14 CEST 2009




On Tue, 12 May 2009 11:05:59 -0400, Joseph Smith <joe at settoplinux.org>
wrote:
> 
> 
> 
> On Tue, 12 May 2009 08:18:20 -0600, Myles Watson <mylesgw at gmail.com>
> wrote:
>> On Tue, May 12, 2009 at 8:12 AM, Joseph Smith <joe at settoplinux.org>
> wrote:
>>>
>>>
>>>>> +#ifdef CONFIG_VIDEO_MB
>>>>> +            /* check for VGA reserved memory
>>>>> +            * possible CONFIG_VIDEO_MB values are 512(kb) and
>> 1(mb)
>>>>> +            */
>>>>> +            if (CONFIG_VIDEO_MB == 512) {
>>>>> +                    tomk -= 512;
>>>>> +                    printk_debug("Allocating 512KB of RAM
>> for VGA\n");
>>>>> +            } else if (CONFIG_VIDEO_MB == 1) {
>>>>> +                    tomk -= 1024 ;
>>>>> +                    printk_debug("Allocating 1MB of RAM
> for
>> VGA\n");
>>>>
>>>>> +            } else {
>>>>> +                    /* assume no vga if incorrect value */
>>>>> +                    tomk == tomk;
>>>>
>>>> Isn't this is no-op? Or am I missing something?
>>>>
>>> I don't know what you mean no-op?
>> tomk==tomk doesn't do anything.  It would probably be better to leave
>> out that else branch.
>> 
> Ahh, I have to disagree. For example, what if someone puts a value of 4
> (CONFIG_VIDEO_MB = 4) in by mistake. It will cause coreboot to crash
> because it will not know what to do with that value...
> 
> The else tomk == tomk is more of a safety net than anything.
> 
> So if you keep the else and someone puts a value of 4 (CONFIG_VIDEO_MB =
> 4)
> in by mistake, coreboot will still boot, and worse case scenario you just
> won't have vga.
> 
Or maybe a switch would be better with tomk == tomk as the default.
-- 
Thanks,
Joseph Smith
Set-Top-Linux
www.settoplinux.org





More information about the coreboot mailing list