[U-Boot] [PATCH] powerpc/p1022ds: boot from SD card/SPI flash with SPL
Zhang Ying-B40530
B40530 at freescale.com
Fri May 10 10:44:29 CEST 2013
This patch needs to be broken into several patches that each take care of one logical problem; it's too hard to properly review (and have the right people pay attention to certain parts) in its current state.
[Zhang Ying]
It only can be broken to two patches, one for SD boot, another for SPI boot.
> diff --git a/arch/powerpc/cpu/mpc85xx/tlb.c
> b/arch/powerpc/cpu/mpc85xx/tlb.c index 0dff37f..d21b324 100644
> --- a/arch/powerpc/cpu/mpc85xx/tlb.c
> +++ b/arch/powerpc/cpu/mpc85xx/tlb.c
> @@ -55,7 +55,7 @@ void init_tlbs(void)
> return ;
> }
>
> -#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_BUILD)
> +#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_BUILD_MINIMAL)
> void read_tlbcam_entry(int idx, u32 *valid, u32 *tsize, unsigned long
> *epn,
> phys_addr_t *rpn)
> {
Aren't you breaking the existing minimal targets?
CONFIG_SPL_BUILD_MINIMAL does not currently exist, and you add it only for P1022DS.
[Zhang Ying]
Yes, it needs to be added for other existing boards that defines CONFIG_SPL_INIT_MINIMAL. It should include p1_p2_rdb_pc.
> @@ -83,5 +107,6 @@ SECTIONS
> *(.sbss*)
> *(.bss*)
> }
> + . = ALIGN(4);
> __bss_end = .;
> }
This seems unrelated.
[Zhang Ying]
It is necessary. In the function clear_bss(), the address of bss is on the basis of the __bss_start, and then to four bytes of incremental growth, finally the last stop is based on the address and __bss_end is equal or not.
> diff --git a/board/freescale/common/sdhc_boot.c
> b/board/freescale/common/sdhc_boot.c
> index e432318..96b0680 100644
> --- a/board/freescale/common/sdhc_boot.c
> +++ b/board/freescale/common/sdhc_boot.c
> + val = *(u16 *)(tmp_buf + ESDHC_BOOT_IMAGE_SIGN_ADDR);
> + if ((u16)ESDHC_BOOT_IMAGE_SIGN != val) {
> + free(tmp_buf);
> + return;
> + }
Why do you need this cast?
[Zhang Ying]
The offset 0x1FE of the config data sector should contain the value 0x55AA. If the value in this location doesn't match 0x55AA, it means that the SD/MMC card doesn't contain a valid user code.
> + offset = *(u32 *)(tmp_buf + ESDHC_BOOT_IMAGE_ADDR);
> + offset += CONFIG_SYS_MMC_U_BOOT_OFFS;
> + /* Get the code size from offset 0x48 */
> + code_len = *(u32 *)(tmp_buf + ESDHC_BOOT_IMAGE_SIZE);
> + code_len -= CONFIG_SYS_MMC_U_BOOT_OFFS;
> + /*
> + * Load U-Boot image from mmc into RAM
> + */
> + /*
> + SDHC card: code offset and length is stored in block units
> rather
> + * than a single byte
> + */
/*
* U-Boot multiline
* comment style is
* like this.
*/
> + blk_start = ALIGN(offset, mmc->read_bl_len) / mmc->read_bl_len;
> + blk_cnt = ALIGN(code_len, mmc->read_bl_len) / mmc->read_bl_len;
> +
> + err = mmc->block_dev.block_read(0, blk_start, blk_cnt,
> + (uchar
> *)CONFIG_SYS_MMC_U_BOOT_DST);
> + if (err != blk_cnt) {
> + free(tmp_buf);
> + return ;
> + }
> +}
> +void mmc_boot(void)
No space before ;
That return is pointless since you're at the end of the function anyway.
[Zhang Ying]
Ok, I can hang the cpu and print the error information.
> + *
> + * 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 __SDHC_BOOT_H_
> +#define __SDHC_BOOT_H_ 1
> +
> +
> +int mmc_get_env_addr(struct mmc *mmc, u32 *env_addr); void
> +mmc_get_env(void); void mmc_boot(void);
> +
> +#endif /* __SDHC_BOOT_H_ */
Does this stuff really belong in board/freescale? Should probably at least be in arch/powerpc/cpu/mpc85xx, if not more generic.
[Zhang Ying]
Ok, whether we can handle like this: sdhc_boot.c and spi_boot.c will be deleted. All this stuff in the sdhc_boot.c will be moved to drivers/mmc/fsl_esdhc.c, and the functions in the spi_boot.c will be moved to drivers/mmc/fsl_espi.c.
> +void hang(void)
> +{
> + puts("### ERROR ### Please RESET the board ###\n");
> + for (;;)
> + ;
> +}
Whitespace
[Zhang Ying]
?
> diff --git a/common/Makefile b/common/Makefile index 0e0fff1..bc80414
> 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -225,6 +225,11 @@ COBJS-$(CONFIG_SPL_NET_SUPPORT) += env_common.o
> COBJS-$(CONFIG_SPL_NET_SUPPORT) += env_flags.o
> COBJS-$(CONFIG_SPL_NET_SUPPORT) += env_nowhere.o
> COBJS-$(CONFIG_SPL_NET_SUPPORT) += miiphyutil.o
> +COBJS-$(CONFIG_SPL_HWCONFIG_SUPPORT) += hwconfig.o
> +COBJS-$(CONFIG_SPL_INIT_DDR_SUPPORT) += ddr_spd.o
> +COBJS-$(CONFIG_SPL_ENV_SUPPORT) += env_attr.o
> +COBJS-$(CONFIG_SPL_ENV_SUPPORT) += env_flags.o
> +COBJS-$(CONFIG_SPL_ENV_SUPPORT) += env_callback.o
CONFIG_SPL_ENV_SUPPORT should replace CONFIG_SPL_NET_SUPPORT here (and add it to the boards that already have CONFIG_SPL_NET_SUPPORT). This sort of refactoring needs to be a separate patch, BTW.
Can ddr_spd.o and hwconfig just use their normal CONFIG symbols (i.e.
move the existing makefile line out of the !SPL ifdef)? It's getting a bit excessive with all the new SPL symbols.
[Zhang Ying]
If do it, the SPL's size will increase. I fear this will affect the other SPL no CONFIG_SPL_NET_SUPPORT is defined.
Maybe one day we can redo it to be a separate config namespace, like I suggested a while ago. :-P It would certainly help with three-stage boot support.
> endif
> COBJS-$(CONFIG_BOUNCE_BUFFER) += bouncebuf.o COBJS-y += console.o
> diff --git a/doc/README.mpc85xx-sd-spi-boot
> b/doc/README.mpc85xx-sd-spi-boot new file mode 100644 index
> 0000000..79f91fc
> --- /dev/null
> +++ b/doc/README.mpc85xx-sd-spi-boot
> @@ -0,0 +1,82 @@
> +----------------------------------------
> +Booting from On-Chip ROM (eSDHC or eSPI)
> +----------------------------------------
> +
> +boot_format is a tool to write SD bootable images to a filesystem
> and build
> +SD/SPI images to a binary file for writing later.
> +
> +When booting from an SD card/MMC, boot_format puts the configuration
> file and
> +the RAM-based U-Boot image on the card.
> +When booting from an EEPROM, boot_format generates a binary image
> that is used
> +to boot from this EEPROM.
> +
> +Where to get boot_format:
> +========================
> +
> +you can browse it online at:
> +http://git.freescale.com/git/cgit.cgi/ppc/sdk/boot-format.git/
> +
> +Building
> +========
> +
> +Run the following to build this project
> +
> + $ make
> +
> +Execution
> +=========
> +
> +boot_format runs under a regular Linux machine and requires a super
> user mode
> +to run. Execute boot_format as follows.
> +
> +For building SD images by writing directly to a file system on SD
> media:
> +
> + $ boot_format $config u-boot.bin -sd $device
> +
> +Where $config is the included config.dat file for your platform and
> $device
> +is the target block device for the SD media on your computer.
> +
> +For build binary images directly a local file:
> +
> + $ boot_format $config u-boot.bin -spi $file
> +
> +Where $file is the target file. Also keep in mind the u-boot.bin
> file needs
> +to be the u-boot built for your particular platform and target media.
> +
> +Hint: To generate a u-boot.bin for a P1022DS booting from SD I would
> run the
> +following in the u-boot repository:
> +
> + $ make P1022DS_SDCARD
s/Hint/Example/ and s/from SD I would run/from SD, run/
[Zhang Ying]
run bootsd? This is another independent requirement. If we want, we can do it in another project.
> diff --git a/lib/Makefile b/lib/Makefile index e901cc7..ab12e63 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -66,6 +66,11 @@ endif
> COBJS-$(CONFIG_SPL_NET_SUPPORT) += errno.o
> COBJS-$(CONFIG_SPL_NET_SUPPORT) += hashtable.o
> COBJS-$(CONFIG_SPL_NET_SUPPORT) += net_utils.o
> +COBJS-$(CONFIG_SPL_ADDR_MAP_SUPPORT) += addr_map.o
> +COBJS-$(CONFIG_SPL_UTILITYLIB_SUPPORT) += hashtable.o
> +COBJS-$(CONFIG_SPL_UTILITYLIB_SUPPORT) += display_options.o
> +COBJS-$(CONFIG_SPL_UTILITYLIB_SUPPORT) += errno.o
As with common/Makefile, Can these just be the normal makefile lines, moved outside the SPL ifdef (and no duplications with CONFIG_SPL_NET_SUPPORT)?
[Zhang Ying]
Mentioned.
-Scott
More information about the U-Boot
mailing list