Skip to content
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

cpu/stm32f3: add support for flashpage and flashpage_raw #11749

Merged
merged 1 commit into from Jul 1, 2019

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Jun 26, 2019

Contribution description

This PR is adding support for periph_flashpage and periph_flashpage_raw on stm32f3 MCU.

I tested it with success on all stm32f3 boards supported in RIOT: nucleo-f303re, nucleo-f303ze, nucleo-f303k8, nucleo-f302r8, nucleo-f334r8

Testing procedure

The following command should work:

make BOARD=<stm32f3 based board> -C tests/periph_flashpage flash test

The boards to test: nucleo-f303{re,ze,k8}, nucleo-f302r8 and nucleo-f334r8

Issues/PRs references

None

@aabadie aabadie added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers Area: cpu Area: CPU/MCU ports labels Jun 26, 2019
@aabadie aabadie requested a review from fjmolinas June 26, 2019 13:00
@aabadie aabadie added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 26, 2019
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

Tested on stm32f302r8, the tests pass and everything seems to work but according to the datasheet HSI also needs to be enabled for erase operations, which is not being done here for F3.

@@ -174,7 +175,7 @@ void flashpage_write_raw(void *target_addr, const void *data, size_t len)

DEBUG("[flashpage_raw] write: now writing the data\n");
#if defined(CPU_FAM_STM32F0) || defined(CPU_FAM_STM32F1) || \
defined(CPU_FAM_STM32L4)
defined(CPU_FAM_STM32L4) || defined(CPU_FAM_STM32F3)
Copy link
Contributor

Choose a reason for hiding this comment

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

Minot nit-picking, this could be in alphabetical order.

*/
#define FLASHPAGE_RAW_BLOCKSIZE (2U)
/* Writing should be always 4 bytes aligned */
#define FLASHPAGE_RAW_ALIGNMENT (4U)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why 4 bytes? The blocksize is 2 bytes, skimming through the datasheet I couldn't find a reference to this being a requirement.

Copy link
Contributor

Choose a reason for hiding this comment

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

I found the answer to this in a comment in tests/periph_flashpage:

The flash page in RAM must be correctly aligned, even in RAM, when
 *          using flashpage_raw. This is because some architecture uses
 *          32 bit alignment implicitly and there are cases (stm32l4) that
 *          requires 64 bit alignment.

So its an architecture requirement, and does need to be 32b aligned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @fjmolinas :)

CNTRL_REG &= ~(FLASH_CR_PG);
#endif
DEBUG("[flashpage_raw] write: done writing data\n");

/* lock the flash module again */
_lock();

#if defined(CPU_FAM_STM32F0) || defined(CPU_FAM_STM32F1)
#if defined(CPU_FAM_STM32F0) || defined(CPU_FAM_STM32F1) || \
Copy link
Contributor

Choose a reason for hiding this comment

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

This also need to be done for erase operations according to the reference manual p68.

For program and erase operations on the Flash memory (write/erase), the internal RC
oscillator (HSI) must be ON.

@fjmolinas
Copy link
Contributor

@aabadie I see you have addressed my comments and I re-tested that tests are still passing. Can you rebase?

@aabadie
Copy link
Contributor Author

aabadie commented Jun 28, 2019

Thanks @fjmolinas, squashed and rebased.

Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK!

@fjmolinas fjmolinas merged commit c3aaf62 into RIOT-OS:master Jul 1, 2019
@aabadie aabadie deleted the pr/cpu/stm32f3_flashpage branch July 2, 2019 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants