boards/cpu: add support for nucleo144-f746 and stm32f7 by haukepetersen · Pull Request #6907 · RIOT-OS/RIOT

haukepetersen · 2017-04-13T20:32:29Z

Almost there, but no UART output. Seems like I missed something, but can't see it right now. @aabadie: feel free to take over this PR - or wait until I am back from holiday...

DipSwitch · 2017-04-13T21:31:51Z

Did you switch the solder jumper configuration?

aabadie · 2017-04-14T12:30:26Z

cpu/stm32f7/ldscripts/stm32f746zg.ld

+{
+    rom (rx)    : ORIGIN = 0x08000000, LENGTH = 1024K
+    ram (rwx)   : ORIGIN = 0x20000000, LENGTH = 320K
+    cpuid (r)   : ORIGIN = 0x1ff80050, LENGTH = 12


this should be 0x1ff0f420 see datasheet section 41.1 page 1654

aabadie · 2017-04-14T12:32:38Z

cpu/stm32f7/include/cpu_conf.h

+}
+#endif
+
+#endif /* STM32L4_CPU_CONF_H */


aabadie · 2017-04-14T12:33:31Z

cpu/stm32f7/vectors.c

+ISR_VECTORS const void *interrupt_vector[] = {
+    /* Exception stack pointer */
+    (void*) (&_estack),             /* pointer to the top of the stack */
+    /* Cortex-M4 handlers */


aabadie · 2017-04-14T12:44:02Z

I allowed myself to push a couple of commits that fix a few problems (cpuid + common nucleo-144 board include). But still no UART or GPIO.

haukepetersen · 2017-04-24T12:05:49Z

@aabadie: thanks for the fixes! Just out of curiosity: were you able to push directly into my branch without having write access? Interesting :-)

@DipSwitch: nope, did not touch the default hardware configuration, must I?

aabadie · 2017-04-24T13:24:28Z

Just out of curiosity: were you able to push directly into my branch without having write access? Interesting :-)

This is one of the GitHub's features. When one creates a PR, there's a checkbox for allowing maintainers of a project to push someone else's branch.

haukepetersen · 2017-04-24T15:23:43Z

Ahh, I see, very interesting, good to know (and quite useful!) :-)

aabadie · 2017-04-24T15:32:28Z

Yes but this is something that we should not abuse, especially with new contributors (even though this is debatable). Otherwise I think it is useful when we want to speed things up by just fixing remaining nits or squashing just before merging. But this is not a very interesting job.

vincent-d · 2017-04-24T17:12:55Z

I have that board: http://www.st.com/en/evaluation-tools/32f746gdiscovery.html
So I tried to port it based on that PR. I'm also not able to have a working UART...

I found an issue with periph_clk_en and periph_clk_dis in stm32_common/cpu_common.c: AHB1 is not enabled for stm32f7. Fixing that should fix the GPIO, but it does not solve the UART issue for me.

haukepetersen · 2017-04-25T19:48:23Z

Alright, after some more hours of debugging I got it to work! Should run fine now. There were actually a couple of issues: wrong flash wait cycle computation, PLL was not set as system clock, HSE was not properly enabled... But all fixed now.

haukepetersen · 2017-04-26T07:26:57Z

Can anyone explain to me, where the cppcheck warning comes from? I thought we got rid of any cppcheck warnings in the code base, so that this (unrelated to this PR) warnings should not happen again?

And anyway, how do we get around this false positive?

miri64 · 2017-04-26T07:32:00Z

Add a suppression comment

/* cppcheck-suppress variableScope
 * variable used in assembly-code below */

miri64 · 2017-04-26T07:33:18Z

(above the line pointed out by cppcheck).

Regarding the supposed fixing: did anyone test locally with the version of cppcheck running on Murdock?

aabadie · 2017-04-26T07:48:15Z

Alright, after some more hours of debugging I got it to work! Should run fine now. There were actually a couple of issues: wrong flash wait cycle computation, PLL was not set as system clock, HSE was not properly enabled... But all fixed now.

Good job ! I'll test asap

aabadie

Some nits and configuration improvements still needed. Otherwise, good job. All STM32 mcu are now supported and full Nucleo support is almost done !

