[PATCH u-boot v2019.04-aspeed-openbmc v2 1/3] aspeed/sdram: Use device tree to configure ECC

[Andrew Jeffery]

On Wed, 21 Sep 2022, at 14:24, Joel Stanley wrote:
> Instead of configuring ECC based on the build config, use a device tree
> property to selectively enable ECC at runtime.
>
> There are two properties:
>
>   aspeed,ecc-enabled;
>   aspeed,ecc-size-mb = "512";
>
> The enabled property is a boolean that enables ECC if it is present.
>
> The size is the number of MB that should be covered by ECC. Setting it
> to zero, or omitting it, defaults the ECC size to "auto detect".
>
>   edac: sdram at 1e6e0000 {
>     compatible = "aspeed,ast2600-sdram-edac";
>     reg = <0x1e6e0000 0x174>;
>     interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
>     aspeed,ecc-enabled;
>     aspeed,ecc-size-mb = "512";
>   };
>
> Signed-off-by: Joel Stanley <joel at jms.id.au>
> ---
> v2:
>   Change property to be aspeed,ecc-size-mb
>   Fix printing of size to use mb
>
>  drivers/ram/aspeed/sdram_ast2500.c | 14 ++++++++++----
>  drivers/ram/aspeed/sdram_ast2600.c | 14 ++++++++++----
>  drivers/ram/aspeed/Kconfig         | 13 -------------
>  3 files changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/ram/aspeed/sdram_ast2500.c 
> b/drivers/ram/aspeed/sdram_ast2500.c
> index 435e1a8cfc1d..9bf4906b5939 100644
> --- a/drivers/ram/aspeed/sdram_ast2500.c
> +++ b/drivers/ram/aspeed/sdram_ast2500.c
> @@ -279,16 +279,16 @@ static void ast2500_sdrammc_calc_size(struct 
> dram_info *info)
>  }
> 
>  #ifdef CONFIG_ASPEED_ECC
> -static void ast2500_sdrammc_ecc_enable(struct dram_info *info)
> +static void ast2500_sdrammc_ecc_enable(struct dram_info *info, u32 
> conf_size_mb)
>  {
>  	struct ast2500_sdrammc_regs *regs = info->regs;
>  	size_t conf_size;
>  	u32 reg;
> 	
> -	conf_size = CONFIG_ASPEED_ECC_SIZE * SDRAM_SIZE_1MB;
> +	conf_size = conf_size_mb * SDRAM_SIZE_1MB;
>  	if (conf_size > info->info.size) {
>  		printf("warning: ECC configured %dMB but actual size is %dMB\n",
> -		       CONFIG_ASPEED_ECC_SIZE,
> +		       conf_size_mb,
>  		       info->info.size / SDRAM_SIZE_1MB);
>  		conf_size = info->info.size;
>  	} else if (conf_size == 0) {
> @@ -371,8 +371,14 @@ static int ast2500_sdrammc_init_ddr4(struct 
> dram_info *info)
>  	writel(SDRAM_MISC_DDR4_TREFRESH, &info->regs->misc_control);
> 
>  #ifdef CONFIG_ASPEED_ECC
> -	ast2500_sdrammc_ecc_enable(info);
> +	if (dev_read_bool(dev, "aspeed,ecc-enabled")) {
> +		u32 ecc_size;
> +
> +		ecc_size = dev_read_u32_default(dev, "aspeed,ecc-size-mb", 0);
> +		ast2500_sdrammc_ecc_enable(priv, ecc_size);

The code you deleted passes `info` to ast2500_sdrammc_ecc_enable() as
the first argument but you're passing `priv` here. Do they alias?

Ah, actually, I think you copied the code from sdram_ast2600.c into
sdram_ast2500.c. I suspect this might not compile?

Andrew