Skip to content

Support for ST STM32 NUCLEO-G431KB Development Board #13979

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 22 commits into from
Feb 3, 2021

Conversation

taunomagnusson
Copy link
Contributor

Summary of changes

Adding support for ST STM32 NUCLEO-G431KB Development Board

Impact of changes

Not applicable

Migration actions required

Not applicable

Documentation

None


Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[X] Tests / results supplied as part of this PR


Passes all "mbed test" tests (mbedgt: test case results: 709 OK)
Verified on HW:
    Blinky program (using LD1 Onboard LED)
    GPIO Ports
    USB Device
    USART
Other Information
    Uses HSI Clock. (HSI is default for NUCLEO G431KB board - solder bridge is needed to enable HSE)
    USB connected to HSI48 Clock.

mbed_test_report.txt


Reviewers

@jeromecoutant


@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Nov 27, 2020
@ciarmcom ciarmcom requested review from jeromecoutant and a team November 27, 2020 23:00
@ciarmcom
Copy link
Member

@taunomagnusson, thank you for your changes.
@jeromecoutant @ARMmbed/team-st-mcd @ARMmbed/mbed-os-maintainers please review.

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Can you also update CMakelists.txt file? I assume one should be added to STM32G4 folder where we select this 431 target, plus also new CMakelists to TARGET_STM32G431xB. This should help you ARMmbed/mbed-os-5-docs#1381 (will be integrated soon to be available publicly in docs).

@mergify mergify bot dismissed 0xc0170’s stale review December 1, 2020 21:28

Pull request has been modified.

@taunomagnusson
Copy link
Contributor Author

taunomagnusson commented Dec 2, 2020 via email

Copy link
Collaborator

@jeromecoutant jeromecoutant left a comment

Choose a reason for hiding this comment

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

For PeripheralsPins.c, try #13992 :-)

python targets/TARGET_STM/tools/STM32_gen_PeripheralPins.py -t M40_Nucleo_NUCLEO-G431KB

@adbridge
Copy link
Contributor

adbridge commented Jan 7, 2021

@jeromecoutant @0xc0170 could you re-review please?

Copy link
Collaborator

@jeromecoutant jeromecoutant left a comment

Choose a reason for hiding this comment

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

I can't accept "USBDEVICE" in the targets.json,
as there is no USB user connector.
Move this in your local mbed_app.json

@jeromecoutant
Copy link
Collaborator

In parallel, I executed test suite, all tests are OK except,

  • drivers-usb-tests-tests-usb_device-* => see previous comment
  • hal-tests-tests-mbed_hal-sleep => to check (us ticker issue...)
  • storage-kvstore-tdbstore-tests-tests-tdbstore-whitebox => to check (issue during Multiple set test)

* | 2- USE_PLL_HSE_XTAL (external 8 MHz xtal)
* | 3- USE_PLL_HSI (internal 16 MHz)
*-----------------------------------------------------------------
* SYSCLK(MHz) | 64
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 be able to copy paste system_clock.c file from #14149

@ciarmcom ciarmcom added the stale Stale Pull Request label Jan 19, 2021
@ciarmcom
Copy link
Member

This pull request has automatically been marked as stale because it has had no recent activity. , please complete review of the changes to move the PR forward. Thank you for your contributions.

@ciarmcom ciarmcom removed the stale Stale Pull Request label Jan 19, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Jan 20, 2021

@taunomagnusson Would you be able to respond to the review or shall this be closed?

@taunomagnusson
Copy link
Contributor Author

Hi - I will fix it. Some other things that took priority. Expect a update in a few days.

@mergify mergify bot removed the needs: work label Jan 25, 2021
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Just one cosmetic change, otherwise LGTM

@mergify mergify bot added needs: work and removed needs: CI labels Jan 25, 2021
* | 3- USE_PLL_HSI (internal 16 MHz)
*-----------------------------------------------------------------
* SYSCLK(MHz) | 160 (default configuration) / 170 (CAN disabled)
* USB capable | NO
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 set as YES ?
(maybe with comment "to be set in mbed_app.json" ?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, you removed the code for that.
So as you want, you can add back clock configuration for USB, or not.

Copy link
Contributor Author

@taunomagnusson taunomagnusson Jan 26, 2021

Choose a reason for hiding this comment

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

Ok, I'll add back the clock config then! Then it's just a targets.json edit to get the USB Device up and running.
Will modify to YES and add some comments. Need to test it a bit.

Copy link
Contributor Author

@taunomagnusson taunomagnusson Jan 26, 2021

Choose a reason for hiding this comment

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

Actually, I see now that the RCC clock is connected to HSI48 in trng_api.c/trng_init using HAL_RCCEx_PeriphCLKConfig.

However, there is no similar construct in the init phase for USB (USBPhyHw.h/USBPhy_STM32.cpp).

I see that RCC_CCIPR/CLK48SEL resets to 00 (HSI48 Selected) though, so if it is preserved after reset USB should work without a HAL_RCCEx_PeriphCLKConfig statement in system_clock.c (or in USBPhy_STM32.cpp).

But is that good practice? Or better to keep the HAL_RCCEx_PeriphCLKConfig enabling statement in either system_clock.c or USBPhy_STM32.cpp? Note that RCC clock is also selected on CLK48SEL, and it adds a HAL_RCCEx_PeriphCLKConfig (in trng_api.c) to enable it in its init procedure.

In addition, several other boards have a CLK48SEL enabling statement using HAL_RCCEx_PeriphCLKConfig in their system_clock.c (DISCO/NUCLEO_F413ZH, NUCLEO_F446RE/ZE, DISCO_F469NI, ...).

Anyway, I've added the HAL_RCCEx_PeriphCLKConfig for USB Clock in G431KB/system_clock.c. I'd recommend this - if you are ok with that you might want to consider it for 474RE as well (to keep the code aligned).

@mergify mergify bot dismissed 0xc0170’s stale review January 26, 2021 01:18

Pull request has been modified.

…on. Also added so that the CK48 Clock Mux selects HSI48 for both RNG and USB. Without these the RNG/USB will be clocked at PLLQ (170 MHz) and not work.
Tauno Magnusson added 2 commits January 26, 2021 03:27
…e HSI48 CLK for USB. Assumption is that the STM32G4 reset value (defaults to HSI48 selected for USB) is preserved throughout Mbed initialization.
…ard platforms (NUCLEO_F413ZH, ...)) it appears that even though the reset value defaults correctly it may be good practice to keep the HAL_RCCEx_PeriphCLKConfig statement that selects the HSI48CLK as source for USB/RNG.
@ciarmcom ciarmcom added the stale Stale Pull Request label Jan 28, 2021
@ciarmcom
Copy link
Member

This pull request has automatically been marked as stale because it has had no recent activity. @ARMmbed/mbed-os-maintainers, please start CI to get the PR merged.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 2, 2021

CI started

@mbed-ci
Copy link

mbed-ci commented Feb 2, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-test ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️

@0xc0170 0xc0170 merged commit f7ed864 into ARMmbed:master Feb 3, 2021
@mergify mergify bot removed the ready for merge label Feb 3, 2021
@0xc0170 0xc0170 removed the stale Stale Pull Request label Feb 10, 2021
@mbedmain mbedmain added release-version: 6.8.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants