Skip to content

blackpill v2 DFU fixes #1152

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.

Sign up for GitHub

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

Open
bm16ton wants to merge 1 commit into
base: main
Choose a base branch
from
Open

blackpill v2 DFU fixes #1152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/platforms/blackpillv2/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,11 @@ void platform_init(void)
rcc_periph_clock_enable(RCC_GPIOA);
rcc_periph_clock_enable(RCC_GPIOC);
rcc_periph_clock_enable(RCC_GPIOB);

gpio_mode_setup(GPIOA, GPIO_MODE_INPUT,
GPIO_PUPD_PULLUP, GPIO0);
/* Check the USER button*/
if (gpio_get(GPIOA, GPIO0) ||
/*blackpillv2 userkey is active low but wo the pullup acts crazy and seemed..sorta active high*/
if (!gpio_get(GPIOA, GPIO0) ||
((magic[0] == BOOTMAGIC0) && (magic[1] == BOOTMAGIC1)))
{
magic[0] = 0;
Expand All @@ -74,8 +76,9 @@ void platform_init(void)
rcc_periph_clock_enable(RCC_CRC);

/* Set up USB Pins and alternate function*/
gpio_mode_setup(GPIOA, GPIO_MODE_AF, GPIO_PUPD_NONE, GPIO9 | GPIO11 | GPIO12);
gpio_set_af(GPIOA, GPIO_AF10, GPIO9 | GPIO10 | GPIO11 | GPIO12);
/*not using PA9 for vbus on blackpillv2 so leavem alone */
gpio_mode_setup(GPIOA, GPIO_MODE_AF, GPIO_PUPD_NONE, GPIO11 | GPIO12);
gpio_set_af(GPIOA, GPIO_AF10, GPIO11 | GPIO12);

GPIOA_OSPEEDR &= 0x3C00000C;
GPIOA_OSPEEDR |= 0x28000008;
Expand Down Expand Up @@ -105,6 +108,8 @@ void platform_init(void)
GPIO_PUPD_NONE,
PWR_BR_PIN);
#endif
/* for dfu-util to boot directly into the newly flashed app*/
Copy link
Member

@dragonmux dragonmux Sep 7, 2023

Choose a reason for hiding this comment

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

Spaces vs tabs - this needs its indentation converting

SCB_VTOR = (uint32_t) 0x08000000;

platform_timing_init();
usbuart_init();
Expand All @@ -128,6 +133,8 @@ void platform_request_boot(void)
uint32_t *magic = (uint32_t *)&_ebss;
magic[0] = BOOTMAGIC0;
magic[1] = BOOTMAGIC1;
/* Reset core to enter bootloader */
Copy link
Member

@dragonmux dragonmux Sep 7, 2023

Choose a reason for hiding this comment

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

We would prefer the blank line to be above this comment, not below

Copy link
Author

@bm16ton bm16ton Sep 7, 2023

Choose a reason for hiding this comment

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

Will do better next time! Everything has changed a lot since this pr was sent. Ill hafta check if adc support has been brought in for various f4 series etc and a few other areas I had added and if anything is missing that I have would a pr attempt for such things be welcome?

Copy link
Member

@dragonmux dragonmux Sep 7, 2023

Choose a reason for hiding this comment

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

You would be welcome to update this PR in that case to reduce the work on both of us


scb_reset_system();
}

Expand Down
3 changes: 2 additions & 1 deletion src/platforms/common/cdcacm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,8 @@ static const char *usb_strings[] = {
serial_no,
"Black Magic GDB Server",
"Black Magic UART Port",
"Black Magic DFU",
/* Required, the line for stm32f1s is "@Internal Flash /0x08000000/8*001Ka,56*001Kg" maybe use an if statement? */
"@Internal Flash /0x08000000/04*016Kg,01*064Kg,03*128Kg",
Copy link
Member

@dragonmux dragonmux Sep 7, 2023

Choose a reason for hiding this comment

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

Having further reviewed all this code, we can with confidence say that this patch is not required and that if anything was, it would go in src/platforms/common/stm32/dfucore.c rather than here as the strings apply to DFU mode not application mode and so apply to the bootloader and not the main firmware:

blackmagic/src/platforms/common/stm32/dfucore.c

Lines 36 to 45 in 8e661a6

#elif defined(STM32F4) || defined(STM32F7)
#define DFU_IFACE_PAGESIZE 128
#if APP_START == 0x08020000
#define DFU_IFACE_STRING_OFFSET 62
#define DFU_IFACE_STRING "@Internal Flash /0x08000000/1*016Ka,3*016Ka,1*064Ka,1*128Kg,002*128Kg"
#elif APP_START == 0x08004000
#define DFU_IFACE_STRING_OFFSET 54
#define DFU_IFACE_STRING "@Internal Flash /0x08000000/1*016Ka,3*016Kg,1*064Kg,000*128Kg"
#endif
#endif

These definitions do look like they need a review though so you would be most welcome to figure out what's going on there and suggest any required patches in this PR

Copy link
Author

@bm16ton bm16ton Sep 7, 2023

Choose a reason for hiding this comment

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

ha my god my memory is bad it was so long ago. The idea which I probly never said so my bad, was to use the internal dfu bootloader on models that support it. Using the stm dfu this would be required for the switching back and forth to work correctly. The usb descriptors for dfu would need to be set this way in the main firmware so upon reset into dfu mode dfu-util would know it was same dfu device. My idea was the in house dfu firmware wasn't needed, and using the internal dfu would save space. But using the inhouse dfu became the decided direction (can't blame yeah, tho I am curious for the reason, just so I can use that knowledge to become a better programmer/contributer)

#if defined(PLATFORM_HAS_TRACESWO)
"Black Magic Trace Capture",
#endif
Expand Down