Re: [PATCH v4 5/8] drivers: ram: Kconfig: Add CONFIG_K3_INLINE_ECC

[Neha Malcom Francis <n-francis@ti.com>]

Hi Bryan

On 23/10/24 20:15, Bryan Brattlof wrote:
> On October 21, 2024 thus sayeth Santhosh Kumar K:
>> From: Neha Malcom Francis <n-francis@ti.com>
>>
>> Add CONFIG_K3_INLINE_ECC so that ECC functions can be compiled into R5 SPL
>> only when the config has been enabled.
>>
>> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
>> ---
>>   drivers/ram/Kconfig | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/ram/Kconfig b/drivers/ram/Kconfig
>> index f7e357f24da7..533d0c629a64 100644
>> --- a/drivers/ram/Kconfig
>> +++ b/drivers/ram/Kconfig
>> @@ -116,6 +116,16 @@ config IMXRT_SDRAM
>>   	  to support external memories like sdram, psram & nand.
>>   	  This driver is for the sdram memory interface with the SEMC.
>>   
>> +config K3_INLINE_ECC
>> +	bool "Enable TI Inline ECC support"
>> +	depends on K3_DDRSS
>> +	help
>> +	  Enable Inline ECC support on K3 platforms. 1/9th of the SDRAM space
>> +	  is used for ECC storage and the rest 8/9th is available for system
>> +	  use. Enabling ECC increases boot time as the ECC protected regions
>> +	  need to be primed with a predefined value prior to enabling ECC
>> +	  check.
>> +
> 
> I agree size will always be a complaint at such an early stage of boot
> but we're already adding the ECC configuration in the memory{} node.
> 
> We should do one or the other but not both. It would be hard to debug
> problems if someone added the protected{} node but not enable this
> config

Actually I do agree with this, maybe we can get rid of the flag ti,ecc-enable 
which is redundant. In multi-DDR systems, it was still a possibility that we 
could use it to select the DDR we want ECC enabled on, but that hasn't been 
tested nor does it make any sense since they'll be interleaved anyways.

-- 
Thanking You
Neha Malcom Francis