Patchwork Kill dead code in Intel E7520 raminit_test

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2009-08-27 10:32:51
Message ID <4A9660D3.8090403@gmx.net>
Download mbox | patch
Permalink /patch/207/
State Superseded
Headers show

Comments

Carl-Daniel Hailfinger - 2009-08-27 10:32:51
/src/northbridge/intel/e7520/raminit_test.c is _the_ file which caused
the stongest WTF!?! reaction ever when I read it.
If redefines pci_read_config*() and pci_write_config*() as simple memory
accesses to a local array without _any_ side effects.
Same for pci_locate_device() which essentially operates on the fake
pci_read_config*().
Its rdmsr() function is hardcoded to return zero and wrmsr() is empty.
It has fake SPD arrays which are ignored.
spd_read_byte() accesses the fake SPD arrays, then the result of
spd_read_byte is discarded.
It messes with setjmp() and longjmp(), but basically only uses them to
hlt() upon invalid parameters to log2().
It calls raminit_main() at least 1 times and at most 73 times.
It uses AMD Athlon dualcore PCI BDF in sdram_initialize() although it is
an Intel chipset.
It writes to stdout and stderr. (Yes, really.)

Looking at this file is like watching a mass freeway pile-up. It's
horrible, but you can't stop looking.

Fix the majority of WTFs.

Can we just rip this file out?

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
ron minnich - 2009-08-27 11:58:30
My bet is this file was test scaffolding that has not been used for
many years. I doubt there is a reason to keep it.

ron
Stefan Reinauer - 2009-10-04 19:13:58
Carl-Daniel Hailfinger wrote:
> /src/northbridge/intel/e7520/raminit_test.c is _the_ file which caused
> the stongest WTF!?! reaction ever when I read it.
> If redefines pci_read_config*() and pci_write_config*() as simple memory
> accesses to a local array without _any_ side effects.
> Same for pci_locate_device() which essentially operates on the fake
> pci_read_config*().
> Its rdmsr() function is hardcoded to return zero and wrmsr() is empty.
> It has fake SPD arrays which are ignored.
> spd_read_byte() accesses the fake SPD arrays, then the result of
> spd_read_byte is discarded.
> It messes with setjmp() and longjmp(), but basically only uses them to
> hlt() upon invalid parameters to log2().
> It calls raminit_main() at least 1 times and at most 73 times.
> It uses AMD Athlon dualcore PCI BDF in sdram_initialize() although it is
> an Intel chipset.
> It writes to stdout and stderr. (Yes, really.)
>
> Looking at this file is like watching a mass freeway pile-up. It's
> horrible, but you can't stop looking.
>
> Fix the majority of WTFs.
>
> Can we just rip this file out?
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
>   

Was this ever merged?

Acked-by: Stefan Reinauer <stepan@coresystems.de>

And, yes, I think we should rip it out.

Patch

Index: LinuxBIOSv2-raminit_test_cleanup/src/northbridge/intel/e7520/raminit_test.c
===================================================================
--- LinuxBIOSv2-raminit_test_cleanup/src/northbridge/intel/e7520/raminit_test.c	(Revision 4591)
+++ LinuxBIOSv2-raminit_test_cleanup/src/northbridge/intel/e7520/raminit_test.c	(Arbeitskopie)
@@ -14,112 +14,18 @@ 
 	return 0;
 }
 
-#define PCI_ADDR(BUS, DEV, FN, WHERE) ( \
-	(((BUS) & 0xFF) << 16) | \
-	(((DEV) & 0x1f) << 11) | \
-	(((FN) & 0x07) << 8) | \
-	((WHERE) & 0xFF))
-
 #define PCI_DEV(BUS, DEV, FN) ( \
 	(((BUS) & 0xFF) << 16) | \
 	(((DEV) & 0x1f) << 11) | \
 	(((FN)  & 0x7) << 8))
 
-#define PCI_ID(VENDOR_ID, DEVICE_ID) \
-	((((DEVICE_ID) & 0xFFFF) << 16) | ((VENDOR_ID) & 0xFFFF))
-
 typedef unsigned device_t;
 
-unsigned char pci_register[256*5*3*256];
-
-static uint8_t pci_read_config8(device_t dev, unsigned where)
-{
-	unsigned addr;
-	addr = dev | where;
-	return pci_register[addr];
-}
-
-static uint16_t pci_read_config16(device_t dev, unsigned where)
-{
-	unsigned addr;
-	addr = dev | where;
-	return pci_register[addr] | (pci_register[addr + 1]  << 8);
-}
-
-static uint32_t pci_read_config32(device_t dev, unsigned where)
-{
-	unsigned addr;
-	uint32_t value;
-	addr = dev | where;
-	value =  pci_register[addr] | 
-		(pci_register[addr + 1]  << 8) |
-		(pci_register[addr + 2]  << 16) |
-		(pci_register[addr + 3]  << 24);
-
-#if 0
-	print_debug("pcir32(");
-	print_debug_hex32(addr);
-	print_debug("):");
-	print_debug_hex32(value);
-	print_debug("\n");
-#endif
-	return value;
-
-}
-
-static void pci_write_config8(device_t dev, unsigned where, uint8_t value)
-{
-	unsigned addr;
-	addr = dev | where;
-	pci_register[addr] = value;
-}
-
-static void pci_write_config16(device_t dev, unsigned where, uint16_t value)
-{
-	unsigned addr;
-	addr = dev | where;
-	pci_register[addr] = value & 0xff;
-	pci_register[addr + 1] = (value >> 8) & 0xff;
-}
-
-static void pci_write_config32(device_t dev, unsigned where, uint32_t value)
-{
-	unsigned addr;
-	addr = dev | where;
-	pci_register[addr] = value & 0xff;
-	pci_register[addr + 1] = (value >> 8) & 0xff;
-	pci_register[addr + 2] = (value >> 16) & 0xff;
-	pci_register[addr + 3] = (value >> 24) & 0xff;
-
-#if 0
-	print_debug("pciw32(");
-	print_debug_hex32(addr);
-	print_debug(", ");
-	print_debug_hex32(value);
-	print_debug(")\n");
-#endif
-}
-
-#define PCI_DEV_INVALID (0xffffffffU)
-static device_t pci_locate_device(unsigned pci_id, device_t dev)
-{
-	for(; dev <= PCI_DEV(255, 31, 7); dev += PCI_DEV(0,0,1)) {
-		unsigned int id;
-		id = pci_read_config32(dev, 0);
-		if (id == pci_id) {
-			return dev;
-		}
-	}
-	return PCI_DEV_INVALID;
-}
-
-
-
-
 static void uart_tx_byte(unsigned char data)
 {
 	write(STDOUT_FILENO, &data, 1);
 }
+
 static void hlt(void)
 {
 	longjmp(end_buf, 2);
@@ -144,161 +50,10 @@ 
         return pow;
 }
 
-typedef struct msr_struct 
-{
-	unsigned lo;
-	unsigned hi;
-} msr_t;
-
-static inline msr_t rdmsr(unsigned index)
-{
-	msr_t result;
-	result.lo = 0;
-	result.hi = 0;
-	return result;
-}
-
-static inline void wrmsr(unsigned index, msr_t msr)
-{
-}
-
 #include "raminit.h"
 
 #define SIO_BASE 0x2e
 
-static void hard_reset(void)
-{
-	/* FIXME implement the hard reset case... */
-	longjmp(end_buf, 3);
-}
-
-static void memreset_setup(void)
-{
-	/* Nothing to do */
-}
-
-static void memreset(int controllers, const struct mem_controller *ctrl)
-{
-	/* Nothing to do */
-}
-
-static inline void activate_spd_rom(const struct mem_controller *ctrl)
-{
-	/* nothing to do */
-}
-
-
-static uint8_t spd_mt4lsdt464a[256] = 
-{
-	0x80, 0x08, 0x04, 0x0C, 0x08, 0x01, 0x40, 0x00, 0x01, 0x70, 
-	0x54, 0x00, 0x80, 0x10, 0x00, 0x01, 0x8F, 0x04, 0x06, 0x01, 
-	0x01, 0x00, 0x0E, 0x75, 0x54, 0x00, 0x00, 0x0F, 0x0E, 0x0F,
-
-	0x25, 0x08, 0x15, 0x08, 0x15, 0x08, 0x00, 0x12, 0x01, 0x4E,
-	0x9C, 0xE4, 0xB7, 0x46, 0x2C, 0xFF, 0x01, 0x02, 0x03, 0x04,
-	0x05, 0x06, 0x07, 0x08, 0x09, 0x01, 0x02, 0x03, 0x04, 0x05,
-	0x06, 0x07, 0x08, 0x09, 0x00,
-};
-
-static uint8_t spd_micron_512MB_DDR333[256] = 
-{
-	0x80, 0x08, 0x07, 0x0d, 0x0b, 0x02, 0x48, 0x00, 0x04, 0x60, 
-	0x70, 0x02, 0x82, 0x04, 0x04, 0x01, 0x0e, 0x04, 0x0c, 0x01, 
-	0x02, 0x26, 0xc0, 0x75, 0x70, 0x00, 0x00, 0x48, 0x30, 0x48, 
-	0x2a, 0x80, 0x80, 0x80, 0x45, 0x45, 0x00, 0x00, 0x00, 0x00, 
-	0x00, 0x3c, 0x48, 0x30, 0x28, 0x50, 0x00, 0x01, 0x00, 0x00, 
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
-	0x00, 0x00, 0x10, 0x6f, 0x2c, 0xff, 0xff, 0xff, 0xff, 0xff, 
-	0xff, 0xff, 0x01, 0x33, 0x36, 0x56, 0x44, 0x44, 0x46, 0x31, 
-	0x32, 0x38, 0x37, 0x32, 0x47, 0x2d, 0x33, 0x33, 0x35, 0x43, 
-	0x33, 0x03, 0x00, 0x03, 0x23, 0x17, 0x07, 0x5a, 0xb2, 0x00, 
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff 
-};
-
-static uint8_t spd_micron_256MB_DDR333[256] = 
-{
-	0x80, 0x08, 0x07, 0x0d, 0x0b, 0x01, 0x48, 0x00, 0x04, 0x60, 
-	0x70, 0x02, 0x82, 0x04, 0x04, 0x01, 0x0e, 0x04, 0x0c, 0x01, 
-	0x02, 0x26, 0xc0, 0x75, 0x70, 0x00, 0x00, 0x48, 0x30, 0x48, 
-	0x2a, 0x80, 0x80, 0x80, 0x45, 0x45, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x3c, 0x48, 0x30, 0x23, 0x50, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x58, 0x2c, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0x01, 0x31, 0x38, 0x56, 0x44, 0x44, 0x46, 0x36, 
-	0x34, 0x37, 0x32, 0x47, 0x2d, 0x33, 0x33, 0x35, 0x43, 0x31,
-	0x20, 0x01, 0x00, 0x03, 0x19, 0x17, 0x05, 0xb2, 0xf4, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 
-};
-
-#define MAX_DIMMS 16
-static uint8_t spd_data[MAX_DIMMS*256];
-
-static unsigned spd_count, spd_fail_count;
-static int spd_read_byte(unsigned device, unsigned address)
-{
-	int result;
-	spd_count++;
-	if ((device < 0x50) || (device >= (0x50 +MAX_DIMMS))) {
-		result = -1;
-	}
-	else {
-		device -= 0x50;
-		
-		if (address > 256) {
-			result = -1;
-		}
-		else if (spd_data[(device << 8) | 2] != 7) {
-			result = -1;
-		}
-		else {
-			result = spd_data[(device << 8) | address];
-		}
-	}
-#if 0
-	print_debug("spd_read_byte(");
-	print_debug_hex32(device);
-	print_debug(", ");
-	print_debug_hex32(address);
-	print_debug(") -> ");
-	print_debug_hex32(result);
-	print_debug("\n");
-#endif
-	if (spd_count >= spd_fail_count) {
-		result = -1;
-	}
-	return result;
-}
-
 /* no specific code here. this should go away completely */
 static void coherent_ht_mainboard(unsigned cpus)
 {
@@ -341,62 +96,13 @@ 
 #endif
 	};
 	console_init();
-	memreset_setup();
 	sdram_initialize(ARRAY_SIZE(cpu), cpu);
 
 }
 
-static void reset_tests(void)
-{
-	/* Clear the results of any previous tests */
-	memset(pci_register, 0, sizeof(pci_register));
-	memset(spd_data, 0, sizeof(spd_data));
-	spd_count = 0;
-	spd_fail_count = UINT_MAX;
-
-	pci_write_config32(PCI_DEV(0, 0x18, 3), NORTHBRIDGE_CAP,
-		NBCAP_128Bit |
-		NBCAP_MP|  NBCAP_BIG_MP |
-		/* NBCAP_ECC | NBCAP_CHIPKILL_ECC | */
-		(NBCAP_MEMCLK_200MHZ << NBCAP_MEMCLK_SHIFT) |
-		NBCAP_MEMCTRL);
-
-	pci_write_config32(PCI_DEV(0, 0x19, 3), NORTHBRIDGE_CAP,
-		NBCAP_128Bit |
-		NBCAP_MP|  NBCAP_BIG_MP |
-		/* NBCAP_ECC | NBCAP_CHIPKILL_ECC | */
-		(NBCAP_MEMCLK_200MHZ << NBCAP_MEMCLK_SHIFT) |
-		NBCAP_MEMCTRL);
-
-#if 0
-	pci_read_config32(PCI_DEV(0, 0x18, 3), NORTHBRIDGE_CAP);
-#endif
-}
-
 static void test1(void)
 {
-	reset_tests();
-
-	memcpy(&spd_data[0*256], spd_micron_512MB_DDR333, 256);
-	memcpy(&spd_data[1*256], spd_micron_512MB_DDR333, 256);
-#if 0
-	memcpy(&spd_data[2*256], spd_micron_512MB_DDR333, 256);
-	memcpy(&spd_data[3*256], spd_micron_512MB_DDR333, 256);
-
-	memcpy(&spd_data[8*256], spd_micron_512MB_DDR333, 256);
-	memcpy(&spd_data[9*256], spd_micron_512MB_DDR333, 256);
-	memcpy(&spd_data[10*256], spd_micron_512MB_DDR333, 256);
-	memcpy(&spd_data[11*256], spd_micron_512MB_DDR333, 256);
-#endif
-
 	raminit_main();
-	
-#if 0
-	print_debug("spd_count: ");
-	print_debug_hex32(spd_count);
-	print_debug("\r\n");
-#endif
-	
 }
 
 
@@ -407,16 +113,7 @@ 
 	if (setjmp(end_buf) != 0) {
 		goto done;
 	}
-	reset_tests();
-	spd_fail_count = i;
 
-	print_debug("\r\nSPD will fail after: ");
-	print_debug_hex32(spd_fail_count);
-	print_debug(" accesses.\r\n");
-	
-	memcpy(&spd_data[0*256], spd_micron_512MB_DDR333, 256);
-	memcpy(&spd_data[1*256], spd_micron_512MB_DDR333, 256);
-	
 	raminit_main();
 
  done: