7
32
Fork
You've already forked blackmagic
23

Feature/blackpillv2: add alternative pinout #1467

Merged
lenvm merged 4 commits from feature/blackpillv2-add-alternative-pinout into main 2023年05月01日 01:00:49 +02:00
lenvm commented 2023年04月23日 20:47:27 +02:00 (Migrated from github.com)
Copy link

Detailed description

This pull request

  • implements function PINOUT_SWITCH() to switch between pinouts. In total 3 alternative pinouts can be defined.
  • adds one alternative pinout for the blackpillv2 platform. The pinout description is found in the README.md of the blackpillv2 platform, modified in this pull request. This pinout uses the pins near the bottom of the board. The alternative pinout is visualized in figure 1.
  • adds the macros to facilitate using either USART1 or USART2 as USBUSART.

The implementation of a function to select alternative pinouts was mentioned in this issue comment.

230974447-ec7b25cb-ee38-4424-a54b-d596888b1aa3
Figure 1: alternative pinout 1

Your checklist for this pull request

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

Closing issues

## Detailed description This pull request - implements function `PINOUT_SWITCH()` to switch between pinouts. In total 3 alternative pinouts can be defined. - adds one alternative pinout for the blackpillv2 platform. The pinout description is found in the README.md of the blackpillv2 platform, modified in this pull request. This pinout uses the pins near the bottom of the board. The alternative pinout is visualized in figure 1. - adds the macros to facilitate using either USART1 or USART2 as USBUSART. The implementation of a function to select alternative pinouts was mentioned in [this issue comment](https://github.com/blackmagic-debug/blackmagic/issues/1418#issuecomment-1507492647). ![230974447-ec7b25cb-ee38-4424-a54b-d596888b1aa3](https://user-images.githubusercontent.com/10454610/233858002-2fd216f7-4585-4437-b64f-d6636a941a12.png) Figure 1: alternative pinout 1 ## 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 (`make PROBE_HOST=native`) * [x] It builds as BMDA (`make PROBE_HOST=hosted`). * [x] It builds for hardware blackpillv2 (`make PROBE_HOST=blackpillv2`) * [x] It builds for hardware blackpillv2 with alternative pinout 1 (`make PROBE_HOST=blackpillv2 ALTERNATIVE_PINOUT=1`) * [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 <!-- put "fixes #XXXX" here to auto-close the issue(s) that your PR fixes (if any). -->
dragonmux left a comment
Copy link

This does complicate the platform and we do have some code changes we've noted during review, but given this solves a problem with the platform for you we're happy to merge this once the review items have been addressed.

This does complicate the platform and we do have some code changes we've noted during review, but given this solves a problem with the platform for you we're happy to merge this once the review items have been addressed.
@ -25,6 +25,10 @@ LDFLAGS += $(LDFLAGS_BOOT)
CFLAGS += -DDFU_SERIAL_LENGTH=13
endif

Given there's only one alternative pinout currently defined, and you're mapping $(ALTERNATIVE_PINOUT) to CFLAGS' -DALTERNATIVE_PINOUT= values 1:1, it seems to us to make sense to pass the value through directly as CFLAGS += -DALTERNATIVE_PINOUT=$(ALTERNATIVE_PINOUT)

Given there's only one alternative pinout currently defined, and you're mapping `$(ALTERNATIVE_PINOUT)` to `CFLAGS`' `-DALTERNATIVE_PINOUT=` values 1:1, it seems to us to make sense to pass the value through directly as `CFLAGS += -DALTERNATIVE_PINOUT=$(ALTERNATIVE_PINOUT)`
@ -4,23 +4,22 @@ Allows the use of a [BlackPill v2](https://github.com/WeActStudio/WeActStudio.Mi

This extra newline should be removed

This extra newline should be removed
@ -30,6 +29,14 @@ make clean
make PROBE_HOST=blackpillv2
```

This extra newline should be removed

This extra newline should be removed
@ -29,49 +29,80 @@

please add || ALTERNATIVE_PINOUT > 3, and drop the #else + #warning branch below - it doesn't make sense to check this condition twice in two different ways.

please add `|| ALTERNATIVE_PINOUT > 3`, and drop the `#else` + `#warning` branch below - it doesn't make sense to check this condition twice in two different ways.

This should be moved onto the line above

This should be moved onto the line above
lenvm (Migrated from github.com) reviewed 2023年04月30日 15:05:12 +02:00
@ -4,23 +4,22 @@ Allows the use of a [BlackPill v2](https://github.com/WeActStudio/WeActStudio.Mi
lenvm (Migrated from github.com) commented 2023年04月30日 15:05:12 +02:00
Copy link

removed

removed
lenvm (Migrated from github.com) reviewed 2023年04月30日 15:05:19 +02:00
@ -30,6 +29,14 @@ make clean
make PROBE_HOST=blackpillv2
```
lenvm (Migrated from github.com) commented 2023年04月30日 15:05:18 +02:00
Copy link

removed

removed
lenvm (Migrated from github.com) reviewed 2023年04月30日 15:07:44 +02:00
@ -25,6 +25,10 @@ LDFLAGS += $(LDFLAGS_BOOT)
CFLAGS += -DDFU_SERIAL_LENGTH=13
endif
lenvm (Migrated from github.com) commented 2023年04月30日 15:07:44 +02:00
Copy link

Updated in the way you suggested.

I have also added ifdef ALTERNATIVE_PINOUT such that the code can still be built without passing the ALTERNATIVE_PINOUT to make.
This was already possible using the implementation you commented on, but was not possible without adding the ifdef line.

Updated in the way you suggested. I have also added `ifdef ALTERNATIVE_PINOUT` such that the code can still be built without passing the `ALTERNATIVE_PINOUT` to make. This was already possible using the implementation you commented on, but was not possible without adding the `ifdef` line.
lenvm (Migrated from github.com) reviewed 2023年04月30日 15:07:59 +02:00
@ -29,49 +29,80 @@
lenvm (Migrated from github.com) commented 2023年04月30日 15:07:59 +02:00
Copy link

Implemented. Thanks!

Implemented. Thanks!
lenvm (Migrated from github.com) reviewed 2023年04月30日 15:08:17 +02:00
@ -29,49 +29,80 @@
lenvm (Migrated from github.com) commented 2023年04月30日 15:08:17 +02:00
Copy link

Implemented 👍

Implemented 👍
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!1467
Reference in a new issue
blackmagic-debug/blackmagic
No description provided.
Delete branch "feature/blackpillv2-add-alternative-pinout"

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?