[PATCH 1/5] Nokia RX-51: Fix invalidating zImage kernel format

[PATCH 1/5] Nokia RX-51: Fix invalidating zImage kernel format

[Pali Rohár]

Prior starting copy of kernel image to target location, invalidate also
zImage magic header. This ensures that on target location would be image
with valid header only in the case valid header was also in the source
location and copy from source to target finished successfully. Copy is
always skipped when kernel image in source location is invalid.

Add also comment to the code which explain what is the code doing.

Fixes: cc434fccba4c ("Nokia RX-51: Add support for booting kernel in zImage format")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 board/nokia/rx51/lowlevel_init.S | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/board/nokia/rx51/lowlevel_init.S b/board/nokia/rx51/lowlevel_init.S
index 11c2cbef89a6..632595f83785 100644
--- a/board/nokia/rx51/lowlevel_init.S
+++ b/board/nokia/rx51/lowlevel_init.S
@@ -72,7 +72,8 @@ copy_kernel_start:
 
 	/* remove header in target kernel */
 	mov	r5, #0
-	str	r5, [r3]
+	str	r5, [r3]	/* remove 4 bytes header of kernel uImage */
+	str	r5, [r3, #36]	/* remove 4 bytes header of kernel zImage */
 
 	/* check for valid kernel uImage */
 	ldr	r4, [r0]	/* r4 - 4 bytes header of kernel */
@@ -93,6 +94,8 @@ copy_kernel_loop:
 	bhi	copy_kernel_loop
 
 copy_kernel_end:
+
+	/* remove header in source kernel image */
 	mov	r5, #0
 	str	r5, [r0]	/* remove 4 bytes header of kernel uImage */
 	str	r5, [r0, #36]	/* remove 4 bytes header of kernel zImage */
-- 
2.20.1

[PATCH 2/5] Nokia RX-51: Use U-Boot generic position independent code

[Pali Rohár]

Switch from custom board specific fixup/copy code to U-Boot generic
position independent code provided by config option POSITION_INDEPENDENT.

This also slightly decrease size of u-boot.bin binary (by 52 bytes). Note
that option POSITION_INDEPENDENT increase size but not more than custom
board fixup/copy code which is being deleted (as it is not needed anymore).

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/arm/mach-omap2/omap3/Kconfig |   1 +
 board/nokia/rx51/lowlevel_init.S  | 127 ++++--------------------------
 configs/nokia_rx51_defconfig      |   1 -
 3 files changed, 16 insertions(+), 113 deletions(-)

diff --git a/arch/arm/mach-omap2/omap3/Kconfig b/arch/arm/mach-omap2/omap3/Kconfig
index 81c898b66e34..18574ab2b2ea 100644
--- a/arch/arm/mach-omap2/omap3/Kconfig
+++ b/arch/arm/mach-omap2/omap3/Kconfig
@@ -107,6 +107,7 @@ config TARGET_OMAP3_LOGIC
 
 config TARGET_NOKIA_RX51
 	bool "Nokia RX51"
+	select POSITION_INDEPENDENT
 
 config TARGET_TAO3530
 	bool "TAO3530"
diff --git a/board/nokia/rx51/lowlevel_init.S b/board/nokia/rx51/lowlevel_init.S
index 632595f83785..dd3c830cc2e2 100644
--- a/board/nokia/rx51/lowlevel_init.S
+++ b/board/nokia/rx51/lowlevel_init.S
@@ -33,16 +33,24 @@ z_magic:		/* LINUX_ARM_ZIMAGE_MAGIC */
 /*
  * Routine: save_boot_params (called after reset from start.S)
  * Description: Copy attached kernel to address KERNEL_ADDRESS
- *              Copy u-boot to address CONFIG_SYS_TEXT_BASE
- *              Return to copied u-boot address
  */
 
 .global save_boot_params
 save_boot_params:
-	/* Get return address */
-	ldr	lr, =save_boot_params_ret
 
-/* Copy valid attached kernel to address KERNEL_ADDRESS */
+/*
+ * Copy valid attached kernel to absolute address KERNEL_ADDRESS
+ *
+ * Nokia X-Loader is loading secondary image to address 0x80400000.
+ * NOLO is loading boot image to random place, so it doesn't really
+ * matter what is set in CONFIG_SYS_TEXT_BASE. We have to detect
+ * KERNEL_OFFSET from the current execution address and copy it to
+ * absolute address KERNEL_ADDRESS.
+ *
+ * Note that U-Boot has to be compiled with CONFIG_POSITION_INDEPENDENT
+ * because it is loaded at random address and not to the fixed address
+ * (CONFIG_SYS_TEXT_BASE).
+ */
 
 copy_kernel_start:
 	adr	r0, relocaddr	/* r0 - address of section relocaddr */
@@ -100,110 +108,5 @@ copy_kernel_end:
 	str	r5, [r0]	/* remove 4 bytes header of kernel uImage */
 	str	r5, [r0, #36]	/* remove 4 bytes header of kernel zImage */
 
-
-/* Fix u-boot code */
-
-fix_start:
-	adr	r0, relocaddr	/* r0 - address of section relocaddr */
-	ldr	r1, relocaddr	/* r1 - address of relocaddr after relocation */
-	cmp	r0, r1
-
-	beq	copy_uboot_end	/* skip if u-boot is on correct address */
-
-	/* r5 - calculated offset */
-	subhi	r5, r0, r1
-	sublo	r5, r1, r0
-
-	/* r6 - maximal u-boot size */
-	ldr	r6, imagesize
-
-	/* r1 - start of u-boot after */
-	ldr	r1, startaddr
-
-	/* r0 - start of u-boot before */
-	addhi	r0, r1, r5
-	sublo	r0, r1, r5
-
-	/* check if we need to move uboot copy code before calling it */
-	cmp	r5, r6
-	bhi	copy_uboot_start /* now coping u-boot code directly is safe */
-
-
-copy_code_start:
-	/* r0 - start of u-boot before */
-	/* r1 - start of u-boot after */
-	/* r6 - maximal u-boot size */
-
-	/* r7 - maximal kernel size */
-	ldr	r7, kernsize
-
-	/* r4 - end of kernel before */
-	add	r4, r0, r6
-	add	r4, r4, r7
-
-	/* r5 - end of u-boot after */
-	ldr	r5, startaddr
-	add	r5, r5, r6
-
-	/* r2 - start of loop code after */
-	cmp	r4, r5		/* higher address (r4 or r5) */
-	movhs	r2, r4
-	movlo	r2, r5
-
-	/* r3 - end of loop code before */
-	adr	r3, end
-
-	/* r4 - end of loop code after */
-	adr	r4, copy_uboot_start
-	sub	r4, r3, r4
-	add	r4, r2, r4
-
-copy_code_loop:
-	ldmdb	r3!, {r7 - r10}
-	stmdb	r4!, {r7 - r10}
-	cmp	r4, r2
-	bhi	copy_code_loop
-
-copy_code_end:
-	mov	pc, r2
-
-
-/*
- * Copy u-boot to address CONFIG_SYS_TEXT_BASE
- *
- * Nokia X-Loader loading secondary image to address 0x80400000
- * NOLO loading boot image to random place, so it doesn't really
- * matter what is set in CONFIG_SYS_TEXT_BASE. We have to copy
- * u-boot to CONFIG_SYS_TEXT_BASE address.
- */
-
-copy_uboot_start:
-	/* r0 - start of u-boot before */
-	/* r1 - start of u-boot after */
-	/* r6 - maximal u-boot size */
-
-	/* r2 - end of u-boot after */
-	add	r2, r1, r6
-
-	/* condition for copying from left to right */
-	cmp	r0, r1
-	addlo	r1, r0, r6	/* r1 - end of u-boot before */
-	blo	copy_uboot_loop_right
-
-copy_uboot_loop_left:
-	ldmia	r0!, {r3 - r10}
-	stmia	r1!, {r3 - r10}
-	cmp	r1, r2
-	blo	copy_uboot_loop_left
-	b	copy_uboot_end
-
-copy_uboot_loop_right:
-	ldmdb	r1!, {r3 - r10}
-	stmdb	r2!, {r3 - r10}
-	cmp	r1, r0
-	bhi	copy_uboot_loop_right
-
-copy_uboot_end:
-	bx	lr
-
-end:
+	/* Returns */
+	b	save_boot_params_ret
diff --git a/configs/nokia_rx51_defconfig b/configs/nokia_rx51_defconfig
index 309cf28269c1..3365f7ae58df 100644
--- a/configs/nokia_rx51_defconfig
+++ b/configs/nokia_rx51_defconfig
@@ -9,7 +9,6 @@ CONFIG_INITRD_TAG=y
 CONFIG_REVISION_TAG=y
 CONFIG_STATIC_MACH_TYPE=y
 CONFIG_MACH_TYPE=1955
-CONFIG_SYS_TEXT_BASE=0x80008000
 CONFIG_SYS_MALLOC_LEN=0xc0000
 CONFIG_NR_DRAM_BANKS=2
 CONFIG_TARGET_NOKIA_RX51=y
-- 
2.20.1

Re: [PATCH 2/5] Nokia RX-51: Use U-Boot generic position independent code

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 546 bytes --]

On Thu, Aug 11, 2022 at 10:27:22PM +0200, Pali Rohár wrote:

> Switch from custom board specific fixup/copy code to U-Boot generic
> position independent code provided by config option POSITION_INDEPENDENT.
> 
> This also slightly decrease size of u-boot.bin binary (by 52 bytes). Note
> that option POSITION_INDEPENDENT increase size but not more than custom
> board fixup/copy code which is being deleted (as it is not needed anymore).
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 3/5] Nokia RX-51: Simplify copy kernel code

[Pali Rohár]

Expression (r + (r0 - r1)) produce same result as (r - (r1 - r0)). So it
does not matter which one is called. Always call the first option and
remove second one.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 board/nokia/rx51/lowlevel_init.S | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/board/nokia/rx51/lowlevel_init.S b/board/nokia/rx51/lowlevel_init.S
index dd3c830cc2e2..4f76e80b20f1 100644
--- a/board/nokia/rx51/lowlevel_init.S
+++ b/board/nokia/rx51/lowlevel_init.S
@@ -55,16 +55,13 @@ save_boot_params:
 copy_kernel_start:
 	adr	r0, relocaddr	/* r0 - address of section relocaddr */
 	ldr	r1, relocaddr	/* r1 - address of relocaddr after relocation */
-	cmp	r0, r1
 
 	/* r4 - calculated offset */
-	subhi	r4, r0, r1
-	sublo	r4, r1, r0
+	sub	r4, r0, r1
 
 	/* r0 - start of kernel before */
 	ldr	r0, startaddr
-	addhi	r0, r0, r4
-	sublo	r0, r0, r4
+	add	r0, r0, r4
 	ldr	r1, kernoffs
 	add	r0, r0, r1
 
-- 
2.20.1

Re: [PATCH 3/5] Nokia RX-51: Simplify copy kernel code

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 334 bytes --]

On Thu, Aug 11, 2022 at 10:27:23PM +0200, Pali Rohár wrote:

> Expression (r + (r0 - r1)) produce same result as (r - (r1 - r0)). So it
> does not matter which one is called. Always call the first option and
> remove second one.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 4/5] Nokia RX-51: Simplify calculation of attached kernel image address

[Pali Rohár]

Now when board starup code does not copy image to CONFIG_SYS_TEXT_BASE
address there is no need to calculate all addresses from base address at
runtime. The only address which needs to be calculated is attached kernel
image address which can be simplified at compile time without need to know
CONFIG_SYS_TEXT_BASE address or relocation address at the runtime.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 board/nokia/rx51/lowlevel_init.S | 21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/board/nokia/rx51/lowlevel_init.S b/board/nokia/rx51/lowlevel_init.S
index 4f76e80b20f1..c1785bc3f72f 100644
--- a/board/nokia/rx51/lowlevel_init.S
+++ b/board/nokia/rx51/lowlevel_init.S
@@ -6,11 +6,8 @@
 
 #include <config.h>
 
-relocaddr:		/* address of this relocaddr section after coping */
-	.word .		/* address of section (calculated at compile time) */
-
-startaddr:		/* address of u-boot after copying */
-	.word CONFIG_SYS_TEXT_BASE
+kernoffs:		/* offset of kernel image from this address */
+	.word KERNEL_OFFSET - (. - CONFIG_SYS_TEXT_BASE)
 
 kernaddr:		/* address of kernel after copying */
 	.word KERNEL_ADDRESS
@@ -18,9 +15,6 @@ kernaddr:		/* address of kernel after copying */
 kernsize:		/* maximal size of kernel image */
 	.word KERNEL_MAXSIZE
 
-kernoffs:		/* offset of kernel image in loaded u-boot */
-	.word KERNEL_OFFSET
-
 imagesize:		/* maximal size of image */
 	.word IMAGE_MAXSIZE
 
@@ -53,16 +47,9 @@ save_boot_params:
  */
 
 copy_kernel_start:
-	adr	r0, relocaddr	/* r0 - address of section relocaddr */
-	ldr	r1, relocaddr	/* r1 - address of relocaddr after relocation */
-
-	/* r4 - calculated offset */
-	sub	r4, r0, r1
-
 	/* r0 - start of kernel before */
-	ldr	r0, startaddr
-	add	r0, r0, r4
-	ldr	r1, kernoffs
+	adr	r0, kernoffs	/* r0 - current address of kernoffs section */
+	ldr	r1, kernoffs	/* r1 - offset of kernel image from kernoffs section */
 	add	r0, r0, r1
 
 	/* r3 - start of kernel after */
-- 
2.20.1

Re: [PATCH 4/5] Nokia RX-51: Simplify calculation of attached kernel image address

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

On Thu, Aug 11, 2022 at 10:27:24PM +0200, Pali Rohár wrote:

