[coreboot] patch for support mainboard pistachio:

[Li, Maggie]

Hi, Carl-Daniel

I have some suggestion about your modification. Please see below.

Best regards
Maggie li

-----Original Message-----
From: Carl-Daniel Hailfinger [mailto:c-d.hailfinger.devel.2006 at gmx.net] 
Sent: Wednesday, December 10, 2008 10:19 AM
To: ron minnich
Cc: Li, Maggie; Bao, Zheng; coreboot at coreboot.org; Xie, Michael; Huang, FrankR; Feng, Libo; Wang, Qingpei
Subject: Re: [coreboot] patch for support mainboard pistachio:

Dear Maggie,

can you please look at the attached patch and review it? I have tried to
improve comments and warn about possible problems in early SB600 setup.

Thank you!

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

The patch is also inlined into this mail for better viewing.

Regards,
Carl-Daniel Hailfinger

Index: LinuxBIOSv2-asus_m2a-vm/src/southbridge/amd/sb600/sb600_early_setup.c
===================================================================
--- LinuxBIOSv2-asus_m2a-vm/src/southbridge/amd/sb600/sb600_early_setup.c	(Revision 3808)
+++ LinuxBIOSv2-asus_m2a-vm/src/southbridge/amd/sb600/sb600_early_setup.c	(Arbeitskopie)
@@ -53,11 +53,15 @@
 
 /***************************************
 * Legacy devices are mapped to LPC space.
-*	serial port 0
+*	Serial port 0
 *	KBC Port
 *	ACPI Micro-controller port
-*	LPC ROM size,
+*	LPC ROM size
+*	This function does not change port 0x80 decoding.
+*	Console output through any port besides 0x3f8 is unsupported.
 * NOTE: Call me ASAP, because I will reset LPC ROM size!
+* Reviewed-by: Carl-Daniel Hailfinger
+* Reviewed against AMD SB600 Register Reference Manual 3.03, section 3.1 (LPC ISA Bridge)
 ***************************************/
 static void sb600_lpc_init(void)
 {
@@ -68,31 +72,38 @@
 	/* Enable lpc controller */
 	dev = pci_locate_device(PCI_ID(0x1002, 0x4385), 0);	/* SMBUS controller */
 	reg32 = pci_read_config32(dev, 0x64);
-	reg32 |= 0x00100000;
+	reg32 |= 1 << 20;
 	pci_write_config32(dev, 0x64, reg32);
 
 	dev = pci_locate_device(PCI_ID(0x1002, 0x438d), 0);	/* LPC Controller */
-	/* Serial 0 */
+	/* Decode port 0x3f8-0x3ff (Serial 0) */
+#warning Serial port decode on LPC is hardcoded to 0x3f8
 	reg8 = pci_read_config8(dev, 0x44);
-	reg8 |= (1 << 6);
+	reg8 |= 1 << 6;
 	pci_write_config8(dev, 0x44, reg8);
 
-	/* PS/2 keyboard, ACPI */
+	/* Decode port 0x60-0x67 (PS/2 keyboard, ACPI) */
[Li, Maggie] Decode port 60 & 64h (PS/2 keyboard), and port 62 & 66h (ACPI)
 	reg8 = pci_read_config8(dev, 0x47);
 	reg8 |= (1 << 5) | (1 << 6);
 	pci_write_config8(dev, 0x47, reg8);
 
 	/* SuperIO, LPC ROM */
 	reg8 = pci_read_config8(dev, 0x48);
-	reg8 |= (1 << 1) | (1 << 0);	/* enable Super IO config port 2e-2h, 4e-4f */
-	reg8 |= (1 << 3) | (1 << 4);	/* enable for LPC ROM address range1&2, Enable 512KB rom access at 0xFFF80000 - 0xFFFFFFFF  */
-	reg8 |= 1 << 6;		/* enable for RTC I/O range */
+	/* Decode ports 0x2e-0x2f, 0x4e-0x4f (SuperI/O configuration) */
+	reg8 |= (1 << 1) | (1 << 0);
+	/* Decode variable LPC ROM address ranges 1&2 (see register 0x68-0x6b, 0x6c-0x6f) */
+	reg8 |= (1 << 3) | (1 << 4);
+	/* Decode port 0x70-0x73 (RTC) */
+	reg8 |= 1 << 6;
 	pci_write_config8(dev, 0x48, reg8);
 
 	/* hardware should enable LPC ROM by pin strapes */
-	/*  rom access at 0xFFF80000/0xFFF00000 - 0xFFFFFFFF */
+	/* ROM access at 0xFFF80000/0xFFF00000 - 0xFFFFFFFF */
 	/* See detail in BDG-215SB600-03.pdf page 15. */
-	pci_write_config16(dev, 0x68, 0x000e);	/* enable LPC ROM range, 0xfff8: 512KB, 0xfff0: 1MB; */
+#warning If Lpc_Rom strap is enabled, the line below has no effect, and if Lpc_Rom strap is disabled, a write to 0x6a is missing
+	pci_write_config16(dev, 0x68, 0x000e);	/* enable LPC ROM range mirroring below 1 MB */
+#warning Due to FWH_*_IDSEL defaults, any FWH ROM chip larger than 512 kB will not work although we enable a range of 1 MB below. LPC ROMs are not affected.
+#warning If Lpc_Rom strap is disabled, a write to 0x6e is missing
 	pci_write_config16(dev, 0x6c, 0xfff0);	/* enable LPC ROM range, 0xfff8: 512KB, 0xfff0: 1MB  */
[Li, Maggie] I think the warning about 0x68 and 0x6c is somewhat confusing. If Lpc_Rom strap is disabled, default values for register 0x6a and 0x6e are correct. Do we have to write them? Should the register 0x68, 0x6c be for setting LPC ROM, and register 0x70 for FWH ROM? Should we write some code for setting FWH ROM here?
 }
 
