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

Nucleo f072 #4949

Merged
merged 4 commits into from
Mar 31, 2016
Merged

Nucleo f072 #4949

merged 4 commits into from
Mar 31, 2016

Conversation

jia200x
Copy link
Member

@jia200x jia200x commented Mar 2, 2016

Hi!

Added support for Nucleo F072 board. I'm having problems with OpenOCD for Mac OS X (I can flash only once, then stops working). Seems to work in Linux.

@haukepetersen
Copy link
Contributor

looks good and works as expected. Could you please rebase and squash your commits? thanks

@haukepetersen haukepetersen added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Mar 2, 2016
@haukepetersen haukepetersen added this to the Release 2016.04 milestone Mar 2, 2016
@haukepetersen haukepetersen self-assigned this Mar 2, 2016
@jia200x
Copy link
Member Author

jia200x commented Mar 2, 2016

Rebased and squashed.
Cheers!

@haukepetersen haukepetersen 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 Mar 2, 2016
@haukepetersen
Copy link
Contributor

hm, seems like something went wrong. I am still seeing a number of commits that don't belong here..

@jia200x
Copy link
Member Author

jia200x commented Mar 2, 2016

Yes, very weird... I have tried several times to rebase, but is not working
as I'm expecting. I think it's faster to create a new branch, add files and
change the variable in vendor header file and PR again. I think I'll do
that, is ok?

2016-03-02 16:08 GMT+01:00 Hauke Petersen notifications@github.com:

hm, seems like something went wrong. I am still seeing a number of commits
that don't belong here..


Reply to this email directly or view it on GitHub
#4949 (comment).

@jia200x
Copy link
Member Author

jia200x commented Mar 2, 2016

Ok, fixed.
Could you test it again just for checking everything went right?

Thank you!

@OlegHahm OlegHahm added the Platform: ARM Platform: This PR/issue effects ARM-based platforms label Mar 3, 2016
@jia200x
Copy link
Member Author

jia200x commented Mar 3, 2016

It's seems now is OK

@haukepetersen haukepetersen added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 3, 2016
@haukepetersen
Copy link
Contributor

looks good and works -> ACK once Travis is green.

@jia200x
Copy link
Member Author

jia200x commented Mar 3, 2016

Cool!
Thank you.

2016-03-03 14:35 GMT+01:00 Hauke Petersen notifications@github.com:

looks good and works -> ACK once Travis is green.


Reply to this email directly or view it on GitHub
#4949 (comment).

@OlegHahm
Copy link
Member

OlegHahm commented Mar 6, 2016

There are some Travis complaints:

boards/nucleo-f072/board.c:45: new blank line at EOF.

boards/nucleo-f072/include/board.h:68: new blank line at EOF.

boards/nucleo-f072/include/periph_conf.h:109: new blank line at EOF.

cpu/stm32f0/include/cpu_conf.h:33: trailing whitespace.

+ 

cpu/stm32f0/include/stm32f072xb.h:1: trailing whitespace.

