* [PATCH 1/7] soundwire: intel: Define stubs when library is disabled
2026-11-02 18:10 [PATCH 0/7] ASoC: Intel: avs: Soundwire support Cezary Rojewski
@ 2026-11-02 18:10 ` Cezary Rojewski
2026-11-02 18:19 ` sashiko-bot
2026-11-02 18:10 ` [PATCH 2/7] soundwire: intel: Allow for dynamic assignment of PDIs Cezary Rojewski
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Cezary Rojewski @ 2026-11-02 18:10 UTC (permalink / raw)
To: linux-sound
Cc: broonie, tiwai, perex, amade, linux-kernel, yung-chuan.liao,
pierre-louis.bossart, vkoul, Cezary Rojewski
The library defines no stubs when it is disabled so the actual users
have to do it locally to allow the code to compilte or, guard the code
that does interact with soundwire with #if(s). Define simple wrappers to
make the life of soundwire-code users easier.
The change allows for a driver to build against SOUNDWIRE_INTEL=n whilst
still calling the stubs in its common code without compilation failing.
The driver needs not to flank its soundwire-code with #if(s) either.
Not all sdw_intel_xxx()s are part of drivers/soundwire. Some definitions
are located within sound/hda/core instead. With the wrappers done,
update Kconfig for sound/hda/core as currently the ACPI-related function
are compiled regardless if SOUNDWIRE_INTEL is selected or not. By being
dependent on SOUNDWIRE_INTEL those functions are compiled only if the
SoundWire was actually requested.
Cc: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
include/linux/soundwire/sdw_intel.h | 34 +++++++++++++++++++++++++++++
sound/hda/core/Kconfig | 2 +-
sound/hda/core/Makefile | 3 +++
3 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h
index 9710f2dc04e2..83021cc0f5b8 100644
--- a/include/linux/soundwire/sdw_intel.h
+++ b/include/linux/soundwire/sdw_intel.h
@@ -358,6 +358,8 @@ struct sdw_intel_res {
struct mutex *eml_lock;
};
+#if IS_ENABLED(CONFIG_SOUNDWIRE_INTEL)
+
/*
* On Intel platforms, the SoundWire IP has dependencies on power
* rails shared with the DSP, and the initialization steps are split
@@ -384,6 +386,38 @@ void sdw_intel_exit(struct sdw_intel_ctx *ctx);
irqreturn_t sdw_intel_thread(int irq, void *dev_id);
+#else
+
+static inline int sdw_intel_acpi_scan(acpi_handle parent_handle, struct sdw_intel_acpi_info *info)
+{
+ return 0;
+}
+
+static inline void sdw_intel_process_wakeen_event(struct sdw_intel_ctx *ctx)
+{
+}
+
+static inline struct sdw_intel_ctx *sdw_intel_probe(struct sdw_intel_res *res)
+{
+ return NULL;
+}
+
+static inline int sdw_intel_startup(struct sdw_intel_ctx *ctx)
+{
+ return 0;
+}
+
+static inline void sdw_intel_exit(struct sdw_intel_ctx *ctx)
+{
+}
+
+static inline irqreturn_t sdw_intel_thread(int irq, void *dev_id)
+{
+ return 0;
+}
+
+#endif
+
#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE BIT(1)
struct sdw_intel;
diff --git a/sound/hda/core/Kconfig b/sound/hda/core/Kconfig
index bfdcf6384c52..f91053dd9f58 100644
--- a/sound/hda/core/Kconfig
+++ b/sound/hda/core/Kconfig
@@ -29,7 +29,7 @@ config SND_INTEL_DSP_CONFIG
tristate
select ACPI_NHLT if ACPI
select SND_INTEL_NHLT if ACPI
- select SND_INTEL_SOUNDWIRE_ACPI if ACPI
+ select SND_INTEL_SOUNDWIRE_ACPI if SOUNDWIRE_INTEL
# this config should be selected only for Intel DSP platforms.
# A fallback is provided so that the code compiles in all cases.
diff --git a/sound/hda/core/Makefile b/sound/hda/core/Makefile
index 89cb46143050..28032ac7bc61 100644
--- a/sound/hda/core/Makefile
+++ b/sound/hda/core/Makefile
@@ -18,5 +18,8 @@ snd-intel-dspcfg-y := intel-dsp-config.o
snd-intel-dspcfg-$(CONFIG_SND_INTEL_NHLT) += intel-nhlt.o
obj-$(CONFIG_SND_INTEL_DSP_CONFIG) += snd-intel-dspcfg.o
+ifneq ($(CONFIG_SND_INTEL_SOUNDWIRE_ACPI),)
snd-intel-sdw-acpi-y := intel-sdw-acpi.o
+endif
+
obj-$(CONFIG_SND_INTEL_SOUNDWIRE_ACPI) += snd-intel-sdw-acpi.o
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 1/7] soundwire: intel: Define stubs when library is disabled
2026-11-02 18:10 ` [PATCH 1/7] soundwire: intel: Define stubs when library is disabled Cezary Rojewski
@ 2026-11-02 18:19 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-11-02 18:19 UTC (permalink / raw)
To: Cezary Rojewski, linux-sound
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] soundwire: intel: link failure when SND_INTEL_DSP_CONFIG=y, SOUNDWIRE_INTEL=m
- [Medium] soundwire: intel: sdw_intel_acpi_scan() stub returns 0 leaving info uninit
--
commit 36be5679dc6f7a17f83bd829bfffe5d60fd5b81f
Author: Cezary Rojewski <cezary.rojewski@intel.com>
soundwire: intel: Define stubs when library is disabled
This commit adds inline stub functions in include/linux/soundwire/sdw_intel.h
when CONFIG_SOUNDWIRE_INTEL is disabled and updates sound/hda/core/Kconfig
and Makefile so SND_INTEL_SOUNDWIRE_ACPI is only selected and built when
SOUNDWIRE_INTEL is enabled.
> diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h
> index 9710f2dc04e29..83021cc0f5b86 100644
> --- a/include/linux/soundwire/sdw_intel.h
> +++ b/include/linux/soundwire/sdw_intel.h
> @@ -358,6 +358,8 @@ struct sdw_intel_res {
> struct mutex *eml_lock;
> };
>
> +#if IS_ENABLED(CONFIG_SOUNDWIRE_INTEL)
> +
> /*
> * On Intel platforms, the SoundWire IP has dependencies on power
> * rails shared with the DSP, and the initialization steps are split
> @@ -384,6 +386,38 @@ void sdw_intel_exit(struct sdw_intel_ctx *ctx);
>
> irqreturn_t sdw_intel_thread(int irq, void *dev_id);
>
> +#else
> +
> +static inline int sdw_intel_acpi_scan(acpi_handle parent_handle, struct sdw_intel_acpi_info *info)
> +{
> + return 0;
> +}
[Severity: Medium]
Should the sdw_intel_acpi_scan() stub return -ENODEV instead of 0?
In sound/hda/core/intel-sdw-acpi.c, sdw_intel_acpi_scan() returns -ENODEV
when no SoundWire controller is found, and only returns 0 after populating
info->handle, info->count, and info->link_mask.
Callers pass an uninitialized struct sdw_intel_acpi_info on the stack and
check for a negative return value before reading its fields, for example in
sound/hda/core/intel-dsp-config.c:snd_intel_dsp_check_soundwire():
struct sdw_intel_acpi_info info;
acpi_handle handle;
int ret;
handle = ACPI_HANDLE(&pci->dev);
if (!handle)
return -ENODEV;
ret = sdw_intel_acpi_scan(handle, &info);
if (ret < 0)
return ret;
return info.link_mask;
With the stub returning 0 when CONFIG_SOUNDWIRE_INTEL=n, callers such as
snd_intel_dsp_check_soundwire(), hda_sdw_acpi_scan() in
sound/soc/sof/intel/hda.c, and avs_sdw_probe() in
sound/soc/intel/avs/soundwire.c will read uninitialized stack memory from
info and treat a SoundWire controller as present, causing subsequent
sdw_intel_probe() calls to return NULL and fail probing with -EINVAL.
[ ... ]
> diff --git a/sound/hda/core/Kconfig b/sound/hda/core/Kconfig
> index bfdcf6384c521..f91053dd9f589 100644
> --- a/sound/hda/core/Kconfig
> +++ b/sound/hda/core/Kconfig
> @@ -29,7 +29,7 @@ config SND_INTEL_DSP_CONFIG
> tristate
> select ACPI_NHLT if ACPI
> select SND_INTEL_NHLT if ACPI
> - select SND_INTEL_SOUNDWIRE_ACPI if ACPI
> + select SND_INTEL_SOUNDWIRE_ACPI if SOUNDWIRE_INTEL
[Severity: Medium]
Can this cause a vmlinux link failure when CONFIG_SND_INTEL_DSP_CONFIG=y
and CONFIG_SOUNDWIRE_INTEL=m?
Because SOUNDWIRE_INTEL is a tristate symbol, "select
SND_INTEL_SOUNDWIRE_ACPI if SOUNDWIRE_INTEL" bounds SND_INTEL_SOUNDWIRE_ACPI
to the minimum of SND_INTEL_DSP_CONFIG and SOUNDWIRE_INTEL. When a built-in
driver selects SND_INTEL_DSP_CONFIG=y (such as CONFIG_SND_HDA_INTEL=y) while
CONFIG_SOUNDWIRE_INTEL=m and CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE=m,
CONFIG_SND_INTEL_SOUNDWIRE_ACPI is built as a loadable module (=m).
In sound/hda/core/intel-dsp-config.c (built-in via SND_INTEL_DSP_CONFIG=y),
IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE) and
IS_ENABLED(CONFIG_SOUNDWIRE_INTEL) both evaluate to true for =m:
sound/hda/core/intel-dsp-config.c:snd_intel_dsp_check_soundwire() {
...
ret = sdw_intel_acpi_scan(handle, &info);
This causes built-in code in snd-intel-dspcfg to reference
sdw_intel_acpi_scan() in the loadable module snd-intel-sdw-acpi.ko. Should
this use a boolean condition instead, matching sound/soc/sof/intel/Kconfig
and sound/soc/intel/Kconfig:
select SND_INTEL_SOUNDWIRE_ACPI if SOUNDWIRE_INTEL != n
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261102181042.1922005-1-cezary.rojewski@intel.com?part=1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/7] soundwire: intel: Allow for dynamic assignment of PDIs
2026-11-02 18:10 [PATCH 0/7] ASoC: Intel: avs: Soundwire support Cezary Rojewski
2026-11-02 18:10 ` [PATCH 1/7] soundwire: intel: Define stubs when library is disabled Cezary Rojewski
@ 2026-11-02 18:10 ` Cezary Rojewski
2026-11-02 18:22 ` sashiko-bot
2026-11-02 18:10 ` [PATCH 3/7] soundwire: intel: Split PDIs and DAIs initialization Cezary Rojewski
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Cezary Rojewski @ 2026-11-02 18:10 UTC (permalink / raw)
To: linux-sound
Cc: broonie, tiwai, perex, amade, linux-kernel, yung-chuan.liao,
pierre-louis.bossart, vkoul, Cezary Rojewski
Currently, PDIs are assigned upfront based on the DAI IDs. Some drivers
do not assign anything upfront and request a free PDI when streaming is
about to start.
Provide assign/release() functions to allow them to be assigned
dynamically. This also streamlines the process of assigning/releasing
PDIs with what is done in HDAudio library (sound/hda) for HDAudio
streams, see snd_hdac_stream_assign/release().
As resource availability-check is typically done during dai->startup()
whilst hardware/firmware configuration occurs on dai->hw_params(), add
sdw_intel_pdi_config() to maintain this logical split - assign on
start(), configure on hw_params(). Existing code does all of that in
intel_hw_params() so the logical split is not possible.
Cc: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
drivers/soundwire/cadence_master.c | 64 +++++++++++++++++++++++++++
drivers/soundwire/cadence_master.h | 24 ++--------
drivers/soundwire/intel.c | 16 +++++++
include/linux/soundwire/sdw_cadence.h | 35 +++++++++++++++
include/linux/soundwire/sdw_intel.h | 7 +++
5 files changed, 125 insertions(+), 21 deletions(-)
create mode 100644 include/linux/soundwire/sdw_cadence.h
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index a0a63b76793e..9375db9c08a2 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -17,6 +17,7 @@
#include <linux/pm_runtime.h>
#include <linux/soundwire/sdw_registers.h>
#include <linux/soundwire/sdw.h>
+#include <linux/soundwire/sdw_cadence.h>
#include <sound/pcm_params.h>
#include <sound/soc.h>
#include <linux/workqueue.h>
@@ -1835,6 +1836,7 @@ int sdw_cdns_probe(struct sdw_cdns *cdns)
mutex_init(&cdns->status_update_lock);
+ spin_lock_init(&cdns->pdi_lock);
INIT_WORK(&cdns->work, cdns_update_slave_status_work);
INIT_DELAYED_WORK(&cdns->attach_dwork, cdns_check_attached_status_dwork);
@@ -1991,6 +1993,68 @@ struct sdw_cdns_pdi *sdw_cdns_alloc_pdi(struct sdw_cdns *cdns,
}
EXPORT_SYMBOL(sdw_cdns_alloc_pdi);
+/**
+ * sdw_cdns_assign_pdi - Assign a PDI
+ * @bus: SoundWire bus instance
+ * @dir: Data direction
+ *
+ * Return: A valid PDI instance or %NULL if none available.
+ */
+struct sdw_cdns_pdi *sdw_cdns_assign_pdi(struct sdw_bus *bus, u32 dir)
+{
+ struct sdw_cdns *cdns = bus_to_cdns(bus);
+ struct sdw_cdns_streams *stream = &cdns->pcm;
+ struct sdw_cdns_pdi *pdis, *pdi;
+ size_t num_pdis;
+ int i;
+
+ if (dir == SDW_DATA_DIR_RX) {
+ num_pdis = stream->num_in;
+ pdis = stream->in;
+ } else {
+ num_pdis = stream->num_out;
+ pdis = stream->out;
+ }
+
+ spin_lock(&cdns->pdi_lock);
+ for (i = 0; i < num_pdis; i++) {
+ if (!pdis[i].assigned) {
+ pdi = &pdis[i];
+ goto found;
+ }
+ }
+
+ /* Indexes 0/1 are reserved for the Bulk transfer. */
+ for (i = 2; i < stream->num_bd; i++) {
+ if (!stream->bd[i].assigned) {
+ pdi = &stream->bd[i];
+ goto found;
+ }
+ }
+
+ spin_unlock(&cdns->pdi_lock);
+ return NULL;
+found:
+ pdi->dir = dir;
+ pdi->assigned = true;
+ spin_unlock(&cdns->pdi_lock);
+ return pdi;
+}
+EXPORT_SYMBOL(sdw_cdns_assign_pdi);
+
+/**
+ * sdw_cdns_release_pdi() - Release a PDI
+ * @pdi: PDI instance
+ */
+void sdw_cdns_release_pdi(struct sdw_bus *bus, struct sdw_cdns_pdi *pdi)
+{
+ struct sdw_cdns *cdns = bus_to_cdns(bus);
+
+ guard(spinlock)(&cdns->pdi_lock);
+ pdi->assigned = false;
+}
+EXPORT_SYMBOL(sdw_cdns_release_pdi);
+
/*
* the MIPI SoundWire CRC8 polynomial is X^8 + X^6 + X^3 + X^2 + 1, MSB first
* The value is (1)01001101 = 0x4D
diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h
index f4e41a9ab165..f52c9cbec8a9 100644
--- a/drivers/soundwire/cadence_master.h
+++ b/drivers/soundwire/cadence_master.h
@@ -6,6 +6,8 @@
#ifndef __SDW_CADENCE_H
#define __SDW_CADENCE_H
+#include <linux/soundwire/sdw_cadence.h>
+
#define SDW_CADENCE_GSYNC_KHZ 4 /* 4 kHz */
#define SDW_CADENCE_GSYNC_HZ (SDW_CADENCE_GSYNC_KHZ * 1000)
@@ -17,27 +19,6 @@
#define SDW_CADENCE_MCP_IP_OFFSET 0x4000
-/**
- * struct sdw_cdns_pdi: PDI (Physical Data Interface) instance
- *
- * @num: pdi number
- * @intel_alh_id: link identifier
- * @l_ch_num: low channel for PDI
- * @h_ch_num: high channel for PDI
- * @ch_count: total channel count for PDI
- * @dir: data direction
- * @type: stream type, (only PCM supported)
- */
-struct sdw_cdns_pdi {
- int num;
- int intel_alh_id;
- int l_ch_num;
- int h_ch_num;
- int ch_count;
- enum sdw_data_direction dir;
- enum sdw_stream_type type;
-};
-
/**
* struct sdw_cdns_streams: Cadence stream data structure
*
@@ -140,6 +121,7 @@ struct sdw_cdns {
int num_ports;
struct sdw_cdns_streams pcm;
+ spinlock_t pdi_lock;
int pdi_loopback_source;
int pdi_loopback_target;
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index af65214836b4..d53fa3d299bc 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -674,6 +674,22 @@ intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi)
intel_writel(alh, SDW_ALH_STRMZCFG(pdi->intel_alh_id), conf);
}
+/**
+ * sdw_intel_pdi_config - Configure specified PDI
+ * @bus: SoundWire bus instance
+ * @pdi: PDI instance to configure
+ */
+void sdw_intel_pdi_config(struct sdw_bus *bus, struct sdw_cdns_pdi *pdi)
+{
+ struct sdw_cdns *cdns = bus_to_cdns(bus);
+ struct sdw_intel *sdw = cdns_to_intel(cdns);
+
+ intel_pdi_shim_configure(sdw, pdi);
+ intel_pdi_alh_configure(sdw, pdi);
+ sdw_cdns_config_stream(cdns, pdi->ch_count, pdi->dir, pdi);
+}
+EXPORT_SYMBOL(sdw_intel_pdi_config);
+
static int intel_params_stream(struct sdw_intel *sdw,
struct snd_pcm_substream *substream,
struct snd_soc_dai *dai,
diff --git a/include/linux/soundwire/sdw_cadence.h b/include/linux/soundwire/sdw_cadence.h
new file mode 100644
index 000000000000..331b21adafc4
--- /dev/null
+++ b/include/linux/soundwire/sdw_cadence.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/* Copyright(c) 2024-2026 Intel Corporation */
+
+#ifndef ___SDW_CADENCE_H
+#define ___SDW_CADENCE_H
+
+#include <linux/soundwire/sdw.h>
+
+/**
+ * struct sdw_cdns_pdi: PDI (Physical Data Interface) instance
+ *
+ * @num: pdi number
+ * @intel_alh_id: link identifier
+ * @l_ch_num: low channel for PDI
+ * @h_ch_num: high channel for PDI
+ * @ch_count: total channel count for PDI
+ * @dir: data direction
+ * @type: stream type, (only PCM supported)
+ * @assigned: whether PDI is in use
+ */
+struct sdw_cdns_pdi {
+ int num;
+ int intel_alh_id;
+ int l_ch_num;
+ int h_ch_num;
+ int ch_count;
+ enum sdw_data_direction dir;
+ enum sdw_stream_type type;
+ bool assigned;
+};
+
+struct sdw_cdns_pdi *sdw_cdns_assign_pdi(struct sdw_bus *bus, u32 dir);
+void sdw_cdns_release_pdi(struct sdw_bus *bus, struct sdw_cdns_pdi *pdi);
+
+#endif
diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h
index 83021cc0f5b8..120f16b8d478 100644
--- a/include/linux/soundwire/sdw_intel.h
+++ b/include/linux/soundwire/sdw_intel.h
@@ -7,6 +7,7 @@
#include <linux/acpi.h>
#include <linux/irqreturn.h>
#include <linux/soundwire/sdw.h>
+#include <linux/soundwire/sdw_cadence.h>
/*********************************************************************
* cAVS and ACE1.x definitions
@@ -386,6 +387,8 @@ void sdw_intel_exit(struct sdw_intel_ctx *ctx);
irqreturn_t sdw_intel_thread(int irq, void *dev_id);
+void sdw_intel_pdi_config(struct sdw_bus *bus, struct sdw_cdns_pdi *pdi);
+
#else
static inline int sdw_intel_acpi_scan(acpi_handle parent_handle, struct sdw_intel_acpi_info *info)
@@ -416,6 +419,10 @@ static inline irqreturn_t sdw_intel_thread(int irq, void *dev_id)
return 0;
}
+static inline void sdw_intel_pdi_config(struct sdw_bus *bus, struct sdw_cdns_pdi *pdi)
+{
+}
+
#endif
#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE BIT(1)
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/7] soundwire: intel: Allow for dynamic assignment of PDIs
2026-11-02 18:10 ` [PATCH 2/7] soundwire: intel: Allow for dynamic assignment of PDIs Cezary Rojewski
@ 2026-11-02 18:22 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-11-02 18:22 UTC (permalink / raw)
To: Cezary Rojewski, linux-sound
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] soundwire: cadence: missing kernel-doc for sdw_cdns_release_pdi() and pdi_lock
--
commit 7d3c13df44bfa3764481f78c64186b31510a9ae2
Author: Cezary Rojewski <cezary.rojewski@intel.com>
soundwire: intel: Allow for dynamic assignment of PDIs
This commit adds sdw_cdns_assign_pdi() and sdw_cdns_release_pdi() to allow
dynamic assignment and release of Cadence PDIs, along with
sdw_intel_pdi_config() to separate PDI assignment during startup from
hardware configuration during hw_params.
> diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
> index a0a63b76793e2..9375db9c08a2d 100644
> --- a/drivers/soundwire/cadence_master.c
> +++ b/drivers/soundwire/cadence_master.c
[ ... ]
> @@ -1991,6 +1993,68 @@ struct sdw_cdns_pdi *sdw_cdns_alloc_pdi(struct sdw_cdns *cdns,
[ ... ]
> +/**
> + * sdw_cdns_release_pdi() - Release a PDI
> + * @pdi: PDI instance
> + */
> +void sdw_cdns_release_pdi(struct sdw_bus *bus, struct sdw_cdns_pdi *pdi)
> +{
> + struct sdw_cdns *cdns = bus_to_cdns(bus);
> +
> + guard(spinlock)(&cdns->pdi_lock);
> + pdi->assigned = false;
> +}
> +EXPORT_SYMBOL(sdw_cdns_release_pdi);
[ ... ]
> diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h
> index f4e41a9ab1658..f52c9cbec8a91 100644
> --- a/drivers/soundwire/cadence_master.h
> +++ b/drivers/soundwire/cadence_master.h
[ ... ]
> @@ -140,6 +121,7 @@ struct sdw_cdns {
> int num_ports;
>
> struct sdw_cdns_streams pcm;
> + spinlock_t pdi_lock;
[Severity: Low]
Should the kernel-doc comments be updated for these additions to avoid W=1
build warnings? Specifically, sdw_cdns_release_pdi() in
drivers/soundwire/cadence_master.c omits the @bus parameter from its
kernel-doc header, and struct sdw_cdns in drivers/soundwire/cadence_master.h
is missing a kernel-doc entry for @pdi_lock.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261102181042.1922005-1-cezary.rojewski@intel.com?part=2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/7] soundwire: intel: Split PDIs and DAIs initialization
2026-11-02 18:10 [PATCH 0/7] ASoC: Intel: avs: Soundwire support Cezary Rojewski
2026-11-02 18:10 ` [PATCH 1/7] soundwire: intel: Define stubs when library is disabled Cezary Rojewski
2026-11-02 18:10 ` [PATCH 2/7] soundwire: intel: Allow for dynamic assignment of PDIs Cezary Rojewski
@ 2026-11-02 18:10 ` Cezary Rojewski
2026-11-02 18:14 ` sashiko-bot
2026-11-02 18:10 ` [PATCH 4/7] ASoC: Intel: avs: Pass irq to dsp_interrupt callback Cezary Rojewski
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Cezary Rojewski @ 2026-11-02 18:10 UTC (permalink / raw)
To: linux-sound
Cc: broonie, tiwai, perex, amade, linux-kernel, yung-chuan.liao,
pierre-louis.bossart, vkoul, Cezary Rojewski
Not every client of the soundwire-driver needs the secondary
soc-component and full range of DAIs immediately when initializing the
device. As function intel_register_dai() does all of that on top of
initializing PDIs, split it into two logical blocks to accommodate for
both approaches. In consequence, register_dai() callback becomes
optional.
To keep things in status quo for existing users, update
intel_link_startup() to call sdw_intel_init_pdis().
Cc: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
drivers/soundwire/intel.c | 24 +++++++++++++++---------
drivers/soundwire/intel.h | 3 ++-
drivers/soundwire/intel_auxdevice.c | 4 ++++
3 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index d53fa3d299bc..708503373183 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1027,23 +1027,29 @@ static int intel_create_dai(struct sdw_cdns *cdns,
return 0;
}
+int sdw_intel_init_pdis(struct sdw_intel *sdw)
+{
+ struct sdw_cdns_stream_config config;
+ int ret;
+
+ /* Read the PDI config and initialize cadence PDI */
+ intel_pdi_init(sdw, &config);
+ ret = sdw_cdns_pdi_init(&sdw->cdns, config);
+ if (ret)
+ return ret;
+
+ intel_pdi_stream_ch_update(sdw, &sdw->cdns.pcm);
+ return 0;
+}
+
static int intel_register_dai(struct sdw_intel *sdw)
{
struct sdw_cdns_dai_runtime **dai_runtime_array;
- struct sdw_cdns_stream_config config;
struct sdw_cdns *cdns = &sdw->cdns;
struct sdw_cdns_streams *stream;
struct snd_soc_dai_driver *dais;
int num_dai, ret, off = 0;
- /* Read the PDI config and initialize cadence PDI */
- intel_pdi_init(sdw, &config);
- ret = sdw_cdns_pdi_init(cdns, config);
- if (ret)
- return ret;
-
- intel_pdi_stream_ch_update(sdw, &sdw->cdns.pcm);
-
/* DAIs are created based on total number of PDIs supported */
num_dai = cdns->pcm.num_pdi;
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index 7a2e7e73ad63..12416cef8224 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -158,7 +158,7 @@ static inline int sdw_intel_register_dai(struct sdw_intel *sdw)
{
if (SDW_INTEL_CHECK_OPS(sdw, register_dai))
return SDW_INTEL_OPS(sdw, register_dai)(sdw);
- return -ENOTSUPP;
+ return 0;
}
static inline void sdw_intel_check_clock_stop(struct sdw_intel *sdw)
@@ -262,6 +262,7 @@ int intel_start_bus_after_reset(struct sdw_intel *sdw);
void intel_check_clock_stop(struct sdw_intel *sdw);
int intel_start_bus_after_clock_stop(struct sdw_intel *sdw);
int intel_stop_bus(struct sdw_intel *sdw, bool clock_stop, bool wake_enable);
+int sdw_intel_init_pdis(struct sdw_intel *sdw);
/* common bank switch routines */
int intel_pre_bank_switch(struct sdw_intel *sdw);
diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c
index 5983fbc5725a..114dadedfa14 100644
--- a/drivers/soundwire/intel_auxdevice.c
+++ b/drivers/soundwire/intel_auxdevice.c
@@ -423,6 +423,10 @@ int intel_link_startup(struct auxiliary_device *auxdev)
if (ret)
goto err_init;
+ ret = sdw_intel_init_pdis(sdw);
+ if (ret)
+ goto err_init;
+
/* Register DAIs */
ret = sdw_intel_register_dai(sdw);
if (ret) {
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 3/7] soundwire: intel: Split PDIs and DAIs initialization
2026-11-02 18:10 ` [PATCH 3/7] soundwire: intel: Split PDIs and DAIs initialization Cezary Rojewski
@ 2026-11-02 18:14 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-11-02 18:14 UTC (permalink / raw)
To: Cezary Rojewski, linux-sound
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] soundwire: intel: sdw_intel_init_pdis() breaks PDI init on ACE2.x
- [Medium] soundwire: intel: missing link power down on error in intel_link_startup()
--
commit 8870f97b0ab1ec823a91e4c81168130f5f4ee09d
Author: Cezary Rojewski <cezary.rojewski@intel.com>
soundwire: intel: Split PDIs and DAIs initialization
This commit splits PDI initialization out of intel_register_dai() in
drivers/soundwire/intel.c into a new sdw_intel_init_pdis() helper and calls
it from intel_link_startup() so that the register_dai() callback can be
optional.
> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> index d53fa3d299bc7..708503373183f 100644
> --- a/drivers/soundwire/intel.c
> +++ b/drivers/soundwire/intel.c
> @@ -1027,22 +1027,28 @@ static int intel_create_dai(struct sdw_cdns *cdns,
> return 0;
> }
>
> -static int intel_register_dai(struct sdw_intel *sdw)
> +int sdw_intel_init_pdis(struct sdw_intel *sdw)
> {
> - struct sdw_cdns_dai_runtime **dai_runtime_array;
> struct sdw_cdns_stream_config config;
> - struct sdw_cdns *cdns = &sdw->cdns;
> - struct sdw_cdns_streams *stream;
> - struct snd_soc_dai_driver *dais;
> - int num_dai, ret, off = 0;
> + int ret;
>
> /* Read the PDI config and initialize cadence PDI */
> intel_pdi_init(sdw, &config);
> - ret = sdw_cdns_pdi_init(cdns, config);
> + ret = sdw_cdns_pdi_init(&sdw->cdns, config);
> if (ret)
> return ret;
>
> intel_pdi_stream_ch_update(sdw, &sdw->cdns.pcm);
> + return 0;
> +}
[Severity: High]
Does calling sdw_intel_init_pdis() unconditionally from intel_link_startup()
break PDI initialization on ACE2.x+ platforms?
intel_link_startup() in drivers/soundwire/intel_auxdevice.c is shared between
cAVS/ACE1.x (sdw_intel_cnl_hw_ops in drivers/soundwire/intel.c) and ACE2.x+
(sdw_intel_lnl_hw_ops in drivers/soundwire/intel_ace2x.c).
However, sdw_intel_init_pdis() is implemented in drivers/soundwire/intel.c
and calls its file-local intel_pdi_init() and intel_pdi_stream_ch_update()
helpers, which access the cAVS/ACE1.x SHIM layout at SDW_SHIM_PCMSCAP(link_id)
and SDW_SHIM_PCMSYCHC(link_id, pdi_num). On ACE2.x+, sdw->link_res->shim
points to the per-link SDW_SHIM2_GENERIC_BASE(link_id), where the PDI
registers are instead at SDW_SHIM2_PCMSCAP and SDW_SHIM2_PCMSYCHC(pdi_num).
Meanwhile, intel_register_dai() in drivers/soundwire/intel_ace2x.c still has
its own PDI initialization:
drivers/soundwire/intel_ace2x.c:intel_register_dai() {
...
/* Read the PDI config and initialize cadence PDI */
intel_pdi_init(sdw, &config);
ret = sdw_cdns_pdi_init(cdns, config);
if (ret)
return ret;
intel_pdi_stream_ch_update(sdw, &sdw->cdns.pcm);
...
}
On ACE2.x+, intel_link_startup() will first run the cAVS/ACE1.x
sdw_intel_init_pdis() against the SHIM2 MMIO space, reading bogus PDI counts
and allocating cdns->pcm.{bd,in,out}, and then sdw_intel_register_dai() will
call sdw_cdns_pdi_init() a second time (leaking the first devm_kcalloc() PDI
allocations and leaving stale pointers in cdns->pcm.in or cdns->pcm.out if a
stream count was non-zero in the bogus read and zero in the SHIM2 read, since
cdns_allocate_pdi() returns early when num == 0). Furthermore, if the now
optional register_dai callback is omitted on ACE2.x+, the SHIM2 PDI
initialization will never run at all.
Should init_pdis be dispatched through struct sdw_intel_hw_ops and split out
of intel_register_dai() in drivers/soundwire/intel_ace2x.c as well?
[ ... ]
> diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c
> index 5983fbc5725a2..114dadedfa14f 100644
> --- a/drivers/soundwire/intel_auxdevice.c
> +++ b/drivers/soundwire/intel_auxdevice.c
> @@ -423,6 +423,10 @@ int intel_link_startup(struct auxiliary_device *auxdev)
> if (ret)
> goto err_init;
>
> + ret = sdw_intel_init_pdis(sdw);
> + if (ret)
> + goto err_init;
[Severity: Medium]
If sdw_intel_init_pdis() fails here in intel_link_startup(), should this jump
to err_power_up instead of err_init?
At this point sdw_intel_link_power_up(sdw) has already succeeded:
drivers/soundwire/intel_auxdevice.c:intel_link_startup() {
...
/* Initialize shim, controller */
ret = sdw_intel_link_power_up(sdw);
if (ret)
goto err_init;
ret = sdw_intel_init_pdis(sdw);
if (ret)
goto err_init;
...
err_power_up:
sdw_intel_link_power_down(sdw);
err_init:
return ret;
}
Jumping to err_init skips sdw_intel_link_power_down(sdw), leaving the link
powered up, sdw->cdns.link_up set to true, and BIT(link_id) set in
*sdw->link_res->shim_mask while sdw->startup_done remains false.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261102181042.1922005-1-cezary.rojewski@intel.com?part=3
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/7] ASoC: Intel: avs: Pass irq to dsp_interrupt callback
2026-11-02 18:10 [PATCH 0/7] ASoC: Intel: avs: Soundwire support Cezary Rojewski
` (2 preceding siblings ...)
2026-11-02 18:10 ` [PATCH 3/7] soundwire: intel: Split PDIs and DAIs initialization Cezary Rojewski
@ 2026-11-02 18:10 ` Cezary Rojewski
2026-11-02 18:10 ` [PATCH 5/7] ASoC: Intel: avs: SoundWire support Cezary Rojewski
` (2 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Cezary Rojewski @ 2026-11-02 18:10 UTC (permalink / raw)
To: linux-sound
Cc: broonie, tiwai, perex, amade, linux-kernel, yung-chuan.liao,
pierre-louis.bossart, vkoul, Cezary Rojewski
Soundwire IRQ thread function requires passing irq as one of the
arguments, as we will want to call it in dsp_interrupt callback, it
needs to be passed throguh.
Signed-off-by: Amadeusz Sławiński <amade@asmblr.net>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/intel/avs/apl.c | 2 +-
sound/soc/intel/avs/avs.h | 6 +++---
sound/soc/intel/avs/cnl.c | 2 +-
sound/soc/intel/avs/core.c | 2 +-
sound/soc/intel/avs/mtl.c | 2 +-
sound/soc/intel/avs/skl.c | 2 +-
6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/soc/intel/avs/apl.c b/sound/soc/intel/avs/apl.c
index cf600e1c986e..3abcdf062035 100644
--- a/sound/soc/intel/avs/apl.c
+++ b/sound/soc/intel/avs/apl.c
@@ -17,7 +17,7 @@
#include "registers.h"
#include "topology.h"
-static irqreturn_t avs_apl_dsp_interrupt(struct avs_dev *adev)
+static irqreturn_t avs_apl_dsp_interrupt(struct avs_dev *adev, int irq)
{
u32 adspis = snd_hdac_adsp_readl(adev, AVS_ADSP_REG_ADSPIS);
irqreturn_t ret = IRQ_NONE;
diff --git a/sound/soc/intel/avs/avs.h b/sound/soc/intel/avs/avs.h
index b4f9d66d55ad..b3af2c4126be 100644
--- a/sound/soc/intel/avs/avs.h
+++ b/sound/soc/intel/avs/avs.h
@@ -45,7 +45,7 @@ struct avs_dsp_ops {
int (* const power)(struct avs_dev *, u32, bool);
int (* const reset)(struct avs_dev *, u32, bool);
int (* const stall)(struct avs_dev *, u32, bool);
- irqreturn_t (* const dsp_interrupt)(struct avs_dev *);
+ irqreturn_t (* const dsp_interrupt)(struct avs_dev *, int irq);
void (* const int_control)(struct avs_dev *, bool);
int (* const load_basefw)(struct avs_dev *, struct firmware *);
int (* const load_lib)(struct avs_dev *, struct firmware *, u32);
@@ -273,8 +273,8 @@ int avs_mtl_core_stall(struct avs_dev *adev, u32 core_mask, bool stall);
int avs_lnl_core_stall(struct avs_dev *adev, u32 core_mask, bool stall);
void avs_mtl_interrupt_control(struct avs_dev *adev, bool enable);
void avs_skl_ipc_interrupt(struct avs_dev *adev);
-irqreturn_t avs_cnl_dsp_interrupt(struct avs_dev *adev);
-irqreturn_t avs_mtl_dsp_interrupt(struct avs_dev *adev);
+irqreturn_t avs_cnl_dsp_interrupt(struct avs_dev *adev, int irq);
+irqreturn_t avs_mtl_dsp_interrupt(struct avs_dev *adev, int irq);
int avs_apl_enable_logs(struct avs_dev *adev, enum avs_log_enable enable, u32 aging_period,
u32 fifo_full_period, unsigned long resource_mask, u32 *priorities);
int avs_icl_enable_logs(struct avs_dev *adev, enum avs_log_enable enable, u32 aging_period,
diff --git a/sound/soc/intel/avs/cnl.c b/sound/soc/intel/avs/cnl.c
index 5b5359e9128b..31fd9f251d6c 100644
--- a/sound/soc/intel/avs/cnl.c
+++ b/sound/soc/intel/avs/cnl.c
@@ -58,7 +58,7 @@ static void avs_cnl_ipc_interrupt(struct avs_dev *adev)
AVS_ADSP_HIPCCTL_DONE | AVS_ADSP_HIPCCTL_BUSY);
}
-irqreturn_t avs_cnl_dsp_interrupt(struct avs_dev *adev)
+irqreturn_t avs_cnl_dsp_interrupt(struct avs_dev *adev, int irq)
{
u32 adspis = snd_hdac_adsp_readl(adev, AVS_ADSP_REG_ADSPIS);
irqreturn_t ret = IRQ_NONE;
diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
index 611ae9f034d4..9413027e1e32 100644
--- a/sound/soc/intel/avs/core.c
+++ b/sound/soc/intel/avs/core.c
@@ -344,7 +344,7 @@ static irqreturn_t avs_dsp_irq_thread(int irq, void *dev_id)
status = readl(bus->ppcap + AZX_REG_PP_PPSTS);
if (status & AZX_PPCTL_PIE)
- avs_dsp_op(adev, dsp_interrupt);
+ avs_dsp_op(adev, dsp_interrupt, irq);
/* Unmask GIE, masked in irq_handler(). */
snd_hdac_chip_updatel(bus, INTCTL, AZX_INT_GLOBAL_EN, AZX_INT_GLOBAL_EN);
diff --git a/sound/soc/intel/avs/mtl.c b/sound/soc/intel/avs/mtl.c
index d8bdd03275d7..29f79d6ff0d4 100644
--- a/sound/soc/intel/avs/mtl.c
+++ b/sound/soc/intel/avs/mtl.c
@@ -166,7 +166,7 @@ static void avs_mtl_ipc_interrupt(struct avs_dev *adev)
AVS_ADSP_HIPCCTL_DONE | AVS_ADSP_HIPCCTL_BUSY);
}
-irqreturn_t avs_mtl_dsp_interrupt(struct avs_dev *adev)
+irqreturn_t avs_mtl_dsp_interrupt(struct avs_dev *adev, int irq)
{
u32 adspis = snd_hdac_adsp_readl(adev, MTL_DWICTL_REG_FINALSTATUSL);
irqreturn_t ret = IRQ_NONE;
diff --git a/sound/soc/intel/avs/skl.c b/sound/soc/intel/avs/skl.c
index 8fb86f364ff3..54c883658cca 100644
--- a/sound/soc/intel/avs/skl.c
+++ b/sound/soc/intel/avs/skl.c
@@ -54,7 +54,7 @@ void avs_skl_ipc_interrupt(struct avs_dev *adev)
AVS_ADSP_HIPCCTL_DONE | AVS_ADSP_HIPCCTL_BUSY);
}
-static irqreturn_t avs_skl_dsp_interrupt(struct avs_dev *adev)
+static irqreturn_t avs_skl_dsp_interrupt(struct avs_dev *adev, int irq)
{
u32 adspis = snd_hdac_adsp_readl(adev, AVS_ADSP_REG_ADSPIS);
irqreturn_t ret = IRQ_NONE;
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 5/7] ASoC: Intel: avs: SoundWire support
2026-11-02 18:10 [PATCH 0/7] ASoC: Intel: avs: Soundwire support Cezary Rojewski
` (3 preceding siblings ...)
2026-11-02 18:10 ` [PATCH 4/7] ASoC: Intel: avs: Pass irq to dsp_interrupt callback Cezary Rojewski
@ 2026-11-02 18:10 ` Cezary Rojewski
2026-11-02 18:29 ` sashiko-bot
2026-11-02 18:10 ` [PATCH 6/7] ASoC: Intel: avs: Expose common PCM functions Cezary Rojewski
2026-11-02 18:10 ` [PATCH 7/7] ASoC: Intel: avs: SoundWire PCM BE operations Cezary Rojewski
6 siblings, 1 reply; 16+ messages in thread
From: Cezary Rojewski @ 2026-11-02 18:10 UTC (permalink / raw)
To: linux-sound
Cc: broonie, tiwai, perex, amade, linux-kernel, yung-chuan.liao,
pierre-louis.bossart, vkoul, Cezary Rojewski
The SoundWire device is part of the Intel HDAudio PCI device. To support
transfer over SoundWire interface it has to be accounted for when
probing the parent device.
Most of the actual job is done by the SoundWire library which is already
part of the kernel. Utilize it in the core functions of the driver to
enumerate SoundWire component and service its interrupts.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/intel/Kconfig | 1 +
sound/soc/intel/avs/Makefile | 4 ++
sound/soc/intel/avs/avs.h | 1 +
sound/soc/intel/avs/cnl.c | 23 ++++++-
sound/soc/intel/avs/core.c | 22 +++++++
sound/soc/intel/avs/loader.c | 3 +
sound/soc/intel/avs/registers.h | 4 ++
sound/soc/intel/avs/soundwire.c | 112 ++++++++++++++++++++++++++++++++
sound/soc/intel/avs/soundwire.h | 56 ++++++++++++++++
9 files changed, 224 insertions(+), 2 deletions(-)
create mode 100644 sound/soc/intel/avs/soundwire.c
create mode 100644 sound/soc/intel/avs/soundwire.h
diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 63367364916a..4e453b55fc04 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -106,6 +106,7 @@ config SND_SOC_INTEL_AVS
select SND_HDA_EXT_CORE
select SND_HDA_DSP_LOADER
select SND_INTEL_DSP_CONFIG
+ select SND_INTEL_SOUNDWIRE_ACPI if SOUNDWIRE_INTEL != n
select WANT_DEV_COREDUMP
help
Enable support for Intel(R) cAVS 1.5 platforms with DSP
diff --git a/sound/soc/intel/avs/Makefile b/sound/soc/intel/avs/Makefile
index 576dc0da381d..1d48953c7a31 100644
--- a/sound/soc/intel/avs/Makefile
+++ b/sound/soc/intel/avs/Makefile
@@ -14,6 +14,10 @@ ifneq ($(CONFIG_DEBUG_FS),)
snd-soc-avs-y += probes.o debugfs.o
endif
+ifneq ($(CONFIG_SOUNDWIRE_INTEL),)
+snd-soc-avs-y += soundwire.o
+endif
+
obj-$(CONFIG_SND_SOC_INTEL_AVS) += snd-soc-avs.o
# Machine support
diff --git a/sound/soc/intel/avs/avs.h b/sound/soc/intel/avs/avs.h
index b3af2c4126be..40e65639adb2 100644
--- a/sound/soc/intel/avs/avs.h
+++ b/sound/soc/intel/avs/avs.h
@@ -136,6 +136,7 @@ struct avs_dev {
void __iomem *dsp_ba;
const struct avs_spec *spec;
struct avs_ipc *ipc;
+ struct sdw_intel_ctx *sdw;
struct avs_fw_cfg fw_cfg;
struct avs_hw_cfg hw_cfg;
diff --git a/sound/soc/intel/avs/cnl.c b/sound/soc/intel/avs/cnl.c
index 31fd9f251d6c..7090b91572d8 100644
--- a/sound/soc/intel/avs/cnl.c
+++ b/sound/soc/intel/avs/cnl.c
@@ -6,11 +6,16 @@
// Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
//
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/soundwire/sdw.h>
+#include <linux/soundwire/sdw_intel.h>
#include <sound/hdaudio_ext.h>
#include "avs.h"
#include "debug.h"
#include "messages.h"
#include "registers.h"
+#include "soundwire.h"
static void avs_cnl_ipc_interrupt(struct avs_dev *adev)
{
@@ -61,16 +66,30 @@ static void avs_cnl_ipc_interrupt(struct avs_dev *adev)
irqreturn_t avs_cnl_dsp_interrupt(struct avs_dev *adev, int irq)
{
u32 adspis = snd_hdac_adsp_readl(adev, AVS_ADSP_REG_ADSPIS);
+ u32 adspis2 = snd_hdac_adsp_readl(adev, AVS_ADSP_REG_ADSPIS2);
irqreturn_t ret = IRQ_NONE;
- if (adspis == UINT_MAX)
+ if (adspis == UINT_MAX && adspis2 == UINT_MAX)
return ret;
- if (adspis & AVS_ADSP_ADSPIS_IPC) {
+ if (adspis != UINT_MAX && (adspis & AVS_ADSP_ADSPIS_IPC)) {
avs_cnl_ipc_interrupt(adev);
ret = IRQ_HANDLED;
}
+ if (adev->sdw) {
+ u32 wakests;
+
+ if (adspis2 != UINT_MAX && (adspis2 & AVS_ADSP_ADSPIS2_SNDW))
+ ret = avs_sdw_thread(irq, adev);
+
+ wakests = snd_hdac_adsp_readl(adev, adev->sdw->shim_base + SDW_SHIM_WAKESTS);
+ if (wakests) {
+ avs_sdw_process_wakeen_event(adev);
+ ret = IRQ_HANDLED;
+ }
+ }
+
return ret;
}
diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
index 9413027e1e32..f9f25f651a9e 100644
--- a/sound/soc/intel/avs/core.c
+++ b/sound/soc/intel/avs/core.c
@@ -31,6 +31,7 @@
#include "debug.h"
#include "messages.h"
#include "pcm.h"
+#include "soundwire.h"
static u32 pgctl_mask = AZX_PGCTL_LSRMD_MASK;
module_param(pgctl_mask, uint, 0444);
@@ -230,6 +231,11 @@ static void avs_hda_probe_work(struct work_struct *work)
list_for_each_entry(hlink, &bus->hlink_list, list)
snd_hdac_ext_bus_link_put(bus, hlink);
+ /* Soundwire component is optional */
+ ret = avs_sdw_probe(adev);
+ if (ret && ret != -ENODEV)
+ dev_err(adev->dev, "failed to probe sdw: %d\n", ret);
+
snd_hdac_ext_bus_ppcap_enable(bus, true);
snd_hdac_ext_bus_ppcap_int_enable(bus, true);
avs_debugfs_init(adev);
@@ -240,6 +246,14 @@ static void avs_hda_probe_work(struct work_struct *work)
acpi_nhlt_get_gbl_table();
+ ret = avs_sdw_startup(adev);
+ if (ret) {
+ dev_err(adev->dev, "failed to startup sdw: %d\n", ret);
+ return;
+ }
+
+ avs_sdw_enable_irq(adev, true);
+
avs_register_all_boards(adev);
/* configure PM */
@@ -561,6 +575,8 @@ static void avs_pci_shutdown(struct pci_dev *pci)
snd_hdac_bus_stop_chip(bus);
snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, false);
+ avs_sdw_enable_irq(adev, false);
+
pci_free_irq(pci, 0, adev);
pci_free_irq(pci, 0, bus);
pci_free_irq_vectors(pci);
@@ -587,6 +603,9 @@ static void avs_pci_remove(struct pci_dev *pci)
avs_dsp_op(adev, int_control, false);
snd_hdac_ext_bus_ppcap_int_enable(bus, false);
+ avs_sdw_enable_irq(adev, false);
+ avs_sdw_exit(adev);
+
/* it is safe to remove all codecs from the system now */
list_for_each_entry_safe(hdev, save, &bus->codec_list, list)
snd_hda_codec_unregister(hdac_to_hda_codec(hdev));
@@ -661,6 +680,7 @@ static int avs_suspend_common(struct avs_dev *adev, bool low_power)
}
avs_ipc_block(adev->ipc);
+ avs_sdw_enable_irq(adev, false);
avs_dsp_op(adev, int_control, false);
snd_hdac_ext_bus_ppcap_int_enable(bus, false);
@@ -720,6 +740,8 @@ static int avs_resume_common(struct avs_dev *adev, bool low_power, bool purge)
return ret;
}
+ avs_sdw_enable_irq(adev, true);
+
return 0;
}
diff --git a/sound/soc/intel/avs/loader.c b/sound/soc/intel/avs/loader.c
index bebdc79ec88e..27d09a3bb5ef 100644
--- a/sound/soc/intel/avs/loader.c
+++ b/sound/soc/intel/avs/loader.c
@@ -18,6 +18,7 @@
#include "messages.h"
#include "registers.h"
#include "topology.h"
+#include "soundwire.h"
#define AVS_ROM_STS_MASK 0xFF
#define AVS_ROM_INIT_DONE 0x1
@@ -345,6 +346,8 @@ static int avs_imr_load_basefw(struct avs_dev *adev)
if (ret < 0)
return ret;
+ avs_sdw_process_wakeen_event(adev);
+
ret = wait_for_completion_timeout(&adev->fw_ready,
msecs_to_jiffies(AVS_FW_INIT_TIMEOUT_MS));
if (!ret) {
diff --git a/sound/soc/intel/avs/registers.h b/sound/soc/intel/avs/registers.h
index 97767882ffa1..9e403cfb04fa 100644
--- a/sound/soc/intel/avs/registers.h
+++ b/sound/soc/intel/avs/registers.h
@@ -25,11 +25,15 @@
#define AVS_ADSP_REG_ADSPCS (AVS_ADSP_GEN_BASE + 0x04)
#define AVS_ADSP_REG_ADSPIC (AVS_ADSP_GEN_BASE + 0x08)
#define AVS_ADSP_REG_ADSPIS (AVS_ADSP_GEN_BASE + 0x0C)
+#define AVS_ADSP_REG_ADSPIC2 (AVS_ADSP_GEN_BASE + 0x10)
+#define AVS_ADSP_REG_ADSPIS2 (AVS_ADSP_GEN_BASE + 0x14)
#define AVS_ADSP_ADSPIC_IPC BIT(0)
#define AVS_ADSP_ADSPIC_CLDMA BIT(1)
#define AVS_ADSP_ADSPIS_IPC BIT(0)
#define AVS_ADSP_ADSPIS_CLDMA BIT(1)
+#define AVS_ADSP_ADSPIC2_SNDW BIT(5)
+#define AVS_ADSP_ADSPIS2_SNDW BIT(5)
#define AVS_ADSPCS_CRST_MASK(cm) (cm)
#define AVS_ADSPCS_CSTALL_MASK(cm) ((cm) << 8)
diff --git a/sound/soc/intel/avs/soundwire.c b/sound/soc/intel/avs/soundwire.c
new file mode 100644
index 000000000000..c2a6db08a3af
--- /dev/null
+++ b/sound/soc/intel/avs/soundwire.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Copyright(c) 2021-2026 Intel Corporation
+//
+// Authors: Cezary Rojewski <cezary.rojewski@intel.com>
+// Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
+//
+
+#include <linux/acpi.h>
+#include <linux/soundwire/sdw.h>
+#include <linux/soundwire/sdw_intel.h>
+#include <sound/hdaudio_ext.h>
+#include "avs.h"
+#include "registers.h"
+#include "soundwire.h"
+
+static struct sdw_intel_hw_ops avs_sdw_cnl_ops;
+
+int avs_sdw_probe(struct avs_dev *adev)
+{
+ struct hdac_bus *hbus = &adev->base.core;
+ struct sdw_intel_acpi_info info;
+ struct sdw_intel_ctx *ctx;
+ struct sdw_intel_res res;
+ int ret;
+
+ ret = sdw_intel_acpi_scan(ACPI_HANDLE(adev->dev), &info);
+ if (ret < 0)
+ return -ENODEV;
+
+ memset(&res, 0, sizeof(res));
+ res.mmio_base = adev->dsp_ba;
+ res.shim_base = SDW_SHIM_BASE;
+ res.alh_base = SDW_ALH_BASE;
+ res.ext = false;
+ res.irq = adev->base.core.irq;
+ res.handle = info.handle;
+ res.parent = adev->dev;
+ res.dev = adev->dev;
+ res.clock_stop_quirks = SDW_INTEL_CLK_STOP_BUS_RESET;
+ res.hbus = hbus;
+
+ memcpy(&avs_sdw_cnl_ops, &sdw_intel_cnl_hw_ops, sizeof(avs_sdw_cnl_ops));
+ avs_sdw_cnl_ops.register_dai = NULL;
+ res.hw_ops = &avs_sdw_cnl_ops;
+
+ /* we could filter links here if needed, e.g.: for quirks */
+ res.count = info.count;
+ res.link_mask = info.link_mask;
+
+ ctx = sdw_intel_probe(&res);
+ if (!ctx) {
+ dev_err(adev->dev, "SoundWire probe failed\n");
+ return -EINVAL;
+ }
+
+ adev->sdw = ctx;
+ return 0;
+}
+
+int avs_sdw_startup(struct avs_dev *adev)
+{
+ if (adev->sdw)
+ return sdw_intel_startup(adev->sdw);
+ return 0;
+}
+
+void avs_sdw_exit(struct avs_dev *adev)
+{
+ if (adev->sdw) {
+ sdw_intel_exit(adev->sdw);
+ adev->sdw = NULL;
+ }
+}
+
+// TODO split tgl vs mtl
+static void sdw_intel_enable_irq(void __iomem *mmio_base, bool enable)
+{
+ u32 val;
+
+ val = readl(mmio_base + AVS_ADSP_REG_ADSPIC2);
+
+ if (enable)
+ val |= AVS_ADSP_ADSPIC2_SNDW;
+ else
+ val &= ~AVS_ADSP_ADSPIC2_SNDW;
+
+ writel(val, mmio_base + AVS_ADSP_REG_ADSPIC2);
+}
+
+void avs_sdw_enable_irq(struct avs_dev *adev, bool enable)
+{
+ if (adev->sdw)
+ sdw_intel_enable_irq(adev->dsp_ba, enable);
+}
+
+void avs_sdw_process_wakeen_event(struct avs_dev *adev)
+{
+ if (adev->sdw)
+ sdw_intel_process_wakeen_event(adev->sdw);
+}
+
+irqreturn_t avs_sdw_thread(int irq, struct avs_dev *adev)
+{
+ if (adev->sdw)
+ return sdw_intel_thread(irq, adev->sdw);
+ return IRQ_HANDLED;
+}
+
+MODULE_IMPORT_NS("SOUNDWIRE_INTEL");
+MODULE_IMPORT_NS("SOUNDWIRE_INTEL_INIT");
+MODULE_IMPORT_NS("SND_INTEL_SOUNDWIRE_ACPI");
diff --git a/sound/soc/intel/avs/soundwire.h b/sound/soc/intel/avs/soundwire.h
new file mode 100644
index 000000000000..96ce29ab21a3
--- /dev/null
+++ b/sound/soc/intel/avs/soundwire.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright(c) 2021-2026 Intel Corporation
+ *
+ * Authors: Cezary Rojewski <cezary.rojewski@intel.com>
+ * Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
+ */
+
+#ifndef __SOUND_SOC_INTEL_AVS_SOUNDWIRE_H
+#define __SOUND_SOC_INTEL_AVS_SOUNDWIRE_H
+
+#include <linux/irqreturn.h>
+
+struct avs_dev;
+
+#if IS_ENABLED(CONFIG_SOUNDWIRE_INTEL)
+
+int avs_sdw_probe(struct avs_dev *adev);
+int avs_sdw_startup(struct avs_dev *adev);
+void avs_sdw_exit(struct avs_dev *adev);
+void avs_sdw_enable_irq(struct avs_dev *adev, bool enable);
+void avs_sdw_process_wakeen_event(struct avs_dev *adev);
+irqreturn_t avs_sdw_thread(int irq, struct avs_dev *adev);
+
+#else
+
+static inline int avs_sdw_probe(struct avs_dev *adev)
+{
+ return 0;
+}
+
+static inline int avs_sdw_startup(struct avs_dev *adev)
+{
+ return 0;
+}
+
+static inline void avs_sdw_exit(struct avs_dev *adev)
+{
+}
+
+static inline void avs_sdw_enable_irq(struct avs_dev *adev, bool enable)
+{
+}
+
+static inline void avs_sdw_process_wakeen_event(struct avs_dev *adev)
+{
+}
+
+static inline irqreturn_t avs_sdw_thread(int irq, struct avs_dev *adev)
+{
+ return IRQ_HANDLED;
+}
+
+#endif
+
+#endif /* __SOUND_SOC_INTEL_AVS_SOUNDWIRE_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 5/7] ASoC: Intel: avs: SoundWire support
2026-11-02 18:10 ` [PATCH 5/7] ASoC: Intel: avs: SoundWire support Cezary Rojewski
@ 2026-11-02 18:29 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-11-02 18:29 UTC (permalink / raw)
To: Cezary Rojewski, linux-sound
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] ASoC: Intel: avs: premature IRQ disable and UAF in avs_sdw_exit()
- [Medium] ASoC: Intel: avs: disabling IRQs before avs_sdw_exit() in avs_pci_remove() breaks child resume
- [Medium] ASoC: Intel: avs: missing link_mask check in avs_sdw_probe()
- [Medium] ASoC: Intel: avs: cAVS SoundWire MMIO offsets used on ACE platforms
- [Low] ASoC: Intel: avs: vmlinux link failure when SND_SOC_INTEL_AVS=y and SOUNDWIRE_INTEL=m
--
commit 99a0d9e68cf08e7f2ab6024f81ba2c7db0e51ac2
Author: Cezary Rojewski <cezary.rojewski@intel.com>
ASoC: Intel: avs: SoundWire support
This commit adds SoundWire enumeration, startup, interrupt handling, and
teardown support to the Intel AVS driver using the SoundWire Intel library.
> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
> index 63367364916ae..4e453b55fc043 100644
> --- a/sound/soc/intel/Kconfig
> +++ b/sound/soc/intel/Kconfig
> @@ -106,6 +106,7 @@ config SND_SOC_INTEL_AVS
> select SND_HDA_EXT_CORE
> select SND_HDA_DSP_LOADER
> select SND_INTEL_DSP_CONFIG
> + select SND_INTEL_SOUNDWIRE_ACPI if SOUNDWIRE_INTEL != n
> select WANT_DEV_COREDUMP
> help
> Enable support for Intel(R) cAVS 1.5 platforms with DSP
> diff --git a/sound/soc/intel/avs/Makefile b/sound/soc/intel/avs/Makefile
> index 576dc0da381d8..1d48953c7a31c 100644
> --- a/sound/soc/intel/avs/Makefile
> +++ b/sound/soc/intel/avs/Makefile
> @@ -14,6 +14,10 @@ ifneq ($(CONFIG_DEBUG_FS),)
> snd-soc-avs-y += probes.o debugfs.o
> endif
>
> +ifneq ($(CONFIG_SOUNDWIRE_INTEL),)
> +snd-soc-avs-y += soundwire.o
> +endif
[Severity: Low]
Will vmlinux fail to link when CONFIG_SND_SOC_INTEL_AVS=y and
CONFIG_SOUNDWIRE_INTEL=m?
In that configuration, sound/soc/intel/avs/Makefile compiles soundwire.o into
the built-in snd-soc-avs.o, and sound/soc/intel/avs/soundwire.h enables the
external declarations under IS_ENABLED(CONFIG_SOUNDWIRE_INTEL). However,
avs_sdw_probe(), avs_sdw_startup(), avs_sdw_exit(),
avs_sdw_process_wakeen_event(), and avs_sdw_thread() in
sound/soc/intel/avs/soundwire.c call sdw_intel_cnl_hw_ops, sdw_intel_probe(),
sdw_intel_startup(), sdw_intel_exit(), sdw_intel_process_wakeen_event(), and
sdw_intel_thread(), which live in the loadable soundwire-intel.ko module.
Should SND_SOC_INTEL_AVS depend on SOUNDWIRE_INTEL || !SOUNDWIRE_INTEL (or
use IS_REACHABLE), similar to how other ASoC drivers handle optional
tristate dependencies?
[ ... ]
> diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
> index 9413027e1e322..f9f25f651a9e2 100644
> --- a/sound/soc/intel/avs/core.c
> +++ b/sound/soc/intel/avs/core.c
[ ... ]
> @@ -587,6 +603,9 @@ static void avs_pci_remove(struct pci_dev *pci)
> avs_dsp_op(adev, int_control, false);
> snd_hdac_ext_bus_ppcap_int_enable(bus, false);
>
> + avs_sdw_enable_irq(adev, false);
> + avs_sdw_exit(adev);
[Severity: Medium]
In avs_pci_remove(), won't disabling interrupts before calling avs_sdw_exit()
cause SoundWire child device resume to time out during driver removal?
Before avs_sdw_exit(adev) is called, avs_pci_remove() runs:
snd_hdac_stop_streams_and_chip(bus);
avs_dsp_op(adev, int_control, false);
snd_hdac_ext_bus_ppcap_int_enable(bus, false);
avs_sdw_enable_irq(adev, false);
avs_sdw_exit(adev);
Inside avs_sdw_exit() -> sdw_intel_exit(), the SoundWire teardown iterates
over ctx->link_list and synchronously runtime-resumes all child peripheral
devices and links via device_for_each_child(bus->dev, NULL,
intel_resume_child_device) before cleaning them up.
Because avs_sdw_probe() sets res.clock_stop_quirks =
SDW_INTEL_CLK_STOP_BUS_RESET, resuming a runtime-suspended link and its
peripherals restarts the bus via sdw_intel_start_bus_after_reset(), waits for
peripheral attachment, and performs Cadence MCP register transfers in
cdns_xfer_msg() waiting on cdns->tx_complete. Those completions and status
updates depend on sdw_cdns_irq() running from avs_dsp_irq_thread() ->
avs_cnl_dsp_interrupt() -> avs_sdw_thread(), which cannot fire once HDA,
PPCTL, and AVS_ADSP_ADSPIC2_SNDW interrupts have been disabled.
[ ... ]
> diff --git a/sound/soc/intel/avs/soundwire.c b/sound/soc/intel/avs/soundwire.c
> new file mode 100644
> index 0000000000000..c2a6db08a3afb
> --- /dev/null
> +++ b/sound/soc/intel/avs/soundwire.c
> @@ -0,0 +1,112 @@
[ ... ]
> +int avs_sdw_probe(struct avs_dev *adev)
> +{
> + struct hdac_bus *hbus = &adev->base.core;
> + struct sdw_intel_acpi_info info;
> + struct sdw_intel_ctx *ctx;
> + struct sdw_intel_res res;
> + int ret;
> +
> + ret = sdw_intel_acpi_scan(ACPI_HANDLE(adev->dev), &info);
> + if (ret < 0)
> + return -ENODEV;
[Severity: Medium]
Should avs_sdw_probe() also check if (!info.link_mask) before proceeding?
In sdw_intel_acpi_scan() -> sdw_intel_scan_controller(), when the ACPI
controller node is present with a valid master count but all links are
disabled in firmware or masked via sdw_link_mask, sdw_intel_acpi_scan()
returns 0 with info.link_mask == 0.
Without checking !info.link_mask, avs_sdw_probe() passes res.count > 0 and
res.link_mask == 0 to sdw_intel_probe(), which allocates and returns a
non-NULL struct sdw_intel_ctx with zero active links. That leaves adev->sdw
non-NULL with the SoundWire SHIM unpowered, causing avs_sdw_enable_irq() to
unmask AVS_ADSP_ADSPIC2_SNDW and avs_cnl_dsp_interrupt() to read
SDW_SHIM_WAKESTS (returning 0xffffffff when unpowered and falsely reporting
IRQ_HANDLED) on every DSP interrupt.
> +
> + memset(&res, 0, sizeof(res));
> + res.mmio_base = adev->dsp_ba;
> + res.shim_base = SDW_SHIM_BASE;
> + res.alh_base = SDW_ALH_BASE;
> + res.ext = false;
> + res.irq = adev->base.core.irq;
> + res.handle = info.handle;
> + res.parent = adev->dev;
> + res.dev = adev->dev;
> + res.clock_stop_quirks = SDW_INTEL_CLK_STOP_BUS_RESET;
> + res.hbus = hbus;
> +
> + memcpy(&avs_sdw_cnl_ops, &sdw_intel_cnl_hw_ops, sizeof(avs_sdw_cnl_ops));
> + avs_sdw_cnl_ops.register_dai = NULL;
> + res.hw_ops = &avs_sdw_cnl_ops;
[Severity: Medium]
What happens when avs_hda_probe_work() calls avs_sdw_probe(),
avs_sdw_startup(), and avs_sdw_enable_irq() on ACE platforms such as
fcl_desc (which sets AVS_PLATATTR_ACE)?
Here avs_sdw_probe() unconditionally sets cAVS 1.8/2.5 BAR4 offsets
(res.mmio_base = adev->dsp_ba, res.shim_base = SDW_SHIM_BASE, res.alh_base =
SDW_ALH_BASE, res.ext = false) and sdw_intel_cnl_hw_ops, and
sdw_intel_enable_irq() unconditionally accesses AVS_ADSP_REG_ADSPIC2 at
dsp_ba + 0x10. Because sdw_intel_cnl_hw_ops.get_link_count is NULL,
sdw_intel_probe() succeeds without touching hardware if SoundWire is present
in ACPI, and subsequent calls to avs_sdw_startup() and avs_sdw_enable_irq()
will read and write cAVS BAR4 offsets (0x2c000, 0x30000, and 0x10) instead of
the ACE multi-link and MTL_DWICTL_BASE / MTL_HfINT_BASE registers.
Should avs_sdw_probe() guard against avs_platattr_test(adev, ACE) until ACE
SoundWire support is wired up?
[ ... ]
> +void avs_sdw_exit(struct avs_dev *adev)
> +{
> + if (adev->sdw) {
> + sdw_intel_exit(adev->sdw);
> + adev->sdw = NULL;
> + }
> +}
[Severity: High]
Can avs_cnl_dsp_interrupt() race with avs_sdw_exit() and dereference
adev->sdw after sdw_intel_exit() has freed it?
In avs_pci_remove(), pci_free_irq(pci, 0, adev) is called after
avs_sdw_exit(adev), and in avs_sdw_exit(), sdw_intel_exit(adev->sdw) frees
ctx->peripherals, ctx->ldev, and ctx before adev->sdw is cleared to NULL.
Meanwhile, avs_cnl_dsp_interrupt() checks if (adev->sdw) without
synchronization and accesses adev->sdw->shim_base, adev->sdw->link_list (via
avs_sdw_thread() -> sdw_intel_thread()), and adev->sdw->ldev (via
avs_sdw_process_wakeen_event() -> sdw_intel_process_wakeen_event()).
Even if MMIO interrupt bits are cleared beforehand, an already-running
avs_dsp_irq_thread() is not synchronized until pci_free_irq(), and if
interrupts remain enabled during sdw_intel_exit() so child devices can
runtime-resume, avs_cnl_dsp_interrupt() can run concurrently while
sdw_intel_exit() frees adev->sdw.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261102181042.1922005-1-cezary.rojewski@intel.com?part=5
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 6/7] ASoC: Intel: avs: Expose common PCM functions
2026-11-02 18:10 [PATCH 0/7] ASoC: Intel: avs: Soundwire support Cezary Rojewski
` (4 preceding siblings ...)
2026-11-02 18:10 ` [PATCH 5/7] ASoC: Intel: avs: SoundWire support Cezary Rojewski
@ 2026-11-02 18:10 ` Cezary Rojewski
2026-11-02 18:11 ` sashiko-bot
2026-11-02 18:18 ` Amadeusz Sławiński
2026-11-02 18:10 ` [PATCH 7/7] ASoC: Intel: avs: SoundWire PCM BE operations Cezary Rojewski
6 siblings, 2 replies; 16+ messages in thread
From: Cezary Rojewski @ 2026-11-02 18:10 UTC (permalink / raw)
To: linux-sound
Cc: broonie, tiwai, perex, amade, linux-kernel, yung-chuan.liao,
pierre-louis.bossart, vkoul, Cezary Rojewski
All the PCM operations are currently bundled together within the pcm.c
file regardless of which transfer type they cover. To allow for
splitting them, provide a header that exposes the common operations
outside.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/intel/avs/avs.h | 8 -------
sound/soc/intel/avs/pcm.c | 42 +++++++++------------------------
sound/soc/intel/avs/pcm.h | 49 +++++++++++++++++++++++++++++++++++++++
3 files changed, 60 insertions(+), 39 deletions(-)
diff --git a/sound/soc/intel/avs/avs.h b/sound/soc/intel/avs/avs.h
index 40e65639adb2..1b2f1e7d0556 100644
--- a/sound/soc/intel/avs/avs.h
+++ b/sound/soc/intel/avs/avs.h
@@ -347,14 +347,6 @@ struct avs_soc_component {
extern const struct snd_soc_dai_ops avs_dai_fe_ops;
-int avs_register_dmic_component(struct avs_dev *adev, const char *name);
-int avs_register_i2s_component(struct avs_dev *adev, const char *name, unsigned long port_mask,
- unsigned long *tdms);
-int avs_register_hda_component(struct avs_dev *adev, const char *name);
-int avs_register_component(struct device *dev, const char *name,
- struct snd_soc_component_driver *drv,
- struct snd_soc_dai_driver *cpu_dais, int num_cpu_dais);
-
int avs_register_all_boards(struct avs_dev *adev);
void avs_unregister_all_boards(struct avs_dev *adev);
diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index f17bbc2ba63d..fb1af3bb493f 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -22,26 +22,6 @@
#include "utils.h"
#include "../../codecs/hda.h"
-struct avs_dma_data {
- struct avs_tplg_path_template *template;
- struct avs_path *path;
- struct avs_dev *adev;
-
- /* LINK-stream utilized in BE operations while HOST in FE ones. */
- union {
- struct hdac_ext_stream *link_stream;
- struct hdac_ext_stream *host_stream;
- };
-
- struct snd_pcm_hw_constraint_list rate_list;
- struct snd_pcm_hw_constraint_list channels_list;
- struct snd_pcm_hw_constraint_list sample_bits_list;
-
- struct work_struct period_elapsed_work;
- struct hdac_ext_link *link;
- struct snd_pcm_substream *substream;
-};
-
static struct avs_tplg_path_template *
avs_dai_find_path_template(struct snd_soc_dai *dai, bool is_fe, int direction)
{
@@ -117,7 +97,7 @@ static int avs_hw_constraints_init(struct snd_pcm_substream *substream, struct s
return 0;
}
-static int avs_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
+int avs_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
{
struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
struct avs_dev *adev = to_avs_dev(dai->component->dev);
@@ -147,7 +127,7 @@ static int avs_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_d
return avs_hw_constraints_init(substream, dai);
}
-static void avs_dai_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
+void avs_dai_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
{
struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
struct avs_dma_data *data;
@@ -165,10 +145,10 @@ static void avs_dai_shutdown(struct snd_pcm_substream *substream, struct snd_soc
kfree(data);
}
-static int avs_dai_hw_params(struct snd_pcm_substream *substream,
- struct snd_pcm_hw_params *fe_hw_params,
- struct snd_pcm_hw_params *be_hw_params, struct snd_soc_dai *dai,
- int dma_id)
+int avs_dai_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *fe_hw_params,
+ struct snd_pcm_hw_params *be_hw_params,
+ struct snd_soc_dai *dai, int dma_id)
{
struct avs_dma_data *data;
struct avs_path *path;
@@ -199,9 +179,9 @@ static int avs_dai_hw_params(struct snd_pcm_substream *substream,
return 0;
}
-static int avs_dai_be_hw_params(struct snd_pcm_substream *substream,
- struct snd_pcm_hw_params *be_hw_params, struct snd_soc_dai *dai,
- int dma_id)
+int avs_dai_be_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *be_hw_params,
+ struct snd_soc_dai *dai, int dma_id)
{
struct snd_pcm_hw_params *fe_hw_params = NULL;
struct snd_soc_pcm_runtime *fe, *be;
@@ -217,7 +197,7 @@ static int avs_dai_be_hw_params(struct snd_pcm_substream *substream,
return avs_dai_hw_params(substream, fe_hw_params, be_hw_params, dai, dma_id);
}
-static int avs_dai_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
+int avs_dai_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
{
struct avs_dma_data *data;
int ret;
@@ -1376,7 +1356,7 @@ static int avs_component_new(struct snd_soc_component *component,
return 0;
}
-static struct snd_soc_component_driver avs_component_driver = {
+struct snd_soc_component_driver avs_component_driver = {
.name = "avs-pcm",
.probe = avs_component_probe,
.remove = avs_component_remove,
diff --git a/sound/soc/intel/avs/pcm.h b/sound/soc/intel/avs/pcm.h
index 0f3615c90398..ac127ade4ca1 100644
--- a/sound/soc/intel/avs/pcm.h
+++ b/sound/soc/intel/avs/pcm.h
@@ -9,8 +9,57 @@
#ifndef __SOUND_SOC_INTEL_AVS_PCM_H
#define __SOUND_SOC_INTEL_AVS_PCM_H
+#include <linux/device.h>
+#include <sound/hdaudio_ext.h>
#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc-acpi.h>
+#include <sound/pcm_params.h>
+#include <sound/soc-acpi.h>
+#include <sound/soc-acpi-intel-match.h>
+#include <sound/soc-component.h>
+#include <sound/soc-dai.h>
+
+struct avs_dma_data {
+ struct avs_tplg_path_template *template;
+ struct avs_path *path;
+ struct avs_dev *adev;
+
+ /* LINK-stream utilized in BE operations while HOST in FE ones. */
+ union {
+ struct hdac_ext_stream *link_stream;
+ struct hdac_ext_stream *host_stream;
+ };
+
+ struct snd_pcm_hw_constraint_list rate_list;
+ struct snd_pcm_hw_constraint_list channels_list;
+ struct snd_pcm_hw_constraint_list sample_bits_list;
+
+ struct work_struct period_elapsed_work;
+ struct hdac_ext_link *link;
+ struct snd_pcm_substream *substream;
+};
void avs_period_elapsed(struct snd_pcm_substream *substream);
+int avs_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai);
+void avs_dai_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai);
+int avs_dai_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *fe_params,
+ struct snd_pcm_hw_params *be_params,
+ struct snd_soc_dai *dai, int dma_id);
+int avs_dai_be_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *be_params,
+ struct snd_soc_dai *dai, int dma_id);
+int avs_dai_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai);
+
+extern struct snd_soc_component_driver avs_component_driver;
+int avs_register_component(struct device *dev, const char *name,
+ struct snd_soc_component_driver *drv,
+ struct snd_soc_dai_driver *cpu_dais, int num_cpu_dais);
+
+int avs_register_dmic_component(struct avs_dev *adev, const char *name);
+int avs_register_i2s_component(struct avs_dev *adev, const char *name, unsigned long port_mask,
+ unsigned long *tdms);
+int avs_register_hda_component(struct avs_dev *adev, const char *name);
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 6/7] ASoC: Intel: avs: Expose common PCM functions
2026-11-02 18:10 ` [PATCH 6/7] ASoC: Intel: avs: Expose common PCM functions Cezary Rojewski
@ 2026-11-02 18:11 ` sashiko-bot
2026-11-02 18:18 ` Amadeusz Sławiński
1 sibling, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-11-02 18:11 UTC (permalink / raw)
To: Cezary Rojewski, linux-sound
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] ASoC: Intel: avs: duplicate header includes in pcm.h
--
commit 9d263961a528ee317534dc04cdc45b51e6460ff0
Author: Cezary Rojewski <cezary.rojewski@intel.com>
ASoC: Intel: avs: Expose common PCM functions
This commit moves struct avs_dma_data and declarations for common DAI and
component registration functions into sound/soc/intel/avs/pcm.h so that PCM
operations can be split across multiple files.
> diff --git a/sound/soc/intel/avs/pcm.h b/sound/soc/intel/avs/pcm.h
> index 0f3615c903982..ac127ade4ca1c 100644
> --- a/sound/soc/intel/avs/pcm.h
> +++ b/sound/soc/intel/avs/pcm.h
> @@ -9,8 +9,57 @@
> #ifndef __SOUND_SOC_INTEL_AVS_PCM_H
> #define __SOUND_SOC_INTEL_AVS_PCM_H
>
> +#include <linux/device.h>
> +#include <sound/hdaudio_ext.h>
> #include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc-acpi.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc-acpi.h>
[Severity: Low]
This isn't a bug, but in sound/soc/intel/avs/pcm.h under
__SOUND_SOC_INTEL_AVS_PCM_H, sound/pcm_params.h and sound/soc-acpi.h are
included twice in a row. Could the duplicate #include directives be
removed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261102181042.1922005-1-cezary.rojewski@intel.com?part=6
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 6/7] ASoC: Intel: avs: Expose common PCM functions
2026-11-02 18:10 ` [PATCH 6/7] ASoC: Intel: avs: Expose common PCM functions Cezary Rojewski
2026-11-02 18:11 ` sashiko-bot
@ 2026-11-02 18:18 ` Amadeusz Sławiński
2026-11-02 18:20 ` Cezary Rojewski
1 sibling, 1 reply; 16+ messages in thread
From: Amadeusz Sławiński @ 2026-11-02 18:18 UTC (permalink / raw)
To: Cezary Rojewski
Cc: linux-sound, broonie, tiwai, perex, linux-kernel, yung-chuan.liao,
pierre-louis.bossart, vkoul
On Mon, 2 Nov 2026 19:10:41 +0100
Cezary Rojewski <cezary.rojewski@intel.com> wrote:
> diff --git a/sound/soc/intel/avs/pcm.h b/sound/soc/intel/avs/pcm.h
> index 0f3615c90398..ac127ade4ca1 100644
> --- a/sound/soc/intel/avs/pcm.h
> +++ b/sound/soc/intel/avs/pcm.h
> @@ -9,8 +9,57 @@
> #ifndef __SOUND_SOC_INTEL_AVS_PCM_H
> #define __SOUND_SOC_INTEL_AVS_PCM_H
>
> +#include <linux/device.h>
> +#include <sound/hdaudio_ext.h>
> #include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc-acpi.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc-acpi.h>
> +#include <sound/soc-acpi-intel-match.h>
> +#include <sound/soc-component.h>
> +#include <sound/soc-dai.h>
Some headers are included twice?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 6/7] ASoC: Intel: avs: Expose common PCM functions
2026-11-02 18:18 ` Amadeusz Sławiński
@ 2026-11-02 18:20 ` Cezary Rojewski
0 siblings, 0 replies; 16+ messages in thread
From: Cezary Rojewski @ 2026-11-02 18:20 UTC (permalink / raw)
To: Amadeusz Sławiński
Cc: linux-sound, broonie, tiwai, perex, linux-kernel, yung-chuan.liao,
pierre-louis.bossart, vkoul
On 11/2/2026 7:18 PM, Amadeusz Sławiński wrote:
> On Mon, 2 Nov 2026 19:10:41 +0100
> Cezary Rojewski <cezary.rojewski@intel.com> wrote:
>
>> diff --git a/sound/soc/intel/avs/pcm.h b/sound/soc/intel/avs/pcm.h
>> index 0f3615c90398..ac127ade4ca1 100644
>> --- a/sound/soc/intel/avs/pcm.h
>> +++ b/sound/soc/intel/avs/pcm.h
>> @@ -9,8 +9,57 @@
>> #ifndef __SOUND_SOC_INTEL_AVS_PCM_H
>> #define __SOUND_SOC_INTEL_AVS_PCM_H
>>
>> +#include <linux/device.h>
>> +#include <sound/hdaudio_ext.h>
>> #include <sound/pcm.h>
>> +#include <sound/pcm_params.h>
>> +#include <sound/soc-acpi.h>
>> +#include <sound/pcm_params.h>
>> +#include <sound/soc-acpi.h>
>> +#include <sound/soc-acpi-intel-match.h>
>> +#include <sound/soc-component.h>
>> +#include <sound/soc-dai.h>
>
> Some headers are included twice?
Indeed, ACK.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 7/7] ASoC: Intel: avs: SoundWire PCM BE operations
2026-11-02 18:10 [PATCH 0/7] ASoC: Intel: avs: Soundwire support Cezary Rojewski
` (5 preceding siblings ...)
2026-11-02 18:10 ` [PATCH 6/7] ASoC: Intel: avs: Expose common PCM functions Cezary Rojewski
@ 2026-11-02 18:10 ` Cezary Rojewski
2026-11-02 18:26 ` sashiko-bot
6 siblings, 1 reply; 16+ messages in thread
From: Cezary Rojewski @ 2026-11-02 18:10 UTC (permalink / raw)
To: linux-sound
Cc: broonie, tiwai, perex, amade, linux-kernel, yung-chuan.liao,
pierre-louis.bossart, vkoul, Cezary Rojewski
Implement the back-end PCM operations for the SoundWire Link to support
data transfer over SoundWire interface. Most of the actual job is done
by the SoundWire library, thus code added here just invokes them in
correct order.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/intel/avs/Makefile | 2 +-
sound/soc/intel/avs/messages.h | 2 +
sound/soc/intel/avs/path.c | 2 +
sound/soc/intel/avs/pcm.h | 2 +
sound/soc/intel/avs/pcm_sdw.c | 276 ++++++++++++++++++++++++++++++++
sound/soc/intel/avs/soundwire.h | 7 +
6 files changed, 290 insertions(+), 1 deletion(-)
create mode 100644 sound/soc/intel/avs/pcm_sdw.c
diff --git a/sound/soc/intel/avs/Makefile b/sound/soc/intel/avs/Makefile
index 1d48953c7a31..07a82a6f5b23 100644
--- a/sound/soc/intel/avs/Makefile
+++ b/sound/soc/intel/avs/Makefile
@@ -15,7 +15,7 @@ snd-soc-avs-y += probes.o debugfs.o
endif
ifneq ($(CONFIG_SOUNDWIRE_INTEL),)
-snd-soc-avs-y += soundwire.o
+snd-soc-avs-y += soundwire.o pcm_sdw.o
endif
obj-$(CONFIG_SND_SOC_INTEL_AVS) += snd-soc-avs.o
diff --git a/sound/soc/intel/avs/messages.h b/sound/soc/intel/avs/messages.h
index 55c04b0142ae..d1ffc640dda2 100644
--- a/sound/soc/intel/avs/messages.h
+++ b/sound/soc/intel/avs/messages.h
@@ -750,6 +750,8 @@ enum avs_dma_type {
AVS_DMA_DMIC_LINK_INPUT = 11,
AVS_DMA_I2S_LINK_OUTPUT = 12,
AVS_DMA_I2S_LINK_INPUT = 13,
+ AVS_DMA_SDW_LINK_OUTPUT = 16,
+ AVS_DMA_SDW_LINK_INPUT = 17,
};
union avs_virtual_index {
diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c
index a8a2b3484338..60d9245894e7 100644
--- a/sound/soc/intel/avs/path.c
+++ b/sound/soc/intel/avs/path.c
@@ -260,6 +260,8 @@ static void avs_init_node_id(union avs_connector_node_id *node_id,
case AVS_DMA_HDA_LINK_OUTPUT:
case AVS_DMA_HDA_LINK_INPUT:
+ case AVS_DMA_SDW_LINK_OUTPUT:
+ case AVS_DMA_SDW_LINK_INPUT:
node_id->vindex = te->copier.vindex.val | dma_id;
break;
diff --git a/sound/soc/intel/avs/pcm.h b/sound/soc/intel/avs/pcm.h
index ac127ade4ca1..66d6896a4af7 100644
--- a/sound/soc/intel/avs/pcm.h
+++ b/sound/soc/intel/avs/pcm.h
@@ -37,6 +37,8 @@ struct avs_dma_data {
struct work_struct period_elapsed_work;
struct hdac_ext_link *link;
+ struct sdw_stream_runtime *sdw_stream;
+ struct sdw_cdns_pdi *pdi;
struct snd_pcm_substream *substream;
};
diff --git a/sound/soc/intel/avs/pcm_sdw.c b/sound/soc/intel/avs/pcm_sdw.c
new file mode 100644
index 000000000000..27cb285eca09
--- /dev/null
+++ b/sound/soc/intel/avs/pcm_sdw.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Copyright(c) 2024-2026 Intel Corporation
+//
+// Authors: Cezary Rojewski <cezary.rojewski@intel.com>
+// Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
+//
+
+#include <linux/device.h>
+#include <linux/soundwire/sdw.h>
+#include <linux/soundwire/sdw_intel.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-acpi.h>
+#include <sound/soc-component.h>
+#include <sound/soc-dai.h>
+#include "avs.h"
+#include "path.h"
+#include "pcm.h"
+#include "soundwire.h"
+#include "topology.h"
+
+static int avs_sdw_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
+{
+ struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+ struct sdw_stream_runtime *sdw_stream;
+ struct snd_soc_acpi_mach *mach;
+ struct snd_soc_dai *codec_dai;
+ struct avs_dma_data *data;
+ struct sdw_cdns_pdi *pdi;
+ struct sdw_slave *codec;
+ int dir, ret;
+
+ mach = dev_get_platdata(dai->component->card->dev);
+ codec = mach->pdata;
+ if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+ dir = SDW_DATA_DIR_RX;
+ else
+ dir = SDW_DATA_DIR_TX;
+
+ ret = avs_dai_startup(substream, dai);
+ if (ret)
+ return ret;
+
+ sdw_stream = sdw_alloc_stream(substream->name, SDW_STREAM_PCM);
+ if (!sdw_stream) {
+ ret = -ENOMEM;
+ goto err_alloc_stream;
+ }
+
+ pdi = sdw_cdns_assign_pdi(codec->bus, dir);
+ if (!pdi) {
+ ret = -EBUSY;
+ goto err_assign_pdi;
+ }
+
+ /* Fulfill the codec needs by setting the sdw_stream. */
+ codec_dai = snd_soc_rtd_to_codec(rtd, 0);
+ snd_soc_dai_set_stream(codec_dai, sdw_stream, substream->stream);
+
+ data = snd_soc_dai_get_dma_data(dai, substream);
+ data->sdw_stream = sdw_stream;
+ data->pdi = pdi;
+ return 0;
+
+err_assign_pdi:
+ sdw_release_stream(sdw_stream);
+err_alloc_stream:
+ avs_dai_shutdown(substream, dai);
+ return ret;
+}
+
+static void avs_sdw_dai_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
+{
+ struct snd_soc_acpi_mach *mach;
+ struct avs_dma_data *data;
+ struct sdw_slave *codec;
+
+ data = snd_soc_dai_get_dma_data(dai, substream);
+ mach = dev_get_platdata(dai->component->card->dev);
+ codec = mach->pdata;
+
+ sdw_cdns_release_pdi(codec->bus, data->pdi);
+ sdw_release_stream(data->sdw_stream);
+ avs_dai_shutdown(substream, dai);
+}
+
+static int avs_sdw_dai_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *hw_params,
+ struct snd_soc_dai *dai)
+{
+ struct sdw_stream_config sconfig = {0};
+ struct sdw_port_config pconfig = {0};
+ struct snd_soc_acpi_mach *mach;
+ struct avs_dma_data *data;
+ struct sdw_cdns_pdi *pdi;
+ struct sdw_slave *codec;
+ int ret;
+
+ data = snd_soc_dai_get_dma_data(dai, substream);
+ if (data->path)
+ return 0;
+
+ mach = dev_get_platdata(dai->component->card->dev);
+ codec = mach->pdata;
+ pdi = data->pdi;
+ pdi->ch_count = params_channels(hw_params);
+ pdi->h_ch_num = pdi->ch_count - 1;
+ pdi->l_ch_num = 0;
+ sdw_intel_pdi_config(codec->bus, pdi);
+
+ sconfig.direction = pdi->dir;
+ sconfig.ch_count = pdi->ch_count;
+ sconfig.frame_rate = params_rate(hw_params);
+ sconfig.type = SDW_STREAM_PCM;
+ sconfig.bps = snd_pcm_format_width(params_format(hw_params));
+ pconfig.num = pdi->num;
+ pconfig.ch_mask = (1 << pdi->ch_count) - 1;
+
+ ret = sdw_stream_add_master(codec->bus, &sconfig, &pconfig, 1, data->sdw_stream);
+ if (ret)
+ return ret;
+
+ ret = avs_dai_be_hw_params(substream, hw_params, dai, pdi->intel_alh_id);
+ if (ret)
+ sdw_stream_remove_master(codec->bus, data->sdw_stream);
+ return ret;
+}
+
+static int avs_sdw_dai_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
+{
+ struct snd_soc_acpi_mach *mach;
+ struct avs_dma_data *data;
+ struct sdw_slave *codec;
+ int ret;
+
+ data = snd_soc_dai_get_dma_data(dai, substream);
+ if (!data->path)
+ return 0;
+
+ mach = dev_get_platdata(dai->component->card->dev);
+ codec = mach->pdata;
+
+ ret = avs_path_reset(data->path);
+ if (ret < 0)
+ dev_err(dai->dev, "reset BE path failed: %d\n", ret);
+ avs_path_free(data->path);
+ data->path = NULL;
+ sdw_deprepare_stream(data->sdw_stream);
+ sdw_stream_remove_master(codec->bus, data->sdw_stream);
+
+ return 0;
+}
+
+static int avs_sdw_dai_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
+{
+ struct avs_dma_data *data;
+ int ret;
+
+ data = snd_soc_dai_get_dma_data(dai, substream);
+
+ ret = sdw_prepare_stream(data->sdw_stream);
+ if (ret) {
+ dev_err(dai->dev, "sdw stream prepare failed: %d\n", ret);
+ return ret;
+ }
+
+ return avs_dai_prepare(substream, dai);
+}
+
+static int avs_sdw_dai_trigger(struct snd_pcm_substream *substream, int cmd,
+ struct snd_soc_dai *dai)
+{
+ struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+ struct avs_dma_data *data;
+ int ret = 0;
+
+ data = snd_soc_dai_get_dma_data(dai, substream);
+
+ switch (cmd) {
+ case SNDRV_PCM_TRIGGER_RESUME:
+ if (rtd->dai_link->ignore_suspend)
+ break;
+ fallthrough;
+ case SNDRV_PCM_TRIGGER_START:
+ case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+ ret = sdw_enable_stream(data->sdw_stream);
+ if (ret)
+ break;
+
+ ret = avs_path_run(data->path, AVS_TPLG_TRIGGER_AUTO);
+ if (ret < 0)
+ dev_err(dai->dev, "run BE path failed: %d\n", ret);
+ break;
+
+ case SNDRV_PCM_TRIGGER_SUSPEND:
+ if (rtd->dai_link->ignore_suspend)
+ break;
+ fallthrough;
+ case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ case SNDRV_PCM_TRIGGER_STOP:
+ ret = avs_path_pause(data->path);
+ if (ret < 0)
+ dev_err(dai->dev, "pause BE path failed: %d\n", ret);
+
+ ret = sdw_disable_stream(data->sdw_stream);
+ if (ret)
+ dev_err(dai->dev, "disable sdw stream failed: %d\n", ret);
+ break;
+
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
+static const struct snd_soc_dai_ops avs_sdw_dai_ops = {
+ .startup = avs_sdw_dai_startup,
+ .shutdown = avs_sdw_dai_shutdown,
+ .hw_params = avs_sdw_dai_hw_params,
+ .hw_free = avs_sdw_dai_hw_free,
+ .prepare = avs_sdw_dai_prepare,
+ .trigger = avs_sdw_dai_trigger,
+};
+
+static const struct snd_soc_dai_driver avs_sdw_dai_template = {
+ .playback = {
+ .channels_min = 1,
+ .channels_max = 8,
+ .rates = SNDRV_PCM_RATE_8000_192000 |
+ SNDRV_PCM_RATE_KNOT,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE |
+ SNDRV_PCM_FMTBIT_S32_LE,
+ .subformats = SNDRV_PCM_SUBFMTBIT_MSBITS_20 |
+ SNDRV_PCM_SUBFMTBIT_MSBITS_24 |
+ SNDRV_PCM_SUBFMTBIT_MSBITS_MAX,
+ },
+ .capture = {
+ .channels_min = 1,
+ .channels_max = 8,
+ .rates = SNDRV_PCM_RATE_8000_192000 |
+ SNDRV_PCM_RATE_KNOT,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE |
+ SNDRV_PCM_FMTBIT_S32_LE,
+ .subformats = SNDRV_PCM_SUBFMTBIT_MSBITS_20 |
+ SNDRV_PCM_SUBFMTBIT_MSBITS_24 |
+ SNDRV_PCM_SUBFMTBIT_MSBITS_MAX,
+ },
+ .ops = &avs_sdw_dai_ops,
+};
+
+int avs_register_sdw_component(struct avs_dev *adev, const char *name, u32 link_id)
+{
+ struct snd_soc_dai_driver *dai;
+ struct device *dev = adev->dev;
+
+ dai = devm_kzalloc(dev, sizeof(*dai), GFP_KERNEL);
+ if (!dai)
+ return -ENOMEM;
+
+ memcpy(dai, &avs_sdw_dai_template, sizeof(*dai));
+
+ dai->name = devm_kasprintf(dev, GFP_KERNEL, "avs-sdw%d-pin", link_id);
+ if (!dai->name)
+ return -ENOMEM;
+
+ dai->playback.stream_name = devm_kasprintf(dev, GFP_KERNEL, "%s Tx", dai->name);
+ dai->capture.stream_name = devm_kasprintf(dev, GFP_KERNEL, "%s Rx", dai->name);
+
+ if (!dai->playback.stream_name || !dai->capture.stream_name)
+ return -ENOMEM;
+
+ return avs_register_component(dev, name, &avs_component_driver, dai, 1);
+}
diff --git a/sound/soc/intel/avs/soundwire.h b/sound/soc/intel/avs/soundwire.h
index 96ce29ab21a3..1cf77d5ec2d8 100644
--- a/sound/soc/intel/avs/soundwire.h
+++ b/sound/soc/intel/avs/soundwire.h
@@ -22,6 +22,8 @@ void avs_sdw_enable_irq(struct avs_dev *adev, bool enable);
void avs_sdw_process_wakeen_event(struct avs_dev *adev);
irqreturn_t avs_sdw_thread(int irq, struct avs_dev *adev);
+int avs_register_sdw_component(struct avs_dev *adev, const char *name, u32 link_id);
+
#else
static inline int avs_sdw_probe(struct avs_dev *adev)
@@ -51,6 +53,11 @@ static inline irqreturn_t avs_sdw_thread(int irq, struct avs_dev *adev)
return IRQ_HANDLED;
}
+static inline int avs_register_sdw_component(struct avs_dev *adev, const char *name, u32 link_id)
+{
+ return 0;
+}
+
#endif
#endif /* __SOUND_SOC_INTEL_AVS_SOUNDWIRE_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 7/7] ASoC: Intel: avs: SoundWire PCM BE operations
2026-11-02 18:10 ` [PATCH 7/7] ASoC: Intel: avs: SoundWire PCM BE operations Cezary Rojewski
@ 2026-11-02 18:26 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-11-02 18:26 UTC (permalink / raw)
To: Cezary Rojewski, linux-sound
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] ASoC: Intel: avs: broken suspend/resume due to avs_sdw_dai_hw_free()
- [High] ASoC: Intel: avs: m_rt leak and UAF on sdw_stream_add_master() error in avs_sdw_dai_hw_params()
- [Medium] ASoC: Intel: avs: dangling codec stream pointer in avs_sdw_dai_shutdown()
- [Medium] ASoC: Intel: avs: missing stream disable rollback in avs_sdw_dai_trigger()
- [Low] ASoC: Intel: avs: link failure when SND_SOC_INTEL_AVS=y and SOUNDWIRE_INTEL=m
--
commit b4101c2b87dd19b6fc2e6be967189ca74f738d4e
Author: Cezary Rojewski <cezary.rojewski@intel.com>
ASoC: Intel: avs: SoundWire PCM BE operations
This commit implements back-end PCM operations for the SoundWire link in
the Intel AVS driver, wiring up DAI startup, shutdown, hw_params, hw_free,
prepare, and trigger callbacks to the SoundWire core and Cadence helpers.
> diff --git a/sound/soc/intel/avs/Makefile b/sound/soc/intel/avs/Makefile
> index 1d48953c7a31c..07a82a6f5b23e 100644
> --- a/sound/soc/intel/avs/Makefile
> +++ b/sound/soc/intel/avs/Makefile
> @@ -15,7 +15,7 @@ snd-soc-avs-y += probes.o debugfs.o
> endif
>
> ifneq ($(CONFIG_SOUNDWIRE_INTEL),)
> -snd-soc-avs-y += soundwire.o
> +snd-soc-avs-y += soundwire.o pcm_sdw.o
> endif
>
> obj-$(CONFIG_SND_SOC_INTEL_AVS) += snd-soc-avs.o
[Severity: Low]
Can this fail to link when CONFIG_SND_SOC_INTEL_AVS=y and
CONFIG_SOUNDWIRE_INTEL=m?
In sound/soc/intel/Kconfig, SND_SOC_INTEL_AVS is tristate and does not
depend on SOUNDWIRE_INTEL || !SOUNDWIRE_INTEL. When SND_SOC_INTEL_AVS=y
and SOUNDWIRE_INTEL=m (with SOUNDWIRE=m and SOUNDWIRE_CADENCE=m),
ifneq ($(CONFIG_SOUNDWIRE_INTEL),) is still true and links pcm_sdw.o into
the built-in snd-soc-avs.o, while sdw_alloc_stream(),
sdw_cdns_assign_pdi(), and the other SoundWire helpers live in loadable
modules.
> diff --git a/sound/soc/intel/avs/pcm_sdw.c b/sound/soc/intel/avs/pcm_sdw.c
> new file mode 100644
> index 0000000000000..27cb285eca091
> --- /dev/null
> +++ b/sound/soc/intel/avs/pcm_sdw.c
> @@ -0,0 +1,276 @@
[ ... ]
> +static int avs_sdw_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
> +{
> + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
> + struct sdw_stream_runtime *sdw_stream;
> + struct snd_soc_acpi_mach *mach;
> + struct snd_soc_dai *codec_dai;
> + struct avs_dma_data *data;
> + struct sdw_cdns_pdi *pdi;
> + struct sdw_slave *codec;
> + int dir, ret;
> +
> + mach = dev_get_platdata(dai->component->card->dev);
> + codec = mach->pdata;
> + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
> + dir = SDW_DATA_DIR_RX;
> + else
> + dir = SDW_DATA_DIR_TX;
> +
> + ret = avs_dai_startup(substream, dai);
> + if (ret)
> + return ret;
> +
> + sdw_stream = sdw_alloc_stream(substream->name, SDW_STREAM_PCM);
> + if (!sdw_stream) {
> + ret = -ENOMEM;
> + goto err_alloc_stream;
> + }
> +
> + pdi = sdw_cdns_assign_pdi(codec->bus, dir);
> + if (!pdi) {
> + ret = -EBUSY;
> + goto err_assign_pdi;
> + }
> +
> + /* Fulfill the codec needs by setting the sdw_stream. */
> + codec_dai = snd_soc_rtd_to_codec(rtd, 0);
> + snd_soc_dai_set_stream(codec_dai, sdw_stream, substream->stream);
> +
> + data = snd_soc_dai_get_dma_data(dai, substream);
> + data->sdw_stream = sdw_stream;
> + data->pdi = pdi;
> + return 0;
> +
> +err_assign_pdi:
> + sdw_release_stream(sdw_stream);
> +err_alloc_stream:
> + avs_dai_shutdown(substream, dai);
> + return ret;
> +}
> +
> +static void avs_sdw_dai_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
> +{
> + struct snd_soc_acpi_mach *mach;
> + struct avs_dma_data *data;
> + struct sdw_slave *codec;
> +
> + data = snd_soc_dai_get_dma_data(dai, substream);
> + mach = dev_get_platdata(dai->component->card->dev);
> + codec = mach->pdata;
> +
> + sdw_cdns_release_pdi(codec->bus, data->pdi);
> + sdw_release_stream(data->sdw_stream);
> + avs_dai_shutdown(substream, dai);
> +}
[Severity: Medium]
Should avs_sdw_dai_startup() check the return value of
snd_soc_dai_set_stream(), and should avs_sdw_dai_shutdown() clear the
stream pointer on codec_dai before freeing data->sdw_stream?
In avs_sdw_dai_shutdown(), sdw_release_stream(data->sdw_stream) frees
data->sdw_stream without calling
snd_soc_dai_set_stream(codec_dai, NULL, substream->stream) (unlike
sdw_shutdown_stream()). If a SoundWire codec driver does not clear its
stream pointer in a .shutdown callback, or if __soc_pcm_open() fails
during codec_dai startup after avs_sdw_dai_startup() has already run (so
soc_pcm_clean() skips codec_dai shutdown during rollback), codec_dai is
left holding a dangling pointer to the freed sdw_stream_runtime.
> +static int avs_sdw_dai_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *hw_params,
> + struct snd_soc_dai *dai)
> +{
> + struct sdw_stream_config sconfig = {0};
> + struct sdw_port_config pconfig = {0};
> + struct snd_soc_acpi_mach *mach;
> + struct avs_dma_data *data;
> + struct sdw_cdns_pdi *pdi;
> + struct sdw_slave *codec;
> + int ret;
> +
> + data = snd_soc_dai_get_dma_data(dai, substream);
> + if (data->path)
> + return 0;
> +
> + mach = dev_get_platdata(dai->component->card->dev);
> + codec = mach->pdata;
> + pdi = data->pdi;
> + pdi->ch_count = params_channels(hw_params);
> + pdi->h_ch_num = pdi->ch_count - 1;
> + pdi->l_ch_num = 0;
> + sdw_intel_pdi_config(codec->bus, pdi);
> +
> + sconfig.direction = pdi->dir;
> + sconfig.ch_count = pdi->ch_count;
> + sconfig.frame_rate = params_rate(hw_params);
> + sconfig.type = SDW_STREAM_PCM;
> + sconfig.bps = snd_pcm_format_width(params_format(hw_params));
> + pconfig.num = pdi->num;
> + pconfig.ch_mask = (1 << pdi->ch_count) - 1;
> +
> + ret = sdw_stream_add_master(codec->bus, &sconfig, &pconfig, 1, data->sdw_stream);
> + if (ret)
> + return ret;
> +
> + ret = avs_dai_be_hw_params(substream, hw_params, dai, pdi->intel_alh_id);
> + if (ret)
> + sdw_stream_remove_master(codec->bus, data->sdw_stream);
> + return ret;
> +}
[Severity: High]
If sdw_stream_add_master() fails here in avs_sdw_dai_hw_params(), does
this leak struct sdw_master_runtime and leave a dangling m_rt->stream
pointer on codec->bus->m_rt_list?
During __soc_pcm_hw_params(), the codec DAI hw_params runs before the CPU
DAI hw_params:
__soc_pcm_hw_params()
soc_pcm_hw_params() [codec DAI]
sdw_stream_add_slave()
sdw_master_rt_alloc() -> allocates m_rt, links onto
codec->bus->m_rt_list with
m_rt->stream = data->sdw_stream
soc_pcm_hw_params() [CPU DAI: avs_sdw_dai_hw_params()]
sdw_stream_add_master()
Because m_rt was already allocated by sdw_stream_add_slave(),
sdw_stream_add_master() sets alloc_master_rt = false and does not free
m_rt (or any allocated master ports) if sdw_master_port_alloc(),
sdw_master_rt_config(), sdw_config_stream(), or sdw_master_port_config()
fails.
When avs_sdw_dai_hw_params() returns that error without calling
sdw_stream_remove_master(), data->path remains NULL. Rollback in
soc_pcm_hw_clean() only calls sdw_stream_remove_slave() on the codec DAI
(which frees s_rt, not m_rt) and skips avs_sdw_dai_hw_free(). When the
substream is then closed, avs_sdw_dai_shutdown() frees data->sdw_stream
while m_rt is still linked on codec->bus->m_rt_list with m_rt->stream
pointing to the freed stream.
> +
> +static int avs_sdw_dai_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
> +{
> + struct snd_soc_acpi_mach *mach;
> + struct avs_dma_data *data;
> + struct sdw_slave *codec;
> + int ret;
> +
> + data = snd_soc_dai_get_dma_data(dai, substream);
> + if (!data->path)
> + return 0;
> +
> + mach = dev_get_platdata(dai->component->card->dev);
> + codec = mach->pdata;
> +
> + ret = avs_path_reset(data->path);
> + if (ret < 0)
> + dev_err(dai->dev, "reset BE path failed: %d\n", ret);
> + avs_path_free(data->path);
> + data->path = NULL;
> + sdw_deprepare_stream(data->sdw_stream);
> + sdw_stream_remove_master(codec->bus, data->sdw_stream);
> +
> + return 0;
> +}
[Severity: High]
Won't calling sdw_stream_remove_master() in avs_sdw_dai_hw_free() break
system suspend and resume when a SoundWire stream is active?
avs_register_sdw_component() registers the component with
avs_component_driver, which uses avs_component_suspend() and
avs_component_resume():
avs_component_suspend()
avs_component_suspend_hw_free()
avs_dai_suspend_be_hw_free()
dai->driver->ops->hw_free() -> avs_sdw_dai_hw_free()
During suspend, avs_dai_suspend_be_hw_free() invokes hw_free only on the
AVS CPU DAI (ASoC does not call hw_free or hw_params on the codec DAI
across suspend/resume). In avs_sdw_dai_hw_free(),
sdw_stream_remove_master() calls sdw_master_rt_free(), which frees both
the master runtime and all attached codec slave runtimes on
m_rt->slave_rt_list, and sets data->sdw_stream->state to
SDW_STREAM_RELEASED.
On resume:
avs_component_resume()
avs_component_resume_hw_params()
avs_dai_resume_hw_params()
avs_sdw_dai_hw_params()
sdw_stream_add_master()
avs_component_resume_prepare()
avs_dai_resume_be_prepare()
avs_sdw_dai_prepare()
sdw_prepare_stream()
Because codec DAI hw_params (sdw_stream_add_slave()) is never called on
resume, m_rt->slave_rt_list remains empty and data->sdw_stream->state
remains SDW_STREAM_RELEASED (only sdw_stream_add_slave() transitions the
stream state to SDW_STREAM_CONFIGURED). Consequently,
sdw_prepare_stream() in avs_sdw_dai_prepare() fails with -EINVAL and
avs_component_pm_op() disconnects the substream.
> +
> +static int avs_sdw_dai_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
> +{
> + struct avs_dma_data *data;
> + int ret;
> +
> + data = snd_soc_dai_get_dma_data(dai, substream);
> +
> + ret = sdw_prepare_stream(data->sdw_stream);
> + if (ret) {
> + dev_err(dai->dev, "sdw stream prepare failed: %d\n", ret);
> + return ret;
> + }
> +
> + return avs_dai_prepare(substream, dai);
> +}
> +
> +static int avs_sdw_dai_trigger(struct snd_pcm_substream *substream, int cmd,
> + struct snd_soc_dai *dai)
> +{
> + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
> + struct avs_dma_data *data;
> + int ret = 0;
> +
> + data = snd_soc_dai_get_dma_data(dai, substream);
> +
> + switch (cmd) {
> + case SNDRV_PCM_TRIGGER_RESUME:
> + if (rtd->dai_link->ignore_suspend)
> + break;
> + fallthrough;
> + case SNDRV_PCM_TRIGGER_START:
> + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> + ret = sdw_enable_stream(data->sdw_stream);
> + if (ret)
> + break;
> +
> + ret = avs_path_run(data->path, AVS_TPLG_TRIGGER_AUTO);
> + if (ret < 0)
> + dev_err(dai->dev, "run BE path failed: %d\n", ret);
> + break;
> +
> + case SNDRV_PCM_TRIGGER_SUSPEND:
> + if (rtd->dai_link->ignore_suspend)
> + break;
> + fallthrough;
> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> + case SNDRV_PCM_TRIGGER_STOP:
> + ret = avs_path_pause(data->path);
> + if (ret < 0)
> + dev_err(dai->dev, "pause BE path failed: %d\n", ret);
> +
> + ret = sdw_disable_stream(data->sdw_stream);
> + if (ret)
> + dev_err(dai->dev, "disable sdw stream failed: %d\n", ret);
> + break;
[Severity: Medium]
If avs_path_run() fails in avs_sdw_dai_trigger(), should
sdw_disable_stream(data->sdw_stream) be called to roll back
sdw_enable_stream()?
When avs_sdw_dai_trigger() returns an error from avs_path_run(), ASoC
does not mark the DAI as triggered or transition the BE to
SND_SOC_DPCM_STATE_START, so STOP will not be called on this DAI. That
leaves data->sdw_stream in SDW_STREAM_ENABLED, causing
sdw_deprepare_stream() in avs_sdw_dai_hw_free() to fail with -EINVAL and
leak the stream's bus bandwidth allocation on codec->bus.
Also, in the SUSPEND/PAUSE_PUSH/STOP case, assigning
ret = sdw_disable_stream(data->sdw_stream) unconditionally overwrites any
negative error returned by avs_path_pause(data->path).
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261102181042.1922005-1-cezary.rojewski@intel.com?part=7
^ permalink raw reply [flat|nested] 16+ messages in thread