On Wed, Aug 20, 2008 at 09:19:23AM -0700, Ed Swierk wrote:
> This patch implements support for the memory controller and PCIe
> interface of the Intel EP80579 Integrated Processor. The memory
> controller code supports only 64-bit-wide DIMMs with x8 devices and
> ECC. It has been tested on a development board using a single Micron
> MT9HTF6472PY-667D2 DIMM. Your mileage will definitely vary with other
> DIMMs.
> 
> Signed-off-by: Ed Swierk <[EMAIL PROTECTED]>


> Index: coreboot-v2-3363/src/northbridge/intel/i3100/raminit_ep80579.c
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3363/src/northbridge/intel/i3100/raminit_ep80579.c
> @@ -0,0 +1,862 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Arastra, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> + *
> + */
> +
> +/* This code is based on src/northbridge/intel/e7520/raminit.c */

We should research svn history for who wrote the e7520/raminit.c
initially (if your code retains substantial parts of the original code),
so we can add the proper (C) line...

Same for the stock i3100 raminit.c we have in svn now.


> +static void sdram_set_registers(const struct mem_controller *ctrl)
> +{
> +     static const unsigned int register_values[] = {
> +             PCI_ADDR(0, 0x00, 0, CKDIS), 0xffff0000, 0x0000ffff,
> +             PCI_ADDR(0, 0x00, 0, DEVPRES), 0x00000000, 0x07420801 | 
> DEVPRES_CONFIG,
> +             PCI_ADDR(0, 0x00, 0, PAM-1), 0xcccccc7f, 0x33333000,
> +             PCI_ADDR(0, 0x00, 0, PAM+3), 0xcccccccc, 0x33333333,
> +             PCI_ADDR(0, 0x00, 0, DEVPRES1), 0xffffffff, 0x0040003a,
> +             PCI_ADDR(0, 0x00, 0, SMRBASE), 0x00000fff, BAR | 0,  
> +     };
> +     int i;
> +     int max;
> +
> +     max = sizeof(register_values)/sizeof(register_values[0]);

ARRAY_SIZE


> +     for(i = 0; i < max; i += 3) {
> +             device_t dev;
> +             unsigned where;
> +             unsigned long reg;
> +             dev = (register_values[i] & ~0xff) - PCI_DEV(0, 0x00, 0) + 
> ctrl->f0;
> +             where = register_values[i] & 0xff;
> +             reg = pci_read_config32(dev, where);
> +             reg &= register_values[i+1];
> +             reg |= register_values[i+2];
> +             pci_write_config32(dev, where, reg);
> +     }
> +}

> +
> +struct dimm_size {
> +     unsigned long side1;
> +     unsigned long side2;
> +};

This is pretty common, we have multiple copies of this struct definition
in v2, can we move it somewhere globally?


> +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, ddr2;
> +     sz.side1 = 0;
> +     sz.side2 = 0;
> +
> +     /* 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 */

Please use the #defines from spd.h (or similar) here, instead of
magic values such as the "3" and others below...


> +     if (value < 0) goto hw_err;
> +     if ((value & 0xf) == 0) goto val_err;
> +     sz.side1 += value & 0xf;
> +
> +     value = spd_read_byte(device, 4);       /* columns */
> +     if (value < 0) goto hw_err;
> +     if ((value & 0xf) == 0) goto val_err;
> +     sz.side1 += value & 0xf;
> +
> +     value = spd_read_byte(device, 17);      /* banks */
> +     if (value < 0) goto hw_err;
> +     if ((value & 0xff) == 0) 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)) goto val_err;
> +     sz.side1 += log2(value);
> +
> +     /* side 2 */
> +     value = spd_read_byte(device, 5);       /* number of ranks */
> +
> +     if (value < 0) goto hw_err;
> +     value &= 7;
> +     value++;
> +     if (value == 1) goto out;
> +     if (value != 2) 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) goto val_err;
> +     sz.side2 -= (value & 0x0f);             /* Subtract out columns on side 
> 1 */
> +     sz.side2 += ((value >> 4) & 0x0f);      /* Add in columns 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:
> +     print_debug("dimm ");
> +     print_debug_hex8(device);
> +     print_debug(" size = ");
> +     print_debug_hex8(sz.side1);
> +     print_debug(".");
> +     print_debug_hex8(sz.side2);
> +     print_debug("\r\n");
> +     return sz;
> +
> +}
> +
> +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]);
> +                     if (sz.side1 < 29) {
> +                             return -1; /* Report SPD error */
> +                     }
> +                     /* convert bits to multiples of 64MB */
> +                     sz.side1 -= 29;
> +                     cum += (1 << sz.side1);

> +                     /* DRB = 0x60 */

Comment not needed, the point of the DRB #define is that you don't have
to know the value here ;-)


> +                     pci_write_config8(ctrl->f0, DRB + (i*2), cum);
> +                     pci_write_config8(ctrl->f0, DRB+1 + (i*2), cum);
> +                     if (spd_read_byte(ctrl->channel0[i], 5) & 0x1) {
> +                             cum <<= 1;
> +                     }
> +             }
> +             else {
> +                     pci_write_config8(ctrl->f0, DRB + (i*2), cum);
> +                     pci_write_config8(ctrl->f0, DRB+1 + (i*2), cum);
> +             }
> +     }
> +     print_debug("DRB = ");
> +     print_debug_hex32(pci_read_config32(ctrl->f0, DRB));
> +     print_debug("\r\n");
> +
> +     cum >>= 1;
> +     /* set TOM top of memory 0xcc */
> +     pci_write_config16(ctrl->f0, TOM, cum);
> +     print_debug("TOM = ");
> +     print_debug_hex16(cum);
> +     print_debug("\r\n");
> +     /* set TOLM top of low memory */
> +     if(cum > 0x18) {
> +             cum = 0x18;
> +     }
> +     cum <<= 11;
> +     /* 0xc4 TOLM */
> +     pci_write_config16(ctrl->f0, TOLM, cum);
> +     print_debug("TOLM = ");
> +     print_debug_hex16(cum);
> +     print_debug("\r\n");
> +     return 0;
> +}
> +
> +
> +static unsigned int spd_detect_dimms(const struct mem_controller *ctrl)
> +{
> +     unsigned dimm_mask;
> +     int i;
> +     dimm_mask = 0;
> +     for(i = 0; i < DIMM_SOCKETS; i++) {
> +             int byte;
> +             unsigned device;
> +             device = ctrl->channel0[i];
> +             if (device) {
> +                     byte = spd_read_byte(device, 2);  /* Type */
> +                     print_debug("spd ");
> +                     print_debug_hex8(device);
> +                     print_debug(" = ");
> +                     print_debug_hex8(byte);
> +                     print_debug("\r\n");
> +                     if (byte == 8) {
> +                             dimm_mask |= (1 << i);
> +                     }
> +             }
> +     }
> +     return dimm_mask;
> +}
> +
> +
> +static int spd_set_row_attributes(const struct mem_controller *ctrl, 

> +             long dimm_mask)

How long must this mask be? Can we use u64 or maybe u32 here? I assume
the length is a fixed, known value?


