[PATCH v2 4/7] riscv: Rework Sifive CLINT as UCLASS_TIMER driver

Sean Anderson seanga2 at gmail.com
Thu Aug 20 12:23:08 CEST 2020


On 8/20/20 3:43 AM, Bin Meng wrote:
> On Wed, Jul 29, 2020 at 5:56 PM Sean Anderson <seanga2 at 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 <seanga2 at 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

This can't be removed because the IPI driver still depends on it.

>> +       { .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

I believe I ran into problems with it not getting called in SPL when
testing. I can look into whether this is still needed.

> 
>> +
>>         /*
>>          * 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?

I don't either, but it fails to build without these two lines.
Previously, regmap was always enabled by the sifive clint. So now that
it no longer enables regmap, this dependency became apparent.

> 
>>         help
>>           This enables DDR support for the platforms based on SiFive FU540 SoC.
>> --
> 
> Regards,
> Bin
> 



More information about the U-Boot mailing list