3

I have written following code which produces pulse of different width.I want the code to produce a single pulse according to select line. If select line is

00 pulse width = 1 us , 01 pulse width = 10 us . . 11 pulse width = 1000 us

The input clock is of 10 Mhz. But according to code I am getting continuous pulse if I don't provide any other value of selection line.How can I achieve only one pulse?

    module pulse(input wire [1:0] sel , //selection lines s1 s0
        input clk,
        input rst_n,
        output reg flag,    //for checking conditions
        output reg [13:0] Q,    // output of 14 bit counter
        output reg pulse,   //output pulse
        output reg count);  //also for checking conditions

wire flag_d , count_d;

assign flag_d = ( (sel == 2'b00 | sel == 2'b01 | sel == 2'b10 | sel == 2'b11) && count == 1'b0)? 1'b1 : flag;
assign count_d = ( (sel == 2'b00 | sel == 2'b01 | sel == 2'b10 | sel == 2'b11) && count == 1'b0)? 1'b1 : count;

always @(posedge clk , negedge rst_n)
begin
    if(!rst_n)
    begin
        Q <= 14'h0;
        count <= 1'b0;
        pulse <= 1'b0;
        flag <= 1'b0;
    end
    else
    begin

        flag <= flag_d;
        count <= count_d;

        if(flag)
        begin
            case(sel)
            2'b00: Q <= 14'd11;//count from 11 to 1
            2'b01: Q <= 14'd101;//count from 101 to 1
            2'b10: Q <= 14'd1001;//count from 1001 to 1
            2'b11: Q <= 14'd10001;//count from 10001 to 1
            default: Q <= 14'd0;    
            endcase

            flag <= 1'b0;
        end
        else
        begin
            if(Q != 14'h1 && Q != 14'h0)
            begin
                Q <= Q - 14'h1;
                pulse  <= 1'b1;
            end
            else
            begin
                pulse <= 1'b0;
                count <= 1'b0;
            end
        end  
    end
end 
endmodule

Is this code in a good coding style considering the synthesis and hardware of the circuit? if not than what changes I should apply?..

Ishita Shah
  • 99
  • 1
  • 4
  • 12
  • What do u mean by " But according to code I am getting continuous pulse if I don't provide any other value of selection line."Please explain – chitranna Aug 26 '15 at 05:14
  • If I am keeping sel = 00 it will give pulse of 1 us and go low and again it will detect sel to be 00 so it will give further 1us pulse..But i want only one pulse even if sel = 00 continuously. – Ishita Shah Aug 26 '15 at 05:16
  • Also 00: Q <= 14'd11;//00000000001010; 01: Q <= 14'd101;//00000001100100; 10: Q <= 14'd1001;//00001111101000; 11: Q <= 14'd10001;//10011100010000; give values 3, 5 , 9 and 17 – chitranna Aug 26 '15 at 05:36
  • I have used "d"-decimal values.So the pulse width in simulation is proper.And can u give example of this 2 flag model,already I have used count and flag reg to determine the conditions. – Ishita Shah Aug 26 '15 at 05:44
  • shouldn't it be 14'd10 14'd100 so on ? 14'd11 converts 00000000001011; – chitranna Aug 26 '15 at 05:55
  • When does the select line changes ? – chitranna Aug 26 '15 at 05:58
  • yeah pulse should be high for 10 clock cycles so I am making Q count from 11 to 1 , 101 to 1 and so on..because Q is 0 at reset.And selection line change is done from testbench. It can be random.. – Ishita Shah Aug 26 '15 at 06:01
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/87949/discussion-between-chitranna-and-ishita-shah). – chitranna Aug 26 '15 at 06:04
  • 1
    The indexes in the case statement are wrong. `10` is ten, `11` is eleven. Should be `2'b10` and `2'b11` – Greg Aug 26 '15 at 17:23

2 Answers2

2

I couldn't figure out the point of flag_d and count_d. Also ( (sel == 2'b00 | sel == 2'b01 | sel == 2'b10 | sel == 2'b11) && count == 1'b0) simplifies to (count == 1'b0). sel should not be Xs or Zs.

I think you want something more like the following:

reg [13:0] next_Q;
always @* begin
  if (Q==0) begin
    case(sel)
    2'b00 : next_Q = 14'd10;
    2'b01 : next_Q = 14'd100;
    2'b10 : next_Q = 14'd1000;
    2'b11 : next_Q = 14'd10000;
    endcase
  end
  else begin
    next_Q = Q - 1;
  end
end
always @(posedge clk, negedge rst_n) begin
  if (!rst_n) begin
    pulse <= 1'b0;
    Q <= 14'd0;
  end
  else begin
    // if (Q==0) pulse <= !pulse; // high and low pulse will have equal if sel is constant
    pulse <= (Q!=0); // or high pulse based on sel, low is one clk
    Q <= next_Q;
  end
end

working example: http://www.edaplayground.com/x/GRv

Greg
  • 18,111
  • 5
  • 46
  • 68
  • According to your code, it is not possible to produce only one or a single pulse because it is going to complement pulse after every counter overflow..also when I give input selection line 00 for some time and 01 after that-it remains in 0 state for count of 100(count value for sel = 01) which is not correct. So is it not possible to produce a single pulse? – Ishita Shah Aug 27 '15 at 06:23
  • Look at the comments next to where I assigned `pulse`. I wasn't sure if you wanted the pulse only for high or both high and low, so I wrote both with one commented out. `sel` is only sampled when `Q==0`, otherwise the pulse width cannot be guaranteed. There is no provided input to know when to start a pulse, so it will repeat. – Greg Aug 27 '15 at 06:42
1
         module pulse(input wire [1:0] sel,         // No need for the sel to be wire 
        input   clk,
        input rst_n,
        output reg [13:0] Q,    
        output reg pulse,   
        input   input_stb,                  // Input is valid
        input   output_ack,     
        output  output_stb,         
        output  input_ack);                // 2 Flag model 

reg s_input_ack ;
reg s_output_stb;
 parameter get_inputs         = 4'd0,
        counter               = 4'd1;

always @(posedge clk , negedge rst_n)
begin

case (state)
    get_inputs:
    s_input_ack <= 1;
    if (s_input_ack && input_a_stb) 
    begin
    s_input_ack <= 0;
            case(sel)
            00: Q <= 14'd11;//00000000001010;
            01: Q <= 14'd101;//00000001100100;
            10: Q <= 14'd1001;//00001111101000;
            11: Q <= 14'd10001;//10011100010000;
            default: Q <= 14'd0;    
            endcase
    state <= counter;
    end

    counter:
        begin
        s_output_stb <= 1;
        if (s_output_stb && output_z_ack)
            begin
                        s_output_stb <= 0;

                if(Q != 14'h1 && Q != 14'h0)
                    begin
                    Q <= Q - 1'b1;
                    pulse  <= 1'b1;
                    end
                else
                    begin
                    pulse <= 1'b0;
                    end
                end
        state <= get_inputs;
        end 

endcase
        if(!rst_n)
    begin
        Q <= 14'h0;
        pulse <= 1'b0;
              s_input_ack <= 0;
              s_output_stb <= 0;
    end
      assign input_ack = s_input_ack;

assign output_stb = s_output_stb; end endmodule *Still needs work , add registers and necessary signals accordingly. Will edit at a later point in time

chitranna
  • 1,579
  • 6
  • 25
  • 42
  • 1
    The asynchronous reset condition should be at the top of the always block `if (!rst_n) begin /* reset values assignment */ end else begin /* synchronous logic assignment */ end` What you have may work in simulation but synthesis may give funky results. The `assign` keyword should not be in an always block. – Greg Aug 26 '15 at 17:29
  • Absolutely , there are a lot of errors.Will edit it when I can access a simulator. But does this matter when asynchronous reset is used ? http://www.sunburst-design.com/papers/CummingsSNUG2003Boston_Resets.pdf. I know it does to synchronous resets but since asynchronous reset is used I thought it is fine – chitranna Aug 26 '15 at 22:48
  • @chitranna: I don't think this code will even create a pulse since u have assigned 'state <= get_inputs' before it completes the whole countdown process. – Ishita Shah Aug 27 '15 at 05:16