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) { ret = uclass_next_device_err(&ret); if (ret) /* Question: How do we differentiate between valid * failure and next instance not being present? */ break; ctrl++; }
info("Initialized %d DRAM controllers\n", ctrl - 1);
[...]
Next time, please respond to the review comment questions so that I know that you have considered and decided something is not necessary or something was missed in the new version - for example what happened to mmc_stop/restart?
mmc_stop/restart were removed (mentioned in series changelog)
Mentioning in diffstat of the patch helps give people the context of the change w.r.t the path itself.