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

[Kim Phillips]

On Mon, 23 Mar 2015 15:06:11 -0500
Rivera Jose-B46482 <German.Rivera at freescale.com> wrote:

> > From: Kim Phillips [mailto:kim.phillips at freescale.com]
> > Sent: Thursday, March 19, 2015 12:53 PM
> > 
> > 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

what defines a 'customer configuration,' and how does that manifest
itself at u-boot boot time?  Is it the amount of time it takes to
load (and execute?) firmare?  Why isn't customer-specific firmware
being loaded via linux?  All u-boot needs is basic networking,
pretty static setup: fixed numbers for both memsize & timeout.

> 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. 

ok, that helps (modulo not knowing what 'DPL' is), but still, the
massive customer configurations should be being loaded via linux'
firmware loading infrastructure: u-boot should be using a static
image for u-boot's needs.

> > > +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. 

is wait_for_mc() only called for the boot command, or all
commands?

> So, checking more frequently than 1 ms is not necessary.

yes it is, because e.g., if it takes 1.1ms we will have wasted 0.9ms
on this.

Kim