Re: [LinuxBIOS] i82830 raminit.c

[Corey Osgood]

Joseph Smith wrote:
> Ok, I think I am ready to test my i82830 raminit.c. I started this
> from the i82810 code, but I want it to be able to configure an
> assymetric dimm. If anyone has a few minutes can you check this out,
> and give me some feedback??
> Thanks - Joe
>
> /*
> This file is part of the LinuxBIOS project.
> Originally written for the i82810 by:
> Copyright (C) 2007 Uwe Hermann
> Copyright (C) 2007 Corey Osgood
> Modified for the i82830 by:

I think you can drop these, perhaps with a comment below stating you're
the one that ported it. Besides, what Uwe wrote was actually for the
i440bx...
> /* Uncomment this to enable debugging output. */
> // #define DEBUG_RAM_SETUP 1

You'll probably want that uncommented before you begin testing.
> /* DRC[6:4] - SDRAM Mode Select (SMS). */
> #define RAM_COMMAND_NOP		 0x1
> #define RAM_COMMAND_PRECHARGE	 0x2
> #define RAM_COMMAND_MRS		 0x3
> #define RAM_COMMAND_CBR		 0x6
> #define RAM_COMMAND_NORMAL	 0x7

Hmm, 0x0 does nothing? Even if you don't use it, just state it for
clarity's sake please.
> static struct dimm_size spd_get_dimm_size(unsigned device)
> {
>    /* Calculate the log base 2 size of a DIMM in bits */
>    struct dimm_size sz;
>    int value, low;
>    sz.side1 = 0;
>    sz.side2 = 0;
>
> /* test for sdram */
>    value = spd_read_byte(device, 2);      /* type */
>       if (value < 0) goto hw_err;
>    if (value != 4) {
>    	print_debug("SPD2 DIMM Is Not Compatable\r\n");
>    	goto val_err;
>    }
>
> /* test for PC133 (i830 only supports PC133) */
>    value = spd_read_byte(device, 9);      /* cycle time */
>       if (value < 0) goto hw_err;
>    if (value != 75) {
>    	print_debug("SPD9 DIMM Is Not PC133 Compatable\r\n");
>    	goto val_err;
>    }
>    value = 0;
>    value = spd_read_byte(device, 10);     /* access time */
>       if (value < 0) goto hw_err;
>    if (value != 54) {
>    	print_debug("SPD10 DIMM Is Not PC133 Compatable\r\n");
>    	goto val_err;
>    }
>
> /* Note it might be easier to use byte 31 here, it has the DIMM size as
>     * a multiple of 4MB.  The way we do it now we can size both
>     * sides of an assymetric dimm.
>     */
>    value = spd_read_byte(device, 3);	/* rows */
>    if (value < 0) goto hw_err;
>    if ((value & 0xf) == 0) {
>    	print_debug("SPD3 Error With Rows\r\n");
>    	goto val_err;
>    }
>    sz.side1 += value & 0xf;
>
> value = spd_read_byte(device, 4);	/* columns */
>    if (value < 0) goto hw_err;
>    if ((value & 0xf) == 0) {
>    	print_debug("SPD4 Error With Columns\r\n");
>    	goto val_err;
>    }
>    sz.side1 += value & 0xf;
>
> value = spd_read_byte(device, 17);	/* banks */
>    if (value < 0) goto hw_err;
>    if ((value & 0xff) == 0) {
>    	print_debug("SPD17 Error With Banks\r\n");
>    	goto val_err;
>    }
>    sz.side1 += log2(value & 0xff);
>
> /* Get the module data width and convert it to a power of two */
>    value = spd_read_byte(device, 7);	/* (high byte) */
>    if (value < 0) goto hw_err;
>    value &= 0xff;
>    value <<= 8;
>
> low = spd_read_byte(device, 6);	/* (low byte) */
>    if (low < 0) goto hw_err;
>    value = value | (low & 0xff);
>    if ((value != 72) && (value != 64)) {
>    	print_debug("SPD6 Error With Data Width\r\n");
>    	goto val_err;
>    }
>    sz.side1 += log2(value);
>
> /* side 2 */
>    value = spd_read_byte(device, 5);	/* number of physical banks */
>    if (value < 0) goto hw_err;
>    value &= 7;
>    if (value == 1) goto out;
>    if (value != 2) {
>    	print_debug("SPD5 Error With Physical Banks\r\n");
>    	goto val_err;
>    }
>
> /* Start with the symmetrical case */
>    sz.side2 = sz.side1;
>
> value = spd_read_byte(device, 3);	/* rows */
>    if (value < 0) goto hw_err;
>    if ((value & 0xf0) == 0) goto out;	/* If symmetrical we are done */
>    sz.side2 -= (value & 0x0f);		/* Subtract out rows on side 1 */
>    sz.side2 += ((value >> 4) & 0x0f);	/* Add in rows on side 2 */
>
> value = spd_read_byte(device, 4);	/* columns */
>    if (value < 0) goto hw_err;
>    if ((value & 0xff) == 0) {
>    	print_debug("SPD4 Error With Side2 Rows\r\n");
>    	goto val_err;
>    }
>    sz.side2 -= (value & 0x0f);		/* Subtract out columns on side 1 */
>    sz.side2 += ((value >> 4) & 0x0f);	/* Add in columsn on side 2 */
>    goto out;
>
> val_err:
>    die("Bad SPD value\r\n");
>
> /* If an hw_error occurs report that I have no memory */
> hw_err:
>    sz.side1 = 0;
>    sz.side2 = 0;
>  out:
>    return sz;
>
> }

