[PATCH u-boot-marvell RFC] PLEASE TEST: ddr: marvell: a38x: fix BYTE_HOMOGENEOUS_SPLIT_OUT decision
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 --- 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,
On Fri, Feb 11, 2022 at 5:25 AM Marek Behún kabel@kernel.org 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
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
On 2/15/22 23:21, Chris Packham wrote:
On Fri, Feb 11, 2022 at 5:25 AM Marek Behún kabel@kernel.org 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
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,-- 2.34.1
Viele Grüße, Stefan Roese
On Wed, 16 Feb 2022 09:41:44 +0100 Stefan Roese sr@denx.de wrote:
On 2/15/22 23:21, Chris Packham wrote:
On Fri, Feb 11, 2022 at 5:25 AM Marek Behún kabel@kernel.org 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
Dear Stefan,
will you apply this (removing the PLEASE TEST: prefix) or should I send it again without it?
I am going to send another patch which will reset the board immediately if DDR training fails, so I can send with that one.
Marek
On 2/16/22 14:26, Marek Behún wrote:
On Wed, 16 Feb 2022 09:41:44 +0100 Stefan Roese sr@denx.de wrote:
On 2/15/22 23:21, Chris Packham wrote:
On Fri, Feb 11, 2022 at 5:25 AM Marek Behún kabel@kernel.org 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
Dear Stefan,
will you apply this (removing the PLEASE TEST: prefix) or should I send it again without it?
I am going to send another patch which will reset the board immediately if DDR training fails, so I can send with that one.
Whatever you prefer. Plan is to apply/push some of the pending patches in the next few days.
Thanks, Stefan
participants (3)
-
Chris Packham -
Marek Behún -
Stefan Roese