ddr_spd: use unsigned type for crc bytes in DDR3/4 SPD check
PATCH 1/1 ddr_spd: use unsigned type for crc bytes in DDR3/4 SPD check
Using signed char type for computed CRC bytes leads to them being sign extended on comparison with unsigned char values from SPD EEPROM struct. This happens as when being compared those values undergo integer promotion that converts them into ints, sign extending signed types. Having most significant byte set for any of computed CRC bytes thus results in the mismatch being erroneously detected.
While at it, also remove redundant type casts.
--- common/ddr_spd.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
common/ddr_spd.c +10 -10
@@ -65,8 +65,8 @@ int ddr3_spd_check(const struct ddr3_spd_eeprom *spd)char *p = (char *)spd;int csum16;int len; -char crc_lsb;/* byte 126 */ -char crc_msb;/* byte 127 */ +unsigned char crc_lsb;/* byte 126 */ +unsigned char crc_msb;/* byte 127 */ /* * SPD byte0[7] - CRC coverage
@@ -77,8 +77,8 @@ int ddr3_spd_check(const struct ddr3_spd_eeprom *spd) len = !(spd->info_size_crc &0x80 ) ?126 :117 ; csum16 =crc_itu_t (0 , p, len); - crc_lsb = (char ) (csum16 &0xff ); - crc_msb = (char ) (csum16 >>8 ); + crc_lsb = csum16 &0xff ; + crc_msb = csum16 >>8 ;if (spd->crc[0 ] == crc_lsb && spd->crc[1 ] == crc_msb) {return 0 ;
@@ -96,14 +96,14 @@ int ddr4_spd_check(const struct ddr4_spd_eeprom *spd)char *p = (char *)spd;int csum16;int len; -char crc_lsb;/* byte 126 */ -char crc_msb;/* byte 127 */ +unsigned char crc_lsb;/* byte 126 */ +unsigned char crc_msb;/* byte 127 */ len =126 ; csum16 =crc_itu_t (0 , p, len); - crc_lsb = (char ) (csum16 &0xff ); - crc_msb = (char ) (csum16 >>8 ); + crc_lsb = csum16 &0xff ; + crc_msb = csum16 >>8 ;if (spd->crc[0 ] != crc_lsb || spd->crc[1 ] != crc_msb) { printf("SPD checksum unexpected.\n"
@@ -117,8 +117,8 @@ int ddr4_spd_check(const struct ddr4_spd_eeprom *spd) len =126 ; csum16 =crc_itu_t (0 , p, len); - crc_lsb = (char ) (csum16 &0xff ); - crc_msb = (char ) (csum16 >>8 ); + crc_lsb = csum16 &0xff ; + crc_msb = csum16 >>8 ;if (spd->mod_section.uc[126 ] != crc_lsb || spd->mod_section.uc[127 ] != crc_msb) {
Signature · 1 line
2.30.2
Sascha Hauer
AppliedQuoted 7 lines · Denis Orlov
Using signed char type for computed CRC bytes leads to them being sign extended on comparison with unsigned char values from SPD EEPROM struct. This happens as when being compared those values undergo integer promotion that converts them into ints, sign extending signed types. Having most significant byte set for any of computed CRC bytes thus results in the mismatch being erroneously detected.
Quoted 6 lines
While at it, also remove redundant type casts.
Signed-off-by Denis Orlov <[email protected]>--- common/ddr_spd.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
Applied, thanks
Sascha
Quoted 61 lines
common/ddr_spd.c +10
-10@@ -65,8 +65,8 @@ int ddr3_spd_check(const struct ddr3_spd_eeprom *spd)char *p = (char *)spd;int csum16;int len; -char crc_lsb;/* byte 126 */ -char crc_msb;/* byte 127 */ +unsigned char crc_lsb;/* byte 126 */ +unsigned char crc_msb;/* byte 127 */ /* * SPD byte0[7] - CRC coverage @@ -77,8 +77,8 @@ int ddr3_spd_check(const struct ddr3_spd_eeprom *spd) len = !(spd->info_size_crc &0x80 ) ?126 :117 ; csum16 =crc_itu_t (0 , p, len); - crc_lsb = (char ) (csum16 &0xff ); - crc_msb = (char ) (csum16 >>8 ); + crc_lsb = csum16 &0xff ; + crc_msb = csum16 >>8 ;if (spd->crc[0 ] == crc_lsb && spd->crc[1 ] == crc_msb) {return 0 ;@@ -96,14 +96,14 @@ int ddr4_spd_check(const struct ddr4_spd_eeprom *spd)char *p = (char *)spd;int csum16;int len; -char crc_lsb;/* byte 126 */ -char crc_msb;/* byte 127 */ +unsigned char crc_lsb;/* byte 126 */ +unsigned char crc_msb;/* byte 127 */ len =126 ; csum16 =crc_itu_t (0 , p, len); - crc_lsb = (char ) (csum16 &0xff ); - crc_msb = (char ) (csum16 >>8 ); + crc_lsb = csum16 &0xff ; + crc_msb = csum16 >>8 ;if (spd->crc[0 ] != crc_lsb || spd->crc[1 ] != crc_msb) { printf("SPD checksum unexpected.\n"@@ -117,8 +117,8 @@ int ddr4_spd_check(const struct ddr4_spd_eeprom *spd) len =126 ; csum16 =crc_itu_t (0 , p, len); - crc_lsb = (char ) (csum16 &0xff ); - crc_msb = (char ) (csum16 >>8 ); + crc_lsb = csum16 &0xff ; + crc_msb = csum16 >>8 ;if (spd->mod_section.uc[126 ] != crc_lsb || spd->mod_section.uc[127 ] != crc_msb) {Signature · 1 line
2.30.2
Signature · 4 lines
Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |