3

I'm trying to remove bitbashing in my design and sending test signals from outside the DUT using a procedure. The format of the serialized message is a start bit of '0', the byte with MSB first, and a stop bit of '1'. The line idles at '1'. I think I'm having issue making use of datatypes to be passed between the procedure and the main process.

Here is my procedure (baudrate is a time constant for the period of the serial clock):

  procedure send_midi_byte (
    signal byte_in  : in  std_logic_vector;
    signal midi_out : out std_logic) is
  begin
    midi_out <= '0';
    wait for baudrate;
    for i in 7 to 0 loop
      midi_out <= byte_in(i);
      wait for baudrate;
    end loop;
    midi_out <= '1';
    wait for baudrate;
  end send_midi_byte;

And here is how I call it to send a few bytes (byte_slv is an 8 element std_logic_vector):

    byte_slv <= x"90";
    send_midi_byte(byte_slv, midi_in_int);

I have tried a few different methods and this is the only one that doesn't give an error, but of course it won't work because of the nonblocking assignments in the procedure meaning my serial signal will just be '1' for the length of time specified in baudrate.

How can I write this procedure correctly?

comc cmoc
  • 47
  • 1
  • 6

3 Answers3

1

As well as Jim's suggestion, fix the loop range and try again before you try anything else. 7 to 0 is a null range; either 0 to 7 or 7 downto 0 are not.

Nonblocking (with regard to assignments) is a meaningless term in VHDL but signal assignment within a loop within a procedure is absolutely fine. It's a postponed assignment which is scheduled to happen when wait for baudrate starts. That, of course, waits for a specified time and the next loop iteration starts after it has completed.

  • Although the usage in the question wasn't right, I don't think it's true to say that nonblocking is a meaningless term in VHDL. You can certainly use VHDL to implement something for which it is important. For example, [VUnit](https://vunit.github.io/) has to document quite clearly which functions are blocking and which are non-blocking. – Harry Feb 29 '20 at 11:23
  • @Harry Edited to clarify. But surely functions can't be blocking? (though procedures of course can). –  Feb 29 '20 at 12:07
0

Issues You have two errors. The first, pointed out by Brian, your range is wrong. An easy way to express the range is to use 'range. This is shown in the code below.

In addition, you want to change the class of your byte_in parameter to constant, also shown in the code below.

Solution

  procedure send_midi_byte (
    constant byte_in  : in  std_logic_vector;
    signal   midi_out : out std_logic
  ) is
  begin
    midi_out <= '0';
    wait for baudrate;
    for i in byte_in'range loop
      midi_out <= byte_in(i);
      wait for baudrate;
    end loop;
    midi_out <= '1';
    wait for baudrate;
  end send_midi_byte;

So why constant? Constant means that the parameter is static for the lifetime of the subprogram call. Constant parameters allow anything that produces a value (literal, expression, or even signal or variable name), such as:

send_midi_byte(x"90", midi_in_int);
for i in 0 to 10 loop
  send_midi_byte(x"90" + i, midi_in_int);
end loop;

You are already familiar with this as all of the language operators (implemented as functions) have constant classed parameters (it is the default and in the standard packages it is common to leave it off).

Guidelines for Parameters
For inputs:
Use a signal when an update is expected on the object - such as using 'event, or using the object in a wait statement. Generally this means the object is coming from the architecture (such as the DUT).
Use a constant when you need to pass a literal value.

For outputs:
Use a signal when passing a value to the architecture (such as the DUT).
Use a variable when passing a value back to the local process.

Why should the parameter be a constant
In your example, it will work with a signal, however, the use model will always be, 1) assign to the signal, and then 2) call the subprogram:

byte_slv <= x"90";
send_midi_byte(byte_slv, midi_in_int);

This may or may not annoy you enough to be a problem. For other procedures, the update of the signal parameter may indeed end up being a delta cycle too late. This too can be worked around in the subprogram easily enough.

OTOH, as soon as you want to call your send_midi_byte procedure from another procedure, it gets more difficult or it does not work at all. In the procedure below, we try testing by sending incrementing bytes to the midi:

procedure send_inc_midi_bytes (
  constant first_byte_in : in std_logic_vector;
  constant number_bytes  : in integer ;
  signal midi_out  : std_logic
) is 
   -- signal byte_in : std_logic_vector(7 downto 0); -- want to do this
begin
  for i in 0 to number_bytes - 1 loop
    byte_in <= byte_in + i ; 
    send_midi_byte(byte_in, midi_out);
  end loop ;
end procedure send_inc_midi_bytes ; 

I realize that fixing the range fixes your exact problem, but some day, you may wish to do more - at that point, you will be wishing that someone suggested that you used a constant instead of a signal.

Jim Lewis
  • 3,601
  • 10
  • 20
  • Thanks for the information on passing parameters, but now I believe I am back to the issue of concurrent statements at least for my for loop. The simulation now sets midi_in_int to 0 for baudrate time, back to 1 for baudrate time, and then resets. It's as if the for loop isn't there. Anything to do with that structure? – comc cmoc Feb 28 '20 at 20:46
  • 2
    Note the direction of the OP's loop parameter specification provides a _null range_. –  Feb 28 '20 at 22:07
  • _Constant means that the parameter is static for the lifetime of the subprogram._ For the lifetime of the subprogram call. Parameter values are dynamically elaborated. Also note the default class for a parameter of mode `in` is constant. Your answer is barking up the wrong tree, the original post not providing a [mcve]. Note the sequential process call won't complete until the loop exits. –  Feb 29 '20 at 00:40
  • @user1155120 Thanks for the corrections. It should be noted that subprograms are only elaborated when they are called, so for a VHDL person, lifetime could only refer to the subprogram call, however, given Verilog/SysteVerilog, I followed your suggestion to change lifetime of subprogram to subprogram call. It should also be noted that a loop with a NULL range is not an error - it simply does not iterate so the loop will exit immediately or perhaps never enter. – Jim Lewis Mar 02 '20 at 14:40
0

I have tried a few different methods and this is the only one that doesn't give an error, but of course it won't work because of the nonblocking assignments in the procedure meaning my serial signal will just be '1' for the length of time specified in baudrate.

The premise of your assertion here is incorrect. The reason why the procedure call fails is the loop parameter specification provides a null range (the loop parameter is evaluated first, a loop statement executes 0 or more times).

How can I write this procedure correctly?

A Minimal, Complete, and Verifiable example with the fix included in procedure send_midi_byte:

library ieee;
use ieee.std_logic_1164.all;

entity cmoc_cmoc is
end entity;

architecture mcve of cmoc_cmoc is
    constant baudrate:  time := 26.925 us;  -- 38.400 baud
    procedure send_midi_byte (
        signal byte_in:   in  std_logic_vector;
        signal midi_out:  out std_logic
    ) is
        alias in_byte: std_logic_vector (7 downto 0) is byte_in;  -- ADDED
    begin
        midi_out <= '0';
        wait for baudrate;
        for i in in_byte'range loop        -- WAS 7 to 0
            midi_out <= in_byte(i); -- WAS byte_in(i);
            wait for baudrate;
        end loop;
        midi_out <= '1';
        wait for baudrate;
    end procedure send_midi_byte;
    signal some_byte:   std_logic_vector (7 downto 0) := x"41";
    signal midi_out:    std_logic := '1';
    type baud is (IDLE,START, BD0, BD1, BD2, BD3, BD4, BD5, BD6, BD7, STOP);
    signal baud_cnt:    baud;
begin
PROCEDURE_CALL:
    process 
    begin
        wait for baudrate;  -- SHOW IDLE on midi_out;
        send_midi_byte(some_byte, midi_out);  -- added second parameter
        wait;
    end process;
BAUD_CTR:
    process
    begin
        if baud_cnt = IDLE then
            wait until midi_out = '0';
        end if;
        loop
            baud_cnt <= baud'RIGHTOF(baud_cnt);
            wait for 0 ns;
            report "baud(" & baud'image(baud_cnt) &
                   ") midi_out = "  & std_ulogic'image(midi_out);
            wait for baudrate;
            if baud_cnt = STOP then
                baud_cnt <= IDLE;
                exit;
            end if;
        end loop;
        wait;
    end process;
end architecture;

Note the class of byte_in isn't provided in the procedure declaration. For a subprogram parameter of mode in the default class is constant.

The MSB first order has been preserved to match your question text. (UARTs transmit the LSB first).

The BAUD_CTR process is an embellishment to demonstrate transmitted baud order.

For a UART with an input byte given as MSB on the left and transmitting the LSB first either the alias range can be reversed or the loop parameter could use `REVERSE_RANGE.

The report statements:

ghdl -r cmoc_cmoc --wave=cmoc_cmoc.ghw
cmoc_cmoc.vhdl:45:13:@26925ns:(report note): baud(start) midi_out = '0'
cmoc_cmoc.vhdl:45:13:@53850ns:(report note): baud(bd0) midi_out = '0'
cmoc_cmoc.vhdl:45:13:@80775ns:(report note): baud(bd1) midi_out = '1'
cmoc_cmoc.vhdl:45:13:@107700ns:(report note): baud(bd2) midi_out = '0'
cmoc_cmoc.vhdl:45:13:@134625ns:(report note): baud(bd3) midi_out = '0'
cmoc_cmoc.vhdl:45:13:@161550ns:(report note): baud(bd4) midi_out = '0'
cmoc_cmoc.vhdl:45:13:@188475ns:(report note): baud(bd5) midi_out = '0'
cmoc_cmoc.vhdl:45:13:@215400ns:(report note): baud(bd6) midi_out = '0'
cmoc_cmoc.vhdl:45:13:@242325ns:(report note): baud(bd7) midi_out = '1'
cmoc_cmoc.vhdl:45:13:@269250ns:(report note): baud(stop) midi_out = '1'

And a waveform showing baud_cnt:

MSB first order

  • This is excellent, and solved the issue. Good to know that "X to Y" gives a null range. – comc cmoc Mar 01 '20 at 05:48
  • X to Y gives a null range only when X is larger. – Jim Lewis Mar 02 '20 at 14:43
  • Solution is problematic as it always requires two steps. Step 1: assign to signal. Step 2: Call subprogram – Jim Lewis Mar 02 '20 at 15:51
  • @JimLewis - You're decrying a lack of a [mcve] (here) and could have requested more information (from the question author) instead of creating a dilemma out of whole cloth. The only requirement here for a multi-byte message is that the assignment to the signal byte_in be valid when the procedure is called.Here the stop bit (the final `wait for baudrate;` in the procedure) could be preceded by a _signal_ assignment requesting new byte_in (not used during the stop bit) value. The signal could even be an index into a message. All that depends on the question. –  Mar 02 '20 at 19:10
  • @user1155120 My claim is that using a signal class parameter for byte_in is a bad coding style. If the byte_in signal is driven in the same process as the call, then it is just extra busy work (as I showed) - while it is not wrong - it seriously limits any further abstraction (as I showed). If the byte_in signal is driven from a separate process, how are you going to keep from corrupting midi_out? Simple you are not going to do asynchronous message handling with signals - instead I generally use shared variables and FIFOs (but this is way out of scope for the OPs question) – Jim Lewis Mar 03 '20 at 20:02
  • Style is not part of the VHDL language, it's superimposed organizationally. Neither the OP's author's profile nor the question's snippet give any indication style concerns may be warranted. "There is no 'additional' in a question". You should have asked for a [Minimal, Complete, and Verifiable example](https://stackoverflow.com/help/minimal-reproducible-example). Instead of simply addressing the visible problem you've pursued a tangent. Note the PROCEDURE_CALL process is asynchronous to BAUD_CTR, they manage to cooperate. –  Mar 03 '20 at 21:36
  • Certainly there are good coding practices and bad coding practices. I certainly respect your knowledge of the LRM. However here you are promoting a bad coding practice - sure I went beyond the question to demonstrate why that is and I don't think there is anything wrong with that. – Jim Lewis Mar 04 '20 at 02:01
  • And yes, distracted by the OPs question, I missed the range issue and based on feedback in the comments corrected it. – Jim Lewis Mar 04 '20 at 02:06
  • @JimLewis Again, ask the question author for a [mcve]. My example has no drivers for byte_slv and can't exhibit the issue you keep on about. I side stepped the issue by providing my own [mcve] while giving him the benefit of the doubt. It's not clear how I can be accused of 'promoting bad coding practice'. The wavy-handed pontification does address the specific problem nor a defect in my answer. –  Mar 04 '20 at 07:27
  • @user1155120 Your description states "Note the class of byte_in isn't provided in the procedure declaration. For a subprogram parameter of mode in the default class is constant" It is not clear why you are saying this since you declared the interface object as a signal. – Jim Lewis Mar 11 '20 at 00:27