I really, _REALLY_ don't like this, even if it does work. Reading from
spd byte 31 gives you the bank size, in units of 4mb, for both sides of
an asymetrical dimm. Please look at the spd spec (Intel or Jedec) to see
what you need to do to read it, it's a bit odd. You need to figure out
how many places the 1 is shifted, the number of places * 4mb is the size
of the bank, if the banks are asymetrical the smaller size is side2. And
if you do decide to use it anyways, please add the original copyright
holder, I think this is from e7501?
> static long spd_set_ram_size(const struct mem_controller *ctrl, long dimm_mask)
> {
>    int i;
>    int cum;
>
> for(i = cum = 0; i < DIMM_SOCKETS; i++) {
>    	struct dimm_size sz;
>    	if (dimm_mask & (1 << i)) {
>    		sz = spd_get_dimm_size(ctrl->channel0[i]);
> 	/* WISHLIST: would be nice to display it as decimal? */
> 	print_debug("DIMM is ");
> 	print_debug_hex8(sz.side1);
> 	print_debug(" On Side 1\r\n");
> 	print_debug("DIMM is ");
> 	print_debug_hex8(sz.side2);
> 	print_debug(" On Side 2\r\n");
>
> 	/* Set the row offset, in KBytes (should this be
> 	* Kbits?). Note that this offset is the start of the
> 	* next row.
> 	*/
> 	row_offset = ((sz.side1 + sz.side2) * 1024);

If you're still in the initial testing stages, try to get one
single-sided dimm working first. Also, if you have a dual-sided dimm,
row_offset needs to be the address at the second side (as far as I
know), so only sz.side1 * 1024. This might be chip-dependent,
board-dependent, or I might be completely wrong...
> static void sdram_enable(int controllers, const struct mem_controller *ctrl) {
>
> int i;
>
> /* Todo: this will currently work with either one dual sided or two
>     * single sided DIMMs. Needs to work with 2 dual sided DIMMs in the
>     * long run.
>     */
>    long mask;
>    uint32_t row_offset;
>
> mask = spd_detect_dimms(ctrl);
>    spd_set_dram_size(ctrl, mask);
>
> /* 1. Apply NOP. */
>    PRINT_DEBUG("RAM Enable 1: Apply NOP\r\n");
>    do_ram_command(ctrl, RAM_COMMAND_NOP, 0, row_offset);

row_offset needs to be passed to spd_set_dram_size and set there, in the
long run. For now, just set row_offset to 0 and don't worry about it,
except possibly during MRS..might want to comment it out in
do_ram_command as well, just to be safe.

Other than that, looks good, so good luck with the testing!

-Corey