[PATCH 00/13] Introduce STM32 DDR PMU for STM32MP platforms

[PATCH 00/13] Introduce STM32 DDR PMU for STM32MP platforms

[Clément Le Goffic]

This patch series introduces the DDR Performance Monitor (DDRPERFM) support for
STM32MP platforms.

The series firstly improves the STM32MP25 RCC driver to make it usable
as an access controller, needed for driver probe.

It also includes the addition of device tree bindings, the HDP driver,
documentation 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 DDR PMU driver.

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
Clément Le Goffic (13):
      bus: firewall: move stm32_firewall header file in include folder
      dt-bindings: stm32: stm32mp25: add `access-controller-cell` property
      clk: stm32mp25: add firewall grant_access ops
      arm64: dts: st: set rcc as an access-controller
      dt-bindings: perf: stm32: introduce DDRPERFM dt-bindings
      perf: stm32: introduce DDRPERFM driver
      Documentation: perf: stm32: add ddrperfm support
      MAINTAINERS: add myself as STM32 DDR PMU maintainer
      ARM: dts: stm32: add ddrperfm on stm32mp131
      ARM: dts: stm32: add ddrperfm on stm32mp151
      arm64: dts: st: add ddrperfm on stm32mp251
      arm64: dts: st: support ddrperfm on stm32mp257f-dk
      arm64: dts: st: support ddrperfm on stm32mp257f-ev1

 Documentation/admin-guide/perf/index.rst           |   1 +
 Documentation/admin-guide/perf/stm32-ddr-pmu.rst   |  86 ++
 .../bindings/clock/st,stm32mp25-rcc.yaml           |   6 +
 .../devicetree/bindings/perf/st,stm32-ddr-pmu.yaml |  93 +++
 MAINTAINERS                                        |   7 +
 arch/arm/boot/dts/st/stm32mp131.dtsi               |   7 +
 arch/arm/boot/dts/st/stm32mp151.dtsi               |   7 +
 arch/arm64/boot/dts/st/stm32mp251.dtsi             |   8 +
 arch/arm64/boot/dts/st/stm32mp257f-dk.dts          |   5 +
 arch/arm64/boot/dts/st/stm32mp257f-ev1.dts         |   5 +
 drivers/bus/stm32_etzpc.c                          |   3 +-
 drivers/bus/stm32_firewall.c                       |   3 +-
 drivers/bus/stm32_rifsc.c                          |   3 +-
 drivers/clk/stm32/clk-stm32mp25.c                  |  40 +-
 drivers/perf/Kconfig                               |  11 +
 drivers/perf/Makefile                              |   1 +
 drivers/perf/stm32_ddr_pmu.c                       | 893 +++++++++++++++++++++
 {drivers => include/linux}/bus/stm32_firewall.h    |   0
 18 files changed, 1172 insertions(+), 7 deletions(-)
---
base-commit: 86731a2a651e58953fc949573895f2fa6d456841
change-id: 20250526-ddrperfm-upstream-bf07f57775da

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

[PATCH 01/13] bus: firewall: move stm32_firewall header file in include folder

[Clément Le Goffic]

Other driver than rifsc and etzpc can implement firewall ops, such as
rcc.
In order for them to have access to the ops and type of this framework,
we need to get the `stm32_firewall.h` file in the include/ folder.

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 drivers/bus/stm32_etzpc.c                       | 3 +--
 drivers/bus/stm32_firewall.c                    | 3 +--
 drivers/bus/stm32_rifsc.c                       | 3 +--
 {drivers => include/linux}/bus/stm32_firewall.h | 0
 4 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/bus/stm32_etzpc.c b/drivers/bus/stm32_etzpc.c
index 7fc0f16960be..4918a14e507e 100644
--- a/drivers/bus/stm32_etzpc.c
+++ b/drivers/bus/stm32_etzpc.c
@@ -5,6 +5,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/bus/stm32_firewall.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/init.h>
@@ -16,8 +17,6 @@
 #include <linux/platform_device.h>
 #include <linux/types.h>
 
-#include "stm32_firewall.h"
-
 /*
  * ETZPC registers
  */
diff --git a/drivers/bus/stm32_firewall.c b/drivers/bus/stm32_firewall.c
index 2fc9761dadec..ef4988054b44 100644
--- a/drivers/bus/stm32_firewall.c
+++ b/drivers/bus/stm32_firewall.c
@@ -5,6 +5,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/bus/stm32_firewall.h>
 #include <linux/bus/stm32_firewall_device.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -18,8 +19,6 @@
 #include <linux/types.h>
 #include <linux/slab.h>
 
-#include "stm32_firewall.h"
-
 /* Corresponds to STM32_FIREWALL_MAX_EXTRA_ARGS + firewall ID */
 #define STM32_FIREWALL_MAX_ARGS		(STM32_FIREWALL_MAX_EXTRA_ARGS + 1)
 
diff --git a/drivers/bus/stm32_rifsc.c b/drivers/bus/stm32_rifsc.c
index 4cf1b60014b7..643ddd0a5f54 100644
--- a/drivers/bus/stm32_rifsc.c
+++ b/drivers/bus/stm32_rifsc.c
@@ -5,6 +5,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/bus/stm32_firewall.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/init.h>
@@ -16,8 +17,6 @@
 #include <linux/platform_device.h>
 #include <linux/types.h>
 
-#include "stm32_firewall.h"
-
 /*
  * RIFSC offset register
  */
diff --git a/drivers/bus/stm32_firewall.h b/include/linux/bus/stm32_firewall.h
similarity index 100%
rename from drivers/bus/stm32_firewall.h
rename to include/linux/bus/stm32_firewall.h

-- 
2.43.0

[PATCH 02/13] dt-bindings: stm32: stm32mp25: add `access-controller-cell` property

[Clément Le Goffic]

RCC is able to check the availability of a clock.
Allow to query the RCC with a firewall ID.

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml b/Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml
index 88e52f10d1ec..83974b7fb82f 100644
--- a/Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml
+++ b/Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml
@@ -31,6 +31,11 @@ properties:
   '#reset-cells':
     const: 1
 
+  '#access-controller-cells':
+    const: 1
+    description:
+      Contains the firewall ID associated to the peripheral.
+
   clocks:
     items:
       - description: CK_SCMI_HSE High Speed External oscillator (8 to 48 MHz)
@@ -123,6 +128,7 @@ required:
   - reg
   - '#clock-cells'
   - '#reset-cells'
+  - '#access-controller-cells'
   - clocks
 
 additionalProperties: false

-- 
2.43.0

[PATCH 03/13] clk: stm32mp25: add firewall grant_access ops

[Clément Le Goffic]

On STM32MP25, the RCC peripheral manages the secure level of resources
that are used by other devices such as clocks.
Declare this peripheral as a firewall controller.

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 drivers/clk/stm32/clk-stm32mp25.c | 40 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/stm32/clk-stm32mp25.c b/drivers/clk/stm32/clk-stm32mp25.c
index 52f0e8a12926..af4bc06d703a 100644
--- a/drivers/clk/stm32/clk-stm32mp25.c
+++ b/drivers/clk/stm32/clk-stm32mp25.c
@@ -4,8 +4,10 @@
  * Author: Gabriel Fernandez <gabriel.fernandez@foss.st.com> for STMicroelectronics.
  */
 
+#include <linux/bus/stm32_firewall.h>
 #include <linux/bus/stm32_firewall_device.h>
 #include <linux/clk-provider.h>
+#include <linux/device.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
 
@@ -1602,6 +1604,11 @@ static int stm32_rcc_get_access(void __iomem *base, u32 index)
 	return 0;
 }
 
