-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
stm32/WEACT_F411_BLACKPILL: Add WeAct F411 'blackpill' board. #15646
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.
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
Conversation
776a0ff
to
686a415
Compare
Code size report:
|
Looks good @andrewleech ! Hey @mcauser, you've worked with these boards a bit haven't you? Would you be able to review? |
I have a F411 board here, sold as Weact Studio F411 Blackpill V3.1. There is one difference in the wiring: for my V3.1 board SPI_MISO is at PA6, not PB4 (and the flash size is 8MB). Otherwise the PR works. |
Thanks @robert-hh that sounds like something that would work as another variant! |
That requires a little bit more awkward approach, since the changes have start in pins.csv and reflected then in a variant-specific mpconfigboard-VARIANT.mk. Mayb by defining two variants of the MISO pin in pins,csv and selecting the right one later. |
686a415
to
c861b3e
Compare
Thanks. These new definitions seem to work, with two minor hiccups.
|
c861b3e
to
7239f4f
Compare
Thanks @robert-hh good catch and suggestions, I've updated as such. |
7239f4f
to
0255d31
Compare
"url": "https://github.com/WeActStudio/WeActStudio.MiniSTM32F4x1", | ||
"variants": { | ||
"V2_FLASH_4MB": "v2 board with 4MB SPI Flash", | ||
"V3_FLASH_8MB": "v3.1 board with 8MB SPI Flash" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other board variants don't have the trailing "B", so I suggest V2_FLASH_4M
and V3_FLASH_8M
as variant names.
extern const struct _mp_spiflash_config_t spiflash_config; | ||
extern struct _spi_bdev_t spi_bdev; | ||
#define MICROPY_HW_SPIFLASH_ENABLE_CACHE (1) | ||
#define MICROPY_HW_BDEV_IOCTL(op, arg) ( \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The below config is now made common in mpconfigboard_common.h
, all you need to do is define MICROPY_HW_BDEV_SPIFLASH
as &spi_bdev
, and also MICROPY_HW_BDEV_SPIFLASH_CONFIG
and MICROPY_HW_BDEV_SPIFLASH_SIZE_BYTES
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I missed that consolidating, nice one thanks!
FLASH_SCK,PA5 | ||
FLASH_MOSI,PA7 | ||
FLASH_MISO_V2,PB4 | ||
FLASH_MISO_V3,PA6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these flash pins intended to be exposed via board name to the user? If not then you can do -FLASH_CS,PA4
(for example); it's still exposed as pyb_pin_FLASH_CS
but the user won't see Pin.board.FLASH_CS
.
I don't mind either way (exposing to user or not).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a few pins here that probably don't really add value for a user, I'd forgotten about the - in this file thanks for the reminder.
#define MICROPY_HW_SPIFLASH_CS (pyb_pin_FLASH_CS) | ||
#define MICROPY_HW_SPIFLASH_SCK (pyb_pin_FLASH_SCK) | ||
#define MICROPY_HW_SPIFLASH_MOSI (pyb_pin_FLASH_MOSI) | ||
#ifdef WEACT_F411_V3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe #if WEACT_F411_V3
?
@andrewleech I built the firmware using your mpconfigboard.h from:
Is there something else I need to do to get the 8MB? I'd be keen to build for the 8MB flash to see if it solves a comms issue for me. |
0255d31
to
1548929
Compare
I've restructured the board and variant configuration a bit in the latest push. v3.1 is now the default as it's what people will get if they buy them new currently, no flash configured. After that there's a variant for v2.0 with 4MB flash - the only difference between v2.0 and v3.1 boards is the pinout of the spiflash I believe so users with v2.0 (no flash) will also be able to use the default build. I then found the boards I own are actually v1.3 which were the original version released a few years ago. On these the spiflash pinout matches v3.1 but does not have a user switch I noticed that on v1.3 and v3.1 boards the spiflash pinout aligns with the spi hardware peripheral so that's now being used. I don't know why they changed it in v2.0, regardless it's configured with softspi. @davefes I'm not quite following if you're trying to copy some of this PR into a different micropython source? This whole PR is the board folder which uses the variant build option to select the alternate configurations. The readme in the stm port folder should give guidance on how this works iirc.
If anyone can I'd appreciate a test on v2.0 or v3.1 boards! |
I tested the V3.1, 25MHz variant with and without flash. It builds and runs. Next week I will get a 8MHz V3.1 board which I then will test. Looking at the config files I would have expected a value of 8 for the 8MHz version at |
After the following change:
it built and runs the base-version. I don't see a readme in the /stm32 folder. How do I change: On the 2nd attempt I had to return to the original: |
PB14,PB14 | ||
PB15,PB15 | ||
PC14,PC14 | ||
PC15,PC15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#include "boards/stm32f4xx_hal_conf_base.h" | ||
|
||
// Oscillator values in Hz | ||
#if MICROPY_HW_HSE_VALUE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case I think it's better to do #ifdef MICROPY_HW_HSE_VALUE
, because if that value is defined it should be used (even if it's 0...).
1548929
to
203a2a9
Compare
Should I be able to make a LFS2 filesystem?
The base version works on Still can't work-out how to build for 8MB. |
Looking at the order confirmations, I did not order a 8MHz version of the board, like I thought I did. Shhhh.... But looking into the F411 data sheet, it is obvious that for the 8MHz version the value of |
.mosi = MICROPY_HW_SPIFLASH_MOSI, | ||
.miso = MICROPY_HW_SPIFLASH_MISO, | ||
}; | ||
#define |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stray line
"V13_FLASH_4M": "v1.3 board with 4MB SPI Flash", | ||
"V20_FLASH_4M": "v2.0 board with 4MB SPI Flash", | ||
"V31_FLASH_8M": "v3.1 board with 8MB SPI Flash", | ||
"V31_XTAL_8M": "v3.1 board with 8MHz crystal" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be clear, this 8MHz crystal version doesn't have SPI flash?
How do people distinguish v3.1 with the different crystals?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can only suggest the way nspsck does here:
https://github.com/nspsck/STM32F411CEU6_BlackPill_Micropython/blob/main/README.md
For 8MHz crystal and 16MB spi flash:
make -j BOARD=WEACTF411CE BOARD_VARIANT=VARIANTS CRYSTAL_FREQ=8 SPI_FLASH_SIZE=16
For 25MHz crystal and 16MB spi flash, you have to leave CRYSTAL_FREQ out:
make -j BOARD=WEACTF411CE BOARD_VARIANT=VARIANTS SPI_FLASH_SIZE=16
maybe ignore BOARD_VARIANT depending on how Andrew builds these variants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far there is only one variant with 8MHz xtal ... no external flash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To build a variant with this PR use:
make BOARD=WEACT_F411_BLACKPILL BOARD_VARIANT=V31_XTAL_8M
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I need to re-load any of the build files since 203a2a9
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably need to re-pull this PR. But some of the comments here need addressing before it will build correctly for all variants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, I'll wait.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be clear, this 8MHz crystal version doesn't have SPI flash?
According to the store pages: No. But the footprint for the flash chip is present, so people could add a flash chip. But then they have to adapt the build files and build the firmware themselves.
How do people distinguish v3.1 with the different crystals?
They have to look at the crystal.
I re-ordered a 8MHz crystal board and will get it next week. Then I can check whether the configuration works. ATM it does not look right, since MICROPY_HW_CLK_PLLM
is defined unconditionally as 25 for all boards. But maybe that definition is not effective.
|
||
#if !MICROPY_HW_ENABLE_INTERNAL_FLASH_STORAGE | ||
|
||
#if WEACT_F411_V2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be WEACT_F411_V20
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof, that's what I get for changing my mind on the formatting of these version defines, thanks for catching it.
#define MICROPY_HW_SPIFLASH_MOSI (pyb_pin_FLASH_MOSI) | ||
#if WEACT_F411_V13 | ||
#define MICROPY_HW_SPIFLASH_MISO (pyb_pin_FLASH_MISO_V13) | ||
#elif WEACT_F411_V2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be WEACT_F411_V20
.
@@ -0,0 +1,3 @@ | |||
CFLAGS += \ | |||
-DWEACT_F411_V13=1 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a makefile, so shouldn't there technically be tab indenting here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, yeah technically it doesn't matter after a line-continuation \
however by convention it is better to use a tab for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I did not realise that when lines are continued they don't need tabs.
But still good to use tabs for consistency.
@@ -0,0 +1 @@ | |||
CFLAGS += -DMICROPY_HW_SPIFLASH_SIZE_BYTES="(8 * 1024 * 1024)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably worth adding -DWEACT_F411_V31=1
for consistency.
@@ -0,0 +1 @@ | |||
CFLAGS += -DMICROPY_HW_HSE_VALUE="(8000000)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add -DWEACT_F411_V31=1
?
362d927
to
7e97cdd
Compare
Note in the latest push I've changed the way the make args are handled to simplify end user building for customised boards, but it's not quite right yet. Don't bother reviewing too much, I'll let everyone here know when it's working. |
Built for 8MB flash, made a LFS2 filesystem and the application runs fine. Still, when running:
I have had difficulties getting to see the 8MB flash before so will try my new boards next week. |
@davefes
The output is somewhat confusing https://docs.python.org/3/library/os.html#os.statvfs |
O dear, thanks for the correction.
8MB Great! |
I only have 2 boards, at the moment. One board without flash and using the base-version and VFAT works, when I change to LFS2 I get the dreaded timeout error on file transfer. The other board (8MB flash) works properly. Suspect a faulty board. |
@andrewleech The 8MHz V3.1 board arrived. The firmware builds and works. USB registers, sleep functions show a reasonable behavior. So I conclude that the clocks are set up properly. |
5 out of 5 new 8MB 25MHz boards work. LFS2 as well. Great work. Thanks for all the effort you put into this. |
ifdef XTAL_FREQ_MHZ | ||
# Add xtal speed to build folder name if set from command line | ||
BUILD := $(BUILD)_XTAL_$(XTAL_FREQ_MHZ)M | ||
endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why this and the above bit that changes BUILD
are needed? The variant name is appended automatically to the board name when doing the build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of the makefile ordering this isn't used / run when a variant is configured - this is just for when one of these settings is passed on the make
command line. I thought this would be most helpful for people manually configuring the FLASH size from command line, it will put the flash size in the build folder name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could I suggest a warning in the README.md to not use A4, A5, A6 or A7 on the 8MB variant. I was vaguely aware that those pins were used for the SPI flash, but did not appreciate the consequences of "accidentally" using them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a note to the readme that's a good suggestion; it's not immediately obvious that the pins needed for "on-board" functions are also exposed to the end user.
f214cc7
to
aecdfc1
Compare
@dpgeorge I've finished simplifying the pll generation script args and addressed the other comments to date thanks. |
I have verified that with this PR:
|
@@ -137,8 +137,11 @@ def search_header(filename, re_include, re_define, lookup, val): | |||
m = regex_define.match(line) | |||
if m: | |||
# found lookup value | |||
found = m.group(3) | |||
if "*" in found or "/" in found: | |||
found = eval(found) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you find cases where this arithmetic was needed? Can you give an example of a board that needs it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic is to support usage like in this PR for
#define HSE_VALUE (MICROPY_HW_HSE_SPEED_MHZ * 1000000) |
Where the clock speed define is reused in two locations, one location needing it is Hz the other needing it in MHz
#define MICROPY_HW_CLK_PLLM (MICROPY_HW_HSE_SPEED_MHZ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thanks for that. We should keep this bit of Python logic then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks but yes I'll add a comment to the section to clarify its purpose
ports/stm32/boards/plli2svalues.py
Outdated
pllm = int(argv[0][len("pllm:") :]) | ||
|
||
if argv[0].startswith("file:"): | ||
# extract HSE_VALUE and from header file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"extract HSE_VALUE from header file"
r'#include "(boards/[A-Za-z0-9_./]+)"', | ||
r"#define +(HSE_VALUE) +\((\(uint32_t\))?([0-9]+)\)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this include regex and its associated logic in search_header
is needed anymore. That simplifies things, and the vals
argument is then also not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes I follow what you mean; the recursive search really isn't needed at all now that it's parsing the pre-processed / merged headers. Stripping that out certainly simplifies the logic!
@@ -230,7 +230,9 @@ def print_table(hse, valid_plls): | |||
|
|||
def search_header_for_hsx_values(filename, vals): | |||
regex_inc = re.compile(r'#include "(boards/[A-Za-z0-9_./]+)"') | |||
regex_def = re.compile(r"#define +(HSE_VALUE|HSI_VALUE) +\((\(uint32_t\))?([0-9]+)\)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex_inc is no longer needed, neither the vals
argument here.
Will this distro end up on the Micropython Downloads page? |
I need to fix the previous few code comments to get it into a mergeable state. |
da203f2
to
2b7cd31
Compare
@dpgeorge I've updated the scripts, thanks for the suggestions! |
Adds board profile for the WeAct F411 'blackpill' which is a quite popular low cost ST dev board. This board also has optional spiflash so can be purchased in a few different configurations. Builds for v3.1 with no SPI Flash by default. Includes variants for different board versions and spi flash sizes. Signed-off-by: Andrew Leech <andrew@alelec.net>
Allows boards to configure their HSE and PLL values in variants. Signed-off-by: Andrew Leech <andrew@alelec.net>
2b7cd31
to
7924b31
Compare
Thanks for updating. Looks good now. |
@andrewleech unfortunately this board does not build: ValueError: file:build-WEACT_F411_BLACKPILL/genhdr/qstr.i.last does not contain a definition of micropy_hw_hse_value It looks like the regex needs to be updated to allow parenthesis within the expression for the value. Eg it can look like this: static uint32_t __attribute__((unused)) micropy_hw_hse_value = ((25) * 1000000); A regex like this is simple and seems to work (the r"static.* +(micropy_hw_hs[ei]_value) = +([0-9 +-/\*()]+);" |
I posted a fix for the above in #16444. |
Oh sorry and thanks, my final cleanup of the board configs was more hassle than I expected! |
Summary
Adds board profile for the WeAct F411 v2.0 'blackpill' which is a quite popular low cost stm dev board.
This board also has optional spiflash so can be purchased in a few different configurations. I've added a variant for 4MB flash.
Existing board profiles for F411 don't work on this board as it's built with a 25Mhz crystal.
This was built on top of #5642 refactored and updated for current compatibility.
Testing
I've used builds from this on two BlackPill boards I own, one with loaded flash, one without