Patchwork ICH5: Add i82801ex_enable_rom() function

login
register
about
Submitter Uwe Hermann
Date 2010-11-25 09:11:22
Message ID <20101125091122.GT21636@greenwood>
Download mbox | patch
Permalink /patch/2358/
State New
Headers show

Comments

Uwe Hermann - 2010-11-25 09:11:22
See patch.


Uwe.
Mark Marshall - 2010-11-25 10:50:19
On 25/11/2010 09:11, Uwe Hermann wrote:
> See patch.
>
Hi.

All of the blocks of code you are removing to a config_u32 and a 
config_u8 write.  The code you've added does a config_u32 and a 
config_u16 write.  Is this correct?  Also the addresses that the writes 
go to are different - the removed code writes to 0xE8 and 0xF0 - your 
new code uses 0xE8 and 0xEE.

These are just comments from inspection of the diff.

MM
Uwe Hermann - 2010-11-25 19:42:54
On Thu, Nov 25, 2010 at 10:50:19AM +0000, Mark Marshall wrote:
> All of the blocks of code you are removing to a config_u32 and a
> config_u8 write.  The code you've added does a config_u32 and a
> config_u16 write.  Is this correct?  Also the addresses that the
> writes go to are different - the removed code writes to 0xE8 and
> 0xF0 - your new code uses 0xE8 and 0xEE.

Yep, I think the patch is correct. Those

  pci_write_config8(dev, 0xf0, 0x00);

lines I removed actually _disabled_ access to the 1MB ROM range,
no idea why. The power-on default is that the range is enabled,
so no need to touch the 0xf0 register (FB_DEC_EN2) or 0xe3 (FB_DEC_EN1).

The two remaining lines in the function are now

  pci_write_config32(dev, FB_SEL1, 0x00000000);
  pci_write_config16(dev, FB_SEL2, 0x0000);

which configures all ROM ranges to be accessible by the FWH chip
with IDSEL = 0, unless I misunderstand something here.


Uwe.
Uwe Hermann - 2010-12-15 09:01:52
On Thu, Nov 25, 2010 at 08:42:54PM +0100, Uwe Hermann wrote:
> On Thu, Nov 25, 2010 at 10:50:19AM +0000, Mark Marshall wrote:
> > All of the blocks of code you are removing to a config_u32 and a
> > config_u8 write.  The code you've added does a config_u32 and a
> > config_u16 write.  Is this correct?  Also the addresses that the
> > writes go to are different - the removed code writes to 0xE8 and
> > 0xF0 - your new code uses 0xE8 and 0xEE.
> 
> Yep, I think the patch is correct. Those
> 
>   pci_write_config8(dev, 0xf0, 0x00);
> 
> lines I removed actually _disabled_ access to the 1MB ROM range,

...oh, my "the 1MB ROM range" was a bit confusing, what those lines
actually did was to _disable_ the following ROM ranges, which happen
to be 1MB in size:

  FB_70_EN -- R/W. Enables decoding two, 1-M flash BIOS memory ranges.
  1 = Enable the following ranges for the flash BIOS
      FF70 0000h ­ FF7F FFFFh
      FF30 0000h ­ FF3F FFFFh
  FB_60_EN -- R/W. Enables decoding two, 1-M flash BIOS memory ranges.
  1 = Enable the following ranges for the flash BIOS
      FF60 0000h ­ FF6F FFFFh
      FF20 0000h ­ FF2F FFFFh
  FB_50_EN -- R/W. Enables decoding two, 1-M flash BIOS memory ranges.
  1 = Enable the following ranges for the flash BIOS
      FF50 0000h ­ FF5F FFFFh
      FF10 0000h ­ FF1F FFFFh
  FB_40_EN -- R/W. Enables decoding two, 1-M flash BIOS memory ranges.
  1 = Enable the following ranges for the flash BIOS
      FF40 0000h ­ FF4F FFFFh
      FF00 0000h ­ FF0F FFFFh


> no idea why. The power-on default is that the range is enabled,
> so no need to touch the 0xf0 register (FB_DEC_EN2) or 0xe3 (FB_DEC_EN1).
> 
> The two remaining lines in the function are now
> 
>   pci_write_config32(dev, FB_SEL1, 0x00000000);
>   pci_write_config16(dev, FB_SEL2, 0x0000);
> 
> which configures all ROM ranges to be accessible by the FWH chip
> with IDSEL = 0, unless I misunderstand something here.

*ping* ?


