Author: linux_junkie Date: Sun Apr 11 07:50:08 2010 New Revision: 5402 URL: https://tracker.coreboot.org/trac/coreboot/changeset/5402
Log: Trivial changes to i82830 raminit.c for USE_PRINTK_IN_CAR. Signed-off-by: Joseph Smith joe@settoplinux.org Acked-by: Joseph Smith joe@settoplinux.org
Modified: trunk/src/northbridge/intel/i82830/raminit.c
Modified: trunk/src/northbridge/intel/i82830/raminit.c ============================================================================== --- trunk/src/northbridge/intel/i82830/raminit.c Sat Apr 10 00:25:27 2010 (r5401) +++ trunk/src/northbridge/intel/i82830/raminit.c Sun Apr 11 07:50:08 2010 (r5402) @@ -30,16 +30,10 @@
/* Debugging macros. */ #if CONFIG_DEBUG_RAM_SETUP -#define PRINT_DEBUG(x) print_debug(x) -#define PRINT_DEBUG_HEX8(x) print_debug_hex8(x) -#define PRINT_DEBUG_HEX16(x) print_debug_hex16(x) -#define PRINT_DEBUG_HEX32(x) print_debug_hex32(x) +#define PRINTK_DEBUG(x...) printk(BIOS_DEBUG, x) #define DUMPNORTH() dump_pci_device(PCI_DEV(0, 0, 0)) #else -#define PRINT_DEBUG(x) -#define PRINT_DEBUG_HEX8(x) -#define PRINT_DEBUG_HEX16(x) -#define PRINT_DEBUG_HEX32(x) +#define PRINTK_DEBUG(x...) #define DUMPNORTH() #endif
@@ -75,37 +69,19 @@ /* Clear bits 29, 10-8, 6-4. */ reg32 &= 0xdffff88f; reg32 |= command << 4; + PRINTK_DEBUG(" Sending RAM command 0x%08x", reg32); pci_write_config32(NORTHBRIDGE, DRC, reg32); - PRINT_DEBUG("RAM command 0x"); - PRINT_DEBUG_HEX32(reg32); - PRINT_DEBUG("\n"); }
static void ram_read32(u8 dimm_start, u32 offset) { if (offset == 0x55aa55aa) { - PRINT_DEBUG(" Reading RAM at 0x"); - PRINT_DEBUG_HEX32(dimm_start * 32 * 1024 * 1024); - PRINT_DEBUG(" => 0x"); - PRINT_DEBUG_HEX32(read32(dimm_start * 32 * 1024 * 1024)); - PRINT_DEBUG("\n"); - - PRINT_DEBUG(" Writing RAM at 0x"); - PRINT_DEBUG_HEX32(dimm_start * 32 * 1024 * 1024); - PRINT_DEBUG(" <= 0x"); - PRINT_DEBUG_HEX32(offset); - PRINT_DEBUG("\n"); + 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); - - PRINT_DEBUG(" Reading RAM at 0x"); - PRINT_DEBUG_HEX32(dimm_start * 32 * 1024 * 1024); - PRINT_DEBUG(" => 0x"); - PRINT_DEBUG_HEX32(read32(dimm_start * 32 * 1024 * 1024)); - PRINT_DEBUG("\n"); + PRINTK_DEBUG(" Reading RAM at 0x%08x => 0x%08x\n", (dimm_start * 32 * 1024 * 1024), read32(dimm_start * 32 * 1024 * 1024)); } else { - PRINT_DEBUG(" Sending RAM command to 0x"); - PRINT_DEBUG_HEX32((dimm_start * 32 * 1024 * 1024) + offset); - PRINT_DEBUG("\n"); + PRINTK_DEBUG(" to 0x%08x\n", (dimm_start * 32 * 1024 * 1024) + offset); read32((dimm_start * 32 * 1024 * 1024) + offset); } } @@ -138,24 +114,22 @@ dimm_end = pci_read_config8(NORTHBRIDGE, DRB + row);
if (dimm_end > dimm_start) { - print_debug("Initializing SDRAM Row "); - print_debug_hex8(row); - print_debug("\n"); + printk(BIOS_DEBUG, "Initializing SDRAM Row %u\n", row);
/* NOP command */ - PRINT_DEBUG(" NOP "); + PRINTK_DEBUG(" NOP\n"); do_ram_command(RAM_COMMAND_NOP); ram_read32(dimm_start, 0); udelay(200);
/* Pre-charge all banks (at least 200 us after NOP) */ - PRINT_DEBUG(" Pre-charging all banks "); + PRINTK_DEBUG(" Pre-charging all banks\n"); do_ram_command(RAM_COMMAND_PRECHARGE); ram_read32(dimm_start, 0); udelay(1);
/* 8 CBR refreshes (Auto Refresh) */ - PRINT_DEBUG(" 8 CBR refreshes "); + PRINTK_DEBUG(" 8 CBR refreshes\n"); for (i = 0; i < 8; i++) { do_ram_command(RAM_COMMAND_CBR); ram_read32(dimm_start, 0); @@ -164,19 +138,19 @@
/* MRS command */ /* TODO: Set offset 0x1d0 according to DRT values */ - PRINT_DEBUG(" MRS "); + PRINTK_DEBUG(" MRS\n"); do_ram_command(RAM_COMMAND_MRS); ram_read32(dimm_start, 0x1d0); udelay(2);
/* Set GMCH-M Mode Select bits back to NORMAL operation mode */ - PRINT_DEBUG(" Normal operation mode "); + PRINTK_DEBUG(" Normal operation mode\n"); do_ram_command(RAM_COMMAND_NORMAL); ram_read32(dimm_start, 0); udelay(1);
/* Perform a dummy memory read/write cycle */ - PRINT_DEBUG(" Performing dummy read/write\n"); + PRINTK_DEBUG(" Performing dummy read/write\n"); ram_read32(dimm_start, 0x55aa55aa); udelay(1); } @@ -253,30 +227,22 @@
/* First check if a DIMM is actually present. */ if (spd_read_byte(device, SPD_MEMORY_TYPE) == 0x4) { - print_debug("Found DIMM in slot "); - print_debug_hex8(i); - print_debug("\n"); - + printk(BIOS_DEBUG, "Found DIMM in slot %u\n", i); sz = spd_get_dimm_size(device); - - /* WISHLIST: would be nice to display it as decimal? */ - print_debug("DIMM is 0x"); - print_debug_hex16(sz.side1); - print_debug(" on side 1\n"); - print_debug("DIMM is 0x"); - print_debug_hex16(sz.side2); - print_debug(" on side 2\n"); + printk(BIOS_DEBUG, " DIMM is %uMB on side 1\n", sz.side1); + printk(BIOS_DEBUG, " DIMM is %uMB on side 2\n", sz.side2);
/* - Memory compatibility checks - */ + /* Test for PC133 (i82830 only supports PC133) */ /* PC133 SPD9 - cycle time is always 75 */ if (spd_read_byte(device, SPD_MIN_CYCLE_TIME_AT_CAS_MAX) != 0x75) { - print_err("SPD9 DIMM Is Not PC133 Compatable\n"); + printk(BIOS_ERR, "SPD9 DIMM Is Not PC133 Compatable\n"); die("HALT\n"); } /* PC133 SPD10 - access time is always 54 */ if (spd_read_byte(device, SPD_ACCESS_TIME_FROM_CLOCK) != 0x54) { - print_err("SPD10 DIMM Is Not PC133 Compatable\n"); + printk(BIOS_ERR, "SPD10 DIMM Is Not PC133 Compatable\n"); die("HALT\n"); }
@@ -285,22 +251,20 @@ * side or larger than 256MB per side. */ if ((sz.side2 != 0) && (sz.side1 != sz.side2)) { - print_err("This northbridge only supports\n"); - print_err("symmetrical dual-sided DIMMs\n"); - print_err("booting as a single-sided DIMM\n"); + printk(BIOS_ERR, "This northbridge only supports\n"); + printk(BIOS_ERR, "symmetrical dual-sided DIMMs\n"); + printk(BIOS_ERR, "booting as a single-sided DIMM\n"); sz.side2 = 0; } if ((sz.side1 < 32)) { - print_err("DIMMs smaller than 32MB per side\n"); - print_err("are not supported on this northbridge\n"); + printk(BIOS_ERR, "DIMMs smaller than 32MB per side\n"); + printk(BIOS_ERR, "are not supported on this northbridge\n"); die("HALT\n"); }
if ((sz.side1 > 256)) { - print_err - ("DIMMs larger than 256MB per side\n"); - print_err - ("are not supported on this northbridge\n"); + printk(BIOS_ERR, "DIMMs larger than 256MB per side\n"); + printk(BIOS_ERR, "are not supported on this northbridge\n"); die("HALT\n"); } /* - End Memory compatibility checks - */ @@ -313,9 +277,7 @@ if (sz.side2) drb2 = sz.side2 / 32; } else { - PRINT_DEBUG("No DIMM found in slot "); - PRINT_DEBUG_HEX8(i); - PRINT_DEBUG("\n"); + printk(BIOS_DEBUG, "No DIMM found in slot %u\n", i);
/* If there's no DIMM in the slot, set value to 0. */ drb1 = 0; @@ -325,30 +287,14 @@ if (i == 0) { pci_write_config8(NORTHBRIDGE, DRB, drb1); pci_write_config8(NORTHBRIDGE, DRB + 1, drb1 + drb2); - PRINT_DEBUG("DRB 0x"); - PRINT_DEBUG_HEX8(DRB); - PRINT_DEBUG(" has been set to 0x"); - PRINT_DEBUG_HEX8(drb1); - PRINT_DEBUG("\n"); - PRINT_DEBUG("DRB1 0x"); - PRINT_DEBUG_HEX8(DRB + 1); - PRINT_DEBUG(" has been set to 0x"); - PRINT_DEBUG_HEX8(drb1 + drb2); - PRINT_DEBUG("\n"); + PRINTK_DEBUG(" DRB 0x%02x has been set to 0x%02x\n", DRB, drb1); + PRINTK_DEBUG(" DRB1 0x%02x has been set to 0x%02x\n", DRB + 1, drb1 + drb2); } else if (i == 1) { value = pci_read_config8(NORTHBRIDGE, DRB + 1); pci_write_config8(NORTHBRIDGE, DRB + 2, value + drb1); pci_write_config8(NORTHBRIDGE, DRB + 3, value + drb1 + drb2); - PRINT_DEBUG("DRB2 0x"); - PRINT_DEBUG_HEX8(DRB + 2); - PRINT_DEBUG(" has been set to 0x"); - PRINT_DEBUG_HEX8(value + drb1); - PRINT_DEBUG("\n"); - PRINT_DEBUG("DRB3 0x"); - PRINT_DEBUG_HEX8(DRB + 3); - PRINT_DEBUG(" has been set to 0x"); - PRINT_DEBUG_HEX8(value + drb1 + drb2); - PRINT_DEBUG("\n"); + PRINTK_DEBUG(" DRB2 0x%02x has been set to 0x%02x\n", DRB + 2, value + drb1); + PRINTK_DEBUG(" DRB3 0x%02x has been set to 0x%02x\n", DRB + 3, value + drb1 + drb2);
/* We need to set the highest DRB value to 0x64 and 0x65. * These are supposed to be "Reserved" but memory will @@ -371,9 +317,7 @@
/* First check if a DIMM is actually present. */ if (spd_read_byte(device, SPD_MEMORY_TYPE) == 0x4) { - print_debug("Found DIMM in slot "); - print_debug_hex8(i); - print_debug(", setting DRA...\n"); + PRINTK_DEBUG("Found DIMM in slot %u\n", i);
dra = 0x00;
@@ -402,7 +346,7 @@ } else if (dra == 16) { dra = 0xF3; /* 16KB */ } else { - print_err("Page size not supported\n"); + printk(BIOS_ERR, "Page size not supported\n"); die("HALT\n"); } } else if (value == 2) { @@ -415,18 +359,16 @@ } else if (dra == 16) { dra = 0x33; /* 16KB */ } else { - print_err("Page size not supported\n"); + printk(BIOS_ERR, "Page size not supported\n"); die("HALT\n"); } } else { - print_err("# of banks of DIMM not supported\n"); + printk(BIOS_ERR, "# of banks of DIMM not supported\n"); die("HALT\n"); }
} else { - PRINT_DEBUG("No DIMM found in slot "); - PRINT_DEBUG_HEX8(i); - PRINT_DEBUG(", setting DRA to 0xFF\n"); + PRINTK_DEBUG("No DIMM found in slot %u\n", i);
/* If there's no DIMM in the slot, set dra value to 0xFF. */ dra = 0xFF; @@ -434,11 +376,7 @@
/* Set the value for DRAM Row Attribute Registers */ pci_write_config8(NORTHBRIDGE, DRA + i, dra); - PRINT_DEBUG("DRA 0x"); - PRINT_DEBUG_HEX8(DRA + i); - PRINT_DEBUG(" has been set to 0x"); - PRINT_DEBUG_HEX8(dra); - PRINT_DEBUG("\n"); + PRINTK_DEBUG(" DRA 0x%02x has been set to 0x%02x\n", DRA + i, dra); } }
@@ -467,7 +405,7 @@
static void sdram_set_registers(void) { - PRINT_DEBUG("Setting initial sdram registers....\n"); + PRINTK_DEBUG("Setting initial sdram registers....\n");
/* Calculate the value for DRT DRAM Timing Register */ set_dram_timing(); @@ -481,7 +419,7 @@ /* Setup DRAM Row Attribute Registers */ set_dram_row_attributes();
- PRINT_DEBUG("Initial sdram registers have been set.\n"); + PRINTK_DEBUG("Initial sdram registers have been set.\n"); }
static void northbridge_set_registers(void) @@ -489,7 +427,7 @@ u16 value; int igd_memory = 0;
- PRINT_DEBUG("Setting initial nothbridge registers....\n"); + PRINTK_DEBUG("Setting initial nothbridge registers....\n");
/* Set the value for Fixed DRAM Hole Control Register */ pci_write_config8(NORTHBRIDGE, FDHC, 0x00); @@ -535,7 +473,7 @@ value |= 1; // 64MB aperture pci_write_config16(NORTHBRIDGE, GCC1, value);
- PRINT_DEBUG("Initial northbridge registers have been set.\n"); + PRINTK_DEBUG("Initial northbridge registers have been set.\n"); }
static void sdram_initialize(void) @@ -545,20 +483,20 @@ /* Setup Initial SDRAM Registers */ sdram_set_registers();
- /* 0. Wait until power/voltages and clocks are stable (200us). */ + /* Wait until power/voltages and clocks are stable (200us). */ udelay(200);
/* Initialize each row of memory one at a time */ initialize_dimm_rows();
/* Enable Refresh */ - PRINT_DEBUG("Enabling Refresh\n"); + PRINTK_DEBUG("Enabling Refresh\n"); reg32 = pci_read_config32(NORTHBRIDGE, DRC); reg32 |= (RAM_COMMAND_REFRESH << 8); pci_write_config32(NORTHBRIDGE, DRC, reg32);
/* Set initialization complete */ - PRINT_DEBUG("Setting initialization complete\n"); + PRINTK_DEBUG("Setting initialization complete\n"); reg32 = pci_read_config32(NORTHBRIDGE, DRC); reg32 |= (RAM_COMMAND_IC << 29); pci_write_config32(NORTHBRIDGE, DRC, reg32); @@ -566,6 +504,6 @@ /* Setup Initial Northbridge Registers */ northbridge_set_registers();
- PRINT_DEBUG("Northbridge following SDRAM init:\n"); + PRINTK_DEBUG("Northbridge following SDRAM init:\n"); DUMPNORTH(); }
On 4/11/10 7:50 AM, repository service wrote:
Author: linux_junkie Date: Sun Apr 11 07:50:08 2010 New Revision: 5402 URL: https://tracker.coreboot.org/trac/coreboot/changeset/5402
Log: Trivial changes to i82830 raminit.c for USE_PRINTK_IN_CAR. Signed-off-by: Joseph Smith joe@settoplinux.org Acked-by: Joseph Smith joe@settoplinux.org
Modified: trunk/src/northbridge/intel/i82830/raminit.c
Modified: trunk/src/northbridge/intel/i82830/raminit.c
--- trunk/src/northbridge/intel/i82830/raminit.c Sat Apr 10 00:25:27 2010 (r5401) +++ trunk/src/northbridge/intel/i82830/raminit.c Sun Apr 11 07:50:08 2010 (r5402) @@ -30,16 +30,10 @@
static void ram_read32(u8 dimm_start, u32 offset) { if (offset == 0x55aa55aa) {
...
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
/* MRS command */ /* TODO: Set offset 0x1d0 according to DRT values */
PRINT_DEBUG(" MRS ");
PRINTK_DEBUG(" MRS\n");
You could make these PRINTK(BIOS_DEBUG, ...) instead of PRINTK_DEBUG(...) to be able to easily use different log levels with one macro.
On 04/11/2010 07:11 AM, Stefan Reinauer wrote:
On 4/11/10 7:50 AM, repository service wrote:
Author: linux_junkie Date: Sun Apr 11 07:50:08 2010 New Revision: 5402 URL: https://tracker.coreboot.org/trac/coreboot/changeset/5402
Log: Trivial changes to i82830 raminit.c for USE_PRINTK_IN_CAR. Signed-off-by: Joseph Smithjoe@settoplinux.org Acked-by: Joseph Smithjoe@settoplinux.org
Modified: trunk/src/northbridge/intel/i82830/raminit.c
Modified: trunk/src/northbridge/intel/i82830/raminit.c
--- trunk/src/northbridge/intel/i82830/raminit.c Sat Apr 10 00:25:27 2010 (r5401) +++ trunk/src/northbridge/intel/i82830/raminit.c Sun Apr 11 07:50:08 2010 (r5402) @@ -30,16 +30,10 @@
static void ram_read32(u8 dimm_start, u32 offset) { if (offset == 0x55aa55aa) {
...
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.
/* MRS command */ /* TODO: Set offset 0x1d0 according to DRT values */
PRINT_DEBUG(" MRS ");
PRINTK_DEBUG(" MRS\n");
You could make these PRINTK(BIOS_DEBUG, ...) instead of PRINTK_DEBUG(...) to be able to easily use different log levels with one macro.
True. If it would make the code more cleaner I could do that. Here is what it looks like with CONFIG_RAM_DEBUG enabled:
coreboot-4.0-r5401M Sun Apr 11 01:30:35 EDT 2010 starting... SMBus controller enabled Setting initial sdram registers.... Found DIMM in slot 0 DIMM is 256MB on side 1 DIMM is 256MB on side 2 DRB 0x60 has been set to 0x08 DRB1 0x61 has been set to 0x10 Found DIMM in slot 1 DIMM is 64MB on side 1 DIMM is 0MB on side 2 DRB2 0x62 has been set to 0x12 DRB3 0x63 has been set to 0x12 Found DIMM in slot 0 DRA 0x70 has been set to 0x22 Found DIMM in slot 1 DRA 0x71 has been set to 0xf1 Initial sdram registers have been set. Initializing SDRAM Row 0 NOP Sending RAM command 0x00000010 to 0x00000000 Pre-charging all banks Sending RAM command 0x00000020 to 0x00000000 8 CBR refreshes Sending RAM command 0x00000060 to 0x00000000 Sending RAM command 0x00000060 to 0x00000000 Sending RAM command 0x00000060 to 0x00000000 Sending RAM command 0x00000060 to 0x00000000 Sending RAM command 0x00000060 to 0x00000000 Sending RAM command 0x00000060 to 0x00000000 Sending RAM command 0x00000060 to 0x00000000 Sending RAM command 0x00000060 to 0x00000000 MRS Sending RAM command 0x00000030 to 0x000001d0 Normal operation mode Sending RAM command 0x00000070 to 0x00000000 Performing dummy read/write Reading RAM at 0x00000000 => 0xbd7f75ff Writing RAM at 0x00000000 <= 0x55aa55aa Reading RAM at 0x00000000 => 0x55aa55aa Initializing SDRAM Row 1 NOP Sending RAM command 0x00000010 to 0x10000000 Pre-charging all banks Sending RAM command 0x00000020 to 0x10000000 8 CBR refreshes Sending RAM command 0x00000060 to 0x10000000 Sending RAM command 0x00000060 to 0x10000000 Sending RAM command 0x00000060 to 0x10000000 Sending RAM command 0x00000060 to 0x10000000 Sending RAM command 0x00000060 to 0x10000000 Sending RAM command 0x00000060 to 0x10000000 Sending RAM command 0x00000060 to 0x10000000 Sending RAM command 0x00000060 to 0x10000000 MRS Sending RAM command 0x00000030 to 0x100001d0 Normal operation mode Sending RAM command 0x00000070 to 0x10000000 Performing dummy read/write Reading RAM at 0x10000000 => 0x57afffff Writing RAM at 0x10000000 <= 0x55aa55aa Reading RAM at 0x10000000 => 0x55aa55aa Initializing SDRAM Row 2 NOP Sending RAM command 0x00000010 to 0x20000000 Pre-charging all banks Sending RAM command 0x00000020 to 0x20000000 8 CBR refreshes Sending RAM command 0x00000060 to 0x20000000 Sending RAM command 0x00000060 to 0x20000000 Sending RAM command 0x00000060 to 0x20000000 Sending RAM command 0x00000060 to 0x20000000 Sending RAM command 0x00000060 to 0x20000000 Sending RAM command 0x00000060 to 0x20000000 Sending RAM command 0x00000060 to 0x20000000 Sending RAM command 0x00000060 to 0x20000000 MRS Sending RAM command 0x00000030 to 0x200001d0 Normal operation mode Sending RAM command 0x00000070 to 0x20000000 Performing dummy read/write Reading RAM at 0x20000000 => 0xf5ba55aa Writing RAM at 0x20000000 <= 0x55aa55aa Reading RAM at 0x20000000 => 0x55aa55aa Enabling Refresh Setting initialization complete Setting initial nothbridge registers.... Initial northbridge registers have been set. Northbridge following SDRAM init: PCI: 00:00.00 00: 86 80 75 35 06 00 10 00 04 00 00 06 00 00 00 00 10: 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 30: 00 00 00 00 40 00 00 00 00 00 00 00 00 00 00 00 40: 09 00 05 01 00 00 00 00 00 00 00 00 02 28 00 0e 50: 72 a0 41 00 00 00 00 00 00 30 33 33 33 33 33 33 60: 08 10 12 12 12 12 00 00 00 00 00 00 00 00 00 00 70: 22 f1 ff ff 00 00 00 00 10 00 00 00 70 01 00 20 80: 00 00 00 00 00 00 00 00 80 60 33 09 00 00 00 00 90: 02 38 00 00 00 00 00 00 00 00 00 00 00 00 00 00 a0: 02 00 20 00 17 02 00 1f 00 00 00 00 00 00 00 00 b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 c0: 00 54 0e 41 a2 99 01 00 c0 00 00 00 00 00 00 00 d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 e0: 00 00 00 00 00 00 00 00 00 00 00 00 1b 49 9b fc f0: 11 11 01 00 00 00 0b 05 35 cf 2f cf 22 cd 1e cc
And here is what it looks like when not enabled:
coreboot-4.0-r5401M Sun Apr 11 01:39:18 EDT 2010 starting... SMBus controller enabled Found DIMM in slot 0 DIMM is 256MB on side 1 DIMM is 256MB on side 2 Found DIMM in slot 1 DIMM is 64MB on side 1 DIMM is 0MB on side 2 Initializing SDRAM Row 0 Initializing SDRAM Row 1 Initializing SDRAM Row 2
Simple and clean :-)
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.
Performing dummy read/write Reading RAM at 0x20000000 => 0xf5ba55aa Writing RAM at 0x20000000 <= 0x55aa55aa Reading RAM at 0x20000000 => 0x55aa55aa
So this read/write is only for debugging? It will break suspend/resume, should you intend to go for that.
Stefan
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.
Hmm, so a preprocessing directive will cure this side effect?
Performing dummy read/write Reading RAM at 0x20000000 => 0xf5ba55aa Writing RAM at 0x20000000<= 0x55aa55aa Reading RAM at 0x20000000 => 0x55aa55aa
So this read/write is only for debugging? It will break suspend/resume, should you intend to go for that.
This is how original bios does it. raminit is run again on resume? That seems strange...
On 04/11/2010 10:23 AM, Stefan Reinauer wrote:
Performing dummy read/write Reading RAM at 0x20000000 => 0xf5ba55aa Writing RAM at 0x20000000<= 0x55aa55aa Reading RAM at 0x20000000 => 0x55aa55aa
So this read/write is only for debugging? It will break suspend/resume, should you intend to go for that.
This is how original bios does it. raminit is run again on resume? That seems strange...
Then why do you only do it if CONFIG_DEBUG_RAM_SETUP is on?
Yes, ram init (parts of it) run on resume.
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.
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 }