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

RISCV: Add some quirks to be compatible with CH32V3x chips #1576

Closed
wants to merge 65 commits into from

Conversation

mean00
Copy link

@mean00 mean00 commented Jul 29, 2023

Detailed description

  • These patches enable breakpoints to work to some extent with the CH32V3x chips (maybe CH32V2x also)

  • The first patch is simple enough, the CH32V3 chips make an unexpected reply when probing the isa width. Adding that error code to the list makes it being detected properly as RV32.

  • The 2nd patch is a bit more quirky. When probing the triggers, the code is expected either to have a reply_ok and a value or an error and then it reads the value the alternate way. The WCH replies ok, but the reply is empty. The patch uses an empty reply as reason to switch to the alternate method.
    I'm really unsure this is the right thing to do and/or if this could have side effects.

With these two patches, breakpoints work to some extend with the CH32V3x.
To test it, you need to have a modified bmpa sw to support the WCH link (see perigoso work) and a board support package for the CH32V3x chip (i have a early development one if interested).

Your checklist for this pull request

  • I've read the Code of Conduct
  • [x ] I've read the guidelines for contributing to this repository
  • [x ] It builds for hardware native (make PROBE_HOST=native)
  • [x ] It builds as BMDA (make PROBE_HOST=hosted)
  • [x ] I've tested it to the best of my ability
  • [x ] My commit messages provide a useful short description of what the commits do

Closing issues

None, this is a part of CH32v3x support.

dragonmux added 30 commits July 26, 2023 03:19
… and a failure warning to the CSR wait complete function
perigoso and others added 4 commits July 26, 2023 03:19
…djust for BMD's representation of JEP-106 codes
… code wrong bus address is different, the chip replies ok when probing the breakpoints but the reply is empty
@dragonmux dragonmux added this to the v2.0 release milestone Jul 29, 2023
@dragonmux dragonmux added HwIssue Mitigation Solving or mitigating a Hardware issue in Software New Target New debug target labels Jul 29, 2023
Copy link
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

There's a small tweak can be made in riscv_heart_discover_triggers(), otherwise this looks good. When we get to the v2.0 cycle we'll aim to schedule this for merge with the WCH RISC-V debug PR from Perigoso.

src/target/riscv_debug.c Outdated Show resolved Hide resolved
@perigoso
Copy link
Contributor

This change is now part of #1399

@dragonmux is it correct that you added the additional RISCV_HART_NOT_SUPP check on the feature/risc-v branch?

@perigoso perigoso mentioned this pull request Aug 11, 2023
6 tasks
@dragonmux
Copy link
Member

We'd have to double-check the code to answer that - could you link a reference to which check you're referring to?

@perigoso
Copy link
Contributor

We'd have to double-check the code to answer that - could you link a reference to which check you're referring to?

Part of this PR was adding RISCV_HART_NOT_SUPP as a valid status here:

		/* If that failed, then find out why and instead try the next narrower width */
		if (hart->status != RISCV_HART_BUS_ERROR && hart->status != RISCV_HART_EXCEPTION)
		switch (hart->status) {
		case RISCV_HART_BUS_ERROR:
		case RISCV_HART_EXCEPTION:
		case RISCV_HART_NOT_SUPP: // WCH CH32Vx chips reply that
			break;
		default:
			return 0;
			break;
		}
		if (hart->access_width == 32U) {

But while cherry-picking this commit I noticed the base branch already had it:

		/* If that failed, then find out why and instead try the next narrower width */
		if (hart->status != RISCV_HART_BUS_ERROR && hart->status != RISCV_HART_EXCEPTION &&
			hart->status != RISCV_HART_NOT_SUPP)
			return 0;
		if (hart->access_width == 32U) {

I assumed this meant you added this as part of the last rebase, but wanted to confirm

@dragonmux
Copy link
Member

Ahhh, yes, we'd done that when preparing the RV branch for handling more than the ESP32-C3, yes

@dragonmux dragonmux force-pushed the feature/risc-v branch 3 times, most recently from adb1b22 to a705f96 Compare September 20, 2023 06:38
@dragonmux dragonmux force-pushed the feature/risc-v branch 3 times, most recently from 94cd98a to 11ec1f1 Compare October 31, 2023 05:34
@dragonmux dragonmux deleted the branch blackmagic-debug:feature/risc-v October 31, 2023 05:44
@dragonmux dragonmux closed this Oct 31, 2023
@dragonmux
Copy link
Member

Note to self: Cherry-pick the fixes on this branch into the ESP32-C3 branch having fubbed merging this to the RISC-V branch by accident.

dragonmux added a commit that referenced this pull request Nov 29, 2023
…s triggered on WCH devices

This re-implements the fix from fubbed PR #1576 with thanks to mean00

Co-Authored-By: Rafael Graça Silva <rg-silva@criticalsoftware.com>
Co-Authored-By: dragonmux <git@dragonmux.network>
dragonmux added a commit that referenced this pull request Nov 29, 2023
…s triggered on WCH devices

This re-implements the fix from fubbed PR #1576 with thanks to mean00

Co-Authored-By: Rafael Graça Silva <rg-silva@criticalsoftware.com>
Co-Authored-By: dragonmux <git@dragonmux.network>
dragonmux added a commit that referenced this pull request Nov 30, 2023
…s triggered on WCH devices

This re-implements the fix from fubbed PR #1576 with thanks to mean00

Co-Authored-By: Rafael Graça Silva <rg-silva@criticalsoftware.com>
Co-Authored-By: dragonmux <git@dragonmux.network>
dragonmux added a commit that referenced this pull request Nov 30, 2023
…s triggered on WCH devices

This re-implements the fix from fubbed PR #1576 with thanks to mean00

Co-Authored-By: mean <fixounet@free.fr>
Co-Authored-By: dragonmux <git@dragonmux.network>
esden pushed a commit that referenced this pull request Nov 30, 2023
…s triggered on WCH devices

This re-implements the fix from fubbed PR #1576 with thanks to mean00

Co-Authored-By: mean <fixounet@free.fr>
Co-Authored-By: dragonmux <git@dragonmux.network>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HwIssue Mitigation Solving or mitigating a Hardware issue in Software New Target New debug target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants