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

["Román Cárdenas Rodríguez" <rcardenas.rod@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4248 bytes --]

Hi, Daniel!

Sorry for that, I’m quite new to this way of contributing (you may notice that I sent the same patch several times, I apologize).

If you check the manual of SiFive E310 (https://cdn.sparkfun.com/assets/7/f/0/2/7/fe310-g002-manual-v19p05.pdf?_gl=1*w2ieef*_ga*MTcyNDI2MjM0Ny4xNjk2ODcwNTM3*_ga_T369JS7J9N*MTY5Njg3MDUzNy4xLjAuMTY5Njg3MDUzNy42MC4wLjA.), you can see in Figure 1 that the CLINT is connected to the real time clock, which also feeds the AON peripheral (they share the same clock).

In page 43, the docs also say that the timer registers of the CLINT count ticks from the rtcclk.

I attached a couple of screenshots to ease the process.

Thank you very much for your time, let me know if I can help you with further documentation

PS: I am currently playing with bare metal applications both in QEMU and a physical SiFive E310 board and I confirm that the CLINT clock in the physical board runs at 32.768 kHz. In QEMU, the same app produces a completely different outcome, as sometimes a new CLINT interrupt is triggered before finishing other tasks.






> On 15 Nov 2023, at 21:51, Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote:
> 
> 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 */


[-- Attachment #2.1: Type: text/html, Size: 6129 bytes --]

[-- Attachment #2.2: Screenshot 2023-11-15 at 21.58.17.png --]
[-- Type: image/png, Size: 142741 bytes --]

[-- Attachment #2.3: Screenshot 2023-11-15 at 21.57.27.png --]
[-- Type: image/png, Size: 314919 bytes --]