[U-Boot] [PATCH 2/8] xtensa: add support for the xtensa processor architecture [2/2]

Max Filippov jcmvbkbc at gmail.com
Fri Jul 15 00:58:27 CEST 2016


Hi Simon,

On Tue, Jul 12, 2016 at 03:56:05PM -0600, Simon Glass wrote:

[...]

> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> Some comments below.
> 
> >
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 566f044..a36ac2f 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -83,6 +83,13 @@ config X86
> >         select DM_SPI
> >         select DM_SPI_FLASH
> >
> > +config XTENSA
> > +       bool "Xtensa architecture"
> > +       select CREATE_ARCH_SYMLINK
> > +       select HAVE_GENERIC_BOARD
> 
> This doesn't exist anymore as the migration is complete.
>
> > +       select SUPPORT_OF_CONTROL
> > +       select SYS_GENERIC_BOARD
> 
> Same here.

Ok, will remove both.

> > diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
> > new file mode 100644
> > index 0000000..e4e3625
> > --- /dev/null
> > +++ b/arch/xtensa/Kconfig
> > @@ -0,0 +1,26 @@
> > +menu "Xtensa architecture"
> > +       depends on XTENSA
> > +
> > +config SYS_ARCH
> > +       string
> > +       default "xtensa"
> > +
> > +config SYS_CPU
> > +       string "Xtensa Core Variant"
> > +
> > +choice
> > +       prompt "Target select"
> > +
> > +
> > +endchoice
> > +
> > +
> > +config HAVE_SYS_ASCDISP
> > +       bool
> > +
> > +config SYS_ASCDISP
> > +       bool "System LCD Display"
> > +       default y
> > +       depends on HAVE_SYS_ASCDISP
> 
> Needs a help message. Should SYS_ASCDISP go in drivers/video?

Ok, will move it there and write help message.

[...]

> > diff --git a/arch/xtensa/cpu/cpu.c b/arch/xtensa/cpu/cpu.c
> > new file mode 100644
> > index 0000000..fb7f810
> > --- /dev/null
> > +++ b/arch/xtensa/cpu/cpu.c
> > @@ -0,0 +1,92 @@
> > +/*
> > + * (C) Copyright 2008 - 2013 Tensilica Inc.
> > + * (C) Copyright 2014 - 2016 Cadence Design Systems Inc.
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + */
> > +
> > +/*
> > + * CPU specific code
> > + */
> > +
> > +#include <common.h>
> > +#include <command.h>
> > +#include <linux/stringify.h>
> > +#include <asm/global_data.h>
> > +#include <asm/cache.h>
> > +#include <asm/string.h>
> > +#include <asm/misc.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +gd_t *gd;
> > +
> > +#if defined(CONFIG_DISPLAY_CPUINFO)
> > +/*
> > + * Print information about the CPU.
> > + */
> > +
> > +int print_cpuinfo(void)
> > +{
> > +       char buf[120], mhz[8];
> > +       uint32_t id0, id1;
> > +
> > +       asm volatile ("rsr %0, 176\n"
> > +                     "rsr %1, 208\n"
> > +                     : "=r"(id0), "=r"(id1));
> > +
> > +       sprintf(buf, "CPU:   Xtensa %s (id: %08x:%08x) at %s MHz\n",
> > +               XCHAL_CORE_ID, id0, id1, strmhz(mhz, gd->cpu_clk));
> > +       puts(buf);
> > +       return 0;
> > +}
> > +#endif
> > +
> > +/*
> > + * Implement the "reset" command.
> > + * We need support from the board, though.
> > + */
> > +
> > +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > +{
> > +       board_reset();
> > +
> > +       /* Shouldn't come back! */
> > +       printf("****** RESET FAILED ******\n");
> 
> All of this can be replaced by a call to sysreset_walk_halt() I think,
> if you have a suitable UCLASS_RESET driver.

Ok, let me look at that.

> > +       return 0;
> > +}
> > +
> > +/*
> > + * We currently run always with caches enabled when running for mmemory.
> 
> Do you mean 'from memory'?

Yes, will fix.

[...]

> > diff --git a/arch/xtensa/cpu/exceptions.c b/arch/xtensa/cpu/exceptions.c
> > new file mode 100644
> > index 0000000..412ca5f
> > --- /dev/null
> > +++ b/arch/xtensa/cpu/exceptions.c
> > @@ -0,0 +1,64 @@
> > +/*
> > + * (C) Copyright 2008 - 2013 Tensilica Inc.
> > + * (C) Copyright 2014 Cadence Design Systems Inc.
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + */
> > +
> > +/*
> > + * Exception handling.
> > + *  We currently don't handle any exception and force a reset.
> > + *  (Note that alloca is a special case and handled in start.S)
> > + */
> > +
> > +#include <common.h>
> > +#include <command.h>
> > +#include <asm/xtensa.h>
> > +#include <asm/string.h>
> > +#include <asm/regs.h>
> > +
> > +typedef void (*handler_t)(struct pt_regs *);
> > +
> > +void xtensa_mem_exc_dummy(struct pt_regs *);
> > +
> > +void unhandled_exception(struct pt_regs *regs)
> > +{
> > +       display_printf("!! EXCCAUSE = %2ld", regs->exccause);
> > +       printf("Unhandled Exception: EXCCAUSE = %ld (pc %lx)\n",
> > +              regs->exccause, regs->pc);
> > +       udelay(10000000);       /* 10s to read display message */
> 
> Probably not needed? Up to you though.

I'd either drop it together with ASCII display, or leave them both.

[...]

> > diff --git a/arch/xtensa/cpu/start.S b/arch/xtensa/cpu/start.S
> > new file mode 100644
> > index 0000000..ac32efb
> > --- /dev/null
> > +++ b/arch/xtensa/cpu/start.S

[...]

> > +3:     /* All code and initalized data segments have been copied. */
> > +
> > +       /* Setup PS, PS.WOE = 1, PS.EXCM = 0, PS.INTLEVEL = EXCM level. */
> > +
> > +#if __XTENSA_CALL0_ABI__
> > +       movi    a2, XCHAL_EXCM_LEVEL
> > +#else
> > +       movi    a2, (1<<PS_WOE_BIT) | XCHAL_EXCM_LEVEL
> > +#endif
> > +       wsr     a2, PS
> > +       rsync
> > +
> > +       /* Clear BSS */
> > +
> > +       movi    a2, _bss_start
> > +       movi    a3, _bss_end
> 
> Can you please use board_init_f_init_reserve(), etc.? You can copy how
> ARM does things perhaps. This should not be in assembly.

We use them, one screen below.
What exactly should not be in assembly? BSS zeroing?

> > +
> > +       __loopt a2, a3, a4, 2
> > +       s32i    a0, a2, 0
> > +       __endla a2, a3, 4
> > +
> > +       /* Writeback */
> > +
> > +       ___flush_dcache_all a2, a3
> > +
> > +#ifdef __XTENSA_WINDOWED_ABI__
> > +       /*
> > +        * In windowed ABI caller and call target need to be within the same
> > +        * gigabyte. Put the rest of the code into the text segment and jump
> > +        * there.
> > +        */
> > +
> > +       movi    a4, .Lboard_init_code
> > +       jx      a4
> > +
> > +       .text
> > +       .align  4
> > +.Lboard_init_code:
> > +#endif
> > +
> > +       movi    a0, 0
> > +       movi    sp, (CONFIG_SYS_TEXT_ADDR - 16) & 0xfffffff0
> > +
> > +#ifdef CONFIG_DEBUG_UART
> > +       movi    a4, debug_uart_init
> > +#ifdef __XTENSA_CALL0_ABI__
> > +       callx0  a4
> > +#else
> > +       callx4  a4
> > +#endif
> > +#endif
> > +
> > +       movi    a4, board_init_f_alloc_reserve
> > +
> > +#ifdef __XTENSA_CALL0_ABI__
> > +       mov     a2, sp
> > +       callx0  a4
> > +       mov     sp, a2
> > +#else
> > +       mov     a6, sp
> > +       callx4  a4
> > +       movsp   sp, a6
> > +#endif
> > +
> > +       movi    a4, board_init_f_init_reserve
> > +
> > +#ifdef __XTENSA_CALL0_ABI__
> > +       callx0  a4
> > +#else
> > +       callx4  a4
> > +#endif
> > +
> > +        /*
> > +        * Call board initialization routine (never returns).
> > +        */
> > +
> > +       movi    a4, board_init_f
> 
> As above. This is using the old approach.

I see the same sequence in the ARM's _main in arch/arm/lib/crt0.S:

        mov     r0, sp
        bl      board_init_f_alloc_reserve
        mov     sp, r0
        /* set up gd here, outside any C code */
        mov     r9, r0
        bl      board_init_f_init_reserve

        mov     r0, #0
        bl      board_init_f

What is 'the new approach' and how does it differ from the old?

[...]

> > +       /* Does not return here. */
> 
> nit: drop periods before */

Ok.

[...]

> > +fast_alloca_exception: /* must be at _WindowUnderflow4 + 16
> 
> */ ?

Oops, right.

[...]

> > diff --git a/arch/xtensa/cpu/u-boot.lds b/arch/xtensa/cpu/u-boot.lds
> > new file mode 100644
> > index 0000000..853ae5a
> > --- /dev/null
> > +++ b/arch/xtensa/cpu/u-boot.lds
> > @@ -0,0 +1,116 @@
> > +/*
> > + * (C) Copyright 2008 - 2013 Tensilica, Inc.
> > + * (C) Copyright 2014 - 2016 Cadence Design Systems Inc.
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + */
> > +
> > +#include <config.h>
> > +#include <asm/ldscript.h>
> > +#include <asm/arch/core.h>
> > +#include <asm/addrspace.h>
> > +#include <asm-offsets.h>
> > +
> > +OUTPUT_ARCH(xtensa)
> > +ENTRY(_start)
> > +
> > +/*
> > + * U-Boot resets from SYSROM and unpacks itself from a ROM store to RAM.
> > + * The reset vector is usually near the base of SYSROM and has room
> > + * above it for the ROM store into which the rest of U-Boot is packed.
> > + * The ROM store also needs to be above any other vectors that are in ROM.
> > + * If a core has its vectors near the top of ROM, this must be edited.
> > + *
> > + * Note that to run C code out of ROM, the processor would have to support
> > + * 'relocatable' exception vectors and provide a scratch memory for the
> > + * initial stack. Not all Xtensa processor configurations support that, so
> > + * we can simplify the boot process and unpack U-Boot to RAM immediately.
> > + * This, however, requires that memory have been initialized throug some
> > + * other means (serial ROM, for example) or are initialized early (requiring
> > + * an assembler function. See start.S for more details)
> > + */
> > +
> > +SECTIONS
> > +{
> > +  . = + SIZEOF_HEADERS;
> > +  SECTION_ResetVector(XCHAL_RESET_VECTOR_VADDR, LMA_EQ_VMA)
> > +
> > +  .reloc_table ALIGN(4) : FOLLOWING(.ResetVector.text)
> > +  {
> > +    __reloc_table_start = ABSOLUTE(.);
> > +#if XCHAL_HAVE_WINDOWED
> > +    RELOCATE2(WindowVectors,text);
> > +#endif
> > +    RELOCATE2(KernelExceptionVector,literal);
> > +    RELOCATE2(KernelExceptionVector,text);
> > +    RELOCATE2(UserExceptionVector,literal);
> > +    RELOCATE2(UserExceptionVector,text);
> > +    RELOCATE2(DoubleExceptionVector,literal);
> > +    RELOCATE2(DoubleExceptionVector,text);
> > +    RELOCATE1(text);
> > +    RELOCATE1(rodata);
> > +    RELOCATE1(data);
> > +    RELOCATE1(u_boot_list);
> > +    __reloc_table_end = ABSOLUTE(.);
> > +  }
> > +
> > +#if XCHAL_HAVE_WINDOWED
> > +  SECTION_VECTOR(WindowVectors,text,XCHAL_WINDOW_VECTORS_VADDR,
> > +                FOLLOWING(.reloc_table))
> > +  SECTION_VECTOR(KernelExceptionVector,literal,XCHAL_KERNEL_VECTOR_VADDR-8,
> > +                FOLLOWING(.WindowVectors.text))
> > +#else
> > +  SECTION_VECTOR(KernelExceptionVector,literal,XCHAL_KERNEL_VECTOR_VADDR-8,
> > +                FOLLOWING(.reloc_table))
> > +#endif
> > +  SECTION_VECTOR(KernelExceptionVector,text,XCHAL_KERNEL_VECTOR_VADDR,
> > +                FOLLOWING(.KernelExceptionVector.literal))
> > +  SECTION_VECTOR(UserExceptionVector,literal,XCHAL_USER_VECTOR_VADDR-8,
> > +                FOLLOWING(.KernelExceptionVector.text))
> > +  SECTION_VECTOR(UserExceptionVector,text,XCHAL_USER_VECTOR_VADDR,
> > +                FOLLOWING(.UserExceptionVector.literal))
> > +  SECTION_VECTOR(DoubleExceptionVector,literal,XCHAL_DOUBLEEXC_VECTOR_VADDR-8,
> > +                FOLLOWING(.UserExceptionVector.text))
> > +  SECTION_VECTOR(DoubleExceptionVector,text,XCHAL_DOUBLEEXC_VECTOR_VADDR,
> > +                FOLLOWING(.DoubleExceptionVector.literal))
> > +
> > +  __monitor_start = CONFIG_SYS_TEXT_ADDR;
> > +
> > +  SECTION_text(CONFIG_SYS_TEXT_ADDR, FOLLOWING(.DoubleExceptionVector.text))
> > +  SECTION_rodata(ALIGN(16), FOLLOWING(.text))
> > +  SECTION_u_boot_list(ALIGN(16), FOLLOWING(.rodata))
> > +  SECTION_data(ALIGN(16), FOLLOWING(.u_boot_list))
> > +
> > +  __reloc_end = .;
> > +  __init_end = .;
> > +
> > +  SECTION_bss(__init_end (OVERLAY),)
> > +
> > +  __monitor_end = .;
> > +
> > +  /*
> > +   * On many Xtensa boards a region of RAM may be mapped to the ROM address
> > +   * space to facilitate on-chip-debug, and U-Boot must fit with that region.
> > +   * The config variables CONFIG_SYS_MONITOR_* define the region.
> > +   * If U-Boot extends beyond this region it will appear discontiguous in the
> > +   * address space and is in danger of overwriting itself during unpacking
> > +   * ("relocation").
> > +   * This causes U-Boot to crash in a way that is difficult to debug. On some
> > +   * boards (such as xtav60) the region is small enough that U-Boot will not
> > +   * fit if compiled entirely with -O0 (a common scenario). To avoid a lengthy
> > +   * debugging session when this happens, ensure a link-time error occurs.
> > +   *
> > +   */
> > +
> > +   ASSERT(__monitor_end - __monitor_start <= CONFIG_SYS_MONITOR_LEN,
> > +          "U-Boot ROM image is too large. Check optimization level.")
> > +
> > +  SECTION_xtensa
> > +  SECTION_debug
> > +
> > +  /DISCARD/ : { *(.dynstr*) }
> > +  /DISCARD/ : { *(.hash*) }
> > +  /DISCARD/ : { *(.interp) }
> > +  /DISCARD/ : { *(.got*) }
> > +  /DISCARD/ : { *(.dynsym) }
> 
> Are you including the list regions, etc.? u_boot_list is what it is in
> most .lds files.

Yes, it's in the SECTION_u_boot_list a few lines above.

[...]

> > diff --git a/arch/xtensa/include/asm/byteorder.h b/arch/xtensa/include/asm/byteorder.h
> > new file mode 100644
> > index 0000000..485bc4b
> > --- /dev/null
> > +++ b/arch/xtensa/include/asm/byteorder.h
> > @@ -0,0 +1,81 @@
> > +/*
> > + * Based on Linux/Xtensa kernel version
> > + *
> > + * Copyright (C) 2001 - 2007 Tensilica Inc.
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + */
> > +
> > +#ifndef _XTENSA_BYTEORDER_H
> > +#define _XTENSA_BYTEORDER_H
> > +
> > +#include <asm/types.h>
> > +
> > +static inline __attribute__((const)) __u32 ___arch__swab32(__u32 x)
> > +{
> > +       __u32 res;
> > +
> > +       /* instruction sequence from Xtensa ISA release 2/2000 */
> > +       __asm__("ssai     8\n\t"
> > +               "srli     %0, %1, 16\n\t"
> > +               "src      %0, %0, %1\n\t"
> > +               "src      %0, %0, %0\n\t"
> > +               "src      %0, %1, %0\n"
> > +               : "=&a" (res)
> > +               : "a" (x)
> > +               );
> > +       return res;
> > +}
> > +
> > +static inline __attribute__((const)) __u16 ___arch__swab16(__u16 x)
> > +{
> > +       /* Given that 'short' values are signed (i.e., can be negative),
> 
> /*
>  * Given

Ok.

[...]

> > diff --git a/arch/xtensa/lib/bootm.c b/arch/xtensa/lib/bootm.c
> > new file mode 100644
> > index 0000000..8c89d2c
> > --- /dev/null
> > +++ b/arch/xtensa/lib/bootm.c

[...]

> > +/*
> > + * Boot Linux.
> > + */
> > +
> > +int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
> > +{
> > +       struct bp_tag *params, *params_start;
> > +       ulong initrd_start, initrd_end;
> > +       char *commandline = getenv("bootargs");
> > +
> > +       if (!(flag & (BOOTM_STATE_OS_GO | BOOTM_STATE_OS_FAKE_GO)))
> > +               return 0;
> > +
> > +       show_boot_progress(15);
> > +
> > +       if (images->rd_start) {
> > +               initrd_start = images->rd_start;
> > +               initrd_end = images->rd_end;
> > +       } else {
> > +               initrd_start = 0;
> > +               initrd_end = 0;
> > +       }
> > +
> > +       params_start = (struct bp_tag *)gd->bd->bi_boot_params;
> 
> Do you not use device tree with Linux? It looks like you need to
> support both that and tags?

We can use device tree with linux, but that's not mandatory.
When it's used we pass DTB in one of the tags.

-- 
Thanks.
-- Max


More information about the U-Boot mailing list