[U-Boot] [PATCH 1/6] ARM: keystone2: Split monitor code / command code

When we switch to including all linker lists in U-Boot it is important to not include commands as that may lead to link errors due to other things we have already discarded. In this case, we split the code for supporting the monitor out from the code for loading it.
Cc: Vitaly Andrianov vitalya@ti.com Cc: Nishanth Menon nm@ti.com Cc: Lokesh Vutla lokeshvutla@ti.com Signed-off-by: Tom Rini trini@konsulko.com --- arch/arm/mach-keystone/Makefile | 3 ++ arch/arm/mach-keystone/cmd_mon.c | 51 +---------------------- arch/arm/mach-keystone/include/mach/mon.h | 6 ++- arch/arm/mach-keystone/mon.c | 63 +++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 52 deletions(-) create mode 100644 arch/arm/mach-keystone/mon.c
diff --git a/arch/arm/mach-keystone/Makefile b/arch/arm/mach-keystone/Makefile index 9713fe4..7f12009 100644 --- a/arch/arm/mach-keystone/Makefile +++ b/arch/arm/mach-keystone/Makefile @@ -8,8 +8,11 @@ obj-y += init.o obj-y += psc.o obj-y += clock.o +obj-y += mon.o +ifndef CONFIG_SPL_BUILD obj-y += cmd_clock.o obj-y += cmd_mon.o +endif obj-y += msmc.o obj-y += ddr3.o cmd_ddr3.o obj-y += keystone.o diff --git a/arch/arm/mach-keystone/cmd_mon.c b/arch/arm/mach-keystone/cmd_mon.c index a539d5d..6a9bdc9 100644 --- a/arch/arm/mach-keystone/cmd_mon.c +++ b/arch/arm/mach-keystone/cmd_mon.c @@ -9,25 +9,9 @@
#include <common.h> #include <command.h> +#include <mach/mon.h> asm(".arch_extension sec\n\t");
-static int mon_install(u32 addr, u32 dpsc, u32 freq) -{ - int result; - - __asm__ __volatile__ ( - "stmfd r13!, {lr}\n" - "mov r0, %1\n" - "mov r1, %2\n" - "mov r2, %3\n" - "blx r0\n" - "ldmfd r13!, {lr}\n" - : "=&r" (result) - : "r" (addr), "r" (dpsc), "r" (freq) - : "cc", "r0", "r1", "r2", "memory"); - return result; -} - static int do_mon_install(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { @@ -64,39 +48,6 @@ static void core_spin(void) } }
-int mon_power_on(int core_id, void *ep) -{ - int result; - - asm volatile ( - "stmfd r13!, {lr}\n" - "mov r1, %1\n" - "mov r2, %2\n" - "mov r0, #0\n" - "smc #0\n" - "ldmfd r13!, {lr}\n" - : "=&r" (result) - : "r" (core_id), "r" (ep) - : "cc", "r0", "r1", "r2", "memory"); - return result; -} - -int mon_power_off(int core_id) -{ - int result; - - asm volatile ( - "stmfd r13!, {lr}\n" - "mov r1, %1\n" - "mov r0, #1\n" - "smc #1\n" - "ldmfd r13!, {lr}\n" - : "=&r" (result) - : "r" (core_id) - : "cc", "r0", "r1", "memory"); - return result; -} - int do_mon_power(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { diff --git a/arch/arm/mach-keystone/include/mach/mon.h b/arch/arm/mach-keystone/include/mach/mon.h index 33a2876..eb7aa93 100644 --- a/arch/arm/mach-keystone/include/mach/mon.h +++ b/arch/arm/mach-keystone/include/mach/mon.h @@ -7,9 +7,11 @@ * SPDX-License-Identifier: GPL-2.0+ */
-#ifndef _MON_H_ -#define _MON_H_ +#ifndef _MACH_MON_H_ +#define _MACH_MON_H_
+int mon_install(u32 addr, u32 dpsc, u32 freq); +int mon_power_on(int core_id, void *ep); int mon_power_off(int core_id);
#endif diff --git a/arch/arm/mach-keystone/mon.c b/arch/arm/mach-keystone/mon.c new file mode 100644 index 0000000..256f630 --- /dev/null +++ b/arch/arm/mach-keystone/mon.c @@ -0,0 +1,63 @@ +/* + * K2HK: secure kernel command file + * + * (C) Copyright 2012-2014 + * Texas Instruments Incorporated, <www.ti.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <command.h> +#include <mach/mon.h> +asm(".arch_extension sec\n\t"); + +int mon_install(u32 addr, u32 dpsc, u32 freq) +{ + int result; + + __asm__ __volatile__ ( + "stmfd r13!, {lr}\n" + "mov r0, %1\n" + "mov r1, %2\n" + "mov r2, %3\n" + "blx r0\n" + "ldmfd r13!, {lr}\n" + : "=&r" (result) + : "r" (addr), "r" (dpsc), "r" (freq) + : "cc", "r0", "r1", "r2", "memory"); + return result; +} + +int mon_power_on(int core_id, void *ep) +{ + int result; + + asm volatile ( + "stmfd r13!, {lr}\n" + "mov r1, %1\n" + "mov r2, %2\n" + "mov r0, #0\n" + "smc #0\n" + "ldmfd r13!, {lr}\n" + : "=&r" (result) + : "r" (core_id), "r" (ep) + : "cc", "r0", "r1", "r2", "memory"); + return result; +} + +int mon_power_off(int core_id) +{ + int result; + + asm volatile ( + "stmfd r13!, {lr}\n" + "mov r1, %1\n" + "mov r0, #1\n" + "smc #1\n" + "ldmfd r13!, {lr}\n" + : "=&r" (result) + : "r" (core_id) + : "cc", "r0", "r1", "memory"); + return result; +}

Now that we have a standard way to power off the hardware, switch to using that rather than our own command.
Cc: Vitaly Andrianov vitalya@ti.com Cc: Nishanth Menon nm@ti.com Cc: Lokesh Vutla lokeshvutla@ti.com Signed-off-by: Tom Rini trini@konsulko.com --- arch/arm/Kconfig | 1 + arch/arm/mach-keystone/Makefile | 1 + arch/arm/mach-keystone/cmd_poweroff.c | 28 ++++++++++++++++++++++++++++ arch/arm/mach-keystone/keystone.c | 29 ----------------------------- 4 files changed, 30 insertions(+), 29 deletions(-) create mode 100644 arch/arm/mach-keystone/cmd_poweroff.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 653ecc8..e5f57ef 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -450,6 +450,7 @@ config ARCH_KEYSTONE bool "TI Keystone" select CPU_V7 select SUPPORT_SPL + select CMD_POWEROFF
config ARCH_MX7 bool "Freescale MX7" diff --git a/arch/arm/mach-keystone/Makefile b/arch/arm/mach-keystone/Makefile index 7f12009..8829e7f 100644 --- a/arch/arm/mach-keystone/Makefile +++ b/arch/arm/mach-keystone/Makefile @@ -12,6 +12,7 @@ obj-y += mon.o ifndef CONFIG_SPL_BUILD obj-y += cmd_clock.o obj-y += cmd_mon.o +obj-y += cmd_poweroff.o endif obj-y += msmc.o obj-y += ddr3.o cmd_ddr3.o diff --git a/arch/arm/mach-keystone/cmd_poweroff.c b/arch/arm/mach-keystone/cmd_poweroff.c new file mode 100644 index 0000000..1b127a8 --- /dev/null +++ b/arch/arm/mach-keystone/cmd_poweroff.c @@ -0,0 +1,28 @@ +/* + * Keystone EVM : Power off + * + * (C) Copyright 2014 + * Texas Instruments Incorporated, <www.ti.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <command.h> +#include <asm/arch/mon.h> +#include <asm/arch/psc_defs.h> +#include <asm/arch/hardware.h> + +int do_poweroff(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + mon_power_off(0); + + psc_disable_module(KS2_LPSC_TETRIS); + psc_disable_domain(KS2_TETRIS_PWR_DOMAIN); + + asm volatile ("isb\n" + "dsb\n" + "wfi\n"); + + return 0; +} diff --git a/arch/arm/mach-keystone/keystone.c b/arch/arm/mach-keystone/keystone.c index a807127..beb8a76 100644 --- a/arch/arm/mach-keystone/keystone.c +++ b/arch/arm/mach-keystone/keystone.c @@ -9,10 +9,8 @@
#include <common.h> #include <asm/io.h> -#include <asm/arch/mon.h> #include <asm/arch/psc_defs.h> #include <asm/arch/hardware.h> -#include <asm/arch/hardware.h>
/** * cpu_to_bus - swap bytes of the 32-bit data if the device is BE @@ -30,22 +28,6 @@ int cpu_to_bus(u32 *ptr, u32 length) return 0; }
-static int turn_off_myself(void) -{ - printf("Turning off ourselves\r\n"); - mon_power_off(0); - - psc_disable_module(KS2_LPSC_TETRIS); - psc_disable_domain(KS2_TETRIS_PWR_DOMAIN); - - asm volatile ("isb\n" - "dsb\n" - "wfi\n"); - - printf("What! Should not see that\n"); - return 0; -} - static void turn_off_all_dsps(int num_dsps) { int i; @@ -59,17 +41,6 @@ static void turn_off_all_dsps(int num_dsps) } }
-int do_killme_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) -{ - return turn_off_myself(); -} - -U_BOOT_CMD( - killme, 1, 0, do_killme_cmd, - "turn off main ARM core", - "turn off main ARM core. Should not live after that :(\n" -); - int misc_init_r(void) { char *env;

On Wed, Mar 16, 2016 at 11:03:04AM -0400, Tom Rini wrote:
Now that we have a standard way to power off the hardware, switch to using that rather than our own command.
Cc: Vitaly Andrianov vitalya@ti.com Cc: Nishanth Menon nm@ti.com Cc: Lokesh Vutla lokeshvutla@ti.com Signed-off-by: Tom Rini trini@konsulko.com
Applied to u-boot/master, thanks!

When we switch to including all linker lists in U-Boot it is important to not include commands as that may lead to link errors due to other things we have already discarded. In this case simply move cmd_ddr3.o over to the list with the rest.
Cc: Vitaly Andrianov vitalya@ti.com Cc: Nishanth Menon nm@ti.com Cc: Lokesh Vutla lokeshvutla@ti.com Signed-off-by: Tom Rini trini@konsulko.com --- arch/arm/mach-keystone/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-keystone/Makefile b/arch/arm/mach-keystone/Makefile index 8829e7f..b2ffe5b 100644 --- a/arch/arm/mach-keystone/Makefile +++ b/arch/arm/mach-keystone/Makefile @@ -13,9 +13,10 @@ ifndef CONFIG_SPL_BUILD obj-y += cmd_clock.o obj-y += cmd_mon.o obj-y += cmd_poweroff.o +obj-y += cmd_ddr3.o endif obj-y += msmc.o -obj-y += ddr3.o cmd_ddr3.o +obj-y += ddr3.o obj-y += keystone.o obj-$(CONFIG_K2E_EVM) += ddr3_spd.o obj-$(CONFIG_K2HK_EVM) += ddr3_spd.o

On Wed, Mar 16, 2016 at 11:03:05AM -0400, Tom Rini wrote:
When we switch to including all linker lists in U-Boot it is important to not include commands as that may lead to link errors due to other things we have already discarded. In this case simply move cmd_ddr3.o over to the list with the rest.
Cc: Vitaly Andrianov vitalya@ti.com Cc: Nishanth Menon nm@ti.com Cc: Lokesh Vutla lokeshvutla@ti.com Signed-off-by: Tom Rini trini@konsulko.com
Reworded slightly and applied to u-boot/master, thanks!

When we switch to including all linker lists in U-Boot it is important to not include commands as that may lead to link errors due to other things we have already discarded. In this case, the SCSI code needs a lot of attention so for now just guard the command portions.
Signed-off-by: Tom Rini trini@konsulko.com --- cmd/scsi.c | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/cmd/scsi.c b/cmd/scsi.c index 951d1e5..8991125 100644 --- a/cmd/scsi.c +++ b/cmd/scsi.c @@ -245,6 +245,7 @@ struct blk_desc *scsi_get_dev(int dev) } #endif
+#ifndef CONFIG_SPL_BUILD /****************************************************************************** * scsi boot command intepreter. Derived from diskboot */ @@ -368,6 +369,27 @@ int do_scsi (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return CMD_RET_USAGE; }
+U_BOOT_CMD( + scsi, 5, 1, do_scsi, + "SCSI sub-system", + "reset - reset SCSI controller\n" + "scsi info - show available SCSI devices\n" + "scsi scan - (re-)scan SCSI bus\n" + "scsi device [dev] - show or set current device\n" + "scsi part [dev] - print partition table of one or all SCSI devices\n" + "scsi read addr blk# cnt - read `cnt' blocks starting at block `blk#'\n" + " to memory address `addr'\n" + "scsi write addr blk# cnt - write `cnt' blocks starting at block\n" + " `blk#' from memory address `addr'" +); + +U_BOOT_CMD( + scsiboot, 3, 1, do_scsiboot, + "boot from SCSI device", + "loadAddr dev:part" +); +#endif + /**************************************************************************************** * scsi_read */ @@ -710,24 +732,3 @@ void scsi_setup_inquiry(ccb * pccb) pccb->cmdlen=6; pccb->msgout[0]=SCSI_IDENTIFY; /* NOT USED */ } - - -U_BOOT_CMD( - scsi, 5, 1, do_scsi, - "SCSI sub-system", - "reset - reset SCSI controller\n" - "scsi info - show available SCSI devices\n" - "scsi scan - (re-)scan SCSI bus\n" - "scsi device [dev] - show or set current device\n" - "scsi part [dev] - print partition table of one or all SCSI devices\n" - "scsi read addr blk# cnt - read `cnt' blocks starting at block `blk#'\n" - " to memory address `addr'\n" - "scsi write addr blk# cnt - write `cnt' blocks starting at block\n" - " `blk#' from memory address `addr'" -); - -U_BOOT_CMD( - scsiboot, 3, 1, do_scsiboot, - "boot from SCSI device", - "loadAddr dev:part" -);

On Wed, Mar 16, 2016 at 11:03:06AM -0400, Tom Rini wrote:
When we switch to including all linker lists in U-Boot it is important to not include commands as that may lead to link errors due to other things we have already discarded. In this case, the SCSI code needs a lot of attention so for now just guard the command portions.
Signed-off-by: Tom Rini trini@konsulko.com
Reworded slightly and applied to u-boot/master, thanks!

When we switch to including all linker lists in U-Boot it is important to not include commands as that may lead to link errors due to other things we have already discarded. In this case change things so that we only build the right objects for SPL or non-SPL
Cc: Albert ARIBAUD (3ADEV) albert.aribaud@3adev.fr Signed-off-by: Tom Rini trini@konsulko.com --- board/work-microwave/work_92105/Makefile | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/board/work-microwave/work_92105/Makefile b/board/work-microwave/work_92105/Makefile index ba31c8e..e26c673 100644 --- a/board/work-microwave/work_92105/Makefile +++ b/board/work-microwave/work_92105/Makefile @@ -5,6 +5,8 @@ # SPDX-License-Identifier: GPL-2.0+ #
-obj-y := work_92105.o work_92105_display.o - -obj-$(CONFIG_SPL_BUILD) += work_92105_spl.o +ifdef CONFIG_SPL_BUILD +obj-y += work_92105_spl.o +else +obj-y += work_92105.o work_92105_display.o +endif

On Wed, Mar 16, 2016 at 11:03:07AM -0400, Tom Rini wrote:
When we switch to including all linker lists in U-Boot it is important to not include commands as that may lead to link errors due to other things we have already discarded. In this case change things so that we only build the right objects for SPL or non-SPL
Cc: Albert ARIBAUD (3ADEV) albert.aribaud@3adev.fr Signed-off-by: Tom Rini trini@konsulko.com
Reworded slightly and applied to u-boot/master, thanks!

On OMAP4 platforms that also need to calculate their DDR settings we are now getting very close to the linker limit size. Since OMAP44XX is only seen with LPDDR2, remove some run time tests for LPDDR2 or DDR3 as we will know that we don't have it for OMAP44XX.
Cc: Nishanth Menon nm@ti.com Signed-off-by: Tom Rini trini@konsulko.com --- arch/arm/cpu/armv7/omap-common/emif-common.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/omap-common/emif-common.c b/arch/arm/cpu/armv7/omap-common/emif-common.c index 697d6e0..9a9c764 100644 --- a/arch/arm/cpu/armv7/omap-common/emif-common.c +++ b/arch/arm/cpu/armv7/omap-common/emif-common.c @@ -195,6 +195,7 @@ void emif_update_timings(u32 base, const struct emif_regs *regs) } }
+#ifndef CONFIG_OMAP44XX static void omap5_ddr3_leveling(u32 base, const struct emif_regs *regs) { struct emif_reg_struct *emif = (struct emif_reg_struct *)base; @@ -405,6 +406,7 @@ static void ddr3_init(u32 base, const struct emif_regs *regs) else dra7_ddr3_init(base, regs); } +#endif
#ifndef CONFIG_SYS_EMIF_PRECALCULATED_TIMING_REGS #define print_timing_reg(reg) debug(#reg" - 0x%08x\n", (reg)) @@ -1178,7 +1180,7 @@ static void do_sdram_init(u32 base) #endif /* CONFIG_SYS_EMIF_PRECALCULATED_TIMING_REGS */
/* - * Initializing the LPDDR2 device can not happen from SDRAM. + * Initializing the DDR device can not happen from SDRAM. * Changing the timing registers in EMIF can happen(going from one * OPP to another) */ @@ -1186,15 +1188,19 @@ static void do_sdram_init(u32 base) if (emif_sdram_type(regs->sdram_config) == EMIF_SDRAM_TYPE_LPDDR2) lpddr2_init(base, regs); +#ifndef CONFIG_OMAP44XX else ddr3_init(base, regs); +#endif } +#ifdef CONFIG_OMAP54X if (warm_reset() && (emif_sdram_type(regs->sdram_config) == EMIF_SDRAM_TYPE_DDR3) && !is_dra7xx()) { set_lpmode_selfrefresh(base); emif_reset_phy(base); omap5_ddr3_leveling(base, regs); } +#endif
/* Write to the shadow registers */ emif_update_timings(base, regs);

