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

Uart status interrupts #21632

Merged
merged 3 commits into from
Mar 5, 2024
Merged

Uart status interrupts #21632

merged 3 commits into from
Mar 5, 2024

Conversation

GregAC
Copy link
Contributor

@GregAC GregAC commented Feb 22, 2024

This includes a commit from #21226 which is a cip_lib change required for all status type interrupts. So need to ensure one of the PRs gets rebased if the other has gone in.

@GregAC GregAC requested a review from a team as a code owner February 22, 2024 17:51
@GregAC GregAC requested review from jdonjdon and msfschaffner and removed request for a team February 22, 2024 17:51
@msfschaffner
Copy link
Contributor

Thanks @GregAC - maybe it makes sense to split out the CIP fixes into a separate PR since those will benefit others as well.
I believe we need to introduce a similar masking in the generated "all interrupts" TLT.
Sorry that I did not get to this yet, will try to look into it on Monday.

CC @andreaskurth @vogelpi @alees24

@andreaskurth
Copy link
Contributor

maybe it makes sense to split out the CIP fixes into a separate PR since those will benefit others as well. I believe we need to introduce a similar masking in the generated "all interrupts" TLT. Sorry that I did not get to this yet, will try to look into it on Monday.

Yes, maybe worth splitting all commits that apply to more IPs than only flash_ctrl into a separate PR, so that those become available asap while the failing tests in #21226 get resolved?

@GregAC GregAC requested a review from a team as a code owner February 29, 2024 18:37
@GregAC GregAC requested review from moidx and removed request for a team February 29, 2024 18:37
@GregAC
Copy link
Contributor Author

GregAC commented Feb 29, 2024

Latest push addresses CI failures, will address the splitting out/merging with other work of the CIP changes separately.

This PR should not be merged until the CIP change work is addressed.

@GregAC GregAC force-pushed the uart_status_interrupts branch 4 times, most recently from a11dad5 to 0e18a48 Compare March 1, 2024 17:05
@GregAC
Copy link
Contributor Author

GregAC commented Mar 2, 2024

I've spun out the first commit, which is a CIP fix that is useful for status interrupt in general to another PR: #21804

As there's some CI issues with the CW310 tests that may take a little longer to solve it'd be good to get the CIP fix in sooner.

The RX and TX watermarks are now status type interrupts. This requires
differently handling. The test has also been altered to transfer smaller
amounts of data at a time to the UART TX FIFO. This produces multiple
rounds of TX watermark followed by more data being sent.

Signed-off-by: Greg Chadwick <[email protected]>
RX watermark interrupt is now a status type and needs to be dealt with
slightly differently.

Signed-off-by: Greg Chadwick <[email protected]>
@GregAC
Copy link
Contributor Author

GregAC commented Mar 4, 2024

@timothytrippel could you check out the OTTF change? (Final commit)

@GregAC
Copy link
Contributor Author

GregAC commented Mar 4, 2024

Thanks to @jwnrt for the pointers so I could debug and (hopefully) fix the final CI issues.

I'll also do a PR to switch TX empty to a status type interrupt as requested in #16693 but I'll do that in a separate PR

@timothytrippel
Copy link
Contributor

@timothytrippel could you check out the OTTF change? (Final commit)

OTTF changes LGTM.

Copy link
Contributor

@andreaskurth andreaskurth left a comment

Choose a reason for hiding this comment

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

DD & DV changes LGTM. Thanks @GregAC 👍

@GregAC GregAC merged commit ffab8da into lowRISC:master Mar 5, 2024
32 checks passed
@GregAC GregAC deleted the uart_status_interrupts branch March 5, 2024 09:27
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.

4 participants