Andrew McRae has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35604 )
Change subject: util/mb/google/hatch: update CRC calculation for correctness ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35604/2/util/mainboard/google/hatch... File util/mainboard/google/hatch/kconfig.py:
https://review.coreboot.org/c/coreboot/+/35604/2/util/mainboard/google/hatch... PS2, Line 68: . I am not sure what the original intended rules are or how the 'correct' value is determined. To me, it would seem natural to use abs() rather than a mask, and I note there are potentially 3 different values that could result: python
a = -12345678 print a % 10000
4322
print (a & 0xffffffff) % 10000
1618
print abs(a) % 10000
5678
Can you add a link to a reference that explains the definitive calculation? It seems that any previous calculations would have given a different result with a -ve value, so if it is going to change anyway, maybe using abs() is better? As a nit, the conversion isn't to an unsigned 32 bit value, but to a 31 bit value with a cleared sign bit (which is the same I guess, but if you referenced it in a language like C that allowed unsigned 32 bit values, you are only using 31 bits). Rambling on, you could also call it a signed 32 bit value that is always positive :-)