Re: [PATCH v2 2/5] ARM: dts: stm32: add TAMP backup register in stm32mp151.dtsi

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dt-bindings: stm32: data race on TAMP registers due to dual regmap mapping
--

commit 8441cd6ab8bc051ddef5b9749ba604477c2ab5e4
Author: Simeon Marijon <simeon.marijon@foss.st.com>

ARM: dts: stm32: add TAMP backup register in stm32mp151.dtsi

This commit adds the TAMP backup registers as individual nvmem cells in the
stm32mp151 device tree. They allow information sharing between different
software components and execution levels.

> diff --git a/arch/arm/boot/dts/st/stm32mp151.dtsi b/arch/arm/boot/dts/st/stm32mp151.dtsi
> --- a/arch/arm/boot/dts/st/stm32mp151.dtsi
> +++ b/arch/arm/boot/dts/st/stm32mp151.dtsi
> @@ -2060,7 +2060,90 @@ i2c6: i2c@5c009000 {
>  
>  		tamp: tamp@5c00a000 {
>  			compatible = "st,stm32-tamp", "syscon", "simple-mfd";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
>  			reg = <0x5c00a000 0x400>;
> +			ranges;
> +
> +			nvram: nvram@5c00a100 {
> +				compatible = "st,stm32mp15-tamp-nvram";
> +				reg = <0x5c00a100 0x80>;

[Severity: High]
Does this child resource definition cause a data race by overlapping the parent
syscon's memory region?

When this node is added, the stm32-tamp-nvram driver binds to it and creates
a second independent MMIO regmap covering the physical address 0x5c00a100:

drivers/nvmem/stm32-tamp-nvram.c:stm32_tamp_nvram_probe() {
    ...
    priv->bkpregs_regmap = devm_regmap_init_mmio(dev, base_addr,
                                                 priv->data->bkpregs_regmap_cfg);
    ...
}

Because this new regmap has its own independent spinlock, it bypasses the
parent syscon regmap's lock.

If a userspace write to the NVMEM cells triggers stm32_tamp_nvram_write_byte(),
it performs a read-modify-write using its independent lock. At the same time,
other drivers like remoteproc might concurrently update the exact same
physical registers using the parent syscon regmap's lock:

drivers/remoteproc/stm32_rproc.c:stm32_rproc_set_m4_state() {
    ...
    err = regmap_update_bits(ddata->m4_state.map,
                             ddata->m4_state.reg,
                             ddata->m4_state.mask,
                             state);
    ...
}

Could this concurrent access with separate locks lead to data corruption in the
TAMP backup registers?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-upstream_tamp_nvram_dts_part-v2-0-bd73158ce46b@foss.st.com?part=2