[PATCH 0/9] Introduce HDP support for STM32MP platforms

[PATCH 0/9] Introduce HDP support for STM32MP platforms

[Clément Le Goffic]

This patch series introduces the Hardware Debug Port (HDP) support for
STM32MP platforms.

It includes the addition of device tree bindings, the HDP driver,
and updates to the device tree files for STM32MP13, STM32MP15,
and STM32MP25 SoCs.
The series also updates the MAINTAINERS file to include myself as the
maintainer for the STM32 HDP driver and adds the necessary
pinmux configurations for HDP pins on STM32MP157C-DK2 as example.

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
Clément Le Goffic (9):
      dt-bindings: pinctrl: stm32: Add HDP includes for stm32mp platforms
      dt-bindings: pinctrl: stm32: Introduce HDP
      pinctrl: stm32: Introduce HDP driver
      MAINTAINERS: Add Clément Le Goffic as STM32 HDP maintainer
      ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp13
      ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp15
      ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp25
      ARM: dts: stm32: add alternate pinmux for HDP pin and add HDP pinctrl node
      ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp157c-dk2 board

 .../bindings/pinctrl/st,stm32-pinctrl-hdp.yaml     |  72 +++++
 MAINTAINERS                                        |   7 +
 arch/arm/boot/dts/st/stm32mp131.dtsi               |   7 +
 arch/arm/boot/dts/st/stm32mp15-pinctrl.dtsi        |  26 ++
 arch/arm/boot/dts/st/stm32mp151.dtsi               |   7 +
 arch/arm/boot/dts/st/stm32mp157c-dk2.dts           |   6 +
 arch/arm64/boot/dts/st/stm32mp251.dtsi             |   7 +
 drivers/pinctrl/stm32/Kconfig                      |  14 +
 drivers/pinctrl/stm32/Makefile                     |   1 +
 drivers/pinctrl/stm32/pinctrl-stm32-hdp.c          | 301 +++++++++++++++++++++
 include/dt-bindings/pinctrl/stm32mp13-hdp.h        | 130 +++++++++
 include/dt-bindings/pinctrl/stm32mp15-hdp.h        | 116 ++++++++
 include/dt-bindings/pinctrl/stm32mp25-hdp.h        | 144 ++++++++++
 13 files changed, 838 insertions(+)
---
base-commit: d01895c5b11849113e70f012d9d142f1d88852f0
change-id: 20250224-hdp-upstream-622e5da14a9f

Best regards,
-- 
Clément Le Goffic <clement.legoffic@foss.st.com>

[PATCH 1/9] dt-bindings: pinctrl: stm32: Add HDP includes for stm32mp platforms

[Clément Le Goffic]

Each file introduces helpers to choose the signal to monitor through the
HDP pin.
Signals are different for each platforms: stm32mp13, stm32mp15, stm32mp25.

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 include/dt-bindings/pinctrl/stm32mp13-hdp.h | 130 +++++++++++++++++++++++++
 include/dt-bindings/pinctrl/stm32mp15-hdp.h | 116 ++++++++++++++++++++++
 include/dt-bindings/pinctrl/stm32mp25-hdp.h | 144 ++++++++++++++++++++++++++++
 3 files changed, 390 insertions(+)

diff --git a/include/dt-bindings/pinctrl/stm32mp13-hdp.h b/include/dt-bindings/pinctrl/stm32mp13-hdp.h
new file mode 100644
index 000000000000..a3487e700143
--- /dev/null
+++ b/include/dt-bindings/pinctrl/stm32mp13-hdp.h
@@ -0,0 +1,130 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */
+/*
+ * Copyright (C) STMicroelectronics 2025 - All Rights Reserved
+ * Author: Clément Le Goffic <clement.legoffic@foss.st.com> for STMicroelectronics.
+ */
+
+#ifndef _DT_BINDINGS_STM32MP13_HDP_H
+#define _DT_BINDINGS_STM32MP13_HDP_H
+
+/* define a macro for each function a HDP pin can transmit */
+#define HDP0_PWR_PWRWAKE_SYS			 "0"
+#define HDP0_PWR_STOP_FORBIDDEN			 "1"
+#define HDP0_PWR_STDBY_WAKEUP			 "2"
+#define HDP0_PWR_ENCOMP_VDDCORE			 "3"
+#define HDP0_BSEC_OUT_SEC_NIDEN			 "4"
+#define HDP0_AIEC_SYS_WAKEUP			 "5"
+#define HDP0_DDRCTRL_LP_REQ			 "8"
+#define HDP0_PWR_DDR_RET_ENABLE_N		 "9"
+#define HDP0_DTS_CLK_PTAT			 "10"
+#define HDP0_SRAM3CTRL_TAMP_ERASE_ACT		 "12"
+#define HDP0_GPOVAL_0				 "15"
+
+#define HDP1_PWR_SEL_VTH_VDDCPU			 "0"
+#define HDP1_PWR_MPU_RAM_LOWSPEED		 "1"
+#define HDP1_CA7_NAXIERRIRQ			 "2"
+#define HDP1_PWR_OKIN_MR			 "3"
+#define HDP1_BSEC_OUT_SEC_DBGEN			 "4"
+#define HDP1_AIEC_C1_WAKEUP			 "5"
+#define HDP1_RCC_PWRDS_MPU			 "6"
+#define HDP1_DDRCTRL_DFI_CTRLUPD_REQ		 "8"
+#define HDP1_DDRCTRL_CACTIVE_DDRC_ASR		 "9"
+#define HDP1_SRAM3CTRL_HW_ERASE_ACT		 "12"
+#define HDP1_NIC400_S0_BREADY			 "13"
+#define HDP1_GPOVAL_1				 "15"
+
+#define HDP2_PWR_PWRWAKE_MPU			 "0"
+#define HDP2_PWR_MPU_CLOCK_DISABLE_ACK		 "1"
+#define HDP2_CA7_NDBGRESET_I			 "2"
+#define HDP2_BSEC_IN_RSTCORE_N			 "4"
+#define HDP2_BSEC_OUT_SEC_BSC_DIS		 "5"
+#define HDP2_DDRCTRL_DFI_INIT_COMPLETE		 "8"
+#define HDP2_DDRCTRL_PERF_OP_IS_REFRESH		 "9"
+#define HDP2_DDRCTRL_GSKP_DFI_LP_REQ		 "10"
+#define HDP2_SRAM3CTRL_SW_ERASE_ACT		 "12"
+#define HDP2_NIC400_S0_BVALID			 "13"
+#define HDP2_GPOVAL_2				 "15"
+
+#define HDP3_PWR_SEL_VTH_VDDCORE		 "0"
+#define HDP3_PWR_MPU_CLOCK_DISABLE_REQ		 "1"
+#define HDP3_CA7_NPMUIRQ0			 "2"
+#define HDP3_CA7_NFIQOUT0			 "3"
+#define HDP3_BSEC_OUT_SEC_DFTLOCK		 "4"
+#define HDP3_BSEC_OUT_SEC_JTAG_DIS		 "5"
+#define HDP3_RCC_PWRDS_SYS			 "6"
+#define HDP3_SRAM3CTRL_TAMP_ERASE_REQ		 "7"
+#define HDP3_DDRCTRL_STAT_DDRC_REG_SELFREF_TYPE0 "8"
+#define HDP3_DTS_VALOBUS1_0			 "10"
+#define HDP3_DTS_VALOBUS2_0			 "11"
+#define HDP3_TAMP_POTENTIAL_TAMP_ERFCFG		 "12"
+#define HDP3_NIC400_S0_WREADY			 "13"
+#define HDP3_NIC400_S0_RREADY			 "14"
+#define HDP3_GPOVAL_3				 "15"
+
+#define HDP4_PWR_STOP2_ACTIVE			 "1"
+#define HDP4_CA7_NL2RESET_I			 "2"
+#define HDP4_CA7_NPORESET_VARM_I		 "3"
+#define HDP4_BSEC_OUT_SEC_DFTEN			 "4"
+#define HDP4_BSEC_OUT_SEC_DBGSWENABLE		 "5"
+#define HDP4_ETH1_OUT_PMT_INTR_O		 "6"
+#define HDP4_ETH2_OUT_PMT_INTR_O		 "7"
+#define HDP4_DDRCTRL_STAT_DDRC_REG_SELFREF_TYPE1 "8"
+#define HDP4_DDRCTRL_CACTIVE_0			 "9"
+#define HDP4_DTS_VALOBUS1_1			 "10"
+#define HDP4_DTS_VALOBUS2_1			 "11"
+#define HDP4_TAMP_NRESET_SRAM_ERCFG		 "12"
+#define HDP4_NIC400_S0_WLAST			 "13"
+#define HDP4_NIC400_S0_RLAST			 "14"
+#define HDP4_GPOVAL_4				 "15"
+
+#define HDP5_CA7_STANDBYWFIL2			 "0"
+#define HDP5_PWR_VTH_VDDCORE_ACK		 "1"
+#define HDP5_CA7_NCORERESET_I			 "2"
+#define HDP5_CA7_NIRQOUT0			 "3"
+#define HDP5_BSEC_IN_PWROK			 "4"
+#define HDP5_BSEC_OUT_SEC_DEVICEEN		 "5"
+#define HDP5_ETH1_OUT_LPI_INTR_O		 "6"
+#define HDP5_ETH2_OUT_LPI_INTR_O		 "7"
+#define HDP5_DDRCTRL_CACTIVE_DDRC		 "8"
+#define HDP5_DDRCTRL_WR_CREDIT_CNT		 "9"
+#define HDP5_DTS_VALOBUS1_2			 "10"
+#define HDP5_DTS_VALOBUS2_2			 "11"
+#define HDP5_PKA_PKA_ITAMP_OUT			 "12"
+#define HDP5_NIC400_S0_WVALID			 "13"
+#define HDP5_NIC400_S0_RVALID			 "14"
+#define HDP5_GPOVAL_5				 "15"
+
+#define HDP6_CA7_STANDBYWFE0			 "0"
+#define HDP6_PWR_VTH_VDDCPU_ACK			 "1"
+#define HDP6_CA7_EVENTO				 "2"
+#define HDP6_BSEC_IN_TAMPER_DET			 "4"
+#define HDP6_BSEC_OUT_SEC_SPNIDEN		 "5"
+#define HDP6_ETH1_OUT_MAC_SPEED_O1		 "6"
+#define HDP6_ETH2_OUT_MAC_SPEED_O1		 "7"
+#define HDP6_DDRCTRL_CSYSACK_DDRC		 "8"
+#define HDP6_DDRCTRL_LPR_CREDIT_CNT		 "9"
+#define HDP6_DTS_VALOBUS1_3			 "10"
+#define HDP6_DTS_VALOBUS2_3			 "11"
+#define HDP6_SAES_TAMPER_OUT			 "12"
+#define HDP6_NIC400_S0_AWREADY			 "13"
+#define HDP6_NIC400_S0_ARREADY			 "14"
+#define HDP6_GPOVAL_6				 "15"
+
+#define HDP7_CA7_STANDBYWFI0			 "0"
+#define HDP7_PWR_RCC_VCPU_RDY			 "1"
+#define HDP7_CA7_EVENTI				 "2"
+#define HDP7_CA7_DBGACK0			 "3"
+#define HDP7_BSEC_OUT_FUSE_OK			 "4"
+#define HDP7_BSEC_OUT_SEC_SPIDEN		 "5"
+#define HDP7_ETH1_OUT_MAC_SPEED_O0		 "6"
+#define HDP7_ETH2_OUT_MAC_SPEED_O0		 "7"
+#define HDP7_DDRCTRL_CSYSREQ_DDRC		 "8"
+#define HDP7_DDRCTRL_HPR_CREDIT_CNT		 "9"
+#define HDP7_DTS_VALOBUS1_4			 "10"
+#define HDP7_DTS_VALOBUS2_4			 "11"
+#define HDP7_RNG_TAMPER_OUT			 "12"
+#define HDP7_NIC400_S0_AWAVALID			 "13"
+#define HDP7_NIC400_S0_ARAVALID			 "14"
+#define HDP7_GPOVAL_7				 "15"
+
+#endif /* _DT_BINDINGS_STM32MP13_HDP_H */
diff --git a/include/dt-bindings/pinctrl/stm32mp15-hdp.h b/include/dt-bindings/pinctrl/stm32mp15-hdp.h
new file mode 100644
index 000000000000..ac32cec5a9f6
--- /dev/null
+++ b/include/dt-bindings/pinctrl/stm32mp15-hdp.h
@@ -0,0 +1,116 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */
+/*
+ * Copyright (C) STMicroelectronics 2025 - All Rights Reserved
+ * Author: Clément Le Goffic <clement.legoffic@foss.st.com> for STMicroelectronics.
+ */
+
+#ifndef _DT_BINDINGS_STM32MP15_HDP_H
+#define _DT_BINDINGS_STM32MP15_HDP_H
+
+/* define a macro for each function a HDP pin can transmit */
+#define HDP0_PWR_PWRWAKE_SYS			 "0"
+#define HDP0_CM4_SLEEPDEEP			 "1"
+#define HDP0_PWR_STDBY_WKUP			 "2"
+#define HDP0_PWR_ENCOMP_VDDCORE			 "3"
+#define HDP0_BSEC_OUT_SEC_NIDEN			 "4"
+#define HDP0_RCC_CM4_SLEEPDEEP			 "6"
+#define HDP0_GPU_DBG7				 "7"
+#define HDP0_DDRCTRL_LP_REQ			 "8"
+#define HDP0_PWR_DDR_RET_ENABLE_N		 "9"
+#define HDP0_DTS_CLK_PTAT			 "10"
+#define HDP0_GPOVAL_0				 "15"
+
+#define HDP1_PWR_PWRWAKE_MCU			 "0"
+#define HDP1_CM4_HALTED				 "1"
+#define HDP1_CA7_NAXIERRIRQ			 "2"
+#define HDP1_PWR_OKIN_MR			 "3"
+#define HDP1_BSEC_OUT_SEC_DBGEN			 "4"
+#define HDP1_EXTI_SYS_WAKEUP			 "5"
+#define HDP1_RCC_PWRDS_MPU			 "6"
+#define HDP1_GPU_DBG6				 "7"
+#define HDP1_DDRCTRL_DFI_CTRLUPD_REQ		 "8"
+#define HDP1_DDRCTRL_CACTIVE_DDRC_ASR		 "9"
+#define HDP1_GPOVAL_1				 "15"
+
+#define HDP2_PWR_PWRWAKE_MPU			 "0"
+#define HDP2_CM4_RXEV				 "1"
+#define HDP2_CA7_NPMUIRQ1			 "2"
+#define HDP2_CA7_NFIQOUT1			 "3"
+#define HDP2_BSEC_IN_RSTCORE_N			 "4"
+#define HDP2_EXTI_C2_WAKEUP			 "5"
+#define HDP2_RCC_PWRDS_MCU			 "6"
+#define HDP2_GPU_DBG5				 "7"
+#define HDP2_DDRCTRL_DFI_INIT_COMPLETE		 "8"
+#define HDP2_DDRCTRL_PERF_OP_IS_REFRESH		 "9"
+#define HDP2_DDRCTRL_GSKP_DFI_LP_REQ		 "10"
+#define HDP2_GPOVAL_2				 "15"
+
+#define HDP3_PWR_SEL_VTH_VDDCORE		 "0"
+#define HDP3_CM4_TXEV				 "1"
+#define HDP3_CA7_NPMUIRQ0			 "2"
+#define HDP3_CA7_NFIQOUT0			 "3"
+#define HDP3_BSEC_OUT_SEC_DFTLOCK		 "4"
+#define HDP3_EXTI_C1_WAKEUP			 "5"
+#define HDP3_RCC_PWRDS_SYS			 "6"
+#define HDP3_GPU_DBG4				 "7"
+#define HDP3_DDRCTRL_STAT_DDRC_REG_SELFREF_TYPE0 "8"
+#define HDP3_DDRCTRL_CACTIVE_1			 "9"
+#define HDP3_DTS_VALOBUS1_0			 "10"
+#define HDP3_DTS_VALOBUS2_0			 "11"
+#define HDP3_GPOVAL_3				 "15"
+
+#define HDP4_PWR_PDDS_NOT_CSTBYDIS		 "0"
+#define HDP4_CM4_SLEEPING			 "1"
+#define HDP4_CA7_NRESET1			 "2"
+#define HDP4_CA7_NIRQOUT1			 "3"
+#define HDP4_BSEC_OUT_SEC_DFTEN			 "4"
+#define HDP4_BSEC_OUT_SEC_DBGSWENABLE		 "5"
+#define HDP4_ETH_OUT_PMT_INTR_O			 "6"
+#define HDP4_GPU_DBG3				 "7"
+#define HDP4_DDRCTRL_STAT_DDRC_REG_SELFREF_TYPE1 "8"
+#define HDP4_DDRCTRL_CACTIVE_0			 "9"
+#define HDP4_DTS_VALOBUS1_1			 "10"
+#define HDP4_DTS_VALOBUS2_1			 "11"
+#define HDP4_GPOVAL_4				 "15"
+
+#define HDP5_CA7_STANDBYWFIL2			 "0"
+#define HDP5_PWR_VTH_VDDCORE_ACK		 "1"
+#define HDP5_CA7_NRESET0			 "2"
+#define HDP5_CA7_NIRQOUT0			 "3"
+#define HDP5_BSEC_IN_PWROK			 "4"
+#define HDP5_BSEC_OUT_SEC_DEVICEEN		 "5"
+#define HDP5_ETH_OUT_LPI_INTR_O			 "6"
+#define HDP5_GPU_DBG2				 "7"
+#define HDP5_DDRCTRL_CACTIVE_DDRC		 "8"
+#define HDP5_DDRCTRL_WR_CREDIT_CNT		 "9"
+#define HDP5_DTS_VALOBUS1_2			 "10"
+#define HDP5_DTS_VALOBUS2_2			 "11"
+#define HDP5_GPOVAL_5				 "15"
+
+#define HDP6_CA7_STANDBYWFI1			 "0"
+#define HDP6_CA7_STANDBYWFE1			 "1"
+#define HDP6_CA7_EVENTO				 "2"
+#define HDP6_CA7_DBGACK1			 "3"
+#define HDP6_BSEC_OUT_SEC_SPNIDEN		 "5"
+#define HDP6_ETH_OUT_MAC_SPEED_O1		 "6"
+#define HDP6_GPU_DBG1				 "7"
+#define HDP6_DDRCTRL_CSYSACK_DDRC		 "8"
+#define HDP6_DDRCTRL_LPR_CREDIT_CNT		 "9"
+#define HDP6_DTS_VALOBUS1_3			 "10"
+#define HDP6_DTS_VALOBUS2_3			 "11"
+#define HDP6_GPOVAL_6				 "15"
+
+#define HDP7_CA7_STANDBYWFI0			 "0"
+#define HDP7_CA7_STANDBYWFE0			 "1"
+#define HDP7_CA7_DBGACK0			 "3"
+#define HDP7_BSEC_OUT_FUSE_OK			 "4"
+#define HDP7_BSEC_OUT_SEC_SPIDEN		 "5"
+#define HDP7_ETH_OUT_MAC_SPEED_O0		 "6"
+#define HDP7_GPU_DBG0				 "7"
+#define HDP7_DDRCTRL_CSYSREQ_DDRC		 "8"
+#define HDP7_DDRCTRL_HPR_CREDIT_CNT		 "9"
+#define HDP7_DTS_VALOBUS1_4			 "10"
+#define HDP7_DTS_VALOBUS2_4			 "11"
+#define HDP7_GPOVAL_7				 "15"
+
+#endif /* _DT_BINDINGS_STM32MP15_HDP_H */
diff --git a/include/dt-bindings/pinctrl/stm32mp25-hdp.h b/include/dt-bindings/pinctrl/stm32mp25-hdp.h
new file mode 100644
index 000000000000..bc84042f1605
--- /dev/null
+++ b/include/dt-bindings/pinctrl/stm32mp25-hdp.h
@@ -0,0 +1,144 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */
+/*
+ * Copyright (C) STMicroelectronics 2025 - All Rights Reserved
+ * Author: Clément Le Goffic <clement.legoffic@foss.st.com> for STMicroelectronics.
+ */
+
+#ifndef _DT_BINDINGS_STM32MP25_HDP_H
+#define _DT_BINDINGS_STM32MP25_HDP_H
+
+/* define a macro for each function a HDP pin can transmit */
+#define HDP0_PWR_PWRWAKE_SYS			     "0"
+#define HDP0_CPU2_SLEEP_DEEP			     "1"
+#define HDP0_BSEC_OUT_TST_SDR_UNLOCK_OR_DISABLE_SCAN "2"
+#define HDP0_BSEC_OUT_NIDENM			     "3"
+#define HDP0_BSEC_OUT_NIDENA			     "4"
+#define HDP0_CPU2_STATE_0			     "5"
+#define HDP0_RCC_PWRDS_SYS			     "6"
+#define HDP0_GPU_DBG7				     "7"
+#define HDP0_DDRSS_CSYSREQ_DDRC			     "8"
+#define HDP0_DDRSS_DFI_PHYUPD_REQ		     "9"
+#define HDP0_CPU3_SLEEP_DEEP			     "10"
+#define HDP0_D2_GBL_PER_CLK_BUS_REQ		     "11"
+#define HDP0_PCIE_USB_CXPL_DEBUG_INFO_EI_0	     "12"
+#define HDP0_PCIE_USB_CXPL_DEBUG_INFO_EI_8	     "13"
+#define HDP0_D3_STATE_0				     "14"
+#define HDP0_GPOVAL_0				     "15"
+
+#define HDP1_PWR_PWRWAKE_CPU2			     "0"
+#define HDP1_CPU2_HALTED			     "1"
+#define HDP1_CPU2_STATE_1			     "2"
+#define HDP1_BSEC_OUT_DBGENM			     "3"
+#define HDP1_BSEC_OUT_DBGENA			     "4"
+#define HDP1_EXTI1_SYS_WAKEUP			     "5"
+#define HDP1_RCC_PWRDS_CPU2			     "6"
+#define HDP1_GPU_DBG6				     "7"
+#define HDP1_DDRSS_CSYSACK_DDRC			     "8"
+#define HDP1_DDRSS_DFI_PHYMSTR_REQ		     "9"
+#define HDP1_CPU3_HALTED			     "10"
+#define HDP1_D2_GBL_PER_DMA_REQ			     "11"
+#define HDP1_PCIE_USB_CXPL_DEBUG_INFO_EI_1	     "12"
+#define HDP1_PCIE_USB_CXPL_DEBUG_INFO_EI_9	     "13"
+#define HDP1_D3_STATE_1				     "14"
+#define HDP1_GPOVAL_1				     "15"
+
+#define HDP2_PWR_PWRWAKE_CPU1			     "0"
+#define HDP2_CPU2_RXEV				     "1"
+#define HDP2_CPU1_NPMUIRQ1			     "2"
+#define HDP2_CPU1_NFIQOUT1			     "3"
+#define HDP2_BSEC_OUT_SHDBGEN			     "4"
+#define HDP2_EXTI1_CPU2_WAKEUP			     "5"
+#define HDP2_RCC_PWRDS_CPU1			     "6"
+#define HDP2_GPU_DBG5				     "7"
+#define HDP2_DDRSS_CACTIVE_DDRC			     "8"
+#define HDP2_DDRSS_DFI_LP_REQ			     "9"
+#define HDP2_CPU3_RXEV				     "10"
+#define HDP2_HPDMA1_CLK_BUS_REQ			     "11"
+#define HDP2_PCIE_USB_CXPL_DEBUG_INFO_EI_2	     "12"
+#define HDP2_PCIE_USB_CXPL_DEBUG_INFO_EI_10	     "13"
+#define HDP2_D3_STATE_2				     "14"
+#define HDP2_GPOVAL_2				     "15"
+
+#define HDP3_PWR_SEL_VTH_VDDCPU			     "0"
+#define HDP3_CPU2_TXEV				     "1"
+#define HDP3_CPU1_NPMUIRQ0			     "2"
+#define HDP3_CPU1_NFIQOUT0			     "3"
+#define HDP3_BSEC_OUT_DDBGEN			     "4"
+#define HDP3_EXTI1_CPU1_WAKEUP			     "5"
+#define HDP3_CPU3_STATE_0			     "6"
+#define HDP3_GPU_DBG4				     "7"
+#define HDP3_DDRSS_MCDCG_EN			     "8"
+#define HDP3_DDRSS_DFI_FREQ_0			     "9"
+#define HDP3_CPU3_TXEV				     "10"
+#define HDP3_HPDMA2_CLK_BUS_REQ			     "11"
+#define HDP3_PCIE_USB_CXPL_DEBUG_INFO_EI_3	     "12"
+#define HDP3_PCIE_USB_CXPL_DEBUG_INFO_EI_11	     "13"
+#define HDP3_D1_STATE_0				     "14"
+#define HDP3_GPOVAL_3				     "15"
+
+#define HDP4_PWR_SEL_VTH_VDDCORE		     "0"
+#define HDP4_CPU2_SLEEPING			     "1"
+#define HDP4_CPU1_EVENTO			     "2"
+#define HDP4_CPU1_NIRQOUT1			     "3"
+#define HDP4_BSEC_OUT_SPNIDENA			     "4"
+#define HDP4_EXTI2_D3_WAKEUP			     "5"
+#define HDP4_ETH1_OUT_PMT_INTR_O		     "6"
+#define HDP4_GPU_DBG3				     "7"
+#define HDP4_DDRSS_DPHYCG_EN			     "8"
+#define HDP4_DDRSS_OBSP0			     "9"
+#define HDP4_CPU3_SLEEPING			     "10"
+#define HDP4_HPDMA3_CLK_BUS_REQ			     "11"
+#define HDP4_PCIE_USB_CXPL_DEBUG_INFO_EI_4	     "12"
+#define HDP4_PCIE_USB_CXPL_DEBUG_INFO_EI_12	     "13"
+#define HDP4_D1_STATE_1				     "14"
+#define HDP4_GPOVAL_4				     "15"
+
+#define HDP5_CPU1_STANDBY_WFIL2			     "0"
+#define HDP5_CPU1_NIRQOUT0			     "3"
+#define HDP5_BSEC_OUT_SPIDENA			     "4"
+#define HDP5_EXTI2_CPU3_WAKEUP			     "5"
+#define HDP5_ETH1_OUT_LPI_INTR_O		     "6"
+#define HDP5_GPU_DBG2				     "7"
+#define HDP5_DDRCTRL_DFI_INIT_START		     "8"
+#define HDP5_DDRSS_OBSP1			     "9"
+#define HDP5_CPU3_STATE_1			     "10"
+#define HDP5_D3_GBL_PER_CLK_BUS_REQ		     "11"
+#define HDP5_PCIE_USB_CXPL_DEBUG_INFO_EI_5	     "12"
+#define HDP5_PCIE_USB_CXPL_DEBUG_INFO_EI_13	     "13"
+#define HDP5_D1_STATE_2				     "14"
+#define HDP5_GPOVAL_5				     "15"
+
+#define HDP6_CPU1_STANDBY_WFI1			     "0"
+#define HDP6_CPU1_STANDBY_WFE1			     "1"
+#define HDP6_CPU1_HALTED1			     "2"
+#define HDP6_CPU1_NAXIERRIRQ			     "3"
+#define HDP6_BSEC_OUT_SPNIDENM			     "4"
+#define HDP6_EXTI2_CPU2_WAKEUP			     "5"
+#define HDP6_ETH2_OUT_PMT_INTR_O		     "6"
+#define HDP6_GPU_DBG1				     "7"
+#define HDP6_DDRSS_DFI_INIT_COMPLETE		     "8"
+#define HDP6_DDRSS_OBSP2			     "9"
+#define HDP6_D2_STATE_0				     "10"
+#define HDP6_D3_GBL_PER_DMA_REQ			     "11"
+#define HDP6_PCIE_USB_CXPL_DEBUG_INFO_EI_6	     "12"
+#define HDP6_PCIE_USB_CXPL_DEBUG_INFO_EI_14	     "13"
+#define HDP6_CPU1_STATE_0			     "14"
+#define HDP6_GPOVAL_6				     "15"
+
+#define HDP7_CPU1_STANDBY_WFI0			     "0"
+#define HDP7_CPU1_STANDBY_WFE0			     "1"
+#define HDP7_CPU1_HALTED0			     "2"
+#define HDP7_BSEC_OUT_SPIDENM			     "4"
+#define HDP7_EXTI2_CPU1__WAKEUP			     "5"
+#define HDP7_ETH2_OUT_LPI_INTR_O		     "6"
+#define HDP7_GPU_DBG0				     "7"
+#define HDP7_DDRSS_DFI_CTRLUPD_REQ		     "8"
+#define HDP7_DDRSS_OBSP3			     "9"
+#define HDP7_D2_STATE_1				     "10"
+#define HDP7_LPDMA1_CLK_BUS_REQ			     "11"
+#define HDP7_PCIE_USB_CXPL_DEBUG_INFO_EI_7	     "12"
+#define HDP7_PCIE_USB_CXPL_DEBUG_INFO_EI_15	     "13"
+#define HDP7_CPU1_STATE_1			     "14"
+#define HDP7_GPOVAL_7				     "15"
+
+#endif /* _DT_BINDINGS_STM32MP25_HDP_H */

