Skip to content

cpu: add stm32f2xx family support (based on #4497)#5397

Merged
kaspar030 merged 11 commits intoRIOT-OS:masterfrom
OTAkeys:pr/stm32f2xx
Aug 31, 2016
Merged

cpu: add stm32f2xx family support (based on #4497)#5397
kaspar030 merged 11 commits intoRIOT-OS:masterfrom
OTAkeys:pr/stm32f2xx

Conversation

@vincent-d
Copy link
Copy Markdown
Member

Hi,

This pull request adds support for stm32f2xx family.
It's based on #4497, thanks @DipSwitch for the initial work.
The last commit adds the nucleo-f207 board based on stm32f207.

DipSwitch and others added 7 commits December 13, 2015 11:00
* Update headers from ST
* Add linker scripts
The driver was imported from stm32f0
Fixes:
 - Fix 12/24h handling (CR flag badly used)
 - Fix interrupt flag clearance and interrupt name
Add missing DMA interrupts for UART
Improve baudrate intialization:
 - Return error if baudrate is theorically unreachable
 - Implement oversampling by 8 method for high baudrates
Add UART hardware flow control support
Ensure uart tx thread safety with a mutex
Allow setting of pins mode per UART
Remove unneeded workaround from stm32f1
Make pullup configurable
Fix OAR1 register initialization
improve i2c read functions with repeatead start conditions
avoid to any loop to become infinite
improve i2c driver error handling
add missing coma in array initializers
@PeterKietzmann PeterKietzmann added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: drivers Area: Device drivers Type: new feature The issue requests / The PR implemements a new feature for RIOT and removed Area: drivers Area: Device drivers labels Apr 26, 2016
@DipSwitch
Copy link
Copy Markdown
Member

DipSwitch commented Apr 27, 2016 via email

@toonst
Copy link
Copy Markdown
Member

toonst commented Apr 28, 2016

@DipSwitch I had a look at the EWM PR, but it is a bit hard to see which parts are for the cpu and which are for the board. Also it seems to incorporate a lot of merges from other PRs. Can you indicate which parts are missing?

We tested this PR extensively on the nucleo-f207 board (and on our own HW) so I think it is ready for a merge.

@vincent-d
Copy link
Copy Markdown
Member Author

What has to be done to have an implementation of the stm32f2 into RIOT ?
What is your feedback on this PR and what exactly prevents it from being merged ?

@miri64
Copy link
Copy Markdown
Member

miri64 commented May 10, 2016

Sorry, a significant portion of the RIOT community that usually looks into this kind of PRs is currently occupied with day job work. This should be finished by tonight.

Optional tip: I'm no expert on porting but usually it helps to split up a PR like this into smaller ones, so people aren't overwhelmed by the sheer number of code lines (34700 is quiet a lot).

@vincent-d
Copy link
Copy Markdown
Member Author

@authmillenon thanks for the reply.

I think the high number of added lines comes mainly from the register header files update.

#undef LED0_ON
#undef LED0_OFF
#undef LED0_TOGGLE
#define LED0_PIN GPIO_PIN(PORT_B, 0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you please group LEDx_* together?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

done!

@toonst
Copy link
Copy Markdown
Member

toonst commented May 31, 2016

Hi guys, can we get an update on this?

@toonst toonst mentioned this pull request Jun 29, 2016
enum {
AHB1, /**< AHB1 bus */
AHB2, /**< AHB2 bus */
AHB3 /**< AHB3 bus */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd move a change here to a separate PR and use it in all other stm32s too then.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You highlighted a good point. Actually, the number of AHB is not the same for all stm32 CPUs. This 'enum' should be part of the specific periph_cpu.h.

@toonst
Copy link
Copy Markdown
Member

toonst commented Jul 18, 2016

@haukepetersen Any idea when you will have time for reviewing this? We'd like to have it inside the next release.

#include "msg.h"
#include "ringbuffer.h"
#include "periph/uart.h"
#include "uart_stdio.h"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what's the reason for this?

Copy link
Copy Markdown
Member

@toonst toonst Jul 18, 2016

Choose a reason for hiding this comment

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

This makes sure UART_STDIO_DEV is defined, so the init command fails when trying to use the shell UART as a parameter.
Maybe there's a better way to do this?
Edit: removed reference to local commit sha which was squashed

@kaspar030
Copy link
Copy Markdown
Contributor

@toonst @haukepetersen is in paternity leave until September.

@emmanuelsearch Could you find out if anyone at FU has this board available?

@kaspar030
Copy link
Copy Markdown
Contributor

I get the following when compiling examples/default:

"make" -C /home/kaspar/src/riot.tmp/sys/shell/commands
dac.c: In function 'dac_set':
dac.c:76:19: error: array subscript is above array bounds [-Werror=array-bounds]
     if (dac_config[line].chan) {
         ~~~~~~~~~~^~~~~~
dac.c: In function 'dac_poweron':
dac.c:91:38: error: array subscript is above array bounds [-Werror=array-bounds]
     DAC->CR |= (1 << (16 * dac_config[line].chan));
                            ~~~~~~~~~~^~~~~~
dac.c: In function 'dac_poweroff':
dac.c:96:39: error: array subscript is above array bounds [-Werror=array-bounds]
     DAC->CR &= ~(1 << (16 * dac_config[line].chan));
                             ~~~~~~~~~~^~~~~~
cc1: all warnings being treated as errors
gcc version 6.1.1 20160526 (Arch Repository)

@kaspar030 kaspar030 self-assigned this Jul 18, 2016
@kaspar030
Copy link
Copy Markdown
Contributor

if anyone at FU has this board available?

If no maintainer has one available, I'd say, once everything compiles, let's merge anyways. Code-wise the PR looks very nice.

@kaspar030 kaspar030 added this to the Release 2016.07 milestone Jul 18, 2016
this is copied from DipSwitch's pr
@toonst
Copy link
Copy Markdown
Member

toonst commented Jul 19, 2016

adressed in 378a7a5. Thanks for reviewing!

@toonst
Copy link
Copy Markdown
Member

toonst commented Jul 19, 2016

I cleaned the history with rebase, should be ready for a merge now.

@kYc0o
Copy link
Copy Markdown
Contributor

kYc0o commented Jul 22, 2016

The commits seem a bit messy... is it possible to squash them and order them by type? for instance all the uart commits in a single one, same for i2c, spi, etc...

Toon Stegen and others added 2 commits July 25, 2016 10:34
changes taken from DipSwitch's board: EMW3162 RIOT-OS#4498 PR.
- Adds a functional implementation of the ADC
- Implements low power mode
This board is based on a 144-pin stm32f207 cortex-m3
@toonst toonst force-pushed the pr/stm32f2xx branch 3 times, most recently from e7f67da to 146d462 Compare July 25, 2016 09:02
@toonst
Copy link
Copy Markdown
Member

toonst commented Jul 25, 2016

@kYc0o addressed. Also rebased onto latest master.

@kYc0o
Copy link
Copy Markdown
Contributor

kYc0o commented Jul 25, 2016

Thanks @toonst ! But sadly it will go into the next release... But as it's ready to merge it will go fast ;)

@kYc0o kYc0o modified the milestones: Release 2016.10, Release 2016.07 Jul 25, 2016
@vincent-d
Copy link
Copy Markdown
Member Author

Hi guys, is there any chance this PR could be merged soon ?

@kaspar030
Copy link
Copy Markdown
Contributor

Compiles fine, looks OK. How do we proceed?

@kaspar030
Copy link
Copy Markdown
Contributor

I think we should merge this, if @vincent-d promises to help with basic maintenance of the port.

(merging code that maintainers cannot test is a more general discussion, I'll try to find consensus at devel@)

@vincent-d
Copy link
Copy Markdown
Member Author

@kaspar030 of course I can help with the maintenance of the port :)

@DipSwitch
Copy link
Copy Markdown
Member

DipSwitch commented Aug 22, 2016 via email

@kaspar030 kaspar030 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 24, 2016
@kaspar030
Copy link
Copy Markdown
Contributor

I think we should merge this

Noone seems to disagree. ACK when Murdock is fine.

@kaspar030
Copy link
Copy Markdown
Contributor

Murdock complains about extra whitespaces in the vendor headers. Could you add those files to the doxygen excludes?

@vincent-d
Copy link
Copy Markdown
Member Author

Vendor headers excluded from doxygen

@kaspar030
Copy link
Copy Markdown
Contributor

Well, it's the static tests that complain. But we don't care, so go!

@kaspar030 kaspar030 merged commit 26e4004 into RIOT-OS:master Aug 31, 2016
@PeterKietzmann PeterKietzmann mentioned this pull request Jan 13, 2017
@toonst toonst deleted the pr/stm32f2xx branch August 21, 2018 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation 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.