[PATCH v2 0/2] sunxi: restore modified memory

Changes in v2: - rename temporary variables - fix types for temporary variables
Andrey Skvortsov (2): sunxi: restore modified memory sunxi: reorganize mctl_mem_matches_* functions
arch/arm/include/asm/arch-sunxi/dram.h | 1 + arch/arm/mach-sunxi/dram_helpers.c | 32 +++++++++++++++++++++----- arch/arm/mach-sunxi/dram_sunxi_dw.c | 13 ----------- 3 files changed, 27 insertions(+), 19 deletions(-)

On A64 with 2G of RAM words at following addresses were modified with 'aa55aa55' value: - 50000000 - 60000000 - 84000000 - 88000000 - 90000000 - A0000000 - A0000200
That made harder to pick memory range for persistent storage in RAM.
Signed-off-by: Andrey Skvortsov andrej.skvortzov@gmail.com --- arch/arm/mach-sunxi/dram_helpers.c | 16 ++++++++++++++-- arch/arm/mach-sunxi/dram_sunxi_dw.c | 16 ++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-sunxi/dram_helpers.c b/arch/arm/mach-sunxi/dram_helpers.c index cdf2750f1c..61a6da84e3 100644 --- a/arch/arm/mach-sunxi/dram_helpers.c +++ b/arch/arm/mach-sunxi/dram_helpers.c @@ -32,12 +32,24 @@ void mctl_await_completion(u32 *reg, u32 mask, u32 val) #ifndef CONFIG_MACH_SUNIV bool mctl_mem_matches(u32 offset) { + u32 val_base; + u32 val_offset; + bool ret; + + /* Save original values */ + val_base = readl(CFG_SYS_SDRAM_BASE); + val_offset = readl((ulong)CFG_SYS_SDRAM_BASE + offset); + /* Try to write different values to RAM at two addresses */ writel(0, CFG_SYS_SDRAM_BASE); writel(0xaa55aa55, (ulong)CFG_SYS_SDRAM_BASE + offset); dsb(); /* Check if the same value is actually observed when reading back */ - return readl(CFG_SYS_SDRAM_BASE) == - readl((ulong)CFG_SYS_SDRAM_BASE + offset); + ret = readl(CFG_SYS_SDRAM_BASE) == readl((ulong)CFG_SYS_SDRAM_BASE + offset); + + /* Restore original values */ + writel(val_base, CFG_SYS_SDRAM_BASE); + writel(val_offset, (ulong)CFG_SYS_SDRAM_BASE + offset); + return ret; } #endif diff --git a/arch/arm/mach-sunxi/dram_sunxi_dw.c b/arch/arm/mach-sunxi/dram_sunxi_dw.c index 9382d3d0be..905a43c918 100644 --- a/arch/arm/mach-sunxi/dram_sunxi_dw.c +++ b/arch/arm/mach-sunxi/dram_sunxi_dw.c @@ -657,13 +657,25 @@ static int mctl_channel_init(uint16_t socid, struct dram_para *para) */ static bool mctl_mem_matches_base(u32 offset, ulong base) { + u32 val_base; + u32 val_offset; + bool ret; + + /* Save original values */ + val_base = readl(base); + val_offset = readl((ulong)base + offset); + /* Try to write different values to RAM at two addresses */ writel(0, base); writel(0xaa55aa55, base + offset); dsb(); /* Check if the same value is actually observed when reading back */ - return readl(base) == - readl(base + offset); + ret = readl(base) == readl(base + offset); + + /* Restore original values */ + writel(val_base, base); + writel(val_offset, (ulong)base + offset); + return ret; }
static void mctl_auto_detect_dram_size_rank(uint16_t socid, struct dram_para *para, ulong base, struct rank_para *rank)

