diff mbox series

[1/5] mips: start.S: Add Octeon boot header compatibility

Message ID 20201016130850.706377-1-sr@denx.de
State Superseded
Delegated to: Daniel Schwierzeck
Headers show
Series [1/5] mips: start.S: Add Octeon boot header compatibility | expand

Commit Message

Stefan Roese Oct. 16, 2020, 1:08 p.m. UTC
Octeon has a specific boot header, when booted via SPI NOR, NAND or MMC.
Here the only 2 instructions are allowed in the first few bytes of the
image. And these instructions need to be one branch and a nop. This
patch adds the necessary nop after the nop, to that the common MIPS
image is compatible with this Octeon header.

The tool to patch the Octeon boot header into the image will be send in
a follow-up patch.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Aaron Williams <awilliams@marvell.com>
Cc: Chandrakala Chavva <cchavva@marvell.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---
 arch/mips/cpu/start.S | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Mark Kettenis Oct. 16, 2020, 1:25 p.m. UTC | #1
> From: Stefan Roese <sr@denx.de>
> Date: Fri, 16 Oct 2020 15:08:46 +0200
> 
> Octeon has a specific boot header, when booted via SPI NOR, NAND or MMC.
> Here the only 2 instructions are allowed in the first few bytes of the
> image. And these instructions need to be one branch and a nop. This
> patch adds the necessary nop after the nop, to that the common MIPS
> image is compatible with this Octeon header.
> 
> The tool to patch the Octeon boot header into the image will be send in
> a follow-up patch.

Since the moved instruction is no longer in a delay slot, you should
probably remove the extra space before the instruction.

Cheers,

Mark

> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Aaron Williams <awilliams@marvell.com>
> Cc: Chandrakala Chavva <cchavva@marvell.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> ---
>  arch/mips/cpu/start.S | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/cpu/start.S b/arch/mips/cpu/start.S
> index d0c412236d..6de2470cc2 100644
> --- a/arch/mips/cpu/start.S
> +++ b/arch/mips/cpu/start.S
> @@ -75,8 +75,13 @@
>  
>  ENTRY(_start)
>  	/* U-Boot entry point */
> +	/*
> +	 * Octeon needs special handling here, as the binary might be
> +	 * patched to add a boot header for SPI, NAND or MMC booting. Only
> +	 * one branch plus nop is allowed here.
> +	 */
>  	b	reset
> -	 mtc0	zero, CP0_COUNT	# clear cp0 count for most accurate boot timing
> +	 nop
>  
>  #if defined(CONFIG_MIPS_INSERT_BOOT_CONFIG)
>  	/*
> @@ -123,6 +128,7 @@ ENTRY(_start)
>  #endif
>  
>  reset:
> +	 mtc0	zero, CP0_COUNT	# clear cp0 count for most accurate boot timing
>  #if __mips_isa_rev >= 6
>  	mfc0	t0, CP0_CONFIG, 5
>  	and	t0, t0, MIPS_CONF5_VP
> -- 
> 2.28.0
> 
>
Daniel Schwierzeck Oct. 26, 2020, 1:42 p.m. UTC | #2
Am Freitag, den 16.10.2020, 15:08 +0200 schrieb Stefan Roese:
> Octeon has a specific boot header, when booted via SPI NOR, NAND or MMC.
> Here the only 2 instructions are allowed in the first few bytes of the
> image. And these instructions need to be one branch and a nop. This
> patch adds the necessary nop after the nop, to that the common MIPS
> image is compatible with this Octeon header.
> 
> The tool to patch the Octeon boot header into the image will be send in
> a follow-up patch.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Aaron Williams <awilliams@marvell.com>
> Cc: Chandrakala Chavva <cchavva@marvell.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> ---
>  arch/mips/cpu/start.S | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/cpu/start.S b/arch/mips/cpu/start.S
> index d0c412236d..6de2470cc2 100644
> --- a/arch/mips/cpu/start.S
> +++ b/arch/mips/cpu/start.S
> @@ -75,8 +75,13 @@
>  
>  ENTRY(_start)
>  	/* U-Boot entry point */
> +	/*
> +	 * Octeon needs special handling here, as the binary might be
> +	 * patched to add a boot header for SPI, NAND or MMC booting. Only
> +	 * one branch plus nop is allowed here.
> +	 */

I'd prefer a shorter and more imperative comment within one multi-line
comment block, e.g.

