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...@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...@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, > +}; _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot