[PATCH u-boot-marvell] ddr: marvell: a38x: fix BYTE_HOMOGENEOUS_SPLIT_OUT decision

[PATCH u-boot-marvell] ddr: marvell: a38x: fix BYTE_HOMOGENEOUS_SPLIT_OUT decision

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

In commit 3fc92a215b69 ("ddr: marvell: a38x: fix SPLIT_OUT_MIX state
decision") I ported a cleaned up and changed version of patch
  mv_ddr: a380: fix SPLIT_OUT_MIX state decision

In the port we removed checking for BYTE_HOMOGENEOUS_SPLIT_OUT bit,
because:
- the fix seemed to work without it
- the bit was checked for only at one place out of two, while the second
  bit, BYTE_SPLIT_OUT_MIX, was checked for in both cases
- without the removal it didn't work on Allied Telesis' x530 board

We recently had a chance to test on more boards, and it seems that the
change needs to be opposite: instead of removing the check for
BYTE_HOMOGENEOUS_SPLIT_OUT from the first if() statement, the check
needs to be added also to the second one - it needs to be at both
places.

With this change all the Turris Omnia boards I have had available to
test seem to work, I didn't encounter not even one failed DDR training.

As last time, I am noting that I do not understand what this code is
actually doing, I haven't studied the DDR training algorithm and
I suspect that no one will be able to explain it to U-Boot contributors,
so we are left with this blind poking in the code with testing whether
it works on several boards and hoping it doesn't break anything for
anyone :-(.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Tested-by: Chris Packham <judge.packham@gmail.com>
Reviewed-by: Stefan Roese <sr@denx.de>
---
 drivers/ddr/marvell/a38x/ddr3_training_centralization.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/ddr/marvell/a38x/ddr3_training_centralization.c b/drivers/ddr/marvell/a38x/ddr3_training_centralization.c
index 42308b6965..be9f985f22 100644
--- a/drivers/ddr/marvell/a38x/ddr3_training_centralization.c
+++ b/drivers/ddr/marvell/a38x/ddr3_training_centralization.c
@@ -180,7 +180,8 @@ static int ddr3_tip_centralization(u32 dev_num, u32 mode)
 							       [bit_id],
 							       EDGE_1);
 					if (current_byte_status &
-					    BYTE_SPLIT_OUT_MIX) {
+					    (BYTE_SPLIT_OUT_MIX |
+					     BYTE_HOMOGENEOUS_SPLIT_OUT)) {
 						if (cur_start_win[bit_id] >= 64)
 							cur_start_win[bit_id] -= 64;
 						else
@@ -197,7 +198,8 @@ static int ddr3_tip_centralization(u32 dev_num, u32 mode)
 							       EDGE_1);
 					if (cur_end_win[bit_id] >= 64 &&
 					    (current_byte_status &
-					     BYTE_SPLIT_OUT_MIX)) {
+					     (BYTE_SPLIT_OUT_MIX |
+					      BYTE_HOMOGENEOUS_SPLIT_OUT))) {
 						cur_end_win[bit_id] -= 64;
 						DEBUG_CENTRALIZATION_ENGINE
 							(DEBUG_LEVEL_INFO,
-- 
2.34.1

Re: [PATCH u-boot-marvell] ddr: marvell: a38x: fix BYTE_HOMOGENEOUS_SPLIT_OUT decision

[Stefan Roese]

On 2/17/22 01:08, Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> In commit 3fc92a215b69 ("ddr: marvell: a38x: fix SPLIT_OUT_MIX state
> decision") I ported a cleaned up and changed version of patch
>    mv_ddr: a380: fix SPLIT_OUT_MIX state decision
> 
> In the port we removed checking for BYTE_HOMOGENEOUS_SPLIT_OUT bit,
> because:
> - the fix seemed to work without it
> - the bit was checked for only at one place out of two, while the second
>    bit, BYTE_SPLIT_OUT_MIX, was checked for in both cases
> - without the removal it didn't work on Allied Telesis' x530 board
> 
> We recently had a chance to test on more boards, and it seems that the
> change needs to be opposite: instead of removing the check for
> BYTE_HOMOGENEOUS_SPLIT_OUT from the first if() statement, the check
> needs to be added also to the second one - it needs to be at both
> places.
> 
> With this change all the Turris Omnia boards I have had available to
> test seem to work, I didn't encounter not even one failed DDR training.
> 
> As last time, I am noting that I do not understand what this code is
> actually doing, I haven't studied the DDR training algorithm and
> I suspect that no one will be able to explain it to U-Boot contributors,
> so we are left with this blind poking in the code with testing whether
> it works on several boards and hoping it doesn't break anything for
> anyone :-(.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Tested-by: Chris Packham <judge.packham@gmail.com>
> Reviewed-by: Stefan Roese <sr@denx.de>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
>   drivers/ddr/marvell/a38x/ddr3_training_centralization.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ddr/marvell/a38x/ddr3_training_centralization.c b/drivers/ddr/marvell/a38x/ddr3_training_centralization.c
> index 42308b6965..be9f985f22 100644
> --- a/drivers/ddr/marvell/a38x/ddr3_training_centralization.c
> +++ b/drivers/ddr/marvell/a38x/ddr3_training_centralization.c
> @@ -180,7 +180,8 @@ static int ddr3_tip_centralization(u32 dev_num, u32 mode)
>   							       [bit_id],
>   							       EDGE_1);
>   					if (current_byte_status &
> -					    BYTE_SPLIT_OUT_MIX) {
> +					    (BYTE_SPLIT_OUT_MIX |
> +					     BYTE_HOMOGENEOUS_SPLIT_OUT)) {
>   						if (cur_start_win[bit_id] >= 64)
>   							cur_start_win[bit_id] -= 64;
>   						else
> @@ -197,7 +198,8 @@ static int ddr3_tip_centralization(u32 dev_num, u32 mode)
>   							       EDGE_1);
>   					if (cur_end_win[bit_id] >= 64 &&
>   					    (current_byte_status &
> -					     BYTE_SPLIT_OUT_MIX)) {
> +					     (BYTE_SPLIT_OUT_MIX |
> +					      BYTE_HOMOGENEOUS_SPLIT_OUT))) {
>   						cur_end_win[bit_id] -= 64;
>   						DEBUG_CENTRALIZATION_ENGINE
>   							(DEBUG_LEVEL_INFO,

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de