On Wed, 6 Dec 2023 22:06:32 +0300 Andrey Skvortsov andrej.skvortzov@gmail.com wrote:
Hi Andrey,
thanks for respinning!
On A64 with 2G of RAM words at following addresses were modified with 'aa55aa55' value:
- 50000000
- 60000000
- 84000000
- 88000000
- 90000000
- A0000000
- A0000200
That made harder to pick memory range for persistent storage in RAM.
I don't think this commit message is really helpful. What about:
------------------------------ Our sunxi DRAM initialisation code does several test accesses to the DRAM array to detect aliasing effects and so determine the correct row/column configuration. This changes the DRAM content, which breaks use cases like soft reset and Linux' ramoops mechanism.
Fix this problem by saving and restoring the content of the DRAM cells that we use for the test writes. ------------------------------
Signed-off-by: Andrey Skvortsov andrej.skvortzov@gmail.com
arch/arm/mach-sunxi/dram_helpers.c | 16 ++++++++++++++-- arch/arm/mach-sunxi/dram_sunxi_dw.c | 16 ++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-sunxi/dram_helpers.c b/arch/arm/mach-sunxi/dram_helpers.c index cdf2750f1c..61a6da84e3 100644 --- a/arch/arm/mach-sunxi/dram_helpers.c +++ b/arch/arm/mach-sunxi/dram_helpers.c @@ -32,12 +32,24 @@ void mctl_await_completion(u32 *reg, u32 mask, u32 val) #ifndef CONFIG_MACH_SUNIV bool mctl_mem_matches(u32 offset) {
- u32 val_base;
- u32 val_offset;
- bool ret;
- /* Save original values */
- val_base = readl(CFG_SYS_SDRAM_BASE);
- val_offset = readl((ulong)CFG_SYS_SDRAM_BASE + offset);
I don't think we need the cast here. And everywhere else, actually.
- /* Try to write different values to RAM at two addresses */ writel(0, CFG_SYS_SDRAM_BASE); writel(0xaa55aa55, (ulong)CFG_SYS_SDRAM_BASE + offset); dsb(); /* Check if the same value is actually observed when reading back */
- return readl(CFG_SYS_SDRAM_BASE) ==
readl((ulong)CFG_SYS_SDRAM_BASE + offset);
- ret = readl(CFG_SYS_SDRAM_BASE) == readl((ulong)CFG_SYS_SDRAM_BASE + offset);
- /* Restore original values */
- writel(val_base, CFG_SYS_SDRAM_BASE);
- writel(val_offset, (ulong)CFG_SYS_SDRAM_BASE + offset);
- return ret;
} #endif diff --git a/arch/arm/mach-sunxi/dram_sunxi_dw.c b/arch/arm/mach-sunxi/dram_sunxi_dw.c index 9382d3d0be..905a43c918 100644 --- a/arch/arm/mach-sunxi/dram_sunxi_dw.c +++ b/arch/arm/mach-sunxi/dram_sunxi_dw.c
I think it would be better to swap the two patches, so to unify the two routines first, then to add the save/restore code. Otherwise you are patching code here that you remove in the next patch.
@@ -657,13 +657,25 @@ static int mctl_channel_init(uint16_t socid, struct dram_para *para) */ static bool mctl_mem_matches_base(u32 offset, ulong base) {
- u32 val_base;
- u32 val_offset;
- bool ret;
- /* Save original values */
- val_base = readl(base);
- val_offset = readl((ulong)base + offset);
base is already ulong, so no cast needed, same below.
Cheers, Andre
- /* Try to write different values to RAM at two addresses */ writel(0, base); writel(0xaa55aa55, base + offset); dsb(); /* Check if the same value is actually observed when reading back */
- return readl(base) ==
readl(base + offset);
- ret = readl(base) == readl(base + offset);
- /* Restore original values */
- writel(val_base, base);
- writel(val_offset, (ulong)base + offset);
- return ret;
}
static void mctl_auto_detect_dram_size_rank(uint16_t socid, struct dram_para *para, ulong base, struct rank_para *rank)

