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

arch/arm/stm32: Basic STM32F7 family support #7220

Closed
wants to merge 21 commits into from
Closed

arch/arm/stm32: Basic STM32F7 family support #7220

wants to merge 21 commits into from

Conversation

pic16f887
Copy link

Related to issue #6981.
Added support for STM32F746XG subfamily.

Signed-off-by: Yurii Hamann yurii@hamann.site

@codecov-io
Copy link

codecov-io commented Apr 26, 2018

Codecov Report

Merging #7220 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #7220      +/-   ##
=========================================
- Coverage   55.03%     55%   -0.03%     
=========================================
  Files         477     474       -3     
  Lines       51744   51718      -26     
  Branches     9949    9935      -14     
=========================================
- Hits        28475   28447      -28     
- Misses      19306   19308       +2     
  Partials     3963    3963
Impacted Files Coverage Δ
...ycle/lifecycle_api/src/test_threads_cancel_abort.c 85.13% <0%> (-3.2%) ⬇️
tests/kernel/alert/alert_api/src/main.c 98.23% <0%> (ø) ⬆️
tests/kernel/mem_pool/mem_pool/src/main.c 92.04% <0%> (ø) ⬆️
subsys/net/ip/l2/ethernet/arp.c 62.75% <0%> (ø) ⬆️
lib/posix/pthread_common.c 75% <0%> (ø) ⬆️
include/misc/slist.h 100% <0%> (ø) ⬆️
samples/testing/integration/src/main.c 90.9% <0%> (ø) ⬆️
tests/kernel/context/src/main.c 82.17% <0%> (ø) ⬆️
.../kernel/threads/lifecycle/lifecycle_api/src/main.c 75% <0%> (ø) ⬆️
tests/kernel/pipe/pipe/src/main.c 75% <0%> (ø) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae5e11b...aa52335. Read the comment docs.

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Hi @pic16f887 ,

Thanks for this work.
Some initial comments on the pull request:

  • focus your work on uart, gpio, exti and clock-control drivers which are required for basic testing (hello_world and blinky). Clean up everything is relates to USB, I2C, SPI, .... It will ease to focus and fasten the review process.
    *Split your work into coherent commit (arch, dts, drivers/clock_control, drivers/uart, ...)
  • Last: please port a F7 board (in a dedicated commit) so the code could at least be compiled

Also, you should add your Signed-Off in the commit message

Thanks again!

config SOC_SERIES
default stm32f7

if USB
Copy link
Member

Choose a reason for hiding this comment

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

Remove USB as it is not yet tested/implemented on this series (limited to F4 for now)


endif #USB

if I2C && (I2C_1 || I2C_2 || I2C_3 || I2C_4)
Copy link
Member

Choose a reason for hiding this comment

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

Remove I2C, not yet tested/supported on F7 series


endif # GPIO_STM32

if ENTROPY_GENERATOR
Copy link
Member

Choose a reason for hiding this comment

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

Remove for now. You could add it in a follow up PR.

#define CONFIG_UART_STM32_PORT_6_IRQ_PRI ST_STM32_USART_40011400_IRQ_0_PRIORITY
#define PORT_6_IRQ ST_STM32_USART_40011400_IRQ_0

#define CONFIG_I2C_1_BASE_ADDRESS ST_STM32_I2C_V1_40005400_BASE_ADDRESS
Copy link
Member

Choose a reason for hiding this comment

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

Remove I2C, SPI, USB and flash

@@ -0,0 +1,43 @@
/*
* Copyright (c) 2018 Yurii Hamann
* Copyright (c) 2016 Linaro Limited.
Copy link
Member

Choose a reason for hiding this comment

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

Keep only your copyright, apply on all files

#include <stm32f7xx_ll_rng.h>
#endif

#ifdef CONFIG_IWDG_STM32
Copy link
Member

Choose a reason for hiding this comment

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

Remove

*
* @return 0
*/
static int st_STM32F7_init(struct device *arg)
Copy link
Member

Choose a reason for hiding this comment

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

keep lower case

* @file Header for STM32F7 pin multiplexing helper
*/

/* Port A */
Copy link
Member

Choose a reason for hiding this comment

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

Clean everything which is not UART

@@ -295,7 +295,7 @@ static int uart_stm32_init(struct device *dev)

LL_USART_Enable(UartInstance);

#if !defined(CONFIG_SOC_SERIES_STM32F4X) && !defined(CONFIG_SOC_SERIES_STM32F1X)
#if !defined(CONFIG_SOC_SERIES_STM32F4X) && !defined(CONFIG_SOC_SERIES_STM32F7X) && !defined(CONFIG_SOC_SERIES_STM32F1X)
Copy link
Member

Choose a reason for hiding this comment

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

Take care to the 80 char limit

label= "I2C_1";
};

