Hi,
On 29.06.2010 17:34, Joshua Roys wrote:
I'm glad it seems useful! Some questions inline:
On 06/29/2010 08:12 AM, Carl-Daniel Hailfinger wrote:
+static void ich_spi_frap(uint32_t frap, int i)
Can you change the name to print_ich9_spi_frap?
I was considering making this function twiddle some bits eventually (if it could). The BM{W,R}AG fields can apparently override the FRAP access controls to the "BIOS Descriptor" region. Unfortunately on my board, the regions were labeled as a flash descriptor and a ME section... do_ich9_spi_frap?
OK.
- msg_pdbg("0x%02X: 0x%08x (FREG%i: %s)\n",
offset, freg, i, region_names[i]);
- base = ICH_FREG_BASE(freg);
- limit = ICH_FREG_LIMIT(freg);
- if (base == 0x1fff && limit == 0) {
/* this FREG is disabled */
return;
This is a bit irritating in the output. Can you print a message that the FREG is disabled? OTOH, "disabled" implies that the region is inaccessible or does not exist. Looking at your log, what does it mean that the BIOS FREG is disabled? No access? Full access? Should the message be "unknown location, full access", or is that incorrect?
If I understand the datasheet correctly it simply means that there is no region in the flash chip that is considered as this (ME, GbE, etc). Maybe something like msg_pdbg("%s region is unused.", region_names[i]);
But your output says:
0x54: 0x00010000 (FREG0: Flash Descriptor) 0x00000000-0x00001fff is Read-only 0x58: 0x00001fff (FREG1: BIOS Descriptor) 0x5C: 0x007f0002 (FREG2: ME) 0x00002000-0x0007ffff is Locked 0x60: 0x00001fff (FREG3: GbE) 0x64: 0x00001fff (FREG4: Platform Data)
That would imply no region of the flash chip is considered to be BIOS/GbE/Platformdata.
Maybe we need to dump the flash descriptor region and analyze it. I think Matthias Wenzel wrote a tool for that: http://github.com/mazzoo/descriptormode Such an analysis would be best done in a separate patch, though.
Thanks for the review,
You're welcome. I'm confident we can commit the next iteration of your patch.
Regards, Carl-Daniel