-- 
2.43.0

[PATCH 2/9] dt-bindings: pinctrl: stm32: Introduce HDP

[Clément Le Goffic]

Introduce dt-bindings for HDP driver.

'HDP' stands for Hardware Debug Port, it is an hardware block in
STMicrolectronics' MPUs that let the user decide which internal SoC's
signal to observe.
It provides 8 ports and for each port there is up to 16 different
signals that can be output.
Signals are different for each MPU.

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 .../bindings/pinctrl/st,stm32-pinctrl-hdp.yaml     | 72 ++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl-hdp.yaml b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl-hdp.yaml
new file mode 100644
index 000000000000..b6787162faaa
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl-hdp.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) STMicroelectronics 2025.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/st,stm32-pinctrl-hdp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STM32 Hardware Debug Port Mux/Config
+
+maintainers:
+  - Clément LE GOFFIC <clement.legoffic@foss.st.com>
+
+description: |
+  STMicroelectronics's STM32 MPUs integrate a Hardware Debug Port (HDP).
+  It allows to output internal signals on SoC's GPIO.
+
+properties:
+  compatible:
+    const: st,stm32mp-hdp
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+patternProperties:
+  '-pins$':
+    type: object
+    $ref: pinmux-node.yaml#
+
+    properties:
+      function:
+        enum: [ "0", "1", "2", "3", "4", "5", "6", "7",
+                "8", "9", "10", "11", "12", "13", "14",
+                "15" ]
+
+      pins:
+        enum: [ hdp0, hdp1, hdp2, hdp3, hdp4, hdp5, hdp6, hdp7 ]
+
+    required:
+      - function
+      - pins
+
+    additionalProperties: false
+
+allOf:
+  - $ref: pinctrl.yaml#
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/stm32mp1-clks.h>
+    #include <dt-bindings/pinctrl/stm32mp15-hdp.h>
+    //Example 1
+    pinctrl@54090000 {
+      compatible = "st,stm32mp-hdp";
+      reg = <0x54090000 0x400>;
+      clocks = <&rcc HDP>;
+      pinctrl-names = "default";
+      pinctrl-0 = <&hdp2>;
+      hdp2-pins {
+        function = HDP2_GPOVAL_2;
+        pins = "hdp2";
+      };
+    };

-- 
2.43.0

[PATCH 3/9] pinctrl: stm32: Introduce HDP driver

[Clément Le Goffic]

This patch introduce the driver for the Hardware Debug Port available on
STM32MP platforms. The HDP allows the observation of internal SoC
signals by using multiplexers. Each HDP port can provide up to 16
internal signals (one of them can be software controlled as a GPO)

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 drivers/pinctrl/stm32/Kconfig             |  14 ++
 drivers/pinctrl/stm32/Makefile            |   1 +
 drivers/pinctrl/stm32/pinctrl-stm32-hdp.c | 301 ++++++++++++++++++++++++++++++
 3 files changed, 316 insertions(+)

diff --git a/drivers/pinctrl/stm32/Kconfig b/drivers/pinctrl/stm32/Kconfig
index 2656d3d3ae40..4b474cfd1f2c 100644
--- a/drivers/pinctrl/stm32/Kconfig
+++ b/drivers/pinctrl/stm32/Kconfig
@@ -57,4 +57,18 @@ config PINCTRL_STM32MP257
 	depends on OF && HAS_IOMEM
 	default MACH_STM32MP25
 	select PINCTRL_STM32
+
+config PINCTRL_STM32_HDP
+	tristate "STMicroelectronics STM32 Hardware Debug Port (HDP) pin control"
+	depends on OF && HAS_IOMEM
+	default ARM64 || (ARM && CPU_V7)
+	select PINMUX
+	select GENERIC_PINCONF
+	select GPIOLIB
+	help
+	  The Hardware Debug Port allows the observation of internal signals.
+	  It uses configurable multiplexer to route signals in a dedicated observation register.
+	  This driver also permits the observation of signals on external SoC pins.
+	  It permits the observation of up to 16 signals per HDP line.
+
 endif
diff --git a/drivers/pinctrl/stm32/Makefile b/drivers/pinctrl/stm32/Makefile
index 7b17464d8de1..98a1bbc7e16c 100644
--- a/drivers/pinctrl/stm32/Makefile
+++ b/drivers/pinctrl/stm32/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_PINCTRL_STM32H743)	+= pinctrl-stm32h743.o
 obj-$(CONFIG_PINCTRL_STM32MP135) += pinctrl-stm32mp135.o
 obj-$(CONFIG_PINCTRL_STM32MP157) += pinctrl-stm32mp157.o
 obj-$(CONFIG_PINCTRL_STM32MP257) += pinctrl-stm32mp257.o
+obj-$(CONFIG_PINCTRL_STM32_HDP) += pinctrl-stm32-hdp.o
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32-hdp.c b/drivers/pinctrl/stm32/pinctrl-stm32-hdp.c
new file mode 100644
index 000000000000..e937d9b9f77a
--- /dev/null
+++ b/drivers/pinctrl/stm32/pinctrl-stm32-hdp.c
@@ -0,0 +1,301 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) STMicroelectronics 2025 - All Rights Reserved
+ * Author: Clément Le Goffic <clement.legoffic@foss.st.com> for STMicroelectronics.
+ */
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/gpio/driver.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+
+#include "../core.h"
+
+#define DRIVER_NAME		"stm32_hdp"
+#define HDP_CTRL_ENABLE		1
+#define HDP_CTRL_DISABLE	0
+#define NB_FUNCTIONS		16
+
+#define HDP_CTRL		0x000
+#define HDP_MUX			0x004
+#define HDP_VAL			0x010
+#define HDP_GPOSET		0x014
+#define HDP_GPOCLR		0x018
+#define HDP_GPOVAL		0x01c
+#define HDP_VERR		0x3f4
+#define HDP_IPIDR		0x3f8
+#define HDP_SIDR		0x3fc
+
+#define HDP_MUX_SHIFT(n)	((n) * 4)
+#define HDP_MUX_MASK(n)		(GENMASK(3, 0) << HDP_MUX_SHIFT(n))
+#define HDP_MUX_GPOVAL(n)	(0xf << HDP_MUX_SHIFT(n))
+
+struct stm32_hdp {
+	struct device *dev;
+	void __iomem *base;
+	struct clk *clk;
+	struct pinctrl_dev *pctl_dev;
+	struct gpio_chip gpio_chip;
+	u32 mux_conf;
+	u32 gposet_conf;
+};
+
+static const struct pinctrl_pin_desc stm32_hdp_pins[] = {
+	PINCTRL_PIN(0, "hdp0"),
+	PINCTRL_PIN(1, "hdp1"),
+	PINCTRL_PIN(2, "hdp2"),
+	PINCTRL_PIN(3, "hdp3"),
+	PINCTRL_PIN(4, "hdp4"),
+	PINCTRL_PIN(5, "hdp5"),
+	PINCTRL_PIN(6, "hdp6"),
+	PINCTRL_PIN(7, "hdp7"),
+};
+
+static const char * const func_name[] = {
+	"0", "1", "2", "3",
+	"4", "5", "6", "7",
+	"8", "9", "10", "11",
+	"12", "13", "14", "15",
+};
+
+static const char * const stm32_hdp_pins_group[] = {
+	"hdp0",
+	"hdp1",
+	"hdp2",
+	"hdp3",
+	"hdp4",
+	"hdp5",
+	"hdp6",
+	"hdp7"
+};
+
+static int stm32_hdp_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+	return GPIO_LINE_DIRECTION_OUT;
+}
+
+static int stm32_hdp_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+	struct stm32_hdp *hdp = gpiochip_get_data(gc);
+
+	if (((hdp->mux_conf & HDP_MUX_MASK(offset))) == HDP_MUX_GPOVAL(offset))
+		return !!(readl_relaxed(hdp->base + HDP_GPOVAL) & BIT(offset));
+	else
+		return !!(readl_relaxed(hdp->base + HDP_VAL) & BIT(offset));
+}
+
+static void stm32_hdp_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
+{
+	struct stm32_hdp *hdp = gpiochip_get_data(gc);
+
+	if (value)
+		writel_relaxed(BIT(offset), hdp->base + HDP_GPOSET);
+	else
+		writel_relaxed(BIT(offset), hdp->base + HDP_GPOCLR);
+}
+
+static int stm32_hdp_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(stm32_hdp_pins);
+}
+
+static const char *stm32_hdp_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
+						    unsigned int selector)
+{
+	return stm32_hdp_pins[selector].name;
+}
+
+static int stm32_hdp_pinctrl_get_group_pins(struct pinctrl_dev *pctldev, unsigned int selector,
+					    const unsigned int **pins, unsigned int *num_pins)
+{
+	*pins = &stm32_hdp_pins[selector].number;
+	*num_pins = 1;
+
+	return 0;
+}
+
+static const struct pinctrl_ops stm32_hdp_pinctrl_ops = {
+	.get_groups_count = stm32_hdp_pinctrl_get_groups_count,
+	.get_group_name	  = stm32_hdp_pinctrl_get_group_name,
+	.get_group_pins	  = stm32_hdp_pinctrl_get_group_pins,
+	.dt_node_to_map	  = pinconf_generic_dt_node_to_map_all,
+	.dt_free_map	  = pinconf_generic_dt_free_map,
+};
+
+static int stm32_hdp_pinmux_get_functions_count(struct pinctrl_dev *pctldev)
+{
+	return NB_FUNCTIONS;
+}
+
+static const char *stm32_hdp_pinmux_get_function_name(struct pinctrl_dev *pctldev,
+						      unsigned int selector)
+{
+	return func_name[selector];
+}
+
+static int stm32_hdp_pinmux_get_function_groups(struct pinctrl_dev *pctldev, unsigned int selector,
+						const char *const **groups,
+						unsigned int *num_groups)
+{
+	*groups = stm32_hdp_pins_group;
+	*num_groups = ARRAY_SIZE(stm32_hdp_pins_group);
+
+	return 0;
+}
+
+static int stm32_hdp_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int func_selector,
+				    unsigned int group_selector)
+{
+	struct stm32_hdp *hdp = pinctrl_dev_get_drvdata(pctldev);
+	unsigned int pin = stm32_hdp_pins[group_selector].number;
+	u32 mux;
+
+	mux = readl_relaxed(hdp->base + HDP_MUX);
+	mux &= ~HDP_MUX_MASK(pin);
+	mux |= func_selector << HDP_MUX_SHIFT(pin);
+
+	writel_relaxed(mux, hdp->base + HDP_MUX);
+	hdp->mux_conf = mux;
+
+	return 0;
+}
+
+static const struct pinmux_ops stm32_hdp_pinmux_ops = {
+	.get_functions_count = stm32_hdp_pinmux_get_functions_count,
+	.get_function_name   = stm32_hdp_pinmux_get_function_name,
+	.get_function_groups = stm32_hdp_pinmux_get_function_groups,
+	.set_mux	     = stm32_hdp_pinmux_set_mux,
+	.gpio_set_direction  = NULL,
+};
+
+static struct pinctrl_desc stm32_hdp_pdesc = {
+	.name	 = DRIVER_NAME,
+	.pins	 = stm32_hdp_pins,
+	.npins	 = ARRAY_SIZE(stm32_hdp_pins),
+	.pctlops = &stm32_hdp_pinctrl_ops,
+	.pmxops	 = &stm32_hdp_pinmux_ops,
+	.owner	 = THIS_MODULE,
+};
+
+static const struct of_device_id stm32_hdp_of_match[] = {
+	{
+		.compatible = "st,stm32mp-hdp"
+	},
+	{}
+};
+
+static int stm32_hdp_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct stm32_hdp *hdp;
+	u8 version;
+	int err;
+
+	hdp = devm_kzalloc(dev, sizeof(*hdp), GFP_KERNEL);
+	if (!hdp)
+		return -ENOMEM;
+	hdp->dev = dev;
+
+	platform_set_drvdata(pdev, hdp);
+
+	hdp->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(hdp->base))
+		return PTR_ERR(hdp->base);
+
+	hdp->clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(hdp->clk))
+		return dev_err_probe(dev, PTR_ERR(hdp->clk), "No HDP clock provided\n");
+
+	err = devm_pinctrl_register_and_init(dev, &stm32_hdp_pdesc, hdp, &hdp->pctl_dev);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to register pinctrl\n");
+
+	err = pinctrl_enable(hdp->pctl_dev);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to enable pinctrl\n");
+
+	hdp->gpio_chip.label	     = "stm32-hdp";
+	hdp->gpio_chip.parent	     = dev;
+	hdp->gpio_chip.get_direction = stm32_hdp_gpio_get_direction;
+	hdp->gpio_chip.get	     = stm32_hdp_gpio_get;
+	hdp->gpio_chip.set	     = stm32_hdp_gpio_set;
+	hdp->gpio_chip.base	     = -1;
+	hdp->gpio_chip.ngpio	     = ARRAY_SIZE(stm32_hdp_pins);
+	hdp->gpio_chip.can_sleep     = true;
+	hdp->gpio_chip.names	     = stm32_hdp_pins_group;
+
+	err = devm_gpiochip_add_data(dev, &hdp->gpio_chip, hdp);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to add gpiochip\n");
+
+	writel_relaxed(HDP_CTRL_ENABLE, hdp->base + HDP_CTRL);
+
+	version = readl_relaxed(hdp->base + HDP_VERR);
+	dev_dbg(dev, "STM32 HDP version %u.%u initialized\n", version >> 4, version & 0x0f);
+
+	return 0;
+}
+
+static void stm32_hdp_remove(struct platform_device *pdev)
+{
+	struct stm32_hdp *hdp = platform_get_drvdata(pdev);
+
+	writel_relaxed(HDP_CTRL_DISABLE, hdp->base + HDP_CTRL);
+}
+
+static int stm32_hdp_suspend(struct device *dev)
+{
+	struct stm32_hdp *hdp = dev_get_drvdata(dev);
+
+	hdp->gposet_conf = readl_relaxed(hdp->base + HDP_GPOSET);
+
+	pinctrl_pm_select_sleep_state(dev);
+
+	clk_disable_unprepare(hdp->clk);
+
+	return 0;
+}
+
+static int stm32_hdp_resume(struct device *dev)
+{
+	struct stm32_hdp *hdp = dev_get_drvdata(dev);
+	int err;
+
+	err = clk_prepare_enable(hdp->clk);
+	if (err)
+		return dev_err_probe(hdp->dev, err, "Failed to prepare_enable clk");
+
+	writel_relaxed(HDP_CTRL_ENABLE, hdp->base + HDP_CTRL);
+	writel_relaxed(hdp->gposet_conf, hdp->base + HDP_GPOSET);
+	writel_relaxed(hdp->mux_conf, hdp->base + HDP_MUX);
+
+	pinctrl_pm_select_default_state(dev);
+
+	return 0;
+}
+
+DEFINE_SIMPLE_DEV_PM_OPS(stm32_hdp_pm_ops, stm32_hdp_suspend, stm32_hdp_resume);
+
+static struct platform_driver stm32_hdp_driver = {
+	.probe = stm32_hdp_probe,
+	.remove = stm32_hdp_remove,
+	.driver = {
+		.name = DRIVER_NAME,
+		.pm = pm_sleep_ptr(&stm32_hdp_pm_ops),
+		.of_match_table = stm32_hdp_of_match,
+	}
+};
+
+module_platform_driver(stm32_hdp_driver);
+
+MODULE_ALIAS("platform:" DRIVER_NAME);
+MODULE_AUTHOR("Clément Le Goffic");
+MODULE_DESCRIPTION("STMicroelectronics STM32 Hardware Debug Port driver");
+MODULE_LICENSE("GPL");

-- 
2.43.0

[PATCH 4/9] MAINTAINERS: Add Clément Le Goffic as STM32 HDP maintainer

[Clément Le Goffic]

Add Clément Le Goffic as STM32 HDP maintainer.

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index efee40ea589f..c277b10cf48b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22425,6 +22425,13 @@ F:	drivers/bus/stm32_etzpc.c
 F:	drivers/bus/stm32_firewall.c
 F:	drivers/bus/stm32_rifsc.c
 
+ST STM32 HDP PINCTRL DRIVER
+M:	Clément Le Goffic <clement.legoffic@foss.st.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl-hdp.yaml
+F:	drivers/pinctrl/stm32/pinctrl-stm32-hdp.c
+F:	include/dt-bindings/pinctrl/stm32mp*-hdp.h
+
 ST STM32 I2C/SMBUS DRIVER
 M:	Pierre-Yves MORDRET <pierre-yves.mordret@foss.st.com>
 M:	Alain Volmat <alain.volmat@foss.st.com>

-- 
2.43.0

[PATCH 5/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp13

[Clément Le Goffic]

Add the hdp devicetree node for stm32mp13 SoC family

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 arch/arm/boot/dts/st/stm32mp131.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/st/stm32mp131.dtsi b/arch/arm/boot/dts/st/stm32mp131.dtsi
index 0019d12c3d3d..73c9971c4c80 100644
--- a/arch/arm/boot/dts/st/stm32mp131.dtsi
+++ b/arch/arm/boot/dts/st/stm32mp131.dtsi
@@ -919,6 +919,13 @@ timer {
 			};
 		};
 
