On Sat, 2015-08-29 at 09:33 +0300, Siarhei Siamashka wrote:
> Add a new field 'spl_addr' to the SoC description structure and adjust
> the code to honor it. This is needed for supporting Allwinner A80.
> 
> Tested on Allwinner A20 by changing the 'spl_addr' to 0x28000 in the
> 'fel' tool and using a custom build of U-Boot (CONFIG_SPL_TEXT_BASE
> changed from 0x20 to 0x28020).
> 
> Signed-off-by: Siarhei Siamashka <[email protected]>
> ---
>  fel-to-spl-thunk.S | 22 ++++++--------
>  fel-to-spl-thunk.h | 89 
> ++++++++++++++++++++++++++----------------------------
>  fel.c              | 29 ++++++++++--------
>  3 files changed, 70 insertions(+), 70 deletions(-)
> 
> diff --git a/fel-to-spl-thunk.S b/fel-to-spl-thunk.S
> index fb5dfde..e3fa6f2 100644
> --- a/fel-to-spl-thunk.S
> +++ b/fel-to-spl-thunk.S
> @@ -69,6 +69,7 @@ SWAPTBL>    >       > .req>         > r4
>  FULLSIZE>    > .req>         > r5
>  BUFSIZE>     >       > .req>         > r6
>  CHECKSUM>    > .req>         > r7
> +SPL_ADDR>    > .req>         > r8
>  
>  entry_point:
>  >    > b>    > setup_stack
> @@ -87,7 +88,7 @@ stack_end:
>  
>  >    > /* A function, which walks the table and swaps all buffers */
>  swap_all_buffers:
> ->    > adr>  > SWAPTBL,   swaptbl_start
> +>    > adr>  > SWAPTBL,   appended_data + 4
>  swap_next_buffer:
>  >    > ldr>  > BUF1,      [SWAPTBL],  #4
>  >    > ldr>  > BUF2,      [SWAPTBL],  #4
> @@ -104,6 +105,7 @@ swap_next_word:
>  >    > b>    > swap_next_buffer
>  
>  setup_stack: /* Save the original SP, LR and CPSR to stack */
> +>    > ldr>  > SPL_ADDR,  appended_data
>  >    > adr>  > BUF1,      stack_end
>  >    > str>  > sp,        [BUF1, #-4]!
>  >    > mov>  > sp,        BUF1
> @@ -125,7 +127,7 @@ setup_stack: /* Save the original SP, LR and CPSR to 
> stack */
>  verify_checksum:
>  >    > movw>         > CHECKSUM,  #0x6c39
>  >    > movt>         > CHECKSUM,  #0x5f0a
> ->    > mov>  > BUF1,      #0
> +>    > mov>  > BUF1,      SPL_ADDR
>  >    > ldr     FULLSIZE,  [BUF1, #16]
>  check_next_word:
>  >    > ldr>  > TMP1,      [BUF1],   #4
> @@ -133,39 +135,35 @@ check_next_word:
>  >    > add>  > CHECKSUM,  CHECKSUM, TMP1
>  >    > bne>  > check_next_word
>  
> ->    > mov>  > BUF1,      #0
> ->    > ldr>  > TMP1,      [BUF1, #12]
> +>    > ldr>  > TMP1,      [SPL_ADDR, #12]
>  >    > subs>         > CHECKSUM,  CHECKSUM, TMP1, lsl #1
>  >    > bne>  > checksum_is_bad
>  
>  >    > /* Change 'eGON.BT0' -> 'eGON.FEL' */
> ->    > mov>  > BUF1,      #0
>  >    > movw>         > TMP1,      (('F' << 8) + '.')
>  >    > movt>         > TMP1,      (('L' << 8) + 'E')
> ->    > str>  > TMP1,      [BUF1, #8]
> +>    > str>  > TMP1,      [SPL_ADDR, #8]
>  
>  >    > /* Call the SPL code */
>  >    > dsb
>  >    > isb
> ->    > blx>  > BUF1
> +>    > blx>  > SPL_ADDR
>  
>  >    > /* Return back to FEL */
>  >    > b>    > return_to_fel
>  
>  cache_is_unsupported:
>  >    > /* Bail out if cache is enabled and change 'eGON.BT0' -> 'eGON.???' */
> ->    > mov>  > BUF1,      #0
>  >    > movw>         > TMP1,      (('?' << 8) + '.')
>  >    > movt>         > TMP1,      (('?' << 8) + '?')
> ->    > str>  > TMP1,      [BUF1, #8]
> +>    > str>  > TMP1,      [SPL_ADDR, #8]
>  >    > b>    > return_to_fel_noswap
>  
>  checksum_is_bad:
>  >    > /* The checksum test failed, so change 'eGON.BT0' -> 'eGON.BAD' */
> ->    > mov>  > BUF1,      #0
>  >    > movw>         > TMP1,      (('B' << 8) + '.')
>  >    > movt>         > TMP1,      (('D' << 8) + 'A')
> ->    > str>  > TMP1,      [BUF1, #8]
> +>    > str>  > TMP1,      [SPL_ADDR, #8]
>  
>  return_to_fel:
>  >    > bl>   > swap_all_buffers
> @@ -175,4 +173,4 @@ return_to_fel_noswap:
>  >    > ldr     sp,        [sp]
>  >    > bx>   > lr
>  
> -swaptbl_start:
> +appended_data:

If I'm reading correctly you've changed things so that there is an
implicit data structure present here, namely the spl addr followed by
the swaptbl (as before).

I think this would be clearer if it were explicit, can you do:
spl_addr:
        .word 0 /* Filled in by fel tool */
swaptbl_start:

or does something prevent this? I suppose it would make the thunk
appear larger are require accessing from C with END-N rather than
END+N, which might be undesirable.

If it isn't possible/desirable maybe just a comment describing what
should live here and/or some #define's for the offsets would be useful?

Doing asm-offset.S style tricks to let the C and ASM code use a shared
data structure is probably overkill at this point, but might be worth
considering if this grows more fields.

Ian.

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to