Re: [U-Boot] [PATCH 04/19] cpu: Add a RISC-V CPU driver

Hi Bin,

On Tue, 2018-11-13 at 00:21 -0800, Bin Meng wrote:

This adds a driver for RISC-V CPU. Note the driver will bind a RISC-V timer driver if "timebase-frequency" property is present in the device tree.

Signed-off-by: Bin Meng bmeng.cn@gmail.com

Since we have the CPU driver, we could also enable CMD_CPU.

drivers/cpu/Kconfig | 6 +++ drivers/cpu/Makefile | 1 + drivers/cpu/riscv_cpu.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+) create mode 100644 drivers/cpu/riscv_cpu.c

diff --git a/drivers/cpu/Kconfig b/drivers/cpu/Kconfig index d405200..3d5729f 100644 --- a/drivers/cpu/Kconfig +++ b/drivers/cpu/Kconfig @@ -13,3 +13,9 @@ config CPU_MPC83XX select CLK_MPC83XX help Support CPU cores for SoCs of the MPC83xx series.

+config CPU_RISCV

• bool "Enable RISC-V CPU driver"
• depends on CPU && RISCV
• help

•  Support CPU cores for RISC-V architecture.
  

diff --git a/drivers/cpu/Makefile b/drivers/cpu/Makefile index 858b037..be0300c 100644 --- a/drivers/cpu/Makefile +++ b/drivers/cpu/Makefile @@ -8,4 +8,5 @@ obj-$(CONFIG_CPU) += cpu-uclass.o

obj-$(CONFIG_ARCH_BMIPS) += bmips_cpu.o obj-$(CONFIG_CPU_MPC83XX) += mpc83xx_cpu.o +obj-$(CONFIG_CPU_RISCV) += riscv_cpu.o obj-$(CONFIG_SANDBOX) += cpu_sandbox.o diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c new file mode 100644 index 0000000..23917db --- /dev/null +++ b/drivers/cpu/riscv_cpu.c @@ -0,0 +1,117 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2018, Bin Meng bmeng.cn@gmail.com
• */

+#include <common.h> +#include <cpu.h> +#include <dm.h> +#include <errno.h> +#include <dm/device-internal.h> +#include <dm/lists.h>

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

• const char *isa;
• isa = dev_read_string(dev, "riscv,isa");
• if (size < (strlen(isa) + 1))

• return -ENOSPC;
  

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

+}

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

• const char *mmu;
• dev_read_u32(dev, "clock-frequency", (u32 *)&info->cpu_freq);
• mmu = dev_read_string(dev, "mmu-type");
• if (!mmu)

• info->features |= BIT(CPU_FEAT_MMU);
  

BBL also disables CPUs without an MMU, so it is good to have this information. Where would you disable the CPU in U-Boot? Maybe in arch_fixup_fdt() or in the CPU driver?

• return 0;

+}

+static int riscv_cpu_get_count(struct udevice *dev) +{

• ofnode node;
• int num = 0;
• ofnode_for_each_subnode(node, dev_ofnode(dev->parent)) {

• const char *device_type;
  

• device_type = ofnode_read_string(node, "device_type");
  

• if (!device_type)
  

• 	continue;
  

• if (strcmp(device_type, "cpu") == 0)
  

• 	num++;
  

• }
• return num;

+}

+static int riscv_cpu_bind(struct udevice *dev) +{

• struct cpu_platdata *plat = dev_get_parent_platdata(dev);
• struct driver *drv;
• struct udevice *timer;
• int ret;
• /* save the hart id */
• plat->cpu_id = dev_read_addr(dev);
• /* first examine the property in current cpu node */
• ret = dev_read_u32(dev, "timebase-frequency", &plat-

...

timebase_freq);

• /* if not found, then look at the parent /cpus node */
• if (ret)

• dev_read_u32(dev->parent, "timebase-frequency",
  

• 	     &plat->timebase_freq);
  

• /*

• * Bind riscv-timer driver on hart 0
  

• *
  

• * We only instantiate one timer device which is enough for U-
  

Boot.

• * Pass the "timebase-frequency" value as the driver data for
  

the

• * timer device.
  

• *
  

• * Return value is not checked since it's possible that the
  

timer

• * driver is not included.
  

• */
  

• if (!plat->cpu_id && plat->timebase_freq) {

• drv = lists_driver_lookup_name("riscv_timer");
  

• if (!drv) {
  

• 	debug("Cannot find the timer driver, not
  

included?\n");

• 	return 0;
  

• }
  

• device_bind_with_driver_data(dev, drv, "riscv_timer",
  

• 			     plat->timebase_freq,
  

ofnode_null(),

• 			     &timer);
  

You don't need the timer variable here, you can just pass NULL.

I did not see that you are not checking the return value here, when I wrote my previous email regarding the syscon driver. So it is not actually a problem if two different device implement the functionality for the syscon APIs. I think it is still worth considering to implement something like the misc driver I suggested, however.

Thanks, Lukas

• }
• return 0;

+}

+static const struct cpu_ops riscv_cpu_ops = {

• .get_desc = riscv_cpu_get_desc,
• .get_info = riscv_cpu_get_info,
• .get_count = riscv_cpu_get_count,

+};

+static const struct udevice_id riscv_cpu_ids[] = {

• { .compatible = "riscv" },
• { }

+};

+U_BOOT_DRIVER(riscv_cpu) = {

• .name = "riscv_cpu",
• .id = UCLASS_CPU,
• .of_match = riscv_cpu_ids,
• .bind = riscv_cpu_bind,
• .ops = &riscv_cpu_ops,
• .flags = DM_FLAG_PRE_RELOC,

+};