On Wed, Jul 29, 2020 at 5:56 PM Sean Anderson <sean...@gmail.com> wrote:
>
> This converts the clint driver from the riscv-specific interface to be a
> DM-based UCLASS_TIMER driver. We also need to re-add the initialization for
> IPI back into the SPL code. This was previously implicitly done when the
> timer was initialized. In addition, the SiFive DDR driver previously
> implicitly depended on the CLINT to select REGMAP.
>
> Unlike Andes's PLMT/PLIC (which AFAIK never have anything pass it a dtb),
> the SiFive CLINT is part of the device tree passed in by qemu. This device
> tree doesn't have a clocks or clock-frequency property on clint, so we need
> to fall back on the timebase-frequency property. Perhaps in the future we
> can get a clock-frequency property added to the qemu dtb.
>
> Signed-off-by: Sean Anderson <sean...@gmail.com>
> ---
> This patch builds but has only been tested on the K210 and QEMU. It has NOT
> been tested on a HiFive.
>
> (no changes since v1)
>
> arch/riscv/Kconfig | 4 --
> arch/riscv/lib/sifive_clint.c | 87 +++++++++++++++++++++++------------
> common/spl/spl_opensbi.c | 5 ++
> drivers/ram/sifive/Kconfig | 2 +
> 4 files changed, 64 insertions(+), 34 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index d9155b9bab..aaa3b833a5 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -155,10 +155,6 @@ config 64BIT
> config SIFIVE_CLINT
> bool
> depends on RISCV_MMODE || SPL_RISCV_MMODE
> - select REGMAP
> - select SYSCON
> - select SPL_REGMAP if SPL
> - select SPL_SYSCON if SPL
> help
> The SiFive CLINT block holds memory-mapped control and status
> registers
> associated with software and timer interrupts.
> diff --git a/arch/riscv/lib/sifive_clint.c b/arch/riscv/lib/sifive_clint.c
> index b9a2c649cc..3345a17ad2 100644
> --- a/arch/riscv/lib/sifive_clint.c
> +++ b/arch/riscv/lib/sifive_clint.c
> @@ -8,9 +8,9 @@
> */
>
> #include <common.h>
> +#include <clk.h>
> #include <dm.h>
> -#include <regmap.h>
> -#include <syscon.h>
> +#include <timer.h>
> #include <asm/io.h>
> #include <asm/syscon.h>
> #include <linux/err.h>
> @@ -24,35 +24,19 @@
>
> DECLARE_GLOBAL_DATA_PTR;
>
> -int riscv_get_time(u64 *time)
> -{
> - /* ensure timer register base has a sane value */
> - riscv_init_ipi();
> -
> - *time = readq((void __iomem *)MTIME_REG(gd->arch.clint));
> -
> - return 0;
> -}
> -
> -int riscv_set_timecmp(int hart, u64 cmp)
> -{
> - /* ensure timer register base has a sane value */
> - riscv_init_ipi();
> -
> - writeq(cmp, (void __iomem *)MTIMECMP_REG(gd->arch.clint, hart));
> -
> - return 0;
> -}
> -
> int riscv_init_ipi(void)
> {
> - if (!gd->arch.clint) {
> - long *ret = syscon_get_first_range(RISCV_SYSCON_CLINT);
> + int ret;
> + struct udevice *dev;
>
> - if (IS_ERR(ret))
> - return PTR_ERR(ret);
> - gd->arch.clint = ret;
> - }
> + ret = uclass_get_device_by_driver(UCLASS_TIMER,
> + DM_GET_DRIVER(sifive_clint), &dev);
> + if (ret)
> + return ret;
> +
> + gd->arch.clint = dev_read_addr_ptr(dev);
> + if (!gd->arch.clint)
> + return -EINVAL;
>
> return 0;
> }
> @@ -78,14 +62,57 @@ int riscv_get_ipi(int hart, int *pending)
> return 0;
> }
>
> +static int sifive_clint_get_count(struct udevice *dev, u64 *count)
> +{
> + *count = readq((void __iomem *)MTIME_REG(dev->priv));
> +
> + return 0;
> +}
> +
> +static const struct timer_ops sifive_clint_ops = {
> + .get_count = sifive_clint_get_count,
> +};
> +
> +static int sifive_clint_probe(struct udevice *dev)
> +{
> + int ret;
> + ofnode cpu;
> + struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> + u32 rate;
> +
> + dev->priv = dev_read_addr_ptr(dev);
> + if (!dev->priv)
> + return -EINVAL;
> +
> + /* Did we get our clock rate from the device tree? */
> + if (uc_priv->clock_rate)
> + return 0;
> +
> + /* Fall back to timebase-frequency */
> + cpu = ofnode_path("/cpus");
> + if (!ofnode_valid(cpu))
> + return -EINVAL;
> +
> + ret = ofnode_read_u32(cpu, "timebase-frequency", &rate);
> + if (ret)
> + return ret;
> +
> + log_warning("missing clocks or clock-frequency property, falling back
> on timebase-frequency\n");
> + uc_priv->clock_rate = rate;
> +
> + return 0;
> +}
> +
> static const struct udevice_id sifive_clint_ids[] = {
> - { .compatible = "riscv,clint0", .data = RISCV_SYSCON_CLINT },
RISCV_SYSCON_CLINT should be removed from syscon.h
> + { .compatible = "riscv,clint0" },
> { }
> };
>
> U_BOOT_DRIVER(sifive_clint) = {
> .name = "sifive_clint",
> - .id = UCLASS_SYSCON,
> + .id = UCLASS_TIMER,
> .of_match = sifive_clint_ids,
> + .probe = sifive_clint_probe,
> + .ops = &sifive_clint_ops,
> .flags = DM_FLAG_PRE_RELOC,
> };
> diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
> index 14f335f75f..3440bc0294 100644
> --- a/common/spl/spl_opensbi.c
> +++ b/common/spl/spl_opensbi.c
> @@ -79,6 +79,11 @@ void spl_invoke_opensbi(struct spl_image_info *spl_image)
> invalidate_icache_all();
>
> #ifdef CONFIG_SPL_SMP
> + /* Initialize the IPI before we use it */
> + ret = riscv_init_ipi();
> + if (ret)
> + hang();
Why above is needed? This is already called in arch_cpu_init_dm() in
arch/riscv/cpu/cpu.c
> +
> /*
> * Start OpenSBI on all secondary harts and wait for acknowledgment.
> *
> diff --git a/drivers/ram/sifive/Kconfig b/drivers/ram/sifive/Kconfig
> index 6aca22ab2a..31ad0a7e45 100644
> --- a/drivers/ram/sifive/Kconfig
> +++ b/drivers/ram/sifive/Kconfig
> @@ -9,5 +9,7 @@ config SIFIVE_FU540_DDR
> bool "SiFive FU540 DDR driver"
> depends on RAM_SIFIVE
> default y if TARGET_SIFIVE_FU540
> + select REGMAP
> + select SPL_REGMAP if SPL
I don't understand why this driver depends on REGMAP?
> help
> This enables DDR support for the platforms based on SiFive FU540
> SoC.
> --
Regards,
Bin
