[coreboot] [PATCH] ICH5: Add i82801ex_enable_rom() function

[Mark Marshall]

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