+/**^M

cpu/stm32f0/include/stm32f072xb.h:2: trailing whitespace.

+  *****************************************************************************

cpu/stm32f0/include/stm32f072xb.h:3: trailing whitespace.

+  * @file    stm32f072xb.h^M

cpu/stm32f0/include/stm32f072xb.h:4: trailing whitespace.

+  * @author  MCD Application Team^M

cpu/stm32f0/include/stm32f072xb.h:5: trailing whitespace.

+  * @version V2.2.3^M

cpu/stm32f0/include/stm32f072xb.h:6: trailing whitespace.

+  * @date    29-January-2016^M

cpu/stm32f0/include/stm32f072xb.h:7: trailing whitespace.

+  * @brief   CMSIS Cortex-M0 Device Peripheral Access Layer Header File. ^M

cpu/stm32f0/include/stm32f072xb.h:8: trailing whitespace.

+  *          This file contains all the peripheral register's definitions, bits

cpu/stm32f0/include/stm32f072xb.h:9: trailing whitespace.

+  *          definitions and memory mapping for STM32F0xx devices.            

cpu/stm32f0/include/stm32f072xb.h:10: trailing whitespace.

+  *            ^M

cpu/stm32f0/include/stm32f072xb.h:11: trailing whitespace.

+  *          This file contains:^M

cpu/stm32f0/include/stm32f072xb.h:12: trailing whitespace.

+  *           - Data structures and the address mapping for all peripherals^M

cpu/stm32f0/include/stm32f072xb.h:13: trailing whitespace.

+  *           - Peripheral's registers declarations and bits definition^M

cpu/stm32f0/include/stm32f072xb.h:14: trailing whitespace.

+  *           - Macros to access peripheral<92>s registers hardware^M

cpu/stm32f0/include/stm32f072xb.h:15: trailing whitespace.

+  *  ^M

cpu/stm32f0/include/stm32f072xb.h:16: trailing whitespace.

+  *****************************************************************************

cpu/stm32f0/include/stm32f072xb.h:17: trailing whitespace.

+  * @attention^M

:

@OlegHahm
Copy link
Member

OlegHahm commented Mar 6, 2016

It also needs some blacklisting for several applications.

@haukepetersen
Copy link
Contributor

The Travis complains come from dos line endings in the vendor header file, so just convert them to unix ones...

@jia200x
Copy link
Member Author

jia200x commented Mar 16, 2016

Hi!

Oops sorry, I forgot to finish this.
What kind of blacklisting do this model need?

I'm fixing it

2016-03-16 10:17 GMT+01:00 Hauke Petersen notifications@github.com:

The Travis complains come from dos line endings in the vendor header file,
so just convert them to unix ones...


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4949 (comment)

@OlegHahm
Copy link
Member

E.g.

gnrc_border_router:nucleo-f072:
Building application "gnrc_border_router" for "nucleo-f072" with MCU "stm32f0".
/usr/bin/../lib/gcc/arm-none-eabi/4.9.3/../../../../arm-none-eabi/bin/ld: /home/travis/build/RIOT-OS/RIOT/examples/gnrc_border_router/bin/nucleo-f072/gnrc_border_router.elf section `.bss' will not fit in region `ram'
/usr/bin/../lib/gcc/arm-none-eabi/4.9.3/../../../../arm-none-eabi/bin/ld: region `ram' overflowed by 3424 bytes
collect2: error: ld returned 1 exit status
make: *** [all] Error 1

this means, that you will have to add the board to the BOARD_INSUFFICIENT_MEMORY list in examples/gnrc_border_router/Makefile.

@jia200x
Copy link
Member Author

jia200x commented Mar 16, 2016

Got it.
Thank you!

2016-03-16 11:54 GMT+01:00 Oleg Hahm notifications@github.com:

E.g.

gnrc_border_router:nucleo-f072:
Building application "gnrc_border_router" for "nucleo-f072" with MCU "stm32f0".
/usr/bin/../lib/gcc/arm-none-eabi/4.9.3/../../../../arm-none-eabi/bin/ld: /home/travis/build/RIOT-OS/RIOT/examples/gnrc_border_router/bin/nucleo-f072/gnrc_border_router.elf section .bss' will not fit in regionram'
/usr/bin/../lib/gcc/arm-none-eabi/4.9.3/../../../../arm-none-eabi/bin/ld: region `ram' overflowed by 3424 bytes
collect2: error: ld returned 1 exit status
make: *** [all] Error 1

this means, that you will have to add the board to the
BOARD_INSUFFICIENT_MEMORY list in examples/gnrc_border_router/Makefile.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4949 (comment)

@jia200x jia200x force-pushed the nucleo-f072 branch 4 times, most recently from 546b2e3 to 06dbcdf Compare March 16, 2016 12:04
@jia200x
Copy link
Member Author

jia200x commented Mar 16, 2016

Done.

I blacklisted the examples, but make command compiles everything and throws
"region 'ram' overflow". Is this a normal behavior?

Cheers!

2016-03-16 11:55 GMT+01:00 Jose Alamos jialamos@uc.cl:

Got it.
Thank you!

2016-03-16 11:54 GMT+01:00 Oleg Hahm notifications@github.com:

E.g.

gnrc_border_router:nucleo-f072:
Building application "gnrc_border_router" for "nucleo-f072" with MCU "stm32f0".
/usr/bin/../lib/gcc/arm-none-eabi/4.9.3/../../../../arm-none-eabi/bin/ld: /home/travis/build/RIOT-OS/RIOT/examples/gnrc_border_router/bin/nucleo-f072/gnrc_border_router.elf section .bss' will not fit in regionram'
/usr/bin/../lib/gcc/arm-none-eabi/4.9.3/../../../../arm-none-eabi/bin/ld: region `ram' overflowed by 3424 bytes
collect2: error: ld returned 1 exit status
make: *** [all] Error 1

this means, that you will have to add the board to the
BOARD_INSUFFICIENT_MEMORY list in examples/gnrc_border_router/Makefile.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4949 (comment)

@jia200x
Copy link
Member Author

jia200x commented Mar 21, 2016

Hi.

Failed again. The examples are not being build. I blacklisted some of them,
but make still tries to build them. It's normal?

Also don't know what happened with the vendor header file, It seems I
commited the old version. I'll fix it and try again.

2016-03-16 13:07 GMT+01:00 Jose Alamos jialamos@uc.cl:

Done.

I blacklisted the examples, but make command compiles everything and
throws "region 'ram' overflow". Is this a normal behavior?

Cheers!

2016-03-16 11:55 GMT+01:00 Jose Alamos jialamos@uc.cl:

Got it.
Thank you!

2016-03-16 11:54 GMT+01:00 Oleg Hahm notifications@github.com:

E.g.

gnrc_border_router:nucleo-f072:
Building application "gnrc_border_router" for "nucleo-f072" with MCU "stm32f0".
/usr/bin/../lib/gcc/arm-none-eabi/4.9.3/../../../../arm-none-eabi/bin/ld: /home/travis/build/RIOT-OS/RIOT/examples/gnrc_border_router/bin/nucleo-f072/gnrc_border_router.elf section .bss' will not fit in regionram'
/usr/bin/../lib/gcc/arm-none-eabi/4.9.3/../../../../arm-none-eabi/bin/ld: region `ram' overflowed by 3424 bytes
collect2: error: ld returned 1 exit status
make: *** [all] Error 1

this means, that you will have to add the board to the
BOARD_INSUFFICIENT_MEMORY list in examples/gnrc_border_router/Makefile.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4949 (comment)

@OlegHahm
Copy link
Member

Somehow you seemed to have reverted the blacklisting in jia200x@06dbcdf

@OlegHahm
Copy link
Member

I'm somewhat scared by the number of added lines, but since most of them are in the (bloated) STM CPU header, I guess this is fine. Additionally this PR introduces a new feature - so it cannot break much.


#define LED_GREEN_ON (LED_GREEN_PORT->BSRR = (1 << LED_GREEN_PIN))
#define LED_GREEN_OFF (LED_GREEN_PORT->BSRR = ((1 << LED_GREEN_PIN) << 16))
#define LED_GREEN_TOGGLE (LED_GREEN_PORT->ODR ^= (1 << LED_GREEN_PIN))
Copy link
Member

Choose a reason for hiding this comment

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

needs to be adapted to #5045.

Copy link
Member Author

Choose a reason for hiding this comment

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

I forgot to remove these lines.
Just done.

@jia200x
Copy link
Member Author

jia200x commented Mar 31, 2016

Beside of the number of lines, the header files changed common variable names and added some legacy variables that breaks compiling. I had to remove the #define DAC1 line due to the new implementation of DAC.

I will be aware if breaks again with a new change.

#define UART_1_AF 0
/** @} */

#define ADC_CONFIG { \
Copy link
Contributor

Choose a reason for hiding this comment

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

missing doxygen for the ADC block

@haukepetersen
Copy link
Contributor

I don't quite understand, why you removed the DAC1 from the vendor header. The device clearly has a DAC included, so you should not remove it. This error should be solved in a different way.

@jia200x
Copy link
Member Author

jia200x commented Mar 31, 2016

Hi.

According to the vendor header file, DAC was defined in the same way as
DAC1 due to legacy reasons. When the DAC1 was present, dac.c file from
stm32_common [1] failed at line 61. (RCC_APB1ENR_DACEN is defined, not
RCC_APB1ENR_DAC1EN).

That was the reason why I removed DAC1.
Should I instead define RCC_APB1ENR_DAC1EN?

2016-03-31 17:26 GMT+02:00 Hauke Petersen notifications@github.com:

I don't quite understand, why you removed the DAC1 from the vendor header.
The device clearly has a DAC included, so you should not remove it. This
error should be solved in a different way.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4949 (comment)

@jia200x
Copy link
Member Author

jia200x commented Mar 31, 2016

[1]
https://github.com/RIOT-OS/RIOT/blob/d411387bec067098f2692de8332d07af82973137/cpu/stm32_common/periph/dac.c

