All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling
@ 2022-06-21 10:07 Lee Jones
  2022-06-21 10:07 ` [PATCH 2/3] ram: rk3399: Fix faulty frequency change reports Lee Jones
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Lee Jones @ 2022-06-21 10:07 UTC (permalink / raw)
  To: lee.jones, sjg, philipp.tomsich, kever.yang; +Cc: cym, u-boot

Functions pointed to by this op pointer can return non-zero values
indicating an error.  Ensure any error value is propagated back up the
call-chain.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/ram/rockchip/sdram_rk3399.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
index c0a06dcaed..0af0fa9e7b 100644
--- a/drivers/ram/rockchip/sdram_rk3399.c
+++ b/drivers/ram/rockchip/sdram_rk3399.c
@@ -3005,7 +3005,9 @@ static int sdram_init(struct dram_info *dram,
 	params->base.stride = calculate_stride(params);
 	dram_all_config(dram, params);
 
-	dram->ops->set_rate_index(dram, params);
+	ret = dram->ops->set_rate_index(dram, params);
+	if (ret)
+		return ret;
 
 	debug("Finish SDRAM initialization...\n");
 	return 0;
-- 
2.37.0.rc0.104.g0611611a94-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] ram: rk3399: Fix faulty frequency change reports
  2022-06-21 10:07 [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling Lee Jones
@ 2022-06-21 10:07 ` Lee Jones
  2022-07-01 12:04   ` Kever Yang
  2022-07-04 10:00   ` Xavier Drudis Ferran
  2022-06-21 10:07 ` [PATCH 3/3] ram: rk3399: Conduct memory training at 400MHz Lee Jones
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Lee Jones @ 2022-06-21 10:07 UTC (permalink / raw)
  To: lee.jones, sjg, philipp.tomsich, kever.yang; +Cc: cym, u-boot

Frequency changes to 400MHz are presently reported as:

  lpddr4_set_rate_0: change freq to 400000000 mhz 0, 1

This is obviously wrong by 6 orders of magnitude.

Ensure frequency changes are reported accurately.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/ram/rockchip/sdram_rk3399.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
index 0af0fa9e7b..34d6c93f95 100644
--- a/drivers/ram/rockchip/sdram_rk3399.c
+++ b/drivers/ram/rockchip/sdram_rk3399.c
@@ -2552,8 +2552,8 @@ static int lpddr4_set_rate(struct dram_info *dram,
 			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
 
 		if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
-			printf("%s: change freq to %d mhz %d, %d\n", __func__,
-			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq,
+			printf("%s: change freq to %dMHz %d, %d\n", __func__,
+			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
 			       ctl_fn, phy_fn);
 	}
 
-- 
2.37.0.rc0.104.g0611611a94-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] ram: rk3399: Conduct memory training at 400MHz
  2022-06-21 10:07 [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling Lee Jones
  2022-06-21 10:07 ` [PATCH 2/3] ram: rk3399: Fix faulty frequency change reports Lee Jones
@ 2022-06-21 10:07 ` Lee Jones
  2022-07-01 12:05   ` Kever Yang
  2022-07-04 10:01   ` Xavier Drudis Ferran
  2022-06-27  8:39 ` [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling Lee Jones
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Lee Jones @ 2022-06-21 10:07 UTC (permalink / raw)
  To: lee.jones, sjg, philipp.tomsich, kever.yang; +Cc: cym, u-boot

Currently the default initialisation frequency is 50MHz.  Although
this does appear to be suitable for some LPDDR4 RAM chips, training at
this low frequency has been seen to cause Column errors, leading to
Capacity check errors on others.

Here we force RAM initialisation to happen at 400MHz before ramping up
to the final value running value of 800MHz after everything has been
successfully configured.

Link: https://lore.kernel.org/u-boot/Yo4v3jUeHXTovjOH@google.com/
Suggested-by: YouMin Chen <cym@rock-chips.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/ram/rockchip/sdram_rk3399.c | 36 +++++++++++++++++------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
index 34d6c93f95..b05c5925d5 100644
--- a/drivers/ram/rockchip/sdram_rk3399.c
+++ b/drivers/ram/rockchip/sdram_rk3399.c
@@ -85,7 +85,7 @@ struct sdram_rk3399_ops {
 	int (*data_training_first)(struct dram_info *dram, u32 channel, u8 rank,
 				   struct rk3399_sdram_params *sdram);
 	int (*set_rate_index)(struct dram_info *dram,
-			      struct rk3399_sdram_params *params);
+			      struct rk3399_sdram_params *params, u32 ctl_fn);
 	void (*modify_param)(const struct chan_info *chan,
 			     struct rk3399_sdram_params *params);
 	struct rk3399_sdram_params *
@@ -1644,7 +1644,8 @@ static int data_training_first(struct dram_info *dram, u32 channel, u8 rank,
 }
 
 static int switch_to_phy_index1(struct dram_info *dram,
-				struct rk3399_sdram_params *params)
+				struct rk3399_sdram_params *params,
+				u32 unused)
 {
 	u32 channel;
 	u32 *denali_phy;
@@ -2539,26 +2540,25 @@ static int lpddr4_set_ctl(struct dram_info *dram,
 }
 
 static int lpddr4_set_rate(struct dram_info *dram,
-			   struct rk3399_sdram_params *params)
+			    struct rk3399_sdram_params *params,
+			    u32 ctl_fn)
 {
-	u32 ctl_fn;
 	u32 phy_fn;
 
-	for (ctl_fn = 0; ctl_fn < 2; ctl_fn++) {
-		phy_fn = lpddr4_get_phy_fn(params, ctl_fn);
+	phy_fn = lpddr4_get_phy_fn(params, ctl_fn);
 
-		lpddr4_set_phy(dram, params, phy_fn, &dfs_cfgs_lpddr4[ctl_fn]);
-		lpddr4_set_ctl(dram, params, ctl_fn,
-			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
+	lpddr4_set_phy(dram, params, phy_fn, &dfs_cfgs_lpddr4[ctl_fn]);
+	lpddr4_set_ctl(dram, params, ctl_fn,
+		       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
 
-		if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
-			printf("%s: change freq to %dMHz %d, %d\n", __func__,
-			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
-			       ctl_fn, phy_fn);
-	}
+	if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
+		printf("%s: change freq to %dMHz %d, %d\n", __func__,
+		       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
+		       ctl_fn, phy_fn);
 
 	return 0;
 }
+
 #endif /* CONFIG_RAM_RK3399_LPDDR4 */
 
 /* CS0,n=1
@@ -2955,6 +2955,12 @@ static int sdram_init(struct dram_info *dram,
 		params->ch[ch].cap_info.rank = rank;
 	}
 
+#if defined(CONFIG_RAM_RK3399_LPDDR4)
+	/* LPDDR4 needs to be trained at 400MHz */
+	lpddr4_set_rate(dram, params, 0);
+	params->base.ddr_freq = dfs_cfgs_lpddr4[0].base.ddr_freq / MHz;
+#endif
+
 	params->base.num_channels = 0;
 	for (channel = 0; channel < 2; channel++) {
 		const struct chan_info *chan = &dram->chan[channel];
@@ -3005,7 +3011,7 @@ static int sdram_init(struct dram_info *dram,
 	params->base.stride = calculate_stride(params);
 	dram_all_config(dram, params);
 
-	ret = dram->ops->set_rate_index(dram, params);
+	ret = dram->ops->set_rate_index(dram, params, 1);
 	if (ret)
 		return ret;
 
-- 
2.37.0.rc0.104.g0611611a94-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling
  2022-06-21 10:07 [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling Lee Jones
  2022-06-21 10:07 ` [PATCH 2/3] ram: rk3399: Fix faulty frequency change reports Lee Jones
  2022-06-21 10:07 ` [PATCH 3/3] ram: rk3399: Conduct memory training at 400MHz Lee Jones
@ 2022-06-27  8:39 ` Lee Jones
  2022-07-01 12:05   ` Kever Yang
  2022-07-01 12:04 ` Kever Yang
  2022-07-04  9:57 ` [SPAM] " Xavier Drudis Ferran
  4 siblings, 1 reply; 16+ messages in thread
From: Lee Jones @ 2022-06-27  8:39 UTC (permalink / raw)
  To: sjg, philipp.tomsich, kever.yang; +Cc: cym, u-boot

On Tue, 21 Jun 2022, Lee Jones wrote:

> Functions pointed to by this op pointer can return non-zero values
> indicating an error.  Ensure any error value is propagated back up the
> call-chain.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/ram/rockchip/sdram_rk3399.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Weekly check-in:

Are these still on someone's radar, or would you like me to [RESEND]?

> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index c0a06dcaed..0af0fa9e7b 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -3005,7 +3005,9 @@ static int sdram_init(struct dram_info *dram,
>  	params->base.stride = calculate_stride(params);
>  	dram_all_config(dram, params);
>  
> -	dram->ops->set_rate_index(dram, params);
> +	ret = dram->ops->set_rate_index(dram, params);
> +	if (ret)
> +		return ret;
>  
>  	debug("Finish SDRAM initialization...\n");
>  	return 0;

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling
  2022-06-21 10:07 [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling Lee Jones
                   ` (2 preceding siblings ...)
  2022-06-27  8:39 ` [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling Lee Jones
@ 2022-07-01 12:04 ` Kever Yang
  2022-07-04  9:57 ` [SPAM] " Xavier Drudis Ferran
  4 siblings, 0 replies; 16+ messages in thread
From: Kever Yang @ 2022-07-01 12:04 UTC (permalink / raw)
  To: Lee Jones, sjg, philipp.tomsich; +Cc: cym, u-boot


On 2022/6/21 18:07, Lee Jones wrote:
> Functions pointed to by this op pointer can return non-zero values
> indicating an error.  Ensure any error value is propagated back up the
> call-chain.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   drivers/ram/rockchip/sdram_rk3399.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index c0a06dcaed..0af0fa9e7b 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -3005,7 +3005,9 @@ static int sdram_init(struct dram_info *dram,
>   	params->base.stride = calculate_stride(params);
>   	dram_all_config(dram, params);
>   
> -	dram->ops->set_rate_index(dram, params);
> +	ret = dram->ops->set_rate_index(dram, params);
> +	if (ret)
> +		return ret;
>   
>   	debug("Finish SDRAM initialization...\n");
>   	return 0;

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] ram: rk3399: Fix faulty frequency change reports
  2022-06-21 10:07 ` [PATCH 2/3] ram: rk3399: Fix faulty frequency change reports Lee Jones
@ 2022-07-01 12:04   ` Kever Yang
  2022-07-04 10:00   ` Xavier Drudis Ferran
  1 sibling, 0 replies; 16+ messages in thread
From: Kever Yang @ 2022-07-01 12:04 UTC (permalink / raw)
  To: Lee Jones, sjg, philipp.tomsich; +Cc: cym, u-boot


On 2022/6/21 18:07, Lee Jones wrote:
> Frequency changes to 400MHz are presently reported as:
>
>    lpddr4_set_rate_0: change freq to 400000000 mhz 0, 1
>
> This is obviously wrong by 6 orders of magnitude.
>
> Ensure frequency changes are reported accurately.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   drivers/ram/rockchip/sdram_rk3399.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index 0af0fa9e7b..34d6c93f95 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -2552,8 +2552,8 @@ static int lpddr4_set_rate(struct dram_info *dram,
>   			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
>   
>   		if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
> -			printf("%s: change freq to %d mhz %d, %d\n", __func__,
> -			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq,
> +			printf("%s: change freq to %dMHz %d, %d\n", __func__,
> +			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
>   			       ctl_fn, phy_fn);
>   	}
>   

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] ram: rk3399: Conduct memory training at 400MHz
  2022-06-21 10:07 ` [PATCH 3/3] ram: rk3399: Conduct memory training at 400MHz Lee Jones
@ 2022-07-01 12:05   ` Kever Yang
  2022-07-04 10:01   ` Xavier Drudis Ferran
  1 sibling, 0 replies; 16+ messages in thread
From: Kever Yang @ 2022-07-01 12:05 UTC (permalink / raw)
  To: Lee Jones, sjg, philipp.tomsich; +Cc: cym, u-boot


On 2022/6/21 18:07, Lee Jones wrote:
> Currently the default initialisation frequency is 50MHz.  Although
> this does appear to be suitable for some LPDDR4 RAM chips, training at
> this low frequency has been seen to cause Column errors, leading to
> Capacity check errors on others.
>
> Here we force RAM initialisation to happen at 400MHz before ramping up
> to the final value running value of 800MHz after everything has been
> successfully configured.
>
> Link: https://lore.kernel.org/u-boot/Yo4v3jUeHXTovjOH@google.com/
> Suggested-by: YouMin Chen <cym@rock-chips.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   drivers/ram/rockchip/sdram_rk3399.c | 36 +++++++++++++++++------------
>   1 file changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index 34d6c93f95..b05c5925d5 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -85,7 +85,7 @@ struct sdram_rk3399_ops {
>   	int (*data_training_first)(struct dram_info *dram, u32 channel, u8 rank,
>   				   struct rk3399_sdram_params *sdram);
>   	int (*set_rate_index)(struct dram_info *dram,
> -			      struct rk3399_sdram_params *params);
> +			      struct rk3399_sdram_params *params, u32 ctl_fn);
>   	void (*modify_param)(const struct chan_info *chan,
>   			     struct rk3399_sdram_params *params);
>   	struct rk3399_sdram_params *
> @@ -1644,7 +1644,8 @@ static int data_training_first(struct dram_info *dram, u32 channel, u8 rank,
>   }
>   
>   static int switch_to_phy_index1(struct dram_info *dram,
> -				struct rk3399_sdram_params *params)
> +				struct rk3399_sdram_params *params,
> +				u32 unused)
>   {
>   	u32 channel;
>   	u32 *denali_phy;
> @@ -2539,26 +2540,25 @@ static int lpddr4_set_ctl(struct dram_info *dram,
>   }
>   
>   static int lpddr4_set_rate(struct dram_info *dram,
> -			   struct rk3399_sdram_params *params)
> +			    struct rk3399_sdram_params *params,
> +			    u32 ctl_fn)
>   {
> -	u32 ctl_fn;
>   	u32 phy_fn;
>   
> -	for (ctl_fn = 0; ctl_fn < 2; ctl_fn++) {
> -		phy_fn = lpddr4_get_phy_fn(params, ctl_fn);
> +	phy_fn = lpddr4_get_phy_fn(params, ctl_fn);
>   
> -		lpddr4_set_phy(dram, params, phy_fn, &dfs_cfgs_lpddr4[ctl_fn]);
> -		lpddr4_set_ctl(dram, params, ctl_fn,
> -			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
> +	lpddr4_set_phy(dram, params, phy_fn, &dfs_cfgs_lpddr4[ctl_fn]);
> +	lpddr4_set_ctl(dram, params, ctl_fn,
> +		       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
>   
> -		if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
> -			printf("%s: change freq to %dMHz %d, %d\n", __func__,
> -			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
> -			       ctl_fn, phy_fn);
> -	}
> +	if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
> +		printf("%s: change freq to %dMHz %d, %d\n", __func__,
> +		       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
> +		       ctl_fn, phy_fn);
>   
>   	return 0;
>   }
> +
>   #endif /* CONFIG_RAM_RK3399_LPDDR4 */
>   
>   /* CS0,n=1
> @@ -2955,6 +2955,12 @@ static int sdram_init(struct dram_info *dram,
>   		params->ch[ch].cap_info.rank = rank;
>   	}
>   
> +#if defined(CONFIG_RAM_RK3399_LPDDR4)
> +	/* LPDDR4 needs to be trained at 400MHz */
> +	lpddr4_set_rate(dram, params, 0);
> +	params->base.ddr_freq = dfs_cfgs_lpddr4[0].base.ddr_freq / MHz;
> +#endif
> +
>   	params->base.num_channels = 0;
>   	for (channel = 0; channel < 2; channel++) {
>   		const struct chan_info *chan = &dram->chan[channel];
> @@ -3005,7 +3011,7 @@ static int sdram_init(struct dram_info *dram,
>   	params->base.stride = calculate_stride(params);
>   	dram_all_config(dram, params);
>   
> -	ret = dram->ops->set_rate_index(dram, params);
> +	ret = dram->ops->set_rate_index(dram, params, 1);
>   	if (ret)
>   		return ret;
>   

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling
  2022-06-27  8:39 ` [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling Lee Jones
@ 2022-07-01 12:05   ` Kever Yang
  2022-07-04  9:23     ` Lee Jones
  0 siblings, 1 reply; 16+ messages in thread
From: Kever Yang @ 2022-07-01 12:05 UTC (permalink / raw)
  To: Lee Jones, sjg, philipp.tomsich; +Cc: cym, u-boot

Hi Lee Jones,


On 2022/6/27 16:39, Lee Jones wrote:
> On Tue, 21 Jun 2022, Lee Jones wrote:
>
>> Functions pointed to by this op pointer can return non-zero values
>> indicating an error.  Ensure any error value is propagated back up the
>> call-chain.
>>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>> ---
>>   drivers/ram/rockchip/sdram_rk3399.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
> Weekly check-in:
>
> Are these still on someone's radar, or would you like me to [RESEND]?

It would be better to add leading message "rockchip:" in the subject.


Thanks,

- Kever

>> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
>> index c0a06dcaed..0af0fa9e7b 100644
>> --- a/drivers/ram/rockchip/sdram_rk3399.c
>> +++ b/drivers/ram/rockchip/sdram_rk3399.c
>> @@ -3005,7 +3005,9 @@ static int sdram_init(struct dram_info *dram,
>>   	params->base.stride = calculate_stride(params);
>>   	dram_all_config(dram, params);
>>   
>> -	dram->ops->set_rate_index(dram, params);
>> +	ret = dram->ops->set_rate_index(dram, params);
>> +	if (ret)
>> +		return ret;
>>   
>>   	debug("Finish SDRAM initialization...\n");
>>   	return 0;

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling
  2022-07-01 12:05   ` Kever Yang
@ 2022-07-04  9:23     ` Lee Jones
  2022-07-04 10:49       ` Kever Yang
  0 siblings, 1 reply; 16+ messages in thread
From: Lee Jones @ 2022-07-04  9:23 UTC (permalink / raw)
  To: Kever Yang; +Cc: sjg, philipp.tomsich, cym, u-boot

On Fri, 01 Jul 2022, Kever Yang wrote:

> Hi Lee Jones,
> 
> 
> On 2022/6/27 16:39, Lee Jones wrote:
> > On Tue, 21 Jun 2022, Lee Jones wrote:
> > 
> > > Functions pointed to by this op pointer can return non-zero values
> > > indicating an error.  Ensure any error value is propagated back up the
> > > call-chain.
> > > 
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > >   drivers/ram/rockchip/sdram_rk3399.c | 4 +++-
> > >   1 file changed, 3 insertions(+), 1 deletion(-)
> > Weekly check-in:
> > 
> > Are these still on someone's radar, or would you like me to [RESEND]?
> 
> It would be better to add leading message "rockchip:" in the subject.

That would be highly unusual.

Patch subjects are usually formed, as follows:

  "<subsystem>: <device>: <description>"

I only see 2 RAM-only patches (out of the 91 that touch this file)
that do as you suggested and you were the author of both of them. :)

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [SPAM] [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling
  2022-06-21 10:07 [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling Lee Jones
                   ` (3 preceding siblings ...)
  2022-07-01 12:04 ` Kever Yang
@ 2022-07-04  9:57 ` Xavier Drudis Ferran
  4 siblings, 0 replies; 16+ messages in thread
From: Xavier Drudis Ferran @ 2022-07-04  9:57 UTC (permalink / raw)
  To: Lee Jones; +Cc: sjg, philipp.tomsich, kever.yang, cym, u-boot

El Tue, Jun 21, 2022 at 10:07:27AM +0000, Lee Jones deia:
> Functions pointed to by this op pointer can return non-zero values
> indicating an error.  Ensure any error value is propagated back up the
> call-chain.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>


My board doesn't suffer with the issue resolved by this series,
however I did apply it and nothing regressed.

Tested-by: Xavier Drudis Ferran <xdrudis@tinet.cat>

> ---
>  drivers/ram/rockchip/sdram_rk3399.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index c0a06dcaed..0af0fa9e7b 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -3005,7 +3005,9 @@ static int sdram_init(struct dram_info *dram,
>  	params->base.stride = calculate_stride(params);
>  	dram_all_config(dram, params);
>  
> -	dram->ops->set_rate_index(dram, params);
> +	ret = dram->ops->set_rate_index(dram, params);
> +	if (ret)
> +		return ret;
>  
>  	debug("Finish SDRAM initialization...\n");
>  	return 0;
> -- 
> 2.37.0.rc0.104.g0611611a94-goog
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] ram: rk3399: Fix faulty frequency change reports
  2022-06-21 10:07 ` [PATCH 2/3] ram: rk3399: Fix faulty frequency change reports Lee Jones
  2022-07-01 12:04   ` Kever Yang
@ 2022-07-04 10:00   ` Xavier Drudis Ferran
  1 sibling, 0 replies; 16+ messages in thread
From: Xavier Drudis Ferran @ 2022-07-04 10:00 UTC (permalink / raw)
  To: Lee Jones; +Cc: sjg, philipp.tomsich, kever.yang, cym, u-boot

El Tue, Jun 21, 2022 at 10:07:28AM +0000, Lee Jones deia:
> Frequency changes to 400MHz are presently reported as:
> 
>   lpddr4_set_rate_0: change freq to 400000000 mhz 0, 1
> 
> This is obviously wrong by 6 orders of magnitude.
> 
> Ensure frequency changes are reported accurately.
>

Not obvious to me what "mhz" means (mHz? MHz? Hz?)
and so not obvious to me by how many orders of magnitude 
it was wrong but in any case now the message is correct and clear.

Tested-by: Xavier Drudis Ferran <xdrudis@tinet.cat>
 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/ram/rockchip/sdram_rk3399.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index 0af0fa9e7b..34d6c93f95 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -2552,8 +2552,8 @@ static int lpddr4_set_rate(struct dram_info *dram,
>  			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
>  
>  		if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
> -			printf("%s: change freq to %d mhz %d, %d\n", __func__,
> -			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq,
> +			printf("%s: change freq to %dMHz %d, %d\n", __func__,
> +			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
>  			       ctl_fn, phy_fn);
>  	}
>  
> -- 
> 2.37.0.rc0.104.g0611611a94-goog
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] ram: rk3399: Conduct memory training at 400MHz
  2022-06-21 10:07 ` [PATCH 3/3] ram: rk3399: Conduct memory training at 400MHz Lee Jones
  2022-07-01 12:05   ` Kever Yang
@ 2022-07-04 10:01   ` Xavier Drudis Ferran
  1 sibling, 0 replies; 16+ messages in thread
From: Xavier Drudis Ferran @ 2022-07-04 10:01 UTC (permalink / raw)
  To: Lee Jones; +Cc: sjg, philipp.tomsich, kever.yang, cym, u-boot

El Tue, Jun 21, 2022 at 10:07:29AM +0000, Lee Jones deia:
> Currently the default initialisation frequency is 50MHz.  Although
> this does appear to be suitable for some LPDDR4 RAM chips, training at
> this low frequency has been seen to cause Column errors, leading to
> Capacity check errors on others.
> 
> Here we force RAM initialisation to happen at 400MHz before ramping up
> to the final value running value of 800MHz after everything has been
> successfully configured.
> 
> Link: https://lore.kernel.org/u-boot/Yo4v3jUeHXTovjOH@google.com/
> Suggested-by: YouMin Chen <cym@rock-chips.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

My board doesn't suffer with the issue resolved by this series,
however I did apply it and nothing regressed.

Tested-by: Xavier Drudis Ferran <xdrudis@tinet.cat>

> ---
>  drivers/ram/rockchip/sdram_rk3399.c | 36 +++++++++++++++++------------
>  1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index 34d6c93f95..b05c5925d5 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -85,7 +85,7 @@ struct sdram_rk3399_ops {
>  	int (*data_training_first)(struct dram_info *dram, u32 channel, u8 rank,
>  				   struct rk3399_sdram_params *sdram);
>  	int (*set_rate_index)(struct dram_info *dram,
> -			      struct rk3399_sdram_params *params);
> +			      struct rk3399_sdram_params *params, u32 ctl_fn);
>  	void (*modify_param)(const struct chan_info *chan,
>  			     struct rk3399_sdram_params *params);
>  	struct rk3399_sdram_params *
> @@ -1644,7 +1644,8 @@ static int data_training_first(struct dram_info *dram, u32 channel, u8 rank,
>  }
>  
>  static int switch_to_phy_index1(struct dram_info *dram,
> -				struct rk3399_sdram_params *params)
> +				struct rk3399_sdram_params *params,
> +				u32 unused)
>  {
>  	u32 channel;
>  	u32 *denali_phy;
> @@ -2539,26 +2540,25 @@ static int lpddr4_set_ctl(struct dram_info *dram,
>  }
>  
>  static int lpddr4_set_rate(struct dram_info *dram,
> -			   struct rk3399_sdram_params *params)
> +			    struct rk3399_sdram_params *params,
> +			    u32 ctl_fn)
>  {
> -	u32 ctl_fn;
>  	u32 phy_fn;
>  
> -	for (ctl_fn = 0; ctl_fn < 2; ctl_fn++) {
> -		phy_fn = lpddr4_get_phy_fn(params, ctl_fn);
> +	phy_fn = lpddr4_get_phy_fn(params, ctl_fn);
>  
> -		lpddr4_set_phy(dram, params, phy_fn, &dfs_cfgs_lpddr4[ctl_fn]);
> -		lpddr4_set_ctl(dram, params, ctl_fn,
> -			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
> +	lpddr4_set_phy(dram, params, phy_fn, &dfs_cfgs_lpddr4[ctl_fn]);
> +	lpddr4_set_ctl(dram, params, ctl_fn,
> +		       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
>  
> -		if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
> -			printf("%s: change freq to %dMHz %d, %d\n", __func__,
> -			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
> -			       ctl_fn, phy_fn);
> -	}
> +	if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
> +		printf("%s: change freq to %dMHz %d, %d\n", __func__,
> +		       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
> +		       ctl_fn, phy_fn);
>  
>  	return 0;
>  }
> +
>  #endif /* CONFIG_RAM_RK3399_LPDDR4 */
>  
>  /* CS0,n=1
> @@ -2955,6 +2955,12 @@ static int sdram_init(struct dram_info *dram,
>  		params->ch[ch].cap_info.rank = rank;
>  	}
>  
> +#if defined(CONFIG_RAM_RK3399_LPDDR4)
> +	/* LPDDR4 needs to be trained at 400MHz */
> +	lpddr4_set_rate(dram, params, 0);
> +	params->base.ddr_freq = dfs_cfgs_lpddr4[0].base.ddr_freq / MHz;
> +#endif
> +
>  	params->base.num_channels = 0;
>  	for (channel = 0; channel < 2; channel++) {
>  		const struct chan_info *chan = &dram->chan[channel];
> @@ -3005,7 +3011,7 @@ static int sdram_init(struct dram_info *dram,
>  	params->base.stride = calculate_stride(params);
>  	dram_all_config(dram, params);
>  
> -	ret = dram->ops->set_rate_index(dram, params);
> +	ret = dram->ops->set_rate_index(dram, params, 1);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.37.0.rc0.104.g0611611a94-goog
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling
  2022-07-04  9:23     ` Lee Jones
@ 2022-07-04 10:49       ` Kever Yang
  2022-07-04 10:58         ` Lee Jones
  0 siblings, 1 reply; 16+ messages in thread
From: Kever Yang @ 2022-07-04 10:49 UTC (permalink / raw)
  To: Lee Jones; +Cc: sjg, philipp.tomsich, cym, u-boot

Hi

On 2022/7/4 17:23, Lee Jones wrote:
> On Fri, 01 Jul 2022, Kever Yang wrote:
>
>> Hi Lee Jones,
>>
>>
>> On 2022/6/27 16:39, Lee Jones wrote:
>>> On Tue, 21 Jun 2022, Lee Jones wrote:
>>>
>>>> Functions pointed to by this op pointer can return non-zero values
>>>> indicating an error.  Ensure any error value is propagated back up the
>>>> call-chain.
>>>>
>>>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>>> ---
>>>>    drivers/ram/rockchip/sdram_rk3399.c | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>> Weekly check-in:
>>>
>>> Are these still on someone's radar, or would you like me to [RESEND]?
>> It would be better to add leading message "rockchip:" in the subject.
> That would be highly unusual.
>
> Patch subjects are usually formed, as follows:

You should able to see many patches with "rockchip:" in U-Boot commit, 
and also many patches

with other platform prefix like "imx:" or "sunxi:".

Not like kernel, U-Boot using the same mailing list for all soc 
platform, so this can help easier

to identify which soc vendor it belongs to.


Thanks,

- Kever

>
>    "<subsystem>: <device>: <description>"
>
> I only see 2 RAM-only patches (out of the 91 that touch this file)
> that do as you suggested and you were the author of both of them. :)
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling
  2022-07-04 10:49       ` Kever Yang
@ 2022-07-04 10:58         ` Lee Jones
  0 siblings, 0 replies; 16+ messages in thread
From: Lee Jones @ 2022-07-04 10:58 UTC (permalink / raw)
  To: Kever Yang; +Cc: sjg, philipp.tomsich, cym, u-boot

On Mon, 04 Jul 2022, Kever Yang wrote:

> Hi
> 
> On 2022/7/4 17:23, Lee Jones wrote:
> > On Fri, 01 Jul 2022, Kever Yang wrote:
> > 
> > > Hi Lee Jones,
> > > 
> > > 
> > > On 2022/6/27 16:39, Lee Jones wrote:
> > > > On Tue, 21 Jun 2022, Lee Jones wrote:
> > > > 
> > > > > Functions pointed to by this op pointer can return non-zero values
> > > > > indicating an error.  Ensure any error value is propagated back up the
> > > > > call-chain.
> > > > > 
> > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > > > ---
> > > > >    drivers/ram/rockchip/sdram_rk3399.c | 4 +++-
> > > > >    1 file changed, 3 insertions(+), 1 deletion(-)
> > > > Weekly check-in:
> > > > 
> > > > Are these still on someone's radar, or would you like me to [RESEND]?
> > > It would be better to add leading message "rockchip:" in the subject.
> > That would be highly unusual.
> > 
> > Patch subjects are usually formed, as follows:
> 
> You should able to see many patches with "rockchip:" in U-Boot commit, and
> also many patches
> 
> with other platform prefix like "imx:" or "sunxi:".
> 
> Not like kernel, U-Boot using the same mailing list for all soc platform, so
> this can help easier
> 
> to identify which soc vendor it belongs to.

I don't see that in the history:

$ git log --oneline -- drivers/ram/
424b3570ef (HEAD -> refs/heads/tb-aosp-lpddr4-ram-train-at-400mhz) ram: rk3399: Conduct memory training at 400MHz
3e28ebd877 ram: rk3399: Fix faulty frequency change reports
647d52dd86 ram: rk3399: Fix .set_rate_index() error handling
f42045b2e7 stm32mp15: replace CONFIG_TFABOOT when it is possible
d36c94279d ram: sifive: Fix -Wint-to-pointer-cast warnings
0cf207ec01 WS cleanup: remove SPACE(s) followed by TAB
0a50b3c97b WS cleanup: remove trailing white space
66356b4c06 WS cleanup: remove trailing empty lines
f0ab8f9fbe clk: Rename clk_get_by_driver_info()
dcfc42b12f treewide: Try to avoid the preprocessor with OF_REAL
9539738509 treewide: Use OF_REAL instead of !OF_PLATDATA
b4c2c151b1 Kconfig: Remove all default n/no options
0b1284eb52 global: Convert simple_strtoul() with decimal to dectoul()
b953ec2bca dm: define LOG_CATEGORY for all uclass
2d46775287 rockchip: rk3568: Add sdram driver
dab18c7aa6 drivers: ram: sifive: rename fu540_ddr and add fu740 support
2ce6dedf0b ram: k3-ddrss: Enable vtt regulator if present
9f9b5c1c16 ram: k3-ddrss: Introduce support for AM642 SoCs
a8c13c777e ram: k3-ddrss: Introduce common driver with J7 SoC support
db2438131d ram: k3-ddrss: Introduce top-level CONFIG_K3_DDRSS
67124b9a74 ram: k3-j721e: Rename to k3-ddrss
036f0c0b66 ram: k3-j721e: lpddr4_ctl_regs: Fix checkpatch issue for types
cc40e4d947 ram: k3-j721e: lpddr4_pi_macros: Fix indentation issues
cde1fcee3e ram: k3-j721e: lpddr4_phy_core_macros: Fix indentation issues
0ef6349326 ram: k3-j721e: lpddr4_ddr_controller_macros: Fix indentation issues
6a0677d0a0 ram: k3-j721e: lpddr4_data_slice_3_macros: Fix indentation issues
f1ce7dd92d ram: k3-j721e: lpddr4_data_slice_2_macros: Fix indentation issues
c1cf7a3d3a ram: k3-j721e: lpddr4_data_slice_1_macros: Fix indentation issues
5bf74a4884 ram: k3-j721e: lpddr4_data_slice_0_macros: Fix indentation issues
6da67b081f ram: k3-j721e: lpddr4_address_slice_0_macros: Fix indentation issues
b0f4ba0242 mips: octeon: Misc changes required because of the newly added headers
3f2e3c7845 Merge tag 'u-boot-stm32-20210409' of https://source.denx.de/u-boot/custodians/u-boot-stm
1f0305e0d0 ram: stm32: fix strsep failed on read only memory
ae2d9506a3 riscv: sifive: Rename fu540 board to unleashed
401d1c4f5d common: Drop asm/global_data.h from common header
fde9314346 ram: aspeed: Add AST2600 DRAM control support
f5abd8a616 ram: k3-j721e: rename BIT_MASK()
66b3b9db69 ram: stm32mp1: migrate trace to dev or log macro
997f7dab9d ram: stm32: migrate trace to log macro
8a8d24bdf1 dm: treewide: Rename ..._platdata variables to just ..._plat
d1998a9fde dm: treewide: Rename ofdata_to_platdata() to of_to_plat()
c69cda25c9 dm: treewide: Rename dev_get_platdata() to dev_get_plat()
caa4daa2ae dm: treewide: Rename 'platdata' variables to just 'plat'
41575d8e4c dm: treewide: Rename auto_alloc_size members to be shorter
2c31d7e746 Merge tag 'u-boot-rockchip-20201031' of https://gitlab.denx.de/u-boot/custodians/u-boot-rockchip
2db36c64bd ram: rockchip: px30: add a config-based ddr selection
c670aeee3d common: rename getc() to getchar()
8d4c596644 ram: imxrt: Include device_compat.h
0474050d46 ram: add ddr4 dual x8 configuration
5d457f8057 ram: move aspeed ram driver into drivers/ directory
15afe725f3 ram: octeon: Add MIPS Octeon3 DDR4 support (part 3/3)
61674a17bc ram: octeon: Add MIPS Octeon3 DDR4 support (part 2/3)
e13bb86588 ram: octeon: Add MIPS Octeon3 DDR4 support (part 1/3)
9981a8009e ram: sifive: Remove regmap dependency
f8c9660bfe ram: sifive: Check return value on clk_enable()
3ab2601052 ram: sifive: Fix compiler warnings for 32-bit
6e802ef540 ram: k3-j721e: Relax version checks for memory controller
0eddd24e89 ti: am654: Drop duplicate dm.h inclusion
ecb70bdb9f ram: sifive: Avoid using hardcoded ram base and size
b32858ca51 rockchip: ram: fix debug funcfion define when RAM_ROCKCHIP_DEBUG not set
40794c825f ram: rk3399: Mark existing prints via RAM_ROCKCHIP_DEBUG
304eaae36b ram: rk3399: Drop debug stride in driver
51f1263d89 dtoc: extend dtoc to use struct driver_info when linking nodes
e3e2470fdd drivers: rename drivers to match compatible string
6c393e8c0f ram: stm32mp1: add size and addr parameter to test all
81b66b9033 ram: stm32mp1: use the DDR size by default in the test addressBus
fcd4890829 ram: stm32mp1: add parameter addr in test FrequencySelectivePattern
1a5be5a416 ram: stm32mp1: protect minimum value in get_bufsize
c514a94abf sifive: fu540: add ddr driver
cd93d625fd common: Drop linux/bitops.h from common header
c05ed00afb common: Drop linux/delay.h from common header
f7ae49fc4f common: Drop log.h from common header
0914011310 command: Remove the cmd_tbl_t typedef
691d719db7 common: Drop init.h from common header
90526e9fba common: Drop net.h from common header
0c27c16495 ram: stm32mp1: Add support for multiple configs
654706be84 configs: stm32mp1: replace STM32MP1_TRUSTED by TFABOOT
9368bdfebd ram: stm32mp1: the property st, phy-cal becomes optional
d424e6786f ram: stm32mp1: reduce delay after BIST reset for tuning
b604a41c6b ram: stm32mp1_ddr: fix self refresh disable during DQS training
8c9ce08075 ram: stm32mp1: update BIST config for tuning
27e7b4edea ram: stm32mp1: tuning: deactivate derating during BIST test
f711d1f080 ram: stm32mp1: tuning: add timeout for polling BISTGSR.BDDONE
1c55a91b9d ram: stm32mp1: don't display the prompt two times
c8eb4e038c ram: stm32mp1: display result for software read DQS gating
e9a20f8a19 ram: stm32mp1: increase vdd2_ddr: buck2 for 32bits LPDDR
e3dc5924ca ram: rockchip: Fix Kconfig dependency for RAM_ROCKCHIP_DEBUG
f217651575 dm: core: Drop the inclusion of linux/compat.h in dm.h
336d4615f8 dm: core: Create a new header file for 'compat' features
61b29b8268 dm: core: Require users of devres to include the header
31531f6fdb ram: rk3328: only do data traning for cs0
b52a199e32 arm: rockchip: Add common cru.h
95052b4b40 ram: rk3399: don't assume phy_io_config() uses real regs
db41d65a97 common: Move hang() to the same header as panic()
9b4a205f45 common: Move RAM-sizing functions to init.h
cd647fc4fb ram: add SDRAM driver for i.MXRT SoCs
54069939be rockchip: rk3308: Add sdram driver
bcfacab517 ram: rk3399: Fix dram setting to make dram more stable
c8863b8508 ram: rk3399: update calculate_stride
da53f0641b ram: rk3399: Sync the io setting from Rockchip vendor code
7cf04ad1f6 ram: rockchip: update lpddr4 timing for rk3399
f2b58f0749 ram: rk3399: add support detect capacity
0cacc27569 ram: rk3399: update the function of sdram_init
410d7863bc ram: rk3399: fix error about get_ddrc0_con reg addr
f8088bfc85 ram: rk3399: Clean up code
a922d0d102 ram: rk3399: migrate to use common code
ca93e32139 ram: rk3328: use common sdram driver
39edfaa758 ram: px30: add sdram driver
691368c7f7 ram: rockchip: add phy driver code for PX30
09d7872336 ram: rockchip: add controller code for PX30
fba7bd4c34 ram: rockchip: Default enable DRAM debug info
ec0d29aefa ram: rockchip: move sdram_debug function into sdram_common
d6647b08b2 ram: rockchip: add common code for sdram driver
5d19ddf0db ram: rockchip: rename sdram_common.c/h to sdram.c
2a2f0b177c ram: rockchip: rename sdram.h to sdram_rk3288.h
c4b9d66f11 ram: rk3328: Fix loading of skew values
18c24c1177 ram: rk3328: Use correct frequency units in function
3bb3f266ee ram: k3-j721e: Add support for J721E DDR controller
34f27b2e86 ram: k3-am654: Do not rely on default values for certain DDR register
c78ac7a0c9 ram: k3-am654: add support for LPDDR4 and DDR3L DDRs
4f24163efa ram: rk3288: Initialize dram for TPL builds
757bca8d19 stm32mp1: ram: add pattern parameter in infinite write test
25331ae1c1 stm32mp1: ram: reload watchdog during ddr test
37f41ae900 stm32mp1: ram: update loop management in infinite test
4b0496fe79 stm32mp1: ram: fix address issue in 2 tests
375c28ac76 stm32mp1: ram: cosmetic: remove unused prototype
e0f907efa5 ram: rk3399: update cap and ddrconfig for each channel after init
85a38742e0 rockchip: ram: add full feature rk3328 DRAM driver
f3d689c0e0 rockchip: rk322x: sdram: use udelay instead of rockchip_udelay
c36abd087a ram: rk3399: Add lpddr4 set rate support
1dd1cb6253 ram: rk3399: Add set_rate sdram rk3399 ops
dd2c633b2a ram: rk3399: Add LPPDDR4-800 timings inc
4f3cc17d38 ram: rk3399: Add LPPDDR4-400 timings inc
a0ded6d317 ram: rk3399: Add LPPDR4 mr detection
299deecf4a ram: rk3399: Handle data training via ops
e6ae37a007 ram: rk3399: Simplify data training first argument
e939f92eae ram: rk3399: Update lpddr4 vref_mode_ac
274c33737b ram: rk3399: Update lpddr4 mode_sel based on io settings
95be76eb5c ram: rk3399: Update lpddr4 vref based on io settings
4eceda01d5 ram: rk3399: Get lpddr4 tsel_rd_en from io settings
f288d54936 ram: rk3399: Configure soc odt support
aa30aae8b4 ram: rk3399: Add tsel control clock drive
2fb2de33b2 ram: sdram: Configure lpddr4 tsel rd, wr based on IO settings
74109de3c2 ram: rk3399: Add IO settings
740409804e ram: rk3399: Don't disable dfi dram clk for lpddr4, rank 1
66912baa0f ram: rk3399: Configure tsel write ca for lpddr4
4e9de9eba8 ram: rk3399: Map chipselect for lpddr4
d3d0099ca6 ram: rk3399: Configure PHY RX_CM_INPUT for lpddr4
f9f32d61a6 ram: rk3399: Configure SLEWP_EN, SLEWN_EN for lpddr4
881860fd34 ram: rk3399: Configure BOOSTP_EN, BOOSTN_EN for lpddr4
009fe1bac9 ram: rk3399: Configure PHY_898, PHY_919 for lpddr4
47627c8a5c ram: rk3399: Avoid two channel ZQ Cal Start at the same time
5cbc866981 ram: rk3399: Don't wait for PLL lock in lpddr4
6cbd2426b3 ram: rk3399: Move mode_sel assignment
c716bf67f5 ram: rk3399: Add lpddr4 rank mask for wdql training
3dae87da89 ram: rk3399: Add lpddr4 rank mask for ca training
b6cf08949d ram: rockchip: Kconfig: Add RK3399 LPDDR4 entry
ba607fafd1 ram: rk3399: Configure phy IO in ds odt
a735550bb8 ram: rk3399: Add DdrMode
ed77ce728a ram: rk3399: Add ddrtimingC0
b713e0291b ram: rk3399: Add ddr version enc macro
01cc103915 ram: rk3399: Introduce sys_reg3 for more capacity info
e0ddb0ba21 ram: rk3399: Rename sys_reg with sys_reg2
879f9fed6a ram: rk3399: Simply existing dram enc macro
a9191b8eec ram: rk3399: Enable sdram debug functions
d0ba88f5dd ram: rk3399: Add rank detection support
1ff5283d92 ram: rk3399: Compute stride for 1 channel a
4b09719c38 ram: rk3399: Compute stride for 2 channels
cb13534abe ram: rk3399: debug: Add sdram_print_stride
79674a6278 ram: rockchip: debug: Get the cs capacity
07894f5aac ram: rockchip: debug: Add sdram_print_ddr_info
07112672a5 ram: rockchip: Add debug sdram driver
82ee138def ram: rockchip: Add initial Kconfig
a0aebe8398 ram: rk3399: Add pctl start support
fe42d4a199 ram: rk3399: Move pwrup_srefresh_exit to dram_info
33921035be ram: rk3399: Add phy pctrl reset support
21cf392b1f ram: rk3399: Use rank mask in wdql data training
708e9a79dc ram: rk3399: Use rank mask in ca data training
01976ae6f5 ram: rk3399: Clear PI_175 interrupts in data training
02fad6f9ed ram: rk3399: Handle data training return types
355490dc5c ram: rockchip: rk3399: Add cap_info structure
9c4d517db8 ram: rk3399: Order tsel variables
30bd86a399 ram: rk3399: s/ca_tsel_wr_select_p/tsel_wr_select_ca_p
a12a5be7a3 ram: rk3399: s/ca_tsel_wr_select_n/tsel_wr_select_ca_n
a5085ee4e8 ram: rk3399: s/tsel_wr_select_p/tsel_wr_select_dq_p
fa2b015b9c ram: rk3399: s/tsel_wr_select_n/tsel_wr_select_dq_n
d4b4bb47c6 ram: rk3399: Handle pctl_cfg return type
fde7f457e1 ram: rk3399: s/sdram_params/params
3eaf539849 ram: rk3399: Some trivial code fixes
63f4d716b1 ram: rk3399: Fix code warnings
588448517f ram: stm32mp1_ram: Fix warnings when compiling with W=1
187c41d783 stm32mp1: ram: add tuning in DDR interactive mode
0d44752442 stm32mp1: ram: add tests in DDR interactive mode
01a7510849 stm32mp1: ram: add interactive mode for DDR configuration
1767ac2d1f stm32mp1: ram: add support for LPDDR2/LPDDR3
53bb831658 stm32mp1: ram: update parameter array initialization
c60fed14f6 stm32mp1: ram: change ddr speed to kHz
0cb1aa9409 stm32mp1: ram: increase the delay after reset to 128 cycles
c3ec370aed stm32mp1: ram: update mask for operating mode
8439e99ddb mpc83xx: Introduce ARCH_MPC837X
61abced70f mpc83xx: Introduce ARCH_MPC836*
9403fc41c7 mpc83xx: Introduce ARCH_MPC831*
4bc97a3b81 mpc83xx: Introduce ARCH_MPC830*
82763349a2 rockchip: ram: rk3399: update for TPL
99a1a5b195 rockchip: dmc: rk3368: update rank number for evb-px5
2f52378736 Revert "rockchip: rk322x: ram: enable DRAM init in SPL instead of TPL"
15f09a1a83 rockchip: use 'arch-rockchip' as header file path
abf2678f0f stm32mp1: add trusted boot with TF-A
c43acfdc24 rockchip: ram: update license for sdram driver
f338cca1d2 rockchip: rk322x: ram: enable DRAM init in SPL instead of TPL
60f633efd5 ram: MediaTek: add DDR3 driver for MT7629 SoC
06bda1259f ram: Introduce K3 AM654 DDR Sub System driver
05e424818b ram: bmips: Remove DM_FLAG_PRE_RELOC flag
22929e1266 drivers: cosmetic: Convert SPDX license tags to Linux Kernel style
e40615565d ram: Add driver for MPC83xx
3e4a68d32b bmips: ram: add an option to force the size of the ram
13a7bfe490 ram: bmips: convert to use live dt
246a5e5fc2 ram: stm32_sdram: Adds stm32f429-disco fixes for HardFault at booting
d35812368a regmap: change regmap_init_mem() to take ofnode instead udevice
4549e789c1 SPDX: Convert all of our multiple license tags to Linux Kernel style
83d290c56f SPDX: Convert all of our single license tags to Linux Kernel style
d024236e5a Remove unnecessary instances of DECLARE_GLOBAL_DATA_PTR
2ebc80e83c driver: ram: rockchip: rk3399: missing counter increment
e70f70aa65 ram: stm32mp1: add driver
a80bf5e46e dm: ram: bmips: add BCM6318 support
0b3f789ad1 ram: stm32: add memory mapping selection support
b7aef28953 rockchip: rk3128: add sdram driver
3bc599c956 stm32: fix STMicroelectronics copyright
9b643e312d treewide: replace with error() with pr_err()
a27290a6f8 rockchip: rk3188: ram: add support for 16bit row address
0176399b79 rockchip: rk322x: add sdram driver
f0768491db rockchip: rk3328: move sdram driver to driver/ram
c9eb7bca4b rockchip: rk3288: move sdram driver to driver/ram
5b67d7010b rockchip: rk3188: move sdram driver to driver/ram
b5934cf67c rockchip: rk3399: move sdram driver to driver/ram
1d70f0ac88 rockchip: rk3368: adjust DMC driver for 32/64bit-aware OF_PLATDATA
93fd5b0ac1 ram: kconfig: s/SPL/TPL/ in TPL_RAM help text
403e9cbcd5 rockchip: rk3368: add DRAM controller driver with DRAM initialisation
c336c3c35f spl: dm: Kconfig: introduce TPL_RAM (in analogy to SPL_RAM)
45233301b6 spl: dm: Kconfig: SPL_RAM depends on SPL_DM
7016651eac ram: stm32: add stm32h7 support
f303aaf21b ram: stm32: add second SDRAM bank management
f39b90dc8c ram: stm32: replace fdtdec_get by ofnode calls
1421e0a375 ram: stm32: get base address from DT
9242ece12b ram: stm32: migrate fmc defines in driver file
14a50e3736 drivers: ram: stm32: fix compilation issue
da409ccc4a dm: core: Replace of_offset with accessor (part 2)
a821c4af79 dm: Rename dev_addr..() functions
9d922450aa dm: Use dm.h header when driver mode is used
5a0efcf78a dm: ram: bmips: add BCM6338/BCM6348 support
2165961c37 dm: ram: bmips: split bcm6358_get_ram_size
b493a3564f dm: ram: remove unneeded brcm,bcm63268-mc id
193030e591 ram: add RAM driver for Broadcom MIPS SoCs
bfea69ad27 stm32f7: sdram: correct sdram configuration as per micron sdram
57af3cc32a stm32f7: stm32f746-disco: read memory info from device tree
6c9a10034a stm32f7: sdram: use sdram device tree node to configure sdram controller
d0b24c1aa9 stm32f7: use clock driver to enable sdram controller clock
910a52ede3 stm32f7: dm: add driver model support for sdram
bf1ae4426b stm32f7: sdram: move sdram driver code to ram drivers area
40c9abbd6b ram: rename CONFIG_SPL_RAM_SUPPORT to CONFIG_SPL_RAM
64ce0cad9e dm: test: Add a test for the ram uclass
6c51df6859 dm: Add support for RAM drivers

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 3/3] ram: rk3399: Conduct memory training at 400MHz
  2022-08-11  7:58 [PATCH 0/3] rockchip: Fix RAM training on RK3399 based platforms (Rock Pi 4) Lee Jones
@ 2022-08-11  7:58 ` Lee Jones
  2022-08-11 14:38   ` Michal Suchánek
  0 siblings, 1 reply; 16+ messages in thread
From: Lee Jones @ 2022-08-11  7:58 UTC (permalink / raw)
  To: u-boot, sjg, philipp.tomsich, kever.yang
  Cc: Lee Jones, YouMin Chen, Xavier Drudis Ferran

Currently the default initialisation frequency is 50MHz.  Although
this does appear to be suitable for some LPDDR4 RAM chips, training at
this low frequency has been seen to cause Column errors, leading to
Capacity check errors on others.

Here we force RAM initialisation to happen at 400MHz before ramping up
to the final value running value of 800MHz after everything has been
successfully configured.

Link: https://lore.kernel.org/u-boot/Yo4v3jUeHXTovjOH@google.com/
Suggested-by: YouMin Chen <cym@rock-chips.com>
Signed-off-by: Lee Jones <lee@kernel.org>
Tested-by: Xavier Drudis Ferran <xdrudis@tinet.cat>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
 drivers/ram/rockchip/sdram_rk3399.c | 36 +++++++++++++++++------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
index 34d6c93f95..b05c5925d5 100644
--- a/drivers/ram/rockchip/sdram_rk3399.c
+++ b/drivers/ram/rockchip/sdram_rk3399.c
@@ -85,7 +85,7 @@ struct sdram_rk3399_ops {
 	int (*data_training_first)(struct dram_info *dram, u32 channel, u8 rank,
 				   struct rk3399_sdram_params *sdram);
 	int (*set_rate_index)(struct dram_info *dram,
-			      struct rk3399_sdram_params *params);
+			      struct rk3399_sdram_params *params, u32 ctl_fn);
 	void (*modify_param)(const struct chan_info *chan,
 			     struct rk3399_sdram_params *params);
 	struct rk3399_sdram_params *
@@ -1644,7 +1644,8 @@ static int data_training_first(struct dram_info *dram, u32 channel, u8 rank,
 }
 
 static int switch_to_phy_index1(struct dram_info *dram,
-				struct rk3399_sdram_params *params)
+				struct rk3399_sdram_params *params,
+				u32 unused)
 {
 	u32 channel;
 	u32 *denali_phy;
@@ -2539,26 +2540,25 @@ static int lpddr4_set_ctl(struct dram_info *dram,
 }
 
 static int lpddr4_set_rate(struct dram_info *dram,
-			   struct rk3399_sdram_params *params)
+			    struct rk3399_sdram_params *params,
+			    u32 ctl_fn)
 {
-	u32 ctl_fn;
 	u32 phy_fn;
 
-	for (ctl_fn = 0; ctl_fn < 2; ctl_fn++) {
-		phy_fn = lpddr4_get_phy_fn(params, ctl_fn);
+	phy_fn = lpddr4_get_phy_fn(params, ctl_fn);
 
-		lpddr4_set_phy(dram, params, phy_fn, &dfs_cfgs_lpddr4[ctl_fn]);
-		lpddr4_set_ctl(dram, params, ctl_fn,
-			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
+	lpddr4_set_phy(dram, params, phy_fn, &dfs_cfgs_lpddr4[ctl_fn]);
+	lpddr4_set_ctl(dram, params, ctl_fn,
+		       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
 
-		if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
-			printf("%s: change freq to %dMHz %d, %d\n", __func__,
-			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
-			       ctl_fn, phy_fn);
-	}
+	if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
+		printf("%s: change freq to %dMHz %d, %d\n", __func__,
+		       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
+		       ctl_fn, phy_fn);
 
 	return 0;
 }
+
 #endif /* CONFIG_RAM_RK3399_LPDDR4 */
 
 /* CS0,n=1
@@ -2955,6 +2955,12 @@ static int sdram_init(struct dram_info *dram,
 		params->ch[ch].cap_info.rank = rank;
 	}
 
+#if defined(CONFIG_RAM_RK3399_LPDDR4)
+	/* LPDDR4 needs to be trained at 400MHz */
+	lpddr4_set_rate(dram, params, 0);
+	params->base.ddr_freq = dfs_cfgs_lpddr4[0].base.ddr_freq / MHz;
+#endif
+
 	params->base.num_channels = 0;
 	for (channel = 0; channel < 2; channel++) {
 		const struct chan_info *chan = &dram->chan[channel];
@@ -3005,7 +3011,7 @@ static int sdram_init(struct dram_info *dram,
 	params->base.stride = calculate_stride(params);
 	dram_all_config(dram, params);
 
-	ret = dram->ops->set_rate_index(dram, params);
+	ret = dram->ops->set_rate_index(dram, params, 1);
 	if (ret)
 		return ret;
 
-- 
2.37.1.559.g78731f0fdb-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] ram: rk3399: Conduct memory training at 400MHz
  2022-08-11  7:58 ` [PATCH 3/3] ram: rk3399: Conduct memory training at 400MHz Lee Jones
@ 2022-08-11 14:38   ` Michal Suchánek
  0 siblings, 0 replies; 16+ messages in thread
From: Michal Suchánek @ 2022-08-11 14:38 UTC (permalink / raw)
  To: Lee Jones
  Cc: u-boot, sjg, philipp.tomsich, kever.yang, YouMin Chen,
	Xavier Drudis Ferran

On Thu, Aug 11, 2022 at 08:58:48AM +0100, Lee Jones wrote:
> Currently the default initialisation frequency is 50MHz.  Although
> this does appear to be suitable for some LPDDR4 RAM chips, training at
> this low frequency has been seen to cause Column errors, leading to
> Capacity check errors on others.
> 
> Here we force RAM initialisation to happen at 400MHz before ramping up
> to the final value running value of 800MHz after everything has been
> successfully configured.
> 
> Link: https://lore.kernel.org/u-boot/Yo4v3jUeHXTovjOH@google.com/
> Suggested-by: YouMin Chen <cym@rock-chips.com>
> Signed-off-by: Lee Jones <lee@kernel.org>
> Tested-by: Xavier Drudis Ferran <xdrudis@tinet.cat>

Also does not cause any regression on a Pinebook Pro

Tested-by: Michal Suchánek <msuchanek@suse.de>

Thanks

Michal

> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>  drivers/ram/rockchip/sdram_rk3399.c | 36 +++++++++++++++++------------
>  1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index 34d6c93f95..b05c5925d5 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -85,7 +85,7 @@ struct sdram_rk3399_ops {
>  	int (*data_training_first)(struct dram_info *dram, u32 channel, u8 rank,
>  				   struct rk3399_sdram_params *sdram);
>  	int (*set_rate_index)(struct dram_info *dram,
> -			      struct rk3399_sdram_params *params);
> +			      struct rk3399_sdram_params *params, u32 ctl_fn);
>  	void (*modify_param)(const struct chan_info *chan,
>  			     struct rk3399_sdram_params *params);
>  	struct rk3399_sdram_params *
> @@ -1644,7 +1644,8 @@ static int data_training_first(struct dram_info *dram, u32 channel, u8 rank,
>  }
>  
>  static int switch_to_phy_index1(struct dram_info *dram,
> -				struct rk3399_sdram_params *params)
> +				struct rk3399_sdram_params *params,
> +				u32 unused)
>  {
>  	u32 channel;
>  	u32 *denali_phy;
> @@ -2539,26 +2540,25 @@ static int lpddr4_set_ctl(struct dram_info *dram,
>  }
>  
>  static int lpddr4_set_rate(struct dram_info *dram,
> -			   struct rk3399_sdram_params *params)
> +			    struct rk3399_sdram_params *params,
> +			    u32 ctl_fn)
>  {
> -	u32 ctl_fn;
>  	u32 phy_fn;
>  
> -	for (ctl_fn = 0; ctl_fn < 2; ctl_fn++) {
> -		phy_fn = lpddr4_get_phy_fn(params, ctl_fn);
> +	phy_fn = lpddr4_get_phy_fn(params, ctl_fn);
>  
> -		lpddr4_set_phy(dram, params, phy_fn, &dfs_cfgs_lpddr4[ctl_fn]);
> -		lpddr4_set_ctl(dram, params, ctl_fn,
> -			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
> +	lpddr4_set_phy(dram, params, phy_fn, &dfs_cfgs_lpddr4[ctl_fn]);
> +	lpddr4_set_ctl(dram, params, ctl_fn,
> +		       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
>  
> -		if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
> -			printf("%s: change freq to %dMHz %d, %d\n", __func__,
> -			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
> -			       ctl_fn, phy_fn);
> -	}
> +	if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
> +		printf("%s: change freq to %dMHz %d, %d\n", __func__,
> +		       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
> +		       ctl_fn, phy_fn);
>  
>  	return 0;
>  }
> +
>  #endif /* CONFIG_RAM_RK3399_LPDDR4 */
>  
>  /* CS0,n=1
> @@ -2955,6 +2955,12 @@ static int sdram_init(struct dram_info *dram,
>  		params->ch[ch].cap_info.rank = rank;
>  	}
>  
> +#if defined(CONFIG_RAM_RK3399_LPDDR4)
> +	/* LPDDR4 needs to be trained at 400MHz */
> +	lpddr4_set_rate(dram, params, 0);
> +	params->base.ddr_freq = dfs_cfgs_lpddr4[0].base.ddr_freq / MHz;
> +#endif
> +
>  	params->base.num_channels = 0;
>  	for (channel = 0; channel < 2; channel++) {
>  		const struct chan_info *chan = &dram->chan[channel];
> @@ -3005,7 +3011,7 @@ static int sdram_init(struct dram_info *dram,
>  	params->base.stride = calculate_stride(params);
>  	dram_all_config(dram, params);
>  
> -	ret = dram->ops->set_rate_index(dram, params);
> +	ret = dram->ops->set_rate_index(dram, params, 1);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.37.1.559.g78731f0fdb-goog
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-08-11 14:38 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-21 10:07 [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling Lee Jones
2022-06-21 10:07 ` [PATCH 2/3] ram: rk3399: Fix faulty frequency change reports Lee Jones
2022-07-01 12:04   ` Kever Yang
2022-07-04 10:00   ` Xavier Drudis Ferran
2022-06-21 10:07 ` [PATCH 3/3] ram: rk3399: Conduct memory training at 400MHz Lee Jones
2022-07-01 12:05   ` Kever Yang
2022-07-04 10:01   ` Xavier Drudis Ferran
2022-06-27  8:39 ` [PATCH 1/3] ram: rk3399: Fix .set_rate_index() error handling Lee Jones
2022-07-01 12:05   ` Kever Yang
2022-07-04  9:23     ` Lee Jones
2022-07-04 10:49       ` Kever Yang
2022-07-04 10:58         ` Lee Jones
2022-07-01 12:04 ` Kever Yang
2022-07-04  9:57 ` [SPAM] " Xavier Drudis Ferran
  -- strict thread matches above, loose matches on Subject: below --
2022-08-11  7:58 [PATCH 0/3] rockchip: Fix RAM training on RK3399 based platforms (Rock Pi 4) Lee Jones
2022-08-11  7:58 ` [PATCH 3/3] ram: rk3399: Conduct memory training at 400MHz Lee Jones
2022-08-11 14:38   ` Michal Suchánek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.