From cdc1152162d5c2bdb773dd31915a7aef4de8f1f0 Mon Sep 17 00:00:00 2001 From: Michal Sieron Date: Thu, 16 Feb 2023 14:27:49 +0100 Subject: [PATCH 1/3] liblitedram/sdram_spd: use temp_len when reading SPD This change should change anything from the point of view of `sdram_read_spd` caller, but it makes it so less I2C reads are actually hapenning. With `len` we read too many bytes and write them to the `buf`. In subsequent iterations we overwrite those bytes as all counters are being updated by the `temp_len`. Nothing terrible, but too many bytes were being read. Signed-off-by: Michal Sieron --- litex/soc/software/liblitedram/sdram_spd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/litex/soc/software/liblitedram/sdram_spd.c b/litex/soc/software/liblitedram/sdram_spd.c index 9101b7734..b92f6597a 100644 --- a/litex/soc/software/liblitedram/sdram_spd.c +++ b/litex/soc/software/liblitedram/sdram_spd.c @@ -48,7 +48,7 @@ bool sdram_read_spd(uint8_t spd, uint16_t addr, uint8_t *buf, uint16_t len, bool temp_len = len; } - ok &= i2c_read(SPD_RW_ADDR(spd), offset, &buf[read_bytes], len, temp_send_stop, 1); + ok &= i2c_read(SPD_RW_ADDR(spd), offset, &buf[read_bytes], temp_len, temp_send_stop, 1); len -= temp_len; read_bytes += temp_len; addr += temp_len; From f84ecaf707d2102dc2e2572b45ded465a236c19b Mon Sep 17 00:00:00 2001 From: Michal Sieron Date: Thu, 16 Feb 2023 14:28:22 +0100 Subject: [PATCH 2/3] liblitedram/sdram_spd: do not send stop symbol According to the SPD specification, we shouldn't send a stop symbol after the write that sets the address counter. Signed-off-by: Michal Sieron --- litex/soc/software/bios/cmds/cmd_litedram.c | 13 ++----------- litex/soc/software/liblitedram/sdram_spd.c | 9 +++------ litex/soc/software/liblitedram/sdram_spd.h | 2 +- litex/soc/software/liblitedram/utils.c | 2 +- 4 files changed, 7 insertions(+), 19 deletions(-) diff --git a/litex/soc/software/bios/cmds/cmd_litedram.c b/litex/soc/software/bios/cmds/cmd_litedram.c index 67f4e3bef..539702258 100644 --- a/litex/soc/software/bios/cmds/cmd_litedram.c +++ b/litex/soc/software/bios/cmds/cmd_litedram.c @@ -403,10 +403,9 @@ static void sdram_spd_handler(int nb_params, char **params) unsigned char spdaddr; unsigned char buf[SDRAM_SPD_SIZE]; int len = sizeof(buf); - bool send_stop = true; if (nb_params < 1) { - printf("sdram_spd []"); + printf("sdram_spd "); return; } @@ -420,15 +419,7 @@ static void sdram_spd_handler(int nb_params, char **params) return; } - if (nb_params > 1) { - send_stop = strtoul(params[1], &c, 0) != 0; - if (*c != 0) { - printf("Incorrect send_stop value"); - return; - } - } - - if (!sdram_read_spd(spdaddr, 0, buf, (uint16_t)len, send_stop)) { + if (!sdram_read_spd(spdaddr, 0, buf, (uint16_t)len)) { printf("Error when reading SPD EEPROM"); return; } diff --git a/litex/soc/software/liblitedram/sdram_spd.c b/litex/soc/software/liblitedram/sdram_spd.c index b92f6597a..b0f94407a 100644 --- a/litex/soc/software/liblitedram/sdram_spd.c +++ b/litex/soc/software/liblitedram/sdram_spd.c @@ -28,11 +28,10 @@ static bool sdram_select_spd_page(uint8_t page) { } #endif -bool sdram_read_spd(uint8_t spd, uint16_t addr, uint8_t *buf, uint16_t len, bool send_stop) { +bool sdram_read_spd(uint8_t spd, uint16_t addr, uint8_t *buf, uint16_t len) { uint8_t page; uint16_t offset; uint16_t temp_len, read_bytes = 0; - bool temp_send_stop = false; bool ok = true; @@ -43,12 +42,10 @@ bool sdram_read_spd(uint8_t spd, uint16_t addr, uint8_t *buf, uint16_t len, bool offset = addr % SDRAM_SPD_PAGE_SIZE; temp_len = SDRAM_SPD_PAGE_SIZE - offset; - if (temp_len >= len) { - temp_send_stop = send_stop; + if (temp_len > len) temp_len = len; - } - ok &= i2c_read(SPD_RW_ADDR(spd), offset, &buf[read_bytes], temp_len, temp_send_stop, 1); + ok &= i2c_read(SPD_RW_ADDR(spd), offset, &buf[read_bytes], temp_len, false, 1); len -= temp_len; read_bytes += temp_len; addr += temp_len; diff --git a/litex/soc/software/liblitedram/sdram_spd.h b/litex/soc/software/liblitedram/sdram_spd.h index cfe06f93e..cf9f321c0 100644 --- a/litex/soc/software/liblitedram/sdram_spd.h +++ b/litex/soc/software/liblitedram/sdram_spd.h @@ -36,7 +36,7 @@ extern "C" { #endif /* CSR_SDRAM_BASE && CONFIG_HAS_I2C */ -bool sdram_read_spd(uint8_t spd, uint16_t addr, uint8_t *buf, uint16_t len, bool send_stop); +bool sdram_read_spd(uint8_t spd, uint16_t addr, uint8_t *buf, uint16_t len); #ifdef __cplusplus } diff --git a/litex/soc/software/liblitedram/utils.c b/litex/soc/software/liblitedram/utils.c index 3b6ef2dc3..adfe188a7 100644 --- a/litex/soc/software/liblitedram/utils.c +++ b/litex/soc/software/liblitedram/utils.c @@ -40,7 +40,7 @@ uint64_t sdram_get_supported_memory(void) { #if defined(SDRAM_PHY_DDR3) || defined(SDRAM_PHY_DDR4) uint8_t buf; - if (!sdram_read_spd(0x0, 4, &buf, 1, true)) { + if (!sdram_read_spd(0x0, 4, &buf, 1)) { printf("Couldn't read SDRAM size from the SPD, defaulting to 256 MB.\n"); return 256 << 20; } From 23d84bf5f56cb24bc75434e6a3224c4287e033ce Mon Sep 17 00:00:00 2001 From: Michal Sieron Date: Thu, 16 Feb 2023 15:40:07 +0100 Subject: [PATCH 3/3] liblitedram/sdram_rcd: fix no I2C case Remove send_stop parameter from the no-I2C case as well. Signed-off-by: Michal Sieron --- litex/soc/software/liblitedram/sdram_spd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/litex/soc/software/liblitedram/sdram_spd.c b/litex/soc/software/liblitedram/sdram_spd.c index b0f94407a..912e73225 100644 --- a/litex/soc/software/liblitedram/sdram_spd.c +++ b/litex/soc/software/liblitedram/sdram_spd.c @@ -54,7 +54,7 @@ bool sdram_read_spd(uint8_t spd, uint16_t addr, uint8_t *buf, uint16_t len) { return ok; } #else /* no CSR_SDRAM_BASE && CONFIG_HAS_I2C */ -bool sdram_read_spd(uint8_t spd, uint16_t addr, uint8_t *buf, uint16_t len, bool send_stop) { +bool sdram_read_spd(uint8_t spd, uint16_t addr, uint8_t *buf, uint16_t len) { return false; } #endif /* CSR_SDRAM_BASE && CONFIG_HAS_I2C */