8
33
Fork
You've already forked blackmagic
23

Feature: Add support for Arterytek MCU AT32F405, F423 #1949

Merged
ALTracer merged 6 commits from feature/at32f405 into main 2024年10月26日 17:07:51 +02:00
ALTracer commented 2024年10月02日 21:32:20 +02:00 (Migrated from github.com)
Copy link

Detailed description

  • This could be described as a new feature (new target support in existing driver)
  • The existing problem is AT32F405 not identified by BMD but likely supported via AT32F435 API.
  • This PR solves it by adding required match code in at32f43_probe.

I do not have any chips accessible for that target and cannot test live, only read refmans. FPEC is at 0x40023C00, which is promising. USD and DBGMCU support not considered yet.
The generic F_SIZE method was used to not carry yet another table of Part IDs.

Your checklist for this pull request

Closing issues

## Detailed description * This could be described as a new feature (new target support in existing driver) * The existing problem is AT32F405 not identified by BMD but likely supported via AT32F435 API. * This PR solves it by adding required match code in `at32f43_probe`. I do not have any chips accessible for that target and cannot test live, only read refmans. FPEC is at 0x40023C00, which is promising. USD and DBGMCU support not considered yet. The generic F_SIZE method was used to not carry yet another table of Part IDs. ## 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 ## Closing issues

This looks like it would be better suited going in the probe routine after all the read-out of IDs is done but before the first dispatch to a detect routine - probably have it print out both idcode and project_id?

This looks like it would be better suited going in the probe routine after all the read-out of IDs is done but before the first dispatch to a detect routine - probably have it print out both `idcode` and `project_id`?
ALTracer (Migrated from github.com) reviewed 2024年10月03日 21:38:36 +02:00
ALTracer (Migrated from github.com) commented 2024年10月03日 21:38:36 +02:00
Copy link

Moved to common probe.

Moved to common probe.
ALTracer commented 2024年10月03日 21:40:49 +02:00 (Migrated from github.com)
Copy link

Enhanced support detail for AT32F405. Added nominal support for AT32F423 as well. Added doc references. Not tested.

Enhanced support detail for AT32F405. Added nominal support for AT32F423 as well. Added doc references. Not tested.
ALTracer commented 2024年10月11日 20:43:45 +02:00 (Migrated from github.com)
Copy link

Tested to detect an AT32F437ZM device even under read protection level 1. That renders not just internal flash but also UID, F_SIZE, and option bytes, readable as 0xFF, but DEBUG_SER_ID can still be used on these three families for identification.
Actually entire DBGMCU support is identical for implemented features.

Tested to detect an AT32F437ZM device even under read protection level 1. That renders not just internal flash but also UID, F_SIZE, and option bytes, readable as 0xFF, but DEBUG_SER_ID can still be used on these three families for identification. Actually entire DBGMCU support is identical for implemented features.
@ -300,9 +374,33 @@ bool at32f43x_probe(target_s *target)
const uint32_t idcode = target_mem32_read32(target, AT32F43x_DBGMCU_IDCODE);

If debug_ser_id is always readable, does it not make more sense (for simpler code) to just always use its value rather than uid_byte? - still report the UID value in the DEBUG_TARGET statement below just in case this assumption that they're always the same fails, but until that is ever found to be the case, just use it anyway so there's less logic and so Flash usage.

If `debug_ser_id` is always readable, does it not make more sense (for simpler code) to just always use its value rather than `uid_byte`? - still report the UID value in the DEBUG_TARGET statement below just in case this assumption that they're always the same fails, but until that is ever found to be the case, just use it anyway so there's less logic and so Flash usage.
ALTracer (Migrated from github.com) reviewed 2024年10月14日 20:25:33 +02:00
@ -300,9 +374,33 @@ bool at32f43x_probe(target_s *target)
const uint32_t idcode = target_mem32_read32(target, AT32F43x_DBGMCU_IDCODE);
ALTracer (Migrated from github.com) commented 2024年10月14日 20:25:33 +02:00
Copy link

No, it is not always present and readable, I have only confirmed it on the one F437 part I tested live on. DEBUG->SER_ID is absent from other Mainstream/Value Line/High Performance lineups, which I intended to migrate away from stm32f1.c into here once this driver learns to use FPEC_BASE, USD_BASE and size, etc.
However, speaking strictly per refmans, you're right that it should work for the three declared families like this, and I can drop the all-Arterytek-generic UID byte read for now (hide behind DEBUG_TARGET) in favor of 0xe0042020 which, when present, is guaranteed to be non-0xFF. I only really trust powered and scanned silicon (what if errata) but it's not on me to maintain this code. It can be re-added in the next refactor with scope widening. Initially here I only wanted F405 in tree.

No, it is not always present and readable, I have only confirmed it on the one F437 part I tested live on. `DEBUG->SER_ID` is absent from other Mainstream/Value Line/High Performance lineups, which I intended to migrate away from stm32f1.c into here once this driver learns to use FPEC_BASE, USD_BASE and size, etc. However, speaking strictly per refmans, you're right that it should work for the three declared families like this, and I can drop the all-Arterytek-generic UID byte read for now (hide behind DEBUG_TARGET) in favor of 0xe0042020 which, when present, is guaranteed to be non-0xFF. I only really trust powered and scanned silicon (what if errata) but it's not on me to maintain this code. It can be re-added in the next refactor with scope widening. Initially here I only wanted F405 in tree.
dragonmux left a comment
Copy link

The patchset looks better - the comments are a lot clearer, thank you. With the only note we found this time taken care of, we'll get this merged.

The patchset looks better - the comments are a lot clearer, thank you. With the only note we found this time taken care of, we'll get this merged.
@ -289,7 +304,66 @@ static bool at32f43_detect(target_s *target, const uint16_t part_id)
return true;

The parens in this ternary can be dropped, and same for the matching one below in the next detect function. If you really want to keep them, they'd go to just before the closing ;, but they are superfluous.

The parens in this ternary can be dropped, and same for the matching one below in the next detect function. If you really want to keep them, they'd go to just before the closing `;`, but they are superfluous.
ALTracer (Migrated from github.com) reviewed 2024年10月26日 14:47:48 +02:00
@ -289,7 +304,66 @@ static bool at32f43_detect(target_s *target, const uint16_t part_id)
return true;
ALTracer (Migrated from github.com) commented 2024年10月26日 14:47:48 +02:00
Copy link

Dropped parens in two ternaries.

Dropped parens in two ternaries.
dragonmux left a comment
Copy link

LGTM, merging. Thank you for the contribution!

LGTM, merging. Thank you for the contribution!
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!1949
Reference in a new issue
blackmagic-debug/blackmagic
No description provided.
Delete branch "feature/at32f405"

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?