On 17.03.2009 01:05, Peter Stuge wrote:
Carl-Daniel Hailfinger wrote:
If someone deliberately changes that register,
Which register? I couldn't find one. :\
See the patch hunk above. Look for CD6 in the SB600 RRG. Notice CD6 is PM_INDEX. CD7 is PM_DATA. Look at section 2.3.3.2 Power Management (PM) Registers. 8F is FakeAsrEn. FakeAsrEn has two interesting fields: UseBypassRom and BypassRomSel.
Fake is the key part of those names. They are overrides only.
Yes. So we leave the overrides alone. That's part I of the solution.
Basically, the patch hunk above does simply remove the forcing of SPI access and relies on the default boot-up method. Back when you NAKed that change, I wondered why but I assumed you wanted some more design to go into it.
The register can not be read to determine what the system booted from as far as I understand.
Correct.
Thus it is not possible to determine whether to use the SB600 SPI driver or use physical address reads and writes.
You can use the half dozen other registers depending indirectly on the ROM strap for a good heuristic. Search for "LPC" and "FWH" in the RRG if you really want that.
For some operations it does not matter, but probe and erase for SPI chips does not work with only physical address accesses.
So what? The timeout part of the patch makes sure SPI accesses won't hang forever. Enable both by default and flashrom may be slow, but it won't hang.
I think the patch is committable,
Please reconsider?
Which part of "the patch works for both LPC and SPI boards" is objectionable?
Anyway, the forced ROM strap override must go. (Post 1.0 we can still enable it again if the user forces SB600 SPI.) The timeout is needed as well. Not having a timeout is waiting for a disaster to happen. Other drivers have timeouts as well.
After that, no controversial changes remain. Sure, conceptually we want to be able to determine the ROM straps, but as long as flashrom is just slow during probe, people won't lose sleep over it.
AFAIK the patch has three Acks or so. This is more than what most other flashrom patches have.
Regards, Carl-Daniel