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

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.

Master needs to:
	1. Put the slave's ucode image into it's own memory space.
	2. Set an inbound SRIO window covered slave's ucode stored in master's
	   memory space.
Slave needs to:
	1. Set a specific TLB entry in order to fetch ucode from master.
	2. Set a LAW entry with the TargetID SRIO1 or SRIO2 for ucode.

Signed-off-by: Liu Gang <Gang.Liu@freescale.com>
Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
---
 arch/powerpc/cpu/mpc8xxx/srio.c        |   14 ++++++++++++++
 board/freescale/common/p_corenet/law.c |    4 ++++
 board/freescale/common/p_corenet/tlb.c |   10 ++++++++++
 include/configs/corenet_ds.h           |   17 ++++++++++++++++-
 4 files changed, 44 insertions(+), 1 deletions(-)

On Tue, Jan 10, 2012 at 5:42 AM, Liu Gang <Gang.Liu@freescale.com> 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.

What kind of ucode is this?  Fman or QE?

> +
> +       /* 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);
> +       out_be32((u32)&srio->riwtar3 + CONFIG_SRIOBOOT_MASTER_PORT * 0x200,

These should be "(void *)" instead of "(u32)".

-----Original Message-----
From: Tabi Timur-B04825 
Sent: Wednesday, January 11, 2012 1:03 AM
To: Liu Gang-B34182
Cc: u-boot@lists.denx.de; Alexandre.Bounine@idt.com; Gala Kumar-B11780; Zang Roy-R61911; Xie Shaohui-B21989
Subject: Re: [U-Boot] [PATCH 6/8] powerpc/boot: Slave uploads ucode when boot from SRIO

On Tue, Jan 10, 2012 at 5:42 AM, Liu Gang <Gang.Liu@freescale.com> 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.

What kind of ucode is this?  Fman or QE?
[Liu Gang-B34182] Right now the ucode is for Fman.

> +
> +       /* 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);
> +       out_be32((u32)&srio->riwtar3 + CONFIG_SRIOBOOT_MASTER_PORT * 
> + 0x200,

These should be "(void *)" instead of "(u32)".
[Liu Gang-B34182] Thanks very much!

--
Timur Tabi
Linux kernel developer at Freescale

Dear Liu Gang,

In message <1326195751-20729-6-git-send-email-Gang.Liu@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

Dear Wolfgang,

On Wed, 2012-01-11 at 08:23 +0100, Wolfgang Denk wrote:
> > +#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?

This means that the TLB entry will be set with attribute "MAS2_I"
and "MAS2_G".

This follows the existing style of the file.

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

Thanks, will add.

Best Regards,

Liu Gang