VHDL: case when using constants constructs

Question

I'm having some trouble with the following statement

if spi_ctr = 9 then
 case spi_op is
 when spi_op = op_get_r_count => spi_reg_out <= ref_count_i;
 when spi_op = op_get_s_count => spi_reg_out <= sig_count_i;
 when spi_op = op_get_g_limit => spi_reg_put <= gate_limit_i;
 end case;

Quartus sinthetizer spits the following errors

Error (10500): VHDL syntax error at spi_iface.vhd(93) near text "="; expecting "!", or "=>"

Error (10500): VHDL syntax error at spi_iface.vhd(94) near text "="; expecting "!", or "=>"

Error (10500): VHDL syntax error at spi_iface.vhd(95) near text "="; expecting "!", or "=>"

i don't see a syntax error around however, i also tried using the following

spi_reg_out <= ref_count_i when spi_op = op_get_r_count else
 sig_count_i when spi_op = op_get_s_count else
 gate_limit_i when spi_op = op_get_g_limit;

and it spits a similar error

the constants are defined as follows

-- read operations
constant op_get_r_count: std_logic_vector(7 downto 0) := "00000001";
constant op_get_s_count: std_logic_vector(7 downto 0) := "00000010";
constant op_get_g_limit: std_logic_vector(7 downto 0) := "00000011";

spi_op is as follows

signal spi_op: std_logic_vector(7 downto 0);

is there something obvious i'm not seeing or you need to use literals in these kind of statements?

--- Edit

after following the recomendations from @Oldfart i changed the code to this

case spi_op is
 when op_get_r_count => spi_reg_out <= ref_count_i;
 when op_get_s_count => spi_reg_out <= sig_count_i;
 when op_get_g_limit => spi_reg_out <= gate_limit_i;
 when others => spi_reg_out <= (others => '0');
 end case;

and voila! it Almost works

however it outputs quite a weird error that really caught me with my pants off

Error (10344): VHDL expression error at spi_iface.vhd(102): expression has 31 elements, but must have 32 elements

?! shouldn't the last statement take care of that?

--- edit again

actually that error was from an other line, dumb me

Oldfart post your answer if you want the kudos :)

• Oldfart: Remove the spi_op = inside the case. The comparison is done by the case statement itself . So instead use: when op_get_r_count => .... It seems you are not using all combinations so, depending on where this code is, this may lead to latches in your code.
• diegogmx: so, i should use when (others => 0) to avoid that? not really sure it will be a problem though it's an spi interface that works within a board ill code too
• diegogmx: pd: obviously (others => 0) wont do since thats just covering the case when spi_op = 0

Answer 1

I'll turn my comment into an answer (Last night it was very, very late).

The case statement itself will do the comparison. Thus another comparison is too much. The correct syntax is:

if spi_ctr = 9 then
 case spi_op is
 when op_get_r_count => spi_reg_out <= ref_count_i;
 when op_get_s_count => spi_reg_out <= sig_count_i;
 when op_get_g_limit => spi_reg_put <= gate_limit_i;
 end case;

If this code is inside a rising_edge(clk) section it is fine as it is.
However if it is outside a clocked section you can get latches. (Not only from the case but also from the if).
In that case you should add a when others => spi_reg_put <= ... but also an else ... section.

Last remark: it is useful to be able to distinguish constants from variables. There are several methods.

Using something like 'const', 'cnst' or 'c_' in the name:
constant const_op_get_r_count: std_logic_vector(7 downto 0) := "00000001";

Or follow the Verilog and C convention to use capitals:
constant OP_GET_R_COUNT: std_logic_vector(7 downto 0) := "00000001";

A colleague of mine prefers both:
constant C_OP_GET_R_COUNT: std_logic_vector(7 downto 0) := "00000001";