> Now when board starup code does not copy image to CONFIG_SYS_TEXT_BASE
> address there is no need to calculate all addresses from base address at
> runtime. The only address which needs to be calculated is attached kernel
> image address which can be simplified at compile time without need to know
> CONFIG_SYS_TEXT_BASE address or relocation address at the runtime.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 5/5] Nokia RX-51: Move board required options from defconfig to Kconfig

[Pali Rohár]

Some of config options are board specific and should be set in into their
default values automatically. So move them from defconfig file to Kconfig
definitions to ensure that possible user custom defconfig files would have
these required options also enabled.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/arm/mach-omap2/omap3/Kconfig | 5 +++++
 board/nokia/rx51/Kconfig          | 6 ++++++
 configs/nokia_rx51_defconfig      | 7 -------
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/omap3/Kconfig b/arch/arm/mach-omap2/omap3/Kconfig
index 18574ab2b2ea..3e97ec262951 100644
--- a/arch/arm/mach-omap2/omap3/Kconfig
+++ b/arch/arm/mach-omap2/omap3/Kconfig
@@ -108,6 +108,11 @@ config TARGET_OMAP3_LOGIC
 config TARGET_NOKIA_RX51
 	bool "Nokia RX51"
 	select POSITION_INDEPENDENT
+	select SKIP_LOWLEVEL_INIT
+	select SUPPORT_PASSING_ATAGS
+	select CMDLINE_TAG
+	select INITRD_TAG
+	select REVISION_TAG
 
 config TARGET_TAO3530
 	bool "TAO3530"
diff --git a/board/nokia/rx51/Kconfig b/board/nokia/rx51/Kconfig
index ec6a571a2263..7cf05077dad1 100644
--- a/board/nokia/rx51/Kconfig
+++ b/board/nokia/rx51/Kconfig
@@ -1,5 +1,8 @@
 if TARGET_NOKIA_RX51
 
+config NR_DRAM_BANKS
+	default 2
+
 config SYS_BOARD
 	default "rx51"
 
@@ -9,4 +12,7 @@ config SYS_VENDOR
 config SYS_CONFIG_NAME
 	default "nokia_rx51"
 
+config SYS_PROMPT
+	default "Nokia RX-51 # "
+
 endif
diff --git a/configs/nokia_rx51_defconfig b/configs/nokia_rx51_defconfig
index 3365f7ae58df..4414ebc25e36 100644
--- a/configs/nokia_rx51_defconfig
+++ b/configs/nokia_rx51_defconfig
@@ -1,16 +1,10 @@
 CONFIG_ARM=y
-CONFIG_SKIP_LOWLEVEL_INIT=y
 # CONFIG_SYS_THUMB_BUILD is not set
 CONFIG_SYS_L2CACHE_OFF=y
 CONFIG_ARCH_OMAP2PLUS=y
-CONFIG_SUPPORT_PASSING_ATAGS=y
-CONFIG_CMDLINE_TAG=y
-CONFIG_INITRD_TAG=y
-CONFIG_REVISION_TAG=y
 CONFIG_STATIC_MACH_TYPE=y
 CONFIG_MACH_TYPE=1955
 CONFIG_SYS_MALLOC_LEN=0xc0000
-CONFIG_NR_DRAM_BANKS=2
 CONFIG_TARGET_NOKIA_RX51=y
 CONFIG_SYS_LOAD_ADDR=0x80000000
 CONFIG_OPTIMIZE_INLINING=y
@@ -26,7 +20,6 @@ CONFIG_USE_PREBOOT=y
 CONFIG_PREBOOT="run preboot"
 # CONFIG_SYS_DEVICE_NULLDEV is not set
 CONFIG_HUSH_PARSER=y
-CONFIG_SYS_PROMPT="Nokia RX-51 # "
 # CONFIG_CMD_BDI is not set
 CONFIG_CMD_BOOTZ=y
 # CONFIG_BOOTM_NETBSD is not set
-- 
2.20.1

Re: [PATCH 5/5] Nokia RX-51: Move board required options from defconfig to Kconfig

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

On Thu, Aug 11, 2022 at 10:27:25PM +0200, Pali Rohár wrote:

> Some of config options are board specific and should be set in into their
> default values automatically. So move them from defconfig file to Kconfig
> definitions to ensure that possible user custom defconfig files would have
> these required options also enabled.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 1/5] Nokia RX-51: Fix invalidating zImage kernel format

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 691 bytes --]

On Thu, Aug 11, 2022 at 10:27:21PM +0200, Pali Rohár wrote:

> Prior starting copy of kernel image to target location, invalidate also
> zImage magic header. This ensures that on target location would be image
> with valid header only in the case valid header was also in the source
> location and copy from source to target finished successfully. Copy is
> always skipped when kernel image in source location is invalid.
> 
> Add also comment to the code which explain what is the code doing.
> 
> Fixes: cc434fccba4c ("Nokia RX-51: Add support for booting kernel in zImage format")
> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]