-
Notifications
You must be signed in to change notification settings - Fork 353
target/riscv: Fix some timeout check order #1265
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
base: riscv
Are you sure you want to change the base?
Conversation
Ubuntu 20.04 is no longer available. See actions/runner-images#11101 Checkpatch-ignore: BAD_SIGN_OFF Change-Id: I0ec3e3342f9212a2a79d8dca6274e7db62ecedab Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
when Debug Module is inactive, accesses to the nextdm may fail. Specifically, nextdm might not return correct data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the change elsewhere as well.
src/target/riscv/riscv-013.c
Outdated
if (!get_field32(dmcontrol, DM_DMCONTROL_DMACTIVE)) | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Detail:
During the refactor, the debug message DM reset initiated
got removed. I am not sure if this was intended or simply an oversight.
I would prefer to keep the message there. The message can also be slightly reworded for clarity.
if (!get_field32(dmcontrol, DM_DMCONTROL_DMACTIVE)) {
LOG_TARGET_DEBUG(target, "The DM has just become deactivated (dmcontrol.dmactive: 1 -> 0).");
break;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed, thanks.
Reorder the timeout check and conditional judgment to ensure that timeout check is not wasted.
c9a7363
to
ded5f59
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lz-bro, to be honest, I don't understand the need for this patch.
dmi_write()
is time-bound by the sameriscv_get_command_timeout_sec()
(seeriscv-openocd/src/target/riscv/riscv-013.c
Line 2531 in 8d5f2fe
static int batch_run_timeout(struct target *target, struct riscv_batch *batch) - IMHO, this is more straightforward when an operation that took longer then the timeout fails, indicating that you need to increase the timeout.
Reorder the timeout check and conditional judgment to ensure that timeout check is not wasted.