sergeykhbr / riscv_vhdl

Portable RISC-V System-on-Chip implementation: RTL, debugger and simulators
http://sergeykhbr.github.io/riscv_vhdl/
Apache License 2.0
628 stars 104 forks source link

gptimer alignment in the second timer is not correct #33

Closed hossameldin1995 closed 4 years ago

hossameldin1995 commented 4 years ago

Hello,

In riscv_vhdl/rtl/misclib/axi4_gptimers.vhd while reading and writing register data for timer (control, init_value and value) you are adding 16 + 8*k while k is the index of timer (0 or 1) to the address to align the registers.

this is the typedef map for the timers in map_gptimers.h file typedef struct gptimer_type { volatile uint32_t control; volatile uint32_t rsv1; volatile uint64_t cur_value; volatile uint64_t init_value; } gptimer_type; typedef struct gptimers_map { uint64_t highcnt; uint32_t pending; uint32_t rsvr[13]; gptimer_type timer[2]; } gptimers_map;

So you are adding 16 for (highcnt, pending and rsvr[13]). the size of them is (8 + 4 + (4*13)) = 64 64 / 4 = 16 and this is what you done in axi4_gptimers.vhd

And you are adding 8 for (gptimer_type). the size of gptimer_type is (8 * 3) = 24 24 / 4 = 6 and you are writing 8 in axi4_gptimers.vhd

I tryed to use the second timer and I noticed that it is not working. So, after debugging I found that it should be 6 instead of 8 in map_gptimers.h, So I changed it and it works fine.

If I mistake something please correct me.

Thanks

sergeykhbr commented 4 years ago

Thank you, for your investigation. Definitely you found a bug. Correct timer's structure should be 8*sizeof(uint32_t) size:

typedef struct gptimer_type {
    volatile uint32_t control;
    volatile uint32_t rsv1;
    volatile uint64_t cur_value;
    volatile uint64_t init_value;
    volatile uint32_t rsrv2[2];
} gptimer_type;
hossameldin1995 commented 4 years ago

Thank you for your clarification.