[coreboot] [PATCH 3/6] Intel EP80579 northbridge
Ed Swierk
eswierk at arastra.com
Thu Sep 4 02:23:11 CEST 2008
Thanks for the feedback. I have addressed most of your comments;
please see the attached patch.
I will address your comment about the duplicate struct dimm_size
declaration separately. Would it make sense to put it in a file common
to all Intel northbridges, say, src/northbridge/intel/mem.h, and then
remove it from the various raminit.c's?
Signed-off-by: Ed Swierk <eswierk at arastra.com>
--Ed
On Tue, Aug 26, 2008 at 11:05 AM, Uwe Hermann <uwe at hermann-uwe.de> wrote:
> 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 <eswierk at arastra.com>
>
>
>> 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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ep80579-northbridge.patch
Type: application/octet-stream
Size: 24728 bytes
Desc: not available
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20080903/c134f0f3/attachment.obj>
More information about the coreboot
mailing list