aabadie · 2017-04-26T07:59:40Z

boards/nucleo144-f746/Makefile.features

+FEATURES_PROVIDED += periph_uart
+
+# load the common Makefile.features for Nucleo boards
+include $(RIOTBOARD)/nucleo-common/Makefile.features


should be nucleo144-common

aabadie · 2017-04-26T08:03:42Z

cpu/stm32f7/include/periph_cpu.h

+
+#ifndef DOXYGEN
+/**
+ * @brief   Override ADC resolution values


@name
btw, I'm wondering if those ADC definitions are required since this MCU doesn't provide the implementations for ADC ?

No, brief is right, but the braces are not required here (since doxygen parses the enum as one type anyway)

Then again, doesn't #ifndef DOXYGEN imply that this is NOT parsed by doxygen. In this case @brief still makes more sense, since this is a brief description, not a name.

removed them for now.

aabadie · 2017-04-26T08:10:03Z

cpu/stm32f7/vectors.c

+ */
+
+/**
+ * @ingroup     cpu_stm32l4


cpu_stm32f7

aabadie · 2017-04-26T08:10:19Z

cpu/stm32f7/vectors.c

+WEAK_DEFAULT void isr_svc(void);
+WEAK_DEFAULT void isr_pendsv(void);
+WEAK_DEFAULT void isr_systick(void);
+/* STM32L4 specific interrupt vectors */


aabadie · 2017-04-26T08:10:40Z

cpu/stm32f7/vectors.c

+    (void*) isr_fpu
+};
+
+#if 0


I guess this block can be removed

aabadie · 2017-04-26T08:12:57Z

boards/nucleo144-f746/include/periph_conf.h

