2
\$\begingroup\$

Module for generating a PWM signal. The req_value_i input gets a duration value of the signal. Furthermore, the module can be stopped by deassertion of the enable_i input.

`timescale 1ns / 1ps
    
 module pwm #
(
    parameter integer PWM_COUNTER_WIDTH = 8
)
( 
    input  logic                             clk_i,
    input  logic                             s_rst_n_i,
    input  logic                             enable_i,
        
    input  logic [PWM_COUNTER_WIDTH - 1 : 0] req_value_i,
    
    output logic                             channel_o  
    );
    
    logic [PWM_COUNTER_WIDTH - 1 : 0] counter;
        
    always_comb begin
        channel_o = (req_value_i != counter) ? 1'h1 : 1'h0;
    end
        
    always_ff @ (posedge clk_i) begin
        if (1'h0 == s_rst_n_i) begin
            counter <= {PWM_COUNTER_WIDTH{1'h0}};
        end
        else if (1'h1 == enable_i) begin
            counter <= counter + 1'h1;
        end
    end
    
endmodule
\$\endgroup\$
2
  • 1
    \$\begingroup\$ I commend you for making the effort to make major edits to your question. Many people simply delete or abandon the question. \$\endgroup\$ – toolic Feb 24 at 18:11
  • \$\begingroup\$ Definitely, I have known how right to publish posts and will try to do it in the future correctly. Thanks =) \$\endgroup\$ – drakonof Feb 25 at 10:58
2
\$\begingroup\$

The layout of your code is consistent and easy to read. I don't see any functional problems.

One consideration for all digital logic is whether you can tolerate glitches on your outputs. If your PWM output is directly driving a load such as a power inverter, you likely want to avoid glitches. To avoid glitches, change your combinational output to a registered output:

always_ff @ (posedge clk_i) begin
    if (1'h0 == s_rst_n_i) begin
        channel_o <= 1'h0;
    end
    else begin
        channel_o <= (req_value_i != counter);
    end
end

I also simplified the expression by removing the ternary code ? 1'h1 : 1'h0. But, this is just a matter of coding style preference.

Another consideration is whether to use a synchronous reset (as you have) or an asynchronous reset. The advantage of asynchronous is that you don't need to have the clock toggling in order to reset your logic. For asynchronous, you would use:

always_ff @ (posedge clk_i or negedge s_rst_n_i) begin
\$\endgroup\$
1
  • 1
    \$\begingroup\$ your comments still are accurate and useful. I am grateful you for your time and helping my evolving. \$\endgroup\$ – drakonof Feb 25 at 11:06

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.