Skip to content

Fix build failure when TIMER_CNT=1 (#7) - #8

Open
anhtuan98 wants to merge 2 commits into
pulp-platform:masterfrom
anhtuan98:fix/timer-cnt-1-clog2
Open

anhtuan98 wants to merge 2 commits into
pulp-platform:masterfrom
anhtuan98:fix/timer-cnt-1-clog2

Conversation

@anhtuan98

Copy link
Copy Markdown

Fixes #7.

Bug

apb_timer fails to elaborate when TIMER_CNT is set to 1. $clog2(TIMER_CNT) is 0 in that case, which makes slave_address_int's declared range [$clog2(TIMER_CNT)-1:0] evaluate to [-1:0] -- an inverted, 2-bit-wide range instead of the intended 0-width -- and the PADDR part-select driving it an inverted [3:4] slice (relative to PADDR's own descending [APB_ADDR_WIDTH-1:0] declaration). This matches the "part-select ... does not match declaration" error from the original report (Vivado Synth 8-523); reproduced here with Verilator:

%Warning-ASCRANGE: src/apb_timer.sv:34:11: Ascending bit range vector: left < right of bit range: [-1:0]
%Warning-SELRANGE: src/apb_timer.sv:37:37: [3:4] Slice range has ascending bit ordering, perhaps you wanted [4:3]
%Error: Exiting due to 14 warning(s)

Fix

Adds a SLAVE_ADDR_BITS localparam that special-cases TIMER_CNT == 1 to a constant-zero, 1-bit slave_address_int, instead of computing a negative width from $clog2(1). With only one timer, no address bits are needed to select among timers, so this sidesteps the edge case entirely rather than working around its symptom. The TIMER_CNT > 1 path is unchanged -- it's the same expression as before, just moved into its own generate branch (gen_slave_address_multi) alongside the new single-timer branch (gen_slave_address_single).

Verified TIMER_CNT=2 and TIMER_CNT=4 are unaffected (re-linted before and after this change).

Test

Added a cocotb regression test (verify/, Verilator) that builds apb_timer with TIMER_CNT=1 and drives the one timer instance over its APB interface: enabling it and confirming the count register advances, and a separate write/read-back check on REG_CMP. Both fail to build against the unmodified RTL (the SELRANGE error above) and pass against this fix.

$clog2(TIMER_CNT) is 0 when TIMER_CNT == 1, which made
slave_address_int's declared range [-1:0] (an inverted, 2-bit-wide
range instead of the intended 0-width) and the PADDR part-select
driving it an inverted [x:x+1] range -- both illegal on a
descending-declared vector, matching the reporter's "part-select ...
does not match declaration" symptom.

Add a SLAVE_ADDR_BITS localparam that special-cases TIMER_CNT == 1 to
a constant-zero, 1-bit slave_address_int instead of computing a
negative width from $clog2(1). There is only one timer in that
configuration, so no address bits are needed to select among timers.
The TIMER_CNT > 1 path is untouched, just moved into its own
generate branch alongside the new single-timer branch.

Fixes pulp-platform#7
…form#7)

Two tests against the sole timer instance in a TIMER_CNT=1 build,
driven over its APB interface (Verilator, cocotb classic Makefile
flow):

- test_single_timer_builds_and_counts: enables the timer and confirms
  its count register actually advances, proving the TIMER_CNT==1
  address-select path reaches the instance (not just that it
  elaborates).
- test_single_timer_compare_register: writes and reads back REG_CMP
  through the same path, as a second, independent register.

Against the unmodified RTL this fails to even build (the SELRANGE
lint error from pulp-platform#7); against the fix in the parent commit both tests
pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Timer does not build if NUM_TIMERS is set to 1

1 participant