7
31
Fork
You've already forked blackmagic
23

WIP: renesas_ra: Add (more) support for RA6M5 and RA2E1 #2164

Draft
desertsagebrush wants to merge 17 commits from desertsagebrush/feat/renesas_ra2e1 into main
pull from: desertsagebrush/feat/renesas_ra2e1
merge into: blackmagic-debug:main
blackmagic-debug:main
blackmagic-debug:feature/bmda-remote-comms
blackmagic-debug:ALTracer/feature/aarch64-ident
blackmagic-debug:feature/better-meson-optimisation-handling
blackmagic-debug:feature/am335x-support
blackmagic-debug:feature/esp32-c3-support
blackmagic-debug:feature/cortex-ar-software-breakpoints
blackmagic-debug:feature/unit-testing
blackmagic-debug:feature/windows-usb-serial-interface-naming
blackmagic-debug:fix/bmp-external-spi
blackmagic-debug:ALTracer/feature/bluepillplus-platform
blackmagic-debug:ALTracer/feature/at32f43x-unrdp
blackmagic-debug:feature/const-correctness
blackmagic-debug:ALTracer/feature/fault_handlers
blackmagic-debug:ALTracer/feature/hazard3-ice40-support
blackmagic-debug:fix/ci-cleanup
blackmagic-debug:ALTracer/fix/gdb-10-12-thread
blackmagic-debug:ALTracer/feature/blackpill-f4-adc
blackmagic-debug:ALTracer/fix/cortex-desc-allocfail-report
blackmagic-debug:ALTracer/feature/spi-perf
blackmagic-debug:ALTracer/feature/calibrate_swd
blackmagic-debug:ALTracer/feature/blank-check
blackmagic-debug:feature/avr
blackmagic-debug:v2.0
blackmagic-debug:v1.9
blackmagic-debug:v1.10
blackmagic-debug:v1.8
desertsagebrush commented 2025年11月11日 22:35:20 +01:00 (Migrated from github.com)
Copy link

Detailed description

Adding/expanding support for Renesas RA6M5, RA4M1, and RA2E1 microcontrollers.

Todo:

  • Add ID codes and confirm probe support
  • Implement flashing for MF4/MF3 flash on the RA2E1 (RV40 flash for the RA6M5 is already supported)
  • Handle RA4 MF3/4 flash gracefully (different block size)
  • Possibly support the OCDREG control registers hanging off of AP1 (APB-AP)

Your checklist for this pull request

## Detailed description Adding/expanding support for Renesas RA6M5, RA4M1, and RA2E1 microcontrollers. Todo: * [X] Add ID codes and confirm probe support * [X] Implement flashing for MF4/MF3 flash on the RA2E1 (RV40 flash for the RA6M5 is already supported) * [ ] Handle RA4 MF3/4 flash gracefully (different block size) * [ ] Possibly support the OCDREG control registers hanging off of AP1 (APB-AP) ## Your checklist for this pull request * [X] I've read the [Code of Conduct](https://github.com/blackmagic-debug/blackmagic/blob/main/CODE_OF_CONDUCT.md) * [X] I've read the [guidelines for contributing](https://github.com/blackmagic-debug/blackmagic/blob/main/CONTRIBUTING.md) to this repository * [X] It builds for hardware native (see [Building the firmware](https://github.com/blackmagic-debug/blackmagic?tab=readme-ov-file#building-black-magic-debug-firmware)) * [X] It builds as BMDA (see [Building the BMDA](https://github.com/blackmagic-debug/blackmagic?tab=readme-ov-file#building-black-magic-debug-app)) * [ ] I've tested it to the best of my ability * [X] My commit messages provide a useful short description of what the commits do
desertsagebrush commented 2025年11月24日 21:38:05 +01:00 (Migrated from github.com)
Copy link

Still todo:

  • Handle the different block size for RA4 flash
  • Update comments/etc to include the new RA family members
  • Handle the OCDREG AP?
  • Test more thoroughly
  • Possibly split the flash routines into their own files?
  • Possibly split the families into two feature flags to reduce the size burden?
Still todo: * [ ] Handle the different block size for RA4 flash * [x] Update comments/etc to include the new RA family members * [ ] Handle the OCDREG AP? * [x] Test more thoroughly * [ ] Possibly split the flash routines into their own files? * [ ] Possibly split the families into two feature flags to reduce the size burden?
dragonmux left a comment
Copy link

