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

target/riscv: Mismatch napot when mcontrol.maskmax is not expected #1133

Conversation

sunnyzhu-learning
Copy link

Due to code conflict,
this PR(https://github.com/riscv-collab/riscv-openocd/pull/1127)closed,
and then recreated a PR to solve question

code review all addressed,@en-sc @JanMatCodasip please check below code,thanks

    1.Remove trigger_request_info::tdata1_ignore_mask
    2.Adding ignore napot matching condition

Signed-off-by: zhusonghe <[email protected]>
tdata1_config_denied = (tdata1 & ~maskmax) != (tdata1_rb & ~maskmax);
} else {
tdata1_config_denied = tdata1 != tdata1_rb;
}
Copy link
Author

Choose a reason for hiding this comment

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

else if() and else () are my considered,please check it is reasonable?

@@ -727,13 +758,11 @@ static int maybe_add_trigger_t1(struct target *target, struct trigger *trigger)
struct trigger_request_info {
riscv_reg_t tdata1;
riscv_reg_t tdata2;
riscv_reg_t tdata1_ignore_mask;
Copy link
Author

Choose a reason for hiding this comment

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

from PR:#1127
according discussion,remove tdata1_ignore_mask

@en-sc
Copy link
Collaborator

en-sc commented Sep 18, 2024

@sunnyzhu-learning, please, take a look at build and Checkpatch failures.

Also I'd like to ask you to continue the review in the previous PR to simplify the review process.
You can do it by hard-resetting the branch with the previous review. Something like:

git checkout Songhe-develop
git reset --hard maskmax-develop
git push -f

And reopen the PR on github.
This will greatly simplify the review.

@sunnyzhu-learning sunnyzhu-learning closed this by deleting the head repository Sep 19, 2024
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