2016-03-31 17:32 GMT+02:00 Jose Alamos jialamos@uc.cl:

Hi.

According to the vendor header file, DAC was defined in the same way as
DAC1 due to legacy reasons. When the DAC1 was present, dac.c file from
stm32_common [1] failed at line 61. (RCC_APB1ENR_DACEN is defined, not
RCC_APB1ENR_DAC1EN).

That was the reason why I removed DAC1.
Should I instead define RCC_APB1ENR_DAC1EN?

2016-03-31 17:26 GMT+02:00 Hauke Petersen notifications@github.com:

I don't quite understand, why you removed the DAC1 from the vendor
header. The device clearly has a DAC included, so you should not remove it.
This error should be solved in a different way.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4949 (comment)

@haukepetersen
Copy link
Contributor

Oh, I see, then never mind the comment about the DAC...

@haukepetersen
Copy link
Contributor

would just be nice if you could squash your commits a little bit, I would suggest 1 commit for adding the CPU, 1 for editing the vendor header, 1 for adding the board, and 1 for the blacklisting...

@jia200x
Copy link
Member Author

jia200x commented Mar 31, 2016

I'm on it. Had some conflicts with rebase, but I'm fixing them.

@haukepetersen
Copy link
Contributor

looks good, just one more thing that Murdock complains about, there are some whitespace errors in the board files:


BUILD OUTPUT of group static-tests:
Current branch HEAD is up to date.
Current branch HEAD is up to date.
boards/nucleo-f072/board.c:34: new blank line at EOF.
boards/nucleo-f072/include/board.h:48: new blank line at EOF.
boards/nucleo-f072/include/periph_conf.h:136: new blank line at EOF.
boards/nucleo-f072/board.c:34: new blank line at EOF.
boards/nucleo-f072/include/board.h:48: new blank line at EOF.
boards/nucleo-f072/include/periph_conf.h:136: new blank line at EOF.
ERROR: This change introduces new whitespace errors

@jia200x
Copy link
Member Author

jia200x commented Mar 31, 2016

Done. It seems it was introducing when I tried to solve conflicts of rebase.

@haukepetersen
Copy link
Contributor

Alright, Murdock is green, code looks good, I'll make an exception here and merge this (as feature freeze is very young and the release branch is not created yet...) -> ACK and go

@haukepetersen haukepetersen merged commit 02df871 into RIOT-OS:master Mar 31, 2016
@jia200x
Copy link
Member Author

jia200x commented Mar 31, 2016

:)

@haukepetersen
Copy link
Contributor

nice work by the way!

@jia200x
Copy link
Member Author

jia200x commented Mar 31, 2016

Thank you!
Helped me a lot to understand PR<->CI-> ACK loop.

I will check for missing documentation about this.
Cheers!

@jia200x jia200x deleted the nucleo-f072 branch April 1, 2016 13:54
@OlegHahm OlegHahm modified the milestones: Release 2016.04, Release 2016.07 Apr 1, 2016
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: 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

3 participants