Why is twice as much subtracted?

[erikwikt]

As the title says, can someone please explain to me why 2 is subtracted from dmaSectorCount on line 83?

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.ALL;

entity Command_DMA_read is
  
  port (
    clk : in std_logic;
    reset : in std_logic;

    din : in std_logic_vector(31 downto 0);
    dout : out std_logic_vector(31 downto 0);

    sector : in std_logic_vector(15 downto 0);
    
    data_avl : in std_logic;
    enw : out std_logic;
    enr : out std_logic
    );

end Command_DMA_read;

architecture behaviour of Command_DMA_read is

  constant iterationLimit : unsigned(6 downto 0) := "1111111";

  type states is (S0, S1, S2, S4, S5);

  signal state, next_state : states;
  
begin  -- behaviour

  STATE_TRANS: process (clk, reset)
  begin  -- process STATE_TRANS
    if reset = '1' then
      state <= S0;
    elsif rising_edge(clk) then
      state <= next_state;
    end if;
  end process STATE_TRANS;

  NEXTSTATEGEN: process(state, data_avl)
    variable dmaIterator : unsigned((iterationLimit'length) downto 0);
    variable dmaSectorCount : unsigned((sector'length - 1) downto 0);
    begin
    case state is
      when S0  =>
        dout <= (others => '0');
        enw <= '0';
        enr <= '0';
	dmaIterator := (others => '0');
        next_state <= S5;
      when S1 =>
	if dmaSectorCount = X"0000" then
	  next_state <= S2;
	elsif data_avl = '1' then
	  if dmaIterator = iterationLimit then
	    enw <= '1';
	    enr <= '1';
	    dmaIterator := dmaIterator;
	    next_state <= S4;
	  else
	    enw <= '1';
	    enr <= '1';
	    dmaIterator := dmaIterator + 1;
	    next_state <= S1;
	  end if;
	else
	  enw <= '0';
          enr <= '1';
	  dmaIterator := dmaIterator;
	  next_state <= S1;
	end if;
      when S2 =>
	enw <= '0';
	enr <= '0';
        next_state <= S2;

      when S4 =>
	enw <= '0';
	enr <= '0';
	dmaIterator := (others => '0');
	dmaSectorCount := dmaSectorCount - 1;
	next_state <= S1;

      when S5 =>
	dmaSectorCount := unsigned(sector);
	next_state <= S1;
      when others =>
        next_state <= S0;
    end case;
  end process NEXTSTATEGEN;
  
end behaviour;

 

[barry]

I think we have a winner in the "MOST OBSCURE QUESTION" contest.

1) There are no line numbers.
2) I don't see ANYWHERE that 2 is being subtracted .
3) Without knowing WHAT you are trying to do, there is no way we can answer this question.

 

[TrickyDicky]

Well I cant answer your specific question, but I can answer the question that will come later : "Why did latches get infered"

Because you didnt assign every signal in every branch in the asynchronous process.

Next question: "Why doesnt it behave on hardware like in my simulation?"

Because there are signals missing from the sensitivity list.

 

[erikwikt]

I have to say that I like TrickyDicky way of dealing with us not so professional with VHDL better then barry..... Points for you TrickyDicky, you deserve them.
I took your advise TrickyDicky and added dmaSectorCount := dmaSectorCount in all the other branches and it worked.

Thank you for your answer.

 

[FvM]

I took your advise TrickyDicky and added dmaSectorCount := dmaSectorCount in all the other branches and it worked.

Fine, but I fear it doesn't really solve the inherent problem of the design and you can't be sure not to get unexpected behaviour later again.

Counting assignments like

dmaIterator := dmaIterator + 1;
[I]or[/I] 
dmaIterator := dmaIterator - 1;

will only work reliable under two conditions
1. they are executed under a clock edge sensitive event, so DFFs are inferred
2. dmaIterator is a temporary variable that is loaded before the counting assignment and not used to hold the counter value itself

 

[erikwikt]

Thank you FvM for your answer.
I took your advise and placed the dmaIterator and dmaSectorCount in individual processes which I enable and reset with some signals.
I attached the code and please comment if you have some spare time. Will this design be synchronised with the clock or is there any pitfalls that I cant see?

  signal DMASectorCount     : unsigned((COM_sata_reg_SectorCountReg_in'length - 1) downto 0);
  signal DMASectorCount_en  : std_logic;
  signal DMASectorCount_rst : std_logic;

  signal DMAIterator     : unsigned((DMAIterationLimit'length - 1) downto 0);
  signal DMAIterator_en  : std_logic;   
  signal DMAIterator_rst : std_logic;

  DMASectorCount_process : process(clk,reset, DMASectorCount_en, DMASectorCount_rst)
    variable DMASectorCount_i : unsigned((COM_sata_reg_SectorCountReg_in'length - 1) downto 0);
  begin
    if reset = '1' or DMASectorCount_rst = '1' then
      DMASectorCount_i := unsigned(COM_sata_reg_SectorCountReg_in);
    elsif rising_edge(clk) then
      if DMASectorCount_en = '1' then
	DMASectorCount_i := DMASectorCount_i - 1;
      else
	DMASectorCount <= DMASectorCount_i;
      end if;
    end if;
  end process;

  DMAIterator_process : process(clk,reset, DMAIterator_en, DMAIterator_rst)
    variable DMAIterator_i : unsigned((DMAIterationLimit'length - 1) downto 0);
  begin
    if reset = '1' or DMAIterator_rst = '1' then
      DMAIterator_i := (others => '0');
    elsif rising_edge(clk) then
      if DMAIterator_en = '1' then
	DMAIterator_i := DMAIterator_i + 1;
      else
	DMAIterator <= DMAIterator_i;
      end if;
    end if;
  end process;

I also added DMAIterator and DMASectorCount to the sensitivity list in the other asynchronous process.

 

[FvM]

First remark, in the synchronous process sensitivity lists, you'll only include the clock and the asynchronous inputs, e.g. not DMASectorCount_en. Having additional signals doesn't hurt, however.

The count operation should basically work correctly, but you should carefully check the timing of asynchronous inputs like DMASectorCount_rst. You don't show how they are generated, but there's a certain risk that they violate setup- and hold specifications. I would prefer synchronous acting reset signals, if ever possible. This may involve an additional delay of one cycle, so the timing should be cheked in any case.