Re: [PATCH] Fix CLINT clock frequency for SiFive E

[Daniel Henrique Barboza]

Hi Roman!

It helps to add the maintainers/reviewers in the CC when sending the patch. You
can see who need to be CCed by using the get_maintainer.pl script. E.g:

./scripts/get_maintainer.pl \[PATCH\]\ Fix\ CLINT\ clock\ frequency\ for\ 
SiFive\ E\ -\ rcardenas.rod@gmail.com\ -\ 2023-11-10\ 1314.eml

Alistair Francis <Alistair.Francis@wdc.com> (supporter:SiFive Machines)
Bin Meng <bin.meng@windriver.com> (supporter:SiFive Machines)
Palmer Dabbelt <palmer@dabbelt.com> (supporter:SiFive Machines)
Weiwei Li <liwei1518@gmail.com> (reviewer:RISC-V TCG CPUs)
Daniel Henrique Barboza <dbarboza@ventanamicro.com> (reviewer:RISC-V TCG CPUs)
Liu Zhiwei <zhiwei_liu@linux.alibaba.com> (reviewer:RISC-V TCG CPUs)
qemu-riscv@nongnu.org (open list:SiFive Machines)
qemu-devel@nongnu.org (open list:All patches CC here)


I'm CCing all these folks in the reply.


On 11/10/23 13:14, rcardenas.rod@gmail.com wrote:
> From: Román Cárdenas Rodríguez <rcardenas.rod@gmail.com>
>
> ---
>   hw/riscv/sifive_e.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> index 0d37adc542..87d9602383 100644
> --- a/hw/riscv/sifive_e.c
> +++ b/hw/riscv/sifive_e.c
> @@ -225,7 +225,7 @@ static void sifive_e_soc_realize(DeviceState *dev, Error 
> **errp)
>               RISCV_ACLINT_SWI_SIZE,
>           RISCV_ACLINT_DEFAULT_MTIMER_SIZE, 0, ms->smp.cpus,
>           RISCV_ACLINT_DEFAULT_MTIMECMP, RISCV_ACLINT_DEFAULT_MTIME,
> -        RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ, false);
> +        SIFIVE_E_LFCLK_DEFAULT_FREQ, false);

I'm not sure if this is correct. The last commit that touched this line was 
b8fb878aa2
("hw/intc: Upgrade the SiFive CLINT implementation to RISC-V ACLINT"). If you 
see the commit
diff, the previous value was:

-            SIFIVE_CLINT_TIMEBASE_FREQ, false);

In this same commit we can see that the existing value of that macro back then 
was:

-    SIFIVE_CLINT_TIMEBASE_FREQ = 10000000


Which is the same value of RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ.


The value you're changing to, SIFIVE_E_LFCLK_DEFAULT_FREQ (32768), seems to be 
related to the
SIFIVE AON watchdog implemented in hw/misc/sifive_e_aon.c:

static void sifive_e_aon_init(Object *obj)
{
    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
    SiFiveEAONState *r = SIFIVE_E_AON(obj);

    memory_region_init_io(&r->mmio, OBJECT(r), &sifive_e_aon_ops, r,
                          TYPE_SIFIVE_E_AON, SIFIVE_E_AON_MAX);
    sysbus_init_mmio(sbd, &r->mmio);

    /* watchdog timer */
    r->wdog_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
                                 sifive_e_aon_wdt_expired_cb, r);
    r->wdogclk_freq = SIFIVE_E_LFCLK_DEFAULT_FREQ;   <==========
    sysbus_init_irq(sbd, &r->wdog_irq);
}



Thanks,


Daniel



>       sifive_e_prci_create(memmap[SIFIVE_E_DEV_PRCI].base);
>   
>       /* AON */