Skip to content
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

Add Pinmux PWM test #370

Merged
merged 2 commits into from
Dec 19, 2024
Merged

Add Pinmux PWM test #370

merged 2 commits into from
Dec 19, 2024

Conversation

AlexJones0
Copy link
Contributor

@AlexJones0 AlexJones0 commented Dec 19, 2024

See the commit messages for more detailed explanations. This PR introduces a new pinmux_pwm_test which tests that a PWM instance can be muxed, and that the existing PWM loopback testing works only when the pinmux is configured correctly. If running in CI, this test would be sufficient to catch the issue fixed in #369.

I've tested this on both FPGA and in simulation locally, and the FPGA in CI is already set up with a jumper cable so this test should be passing. I've also added the loopback to the simulation model so this can pass there as well.

Copy link
Contributor

@marnovandermaas marnovandermaas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me and passes in CI. One minor nit which is optional.

dv/verilator/top_verilator.sv Show resolved Hide resolved
This loopback can be physically present on the board for tests on the
FPGA, but must be modelled in Verilator simulation by directly
connecting the output from PMOD0 pin 8 to PMOD0 pin 10 in
`top_verilator.sv`. This will allow the Pinmux PWM test to pass in
simulation as well as on FPGA.
This commit introduces a new `pinmux_pwm_test` that will run in CI,
which tests that PWM appears to be working correctly when it is muxed
over PMOD, and that it doesn't work when disabled via Pinmux.

This re-uses the loopback testing code from the PWM tests that reads the
PWM output via GPIO to determine the duty cycle and period, and use that
to verify that PWM operation is as expected. There is some small
refactoring to make that possible, including a fix in which the PWM
loopback test was accidentally using hardcoded GPIO 0 instead of the
specified pin constant (which was also 0, hence no previous problems).

Requires an additional jumper cable to FPGAs to pass the test, which can
be optionally disabled by the same `PINMUX_CABLE_CONNECTIONS_AVAILABLE`
method as for tests where this was previously required.
@AlexJones0
Copy link
Contributor Author

Force push just addresses the typo in the commit message.

@AlexJones0 AlexJones0 merged commit e7088a6 into lowRISC:main Dec 19, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants