[RFC] v2: register reductions in i440bx/debug.c - coreboot

28 Aug 2008

Hello all,
the appended patch is single handedly able to enforce a
successful compilation for the mainboard msi/ms6119,
given that dump_spd_registers is activated in ms6119/auto.c.
Without the patch, romcc aborts compilation due to lack
of free processor registers.
A similar patch and some furher reductions have been
applied to my work on ms6147, and there produced a
bootable image.
I did try to introduce other reductions further into
the code base, but I was not successful in getting
compilable code until I removed the assignment
device = ctrl->channel0[0];
in favour of explicit calculations in this very file
i440bx/debug.c.
Instead of the present patch, I also tried some variations
in using a macro to perform the replacement of the original
variable 'device' (meant to improve readablility), but some
of those caused romcc to get stuck in a seemingly infinite
loop, so there is something wrong with the parsing performed
by romcc.
Now, I would like to know, if the kind of code that the present
patch introduces is at all acceptable by the project. In particular,
it is that repeated 'ctrl->channel0[i]' that comes to my mind.
The additional if-clause that I have introduced, but not fully
activated, does produce a further reduction of register use.
Any final patch will also have to touch the code pertaining
to the case of two memory channels, a case which is presently
deactivated in the repository code base.
Best regards,
Mats Erik Andersson
---
Index: ../src/northbridge/intel/i440bx/debug.c
===================================================================

--- ../src/northbridge/intel/i440bx/debug.c	(revision 3547)
+++ ../src/northbridge/intel/i440bx/debug.c	(arbetskopia)
@@ -4,29 +4,40 @@
    int i;
    print_debug("\r\n");
    for(i = 0; i < 4; i++) {
-		unsigned device;
-		device = ctrl->channel0[i];
-		if (device) {
+		/* Repeated calculation of device value
+		 * reduces the need of processor registers. */
+		if (ctrl->channel0[i]) {
    		int j;
    		print_debug("dimm: "); 
    		print_debug_hex8(i); 
    		print_debug(".0: ");
-			print_debug_hex8(device);
+			print_debug_hex8(ctrl->channel0[i]);
    		for(j = 0; j < 256; j++) {
+#if 1
    			int status;
-				unsigned char byte;
    			if ((j & 0xf) == 0) {
    				print_debug("\r\n");
    				print_debug_hex8(j);
    				print_debug(": ");
    			}
-				status = spd_read_byte(device, j);
+				status = spd_read_byte(ctrl->channel0[i], j);
    			if (status < 0) {
    				print_debug("bad device\r\n");
    				break;
    			}
-				byte = status & 0xff;
-				print_debug_hex8(byte);
+				print_debug_hex8(status & 0xff);
+#else	/* Further reduced use of processor registers */
+				if ((j & 0xf) == 0) {
+					print_debug("\r\n");
+					print_debug_hex8(j);
+					print_debug(": ");
+				}
+				if (spd_read_byte(ctrl->channel0[i], j) < 0) {
+					print_debug("bad device\r\n");
+					break;
+				}
+				print_debug_hex8(spd_read_byte(ctrl->channel0[i], j) & 0xff);
+#endif
    			print_debug_char(' ');
    		}
    		print_debug("\r\n");

    