7
31
Fork
You've already forked blackmagic
23

Add basic TMS570 support #2090

Open
xobs wants to merge 27 commits from xobs/feature/tms570-support into main
pull from: xobs/feature/tms570-support
merge into: blackmagic-debug:main
blackmagic-debug:main
blackmagic-debug:fix/pre-bmp-v3-cross-file-cleanup
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
xobs commented 2025年03月12日 04:12:40 +01:00 (Migrated from github.com)
Copy link

Detailed description

Add basic support for TI TMS570. This supports enumeration of the target, including configuring the ICEPICK-TYPE-C device present on the chip to enable enumeration.

This does not yet support flashing. Support for writing flash will need to come once the flash process is reverse engineered, or a flash stub will need to be created using official firmware.

Your checklist for this pull request

## Detailed description Add basic support for TI TMS570. This supports enumeration of the target, including configuring the ICEPICK-TYPE-C device present on the chip to enable enumeration. This does not yet support flashing. Support for writing flash will need to come once the flash process is reverse engineered, or a flash stub will need to be created using official firmware. ## 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)) * [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
dragonmux left a comment
Copy link

This looks to be progressing nicely, so we are giving you an interim review. Think our plan for merging this will be to split the AM335x support branch into the ICEPick part and then the AM335x part, and PR the ICEPick part for v2.1, that way this PR has a cleaner base to work from. If that sounds good to you, then we'll get the branches sorted out.

This looks to be progressing nicely, so we are giving you an interim review. Think our plan for merging this will be to split the AM335x support branch into the ICEPick part and then the AM335x part, and PR the ICEPick part for v2.1, that way this PR has a cleaner base to work from. If that sounds good to you, then we'll get the branches sorted out.

Extraneous new line.

Extraneous new line.

The braces here are superfluous and can be dropped.

The braces here are superfluous and can be dropped.

Please use a switch/case block set here as in the example in the function above.

Please use a `switch`/`case` block set here as in the example in the function above.
@ -1163,3 +1221,3 @@
adiv5_dp_write(priv->base.ap->dp, ADIV5_AP_DB(CORTEXAR_BANKED_DTRTX), src[offset]);
cortexar_endian_dp_write(target, ADIV5_AP_DB(CORTEXAR_BANKED_DTRTX), src[offset]);
/* Now read out the status from the DCSR in case anything went wrong */
const uint32_t status = adiv5_dp_read(priv->base.ap->dp, ADIV5_AP_DB(CORTEXAR_BANKED_DCSR));

The braces here are superfluous and can be dropped. We would be surprised if src needs casing at all, it should automatically cast to void * and get const added as needed (removal of const requires casts, not addition).

Please suffix 4 with U to make the multiplication unsigned.

The braces here are superfluous and can be dropped. We would be surprised if `src` needs casing at all, it should automatically cast to `void *` and get `const` added as needed (removal of const requires casts, not addition). Please suffix `4` with `U` to make the multiplication unsigned.
@ -1325,6 +1427,7 @@ static size_t cortexar_reg_width(const size_t reg)
static size_t cortexar_reg_read(target_s *const target, const uint32_t reg, void *const data, const size_t max)

Should this not be write_le8()?

Should this not be `write_le8()`?
xobs (Migrated from github.com) reviewed 2025年04月03日 03:14:06 +02:00
@ -1163,3 +1221,3 @@
adiv5_dp_write(priv->base.ap->dp, ADIV5_AP_DB(CORTEXAR_BANKED_DTRTX), src[offset]);
cortexar_endian_dp_write(target, ADIV5_AP_DB(CORTEXAR_BANKED_DTRTX), src[offset]);
/* Now read out the status from the DCSR in case anything went wrong */
const uint32_t status = adiv5_dp_read(priv->base.ap->dp, ADIV5_AP_DB(CORTEXAR_BANKED_DCSR));
xobs (Migrated from github.com) commented 2025年04月03日 03:14:06 +02:00
Copy link

The cast is required because the read_* functions take a const uint8_t *const and not void *:

../src/target/cortexar.c:1229:21: warning: incompatible pointer types passing 'const uint32_t *const' (aka 'const unsigned int *const') to parameter of type 'const uint8_t *' (aka 'const unsigned char *') [-Wincompatible-pointer-types]
 1229 | value = read_be4(src, offset * 4);
 | ^~~
../src/include/buffer_utils.h:97:54: note: passing argument to parameter 'buffer' here
 97 | static inline uint32_t read_be4(const uint8_t *const buffer, const size_t offset)
 | ^
The cast is required because the `read_*` functions take a `const uint8_t *const` and not `void *`: ``` ../src/target/cortexar.c:1229:21: warning: incompatible pointer types passing 'const uint32_t *const' (aka 'const unsigned int *const') to parameter of type 'const uint8_t *' (aka 'const unsigned char *') [-Wincompatible-pointer-types] 1229 | value = read_be4(src, offset * 4); | ^~~ ../src/include/buffer_utils.h:97:54: note: passing argument to parameter 'buffer' here 97 | static inline uint32_t read_be4(const uint8_t *const buffer, const size_t offset) | ^ ```
xobs (Migrated from github.com) reviewed 2025年04月03日 03:15:17 +02:00
@ -1325,6 +1427,7 @@ static size_t cortexar_reg_width(const size_t reg)
static size_t cortexar_reg_read(target_s *const target, const uint32_t reg, void *const data, const size_t max)
xobs (Migrated from github.com) commented 2025年04月03日 03:15:17 +02:00
Copy link

It should be.

It should be.
@ -1163,3 +1221,3 @@
adiv5_dp_write(priv->base.ap->dp, ADIV5_AP_DB(CORTEXAR_BANKED_DTRTX), src[offset]);
cortexar_endian_dp_write(target, ADIV5_AP_DB(CORTEXAR_BANKED_DTRTX), src[offset]);
/* Now read out the status from the DCSR in case anything went wrong */
const uint32_t status = adiv5_dp_read(priv->base.ap->dp, ADIV5_AP_DB(CORTEXAR_BANKED_DCSR));

In that case, you can simplify the cast to just (const void *) - the compiler will add the final const for us.

In that case, you can simplify the cast to just `(const void *)` - the compiler will add the final `const` for us.
This pull request has changes conflicting with the target branch.
  • src/platforms/hosted/dap_jtag.c
  • src/target/icepick.c
  • src/target/jtag_devs.c
  • src/target/jtag_scan.c
  • src/target/meson.build
  • src/target/samx5x.c
  • src/target/target_probe.c
  • src/target/target_probe.h
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 xobs/feature/tms570-support:xobs/feature/tms570-support
git switch xobs/feature/tms570-support
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!2090
Reference in a new issue
blackmagic-debug/blackmagic
No description provided.
Delete branch "xobs/feature/tms570-support"

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?