[U-Boot] [PATCH] powerpc/CoreNet: add tool to support pbl image build.

[Scott Wood <scottwood@freescale.com>]

On 06/04/2012 03:57 AM, Shaohui Xie wrote:
> From: Shaohui Xie <b21989@freescale.com>
> 
> Provides a tool to build boot Image for PBL(Pre boot loader) which is
> used on Freescale CoreNet SoCs, PBL can be used to load some instructions
> and/or data for pre-initialization. The default output image is u-boot.pbl,
> for more details please refer to doc/README.pblimage.
> 
> Signed-off-by: Shaohui Xie <b21989@freescale.com>
> ---
> rebased to lasted tree.
> 
>  Makefile                                |    5 +
>  board/freescale/corenet_ds/config.mk    |   26 +++
>  board/freescale/corenet_ds/pblimage.cfg |   60 ++++++
>  common/image.c                          |    1 +
>  doc/README.pblimage                     |  140 +++++++++++++
>  include/image.h                         |    1 +
>  tools/Makefile                          |    2 +
>  tools/mkimage.c                         |    5 +
>  tools/mkimage.h                         |    2 +
>  tools/pblimage.c                        |  329 +++++++++++++++++++++++++++++++
>  tools/pblimage.h                        |   36 ++++
>  11 files changed, 607 insertions(+), 0 deletions(-)
>  create mode 100644 board/freescale/corenet_ds/config.mk
>  create mode 100644 board/freescale/corenet_ds/pblimage.cfg
>  create mode 100644 doc/README.pblimage
>  create mode 100644 tools/pblimage.c
>  create mode 100644 tools/pblimage.h
> 
> diff --git a/Makefile b/Makefile
> index 57ad45b..99f993a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -416,6 +416,10 @@ $(obj)u-boot.kwb:       $(obj)u-boot.bin
>  		$(obj)tools/mkimage -n $(CONFIG_SYS_KWD_CONFIG) -T kwbimage \
>  		-a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_TEXT_BASE) -d $< $@
*>
> +$(obj)u-boot.pbl:       $(obj)u-boot.bin
> +		$(obj)tools/mkimage -n $(CONFIG_PBL_CONFIG) -T pblimage \
> +		-d $< $@
> +
>  $(obj)u-boot.sha1:	$(obj)u-boot.bin
>  		$(obj)tools/ubsha1 $(obj)u-boot.bin
>  
> @@ -773,6 +777,7 @@ clobber:	tidy
>  		$(obj)cscope.* $(obj)*.*~
>  	@rm -f $(obj)u-boot $(obj)u-boot.map $(obj)u-boot.hex $(ALL-y)
>  	@rm -f $(obj)u-boot.kwb
> +	@rm -f $(obj)u-boot.pbl
>  	@rm -f $(obj)u-boot.imx
>  	@rm -f $(obj)u-boot.ubl
>  	@rm -f $(obj)u-boot.ais
> diff --git a/board/freescale/corenet_ds/config.mk b/board/freescale/corenet_ds/config.mk
> new file mode 100644
> index 0000000..72464dc
> --- /dev/null
> +++ b/board/freescale/corenet_ds/config.mk
> @@ -0,0 +1,26 @@
> +#
> +# Copyright 2012 Freescale Semiconductor, Inc.
> +#
> +# See file CREDITS for list of people who contributed to this
> +# project.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> +# MA 02111-1307 USA
> +#
> +
> +ifeq ($(CONFIG_RAMBOOT_PBL), y)
> +CONFIG_PBL_CONFIG = $(SRCTREE)/$(CONFIG_BOARDDIR)/pblimage.cfg
> +ALL-y += $(obj)u-boot.pbl
> +endif

Why is this specific to corenet_ds?

