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

[Andre Przywara]

On Wed,  6 Dec 2023 22:06:32 +0300
Andrey Skvortsov <andrej.skvortzov at 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 at 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)