+    {
+        .dev        = USART2,
+        .rcc_mask   = RCC_APB1ENR_USART2EN,
+        .rx_pin     = GPIO_PIN(PORT_A, 3),


I would prefer to use PG9 and PG14 for this UART. Then it corresponds to the Arduino RX/TX pins. See what's done the the other nucleo144 boards.

changed pins to PD5 and PD6

aabadie · 2017-04-26T08:14:17Z

boards/nucleo144-f746/include/periph_conf.h

+    {
+        .dev        = USART1,
+        .rcc_mask   = RCC_APB2ENR_USART1EN,
+        .rx_pin     = GPIO_PIN(PORT_B, 7),


Same here there's a USART text printed next to CN8, I would prefer to use one the USART pins available there (probably PD6,PD5)

cant find any more useful mapping these pins...

use USART2, AF7 instead

aabadie · 2017-04-26T08:15:56Z

boards/nucleo144-f746/Makefile.features

+FEATURES_PROVIDED += periph_cpuid
+FEATURES_PROVIDED += periph_gpio
+FEATURES_PROVIDED += periph_hwrng
+FEATURES_PROVIDED += periph_timer


For completeness, we could add some PWM configuration. Maybe it's better to add it in a follow-up PR ?

I would prefer doing it in a follow-up

aabadie · 2017-04-26T08:18:49Z

I also noticed the rework of clock initialization that I think is a nice addition. Do you think it could be generalized to all STM32 ?

aabadie · 2017-04-26T08:21:06Z

Tested on board: no UART. Is there any special change required on the board or can I use the default configuration ?

vincent-d · 2017-04-26T08:07:50Z

cpu/stm32_common/cpu_common.c

            break;
 #elif defined(CPU_FAM_STM32F2) || defined(CPU_FAM_STM32F4) \
-            || defined(CPU_FAM_STM32L4)
+            || defined(CPU_FAM_STM32L4) || defined(CPU_FAM_STM32F7)


Should be added in periph_clk_dis as well

indentation looks off

haukepetersen · 2017-04-26T12:29:30Z

I also noticed the rework of clock initialization that I think is a nice addition. Do you think it could be generalized to all STM32 ?

Yes, thats the point of it. But lets go in small steps, the next one being in #6970

haukepetersen · 2017-04-26T12:30:05Z

Tested on board: no UART. Is there any special change required on the board or can I use the default configuration ?

Nope, didn't touch any (HW) configuration on the board. Mine works as is with this branch.

haukepetersen · 2017-05-04T08:49:58Z

good catch -> seems there was a reason I had that block of out-commented interrupt vectors still in the the vectors file earlier... Fixed.

Also explicitly tested UART_DEV(1) and confirmed it working:

2017-05-04 10:47:23,536 - INFO # main(): This is RIOT! (Version: 2017.04-devel-916-gd3e24a-prag-add_board_nucleo144-f746)
2017-05-04 10:47:23,536 - INFO # 
2017-05-04 10:47:23,539 - INFO # Manual UART driver test application
2017-05-04 10:47:23,542 - INFO # ===================================
2017-05-04 10:47:23,547 - INFO # This application is intended for testing additional UART
2017-05-04 10:47:23,552 - INFO # interfaces, that might be defined for a board. The 'primary' UART
2017-05-04 10:47:23,558 - INFO # interface is tested implicitly, as it is running the shell...
2017-05-04 10:47:23,558 - INFO # 
2017-05-04 10:47:23,564 - INFO # When receiving data on one of the additional UART interfaces, this
2017-05-04 10:47:23,569 - INFO # data will be outputted via STDIO. So the easiest way to test an 
2017-05-04 10:47:23,575 - INFO # UART interface, is to simply connect the RX with the TX pin. Then 
2017-05-04 10:47:23,581 - INFO # you can send data on that interface and you should see the data 
2017-05-04 10:47:23,583 - INFO # being printed to STDOUT
2017-05-04 10:47:23,583 - INFO # 
2017-05-04 10:47:23,587 - INFO # NOTE: all strings need to be '\n' terminated!
2017-05-04 10:47:23,587 - INFO # 
2017-05-04 10:47:23,588 - INFO # UART INFO:
2017-05-04 10:47:23,591 - INFO # Available devices:               3
2017-05-04 10:47:23,595 - INFO # UART used for STDIO (the shell): UART_DEV(0)
2017-05-04 10:47:23,595 - INFO # 
> init 1 9600
2017-05-04 10:47:28,817 - INFO #  init 1 9600
2017-05-04 10:47:28,820 - INFO # Successfully initialized UART_DEV(1)
> send 1 n
2017-05-04 10:47:32,642 - INFO #  send 1 n
2017-05-04 10:47:32,645 - INFO # UART_DEV(1) TX: n
2017-05-04 10:47:32,648 - INFO # UART_DEV(1) RX: n\n
> send 1 blubbbar
2017-05-04 10:47:36,467 - INFO #  send 1 blubbbar
2017-05-04 10:47:36,469 - INFO # UART_DEV(1) TX: blubbbar
> 2017-05-04 10:47:36,481 - INFO #  UART_DEV(1) RX: blubbbar\n

aabadie · 2017-05-04T17:30:26Z

Had a look at the changes again. They all looks good to me now so it's time for squashing here.
I'll give another try next week (I won't have the board with me before tuesday) to see if the UART still doesn't work.

What's the version of your compiler ? (I have gcc 4.9.3) Maybe I can try another one to see if it makes a difference.

haukepetersen · 2017-05-05T11:28:13Z

arm-none-eabi-gcc (GNU Tools for ARM Embedded Processors) 5.4.1 20160919 (release) [ARM/embedded-5-branch revision 240496]
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

haukepetersen · 2017-05-05T11:33:44Z

squashed

haukepetersen · 2017-05-05T11:35:03Z

@vincent-d: are all your change requests fulfilled?

vincent-d

Found another issue, sorry.

vincent-d · 2017-05-05T15:52:43Z

cpu/stm32f7/vectors.c

+    (void*) isr_rtc_wkup,
+    (void*) isr_flash,
+    (void*) isr_rcc,
+    (void*) isr_exti0,


It seems like every other stm32 define every isr_exti* as isr_exti, gpio interrupt are crashing otherwise.

Yes, that's correct. This change was lost when I copied in the correct isr table...

haukepetersen · 2017-05-08T07:16:28Z

fixed exti ISR vectors

haukepetersen · 2017-05-10T10:24:50Z

@aabadie: what is your verdict, are you ready to approve as well?

aabadie · 2017-05-10T11:08:24Z

what is your verdict, are you ready to approve as well?

I re-tested yesterday by flashing an mbed generated firmware that should print something on UART: didn't work. So there's probably something broken on the board I have.
BTW, the changes are fine here: ACK

aabadie · 2017-05-10T11:09:15Z

Re-triggered a build on Murdock2

aabadie · 2017-05-10T14:05:54Z

All green: Go!

Tenderson · 2017-08-20T11:37:29Z

Hi, For the BOARD=nucleo144-f767, the support for the I2C and SPI is not yet defined in there. Is there any plan for this update?

Thank you.

haukepetersen · 2017-08-21T07:51:36Z

We just need to find someone doing this, so feel free to port them :-)

haukepetersen added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: new feature The issue requests / The PR implemements a new feature for RIOT State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Apr 13, 2017

haukepetersen requested a review from aabadie April 13, 2017 20:32

aabadie self-assigned this Apr 14, 2017

aabadie reviewed Apr 14, 2017

View reviewed changes

cpu/stm32f7/include/cpu_conf.h Outdated

}

#endif

#endif /* STM32L4_CPU_CONF_H */

Copy link

Contributor

aabadie Apr 14, 2017

Choose a reason for hiding this comment

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

F7

aabadie reviewed Apr 14, 2017

View reviewed changes

haukepetersen force-pushed the add_board_nucleo144-f746 branch from d4c2080 to 631559f Compare April 25, 2017 19:46

haukepetersen added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Apr 25, 2017

haukepetersen mentioned this pull request Apr 26, 2017

boards/nucleo-l1: use LSI as default RTC clock #6504

Closed

aabadie requested changes Apr 26, 2017

View reviewed changes

vincent-d requested changes Apr 26, 2017

View reviewed changes

haukepetersen force-pushed the add_board_nucleo144-f746 branch from d3e24a8 to 144409c Compare May 4, 2017 08:48

haukepetersen force-pushed the add_board_nucleo144-f746 branch from 144409c to b8dc85f Compare May 5, 2017 11:33

haukepetersen added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 5, 2017

vincent-d approved these changes May 5, 2017

View reviewed changes

vincent-d requested changes May 5, 2017

View reviewed changes

haukepetersen added 4 commits May 8, 2017 09:16

core/panic: enable panic modes for Cortex-M7

f914ae4

cpu/cortex_common: added support for Cortex-M7

10a7486

cpu/stm32: added support for STM32F7

7079d48

boards: added support for nucleo144-f746

17fedc7

haukepetersen force-pushed the add_board_nucleo144-f746 branch from b8dc85f to 17fedc7 Compare May 8, 2017 07:16

vincent-d approved these changes May 9, 2017

View reviewed changes

aabadie approved these changes May 10, 2017

View reviewed changes

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 May 10, 2017

aabadie merged commit 40de3c6 into RIOT-OS:master May 10, 2017

haukepetersen deleted the add_board_nucleo144-f746 branch May 11, 2017 08:45

aabadie modified the milestone: Release 2017.07 Jun 30, 2017

Conversation

haukepetersen commented Apr 13, 2017

Uh oh!

DipSwitch commented Apr 13, 2017 via email • edited by aabadie Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabadie commented Apr 14, 2017

Uh oh!

haukepetersen commented Apr 24, 2017

Uh oh!

aabadie commented Apr 24, 2017

Uh oh!

haukepetersen commented Apr 24, 2017

Uh oh!

aabadie commented Apr 24, 2017

Uh oh!

vincent-d commented Apr 24, 2017

Uh oh!

haukepetersen commented Apr 25, 2017

Uh oh!

haukepetersen commented Apr 26, 2017

Uh oh!

miri64 commented Apr 26, 2017

Uh oh!

miri64 commented Apr 26, 2017

Uh oh!

aabadie commented Apr 26, 2017

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabadie commented Apr 26, 2017

Uh oh!

aabadie commented Apr 26, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haukepetersen commented Apr 26, 2017

DipSwitch commented Apr 13, 2017 via email •

edited by aabadie

Loading

Tenderson commented Aug 20, 2017 •

edited

Loading