-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pwm_gen_tb: Add tb for axi_pwm_gen #57
base: main
Are you sure you want to change the base?
Conversation
alin724
commented
Nov 12, 2023
- pwm_gen_tb - configuration task - configure period, width, offset of different channels;
- HDL repo PR;
Signed-off-by: Alin-Tudor Sferle <[email protected]>
$display("[%t] axi_pwm_gen started.", $time); | ||
#100000; | ||
|
||
axi_write (`AXI_PWMGEN + 32'h00000010, 'h2); // stop PWM_GEN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'h2 should be changed to 'h1. 'h2 loads the current configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, shouldn't we use the following format instead of manually setting the address?
axi_write (AXI_PWMGEN + GetAddrs(REG_RSTN),
SET_REG_RSTN_RESET(1));
.pwm_gen_o_13 (pwm_gen_o_13), | ||
.pwm_gen_o_14 (pwm_gen_o_14), | ||
.pwm_gen_o_15 (pwm_gen_o_15), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove extra line.
input pwm_gen_o_13, | ||
input pwm_gen_o_14, | ||
input pwm_gen_o_15, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra line
|
||
task test_config; | ||
begin | ||
#100 axi_write (`AXI_PWMGEN + GetAddrs(REG_RSTN), `SET_REG_RSTN_RESET(0)); // PWM_GEN reset in regmap (ACTIVE HIGH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The #100 delays can be removed in all these axi writes, as they do not affect the functionality and are not mandatory.