[PATCH v3 0/6] ASoC: Intel: catpt: Round of fixes and PM changes

[PATCH v3 0/6] ASoC: Intel: catpt: Round of fixes and PM changes

[Cezary Rojewski]

Set of changes addressing gaps in DRAM offset checks, error paths and
PM.

Three fixes lead the way, all straight-forward. Followed up by a small
refactor to replace manual arithmetic with resource_xxx() API. The last
two relate to the power management.

The standing out PM change is removal of the catpt-driver as a
system-suspend (S3) blocker. This is a suggestion from Andy as indeed,
audio is not a critical component that should prevent the system from
going into S3. Whatever happens, the driver can recover on a follow up
resume (S3 -> S0).


All points below suggested by Andy.

Changes in v3:
- reordered the patches so that all 3 fixes are at the start of the
  patchset and added missing 'Fixes:' tag
- small spacing adjustments in patch 2/6

Changes in v2:
- removed the last patch from v1 "ASoC: Intel: catpt: Drop
  catpt_runtime_resume()"
- added new patch "ASoC: Intel: catpt: Switch to resource_xxx() API"
  which refactors existing code that manupulates .start, .end and size
  of resources to publicly available interface
- patches 1/6 and 3/6 have had their message justified to improve
  readability

Cezary Rojewski (6):
  ASoC: Intel: catpt: Fix error path in hw_params()
  ASoC: Intel: catpt: Fix probing order of driver components
  ASoC: Intel: catpt: Fix offset checks
  ASoC: Intel: catpt: Switch to resource_xxx() API
  ASoC: Intel: catpt: Do not ignore errors on runtime resume
  ASoC: Intel: catpt: Do not block the system from suspending

 sound/soc/intel/catpt/device.c | 26 ++++++++++++++++++--------
 sound/soc/intel/catpt/loader.c | 19 ++++++++++---------
 sound/soc/intel/catpt/pcm.c    | 16 +++++++++-------
 sound/soc/intel/catpt/sysfs.c  |  2 +-
 4 files changed, 38 insertions(+), 25 deletions(-)

-- 
2.25.1

[PATCH v3 1/6] ASoC: Intel: catpt: Fix error path in hw_params()

[Cezary Rojewski]

Do not leave any resources hanging on the DSP side if
applying user settings fails.

Fixes: 768a3a3b327d ("ASoC: Intel: catpt: Optimize applying user settings")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/catpt/pcm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/catpt/pcm.c b/sound/soc/intel/catpt/pcm.c
index f15385683d9c..9baaad13a273 100644
--- a/sound/soc/intel/catpt/pcm.c
+++ b/sound/soc/intel/catpt/pcm.c
@@ -417,8 +417,10 @@ static int catpt_dai_hw_params(struct snd_pcm_substream *substream,
 		return CATPT_IPC_ERROR(ret);
 
 	ret = catpt_dai_apply_usettings(dai, stream);
-	if (ret)
+	if (ret) {
+		catpt_ipc_free_stream(cdev, stream->info.stream_hw_id);
 		return ret;
+	}
 
 	stream->allocated = true;
 	return 0;
-- 
2.25.1

[PATCH v3 2/6] ASoC: Intel: catpt: Fix probing order of driver components

[Cezary Rojewski]

catpt_dai_pcm_new() is called during the bring up sequence of the
machine board device which is a different device to the parent (DSP)
device yet utilizes pm_runtime_xxx() against it in order to send IPCs.
If the parent's pm_runtime is not configured before that happens,
errors will occur.

Fixes: 7a10b66a5df9 ("ASoC: Intel: catpt: Device driver lifecycle")
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/catpt/device.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/sound/soc/intel/catpt/device.c b/sound/soc/intel/catpt/device.c
index faa916f40069..eed330bc82b6 100644
--- a/sound/soc/intel/catpt/device.c
+++ b/sound/soc/intel/catpt/device.c
@@ -184,22 +184,25 @@ static int catpt_probe_components(struct catpt_dev *cdev)
 		goto err_boot_fw;
 	}
 
-	ret = catpt_register_board(cdev);
-	if (ret) {
-		dev_err(cdev->dev, "register board failed: %d\n", ret);
-		goto err_reg_board;
-	}
-
 	/* reflect actual ADSP state in pm_runtime */
 	pm_runtime_set_active(cdev->dev);
 
 	pm_runtime_set_autosuspend_delay(cdev->dev, 2000);
 	pm_runtime_use_autosuspend(cdev->dev);
 	pm_runtime_mark_last_busy(cdev->dev);
+	/* Enable PM before spawning child device. See catpt_dai_pcm_new(). */
 	pm_runtime_enable(cdev->dev);
+
+	ret = catpt_register_board(cdev);
+	if (ret) {
+		dev_err(cdev->dev, "register board failed: %d\n", ret);
+		goto err_reg_board;
+	}
+
 	return 0;
 
 err_reg_board:
+	pm_runtime_disable(cdev->dev);
 	snd_soc_unregister_component(cdev->dev);
 err_boot_fw:
 	catpt_dmac_remove(cdev);
-- 
2.25.1

[PATCH v3 3/6] ASoC: Intel: catpt: Fix offset checks

[Cezary Rojewski]

Verify if the entire block is found within DRAM, not just
the start of it.

Fixes: a9aa6fb3eb6c ("ASoC: Intel: catpt: Firmware loading and context restore")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/catpt/loader.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/intel/catpt/loader.c b/sound/soc/intel/catpt/loader.c
index 696d84314eeb..5804de1d89e3 100644
--- a/sound/soc/intel/catpt/loader.c
+++ b/sound/soc/intel/catpt/loader.c
@@ -216,7 +216,7 @@ static int catpt_restore_memdumps(struct catpt_dev *cdev, struct dma_chan *chan)
 			continue;
 
 		off = catpt_to_host_offset(info->offset);
