Acked-by: Maggie Li <[email protected]> Best regards Maggie li
-----Original Message----- From: Carl-Daniel Hailfinger [mailto:[email protected]] Sent: Friday, December 12, 2008 10:50 AM To: Li, Maggie Cc: Bao, Zheng; [email protected]; ron minnich; Xie, Michael; Huang, FrankR; Feng, Libo; Wang, Qingpei Subject: Re: [coreboot] patch for support mainboard pistachio: Hi Maggie, thanks for the new review. I have changed the patch according to your suggestions. Do you think the new patch is OK? Regards, Carl-Daniel On 11.12.2008 09:44, Li, Maggie wrote: > Hi, Carl-Daniel > > I have some suggestion below for you. > > Best regards > Maggie li > > -----Original Message----- > From: Carl-Daniel Hailfinger [mailto:[email protected]] > Sent: Thursday, December 11, 2008 10:13 AM > To: Li, Maggie > Cc: ron minnich; Bao, Zheng; [email protected]; Xie, Michael; Huang, > FrankR; Feng, Libo; Wang, Qingpei > Subject: Re: [coreboot] patch for support mainboard pistachio: > > Hi Maggie, > > thank you for the fast review! > I have changed the things you suggested in the hope this will be better. > Please see below. > > Best regards, > Carl-Daniel Hailfinger > > On 10.12.2008 07:56, Li, Maggie wrote: > >> Hi, Carl-Daniel >> >> I have some suggestion about your modification. Please see below. >> >> Best regards >> Maggie li >> >> -----Original Message----- >> From: Carl-Daniel Hailfinger [mailto:[email protected]] >> Sent: Wednesday, December 10, 2008 10:19 AM >> To: ron minnich >> Cc: Li, Maggie; Bao, Zheng; [email protected]; 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 <[email protected]> >> >> 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) >> >> > > Thanks, I changed this. > > > >> 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? >> >> > > Yes, the warning was confusing. My apologies. > > Will the settings in register 0x68 and 0x6c also affect FWH and SPI? If > yes, we can simply set them unconditionally. > > I have prepared a new patch which assumes 0x68 and 0x6c affect FWH and > SPI. Setting these registers again (if LPC strap was active) is not a > problem and it makes the code simpler. > > [Li, Maggie] I did a test on my pistachio board to find that 0x68 and 0x6c do > affect SPI. I have not a FWH board, so I am not sure whether FWH will be > affected. > Thanks for testing! There are only very few RS690 boards for Intel out there and I think they are no longer in production, so we can ignore them for now. >> } >> >> @@ -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 */ >> >> > > I am not sure about this comment. If IO Base and IO Limit are 0xf000, > nothing will be passed through. Am I wrong? > [Li, Maggie] The same IO Base and IO Limit here is meaningful when we set the > bridge to be subtractive. At early setup stage, we have to make sure that > data can go through port 0x80. After resource is allocated, the bridge has > its own positive and subtractive window. Any data going to the devices behind > the bridge can go through positive window. Data to port 0x80 will go through > subtractive window if no other bridge accepts it after four cycles. > > I think it is better to remove the comment "/* FIXME: This does not make > sense if we want to decode port 0x80 */" now. > Yes, thanks! I added your explanation as comment to the code. If you agree to the new patch, it would be great if you could give an Acked-by: statement. Improve comments in early SB600 setup, handle non-LPC strapping and document verification against the data sheets. Signed-off-by: Carl-Daniel Hailfinger <[email protected]> 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 (working copy) @@ -53,11 +53,17 @@ /*************************************** * 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. +* If you use FWH ROMs, you have to setup IDSEL. * NOTE: Call me ASAP, because I will reset LPC ROM size! +* Reviewed-by: Carl-Daniel Hailfinger +* Reviewed against AMD SB600 Register Reference Manual rev. 3.03, section 3.1 +* (LPC ISA Bridge) ***************************************/ static void sb600_lpc_init(void) { @@ -68,32 +74,42 @@ /* 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 & 0x64 (PS/2 keyboard) and port 0x62 & 0x66 (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 */ + /* hardware should enable LPC ROM by pin straps */ + /* 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; */ - pci_write_config16(dev, 0x6c, 0xfff0); /* enable LPC ROM range, 0xfff8: 512KB, 0xfff0: 1MB */ + /* enable LPC ROM range mirroring start 0x000e(0000) */ + pci_write_config16(dev, 0x68, 0x000e); + /* enable LPC ROM range mirroring end 0x000f(ffff) */ + pci_write_config16(dev, 0x6a, 0x000f); + /* enable LPC ROM range start, 0xfff8(0000): 512KB, 0xfff0(0000): 1MB */ + pci_write_config16(dev, 0x6c, 0xfff0); + /* enable LPC ROM range end at 0xffff(ffff) */ + pci_write_config16(dev, 0x6e, 0xffff); } /* what is its usage? */ @@ -201,26 +217,36 @@ /* 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); + /* The same IO Base and IO Limit here is meaningful because we set the + * bridge to be subtractive. During early setup stage, we have to make + * sure that data can go through 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 +260,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/ -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