+static int stm32mp25_rcc_grant_access(struct stm32_firewall_controller *ctrl, u32 firewall_id)
+{
+	return stm32_rcc_get_access(ctrl->mmio, firewall_id);
+}
+
 static int stm32mp25_check_security(struct device_node *np, void __iomem *base,
 				    const struct clock_config *cfg)
 {
@@ -1970,6 +1977,7 @@ MODULE_DEVICE_TABLE(of, stm32mp25_match_data);
 
 static int stm32mp25_rcc_clocks_probe(struct platform_device *pdev)
 {
+	struct stm32_firewall_controller *rcc_controller;
 	struct device *dev = &pdev->dev;
 	void __iomem *base;
 	int ret;
@@ -1982,7 +1990,36 @@ static int stm32mp25_rcc_clocks_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	return stm32_rcc_init(dev, stm32mp25_match_data, base);
+	ret = stm32_rcc_init(dev, stm32mp25_match_data, base);
+	if (ret)
+		return ret;
+
+	rcc_controller = devm_kzalloc(&pdev->dev, sizeof(*rcc_controller), GFP_KERNEL);
+	if (!rcc_controller)
+		return -ENOMEM;
+
+	rcc_controller->dev = dev;
+	rcc_controller->mmio = base;
+	rcc_controller->name = dev_driver_string(dev);
+	rcc_controller->type = STM32_PERIPHERAL_FIREWALL;
+	rcc_controller->grant_access = stm32mp25_rcc_grant_access;
+
+	platform_set_drvdata(pdev, rcc_controller);
+
+	ret = stm32_firewall_controller_register(rcc_controller);
+	if (ret) {
+		dev_err(dev, "Couldn't register as a firewall controller: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void stm32mp25_rcc_clocks_remove(struct platform_device *pdev)
+{
+	struct stm32_firewall_controller *rcc_controller = platform_get_drvdata(pdev);
+
+	stm32_firewall_controller_unregister(rcc_controller);
 }
 
 static struct platform_driver stm32mp25_rcc_clocks_driver = {
@@ -1991,6 +2028,7 @@ static struct platform_driver stm32mp25_rcc_clocks_driver = {
 		.of_match_table = stm32mp25_match_data,
 	},
 	.probe = stm32mp25_rcc_clocks_probe,
+	.remove = stm32mp25_rcc_clocks_remove,
 };
 
 static int __init stm32mp25_clocks_init(void)

-- 
2.43.0

[PATCH 04/13] arm64: dts: st: set rcc as an access-controller

[Clément Le Goffic]

RCC now implements firewall access ops to check the access to
resources. Allow client nodes to query the RCC with one firewall ID.

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

diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
index 8d87865850a7..0683c2d5cb6f 100644
--- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
@@ -1153,6 +1153,7 @@ rcc: clock-controller@44200000 {
 			reg = <0x44200000 0x10000>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;
+			#access-controller-cells = <1>;
 			clocks = <&scmi_clk CK_SCMI_HSE>,
 				<&scmi_clk CK_SCMI_HSI>,
 				<&scmi_clk CK_SCMI_MSI>,

-- 
2.43.0

[PATCH 05/13] dt-bindings: perf: stm32: introduce DDRPERFM dt-bindings

[Clément Le Goffic]

DDRPERFM is the DDR Performance Monitor embedded in STM32MPU SoC.
It allows to monitor DDR events that come from the DDR Controller
such as read or write events.

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

diff --git a/Documentation/devicetree/bindings/perf/st,stm32-ddr-pmu.yaml b/Documentation/devicetree/bindings/perf/st,stm32-ddr-pmu.yaml
new file mode 100644
index 000000000000..35d34782865b
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/st,stm32-ddr-pmu.yaml
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/perf/st,stm32-ddr-pmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+maintainers:
+  - Clément Le Goffic <clement.legoffic@foss.st.com>
+
+title: STMicroelectronics STM32 DDR Performance Monitor (DDRPERFM)
+
+properties:
+  compatible:
+    enum:
+      - st,stm32mp131-ddr-pmu
+      - st,stm32mp151-ddr-pmu
+      - st,stm32mp251-ddr-pmu
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    description: Reference clock for the DDR Performance Monitor
+    maxItems: 1
+
+  resets:
+    description: Reset control for the DDR Performance Monitor
+    maxItems: 1
+
+  access-controllers:
+    minItems: 1
+    maxItems: 2
+
+  st,dram-type:
+    description: |
+      This property is used to specify the type of DRAM memory connected to the
+      associated memory controller. It is required for the DDR Performance Monitor
+      to correctly interpret the performance data.
+      0 = LPDDR4,
+      1 = LPDDR3,
+      2 = DDR4,
+      3 = DDR3
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2, 3]
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - st,stm32mp131-ddr-pmu
+              - st,stm32mp151-ddr-pmu
+    then:
+      required:
+        - clocks
+        - resets
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: st,stm32mp251-ddr-pmu
+    then:
+      required:
+        - access-controllers
+        - st,dram-type
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/stm32mp1-clks.h>
+    #include <dt-bindings/reset/stm32mp1-resets.h>
+
+    perf@5a007000 {
+        compatible = "st,stm32mp151-ddr-pmu";
+        reg = <0x5a007000 0x400>;
+        clocks = <&rcc DDRPERFM>;
+        resets = <&rcc DDRPERFM_R>;
+    };
+
+  - |
+    perf@48041000 {
+      compatible = "st,stm32mp251-ddr-pmu";
+      reg = <0x48041000 0x400>;
+      access-controllers = <&rcc 104>;
+      st,dram-type = <2>;
+    };

-- 
2.43.0

[PATCH 06/13] perf: stm32: introduce DDRPERFM driver

[Clément Le Goffic]

Introduce the driver for the DDR Performance Monitor available on
STM32MPU SoC.

On STM32MP2 platforms, the DDRPERFM allows to monitor up to 8 DDR events
that come from the DDR Controller such as read or write events.

On STM32MP1 platforms, the DDRPERFM cannot monitor any event on any
counter, there is a notion of set of events.
Events from different sets cannot be monitored at the same time.
The first chosen event selects the set.
The set is coded in the first two bytes of the config value which is on 4
bytes.

On STM32MP25x series, the DDRPERFM clock is shared with the DDR controller
and may be secured by bootloaders.
Access controllers allow to check access to a resource. Use the access
controller defined in the devicetree to know about the access to the
DDRPERFM clock.

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 drivers/perf/Kconfig         |  11 +
 drivers/perf/Makefile        |   1 +
 drivers/perf/stm32_ddr_pmu.c | 893 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 905 insertions(+)

diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index 278c929dc87a..5118535134ee 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -198,6 +198,17 @@ config QCOM_L3_PMU
 	   Adds the L3 cache PMU into the perf events subsystem for
 	   monitoring L3 cache events.
 
+config STM32_DDR_PMU
+	tristate "STM32 DDR PMU"
+	depends on ARCH_STM32 || COMPILE_TEST
+	default m
+	help
+	  Provides support for the DDR performance monitor on STM32MPU platforms.
+	  The monitor provides counters for memory related events.
+	  It allows developers to analyze and optimize DDR performance.
+	  Enabling this feature is useful for performance tuning and debugging memory
+	  subsystem issues on supported hardware.
+
 config THUNDERX2_PMU
 	tristate "Cavium ThunderX2 SoC PMU UNCORE"
 	depends on ARCH_THUNDER2 || COMPILE_TEST
diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
index de71d2574857..7f83b50feb71 100644
--- a/drivers/perf/Makefile
+++ b/drivers/perf/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_RISCV_PMU) += riscv_pmu.o
 obj-$(CONFIG_RISCV_PMU_LEGACY) += riscv_pmu_legacy.o
 obj-$(CONFIG_RISCV_PMU_SBI) += riscv_pmu_sbi.o
 obj-$(CONFIG_STARFIVE_STARLINK_PMU) += starfive_starlink_pmu.o
+obj-$(CONFIG_STM32_DDR_PMU) += stm32_ddr_pmu.o
 obj-$(CONFIG_THUNDERX2_PMU) += thunderx2_pmu.o
 obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
 obj-$(CONFIG_ARM_SPE_PMU) += arm_spe_pmu.o
diff --git a/drivers/perf/stm32_ddr_pmu.c b/drivers/perf/stm32_ddr_pmu.c
new file mode 100644
index 000000000000..c0bce1f446a0
--- /dev/null
+++ b/drivers/perf/stm32_ddr_pmu.c
@@ -0,0 +1,893 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025, STMicroelectronics - All Rights Reserved
+ * Author: Clément Le Goffic <clement.legoffic@foss.st.com> for STMicroelectronics.
+ */
+
+#include <linux/bus/stm32_firewall_device.h>
+#include <linux/clk.h>
+#include <linux/hrtimer.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/perf_event.h>
+#include <linux/reset.h>
+
+#define DRIVER_NAME		"stm32_ddr_pmu"
+
+/*
+ * The PMU is able to freeze all counters and generate an interrupt when there
+ * is a counter overflow. But, relying on this means that we lose all the
+ * events that occur between the freeze and the interrupt handler execution.
+ * So we use a polling mechanism to avoid this lost of information.
+ * The fastest counter can overflow in ~7s @600MHz (that is the maximum DDR
+ * frequency supported on STM32MP257), so we poll in 3.5s intervals to ensure
+ * we don't reach this limit.
+ */
+#define POLL_MS			3500
+
+#define DDRPERFM_CTRL		0x000
+#define DDRPERFM_CFG		0x004
+#define DDRPERFM_STATUS		0x008
+#define DDRPERFM_CLR		0x00C
+#define DDRPERFM_TCNT		0x020
+#define DDRPERFM_EVCNT(X)	(0x030 + 8 * (X))
+
+#define DDRPERFM_MP2_CFG0	0x010
+#define DDRPERFM_MP2_CFG1	0x014
+#define DDRPERFM_MP2_CFG5	0x024
+#define DDRPERFM_MP2_DRAMINF	0x028
+#define DDRPERFM_MP2_EVCNT(X)	(0x040 + 4 * (X))
+#define DDRPERFM_MP2_TCNT	0x060
+#define DDRPERFM_MP2_STATUS	0x080
+
+#define MP1_STATUS_BUSY		BIT(16)
+#define MP2_STATUS_BUSY		BIT(31)
+
+#define CTRL_START		BIT(0)
+#define CTRL_STOP		BIT(1)
+
+#define CFG_SEL_MSK		GENMASK(17, 16)
+#define CFG_SEL_SHIFT		16
+#define CFG_EN_MSK		GENMASK(3, 0)
+
+#define MP1_CLR_CNT		GENMASK(3, 0)
+#define MP1_CLR_TIME		BIT(31)
+#define MP2_CLR_CNT		GENMASK(7, 0)
+#define MP2_CLR_TIME		BIT(8)
+
+/* 4 event counters plus 1 dedicated to time */
+#define MP1_CNT_NB		(4 + 1)
+/* Index of the time dedicated counter */
+#define MP1_TIME_CNT_IDX	4
+
+/* 8 event counters plus 1 dedicated to time */
+#define MP2_CNT_NB		(8 + 1)
+/* Index of the time dedicated counter */
+#define MP2_TIME_CNT_IDX	8
+/* 4 event counters per register */
+#define MP2_CNT_SEL_PER_REG	4
+
+/* Arbitrary value used to identify a time event */
+#define TIME_CNT		64
+
+struct stm32_ddr_pmu_reg {
+	unsigned int reg;
+	u32 mask;
+};
+
+struct stm32_ddr_cnt {
+	int idx;
+	struct perf_event *evt;
+	struct list_head cnt_list;
+};
+
+struct stm32_ddr_pmu_regspec {
+	const struct stm32_ddr_pmu_reg stop;
+	const struct stm32_ddr_pmu_reg start;
+	const struct stm32_ddr_pmu_reg enable;
+	const struct stm32_ddr_pmu_reg status;
+	const struct stm32_ddr_pmu_reg clear_cnt;
+	const struct stm32_ddr_pmu_reg clear_time;
+	const struct stm32_ddr_pmu_reg cfg;
+	const struct stm32_ddr_pmu_reg cfg0;
+	const struct stm32_ddr_pmu_reg cfg1;
+	const struct stm32_ddr_pmu_reg dram_inf;
+	const struct stm32_ddr_pmu_reg counter_time;
+	const struct stm32_ddr_pmu_reg counter_evt[];
+};
+
+struct stm32_ddr_pmu {
+	struct pmu pmu;
+	void __iomem *membase;
+	struct device *dev;
+	struct clk *clk;
+	const struct stm32_ddr_pmu_cfg *cfg;
+	struct hrtimer hrtimer;
+	ktime_t poll_period;
+	int selected_set;
+	u32 dram_type;
+	struct list_head counters[];
+};
+
+struct stm32_ddr_pmu_cfg {
+	const struct stm32_ddr_pmu_regspec *regs;
+	const struct attribute_group **attribute;
+	u32 counters_nb;
+	u32 evt_counters_nb;
+	u32 time_cnt_idx;
+	struct stm32_ddr_cnt * (*get_counter)(struct stm32_ddr_pmu *p, struct perf_event *e);
+};
+
+#define EVENT_NUMBER(group, index)	(((group) << 8) | (index))
+#define GROUP_VALUE(event_number)		((event_number) >> 8)
+#define EVENT_INDEX(event_number)	((event_number) & 0xFF)
+
+/* MP1 ddrperfm events */
+enum stm32_ddr_pmu_events_mp1 {
+	PERF_OP_IS_RD			= EVENT_NUMBER(0, 0),
+	PERF_OP_IS_WR			= EVENT_NUMBER(0, 1),
+	PERF_OP_IS_ACTIVATE		= EVENT_NUMBER(0, 2),
+	CTL_IDLE			= EVENT_NUMBER(0, 3),
+	PERF_HPR_REQ_WITH_NO_CREDIT	= EVENT_NUMBER(1, 0),
+	PERF_LPR_REQ_WITH_NO_CREDIT	= EVENT_NUMBER(1, 1),
+	CACTIVE_DDRC			= EVENT_NUMBER(1, 3),
+	PERF_OP_IS_ENTER_POWERDOWN	= EVENT_NUMBER(2, 0),
+	PERF_OP_IS_REFRESH		= EVENT_NUMBER(2, 1),
+	PERF_SELFRESH_MODE		= EVENT_NUMBER(2, 2),
+	DFI_LP_REQ			= EVENT_NUMBER(2, 3),
+	PERF_HPR_XACT_WHEN_CRITICAL	= EVENT_NUMBER(3, 0),
+	PERF_LPR_XACT_WHEN_CRITICAL	= EVENT_NUMBER(3, 1),
+	PERF_WR_XACT_WHEN_CRITICAL	= EVENT_NUMBER(3, 2),
+	DFI_LP_REQ_SCND			= EVENT_NUMBER(3, 3),
+};
+
+/* MP2 ddrperfm events */
+enum stm32_ddr_pmu_events_mp2 {
+	DFI_IS_ACT			= EVENT_NUMBER(0, 0),
+	DFI_IS_PREPB			= EVENT_NUMBER(0, 1),
+	DFI_IS_PREAB			= EVENT_NUMBER(0, 2),
+	DFI_IS_RD			= EVENT_NUMBER(0, 3),
+	DFI_IS_RDA			= EVENT_NUMBER(0, 4),
+	DFI_IS_WR			= EVENT_NUMBER(0, 6),
+	DFI_IS_WRA			= EVENT_NUMBER(0, 7),
+	DFI_IS_MWR			= EVENT_NUMBER(0, 9),
+	DFI_IS_MWRA			= EVENT_NUMBER(0, 10),
+	DFI_IS_MRW			= EVENT_NUMBER(0, 12),
+	DFI_IS_MRR			= EVENT_NUMBER(0, 13),
+	DFI_IS_REFPB			= EVENT_NUMBER(0, 14),
+	DFI_IS_REFAB			= EVENT_NUMBER(0, 15),
+	DFI_IS_MPC			= EVENT_NUMBER(0, 16),
+	PERF_OP_IS_ACT			= EVENT_NUMBER(0, 32),
+	PERF_OP_IS_RD_MP2		= EVENT_NUMBER(0, 33),
+	PERF_OP_IS_WR_MP2		= EVENT_NUMBER(0, 34),
+	PERF_OP_IS_MWR			= EVENT_NUMBER(0, 35),
+	PERF_OP_IS_REF			= EVENT_NUMBER(0, 36),
+	PERF_OP_IS_CRIT_REF		= EVENT_NUMBER(0, 37),
+	PERF_OP_IS_SPEC_REF		= EVENT_NUMBER(0, 38),
+	PERF_OP_IS_ZQCAL		= EVENT_NUMBER(0, 39),
+	PERF_OP_IS_ENTER_POWDN		= EVENT_NUMBER(0, 40),
+	PERF_OP_IS_ENTER_SELFREF	= EVENT_NUMBER(0, 41),
+	PERF_OP_IS_PRE			= EVENT_NUMBER(0, 42),
+	PERF_OP_IS_PRE_FOR_RDWR		= EVENT_NUMBER(0, 43),
+	PERF_OP_IS_PRE_FOR_OTHERS	= EVENT_NUMBER(0, 44),
+	PERF_OP_IS_RD_ACTIVATE		= EVENT_NUMBER(0, 45),
+	PERF_HPR_REQ_WITH_NOCREDIT	= EVENT_NUMBER(0, 48),
+	PERF_LPR_REQ_WITH_NOCREDIT	= EVENT_NUMBER(0, 49),
+	PERF_HPR_XACT_WHEN_CRITICAL_MP2	= EVENT_NUMBER(0, 50),
+	PERF_LPR_XACT_WHEN_CRITICAL_MP2	= EVENT_NUMBER(0, 51),
+	PERF_WR_XACT_WHEN_CRITICAL_MP2	= EVENT_NUMBER(0, 52),
+	PERF_RDWR_TRANSITIONS		= EVENT_NUMBER(0, 53),
+	PERF_WAR_HAZARD			= EVENT_NUMBER(0, 54),
+	PERF_RAW_HAZARD			= EVENT_NUMBER(0, 55),
+	PERF_WAW_HAZARD			= EVENT_NUMBER(0, 56),
+	PERF_RANK			= EVENT_NUMBER(0, 58),
+	PERF_READ_BYPASS		= EVENT_NUMBER(0, 59),
+	PERF_ACT_BYPASS			= EVENT_NUMBER(0, 60),
+	PERF_WINDOW_LIMIT_REACHED_RD	= EVENT_NUMBER(0, 61),
+	PERF_WINDOW_LIMIT_REACHED_WR	= EVENT_NUMBER(0, 62),
+	NO_EVENT			= EVENT_NUMBER(0, 63),
+};
+
+static struct stm32_ddr_pmu *to_stm32_ddr_pmu(struct pmu *p)
+{
+	return container_of(p, struct stm32_ddr_pmu, pmu);
+}
+
+static struct stm32_ddr_pmu *hrtimer_to_stm32_ddr_pmu(struct hrtimer *h)
+{
+	return container_of(h, struct stm32_ddr_pmu, hrtimer);
+}
+
+static void stm32_ddr_start_counters(struct stm32_ddr_pmu *pmu)
+{
+	const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
+
+	writel_relaxed(r->start.mask, pmu->membase + r->start.reg);
+}
+
+static void stm32_ddr_stop_counters(struct stm32_ddr_pmu *pmu)
+{
+	const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
+
+	writel_relaxed(r->stop.mask, pmu->membase + r->stop.reg);
+}
+
+static void stm32_ddr_clear_time_counter(struct stm32_ddr_pmu *pmu)
+{
+	const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
+
+	writel_relaxed(r->clear_time.mask, pmu->membase + r->clear_time.reg);
+}
+
+static void stm32_ddr_clear_event_counter(struct stm32_ddr_pmu *pmu, struct stm32_ddr_cnt *counter)
+{
+	const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
+
+	writel_relaxed(r->clear_cnt.mask & BIT(counter->idx), pmu->membase + r->clear_cnt.reg);
+}
+
+static void stm32_ddr_clear_counter(struct stm32_ddr_pmu *pmu, struct stm32_ddr_cnt *counter)
+{
+	const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
+	u32 status = readl_relaxed(pmu->membase + r->status.reg);
+
+	if (counter->idx == pmu->cfg->time_cnt_idx)
+		stm32_ddr_clear_time_counter(pmu);
+	else
+		stm32_ddr_clear_event_counter(pmu, counter);
+
+	if (status & r->status.mask)
+		dev_err(pmu->dev, "Failed to clear counter %i because the PMU is busy\n",
+			counter->idx);
+}
+
+static void stm32_ddr_counter_enable(struct stm32_ddr_pmu *pmu, struct stm32_ddr_cnt *counter)
+{
+	const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
+	u32 val = readl_relaxed(pmu->membase + r->enable.reg);
+
+	val |= BIT(counter->idx);
+	writel_relaxed(val, pmu->membase + r->enable.reg);
+}
+
+static void stm32_ddr_counter_disable(struct stm32_ddr_pmu *pmu, struct stm32_ddr_cnt *counter)
+{
+	const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
+	u32 val = readl_relaxed(pmu->membase + r->enable.reg);
+
+	val &= ~BIT(counter->idx);
+	writel_relaxed(val, pmu->membase + r->enable.reg);
+}
+
+static int stm32_ddr_sel_evnt(struct stm32_ddr_pmu *pmu, struct stm32_ddr_cnt *counter)
+{
+	const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
+	u32 cnt_sel_val;
+
+	u32 group_val = GROUP_VALUE(counter->evt->attr.config);
+	u32 evt_val = EVENT_INDEX(counter->evt->attr.config);
+
+	if (pmu->selected_set != -1 && pmu->selected_set != group_val) {
+		dev_err(pmu->dev, "Selected events are from different set\n");
+		return -EINVAL;
+	}
+	pmu->selected_set = group_val;
+
+	if (pmu->cfg->regs->cfg.reg) {
+		cnt_sel_val = readl_relaxed(pmu->membase + r->cfg.reg);
+		cnt_sel_val &= ~CFG_SEL_MSK;
+		cnt_sel_val |= (CFG_SEL_MSK & (group_val << CFG_SEL_SHIFT));
+		writel_relaxed(cnt_sel_val, pmu->membase + r->cfg.reg);
+
+		return 0;
+	}
+
+	/* We assume cfg0 and cfg1 are filled in the match data */
+	u32 cnt_idx = counter->idx;
+	u32 cnt_sel_evt_reg = r->cfg0.reg;
+
+	if (!(cnt_idx < MP2_CNT_SEL_PER_REG)) {
+		cnt_sel_evt_reg = r->cfg1.reg;
+		cnt_idx -= MP2_CNT_SEL_PER_REG;
+	}
+
+	cnt_sel_val = readl_relaxed(pmu->membase + cnt_sel_evt_reg);
+	cnt_sel_val &= ~GENMASK(8 * cnt_idx + 7, 8 * cnt_idx);
+	cnt_sel_val |= evt_val << (8 * cnt_idx);
+
+	writel_relaxed(cnt_sel_val, pmu->membase + cnt_sel_evt_reg);
+
+	return 0;
+}
+
+static struct stm32_ddr_cnt *stm32_ddr_pmu_get_event_counter_mp1(struct stm32_ddr_pmu *pmu,
+								 struct perf_event *event)
+{
+	u32 config = event->attr.config;
+	u32 event_idx = EVENT_INDEX(config);
+	struct stm32_ddr_cnt *cnt;
+
+	cnt = kzalloc(sizeof(*cnt), GFP_KERNEL);
+	if (!cnt)
+		return ERR_PTR(-ENOMEM);
+
+	cnt->evt = event;
+	cnt->idx = event_idx;
+	event->pmu_private = cnt;
+	list_add(&cnt->cnt_list, &pmu->counters[event_idx]);
+
+	return cnt;
+}
+
+static struct stm32_ddr_cnt *stm32_ddr_pmu_get_event_counter_mp2(struct stm32_ddr_pmu *pmu,
+								 struct perf_event *event)
+{
+	struct stm32_ddr_cnt *cnt;
+	int idx = -1;
+
+	/* Loop on all the counters except TIME_CNT_IDX */
+	for (int i = 0; i < pmu->cfg->evt_counters_nb; i++) {
+		u64 config;
+
+		if (list_empty(&pmu->counters[i])) {
+			idx = i;
+			continue;
+		}
+		config = list_first_entry(&pmu->counters[i], struct stm32_ddr_cnt,
+					  cnt_list)->evt->attr.config;
+		if (config == event->attr.config) {
+			idx = i;
+			break;
+		}
+	}
+
+	if (idx == -1)
+		return ERR_PTR(-ENOENT);
+
+	cnt = kzalloc(sizeof(*cnt), GFP_KERNEL);
+	if (!cnt)
+		return ERR_PTR(-ENOMEM);
+
+	cnt->evt = event;
+	cnt->idx = idx;
+	event->pmu_private = cnt;
+
+	list_add(&cnt->cnt_list, &pmu->counters[idx]);
+
+	return cnt;
+}
+
+static inline struct stm32_ddr_cnt *stm32_get_event_counter(struct stm32_ddr_pmu *pmu,
+							    struct perf_event *event)
+{
+	return pmu->cfg->get_counter(pmu, event);
+}
+
+static int stm32_ddr_pmu_get_counter(struct stm32_ddr_pmu *pmu, struct perf_event *event)
+{
+	u32 time_cnt_idx = pmu->cfg->time_cnt_idx;
+	u32 config = event->attr.config;
+	struct stm32_ddr_cnt *cnt;
+
+	if (!pmu || !event)
+		return -EINVAL;
+
+	pmu->selected_set = GROUP_VALUE(config);
+
+	if (config == TIME_CNT) {
+		cnt = kzalloc(sizeof(*cnt), GFP_KERNEL);
+		if (!cnt)
+			return -ENOMEM;
+
+		cnt->evt = event;
+		cnt->idx = time_cnt_idx;
+		event->pmu_private = cnt;
+		list_add(&cnt->cnt_list, &pmu->counters[time_cnt_idx]);
+
+		return 0;
+	}
+
+	cnt = stm32_get_event_counter(pmu, event);
+	if (IS_ERR(cnt))
+		return PTR_ERR(cnt);
+
+	if (list_count_nodes(&cnt->cnt_list) == 1) {
+		stm32_ddr_stop_counters(pmu);
+		stm32_ddr_sel_evnt(pmu, cnt);
+		stm32_ddr_counter_enable(pmu, cnt);
+		stm32_ddr_start_counters(pmu);
+	}
+
+	return 0;
+}
+
+static void stm32_ddr_pmu_free_counter(struct stm32_ddr_pmu *pmu,
+				       struct stm32_ddr_cnt *counter)
+{
+	size_t count = list_count_nodes(&counter->cnt_list);
+
+	if (counter->evt->attr.config != TIME_CNT && count == 1)
+		stm32_ddr_counter_disable(pmu, counter);
+
+	list_del(&counter->cnt_list);
+	kfree(counter);
+}
+
+static void stm32_ddr_pmu_event_update_list(struct stm32_ddr_pmu *pmu, struct list_head *list)
+{
+	struct stm32_ddr_cnt *counter = list_first_entry(list, struct stm32_ddr_cnt, cnt_list);
+	const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
+	u32 val;
+
+	if (counter->evt->attr.config != TIME_CNT)
+		val = readl_relaxed(pmu->membase + r->counter_evt[counter->idx].reg);
+	else
+		val = readl_relaxed(pmu->membase + r->counter_time.reg);
+
+	stm32_ddr_clear_counter(pmu, counter);
+
+	list_for_each_entry(counter, list, cnt_list)
+		local64_add(val, &counter->evt->count);
+}
+
+static void stm32_ddr_pmu_event_read(struct perf_event *event)
+{
+	struct stm32_ddr_pmu *pmu = to_stm32_ddr_pmu(event->pmu);
+	struct stm32_ddr_cnt *cnt = event->pmu_private;
+
+	hrtimer_start(&pmu->hrtimer, pmu->poll_period, HRTIMER_MODE_REL_PINNED);
+
+	stm32_ddr_stop_counters(pmu);
+
+	stm32_ddr_pmu_event_update_list(pmu, &pmu->counters[cnt->idx]);
+
+	stm32_ddr_start_counters(pmu);
+}
+
+static void stm32_ddr_pmu_event_start(struct perf_event *event, int flags)
+{
+	struct stm32_ddr_pmu *pmu = to_stm32_ddr_pmu(event->pmu);
+	struct stm32_ddr_cnt *counter = event->pmu_private;
+	struct hw_perf_event *hw = &event->hw;
+
+	if (WARN_ON_ONCE(!(hw->state & PERF_HES_STOPPED)))
+		return;
+
+	if (flags & PERF_EF_RELOAD)
+		WARN_ON_ONCE(!(hw->state & PERF_HES_UPTODATE));
+
+	stm32_ddr_stop_counters(pmu);
+
+	if (list_count_nodes(&counter->cnt_list) == 1)
+		stm32_ddr_clear_counter(pmu, counter);
+	else
+		stm32_ddr_pmu_event_update_list(pmu, &pmu->counters[counter->idx]);
+
+	stm32_ddr_start_counters(pmu);
+	local64_set(&hw->prev_count, 0);
+	hw->state = 0;
+}
+
+static void stm32_ddr_pmu_event_stop(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hw = &event->hw;
+
+	if (WARN_ON_ONCE(hw->state & PERF_HES_STOPPED))
+		return;
+
+	hw->state |= PERF_HES_STOPPED;
+
+	if (flags & PERF_EF_UPDATE) {
+		stm32_ddr_pmu_event_read(event);
+		hw->state |= PERF_HES_UPTODATE;
+	}
+}
+
+static int stm32_ddr_pmu_event_add(struct perf_event *event, int flags)
+{
+	struct stm32_ddr_pmu *pmu = to_stm32_ddr_pmu(event->pmu);
+	int ret;
+
+	clk_enable(pmu->clk);
+
+	hrtimer_start(&pmu->hrtimer, pmu->poll_period, HRTIMER_MODE_REL_PINNED);
+
+	ret = stm32_ddr_pmu_get_counter(pmu, event);
+	if (ret)
+		return ret;
+
+	event->hw.state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
+
+	if (flags & PERF_EF_START)
+		stm32_ddr_pmu_event_start(event, flags);
+
+	return 0;
+}
+
+static void stm32_ddr_pmu_event_del(struct perf_event *event, int flags)
+{
+	struct stm32_ddr_pmu *pmu = to_stm32_ddr_pmu(event->pmu);
+	struct stm32_ddr_cnt *counter = event->pmu_private;
+	bool events = true;
+
+	stm32_ddr_pmu_event_stop(event, PERF_EF_UPDATE);
+
+	stm32_ddr_pmu_free_counter(pmu, counter);
+
+	for (int i = 0; i < pmu->cfg->counters_nb; i++)
+		events = !list_empty(&pmu->counters[i]);
+
+	/* If there is activity nothing to do */
+	if (events)
+		return;
+
+	hrtimer_cancel(&pmu->hrtimer);
+	stm32_ddr_stop_counters(pmu);
+
+	pmu->selected_set = -1;
+
+	clk_disable(pmu->clk);
+}
+
+static int stm32_ddr_pmu_event_init(struct perf_event *event)
+{
+	if (event->attr.type != event->pmu->type)
+		return -ENOENT;
+
+	if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
+		return -EINVAL;
+
+	return 0;
+}
+
+static enum hrtimer_restart stm32_ddr_pmu_poll(struct hrtimer *hrtimer)
+{
+	struct stm32_ddr_pmu *pmu = hrtimer_to_stm32_ddr_pmu(hrtimer);
+
+	stm32_ddr_stop_counters(pmu);
+
+	for (int i = 0; i < MP2_CNT_NB; i++)
+		if (!list_empty(&pmu->counters[i]))
+			stm32_ddr_pmu_event_update_list(pmu, &pmu->counters[i]);
+
+	if (list_empty(&pmu->counters[pmu->cfg->time_cnt_idx]))
+		stm32_ddr_clear_time_counter(pmu);
+
+	stm32_ddr_start_counters(pmu);
+
+	hrtimer_forward_now(hrtimer, pmu->poll_period);
+
+	return HRTIMER_RESTART;
+}
+
+static ssize_t stm32_ddr_pmu_sysfs_show(struct device *dev, struct device_attribute *attr,
+					char *buf)
+{
+	struct perf_pmu_events_attr *pmu_attr;
+
+	pmu_attr = container_of(attr, struct perf_pmu_events_attr, attr);
+
+	return sysfs_emit(buf, "event=0x%02llx\n", pmu_attr->id);
+}
+
+#define STM32_DDR_PMU_EVENT_ATTR(_name, _id)			\
+	PMU_EVENT_ATTR_ID(_name, stm32_ddr_pmu_sysfs_show, _id)
+
+static struct attribute *stm32_ddr_pmu_events_attrs_mp[] = {
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_rd, PERF_OP_IS_RD),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_wr, PERF_OP_IS_WR),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_activate, PERF_OP_IS_ACTIVATE),
+	STM32_DDR_PMU_EVENT_ATTR(ctl_idle, CTL_IDLE),
+	STM32_DDR_PMU_EVENT_ATTR(perf_hpr_req_with_no_credit, PERF_HPR_REQ_WITH_NO_CREDIT),
+	STM32_DDR_PMU_EVENT_ATTR(perf_lpr_req_with_no_credit, PERF_LPR_REQ_WITH_NO_CREDIT),
+	STM32_DDR_PMU_EVENT_ATTR(cactive_ddrc, CACTIVE_DDRC),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_enter_powerdown, PERF_OP_IS_ENTER_POWERDOWN),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_refresh, PERF_OP_IS_REFRESH),
+	STM32_DDR_PMU_EVENT_ATTR(perf_selfresh_mode, PERF_SELFRESH_MODE),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_lp_req, DFI_LP_REQ),
+	STM32_DDR_PMU_EVENT_ATTR(perf_hpr_xact_when_critical, PERF_HPR_XACT_WHEN_CRITICAL),
+	STM32_DDR_PMU_EVENT_ATTR(perf_lpr_xact_when_critical, PERF_LPR_XACT_WHEN_CRITICAL),
+	STM32_DDR_PMU_EVENT_ATTR(perf_wr_xact_when_critical, PERF_WR_XACT_WHEN_CRITICAL),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_lp_req_cpy, DFI_LP_REQ),  /* Suffixed '_cpy' to allow the
+								* choice between sets 2 and 3
+								*/
+	STM32_DDR_PMU_EVENT_ATTR(time_cnt, TIME_CNT),
+	NULL,
+};
+
+static struct attribute_group stm32_ddr_pmu_events_attrs_group_mp = {
+	.name = "events",
+	.attrs = stm32_ddr_pmu_events_attrs_mp,
+};
+
+static struct attribute *stm32_ddr_pmu_events_attrs_mp2[] = {
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_act, DFI_IS_ACT),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_prepb, DFI_IS_PREPB),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_preab, DFI_IS_PREAB),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_rd, DFI_IS_RD),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_rda, DFI_IS_RDA),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_wr, DFI_IS_WR),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_wra, DFI_IS_WRA),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_mwr, DFI_IS_MWR),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_mwra, DFI_IS_MWRA),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_mrw, DFI_IS_MRW),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_mrr, DFI_IS_MRR),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_refpb, DFI_IS_REFPB),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_refab, DFI_IS_REFAB),
+	STM32_DDR_PMU_EVENT_ATTR(dfi_is_mpc, DFI_IS_MPC),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_act, PERF_OP_IS_ACT),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_rd, PERF_OP_IS_RD),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_wr, PERF_OP_IS_WR),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_mwr, PERF_OP_IS_MWR),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_ref, PERF_OP_IS_REF),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_crit_ref, PERF_OP_IS_CRIT_REF),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_spec_ref, PERF_OP_IS_SPEC_REF),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_zqcal, PERF_OP_IS_ZQCAL),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_enter_powdn, PERF_OP_IS_ENTER_POWDN),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_enter_selfref, PERF_OP_IS_ENTER_SELFREF),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_pre, PERF_OP_IS_PRE),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_pre_for_rdwr, PERF_OP_IS_PRE_FOR_RDWR),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_pre_for_others, PERF_OP_IS_PRE_FOR_OTHERS),
+	STM32_DDR_PMU_EVENT_ATTR(perf_op_is_rd_activate, PERF_OP_IS_RD_ACTIVATE),
+	STM32_DDR_PMU_EVENT_ATTR(perf_hpr_req_with_nocredit, PERF_HPR_REQ_WITH_NOCREDIT),
+	STM32_DDR_PMU_EVENT_ATTR(perf_lpr_req_with_nocredit, PERF_LPR_REQ_WITH_NOCREDIT),
+	STM32_DDR_PMU_EVENT_ATTR(perf_hpr_xact_when_critical, PERF_HPR_XACT_WHEN_CRITICAL),
+	STM32_DDR_PMU_EVENT_ATTR(perf_lpr_xact_when_critical, PERF_LPR_XACT_WHEN_CRITICAL),
+	STM32_DDR_PMU_EVENT_ATTR(perf_wr_xact_when_critical, PERF_WR_XACT_WHEN_CRITICAL),
+	STM32_DDR_PMU_EVENT_ATTR(perf_rdwr_transitions, PERF_RDWR_TRANSITIONS),
+	STM32_DDR_PMU_EVENT_ATTR(perf_war_hazard, PERF_WAR_HAZARD),
+	STM32_DDR_PMU_EVENT_ATTR(perf_raw_hazard, PERF_RAW_HAZARD),
+	STM32_DDR_PMU_EVENT_ATTR(perf_waw_hazard, PERF_WAW_HAZARD),
+	STM32_DDR_PMU_EVENT_ATTR(perf_rank, PERF_RANK),
+	STM32_DDR_PMU_EVENT_ATTR(perf_read_bypass, PERF_READ_BYPASS),
+	STM32_DDR_PMU_EVENT_ATTR(perf_act_bypass, PERF_ACT_BYPASS),
+	STM32_DDR_PMU_EVENT_ATTR(perf_window_limit_reached_rd, PERF_WINDOW_LIMIT_REACHED_RD),
+	STM32_DDR_PMU_EVENT_ATTR(perf_window_limit_reached_wr, PERF_WINDOW_LIMIT_REACHED_WR),
+	STM32_DDR_PMU_EVENT_ATTR(time_cnt, TIME_CNT),
+	NULL
+};
+
+static struct attribute_group stm32_ddr_pmu_events_attrs_group_mp2 = {
+	.name = "events",
+	.attrs = stm32_ddr_pmu_events_attrs_mp2,
+};
+
+PMU_FORMAT_ATTR(event, "config:0-8");
+
+static struct attribute *stm32_ddr_pmu_format_attrs[] = {
+	&format_attr_event.attr,
+	NULL,
+};
+
+static const struct attribute_group stm32_ddr_pmu_format_attr_group = {
+	.name = "format",
+	.attrs = stm32_ddr_pmu_format_attrs,
+};
+
+static const struct attribute_group *stm32_ddr_pmu_attr_groups_mp1[] = {
+	&stm32_ddr_pmu_events_attrs_group_mp,
+	&stm32_ddr_pmu_format_attr_group,
+	NULL,
+};
+
+static const struct attribute_group *stm32_ddr_pmu_attr_groups_mp2[] = {
+	&stm32_ddr_pmu_events_attrs_group_mp2,
+	&stm32_ddr_pmu_format_attr_group,
+	NULL,
+};
+
+static int stm32_ddr_pmu_device_probe(struct platform_device *pdev)
+{
+	struct stm32_firewall firewall;
+	struct stm32_ddr_pmu *pmu;
+	struct reset_control *rst;
+	struct resource *res;
+	int ret;
+
+	pmu = devm_kzalloc(&pdev->dev, struct_size(pmu, counters, MP2_CNT_NB), GFP_KERNEL);
+	if (!pmu)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, pmu);
+	pmu->dev = &pdev->dev;
+
+	pmu->cfg = device_get_match_data(&pdev->dev);
+
+	pmu->membase = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	if (IS_ERR(pmu->membase))
+		return PTR_ERR(pmu->membase);
+
+	if (of_property_present(pmu->dev->of_node, "access-controllers")) {
+		ret = stm32_firewall_get_firewall(pmu->dev->of_node, &firewall, 1);
+		if (ret) {
+			dev_err(pmu->dev, "Failed to get firewall\n");
+			return ret;
+		}
+		ret = stm32_firewall_grant_access_by_id(&firewall, firewall.firewall_id);
+		if (ret) {
+			dev_err(pmu->dev, "Failed to grant access\n");
+			return ret;
+		}
+	}
+
+	if (of_property_present(pmu->dev->of_node, "clocks")) {
+		pmu->clk = devm_clk_get_prepared(pmu->dev, NULL);
+		if (IS_ERR(pmu->clk)) {
+			dev_err(pmu->dev, "Failed to get clock\n");
+			return PTR_ERR(pmu->clk);
+		}
+	}
+
+	clk_enable(pmu->clk);
+
+	if (of_property_present(pdev->dev.of_node, "resets")) {
+		rst = devm_reset_control_get(&pdev->dev, NULL);
+		if (IS_ERR(rst)) {
+			dev_err(&pdev->dev, "Failed to get reset\n");
+			ret = PTR_ERR(rst);
+			goto err_clk;
+		}
+		reset_control_assert(rst);
+		reset_control_deassert(rst);
+	}
+
+	pmu->poll_period = ms_to_ktime(POLL_MS);
+	hrtimer_setup(&pmu->hrtimer, stm32_ddr_pmu_poll, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+
+	for (int i = 0; i < MP2_CNT_NB; i++)
+		INIT_LIST_HEAD(&pmu->counters[i]);
+
+	pmu->selected_set = -1;
+
+	pmu->pmu = (struct pmu) {
+		.task_ctx_nr = perf_invalid_context,
+		.start = stm32_ddr_pmu_event_start,
+		.stop = stm32_ddr_pmu_event_stop,
+		.add = stm32_ddr_pmu_event_add,
+		.del = stm32_ddr_pmu_event_del,
+		.read = stm32_ddr_pmu_event_read,
+		.event_init = stm32_ddr_pmu_event_init,
+		.attr_groups = pmu->cfg->attribute,
+		.module = THIS_MODULE,
+	};
+
+	ret = perf_pmu_register(&pmu->pmu, DRIVER_NAME, -1);
+	if (ret) {
+		dev_err(&pdev->dev, "Couldn't register DDRPERFM driver as a PMU\n");
+		goto err_clk;
+	}
+
+	if (pmu->cfg->regs->dram_inf.reg) {
+		ret = of_property_read_u32(pdev->dev.of_node, "st,dram-type", &pmu->dram_type);
+		if (ret) {
+			dev_err(&pdev->dev, "Missing device-tree property 'st,dram-type'\n");
+			perf_pmu_unregister(&pmu->pmu);
+
+			return ret;
+		}
+
+		writel_relaxed(pmu->dram_type, pmu->membase + pmu->cfg->regs->dram_inf.reg);
+	}
+
+	clk_disable(pmu->clk);
+
+	return 0;
+
+err_clk:
+	clk_disable_unprepare(pmu->clk);
+
+	return ret;
+}
+
+static void stm32_ddr_pmu_device_remove(struct platform_device *pdev)
+{
+	struct stm32_ddr_pmu *stm32_ddr_pmu = platform_get_drvdata(pdev);
+
+	perf_pmu_unregister(&stm32_ddr_pmu->pmu);
+}
+
+static int __maybe_unused stm32_ddr_pmu_device_resume(struct device *dev)
+{
+	struct stm32_ddr_pmu *pmu = dev_get_drvdata(dev);
+
+	clk_enable(pmu->clk);
+	writel_relaxed(pmu->dram_type, pmu->membase + pmu->cfg->regs->dram_inf.reg);
+	clk_disable(pmu->clk);
+
+	return 0;
+}
+
+static const struct stm32_ddr_pmu_regspec stm32_ddr_pmu_regspec_mp1 = {
+	.stop =		{ DDRPERFM_CTRL, CTRL_STOP },
+	.start =	{ DDRPERFM_CTRL, CTRL_START },
+	.enable =	{ DDRPERFM_CFG },
+	.cfg =		{ DDRPERFM_CFG },
+	.status =	{ DDRPERFM_STATUS, MP1_STATUS_BUSY },
+	.clear_cnt =	{ DDRPERFM_CLR, MP1_CLR_CNT},
+	.clear_time =	{ DDRPERFM_CLR, MP1_CLR_TIME},
+	.counter_time =	{ DDRPERFM_TCNT },
+	.counter_evt =	{
+				{ DDRPERFM_EVCNT(0) },
+				{ DDRPERFM_EVCNT(1) },
+				{ DDRPERFM_EVCNT(2) },
+				{ DDRPERFM_EVCNT(3) },
+	},
+};
+
+static const struct stm32_ddr_pmu_regspec stm32_ddr_pmu_regspec_mp2 = {
+	.stop =		{ DDRPERFM_CTRL, CTRL_STOP },
+	.start =	{ DDRPERFM_CTRL, CTRL_START },
+	.status =	{ DDRPERFM_MP2_STATUS, MP2_STATUS_BUSY },
+	.clear_cnt =	{ DDRPERFM_CLR, MP2_CLR_CNT},
+	.clear_time =	{ DDRPERFM_CLR, MP2_CLR_TIME},
+	.cfg0 =		{ DDRPERFM_MP2_CFG0 },
+	.cfg1 =		{ DDRPERFM_MP2_CFG1 },
+	.enable =	{ DDRPERFM_MP2_CFG5 },
+	.dram_inf =	{ DDRPERFM_MP2_DRAMINF },
+	.counter_time =	{ DDRPERFM_MP2_TCNT },
+	.counter_evt =	{
+				{ DDRPERFM_MP2_EVCNT(0) },
+				{ DDRPERFM_MP2_EVCNT(1) },
+				{ DDRPERFM_MP2_EVCNT(2) },
+				{ DDRPERFM_MP2_EVCNT(3) },
+				{ DDRPERFM_MP2_EVCNT(4) },
+				{ DDRPERFM_MP2_EVCNT(5) },
+				{ DDRPERFM_MP2_EVCNT(6) },
+				{ DDRPERFM_MP2_EVCNT(7) },
+	},
+};
+
+static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp1 = {
+	.regs = &stm32_ddr_pmu_regspec_mp1,
+	.attribute = stm32_ddr_pmu_attr_groups_mp1,
+	.counters_nb = MP1_CNT_NB,
+	.evt_counters_nb = MP1_CNT_NB - 1, /* Time counter is not an event counter */
+	.time_cnt_idx = MP1_TIME_CNT_IDX,
+	.get_counter = stm32_ddr_pmu_get_event_counter_mp1,
+};
+
+static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp2 = {
+	.regs = &stm32_ddr_pmu_regspec_mp2,
+	.attribute = stm32_ddr_pmu_attr_groups_mp2,
+	.counters_nb = MP2_CNT_NB,
+	.evt_counters_nb = MP2_CNT_NB - 1, /* Time counter is an event counter */
+	.time_cnt_idx = MP2_TIME_CNT_IDX,
+	.get_counter = stm32_ddr_pmu_get_event_counter_mp2,
+};
+
+static const struct dev_pm_ops stm32_ddr_pmu_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(NULL, stm32_ddr_pmu_device_resume)
+};
+
+static const struct of_device_id stm32_ddr_pmu_of_match[] = {
+	{
+		.compatible = "st,stm32mp131-ddr-pmu",
+		.data = &stm32_ddr_pmu_cfg_mp1
+	},
+	{
+		.compatible = "st,stm32mp151-ddr-pmu",
+		.data = &stm32_ddr_pmu_cfg_mp1
+	},
+	{
+		.compatible = "st,stm32mp251-ddr-pmu",
+		.data = &stm32_ddr_pmu_cfg_mp2
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, stm32_ddr_pmu_of_match);
+
+static struct platform_driver stm32_ddr_pmu_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.pm = &stm32_ddr_pmu_pm_ops,
+		.of_match_table = of_match_ptr(stm32_ddr_pmu_of_match),
+	},
+	.probe = stm32_ddr_pmu_device_probe,
+	.remove = stm32_ddr_pmu_device_remove,
+};
+
+module_platform_driver(stm32_ddr_pmu_driver);
+
+MODULE_AUTHOR("Clément Le Goffic");
+MODULE_DESCRIPTION("STMicroelectronics STM32 DDR performance monitor driver");
+MODULE_LICENSE("GPL");

-- 
2.43.0

[PATCH 07/13] Documentation: perf: stm32: add ddrperfm support

[Clément Le Goffic]

The DDRPERFM is the DDR Performance Monitor embedded in STM32MPU SoC.

This documentation introduces the DDRPERFM, the stm32-ddr-pmu driver
supporting it and how to use it with the perf tool.

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 Documentation/admin-guide/perf/index.rst         |  1 +
 Documentation/admin-guide/perf/stm32-ddr-pmu.rst | 86 ++++++++++++++++++++++++
 2 files changed, 87 insertions(+)

diff --git a/Documentation/admin-guide/perf/index.rst b/Documentation/admin-guide/perf/index.rst
index 072b510385c4..33aedc4ee5c3 100644
--- a/Documentation/admin-guide/perf/index.rst
+++ b/Documentation/admin-guide/perf/index.rst
@@ -29,3 +29,4 @@ Performance monitor support
    cxl
    ampere_cspmu
    mrvl-pem-pmu
+   stm32-ddr-pmu
diff --git a/Documentation/admin-guide/perf/stm32-ddr-pmu.rst b/Documentation/admin-guide/perf/stm32-ddr-pmu.rst
new file mode 100644
index 000000000000..5b02bf44dd7a
--- /dev/null
+++ b/Documentation/admin-guide/perf/stm32-ddr-pmu.rst
@@ -0,0 +1,86 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+========================================
+STM32 DDR Performance Monitor (DDRPERFM)
+========================================
+
+The DDRPERFM is the DDR Performance Monitor embedded in STM32MPU SoC.
+The DDR controller provides events to DDRPERFM, once selected they are counted in the DDRPERFM
+peripheral.
+
+In MP1 family, the DDRPERFM is able to count 4 different events at the same time.
+However, the 4 events must belong to the same set.
+One hardware counter is dedicated to the time counter, `time_cnt`.
+
+In MP2 family, the DDRPERFM is able to select between 44 different DDR events.
+As for MP1, there is a dedicated hardware counter for the time.
+It is incremented every 4 DDR clock cycles.
+All the other counters can be freely allocated to count any other DDR event.
+
+The stm32-ddr-pmu driver relies on the perf PMU framework to expose the counters via sysfs:
+
+On MP1:
+
+    .. code-block:: bash
+
+        $ ls /sys/bus/event_source/devices/stm32_ddr_pmu/events/
+        cactive_ddrc                 perf_lpr_req_with_no_credit  perf_op_is_wr
+        ctl_idle                     perf_lpr_xact_when_critical  perf_selfresh_mode
+        dfi_lp_req                   perf_op_is_activate          perf_wr_xact_when_critical
+        dfi_lp_req_cpy               perf_op_is_enter_powerdown   time_cnt
+        perf_hpr_req_with_no_credit  perf_op_is_rd
+        perf_hpr_xact_when_critical  perf_op_is_refresh
+
+On MP2:
+
+    .. code-block:: bash
+
+        $ ls /sys/bus/event_source/devices/stm32_ddr_pmu/events/
+        dfi_is_act       perf_hpr_req_with_nocredit   perf_op_is_spec_ref
+        dfi_is_mpc       perf_hpr_xact_when_critical  perf_op_is_wr
+        dfi_is_mrr       perf_lpr_req_with_nocredit   perf_op_is_zqcal
+        dfi_is_mrw       perf_lpr_xact_when_critical  perf_rank
+        dfi_is_mwr       perf_op_is_act               perf_raw_hazard
+        dfi_is_mwra      perf_op_is_crit_ref          perf_rdwr_transitions
+        dfi_is_preab     perf_op_is_enter_powdn       perf_read_bypass
+        dfi_is_prepb     perf_op_is_enter_selfref     perf_war_hazard
+        dfi_is_rd        perf_op_is_mwr               perf_waw_hazard
+        dfi_is_rda       perf_op_is_pre               perf_window_limit_reached_rd
+        dfi_is_refab     perf_op_is_pre_for_others    perf_window_limit_reached_wr
+        dfi_is_refpb     perf_op_is_pre_for_rdwr      perf_wr_xact_when_critical
+        dfi_is_wr        perf_op_is_rd                time_cnt
+        dfi_is_wra       perf_op_is_rd_activate
+        perf_act_bypass  perf_op_is_ref
+
+
+The perf PMU framework is usually invoked via the 'perf stat' tool.
+
+
+Example:
+
+    .. code-block:: bash
+
+        $ perf stat --timeout 60000 -e stm32_ddr_pmu/dfi_is_act/,\
+        > stm32_ddr_pmu/dfi_is_rd/,\
+        > stm32_ddr_pmu/dfi_is_wr/,\
+        > stm32_ddr_pmu/dfi_is_refab/,\
+        > stm32_ddr_pmu/dfi_is_mrw/,\
+        > stm32_ddr_pmu/dfi_is_rda/,\
+        > stm32_ddr_pmu/dfi_is_wra/,\
+        > stm32_ddr_pmu/dfi_is_mrr/,\
+        > stm32_ddr_pmu/time_cnt/ \
+        > -a sleep 5
+
+         Performance counter stats for 'system wide':
+
+                    481025      stm32_ddr_pmu/dfi_is_act/
+                    732166      stm32_ddr_pmu/dfi_is_rd/
+                    144926      stm32_ddr_pmu/dfi_is_wr/
+                    644154      stm32_ddr_pmu/dfi_is_refab/
+                         0      stm32_ddr_pmu/dfi_is_mrw/
+                         0      stm32_ddr_pmu/dfi_is_rda/
+                         0      stm32_ddr_pmu/dfi_is_wra/
+                         0      stm32_ddr_pmu/dfi_is_mrr/
+                 752347686      stm32_ddr_pmu/time_cnt/
+
+               5.014910750 seconds time elapsed

-- 
2.43.0

[PATCH 08/13] MAINTAINERS: add myself as STM32 DDR PMU maintainer

[Clément Le Goffic]

Add Clément Le Goffic as STM32 DDR PMU 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 c3f7fbd0d67a..4211133e731e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23451,6 +23451,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/power/supply/st,stc3117.yaml
 F:	drivers/power/supply/stc3117_fuel_gauge.c
 
+ST STM32 DDR PMU
+M:	Clément Le Goffic <clement.legoffic@foss.st.com>
+S:	Maintained
+F:	Documentation/admin-guide/perf/stm32-ddr-pmu.rst
+F:	Documentation/devicetree/bindings/perf/st,stm32-ddr-pmu.yaml
+F:	drivers/perf/stm32_ddr-pmu.c
+
 ST STM32 FIREWALL
 M:	Gatien Chevallier <gatien.chevallier@foss.st.com>
 S:	Maintained

-- 
2.43.0

[PATCH 09/13] ARM: dts: stm32: add ddrperfm on stm32mp131

[Clément Le Goffic]

The DDRPERFM is the DDR Performance Monitor embedded in STM32MP131 SoC.

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 492bcf586361..e097723789aa 100644
--- a/arch/arm/boot/dts/st/stm32mp131.dtsi
+++ b/arch/arm/boot/dts/st/stm32mp131.dtsi
@@ -998,6 +998,13 @@ iwdg2: watchdog@5a002000 {
 			status = "disabled";
 		};
 
+		ddrperfm: perf@5a007000 {
+			compatible = "st,stm32mp131-ddr-pmu";
+			reg = <0x5a007000 0x400>;
+			clocks = <&rcc DDRPERFM>;
+			resets = <&rcc DDRPERFM_R>;
+		};
+
 		rtc: rtc@5c004000 {
 			compatible = "st,stm32mp1-rtc";
 			reg = <0x5c004000 0x400>;

-- 
2.43.0

[PATCH 10/13] ARM: dts: stm32: add ddrperfm on stm32mp151

[Clément Le Goffic]

The DDRPERFM is the DDR Performance Monitor embedded in STM32MP151 SoC.

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 0daa8ffe2ff5..82b1d585a099 100644
--- a/arch/arm/boot/dts/st/stm32mp151.dtsi
+++ b/arch/arm/boot/dts/st/stm32mp151.dtsi
@@ -383,6 +383,13 @@ usbphyc_port1: usb-phy@1 {
 			};
 		};
 
+		ddrperfm: perf@5a007000 {
+			compatible = "st,stm32mp151-ddr-pmu";
+			reg = <0x5a007000 0x400>;
+			clocks = <&rcc DDRPERFM>;
+			resets = <&rcc DDRPERFM_R>;
+		};
+
 		rtc: rtc@5c004000 {
 			compatible = "st,stm32mp1-rtc";
 			reg = <0x5c004000 0x400>;

-- 
2.43.0

[PATCH 11/13] arm64: dts: st: add ddrperfm on stm32mp251

[Clément Le Goffic]

The DDRPERFM is the DDR Performance Monitor embedded in STM32MP251 SoC.

Keep the node disabled at SoC level as it requires the property
`st,dram-type` which is provided in board dtsi file.

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 0683c2d5cb6f..7f138324610a 100644
--- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
@@ -1577,5 +1577,12 @@ exti2: interrupt-controller@46230000 {
 				<0>,
 				<&intc GIC_SPI 213 IRQ_TYPE_LEVEL_HIGH>;	/* EXTI_70 */
 		};
+
+		ddrperfm: perf@48041000 {
+			compatible = "st,stm32mp251-ddr-pmu";
+			reg = <0x48041000 0x400>;
+			access-controllers = <&rcc 104>;
+			status = "disabled";
+		};
 	};
 };

-- 
2.43.0

[PATCH 12/13] arm64: dts: st: support ddrperfm on stm32mp257f-dk

[Clément Le Goffic]

Configure DDRPERFM node on stm32mp257f-dk board.
Disable the node as DDRPERFM will produce an error message if it's clock
(shared with the DDRCTRL on STM32MP25x) is secured by common bootloaders.

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

diff --git a/arch/arm64/boot/dts/st/stm32mp257f-dk.dts b/arch/arm64/boot/dts/st/stm32mp257f-dk.dts
index a278a1e3ce03..e59ef18de7c3 100644
--- a/arch/arm64/boot/dts/st/stm32mp257f-dk.dts
+++ b/arch/arm64/boot/dts/st/stm32mp257f-dk.dts
@@ -77,6 +77,11 @@ &arm_wdt {
 	status = "okay";
 };
 
+&ddrperfm {
+	st,dram-type = <0>;
+	status = "disabled";
+};
+
 &scmi_regu {
 	scmi_vddio1: regulator@0 {
 		regulator-min-microvolt = <1800000>;

-- 
2.43.0

[PATCH 13/13] arm64: dts: st: support ddrperfm on stm32mp257f-ev1

[Clément Le Goffic]

Configure DDRPERFM node on stm32mp257f-ev1 board.
Disable the node as DDRPERFM will produce an error message if it's clock
(shared with the DDRCTRL on STM32MP25x) is secured by common bootloaders.

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

diff --git a/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts b/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts
index 2f561ad40665..a48fb7b33198 100644
--- a/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts
+++ b/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts
@@ -123,6 +123,11 @@ csi_source: endpoint {
 	};
 };
 
+&ddrperfm {
+	st,dram-type = <2>;
+	status = "disabled";
+};
+
 &dcmipp {
 	status = "okay";
 	port {

-- 
2.43.0

Re: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver

[Krzysztof Kozlowski]

On 23/06/2025 11:27, Clément Le Goffic wrote:
> +	if (of_property_present(pmu->dev->of_node, "access-controllers")) {
> +		ret = stm32_firewall_get_firewall(pmu->dev->of_node, &firewall, 1);
> +		if (ret) {
> +			dev_err(pmu->dev, "Failed to get firewall\n");
> +			return ret;
> +		}
> +		ret = stm32_firewall_grant_access_by_id(&firewall, firewall.firewall_id);
> +		if (ret) {
> +			dev_err(pmu->dev, "Failed to grant access\n");
> +			return ret;
> +		}
> +	}
> +
> +	if (of_property_present(pmu->dev->of_node, "clocks")) {

No, don't open-code get clk optional.

> +		pmu->clk = devm_clk_get_prepared(pmu->dev, NULL);
> +		if (IS_ERR(pmu->clk)) {
> +			dev_err(pmu->dev, "Failed to get clock\n");
> +			return PTR_ERR(pmu->clk);
> +		}
> +	}
> +
> +	clk_enable(pmu->clk);
> +
> +	if (of_property_present(pdev->dev.of_node, "resets")) {
> +		rst = devm_reset_control_get(&pdev->dev, NULL);
> +		if (IS_ERR(rst)) {
> +			dev_err(&pdev->dev, "Failed to get reset\n");
> +			ret = PTR_ERR(rst);
> +			goto err_clk;
> +		}
> +		reset_control_assert(rst);
> +		reset_control_deassert(rst);
> +	}
> +
> +	pmu->poll_period = ms_to_ktime(POLL_MS);
> +	hrtimer_setup(&pmu->hrtimer, stm32_ddr_pmu_poll, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +
> +	for (int i = 0; i < MP2_CNT_NB; i++)
> +		INIT_LIST_HEAD(&pmu->counters[i]);
> +
> +	pmu->selected_set = -1;
> +
> +	pmu->pmu = (struct pmu) {
> +		.task_ctx_nr = perf_invalid_context,
> +		.start = stm32_ddr_pmu_event_start,
> +		.stop = stm32_ddr_pmu_event_stop,
> +		.add = stm32_ddr_pmu_event_add,
> +		.del = stm32_ddr_pmu_event_del,
> +		.read = stm32_ddr_pmu_event_read,
> +		.event_init = stm32_ddr_pmu_event_init,
> +		.attr_groups = pmu->cfg->attribute,
> +		.module = THIS_MODULE,
> +	};
> +
> +	ret = perf_pmu_register(&pmu->pmu, DRIVER_NAME, -1);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Couldn't register DDRPERFM driver as a PMU\n");
> +		goto err_clk;
> +	}
> +
> +	if (pmu->cfg->regs->dram_inf.reg) {
> +		ret = of_property_read_u32(pdev->dev.of_node, "st,dram-type", &pmu->dram_type);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Missing device-tree property 'st,dram-type'\n");
> +			perf_pmu_unregister(&pmu->pmu);
> +
> +			return ret;
> +		}
> +
> +		writel_relaxed(pmu->dram_type, pmu->membase + pmu->cfg->regs->dram_inf.reg);
> +	}
> +
> +	clk_disable(pmu->clk);

Why do you keep clock prepared? This device does not know what sort of
clock it gets, so you end up with clock always active for example and
this being no-op.

> +
> +	return 0;
> +
> +err_clk:
> +	clk_disable_unprepare(pmu->clk);
> +
> +	return ret;
> +}
> +
> +static void stm32_ddr_pmu_device_remove(struct platform_device *pdev)
> +{
> +	struct stm32_ddr_pmu *stm32_ddr_pmu = platform_get_drvdata(pdev);
> +
> +	perf_pmu_unregister(&stm32_ddr_pmu->pmu);
> +}
> +
> +static int __maybe_unused stm32_ddr_pmu_device_resume(struct device *dev)
> +{
> +	struct stm32_ddr_pmu *pmu = dev_get_drvdata(dev);
> +
> +	clk_enable(pmu->clk);
> +	writel_relaxed(pmu->dram_type, pmu->membase + pmu->cfg->regs->dram_inf.reg);
> +	clk_disable(pmu->clk);
> +
> +	return 0;
> +}
> +
> +static const struct stm32_ddr_pmu_regspec stm32_ddr_pmu_regspec_mp1 = {
> +	.stop =		{ DDRPERFM_CTRL, CTRL_STOP },
> +	.start =	{ DDRPERFM_CTRL, CTRL_START },
> +	.enable =	{ DDRPERFM_CFG },
> +	.cfg =		{ DDRPERFM_CFG },
> +	.status =	{ DDRPERFM_STATUS, MP1_STATUS_BUSY },
> +	.clear_cnt =	{ DDRPERFM_CLR, MP1_CLR_CNT},
> +	.clear_time =	{ DDRPERFM_CLR, MP1_CLR_TIME},
> +	.counter_time =	{ DDRPERFM_TCNT },
> +	.counter_evt =	{
> +				{ DDRPERFM_EVCNT(0) },
> +				{ DDRPERFM_EVCNT(1) },
> +				{ DDRPERFM_EVCNT(2) },
> +				{ DDRPERFM_EVCNT(3) },
> +	},
> +};
> +
> +static const struct stm32_ddr_pmu_regspec stm32_ddr_pmu_regspec_mp2 = {
> +	.stop =		{ DDRPERFM_CTRL, CTRL_STOP },
> +	.start =	{ DDRPERFM_CTRL, CTRL_START },
> +	.status =	{ DDRPERFM_MP2_STATUS, MP2_STATUS_BUSY },
> +	.clear_cnt =	{ DDRPERFM_CLR, MP2_CLR_CNT},
> +	.clear_time =	{ DDRPERFM_CLR, MP2_CLR_TIME},
> +	.cfg0 =		{ DDRPERFM_MP2_CFG0 },
> +	.cfg1 =		{ DDRPERFM_MP2_CFG1 },
> +	.enable =	{ DDRPERFM_MP2_CFG5 },
> +	.dram_inf =	{ DDRPERFM_MP2_DRAMINF },
> +	.counter_time =	{ DDRPERFM_MP2_TCNT },
> +	.counter_evt =	{
> +				{ DDRPERFM_MP2_EVCNT(0) },
> +				{ DDRPERFM_MP2_EVCNT(1) },
> +				{ DDRPERFM_MP2_EVCNT(2) },
> +				{ DDRPERFM_MP2_EVCNT(3) },
> +				{ DDRPERFM_MP2_EVCNT(4) },
> +				{ DDRPERFM_MP2_EVCNT(5) },
> +				{ DDRPERFM_MP2_EVCNT(6) },
> +				{ DDRPERFM_MP2_EVCNT(7) },
> +	},
> +};
> +
> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp1 = {
> +	.regs = &stm32_ddr_pmu_regspec_mp1,
> +	.attribute = stm32_ddr_pmu_attr_groups_mp1,
> +	.counters_nb = MP1_CNT_NB,
> +	.evt_counters_nb = MP1_CNT_NB - 1, /* Time counter is not an event counter */
> +	.time_cnt_idx = MP1_TIME_CNT_IDX,
> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp1,
> +};
> +
> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp2 = {
> +	.regs = &stm32_ddr_pmu_regspec_mp2,
> +	.attribute = stm32_ddr_pmu_attr_groups_mp2,
> +	.counters_nb = MP2_CNT_NB,
> +	.evt_counters_nb = MP2_CNT_NB - 1, /* Time counter is an event counter */
> +	.time_cnt_idx = MP2_TIME_CNT_IDX,
> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp2,
> +};
> +
> +static const struct dev_pm_ops stm32_ddr_pmu_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(NULL, stm32_ddr_pmu_device_resume)
> +};
> +
> +static const struct of_device_id stm32_ddr_pmu_of_match[] = {
> +	{
> +		.compatible = "st,stm32mp131-ddr-pmu",
> +		.data = &stm32_ddr_pmu_cfg_mp1
> +	},
> +	{
> +		.compatible = "st,stm32mp151-ddr-pmu",
> +		.data = &stm32_ddr_pmu_cfg_mp1

So devices are compatible, thus express it correctly and drop this.

> +	},
> +	{
> +		.compatible = "st,stm32mp251-ddr-pmu",
> +		.data = &stm32_ddr_pmu_cfg_mp2
> +	},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, stm32_ddr_pmu_of_match);
> +
> +static struct platform_driver stm32_ddr_pmu_driver = {
> +	.driver = {
> +		.name = DRIVER_NAME,
> +		.pm = &stm32_ddr_pmu_pm_ops,
> +		.of_match_table = of_match_ptr(stm32_ddr_pmu_of_match),

Drop of_match_ptr, you have here warnings.



Best regards,
Krzysztof

Re: [PATCH 05/13] dt-bindings: perf: stm32: introduce DDRPERFM dt-bindings

[Krzysztof Kozlowski]

On 23/06/2025 11:27, Clément Le Goffic wrote:
> DDRPERFM is the DDR Performance Monitor embedded in STM32MPU SoC.
> It allows to monitor DDR events that come from the DDR Controller
> such as read or write events.
> 

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
> ---
>  .../devicetree/bindings/perf/st,stm32-ddr-pmu.yaml | 93 ++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/perf/st,stm32-ddr-pmu.yaml b/Documentation/devicetree/bindings/perf/st,stm32-ddr-pmu.yaml
> new file mode 100644
> index 000000000000..35d34782865b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/perf/st,stm32-ddr-pmu.yaml
> @@ -0,0 +1,93 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/perf/st,stm32-ddr-pmu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +maintainers:
> +  - Clément Le Goffic <clement.legoffic@foss.st.com>
> +
> +title: STMicroelectronics STM32 DDR Performance Monitor (DDRPERFM)
> +
> +properties:
> +  compatible:
> +    enum:
> +      - st,stm32mp131-ddr-pmu
> +      - st,stm32mp151-ddr-pmu

These are compatible, aren't they?

> +      - st,stm32mp251-ddr-pmu
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    description: Reference clock for the DDR Performance Monitor

Drop description, obvious.

> +    maxItems: 1
> +
> +  resets:
> +    description: Reset control for the DDR Performance Monitor

Drop description, obvious.


> +    maxItems: 1
> +
> +  access-controllers:
> +    minItems: 1
> +    maxItems: 2
> +
> +  st,dram-type:
> +    description: |
> +      This property is used to specify the type of DRAM memory connected to the
> +      associated memory controller. It is required for the DDR Performance Monitor
> +      to correctly interpret the performance data.
> +      0 = LPDDR4,
> +      1 = LPDDR3,
> +      2 = DDR4,
> +      3 = DDR3
> +    $ref: /schemas/types.yaml#/definitions/uint32

No, use standard JEDEC memory bindings (memory controllers) if you need
to describe the memory, otherwise you duplicate that binding and
duplicate the memory information.

> +    enum: [0, 1, 2, 3]
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - st,stm32mp131-ddr-pmu
> +              - st,stm32mp151-ddr-pmu
> +    then:
> +      required:
> +        - clocks
> +        - resets
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: st,stm32mp251-ddr-pmu
> +    then:
> +      required:
> +        - access-controllers
> +        - st,dram-type
> +
> +additionalProperties: false
Best regards,
Krzysztof

Re: [PATCH 02/13] dt-bindings: stm32: stm32mp25: add `access-controller-cell` property

[Rob Herring (Arm)]

On Mon, 23 Jun 2025 11:27:07 +0200, Clément Le Goffic wrote:
> RCC is able to check the availability of a clock.
> Allow to query the RCC with a firewall ID.
> 
> Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
> ---
>  Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/st,stm32mp25-rcc.example.dtb: clock-controller@44200000 (st,stm32mp25-rcc): '#access-controller-cells' is a required property
	from schema $id: http://devicetree.org/schemas/clock/st,stm32mp25-rcc.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250623-ddrperfm-upstream-v1-2-7dffff168090@foss.st.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 05/13] dt-bindings: perf: stm32: introduce DDRPERFM dt-bindings

[Clement LE GOFFIC]

On 6/23/25 11:48, Krzysztof Kozlowski wrote:
> On 23/06/2025 11:27, Clément Le Goffic wrote:
>> DDRPERFM is the DDR Performance Monitor embedded in STM32MPU SoC.
>> It allows to monitor DDR events that come from the DDR Controller
>> such as read or write events.
>>
> 
> A nit, subject: drop second/last, redundant "bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
Oops, you're right thank you

>> Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
>> ---
>>   .../devicetree/bindings/perf/st,stm32-ddr-pmu.yaml | 93 ++++++++++++++++++++++
>>   1 file changed, 93 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/perf/st,stm32-ddr-pmu.yaml b/Documentation/devicetree/bindings/perf/st,stm32-ddr-pmu.yaml
>> new file mode 100644
>> index 000000000000..35d34782865b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/perf/st,stm32-ddr-pmu.yaml
>> @@ -0,0 +1,93 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/perf/st,stm32-ddr-pmu.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +maintainers:
>> +  - Clément Le Goffic <clement.legoffic@foss.st.com>
>> +
>> +title: STMicroelectronics STM32 DDR Performance Monitor (DDRPERFM)
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - st,stm32mp131-ddr-pmu
>> +      - st,stm32mp151-ddr-pmu
> 
> These are compatible, aren't they?

Yes they are

>> +      - st,stm32mp251-ddr-pmu
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    description: Reference clock for the DDR Performance Monitor
> 
> Drop description, obvious.

Ok

> 
>> +    maxItems: 1
>> +
>> +  resets:
>> +    description: Reset control for the DDR Performance Monitor
> 
> Drop description, obvious.

Ok

> 
> 
>> +    maxItems: 1
>> +
>> +  access-controllers:
>> +    minItems: 1
>> +    maxItems: 2
>> +
>> +  st,dram-type:
>> +    description: |
>> +      This property is used to specify the type of DRAM memory connected to the
>> +      associated memory controller. It is required for the DDR Performance Monitor
>> +      to correctly interpret the performance data.
>> +      0 = LPDDR4,
>> +      1 = LPDDR3,
>> +      2 = DDR4,
>> +      3 = DDR3
>> +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> No, use standard JEDEC memory bindings (memory controllers) if you need
> to describe the memory, otherwise you duplicate that binding and
> duplicate the memory information.

Ok didn't know about it, I'll take a look

Best regard,
Clément

>> +    enum: [0, 1, 2, 3]
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - st,stm32mp131-ddr-pmu
>> +              - st,stm32mp151-ddr-pmu
>> +    then:
>> +      required:
>> +        - clocks
>> +        - resets
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: st,stm32mp251-ddr-pmu
>> +    then:
>> +      required:
>> +        - access-controllers
>> +        - st,dram-type
>> +
>> +additionalProperties: false
> Best regards,
> Krzysztof

Re: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver

[Clement LE GOFFIC]

On 6/23/25 11:45, Krzysztof Kozlowski wrote:
> On 23/06/2025 11:27, Clément Le Goffic wrote:
>> +	if (of_property_present(pmu->dev->of_node, "access-controllers")) {
>> +		ret = stm32_firewall_get_firewall(pmu->dev->of_node, &firewall, 1);
>> +		if (ret) {
>> +			dev_err(pmu->dev, "Failed to get firewall\n");
>> +			return ret;
>> +		}
>> +		ret = stm32_firewall_grant_access_by_id(&firewall, firewall.firewall_id);
>> +		if (ret) {
>> +			dev_err(pmu->dev, "Failed to grant access\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	if (of_property_present(pmu->dev->of_node, "clocks")) {
> 
> No, don't open-code get clk optional.

Ok

> 
>> +		pmu->clk = devm_clk_get_prepared(pmu->dev, NULL);
>> +		if (IS_ERR(pmu->clk)) {
>> +			dev_err(pmu->dev, "Failed to get clock\n");
>> +			return PTR_ERR(pmu->clk);
>> +		}
>> +	}
>> +
>> +	clk_enable(pmu->clk);
>> +
>> +	if (of_property_present(pdev->dev.of_node, "resets")) {
>> +		rst = devm_reset_control_get(&pdev->dev, NULL);
>> +		if (IS_ERR(rst)) {
>> +			dev_err(&pdev->dev, "Failed to get reset\n");
>> +			ret = PTR_ERR(rst);
>> +			goto err_clk;
>> +		}
>> +		reset_control_assert(rst);
>> +		reset_control_deassert(rst);
>> +	}
>> +
>> +	pmu->poll_period = ms_to_ktime(POLL_MS);
>> +	hrtimer_setup(&pmu->hrtimer, stm32_ddr_pmu_poll, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>> +
>> +	for (int i = 0; i < MP2_CNT_NB; i++)
>> +		INIT_LIST_HEAD(&pmu->counters[i]);
>> +
>> +	pmu->selected_set = -1;
>> +
>> +	pmu->pmu = (struct pmu) {
>> +		.task_ctx_nr = perf_invalid_context,
>> +		.start = stm32_ddr_pmu_event_start,
>> +		.stop = stm32_ddr_pmu_event_stop,
>> +		.add = stm32_ddr_pmu_event_add,
>> +		.del = stm32_ddr_pmu_event_del,
>> +		.read = stm32_ddr_pmu_event_read,
>> +		.event_init = stm32_ddr_pmu_event_init,
>> +		.attr_groups = pmu->cfg->attribute,
>> +		.module = THIS_MODULE,
>> +	};
>> +
>> +	ret = perf_pmu_register(&pmu->pmu, DRIVER_NAME, -1);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Couldn't register DDRPERFM driver as a PMU\n");
>> +		goto err_clk;
>> +	}
>> +
>> +	if (pmu->cfg->regs->dram_inf.reg) {
>> +		ret = of_property_read_u32(pdev->dev.of_node, "st,dram-type", &pmu->dram_type);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "Missing device-tree property 'st,dram-type'\n");
>> +			perf_pmu_unregister(&pmu->pmu);
>> +
>> +			return ret;
>> +		}
>> +
>> +		writel_relaxed(pmu->dram_type, pmu->membase + pmu->cfg->regs->dram_inf.reg);
>> +	}
>> +
>> +	clk_disable(pmu->clk);
> 
> Why do you keep clock prepared? This device does not know what sort of
> clock it gets, so you end up with clock always active for example and
> this being no-op.

Ok will disable_unprepare

>> +
>> +	return 0;
>> +
>> +err_clk:
>> +	clk_disable_unprepare(pmu->clk);
>> +
>> +	return ret;
>> +}
>> +
>> +static void stm32_ddr_pmu_device_remove(struct platform_device *pdev)
>> +{
>> +	struct stm32_ddr_pmu *stm32_ddr_pmu = platform_get_drvdata(pdev);
>> +
>> +	perf_pmu_unregister(&stm32_ddr_pmu->pmu);
>> +}
>> +
>> +static int __maybe_unused stm32_ddr_pmu_device_resume(struct device *dev)
>> +{
>> +	struct stm32_ddr_pmu *pmu = dev_get_drvdata(dev);
>> +
>> +	clk_enable(pmu->clk);
>> +	writel_relaxed(pmu->dram_type, pmu->membase + pmu->cfg->regs->dram_inf.reg);
>> +	clk_disable(pmu->clk);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct stm32_ddr_pmu_regspec stm32_ddr_pmu_regspec_mp1 = {
>> +	.stop =		{ DDRPERFM_CTRL, CTRL_STOP },
>> +	.start =	{ DDRPERFM_CTRL, CTRL_START },
>> +	.enable =	{ DDRPERFM_CFG },
>> +	.cfg =		{ DDRPERFM_CFG },
>> +	.status =	{ DDRPERFM_STATUS, MP1_STATUS_BUSY },
>> +	.clear_cnt =	{ DDRPERFM_CLR, MP1_CLR_CNT},
>> +	.clear_time =	{ DDRPERFM_CLR, MP1_CLR_TIME},
>> +	.counter_time =	{ DDRPERFM_TCNT },
>> +	.counter_evt =	{
>> +				{ DDRPERFM_EVCNT(0) },
>> +				{ DDRPERFM_EVCNT(1) },
>> +				{ DDRPERFM_EVCNT(2) },
>> +				{ DDRPERFM_EVCNT(3) },
>> +	},
>> +};
>> +
>> +static const struct stm32_ddr_pmu_regspec stm32_ddr_pmu_regspec_mp2 = {
>> +	.stop =		{ DDRPERFM_CTRL, CTRL_STOP },
>> +	.start =	{ DDRPERFM_CTRL, CTRL_START },
>> +	.status =	{ DDRPERFM_MP2_STATUS, MP2_STATUS_BUSY },
>> +	.clear_cnt =	{ DDRPERFM_CLR, MP2_CLR_CNT},
>> +	.clear_time =	{ DDRPERFM_CLR, MP2_CLR_TIME},
>> +	.cfg0 =		{ DDRPERFM_MP2_CFG0 },
>> +	.cfg1 =		{ DDRPERFM_MP2_CFG1 },
>> +	.enable =	{ DDRPERFM_MP2_CFG5 },
>> +	.dram_inf =	{ DDRPERFM_MP2_DRAMINF },
>> +	.counter_time =	{ DDRPERFM_MP2_TCNT },
>> +	.counter_evt =	{
>> +				{ DDRPERFM_MP2_EVCNT(0) },
>> +				{ DDRPERFM_MP2_EVCNT(1) },
>> +				{ DDRPERFM_MP2_EVCNT(2) },
>> +				{ DDRPERFM_MP2_EVCNT(3) },
>> +				{ DDRPERFM_MP2_EVCNT(4) },
>> +				{ DDRPERFM_MP2_EVCNT(5) },
>> +				{ DDRPERFM_MP2_EVCNT(6) },
>> +				{ DDRPERFM_MP2_EVCNT(7) },
>> +	},
>> +};
>> +
>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp1 = {
>> +	.regs = &stm32_ddr_pmu_regspec_mp1,
>> +	.attribute = stm32_ddr_pmu_attr_groups_mp1,
>> +	.counters_nb = MP1_CNT_NB,
>> +	.evt_counters_nb = MP1_CNT_NB - 1, /* Time counter is not an event counter */
>> +	.time_cnt_idx = MP1_TIME_CNT_IDX,
>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp1,
>> +};
>> +
>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp2 = {
>> +	.regs = &stm32_ddr_pmu_regspec_mp2,
>> +	.attribute = stm32_ddr_pmu_attr_groups_mp2,
>> +	.counters_nb = MP2_CNT_NB,
>> +	.evt_counters_nb = MP2_CNT_NB - 1, /* Time counter is an event counter */
>> +	.time_cnt_idx = MP2_TIME_CNT_IDX,
>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp2,
>> +};
>> +
>> +static const struct dev_pm_ops stm32_ddr_pmu_pm_ops = {
>> +	SET_SYSTEM_SLEEP_PM_OPS(NULL, stm32_ddr_pmu_device_resume)
>> +};
>> +
>> +static const struct of_device_id stm32_ddr_pmu_of_match[] = {
>> +	{
>> +		.compatible = "st,stm32mp131-ddr-pmu",
>> +		.data = &stm32_ddr_pmu_cfg_mp1
>> +	},
>> +	{
>> +		.compatible = "st,stm32mp151-ddr-pmu",
>> +		.data = &stm32_ddr_pmu_cfg_mp1
> 
> So devices are compatible, thus express it correctly and drop this.
> 
>> +	},
>> +	{
>> +		.compatible = "st,stm32mp251-ddr-pmu",
>> +		.data = &stm32_ddr_pmu_cfg_mp2
>> +	},
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32_ddr_pmu_of_match);
>> +
>> +static struct platform_driver stm32_ddr_pmu_driver = {
>> +	.driver = {
>> +		.name = DRIVER_NAME,
>> +		.pm = &stm32_ddr_pmu_pm_ops,
>> +		.of_match_table = of_match_ptr(stm32_ddr_pmu_of_match),
> 
> Drop of_match_ptr, you have here warnings.
> 
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver

[kernel test robot]

Hi Clément,

kernel test robot noticed the following build errors:

[auto build test ERROR on 86731a2a651e58953fc949573895f2fa6d456841]

url:    https://github.com/intel-lab-lkp/linux/commits/Cl-ment-Le-Goffic/bus-firewall-move-stm32_firewall-header-file-in-include-folder/20250623-173554
base:   86731a2a651e58953fc949573895f2fa6d456841
patch link:    https://lore.kernel.org/r/20250623-ddrperfm-upstream-v1-6-7dffff168090%40foss.st.com
patch subject: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver
config: i386-buildonly-randconfig-002-20250624 (https://download.01.org/0day-ci/archive/20250624/202506240401.zlRG1qiO-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250624/202506240401.zlRG1qiO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506240401.zlRG1qiO-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/perf/stm32_ddr_pmu.c: In function 'stm32_ddr_start_counters':
>> drivers/perf/stm32_ddr_pmu.c:205:9: error: implicit declaration of function 'writel_relaxed' [-Werror=implicit-function-declaration]
     205 |         writel_relaxed(r->start.mask, pmu->membase + r->start.reg);
         |         ^~~~~~~~~~~~~~
   drivers/perf/stm32_ddr_pmu.c: In function 'stm32_ddr_clear_counter':
>> drivers/perf/stm32_ddr_pmu.c:232:22: error: implicit declaration of function 'readl_relaxed' [-Werror=implicit-function-declaration]
     232 |         u32 status = readl_relaxed(pmu->membase + r->status.reg);
         |                      ^~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/writel_relaxed +205 drivers/perf/stm32_ddr_pmu.c

   200	
   201	static void stm32_ddr_start_counters(struct stm32_ddr_pmu *pmu)
   202	{
   203		const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
   204	
 > 205		writel_relaxed(r->start.mask, pmu->membase + r->start.reg);
   206	}
   207	
   208	static void stm32_ddr_stop_counters(struct stm32_ddr_pmu *pmu)
   209	{
   210		const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
   211	
   212		writel_relaxed(r->stop.mask, pmu->membase + r->stop.reg);
   213	}
   214	
   215	static void stm32_ddr_clear_time_counter(struct stm32_ddr_pmu *pmu)
   216	{
   217		const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
   218	
   219		writel_relaxed(r->clear_time.mask, pmu->membase + r->clear_time.reg);
   220	}
   221	
   222	static void stm32_ddr_clear_event_counter(struct stm32_ddr_pmu *pmu, struct stm32_ddr_cnt *counter)
   223	{
   224		const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
   225	
   226		writel_relaxed(r->clear_cnt.mask & BIT(counter->idx), pmu->membase + r->clear_cnt.reg);
   227	}
   228	
   229	static void stm32_ddr_clear_counter(struct stm32_ddr_pmu *pmu, struct stm32_ddr_cnt *counter)
   230	{
   231		const struct stm32_ddr_pmu_regspec *r = pmu->cfg->regs;
 > 232		u32 status = readl_relaxed(pmu->membase + r->status.reg);
   233	
   234		if (counter->idx == pmu->cfg->time_cnt_idx)
   235			stm32_ddr_clear_time_counter(pmu);
   236		else
   237			stm32_ddr_clear_event_counter(pmu, counter);
   238	
   239		if (status & r->status.mask)
   240			dev_err(pmu->dev, "Failed to clear counter %i because the PMU is busy\n",
   241				counter->idx);
   242	}
   243	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver

[Clement LE GOFFIC]

On 6/23/25 11:45, Krzysztof Kozlowski wrote:
[...]

Hi Krzysztof,

Sorry I forgot to address comments below.

>> +
>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp1 = {
>> +	.regs = &stm32_ddr_pmu_regspec_mp1,
>> +	.attribute = stm32_ddr_pmu_attr_groups_mp1,
>> +	.counters_nb = MP1_CNT_NB,
>> +	.evt_counters_nb = MP1_CNT_NB - 1, /* Time counter is not an event counter */
>> +	.time_cnt_idx = MP1_TIME_CNT_IDX,
>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp1,
>> +};
>> +
>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp2 = {
>> +	.regs = &stm32_ddr_pmu_regspec_mp2,
>> +	.attribute = stm32_ddr_pmu_attr_groups_mp2,
>> +	.counters_nb = MP2_CNT_NB,
>> +	.evt_counters_nb = MP2_CNT_NB - 1, /* Time counter is an event counter */
>> +	.time_cnt_idx = MP2_TIME_CNT_IDX,
>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp2,
>> +};
>> +
>> +static const struct dev_pm_ops stm32_ddr_pmu_pm_ops = {
>> +	SET_SYSTEM_SLEEP_PM_OPS(NULL, stm32_ddr_pmu_device_resume)
>> +};
>> +
>> +static const struct of_device_id stm32_ddr_pmu_of_match[] = {
>> +	{
>> +		.compatible = "st,stm32mp131-ddr-pmu",
>> +		.data = &stm32_ddr_pmu_cfg_mp1
>> +	},
>> +	{
>> +		.compatible = "st,stm32mp151-ddr-pmu",
>> +		.data = &stm32_ddr_pmu_cfg_mp1
> 
> So devices are compatible, thus express it correctly and drop this.

Ok so I assume this comes with your comment in the bindings and 
basically don't get you point here.
Can you please be more precise ?

> 
>> +	},
>> +	{
>> +		.compatible = "st,stm32mp251-ddr-pmu",
>> +		.data = &stm32_ddr_pmu_cfg_mp2
>> +	},
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32_ddr_pmu_of_match);
>> +
>> +static struct platform_driver stm32_ddr_pmu_driver = {
>> +	.driver = {
>> +		.name = DRIVER_NAME,
>> +		.pm = &stm32_ddr_pmu_pm_ops,
>> +		.of_match_table = of_match_ptr(stm32_ddr_pmu_of_match),
> 
> Drop of_match_ptr, you have here warnings.

Yes Indeed.
I'll also fix the pm pointer by using "pm_sleep_ptr".

Best regards,
Clément

> 
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver

[Krzysztof Kozlowski]

On 24/06/2025 12:43, Clement LE GOFFIC wrote:
> On 6/23/25 11:45, Krzysztof Kozlowski wrote:
> [...]
> 
> Hi Krzysztof,
> 
> Sorry I forgot to address comments below.
> 
>>> +
>>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp1 = {
>>> +	.regs = &stm32_ddr_pmu_regspec_mp1,
>>> +	.attribute = stm32_ddr_pmu_attr_groups_mp1,
>>> +	.counters_nb = MP1_CNT_NB,
>>> +	.evt_counters_nb = MP1_CNT_NB - 1, /* Time counter is not an event counter */
>>> +	.time_cnt_idx = MP1_TIME_CNT_IDX,
>>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp1,
>>> +};
>>> +
>>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp2 = {
>>> +	.regs = &stm32_ddr_pmu_regspec_mp2,
>>> +	.attribute = stm32_ddr_pmu_attr_groups_mp2,
>>> +	.counters_nb = MP2_CNT_NB,
>>> +	.evt_counters_nb = MP2_CNT_NB - 1, /* Time counter is an event counter */
>>> +	.time_cnt_idx = MP2_TIME_CNT_IDX,
>>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp2,
>>> +};
>>> +
>>> +static const struct dev_pm_ops stm32_ddr_pmu_pm_ops = {
>>> +	SET_SYSTEM_SLEEP_PM_OPS(NULL, stm32_ddr_pmu_device_resume)
>>> +};
>>> +
>>> +static const struct of_device_id stm32_ddr_pmu_of_match[] = {
>>> +	{
>>> +		.compatible = "st,stm32mp131-ddr-pmu",
>>> +		.data = &stm32_ddr_pmu_cfg_mp1
>>> +	},
>>> +	{
>>> +		.compatible = "st,stm32mp151-ddr-pmu",
>>> +		.data = &stm32_ddr_pmu_cfg_mp1
>>
>> So devices are compatible, thus express it correctly and drop this.
> 
> Ok so I assume this comes with your comment in the bindings and 
> basically don't get you point here.
> Can you please be more precise ?

Express compatibility in the bindings, like 90% of SoCs are doing, so
with proper fallback and drop this entry in the table. My comment was
pretty precise, because this is completely standard pattern, also used
already in stm32.

Best regards,
Krzysztof

Re: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver

[Clement LE GOFFIC]

On 6/25/25 08:35, Krzysztof Kozlowski wrote:
> On 24/06/2025 12:43, Clement LE GOFFIC wrote:
>> On 6/23/25 11:45, Krzysztof Kozlowski wrote:
>> [...]
>>
>> Hi Krzysztof,
>>
>> Sorry I forgot to address comments below.
>>
>>>> +
>>>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp1 = {
>>>> +	.regs = &stm32_ddr_pmu_regspec_mp1,
>>>> +	.attribute = stm32_ddr_pmu_attr_groups_mp1,
>>>> +	.counters_nb = MP1_CNT_NB,
>>>> +	.evt_counters_nb = MP1_CNT_NB - 1, /* Time counter is not an event counter */
>>>> +	.time_cnt_idx = MP1_TIME_CNT_IDX,
>>>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp1,
>>>> +};
>>>> +
>>>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp2 = {
>>>> +	.regs = &stm32_ddr_pmu_regspec_mp2,
>>>> +	.attribute = stm32_ddr_pmu_attr_groups_mp2,
>>>> +	.counters_nb = MP2_CNT_NB,
>>>> +	.evt_counters_nb = MP2_CNT_NB - 1, /* Time counter is an event counter */
>>>> +	.time_cnt_idx = MP2_TIME_CNT_IDX,
>>>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp2,
>>>> +};
>>>> +
>>>> +static const struct dev_pm_ops stm32_ddr_pmu_pm_ops = {
>>>> +	SET_SYSTEM_SLEEP_PM_OPS(NULL, stm32_ddr_pmu_device_resume)
>>>> +};
>>>> +
>>>> +static const struct of_device_id stm32_ddr_pmu_of_match[] = {
>>>> +	{
>>>> +		.compatible = "st,stm32mp131-ddr-pmu",
>>>> +		.data = &stm32_ddr_pmu_cfg_mp1
>>>> +	},
>>>> +	{
>>>> +		.compatible = "st,stm32mp151-ddr-pmu",
>>>> +		.data = &stm32_ddr_pmu_cfg_mp1
>>>
>>> So devices are compatible, thus express it correctly and drop this.
>>
>> Ok so I assume this comes with your comment in the bindings and
>> basically don't get you point here.
>> Can you please be more precise ?
> 
> Express compatibility in the bindings, like 90% of SoCs are doing, so
> with proper fallback and drop this entry in the table. My comment was
> pretty precise, because this is completely standard pattern, also used
> already in stm32.
> 

Ok I remember your discussion with Alex in my V1 of pinctrl-hdp :
https://lore.kernel.org/all/1de58672-5355-4b75-99f4-c48687017d2f@kernel.org/

Does it suits you :
In the SoC DT:
MP13: compatible = "st,stm32mp131-ddr-pmu", "st,stm32mp1-ddr-pmu";
MP15: compatible = "st,stm32mp151-ddr-pmu", "st,stm32mp1-ddr-pmu";
MP25: compatible = "st,stm32mp251-ddr-pmu";

In the bindings:
properties:
   compatible:
     enum:
       - st,stm32mp1-ddr-pmu
       - st,stm32mp131-ddr-pmu
       - st,stm32mp151-ddr-pmu
       - st,stm32mp251-ddr-pmu

In the driver:
static const struct of_device_id stm32_ddr_pmu_of_match[] = {
	{
		.compatible = "st,stm32mp1-ddr-pmu",
		.data = &stm32_ddr_pmu_cfg_mp1
	},
	{
		.compatible = "st,stm32mp251-ddr-pmu",
		.data = &stm32_ddr_pmu_cfg_mp2
	},
	{ },
};

Best regards,
Clément

> Best regards,
> Krzysztof

Re: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver

[Krzysztof Kozlowski]

On 25/06/2025 10:33, Clement LE GOFFIC wrote:
> On 6/25/25 08:35, Krzysztof Kozlowski wrote:
>> On 24/06/2025 12:43, Clement LE GOFFIC wrote:
>>> On 6/23/25 11:45, Krzysztof Kozlowski wrote:
>>> [...]
>>>
>>> Hi Krzysztof,
>>>
>>> Sorry I forgot to address comments below.
>>>
>>>>> +
>>>>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp1 = {
>>>>> +	.regs = &stm32_ddr_pmu_regspec_mp1,
>>>>> +	.attribute = stm32_ddr_pmu_attr_groups_mp1,
>>>>> +	.counters_nb = MP1_CNT_NB,
>>>>> +	.evt_counters_nb = MP1_CNT_NB - 1, /* Time counter is not an event counter */
>>>>> +	.time_cnt_idx = MP1_TIME_CNT_IDX,
>>>>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp1,
>>>>> +};
>>>>> +
>>>>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp2 = {
>>>>> +	.regs = &stm32_ddr_pmu_regspec_mp2,
>>>>> +	.attribute = stm32_ddr_pmu_attr_groups_mp2,
>>>>> +	.counters_nb = MP2_CNT_NB,
>>>>> +	.evt_counters_nb = MP2_CNT_NB - 1, /* Time counter is an event counter */
>>>>> +	.time_cnt_idx = MP2_TIME_CNT_IDX,
>>>>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp2,
>>>>> +};
>>>>> +
>>>>> +static const struct dev_pm_ops stm32_ddr_pmu_pm_ops = {
>>>>> +	SET_SYSTEM_SLEEP_PM_OPS(NULL, stm32_ddr_pmu_device_resume)
>>>>> +};
>>>>> +
>>>>> +static const struct of_device_id stm32_ddr_pmu_of_match[] = {
>>>>> +	{
>>>>> +		.compatible = "st,stm32mp131-ddr-pmu",
>>>>> +		.data = &stm32_ddr_pmu_cfg_mp1
>>>>> +	},
>>>>> +	{
>>>>> +		.compatible = "st,stm32mp151-ddr-pmu",
>>>>> +		.data = &stm32_ddr_pmu_cfg_mp1
>>>>
>>>> So devices are compatible, thus express it correctly and drop this.
>>>
>>> Ok so I assume this comes with your comment in the bindings and
>>> basically don't get you point here.
>>> Can you please be more precise ?
>>
>> Express compatibility in the bindings, like 90% of SoCs are doing, so
>> with proper fallback and drop this entry in the table. My comment was
>> pretty precise, because this is completely standard pattern, also used
>> already in stm32.
>>
> 
> Ok I remember your discussion with Alex in my V1 of pinctrl-hdp :
> https://lore.kernel.org/all/1de58672-5355-4b75-99f4-c48687017d2f@kernel.org/
> 
> Does it suits you :
> In the SoC DT:
> MP13: compatible = "st,stm32mp131-ddr-pmu", "st,stm32mp1-ddr-pmu";
> MP15: compatible = "st,stm32mp151-ddr-pmu", "st,stm32mp1-ddr-pmu";

No, because I did not say to change other entry in the table. Please
read again what I asked: drop this. "This" means ONLY this entry. "Drop
this" does not mean "change something else". Do not change other entries
by introducing some generic compatible. That's not the pattern ever
endorsed by DT maintainers. Add front compatible and you are done,
smallest amount of changes, most obvious code.

Best regards,
Krzysztof

Re: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver

[Clement LE GOFFIC]

On 6/25/25 10:48, Krzysztof Kozlowski wrote:
> On 25/06/2025 10:33, Clement LE GOFFIC wrote:
>> On 6/25/25 08:35, Krzysztof Kozlowski wrote:
>>> On 24/06/2025 12:43, Clement LE GOFFIC wrote:
>>>> On 6/23/25 11:45, Krzysztof Kozlowski wrote:
>>>> [...]
>>>>
>>>> Hi Krzysztof,
>>>>
>>>> Sorry I forgot to address comments below.
>>>>
>>>>>> +
>>>>>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp1 = {
>>>>>> +	.regs = &stm32_ddr_pmu_regspec_mp1,
>>>>>> +	.attribute = stm32_ddr_pmu_attr_groups_mp1,
>>>>>> +	.counters_nb = MP1_CNT_NB,
>>>>>> +	.evt_counters_nb = MP1_CNT_NB - 1, /* Time counter is not an event counter */
>>>>>> +	.time_cnt_idx = MP1_TIME_CNT_IDX,
>>>>>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp1,
>>>>>> +};
>>>>>> +
>>>>>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp2 = {
>>>>>> +	.regs = &stm32_ddr_pmu_regspec_mp2,
>>>>>> +	.attribute = stm32_ddr_pmu_attr_groups_mp2,
>>>>>> +	.counters_nb = MP2_CNT_NB,
>>>>>> +	.evt_counters_nb = MP2_CNT_NB - 1, /* Time counter is an event counter */
>>>>>> +	.time_cnt_idx = MP2_TIME_CNT_IDX,
>>>>>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp2,
>>>>>> +};
>>>>>> +
>>>>>> +static const struct dev_pm_ops stm32_ddr_pmu_pm_ops = {
>>>>>> +	SET_SYSTEM_SLEEP_PM_OPS(NULL, stm32_ddr_pmu_device_resume)
>>>>>> +};
>>>>>> +
>>>>>> +static const struct of_device_id stm32_ddr_pmu_of_match[] = {
>>>>>> +	{
>>>>>> +		.compatible = "st,stm32mp131-ddr-pmu",
>>>>>> +		.data = &stm32_ddr_pmu_cfg_mp1
>>>>>> +	},
>>>>>> +	{
>>>>>> +		.compatible = "st,stm32mp151-ddr-pmu",
>>>>>> +		.data = &stm32_ddr_pmu_cfg_mp1
>>>>>
>>>>> So devices are compatible, thus express it correctly and drop this.
>>>>
>>>> Ok so I assume this comes with your comment in the bindings and
>>>> basically don't get you point here.
>>>> Can you please be more precise ?
>>>
>>> Express compatibility in the bindings, like 90% of SoCs are doing, so
>>> with proper fallback and drop this entry in the table. My comment was
>>> pretty precise, because this is completely standard pattern, also used
>>> already in stm32.
>>>
>>
>> Ok I remember your discussion with Alex in my V1 of pinctrl-hdp :
>> https://lore.kernel.org/all/1de58672-5355-4b75-99f4-c48687017d2f@kernel.org/
>>
>> Does it suits you :
>> In the SoC DT:
>> MP13: compatible = "st,stm32mp131-ddr-pmu", "st,stm32mp1-ddr-pmu";
>> MP15: compatible = "st,stm32mp151-ddr-pmu", "st,stm32mp1-ddr-pmu";
> 
> No, because I did not say to change other entry in the table. Please
> read again what I asked: drop this. "This" means ONLY this entry. "Drop
> this" does not mean "change something else". Do not change other entries
> by introducing some generic compatible. That's not the pattern ever
> endorsed by DT maintainers. Add front compatible and you are done,
> smallest amount of changes, most obvious code.
>

Ok so in the SoC DT I'll keep:
MP13: compatible = "st,stm32mp131-ddr-pmu";
MP15: compatible = "st,stm32mp151-ddr-pmu", "st,stm32mp131-ddr-pmu";

Thanks for clarifying.

Best regards,
Clément

> Best regards,
> Krzysztof

Re: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver

[kernel test robot]

Hi Clément,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 86731a2a651e58953fc949573895f2fa6d456841]

url:    https://github.com/intel-lab-lkp/linux/commits/Cl-ment-Le-Goffic/bus-firewall-move-stm32_firewall-header-file-in-include-folder/20250623-173554
base:   86731a2a651e58953fc949573895f2fa6d456841
patch link:    https://lore.kernel.org/r/20250623-ddrperfm-upstream-v1-6-7dffff168090%40foss.st.com
patch subject: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver
config: i386-buildonly-randconfig-004-20250627 (https://download.01.org/0day-ci/archive/20250627/202506270708.6w6phhmi-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250627/202506270708.6w6phhmi-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506270708.6w6phhmi-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/perf/stm32_ddr_pmu.c: In function 'stm32_ddr_start_counters':
   drivers/perf/stm32_ddr_pmu.c:205:9: error: implicit declaration of function 'writel_relaxed' [-Werror=implicit-function-declaration]
     205 |         writel_relaxed(r->start.mask, pmu->membase + r->start.reg);
         |         ^~~~~~~~~~~~~~
   drivers/perf/stm32_ddr_pmu.c: In function 'stm32_ddr_clear_counter':
   drivers/perf/stm32_ddr_pmu.c:232:22: error: implicit declaration of function 'readl_relaxed' [-Werror=implicit-function-declaration]
     232 |         u32 status = readl_relaxed(pmu->membase + r->status.reg);
         |                      ^~~~~~~~~~~~~
   drivers/perf/stm32_ddr_pmu.c: At top level:
>> drivers/perf/stm32_ddr_pmu.c:862:34: warning: 'stm32_ddr_pmu_of_match' defined but not used [-Wunused-const-variable=]
     862 | static const struct of_device_id stm32_ddr_pmu_of_match[] = {
         |                                  ^~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/stm32_ddr_pmu_of_match +862 drivers/perf/stm32_ddr_pmu.c

   861	
 > 862	static const struct of_device_id stm32_ddr_pmu_of_match[] = {
   863		{
   864			.compatible = "st,stm32mp131-ddr-pmu",
   865			.data = &stm32_ddr_pmu_cfg_mp1
   866		},
   867		{
   868			.compatible = "st,stm32mp151-ddr-pmu",
   869			.data = &stm32_ddr_pmu_cfg_mp1
   870		},
   871		{
   872			.compatible = "st,stm32mp251-ddr-pmu",
   873			.data = &stm32_ddr_pmu_cfg_mp2
   874		},
   875		{ },
   876	};
   877	MODULE_DEVICE_TABLE(of, stm32_ddr_pmu_of_match);
   878	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver

[Philipp Zabel]

On Mo, 2025-06-23 at 11:27 +0200, Clément Le Goffic wrote:
> Introduce the driver for the DDR Performance Monitor available on
> STM32MPU SoC.
> 
> On STM32MP2 platforms, the DDRPERFM allows to monitor up to 8 DDR events
> that come from the DDR Controller such as read or write events.
> 
> On STM32MP1 platforms, the DDRPERFM cannot monitor any event on any
> counter, there is a notion of set of events.
> Events from different sets cannot be monitored at the same time.
> The first chosen event selects the set.
> The set is coded in the first two bytes of the config value which is on 4
> bytes.
> 
> On STM32MP25x series, the DDRPERFM clock is shared with the DDR controller
> and may be secured by bootloaders.
> Access controllers allow to check access to a resource. Use the access
> controller defined in the devicetree to know about the access to the
> DDRPERFM clock.
> 
> Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
> ---
>  drivers/perf/Kconfig         |  11 +
>  drivers/perf/Makefile        |   1 +
>  drivers/perf/stm32_ddr_pmu.c | 893 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 905 insertions(+)
> 
[...]
> diff --git a/drivers/perf/stm32_ddr_pmu.c b/drivers/perf/stm32_ddr_pmu.c
> new file mode 100644
> index 000000000000..c0bce1f446a0
> --- /dev/null
> +++ b/drivers/perf/stm32_ddr_pmu.c
> @@ -0,0 +1,893 @@
[...]
> +	if (of_property_present(pdev->dev.of_node, "resets")) {
> +		rst = devm_reset_control_get(&pdev->dev, NULL);

Use devm_reset_control_get_optional_exclusive() instead. It returns
NULL if the device tree doesn't contain a resets property.

> +		if (IS_ERR(rst)) {
> +			dev_err(&pdev->dev, "Failed to get reset\n");

Please consider using dev_err_probe() instead.

> +			ret = PTR_ERR(rst);
> +			goto err_clk;
> +		}
> +		reset_control_assert(rst);
> +		reset_control_deassert(rst);

These can be done unconditionally, as they are no-ops for rst == NULL. 

regards
Philipp

Re: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver

[Clement LE GOFFIC]

Hi Philip

On 6/30/25 10:38, Philipp Zabel wrote:
> On Mo, 2025-06-23 at 11:27 +0200, Clément Le Goffic wrote:
>> Introduce the driver for the DDR Performance Monitor available on
>> STM32MPU SoC.
>>
>> On STM32MP2 platforms, the DDRPERFM allows to monitor up to 8 DDR events
>> that come from the DDR Controller such as read or write events.
>>
>> On STM32MP1 platforms, the DDRPERFM cannot monitor any event on any
>> counter, there is a notion of set of events.
>> Events from different sets cannot be monitored at the same time.
>> The first chosen event selects the set.
>> The set is coded in the first two bytes of the config value which is on 4
>> bytes.
>>
>> On STM32MP25x series, the DDRPERFM clock is shared with the DDR controller
>> and may be secured by bootloaders.
>> Access controllers allow to check access to a resource. Use the access
>> controller defined in the devicetree to know about the access to the
>> DDRPERFM clock.
>>
>> Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
>> ---
>>   drivers/perf/Kconfig         |  11 +
>>   drivers/perf/Makefile        |   1 +
>>   drivers/perf/stm32_ddr_pmu.c | 893 +++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 905 insertions(+)
>>
> [...]
>> diff --git a/drivers/perf/stm32_ddr_pmu.c b/drivers/perf/stm32_ddr_pmu.c
>> new file mode 100644
>> index 000000000000..c0bce1f446a0
>> --- /dev/null
>> +++ b/drivers/perf/stm32_ddr_pmu.c
>> @@ -0,0 +1,893 @@
> [...]
>> +	if (of_property_present(pdev->dev.of_node, "resets")) {
>> +		rst = devm_reset_control_get(&pdev->dev, NULL);
> 
> Use devm_reset_control_get_optional_exclusive() instead. It returns
> NULL if the device tree doesn't contain a resets property.

Ok I will have a look, thank you

> 
>> +		if (IS_ERR(rst)) {
>> +			dev_err(&pdev->dev, "Failed to get reset\n");
> 
> Please consider using dev_err_probe() instead.

Ok

>> +			ret = PTR_ERR(rst);
>> +			goto err_clk;
>> +		}
>> +		reset_control_assert(rst);
>> +		reset_control_deassert(rst);
> 
> These can be done unconditionally, as they are no-ops for rst == NULL.

Indeed

Best regards,
Clément