7
\$\begingroup\$

I have the following Verilog code which sends 8 bytes to the serial port successively after a button is pressed.

The problem is, the bytes are sent out of order as to what I would expect.

For example, if I send out the bytes 0xDE, 0xAD, 0xBE, 0xEF, 0xDE, 0xAD, 0xBE, 0xEF - the PC gets 0xEF, 0xAD, 0xEF, 0xAD and sometimes does not receive the rest and will hang-up.

I've looked at this code many times, and can't seem to figure out why this might be. Am I using the part-select incorrectly? I don't think so, but I don't know what else could be the issue.

I've attached a second version of this code that does work(meaning all the bytes are received and in the correct order), but it takes an extra clock cycle because it updates the data in the shift register after each transmission. If you can see any reason why the first version posted does not work, but the second does - please let me know!

module transmission_test_2(sysclk, rxd, txd, LED, button);

input sysclk, rxd, button;
output txd;
output reg LED;

wire receiving_complete, isReceiving, isTransmitting, isError;
reg begin_transmit;
reg [7:0] tbyte;

wire [7:0] rbyte;

reg [2:0] state;

reg [63:0] plain_text;

integer byteN;


parameter IDLE = 0, BEGIN_TRANSMISSION = 1, UPDATE_DATA = 2, SEND_BYTES = 3;


uart uart1(
.clk(sysclk),
.rx(rxd),
.tx(txd),
.transmit(begin_transmit),
.tx_byte(tbyte),
.received(receiving_complete),
.rx_byte(rbyte),
.is_receiving(isReceiving), 
.is_transmitting(isTransmitting),
.recv_error(isError)
);


