[U-Boot] [PATCH] socfpga: stratix10: fix sdram_calculate_size
Incorrect type of size variable results in 0 being returned for sdram sizes greater than or equal to 4GB.
Signed-off-by: Dalon Westergreen dwesterg@gmail.com --- drivers/ddr/altera/sdram_s10.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c index 48f4f47b14..dad0147b64 100644 --- a/drivers/ddr/altera/sdram_s10.c +++ b/drivers/ddr/altera/sdram_s10.c @@ -375,7 +375,7 @@ unsigned long sdram_calculate_size(void) { u32 dramaddrw = hmc_readl(DRAMADDRW);
- u32 size = 1 << (DRAMADDRW_CFG_CS_ADDR_WIDTH(dramaddrw) + + unsigned long size = 1 << (DRAMADDRW_CFG_CS_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_BANK_GRP_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_BANK_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_ROW_ADDR_WIDTH(dramaddrw) +
On 09/10/2018 10:41 PM, Dalon Westergreen wrote:
Incorrect type of size variable results in 0 being returned for sdram sizes greater than or equal to 4GB.
Signed-off-by: Dalon Westergreen dwesterg@gmail.com
drivers/ddr/altera/sdram_s10.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c index 48f4f47b14..dad0147b64 100644 --- a/drivers/ddr/altera/sdram_s10.c +++ b/drivers/ddr/altera/sdram_s10.c @@ -375,7 +375,7 @@ unsigned long sdram_calculate_size(void) { u32 dramaddrw = hmc_readl(DRAMADDRW);
- u32 size = 1 << (DRAMADDRW_CFG_CS_ADDR_WIDTH(dramaddrw) +
- unsigned long size = 1 << (DRAMADDRW_CFG_CS_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_BANK_GRP_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_BANK_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_ROW_ADDR_WIDTH(dramaddrw) +
Change the function to report size in MiB, prescale it here and you won't have problems for a while . How does that sound ?
On Tue, 2018-09-11 at 11:31 +0200, Marek Vasut wrote:
On 09/10/2018 10:41 PM, Dalon Westergreen wrote: Incorrect type of size variable results in 0 beingreturned for sdram sizes greater than or equal to4GB. Signed-off-by: Dalon Westergreen dwesterg@gmail.com--- drivers/ddr/altera/sdram_s10.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.cindex 48f4f47b14..dad0147b64 100644--- a/drivers/ddr/altera/sdram_s10.c+++ b/drivers/ddr/altera/sdram_s10.c@@ -375,7 +375,7 @@ unsigned long sdram_calculate_size(void) { u32 dramaddrw = hmc_readl(DRAMADDRW); - u32 size = 1 << (DRAMADDRW_CFG_CS_ADDR_WIDTH(dramaddrw) ++ unsigned long size = 1 << (DRAMADDRW_CFG_CS_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_B ANK_GRP_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_BANK_ADDR _WIDTH(dramaddrw) + DRAMADDRW_CFG_ROW_ADDR_WIDTH(dramaddrw)
Change the function to report size in MiB, prescale it here and youwon't have problems for a while . How does that sound ?
Prefer not to as this will be used for comparisons later which would mean we would need to scale #defines from linux/sizes.h. --dalon
On 09/11/2018 03:28 PM, Dalon L Westergreen wrote:
On Tue, 2018-09-11 at 11:31 +0200, Marek Vasut wrote:
On 09/10/2018 10:41 PM, Dalon Westergreen wrote: Incorrect type of size variable results in 0 being returned for sdram sizes greater than or equal to 4GB.
Signed-off-by: Dalon Westergreen <dwesterg@gmail.com mailto:dwesterg@gmail.com>
drivers/ddr/altera/sdram_s10.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c index 48f4f47b14..dad0147b64 100644 --- a/drivers/ddr/altera/sdram_s10.c +++ b/drivers/ddr/altera/sdram_s10.c @@ -375,7 +375,7 @@ unsigned long sdram_calculate_size(void) { u32 dramaddrw = hmc_readl(DRAMADDRW);
- u32 size = 1 << (DRAMADDRW_CFG_CS_ADDR_WIDTH(dramaddrw) +
- unsigned long size = 1 << (DRAMADDRW_CFG_CS_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_BANK_GRP_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_BANK_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_ROW_ADDR_WIDTH(dramaddrw) +
Change the function to report size in MiB, prescale it here and you won't have problems for a while . How does that sound ?
Prefer not to as this will be used for comparisons later which would mean we would need to scale #defines from linux/sizes.h.
What comparisons ? I only see it assigned to gd->sdram_size , in which case it might rather make sense to adjust the type of the function to unsigned long long (so it returns u64).
On Tue, 2018-09-11 at 15:52 +0200, Marek Vasut wrote:
On 09/11/2018 03:28 PM, Dalon L Westergreen wrote: On Tue, 2018-09-11 at 11:31 +0200, Marek Vasut wrote: On 09/10/2018 10:41 PM, Dalon Westergreen wrote:Incorrect type of size variable results in 0 beingreturned for sdram sizes greater than or equal to4GB. Signed-off-by: Dalon Westergreen <dwesterg@gmail.com mailto: dwesterg@gmail.com>--- drivers/ddr/altera/sdram_s10.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.cindex 48f4f47b14..dad0147b64 100644--- a/drivers/ddr/altera/sdram_s10.c+++ b/drivers/ddr/altera/sdram_s10.c@@ -375,7 +375,7 @@ unsigned long sdram_calculate_size(void) { u32 dramaddrw = hmc_readl(DRAMADDRW); - u32 size = 1 << (DRAMADDRW_CFG_CS_ADDR_WIDTH(dramaddrw) ++ unsigned long size = 1 << (DRAMADDRW_CFG_CS_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_B ANK_GRP_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_BANK_ADDR _WIDTH(dramaddrw) + DRAMADDRW_CFG_ROW_ADDR_WIDTH(dramaddrw)
Change the function to report size in MiB, prescale it here and youwon't have problems for a while . How does that sound ? Prefer not to as this will be used for comparisons later which wouldmean we would need to scale #defines from linux/sizes.h. What comparisons ? I only see it assigned to gd->sdram_size , in whichcase it might rather make sense to adjust the type of the function tounsigned long long (so it returns u64).
Yes, i think the function should be phys_size_t type, i will adjust it. There are no comparisons as ofyet, but there likely will be a need to do so to deal with some corner cases. --dalon
On 09/11/2018 05:13 PM, Dalon L Westergreen wrote:
On Tue, 2018-09-11 at 15:52 +0200, Marek Vasut wrote:
On 09/11/2018 03:28 PM, Dalon L Westergreen wrote: On Tue, 2018-09-11 at 11:31 +0200, Marek Vasut wrote: On 09/10/2018 10:41 PM, Dalon Westergreen wrote: Incorrect type of size variable results in 0 being returned for sdram sizes greater than or equal to 4GB.
Signed-off-by: Dalon Westergreen <dwesterg@gmail.com mailto:dwesterg@gmail.com <mailto:dwesterg@gmail.com mailto:dwesterg@gmail.com>>
drivers/ddr/altera/sdram_s10.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c index 48f4f47b14..dad0147b64 100644 --- a/drivers/ddr/altera/sdram_s10.c +++ b/drivers/ddr/altera/sdram_s10.c @@ -375,7 +375,7 @@ unsigned long sdram_calculate_size(void) { u32 dramaddrw = hmc_readl(DRAMADDRW);
- u32 size = 1 << (DRAMADDRW_CFG_CS_ADDR_WIDTH(dramaddrw) +
- unsigned long size = 1 << (DRAMADDRW_CFG_CS_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_BANK_GRP_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_BANK_ADDR_WIDTH(dramaddrw) + DRAMADDRW_CFG_ROW_ADDR_WIDTH(dramaddrw) +
Change the function to report size in MiB, prescale it here and you won't have problems for a while . How does that sound ?
Prefer not to as this will be used for comparisons later which would mean we would need to scale #defines from linux/sizes.h.
What comparisons ? I only see it assigned to gd->sdram_size , in which case it might rather make sense to adjust the type of the function to unsigned long long (so it returns u64).
Yes, i think the function should be phys_size_t type, i will adjust it. There are no comparisons as of yet, but there likely will be a need to do so to deal with some corner cases.
OK, sounds good , thanks !
participants (3)
-
Dalon L Westergreen -
Dalon Westergreen -
Marek Vasut