Re: [PATCH v5 10/11] ram: starfive: Read memory size information from EEPROM

2023-10-10 Thread Heinrich Schuchardt 

On 11.10.23 04:54, Heinrich Schuchardt wrote:

On 15.06.23 11:36, Yanhong Wang wrote:

StarFive VisionFive 2 has two versions, 1.2A and 1.3B, each version of
DDR capacity includes 2G/4G/8G, a DT can not support multiple
capacities, so the capacity size information is recorded to EEPROM, when
DDR initialization required capacity size information is read from
EEPROM.

If there is no information in EEPROM, it is initialized with the default
size defined in DT.

Signed-off-by: Yanhong Wang 
Reviewed-by: Leo Yu-Chi Liang 
---
  arch/riscv/cpu/jh7110/spl.c | 32 -
  drivers/ram/starfive/starfive_ddr.c |  2 --
  2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/cpu/jh7110/spl.c b/arch/riscv/cpu/jh7110/spl.c
index 104f0fe949..72adcefa0e 100644
--- a/arch/riscv/cpu/jh7110/spl.c
+++ b/arch/riscv/cpu/jh7110/spl.c
@@ -3,19 +3,49 @@
   * Copyright (C) 2022 StarFive Technology Co., Ltd.
   * Author: Yanhong Wang
   */
-
+#include 
+#include 
  #include 
  #include 
  #include 
+#include 
  #include 
+#include 
  #define CSR_U74_FEATURE_DISABLE    0x7c1
  #define L2_LIM_MEM_END    0x81FUL
