mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Denis Orlov <denorl2009@gmail.com>
To: barebox@lists.infradead.org
Cc: Denis Orlov <denorl2009@gmail.com>
Subject: [PATCH] ddr_spd: use unsigned type for crc bytes in DDR3/4 SPD check
Date: Fri, 13 Jan 2023 17:06:48 +0300	[thread overview]
Message-ID: <20230113140648.31572-1-denorl2009@gmail.com> (raw)

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.

Signed-off-by: Denis Orlov <denorl2009@gmail.com>
---
 common/ddr_spd.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/common/ddr_spd.c b/common/ddr_spd.c
index 52773178e7..dd3b8511e6 100644
--- a/common/ddr_spd.c
+++ b/common/ddr_spd.c
@@ -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) {
-- 
2.30.2




             reply	other threads:[~2023-01-13 14:08 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-13 14:06 Denis Orlov [this message]
2023-01-16 12:40 ` Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230113140648.31572-1-denorl2009@gmail.com \
    --to=denorl2009@gmail.com \
    --cc=barebox@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox