ASoC: Intel: catpt: Error handling and debug improvements

Cezary Rojewski <cezary.rojewski@intel.com> says:

Outcome of a long debug to solve one, long-standing bug ocurring very
rarely on Haswell/Broadwell machines during the boot procedure of the
AudioDSP firmware.  Clever/unfortunate user can increase the
reproduction rare to 100%.

The bug: an exception occurring early during FW boot (firmware side, not
the software one) leaves the firmware hanging and the existing software
code is incappable of recognizing such problem.  The only solution a
user currently has is: rmmod and then modprobe the driver.

Recently, together with Krzysztof from the firmware team decided to take
it up and clear the dashboard.

The exception handling takes just a few lines of code (all part of the
first patch), everything else that this patchset is composed of improves
the debugability and logging.  If anything similar pops up, the
developers can see what's going on.

Link: https://patch.msgid.link/20260528083444.1439233-1-cezary.rojewski@intel.com

sound/soc/intel/catpt/Makefile

@@ -1,7 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 snd-soc-catpt-y := device.o dsp.o loader.o ipc.o messages.o pcm.o sysfs.o
 
+snd-soc-catpt-y += trace.o
 # tell define_trace.h where to find the trace header
-CFLAGS_device.o := -I$(src)
+CFLAGS_trace.o := -I$(src)
 
 obj-$(CONFIG_SND_SOC_INTEL_CATPT) += snd-soc-catpt.o

sound/soc/intel/catpt/device.c

@@ -25,9 +25,6 @@
 #include "core.h"
 #include "registers.h"
 
-#define CREATE_TRACE_POINTS
-#include "trace.h"
-
 static int catpt_do_suspend(struct device *dev)
 {
 	struct catpt_dev *cdev = dev_get_drvdata(dev);
@@ -157,7 +154,7 @@ static int catpt_register_board(struct catpt_dev *cdev)
 					PLATFORM_DEVID_NONE,
 					(const void *)mach, sizeof(*mach));
 	if (IS_ERR(board)) {
-		dev_err(cdev->dev, "board register failed\n");
+		dev_err(cdev->dev, "register board failed: %ld\n", PTR_ERR(board));
 		return PTR_ERR(board);
 	}
 

sound/soc/intel/catpt/ipc.c

@@ -210,6 +210,7 @@ static void catpt_dsp_process_response(struct catpt_dev *cdev, u32 header)
 		memcpy_fromio(&config, cdev->lpe_ba + off, sizeof(config));
 		trace_catpt_ipc_payload((u8 *)&config, sizeof(config));
 
+		dev_dbg(cdev->dev, "FW READY 0x%08x\n", header);
 		catpt_ipc_arm(ipc, &config);
 		complete(&cdev->fw_ready);
 		return;
@@ -220,6 +221,13 @@ static void catpt_dsp_process_response(struct catpt_dev *cdev, u32 header)
 		dev_err(cdev->dev, "ADSP device coredump received\n");
 		ipc->ready = false;
 		catpt_coredump(cdev);
+
+		if (catpt_readl_dram(cdev, COREDUMP) == CATPT_COREDUMP_REQUEST) {
+			dev_dbg(cdev->dev, "releasing firmware from the coredump state\n");
+			catpt_writel_dram(cdev, COREDUMP, CATPT_COREDUMP_RELEASE);
+		}
+
+		complete(&cdev->fw_ready);
 		/* TODO: attempt recovery */
 		break;
 

sound/soc/intel/catpt/loader.c

@@ -624,6 +624,9 @@ int catpt_boot_firmware(struct catpt_dev *cdev, bool restore)
 	if (!ret) {
 		dev_err(cdev->dev, "firmware ready timeout\n");
 		return -ETIMEDOUT;
+	/* Wake up does not mean FW is ready, an exception could occur. */
+	} else if (!cdev->ipc.ready) {
+		return -EREMOTEIO;
 	}
 
 	/* update sram pg & clock once done booting */

sound/soc/intel/catpt/registers.h

@@ -124,6 +124,11 @@
 #define CATPT_SSCR2_DEFAULT		0x0
 #define CATPT_SSPSP2_DEFAULT		0x0
 
+/* Coredump register and its states */
+#define CATPT_DRAM_COREDUMP		0x1F4
+#define CATPT_COREDUMP_REQUEST		UINT_MAX
+#define CATPT_COREDUMP_RELEASE		0
+
 /* Physically the same block, access address differs between host and dsp */
 #define CATPT_DSP_DRAM_OFFSET		0x400000
 #define catpt_to_host_offset(offset)	((offset) & ~(CATPT_DSP_DRAM_OFFSET))
@@ -137,6 +142,8 @@
 
 /* registry I/O helpers */
 
+#define catpt_dram_addr(cdev) \
+	((cdev)->lpe_ba + (cdev)->spec->host_dram_offset)
 #define catpt_shim_addr(cdev) \
 	((cdev)->lpe_ba + (cdev)->spec->host_shim_offset)
 #define catpt_dma_addr(cdev, dma) \
@@ -151,6 +158,11 @@
 #define catpt_writel_ssp(cdev, ssp, reg, val) \
 	writel(val, catpt_ssp_addr(cdev, ssp) + (reg))
 
+#define catpt_readl_dram(cdev, reg) \
+	readl(catpt_dram_addr(cdev) + CATPT_DRAM_##reg)
+#define catpt_writel_dram(cdev, reg, val) \
+	writel(val, catpt_dram_addr(cdev) + CATPT_DRAM_##reg)
+
 #define catpt_readl_shim(cdev, reg) \
 	readl(catpt_shim_addr(cdev) + CATPT_SHIM_##reg)
 #define catpt_writel_shim(cdev, reg, val) \

sound/soc/intel/catpt/trace.c

@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/types.h>
+
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
+#define BYTES_PER_LINE 16
+#define MAX_CHUNK_SIZE ((PAGE_SIZE - 150) /* Place for trace header */	\
+			/ (2 * BYTES_PER_LINE + 4) /* chars per line */	\
+			* BYTES_PER_LINE)
+
+void trace_catpt_ipc_payload(const void *data, size_t size)
+{
+	size_t remaining = size;
+	size_t offset = 0;
+
+	while (remaining > 0) {
+		u32 chunk;
+
+		chunk = min_t(size_t, remaining, MAX_CHUNK_SIZE);
+		trace_catpt_ipc_payload_chunk(data, chunk, offset, size);
+
+		remaining -= chunk;
+		offset += chunk;
+	}
+}

sound/soc/intel/catpt/trace.h

@@ -51,29 +51,37 @@ DEFINE_EVENT(catpt_ipc_msg, catpt_ipc_notify,
 	TP_ARGS(header)
 );
 
-TRACE_EVENT_CONDITION(catpt_ipc_payload,
+TRACE_EVENT_CONDITION(catpt_ipc_payload_chunk,
 
-	TP_PROTO(const u8 *data, size_t size),
+	TP_PROTO(const u8 *data, size_t size, size_t offset, size_t total),
 
-	TP_ARGS(data, size),
+	TP_ARGS(data, size, offset, total),
 
 	TP_CONDITION(data && size),
 
 	TP_STRUCT__entry(
-		__dynamic_array(u8, buf, size)
+		__dynamic_array(u8,	buf,	size	)
+		__field(size_t,		offset		)
+		__field(size_t,		pos		)
+		__field(size_t,		total		)
 	),
 
 	TP_fast_assign(
-		memcpy(__get_dynamic_array(buf), data, size);
+		memcpy(__get_dynamic_array(buf), data + offset, size);
+		__entry->offset = offset;
+		__entry->pos = offset + size;
+		__entry->total = total;
 	),
 
-	TP_printk("%u byte(s)%s",
-		  __get_dynamic_array_len(buf),
+	TP_printk("range %zu-%zu out of %zu bytes%s",
+		  __entry->offset, __entry->pos, __entry->total,
 		  __print_hex_dump("", DUMP_PREFIX_NONE, 16, 4,
 				   __get_dynamic_array(buf),
 				   __get_dynamic_array_len(buf), false))
 );
 
+void trace_catpt_ipc_payload(const void *data, size_t size);
+
 #endif /* __SND_SOC_INTEL_CATPT_TRACE_H */
 
 /* This part must be outside protection */