[XS] Change in flashrom[main]: flashrom: Don't throw around "delay 1 second" so lightly

[Brian Norris (Code Review)]

Attention is currently required from: Brian Norris, Nikolai Artemiev, Peter Marheine.

Hello Nikolai Artemiev, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/80807?usp=email

to look at the new patch set (#5).

Change subject: flashrom: Don't throw around "delay 1 second" so lightly
......................................................................

flashrom: Don't throw around "delay 1 second" so lightly

Waiting a full second is a very long time, especially when
default_delay() chooses to busy-loop. This code has been around for a
decade, with vague references to user reports:

commit 8ab49e72af8465d4527de2ec37b22cd44f7a1169
  Date:   Wed Aug 19 13:55:34 2009 +0000
  Disallow erase/write for known bad chips so people won't try without a clear understanding

Still, this logic does not belong in the high-level library logic, used
by all programmers and all chips. If there is a timing issue, it should
either be encoded in the appropriate programmer or flashchip timing.

Now, this change has some (hopefully [1] tiny) chance of regression, so
we have the following considerations:

1. emergency_help_message() already provides documentation on how to
   contact support, in case we need to handle any user-reported
   regressions.
2. If there is any regression here, it's only in the --verify code; so
   we can always provide workarounds for testing this, to determine
   whether this change may have been at fault. For example, something
   like:

flashrom --write /my/new/image.bin --noverify
     sleep 1
     flashrom --read /tmp/bar.bin
     cmp /my/new/image.bin /tmp/bar.bin

If such problems occur, we can collect system/programmer/chip info to
   try to encode a more targeted delay into the appropriate
   chip/programmer implementation, and avoid penalizing the entire
   project like this.
3. we already have (embedded in erase_write()) erase verification that
   performs no such delay. So depending on the type of timing error that
   this delay was attempting to cover, we may have some proof that this
   delay is no longer necessary (or at least, that whatever systems were
   needing this delay in the first place are no longer caring about
   flashrom).

[1] Famous last words.

BUG=b:325539928
TEST=`flashrom_tester --flashrom_binary=$(which flashrom) \
      internal Erase_and_Write Fail_to_verify`,
TEST=`vpd -i RW_VPD -s foo=bar; vpd -i RW_VPD -l; \
      vpd -i RW_VPD -d foo; vpd -i RW_VPD -l`
TEST=`elogtool list; elogtool add 0xa7; elogtool list`

on (at least) 2 systems:
   #1: Kukui/Kakadu rev2 - MTD programmer /
       kernel 5.10.215-24542-g0515a679eb42 /
       CrOS ~ 15857.0.0
   #2: Zork/Dirinboz rev2 -
       chip name: vendor="Winbond" name="W25Q128.JW.DTR" /
       BIOS: Google_Dirinboz.13434.688.0 /
       kernel 5.4.267-21940-g67f70e251a74 /
       CrOS ~ 15753.43.0

Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc
Signed-off-by: Brian Norris 
---
M flashrom.c
1 file changed, 0 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/07/80807/5