[U-Boot] [PATCH 4/8] powerpc/boot: Master module for boot from SRIO

Liu Gang Gang.Liu at freescale.com
Fri Jan 13 07:46:10 CET 2012


Dear Wolfgang,

On Wed, 2012-01-11 at 08:31 +0100, Wolfgang Denk wrote:
> > 	3. Normally boot from local Nor flash.
> 
> Please use "NOR flash" (or "nor flash", if you insist). "Nor" makes
> no sense. Please fix globally.

Thanks, will modify.

> > +	printf("SRIOBOOT - MASTER: Master port [ %d ] for srio boot.\n",
> > +			CONFIG_SRIOBOOT_MASTER_PORT);
> > +	/* configure inbound window1 for slave's u-boot image */
> > +	printf("SRIOBOOT - MASTER: Inbound window1 for slave's image; "
> > +			"Local = 0x%llx, Srio = 0x%llx, Size = 0x%x\n",
> > +			(u64)CONFIG_SRIOBOOT_SLAVE_IMAGE_LAW_PHYS1,
> > +			(u64)CONFIG_SRIOBOOT_SLAVE_IMAGE_SRIO_PHYS1,
> > +			CONFIG_SRIOBOOT_SLAVE_IMAGE_SIZE);
> 
> As mentioned before, this looks a lot like debug code, that should be
> removed from a production version.  Use debug() instead?

These parameters are very important for the boot from srio, for the
different productions may should be different values. So I think that
would be better to keep these informations. I'll use debug() instead!

> This comment applies to the whole patch series:
> 
> - Get rid of the base address + oofset notation.  User C structs
>   instead.
> - Get rid of hard coded magic numbers. #define the needed values in a
>   readable way.

Thanks, will modify.

Best Regards,

Liu Gang




More information about the U-Boot mailing list