/*
 * U-Boot entry point.
 * Do not add instructions to the branch delay slot! Some SoC's 
 * like Octeon might patch the final U-Boot binary at this location
 * with additional boot headers.
 */

>  	b	reset
> -	 mtc0	zero, CP0_COUNT	# clear cp0 count for most accurate boot timing
> +	 nop
>  
>  #if defined(CONFIG_MIPS_INSERT_BOOT_CONFIG)
>  	/*
> @@ -123,6 +128,7 @@ ENTRY(_start)
>  #endif
>  
>  reset:
> +	 mtc0	zero, CP0_COUNT	# clear cp0 count for most accurate boot timing

the extra indentation of one space is only required for instructions in
delay slots and needs to be removed.

>  #if __mips_isa_rev >= 6
>  	mfc0	t0, CP0_CONFIG, 5
>  	and	t0, t0, MIPS_CONF5_VP
Stefan Roese Oct. 28, 2020, 7:21 a.m. UTC | #3
On 26.10.20 14:42, Daniel Schwierzeck wrote:
> Am Freitag, den 16.10.2020, 15:08 +0200 schrieb Stefan Roese:
>> Octeon has a specific boot header, when booted via SPI NOR, NAND or MMC.
>> Here the only 2 instructions are allowed in the first few bytes of the
>> image. And these instructions need to be one branch and a nop. This
>> patch adds the necessary nop after the nop, to that the common MIPS
>> image is compatible with this Octeon header.
>>
>> The tool to patch the Octeon boot header into the image will be send in
>> a follow-up patch.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Aaron Williams <awilliams@marvell.com>
>> Cc: Chandrakala Chavva <cchavva@marvell.com>
>> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>> ---
>>   arch/mips/cpu/start.S | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/mips/cpu/start.S b/arch/mips/cpu/start.S
>> index d0c412236d..6de2470cc2 100644
>> --- a/arch/mips/cpu/start.S
>> +++ b/arch/mips/cpu/start.S
>> @@ -75,8 +75,13 @@
>>   
>>   ENTRY(_start)
>>   	/* U-Boot entry point */
>> +	/*
>> +	 * Octeon needs special handling here, as the binary might be
>> +	 * patched to add a boot header for SPI, NAND or MMC booting. Only
>> +	 * one branch plus nop is allowed here.
>> +	 */
> 
> I'd prefer a shorter and more imperative comment within one multi-line
> comment block, e.g.
> 
> /*
>   * U-Boot entry point.
>   * Do not add instructions to the branch delay slot! Some SoC's
>   * like Octeon might patch the final U-Boot binary at this location
>   * with additional boot headers.
>   */

Even better. Will change in v2.

>>   	b	reset
>> -	 mtc0	zero, CP0_COUNT	# clear cp0 count for most accurate boot timing
>> +	 nop
>>   
>>   #if defined(CONFIG_MIPS_INSERT_BOOT_CONFIG)
>>   	/*
>> @@ -123,6 +128,7 @@ ENTRY(_start)
>>   #endif
>>   
>>   reset:
>> +	 mtc0	zero, CP0_COUNT	# clear cp0 count for most accurate boot timing
> 
> the extra indentation of one space is only required for instructions in
> delay slots and needs to be removed.

Yes, thanks for catching.

Thanks,
Stefan
diff mbox series

Patch

diff --git a/arch/mips/cpu/start.S b/arch/mips/cpu/start.S
index d0c412236d..6de2470cc2 100644
--- a/arch/mips/cpu/start.S
+++ b/arch/mips/cpu/start.S
@@ -75,8 +75,13 @@ 
 
 ENTRY(_start)
 	/* U-Boot entry point */
+	/*
+	 * Octeon needs special handling here, as the binary might be
+	 * patched to add a boot header for SPI, NAND or MMC booting. Only
+	 * one branch plus nop is allowed here.
+	 */
 	b	reset
-	 mtc0	zero, CP0_COUNT	# clear cp0 count for most accurate boot timing
+	 nop
 
 #if defined(CONFIG_MIPS_INSERT_BOOT_CONFIG)
 	/*
@@ -123,6 +128,7 @@  ENTRY(_start)
 #endif
 
 reset:
+	 mtc0	zero, CP0_COUNT	# clear cp0 count for most accurate boot timing
 #if __mips_isa_rev >= 6
 	mfc0	t0, CP0_CONFIG, 5
 	and	t0, t0, MIPS_CONF5_VP