can someone explain this please

[Paul]

Hi

I've been coding a rs-232 rx/tx module. after 3 days, I've finally
track down a weird bug. May be someone here can help explain this.

input RXD; reg RXD_r;
output [7:0] din; reg [7:0] din_r;
reg [7:0] step;

always @(posedge clk) begin
RXD_r<=~RXD;
.....
.....
din_r<=din_r+(1<<(step-1))*(~RXD_r); // bugged version
din_r[step-1]<=(~RXD_r); // finally the correct
version

In the bugged version, "din_r" always return "2" bigger than what was
actually sent. (note: for both versions, din_r was properly at "0"
before each byte was sampled. so it's not that din_r was not properly
zeroed out. the two versions have only the one line difference.)

Although my module is working, I still can't explain why...

Thanks

 

[Shalom Bresticker]

You have not explained what 'step' is.

Shalom


Paul wrote:

Hi

I've been coding a rs-232 rx/tx module. after 3 days, I've finally
track down a weird bug. May be someone here can help explain this.

input RXD; reg RXD_r;
output [7:0] din; reg [7:0] din_r;
reg [7:0] step;

always @(posedge clk) begin
RXD_r<=~RXD;
....
....
din_r<=din_r+(1<<(step-1))*(~RXD_r); // bugged version
din_r[step-1]<=(~RXD_r); // finally the correct
version

In the bugged version, "din_r" always return "2" bigger than what was
actually sent. (note: for both versions, din_r was properly at "0"
before each byte was sampled. so it's not that din_r was not properly
zeroed out. the two versions have only the one line difference.)

Although my module is working, I still can't explain why...

Thanks

--
Shalom Bresticker Shalom.Bresticker@motorola.com
Design & Reuse Methodology Tel: +972 9 9522268
Motorola Semiconductor Israel, Ltd. Fax: +972 9 9522890
POB 2208, Herzlia 46120, ISRAEL Cell: +972 50 441478

 

[Paul]

Shalom Bresticker <Shalom.Bresticker@motorola.com> wrote in message news:<3FF9437C.A7DAB99C@motorola.com>...

You have not explained what 'step' is.

Here is the entire program

`define LENGTH (50000000/38400)
module top2(clk,RXD,TXD,led0,led1);
input clk;
input RXD;
output TXD,led0,led1;

reg [1:0] s1;

reg [7:0] char_w;
wire [7:0] char_r;
reg go_r,go_w;
send qq1 (clk,char_w,go_w,ok_w,TXD);
recv qq2 (clk,char_r,go_r,ok_r,RXD);

assign led0=~char_w[6];
assign led1=~char_w[7];

always @(posedge clk) begin
case (s1)
0: begin s1<=s1+1; go_r<=1; go_w<=0; end
1:if (ok_r) begin s1<=s1+1; go_r<=0; char_w<=char_r; end
2: begin s1<=s1+1; go_w<=1; end
3:if (ok_w) begin s1<=s1+1; go_w<=0; end
// default: s1<=s1+1;
endcase
end

endmodule


module send(clk, din, go, ok, TXD);

input clk;
input [7:0] din;
input go;
output ok; reg ok_r;
output TXD; reg TXD_r;
reg [31:0] cnt;
reg [7:0] step;

assign TXD=~TXD_r;
assign ok=ok_r;

reg s1;
always @(posedge clk) begin
case (s1)
0 : begin
cnt<=0;
step<=0;
ok_r<=0;
TXD_r<=0;
s1<=1;
end
1 : if (go != 0)
begin
cnt<=cnt+1;
if (cnt==`LENGTH) begin step<=step+1; cnt<=0; end
if (step==12) begin s1<=0;ok_r<=1; end
else if (step==0) TXD_r<=1;
else if (step>=9) TXD_r<=0;
else if ((step>0)&&(step<9)) TXD_r<=~din[step-1];
end
endcase
end
endmodule

module recv(clk, din, go, ok, RXD);
input clk;
output [7:0] din; reg [7:0] din_r;
input go;
output ok; reg ok_r;
input RXD; reg RXD_r;
reg [31:0] cnt;
reg [7:0] step;

assign ok=ok_r;
assign din=din_r;

reg s1;
reg s2;
always @(posedge clk) begin
RXD_r<=~RXD;
case (s1)
0 : begin
cnt<=0;
step<=0;
ok_r<=0;
s1<=1;
end
1 : if (go != 0)
case (s2)
0 : if (RXD_r == 1) begin s2<=1;din_r<=0; end
1 : begin
cnt<=cnt+1;
if (cnt==`LENGTH) begin step<=step+1; cnt<=0; end
if (step==9) begin s1<=0;s2<=0;ok_r<=1; end
else if (step==0) begin end
else if ((step>0)&&(step<9)&&(cnt==(`LENGTH/16)))
din_r[step-1]<=~RXD_r;
end
endcase
endcase
end
endmodule

 

[Steven Sharp]

paulw@mmail.ath.cx (Paul) wrote in message news:<3ba4d769.0401040259.2c8184e1@posting.google.com>...

Hi

I've been coding a rs-232 rx/tx module. after 3 days, I've finally
track down a weird bug. May be someone here can help explain this.

input RXD; reg RXD_r;
....
....
din_r<=din_r+(1<<(step-1))*(~RXD_r); // bugged version

While there isn't enough detail here to be sure, I do see a potential
problem with this code. I suspect that (~RSD_r) is not producing the
value that you expect. You probably expect it to have the value 0 or 1.

In Verilog, all operands are extended to the full width of the expression
that they are in before any operations are performed. This helps avoid
overflowing intermediate results. That means that the value of RXD_r will
get extended to the full width of the expression, and then that resulting
vector value will get inverted. The width of the expression is essentially
the width of the widest operand, which is 32 bits for the unsized decimal
constant 1. The final result will get truncated back to the width of din_r
during assignment.

So if RXD_r is 1'b0, then it gets extended to 32'b0, which is then inverted
by the ~ to produce 32'b11111111111111111111111111111111, not 1. And if
RXD_r is 1'b1, then it gets extended to 32'b1, which is then inverted to
produce 32'b11111111111111111111111111111110, not 0. Then you are
multiplying by those values, which is probably not what you wanted.

You could get around this by using a logical not (!), which produces a
one-bit 0 or 1. However, this whole approach of using multiplication to
set or clear another bit is a bad idea. It is fine as a kludge in a
language that has no logical AND operator, but Verilog has them. And
even that is unnecessary in this case. Why not just shift the value
of RXD_r instead: din_r <= din_r | (RXD_r << (step-1)); Here the width
extension rules do what they are supposed to, ensuring that RXD_r gets
extended to the width of din_r before shifting, avoiding premature
overflow of the shifted bit off the end. Also note the use of logical
OR instead of addition. If you are designing hardware, you don't want
to use unnecessarily expensive operations. Addition is more expensive
than a logical operation, and multiplication is extremely expensive.

Even better is to write exactly what you are trying to do in simplest
form: set the (step-1) bit of din_r to the value of RXD_r, without
affecting the other bits. Which is what your final working version did.