> diff --git a/board/freescale/corenet_ds/pblimage.cfg b/board/freescale/corenet_ds/pblimage.cfg
> new file mode 100644
> index 0000000..898fe6d
> --- /dev/null
> +++ b/board/freescale/corenet_ds/pblimage.cfg
> @@ -0,0 +1,60 @@
> +#
> +# Copyright 2012 Freescale Semiconductor, Inc.
> +# Written-by: Shaohui Xie<b21989@freescale.com>
> +#
> +# See file CREDITS for list of people who contributed to this
> +# project.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA 02110-1301 USA
> +#
> +# Refer docs/README.pblimage for more details about how-to configure
> +# and create PBL boot image
> +#
> +
> +#PBL preamble and RCW header
> +aa55aa55 010e0100
> +#64 bytes RCW data for P4080, replace it when building image
> +#for P3041DS or P5020DS.
> +4c580000 00000000 18185218 0000cccc
> +40464000 3c3c2000 58000000 61000000
> +00000000 00000000 00000000 008b6000
> +00000000 00000000 00000000 00000000

Could you have the tool source this from a separate file, rather than
require the user to replace it manually?

Talk to Timur (when he gets back from vacation in a couple weeks) about
his RCW tool and how best to accept the output it produces.

> +#PBI commands
> +#Initialize CPC1
> +09010000 00200400
> +09138000 00000000
> +091380c0 00000100
> +09010100 00000000
> +09010104 fff0000b
> +09010f00 08000000
> +09010000 80000000
> +#Configure LAW for CPC1
> +09000d00 00000000
> +09000d04 fff00000
> +09000d08 81000013
> +09000010 00000000
> +09000014 ff000000
> +09000018 81000000
> +#Initialize eSPI controller
> +09110000 80000403
> +09110020 2d170008
> +09110024 00100008
> +09110028 00100008
> +0911002c 00100008
> +#Flush PBL data
> +09138000 00000000
> +091380c0 00000000

Why is eSPI in here?  Isn't this supposed to just generically write an
image into CPC SRAM?

> diff --git a/doc/README.pblimage b/doc/README.pblimage
> new file mode 100644
> index 0000000..73d90f1
> --- /dev/null
> +++ b/doc/README.pblimage
> @@ -0,0 +1,140 @@
> +------------------------------------------------------------------
> +Freescale PBL(pre-boot loader) Boot Image generation using mkimage
> +------------------------------------------------------------------
> +
> +This document describes the U-Boot feature as it
> +is implemented for the Freescale CoreNet SoC.
> +
> +The CoreNet SoC's can boot directly from eSPI FLASH, SD/MMC.
> +
> +for more details refer section 5 Pre-boot loader specifications.
> +
> +Building PBL Boot Image and boot steps
> +--------------------------------------
> +
> +1. Building PBL Boot Image.
> +   The default Image is u-boot.pbl. Before build the iamge, make sure
> +   rcw data in pblimage.cfg is suitable for the platform, if not, this
> +   need to be fixed in step 2.
> +
> +   For eSPI boot:
> +	To build the eSPI boot image for P4080DS:
> +	make P4080DS_SPIFLASH
> +
> +	To build the eSPI boot image for P3041DS:
> +	make P3041DS_SPIFLASH
> +
> +	To build the eSPI boot image for P5020DS:
> +	make P5020DS_SPIFLASH
> +
> +   For SD boot:
> +	To build the SD boot image for P4080DS:
> +	make P4080DS_SDCARD
> +
> +	To build the SD boot image for P3041DS:
> +	make P3041DS_SDCARD
> +
> +	To build the SD boot image for P5020DS:
> +	make P5020DS_SDCARD
> +
> +   For Nand boot(P4080DS does not have Nand chip):
> +	To build the PBL boot image for P3041DS:
> +	make P3041DS_NAND
> +
> +	To build the PBL boot image for P5020DS:
> +	make P5020DS_NAND


> +
> +
> +2. Command below provided a way to re-build the PBL boot image if the
> +configuration file needes to be modified while the u-boot.bin does not
> +need to be re-build.
> +
> +Command syntax:
> +--------------
> +./tools/mkimage -n <board specific configuration file> \
> +		-T pblimage -d <input_raw_binary> <output_pblboot_file>
> +
> +for ex.
> +./tools/mkimage -n ./board/freescale/corenet_ds/pblimage.cfg \
> +		-T pblimage -d u-boot.bin u-boot.pbl
> +
> +
> +3. pblimage support available with mkimage utility will generate Freescale PBL
> +boot image that can be flashed on the board eSPI flash, SD/MMC. Following steps
> +described how to boot from eSPI flash, SD/MMC.
> +
> +	1). Boot from eSPI flash
> +	Write u-boot.pbl to eSPI flash from offset 0x0.
> +	for ex in u-boot:
> +	=>tftp 100000 u-boot.pbl
> +	=>sf probe 0
> +	=>sf erase 0 100000
> +	=>sf write 100000 0 $filesize
> +	Change SW1[1:5] = off off on off on.
> +
> +	2). Boot from SD/MMC
> +	Write u-boot.pbl to SD/MMC from offset 0x1000.
> +	for ex in u-boot:
> +	=>tftp 100000 u-boot.pbl
> +	=>mmcinfo
> +	=>mmc write 100000 8 441
> +	Change SW1[1:5] = off off on on off.
> +
> +	3). Boot from Nand
> +	Write u-boot.pbl to Nand from offset 0x0, Note that in case of eLBC NAND
> +	flash, the address starts from the first good block.
> +	for ex in u-boot:
> +	=>tftp 100000 u-boot.pbl
> +	=>nand info
> +	=>nand erase 0 100000
> +	=>nand write 100000 0 $filesize
> +	Change SW1[1:5] = off on off off on
> +	Change SW7[1:4] = on off off on

How do you load the environment?  We should find a way, possibly using
SPL, to have the environment ready early.  We do not want to wait post
relocation for the full NAND/SD/SPI driver to load the environment.

> diff --git a/tools/mkimage.h b/tools/mkimage.h
> index 5fe1a48..a886305 100644
> --- a/tools/mkimage.h
> +++ b/tools/mkimage.h
> @@ -147,6 +147,8 @@ void mkimage_register (struct image_type_params *tparams);
>   *
>   * Supported image types init functions
>   */
> +void pbl_load_uboot(int, struct mkimage_params *);

Please provide names for parameters.

> +/*
> + * PBL can load 64 bytes each time in MAX, so the u-boot need to be splited into
> + * pieces of 64 bytes, PBL needs a command for each piece, the command looks
> + * like 81xxxxxx, the "xxxxxx" is offset, it starts from F80000 in our case.
> + */

Better wording:

  The PBL can load up to 64 bytes at a time, so we split the U-Boot
  image into 64 byte chunks.  PBL needs a command for each piece, of
  the form "81xxxxxx", where "xxxxxx" is the offset.

Why are we hardcoding 0xf80000?  Where does it come from?

> +static unsigned int uboot_label = 0x81F80000;

I'm not sure that label is the right word here.  Maybe "next_pbl_cmd"?

> +/*
> + * need to store all bytes in memory for calculating crc32, then write the
> + * bytes to image file for PBL boot.
> + */
> +static unsigned char mem_buf[600000];

600000 is arbitrary.  Do you have any checking if the image size exceeds
this?  Can you dynamically allocate the buffer instead?

> +static unsigned char *pmem_buf = mem_buf;
> +static int mem_byte_cnt;

s/mem_byte_cnt/pbl_size/

> +static char *fname = "Unknown";
> +static int lineno = -1;
> +static struct pbl_header pblimage_header;
> +
> +static union
> +{
> +	  char c[4];
> +	    unsigned char l;
> +} endian_test = { {'l', '?', '?', 'b'} };

Please fix whitespace.