mctl_mem_matches and mctl_mem_matches_base identical functions. To avoid code duplication move them to dram_helpers and make mctl_mem_matches use generic mctl_mem_matches_base.
Signed-off-by: Andrey Skvortsov andrej.skvortzov@gmail.com --- arch/arm/include/asm/arch-sunxi/dram.h | 1 + arch/arm/mach-sunxi/dram_helpers.c | 26 +++++++++++++++++--------- arch/arm/mach-sunxi/dram_sunxi_dw.c | 25 ------------------------- 3 files changed, 18 insertions(+), 34 deletions(-)
diff --git a/arch/arm/include/asm/arch-sunxi/dram.h b/arch/arm/include/asm/arch-sunxi/dram.h index 682daae6b1..9d21b49241 100644 --- a/arch/arm/include/asm/arch-sunxi/dram.h +++ b/arch/arm/include/asm/arch-sunxi/dram.h @@ -40,5 +40,6 @@ unsigned long sunxi_dram_init(void); void mctl_await_completion(u32 *reg, u32 mask, u32 val); bool mctl_mem_matches(u32 offset); +bool mctl_mem_matches_base(u32 offset, ulong base);
#endif /* _SUNXI_DRAM_H */ diff --git a/arch/arm/mach-sunxi/dram_helpers.c b/arch/arm/mach-sunxi/dram_helpers.c index 61a6da84e3..c86d20d22b 100644 --- a/arch/arm/mach-sunxi/dram_helpers.c +++ b/arch/arm/mach-sunxi/dram_helpers.c @@ -25,31 +25,39 @@ void mctl_await_completion(u32 *reg, u32 mask, u32 val) }
/* - * Test if memory at offset offset matches memory at begin of DRAM + * Test if memory at offset matches memory at a certain base * * Note: dsb() is not available on ARMv5 in Thumb mode */ #ifndef CONFIG_MACH_SUNIV -bool mctl_mem_matches(u32 offset) +bool mctl_mem_matches_base(u32 offset, ulong base) { u32 val_base; u32 val_offset; bool ret;
/* Save original values */ - val_base = readl(CFG_SYS_SDRAM_BASE); - val_offset = readl((ulong)CFG_SYS_SDRAM_BASE + offset); + val_base = readl(base); + val_offset = readl((ulong)base + offset);
/* Try to write different values to RAM at two addresses */ - writel(0, CFG_SYS_SDRAM_BASE); - writel(0xaa55aa55, (ulong)CFG_SYS_SDRAM_BASE + offset); + writel(0, base); + writel(0xaa55aa55, base + offset); dsb(); /* Check if the same value is actually observed when reading back */ - ret = readl(CFG_SYS_SDRAM_BASE) == readl((ulong)CFG_SYS_SDRAM_BASE + offset); + ret = readl(base) == readl(base + offset);
/* Restore original values */ - writel(val_base, CFG_SYS_SDRAM_BASE); - writel(val_offset, (ulong)CFG_SYS_SDRAM_BASE + offset); + writel(val_base, base); + writel(val_offset, (ulong)base + offset); return ret; } + +/* + * Test if memory at offset matches memory at begin of DRAM + */ +bool mctl_mem_matches(u32 offset) +{ + return mctl_mem_matches_base(offset, CFG_SYS_SDRAM_BASE); +} #endif diff --git a/arch/arm/mach-sunxi/dram_sunxi_dw.c b/arch/arm/mach-sunxi/dram_sunxi_dw.c index 905a43c918..2e8dd40b97 100644 --- a/arch/arm/mach-sunxi/dram_sunxi_dw.c +++ b/arch/arm/mach-sunxi/dram_sunxi_dw.c @@ -652,31 +652,6 @@ static int mctl_channel_init(uint16_t socid, struct dram_para *para) return 0; }
-/* - * Test if memory at offset offset matches memory at a certain base - */ -static bool mctl_mem_matches_base(u32 offset, ulong base) -{ - u32 val_base; - u32 val_offset; - bool ret; - - /* Save original values */ - val_base = readl(base); - val_offset = readl((ulong)base + offset); - - /* Try to write different values to RAM at two addresses */ - writel(0, base); - writel(0xaa55aa55, base + offset); - dsb(); - /* Check if the same value is actually observed when reading back */ - ret = readl(base) == readl(base + offset); - - /* Restore original values */ - writel(val_base, base); - writel(val_offset, (ulong)base + offset); - return ret; -}
static void mctl_auto_detect_dram_size_rank(uint16_t socid, struct dram_para *para, ulong base, struct rank_para *rank) {
participants (2)
-
Andre Przywara
-
Andrey Skvortsov