[U-Boot] [PATCH 6/8] powerpc/boot: Slave uploads ucode when boot from SRIO

[Wolfgang Denk]

Dear Liu Gang,

In message <1326195751-20729-6-git-send-email-Gang.Liu at freescale.com> you wrote:
> When boot from SRIO, slave's ucode can be stored in master's memory space,
> then slave can fetch the ucode image through SRIO interface.
...
> +	/* configure inbound window for slave's ucode */
> +	printf("SRIOBOOT - MASTER: Inbound window for slave's ucode; "
> +			"Local = 0x%llx, Srio = 0x%llx, Size = 0x%x\n",
> +			(u64)CONFIG_SRIOBOOT_SLAVE_UCODE_LAW_PHYS,
> +			(u64)CONFIG_SRIOBOOT_SLAVE_UCODE_SRIO_PHYS,
> +			CONFIG_SRIOBOOT_SLAVE_UCODE_SIZE);

Is this really needed in production code?  Or should this be changed
into a debug() ?

> +	out_be32((u32)&srio->riwtar3 + CONFIG_SRIOBOOT_MASTER_PORT * 0x200,
> +			CONFIG_SRIOBOOT_SLAVE_UCODE_LAW_PHYS >> 12);
> +	out_be32((u32)&srio->riwbar3 + CONFIG_SRIOBOOT_MASTER_PORT * 0x200,
> +			CONFIG_SRIOBOOT_SLAVE_UCODE_SRIO_PHYS >> 12);

NAK.  We don't allow base address + offset notation.  Please use
proper C structs instead.

Please fix globally.

> +	out_be32((u32)&srio->riwar3 + CONFIG_SRIOBOOT_MASTER_PORT * 0x200,
> +			0x80f55000

Please don't hard code magic numbers.

Please fix globally.

> +#ifdef CONFIG_SRIOBOOT_SLAVE
> +	/*
> +	 * *I*G - SRIOBOOT-SLAVE. 1M space from 0xffe00000 for fetching ucode
> +	 * and ENV from master
> +	 */

What is this "*I*G - " doing here?

> +/*
> + *for slave UCODE instored in master memory space,
> + *PHYS must be aligned based on the SIZE
> + */

Please add a space between the '*' anf the text.

Please fix globally.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Given a choice between two theories, take the one which is funnier.