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

STM32F7 SoC family and STM32F746G Discovery board basic support #7284

Merged
merged 9 commits into from
Jun 28, 2018

Conversation

pic16f887
Copy link

@erwango, @ydamigos, @dbkinder

I have closed pull request #7220 and created a new one. I had to drop previous PR because it was a mess which is hard to resolve to me (I am newbie in git and GitHub). Sorry for inconvenience.

@codecov-io
Copy link

codecov-io commented May 1, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7284      +/-   ##
==========================================
- Coverage   64.61%   64.54%   -0.08%     
==========================================
  Files         421      420       -1     
  Lines       40296    40117     -179     
  Branches     6803     6762      -41     
==========================================
- Hits        26037    25892     -145     
+ Misses      11126    11108      -18     
+ Partials     3133     3117      -16
Impacted Files Coverage Δ
boards/posix/native_posix/irq_handler.c 80.24% <0%> (-4.64%) ⬇️
subsys/net/ip/tcp.c 55.24% <0%> (-3.58%) ⬇️
kernel/queue.c 82.4% <0%> (-2.55%) ⬇️
subsys/net/ip/l2/ethernet/arp.c 62.75% <0%> (-0.98%) ⬇️
subsys/net/ip/dhcpv4.c 5.72% <0%> (-0.29%) ⬇️
kernel/sched.c 81.17% <0%> (-0.24%) ⬇️
subsys/net/ip/icmpv6.c 34.72% <0%> (-0.13%) ⬇️
subsys/net/ip/connection.c 78.35% <0%> (-0.06%) ⬇️
.../sched/schedule_api/src/test_priority_scheduling.c 100% <0%> (ø) ⬆️
include/drivers/bluetooth/hci_driver.h 0% <0%> (ø) ⬆️
... and 25 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 fb6f9b7...7c027e5. Read the comment docs.

- DMA Controller

More information about STM32F746NGH6 can be found here:
- `STM32F746NGH6 on www.st.com`_
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a blank line before the first list item. Also, don't indent, so should look like this:

More information about STM32F746NGH6 can be found here:

- `STM32F746NGH6 on www.st.com`_
- `STM32F74xxx reference manual`_


Other hardware features are not yet supported on Zephyr porting.

The default configuration can be found in the defconfig file:
Copy link
Contributor

Choose a reason for hiding this comment

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

This renders better written as:

The default configuration can be found in the defconfig file:
``boards/arm/stm32f746g_disco/stm32f746g_disco_defconfig``

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 have fixed it.

@pic16f887 pic16f887 force-pushed the master branch 6 times, most recently from 1c3b4b3 to ca60dac Compare May 1, 2018 19:00
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.

One more edit, and you've got some problems reported by shippable: https://app.shippable.com/github/zephyrproject-rtos/zephyr/runs/14153/5/tests


More information about STM32F746NGH6 can be found here:

- `STM32F746NGH6 on www.st.com`_
Copy link
Contributor

@dbkinder dbkinder May 1, 2018

Choose a reason for hiding this comment

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

These two lines still need to be not indented...

Copy link
Author

Choose a reason for hiding this comment

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

Shippable prints warnings like this:
"B4 Second line is not empty". Does this mean that commit message body should have only one line?
Also there is one failed test, but I don't understand how check which exactly test was failed. Are these tests from sanity check?

@ydamigos
Copy link
Collaborator

ydamigos commented May 2, 2018

Shippable prints warnings like this:
"B4 Second line is not empty". Does this mean that commit message body should have only one line?

@pic16f887 It means you need an empty line between commit message header and body.
The commit messages should follow the format:

Commit message header
--empty line--
Commit message body (multiple lines)
--empty line
Signed-off-by: ...

Please also check: http://docs.zephyrproject.org/1.9.0/contribute/contribute_guidelines.html#commit-guidelines

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.

Additional remarks but it goes in good direction.

General points:

  • Add at least a one line comment in your commit messages, otherwise CI will fail
  • Enhance commit separation (lot of gpio, clock, uart related code ) in initial commit

Specific for Clock control:
You need to take car of the code under ifdef F4X in stm32_ll_clock.c, it is applicable to F7 as well.


config NUM_IRQS
int
default 97
Copy link
Member

Choose a reason for hiding this comment

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

98 (when counting IRQ 0)

} bit;
};

/* 3.8.7 Embedded flash registers */
Copy link
Member

Choose a reason for hiding this comment

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

3.7

*
* Based on reference manual:
*
* Chapter 3.4: Embedded Flash Memory
Copy link
Member

Choose a reason for hiding this comment

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

Chapter 3:

*/