Uwe.
Mark Marshall - 2010-12-15 10:26:44
On 15/12/2010 09:01, Uwe Hermann wrote:
> On Thu, Nov 25, 2010 at 08:42:54PM +0100, Uwe Hermann wrote:
>> On Thu, Nov 25, 2010 at 10:50:19AM +0000, Mark Marshall wrote:
>>> All of the blocks of code you are removing to a config_u32 and a
>>> config_u8 write.  The code you've added does a config_u32 and a
>>> config_u16 write.  Is this correct?  Also the addresses that the
>>> writes go to are different - the removed code writes to 0xE8 and
>>> 0xF0 - your new code uses 0xE8 and 0xEE.
>>
>> Yep, I think the patch is correct. Those
>>
>>    pci_write_config8(dev, 0xf0, 0x00);
>>
>> lines I removed actually _disabled_ access to the 1MB ROM range,
>
> ...oh, my "the 1MB ROM range" was a bit confusing, what those lines
> actually did was to _disable_ the following ROM ranges, which happen
> to be 1MB in size:
>
>    FB_70_EN -- R/W. Enables decoding two, 1-M flash BIOS memory ranges.
>    1 = Enable the following ranges for the flash BIOS
>        FF70 0000h ­ FF7F FFFFh
>        FF30 0000h ­ FF3F FFFFh
>    FB_60_EN -- R/W. Enables decoding two, 1-M flash BIOS memory ranges.
>    1 = Enable the following ranges for the flash BIOS
>        FF60 0000h ­ FF6F FFFFh
>        FF20 0000h ­ FF2F FFFFh
>    FB_50_EN -- R/W. Enables decoding two, 1-M flash BIOS memory ranges.
>    1 = Enable the following ranges for the flash BIOS
>        FF50 0000h ­ FF5F FFFFh
>        FF10 0000h ­ FF1F FFFFh
>    FB_40_EN -- R/W. Enables decoding two, 1-M flash BIOS memory ranges.
>    1 = Enable the following ranges for the flash BIOS
>        FF40 0000h ­ FF4F FFFFh
>        FF00 0000h ­ FF0F FFFFh
>
>
>> no idea why. The power-on default is that the range is enabled,
>> so no need to touch the 0xf0 register (FB_DEC_EN2) or 0xe3 (FB_DEC_EN1).
>>
>> The two remaining lines in the function are now
>>
>>    pci_write_config32(dev, FB_SEL1, 0x00000000);
>>    pci_write_config16(dev, FB_SEL2, 0x0000);
>>
>> which configures all ROM ranges to be accessible by the FWH chip
>> with IDSEL = 0, unless I misunderstand something here.
>
> *ping* ?

Ok, looks good to me then.  I was really only worried that the 
description din't seem to match what you had changed, but if you think 
its right then that's OK.

MM

Patch

ICH5: Add i82801ex_enable_rom() function.

Replace multiple custom instances in romstage.c files with one global
i82801ex_enable_rom() function as the other chipsets do.
This is also a requirement for switching to TINY_BOOTBLOCK later.

x6dhe_g2/romstage.c: This one was buggy anyway, used the wrong PCI ID
(probably copy-paste from some other chipset).

s2735/romstage.c: This one didn't have any such function at all, add it.

Signed-off-by: Uwe Hermann <uwe@hermann-uwe.de>

Index: src/southbridge/intel/i82801ex/i82801ex_enable_rom.c
===================================================================
--- src/southbridge/intel/i82801ex/i82801ex_enable_rom.c	(Revision 0)
+++ src/southbridge/intel/i82801ex/i82801ex_enable_rom.c	(Revision 0)
@@ -0,0 +1,37 @@ 
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2010 Uwe Hermann <uwe@hermann-uwe.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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
+ */
+
+#include <stdint.h>
+#include <arch/io.h>
+#include <arch/romcc_io.h>
+#include "i82801ex.h"
+
+static void i82801ex_enable_rom(void)
+{
+	device_t dev;
+
+	dev = PCI_DEV(0, 0x1f, 0); /* LPC (D31:F0) */
+
+	/* FB_DEC_EN1/FB_DEC_EN2 defaults are OK (full ROM access enabled). */
+
+	/* Setup all ROM ranges for access by the chip with IDSEL = 0. */
+	pci_write_config32(dev, FB_SEL1, 0x00000000);
+	pci_write_config16(dev, FB_SEL2, 0x0000);
+}
Index: src/southbridge/intel/i82801ex/chip.h
===================================================================
--- src/southbridge/intel/i82801ex/chip.h	(Revision 6122)
+++ src/southbridge/intel/i82801ex/chip.h	(Arbeitskopie)
@@ -1,6 +1,8 @@ 
 #ifndef I82801EX_CHIP_H
 #define I82801EX_CHIP_H
 
+#include <device/device.h>
+
 struct southbridge_intel_i82801ex_config
 {
 
Index: src/southbridge/intel/i82801ex/i82801ex.h
===================================================================
--- src/southbridge/intel/i82801ex/i82801ex.h	(Revision 6122)
+++ src/southbridge/intel/i82801ex/i82801ex.h	(Arbeitskopie)
@@ -1,10 +1,14 @@ 
 #ifndef I82801EX_H
 #define I82801EX_H
 
+#if !defined(__PRE_RAM__)
+
 #include "chip.h"
 
 extern void i82801ex_enable(device_t dev);
 
+#endif
+
 #define PCI_DMA_CFG     0x90
 #define SERIRQ_CNTL     0x64
 #define GEN_CNTL        0xd0
@@ -12,4 +16,7 @@ 
 #define RTC_CONF        0xd8
 #define GEN_PMCON_3     0xa4
 
+#define FB_SEL1		0xe8
+#define FB_SEL2		0xee
+
 #endif /* I82801EX_H */
Index: src/mainboard/supermicro/x6dhe_g2/romstage.c
===================================================================
--- src/mainboard/supermicro/x6dhe_g2/romstage.c	(Revision 6122)
+++ src/mainboard/supermicro/x6dhe_g2/romstage.c	(Arbeitskopie)
@@ -7,6 +7,7 @@ 
 #include <stdlib.h>
 #include <console/console.h>
 #include "southbridge/intel/i82801ex/i82801ex_early_smbus.c"
+#include "southbridge/intel/i82801ex/i82801ex_enable_rom.c"
 #include "northbridge/intel/e7520/raminit.h"
 #include "superio/nsc/pc87427/pc87427.h"
 #include "cpu/x86/lapic/boot_cpu.c"
@@ -69,14 +70,7 @@ 
 	/* Halt if there was a built in self test failure */
 //	report_bist_failure(bist);
 
-	/* MOVE ME TO A BETTER LOCATION !!! */
-	/* config LPC decode for flash memory access */
-        device_t dev;
-        dev = pci_locate_device(PCI_ID(0x8086, 0x25a1), 0);
-        if (dev == PCI_DEV_INVALID)
-                die("Missing ich5r?");
-        pci_write_config32(dev, 0xe8, 0x00000000);
-        pci_write_config8(dev, 0xf0, 0x00);
+	i82801ex_enable_rom();
 
 #if 0
 	display_cpuid_update_microcode();
Index: src/mainboard/supermicro/x6dhr_ig/romstage.c
===================================================================
--- src/mainboard/supermicro/x6dhr_ig/romstage.c	(Revision 6122)
+++ src/mainboard/supermicro/x6dhr_ig/romstage.c	(Arbeitskopie)
@@ -7,6 +7,7 @@ 
 #include <stdlib.h>
 #include <console/console.h>
 #include "southbridge/intel/i82801ex/i82801ex_early_smbus.c"
+#include "southbridge/intel/i82801ex/i82801ex_enable_rom.c"
 #include "northbridge/intel/e7520/raminit.h"
 #include "superio/winbond/w83627hf/w83627hf.h"
 #include "cpu/x86/lapic/boot_cpu.c"
@@ -70,14 +71,7 @@ 
 	/* Halt if there was a built in self test failure */
 //	report_bist_failure(bist);
 
-	/* MOVE ME TO A BETTER LOCATION !!! */
-	/* config LPC decode for flash memory access */
-        device_t dev;
-        dev = pci_locate_device(PCI_ID(0x8086, 0x24d0), 0);
-        if (dev == PCI_DEV_INVALID)
-                die("Missing ich5?");
-        pci_write_config32(dev, 0xe8, 0x00000000);
-        pci_write_config8(dev, 0xf0, 0x00);
+	i82801ex_enable_rom();
 
 #if 0
 	display_cpuid_update_microcode();
Index: src/mainboard/supermicro/x6dhr_ig2/romstage.c
===================================================================
--- src/mainboard/supermicro/x6dhr_ig2/romstage.c	(Revision 6122)
+++ src/mainboard/supermicro/x6dhr_ig2/romstage.c	(Arbeitskopie)
@@ -7,6 +7,7 @@ 
 #include <stdlib.h>
 #include <console/console.h>
 #include "southbridge/intel/i82801ex/i82801ex_early_smbus.c"
+#include "southbridge/intel/i82801ex/i82801ex_enable_rom.c"
 #include "northbridge/intel/e7520/raminit.h"
 #include "superio/winbond/w83627hf/w83627hf.h"
 #include "cpu/x86/lapic/boot_cpu.c"
@@ -70,14 +71,7 @@ 
 	/* Halt if there was a built in self test failure */
 //	report_bist_failure(bist);
 
-	/* MOVE ME TO A BETTER LOCATION !!! */
-	/* config LPC decode for flash memory access */
-        device_t dev;
-        dev = pci_locate_device(PCI_ID(0x8086, 0x24d0), 0);
-        if (dev == PCI_DEV_INVALID)
-                die("Missing ich5?");
-        pci_write_config32(dev, 0xe8, 0x00000000);
-        pci_write_config8(dev, 0xf0, 0x00);
+	i82801ex_enable_rom();
 
 #if 0
 	display_cpuid_update_microcode();
Index: src/mainboard/tyan/s2735/romstage.c
===================================================================
--- src/mainboard/tyan/s2735/romstage.c	(Revision 6122)
+++ src/mainboard/tyan/s2735/romstage.c	(Arbeitskopie)
@@ -10,6 +10,7 @@ 
 #include <lib.h>
 #include <spd.h>
 #include "southbridge/intel/i82801ex/i82801ex_early_smbus.c"
+#include "southbridge/intel/i82801ex/i82801ex_enable_rom.c"
 #include "northbridge/intel/e7501/raminit.h"
 #include "northbridge/intel/e7501/debug.c"
 #include "superio/winbond/w83627hf/w83627hf_early_serial.c"
@@ -56,6 +57,8 @@ 
 	/* Halt if there was a built in self test failure */
 	report_bist_failure(bist);
 
+	i82801ex_enable_rom();
+
 	if (bios_reset_detected())
 		hard_reset();
 
Index: src/mainboard/dell/s1850/romstage.c
===================================================================
--- src/mainboard/dell/s1850/romstage.c	(Revision 6122)
+++ src/mainboard/dell/s1850/romstage.c	(Arbeitskopie)
@@ -7,6 +7,7 @@ 
 #include <stdlib.h>
 #include <console/console.h>
 #include "southbridge/intel/i82801ex/i82801ex_early_smbus.c"
+#include "southbridge/intel/i82801ex/i82801ex_enable_rom.c"
 #include "northbridge/intel/e7520/raminit.h"
 #include "superio/nsc/pc8374/pc8374_early_init.c"
 #include "cpu/x86/lapic/boot_cpu.c"
@@ -282,15 +283,7 @@ 
 	/* Halt if there was a built in self test failure */
 //	report_bist_failure(bist);
 
-	/* MOVE ME TO A BETTER LOCATION !!! */
-	/* config LPC decode for flash memory access */
-        device_t dev;
-        dev = pci_locate_device(PCI_ID(0x8086, 0x24d0), 0);
-        if (dev == PCI_DEV_INVALID) {
-                die("Missing ich5?");
-        }
-        pci_write_config32(dev, 0xe8, 0x00000000);
-        pci_write_config8(dev, 0xf0, 0x00);
+	i82801ex_enable_rom();
 
 #if 0
 	display_cpuid_update_microcode();
Index: src/mainboard/intel/jarrell/romstage.c
===================================================================
--- src/mainboard/intel/jarrell/romstage.c	(Revision 6122)
+++ src/mainboard/intel/jarrell/romstage.c	(Arbeitskopie)
@@ -7,6 +7,7 @@ 
 #include <stdlib.h>
 #include <console/console.h>
 #include "southbridge/intel/i82801ex/i82801ex_early_smbus.c"
+#include "southbridge/intel/i82801ex/i82801ex_enable_rom.c"
 #include "northbridge/intel/e7520/raminit.h"
 #include "superio/nsc/pc87427/pc87427.h"
 #include "cpu/x86/lapic/boot_cpu.c"
@@ -74,14 +75,7 @@ 
 	pc87427_enable_dev(PC87427_XBUS_DEV, SIO_XBUS_BASE);
 	xbus_cfg(PC87427_XBUS_DEV);
 
-	/* MOVE ME TO A BETTER LOCATION !!! */
-	/* config LPC decode for flash memory access */
-        device_t dev;
-        dev = pci_locate_device(PCI_ID(0x8086, 0x24d0), 0);
-        if (dev == PCI_DEV_INVALID)
-                die("Missing ich5?");
-        pci_write_config32(dev, 0xe8, 0x00000000);
-        pci_write_config8(dev, 0xf0, 0x00);
+	i82801ex_enable_rom();
 
 #if 0
 	print_pci_devices();