+DECLARE_GLOBAL_DATA_PTR;
+
+static bool check_ddr_size(phys_size_t size)
+{
+    switch (size) {
+    case SZ_2:
+    case SZ_4:
+    case SZ_8:
+    case SZ_16:
+    return true;
+    default:
+    return false;
+    }
+}
+
  int spl_soc_init(void)
  {
  int ret;
  struct udevice *dev;
+    phys_size_t size;
+
+    ret = fdtdec_setup_mem_size_base();
+    if (ret)
+    return ret;
+
+    /* Read the definition of the DDR size from eeprom, and if not,
+ * use the definition in DT
+ */
+    size = (get_ddr_size_from_eeprom() >> 16) & 0xFF;


On origin/master for a 4 GiB board with a serial number 
VF7110B1-2253-D004E000-4000 this always fails. This results in 
memory reported as 8 GiB and Linux crashing.


i2c_get_chip_for_busnum() returns -EINVAL in read_eeprom() 
[board/starfive/visionfive2/visionfive2-i2c-eeprom.c:335].


Is the driver model really expected to be fully initialized before 
setting up memory?


Best regards

Heinrich


The root cause seems to be in dw_i2c_calc_timing(). Probing of the 
i2c_designware device fails with:


dw_i2c: mode 0, ic_clk 100, speed 10, period 10 rise 1 fall 1 
tlow 5 thigh 4 spk 0

dw_i2c: bad counts. hcnt = -4 lcnt = 4
device_probe: i2c@1205 failed to probe -22

When I change the hcnt timing calculation, by replacing the offset of 7 
to 1, the device is probed correctly and I get the correct memory size 
for my board.


Best regards

Heinrich





+    if (check_ddr_size(size))
+    gd->ram_size = size << 30;
  /* DDR init */
  ret = uclass_get_device(UCLASS_RAM, 0, &dev);
diff --git a/drivers/ram/starfive/starfive_ddr.c 
b/drivers/ram/starfive/starfive_ddr.c

index 553f2ce6f4..a0a3d6b33d 100644
--- a/drivers/ram/starfive/starfive_ddr.c
+++ b/drivers/ram/starfive/starfive_ddr.c
@@ -72,8 +72,6 @@ static int starfive_ddr_probe(struct udevice *dev)
  u64 rate;
  int ret;
-    /* Read memory base and size from DT */
-    fdtdec_setup_mem_size_base();
  priv->info.base = gd->ram_base;
  priv->info.size = gd->ram_size;

Re: [PATCH v5 10/11] ram: starfive: Read memory size information from EEPROM

2023-10-10 Thread Heinrich Schuchardt 

On 15.06.23 11:36, Yanhong Wang wrote:

StarFive VisionFive 2 has two versions, 1.2A and 1.3B, each version of
DDR capacity includes 2G/4G/8G, a DT can not support multiple
capacities, so the capacity size information is recorded to EEPROM, when
DDR initialization required capacity size information is read from
EEPROM.

If there is no information in EEPROM, it is initialized with the default
size defined in DT.

Signed-off-by: Yanhong Wang 
Reviewed-by: Leo Yu-Chi Liang 
---
  arch/riscv/cpu/jh7110/spl.c | 32 -
  drivers/ram/starfive/starfive_ddr.c |  2 --
  2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/cpu/jh7110/spl.c b/arch/riscv/cpu/jh7110/spl.c
index 104f0fe949..72adcefa0e 100644
--- a/arch/riscv/cpu/jh7110/spl.c
+++ b/arch/riscv/cpu/jh7110/spl.c
@@ -3,19 +3,49 @@
   * Copyright (C) 2022 StarFive Technology Co., Ltd.
   * Author: Yanhong Wang
   */
-
+#include 
+#include 
  #include 
  #include 
  #include 
+#include 
  #include 
+#include 
  
  #define CSR_U74_FEATURE_DISABLE	0x7c1

  #define L2_LIM_MEM_END0x81FUL
  
+DECLARE_GLOBAL_DATA_PTR;

+
+static bool check_ddr_size(phys_size_t size)
+{
+   switch (size) {
+   case SZ_2:
+   case SZ_4:
+   case SZ_8:
+   case SZ_16:
+   return true;
+   default:
+   return false;
+   }
+}
+
  int spl_soc_init(void)
  {
int ret;
struct udevice *dev;
+   phys_size_t size;
+
+   ret = fdtdec_setup_mem_size_base();
+   if (ret)
+   return ret;
+
+   /* Read the definition of the DDR size from eeprom, and if not,
+* use the definition in DT
+*/
+   size = (get_ddr_size_from_eeprom() >> 16) & 0xFF;


On origin/master for a 4 GiB board with a serial number 
VF7110B1-2253-D004E000-4000 this always fails. This results in 
memory reported as 8 GiB and Linux crashing.


i2c_get_chip_for_busnum() returns -EINVAL in read_eeprom() 
[board/starfive/visionfive2/visionfive2-i2c-eeprom.c:335].


Is the driver model really expected to be fully initialized before 
setting up memory?


Best regards

Heinrich


+   if (check_ddr_size(size))
+   gd->ram_size = size << 30;
  
  	/* DDR init */

ret = uclass_get_device(UCLASS_RAM, 0, &dev);
diff --git a/drivers/ram/starfive/starfive_ddr.c 
b/drivers/ram/starfive/starfive_ddr.c
index 553f2ce6f4..a0a3d6b33d 100644
--- a/drivers/ram/starfive/starfive_ddr.c
+++ b/drivers/ram/starfive/starfive_ddr.c
@@ -72,8 +72,6 @@ static int starfive_ddr_probe(struct udevice *dev)
u64 rate;
int ret;
  
-	/* Read memory base and size from DT */

-   fdtdec_setup_mem_size_base();
priv->info.base = gd->ram_base;
priv->info.size = gd->ram_size;

Re: [PATCH v5 10/11] ram: starfive: Read memory size information from EEPROM

2023-10-10 Thread Andreas Schwab 
On Jun 15 2023, Yanhong Wang wrote:

> StarFive VisionFive 2 has two versions, 1.2A and 1.3B, each version of
> DDR capacity includes 2G/4G/8G, a DT can not support multiple
> capacities, so the capacity size information is recorded to EEPROM, when
> DDR initialization required capacity size information is read from
> EEPROM.

Does that acutally work?  I see that read_eeprom fails in SPL.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH v5 10/11] ram: starfive: Read memory size information from EEPROM

2023-07-11 Thread Leo Liang 
On Thu, Jun 15, 2023 at 05:36:51PM +0800, Yanhong Wang wrote:
> StarFive VisionFive 2 has two versions, 1.2A and 1.3B, each version of
> DDR capacity includes 2G/4G/8G, a DT can not support multiple
> capacities, so the capacity size information is recorded to EEPROM, when
> DDR initialization required capacity size information is read from
> EEPROM.
> 
> If there is no information in EEPROM, it is initialized with the default
> size defined in DT.
> 
> Signed-off-by: Yanhong Wang 
> ---
>  arch/riscv/cpu/jh7110/spl.c | 32 -
>  drivers/ram/starfive/starfive_ddr.c |  2 --
>  2 files changed, 31 insertions(+), 3 deletions(-)

Reviewed-by: Leo Yu-Chi Liang

Re: [PATCH v5 10/11] ram: starfive: Read memory size information from EEPROM

2023-06-20 Thread Rick Chen 
> From: Yanhong Wang 
> Sent: Thursday, June 15, 2023 5:37 PM
> To: u-boot@lists.denx.de; Rick Jian-Zhi Chen(陳建志) ; Leo 
> Yu-Chi Liang(梁育齊) ; Joe Hershberger 
> ; Ramon Fried 
> Cc: Yanhong Wang ; Torsten Duwe ; 
> Leyfoon Tan ; samin . guo 
> ; Walker Chen ; Hal 
> Feng 
> Subject: [PATCH v5 10/11] ram: starfive: Read memory size information from 
> EEPROM
>
> StarFive VisionFive 2 has two versions, 1.2A and 1.3B, each version of DDR 
> capacity includes 2G/4G/8G, a DT can not support multiple capacities, so the 
> capacity size information is recorded to EEPROM, when DDR initialization 
> required capacity size information is read from EEPROM.
>
> If there is no information in EEPROM, it is initialized with the default size 
> defined in DT.
>
> Signed-off-by: Yanhong Wang 
> ---
>  arch/riscv/cpu/jh7110/spl.c | 32 -
>  drivers/ram/starfive/starfive_ddr.c |  2 --
>  2 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/cpu/jh7110/spl.c b/arch/riscv/cpu/jh7110/spl.c index 
> 104f0fe949..72adcefa0e 100644
> --- a/arch/riscv/cpu/jh7110/spl.c
> +++ b/arch/riscv/cpu/jh7110/spl.c
> @@ -3,19 +3,49 @@
>   * Copyright (C) 2022 StarFive Technology Co., Ltd.
>   * Author: Yanhong Wang
>   */
> -
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> +#include 
>
>  #define CSR_U74_FEATURE_DISABLE0x7c1
>  #define L2_LIM_MEM_END 0x81FUL
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static bool check_ddr_size(phys_size_t size) {
> +   switch (size) {
> +   case SZ_2:
> +   case SZ_4:
> +   case SZ_8:
> +   case SZ_16:

In commit message, it describes that "DDR capacity includes 2G/4G/8G".
Is it mismatch here ?

> +   return true;
> +   default:
> +   return false;
> +   }
> +}
> +
>  int spl_soc_init(void)
>  {
> int ret;
> struct udevice *dev;
> +   phys_size_t size;
> +
> +   ret = fdtdec_setup_mem_size_base();
> +   if (ret)
> +   return ret;

It maybe unnecessary to add return above. If it fail to parse memory
node from DT, then there
has no chance to get ddr size from eeprom.

Thanks,
Rick

> +
> +   /* Read the definition of the DDR size from eeprom, and if not,
> +* use the definition in DT
> +*/
> +   size = (get_ddr_size_from_eeprom() >> 16) & 0xFF;
> +   if (check_ddr_size(size))
> +   gd->ram_size = size << 30;
>
> /* DDR init */
> ret = uclass_get_device(UCLASS_RAM, 0, &dev); diff --git 
> a/drivers/ram/starfive/starfive_ddr.c b/drivers/ram/starfive/starfive_ddr.c
> index 553f2ce6f4..a0a3d6b33d 100644
> --- a/drivers/ram/starfive/starfive_ddr.c
> +++ b/drivers/ram/starfive/starfive_ddr.c
> @@ -72,8 +72,6 @@ static int starfive_ddr_probe(struct udevice *dev)
> u64 rate;
> int ret;
>
> -   /* Read memory base and size from DT */
> -   fdtdec_setup_mem_size_base();
> priv->info.base = gd->ram_base;
> priv->info.size = gd->ram_size;
>
> --
> 2.17.1

[PATCH v5 10/11] ram: starfive: Read memory size information from EEPROM

2023-06-15 Thread Yanhong Wang 
StarFive VisionFive 2 has two versions, 1.2A and 1.3B, each version of
DDR capacity includes 2G/4G/8G, a DT can not support multiple
capacities, so the capacity size information is recorded to EEPROM, when
DDR initialization required capacity size information is read from
EEPROM.

If there is no information in EEPROM, it is initialized with the default
size defined in DT.

Signed-off-by: Yanhong Wang 
---
 arch/riscv/cpu/jh7110/spl.c | 32 -
 drivers/ram/starfive/starfive_ddr.c |  2 --
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/cpu/jh7110/spl.c b/arch/riscv/cpu/jh7110/spl.c
index 104f0fe949..72adcefa0e 100644
--- a/arch/riscv/cpu/jh7110/spl.c
+++ b/arch/riscv/cpu/jh7110/spl.c
@@ -3,19 +3,49 @@
  * Copyright (C) 2022 StarFive Technology Co., Ltd.
  * Author: Yanhong Wang
  */
-
+#include 
+#include 
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 
 #define CSR_U74_FEATURE_DISABLE0x7c1
 #define L2_LIM_MEM_END 0x81FUL
 
+DECLARE_GLOBAL_DATA_PTR;
+
+static bool check_ddr_size(phys_size_t size)
+{
+   switch (size) {
+   case SZ_2:
+   case SZ_4:
+   case SZ_8:
+   case SZ_16:
+   return true;
+   default:
+   return false;
+   }
+}
+
 int spl_soc_init(void)
 {
int ret;
struct udevice *dev;
+   phys_size_t size;
+
+   ret = fdtdec_setup_mem_size_base();
+   if (ret)
+   return ret;
+
+   /* Read the definition of the DDR size from eeprom, and if not,
+* use the definition in DT
+*/
+   size = (get_ddr_size_from_eeprom() >> 16) & 0xFF;
+   if (check_ddr_size(size))
+   gd->ram_size = size << 30;
 
/* DDR init */
ret = uclass_get_device(UCLASS_RAM, 0, &dev);
diff --git a/drivers/ram/starfive/starfive_ddr.c 
b/drivers/ram/starfive/starfive_ddr.c
index 553f2ce6f4..a0a3d6b33d 100644
--- a/drivers/ram/starfive/starfive_ddr.c
+++ b/drivers/ram/starfive/starfive_ddr.c
@@ -72,8 +72,6 @@ static int starfive_ddr_probe(struct udevice *dev)
u64 rate;
int ret;
 
-   /* Read memory base and size from DT */
-   fdtdec_setup_mem_size_base();
priv->info.base = gd->ram_base;
priv->info.size = gd->ram_size;
 
-- 
2.17.1