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

Various cleanup #43

Merged
merged 6 commits into from
Feb 10, 2024
Merged

Various cleanup #43

merged 6 commits into from
Feb 10, 2024

Conversation

micprog
Copy link
Member

@micprog micprog commented Feb 9, 2024

  • Update build flow/testbench
  • Fix testbench for badly addressed bursts
  • Add types to all parameters
  • Replace AXI interfaces with structs

Fixes r.last signal for accidental bursts to apb
@micprog
Copy link
Member Author

micprog commented Feb 9, 2024

I can also split this PR up a bit, there are quite a few changes coming together here...

Copy link
Contributor

@yvantor yvantor left a comment

Choose a reason for hiding this comment

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

The PR looks good to me. I just noticed that the DMA tests [do not even start])https://iis-git.ee.ethz.ch/github-mirror/pulp_cluster/-/jobs/996194#L26), and the same is for the parallel bare tests.

@micprog micprog force-pushed the michaero/cleanup-2 branch from db3e67a to db4b58b Compare February 9, 2024 16:16
@micprog
Copy link
Member Author

micprog commented Feb 9, 2024

There were some more issues due to bad testing on my part, sorry. Should be fixed now (with some speed improvements and no more warnings). Might also be worth it to ensure the CI works properly and reports fails when it should...

@yvantor
Copy link
Contributor

yvantor commented Feb 9, 2024

There were some more issues due to bad testing on my part, sorry. Should be fixed now (with some speed improvements and no more warnings). Might also be worth it to ensure the CI works properly and reports fails when it should...

I think the problem is that the CI greps for a fatal or some error message (linked to the returned value I think) to understand if a failed, and if the grep does not succeed (as in the case of a test that did not even start) it just returns success. I agree that this should be improved. Also making the regressions' outputs more homogeneous (like all of them printing either "Success!" or "Failed!" at the end) could maybe help.

@yvantor
Copy link
Contributor

yvantor commented Feb 9, 2024

@OttG as soon as @micprog merges [#44] and the CI completes, this can also be merged.

* Add `run_and_exit.tcl`; use specific commits for regressions and runtime.

* Update nonfree flow

* Fix environment sourcing

* Adapt run scripts to use VSIM.

* Add error suppression for WLF logging.

---------

Co-authored-by: Yvan Tortorella <ytortorella@lagrev3.ee.ethz.ch>
Co-authored-by: Michael Rogenmoser <michael@rogenmoser.us>
@OttG
Copy link
Collaborator

OttG commented Feb 10, 2024

Everything seems fine now, CI is passing. Merging now.

@OttG OttG merged commit 90626e0 into master Feb 10, 2024
5 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.

3 participants