/* 6.4 GPIO registers - each GPIO port controls 16 pins */
struct stm32f7x_gpio {
Copy link
Member

Choose a reason for hiding this comment

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

Please use exact names as mentioned in Reference manual:
moder, otyper, ospeedr, ....

Copy link
Author

@pic16f887 pic16f887 May 2, 2018

Choose a reason for hiding this comment

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

In my opinion names like moder aren't human readable. I think it would better to left it like for STMF4 series or add ending reg, for example mode_reg.

Copy link
Member

Choose a reason for hiding this comment

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

I think that we should use naming rules that are not subjective, and then will not vary depending on the developer.
Hence, when possible, aligning on ref manual is the better option to me.

irq_unlock(key);

/* Update CMSIS SystemCoreClock variable (HCLK) */
/* At reset, system core clock is set to 25 MHz from HSI */
Copy link
Member

Choose a reason for hiding this comment

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

HSI is 16MHz

#include <stm32f7xx_ll_bus.h>
#include <stm32f7xx_ll_rcc.h>
#include <stm32f7xx_ll_system.h>
#include <stm32f7xx_ll_spi.h>
Copy link
Member

Choose a reason for hiding this comment

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

Remove spi


#define CONFIG_NUM_IRQ_PRIO_BITS ARM_V7M_NVIC_E000E100_ARM_NUM_IRQ_PRIORITY_BITS

#define CONFIG_UART_STM32_PORT_1_BASE_ADDRESS ST_STM32_USART_40011000_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.

Move these to the commit where you introduce serial driver

@pic16f887 pic16f887 closed this May 2, 2018
@pic16f887 pic16f887 reopened this May 2, 2018
@erwango erwango added the platform: STM32 ST Micro STM32 label May 3, 2018
@erwango
Copy link
Member

erwango commented May 3, 2018

Seems we've lost some of your commits ..

@pic16f887
Copy link
Author

I dropped old commits. I attached only base STM32F7 commit. I will attach rest of updated commits soon.

@pic16f887 pic16f887 force-pushed the master branch 2 times, most recently from 13b1db7 to a164477 Compare May 4, 2018 05:00
@pic16f887
Copy link
Author

pic16f887 commented May 7, 2018

I committed updated code.

@tagunil
Copy link
Collaborator

tagunil commented May 7, 2018

@erwango, what is the policy for IRQ handlers for EXTI >= 16? F4 (and now F7) port enables them in stm32_exti_enable, other families do not. Which way should it be unified?

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.

one more edit and we're good to go

- On-board ST-LINK/V2-1 supporting USB re-enumeration capability
- Five power supply options:

- ST LINK/V2-1
Copy link
Contributor

Choose a reason for hiding this comment

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

The sublist should be indented to the first character of the text of the parent list:

- Five power supply options:

  - ST LINK/V2-1

If you indent it too much, it changes the way the sublist is rendered.

Copy link
Author

Choose a reason for hiding this comment

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

I have fixed it and updated the commit.

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!

@pic16f887
Copy link
Author

@erwango,
Could you please review my latest changes?

* Tick interrupt priority is not used
* @return HAL status
*/
uint32_t HAL_GetTick(void)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you remove this function? It is now provided by arch/arm/soc/st_stm32/common/stm32cube_hal.c

@erwango
Copy link
Member

erwango commented Jun 13, 2018

@pic16f887, can you rebase on top of just merged #7997 ?

@pic16f887
Copy link
Author

@erwango
I have rebased the code.

@@ -521,6 +559,59 @@ static void __stm32_exti_connect_irqs(struct device *dev)
CONFIG_EXTI_STM32_RTC_WKUP_IRQ_PRI,
__stm32_exti_isr_22, DEVICE_GET(exti_stm32),
0);
+#elif CONFIG_SOC_SERIES_STM32F7X
Copy link
Collaborator

Choose a reason for hiding this comment

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

The shippable build failed due to this leftover from the rebase. Same issue a few lines below.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you. I have update the code.

config GPIO_STM32_PORTK
default y

+endif # GPIO_STM32
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just realized there is still a small rebase issue, there is a + before the endif.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you. I have updated the code.

@aurel32
Copy link
Collaborator

aurel32 commented Jun 18, 2018

Hi @pic16f887, following the merge of #8359, this patchset needs yet another rebase :-(. In the patch serial: stm32: STM32F7 UART support, the part changing drivers/serial/uart_stm32.c should simply be dropped.

Yurii Hamann added 9 commits June 18, 2018 22:47
The patch includes support for STM32F746xG subfamily.
Related to issue zephyrproject-rtos#6981.

Signed-off-by: Yurii Hamann <yurii@hamann.site>
This patch adds clock control support for STM32F7 family
microcontrollers.

Signed-off-by: Yurii Hamann <yurii@hamann.site>
Added flash memory support for STM32F7 family microcontrollers

Signed-off-by: Yurii Hamann <yurii@hamann.site>
This patch adds GPIO support for STM32F7 family microcontrollers.

Signed-off-by: Yurii Hamann <yurii@hamann.site>
This patch adds pinmux header file for STM32F7 famlily
microcontrollers

Signed-off-by: Yurii Hamann <yurii@hamann.site>
The patch adds serial driver support for STM32F7 family
microcontrollers, includes pinmux definitions and DTS fixup file.

Signed-off-by: Yurii Hamann <yurii@hamann.site>
This patch adds EXTI support for STM32F7 family
microcontrollers.

Signed-off-by: Yurii Hamann <yurii@hamann.site>
This patch includes:
STM32F7 family device tree file with basic and UART definitions.
STM32F746 subfamily device tree file.
Memory definitions for STM32F746xG subfamily.

Signed-off-by: Yurii Hamann <yurii@hamann.site>
The patch contains basic code support, documentation, dts
file and openocd configuration for STM32F746G discovery board.
This patch related to the issue zephyrproject-rtos#6982.

Signed-off-by: Yurii Hamann <yurii@hamann.site>
@pic16f887
Copy link
Author

Hi @aurel32,
I have rebased the code.

@aurel32
Copy link
Collaborator

aurel32 commented Jun 19, 2018

Hi @pic16f887. Thanks for the rebase, I have just checked, it's all good.

@galak galak merged commit e69c058 into zephyrproject-rtos:master Jun 28, 2018
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.

9 participants