On Tue, May 12, 2009 at 4:57 PM, Uwe Hermann uwe@hermann-uwe.de wrote:
Quick review below, I didn't yet find the time to test on hardware, will hopefully be able to do that today.
- val = 0;
- val = pci_read_config8(PCI_DEV(0, 0, 0), MISSC2);
The 'val = 0' should no be needed if you do pci_read_config8() right after that.
agreed. its just a leftover. the 'val=0' can be removed.
- val |= 0x06;
- val |= 0xc6;
Why this? Only the last line alone should also do?
same here - the 'val += 0x06' is a leftover from the original code. it can be removed.
+#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?
no, you're not missing a thing. i actually copied this from other northbridge code and i liked this no-op because its a cleaner code. from a programmer side of view, it is redundant...
Uwe.
http://www.hermann-uwe.de | http://www.holsham-traders.de http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
I hope you'll get a chance to play with the patch and maybe fix the HIGH_TABLE issue.
Blessings, Elia.