Re: [U-Boot] [PATCH v2] nios2: convert nios2 cpu to driver model

On Tuesday, September 29, 2015 at 03:28:01 AM, Thomas Chou wrote:

Convert nios2 cpu to driver model. The cpu parameters are extracted from device tree and saved to global data structure. We will use them to replace the custom_fpga.h .

Signed-off-by: Thomas Chou thomas@wytron.com.tw

Hi!

Minor nitpicks below.

---

v2 move cpu param setup to arch_cpu_init_dm, remove probe.

arch/Kconfig | 3 ++ arch/nios2/cpu/cpu.c | 76 ++++++++++++++++++++++++++++++++++-- arch/nios2/dts/3c120_devboard.dts | 1 + arch/nios2/include/asm/global_data.h | 8 ++++ configs/nios2-generic_defconfig | 2 - 5 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig index 207c778..9be1538 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -64,6 +64,9 @@ config NIOS2 select HAVE_GENERIC_BOARD select SYS_GENERIC_BOARD select SUPPORT_OF_CONTROL

• select OF_CONTROL
• select DM
• select CPU

What's this CONFIG_CPU for please ?

config OPENRISC bool "OpenRISC architecture" diff --git a/arch/nios2/cpu/cpu.c b/arch/nios2/cpu/cpu.c index 39ae972..88984e2 100644 --- a/arch/nios2/cpu/cpu.c +++ b/arch/nios2/cpu/cpu.c @@ -6,7 +6,9 @@ */

#include <common.h> -#include <asm/nios2.h> +#include <cpu.h> +#include <dm.h> +#include <errno.h> #include <asm/cache.h>

DECLARE_GLOBAL_DATA_PTR; @@ -51,10 +53,78 @@ void dcache_disable(void) flush_dcache(CONFIG_SYS_DCACHE_SIZE, CONFIG_SYS_DCACHELINE_SIZE); }

-int arch_cpu_init(void) +int arch_cpu_init_dm(void) {

• gd->cpu_clk = CONFIG_SYS_CLK_FREQ;

• struct udevice *dev;
• uclass_first_device(UCLASS_CPU, &dev);
• if (!dev)

• return -ENODEV;
  

• gd->cpu_clk = fdtdec_get_int(gd->fdt_blob, dev->of_offset,

• "clock-frequency", 0);
  

• gd->arch.dcache_line_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset,

• "dcache-line-size", 0);
  

• gd->arch.icache_line_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset,

• "icache-line-size", 0);
  

• gd->arch.dcache_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset,

• "dcache-size", 0);
  

• gd->arch.icache_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset,

• "icache-size", 0);
  

• gd->arch.reset_addr = fdtdec_get_int(gd->fdt_blob, dev->of_offset,

• "altr,reset-addr", 0);
  

• gd->arch.exception_addr = fdtdec_get_int(gd->fdt_blob, dev->of_offset,

• "altr,exception-addr", 0);
  

• gd->arch.has_mmu = fdtdec_get_int(gd->fdt_blob, dev->of_offset,

• "altr,has-mmu", 0);
  

Shouldn't there be some sort of return value checking here ?

• gd->arch.io_region_base = gd->arch.has_mmu ? 0xe0000000 : 0x8000000; gd->ram_size = CONFIG_SYS_SDRAM_SIZE;

  return 0;

}

+static int altera_nios2_get_desc(struct udevice *dev, char *buf, int size) +{

• const char *cpu_name = "Nios-II";
• if (size < strlen(cpu_name))

• return -ENOSPC;
  

• strcpy(buf, cpu_name);
• return 0;

+}

+static int altera_nios2_get_info(struct udevice *dev, struct cpu_info *info) +{

• info->cpu_freq = gd->cpu_clk;
• info->features = 1 << CPU_FEAT_L1_CACHE

• | (gd->arch.has_mmu ? 1 << CPU_FEAT_MMU : 0);
  

I'd add parenthesis around the bitshifts, for the sake of clarity. Also, please put the ORR operator at the end of the line.

• return 0;

+}

Thanks a lot :)