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.
> > 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,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5
849D 1736 249D