Done a review - very happy with the general direction you're managing to take this, there are only a few fairly small details we spotted that could do with being cleaned up/sorted.

Done a review - very happy with the general direction you're managing to take this, there are only a few fairly small details we spotted that could do with being cleaned up/sorted.
@ -179,2 +238,4 @@
#define RENESAS_FIXED2_MCUVER UINT32_C(0x010081b0) /* MCU Version Register */
// R01UH1143EJ0110, Flash Register Descriptions, §29.3.24-26, pg 685
#define RENESAS_FIXED3_UID UINT32_C(0x01011070) /* Unique ID Register */

We're aware this is, on your part, a "following the style set before", however the parens here are not doing anything useful and can (should) be dropped please. Their presence doesn't follow any of the rest of the code base.

We're aware this is, on your part, a "following the style set before", however the parens here are not doing anything useful and can (should) be dropped please. Their presence doesn't follow any of the rest of the code base.
@ -580,0 +1077,4 @@
target_add_commands(target, renesas_cmd_list, target->driver);
return true;
/* These are the MRAM/SiP models */

Please include a note of where exactly that 5μs delay might be needed as this may become relevant in the future. Otherwise, sound logic and reasoning 👍🏼

Please include a note of where exactly that 5μs delay might be needed as this may become relevant in the future. Otherwise, sound logic and reasoning 👍🏼

'Flash not 'flash please; this is a serious enough thing that we might suggest making it DEBUG_ERROR or, if possible to get it back through GDB at this moment in the protocol, a gdb_outf() call so the user definitely gets to see this.

`'Flash` not `'flash` please; this is a serious enough thing that we might suggest making it DEBUG_ERROR or, if possible to get it back through GDB at this moment in the protocol, a `gdb_outf()` call so the user definitely gets to see this.

The const's in these casts do nothing and can (should) be dropped. Casting to uint16_t likewise has the same value effect as masking by 0xffffU so the mask can technically be dropped (though having that explicit is no bad thing).

The `const`'s in these casts do nothing and can (should) be dropped. Casting to `uint16_t` likewise has the same value effect as masking by `0xffffU` so the mask can technically be dropped (though having that explicit is no bad thing).

If it's possible to express this as a for loop over len, keeping len and addr constant, it would be better - the compiler's able to generate better code for this and prove to itself the correctness of the loop for optimisation purposes. Adding/tracking an offset turns out to be ~free due to instruction set things the compiler can take advantage of.

If it's possible to express this as a `for` loop over `len`, keeping `len` and `addr` constant, it would be better - the compiler's able to generate better code for this and prove to itself the correctness of the loop for optimisation purposes. Adding/tracking an offset turns out to be ~free due to instruction set things the compiler can take advantage of.

Same query here, especially as src could be cast to a buffer pointer so you only have one cast to uint8_t to do and then everything else already uses the right pointer width for the remainder

Same query here, especially as `src` could be cast to a buffer pointer so you only have one cast to `uint8_t` to do and then everything else already uses the right pointer width for the remainder

((const uint16_t *)src)[0]/((const uint16_t *)src)[1] would we think be clearer here what you're intending to do.

`((const uint16_t *)src)[0]`/`((const uint16_t *)src)[1]` would we think be clearer here what you're intending to do.

Having this duplicated is fine - often de-dupe attempts result in more complex/spaghetti code, so this is more than fine with us.

Having this duplicated is fine - often de-dupe attempts result in more complex/spaghetti code, so this is more than fine with us.
desertsagebrush (Migrated from github.com) reviewed 2025年11月26日 19:11:13 +01:00
@ -179,2 +238,4 @@
#define RENESAS_FIXED2_MCUVER UINT32_C(0x010081b0) /* MCU Version Register */
// R01UH1143EJ0110, Flash Register Descriptions, §29.3.24-26, pg 685
#define RENESAS_FIXED3_UID UINT32_C(0x01011070) /* Unique ID Register */
desertsagebrush (Migrated from github.com) commented 2025年11月26日 19:11:13 +01:00
Copy link

Sounds good! Would you like me to remove those from other places in this file while I'm at it?

Sounds good! Would you like me to remove those from other places in this file while I'm at it?
desertsagebrush (Migrated from github.com) reviewed 2025年11月26日 19:29:09 +01:00
@ -580,0 +1077,4 @@
target_add_commands(target, renesas_cmd_list, target->driver);
return true;
/* These are the MRAM/SiP models */
desertsagebrush (Migrated from github.com) commented 2025年11月26日 19:29:08 +01:00
Copy link

Sounds good; Would you like me to see about doing the same thing for the RV40 flash functions, which follow the same pattern?

Sounds good; Would you like me to see about doing the same thing for the RV40 flash functions, which follow the same pattern?
desertsagebrush commented 2025年11月26日 21:51:55 +01:00 (Migrated from github.com)
Copy link

Currently, it mostly works; for some reason, repeated scans of the same target may not properly identify the device and report it as an unknown device (but it does call the renesas_ra_probe method, so something is afoot in there, it seems). Also, sometimes the device seems to report that it has erased successfully but then throws a programming error every time it tries to program a previously written block. My guess is that there needs to be a delay post calling erase so that it can finish actually erasing even though the command is reporting that it finished properly, but I could be wrong.

Progress though!

Currently, it mostly works; for some reason, repeated scans of the same target may not properly identify the device and report it as an unknown device (but it does call the `renesas_ra_probe` method, so something is afoot in there, it seems). Also, sometimes the device seems to report that it has erased successfully but then throws a programming error every time it tries to program a previously written block. My guess is that there needs to be a delay post calling erase so that it can finish actually erasing even though the command is reporting that it finished properly, but I could be wrong. Progress though!
desertsagebrush (Migrated from github.com) reviewed 2025年11月26日 21:53:23 +01:00
@ -580,0 +1077,4 @@
target_add_commands(target, renesas_cmd_list, target->driver);
return true;
/* These are the MRAM/SiP models */
desertsagebrush (Migrated from github.com) commented 2025年11月26日 21:53:23 +01:00
Copy link

I ended up removing this because of other reasons, but definitely makes sense.

I ended up removing this because of other reasons, but definitely makes sense.
@ -179,2 +238,4 @@
#define RENESAS_FIXED2_MCUVER UINT32_C(0x010081b0) /* MCU Version Register */
// R01UH1143EJ0110, Flash Register Descriptions, §29.3.24-26, pg 685
#define RENESAS_FIXED3_UID UINT32_C(0x01011070) /* Unique ID Register */

Yes, please.

Yes, please.
@ -580,0 +1077,4 @@
target_add_commands(target, renesas_cmd_list, target->driver);
return true;
/* These are the MRAM/SiP models */

That would be good, yes 🙂

That would be good, yes 🙂
This pull request has changes conflicting with the target branch.
  • src/target/renesas_ra.c
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin desertsagebrush/feat/renesas_ra2e1:desertsagebrush/feat/renesas_ra2e1
git switch desertsagebrush/feat/renesas_ra2e1
Sign in to join this conversation.
No reviewers
Labels
Clear labels
BMD App
Black Magic Debug App (aka. PC hosted) (not firmware)
BMP Firmware
Black Magic Probe Firmware (not PC hosted software)
Bug
Confirmed bug
Build system
Build system
Can't reproduce
Maintainers can't reproduce this problem
CI
Continuous Integration System
Contribution wanted
User contributions welcome
Documentation
Project documentation
Draft
Work in progress draft
Duplicate
This issue or pull request already exists
Enhancement
General project improvement
Feedback wanted
Requires additional submitter feedback
Foreign Host Board
Non Native hardware to runing Black Magic firmware on
GDB
Issue/PR related to GDB
Good first issue
Good for newcommers
HwIssue Mitigation
Solving or mitigating a Hardware issue in Software
Information Needed
Maintainers need more information
NativeHardware
Official Black Magic Debug Hardware
New Host Board
New hardware to run Black Magic firmware on
New Target
New debug target
Off Topic
Something that does not involve the project in any way
Potential Bug
A potential, unconfirmed or very special circumstance bug
Regression
Bug caused by a regression
User Interest Needed
More user interest required before consideration
User Testing Needed
Looking for user testing reports
Won't fix
Outside of the project scope or works as intended
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
blackmagic-debug/blackmagic!2164
Reference in a new issue
blackmagic-debug/blackmagic
No description provided.
Delete branch "desertsagebrush/feat/renesas_ra2e1"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?