always @(posedge sysclk)
begin
    begin_transmit = 1'b0;
    case(state)
    IDLE: begin
        if(button==1'b0) begin
            LED = 1'b1;
            plain_text = 64'hDEADBEEFDEADBEEF;
            state = BEGIN_TRANSMISSION;
        end else begin
            LED <= 1'b0;
        end
    end
    BEGIN_TRANSMISSION: begin
        tbyte = plain_text[7:0];
        begin_transmit = 1'b1;
        byteN = 1;
        state = SEND_BYTES;
    end
    SEND_BYTES: begin
        if(!isTransmitting) begin
            tbyte = plain_text[byteN*8 +: 8];
            begin_transmit = 1'b1;
            byteN = byteN + 1;
            if(byteN == 8) begin
                state = IDLE;
            end
        end
    end
    endcase
end

endmodule

Second "working" version:

module transmission_test(sysclk, rxd, txd, LED, button);


input sysclk, rxd, button;

output txd;

output reg LED;


wire receiving_complete, isReceiving, isTransmitting, isError;

reg begin_transmit;

reg [7:0] tbyte;


wire [7:0] rbyte;


reg [2:0] state;

reg [63:0] plain_text;

integer bytes_remaining;


parameter IDLE = 0, BEGIN_TRANSMISSION = 1, UPDATE_DATA = 2, SEND_BYTES = 3, DONE = 4;


uart uart1(
.clk(sysclk),
.rx(rxd),
.tx(txd),
.transmit(begin_transmit),
.tx_byte(tbyte),
.received(receiving_complete),
.rx_byte(rbyte),
.is_receiving(isReceiving), 
.is_transmitting(isTransmitting),
.recv_error(isError)
);


always @(posedge sysclk)
begin
    begin_transmit = 1'b0;
    case(state)
    IDLE: begin
        if(button==1'b0) begin
            LED = 1'b1;
            plain_text = 64'hDEADBEEFDEADBEEF;
            state = BEGIN_TRANSMISSION;
        end else begin
            LED <= 1'b0;
        end
    end
    BEGIN_TRANSMISSION: begin
        tbyte = plain_text[7:0];
        begin_transmit = 1'b1;
        bytes_remaining = 7;
        state = UPDATE_DATA;
    end
    UPDATE_DATA: begin
        plain_text = plain_text >> 8;
        state = SEND_BYTES;
    end
    SEND_BYTES: begin
        if(!isTransmitting) begin
            tbyte = plain_text[7:0];
            begin_transmit = 1'b1;
            bytes_remaining = bytes_remaining - 1;
            if(bytes_remaining == 0) begin
                state = IDLE;
            end else begin
                state = UPDATE_DATA;
            end
        end
    end
    endcase
end

endmodule

REVISION:

Following advice from @dwikle - instead of using one "catch-all" state that cycles through sending all 8 bytes - I created a seperate state for each byte that needed to be sent.

Here is the code:

module transmission_test_3(sysclk, rxd, txd, LED, button);

input sysclk, rxd, button;
output txd;
output reg LED;

wire receiving_complete, isReceiving, isTransmitting, isError, reset;
reg begin_transmit;
reg [7:0] tbyte;

wire [7:0] rbyte;

reg [2:0] state = 4'b0;
reg [63:0] plain_text = 64'h0;

uart uart1(
.clk(sysclk),
.rst(reset),
.rx(rxd),
.tx(txd),
.transmit(begin_transmit),
.tx_byte(tbyte),
.received(receiving_complete),
.rx_byte(rbyte),
.is_receiving(isReceiving), 
.is_transmitting(isTransmitting),
.recv_error(isError)
);

always @(posedge sysclk)
begin
    begin_transmit = 1'b0;
    case(state)
    4'b0000: begin
        if(button==1'b0) begin
            LED = 1'b1;
            plain_text = 64'hDEADBEEFAAABACAD;
            state = 4'b0001;
        end else begin
            LED = 1'b0;
        end
    end
    4'b0001: begin
        tbyte = plain_text[7:0];
        begin_transmit = 1'b1;
        state = 4'b0010;
    end
    4'b0010: begin
        if(!isTransmitting) begin
            tbyte = plain_text[15:8];
            begin_transmit = 1'b1;
            state = 4'b0011;
        end
    end
    4'b0011: begin
            if(!isTransmitting) begin
                tbyte = plain_text[23:16];
                begin_transmit = 1'b1;
                state = 4'b0100;
            end
        end
    4'b0100: begin
        if(!isTransmitting) begin
            tbyte = plain_text[31:24];
            begin_transmit = 1'b1;
            state = 4'b0101;
        end
    end
    4'b0101: begin
        if(!isTransmitting) begin
            tbyte = plain_text[39:32];
            begin_transmit = 1'b1;
            state = 4'b0110;
        end
    end
    4'b0110: begin
        if(!isTransmitting) begin
            tbyte = plain_text[47:40];
            begin_transmit = 1'b1;
            state = 4'b0111;
        end
    end
    4'b0111: begin
        if(!isTransmitting) begin
            tbyte = plain_text[55:48];
            begin_transmit = 1'b1;
            state = 4'b1000;
        end
    end
    4'b1000: begin
        if(!isTransmitting) begin
            tbyte = plain_text[63:56];
            begin_transmit = 1'b1;
            state = 4'b0000;
        end
    end
    endcase
end
endmodule

However, the results are still the same - I receive every other byte. What gives?

Any ideas?

P.S. - I've posted the UART code on my google docs - if you need to take a look. :)

UART module

UPDATE:

The original question for this thread came from attempting to isolate a communication issue in a larger module.

The project simply accepts 64-bit (8 bytes) of data from the PC - encrypts the data with the DES algorithm - and sends the encrypted message (64-bit, again) back.

The transfer works, but (what seems to be arbitrarily) hangs up. When attempting to do 1000 encryptions, I can on average handle around 250 - sometimes doing all 1000 successfully, and sometimes maybe only doing 20 or 50.

I had thought that the transfer was bottle-necking on the transmission side of the communication - which is was I was rewriting the transmission and hence the purpose of this thread. I've come to realize, however, that the problem actually lies in the reception side of the transfer. What seems to be happening is that after doing several successful runs, the state machine collects 7 bytes again and then somehow misses or the last byte of the 64-bit chunk and gets stuck in the state looking for the last byte.

I've tried simulating the design - but, I feel like the stimulus I'm providing isn't quite kosher. The testbench should send 8 bytes worth of data over the uart - and then the state machine should chug along, encrypting and sending back the message.

However, providing different vectors of data to the rx line of the uart gives drastically different results. Depending on the data I'm using, the uart sometimes recieves only 7 bytes (which is the problem I'm looking at with real hardware) or even returns errors.

So, the problems I'm trying to figure out are:

1) I can successfully receive and transmit several tens to hundreds of encryptions - but the communication hangs up at arbitrary points - and it seems to be that when this happens, the state machine has collected 7 bytes and is looking for the last.

2) To diagnose this problem, I tried looking into the simulation results. However - even these seem to give unexpected behavior - and I'm afraid perhaps I'm giving stimuli incorrectly.

Any comments or suggestions for the implementation of the testbench - or what may be causing the communication to cease would be greatly appreciated. If these questions seem elementary I apologize - I'm still learning.

I've attached a zip on my google docs with all the relevant files, including the testbench.

https://docs.google.com/open?id=0B4WyEjzmIhtNN0V6a0x5U19SMUU

I'll also post the top-level module here for reference.

module rs232_neek(sysclk, rxd, txd, reset, LED);

input sysclk, rxd, reset;
output txd;
wire receiving_complete, isTransmitting, isReceiving, isError;

output reg [3:0] LED;   //The LEDs are used simply as debug - to determine which state the machine gets held-up in.

reg begin_transmit;
reg [7:0] tbyte;

wire [7:0] rbyte;

parameter FIRST_BYTE = 0, GET_BYTES = 1, BEGIN_ENC = 2, CHECK_ENC_STATUS = 3,     BEGIN_TRANSMISSION = 4, SEND_BYTES = 5;

reg [2:0] state = 3'b0;
integer byteN = 0;

reg [3:0] sel = 4'b0;
reg [63:0] plain_text;
reg [63:0] cipher_text;
wire [63:0] cipher_net;

uart uart1(
.clk(sysclk),
 .rst(~reset),
.rx(rxd),
.tx(txd),
.transmit(begin_transmit),
.tx_byte(tbyte),
.received(receiving_complete),
.rx_byte(rbyte),
.is_transmitting(isTransmitting),
.is_receiving(isReceiving),
.recv_error(isError)
);

des des1(
 .clk(sysclk),
 .key(56'h0),
 .roundSel(sel),
 .decrypt(1'b0),
 .desIn(plain_text),
 .desOut(cipher_net)
 );

always @(posedge sysclk)
begin

    if(~reset) begin
        state = FIRST_BYTE;
    end

    LED = 4'b1111;

    case(state)
        FIRST_BYTE: begin
            LED[0] = 1'b0;
            begin_transmit = 1'b0;
            if(receiving_complete) begin
               plain_text[7:0] = rbyte;
                byteN = 1;
                state = GET_BYTES;
            end
        end
        GET_BYTES: begin
            LED[1] = 1'b0;
            if(receiving_complete) begin
                plain_text[byteN*8 +: 8] = rbyte;
                byteN = byteN + 1;
                if(byteN == 8) begin
                    state = BEGIN_ENC;
                end
            end
        end
        BEGIN_ENC: begin
            sel = 4'b0;
            state = CHECK_ENC_STATUS;
        end
        CHECK_ENC_STATUS: begin
            LED[2] = 1'b0;
            sel = sel + 1;
            if(sel == 15) begin
                state = BEGIN_TRANSMISSION;
            end
        end
        BEGIN_TRANSMISSION: begin
            cipher_text = cipher_net;
            tbyte = cipher_text[7:0];
            begin_transmit = 1'b1;
            byteN = 1;
            state = SEND_BYTES;
        end
        SEND_BYTES: begin
            LED[3] = 1'b0;
            if(!isTransmitting && !begin_transmit) begin
                tbyte = cipher_text[byteN*8 +: 8];
                begin_transmit = 1'b1;
                byteN = byteN + 1;
                if(byteN == 8) begin
                    state = FIRST_BYTE;
                end
            end else begin
                begin_transmit = 1'b0;
            end
        end
    endcase
end

endmodule
\$\endgroup\$
  • 1
    \$\begingroup\$ To see what happens exactly I think it would be a good idea to use 8 different codes. If you now receive 0xBE you don't know if it's the first or the second. \$\endgroup\$ – stevenvh Aug 30 '12 at 18:08
  • \$\begingroup\$ Frankly, I'm not sure how either one of them is working. Is "begin_transmit" supposed to pulse for each byte sent? I see where you're setting it, but not where it gets cleared. \$\endgroup\$ – Dave Tweed Aug 30 '12 at 18:21
  • \$\begingroup\$ Yes, it's suppose to pulse for each byte. I'm attempting to clear the flag at the beginning of the always block. \$\endgroup\$ – kbarber Aug 31 '12 at 23:40
  • \$\begingroup\$ I've also tried using different vectors as input, just to be sure. Same problem. \$\endgroup\$ – kbarber Sep 1 '12 at 0:34
4
\$\begingroup\$

The problem is that your "transmission_test" code is written such that it assumes that "is_transmitting" from the UART goes true before the clock edge that follows the assertion of "begin_transmit". It doesn't -- it takes a clock cycle before the UART transmitter moves out of its "idle" state, after which "is_transmitting" is true.

As a result, your "transmission_test" state machine is advancing two states for each byte transmitted, and you see only every other byte at the output of the UART.

There are many minor problems with your code samples, but the main thing you need to fix is to verify that "is_transmitting" has gone true before you advance to the next byte in the message.

This would have been pretty obvious if you had taken the time to simulate this project. I can't overemphasize the importance of simulation in verifying FPGA code. Take the time to get familiar with your simulator and learn how to write good module testbenches.

\$\endgroup\$
  • \$\begingroup\$ Dave - thanks a lot for the response, and taking the time to look into the problem. Obviously, I should have analyzed simulation results more closely. I'm still learning most of the nuances of Verilog and had assumed that I was over-looking something in the code itself. You mentioned that there are several other issues in the code - I would love if you could shed some light on that. Like I said, I am still learning to write good HDL and appreciate any comments or criticism. Thank you! \$\endgroup\$ – kbarber Sep 2 '12 at 6:02
  • \$\begingroup\$ @KristinBarber: Well, just looking at 'transmission_test_3.v', there's no assignment to the "reset" signal. I'm not sure what your intention was with the "button" signal, but the way it's set up, the circuit will transmit continuously as long as it's active (low), rather than sending one message per button press. And I suppose it's more of a style issue than a bug, but I really don't like the "default assignment" to "begin transmit" outside the case statement. I missed this the first time around, and this sort of thing makes your code hard to read in general. \$\endgroup\$ – Dave Tweed Sep 2 '12 at 13:36
  • \$\begingroup\$ @KristinBarber: The uart.v code itself has some more serious problems, although it looks like you got this from someone else. The "if (rst) begin ..." block is completely ineffective, since it is overridden by the rest of the code inside the always block -- all of that other code should be inside an "else" that's associated with the "if (rst)". And there's a lot of internal state that isn't set to known values by the reset anyway. It might not really matter in the end, but if so, it should be clearly documented as such. \$\endgroup\$ – Dave Tweed Sep 2 '12 at 13:45
  • \$\begingroup\$ Dave - Thanks for all of the input. I could really use some help diagnosing a larger problem at hand - that I've outlined in an update to the original question posting. If you could take a look at that it would help me tremendously. \$\endgroup\$ – kbarber Sep 3 '12 at 6:45
  • \$\begingroup\$ @kbarber: The problem with your testbench is that it only sends one start bit at the beginning of the message, rather than one start bit per byte. This is causing data-dependent behavior because the UART is interpreting the first zero in the data as a start bit rather than a data bit. I'm still looking into possible problems with the actual hardware. \$\endgroup\$ – Dave Tweed Sep 3 '12 at 20:56
0
\$\begingroup\$

Without knowing the requirements of the UART, my guess is that the problem lies with begin_transmit. In your "working" case begin_transmit will get cleared for each byte since you are moving through state UPDATE_DATA. However, for the non-working case, you will stay in state SEND_BYTES for the entire transmission, and begin_transmit will only be cleared if isTransmitting == 1. (Note that begin_transmit is assigned to 0 before the case statement, creating a default assignment.)

I simulated the non-working case and begin_transmit is only pulsing every other byte, which would explain what you are seeing if the UART expects to see a posedge on begin_transmit. Without the UART model, I modeled isTransmitting like this for simulation.

always @(posedge sysclk) begin
  isTransmitting <= begin_transmit;
end
\$\endgroup\$
  • \$\begingroup\$ Oh, that's enlightening - actually. I was under the impression that at every time the always block was entered, any statements would be evaluated and THEN the case would be evaluated. However, you're saying that in a situation such as this - anything before a case statement in an always block gets treated as a default to the case? \$\endgroup\$ – kbarber Aug 31 '12 at 23:36
  • \$\begingroup\$ I've updated my original post to accommodate for these suggestions. I'm still having issues, though. Please take a look and let me know if you have any further suggestions - if possible. \$\endgroup\$ – kbarber Sep 1 '12 at 2:57
  • \$\begingroup\$ Regarding the "default", it seems to me that he said that if none of the cases are executed, or if an if-test within the selected case causes the rest of the code to be skipped, then the variable will keep the value it had before--so, it retains the one it got at the top of the block--and hence, that is essentially a default value. Default, as in, "this will be the value unless something else changes it". \$\endgroup\$ – gbarry Sep 2 '12 at 4:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.