i2c2: i2c@40005800 {
Copy link
Member

Choose a reason for hiding this comment

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

clean up i2c spi and USB


config SOC_SERIES_STM32F7X
bool "STM32F7x Series MCU"
select CPU_CORTEX_M
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should remove this. CPU_CORTEX_M is selected when you select CPU_CORTEX_M7.

Yurii Hamann added 3 commits April 27, 2018 22:26
Includes support for STM32F746xG subfamily
Related to the issue #6981

Signed-off-by: Yurii Hamann <yurii@hamann.site>
Related to issue #6982

Signed-off-by: Yurii Hamann <yurii@hamann.site>
@pic16f887 pic16f887 requested a review from dbkinder as a code owner April 27, 2018 19:35
Yurii Hamann added 6 commits April 27, 2018 22:36
Signed-off-by: Yurii Hamann <yurii@hamann.site>
Signed-off-by: Yurii Hamann <yurii@hamann.site>
Signed-off-by: Yurii Hamann <yurii@hamann.site>
Signed-off-by: Yurii Hamann <yurii@hamann.site>
Signed-off-by: Yurii Hamann <yurii@hamann.site>
Signed-off-by: Yurii Hamann <yurii@hamann.site>
@pic16f887
Copy link
Author

Hello @erwango ,
I have split the commit into smaller ones as you asked me. I added support for STM32F746G Discovery board. I tested it on on "hello_world" and "blinky" demo.

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

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

Thanks for the documentation. A few fixes...

Overview
********

The discovery kit enables a wide diversity of applications taking benefit from audio, multi-sensor support, graphics, security, video and high-speed connectivity features.
Copy link
Contributor

Choose a reason for hiding this comment

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

Preference is for lines to be <80 characters

Change the ending to read:

... security, video, and high-speed connectivity features. Important board features include:

System Clock
============

STM32F746G System Clock could be driven by internal or external oscillator,
Copy link
Contributor

Choose a reason for hiding this comment

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

lots of missing articles (the, a, an):

The STM32F746G System Clock can be driven by an internal or external oscillator,
as well as by the main PLL clock. By default, the System clock is driven by the PLL
clock at 216MHz, driven by a 25MHz high speed external clock.

Serial Port
===========

The STM32F746G Discovery kit has up to 8 UARTs. The Zephyr console output is assigned to UART1.
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect the kit has a fixed number of UARTs, so it's better to say:

The STM32F746G Discovery kit has 8 UARTs.

Copy link
Author

Choose a reason for hiding this comment

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

@dbkinder,
I am not sure that all 8 UARTs are available. Many ports are occupied by LCD display with touchscreen and other peripherals.

===========

The STM32F746G Discovery kit has up to 8 UARTs. The Zephyr console output is assigned to UART1.
Default settings are 115200 8N1.
Copy link
Contributor

Choose a reason for hiding this comment

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

Default communication settings...

========

STM32F746G Discovery kit includes an ST-LINK/V2 embedded debug tool interface.
This interface is supported by the openocd version included in Zephyr SDK.
Copy link
Contributor

Choose a reason for hiding this comment

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

in the Zephyr

* @file SoC configuration macros for the ST STM32F7 family processors.
*
* Based on reference manual:
* RM0368 Reference manual STM32F401xB/C and STM32F401xD/E
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add STM32F7 reference manuals.

#include <arm/armv7-m.dtsi>
#include <st/mem.h>
#include <dt-bindings/clock/stm32_clock.h>
#include <dt-bindings/i2c/i2c.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could remove this.

};

soc {
flash-controller@40023c00 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we support flash controller for stm32f7 family? I couldn't find it in the flash drivers. If not, please remove this node.

Copy link
Collaborator

@ydamigos ydamigos left a comment

Choose a reason for hiding this comment

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

You should cleanup your PR. Please drop the commits that you revert and fix your commits body to make shippable happy.

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

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

documentation seems to be missing now? And there are problems with the commit messages reported by shippable: (https://app.shippable.com/github/zephyrproject-rtos/zephyr/runs/14066/5/tests)

Yurii Hamann added 8 commits May 1, 2018 18:06
The patch contains basic code support, documentation, dts file
and openocd configuration for STM32F746G discovery board
Patch related to issue #6982

Signed-off-by: Yurii Hamann <yurii@hamann.site>
Includes support for STM32F746xG subfamily
Related to the issue #6981

Signed-off-by: Yurii Hamann <yurii@hamann.site>
Signed-off-by: Yurii Hamann <yurii@hamann.site>
Signed-off-by: Yurii Hamann <yurii@hamann.site>
Signed-off-by: Yurii Hamann <yurii@hamann.site>
Signed-off-by: Yurii Hamann <yurii@hamann.site>
Signed-off-by: Yurii Hamann <yurii@hamann.site>
Signed-off-by: Yurii Hamann <yurii@hamann.site>
Yurii Hamann added 2 commits May 1, 2018 18:09
The patch contains basic code support, documentation, dts file
and openocd configuration for STM32F746G discovery board
Patch related to issue #6982

Signed-off-by: Yurii Hamann <yurii@hamann.site>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants