Skip to content

arm bug minor cpu/stm32f1 Fixed wrong flash page sizes#8331

Merged
smlng merged 1 commit intoRIOT-OS:masterfrom
Winkelkatze:fixed_stm32f1_page_size
Jan 10, 2018
Merged

arm bug minor cpu/stm32f1 Fixed wrong flash page sizes#8331
smlng merged 1 commit intoRIOT-OS:masterfrom
Winkelkatze:fixed_stm32f1_page_size

Conversation

@Winkelkatze
Copy link
Copy Markdown
Contributor

@Winkelkatze Winkelkatze commented Jan 7, 2018

Contribution description

This PR fixes a wrong FLASHPAGE_SIZE value for STM32F103x8 and STM32F103xB CPUs. As stated in the datasheet [1] the page size of a flash page is 1024 bytes for these controllers.
The old value (2048) is still correct for the STM32F103xC, STM32F103xD and STM32F103xE [2].

[1] https://www.st.com/content/ccc/resource/technical/document/datasheet/33/d4/6f/1d/df/0b/4c/6d/CD00161566.pdf/files/CD00161566.pdf/jcr:content/translations/en.CD00161566.pdf (p. 57)
[2] https://www.st.com/content/ccc/resource/technical/document/datasheet/59/f6/fa/84/20/4e/4c/59/CD00191185.pdf/files/CD00191185.pdf/jcr:content/translations/en.CD00191185.pdf (p. 64)

Issues/PRs references

None

@aabadie aabadie added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Jan 7, 2018
smlng
smlng suggested changes Jan 8, 2018
View reviewed changes
cpu/stm32f1/include/cpu_conf.h
* @{
*/
#if defined(CPU_MODEL_STM32F103C8) || defined(CPU_MODEL_STM32F103CB) || defined(CPU_MODEL_STM32F103RB)
#define FLASHPAGE_SIZE (1024U)
Copy link
Copy Markdown
Member

@smlng smlng Jan 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

don't we need to fix the FLASHPAGE_NUMOF for those CPU variants below then, too? That is 64U for STM32F103C8 and 128U for STM32F103CB and STM32F103RB.

Copy link
Copy Markdown
Contributor Author

@Winkelkatze Winkelkatze Jan 8, 2018

Choose a reason for hiding this comment

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

Indeed, I overlooked this and fixed it now. Thanks for being watchful.

@smlng
Copy link
Copy Markdown
Member

smlng commented Jan 9, 2018

please squash and reword your commit message to something like cpu/stm32f1: fix flash page sizes, the important point is to have the module name (here cpu/stm32f1) up front to give a hint where changes apply.

Also: thanks for contributing!

@smlng smlng added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 9, 2018
@smlng smlng added CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable 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 Jan 9, 2018
@Winkelkatze Winkelkatze force-pushed the fixed_stm32f1_page_size branch from 432cf59 to 2925ef9 Compare January 9, 2018 21:08
@Winkelkatze
Copy link
Copy Markdown
Contributor Author

Winkelkatze commented Jan 9, 2018

OK, now i squashed the PR (at least I think I have ^^).
Thanks sming for the constructive feedback, I'll try to get things right at the first time for my next contributions.

@smlng smlng added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 9, 2018
@smlng
Copy link
Copy Markdown
Member

smlng commented Jan 9, 2018

perfect, looks good now!

@smlng
Copy link
Copy Markdown
Member

smlng commented Jan 9, 2018

I make a quick runtime test with a stm32 bluepill tomorrow, just to be sure. Otherwise, all good!

@smlng
Copy link
Copy Markdown
Member

smlng commented Jan 10, 2018

verified by testing on bluepill with stm32f103c8t6 CPU, and flash page size 1024 -> works like charm.

ACK & GO!

@smlng smlng merged commit e38b5e7 into RIOT-OS:master Jan 10, 2018
@aabadie aabadie added this to the Release 2018.01 milestone Jan 18, 2018
@kb2ma kb2ma mentioned this pull request Apr 10, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@smlng smlng smlng approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2018.01

Development

Successfully merging this pull request may close these issues.

3 participants

@Winkelkatze @smlng @aabadie