drivers: clock_control: stm32: don't enable RUNTIME_NMI all the time

[mathieuchopstm]

The Clock Security System (CSS) feature signals failure of an external oscillator by triggering an NMI. As such, when this feature is enabled, RUNTIME_NMI must also be enabled such that the NMI handler can be modified to point to the appropriate function.

The STM32 clock control Kconfig checks whether the CSS has been enabled in Device Tree, and forcefully selects RUNTIME_NMI if enabled since the driver code will require it. However, the check has been implemented improperly: dt_nodelabel_has_prop was used instead of dt_nodelabel_bool_prop, an error similar to using DT_NODE_HAS_PROP() instead of DT_PROP() in C code.

Since the property always exists, as long as the HSE is enabled, the RUNTIME_NMI option is always select'ed, even if not actually required.

Use the correct Kconfig function to ensure RUNTIME_NMI is select'ed only when it is required, instead of whenever HSE is enabled regardless of CSS.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[etienne-lms]

Despite your change is consistent, I don't understand why the previous implementation led to false positive detection of HSE CSS enable.
The previous implementation checked both clk_hse node being enabled and the node having property css-enabled.

[mathieuchopstm]

Despite your change is consistent, I don't understand why the previous implementation led to false positive detection of HSE CSS enable. The previous implementation checked both clk_hse node being enabled and the node having property css-enabled.

zephyr/include/zephyr/devicetree.h

Lines 3771 to 3785 in 21b20de

	/**
	* @brief Does a devicetree node have a property?
	*
	* Tests whether a devicetree node has a property defined.
	*
	* This tests whether the property is defined at all, not whether a
	* boolean property is true or false. To get a boolean property's
	* truth value, use DT_PROP(node_id, prop) instead.
	*
	* @param node_id node identifier
	* @param prop lowercase-and-underscores property name
	* @return 1 if the node has the property, 0 otherwise.
	*/
	#define DT_NODE_HAS_PROP(node_id, prop) \
	IS_ENABLED(DT_CAT4(node_id, _P_, prop, _EXISTS))

Emphasis mine: To get a boolean property's truth value, use DT_PROP(node_id, prop) instead.

My understanding is that boolean properties are treated specially: a regular property is !EXISTS if not present in DTS, and EXISTS if present (DT_PROP() returning its value), but boolean values are always EXISTS and the return value of DT_PROP() depends on whether they are absent (0) or present (1) in the DTS.

You can verify locally that RUNTIME_NMI is forcefully enabled by running west build -b nucleo_c071rb samples/hello_world -DCONFIG_RUNTIME_NMI=n which will print the following warning:

warning: RUNTIME_NMI (defined at arch/arm/core/Kconfig:160, arch/arm/core/cortex_a_r/Kconfig:64,
arch/arm/core/cortex_a_r/Kconfig:215) was assigned the value 'n' but got the value 'y'. See
http://docs.zephyrproject.org/latest/kconfig.html#CONFIG_RUNTIME_NMI and/or look up RUNTIME_NMI in
the menuconfig/guiconfig interface. The Application Development Primer, Setting Configuration
Values, and Kconfig - Tips and Best Practices sections of the manual might be helpful too.

and running west build -t menuconfig will reveal that it has been select'ed by the STM32 clock control Kconfig as I describe:

[etienne-lms]

Indeed, if 'prop' exists inside the node means the property is meaningful for that node, not it is set in the targeted node.
Quite subtle. Thanks for the details.