Re: [PATCH] arm: mach-k3: j721s2_init: Support less than max DDR controllers

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

Hi Udit

On 30/01/24 13:53, Kumar, Udit wrote:
> 
> On 1/30/2024 1:11 PM, Neha Malcom Francis wrote:
>> The number of DDR controllers to be initialised and used should depend
>> on the device tree with the constraint of the maximum number of
>> controllers the device supports. Since J721S2 has multiple (2)
>> controllers, instead of hardcoding the number of probes, move to
>> depending on the device tree UCLASS_RAM nodes present.
>>
>> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
>> ---
>> Boot logs:
>> https://gist.github.com/nehamalcom/07fedf4aa173590214b5cef6e1688fa1
>>
>> This was also parallely proposed in [1] on the mailing-list for J784S4.
>>
>> [1] https://lore.kernel.org/all/3a7c817b-de29-463a-b4b6-d62c0df66ade@ti.com/
>>
>>   arch/arm/mach-k3/j721s2_init.c | 15 +++++++++++----
>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/mach-k3/j721s2_init.c b/arch/arm/mach-k3/j721s2_init.c
>> index fb0708bae1..ff21619506 100644
>> --- a/arch/arm/mach-k3/j721s2_init.c
>> +++ b/arch/arm/mach-k3/j721s2_init.c
>> @@ -213,10 +213,12 @@ bool check_rom_loaded_sysfw(void)
>>       return is_rom_loaded_sysfw(&bootdata);
>>   }
>> +#define J721S2_MAX_CONTROLLERS    2
>> +
>>   void k3_mem_init(void)
>>   {
>>       struct udevice *dev;
>> -    int ret;
>> +    int ret, ctr = 1;
>>       if (IS_ENABLED(CONFIG_K3_J721E_DDRSS)) {
>>           ret = uclass_get_device_by_name(UCLASS_MISC, "msmc", &dev);
>> @@ -227,9 +229,14 @@ void k3_mem_init(void)
>>           if (ret)
>>               panic("DRAM 0 init failed: %d\n", ret);
>> -        ret = uclass_next_device_err(&dev);
>> -        if (ret)
>> -            panic("DRAM 1 init failed: %d\n", ret);
> 
> Since there are two controllers only and you need to call uclass_get_device and 
> uclass_next_device_err
> 
> what about just checking error in above removed code, something like
> 
>          ret = uclass_next_device_err(&dev);
>          if (ret!=ENODEV)
>              panic("DRAM 1 init failed: %d\n", ret);
> 
> 

Yes... since AEP has only 2 controllers, this will suffice.

>> +        while (ctr < J721S2_MAX_CONTROLLERS) {
>> +            ret = uclass_next_device_err(&dev);
>> +            if (ret == -ENODEV)
>> +                break;
>> +            if (ret)
>> +                panic("DRAM %d init failed: %d\n", ctr, ret);
>> +            ctr++;
>> +        }
>>       }
>>       spl_enable_cache();
>>   }

-- 
Thanking You
Neha Malcom Francis