[coreboot] patch: dbe62 boots to FILO!
Uwe Hermann
uwe at hermann-uwe.de
Thu Apr 3 14:09:36 CEST 2008
On Thu, Apr 03, 2008 at 12:26:11PM +0200, Carl-Daniel Hailfinger wrote:
> The patch looks nice, but there are a few small problems.
> Otherwise, the patch looks fine.
>
> Reworked patch follows. It should be identical from a code point of
> view, but it would be great if you could test anyway.
> Please note that the patch is against HEAD. Same patch is also attached.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Please keep the Signed-off-by from Ron, i.e. the commit message (and
follow-up patches) should have _both_ Signed-off-by's.
> Index: LinuxBIOSv3-dbe62/southbridge/amd/cs5536/cs5536.c
> ===================================================================
> --- LinuxBIOSv3-dbe62/southbridge/amd/cs5536/cs5536.c (Revision 647)
> +++ LinuxBIOSv3-dbe62/southbridge/amd/cs5536/cs5536.c (Arbeitskopie)
> @@ -258,6 +258,7 @@
>
> /* COM1 */
> if (sb->com1_enable) {
> + printk(BIOS_SPEW, "uarts_init: enable com1\n");
com1 -> COM1 maybe.
> /* Set the address. */
> switch (sb->com1_address) {
> case 0x3F8:
> @@ -308,6 +309,7 @@
> wrmsr(MDD_UART1_CONF, msr);
> } else {
> /* Reset and disable COM1. */
> + printk(BIOS_SPEW, "uarts_init: disable com1\n");
Ditto (and in various other places).
> Index: LinuxBIOSv3-dbe62/mainboard/artecgroup/dbe62/initram.c
> ===================================================================
> --- LinuxBIOSv3-dbe62/mainboard/artecgroup/dbe62/initram.c (Revision 647)
> +++ LinuxBIOSv3-dbe62/mainboard/artecgroup/dbe62/initram.c (Arbeitskopie)
> @@ -33,9 +33,9 @@
> #include <northbridge/amd/geodelx/raminit.h>
> #include <spd.h>
>
> -#define MANUALCONF 0 /* Do automatic strapped PLL config */
> -#define PLLMSRHI 0x00001490 /* manual settings for the PLL */
> -#define PLLMSRLO 0x02000030
> +#define MANUALCONF 1 /* Do manual strapped PLL config */
> +#define PLLMSRHI 0x000003d9 /* manual settings for the PLL */
> +#define PLLMSRLO 0x07de0080 /* from fuctory bios */
> #define DIMM0 ((u8) 0xA0)
> #define DIMM1 ((u8) 0xA2)
>
> @@ -50,28 +50,45 @@
> u8 address;
> u8 data;
> };
> +/*
> +ok, This is what I came up with. I would be interested in the results.
> +spd : value(hex)
> +4: 8
> +5: 1
> +9: <= 7
> +12: 82
> +17: 4
> +31: 40
> +18: 10
> +23: 0
> +25: 0
> +27: 58
> +28: 3c
> +29: 58
> +30: 2d
> +42:4b
>
> +I may have missed one so let me know. Also you might find this document helpful.
> +*/
Yep, needs a more verbose comment IMO. Also, please use the standard
commenting style
/*
* Foo.
* Bar.
*/
(or Doxygen-style if it should end up in Doxygen's output)
> + dumplxmsrs();
> /* Check low memory */
> - ram_check(0x00000000, 640*1024);
> + /* passed. Don't bother any more */
This comment doesn't make much sense.
> + /* Note that the range 0x87000 will fail; that's the stack! */
> + /* ram_check(0x00000000, 640*1024);*/
>
> printk(BIOS_DEBUG, "stage1 returns\n");
> return 0;
> Index: LinuxBIOSv3-dbe62/northbridge/amd/geodelx/raminit.c
> ===================================================================
> --- LinuxBIOSv3-dbe62/northbridge/amd/geodelx/raminit.c (Revision 647)
> +++ LinuxBIOSv3-dbe62/northbridge/amd/geodelx/raminit.c (Arbeitskopie)
> @@ -35,6 +35,43 @@
>
> u8 spd_read_byte(u16 device, u8 address);
>
> +
> +/**
> + * Dump key MSR values for ram init. You can call this function and then use it to
ram -> RAM.
> + * compare to a fuctory bios setting.
bios -> BIOS
> + * @param level printk level
> + */
> +void dumplxmsrs(void)
> +{
> + static unsigned long msrs[] = {
> + MC_CF07_DATA,
> + MC_CF8F_DATA,
> + MC_CF1017_DATA,
> + GLCP_DELAY_CONTROLS,
> + MC_CFCLK_DBUG,
> + MC_CF_PMCTR,
> + GLCP_SYS_RSTPLL
> + };
> + static char *msrnames[] = {
Can be 'const' too, I guess.
(same for the other file)
> + "MC_CF07_DATA",
> + "MC_CF8F_DATA",
> + "MC_CF1017_DATA",
> + "GLCP_DELAY_CONTROLS",
> + "MC_CFCLK_DBUG",
> + "MC_CF_PMCTR",
> + "PLL reg"
> + };
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(msrs); i++) {
> + struct msr msr;
> + msr = rdmsr(msrs[i]);
> + printk(BIOS_DEBUG, "%s (%lx): %x.%x\n", msrnames[i], msrs[i],
^^
only one space
(same for the other file)
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