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

[Auer, Lukas]

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 at 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 at 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,
> +};