Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

STM32H7 Flash Support? #1070

Closed
thestumbler opened this issue Sep 3, 2023 · 5 comments
Closed

STM32H7 Flash Support? #1070

thestumbler opened this issue Sep 3, 2023 · 5 comments

Comments

@thestumbler
Copy link

I'm porting my first project to MODM, which is a simple boot loader. The original code ran on an STM32F405, and now being adapted to H723 board (and similar ones in the same H7 group). Briefly glancing through the flash code for STM32, it appears that only G0, G4, F1, and F4 families are supported.

How big a deal is it to add H7 to the mix? In some regards it seems easier than the F4, where I had to keep track of varying sector sizes -- these H7's of interest all have 8 equal-sized 128 KB sectors.

@salkinium
Copy link
Member

I personally just enable the family in the module.lb and see what breaks ;-p
The Flash driver should be relatively simple to adapt, particularly because there are already a few flavors to extrapolate from.

@thestumbler
Copy link
Author

Working on it (between fits of coughing because I caught Covid for the 2nd time). I got erase sector working. Debugging the program function now.

A few notes on the logic of the flash.cpp/hop code:

(1) It seems the code is using the terms SECTORS and PAGES as flag to mean fixed- vs variable-sized flash sectors / pages. Since I haven't run across this distinction in the terminology before, it seems a bit too subtle. Would it be better to use a single term for flash sectors, be that sectors or pages, and let the sector size regularity be expressed in a clearer manner by another flag?

In my C code that I am porting from, I handled sector sizes with two constant arrays, one of sector starting addresses and one of sector sizes -- and of course a constant containing the total number of sectors.

The logic used to handle the variable size sectors is single purpose only for the F4xx family. I haven't surveyed all the STM32 offerings, but I'm surprised that there's only one pattern of unequal sectors sizes across all supported ST's offerings.

(2) Despite having a more complex method of flash control, H7 vs F4 for example, many basic operations follow a similar pattern and use the same or similar registers. But a few of those registers and constants have been ever-so-slightly renamed in the H7 family. For example, FLASH_CR vs FLASH_CR1, or STRT vs START. For now I just rolled that into the H7 logic, although in a some cases this results in a few lines of code that's otherwise identical except for the names.

I'm not sure it would work within your framework's guidelines, but I was tempted to fix that in the ST header file in some say so that the same code worked in both cases, maybe using something like a define for a new preprocessor variable FLASH_CR which would point to the correct one. Kind of like you do for FLASH_ERR

(3) Speaking of FLASH_ERR, while fixing a stupid mistake I made, I thought I was causing a problem by writing 1s to reserved bits in the SR. The manual clearly says those bits must be kept as zeros, and the existing method using FLASH_ERR seems to violate this. In hindsight this may have been a non-issue and writing ones may be harmless, but thought I'd point it out. And regardless, for the H7 I needed a different pattern in FLASH_ERR to clear all errors, because the bits are in different positions and also entire lower nibble are non-writable status bits (vs only the single lower bit of the F4 family)

(4) Where is the syntax of the .in file logic explained? I see mention of modified Jinra2 syntax using %%. Clicking on the link however shows syntax like {% something %}. I'm guessing that's replaced by double %s? Anyway I was trying to find whether elif is supported in the if-else construct, and I concluded it was.

(5) How should the include path look for flash.hop? Initially I copied and pasted <modm/platform /flash.hpp> from another MCU's example, and I could swear that was working at first. Then suddenly it quit working. I then realized that perhaps <modm/platform/flash /flash.hpp> was correct, since that's really the file path. This continues to work, and I'm baffled how the wrong one ever did work.

@salkinium
Copy link
Member

salkinium commented Sep 4, 2023

Would it be better to use a single term for flash sectors, be that sectors or pages, and let the sector size regularity be expressed in a clearer manner by another flag?

Well… yeah. But ST's documentation is inconsistent. In one family they are called pages while in others sectors. We're currently forwarding the sector API to the page implementation.

but I'm surprised that there's only one pattern of unequal sectors sizes across all supported ST's offerings.

There are probably a few more, but we only implemented it for a few families yet.

In my C code that I am porting from, I handled sector sizes with two constant arrays

I don't understand why I put the algorithm into the .cpp file anyways, this could be made constexpr and probably more efficient with an constexpr array. Feel free to move the implementation to the header and refactor.

I'm not sure it would work within your framework's guidelines, but I was tempted to fix that in the ST header file in some say so that the same code worked in both cases, maybe using something like a define for a new preprocessor variable FLASH_CR which would point to the correct one. Kind of like you do for FLASH_ERR

We try to stay as close as possible to the ST Headers, even if that's annoying to code. We typically encode such naming differences in the module.lb and pass them as a substitution into the jinja template, so basically doing what the CPP does explicitly. This makes it easier to read the generated code since there are fewer "CPP indirections". It's a subjective choice.

Where is the syntax of the .in file logic explained?

It's the standard Jinja2 syntax with some tweaks. You can use %% and %# in addition to the normal statements. We added the shortcuts to type less, but we sometimes mix the syntaxes for better whitespace control.

How should the include path look for flash.hop?

Not at all. The <modm/platform.hpp> file is generated to auto-include all header files from the platform folder. That way you don't need to also adapt the source code if your already adapting the project.xml. We briefly entertained the idea of making a <modm.hpp> that includes the whole generated library, but it seemed a bit too much.

@thestumbler
Copy link
Author

thestumbler commented Sep 4, 2023

But ST's documentation is inconsistent.

I meant to mention that as well. Even though they renamed a few registers like FLASH->CR to CR1, in the text description in the data sheet they sometimes use the old style (just CR).

@thestumbler
Copy link
Author

Brief update, I have it basically working. That H7 flash controller is way different, and sometimes in subtle ways. Once I have it more proven out, I'll report fuller.

@modm-io modm-io locked and limited conversation to collaborators May 4, 2024
@salkinium salkinium converted this issue into discussion #1162 May 4, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Development

No branches or pull requests

2 participants