[U-Boot] [PATCH] fsl: board EEPROM has the CRC in the wrong location

[Scott Wood <scottwood@freescale.com>]

On 07/13/2012 04:25 PM, Wolfgang Denk wrote:
> Dear Tabi Timur-B04825,
> 
> In message <50001038.50303@freescale.com> you wrote:
>>>
>> York is mistaken.  The CRC was always at location 0xFC, but for some 
>> reason, when I wrote the code, I put it at 0xCC.  Now I'm fixing it, and 
>> providing some backwards compatibility to avoid causing problems for 
>> people who upgrade U-Boot on existing boards.  I don't see how this is 
>> controversial in any way.
> 
> In case you have an EEPROM with correct layout (CRC at 0xFC) but
> incorrect CRC, you will access random data and interpret this as CRC.
> This is provoking undefined behaviour.
> 
> Yes, it is inlikely that you happen to find a matching CRC in such a
> case, but it is possible.
> 
> Undefined behaviour is something you must avoid.

Is it any more likely that this will happen, than that you'll see
arbitrary data accidentally match a magic number that identifies a data
format?

> If you want, then rather provide an update tool that theuser can use
> (manually!) to update, but this should be done once, and with explicit
> confirmation from the user, never automagically.

In the real world, this will result in more problems than Timur's
approach (even if the "problems" are just increased support burden from
users asking why ethernet isn't working any more).  The odds of a user
screwing up (or simply not being aware that this update of U-Boot
requires special migration steps) is much more than one in four billion.
 Perhaps this could be limited to boards that are known to have had the
bug in the past?

Timur, I know you said you don't control the format, but could you ask
for a version number bump so that going forward there's a way to
unambiguously mark the contents as "good" (the spec wouldn't change, but
there would be no known implementations of v2 with this bug)?

If not, and Wolfgang still refuses to accept this, what about checking
the old location on a CRC fail, and if the old CRC passes, don't
automatically use it but print a message telling the user that they
probably need to run the migration command?

-Scott