Rework STM32H7x clocking configuration

[multiplemonomials]

Summary of changes

I started this effort to track down a bug that USC RPL was seeing, where the STM32H743 processor was unable to reliably output UART data when set to use the internal oscillator. I suspected some sort of clock issue, but wasn't quite prepared for the whole nest of bugs I'd find when I lifted up the rock of STM32H7xx clocking.

Significant Issues:

Overclocking whether you want it or not

Something that STMicro doesn't put front and center about STM32H7x devices is that the stated speeds of chips in the family are technically "overclocks" -- e.g. the STM32H743 is advertised as a 480MHz processor, but can't actually sustain that speed in all power configurations (e.g. external SMPS) or temperature ranges. This is okay, as there are some applications that will care more about temperature range / power sources and others that will care more about raw speed. However, developers need some method to control this, and right now that isn't really possible.

This PR adds a new "target.enable-overdrive-mode" option which can be used to control the overdrive mode. In most cases, it defaults to on, meaning the clocks stay the same as before. However, for boards that use SMPS mode, I made sure to default it to off so that we are no longer running the boards in a technically invalid clock configuration.

Voltage scale settings

STM32H7 chips have a number of different settings for the core voltage regulator -- VOS0 (highest voltage) through VOS3 (lowest voltage) are common for normal operation. The datasheet quite clearly specifies that in order to use overdrive mode, you have to set the regulator to VOS0 range. However, precisely none of the system_clock.c files had this correct, and all were using VOS1. This means that we have been undervolting the CPU cores on all STM32H7x chips up until now! Frankly I'm not sure how we got away with this for this long but it needs to be fixed to prevent potential crashes & undefined behavior.

In this PR I updated all the system_clock.c files to use VOS0 in overdrive mode and VOS1 in non-overdrive mode.

AnalogIn failure to init on HSI oscillator

The existing STM32H7 ADC code initializes PLL2 to produce the 60MHz ADC clock. However, the code makes an implicit assumption that the PLL clock source is an 8MHz oscillator, so if you use the 64MHz HSI or an external oscillator faster than 8MHz, the ADC will not initialize correctly (I noticed this when running the arduino-pinmap-verification test under HSI oscillator).

If we wanted to get very fancy with it, we'd have to update this code to have similar logic to system_clock.c and take into account the current external oscillator and its frequency when configuring the PLL. However, I think that for now the easier option is to just clock the ADC off of the HSI oscillator and run it at 32MHz. This makes the clocking code an order of magnitude simpler at the cost of somewhat reducing the ADC speed.

However, I'd argue that anyone interested in very, very fast ADC measurements (MHz or greater) is not going to be using the Mbed API in the first place -- they are probably going to be using DMA and conversion sequences and all that jazz, and they would probably end up using STM32Cube IDE or another library to do it. So I think that a simple, reliable clock setup for the ADC when used through Mbed is not a bad idea. This is especially true considering that the datasheets leave some question as to what the max acceptable operating frequency is for the ADC, and STM32Cube disagrees with the datasheet (see here)

HSI oscillator calibration

As for the issue I started this project to fix, I eventually tracked that down (via comparing STM32Cube generated code) to one missing line in the system_clock.c file:

RCC_OscInitStruct.HSICalibrationValue = RCC_HSICALIBRATION_DEFAULT;

Without this line, I believe that the default zero value for HSICalibrationValue causes the HSI oscillator to be WAY out of trim, to the point that UART does not work reliably. I have now gone through and made sure that all the HSI oscillator configs have this.

Minor Fixes

Wider range of input frequencies

In my experience, 8MHz TCXOs are weirdly hard to source, so there have been many boards where I've had to mess around with Mbed clocking configuration guts just so that I could use a different HSE oscillator frequency. I say it's time to fix this issue once and for all. Using the amazing power of the new Division Operator (TM), we can fix it so that the PLL configuration dynamically updates itself for frequencies in a certain range (4-50MHz) and divisible by certain values (2 or 5). This should make it a snap for projects to set the frequency without using a custom target as they can just set the "target.hse_value" option.

Missing & duplicated system_clock.c files

A few of the STM32H7xx targets, e.g. STM32H753, were outright missing system_clock.c files. Additionally, system_clock.c files with virtually the same content were duplicated between several targets, making it easy for bugfixes to be missed for some targets while updating others.