@@ -201,26 +212,33 @@
 	/* P2P Bridge */
 	dev = pci_locate_device(PCI_ID(0x1002, 0x4384), 0);
 
+	/* Chip Control: Enable subtractive decoding */
 	byte = pci_read_config8(dev, 0x40);
 	byte |= 1 << 5;
 	pci_write_config8(dev, 0x40, byte);
 
+	/* Misc Control: Enable subtractive decoding if 0x40 bit 5 is set */
 	byte = pci_read_config8(dev, 0x4B);
 	byte |= 1 << 7;
 	pci_write_config8(dev, 0x4B, byte);
 
+	/* FIXME: This does not make sense if we want to decode port 0x80 */
+	/* IO Base: 0xf000 */
 	byte = pci_read_config8(dev, 0x1C);
 	byte |= 0xF << 4;
 	pci_write_config8(dev, 0x1C, byte);
 
+	/* IO Limit: 0xf000 */
 	byte = pci_read_config8(dev, 0x1D);
 	byte |= 0xF << 4;
 	pci_write_config8(dev, 0x1D, byte);
 
+	/* PCI Command: Enable IO response */
 	byte = pci_read_config8(dev, 0x04);
 	byte |= 1 << 0;
 	pci_write_config8(dev, 0x04, byte);
 
+	/* LPC controller */
 	dev = pci_locate_device(PCI_ID(0x1002, 0x438D), 0);
 
 	byte = pci_read_config8(dev, 0x4A);
@@ -234,13 +252,13 @@
 	device_t dev;
 	u32 reg32;
 
-	/* enable lpc controller */
+	/* Enable LPC controller */
 	dev = pci_locate_device(PCI_ID(0x1002, 0x4385), 0);
 	reg32 = pci_read_config32(dev, 0x64);
 	reg32 |= 0x00100000;	/* lpcEnable */
 	pci_write_config32(dev, 0x64, reg32);
 
-	/* enable prot80 LPC decode in pci function 3 configuration space. */
+	/* Enable port 80 LPC decode in pci function 3 configuration space. */
 	dev = pci_locate_device(PCI_ID(0x1002, 0x438d), 0);
 	byte = pci_read_config8(dev, 0x4a);
 	byte |= 1 << 5;		/* enable port 80 */


-- 
http://www.hailfinger.org/