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 diagnostics for non-implemented data watchpoints #897

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

aap-sc
Copy link
Collaborator

@aap-sc aap-sc commented Jul 31, 2023

Change-Id: If5031c6d8cea1bfcc34bb65fd766f232498ed7ea

Copy link
Collaborator

@timsifive timsifive left a comment

Choose a reason for hiding this comment

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

Looks good overall.

// replaced by WATCHPOINT_IGNORE_DATA_VALUE_MASK once it is available
// See: https://review.openocd.org/c/openocd/+/7840
if (watchpoint->mask != ~(typeof(watchpoint->mask))0) {
LOG_ERROR("watchpoints on data values are not implemented");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
LOG_ERROR("watchpoints on data values are not implemented");
LOG_TARGET_ERROR(target, "watchpoints on data values are not implemented");

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

addressed

@aap-sc aap-sc force-pushed the aap-sc/wp_data_not_implemented branch from 2f5e289 to f36ae13 Compare August 1, 2023 16:54
@aap-sc aap-sc requested a review from timsifive August 1, 2023 16:58
MarekVCodasip
MarekVCodasip previously approved these changes Aug 2, 2023
Copy link
Collaborator

@MarekVCodasip MarekVCodasip left a comment

Choose a reason for hiding this comment

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

But that is a minor thing, giving approve.

// replaced by WATCHPOINT_IGNORE_DATA_VALUE_MASK once it is available
// See: https://review.openocd.org/c/openocd/+/7840
if (watchpoint->mask != ~(typeof(watchpoint->mask))0) {
LOG_TARGET_ERROR(target, "watchpoints on data values are not implemented");
Copy link
Collaborator

@MarekVCodasip MarekVCodasip Aug 2, 2023

Choose a reason for hiding this comment

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

One tiny thing, I would capitalize the first letter in the "watchpoints" error log

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

addressed

Change-Id: If5031c6d8cea1bfcc34bb65fd766f232498ed7ea
Signed-off-by: Parshintsev Anatoly <[email protected]>
@timsifive timsifive merged commit 68ad364 into riscv-collab:riscv Aug 14, 2023
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