I have broken it up so that there is now one system_clock.c file for each nominal max frequency (e.g. STM32H74x/75x are all 480MHz, STM32H72x/73x are all 550MHz, STM32H7Ax/Bx are all 280MHz), and each MCU target has a label indicating what frequency it's designed for. This cuts down on the duplication by quite a lot, and it also ensures that each STM32H7x MCU always has a system_clock.c file associated with it.

I realize that system_clock.c duplication is probably an issue for all the STM32 targets, but I figure we gotta start somewhere! Hopefully future STM targets added can use a similar setup.

Missing I2C timing values for some configurations.

Until now, the STM32H7x code only contained I2C timing values for MCUs running at 480MHz (I2C peripheral at 120MHz). This meant that MCUs in this line that did not run at 480MHz would always get an "assertion failed, enable I2C timing value algo" error when you tried to use I2C. I have added the missing timing value constants, and also a second set of constants for when overdrive is disabled.

Note that an alternative having to keep adding these constants could be running I2C off of CLKPER which is 64MHz derived from HSI, which is at a fixed frequency regardless of core clock. But I didn't want to change this without a good reason to.

Standardized HSE Value Option

Now, every STM32H7x MCU implements the "target.hse_value" option, via the same code. This significantly cuts down on duplicated code in targets.json and also makes the user experience better as the same option always works on any STM32H7x MCU.

Impact of changes

• Fix for potential STM32H7xx stability issue when running at full frequency
• Fix ADC not initializing on STM32H7xx when running on HSI oscillator or using higher HSE value
• Fix STM32H7xx UART not working reliably when running from HSI
• Enable STM32H7xx MCUs to use a much wider range of hse_value options
• Enable target.hse_value option for all STM32H7xx MCUs
• Fix STM32H7xx MCUs other than STM32H74x being unable to use I2C with default configuration
• Add target.enable-overdrive-mode option to control STM32H7xx overdrive/overclocking

Migration actions required

Documentation

None (options added have documentation)

---

Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[X] 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)
[X] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

I ran Greentea locally. With this PR, a NUCLEO_H743ZI2 in HSI mode can now initialize the ADC and communicate over UART.

---

Reviewers

@jeromecoutant @0xc0170

---

[agausmann]

I think MCU_STM32H743xI also needs the label STM32H7_480MHZ? That is currently missing from targets.json, and it seems to cause a linker error undefined reference to 'SetSysClock'

I would make a proper code suggestion but github doesn't let you write comments outside of the diff context it shows.

mbed-os/targets/targets.json

Lines 3297 to 3312 in 4e1f335

	"MCU_STM32H743xI": {
	"inherits": [
	"MCU_STM32H7"
	],
	"public": false,
	"core": "Cortex-M7FD",
	"extra_labels_add": [
	"STM32H743xI"
	],
	"macros_add": [
	"STM32H743xx"
	],
	"overrides": {
	"system_power_supply": "PWR_LDO_SUPPLY"
	}
	},

[0xc0170]

@jeromecoutant please can you review again?

[mbed-ci]

Jenkins CI Test : ❌ FAILED

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

CLICK for Detailed Summary

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

[0xc0170]

@multiplemonomials Please review the logs, there are build failures.

[multiplemonomials]

@0xc0170 try it again, fixed an issue with Portenta

Pull request has been modified.

[0xc0170]

CI restarted

[mbed-ci]

Jenkins CI Test : ❌ FAILED

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

CLICK for Detailed Summary

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

[jeromecoutant]

Why do you remove this file ?

[multiplemonomials]

Now the overriding that was done in this file can be done in targets.json, because now the STM32H7 system_clock.c files support a wide range of HSE_VALUEs

[jeromecoutant]

But you change default configuration for this portenta board ...

[multiplemonomials]

But, I added new stuff to targets.json that should replicate what this clock override file was doing

[multiplemonomials]

@0xc0170 give it another shot!

[multiplemonomials]

@0xc0170 could you restart CI please?

[mbed-ci]

Jenkins CI Test : ❌ FAILED

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

CLICK for Detailed Summary

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

[multiplemonomials]

@0xc0170 None of the tests that failed are related to STM32H7, so it seems like they are random or unrelated failures. This should be OK to merge!

[0xc0170]

Yes, that is correct, merging now.