> +#define ENDIANNESS ((char)endian_test.l)
> +
> +static void generate_pbl_cmd(void)
> +{
> +	unsigned int val = uboot_label;

s/unsigned int/uint32_t/

> +	uboot_label += 0x40;
> +	int i;
> +
> +	for (i = 3; i >= 0; i--) {
> +		*pmem_buf++ = (val >> (i * 8)) & 0xff;
> +		mem_byte_cnt++;
> +	}
> +}
> +
> +static void pbl_fget(size_t size, FILE *stream)
> +{
> +	unsigned char c;
> +	while (size) {
> +		c = (unsigned char)fgetc(stream);

Check for fgetc() returning EOF before converting to unsigned char.

> +/* load splited u-boot with PBI command 81xxxxxx. */

s/splited/split/

> +static void pbl_parser(char *name)
> +{
> +	FILE *fd = NULL;
> +	char *line = NULL;
> +	char *token, *saveptr1, *saveptr2;
> +	size_t len = 0;
> +
> +	fname = name;
> +	fd = fopen(name, "r");
> +	if (fd == NULL) {
> +		printf("Error:%s - Can't open\n", fname);
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	while ((getline(&line, &len, fd)) > 0) {
> +		lineno++;
> +		token = strtok_r(line, "\r\n", &saveptr1);

Why do you need to tokenize if you're using getline()?

> +static uint32_t crc_table[256];
> +
> +static void make_crc_table(void)
> +{
> +	uint32_t mask;
> +	int i, j;
> +	uint32_t poly; /* polynomial exclusive-or pattern */
> +
> +	/*
> +	 * the polynomial used by PBL is 1 + x1 + x2 + x4 + x5 + x7 + x8 + x10
> +	 * + x11 + x12 + x16 + x22 + x23 + x26 + x32.
> +	 */
> +	poly = 0x04c11db7;
> +
> +	for (i = 0; i < 256; i++) {
> +		mask = i << 24;
> +		for (j = 0; j < 8; j++) {
> +			if (mask & 0x80000000)
> +				mask = (mask << 1) ^ poly;
> +			else
> +				mask <<= 1;
> +		}
> +		crc_table[i] = mask;
> +	}
> +}
> +
> +unsigned long pbl_crc32(unsigned long crc, const char *buf, unsigned int len)
> +{
> +	unsigned int crc32_val = 0xffffffff;
> +	unsigned int xor = 0x0;
> +	int i;
> +
> +	make_crc_table();
> +
> +	for (i = 0; i < len; i++)
> +		crc32_val = (crc32_val << 8) ^
> +			crc_table[(crc32_val >> 24) ^ (*buf++ & 0xff)];
> +
> +	crc32_val = crc32_val ^ xor;
> +	if (crc32_val < 0) {
> +		crc32_val += 0xffffffff;
> +		crc32_val += 1;
> +	}
> +	return crc32_val;
> +}

Is this the standard CRC32 function (which we should be able to pull in
from elsewhere instead of reimplementing here) or is it something
different (which deserves a comment)?

> +static unsigned int reverse_byte(unsigned int val)
> +{
> +	unsigned int temp;
> +	unsigned char *p1;
> +	int j;
> +
> +	temp = val;
> +	p1 = (unsigned char *)&temp;
> +	for (j = 3; j >= 0; j--)
> +		*p1++ = (val >> (j * 8)) & 0xff;
> +	return temp;
> +}
> +
> +/* write end command and crc command to memory. */
> +static void add_end_cmd(void)
> +{
> +	unsigned int pbl_end_cmd[4] = {0x09138000, 0x00000000,
> +		0x091380c0, 0x00000000};

s/unsigned int/uint32_t/

> +	unsigned int crc32_pbl;
> +	int i;
> +	unsigned char *p = (unsigned char *)&pbl_end_cmd;
> +
> +	if (ENDIANNESS == 'l') {
> +		for (i = 0; i < 4; i++)
> +			pbl_end_cmd[i] = reverse_byte(pbl_end_cmd[i]);
> +	}

Can you use htonl() or similar, to avoid the endian test?  Or have a
function to write out a 32-bit word by bytes, similar to what you
open-code in generate_pbl_cmd().


> +	for (i = 0; i < 16; i++) {
> +		*pmem_buf++ = *p++;
> +		mem_byte_cnt++;
> +	}

memcpy()?

> +	if (((mem_byte_cnt) % 16) != 0) {

Unnecessary parentheses.

> +		for (i = 0; i < 8; i++) {
> +			*pmem_buf++ = 0x0;
> +			mem_byte_cnt++;
> +		}
> +	}
> +	if ((mem_byte_cnt % 16 != 0)) {
> +		printf("Error: Bad size of image file\n");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +}

No blank line at end of block.

> diff --git a/tools/pblimage.h b/tools/pblimage.h
> new file mode 100644
> index 0000000..887d4c9
> --- /dev/null
> +++ b/tools/pblimage.h
> @@ -0,0 +1,36 @@
> +/*
> + * Copyright 2012 Freescale Semiconductor, Inc.
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#ifndef _PBLIMAGE_H_
> +#define _PBLIMAGE_H_

s/_PBLIMAGE_H_/PBLIMAGE_H/

Leading underscores are reserved to the C implementation when followed
by an uppercase letter or another underscore, or when in file scope.

I know most of the other tools/ headers do this, but it's wrong.

-Scott