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

doc: trd104: fix pointer->parameter #4299

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Conversation

bradjc
Copy link
Contributor

@bradjc bradjc commented Jan 8, 2025

Pull Request Overview

I believe that referring to the "appdata" in subscribe as an "application data pointer" in TRD104 is unintentional, and that it should be "application data parameter" to match the earlier description. I believe this stems from userspace largely using that parameter to store a pointer, but we don't require that (and can't enforce it). This clarifies the document.

Testing Strategy

docs

TODO or Help Wanted

n/a

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make prepush.

jrvanwhy
jrvanwhy previously approved these changes Jan 8, 2025
lschuermann
lschuermann previously approved these changes Jan 8, 2025
@lschuermann
Copy link
Member

--------------------------------------------------------------------------------
Experienced errors during the test: Timeout Error: Expected "Userspace call to read console returned: Tock is awesome!" but got "`\r``\r``\n`
" (after waiting 120000 ms)

Huh, let's see whether re-opening fixes this LiteX test fluke.

@lschuermann lschuermann closed this Jan 8, 2025
@lschuermann lschuermann reopened this Jan 8, 2025
@lschuermann lschuermann added the last-call Final review period for a pull request. label Jan 12, 2025
@lschuermann
Copy link
Member

@bradjc Can you bump the draft version of this?

I believe that referring to the "appdata" in subscribe as an
"application data pointer" is unintentional, and that it should be
"application data parameter" to match the earlier description. I believe
this stems from userspace largely using that parameter to store a
pointer, but we don't require that (and can't enforce it). This
clarifies the document.
@ppannuto ppannuto dismissed stale reviews from lschuermann and jrvanwhy via a31b20b January 15, 2025 17:52
@ppannuto ppannuto force-pushed the trd104-parameter-reword branch from 6b0fdad to a31b20b Compare January 15, 2025 17:52
@ppannuto ppannuto enabled auto-merge January 15, 2025 17:52
@ppannuto
Copy link
Member

@lschuermann looks like something weird with litex still?

@lschuermann
Copy link
Member

@lschuermann looks like something weird with litex still?

Ever since I switched it to use the MLFQ scheduler it's been failing pretty non-deterministically. This is definitely something we should investigate, I think chances are that it's actually something related to MLFQ.

@ppannuto
Copy link
Member

Should we mark litex as a non-required status check until the MLFQ issue is resolved?

@lschuermann
Copy link
Member

Should we mark litex as a non-required status check until the MLFQ issue is resolved?

Done.

@lschuermann lschuermann disabled auto-merge January 16, 2025 12:53
@lschuermann lschuermann added this pull request to the merge queue Jan 16, 2025
Merged via the queue into master with commit 6836d23 Jan 16, 2025
16 of 21 checks passed
@lschuermann lschuermann deleted the trd104-parameter-reword branch January 16, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation last-call Final review period for a pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants