[PATCH 0/3] firmware: samsung: add ACPM debugfs support

[PATCH 0/3] firmware: samsung: add ACPM debugfs support

[Tudor Ambarus]

The ACPM firmware saves debug information to SRAM. Add debugfs entries
in order to expose the ACPM logs.

acpm_framework/logb_gprio_level controls the ACPM print verbosity to
the SRAM log buffer. It encodes a 64 bit value, 4 bits for each of the
16 Plugin IDs, with verbosity levels from 0xf (log error) to
0x0 (log debug).

acpm_framework/log_level has a maximum value of 2 and controls which
SRAM log buffers are printed.

Finally, acpm_framework/acpm_debug_cmd provides a way to issue
ACPM DEBUG commands to the firmware.

Add ACPM debugfs support with the above capabilities.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
Tudor Ambarus (3):
      firmware: exynos-acpm: rename exynos-acpm.h to exynos-acpm-xfer.h
      firmware: exynos-acpm: move common structures to exynos-acpm.h
      firmware: samsung: add ACPM debugfs support

 drivers/firmware/samsung/Makefile              |   1 +
 drivers/firmware/samsung/exynos-acpm-debugfs.c | 359 +++++++++++++++++++++++++
 drivers/firmware/samsung/exynos-acpm-pmic.c    |   2 +-
 drivers/firmware/samsung/exynos-acpm-xfer.h    |  23 ++
 drivers/firmware/samsung/exynos-acpm.c         |  61 ++---
 drivers/firmware/samsung/exynos-acpm.h         |  95 ++++++-
 6 files changed, 485 insertions(+), 56 deletions(-)
---
base-commit: f4d315a1f9d8fcaf608f283f01ce4d66160f44a1
change-id: 20250221-acpm-debugfs-d196d08e6462

Best regards,
-- 
Tudor Ambarus <tudor.ambarus@linaro.org>

[PATCH 1/3] firmware: exynos-acpm: rename exynos-acpm.h to exynos-acpm-xfer.h

[Tudor Ambarus]

Rename header file to better indicate what it is used for.
This is a preparation patch, as exynos-acpm.h will be later on
introduced and included between the core driver and its debugfs
logging feature.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/firmware/samsung/exynos-acpm-pmic.c                    | 2 +-
 drivers/firmware/samsung/{exynos-acpm.h => exynos-acpm-xfer.h} | 6 +++---
 drivers/firmware/samsung/exynos-acpm.c                         | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/samsung/exynos-acpm-pmic.c b/drivers/firmware/samsung/exynos-acpm-pmic.c
index 85e90d236da2..9ad4cb82fae1 100644
--- a/drivers/firmware/samsung/exynos-acpm-pmic.c
+++ b/drivers/firmware/samsung/exynos-acpm-pmic.c
@@ -9,7 +9,7 @@
 #include <linux/ktime.h>
 #include <linux/types.h>
 
-#include "exynos-acpm.h"
+#include "exynos-acpm-xfer.h"
 #include "exynos-acpm-pmic.h"
 
 #define ACPM_PMIC_CHANNEL		GENMASK(15, 12)
diff --git a/drivers/firmware/samsung/exynos-acpm.h b/drivers/firmware/samsung/exynos-acpm-xfer.h
similarity index 79%
rename from drivers/firmware/samsung/exynos-acpm.h
rename to drivers/firmware/samsung/exynos-acpm-xfer.h
index 2d14cb58f98c..0b76a270b773 100644
--- a/drivers/firmware/samsung/exynos-acpm.h
+++ b/drivers/firmware/samsung/exynos-acpm-xfer.h
@@ -4,8 +4,8 @@
  * Copyright 2020 Google LLC.
  * Copyright 2024 Linaro Ltd.
  */
-#ifndef __EXYNOS_ACPM_H__
-#define __EXYNOS_ACPM_H__
+#ifndef __EXYNOS_ACPM_XFER_H__
+#define __EXYNOS_ACPM_XFER_H__
 
 struct acpm_xfer {
 	const u32 *txd;
@@ -20,4 +20,4 @@ struct acpm_handle;
 int acpm_do_xfer(const struct acpm_handle *handle,
 		 const struct acpm_xfer *xfer);
 
-#endif /* __EXYNOS_ACPM_H__ */
+#endif /* __EXYNOS_ACPM_XFER_H__ */
diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c
index a85b2dbdd9f0..3c14afc89fd7 100644
--- a/drivers/firmware/samsung/exynos-acpm.c
+++ b/drivers/firmware/samsung/exynos-acpm.c
@@ -27,7 +27,7 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
-#include "exynos-acpm.h"
+#include "exynos-acpm-xfer.h"
 #include "exynos-acpm-pmic.h"
 
 #define ACPM_PROTOCOL_SEQNUM		GENMASK(21, 16)

-- 
2.48.1.601.g30ceb7b040-goog

[PATCH 2/3] firmware: exynos-acpm: move common structures to exynos-acpm.h

[Tudor Ambarus]

Prepare for the ACPM logging feature addition. ACPM is capable of logging
things to SRAM. The logging feature needs access to struct acpm_info
in order to get the sram_base, to the configuration data from SRAM, and to
the struct acpm_queue internal driver representation of a queue.
Move these structs to a common exynos-acpm.h.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/firmware/samsung/exynos-acpm.c | 48 +-------------------------
 drivers/firmware/samsung/exynos-acpm.h | 63 ++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 47 deletions(-)

diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c
index 3c14afc89fd7..8d83841f1d62 100644
--- a/drivers/firmware/samsung/exynos-acpm.c
+++ b/drivers/firmware/samsung/exynos-acpm.c
@@ -12,7 +12,6 @@
 #include <linux/container_of.h>
 #include <linux/delay.h>
 #include <linux/device.h>
-#include <linux/firmware/samsung/exynos-acpm-protocol.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/mailbox/exynos-message.h>
@@ -27,7 +26,7 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
-#include "exynos-acpm-xfer.h"
+#include "exynos-acpm.h"
 #include "exynos-acpm-pmic.h"
 
 #define ACPM_PROTOCOL_SEQNUM		GENMASK(21, 16)
@@ -38,20 +37,6 @@
 
 #define ACPM_GS101_INITDATA_BASE	0xa000
 
-/**
- * struct acpm_shmem - shared memory configuration information.
- * @reserved:	unused fields.
- * @chans:	offset to array of struct acpm_chan_shmem.
- * @reserved1:	unused fields.
- * @num_chans:	number of channels.
- */
-struct acpm_shmem {
-	u32 reserved[2];
-	u32 chans;
-	u32 reserved1[3];
-	u32 num_chans;
-};
-
 /**
  * struct acpm_chan_shmem - descriptor of a shared memory channel.
  *
@@ -85,19 +70,6 @@ struct acpm_chan_shmem {
 	u32 poll_completion;
 };
 
-/**
- * struct acpm_queue - exynos acpm queue.
- *
- * @rear:	rear address of the queue.
- * @front:	front address of the queue.
- * @base:	base address of the queue.
- */
-struct acpm_queue {
-	void __iomem *rear;
-	void __iomem *front;
-	void __iomem *base;
-};
-
 /**
  * struct acpm_rx_data - RX queue data.
  *
@@ -155,24 +127,6 @@ struct acpm_chan {
 	struct acpm_rx_data rx_data[ACPM_SEQNUM_MAX];
 };
 
-/**
- * struct acpm_info - driver's private data.
- * @shmem:	pointer to the SRAM configuration data.
- * @sram_base:	base address of SRAM.
- * @chans:	pointer to the ACPM channel parameters retrieved from SRAM.
- * @dev:	pointer to the exynos-acpm device.
- * @handle:	instance of acpm_handle to send to clients.
- * @num_chans:	number of channels available for this controller.
- */
-struct acpm_info {
-	struct acpm_shmem __iomem *shmem;
-	void __iomem *sram_base;
-	struct acpm_chan *chans;
-	struct device *dev;
-	struct acpm_handle handle;
-	u32 num_chans;
-};
-
 /**
  * struct acpm_match_data - of_device_id data.
  * @initdata_base:	offset in SRAM where the channels configuration resides.
diff --git a/drivers/firmware/samsung/exynos-acpm.h b/drivers/firmware/samsung/exynos-acpm.h
new file mode 100644
index 000000000000..c212fe28758a
--- /dev/null
+++ b/drivers/firmware/samsung/exynos-acpm.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright 2020 Samsung Electronics Co., Ltd.
+ * Copyright 2020 Google LLC.
+ * Copyright 2024 Linaro Ltd.
+ */
+#ifndef __EXYNOS_ACPM_H__
+#define __EXYNOS_ACPM_H__
+
+#include <linux/debugfs.h>
+#include <linux/firmware/samsung/exynos-acpm-protocol.h>
+#include <linux/types.h>
+
+#include "exynos-acpm-xfer.h"
+
+/**
+ * struct acpm_shmem - shared memory configuration information.
+ * @reserved:	unused fields.
+ * @chans:	offset to array of struct acpm_chan_shmem.
+ * @reserved1:	unused fields.
+ * @num_chans:	number of channels.
+ */
+struct acpm_shmem {
+	u32 reserved[2];
+	u32 chans;
+	u32 reserved1[3];
+	u32 num_chans;
+};
+
+/**
+ * struct acpm_queue - exynos acpm queue.
+ * @rear:	rear address of the queue.
+ * @front:	front address of the queue.
+ * @base:	base address of the queue.
+ */
+struct acpm_queue {
+	void __iomem *rear;
+	void __iomem *front;
+	void __iomem *base;
+};
+
+struct device;
+struct acpm_chan;
+
+/**
+ * struct acpm_info - driver's private data.
+ * @shmem:	pointer to the SRAM configuration data.
+ * @sram_base:	base address of SRAM.
+ * @chans:	pointer to the ACPM channel parameters retrieved from SRAM.
+ * @dev:	pointer to the exynos-acpm device.
+ * @handle:	instance of acpm_handle to send to clients.
+ * @num_chans:	number of channels available for this controller.
+ */
+struct acpm_info {
+	struct acpm_shmem __iomem *shmem;
+	void __iomem *sram_base;
+	struct acpm_chan *chans;
+	struct device *dev;
+	struct acpm_handle handle;
+	u32 num_chans;
+};
+
+#endif /* __EXYNOS_ACPM_H__ */

-- 
2.48.1.601.g30ceb7b040-goog

[PATCH 3/3] firmware: samsung: add ACPM debugfs support

[Tudor Ambarus]

The ACPM firmware saves debug information to SRAM. Add debugfs entries
in order to expose the ACPM logs.

acpm_framework/logb_gprio_level controls the ACPM print verbosity to
the SRAM log buffer. It encodes a 64 bit value, 4 bits for each of the
16 Plugin IDs, with verbosity levels from 0xf (log error) to
0x0 (log debug).

echo 0xffffffffffffff1f > /sys/kernel/debug/acpm_framework/logb_gprio_level
Will allow only LOG_ERR prints for all Plugin IDs but Plugin ID 1,
which will issue prints for any log levels greater or equal to 1.
On the ACPM firmware side, logb_gprio_level has a default value of zero,
all logs enabled for all Plugin IDs.

acpm_framework/log_level has a maximum value of 2 and controls which
SRAM log buffers are printed.

Finally, acpm_framework/acpm_debug_cmd provides a way to issue
ACPM DEBUG commands to the firmware.

Add ACPM debugfs support with the above capabilities.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/firmware/samsung/Makefile              |   1 +
 drivers/firmware/samsung/exynos-acpm-debugfs.c | 359 +++++++++++++++++++++++++
 drivers/firmware/samsung/exynos-acpm.c         |  15 ++
 drivers/firmware/samsung/exynos-acpm.h         |  37 +++
 4 files changed, 412 insertions(+)

diff --git a/drivers/firmware/samsung/Makefile b/drivers/firmware/samsung/Makefile
index 7b4c9f6f34f5..ca6b71872ac3 100644
--- a/drivers/firmware/samsung/Makefile
+++ b/drivers/firmware/samsung/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 acpm-protocol-objs			:= exynos-acpm.o exynos-acpm-pmic.o
+acpm-protocol-$(CONFIG_DEBUG_FS)	+= exynos-acpm-debugfs.o
 obj-$(CONFIG_EXYNOS_ACPM_PROTOCOL)	+= acpm-protocol.o
diff --git a/drivers/firmware/samsung/exynos-acpm-debugfs.c b/drivers/firmware/samsung/exynos-acpm-debugfs.c
new file mode 100644
index 000000000000..d839321d8901
--- /dev/null
+++ b/drivers/firmware/samsung/exynos-acpm-debugfs.c
@@ -0,0 +1,359 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2020 Samsung Electronics Co., Ltd.
+ * Copyright 2020 Google LLC.
+ * Copyright 2025 Linaro Ltd.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/container_of.h>
+#include <linux/debugfs.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/ktime.h>
+#include <linux/math64.h>
+#include <linux/workqueue.h>
+
+#include "exynos-acpm.h"
+
+#define ACPM_DEBUG_CMD				BIT(14)
+
+#define ACPM_PRINT_CONFIG			GENMASK(15, 14)
+#define ACPM_PRINT_CMD				BIT(13)
+#define ACPM_PRINT_SET_LOGB_GPRIO_LEVEL		1
+#define ACPM_PRINT_GET_LOGB_GPRIO_LEVEL		3
+
+#define ACPM_LOG_LEVEL_MAX			2
+#define ACPM_LOG_POLL_PERIOD_US			500
+
+/* Tick runs at 49.152 MHz, the period below is in picoseconds. */
+#define ACPM_APM_SYSTICK_PERIOD_PS		20345
+
+#define ACPM_DEBUGFS_ROOT "acpm_framework"
+
+enum acpm_debug_commands {
+	ACPM_DEBUG_DISABLE_WATCHDOG,
+	ACPM_DEBUG_ENABLE_WATCHDOG,
+	ACPM_DEBUG_SOFT_LOCKUP,
+	ACPM_DEBUG_HARD_LOCKUP,
+	ACPM_DBUG_EXCEPTION,
+	ACPM_DEBUG_NOTIFY_SHUTDOWN,
+	ACPM_DEBUG_RAMDUMP_ON,
+	ACPM_DEBUG_MAX,
+};
+
+struct acpm_log_buf {
+	struct acpm_queue q;
+	unsigned int qlen;
+	unsigned int mlen;
+	unsigned int rear_index;
+};
+
+struct acpm_log_info {
+	struct workqueue_struct *wq;
+	struct acpm_info *acpm;
+	struct delayed_work work;
+	struct acpm_log_buf normal;
+	struct acpm_log_buf preempt;
+	unsigned int level;
+	unsigned int poll_period;
+};
+
+union acpm_log_entry {
+	u32 raw[4];
+	struct {
+		u32 systicks0 : 24;
+		u32 dummy : 2;
+		u32 is_err : 1;
+		u32 is_raw : 1;
+		u32 plugin_id : 4;
+		u32 systicks24;
+		u32 msg : 24;
+		u32 systicks56 : 8;
+		u32 data;
+	} __packed;
+};
+
+static struct dentry *rootdir;
+
+static DEFINE_MUTEX(acpm_log_level_mutex);
+
+static void acpm_log_print_entry(struct acpm_info *acpm,
+				 const union acpm_log_entry *log_entry)
+{
+	u64 systicks, time, msg;
+
+	if (log_entry->is_err)
+		return;
+
+	if (log_entry->is_raw) {
+		dev_info(acpm->dev, "[ACPM_FW raw] : id:%u, %x, %x, %x\n",
+			 log_entry->plugin_id, log_entry->raw[1],
+			 log_entry->raw[2], log_entry->raw[3]);
+	} else {
+		systicks = ((u64)(log_entry->systicks56) << 56) +
+			   ((u64)(log_entry->systicks24) << 24) +
+			   log_entry->systicks0;
+
+		/* report time in ns */
+		time = mul_u64_u32_div(systicks, ACPM_APM_SYSTICK_PERIOD_PS,
+				       1000);
+
+		msg = readl(acpm->sram_base + log_entry->msg);
+
+		dev_info(acpm->dev, "[ACPM_FW] : %llu id:%u, %s, %x\n", time,
+			 log_entry->plugin_id, (char *)&msg, log_entry->data);
+	}
+}
+
+static void acpm_log_print_entries(struct acpm_info *acpm,
+				   struct acpm_log_buf *lbuf)
+{
+	union acpm_log_entry log_entry = {0};
+	u32 front, rear;
+
+	front = readl(lbuf->q.front);
+	rear = lbuf->rear_index;
+
+	while (rear != front) {
+		__ioread32_copy(&log_entry, lbuf->q.base + lbuf->mlen * rear,
+				sizeof(log_entry) / 4);
+
+		acpm_log_print_entry(acpm, &log_entry);
+
+		if (lbuf->qlen == rear + 1)
+			rear = 0;
+		else
+			rear++;
+
+		lbuf->rear_index = rear;
+		front = readl(lbuf->q.front);
+	}
+}
+
+static void acpm_log_print(struct acpm_info *acpm)
+{
+	struct acpm_log_info *acpm_log = acpm->log;
+
+	guard(mutex)(&acpm_log_level_mutex);
+
+	if (acpm_log->level == 0)
+		return;
+
+	if (acpm_log->level == ACPM_LOG_LEVEL_MAX)
+		acpm_log_print_entries(acpm, &acpm_log->preempt);
+
+	acpm_log_print_entries(acpm, &acpm_log->normal);
+}
+
+static void acpm_work_fn(struct work_struct *work)
+{
+	struct acpm_log_info *acpm_log =
+		container_of(work, struct acpm_log_info, work.work);
+	struct acpm_info *acpm = acpm_log->acpm;
+
+	acpm_log_print(acpm);
+
+	queue_delayed_work(acpm_log->wq, &acpm_log->work,
+			   msecs_to_jiffies(acpm_log->poll_period));
+}
+
+static int acpm_log_level_get(void *data, u64 *val)
+{
+	struct acpm_info *acpm = data;
+
+	*val = acpm->log->level;
+
+	return 0;
+}
+
+static int acpm_log_level_set(void *data, u64 val)
+{
+	struct acpm_info *acpm = data;
+	struct acpm_log_info *acpm_log = acpm->log;
+
+	if (val > ACPM_LOG_LEVEL_MAX) {
+		dev_err(acpm->dev, "Log level %llu out of range [0:%u]!\n",
+			val, ACPM_LOG_LEVEL_MAX);
+		return -EINVAL;
+	}
+
+	scoped_guard(mutex, &acpm_log_level_mutex)
+		acpm_log->level = val;
+
+	if (acpm_log->level == 0)
+		cancel_delayed_work_sync(&acpm_log->work);
+	else
+		queue_delayed_work(acpm_log->wq, &acpm_log->work,
+				   msecs_to_jiffies(acpm_log->poll_period));
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(acpm_log_level_fops, acpm_log_level_get,
+			 acpm_log_level_set, "0%llu\n");
+
+/**
+ * acpm_logb_gprio_level_get() - get ACPM Log Buffer Group Priority logging
+ * level.
+ * @data:	pointer to the driver data.
+ * @val:	pointer where the ACPM Log Buffer Group Priority logging level
+ *		will be saved.
+ *
+ * The 64 bit hex value encodes the plugin ID log level request on 4 bits,
+ * supporting a maximum of 16 plugin IDs. Plugin ID 0 is described by
+ * GENMASK(3, 0), followed by the other plugin IDs in ascending order, up to
+ * plugin ID 15 which is described by GENMASK(63, 60).
+ * Value 0xf is log error level, and 0x0 is log debug level.
+ */
+static int acpm_logb_gprio_level_get(void *data, u64 *val)
+{
+	struct acpm_info *acpm = data;
+	struct acpm_xfer xfer;
+	u32 cmd[4] = {0};
+	int ret;
+
+	cmd[0] = ACPM_PRINT_CMD |
+		 FIELD_PREP(ACPM_PRINT_CONFIG, ACPM_PRINT_GET_LOGB_GPRIO_LEVEL);
+
+	xfer.txd = cmd;
+	xfer.txlen = sizeof(cmd);
+	xfer.rxd = cmd;
+	xfer.rxlen = sizeof(cmd);
+	xfer.acpm_chan_id = acpm->mbox_dbg_chan;
+
+	ret = acpm_do_xfer(&acpm->handle, &xfer);
+	if (!ret)
+		*val = (((u64)xfer.rxd[2]) << 32) | xfer.rxd[1];
+
+	return ret;
+}
+
+/**
+ * acpm_logb_gprio_level_set() - set ACPM Log Buffer Group Priority logging
+ * level.
+ * @data:	pointer to the driver data.
+ * @val:	64 bit hex value to set.
+ * The 64 bit hex value encodes the plugin ID log level request on 4 bits,
+ * supporting a maximum of 16 plugin IDs. Plugin ID 0 is described by
+ * GENMASK(3, 0), followed by the other plugin IDs in ascending order, up to
+ * plugin ID 15 which is described by GENMASK(63, 60).
+ * Value 0xf is log error level, and 0x0 is log debug level.
+ */
+static int acpm_logb_gprio_level_set(void *data, u64 val)
+{
+	struct acpm_info *acpm = data;
+	struct acpm_xfer xfer = {0};
+	u32 cmd[4] = {0};
+
+	cmd[0] = ACPM_PRINT_CMD |
+		 FIELD_PREP(ACPM_PRINT_CONFIG, ACPM_PRINT_SET_LOGB_GPRIO_LEVEL);
+	cmd[1] = val;
+	cmd[2] = val >> 32;
+
+	xfer.txd = cmd;
+	xfer.txlen = sizeof(cmd);
+	xfer.acpm_chan_id = acpm->mbox_dbg_chan;
+
+	return acpm_do_xfer(&acpm->handle, &xfer);
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(acpm_logb_gprio_level_fops, acpm_logb_gprio_level_get,
+			 acpm_logb_gprio_level_set, "0x%016llx\n");
+
+static int acpm_debug_cmd_set(void *data, u64 val)
+{
+	struct acpm_info *acpm = data;
+	struct acpm_xfer xfer = {0};
+	u32 cmd[4] = {0};
+
+	if (val >= ACPM_DEBUG_MAX) {
+		dev_err(acpm->dev, "sub-cmd:%llu out of range!\n", val);
+		return 0;
+	}
+
+	cmd[0] = val | ACPM_DEBUG_CMD;
+
+	xfer.txd = cmd;
+	xfer.txlen = sizeof(cmd);
+	xfer.acpm_chan_id = acpm->mbox_dbg_chan;
+
+	return acpm_do_xfer(&acpm->handle, &xfer);
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(acpm_debug_cmd_fops, NULL, acpm_debug_cmd_set,
+			 "0x%016llx\n");
+
+static void acpm_debugfs_init(struct acpm_info *acpm)
+{
+	rootdir = debugfs_create_dir(ACPM_DEBUGFS_ROOT, NULL);
+
+	debugfs_create_file("log_level", 0644, rootdir, acpm,
+			    &acpm_log_level_fops);
+	debugfs_create_file("logb_gprio_level", 0644, rootdir, acpm,
+			    &acpm_logb_gprio_level_fops);
+	debugfs_create_file("acpm_debug_cmd", 0644, rootdir, acpm,
+			    &acpm_debug_cmd_fops);
+}
+
+/**
+ * acpm_debug_get_params() - get debug parameters of the normal and preempt
+ * queues.
+ * @acpm:	pointer to the driver data.
+ */
+static void acpm_debug_get_params(struct acpm_info *acpm)
+{
+	struct acpm_shmem __iomem *shmem = acpm->shmem;
+	void __iomem *base = acpm->sram_base;
+	struct acpm_log_info *acpm_log = acpm->log;
+	struct acpm_log_buf *lbuf;
+
+	lbuf = &acpm_log->normal;
+	lbuf->q.base = base + readl(&shmem->log_base);
+	lbuf->q.rear = base + readl(&shmem->log_rear);
+	lbuf->q.front = base + readl(&shmem->log_front);
+	lbuf->qlen = readl(&shmem->log_qlen);
+	lbuf->mlen = readl(&shmem->log_mlen);
+
+	lbuf = &acpm_log->preempt;
+	lbuf->q.base = base + readl(&shmem->preempt_log_base);
+	lbuf->q.rear = base + readl(&shmem->preempt_log_rear);
+	lbuf->q.front = base + readl(&shmem->preempt_log_front);
+	lbuf->qlen = readl(&shmem->preempt_log_qlen);
+	lbuf->mlen = acpm_log->normal.mlen;
+}
+
+/**
+ * acpm_debugfs_register() - register ACPM debug capabilities via debugfs.
+ * @acpm:	pointer to the driver data.
+ */
+int acpm_debugfs_register(struct acpm_info *acpm)
+{
+	struct acpm_log_info *acpm_log;
+
+	acpm_log = devm_kzalloc(acpm->dev, sizeof(*acpm_log), GFP_KERNEL);
+	if (!acpm_log)
+		return -ENOMEM;
+
+	acpm->log = acpm_log;
+	acpm_log->acpm = acpm;
+
+	acpm_log->wq = alloc_workqueue("exynos-acpm-log-wq", 0, 0);
+	if (!acpm_log->wq)
+		return -ENOMEM;
+
+	INIT_DELAYED_WORK(&acpm_log->work, acpm_work_fn);
+	acpm_log->poll_period = ACPM_LOG_POLL_PERIOD_US;
+
+	acpm_debug_get_params(acpm);
+
+	acpm_debugfs_init(acpm);
+
+	return 0;
+}
+
+void acpm_debugfs_remove(void)
+{
+	debugfs_remove_recursive(rootdir);
+}
diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c
index 8d83841f1d62..53f0d3db3400 100644
--- a/drivers/firmware/samsung/exynos-acpm.c
+++ b/drivers/firmware/samsung/exynos-acpm.c
@@ -36,6 +36,7 @@
 #define ACPM_TX_TIMEOUT_US		500000
 
 #define ACPM_GS101_INITDATA_BASE	0xa000
+#define ACPM_GS101_MBOX_DBG_CHAN	4
 
 /**
  * struct acpm_chan_shmem - descriptor of a shared memory channel.
@@ -130,9 +131,11 @@ struct acpm_chan {
 /**
  * struct acpm_match_data - of_device_id data.
  * @initdata_base:	offset in SRAM where the channels configuration resides.
+ * @mbox_dbg_chan:	mailbox channel number used for ACPM debug.
  */
 struct acpm_match_data {
 	loff_t initdata_base;
+	unsigned int mbox_dbg_chan;
 };
 
 #define client_to_acpm_chan(c) container_of(c, struct acpm_chan, cl)
@@ -577,12 +580,17 @@ static int acpm_probe(struct platform_device *pdev)
 				     "Failed to get match data.\n");
 
 	acpm->shmem = acpm->sram_base + match_data->initdata_base;
+	acpm->mbox_dbg_chan = match_data->mbox_dbg_chan;
 	acpm->dev = dev;
 
 	ret = acpm_channels_init(acpm);
 	if (ret)
 		return ret;
 
+	ret = acpm_debugfs_register(acpm);
+	if (ret)
+		return ret;
+
 	acpm_setup_ops(acpm);
 
 	platform_set_drvdata(pdev, acpm);
@@ -590,6 +598,11 @@ static int acpm_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static void acpm_remove(struct platform_device *pdev)
+{
+	acpm_debugfs_remove();
+}
+
 /**
  * acpm_handle_put() - release the handle acquired by acpm_get_by_phandle.
  * @handle:	Handle acquired by acpm_get_by_phandle.
@@ -698,6 +711,7 @@ const struct acpm_handle *devm_acpm_get_by_phandle(struct device *dev,
 
 static const struct acpm_match_data acpm_gs101 = {
 	.initdata_base = ACPM_GS101_INITDATA_BASE,
+	.mbox_dbg_chan = ACPM_GS101_MBOX_DBG_CHAN,
 };
 
 static const struct of_device_id acpm_match[] = {
@@ -711,6 +725,7 @@ MODULE_DEVICE_TABLE(of, acpm_match);
 
 static struct platform_driver acpm_driver = {
 	.probe	= acpm_probe,
+	.remove = acpm_remove,
 	.driver	= {
 		.name = "exynos-acpm-protocol",
 		.of_match_table	= acpm_match,
diff --git a/drivers/firmware/samsung/exynos-acpm.h b/drivers/firmware/samsung/exynos-acpm.h
index c212fe28758a..d22ffada29a6 100644
--- a/drivers/firmware/samsung/exynos-acpm.h
+++ b/drivers/firmware/samsung/exynos-acpm.h
@@ -19,12 +19,36 @@
  * @chans:	offset to array of struct acpm_chan_shmem.
  * @reserved1:	unused fields.
  * @num_chans:	number of channels.
+ * @reserved2:	unused fields.
+ * @log_rear:	rear pointer of APM log queue.
+ * @log_front:	front pointer of APM log queue.
+ * @log_base:	base address of APM log queue.
+ * @log_mlen:	log message length.
+ * @log_qlen:	log queue length.
+ * @reserved3:	unused fields.
+ * @preempt_log_rear:	rear pointer of APM preempt log queue.
+ * @preempt_log_front:	front pointer of APM preempt log queue.
+ * @preempt_log_base:	base address of APM preempt log queue.
+ * @preempt_log_qlen:	preempt log queue length.
+ * @reserved4:	unused fields.
  */
 struct acpm_shmem {
 	u32 reserved[2];
 	u32 chans;
 	u32 reserved1[3];
 	u32 num_chans;
+	u32 reserved2[6];
+	u32 log_rear;
+	u32 log_front;
+	u32 log_base;
+	u32 log_mlen;
+	u32 log_qlen;
+	u32 reserved3[24];
+	u32 preempt_log_rear;
+	u32 preempt_log_front;
+	u32 preempt_log_base;
+	u32 preempt_log_qlen;
+	u32 reserved4[64];
 };
 
 /**
@@ -41,23 +65,36 @@ struct acpm_queue {
 
 struct device;
 struct acpm_chan;
+struct acpm_log_info;
 
 /**
  * struct acpm_info - driver's private data.
  * @shmem:	pointer to the SRAM configuration data.
  * @sram_base:	base address of SRAM.
+ * @log:	pointer to the ACPM logging info.
  * @chans:	pointer to the ACPM channel parameters retrieved from SRAM.
  * @dev:	pointer to the exynos-acpm device.
  * @handle:	instance of acpm_handle to send to clients.
+ * @mbox_dbg_chan: mailbox debug channel.
  * @num_chans:	number of channels available for this controller.
  */
 struct acpm_info {
 	struct acpm_shmem __iomem *shmem;
 	void __iomem *sram_base;
+	struct acpm_log_info *log;
 	struct acpm_chan *chans;
 	struct device *dev;
 	struct acpm_handle handle;
+	unsigned int mbox_dbg_chan;
 	u32 num_chans;
 };
 
+#ifdef CONFIG_DEBUG_FS
+int acpm_debugfs_register(struct acpm_info *acpm);
+void acpm_debugfs_remove(void);
+#else
+static inline int acpm_debugfs_register(struct acpm_info *acpm) {}
+static inline void acpm_debugfs_remove(void) {}
+#endif
+
 #endif /* __EXYNOS_ACPM_H__ */

-- 
2.48.1.601.g30ceb7b040-goog

Re: [PATCH 3/3] firmware: samsung: add ACPM debugfs support

[Krzysztof Kozlowski]

On 24/02/2025 09:01, Tudor Ambarus wrote:
> The ACPM firmware saves debug information to SRAM. Add debugfs entries
> in order to expose the ACPM logs.
> 
> acpm_framework/logb_gprio_level controls the ACPM print verbosity to
> the SRAM log buffer. It encodes a 64 bit value, 4 bits for each of the
> 16 Plugin IDs, with verbosity levels from 0xf (log error) to
> 0x0 (log debug).
> 
> echo 0xffffffffffffff1f > /sys/kernel/debug/acpm_framework/logb_gprio_level
> Will allow only LOG_ERR prints for all Plugin IDs but Plugin ID 1,
> which will issue prints for any log levels greater or equal to 1.
> On the ACPM firmware side, logb_gprio_level has a default value of zero,
> all logs enabled for all Plugin IDs.
> 
> acpm_framework/log_level has a maximum value of 2 and controls which
> SRAM log buffers are printed.
> 
> Finally, acpm_framework/acpm_debug_cmd provides a way to issue
> ACPM DEBUG commands to the firmware.

Please add something like above also as a comment to the driver, so the
interface will be documented.

> 
> Add ACPM debugfs support with the above capabilities.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> ---


...

> +
> +union acpm_log_entry {
> +	u32 raw[4];
> +	struct {
> +		u32 systicks0 : 24;
> +		u32 dummy : 2;
> +		u32 is_err : 1;
> +		u32 is_raw : 1;
> +		u32 plugin_id : 4;
> +		u32 systicks24;
> +		u32 msg : 24;
> +		u32 systicks56 : 8;
> +		u32 data;
> +	} __packed;
> +};
> +
> +static struct dentry *rootdir;

exynos-apcm.c is not a singleton, so neither should this be. You should
create entries per device (so with device name as subdirectory), just
for correctness.

> +
> +static DEFINE_MUTEX(acpm_log_level_mutex);

And this also looks per-device-instance.

> +
> +static void acpm_log_print_entry(struct acpm_info *acpm,
> +				 const union acpm_log_entry *log_entry)
> +{
> +	u64 systicks, time, msg;
> +
> +	if (log_entry->is_err)
> +		return;
> +
> +	if (log_entry->is_raw) {
> +		dev_info(acpm->dev, "[ACPM_FW raw] : id:%u, %x, %x, %x\n",
> +			 log_entry->plugin_id, log_entry->raw[1],
> +			 log_entry->raw[2], log_entry->raw[3]);
> +	} else {
> +		systicks = ((u64)(log_entry->systicks56) << 56) +
> +			   ((u64)(log_entry->systicks24) << 24) +
> +			   log_entry->systicks0;
> +
> +		/* report time in ns */
> +		time = mul_u64_u32_div(systicks, ACPM_APM_SYSTICK_PERIOD_PS,
> +				       1000);
> +
> +		msg = readl(acpm->sram_base + log_entry->msg);
> +
> +		dev_info(acpm->dev, "[ACPM_FW] : %llu id:%u, %s, %x\n", time,
> +			 log_entry->plugin_id, (char *)&msg, log_entry->data);


I don't think these should be printed to dmesg - these are not system
logs. You either return the contents to the caller's read() on debugfs
entry or, if this is anyhow crashdump related, it goes to
pstore/minidump once triggered. Or to ramoops.

Depends what these logs are (so please also explain what do you find
there in the commit msg).

Maybe something like CHROMEOS_PSTORE?

IOW, if enabled, this should go to ramoops/pstore unconditionally. For
runtime debugging this should be returned somehow to the userspace
reading the file. I think usually debugfs and sysfs is not expected to
provide more than PAGE_SIZE data, so this second part has to be
rethinked still.

> +	}
> +}
> +
> +static void acpm_log_print_entries(struct acpm_info *acpm,
> +				   struct acpm_log_buf *lbuf)
> +{
> +	union acpm_log_entry log_entry = {0};
> +	u32 front, rear;
> +
> +	front = readl(lbuf->q.front);
> +	rear = lbuf->rear_index;
> +
> +	while (rear != front) {
> +		__ioread32_copy(&log_entry, lbuf->q.base + lbuf->mlen * rear,
> +				sizeof(log_entry) / 4);
> +
> +		acpm_log_print_entry(acpm, &log_entry);
> +
> +		if (lbuf->qlen == rear + 1)
> +			rear = 0;
> +		else
> +			rear++;
> +
> +		lbuf->rear_index = rear;
> +		front = readl(lbuf->q.front);
> +	}
> +}
> +
> +static void acpm_log_print(struct acpm_info *acpm)
> +{
> +	struct acpm_log_info *acpm_log = acpm->log;
> +
> +	guard(mutex)(&acpm_log_level_mutex);
> +
> +	if (acpm_log->level == 0)
> +		return;
> +
> +	if (acpm_log->level == ACPM_LOG_LEVEL_MAX)
> +		acpm_log_print_entries(acpm, &acpm_log->preempt);
> +
> +	acpm_log_print_entries(acpm, &acpm_log->normal);
> +}
> +
> +static void acpm_work_fn(struct work_struct *work)
> +{
> +	struct acpm_log_info *acpm_log =
> +		container_of(work, struct acpm_log_info, work.work);
> +	struct acpm_info *acpm = acpm_log->acpm;
> +
> +	acpm_log_print(acpm);
> +
> +	queue_delayed_work(acpm_log->wq, &acpm_log->work,
> +			   msecs_to_jiffies(acpm_log->poll_period));
> +}
> +
> +static int acpm_log_level_get(void *data, u64 *val)
> +{
> +	struct acpm_info *acpm = data;
> +
> +	*val = acpm->log->level;
> +
> +	return 0;
> +}
> +
> +static int acpm_log_level_set(void *data, u64 val)
> +{
> +	struct acpm_info *acpm = data;
> +	struct acpm_log_info *acpm_log = acpm->log;
> +
> +	if (val > ACPM_LOG_LEVEL_MAX) {
> +		dev_err(acpm->dev, "Log level %llu out of range [0:%u]!\n",
> +			val, ACPM_LOG_LEVEL_MAX);
> +		return -EINVAL;
> +	}
> +
> +	scoped_guard(mutex, &acpm_log_level_mutex)
> +		acpm_log->level = val;
> +
> +	if (acpm_log->level == 0)
> +		cancel_delayed_work_sync(&acpm_log->work);
> +	else
> +		queue_delayed_work(acpm_log->wq, &acpm_log->work,
> +				   msecs_to_jiffies(acpm_log->poll_period));
> +	return 0;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(acpm_log_level_fops, acpm_log_level_get,
> +			 acpm_log_level_set, "0%llu\n");

I also do not think debugfs is a knob to control loglevel of messages
going to dmesg.

Best regards,
Krzysztof

Re: [PATCH 3/3] firmware: samsung: add ACPM debugfs support

[Tudor Ambarus]

Hi, Krzysztof,

On 3/5/25 7:37 PM, Krzysztof Kozlowski wrote:
> On 24/02/2025 09:01, Tudor Ambarus wrote:

cut
> Please add something like above also as a comment to the driver, so the
> interface will be documented.

okay, will add.

cut

>> +static struct dentry *rootdir;
> 
> exynos-apcm.c is not a singleton, so neither should this be. You should
> create entries per device (so with device name as subdirectory), just
> for correctness.
> 
>> +
>> +static DEFINE_MUTEX(acpm_log_level_mutex);
> 
> And this also looks per-device-instance.

okay, will create per device instances.

>> +
>> +static void acpm_log_print_entry(struct acpm_info *acpm,
>> +				 const union acpm_log_entry *log_entry)
>> +{

cut

>> +		dev_info(acpm->dev, "[ACPM_FW] : %llu id:%u, %s, %x\n", time,
>> +			 log_entry->plugin_id, (char *)&msg, log_entry->data);
> 
> 
> I don't think these should be printed to dmesg - these are not system
> logs. You either return the contents to the caller's read() on debugfs
> entry or, if this is anyhow crashdump related, it goes to
> pstore/minidump once triggered. Or to ramoops.
> 
> Depends what these logs are (so please also explain what do you find
> there in the commit msg).
> 
> Maybe something like CHROMEOS_PSTORE?
> 
> IOW, if enabled, this should go to ramoops/pstore unconditionally. For
> runtime debugging this should be returned somehow to the userspace
> reading the file. I think usually debugfs and sysfs is not expected to
> provide more than PAGE_SIZE data, so this second part has to be
> rethinked still.
> 

This is a logging feature, it's not oops/panic related. These logs are
referred to as "block logs". A "block" is the start of a mailbox command
to its end, so it logs every ACPM mailbox command issued to the
firmware. After each end of a block, we see the state of all regulators,
frequencies and devices up/down extracted from the block.

These are indeed system logs, and using the dmesg ring buffer seems fine
as we typically care about the recent logs, we don't care if the ring
starts all over again.

>> +DEFINE_DEBUGFS_ATTRIBUTE(acpm_log_level_fops, acpm_log_level_get,
>> +			 acpm_log_level_set, "0%llu\n");
> 
> I also do not think debugfs is a knob to control loglevel of messages
> going to dmesg.
> 

This debugfs entry controls which SRAM log ring buffers are queried -
there are 2 SRAM buffers.

I guess you thought about the "logb_gprio_level" entry, which controls
the ACPM print verbosity. 0xf being ERR, and 0x0 all logs. The firmware
defaults on printing all logs. I can't tell right now other way of
controlling the logs verbosity, maybe via sysfs, but it's equivalent and
with more restrictions. Or maybe you think that I shall always print all
logs?

Thanks,
ta

Re: [PATCH 3/3] firmware: samsung: add ACPM debugfs support

[Krzysztof Kozlowski]

On 12/03/2025 08:11, Tudor Ambarus wrote:
> 
>>> +		dev_info(acpm->dev, "[ACPM_FW] : %llu id:%u, %s, %x\n", time,
>>> +			 log_entry->plugin_id, (char *)&msg, log_entry->data);
>>
>>
>> I don't think these should be printed to dmesg - these are not system
>> logs. You either return the contents to the caller's read() on debugfs
>> entry or, if this is anyhow crashdump related, it goes to
>> pstore/minidump once triggered. Or to ramoops.
>>
>> Depends what these logs are (so please also explain what do you find
>> there in the commit msg).
>>
>> Maybe something like CHROMEOS_PSTORE?
>>
>> IOW, if enabled, this should go to ramoops/pstore unconditionally. For
>> runtime debugging this should be returned somehow to the userspace
>> reading the file. I think usually debugfs and sysfs is not expected to
>> provide more than PAGE_SIZE data, so this second part has to be
>> rethinked still.
>>
> 
> This is a logging feature, it's not oops/panic related. These logs are
> referred to as "block logs". A "block" is the start of a mailbox command
> to its end, so it logs every ACPM mailbox command issued to the
> firmware. After each end of a block, we see the state of all regulators,
> frequencies and devices up/down extracted from the block.
> 
> These are indeed system logs, and using the dmesg ring buffer seems fine
> as we typically care about the recent logs, we don't care if the ring
> starts all over again.

I claim these are not system logs, but firmware logs, thus dmesg ring
buffer is not suitable for them. Where do the other vendors log remoteprocs?


Best regards,
Krzysztof