[U-Boot] [PATCH 14/28] drivers/fsl-mc: Changed MC firmware loading for new boot architecture

Jose Rivera
2015-03-23T20:06:11Z

> -----Original Message-----
> From: Kim Phillips [mailto:kim.phillips at freescale.com]
> Sent: Thursday, March 19, 2015 12:53 PM
> To: Sun York-R58495
> Cc: u-boot at lists.denx.de; Rivera Jose-B46482
> Subject: Re: [U-Boot] [PATCH 14/28] drivers/fsl-mc: Changed MC firmware
> loading for new boot architecture
>
> On Thu, 19 Mar 2015 09:45:45 -0700
> York Sun <yorksun@freescale.com> wrote:
>
> > From: "J. German Rivera" <German.Rivera@freescale.com>
> >
> > Changed MC firmware loading to comply with the new MC boot
> architecture.
> > Flush D-cache hierarchy after loading MC images. Add environment
> > variables "mcboottimeout" for MC boot timeout in milliseconds,
> > "mcmemsize" for MC DRAM block size. Check MC boot status before
> > calling flib functions.
>
> Can we just assign actual and/or optimal values for 'mcboottimeout'
> and 'mcmemsize' instead of making them environment variables?
>
Having environment variables gives us the flexibility if these
values need to be changed for a given customer configuration. The actual
boot time of the MC and the amount of memory needed by the MC is dependent
on how big/complex is the DPL used. Also, the memory needed by the MC
needs to account for how much memory is needed for AIOP programs,
which may depend on how big/complex they are.

> > +static int wait_for_mc(bool booting_mc, u32 *final_reg_gsr) {
> > + u32 reg_gsr;
> > + u32 mc_fw_boot_status;
> > + unsigned long timeout_ms = get_mc_boot_timeout_ms();
> > + struct mc_ccsr_registers __iomem *mc_ccsr_regs = MC_CCSR_BASE_ADDR;
> > +
> > + dmb();
> > + debug("Polling mc_ccsr_regs->reg_gsr ...\n");
> > + assert(timeout_ms > 0);
> > + for (;;) {
> > + udelay(1000); /* throttle polling */
>
> does this really need to be a whole 1ms?

It is unlikely that the MC fw will boot in less than 1 ms.
So, checking more frequently than 1 ms is not necessary.

>
> > @@ -318,14 +545,28 @@ int get_mc_boot_status(void)
> >
> > /**
> > * Return the actual size of the MC private DRAM block.
> > - *
> > - * NOTE: For now this function always returns the minimum required
> > size,
> > - * However, in the future, the actual size may be obtained from an
> > environment
> > - * variable.
> > */
>
> why?
>
See answer above.

German