[coreboot] flashrom: Cache probed id:s

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Tue Jun 24 11:16:37 CEST 2008

On 24.06.2008 06:38, Peter Stuge wrote:
> On Tue, Jun 24, 2008 at 04:51:00AM +0200, Carl-Daniel Hailfinger wrote:
>>> Best case it is merely pointless to repeat a probe sequence.
>>> Worst case it causes system instability, as we saw with the AMIC A49LF040A.
>> This problem is not fixed by this patch.
> No, but it is one step closer to well-defined, developer-friendly
> behavior. The next step is to add a simple list with probe functions,
> and execute each one exactly once before the list of flash chips is
> even considered.
> That way there is a single, simple, definition of the order in which
> probes happen, regardless of the order of entries in flashchips.c,
> which I very much like having sorted alphabetically.

And that won't help the AMIC chip. Remember that one probing for the
W29EE already kills the AMIC chip. Your change is simply orthogonal to
the problem.

>>> This also shortens flashrom's execution time roughly one second.
>>> Signed-off-by: Peter Stuge <peter at stuge.se>
>> NACK. This patch randomly (depending on flashchips.c order) breaks
>> probing for ~80% of the chips we currently support. If it works for
>> you. you are just lucky.
> Can you expand on why this would happen?
> Keep in mind that each probe function always executes the same code,
> no matter which chip is being probed for.
> Each probe function implements a particular hardware communication
> sequence to read some ID from chip. The function then compares that
> ID (same ID every call) to the parameters specified by the caller
> (different ID every call) and returns true if they match. My patch
> optimizes away the hardware communication for all probe function
> calls except the very first one. The ID can not change between calls,
> because the hardware communication sequence is identical.

Au contraire. The pointer to the (bios) probe location changes depending
on flash chip size. So the hardware communication sequence is NOT identical.

> If the ID read from hardware _did_ change between calls (as was the
> case in a report to flashrom@ when the delay in probe_jedec() was too
> short, before the 2ms patch) that particular probe function is not
> working correctly.

We haven dozens of reports (unrelated to probing delays) where
probe_jedec gives different results for each probe location, exactly
like expected.

There are three ways to solve this:
- Keep probe results per function per chip size (ugly)
- Create different probe_jedec_* functions per chip size
(probe_jedec_512k, probe_jedec_1024k...) (embarrassing)
- Leave the code as-is (preferred).



More information about the coreboot mailing list