> +{
> +     int value;
> +     int i;
> +
> +     for (i = 0; i < DIMM_SOCKETS; i++) {
> +             uint32_t dra = 0;
> +             int reg = 0;
> +
> +             if (!(dimm_mask & (1 << i))) {
> +                     continue;
> +             }
> +
> +             value = spd_read_byte(ctrl->channel0[i], 3);    /* rows */
> +             if (value < 0) die("Bad SPD data\r\n");
> +             if ((value & 0xf) == 0) die("Invalid # of rows\r\n");
> +             dra |= (((value-13) & 0x7) << 23);
> +             dra |= (((value-13) & 0x7) << 29);
> +             reg += value & 0xf;
> +
> +             value = spd_read_byte(ctrl->channel0[i], 4);    /* columns */
> +             if (value < 0) die("Bad SPD data\r\n");
> +             if ((value & 0xf) == 0) die("Invalid # of columns\r\n");
> +             dra |= (((value-10) & 0x7) << 20);
> +             dra |= (((value-10) & 0x7) << 26);
> +             reg += value & 0xf;
> +
> +             value = spd_read_byte(ctrl->channel0[i], 17);   /* banks */
> +             if (value < 0) die("Bad SPD data\r\n");
> +             if ((value & 0xff) == 0) die("Invalid # of banks\r\n");
> +             reg += log2(value & 0xff);
> +
> +             print_debug("dimm ");
> +             print_debug_hex8(i);
> +             print_debug(" reg = ");
> +             print_debug_hex8(reg);
> +             print_debug("\r\n");
> +
> +             /* set device density */
> +             dra |= ((31-reg));
> +             dra |= ((31-reg) << 6);
> +
> +             /* set device width (x8) */
> +             dra |= (1 << 4);
> +             dra |= (1 << 10);
> +     
> +             /* set device type (registered) */
> +             dra |= (1 << 14);
> +
> +             /* set number of ranks (0=single, 1=dual) */
> +             value = spd_read_byte(ctrl->channel0[i], 5);
> +             dra |= ((value & 0x1) << 17);
> +
> +             print_debug("DRA");
> +             print_debug_hex8(i);
> +             print_debug(" = ");
> +             print_debug_hex32(dra);
> +             print_debug("\r\n");
> +
> +             /* 0x70 DRA */
> +             pci_write_config32(ctrl->f0, DRA + (i*4), dra);
> +     }
> +     return 0;
> +}
> +
> +
> +static uint32_t spd_set_drt_attributes(const struct mem_controller *ctrl, 
> +             long dimm_mask, uint32_t drc)
> +{
> +     int i;
> +     uint32_t val, val1;
> +     uint32_t cl;
> +     uint32_t trc = 0;
> +     uint32_t trfc = 0;
> +     uint32_t tras = 0;
> +     uint32_t trtp = 0;
> +     uint32_t twtr = 0;
> +     int index = drc & 0x00000003;
> +     int ci;
> +     static const uint8_t latencies[] = { /* 533, 800, 400, 667 */
> +             0x10, 0x60, 0x10, 0x20 };
> +     static const uint32_t drt0[] = { /* 533, 800, 400, 667 */
> +             0x24240002, 0x24360002, 0x24220002, 0x24360002 };
> +     static const uint32_t drt1[] = { /* 533, 800, 400, 667 */
> +             0x00400000, 0x00900000, 0x00200000, 0x00700000 };
> +     static const uint32_t magic[] = { /* 533, 800, 400, 667 */
> +             0x007b8221, 0x00b94331, 0x005ca1a1, 0x009a62b1 };
> +     static const uint32_t mrs[] = { /* 533, 800, 400, 667 */
> +             0x07020000, 0x0b020000, 0x05020000, 0x09020000 };
> +     static const int cycle[] = { /* 533, 800, 400, 667 */
> +             15, 10, 20, 12 }; /* cycle time in 1/4 ns units */
> +     static const int byte40rem[] = {
> +             0, 1, 2, 2, 3, 3, 0, 0 }; /* byte 40 remainder in 1/4 ns units 
> */
> +
> +     /* CAS latency in cycles */
> +     val = latencies[index];
> +     for (i = 0; i < DIMM_SOCKETS; i++) {
> +             if (!(dimm_mask & (1 << i)))
> +                     continue;
> +             val &= spd_read_byte(ctrl->channel0[i], 18);
> +     }
> +     if (val & 0x10)
> +             cl = 4;
> +     else if (val & 0x20)
> +             cl = 5;
> +     else if (val & 0x40)
> +             cl = 6;
> +     else
> +             die("CAS latency mismatch\r\n");
> +     print_debug("cl = ");
> +     print_debug_hex8(cl);
> +     print_debug("\r\n");
> +
> +     ci = cycle[index];
> +
> +     /* Trc, Trfc in cycles */
> +     for (i = 0; i < DIMM_SOCKETS; i++) {
> +             if (!(dimm_mask & (1 << i)))
> +                     continue;
> +             val1 = spd_read_byte(ctrl->channel0[i], 40);
> +             val = spd_read_byte(ctrl->channel0[i], 41);
> +             val <<= 2; /* convert to 1/4 ns */
> +             val += byte40rem[(val1 >> 4) & 0x7];
> +             val = (val + ci - 1) / ci + 1; /* convert to cycles */
> +             if (trc < val)
> +                     trc = val;
> +             val = spd_read_byte(ctrl->channel0[i], 42);
> +             val <<= 2; /* convert to 1/4 ns */
> +             if (val1 & 0x01)
> +                     val += 1024;
> +             val += byte40rem[(val1 >> 1) & 0x7];
> +             val = (val + ci - 1) / ci; /* convert to cycles */
> +             if (trfc < val)
> +                     trfc = val;
> +     }
> +     print_debug("trc = ");
> +     print_debug_hex8(trc);
> +     print_debug("\r\n");
> +     print_debug("trfc = ");
> +     print_debug_hex8(trfc);
> +     print_debug("\r\n");
> +
> +     /* Tras, Trtp, Twtr in cycles */
> +     for (i = 0; i < DIMM_SOCKETS; i++) {
> +             if (!(dimm_mask & (1 << i)))
> +                     continue;
> +             val = spd_read_byte(ctrl->channel0[i], 30);
> +             val <<= 2; /* convert to 1/4 ns */
> +             val = (val + ci - 1) / ci; /* convert to cycles */
> +             if (tras < val)
> +                     tras = val;
> +             val = spd_read_byte(ctrl->channel0[i], 38);
> +             val = (val + ci - 1) / ci; /* convert to cycles */
> +             if (trtp < val)
> +                     trtp = val;
> +             val = spd_read_byte(ctrl->channel0[i], 37);
> +             val = (val + ci - 1) / ci; /* convert to cycles */
> +             if (twtr < val)
> +                     twtr = val;
> +     }
> +     print_debug("tras = ");
> +     print_debug_hex8(tras);
> +     print_debug("\r\n");
> +     print_debug("trtp = ");
> +     print_debug_hex8(trtp);
> +     print_debug("\r\n");
> +     print_debug("twtr = ");
> +     print_debug_hex8(twtr);
> +     print_debug("\r\n");
> +
> +     val = (drt0[index] | ((trc - 11) << 12) | ((cl - 3) << 9)
> +            | ((cl - 3) << 6) | ((cl - 3) << 3));
> +     print_debug("drt0 = ");
> +     print_debug_hex32(val);
> +     print_debug("\r\n");
> +     pci_write_config32(ctrl->f0, DRT0, val);
> +
> +     val = (drt1[index] | ((tras - 8) << 28) | ((trtp - 2) << 25)
> +            | (twtr << 15));
> +     print_debug("drt1 = ");
> +     print_debug_hex32(val);
> +     print_debug("\r\n");
> +     pci_write_config32(ctrl->f0, DRT1, val);
> +
> +     val = (magic[index]);
> +     print_debug("magic = ");
> +     print_debug_hex32(val);
> +     print_debug("\r\n");
> +     pci_write_config32(PCI_DEV(0, 0x08, 0), 0xcc, val);
> +
> +     val = (mrs[index] | (cl << 20));
> +     print_debug("mrs = ");
> +     print_debug_hex32(val);
> +     print_debug("\r\n");
> +     return val;
> +}
> +
> +static int spd_set_dram_controller_mode(const struct mem_controller *ctrl, 
> +             long dimm_mask)
> +{
> +     int value;
> +     int drc = 0;
> +     int i;
> +     msr_t msr;
> +     uint8_t cycle = 0x25;
> +
> +     /* 0x7c DRC */
> +     for (i = 0; i < DIMM_SOCKETS; i++) {
> +             if (!(dimm_mask & (1 << i)))
> +                     continue;
> +             if ((spd_read_byte(ctrl->channel0[i], 6) & 0xf0) != 0x40)
> +                     die("ERROR: Only 64-bit DIMMs supported\r\n");
> +             if (!(spd_read_byte(ctrl->channel0[i], 11) & 0x02))
> +                     die("ERROR: Only ECC DIMMs supported\r\n");
> +             if (spd_read_byte(ctrl->channel0[i], 13) != 0x08)
> +                     die("ERROR: Only x8 DIMMs supported\r\n");
> +
> +             value = spd_read_byte(ctrl->channel0[i], 9);    /* cycle time */
> +             if (value > cycle)
> +                     cycle = value;
> +     }
> +     print_debug("cycle = ");
> +     print_debug_hex8(cycle);
> +     print_debug("\r\n");
> +
> +     drc |= (1 << 20); /* enable ECC */
> +     drc |= (3 << 30); /* enable CKE on each dimm */
> +     drc |= (1 << 4); /* enable CKE globally */
> +

> +     /* eswierk check: */

Done already? Remove? Turn into "TODO"?


> +     /* set front side bus speed */
> +     msr = rdmsr(0xcd); /* returns 0 on Pentium M 90nm */
> +     print_debug("msr 0xcd = ");
> +     print_debug_hex32(msr.hi);
> +     print_debug_hex32(msr.lo);
> +     print_debug("\r\n");
> +

> +     /* eswierk check that this msr really indicates fsb speed! */

Ditto.


> +     if (msr.lo & 0x07) {
> +             print_info("533 MHz FSB\r\n");
> +             if (cycle <= 0x25) {
> +                     drc |= 0x5;
> +                     print_info("400 MHz DDR\r\n");
> +             } else if (cycle <= 0x30) {
> +                     drc |= 0x7;
> +                     print_info("333 MHz DDR\r\n");
> +             } else if (cycle <= 0x3d) {
> +                     drc |= 0x4;
> +                     print_info("266 MHz DDR\r\n");
> +             } else {
> +                     drc |= 0x2;
> +                     print_info("200 MHz DDR\r\n");
> +             }
> +     }
> +     else {
> +             print_info("400 MHz FSB\r\n");
> +             if (cycle <= 0x30) {
> +                     drc |= 0x7;
> +                     print_info("333 MHz DDR\r\n");
> +             } else if (cycle <= 0x3d) {
> +                     drc |= 0x0;
> +                     print_info("266 MHz DDR\r\n");
> +             } else {
> +                     drc |= 0x2;
> +                     print_info("200 MHz DDR\r\n");
> +             }
> +     }
> +
> +     print_debug("DRC = ");
> +     print_debug_hex32(drc);
> +     print_debug("\r\n");
> +
> +     return drc;
> +}
> +
> +static void sdram_set_spd_registers(const struct mem_controller *ctrl) 
> +{
> +     long dimm_mask;
> +     int i;
> +
> +     /* Test if we can read the spd */
> +     dimm_mask = spd_detect_dimms(ctrl);
> +     if (!(dimm_mask & ((1 << DIMM_SOCKETS) - 1))) {
> +             print_err("No memory for this cpu\r\n");
> +             return;
> +     }
> +     return;
> +}
> +
> +static void do_delay(void)
> +{
> +     int i;
> +     unsigned char b;
> +     for(i=0;i<16;i++)
> +             b=inb(0x80);
> +}    

