[coreboot] [PATCH] v2: Original import for mainboard msi/ms6147

Uwe Hermann uwe at hermann-uwe.de
Mon Oct 27 15:06:38 CET 2008


Hi,

thanks a lot for your contribution!

On Mon, Sep 15, 2008 at 08:10:09PM +0200, Mats Erik Andersson wrote:
> Action: First import of code basis for mainboard msi/ms6147.
> 
> Signed-by: Mats Erik Andersson <mats.andersson at gisladisker.org>
        ^
 Signed-off-by please, fixed it in the commit


> My main contribution is to implement a functional detection
> of populated RAM slots. The code has proven its ability to
> detect, configure, and successfully initialize memory
> ranging from 16MB+16MB in one slot, up to 64MB+64MB+64MB+64MB
> in two slots. The mainboard has only two SDRAM slots, but the
> code is able to work with one to four slots.

Sorry for the long delay, I've finally found some time to review this.

The board code is committed in r3695 with some changes:

 - All files changed to (C) 2008 Mats Erik Andersson <mats.andersson at gisladisker.org>
   They are all pretty small/trivial and don't warrant carrying around
   my (C) lines...

 - I changes the board to resemble all other 440BX ones in svn, i.e. I
   didn't commit your changes to initram etc. (yet).
   All raminit changes should be in the global 440BX code so that _all_
   440BX boards benefit of it.

   Please repost your latest raminit code with a Signed-off-by for
   review/testing, and split it up in multiple parts ideally. I think
   at least the do_ram_command() changes are easy to split off and
   review/test independently of the rest.

 - Some other small changes, see below.


The committed code is build-tested by me, no romcc issues so far.


> +          device pnp 3f0.7 on		# GPIO 1
> +          end
> +          device pnp 3f0.8 on		# GPIO 2
> +          end
> +          device pnp 3f0.9 off		# GPIO 3

Any particular reason why this one is off, and the others not?
Can you post your 'superiotool -dV' output?


> +          end
> +          device pnp 3f0.a on		# ACPI
> +          end
> +        end
> +      end
> +      device pci 7.1 on	end		# IDE
> +      device pci 7.2 on			# USB


> +	  	# irq 0x3c = 0x0a

Why this?


> +      end
> +      device pci 7.3 on end		# ACPI
> +      register "ide0_enable" = "1"
> +      register "ide1_enable" = "1"
> +      register "ide_legacy_enable" = "1"
> +      # Enable UDMA/33 for higher speed if your IDE device(s) support it.
> +      register "ide0_drive0_udma33_enable" = "1"
> +      register "ide0_drive1_udma33_enable" = "0"
> +      register "ide1_drive0_udma33_enable" = "0"
> +      register "ide1_drive1_udma33_enable" = "0"

I changed them all to "1" for now, most hardware should support that
so a default-on setting makes sense IMHO.


> Index: src/mainboard/msi/ms6147/irq_tables.c
> ===================================================================
> --- src/mainboard/msi/ms6147/irq_tables.c	(revision 0)
> +++ src/mainboard/msi/ms6147/irq_tables.c	(revision 0)
> @@ -0,0 +1,46 @@

I added the usual copyright header here, too.


> +const struct irq_routing_table intel_irq_routing_table = {
> +	PIRQ_SIGNATURE,  /* u32 signature */
> +	PIRQ_VERSION,    /* u16 version   */
> +	32+16*8,	 /* There can be total 8 devices on the bus */
              ^
     This one should be IRQ_SLOT_COUNT (from targets/.../Config.lb)
     I fixed that and some other small issues in the commit.


> +#include "mainboard/asus/mew-vm/debug.c"       /* FIXME */

Changed to "lib/debug.c".


> +//#include "northbridge/intel/i440bx/raminit.c"
> +#include "mainboard/msi/ms6147/raminit.c"
> +//#include "northbridge/intel/i440bx/debug.c"
> +#include "mainboard/msi/ms6147/debug.c"
> +#include "mainboard/msi/ms6147/sdram.c"
> +
> +static void main(unsigned long bist)
> +{
> +	static const struct mem_controller memctrl[] = {
> +		{
> +			.d0 = PCI_DEV(0, 0, 0),
> +			.channel0 = {0x50, 0x51, 0x52, 0x53},
> +		}
> +	};
> +
> +	if (bist == 0)
> +		early_mtrr_init();
> +
> +	w83977tf_enable_serial(SERIAL_DEV, TTYS0_BASE);
> +	uart_init();
> +	console_init();
> +	report_bist_failure(bist);
> +	enable_smbus();
> +	//dump_spd_registers(memctrl[0].channel0[0]);
> +	sdram_initialize(memctrl);

Some small changes here, see commit.


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




More information about the coreboot mailing list