Quartus 21, Critical Warning 'will power up to High'

[andkae]

Hi Stephan,

at the moment i receive an critical warning from the synthesis tool

Critical Warning (18010): Register neorv32_top:neorv32_i|neorv32_cpu:neorv32_cpu_inst|neorv32_cpu_control:neorv32_cpu_control_inst|execute_engine.state[3] will power up to High

In the Reset branch is the following assigned:

neorv32/rtl/core/neorv32_cpu_control.vhd

Line 667 in db446b1

execute_engine.state <= BRANCHED; -- reset is a branch from "somewhere"

If the assigned element in the reset path is not the first element from the type defintion you'll receive this warning from Quartus

neorv32/rtl/core/neorv32_cpu_control.vhd

Line 190 in db446b1

type execute_engine_state_t is (DISPATCH, TRAP_ENTER, TRAP_START, TRAP_EXIT, TRAP_EXECUTE,

neorv32/rtl/core/neorv32_cpu_control.vhd

Line 191 in db446b1

EXECUTE, ALU_WAIT, BRANCH, BRANCHED, SYSTEM, MEM_REQ, MEM_WAIT);

Would you see an negativ effect if we change the type definition to following:

  type execute_engine_state_t is (BRANCHED, DISPATCH, TRAP_ENTER, TRAP_START, TRAP_EXIT, TRAP_EXECUTE,
                                  EXECUTE, ALU_WAIT, BRANCH, SYSTEM, MEM_REQ, MEM_WAIT);

If no i can test to check if this change would avoid this warning.

BR,
Andreas

[GideonZ]

Hi Andreas, Is this warning relevant if the core is being reset for some cycles after power on? Best regards, Gideon

…

On Mon, Sep 19, 2022 at 3:30 PM Andreas ***@***.***> wrote: Hi Stephan, at the moment i receive an critical warning from the synthesis tool Critical Warning (18010): Register neorv32_top:neorv32_i|neorv32_cpu:neorv32_cpu_inst|neorv32_cpu_control:neorv32_cpu_control_inst|execute_engine.state[3] will power up to High In the Reset branch is the following assigned: https://github.com/stnolting/neorv32/blob/db446b10f1810f8010d0a1430f59694dc499ee5c/rtl/core/neorv32_cpu_control.vhd#L667 If the assigned element in the reset path is not the first element from the type defintion you'll receive this warning from Quartus https://github.com/stnolting/neorv32/blob/db446b10f1810f8010d0a1430f59694dc499ee5c/rtl/core/neorv32_cpu_control.vhd#L190 https://github.com/stnolting/neorv32/blob/db446b10f1810f8010d0a1430f59694dc499ee5c/rtl/core/neorv32_cpu_control.vhd#L191 Would you see an negativ effect if we change the type definition to following: type execute_engine_state_t is (BRANCHED, DISPATCH, TRAP_ENTER, TRAP_START, TRAP_EXIT, TRAP_EXECUTE, EXECUTE, ALU_WAIT, BRANCH, SYSTEM, MEM_REQ, MEM_WAIT); If no i can test to check if this change would avoid this warning. BR, Andreas — Reply to this email directly, view it on GitHub <#415>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACUFDSK3JDJNUURRMSL3Z4TV7BTINANCNFSM6AAAAAAQQDQODM> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[andkae]

Is this warning relevant if the core is being reset for some cycles after power on?

Here is the problem a little more discussed: https://www.mikrocontroller.net/topic/491820
The problem is, that the reset value differs from the initial value. That means the FSM will start not in the reset state, will run at least one clock cycle until the synchron reset is propagated.

[stnolting]

Hey there!

This is an interesting issue. I have never thought about the encoding of the FSM's reset state and I think I have never seen a critical warning like this in Quartus...

Is this warning relevant if the core is being reset for some cycles after power on?

From a functional point of view there should be no problem. The processor's "reset generator" ensures that any external reset request stays active for at least 4 clock cycles - so there should be sufficient time for the FSM to move to the correct reset state.

neorv32/rtl/core/neorv32_top.vhd

Lines 453 to 475 in ca9d5b1

	-- Reset Generator ------------------------------------------------------------------------
	-- -------------------------------------------------------------------------------------------
	reset_generator: process(rstn_i, clk_i)
	begin
	if (rstn_i = '0') then
	rstn_ext_sreg <= (others => '0');
	rstn_int_sreg <= (others => '0');
	rstn_ext <= '0';
	rstn_int <= '0';
	elsif falling_edge(clk_i) then -- inverted clock to release reset _before_ all FFs trigger (rising edge)
	-- external reset --
	rstn_ext_sreg <= rstn_ext_sreg(rstn_ext_sreg'left-1 downto 0) & '1'; -- active for at least <rstn_ext_sreg'size> clock cycles
	-- internal reset --
	if (rstn_wdt = '0') or (dci_ndmrstn = '0') then -- sync reset sources
	rstn_int_sreg <= (others => '0');
	else
	rstn_int_sreg <= rstn_int_sreg(rstn_int_sreg'left-1 downto 0) & '1'; -- active for at least <rstn_int_sreg'size> clock cycles
	end if;
	-- reset nets --
	rstn_ext <= and_reduce_f(rstn_ext_sreg); -- external reset (via reset pin)
	rstn_int <= and_reduce_f(rstn_int_sreg); -- internal reset (via reset pin, WDT or OCD)
	end if;
	end process reset_generator;

Would you see an negativ effect if we change the type definition to following:

I just had a quick look at https://www.mikrocontroller.net/topic/491820 (-> google translate). Maybe it would be a good thing to change the state encoding / the state sequence definitions - at least this would not break a thing. If you like you can do a PR 😉

[andkae]

Okay, perfect, i will test on my design. And if it works i prepare an PR.

[akaeba]

Hi Stephan,

PR #423 is ready. After this change was the warning in Quartus gone.

BR,
Andreas