Skip to content

boards: arm: add support for stm32f072b_disco board #5405

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

Merged
merged 2 commits into from
Jan 26, 2018
Merged

boards: arm: add support for stm32f072b_disco board #5405

merged 2 commits into from
Jan 26, 2018

Conversation

dwagenk
Copy link
Contributor

@dwagenk dwagenk commented Dec 15, 2017

No description provided.

@SebastianBoe SebastianBoe removed their request for review December 15, 2017 09:08
@dwagenk
Copy link
Contributor Author

dwagenk commented Dec 15, 2017

shippable returns unstable with linking errors in some tests (e.g. tests/net/ieee802154/l2/test, tests/net/lib/mqtt_subscriber/test), because of small SRAM. I expected
ram: 16
at stm32f072b_disco.yaml to prevent the tests beeing run for this target.

How should this be solved?

@galak galak self-assigned this Dec 15, 2017
@galak galak added area: ARM ARM (32-bit) Architecture area: Boards platform: STM32 ST Micro STM32 labels Dec 15, 2017
@galak
Copy link
Collaborator

galak commented Dec 15, 2017

shippable returns unstable with linking errors in some tests (e.g. tests/net/ieee802154/l2/test, tests/net/lib/mqtt_subscriber/test), because of small SRAM. I expected
ram: 16
at stm32f072b_disco.yaml to prevent the tests beeing run for this target.

How should this be solved?

For most of the smaller ram boards we've done something like the following in BOARD.yaml

testing:
  ignore_tags:
    - net
    - bluetooth

See boards/arm/frdm_kl25z/frdm_kl25z.yaml as an example

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 formatting issues


- Six LEDs:

- LD1 (red/green) for USB communication
Copy link
Contributor

Choose a reason for hiding this comment

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

This sublist looks better when it's not indented so much, 4 spaces is sufficient. Otherwise the html that's generated has a vertical bar added on the left because it looks like a quoted text block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually I had to go down to 2 Spaces to remove the quote-bar.

dbkinder
dbkinder previously approved these changes Dec 18, 2017
@dwagenk
Copy link
Contributor Author

dwagenk commented Dec 19, 2017

fixup:
I missed, that doc and pinmux.c were out of sync.
Added I2C_1 to pinmux.c and removed I2C_2 from doc.

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.

SoC STM32F072xB is being introduced as part of #5171 which should be close to merge.
I think you should wait it is merged and rebase on top of it.
Otherwise LGTM

@superna9999
Copy link
Collaborator

Yep, I think also, you also should enable FLASH in the defconfig.

Apart that, LGTM

superna9999
superna9999 previously approved these changes Dec 21, 2017
@galak galak force-pushed the arm branch 2 times, most recently from ee5b12f to f95e5fd Compare January 11, 2018 22:07
@galak
Copy link
Collaborator

galak commented Jan 17, 2018

Can you rebase this?

@dwagenk
Copy link
Contributor Author

dwagenk commented Jan 18, 2018

@galak i'm waiting for #5171 to get merged. will rebase and clean up then.

@dwagenk dwagenk changed the base branch from arm to master January 18, 2018 11:09
@dwagenk
Copy link
Contributor Author

dwagenk commented Jan 18, 2018

rebased onto #5171 that introduces stm32f072xb SoC.

@superna9999 I removed FLASH from the documentation, since I didn't test it.

@dwagenk
Copy link
Contributor Author

dwagenk commented Jan 18, 2018

recheck

@codecov-io
Copy link

codecov-io commented Jan 18, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5405      +/-   ##
==========================================
- Coverage   54.37%   54.33%   -0.05%     
==========================================
  Files         457      457              
  Lines       43298    43298              
  Branches     8302     8302              
==========================================
- Hits        23545    23524      -21     
- Misses      19614    19635      +21     
  Partials      139      139
Impacted Files Coverage Δ
...sts/net/ieee802154/l2/src/ieee802154_fake_driver.c 74.35% <0%> (-15.39%) ⬇️
include/net/net_ip.h 67.36% <0%> (-4.87%) ⬇️
subsys/net/ip/6lo.c 38.73% <0%> (-2.79%) ⬇️
tests/kernel/obj_tracing/src/philosopher.c 88.88% <0%> (+16.66%) ⬆️

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 f5d64aa...f5c3dbc. Read the comment docs.

@dwagenk
Copy link
Contributor Author

dwagenk commented Jan 18, 2018

do not merge

I had some issues with the uart configuration right now and want to look into it tomorrow


