[U-Boot] [PATCH v2 1/4] riscv: hart_lottery and available harts features can be selectable

[Bin Meng]

Hi Rick,

On Wed, Apr 24, 2019 at 2:37 PM Andes <uboot at andestech.com> wrote:
>
> From: Rick Chen <rick at andestech.com>
>

I would write the commit title and message as:

riscv: Introduce CONFIG_XIP to support booting from flash

When U-Boot boots from flash, during the boot process, hart_lottery
and available_harts_lock variable addresses point to flash which is
not writable. This causes boot failures on AE350. Introduce a config
option CONFIG_XIP to support such configuration.

> flash.
>
> Add CONFIG_XIP to NOT select this two features. It's default value
> will say NO for booting from ram.
>
> AE350 will encounter the the write failure problem since
> hart_lottery and available_harts_lock was not in ram address but
> in flash address when booing from flash.
>
> This patch can help to fix the write failure problem when AE350
> booting from flash by disabling this two features.
>
> Signed-off-by: Rick Chen <rick at andestech.com>
> Cc: Greentime Hu <greentime at andestech.com>
> ---
>  arch/riscv/Kconfig                   | 10 ++++++++++
>  arch/riscv/cpu/cpu.c                 |  3 ++-
>  arch/riscv/cpu/start.S               |  7 ++++++-
>  arch/riscv/include/asm/global_data.h |  2 ++
>  arch/riscv/lib/asm-offsets.c         |  2 ++
>  arch/riscv/lib/smp.c                 |  2 ++
>  6 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index ae8ff7b..fb9a8c6 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -162,6 +162,16 @@ config SBI_IPI
>         default y if RISCV_SMODE
>         depends on SMP
>
> +config XIP
> +       bool "XIP mode"
> +       default n

nits: this is not necessary as by default all config options are n

> +       help
> +         XIP (eXecute In Place) is a method for executing code directly
> +         from a serial NOR flash memory without copying the code to ram.

It's not necessary to be a serial NOR flash. A parallel flash can be
the same. I think you can just mention NOR flash memory.

> +         This must NOT support hart lottery and available harts features.
> +         These two feature only can be enabled when U-Boot booting from
> +         ram, but shall be disabled when booting from flash.

remove the rest of the help message, and something like this:

Say yes here if U-Boot boots from flash directly.

> +
>  config STACK_SIZE_SHIFT
>         int
>         default 13
> diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> index c32de8a..768c44c 100644
> --- a/arch/riscv/cpu/cpu.c
> +++ b/arch/riscv/cpu/cpu.c
> @@ -16,13 +16,14 @@
>   * before the bss section is available.
>   */
>  phys_addr_t prior_stage_fdt_address __attribute__((section(".data")));
> +#ifndef CONFIG_XIP
>  u32 hart_lottery __attribute__((section(".data"))) = 0;
> -
>  /*
>   * The main hart running U-Boot has acquired available_harts_lock until it has
>   * finished initialization of global data.
>   */
>  u32 available_harts_lock = 1;
> +#endif
>
>  static inline bool supports_extension(char ext)
>  {
> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> index a4433fb..41d9a32 100644
> --- a/arch/riscv/cpu/start.S
> +++ b/arch/riscv/cpu/start.S
> @@ -98,6 +98,7 @@ call_board_init_f_0:
>         mv      sp, a0
>  #endif
>
> +#ifndef CONFIG_XIP
>         /*
>          * Pick hart to initialize global data and run U-Boot. The other harts
>          * wait for initialization to complete.
> @@ -106,6 +107,9 @@ call_board_init_f_0:
>         li      s2, 1
>         amoswap.w s2, t1, 0(t0)
>         bnez    s2, wait_for_gd_init
> +#else
> +       bnez    tp, secondary_hart_loop
> +#endif
>
>         la      t0, prior_stage_fdt_address
>         SREG    s1, 0(t0)
> @@ -115,6 +119,7 @@ call_board_init_f_0:
>         /* save the boot hart id to global_data */
>         SREG    tp, GD_BOOT_HART(gp)
>
> +#ifndef CONFIG_XIP
>         la      t0, available_harts_lock
>         fence   rw, w
>         amoswap.w zero, zero, 0(t0)
> @@ -141,7 +146,7 @@ wait_for_gd_init:
>          * secondary_hart_loop.
>          */
>         bnez    s2, secondary_hart_loop
> -

Please keep the original blank line here.

> +#endif
>         /* Enable cache */
>         jal     icache_enable
>         jal     dcache_enable
> diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
> index dffcd45..b74bd7e 100644
> --- a/arch/riscv/include/asm/global_data.h
> +++ b/arch/riscv/include/asm/global_data.h
> @@ -27,7 +27,9 @@ struct arch_global_data {
>  #ifdef CONFIG_SMP
>         struct ipi_data ipi[CONFIG_NR_CPUS];
>  #endif
> +#ifndef CONFIG_XIP
>         ulong available_harts;
> +#endif
>  };
>
>  #include <asm-generic/global_data.h>
> diff --git a/arch/riscv/lib/asm-offsets.c b/arch/riscv/lib/asm-offsets.c
> index f998402..4fa4fd3 100644
> --- a/arch/riscv/lib/asm-offsets.c
> +++ b/arch/riscv/lib/asm-offsets.c
> @@ -14,7 +14,9 @@
>  int main(void)
>  {
>         DEFINE(GD_BOOT_HART, offsetof(gd_t, arch.boot_hart));
> +#ifndef CONFIG_XIP
>         DEFINE(GD_AVAILABLE_HARTS, offsetof(gd_t, arch.available_harts));
> +#endif
>
>         return 0;
>  }
> diff --git a/arch/riscv/lib/smp.c b/arch/riscv/lib/smp.c
> index caa292c..cc66f15 100644
> --- a/arch/riscv/lib/smp.c
> +++ b/arch/riscv/lib/smp.c
> @@ -63,9 +63,11 @@ static int send_ipi_many(struct ipi_data *ipi)
>                         continue;
>                 }
>
> +#ifndef CONFIG_XIP
>                 /* skip if hart is not available */
>                 if (!(gd->arch.available_harts & (1 << reg)))
>                         continue;
> +#endif
>
>                 gd->arch.ipi[reg].addr = ipi->addr;
>                 gd->arch.ipi[reg].arg0 = ipi->arg0;
> --

Regards,
Bin