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: select DMI IR on batch access #1089

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

en-sc
Copy link
Collaborator

@en-sc en-sc commented Jun 13, 2024

Without the selection the TAP can be left in bypass.

Change-Id: I79c6bf74802dc9c9475947d1787a3d0b797f3952

@en-sc en-sc requested a review from JanMatCodasip June 13, 2024 17:48
@en-sc en-sc self-assigned this Jun 13, 2024
@en-sc
Copy link
Collaborator Author

en-sc commented Jun 13, 2024

@JanMatCodasip, can you please prioritize looking at this one. It's an uncaught bug in #1073.

JanMatCodasip
JanMatCodasip previously approved these changes Jun 14, 2024
Copy link
Collaborator

@JanMatCodasip JanMatCodasip left a comment

Choose a reason for hiding this comment

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

@en-sc I have checked this fix visually and it looks good. Thank you.

Few additional notes (perhaps for future pull requests):

  • The calls to select_dmi() in assert_reset(), deassert_reset() and read_memory_progbuf() appear to be redundant. The underlying batch API or dm_read/dm_write() API should always take care of that.
  • I would be in favor of having assertions that the DMI has been selected, however the BSCAN tunnel support complicates that - the assertions would not be simple.
  • Furthermore, I am afraid we don't have any way how to test the BSCAN tunnel support. (Or do we?)
  • I would eventually like to move all the JTAG DTM related items to their own module (riscv013-jtag-dtm.c/h). Then the select_dmi() could be called directly from batch.c, which would put the select_dmi() call closer to the actual execution of the JTAG scans.

Without the selection the TAP can be left in bypass.

Change-Id: I79c6bf74802dc9c9475947d1787a3d0b797f3952
Signed-off-by: Evgeniy Naydanov <[email protected]>
@en-sc
Copy link
Collaborator Author

en-sc commented Jun 14, 2024

@JanMatCodasip, please, take a look again.
I would like to separate the check in select_dmi() into #1091 and test it some more.

@en-sc
Copy link
Collaborator Author

en-sc commented Jun 14, 2024

  • The calls to select_dmi() in assert_reset(), deassert_reset() and read_memory_progbuf() appear to be redundant. The underlying batch API or dm_read/dm_write() API should always take care of that.

I think you are right. Filed #1092.

  • I would be in favor of having assertions that the DMI has been selected, however the BSCAN tunnel support complicates that - the assertions would not be simple.

I'm not sure it's worthwhile. We do have assertions already JTAG common interface that cover this.

  • Furthermore, I am afraid we don't have any way how to test the BSCAN tunnel support. (Or do we?)

AFAIK, we don't.

  • I would eventually like to move all the JTAG DTM related items to their own module (riscv013-jtag-dtm.c/h). Then the select_dmi() could be called directly from batch.c, which would put the select_dmi() call closer to the actual execution of the JTAG scans.

I'd like that to. This is essentially the goal of all these commits -- to tidy up the work with DM and move it to a separate file after this.

Copy link
Collaborator

@JanMatCodasip JanMatCodasip left a comment

Choose a reason for hiding this comment

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

LGTM

@JanMatCodasip
Copy link
Collaborator

en-sc: I would like to separate the check in select_dmi() into #1091 and test it some more.

Sure, no problem!

JanMatCodsip: I would be in favor of having assertions that the DMI has been selected (...)

en-sc: I'm not sure it's worthwhile. We do have assertions already JTAG common interface that cover this.

Ok, let's leave it as is, without additional asserts.

JanMatCodasip: I would eventually like to move all the JTAG DTM related items to their own module (...)

en-sc: I'd like that to. This is essentially the goal of all these commits (...)

Great :)

@aap-sc aap-sc merged commit fdd07f1 into riscv-collab:riscv Jun 17, 2024
4 checks passed
@en-sc en-sc deleted the en-sc/batch-select-dmi branch August 14, 2024 18:38
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