On Wed, Mar 16, 2016 at 11:03:08AM -0400, Tom Rini wrote:
On OMAP4 platforms that also need to calculate their DDR settings we are now getting very close to the linker limit size. Since OMAP44XX is only seen with LPDDR2, remove some run time tests for LPDDR2 or DDR3 as we will know that we don't have it for OMAP44XX.
Cc: Nishanth Menon nm@ti.com Signed-off-by: Tom Rini trini@konsulko.com
Applied to u-boot/master, thanks!

Hello Tom,
On Wed, 16 Mar 2016 11:03:03 -0400, Tom Rini trini@konsulko.com wrote:
When we switch to including all linker lists in U-Boot it is important to not include commands as that may lead to link errors due to other things we have already discarded. In this case, we split the code for supporting the monitor out from the code for loading it.
Not sure I'm understanding this commit message. Can you clarify?
Amicalement,

On Wed, Mar 16, 2016 at 06:15:19PM +0100, Albert ARIBAUD wrote:
Hello Tom,
On Wed, 16 Mar 2016 11:03:03 -0400, Tom Rini trini@konsulko.com wrote:
When we switch to including all linker lists in U-Boot it is important to not include commands as that may lead to link errors due to other things we have already discarded. In this case, we split the code for supporting the monitor out from the code for loading it.
Not sure I'm understanding this commit message. Can you clarify?
sigh, I missed a word in this series, "... linker lists in U-Boot SPL it is ..."

On Wed, Mar 16, 2016 at 11:03:03AM -0400, Tom Rini wrote:
When we switch to including all linker lists in U-Boot it is important to not include commands as that may lead to link errors due to other things we have already discarded. In this case, we split the code for supporting the monitor out from the code for loading it.
Cc: Vitaly Andrianov vitalya@ti.com Cc: Nishanth Menon nm@ti.com Cc: Lokesh Vutla lokeshvutla@ti.com Signed-off-by: Tom Rini trini@konsulko.com
Reworded slightly and applied to u-boot/master, thanks!
participants (2)
-
Albert ARIBAUD
-
Tom Rini