On 29/01/24 17:49, Nishanth Menon wrote:
On 17:02-20240129, Apurva Nandan wrote:
Hi,

On 24/01/24 02:17, Nishanth Menon wrote:
On 20:21-20240123, Apurva Nandan wrote:
[...]
+void k3_mem_init(void)
+{
+       struct udevice *dev;
+       int ret, ctr = 1;
+
+       if (IS_ENABLED(CONFIG_K3_J721E_DDRSS)) {
+               ret = uclass_get_device(UCLASS_RAM, 0, &dev);
+               if (ret)
+                       panic("DRAM 0 init failed: %d\n", ret);
+
+               while (dev) {
why loop on dev? is it possible to have ret != 0 and dev = 0?

Some variable needs to be used for loop condition, do you want it to be ret?
or maybe you can suggest your idea for this please.
+                       ret = uclass_next_device_err(&dev);
+                       if (ret) {
+                               printf("Initialized %d DRAM controllers\n", 
ctr);
+                               break;
+                       }
+                       ctr++;
What is the use of ctr++ ?? please do a limit check for instances.
This is to keep the logic independent of board evm, so that no include of
EVM config is needed.
ctr is just used to notify user about how many DDR are up during boot, else
it is not needed.

I can remove the ctr and printf, if you want.

For a limit check, how can we get number of DDR instances on the EVM, I
don't know, can you please suggest some way?

There is no config that stores this info afaik.
Why? J784s4 has only specific number of controllerns, correct?

A variant of the below -> but still have a question:

while (ctrl < J784S4_MAX_CONTROLLERS) {
Is J784S4_MAX_CONTROLLERS going to be a #define in j784s4_init.c

Or a Kconfig option like:

config DDR_MAX_CONTROLLERS
     int "Max number of DRAM controllers"
     default 4 if SOC_K3_J784S4
     default 1
     help

I dont see a need for Kconfig - but that is just me.
Ohkay, sending with #define.
        ret = uclass_next_device_err(&ret);
        if (ret) /* Question: How do we differentiate between valid
                  * failure and next instance not being present? */
                break;
how about:

             ...
             if (ret == -ENODEV)
                 break;

             if (ret)
                  panic("DRAM %d init failed: %d\n", ctrl,  ret);
             ...
What ever is appropriate.


--
Regards,
Apurva Nandan,
Texas Instruments.

Reply via email to