On 07/08/25 10:28, Kumar, Udit wrote:
>
> On 7/30/2025 7:07 PM, Neha Malcom Francis wrote:
>> The existing approach does not account for interleaving in the DDRs when
>> setting up regions. There is support for MSMC to calculate the regions
>> for each DDR, so modify k3_ddrss_probe to set the regions accordingly
>> for multi-DDR systems.
>>
>> Signed-off-by: Neha Malcom Francis <n-fran...@ti.com>
>> ---
>> drivers/ram/k3-ddrss/k3-ddrss.c | 58 ++++++++++++++++++++++++++++-----
>> 1 file changed, 50 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c
>> b/drivers/ram/k3-ddrss/k3-ddrss.c
>> index 1c2e5e1835b..e00711a2235 100644
>> --- a/drivers/ram/k3-ddrss/k3-ddrss.c
>> +++ b/drivers/ram/k3-ddrss/k3-ddrss.c
>> @@ -828,11 +828,12 @@ static void k3_ddrss_lpddr4_ecc_init(struct
>> k3_ddrss_desc *ddrss)
>> static int k3_ddrss_probe(struct udevice *dev)
>> {
>> - u64 end;
>> + u64 end, bank0, bank1;
>> int ret;
>> struct k3_ddrss_desc *ddrss = dev_get_priv(dev);
>> - __maybe_unused u32 ddr_ram_size, ecc_res;
>> + __maybe_unused u32 inst, ddr_ram_size, ecc_res, st;
>
> size of ddr may go above 4G
For this and the other remaining comments; will address them in v3. Thanks!
>
>
>> __maybe_unused struct k3_ddrss_ecc_region *range =
>> &ddrss->ecc_range;
>> + __maybe_unused struct k3_msmc *msmc_parent = NULL;
>> debug("%s(dev=%p)\n", __func__, dev);
>> @@ -873,26 +874,67 @@ static int k3_ddrss_probe(struct udevice *dev)
>> k3_ddrss_ddr_inline_ecc_base_size_calc(range);
>> end = ddrss->ecc_range.start + ddrss->ecc_range.range;
>> + inst = ddrss->instance;
>> ddr_ram_size = ddrss->ddr_ram_size;
>> ecc_res = ddrss->ecc_reserved_space;
>> + bank0 = ddrss->ddr_bank_base[0];
>> + bank1 = ddrss->ddr_bank_base[1];
>> if (!range->range) {
>> /* Configure entire DDR space by default */
>> debug("%s: Defaulting to protecting entire DDR space
>> using inline ECC\n",
>> __func__);
>> - ddrss->ecc_range.start = ddrss->ddr_bank_base[0];
>> + ddrss->ecc_range.start = bank0;
>> ddrss->ecc_range.range = ddr_ram_size - ecc_res;
>> } else {
>> ddrss->ecc_range.start = range->start;
>> ddrss->ecc_range.range = range->range;
>> }
>> - if (end > (ddr_ram_size - ecc_res))
>> - ddrss->ecc_regions[0].range = ddr_ram_size - ecc_res;
>> - else
>> - ddrss->ecc_regions[0].range = ddrss->ecc_range.range;
>> + st = ddrss->ecc_range.start;
>> +
>> + if (!CONFIG_IS_ENABLED(K3_MULTI_DDR)) {
>> + if (end > (ddr_ram_size - ecc_res))
>> + ddrss->ecc_regions[0].range = ddr_ram_size - ecc_res;
>> + else
>> + ddrss->ecc_regions[0].range = ddrss->ecc_range.range;
>> - ddrss->ecc_regions[0].start = ddrss->ecc_range.start -
>> ddrss->ddr_bank_base[0];
>> + /* Check in which bank we are */
>> + if (st > bank1)
>> + ddrss->ecc_regions[0].start = st - bank1 +
>> ddrss->ddr_bank_size[0];
>> + else
>> + ddrss->ecc_regions[0].start = st - bank0;
>> + } else {
>> + /* For multi-DDR, we rely on MSMC's calculation of
>> regions for each DDR */
>> + msmc_parent = kzalloc(sizeof(msmc_parent), GFP_KERNEL);
>> + if (!msmc_parent)
>> + return -ENOMEM;
>> +
>> + msmc_parent = dev_get_priv(dev->parent);
>> + if (!msmc_parent) {
>> + printf("%s: could not get MSMC parent to set up
>> inline ECC regions\n",
>> + __func__);
>> + kfree(msmc_parent);
>> + return -EINVAL;
>> + }
>> +
>> + if (msmc_parent->R0[0].start < 0) {
>> + /* Configure entire DDR space by default */
>> + ddrss->ecc_regions[0].start = ddrss->ddr_bank_base[0];
>> + ddrss->ecc_regions[0].range = ddr_ram_size - ecc_res;
>> + } else {
>> + end = msmc_parent->R0[inst].start +
>> msmc_parent->R0[inst].range;
>> +
>> + if (end > (ddr_ram_size - ecc_res))
>> + ddrss->ecc_regions[0].range = ddr_ram_size -
>> ecc_res;
>> + else
>> + ddrss->ecc_regions[0].range =
>> msmc_parent->R0[inst].range;
>> +
>> + ddrss->ecc_regions[0].start =
>> msmc_parent->R0[inst].start;
>> + }
>> +
>> + kfree(msmc_parent);
>> + }
>> k3_ddrss_lpddr4_ecc_init(ddrss);
>> }
--
Thanking You
Neha Malcom Francis
