-
Notifications
You must be signed in to change notification settings - Fork 178
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
In Review: Support WeAct mini stm32h7 #144
Conversation
thank you for your effort along with the pending list. For external flash partition make sure you follow the circuitpython stm32 partition scheme https://github.com/adafruit/circuitpython/tree/main/ports/stm/boards . It will require qspi driver to flash external flash, a new port may take quite a bit of time. I will try to help whenever I could. Though currently I don't have this board and it is hard to get anything delivered to my location due to strict lockdown. |
I would appreciate any help I can get.
Looking at the circuitpython repo, I am still quite unsure of what the stm32 partition scheme is and how it would affect tinyuf2. I'm going to try and get the QSPI and SPI memories working first and then continue with the backlog. Ideally I would want all of the memory devices to be available for flash download. (internal + SPI + QSPI) |
1e488dd
to
66f7640
Compare
I have added SPI and QSPI W25Qx drivers from the MiniSTM32H7xx repository. Still in early stages but you can somewhat load code into the QSPI memory and jump to it. I tried loading a simply blinky program to the QSPI flash. The interrupts seem to break after jumping to the QSPI code.
The initial 64KB is for tinyuf2. Programming the PFLASH is still pending since it would require running off of RAM and I'm struggling with getting the board to load code into RAM. |
yeah, I think your command is correct, you could try to enable the logging and print out the received address and contents and compare against the bin file to see if it matches. Just fake the actual flashing for testing. |
Thanks! I finally figured out why the interrupts were not working. The default The bootloader works for QSPI flash now, if you compile your application with VECT_TAB_SRAM :) |
I have most things working and implemented now, but there are a few things to test (writing data to SPI flash, self-update and uf2 read). I could finish the remaining work along with the review. The flash driver code isn't very pretty but it works, so I will leave it as is till I can find a portable working replacement. Also the port doesn't handle PFLASH right now (since my board only has the single sector, single bank variant) |
I know there hasn't been any movement out here for 2 years, but i recently bought this board and would like having uf2 on it.
Waiting for a STLink to get delivered, can't provide further info for now |
I am not entirely sure as well, PR is still marked as WIP and I kind of forgot about this. @bhav97 do you think this is ready for review and/or merging ? |
@hathach , @elpekenin As for the implementation, all the board_* callouts are available. I believe we can start a review now. In general I have 2 concerns for this PR
I believe stm32dfu doesn't support programming external memory so its possible that tinyuf2 was overwritten. (The MCU variant on the board only has a single flash sector of 128K so its not really possible to program the "internal" flash without overwriting tinyuf2. ) |
Sure, but i mean it doesnt work after flashing the tinyuf2.bin back at it. Which doesnt make any sense in my potato brain |
af609f0
to
55d76f7
Compare
That's not good. Does the blue LED stay on? Does double tap bring up the display? |
Can double check later but Led blinked on both events (plugging wire and pressing button), screen did nothing (not even backlight i think) Nothing in double press of either button |
Went to the mk file to add |
ports/stm32h7/port.mk
Outdated
|
||
# Compiler flags | ||
CFLAGS += \ | ||
-flto \ |
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 LTO is what "broke" code for me (probably gcc-version dependant issue if it worked for you...), do we really need to optimize tinyuf2 code if we aren't using the built-in flash?
Perhaps individual boards using H7xx could opt-in this config, instead of "forcing" its use having it at the MCU level
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.
That's a good idea, I copied the F4-mkfiles for this port, so there maybe a few more gotchas.
what version of GCC are you compiling with?
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.
10.3.1 (from ubuntu on WSL2 if that matters)
elpekenin@elPeCenin:~$ arm-none-eabi-gcc --version
arm-none-eabi-gcc (15:10.3-2021.07-4) 10.3.1 20210621 (release)
I cant get the flashed .uf2
to run, i just get back to bootloader every single time, do you happen to know what im doing wrong here? https://github.com/elpekenin/qmk_firmware/blob/579e150c72899e78f06e42aefc27d1b71dcaf7ba/keyboards/handwired/onekey/weact_stm32h750vbt6/ld/STM32H750xB_qspi_tinyuf2.ld. Based this off the .ld
on ChibiOS and did also take a look on a .ld
on your GitHub
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.
arm-none-eabi-gcc (15:10.3-2021.07-4) 10.3.1 20210621 (release)
vs
Using built-in specs.
COLLECT_GCC=arm-none-eabi-gcc
COLLECT_LTO_WRAPPER=~/Workspace/toolchains/gcc-arm-none-eabi-10.3-2021.10/bin/../lib/gcc/arm-none-eabi/10.3.1/lto-wrapper
Target: arm-none-eabi
Configured with:
Thread model: single
Supported LTO compression algorithms: zlib
gcc version 10.3.1 20210824 (release) (GNU Arm Embedded Toolchain 10.3-2021.10)
Definitely seems like something LTO -related is different between our toolchains.
I'm not sure how qmk is laid out in memory, but in your ld, can you try creating 2 memory regions?
Something like
flash0 : org = 0x90000000, len = 0x300
flash1 : org = 0x90000300, len = 8m-0x300
to explicitly ensure that the vector table comes first
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 build should output a .map file to show you the final memory layout. The vector table should have its out input section to ensure it is placed correctly.
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 feel like we should move this to Discord or at least an issue, to stop cluttering the PR (sorry), please answer this message in any way and @ me into it. Unless you think it's better staying here, that is.
The map file looks correct(ish)? Idk, im as noob as you can be with ld stuff....
https://gist.github.com/elpekenin/95eab3c7831187b86f29b983050e5a4e (line 2551)
55d76f7
to
dc1f5b5
Compare
dc1f5b5
to
5090a44
Compare
@hathach I have rebased over the latest changes, cleaned up the code and history |
superb thank you very much, I will review this as soon as I could. There is no problem with H7 having only this as board. We can refactor this later on when new board is added. Thank you very much for updating the PR. |
#if BOARD_QSPI_FLASH_EN | ||
// addr += QSPI_BASE_ADDR; | ||
if (IS_QSPI_ADDR(addr)) | ||
{ | ||
(void) W25qxx_Read(data, addr - QSPI_BASE_ADDR, len); | ||
return; | ||
} | ||
#endif | ||
|
||
#if BOARD_AXISRAM_EN | ||
if (IS_AXISRAM_ADDR(addr) && IS_AXISRAM_ADDR(addr + len - 1)) | ||
{ | ||
memcpy(data, (void *) addr, len); | ||
return; | ||
} | ||
#endif // BOARD_AXISRAM_EN | ||
|
||
if (IS_PFLASH_ADDR(addr)) |
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.
Missing SPI check here, only doing QSPI/RAM/InternalFlash
I found this while playing around with the code to try and get my .uf2 to run...
Will try and double-check for similar missing pieces as soon as i get this working...
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.
board_flash_read
is used to provide data for the CURRENT.uf2
, but it's not really possible to target multiple memories for a readout. (maybe if we readout internal/QSPI/SPI flash and then concatenate all data into CURRENT.uf2)
Internally within the port it's used to read the vector table, and the SPI flash cannot contain executables, so I left it out of board_flash_read
.
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.
all changes look great, this PR definitely needs lots of work. I actualy don't have the hardware to test with. Therefore I could only go through changes only and so far so good. There is only minor requests
- suppress warnings in default handler
- rename the board from mini_stm32hxx_v11 to mini_stm32h750_v11. WeAct has 2 version, though the H743 variant will likely to use on-chip flash for firmware instead of off-chip spi/qspi one. They are different mcus anyway, so it is better to specify the name.
Indeed, there maybe some work to also add on-chip flash support for mcu such as h743, however, I think this is too good to merge now provided it works (since I couldn't test). And we could worry about other MCU later on.
|
||
__attribute__((optimize("O0"))) | ||
void fault_handler(sFrame *fr) | ||
{ |
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.
please add (void) fr;
to suppress unused warning
@bhav97 if you are busy, I could make the review changes myself so that we could finally merge this :) |
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.
perfect, thank you very much for your work and patient. No worry about the other issue, this PR already done an amazing lots of works. We can always fix it later, and/or add support for on-flash, refactor etc...
I'm trying to add support for WeAct MiniSTM32H7xx
Pending
board_flash_flush