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

[Jose Rivera]

> -----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 at freescale.com> wrote:
> 
> > From: "J. German Rivera" <German.Rivera at 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