Re: [PATCH 08/10] arm64: dts: st: Add pinmux nodes for DH electronics STM32MP23xx/STM32MP25xx DHCOS SoM and Breakout Board
From: sashiko-bot@kernel.org
Date: 2026-07-11 21:17:44
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] Conflicting alternate function (AF) configurations for SDMMC3_D3 between active and open-drain states. - [High] The `eth2_rgmii_pins_b` node is missing the `st,io-sync = "data on both edges";` property required for Double Data Rate (DDR) signaling in RGMII. - [Medium] Active and sleep pinmux configurations for ETH2 MDIO use conflicting pins for ETH_MDC. - [Low] Misleading inline comments in DT pinmux nodes copy-pasted from other nodes. --
--- Patch [8]: [PATCH 08/10] arm64: dts: st: Add pinmux nodes for DH electronics STM32MP23xx/STM32MP25xx DHCOS SoM and Breakout Board ---commit c98eed614c4613fe217412814e7be996b10ee304 Author: Marek Vasut [off-list ref] arm64: dts: st: Add pinmux nodes for DH electronics STM32MP23xx/STM32MP25xx DHCOS SoM and Breakout Board This patch adds new pinmux nodes for the DH electronics STM32MP2 DHCOS SoM and Breakout Board. It includes configurations for ETH2, I2C8, MCO1, SDMMC, SPI, UART, and USART interfaces.
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi b/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi index 456ece7f8ebc3..1aadbc6b47da9 100644 --- a/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi +++ b/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi@@ -182,6 +182,30 @@ pins5 { }; }; + /omit-if-no-ref/ + eth2_mdio_pins_a: eth2-mdio-0 { + pins1 { + pinmux = <STM32_PINMUX('G', 4, AF11)>; /* ETH_MDC */ + bias-disable; + drive-push-pull; + slew-rate = <3>; + };
[ ... ]
+ /omit-if-no-ref/
+ eth2_mdio_sleep_pins_a: eth2-mdio-sleep-0 {
+ pins {
+ pinmux = <STM32_PINMUX('C', 6, ANALOG)>, /* ETH_MDC */[Severity: Medium] In eth2_mdio_pins_a, ETH_MDC is mapped to PG4. Does this mismatched pinmux configuration leave PG4 misconfigured and incorrectly apply analog mode to PC6 instead of PG4 during power management transitions?
+ <STM32_PINMUX('C', 5, ANALOG)>; /* ETH_MDIO */
+ };
+ };[ ... ]
+ /omit-if-no-ref/
+ eth2_rgmii_pins_b: eth2-rgmii-1 {
+ pins1 {[Severity: High] This node configures the RGMII data pins, but it doesn't include the st,io-sync = "data on both edges"; property required for Double Data Rate (DDR) signaling. Will this cause the MAC to use Single Data Rate sampling instead of RGMII DDR mode, resulting in timing violations?
+ pinmux = <STM32_PINMUX('C', 7, AF10)>, /* ETH_RGMII_TXD0 */
+ <STM32_PINMUX('C', 8, AF10)>, /* ETH_RGMII_TXD1 */
+ <STM32_PINMUX('C', 9, AF10)>, /* ETH_RGMII_TXD2 */
+ <STM32_PINMUX('C', 10, AF10)>, /* ETH_RGMII_TXD3 */
+ <STM32_PINMUX('C', 4, AF10)>; /* ETH_RGMII_TX_CTL */
+ bias-disable;
+ drive-push-pull;
+ slew-rate = <3>;
+ };
+ pins2 {
+ pinmux = <STM32_PINMUX('F', 8, AF10)>, /* ETH_RGMII_CLK125 */
+ <STM32_PINMUX('F', 7, AF10)>; /* ETH_MDC */[Severity: Low] This isn't a bug, but should this be labeled ETH_RGMII_GTX_CLK rather than ETH_MDC, to match the sleep state configuration for PF7 below?
+ bias-disable; + drive-push-pull; + slew-rate = <3>; + };
[ ... ]
+ /omit-if-no-ref/
+ sdmmc3_b4_pins_b: sdmmc3-b4-1 {
+ pins1 {
+ pinmux = <STM32_PINMUX('B', 14, AF10)>, /* SDMMC3_D0 */
+ <STM32_PINMUX('D', 13, AF10)>, /* SDMMC3_D1 */
+ <STM32_PINMUX('B', 12, AF10)>, /* SDMMC3_D2 */
+ <STM32_PINMUX('D', 14, AF12)>, /* SDMMC3_D3 */
+ <STM32_PINMUX('D', 12, AF10)>; /* SDMMC3_CMD */[ ... ]
+ /omit-if-no-ref/
+ sdmmc3_b4_od_pins_b: sdmmc3-b4-od-1 {
+ pins1 {
+ pinmux = <STM32_PINMUX('B', 14, AF10)>, /* SDMMC3_D0 */
+ <STM32_PINMUX('D', 13, AF10)>, /* SDMMC3_D1 */
+ <STM32_PINMUX('B', 12, AF10)>, /* SDMMC3_D2 */
+ <STM32_PINMUX('D', 14, AF10)>; /* SDMMC3_D3 */[Severity: High] The active state node (sdmmc3_b4_pins_b) configures PD14 with AF12 for SDMMC3_D3. Does configuring PD14 with AF10 here in the open-drain state disconnect the data line from the SDMMC controller during operations that use open-drain mode?
+ slew-rate = <2>; + drive-push-pull; + bias-disable; + };
[ ... ]
+ /omit-if-no-ref/
+ i2c8_pins_b: i2c8-1 {
+ pins {
+ pinmux = <STM32_PINMUX('Z', 4, AF8)>, /* I2C1_SCL */
+ <STM32_PINMUX('Z', 9, AF8)>; /* I2C1_SDA */[Severity: Low] This isn't a bug, but the comments label these pins as I2C1 instead of I2C8. Were these comments copied from an earlier I2C1 node?
+ bias-disable;
+ drive-open-drain;
+ slew-rate = <0>;
+ };
+ };
+
+ /omit-if-no-ref/
+ i2c8_sleep_pins_b: i2c8-sleep-1 {
+ pins {
+ pinmux = <STM32_PINMUX('Z', 4, ANALOG)>, /* I2C1_SCL */
+ <STM32_PINMUX('Z', 9, ANALOG)>; /* I2C1_SDA */[Severity: Low] This isn't a bug, but the same typo labels these pins as I2C1 instead of I2C8.
+ }; + };
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260711210131.236025-1-marex@nabladev.com?part=8