Please use the global delay function, no need for yet another one.


> +
> +#define TIMEOUT_LOOPS 300000
> +
> +#define DCALCSR  0x040
> +#define DCALADDR 0x044
> +#define DCALDATA 0x048
> +#define MBCSR    0x140
> +#define MBADDR   0x144
> +#define MBDATA   0x148
> +#define DDRIOMC2 0x268

Move to some *.h file?

> +
> +static void set_on_dimm_termination_enable(const struct mem_controller *ctrl)
> +{
> +     unsigned char c1,c2;
> +        unsigned int dimm,i;
> +        unsigned int data32;
> +     unsigned int t4;
> + 
> +     /* Set up northbridge values */
> +     /* ODT enable */
> +     pci_write_config32(ctrl->f0, SDRC, 0xa0002c30);
> +
> +     c1 = pci_read_config8(ctrl->f0, DRB);
> +     c2 = pci_read_config8(ctrl->f0, DRB+2);
> +     if (c1 == c2) {
> +             /* 1 single-rank DIMM */
> +             data32 = 0x00000010;
> +     }
> +     else {
> +             /* 2 single-rank DIMMs or 1 double-rank DIMM */
> +             data32 = 0x00002010;
> +     }
> +
> +     print_debug("ODT Value = ");
> +     print_debug_hex32(data32);
> +     print_debug("\r\n");
> +
> +     pci_write_config32(ctrl->f0, DDR2ODTC, data32);
> +
> +     for(i=0;i<2;i++) {
> +             print_debug("ODT CS");
> +             print_debug_hex8(i);
> +             print_debug("\r\n");
> +
> +             write32(BAR+DCALADDR, 0x0b840001);
> +             write32(BAR+DCALCSR, 0x80000003 | ((i+1)<<21));
> +             data32 = read32(BAR+DCALCSR);
> +             while (data32 & 0x80000000)
> +                     data32 = read32(BAR+DCALCSR);
> +     }
> +}
> +
> +
> +static void dump_dcal_regs(void)
> +{
> +     int i;
> +     for (i = 0x0; i < 0x2a0; i += 4) {
> +             if ((i % 16) == 0) {
> +                     print_debug("\r\n");
> +                     print_debug_hex16(i);
> +                     print_debug(": ");
> +             }
> +             print_debug_hex32(read32(BAR+i));
> +             print_debug(" ");
> +     }
> +     print_debug("\r\n");
> +}
> +
> +
> +static void sdram_enable(int controllers, const struct mem_controller *ctrl)
> +{
> +     int i;
> +     int cs;
> +     long mask;
> +     uint32_t drc;
> +     uint32_t data32;
> +     uint32_t mode_reg;
> +     msr_t msr;
> +     uint16_t data16;
> +
> +     mask = spd_detect_dimms(ctrl);
> +     print_debug("Starting SDRAM Enable\r\n");
> +
> +     /* set dram type and Front Side Bus freq. */
> +     drc = spd_set_dram_controller_mode(ctrl, mask);
> +     if (drc == 0) {
> +             die("Error calculating DRC\r\n");
> +     }
> +     data32 = drc & ~(3 << 20);  /* clear ECC mode */
> +     data32 = data32 | (3 << 5);  /* temp turn off ODT */
> +     /* Set dram controller mode */
> +     /* 0x7c DRC */
> +     pci_write_config32(ctrl->f0, DRC, data32);
> +     
> +             /* turn the clocks on */
> +     /* 0x8c CKDIS */
> +     pci_write_config16(ctrl->f0, CKDIS, 0x0000);
> +     
> +             /* program row size DRB */
> +     spd_set_ram_size(ctrl, mask);
> +
> +             /* program row attributes DRA */
> +     spd_set_row_attributes(ctrl, mask);
> +
> +             /* program DRT timing values */ 
> +     mode_reg = spd_set_drt_attributes(ctrl, mask, drc);

The indentation of the comments here, and the coding style in general
are not quite correct in various places. Let's commit this anyway for
now, I'll do an 'indent'-run on the code later though, invoked as
listed here: http://www.coreboot.org/Development_Guidelines#Coding_Style


> +
> +     dump_dcal_regs();
> +
> +     for(cs=0;cs<2;cs++) { /* loop through each dimm to test */
> +             print_debug("NOP CS");
> +             print_debug_hex8(cs);
> +             print_debug("\r\n");
> +             /* Apply NOP */

Wrong comment (or comment in the wrong place)? This only does a delay, no?


> +             do_delay();
> +
> +             write32(BAR+DCALCSR, (0x00000000 | ((cs+1)<<21)));
> +             write32(BAR+DCALCSR, (0x80000000 | ((cs+1)<<21)));
> +
> +             data32 = read32(BAR+DCALCSR);
> +             while(data32 & 0x80000000)
> +                     data32 = read32(BAR+DCALCSR);
> +     }
> +     
> +     /* Apply NOP */

Ditto.


> +     do_delay();
> +
> +     for(cs=0;cs<2;cs++) {
> +             print_debug("NOP CS");
> +             print_debug_hex8(cs);
> +             print_debug("\r\n");
> +             write32(BAR + DCALCSR, (0x80000000 | ((cs+1)<<21))); 
> +             data32 = read32(BAR+DCALCSR);
> +             while(data32 & 0x80000000)
> +                     data32 = read32(BAR+DCALCSR);
> +     }
> +
> +     /* Precharge all banks */
> +     do_delay();
> +     for(cs=0;cs<2;cs++) {   
> +             print_debug("Precharge CS");
> +             print_debug_hex8(cs);
> +             print_debug("\r\n");
> +             write32(BAR+DCALADDR, 0x04000000);
> +             write32(BAR+DCALCSR, (0x80000002 | ((cs+1)<<21)));
> +             data32 = read32(BAR+DCALCSR);
> +             while(data32 & 0x80000000)
> +                     data32 = read32(BAR+DCALCSR);
> +     }
> +             
> +     /* EMRS: Enable DLLs, set OCD calibration mode to default */
> +     do_delay();
> +     for(cs=0;cs<2;cs++) {   
> +             print_debug("EMRS CS");
> +             print_debug_hex8(cs);
> +             print_debug("\r\n");
> +             write32(BAR+DCALADDR, 0x0b840001);
> +             write32(BAR+DCALCSR, (0x80000003 | ((cs+1)<<21)));
> +             data32 = read32(BAR+DCALCSR);
> +             while(data32 & 0x80000000)
> +                     data32 = read32(BAR+DCALCSR);
> +     }
> +     /* MRS: Reset DLLs */
> +     do_delay();
> +     for(cs=0;cs<2;cs++) {   
> +             print_debug("MRS CS");
> +             print_debug_hex8(cs);
> +             print_debug("\r\n");
> +             write32(BAR+DCALADDR, mode_reg);
> +             write32(BAR+DCALCSR, (0x80000003 | ((cs+1)<<21)));
> +             data32 = read32(BAR+DCALCSR);
> +             while(data32 & 0x80000000)
> +                     data32 = read32(BAR+DCALCSR);
> +     }
> +
> +     /* Precharge all banks */
> +     do_delay();
> +     do_delay();
> +     do_delay();
> +     for(cs=0;cs<2;cs++) {   
> +             print_debug("Precharge CS");
> +             print_debug_hex8(cs);
> +             print_debug("\r\n");
> +             write32(BAR+DCALADDR, 0x04000000);
> +             write32(BAR+DCALCSR, (0x80000002 | ((cs+1)<<21)));
> +             data32 = read32(BAR+DCALCSR);
> +             while(data32 & 0x80000000)
> +                     data32 = read32(BAR+DCALCSR);
> +     }

> +     /* Do 2 refreshes */
> +     do_delay();
> +     for(cs=0;cs<2;cs++) {   
> +             print_debug("Refresh CS");
> +             print_debug_hex8(cs);
> +             print_debug("\r\n");
> +             write32(BAR+DCALCSR, (0x80000001 | ((cs+1)<<21)));
> +             data32 = read32(BAR+DCALCSR);
> +             while(data32 & 0x80000000)
> +                     data32 = read32(BAR+DCALCSR);
> +     }
> +     do_delay();
> +     for(cs=0;cs<2;cs++) {   
> +             print_debug("Refresh CS");
> +             print_debug_hex8(cs);
> +             print_debug("\r\n");
> +             write32(BAR+DCALCSR, (0x80000001 | ((cs+1)<<21)));
> +             data32 = read32(BAR+DCALCSR);
> +             while(data32 & 0x80000000)
> +                     data32 = read32(BAR+DCALCSR);
> +     }

As far as I can see both blocks are identical? Why not a 'for' loop then?
This would also make a smaller coreboot (binary) image (less
code/printfs, less strings for the debug output, etc.).

Are there code issues or romcc issues?


> +     do_delay();
> +     /* for good luck do 6 more */

Better comment please, e.g. mention the reason why multiple runs are
needed and why 6 and not some other number. I assume the SPD/RAM/NB
datasheet says so?


> +     for(cs=0;cs<2;cs++) {
> +             write32(BAR+DCALCSR, (0x80000001 | ((cs+1)<<21)));
> +     }
> +     do_delay();
> +     for(cs=0;cs<2;cs++) {
> +             write32(BAR+DCALCSR, (0x80000001 | ((cs+1)<<21)));
> +     }
> +     do_delay();
> +     for(cs=0;cs<2;cs++) {
> +             write32(BAR+DCALCSR, (0x80000001 | ((cs+1)<<21)));
> +     }
> +     do_delay();
> +     for(cs=0;cs<2;cs++) {
> +             write32(BAR+DCALCSR, (0x80000001 | ((cs+1)<<21)));
> +     }
> +     do_delay();
> +     for(cs=0;cs<2;cs++) {
> +             write32(BAR+DCALCSR, (0x80000001 | ((cs+1)<<21)));
> +     }
> +     do_delay();
> +     for(cs=0;cs<2;cs++) {
> +             write32(BAR+DCALCSR, (0x80000001 | ((cs+1)<<21)));
> +     }
> +     do_delay();

Same as above, please pack this into a loop if possible.


> +     /* MRS: Set DLLs to normal */
> +     do_delay();
> +     for(cs=0;cs<2;cs++) {   
> +             print_debug("MRS CS");
> +             print_debug_hex8(cs);
> +             print_debug("\r\n");
> +             write32(BAR+DCALADDR, (mode_reg & ~(1<<24)));
> +             write32(BAR+DCALCSR, (0x80000003 | ((cs+1)<<21)));
> +             data32 = read32(BAR+DCALCSR);
> +             while(data32 & 0x80000000)
> +                     data32 = read32(BAR+DCALCSR);
> +     }
> +
> +     /* EMRS: Enable DLLs */
> +     do_delay();
> +     for(cs=0;cs<2;cs++) {
> +             print_debug("EMRS CS");
> +             print_debug_hex8(cs);
> +             print_debug("\r\n");
> +             write32(BAR+DCALADDR, 0x0b840001);
> +             write32(BAR+DCALCSR, (0x80000003 | ((cs+1)<<21)));
> +             data32 = read32(BAR+DCALCSR);
> +             while(data32 & 0x80000000)
> +                     data32 = read32(BAR+DCALCSR);
> +        }
> +
> +     do_delay();
> +     /* No command */
> +     write32(BAR+DCALCSR, 0x0000000f);
> +
> +     write32(BAR, 0x00100000);
> +
> +     /* enable on dimm termination */
> +     set_on_dimm_termination_enable(ctrl);
> +
> +     dump_dcal_regs();
> +
> +     /* receive enable calibration */
> +     do_delay();
> +     for(cs=0;cs<1;cs++) {
> +             print_debug("receive enable calibration CS");
> +             print_debug_hex8(cs);
> +             print_debug("\r\n");
> +             write32(BAR+DCALCSR, (0x8000000c | ((cs+1)<<21)));
> +             data32 = read32(BAR+DCALCSR);
> +             while(data32 & 0x80000000)
> +                     data32 = read32(BAR+DCALCSR);
> +     }
> +     
> +     dump_dcal_regs();
> +
> +     /* Adjust RCOMP */
> +     data32 = read32(BAR+DDRIOMC2);
> +     data32 &= ~(0xf << 16);
> +     data32 |= (0xb << 16);
> +     write32(BAR+DDRIOMC2, data32);
> +
> +     dump_dcal_regs();
> +
> +     /* 0x7c DRC */
> +     data32 = drc & ~(3 << 20);  /* clear ECC mode */
> +     pci_write_config32(ctrl->f0, DRC, data32);      
> +     write32(BAR+DCALCSR, 0x0008000f);
> +
> +     /* Clear memory and init ECC */
> +     for(cs=0;cs<2;cs++) {   
> +             if (!(mask & (1<<cs)))
> +                     continue;
> +             print_debug("clear memory CS");
> +             print_debug_hex8(cs);
> +             print_debug("\r\n");
> +             write32(BAR+MBCSR, 0xa00000f0 | ((cs+1)<<20) | (0<<16));
> +             data32 = read32(BAR+MBCSR);
> +             while(data32 & 0x80000000)
> +                     data32 = read32(BAR+MBCSR);
> +             if(data32 & 0x40000000)
> +                     print_debug("failed!\r\n");
> +     }
> +
> +     /* Clear read/write FIFO pointers */
> +     print_debug("clear read/write fifo pointers\r\n");
> +     write32(BAR+DDRIOMC2, read32(BAR+DDRIOMC2) | (1<<15));
> +     do_delay();
> +     write32(BAR+DDRIOMC2, read32(BAR+DDRIOMC2) & ~(1<<15));
> +     do_delay();
> +
> +     dump_dcal_regs();
> +
> +     print_debug("Done\r\n");
> +
> +     /* Set initialization complete */
> +     /* 0x7c DRC */
> +     drc |= (1 << 29);
> +     drc |= (3 << 30);
> +     data32 = drc & ~(3 << 20);  /* clear ECC mode */
> +     pci_write_config32(ctrl->f0, DRC, data32);      
> +
> +     /* Set the ECC mode */
> +     pci_write_config32(ctrl->f0, DRC, drc); 
> +
> +     /* The memory is now setup, use it */
> +     cache_lbmem(MTRR_TYPE_WRBACK);
> +}
> +
> +static inline int memory_initialized(void)
> +{
> +     return pci_read_config32(PCI_DEV(0, 0x00, 0), DRC) & (1 << 29);
> +}
> Index: coreboot-v2-3363/src/northbridge/intel/i3100/raminit_ep80579.h
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3363/src/northbridge/intel/i3100/raminit_ep80579.h
> @@ -0,0 +1,33 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Arastra, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> + *
> + */
> +

> +/* This code is based on src/northbridge/intel/e7520/raminit.h */

Nah, drop this, too trivial. This is soleley (C) Arastra, IMHO.


> +
> +#ifndef NORTHBRIDGE_INTEL_EP80579_RAMINIT_H
> +#define NORTHBRIDGE_INTEL_EP80579_RAMINIT_H

NORTHBRIDGE_INTEL_I3100_RAMINIT_EP80579_H for consistency.


> +
> +#define DIMM_SOCKETS 2
> +struct mem_controller {
> +     unsigned node_id;

uint16_t or u16 here, as also done below?

> +     device_t f0;
> +     uint16_t channel0[DIMM_SOCKETS];
> +};
> +
> +#endif
> Index: coreboot-v2-3363/src/northbridge/intel/i3100/ep80579.h
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3363/src/northbridge/intel/i3100/ep80579.h
> @@ -0,0 +1,61 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Arastra, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> + *
> + */
> +
> +#ifndef NORTHBRIDGE_INTEL_EP80579_H
> +#define NORTHBRIDGE_INTEL_EP80579_H

NORTHBRIDGE_INTEL_I3100_EP80579_H for consistency.


> +#define VID       0x00
> +#define DID       0x02
> +#define PCICMD    0x04
> +#define PCISTS    0x06
> +#define RID       0x08

These five are generic and already in pci.h I think (and not used in your
code anyway), I'd drop them.


> +#define SMRBASE   0x14
> +#define MCHCFG0        0x50
> +#define FDHC      0x58
> +#define PAM       0x59
> +#define DRB       0x60
> +#define DRT1      0x64
> +#define DRA       0x70
> +#define DRT0      0x78
> +#define DRC       0x7c
> +#define ECCDIAG   0x84
> +#define SDRC      0x88
> +#define CKDIS     0x8c
> +#define CKEDIS    0x8d
> +#define DEVPRES   0x9c
> +#define  DEVPRES_D0F0 (1 << 0)
> +#define  DEVPRES_D1F0 (1 << 1)
> +#define  DEVPRES_D2F0 (1 << 2)
> +#define  DEVPRES_D3F0 (1 << 3)
> +#define  DEVPRES_D4F0 (1 << 4)
> +#define  DEVPRES_D10F0 (1 << 5)
> +#define EXSMRC    0x9d
> +#define SMRAM     0x9e
> +#define EXSMRAMC  0x9f
> +#define DDR2ODTC  0xb0
> +#define TOLM      0xc4
> +#define REMAPBASE 0xc6
> +#define REMAPLIMIT 0xc8
> +#define REMAPOFFSET 0xca
> +#define TOM       0xcc
> +#define HECBASE   0xce
> +#define DEVPRES1  0xf4
> +
> +#endif


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

--
coreboot mailing list
[email protected]
http://www.coreboot.org/mailman/listinfo/coreboot

Reply via email to