+		hdp: pinctrl@5002a000 {
+			compatible = "st,stm32mp-hdp";
+			reg = <0x5002a000 0x400>;
+			clocks = <&rcc HDP>;
+			status = "disabled";
+		};
+
 		mdma: dma-controller@58000000 {
 			compatible = "st,stm32h7-mdma";
 			reg = <0x58000000 0x1000>;

-- 
2.43.0

[PATCH 6/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp15

[Clément Le Goffic]

Add the hdp devicetree node for stm32mp15 SoC family

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 arch/arm/boot/dts/st/stm32mp151.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/st/stm32mp151.dtsi b/arch/arm/boot/dts/st/stm32mp151.dtsi
index b9a87fbe971d..f16307887488 100644
--- a/arch/arm/boot/dts/st/stm32mp151.dtsi
+++ b/arch/arm/boot/dts/st/stm32mp151.dtsi
@@ -270,6 +270,13 @@ dts: thermal@50028000 {
 			status = "disabled";
 		};
 
+		hdp: pinctrl@5002a000 {
+			compatible = "st,stm32mp-hdp";
+			reg = <0x5002a000 0x400>;
+			clocks = <&rcc HDP>;
+			status = "disabled";
+		};
+
 		mdma1: dma-controller@58000000 {
 			compatible = "st,stm32h7-mdma";
 			reg = <0x58000000 0x1000>;

-- 
2.43.0

[PATCH 7/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp25

[Clément Le Goffic]

Add the hdp devicetree node for stm32mp25 SoC family

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 arch/arm64/boot/dts/st/stm32mp251.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
index f3c6cdfd7008..43aaed4fcf10 100644
--- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
@@ -918,6 +918,13 @@ package_otp@1e8 {
 			};
 		};
 
+		hdp: pinctrl@44090000 {
+			compatible = "st,stm32mp-hdp";
+			reg = <0x44090000 0x400>;
+			clocks = <&rcc CK_BUS_HDP>;
+			status = "disabled";
+		};
+
 		rcc: clock-controller@44200000 {
 			compatible = "st,stm32mp25-rcc";
 			reg = <0x44200000 0x10000>;

-- 
2.43.0

[PATCH 8/9] ARM: dts: stm32: add alternate pinmux for HDP pin and add HDP pinctrl node

[Clément Le Goffic]

Introduce hdp node to output a user defined value on port hdp2.
Add pinctrl nodes to be able to output this signal on one SoC pin.

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 arch/arm/boot/dts/st/stm32mp15-pinctrl.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/st/stm32mp15-pinctrl.dtsi b/arch/arm/boot/dts/st/stm32mp15-pinctrl.dtsi
index 95fafc51a1c8..76ff0d6634b4 100644
--- a/arch/arm/boot/dts/st/stm32mp15-pinctrl.dtsi
+++ b/arch/arm/boot/dts/st/stm32mp15-pinctrl.dtsi
@@ -4,6 +4,15 @@
  * Author: Ludovic Barre <ludovic.barre@st.com> for STMicroelectronics.
  */
 #include <dt-bindings/pinctrl/stm32-pinfunc.h>
+#include <dt-bindings/pinctrl/stm32mp15-hdp.h>
+
+&hdp {
+	/omit-if-no-ref/
+	hdp2_gpo: hdp2-pins {
+		function = HDP2_GPOVAL_2;
+		pins = "hdp2";
+	};
+};
 
 &pinctrl {
 	/omit-if-no-ref/
@@ -687,6 +696,23 @@ pins {
 		};
 	};
 
+	/omit-if-no-ref/
+	hdp2_pins_a: hdp2-0 {
+		pins {
+			pinmux = <STM32_PINMUX('E', 13, AF0)>; /* HDP2 */
+			bias-disable;
+			drive-push-pull;
+			slew-rate = <2>;
+		};
+	};
+
+	/omit-if-no-ref/
+	hdp2_sleep_pins_a: hdp2-sleep-0 {
+		pins {
+			pinmux = <STM32_PINMUX('E', 13, ANALOG)>; /* HDP2 */
+		};
+	};
+
 	/omit-if-no-ref/
 	i2c1_pins_a: i2c1-0 {
 		pins {

-- 
2.43.0

[PATCH 9/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp157c-dk2 board

[Clément Le Goffic]

On the stm32mp157fc-dk2 board, we can observe the hdp GPOVAL function on
SoC pin E13 accessible on the pin 5 on the Arduino connector CN13.
Add the relevant configuration but keep it disabled as it's used for
debug only.

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 arch/arm/boot/dts/st/stm32mp157c-dk2.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/st/stm32mp157c-dk2.dts b/arch/arm/boot/dts/st/stm32mp157c-dk2.dts
index 5f9c0160a9c4..b183b51f8f50 100644
--- a/arch/arm/boot/dts/st/stm32mp157c-dk2.dts
+++ b/arch/arm/boot/dts/st/stm32mp157c-dk2.dts
@@ -63,6 +63,12 @@ &dsi_out {
 	remote-endpoint = <&panel_in>;
 };
 
+&hdp {
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&hdp2_gpo &hdp2_pins_a>;
+	pinctrl-1 = <&hdp2_sleep_pins_a>;
+};
+
 &i2c1 {
 	touchscreen@38 {
 		compatible = "focaltech,ft6236";

-- 
2.43.0

Re: [PATCH 3/9] pinctrl: stm32: Introduce HDP driver

[Krzysztof Kozlowski]

On 25/02/2025 09:48, Clément Le Goffic wrote:
> This patch introduce the driver for the Hardware Debug Port available on

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> STM32MP platforms. The HDP allows the observation of internal SoC
> signals by using multiplexers. Each HDP port can provide up to 16
> internal signals (one of them can be software controlled as a GPO)



....

> +
> +static int stm32_hdp_suspend(struct device *dev)
> +{
> +	struct stm32_hdp *hdp = dev_get_drvdata(dev);
> +
> +	hdp->gposet_conf = readl_relaxed(hdp->base + HDP_GPOSET);
> +
> +	pinctrl_pm_select_sleep_state(dev);
> +
> +	clk_disable_unprepare(hdp->clk);
> +
> +	return 0;
> +}
> +
> +static int stm32_hdp_resume(struct device *dev)
> +{
> +	struct stm32_hdp *hdp = dev_get_drvdata(dev);
> +	int err;
> +
> +	err = clk_prepare_enable(hdp->clk);
> +	if (err)
> +		return dev_err_probe(hdp->dev, err, "Failed to prepare_enable clk");


That's wrong, it is not a probe path.

> +
> +	writel_relaxed(HDP_CTRL_ENABLE, hdp->base + HDP_CTRL);
> +	writel_relaxed(hdp->gposet_conf, hdp->base + HDP_GPOSET);
> +	writel_relaxed(hdp->mux_conf, hdp->base + HDP_MUX);
> +
> +	pinctrl_pm_select_default_state(dev);
> +
> +	return 0;
> +}
> +
> +DEFINE_SIMPLE_DEV_PM_OPS(stm32_hdp_pm_ops, stm32_hdp_suspend, stm32_hdp_resume);
> +
> +static struct platform_driver stm32_hdp_driver = {
> +	.probe = stm32_hdp_probe,
> +	.remove = stm32_hdp_remove,
> +	.driver = {
> +		.name = DRIVER_NAME,
> +		.pm = pm_sleep_ptr(&stm32_hdp_pm_ops),
> +		.of_match_table = stm32_hdp_of_match,
> +	}
> +};
> +
> +module_platform_driver(stm32_hdp_driver);
> +
> +MODULE_ALIAS("platform:" DRIVER_NAME);


You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong (e.g. misses either
entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
for incomplete ID table.




Best regards,
Krzysztof

Re: [PATCH 1/9] dt-bindings: pinctrl: stm32: Add HDP includes for stm32mp platforms

[Krzysztof Kozlowski]

On 25/02/2025 09:48, Clément Le Goffic wrote:
> Each file introduces helpers to choose the signal to monitor through the
> HDP pin.
> Signals are different for each platforms: stm32mp13, stm32mp15, stm32mp25.

Headers are part of bindings commit, assuming this stays...


> 
> Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
> ---
>  include/dt-bindings/pinctrl/stm32mp13-hdp.h | 130 +++++++++++++++++++++++++
>  include/dt-bindings/pinctrl/stm32mp15-hdp.h | 116 ++++++++++++++++++++++
>  include/dt-bindings/pinctrl/stm32mp25-hdp.h | 144 ++++++++++++++++++++++++++++
>  3 files changed, 390 insertions(+)
> 
> diff --git a/include/dt-bindings/pinctrl/stm32mp13-hdp.h b/include/dt-bindings/pinctrl/stm32mp13-hdp.h
> new file mode 100644
> index 000000000000..a3487e700143
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/stm32mp13-hdp.h
> @@ -0,0 +1,130 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */
> +/*
> + * Copyright (C) STMicroelectronics 2025 - All Rights Reserved
> + * Author: Clément Le Goffic <clement.legoffic@foss.st.com> for STMicroelectronics.
> + */
> +
> +#ifndef _DT_BINDINGS_STM32MP13_HDP_H
> +#define _DT_BINDINGS_STM32MP13_HDP_H
> +
> +/* define a macro for each function a HDP pin can transmit */
> +#define HDP0_PWR_PWRWAKE_SYS			 "0"


Why this is a string not a number?

Where is it used? I don't see usage in the driver, so this does not look
like binding (and DTS is not a driver).

Best regards,
Krzysztof

Re: [PATCH 2/9] dt-bindings: pinctrl: stm32: Introduce HDP

[Krzysztof Kozlowski]

On 25/02/2025 09:48, Clément Le Goffic wrote:
> +
> +maintainers:
> +  - Clément LE GOFFIC <clement.legoffic@foss.st.com>
> +
> +description: |


Do not need '|' unless you need to preserve formatting.

> +  STMicroelectronics's STM32 MPUs integrate a Hardware Debug Port (HDP).
> +  It allows to output internal signals on SoC's GPIO.
> +
> +properties:
> +  compatible:
> +    const: st,stm32mp-hdp

There is a mess in STM SoCs. Sometimes you call SoC stm32, sometimes
stm32mp and sometimes stm32mpXX.

Define for all your STM contributions what is the actual SoC. This
feedback was already given to ST.

> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +patternProperties:
> +  '-pins$':
> +    type: object
> +    $ref: pinmux-node.yaml#
> +
> +    properties:
> +      function:
> +        enum: [ "0", "1", "2", "3", "4", "5", "6", "7",
> +                "8", "9", "10", "11", "12", "13", "14",
> +                "15" ]

Function which has a number is not really useful. What does it even express?


> +
> +      pins:
> +        enum: [ hdp0, hdp1, hdp2, hdp3, hdp4, hdp5, hdp6, hdp7 ]
> +
> +    required:
> +      - function
> +      - pins
> +
> +    additionalProperties: false
> +
> +allOf:
> +  - $ref: pinctrl.yaml#
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/stm32mp1-clks.h>
> +    #include <dt-bindings/pinctrl/stm32mp15-hdp.h>
> +    //Example 1

Drop


Best regards,
Krzysztof

Re: [PATCH 7/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp25

[Krzysztof Kozlowski]

On 25/02/2025 09:48, Clément Le Goffic wrote:
> Add the hdp devicetree node for stm32mp25 SoC family
> 
> Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
> ---
>  arch/arm64/boot/dts/st/stm32mp251.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
> index f3c6cdfd7008..43aaed4fcf10 100644
> --- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
> +++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
> @@ -918,6 +918,13 @@ package_otp@1e8 {
>  			};
>  		};
>  
> +		hdp: pinctrl@44090000 {
> +			compatible = "st,stm32mp-hdp";

So here again - you have stm32mp251 SoC, but use entirely different
compatible.

Same feedback as with previous patchsets from ST. Please take it inside,
digest internally and do not repeat same issues.

Best regards,
Krzysztof

Re: [PATCH 1/9] dt-bindings: pinctrl: stm32: Add HDP includes for stm32mp platforms

[Clement LE GOFFIC]

On 2/25/25 14:01, Krzysztof Kozlowski wrote:
> On 25/02/2025 09:48, Clément Le Goffic wrote:
>> Each file introduces helpers to choose the signal to monitor through the
>> HDP pin.
>> Signals are different for each platforms: stm32mp13, stm32mp15, stm32mp25.
> 
> Headers are part of bindings commit, assuming this stays...

Ok will squash with the next patch

>>
>> Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
>> ---
>>   include/dt-bindings/pinctrl/stm32mp13-hdp.h | 130 +++++++++++++++++++++++++
>>   include/dt-bindings/pinctrl/stm32mp15-hdp.h | 116 ++++++++++++++++++++++
>>   include/dt-bindings/pinctrl/stm32mp25-hdp.h | 144 ++++++++++++++++++++++++++++
>>   3 files changed, 390 insertions(+)
>>
>> diff --git a/include/dt-bindings/pinctrl/stm32mp13-hdp.h b/include/dt-bindings/pinctrl/stm32mp13-hdp.h
>> new file mode 100644
>> index 000000000000..a3487e700143
>> --- /dev/null
>> +++ b/include/dt-bindings/pinctrl/stm32mp13-hdp.h
>> @@ -0,0 +1,130 @@
>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */
>> +/*
>> + * Copyright (C) STMicroelectronics 2025 - All Rights Reserved
>> + * Author: Clément Le Goffic <clement.legoffic@foss.st.com> for STMicroelectronics.
>> + */
>> +
>> +#ifndef _DT_BINDINGS_STM32MP13_HDP_H
>> +#define _DT_BINDINGS_STM32MP13_HDP_H
>> +
>> +/* define a macro for each function a HDP pin can transmit */
>> +#define HDP0_PWR_PWRWAKE_SYS			 "0"
> 
> 
> Why this is a string not a number?
> 
> Where is it used? I don't see usage in the driver, so this does not look
> like binding (and DTS is not a driver).

Those files are helpers for the devicetrees and may be included in 
stm32mp*-pinctrl.dtsi files.
It is a string because it is an helper for the `function` property of 
`pinmux-node.yaml` which is a string.

I understand that having a number as a string is not easily understandable.
I'll consider it in a V2 by trying to use the `pinmux` property.

> Best regards,
> Krzysztof

Best regards,

Clément

Re: [PATCH 2/9] dt-bindings: pinctrl: stm32: Introduce HDP

[Clement LE GOFFIC]

On 2/25/25 14:04, Krzysztof Kozlowski wrote:
> On 25/02/2025 09:48, Clément Le Goffic wrote:
>> +
>> +maintainers:
>> +  - Clément LE GOFFIC <clement.legoffic@foss.st.com>
>> +
>> +description: |
> 
> 
> Do not need '|' unless you need to preserve formatting.

Ok

>> +  STMicroelectronics's STM32 MPUs integrate a Hardware Debug Port (HDP).
>> +  It allows to output internal signals on SoC's GPIO.
>> +
>> +properties:
>> +  compatible:
>> +    const: st,stm32mp-hdp
> 
> There is a mess in STM SoCs. Sometimes you call SoC stm32, sometimes
> stm32mp and sometimes stm32mpXX.
> 
> Define for all your STM contributions what is the actual SoC. This
> feedback was already given to ST.
> 
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +patternProperties:
>> +  '-pins$':
>> +    type: object
>> +    $ref: pinmux-node.yaml#
>> +
>> +    properties:
>> +      function:
>> +        enum: [ "0", "1", "2", "3", "4", "5", "6", "7",
>> +                "8", "9", "10", "11", "12", "13", "14",
>> +                "15" ]
> 
> Function which has a number is not really useful. What does it even express?

As said in my previous answer, function names are very different from 
one platform to another. Numbers were used as string to be generic.
I'll consider it in a V2.

> 
>> +
>> +      pins:
>> +        enum: [ hdp0, hdp1, hdp2, hdp3, hdp4, hdp5, hdp6, hdp7 ]
>> +
>> +    required:
>> +      - function
>> +      - pins
>> +
>> +    additionalProperties: false
>> +
>> +allOf:
>> +  - $ref: pinctrl.yaml#
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/stm32mp1-clks.h>
>> +    #include <dt-bindings/pinctrl/stm32mp15-hdp.h>
>> +    //Example 1
> 
> Drop

Ok

> 
> Best regards,
> Krzysztof

Re: [PATCH 3/9] pinctrl: stm32: Introduce HDP driver

[Clement LE GOFFIC]

On 2/25/25 13:59, Krzysztof Kozlowski wrote:
> On 25/02/2025 09:48, Clément Le Goffic wrote:
>> This patch introduce the driver for the Hardware Debug Port available on
> 
> Please do not use "This commit/patch/change", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

Ok

> 
>> STM32MP platforms. The HDP allows the observation of internal SoC
>> signals by using multiplexers. Each HDP port can provide up to 16
>> internal signals (one of them can be software controlled as a GPO)
> 
> 
> 
> ....
> 
>> +
>> +static int stm32_hdp_suspend(struct device *dev)
>> +{
>> +	struct stm32_hdp *hdp = dev_get_drvdata(dev);
>> +
>> +	hdp->gposet_conf = readl_relaxed(hdp->base + HDP_GPOSET);
>> +
>> +	pinctrl_pm_select_sleep_state(dev);
>> +
>> +	clk_disable_unprepare(hdp->clk);
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_hdp_resume(struct device *dev)
>> +{
>> +	struct stm32_hdp *hdp = dev_get_drvdata(dev);
>> +	int err;
>> +
>> +	err = clk_prepare_enable(hdp->clk);
>> +	if (err)
>> +		return dev_err_probe(hdp->dev, err, "Failed to prepare_enable clk");
> 
> 
> That's wrong, it is not a probe path.

Indeed

> 
>> +
>> +	writel_relaxed(HDP_CTRL_ENABLE, hdp->base + HDP_CTRL);
>> +	writel_relaxed(hdp->gposet_conf, hdp->base + HDP_GPOSET);
>> +	writel_relaxed(hdp->mux_conf, hdp->base + HDP_MUX);
>> +
>> +	pinctrl_pm_select_default_state(dev);
>> +
>> +	return 0;
>> +}
>> +
>> +DEFINE_SIMPLE_DEV_PM_OPS(stm32_hdp_pm_ops, stm32_hdp_suspend, stm32_hdp_resume);
>> +
>> +static struct platform_driver stm32_hdp_driver = {
>> +	.probe = stm32_hdp_probe,
>> +	.remove = stm32_hdp_remove,
>> +	.driver = {
>> +		.name = DRIVER_NAME,
>> +		.pm = pm_sleep_ptr(&stm32_hdp_pm_ops),
>> +		.of_match_table = stm32_hdp_of_match,
>> +	}
>> +};
>> +
>> +module_platform_driver(stm32_hdp_driver);
>> +
>> +MODULE_ALIAS("platform:" DRIVER_NAME);
> 
> 
> You should not need MODULE_ALIAS() in normal cases. If you need it,
> usually it means your device ID table is wrong (e.g. misses either
> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
> for incomplete ID table.
> 

Ok

> 
> 
> Best regards,
> Krzysztof

Best regards,

Clément

Re: [PATCH 7/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp25

[Clement LE GOFFIC]

On 2/25/25 14:05, Krzysztof Kozlowski wrote:
> On 25/02/2025 09:48, Clément Le Goffic wrote:
>> Add the hdp devicetree node for stm32mp25 SoC family
>>
>> Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
>> ---
>>   arch/arm64/boot/dts/st/stm32mp251.dtsi | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
>> index f3c6cdfd7008..43aaed4fcf10 100644
>> --- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
>> +++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
>> @@ -918,6 +918,13 @@ package_otp@1e8 {
>>   			};
>>   		};
>>   
>> +		hdp: pinctrl@44090000 {
>> +			compatible = "st,stm32mp-hdp";
> 
> So here again - you have stm32mp251 SoC, but use entirely different
> compatible.

Ok so I will use "st,stm32mp15-hdp"

> Same feedback as with previous patchsets from ST. Please take it inside,
> digest internally and do not repeat same issues.
> 
> Best regards,
> Krzysztof

Best regards,

Clément

Re: [PATCH 1/9] dt-bindings: pinctrl: stm32: Add HDP includes for stm32mp platforms

[Krzysztof Kozlowski]

On 25/02/2025 16:46, Clement LE GOFFIC wrote:
>>
>> Why this is a string not a number?
>>
>> Where is it used? I don't see usage in the driver, so this does not look
>> like binding (and DTS is not a driver).
> 
> Those files are helpers for the devicetrees and may be included in 
> stm32mp*-pinctrl.dtsi files.

So not a binding, see other platforms/header files how it is done.

> It is a string because it is an helper for the `function` property of 
> `pinmux-node.yaml` which is a string.
> 
> I understand that having a number as a string is not easily understandable.
> I'll consider it in a V2 by trying to use the `pinmux` property.

Let's keep discussing this part in the bindings doc patch.

Best regards,
Krzysztof

Re: [PATCH 2/9] dt-bindings: pinctrl: stm32: Introduce HDP

[Krzysztof Kozlowski]

On 25/02/2025 16:51, Clement LE GOFFIC wrote:
> On 2/25/25 14:04, Krzysztof Kozlowski wrote:
>> On 25/02/2025 09:48, Clément Le Goffic wrote:
>>> +
>>> +maintainers:
>>> +  - Clément LE GOFFIC <clement.legoffic@foss.st.com>
>>> +
>>> +description: |
>>
>>
>> Do not need '|' unless you need to preserve formatting.
> 
> Ok
> 
>>> +  STMicroelectronics's STM32 MPUs integrate a Hardware Debug Port (HDP).
>>> +  It allows to output internal signals on SoC's GPIO.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: st,stm32mp-hdp
>>
>> There is a mess in STM SoCs. Sometimes you call SoC stm32, sometimes
>> stm32mp and sometimes stm32mpXX.
>>
>> Define for all your STM contributions what is the actual SoC. This
>> feedback was already given to ST.
>>
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +patternProperties:
>>> +  '-pins$':
>>> +    type: object
>>> +    $ref: pinmux-node.yaml#
>>> +
>>> +    properties:
>>> +      function:
>>> +        enum: [ "0", "1", "2", "3", "4", "5", "6", "7",
>>> +                "8", "9", "10", "11", "12", "13", "14",
>>> +                "15" ]
>>
>> Function which has a number is not really useful. What does it even express?
> 
> As said in my previous answer, function names are very different from 
> one platform to another. Numbers were used as string to be generic.
> I'll consider it in a V2.

What does it mean "one platform to another"? This is one platform! Is
this some sort of continuation of SoC compatible mess?

What are the exact functions written in datasheet?

Best regards,
Krzysztof

Re: [PATCH 7/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp25

[Krzysztof Kozlowski]

On 25/02/2025 17:09, Clement LE GOFFIC wrote:
> On 2/25/25 14:05, Krzysztof Kozlowski wrote:
>> On 25/02/2025 09:48, Clément Le Goffic wrote:
>>> Add the hdp devicetree node for stm32mp25 SoC family
>>>
>>> Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
>>> ---
>>>   arch/arm64/boot/dts/st/stm32mp251.dtsi | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
>>> index f3c6cdfd7008..43aaed4fcf10 100644
>>> --- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
>>> +++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
>>> @@ -918,6 +918,13 @@ package_otp@1e8 {
>>>   			};
>>>   		};
>>>   
>>> +		hdp: pinctrl@44090000 {
>>> +			compatible = "st,stm32mp-hdp";
>>
>> So here again - you have stm32mp251 SoC, but use entirely different
>> compatible.
> 
> Ok so I will use "st,stm32mp15-hdp"


This means this is stm32mp15 SoC. I do not see such SoC on list of your
SoCs in bindings. What's more, there are no bindings for other SoC
components for stm32mp15!

Something is here not matching - this change, this DTSI, top level
bindings or all of your SoC device/blocks bindings.



Best regards,
Krzysztof

Re: [PATCH 7/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp25

[Alexandre TORGUE]

Hi Krzysztof

On 2/26/25 08:23, Krzysztof Kozlowski wrote:
> On 25/02/2025 17:09, Clement LE GOFFIC wrote:
>> On 2/25/25 14:05, Krzysztof Kozlowski wrote:
>>> On 25/02/2025 09:48, Clément Le Goffic wrote:
>>>> Add the hdp devicetree node for stm32mp25 SoC family
>>>>
>>>> Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
>>>> ---
>>>>    arch/arm64/boot/dts/st/stm32mp251.dtsi | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
>>>> index f3c6cdfd7008..43aaed4fcf10 100644
>>>> --- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
>>>> +++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
>>>> @@ -918,6 +918,13 @@ package_otp@1e8 {
>>>>    			};
>>>>    		};
>>>>    
>>>> +		hdp: pinctrl@44090000 {
>>>> +			compatible = "st,stm32mp-hdp";
>>>
>>> So here again - you have stm32mp251 SoC, but use entirely different
>>> compatible.
>>
>> Ok so I will use "st,stm32mp15-hdp"
> 
> 
> This means this is stm32mp15 SoC. I do not see such SoC on list of your
> SoCs in bindings. What's more, there are no bindings for other SoC
> components for stm32mp15!

Yes stm32mp15 is not a "real SoC". I agree that at the beginning of the 
STM32 story we didn't have a clear rule/view to correctly naming our 
compatible. We tried to improve the situation to avoid compatible like 
"st,stm32", "st,stm32mp" or "st,stm32mp1". So we introduced 
"st,stm32mp13", "st,stm32mp15" or "st,stm32mp25" for new drivers. So yes 
it represents a SoC family and not a real SoC. We haven't had much 
negative feedback it.

But, if it's not clean to do it in this way, lets define SoC compatible 
for any new driver.
For the HDP case it is: "st,stm32mp157" and used for STM32MP13, 
STM32MP15 end STM32MP25 SoC families (if driver is the same for all 
those SoCs).

regards
Alex


> Something is here not matching - this change, this DTSI, top level
> bindings or all of your SoC device/blocks bindings.
> 
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH 2/9] dt-bindings: pinctrl: stm32: Introduce HDP

[Clement LE GOFFIC]

On 2/26/25 08:21, Krzysztof Kozlowski wrote:
> On 25/02/2025 16:51, Clement LE GOFFIC wrote:
>> On 2/25/25 14:04, Krzysztof Kozlowski wrote:
>>> On 25/02/2025 09:48, Clément Le Goffic wrote:
>>>> +
>>>> +maintainers:
>>>> +  - Clément LE GOFFIC <clement.legoffic@foss.st.com>
>>>> +
>>>> +description: |
>>>
>>>
>>> Do not need '|' unless you need to preserve formatting.
>>
>> Ok
>>
>>>> +  STMicroelectronics's STM32 MPUs integrate a Hardware Debug Port (HDP).
>>>> +  It allows to output internal signals on SoC's GPIO.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: st,stm32mp-hdp
>>>
>>> There is a mess in STM SoCs. Sometimes you call SoC stm32, sometimes
>>> stm32mp and sometimes stm32mpXX.
>>>
>>> Define for all your STM contributions what is the actual SoC. This
>>> feedback was already given to ST.
>>>
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  clocks:
>>>> +    maxItems: 1
>>>> +
>>>> +patternProperties:
>>>> +  '-pins$':
>>>> +    type: object
>>>> +    $ref: pinmux-node.yaml#
>>>> +
>>>> +    properties:
>>>> +      function:
>>>> +        enum: [ "0", "1", "2", "3", "4", "5", "6", "7",
>>>> +                "8", "9", "10", "11", "12", "13", "14",
>>>> +                "15" ]
>>>
>>> Function which has a number is not really useful. What does it even express?
>>
>> As said in my previous answer, function names are very different from
>> one platform to another. Numbers were used as string to be generic.
>> I'll consider it in a V2.
> 
> What does it mean "one platform to another"? This is one platform! Is
> this some sort of continuation of SoC compatible mess?

I may used incorrectly the word platform.
This driver is the same for the three SoC families STM32MP13, STM32MP15 
and STM32MP25 because the hardware is mostly the same.

Why mostly ?

The peripheral is behaving as a mux, there are 8 HDP ports, for each 
port there is up to 16 possible hardware signals. Numbered from 0 to 15.
Each of this number represent a signal on the port.

But the hardware signal behind the number is not the same from one SoC 
family to another.
As example, in STM32MP15 family the HDP is able to output GPU hardware 
signals because the family has a GPU but in the STM32MP13 family this 
signal is not present.

The purpose of my helpers was to give a readable name to facilitate the 
configuration in boards devicetree's. If needed I can get rid of that 
and use only the number as string.

> What are the exact functions written in datasheet?

The exact functions name written in the datasheet are the ones of my 
helper file without the HDP prefix.


> Best regards,
> Krzysztof

Re: [PATCH 2/9] dt-bindings: pinctrl: stm32: Introduce HDP

[Krzysztof Kozlowski]

On 26/02/2025 11:52, Clement LE GOFFIC wrote:
> On 2/26/25 08:21, Krzysztof Kozlowski wrote:
>> On 25/02/2025 16:51, Clement LE GOFFIC wrote:
>>> On 2/25/25 14:04, Krzysztof Kozlowski wrote:
>>>> On 25/02/2025 09:48, Clément Le Goffic wrote:
>>>>> +
>>>>> +maintainers:
>>>>> +  - Clément LE GOFFIC <clement.legoffic@foss.st.com>
>>>>> +
>>>>> +description: |
>>>>
>>>>
>>>> Do not need '|' unless you need to preserve formatting.
>>>
>>> Ok
>>>
>>>>> +  STMicroelectronics's STM32 MPUs integrate a Hardware Debug Port (HDP).
>>>>> +  It allows to output internal signals on SoC's GPIO.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    const: st,stm32mp-hdp
>>>>
>>>> There is a mess in STM SoCs. Sometimes you call SoC stm32, sometimes
>>>> stm32mp and sometimes stm32mpXX.
>>>>
>>>> Define for all your STM contributions what is the actual SoC. This
>>>> feedback was already given to ST.
>>>>
>>>>> +
>>>>> +  reg:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  clocks:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +patternProperties:
>>>>> +  '-pins$':
>>>>> +    type: object
>>>>> +    $ref: pinmux-node.yaml#
>>>>> +
>>>>> +    properties:
>>>>> +      function:
>>>>> +        enum: [ "0", "1", "2", "3", "4", "5", "6", "7",
>>>>> +                "8", "9", "10", "11", "12", "13", "14",
>>>>> +                "15" ]
>>>>
>>>> Function which has a number is not really useful. What does it even express?
>>>
>>> As said in my previous answer, function names are very different from
>>> one platform to another. Numbers were used as string to be generic.
>>> I'll consider it in a V2.
>>
>> What does it mean "one platform to another"? This is one platform! Is
>> this some sort of continuation of SoC compatible mess?
> 
> I may used incorrectly the word platform.
> This driver is the same for the three SoC families STM32MP13, STM32MP15 

That's driver and it is fine, but we talk about hardware here. The
binding is for given specific hardware.

> and STM32MP25 because the hardware is mostly the same.
> 
> Why mostly ?
> 
> The peripheral is behaving as a mux, there are 8 HDP ports, for each 
> port there is up to 16 possible hardware signals. Numbered from 0 to 15.
> Each of this number represent a signal on the port.
> 
> But the hardware signal behind the number is not the same from one SoC 
> family to another.
> As example, in STM32MP15 family the HDP is able to output GPU hardware 
> signals because the family has a GPU but in the STM32MP13 family this 
> signal is not present.

It looks like you have clear mapping between function and port number
(your header also suggests that), so the function property should follow
that user-visible function.

Just like we do for many other architectures - it is not that very, very
different, I think. all of platform hardwares do not operate on strings
but some bits in registers (so numbers) but all (ideally) bindings
operate on strings. You created here exception on basis this is somehow
special, but the point is: it is not special.

> 
> The purpose of my helpers was to give a readable name to facilitate the 
> configuration in boards devicetree's. If needed I can get rid of that 
> and use only the number as string.

If you use "names" you do not need even that helper header.

> 
>> What are the exact functions written in datasheet?
> 
> The exact functions name written in the datasheet are the ones of my 
> helper file without the HDP prefix.

so full strings "pwr_pwrwake_sys" and these should be used.

Best regards,
Krzysztof

Re: [PATCH 7/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp25

[Krzysztof Kozlowski]

On 26/02/2025 10:33, Alexandre TORGUE wrote:
>>>>> +		hdp: pinctrl@44090000 {
>>>>> +			compatible = "st,stm32mp-hdp";
>>>>
>>>> So here again - you have stm32mp251 SoC, but use entirely different
>>>> compatible.
>>>
>>> Ok so I will use "st,stm32mp15-hdp"
>>
>>
>> This means this is stm32mp15 SoC. I do not see such SoC on list of your
>> SoCs in bindings. What's more, there are no bindings for other SoC
>> components for stm32mp15!
> 
> Yes stm32mp15 is not a "real SoC". I agree that at the beginning of the 
> STM32 story we didn't have a clear rule/view to correctly naming our 
> compatible. We tried to improve the situation to avoid compatible like 
> "st,stm32", "st,stm32mp" or "st,stm32mp1". So we introduced 
> "st,stm32mp13", "st,stm32mp15" or "st,stm32mp25" for new drivers. So yes 
> it represents a SoC family and not a real SoC. We haven't had much 
> negative feedback it.
> 
> But, if it's not clean to do it in this way, lets define SoC compatible 
> for any new driver.

Compatibles are for hardware.

> For the HDP case it is: "st,stm32mp157" and used for STM32MP13, 
> STM32MP15 end STM32MP25 SoC families (if driver is the same for all 
> those SoCs).

No, it's three compatibles, because you have three SoCs. BTW, writing
bindings (and online resources and previous reviews and my talks) are
saying that, so we do not ask for anything new here, anything different.
At least not new when looking at last 5 years, because 10 years ago many
rules were relaxed...



Best regards,
Krzysztof

Re: [PATCH 7/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp25

[Alexandre TORGUE]

On 2/26/25 16:08, Krzysztof Kozlowski wrote:
> On 26/02/2025 10:33, Alexandre TORGUE wrote:
>>>>>> +		hdp: pinctrl@44090000 {
>>>>>> +			compatible = "st,stm32mp-hdp";
>>>>>
>>>>> So here again - you have stm32mp251 SoC, but use entirely different
>>>>> compatible.
>>>>
>>>> Ok so I will use "st,stm32mp15-hdp"
>>>
>>>
>>> This means this is stm32mp15 SoC. I do not see such SoC on list of your
>>> SoCs in bindings. What's more, there are no bindings for other SoC
>>> components for stm32mp15!
>>
>> Yes stm32mp15 is not a "real SoC". I agree that at the beginning of the
>> STM32 story we didn't have a clear rule/view to correctly naming our
>> compatible. We tried to improve the situation to avoid compatible like
>> "st,stm32", "st,stm32mp" or "st,stm32mp1". So we introduced
>> "st,stm32mp13", "st,stm32mp15" or "st,stm32mp25" for new drivers. So yes
>> it represents a SoC family and not a real SoC. We haven't had much
>> negative feedback it.
>>
>> But, if it's not clean to do it in this way, lets define SoC compatible
>> for any new driver.
> 
> Compatibles are for hardware.
> 
>> For the HDP case it is: "st,stm32mp157" and used for STM32MP13,
>> STM32MP15 end STM32MP25 SoC families (if driver is the same for all
>> those SoCs).
> 
> No, it's three compatibles, because you have three SoCs. BTW, writing
> bindings (and online resources and previous reviews and my talks) are
> saying that, so we do not ask for anything new here, anything different.
> At least not new when looking at last 5 years, because 10 years ago many
> rules were relaxed...

So adding 3 times the same IP in 3 different SoCs implies to have 3 
different compatibles. So each time we use this same IP in a new SoC, we 
have to add a new compatible. My (wrong) understanding was: as we have 
the same IP (same hardware) in each SoC we have the same compatible (and 
IP integration differences (clocks, interrupts) are handled by DT 
properties.

> 
> 
> Best regards,
> Krzysztof

Re: [PATCH 7/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp25

[Alexandre TORGUE]

On 2/26/25 16:30, Alexandre TORGUE wrote:
> 
> 
> On 2/26/25 16:08, Krzysztof Kozlowski wrote:
>> On 26/02/2025 10:33, Alexandre TORGUE wrote:
>>>>>>> +        hdp: pinctrl@44090000 {
>>>>>>> +            compatible = "st,stm32mp-hdp";
>>>>>>
>>>>>> So here again - you have stm32mp251 SoC, but use entirely different
>>>>>> compatible.
>>>>>
>>>>> Ok so I will use "st,stm32mp15-hdp"
>>>>
>>>>
>>>> This means this is stm32mp15 SoC. I do not see such SoC on list of your
>>>> SoCs in bindings. What's more, there are no bindings for other SoC
>>>> components for stm32mp15!
>>>
>>> Yes stm32mp15 is not a "real SoC". I agree that at the beginning of the
>>> STM32 story we didn't have a clear rule/view to correctly naming our
>>> compatible. We tried to improve the situation to avoid compatible like
>>> "st,stm32", "st,stm32mp" or "st,stm32mp1". So we introduced
>>> "st,stm32mp13", "st,stm32mp15" or "st,stm32mp25" for new drivers. So yes
>>> it represents a SoC family and not a real SoC. We haven't had much
>>> negative feedback it.
>>>
>>> But, if it's not clean to do it in this way, lets define SoC compatible
>>> for any new driver.
>>
>> Compatibles are for hardware.
>>
>>> For the HDP case it is: "st,stm32mp157" and used for STM32MP13,
>>> STM32MP15 end STM32MP25 SoC families (if driver is the same for all
>>> those SoCs).
>>
>> No, it's three compatibles, because you have three SoCs. BTW, writing
>> bindings (and online resources and previous reviews and my talks) are
>> saying that, so we do not ask for anything new here, anything different.
>> At least not new when looking at last 5 years, because 10 years ago many
>> rules were relaxed...
> 
> So adding 3 times the same IP in 3 different SoCs implies to have 3 
> different compatibles. So each time we use this same IP in a new SoC, we 
> have to add a new compatible. My (wrong) understanding was: as we have 
> the same IP (same hardware) in each SoC we have the same compatible (and 
> IP integration differences (clocks, interrupts) are handled by DT 
> properties.

Just to complete, reading the Linux kernel doc, as device are same we 
will use fallbacks like this:

MP15: compatible = "st,stm32mp151-hdp";
MP13: compatible = "st,stm32mp131-hdp", "st,stm32mp151-hdp";
MP25: compatible = "st,stm32mp251-hdp", "st,stm32mp151-hdp";

> 
>>
>>
>> Best regards,
>> Krzysztof

Re: [PATCH 7/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp25

[Krzysztof Kozlowski]

On 26/02/2025 16:30, Alexandre TORGUE wrote:
> 
> 
> On 2/26/25 16:08, Krzysztof Kozlowski wrote:
>> On 26/02/2025 10:33, Alexandre TORGUE wrote:
>>>>>>> +		hdp: pinctrl@44090000 {
>>>>>>> +			compatible = "st,stm32mp-hdp";
>>>>>>
>>>>>> So here again - you have stm32mp251 SoC, but use entirely different
>>>>>> compatible.
>>>>>
>>>>> Ok so I will use "st,stm32mp15-hdp"
>>>>
>>>>
>>>> This means this is stm32mp15 SoC. I do not see such SoC on list of your
>>>> SoCs in bindings. What's more, there are no bindings for other SoC
>>>> components for stm32mp15!
>>>
>>> Yes stm32mp15 is not a "real SoC". I agree that at the beginning of the
>>> STM32 story we didn't have a clear rule/view to correctly naming our
>>> compatible. We tried to improve the situation to avoid compatible like
>>> "st,stm32", "st,stm32mp" or "st,stm32mp1". So we introduced
>>> "st,stm32mp13", "st,stm32mp15" or "st,stm32mp25" for new drivers. So yes
>>> it represents a SoC family and not a real SoC. We haven't had much
>>> negative feedback it.
>>>
>>> But, if it's not clean to do it in this way, lets define SoC compatible
>>> for any new driver.
>>
>> Compatibles are for hardware.
>>
>>> For the HDP case it is: "st,stm32mp157" and used for STM32MP13,
>>> STM32MP15 end STM32MP25 SoC families (if driver is the same for all
>>> those SoCs).
>>
>> No, it's three compatibles, because you have three SoCs. BTW, writing
>> bindings (and online resources and previous reviews and my talks) are
>> saying that, so we do not ask for anything new here, anything different.
>> At least not new when looking at last 5 years, because 10 years ago many
>> rules were relaxed...
> 
> So adding 3 times the same IP in 3 different SoCs implies to have 3 

Yes. Always, as requested by writing bindings.

> different compatibles. So each time we use this same IP in a new SoC, we 
> have to add a new compatible. My (wrong) understanding was: as we have 

Yes, as requested by writing bindings and followed up by all recent
platforms having decent/active upstream support. See qcom, nxp, renesas
for example.

> the same IP (same hardware) in each SoC we have the same compatible (and 

You do not have same hardware. You have same IP, or almost same because
they are almost never same, implemented in different hardware.

> IP integration differences (clocks, interrupts) are handled by DT 
> properties.

Which binding doc/guide suggested such way? Countless reviews from DT
maintainers were saying opposite.

Best regards,
Krzysztof

Re: [PATCH 7/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp25

[Krzysztof Kozlowski]

On 26/02/2025 17:54, Alexandre TORGUE wrote:
>>>> But, if it's not clean to do it in this way, lets define SoC compatible
>>>> for any new driver.
>>>
>>> Compatibles are for hardware.
>>>
>>>> For the HDP case it is: "st,stm32mp157" and used for STM32MP13,
>>>> STM32MP15 end STM32MP25 SoC families (if driver is the same for all
>>>> those SoCs).
>>>
>>> No, it's three compatibles, because you have three SoCs. BTW, writing
>>> bindings (and online resources and previous reviews and my talks) are
>>> saying that, so we do not ask for anything new here, anything different.
>>> At least not new when looking at last 5 years, because 10 years ago many
>>> rules were relaxed...
>>
>> So adding 3 times the same IP in 3 different SoCs implies to have 3 
>> different compatibles. So each time we use this same IP in a new SoC, we 
>> have to add a new compatible. My (wrong) understanding was: as we have 
>> the same IP (same hardware) in each SoC we have the same compatible (and 
>> IP integration differences (clocks, interrupts) are handled by DT 
>> properties.
> 
> Just to complete, reading the Linux kernel doc, as device are same we 
> will use fallbacks like this:
> 
> MP15: compatible = "st,stm32mp151-hdp";
> MP13: compatible = "st,stm32mp131-hdp", "st,stm32mp151-hdp";
> MP25: compatible = "st,stm32mp251-hdp", "st,stm32mp151-hdp";

Yes, this looks correct.

Best regards,
Krzysztof

Re: [PATCH 7/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp25

[Krzysztof Kozlowski]

On 26/02/2025 22:26, Krzysztof Kozlowski wrote:
> On 26/02/2025 16:30, Alexandre TORGUE wrote:
>>
>>
>> On 2/26/25 16:08, Krzysztof Kozlowski wrote:
>>> On 26/02/2025 10:33, Alexandre TORGUE wrote:
>>>>>>>> +		hdp: pinctrl@44090000 {
>>>>>>>> +			compatible = "st,stm32mp-hdp";
>>>>>>>
>>>>>>> So here again - you have stm32mp251 SoC, but use entirely different
>>>>>>> compatible.
>>>>>>
>>>>>> Ok so I will use "st,stm32mp15-hdp"
>>>>>
>>>>>
>>>>> This means this is stm32mp15 SoC. I do not see such SoC on list of your
>>>>> SoCs in bindings. What's more, there are no bindings for other SoC
>>>>> components for stm32mp15!
>>>>
>>>> Yes stm32mp15 is not a "real SoC". I agree that at the beginning of the
>>>> STM32 story we didn't have a clear rule/view to correctly naming our
>>>> compatible. We tried to improve the situation to avoid compatible like
>>>> "st,stm32", "st,stm32mp" or "st,stm32mp1". So we introduced
>>>> "st,stm32mp13", "st,stm32mp15" or "st,stm32mp25" for new drivers. So yes
>>>> it represents a SoC family and not a real SoC. We haven't had much
>>>> negative feedback it.
>>>>
>>>> But, if it's not clean to do it in this way, lets define SoC compatible
>>>> for any new driver.
>>>
>>> Compatibles are for hardware.
>>>
>>>> For the HDP case it is: "st,stm32mp157" and used for STM32MP13,
>>>> STM32MP15 end STM32MP25 SoC families (if driver is the same for all
>>>> those SoCs).
>>>
>>> No, it's three compatibles, because you have three SoCs. BTW, writing
>>> bindings (and online resources and previous reviews and my talks) are
>>> saying that, so we do not ask for anything new here, anything different.
>>> At least not new when looking at last 5 years, because 10 years ago many
>>> rules were relaxed...
>>
>> So adding 3 times the same IP in 3 different SoCs implies to have 3 
> 
> Yes. Always, as requested by writing bindings.
> 
>> different compatibles. So each time we use this same IP in a new SoC, we 
>> have to add a new compatible. My (wrong) understanding was: as we have 
> 
> Yes, as requested by writing bindings and followed up by all recent
> platforms having decent/active upstream support. See qcom, nxp, renesas
> for example.
> 
>> the same IP (same hardware) in each SoC we have the same compatible (and 
> 
> You do not have same hardware. You have same IP, or almost same because
> they are almost never same, implemented in different hardware.
> 
>> IP integration differences (clocks, interrupts) are handled by DT 
>> properties.
> 
> Which binding doc/guide suggested such way? Countless reviews from DT
> maintainers were saying opposite.
I was not precise: IP integration differences are of course handles as
DT properties, but I wanted to say that it does not solve the problem
that IP integration means you might have differences in this device and
you should have different quirks.

And the example in this patchset: entirely different pin functions is a
proof. This device behaves/operates/integrates differently, thus
different compatible.

Best regards,
Krzysztof

Re: [PATCH 2/9] dt-bindings: pinctrl: stm32: Introduce HDP

[Clement LE GOFFIC]

On 2/26/25 16:05, Krzysztof Kozlowski wrote:
> On 26/02/2025 11:52, Clement LE GOFFIC wrote:
>> On 2/26/25 08:21, Krzysztof Kozlowski wrote:
>>> On 25/02/2025 16:51, Clement LE GOFFIC wrote:
>>>> On 2/25/25 14:04, Krzysztof Kozlowski wrote:
>>>>> On 25/02/2025 09:48, Clément Le Goffic wrote:
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Clément LE GOFFIC <clement.legoffic@foss.st.com>
>>>>>> +
>>>>>> +description: |
>>>>>
>>>>>
>>>>> Do not need '|' unless you need to preserve formatting.
>>>>
>>>> Ok
>>>>
>>>>>> +  STMicroelectronics's STM32 MPUs integrate a Hardware Debug Port (HDP).
>>>>>> +  It allows to output internal signals on SoC's GPIO.
>>>>>> +
>>>>>> +properties:
>>>>>> +  compatible:
>>>>>> +    const: st,stm32mp-hdp
>>>>>
>>>>> There is a mess in STM SoCs. Sometimes you call SoC stm32, sometimes
>>>>> stm32mp and sometimes stm32mpXX.
>>>>>
>>>>> Define for all your STM contributions what is the actual SoC. This
>>>>> feedback was already given to ST.
>>>>>
>>>>>> +
>>>>>> +  reg:
>>>>>> +    maxItems: 1
>>>>>> +
>>>>>> +  clocks:
>>>>>> +    maxItems: 1
>>>>>> +
>>>>>> +patternProperties:
>>>>>> +  '-pins$':
>>>>>> +    type: object
>>>>>> +    $ref: pinmux-node.yaml#
>>>>>> +
>>>>>> +    properties:
>>>>>> +      function:
>>>>>> +        enum: [ "0", "1", "2", "3", "4", "5", "6", "7",
>>>>>> +                "8", "9", "10", "11", "12", "13", "14",
>>>>>> +                "15" ]
>>>>>
>>>>> Function which has a number is not really useful. What does it even express?
>>>>
>>>> As said in my previous answer, function names are very different from
>>>> one platform to another. Numbers were used as string to be generic.
>>>> I'll consider it in a V2.
>>>
>>> What does it mean "one platform to another"? This is one platform! Is
>>> this some sort of continuation of SoC compatible mess?
>>
>> I may used incorrectly the word platform.
>> This driver is the same for the three SoC families STM32MP13, STM32MP15
> 
> That's driver and it is fine, but we talk about hardware here. The
> binding is for given specific hardware.
> 
>> and STM32MP25 because the hardware is mostly the same.
>>
>> Why mostly ?
>>
>> The peripheral is behaving as a mux, there are 8 HDP ports, for each
>> port there is up to 16 possible hardware signals. Numbered from 0 to 15.
>> Each of this number represent a signal on the port.
>>
>> But the hardware signal behind the number is not the same from one SoC
>> family to another.
>> As example, in STM32MP15 family the HDP is able to output GPU hardware
>> signals because the family has a GPU but in the STM32MP13 family this
>> signal is not present.
> 
> It looks like you have clear mapping between function and port number
> (your header also suggests that), so the function property should follow
> that user-visible function.
> 
> Just like we do for many other architectures - it is not that very, very
> different, I think. all of platform hardwares do not operate on strings
> but some bits in registers (so numbers) but all (ideally) bindings
> operate on strings. You created here exception on basis this is somehow
> special, but the point is: it is not special.
> 
>>
>> The purpose of my helpers was to give a readable name to facilitate the
>> configuration in boards devicetree's. If needed I can get rid of that
>> and use only the number as string.
> 
> If you use "names" you do not need even that helper header.
> 
>>
>>> What are the exact functions written in datasheet?
>>
>> The exact functions name written in the datasheet are the ones of my
>> helper file without the HDP prefix.
> 
> so full strings "pwr_pwrwake_sys" and these should be used.

Ok so in the V2, I'll keep the 'function' property of the pinmux and use 
signal names such as 'pwr_pwrwake_sys' to select signals in the DT.
The signal names are different from one SoC to another (stm32mp131, 
stm32mp151 and stm32mp251) so I'll need compatible data and the 
compatibles will be:

MP15: compatible = "st,stm32mp151-hdp";
MP13: compatible = "st,stm32mp131-hdp";
MP25: compatible = "st,stm32mp251-hdp";


> Best regards,
> Krzysztof

Re: [PATCH 7/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp25

[Alexandre TORGUE]

On 2/26/25 22:31, Krzysztof Kozlowski wrote:
> On 26/02/2025 22:26, Krzysztof Kozlowski wrote:
>> On 26/02/2025 16:30, Alexandre TORGUE wrote:
>>>
>>>
>>> On 2/26/25 16:08, Krzysztof Kozlowski wrote:
>>>> On 26/02/2025 10:33, Alexandre TORGUE wrote:
>>>>>>>>> +		hdp: pinctrl@44090000 {
>>>>>>>>> +			compatible = "st,stm32mp-hdp";
>>>>>>>>
>>>>>>>> So here again - you have stm32mp251 SoC, but use entirely different
>>>>>>>> compatible.
>>>>>>>
>>>>>>> Ok so I will use "st,stm32mp15-hdp"
>>>>>>
>>>>>>
>>>>>> This means this is stm32mp15 SoC. I do not see such SoC on list of your
>>>>>> SoCs in bindings. What's more, there are no bindings for other SoC
>>>>>> components for stm32mp15!
>>>>>
>>>>> Yes stm32mp15 is not a "real SoC". I agree that at the beginning of the
>>>>> STM32 story we didn't have a clear rule/view to correctly naming our
>>>>> compatible. We tried to improve the situation to avoid compatible like
>>>>> "st,stm32", "st,stm32mp" or "st,stm32mp1". So we introduced
>>>>> "st,stm32mp13", "st,stm32mp15" or "st,stm32mp25" for new drivers. So yes
>>>>> it represents a SoC family and not a real SoC. We haven't had much
>>>>> negative feedback it.
>>>>>
>>>>> But, if it's not clean to do it in this way, lets define SoC compatible
>>>>> for any new driver.
>>>>
>>>> Compatibles are for hardware.
>>>>
>>>>> For the HDP case it is: "st,stm32mp157" and used for STM32MP13,
>>>>> STM32MP15 end STM32MP25 SoC families (if driver is the same for all
>>>>> those SoCs).
>>>>
>>>> No, it's three compatibles, because you have three SoCs. BTW, writing
>>>> bindings (and online resources and previous reviews and my talks) are
>>>> saying that, so we do not ask for anything new here, anything different.
>>>> At least not new when looking at last 5 years, because 10 years ago many
>>>> rules were relaxed...
>>>
>>> So adding 3 times the same IP in 3 different SoCs implies to have 3
>>
>> Yes. Always, as requested by writing bindings.
>>
>>> different compatibles. So each time we use this same IP in a new SoC, we
>>> have to add a new compatible. My (wrong) understanding was: as we have
>>
>> Yes, as requested by writing bindings and followed up by all recent
>> platforms having decent/active upstream support. See qcom, nxp, renesas
>> for example.
>>
>>> the same IP (same hardware) in each SoC we have the same compatible (and
>>
>> You do not have same hardware. You have same IP, or almost same because
>> they are almost never same, implemented in different hardware.
>>
>>> IP integration differences (clocks, interrupts) are handled by DT
>>> properties.
>>
>> Which binding doc/guide suggested such way? Countless reviews from DT
>> maintainers were saying opposite.
> I was not precise: IP integration differences are of course handles as
> DT properties, but I wanted to say that it does not solve the problem
> that IP integration means you might have differences in this device and
> you should have different quirks.

Yes I agree. We'll take care of it for future development. Maybe, It 
would be nice to apply this rule in our current drivers/DT already 
upstream ?

> 
> And the example in this patchset: entirely different pin functions is a
> proof. This device behaves/operates/integrates differently, thus
> different compatible.

Yes, discussing with Clement, it is clear that we need 3 different 
compatibles.

> Best regards,
> Krzysztof