Edit:
Errors resolved, please review and marge after #5171

Background: UART2 is not really usable on this board. Pinmuxing it to PA14/PA15 works sometimes, but interferes with programming/debugging port and PA2/PA3 are occupied by linear touch sensor.
So I removed UART2 from board support commit completely.

@dwagenk
Copy link
Contributor Author

dwagenk commented Jan 26, 2018

now that #5171 is merged this should be ready to get merged as well.

@ydamigos can you please take a look at the SPI configuration through device tree? SPI_loopback test works, but I'm not familiar with dts, so I stay a little uncertain, if all is correct. I basically copied your work from 96b_carbon board and adjustet the dts.fixup file to use the SPI_FIFO versions instead.

@superna9999 @erwango @dbkinder can you please take a look at it again?

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.

LGTM. Some minor non blocking comments. I don't have the hardware to test it.

CONFIG_BUILD_TIMESTAMP=y
CONFIG_SYS_LOG=y
CONFIG_SPI=y
CONFIG_SPI_LEGACY_API=n
Copy link
Collaborator

Choose a reason for hiding this comment

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

It isn't needed. It defaults to n in arm/soc/st_stm32/common/Kconfig.defconfig.series when SPI and one SPI_x port are enabled.

CONFIG_SPI=y
CONFIG_SPI_LEGACY_API=n
CONFIG_SYS_LOG_SPI_LEVEL=1
CONFIG_SPI_STM32=y
Copy link
Collaborator

Choose a reason for hiding this comment

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

It isn't needed. It defaults to y in arm/soc/st_stm32/common/Kconfig.defconfig.series when SPI and one SPI_x port are enabled.

CONFIG_SPI_LEGACY_API=n
CONFIG_SYS_LOG_SPI_LEVEL=1
CONFIG_SPI_STM32=y
CONFIG_SPI_STM32_INTERRUPT=y
Copy link
Collaborator

Choose a reason for hiding this comment

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

It isn't needed. It defaults to y in boards Kconfig.defconfig when SPI is enabled.

CONFIG_SYS_LOG_SPI_LEVEL=1
CONFIG_SPI_STM32=y
CONFIG_SPI_STM32_INTERRUPT=y
CONFIG_SPI_1=y
Copy link
Collaborator

Choose a reason for hiding this comment

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

It isn't needed. It defaults to y in boards Kconfig.defconfig when SPI is enabled.

@erwango
Copy link
Member

erwango commented Jan 26, 2018

@dwagenk : Approved, but please take into account @ydamigos comments

@dwagenk
Copy link
Contributor Author

dwagenk commented Jan 26, 2018

@erwango @ydamigos what do you think of moving those changes to a seperate PR that cleans up the spi_loopback test?
It would effect also those files in tests\drivers\spi\spi_loopback:

@ydamigos
Copy link
Collaborator

what do you think of moving those changes to a seperate PR that cleans up the spi_loopback test?

@dwagenk It's fine with me.

@erwango
Copy link
Member

erwango commented Jan 26, 2018

what do you think of moving those changes to a seperate PR that cleans up the spi_loopback test?

@dwagenk : Sure, go ahead

@dwagenk
Copy link
Contributor Author

dwagenk commented Jan 26, 2018

I won't have time for a follow up pr for the spi_loopback test before monday, but would like to get this closed soon (it's been open quite some time). Can you merge it now anyhow, or at least the two first commits (without adding the board to spi_loopback test)?

@erwango
Copy link
Member

erwango commented Jan 26, 2018

Can you merge it now anyhow, or at least the two first commits (without adding the board to spi_loopback test)?

@dwagenk: Maybe you can just remove last commit.

Daniel Wagenknecht added 2 commits January 26, 2018 09:21
Adds pinmux defines to use I2C2 at PB10/PB11 for
stm32f0-based boards.
Needed for stm32f072b_disco board to use extension
connector

Signed-off-by: Daniel Wagenknecht <[email protected]>
Support the ST STM32F072B-DISCO discovery board with
STM32F072RB SoC

Signed-off-by: Daniel Wagenknecht <[email protected]>
@galak
Copy link
Collaborator

galak commented Jan 26, 2018

I pushed an update and dropped the SPI patch so we can merge this board port

@dwagenk
Copy link
Contributor Author

dwagenk commented Jan 29, 2018

@galak thanks!

@dwagenk dwagenk deleted the stm32f072b_disco branch February 15, 2018 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARM ARM (32-bit) Architecture area: Boards platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants