[coreboot] r5402 ...
Joseph Smith
joe at settoplinux.org
Sun Apr 11 17:49:12 CEST 2010
On 04/11/2010 11:12 AM, Joseph Smith wrote:
> On 04/11/2010 10:23 AM, Stefan Reinauer wrote:
>> Hi,
>>>>> + PRINTK_DEBUG(" Reading RAM at 0x%08x => 0x%08x\n", (dimm_start *
>>>>> 32 * 1024 * 1024), read32(dimm_start * 32 * 1024 * 1024));
>>>>>
>>>>>
>>>> There is a good chance that this read does not happen unless debugging
>>>> is enabled. Is that on purpose?
>>>> It would be clearer to pull this out of the PRINTK like this:
>>>>
>>>> #if CONFIG_RAM_DEBUG
>>>> u32 value = read32(dimm_start * 32 * 1024 * 1024);
>>>> PRINTK_DEBUG(" Reading RAM at 0x%08x => 0x%08x\n", (dimm_start * 32
>>>> * 1024 * 1024), value);
>>>> #endif
>>>>
>>>
>>> Yes that is on purpose. I guess I could use a preprocessing directive,
>>> but I thought that was the point of the macro.
>> well yes and no. There is no guarantee that the read is executed even if
>> CONFIG_RAM_DEBUG is on. So it is not side effect free.
>>
> Oh i get it, no I want ram_read32() to happen whether CONFIG_RAM_DEBUG
> is enabled or not, thanks for the catch.
>
So how about something like this:
static void ram_read32(u8 dimm_start, u32 offset)
{
#if CONFIG_DEBUG_RAM_SETUP
if (offset == 0x55aa55aa) {
PRINTK_DEBUG(" Reading RAM at 0x%08x => 0x%08x\n", (dimm_start * 32 *
1024 * 1024), read32(dimm_start * 32 * 1024 * 1024));
PRINTK_DEBUG(" Writing RAM at 0x%08x <= 0x%08x\n", (dimm_start * 32 *
1024 * 1024), offset);
write32(dimm_start * 32 * 1024 * 1024, offset);
PRINTK_DEBUG(" Reading RAM at 0x%08x => 0x%08x\n", (dimm_start * 32 *
1024 * 1024), read32(dimm_start * 32 * 1024 * 1024));
} else {
PRINTK_DEBUG(" to 0x%08x\n", (dimm_start * 32 * 1024 * 1024) + offset);
read32((dimm_start * 32 * 1024 * 1024) + offset);
}
#else
if (offset == 0x55aa55aa) {
read32(dimm_start * 32 * 1024 * 1024);
write32(dimm_start * 32 * 1024 * 1024, offset);
read32(dimm_start * 32 * 1024 * 1024);
} else {
read32((dimm_start * 32 * 1024 * 1024) + offset);
}
#endif
}
--
Thanks,
Joseph Smith
Set-Top-Linux
www.settoplinux.org
More information about the coreboot
mailing list