-		if (off < cdev->dram.start || off > cdev->dram.end)
+		if (off < cdev->dram.start || off + info->size >= cdev->dram.end)
 			continue;
 
 		dev_dbg(cdev->dev, "restoring memdump: off 0x%08x size %d\n",
@@ -261,12 +261,12 @@ static int catpt_restore_fwimage(struct catpt_dev *cdev,
 			continue;
 
 		off = catpt_to_host_offset(info->offset);
-		if (off < cdev->dram.start || off > cdev->dram.end)
-			continue;
-
 		r2.start = off;
 		r2.end = r2.start + info->size - 1;
 
+		if (r2.start < cdev->dram.start || r2.end > cdev->dram.end)
+			continue;
+
 		if (!resource_intersection(&r2, &r1, &common))
 			continue;
 		/* calculate start offset of common data area */
-- 
2.25.1

[PATCH v3 4/6] ASoC: Intel: catpt: Switch to resource_xxx() API

[Cezary Rojewski]

There is a number of interfaces available for manipulating instances of
struct resource. To improve readability, move away from manual editing
in favor of the common interface.

While at it, adjust spacing so that both code blocks, while found in
separate functions, looks cohesive.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/catpt/loader.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/sound/soc/intel/catpt/loader.c b/sound/soc/intel/catpt/loader.c
index 5804de1d89e3..6fd1a32f3863 100644
--- a/sound/soc/intel/catpt/loader.c
+++ b/sound/soc/intel/catpt/loader.c
@@ -208,6 +208,7 @@ static int catpt_restore_memdumps(struct catpt_dev *cdev, struct dma_chan *chan)
 
 	for (i = 0; i < cdev->dx_ctx.num_meminfo; i++) {
 		struct catpt_save_meminfo *info;
+		struct resource r = {};
 		u32 off;
 		int ret;
 
@@ -216,7 +217,8 @@ static int catpt_restore_memdumps(struct catpt_dev *cdev, struct dma_chan *chan)
 			continue;
 
 		off = catpt_to_host_offset(info->offset);
-		if (off < cdev->dram.start || off + info->size >= cdev->dram.end)
+		resource_set_range(&r, off, info->size);
+		if (!resource_contains(&cdev->dram, &r))
 			continue;
 
 		dev_dbg(cdev->dev, "restoring memdump: off 0x%08x size %d\n",
@@ -239,32 +241,31 @@ static int catpt_restore_fwimage(struct catpt_dev *cdev,
 				 struct dma_chan *chan, dma_addr_t paddr,
 				 struct catpt_fw_block_hdr *blk)
 {
-	struct resource r1, r2, common;
+	struct resource r1 = {};
 	int i;
 
 	print_hex_dump_debug(__func__, DUMP_PREFIX_OFFSET, 8, 4,
 			     blk, sizeof(*blk), false);
 
-	r1.start = cdev->dram.start + blk->ram_offset;
-	r1.end = r1.start + blk->size - 1;
+	resource_set_range(&r1, cdev->dram.start + blk->ram_offset, blk->size);
+
 	/* advance to data area */
 	paddr += sizeof(*blk);
 
 	for (i = 0; i < cdev->dx_ctx.num_meminfo; i++) {
 		struct catpt_save_meminfo *info;
+		struct resource common = {};
+		struct resource r2 = {};
 		u32 off;
 		int ret;
 
 		info = &cdev->dx_ctx.meminfo[i];
-
 		if (info->source != CATPT_DX_TYPE_FW_IMAGE)
 			continue;
 
 		off = catpt_to_host_offset(info->offset);
-		r2.start = off;
-		r2.end = r2.start + info->size - 1;
-
-		if (r2.start < cdev->dram.start || r2.end > cdev->dram.end)
+		resource_set_range(&r2, off, info->size);
+		if (!resource_contains(&cdev->dram, &r2))
 			continue;
 
 		if (!resource_intersection(&r2, &r1, &common))
-- 
2.25.1

[PATCH v3 5/6] ASoC: Intel: catpt: Do not ignore errors on runtime resume

[Cezary Rojewski]

If pm_runtime_resume_and_get() fails, follow up pm_runtime_xxx()
operate on device in erroneous state.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/catpt/pcm.c   | 12 ++++++------
 sound/soc/intel/catpt/sysfs.c |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/sound/soc/intel/catpt/pcm.c b/sound/soc/intel/catpt/pcm.c
index 9baaad13a273..abd1cb07c60c 100644
--- a/sound/soc/intel/catpt/pcm.c
+++ b/sound/soc/intel/catpt/pcm.c
@@ -671,7 +671,7 @@ static int catpt_dai_pcm_new(struct snd_soc_pcm_runtime *rtm,
 		return 0;
 
 	ret = pm_runtime_resume_and_get(cdev->dev);
-	if (ret < 0 && ret != -EACCES)
+	if (ret)
 		return ret;
 
 	ret = catpt_ipc_set_device_format(cdev, &devfmt);
@@ -874,7 +874,7 @@ static int catpt_mixer_volume_get(struct snd_kcontrol *kcontrol,
 	int i;
 
 	ret = pm_runtime_resume_and_get(cdev->dev);
-	if (ret < 0 && ret != -EACCES)
+	if (ret)
 		return ret;
 
 	for (i = 0; i < CATPT_CHANNELS_MAX; i++) {
@@ -895,7 +895,7 @@ static int catpt_mixer_volume_put(struct snd_kcontrol *kcontrol,
 	int ret;
 
 	ret = pm_runtime_resume_and_get(cdev->dev);
-	if (ret < 0 && ret != -EACCES)
+	if (ret)
 		return ret;
 
 	ret = catpt_set_dspvol(cdev, cdev->mixer.mixer_hw_id,
@@ -926,7 +926,7 @@ static int catpt_stream_volume_get(struct snd_kcontrol *kcontrol,
 	}
 
 	ret = pm_runtime_resume_and_get(cdev->dev);
-	if (ret < 0 && ret != -EACCES)
+	if (ret)
 		return ret;
 
 	for (i = 0; i < CATPT_CHANNELS_MAX; i++) {
@@ -957,7 +957,7 @@ static int catpt_stream_volume_put(struct snd_kcontrol *kcontrol,
 	}
 
 	ret = pm_runtime_resume_and_get(cdev->dev);
-	if (ret < 0 && ret != -EACCES)
+	if (ret)
 		return ret;
 
 	ret = catpt_set_dspvol(cdev, stream->info.stream_hw_id,
@@ -1033,7 +1033,7 @@ static int catpt_loopback_switch_put(struct snd_kcontrol *kcontrol,
 	}
 
 	ret = pm_runtime_resume_and_get(cdev->dev);
-	if (ret < 0 && ret != -EACCES)
+	if (ret)
 		return ret;
 
 	ret = catpt_ipc_mute_loopback(cdev, stream->info.stream_hw_id, mute);
diff --git a/sound/soc/intel/catpt/sysfs.c b/sound/soc/intel/catpt/sysfs.c
index 048253002ec8..e961e172f9b7 100644
--- a/sound/soc/intel/catpt/sysfs.c
+++ b/sound/soc/intel/catpt/sysfs.c
@@ -16,7 +16,7 @@ static ssize_t fw_version_show(struct device *dev,
 	int ret;
 
 	ret = pm_runtime_resume_and_get(cdev->dev);
-	if (ret < 0 && ret != -EACCES)
+	if (ret)
 		return ret;
 
 	ret = catpt_ipc_get_fw_version(cdev, &version);
-- 
2.25.1

[PATCH v3 6/6] ASoC: Intel: catpt: Do not block the system from suspending

[Cezary Rojewski]

Even if something goes wrong when performing suspend on DSP, from the
system perspective the component is not critical enough to block the
suspend operation entirely. Leaving recovery to next resume() suffices.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/catpt/device.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/catpt/device.c b/sound/soc/intel/catpt/device.c
index eed330bc82b6..d13062c8e907 100644
--- a/sound/soc/intel/catpt/device.c
+++ b/sound/soc/intel/catpt/device.c
@@ -28,7 +28,7 @@
 #define CREATE_TRACE_POINTS
 #include "trace.h"
 
-static int catpt_suspend(struct device *dev)
+static int catpt_do_suspend(struct device *dev)
 {
 	struct catpt_dev *cdev = dev_get_drvdata(dev);
 	struct dma_chan *chan;
@@ -72,6 +72,13 @@ static int catpt_suspend(struct device *dev)
 	return catpt_dsp_power_down(cdev);
 }
 
+/* Do not block the system from suspending, recover on resume() if needed. */
+static int catpt_suspend(struct device *dev)
+{
+	catpt_do_suspend(dev);
+	return 0;
+}
+
 static int catpt_resume(struct device *dev)
 {
 	struct catpt_dev *cdev = dev_get_drvdata(dev);
@@ -114,7 +121,7 @@ static int catpt_runtime_suspend(struct device *dev)
 	}
 	module_put(dev->driver->owner);
 
-	return catpt_suspend(dev);
+	return catpt_do_suspend(dev);
 }
 
 static int catpt_runtime_resume(struct device *dev)
-- 
2.25.1

Re: [PATCH v3 0/6] ASoC: Intel: catpt: Round of fixes and PM changes

[Andy Shevchenko]

On Fri, Nov 28, 2025 at 10:34:06AM +0100, Cezary Rojewski wrote:
> Set of changes addressing gaps in DRAM offset checks, error paths and
> PM.
> 
> Three fixes lead the way, all straight-forward. Followed up by a small
> refactor to replace manual arithmetic with resource_xxx() API. The last
> two relate to the power management.
> 
> The standing out PM change is removal of the catpt-driver as a
> system-suspend (S3) blocker. This is a suggestion from Andy as indeed,
> audio is not a critical component that should prevent the system from
> going into S3. Whatever happens, the driver can recover on a follow up
> resume (S3 -> S0).

v2 had been applied AFAICS.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/6] ASoC: Intel: catpt: Round of fixes and PM changes

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 492 bytes --]

On Fri, Nov 28, 2025 at 12:45:09PM +0200, Andy Shevchenko wrote:
> On Fri, Nov 28, 2025 at 10:34:06AM +0100, Cezary Rojewski wrote:

> > The standing out PM change is removal of the catpt-driver as a
> > system-suspend (S3) blocker. This is a suggestion from Andy as indeed,
> > audio is not a critical component that should prevent the system from
> > going into S3. Whatever happens, the driver can recover on a follow up
> > resume (S3 -> S0).

> v2 had been applied AFAICS.

Yes, it was.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 0/6] ASoC: Intel: catpt: Round of fixes and PM changes

[Cezary Rojewski]

On 2025-11-28 12:47 PM, Mark Brown wrote:
> On Fri, Nov 28, 2025 at 12:45:09PM +0200, Andy Shevchenko wrote:
>> On Fri, Nov 28, 2025 at 10:34:06AM +0100, Cezary Rojewski wrote:
> 
>>> The standing out PM change is removal of the catpt-driver as a
>>> system-suspend (S3) blocker. This is a suggestion from Andy as indeed,
>>> audio is not a critical component that should prevent the system from
>>> going into S3. Whatever happens, the driver can recover on a follow up
>>> resume (S3 -> S0).
> 
>> v2 had been applied AFAICS.
> 
> Yes, it was.

Sorry for the spam then, didn't notice. Nothing major has been changed 
in v3 so it can be ignored.