<div dir="ltr">On Tue, Oct 14, 2008 at 6:32 AM, Carl-Daniel Hailfinger <span dir="ltr"><<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>></span> wrote:<br><div class="gmail_quote">
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class="Ih2E3d">On 14.10.2008 06:21, Corey Osgood wrote:<br>
</div></blockquote><div><snip> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">> +/* Is this correct? */<br>
> +static void read32(u32 addr)<br>
> +{<br>
> + u32 val;<br>
> + volatile unsigned long *x;<br>
> + x = (void *)addr;<br>
> + val = *x;<br>
> +}<br>
><br>
<br>
Can't you use readl() instead?</blockquote><div><snip> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>
> + read32(0x800);<br>
</blockquote><div><br>By using readl(), I'm getting<br><br>/home/corey/coreboot/coreboot-v3/northbridge/via/cn700/initram.c:69: warning: passing argument 1 of 'readl' makes pointer from integer without a cast<br><br>
Where the above line is 69, this pops up on every readl() call. I know it's probably something stupid I'm not doing, what is it?<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> + } else {<br>
> + read32(0x121c20);<br>
> + read32(0x120020);<br>
> + }<br>
> + break;<br>
><br>
<br>
The logic above may be correct, but I have no way to figure it out.<br>
Could you maybe use symbolic constants for 0x12000, 0x800, 0x121c20,<br>
01120020?</blockquote><div><br>These are MRS/EMRS addresses, the first values are for DLL reset, second is for OCD calibration, there's a comment that explains what each offset is for now. <br></div><div> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> + case RAM_COMMAND_CBR:<br>
> + for(j = 0; j < 8; j++) {<br>
> + read32((reg8 << 26));<br>
> + udelay(100);<br>
> + }<br>
> + printk(BIOS_SPEW, "'CBR' to 0x%x", addr_offset);<br>
> + break;<br>
> + };<br>
> +<br>
> + /* NOTE: Dual-sided and multi-dimm ready */<br>
> + read32(0 + addr_offset);<br>
> + for(i = 0; i < (ARRAY_SIZE(dev->spd_channel0) * 2); i++) {<br>
> + reg8 = pci_conf1_read_config8(dev->d0f3, RANK0_START + i);<br>
> + if(reg8) {<br>
> + read32((reg8 << 26) + addr_offset);<br>
> + printk(BIOS_SPEW, ", 0x%x", (reg8 << 26) + addr_offset);<br>
> + if(command == RAM_COMMAND_MRS)<br>
> + {<br>
> + if(addr_offset == 0x12000)<br>
> + {<br>
> + read32((reg8 << 26) + 0x800);<br>
> + } else {<br>
> + read32((reg8 << 26) + 0x121c20);<br>
> + read32((reg8 << 26) + 0x120020);<br>
><br>
<br>
Same magic values as above.</blockquote><div><br>I didn't bother to re-comment these, it's the same as above. <br></div><div> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> + }<br>
> + } else if(command == RAM_COMMAND_CBR)<br>
> + {<br>
> + for(j = 0; j < 8; j++) {<br>
> + read32((reg8 << 26));<br>
> + udelay(100);<br>
> + }<br>
> + }<br>
> + }<br>
> + }<br>
> + printk(BIOS_SPEW, "\n");<br>
> +}<br>
> +<br>
> +/**<br>
> + * Configure the bus between the cpu and the northbridge. This might be able to<br>
> + * be moved to post-ram code in the future. For the most part, these registers<br>
> + * should not be messed around with. These are too complex to explain short of<br>
> + * copying the datasheets into the comments, but most of these values are from<br>
> + * the BIOS Porting Guide, so they should work on any board. If they don't,<br>
> + * try the values from your factory BIOS.<br>
><br>
<br>
Hm. Good leading explanation. If you could add a pointer to which data<br>
sheet sections mainly have this info, it would be nice. Your choice.<br>
The inline comments are really readable and I like them very much.</blockquote><div><br>The only problem with that is Via has come out with several newer versions of the datasheet. If you were to get one from them today, it would likely be different section numbers then I've got. <br>
</div><div> <br><more snipping><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">> + /* Set WR=5 and RFC */<br>
> + //pci_conf1_write_config8(dev->d0f3, 0x61, 0x94);//c7<br>
><br>
<br>
And here.<br>
<br>
> + /* Set CAS=5 */<br>
> + //pci_conf1_write_config8(dev->d0f3, 0x62, 0x7a);//af<br>
> + //pci_conf1_write_config8(dev->d0f3, 0x63, 0x00);//ca<br>
> + //pci_conf1_write_config8(dev->d0f3, 0x64, 0x88);<br>
><br>
<br>
And here.</blockquote><div><br>These are "safe" DRAM timings, eventually I'll make a Kconfig option to enable them, in case the ones set by SPD turn out to be too tight. <br></div><div> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> + if(!spd_data || spd_data == 0xff) {<br>
> + printk(BIOS_DEBUG, "No memory in slot %d\n", i);<br>
> + return 0;<br>
> + }<br>
> + else if(spd_data >= 0x10)<br>
> + spd_data = spd_data >> 4;<br>
> + else<br>
> + spd_data = spd_data << 4;<br>
><br>
<br>
Hm. Are you trying to just flip the nibbles or are you selectively<br>
picking one nibble? For the former, try this:<br>
spd_data = (spd_data >> 4) | (spd_data << 4) & 0xff;</blockquote><div><br>That should work, I'll try it out just to be sure. <br></div><div> </div><div>Thanks,<br>Corey <br></div></div></div>