[U-Boot] [PATCH] spl: consolidate arch/arm/include/asm/arch-*/spl.h

Stefano Babic
2014-04-17T12:59:27Z

Hi Masahiro,

On 16/04/2014 08:44, Masahiro Yamada wrote:
> arch/arm/include/asm/spl.h requires all SoCs to have
> arch/arm/include/asm/arch-*/spl.h.
>
> But many of them just define BOOT_DEVICE_* macros.
>
> Those macros are used in the "switch (boot_device) { ... }"
> statement in common/spl/spl.c.
>
> So they should not be archtecture specific, but described as
> a simpile enumeration.

Absolutely right !

>
> This commit merge most of arch/arm/include/asm/arch-*/spl.h
> into arch/arm/include/asm/spl.h.
>
> With a little more effort, arch-zynq/spl.h and arch-socfpga/spl.h
> will be merged, while I am not sure about OMAP and Exynos.
>
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Cc: Tom Rini <trini@ti.com>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Andreas Bie?mann <andreas.devel@googlemail.com>
> Cc: Stephen Warren <swarren@nvidia.com>
> Cc: Tom Warren <twarren@nvidia.com>
> CC: Stefano Babic <sbabic@denx.de>
> CC: Minkyu Kang <mk7.kang@samsung.com>
> Cc: Dinh Nguyen <dinguyen@altera.com>
> ---
> arch/arm/cpu/arm720t/tegra-common/spl.c | 2 +-
> arch/arm/include/asm/arch-at91/spl.h | 24 ------------------------
> arch/arm/include/asm/arch-davinci/spl.h | 16 ----------------
> arch/arm/include/asm/arch-mx35/spl.h | 22 ----------------------
> arch/arm/include/asm/arch-mx5/spl.h | 13 -------------
> arch/arm/include/asm/arch-tegra114/spl.h | 22 ----------------------
> arch/arm/include/asm/arch-tegra124/spl.h | 13 -------------
> arch/arm/include/asm/arch-tegra20/spl.h | 12 ------------
> arch/arm/include/asm/arch-tegra30/spl.h | 12 ------------
> arch/arm/include/asm/spl.h | 18 ++++++++++++++++++
> board/denx/m53evk/m53evk.c | 2 +-
> 11 files changed, 20 insertions(+), 136 deletions(-)
> delete mode 100644 arch/arm/include/asm/arch-at91/spl.h
> delete mode 100644 arch/arm/include/asm/arch-davinci/spl.h
> delete mode 100644 arch/arm/include/asm/arch-mx35/spl.h
> delete mode 100644 arch/arm/include/asm/arch-mx5/spl.h
> delete mode 100644 arch/arm/include/asm/arch-tegra114/spl.h
> delete mode 100644 arch/arm/include/asm/arch-tegra124/spl.h
> delete mode 100644 arch/arm/include/asm/arch-tegra20/spl.h
> delete mode 100644 arch/arm/include/asm/arch-tegra30/spl.h
>
> diff --git a/arch/arm/cpu/arm720t/tegra-common/spl.c b/arch/arm/cpu/arm720t/tegra-common/spl.c
> index 5171a8f..8147806 100644
> --- a/arch/arm/cpu/arm720t/tegra-common/spl.c
> +++ b/arch/arm/cpu/arm720t/tegra-common/spl.c
> @@ -14,7 +14,7 @@
> #include <asm/arch/pinmux.h>
> #include <asm/arch/tegra.h>
> #include <asm/arch-tegra/board.h>
> -#include <asm/arch/spl.h>
> +#include <asm/spl.h>
> #include "cpu.h"
>
> void spl_board_init(void)
> diff --git a/arch/arm/include/asm/arch-at91/spl.h b/arch/arm/include/asm/arch-at91/spl.h
> deleted file mode 100644
> index d8a87da..0000000
> --- a/arch/arm/include/asm/arch-at91/spl.h
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/*
> - * Copyright (C) 2013 Atmel Corporation
> - * Bo Shen <voice.shen@atmel.com>
> - *
> - * SPDX-License-Identifier: GPL-2.0+
> - */
> -
> -#ifndef _ASM_ARCH_SPL_H_
> -#define _ASM_ARCH_SPL_H_
> -
> -enum {
> - BOOT_DEVICE_NONE,
> -#ifdef CONFIG_SYS_USE_MMC
> - BOOT_DEVICE_MMC1,
> - BOOT_DEVICE_MMC2,
> - BOOT_DEVICE_MMC2_2,
> -#elif CONFIG_SYS_USE_NANDFLASH
> - BOOT_DEVICE_NAND,
> -#elif CONFIG_SYS_USE_SERIALFLASH
> - BOOT_DEVICE_SPI,
> -#endif
> -};
> -
> -#endif
> diff --git a/arch/arm/include/asm/arch-davinci/spl.h b/arch/arm/include/asm/arch-davinci/spl.h
> deleted file mode 100644
> index 5afe0d4..0000000
> --- a/arch/arm/include/asm/arch-davinci/spl.h
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -/*
> - * (C) Copyright 2012
> - * Texas Instruments, <www.ti.com>
> - *
> - * SPDX-License-Identifier: GPL-2.0+
> - */
> -#ifndef _ASM_ARCH_SPL_H_
> -#define _ASM_ARCH_SPL_H_
> -
> -#define BOOT_DEVICE_NAND 1
> -#define BOOT_DEVICE_SPI 2
> -#define BOOT_DEVICE_MMC1 3
> -#define BOOT_DEVICE_MMC2 4 /* dummy */
> -#define BOOT_DEVICE_MMC2_2 5 /* dummy */
> -
> -#endif
> diff --git a/arch/arm/include/asm/arch-mx35/spl.h b/arch/arm/include/asm/arch-mx35/spl.h
> deleted file mode 100644
> index d0efec2..0000000
> --- a/arch/arm/include/asm/arch-mx35/spl.h
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -/*
> - * (C) Copyright 2012
> - * Texas Instruments, <www.ti.com>
> - *
> - * SPDX-License-Identifier: GPL-2.0+
> - */
> -#ifndef _ASM_ARCH_SPL_H_
> -#define _ASM_ARCH_SPL_H_
> -
> -#define BOOT_DEVICE_NONE 0
> -#define BOOT_DEVICE_XIP 1
> -#define BOOT_DEVICE_XIPWAIT 2
> -#define BOOT_DEVICE_NAND 3
> -#define BOOT_DEVICE_ONENAND 4
> -#define BOOT_DEVICE_MMC1 5
> -#define BOOT_DEVICE_MMC2 6
> -#define BOOT_DEVICE_MMC2_2 7
> -#define BOOT_DEVICE_NOR 8
> -#define BOOT_DEVICE_I2C 9
> -#define BOOT_DEVICE_SPI 10
> -
> -#endif
> diff --git a/arch/arm/include/asm/arch-mx5/spl.h b/arch/arm/include/asm/arch-mx5/spl.h
> deleted file mode 100644
> index 20c6cae..0000000
> --- a/arch/arm/include/asm/arch-mx5/spl.h
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -/*
> - * Copyright (C) 2013 Marek Vasut <marex@denx.de>
> - *
> - * SPDX-License-Identifier: GPL-2.0+
> - */
> -
> -#ifndef __ASM_ARCH_SPL_H__
> -#define __ASM_ARCH_SPL_H__
> -
> -#define BOOT_DEVICE_NONE 0
> -#define BOOT_DEVICE_NAND 1
> -
> -#endif /* __ASM_ARCH_SPL_H__ */
> diff --git a/arch/arm/include/asm/arch-tegra114/spl.h b/arch/arm/include/asm/arch-tegra114/spl.h
> deleted file mode 100644
> index ebb16fe..0000000
> --- a/arch/arm/include/asm/arch-tegra114/spl.h
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -/*
> - * Copyright (c) 2010-2013, NVIDIA CORPORATION. All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms and conditions of the GNU General Public License,
> - * version 2, as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope 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, see <http://www.gnu.org/licenses/>.
> - */
> -
> -#ifndef _ASM_ARCH_SPL_H_
> -#define _ASM_ARCH_SPL_H_
> -
> -#define BOOT_DEVICE_RAM 1
> -
> -#endif
> diff --git a/arch/arm/include/asm/arch-tegra124/spl.h b/arch/arm/include/asm/arch-tegra124/spl.h
> deleted file mode 100644
> index e266395..0000000
> --- a/arch/arm/include/asm/arch-tegra124/spl.h
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -/*
> - * (C) Copyright 2010-2013
> - * NVIDIA Corporation <www.nvidia.com>
> - *
> - * SPDX-License-Identifier: GPL-2.0+
> - */
> -
> -#ifndef _ASM_ARCH_SPL_H_
> -#define _ASM_ARCH_SPL_H_
> -
> -#define BOOT_DEVICE_RAM 1
> -
> -#endif /* _ASM_ARCH_SPL_H_ */
> diff --git a/arch/arm/include/asm/arch-tegra20/spl.h b/arch/arm/include/asm/arch-tegra20/spl.h
> deleted file mode 100644
> index 8953b00..0000000
> --- a/arch/arm/include/asm/arch-tegra20/spl.h
> +++ /dev/null
> @@ -1,12 +0,0 @@
> -/*
> - * (C) Copyright 2012
> - * NVIDIA Corporation <www.nvidia.com>
> - *
> - * SPDX-License-Identifier: GPL-2.0+
> - */
> -#ifndef _ASM_ARCH_SPL_H_
> -#define _ASM_ARCH_SPL_H_
> -
> -#define BOOT_DEVICE_RAM 1
> -
> -#endif
> diff --git a/arch/arm/include/asm/arch-tegra30/spl.h b/arch/arm/include/asm/arch-tegra30/spl.h
> deleted file mode 100644
> index 8953b00..0000000
> --- a/arch/arm/include/asm/arch-tegra30/spl.h
> +++ /dev/null
> @@ -1,12 +0,0 @@
> -/*
> - * (C) Copyright 2012
> - * NVIDIA Corporation <www.nvidia.com>
> - *
> - * SPDX-License-Identifier: GPL-2.0+
> - */
> -#ifndef _ASM_ARCH_SPL_H_
> -#define _ASM_ARCH_SPL_H_
> -
> -#define BOOT_DEVICE_RAM 1
> -
> -#endif
> diff --git a/arch/arm/include/asm/spl.h b/arch/arm/include/asm/spl.h
> index 90e5a9d..2e94a09 100644
> --- a/arch/arm/include/asm/spl.h
> +++ b/arch/arm/include/asm/spl.h
> @@ -7,9 +7,27 @@
> #ifndef _ASM_SPL_H_
> #define _ASM_SPL_H_
>
> +#if defined(CONFIG_OMAP) || defined(CONFIG_SOCFPGA) || defined(CONFIG_ZYNQ) \
> + || defined(CONFIG_EXYNOS4) || defined(CONFIG_EXYNOS5)
> /* Platform-specific defines */
> #include <asm/arch/spl.h>
>
> +#else
> +enum {
> + BOOT_DEVICE_RAM,
> + BOOT_DEVICE_MMC1,
> + BOOT_DEVICE_MMC2,
> + BOOT_DEVICE_MMC2_2,
> + BOOT_DEVICE_NAND,
> + BOOT_DEVICE_ONENAND,
> + BOOT_DEVICE_NOR,
> + BOOT_DEVICE_UART,
> + BOOT_DEVICE_SPI,
> + BOOT_DEVICE_I2C,
> + BOOT_DEVICE_NONE
> +};
> +#endif
> +
> /* Linker symbols. */
> extern char __bss_start[], __bss_end[];
>
> diff --git a/board/denx/m53evk/m53evk.c b/board/denx/m53evk/m53evk.c
> index 74f9501..5dd6cdd 100644
> --- a/board/denx/m53evk/m53evk.c
> +++ b/board/denx/m53evk/m53evk.c
> @@ -14,7 +14,7 @@
> #include <asm/arch/clock.h>
> #include <asm/arch/iomux-mx53.h>
> #include <asm/imx-common/mx5_video.h>
> -#include <asm/arch/spl.h>
> +#include <asm/spl.h>
> #include <asm/errno.h>
> #include <netdev.h>
> #include <i2c.h>
>

Acked-by: Stefano Babic <sbabic@denx.de>

By the way, an issue we have with the current implementation is that we
need to add a further enum for each instance of the same boot device
type. You can see with BOOT_DEVICE_MMC1, BOOT_DEVICE_MMC2,
BOOT_DEVICE_MMC2_2. I will presume we will have the same with SPI and
I2C, more uncommon with NAND. What about (I am OT here, it should be
done in a separate patch) if we will change spl_boot_device(), that now
returns this enum, combining in the return value the instance of the
device ? (Example: (BOOT_DEVICE_MMC << 16 | 2) for MMC2)

Best regards,
Stefano

--
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
=====================================================================