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

[PATCH v2 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.

The first three patches are straight-forward, the last three 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).


Changes in v2:
  All points below suggested by Andy.
- 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 offset checks
  ASoC: Intel: catpt: Switch to resource_xxx() API
  ASoC: Intel: catpt: Fix error path in hw_params()
  ASoC: Intel: catpt: Fix probing order of driver components
  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 | 18 +++++++++---------
 sound/soc/intel/catpt/pcm.c    | 16 +++++++++-------
 sound/soc/intel/catpt/sysfs.c  |  2 +-
 4 files changed, 37 insertions(+), 25 deletions(-)

-- 
2.25.1

[PATCH v2 1/6] ASoC: Intel: catpt: Fix offset checks

[Cezary Rojewski]

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

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 v2 2/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>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/catpt/loader.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/sound/soc/intel/catpt/loader.c b/sound/soc/intel/catpt/loader.c
index 5804de1d89e3..f5705cd2c1e1 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,30 @@ 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 v2 3/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")
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 v2 4/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.

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 v2 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 v2 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 v2 2/6] ASoC: Intel: catpt: Switch to resource_xxx() API

[Andy Shevchenko]

On Wed, Nov 26, 2025 at 10:55:19AM +0100, Cezary Rojewski wrote:
> 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.

...

> -	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);

+ blank line.

And strictly speaking we have resource_size(&cdev->dram).

>  	/* advance to data area */
>  	paddr += sizeof(*blk);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 4/6] ASoC: Intel: catpt: Fix probing order of driver components

[Andy Shevchenko]

On Wed, Nov 26, 2025 at 10:55:21AM +0100, Cezary Rojewski wrote:
> 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.

Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/6] ASoC: Intel: catpt: Fix error path in hw_params()

[Andy Shevchenko]

On Wed, Nov 26, 2025 at 10:55:20AM +0100, Cezary Rojewski wrote:
> Do not leave any resources hanging on the DSP side if
> applying user settings fails.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/6] ASoC: Intel: catpt: Switch to resource_xxx() API

[Andy Shevchenko]

On Wed, Nov 26, 2025 at 10:55:19AM +0100, Cezary Rojewski wrote:
> 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.

A nit-pick in the other mail, but in general

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/6] ASoC: Intel: catpt: Fix offset checks

[Andy Shevchenko]

On Wed, Nov 26, 2025 at 10:55:18AM +0100, Cezary Rojewski wrote:
> Verify if the entire block is found within DRAM, not just
> the start of it.

Don't we want a Fixes tag?
If not, perhaps make it go after the fixes in the series?

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

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

[Mark Brown]

On Wed, 26 Nov 2025 10:55:17 +0100, Cezary Rojewski wrote:
> Set of changes addressing gaps in DRAM offset checks, error paths and
> PM.
> 
> The first three patches are straight-forward, the last three 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).
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/6] ASoC: Intel: catpt: Fix offset checks
      commit: 1a0ce0a1e6d2e6e03469eb5233e2a571511b1ae1
[2/6] ASoC: Intel: catpt: Switch to resource_xxx() API
      commit: ea38b262a2df7c6f29753565fbe2db8492740f28
[3/6] ASoC: Intel: catpt: Fix error path in hw_params()
      commit: 86a5b621be658fc8fe594ca6db317d64de30cce1
[4/6] ASoC: Intel: catpt: Fix probing order of driver components
      commit: 16e17736282fea02c1a156b242c2dbf35d94e9a2
[5/6] ASoC: Intel: catpt: Do not ignore errors on runtime resume
      commit: 8a342b2be1c84ac205ccd8eb9cc5d8eb5871af47
[6/6] ASoC: Intel: catpt: Do not block the system from suspending
      commit: 56736543b570a1a16fbc4e3be1776a476c9ca14a

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark