Re: [PATCH 09/10] arm64: dts: st: Add support for DH electronics STM32MP23xx/STM32MP25xx DHCOS SoM and Breakout Board and DHSBC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] SoC hardware mismatch between included DTSI and root compatible string
- [High] Incorrect SPI chip select polarity (GPIO_ACTIVE_HIGH) breaks SPI communication
- [Low] Dangling uart8 alias in common DTSI file
--

--- Patch [9]: [PATCH 09/10] arm64: dts: st: Add support for DH electronics STM32MP23xx/STM32MP25xx DHCOS SoM and Breakout Board and DHSBC ---
commit 703b623c315fcfbe4a6aecf3454ed4e9fa856d0e
Author: Marek Vasut <marex@nabladev.com>

arm64: dts: st: Add support for DH electronics STM32MP23xx/STM32MP25xx DHCOS SoM and Breakout Board and DHSBC

This commit adds device tree definitions for the DH electronics DHCOS 
system-on-module based on STM32MP25xx and STM32MP23xx SoCs. It also includes 
support for the corresponding Breakout Board and DHSBC carrier boards.

> diff --git a/arch/arm64/boot/dts/st/stm32mp23xx-dhcos-bb.dts b/arch/arm64/boot/dts/st/stm32mp23xx-dhcos-bb.dts
> new file mode 100644
> index 0000000000000..125c76fe3e7be
> --- /dev/null
> +++ b/arch/arm64/boot/dts/st/stm32mp23xx-dhcos-bb.dts
> @@ -0,0 +1,15 @@
[ ... ]
> +#include "stm32mp235.dtsi"
> +#include "stm32mp23xc.dtsi"
> +#include "stm32mp23xx-dhcos-som.dtsi"
> +
> +/ {
> +	model = "DH electronics STM32MP23xx DHCOS Breakout Board";
> +	compatible = "dh,stm32mp231a-dhcos-bb", "dh,stm32mp231a-dhcos-som", "st,stm32mp231";

[Severity: High]
Does this compatible string mismatch the included DTSI?

This file includes stm32mp235.dtsi, which usually defines additional hardware 
capabilities like secondary CPUs, but sets the root compatible to st,stm32mp231. 
If the board physically has a single-core SoC, could the kernel attempt to 
initialize non-existent hardware and fail?

> +};

> diff --git a/arch/arm64/boot/dts/st/stm32mp255c-dhcos-dhsbc.dts b/arch/arm64/boot/dts/st/stm32mp255c-dhcos-dhsbc.dts
> new file mode 100644
> index 0000000000000..1d750d2787d6f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/st/stm32mp255c-dhcos-dhsbc.dts
[ ... ]
> +&spi1 {
> +	pinctrl-names = "default", "sleep";
> +	pinctrl-0 = <&spi1_pins_a>;
> +	pinctrl-1 = <&spi1_sleep_pins_a>;
> +	cs-gpios = <&gpioh 3 0>;

[Severity: High]
Does this cs-gpios definition use the correct polarity?

The value 0 (GPIO_ACTIVE_HIGH) is specified, but standard SPI devices, 
including the st33htpm-spi TPM attached below, typically require an active-low 
chip select. Could this active-high polarity break communication with the TPM?

> +	status = "okay";
> +
> +	st33htph: tpm@0 {
> +		compatible = "st,st33htpm-spi", "tcg,tpm_tis-spi";
> +		reg = <0>;
[ ... ]
> +&spi8 {
> +	pinctrl-names = "default", "sleep";
> +	pinctrl-0 = <&spi8_pins_b>;
> +	pinctrl-1 = <&spi8_sleep_pins_b>;
> +	cs-gpios = <&gpioz 6 0>;

[Severity: High]
Is the chip select polarity correct here as well?

This also specifies 0 (GPIO_ACTIVE_HIGH) for the SPI8 chip select.

> +};

> diff --git a/arch/arm64/boot/dts/st/stm32mp25xx-dhcos-bb.dts b/arch/arm64/boot/dts/st/stm32mp25xx-dhcos-bb.dts
> new file mode 100644
> index 0000000000000..cf66e8e48c99a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/st/stm32mp25xx-dhcos-bb.dts
> @@ -0,0 +1,15 @@
[ ... ]
> +#include "stm32mp255.dtsi"
> +#include "stm32mp25xc.dtsi"
> +#include "stm32mp25xx-dhcos-som.dtsi"
> +
> +/ {
> +	model = "DH electronics STM32MP25xx DHCOS Breakout Board";
> +	compatible = "dh,stm32mp251a-dhcos-bb", "dh,stm32mp251a-dhcos-som", "st,stm32mp251";

[Severity: High]
Does this compatible string mismatch the included DTSI?

Similar to the mp23xx board, this includes stm32mp255.dtsi but declares the
compatible string as st,stm32mp251.

> +};

> diff --git a/arch/arm64/boot/dts/st/stm32mp2xxx-dhcos-som.dtsi b/arch/arm64/boot/dts/st/stm32mp2xxx-dhcos-som.dtsi
> new file mode 100644
> index 0000000000000..330c7c6768b6c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/st/stm32mp2xxx-dhcos-som.dtsi
> @@ -0,0 +1,442 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> +/*
> + * Copyright (C) 2025-2026 Marek Vasut <marex@nabladev.com>
> + */
> +
> +/ {
> +	aliases {
> +		mmc0 = &sdmmc2;
> +		mmc1 = &sdmmc3;
> +		serial0 = &usart6;
> +		serial1 = &uart8;

[Severity: Low]
Does this create a dangling alias on STM32MP23xx platforms?

Since this is a common DTSI file included by stm32mp23xx-dhcos-som.dtsi, 
assigning serial1 to uart8 might be problematic if uart8 does not exist on 
STM32MP23xx SoCs.

> +		eeprom0 = &eeprom0;
> +		eeprom0wl = &eeprom0wl;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711210131.236025-1-marex@nabladev.com?part=9