* [PATCH 1/2] ASoC: sdw_utils: Remove num_platforms from simple DAI helper
@ 2025-04-29 9:39 Charles Keepax
2025-04-29 9:39 ` [PATCH 2/2] ASoC: Intel: sof_sdw: Don't bother to set platform string Charles Keepax
2025-05-04 0:08 ` [PATCH 1/2] ASoC: sdw_utils: Remove num_platforms from simple DAI helper Mark Brown
0 siblings, 2 replies; 6+ messages in thread
From: Charles Keepax @ 2025-04-29 9:39 UTC (permalink / raw)
To: broonie
Cc: lgirdwood, yung-chuan.liao, peter.ujfalusi, ranjani.sridharan,
kai.vehmanen, pierre-louis.bossart, Vijendar.Mukunda, linux-sound,
patches
There is no point in passing num_platforms into
asoc_sdw_init_simple_dai_link(). Firstly, as a single pointer for the
component name is passed in only a single string can be passed and
secondly if it is a complex DAI with multiple platforms it would make
more sense to use asoc_sdw_init_dai_link().
Fixes: 59f8b622d52e ("ASoC: intel/sdw_utils: refactor init_dai_link() and init_simple_dai_link()")
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
include/sound/soc_sdw_utils.h | 5 ++---
sound/soc/amd/acp/acp-sdw-legacy-mach.c | 2 +-
sound/soc/amd/acp/acp-sdw-sof-mach.c | 1 -
sound/soc/intel/boards/sof_sdw.c | 7 +------
sound/soc/sdw_utils/soc_sdw_utils.c | 9 ++++-----
5 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/include/sound/soc_sdw_utils.h b/include/sound/soc_sdw_utils.h
index 36a4a1e1d8ca..64fee97efbaf 100644
--- a/include/sound/soc_sdw_utils.h
+++ b/include/sound/soc_sdw_utils.h
@@ -159,9 +159,8 @@ void asoc_sdw_init_dai_link(struct device *dev, struct snd_soc_dai_link *dai_lin
int asoc_sdw_init_simple_dai_link(struct device *dev, struct snd_soc_dai_link *dai_links,
int *be_id, char *name, int playback, int capture,
const char *cpu_dai_name, const char *platform_comp_name,
- int num_platforms, const char *codec_name,
- const char *codec_dai_name, int no_pcm,
- int (*init)(struct snd_soc_pcm_runtime *rtd),
+ const char *codec_name, const char *codec_dai_name,
+ int no_pcm, int (*init)(struct snd_soc_pcm_runtime *rtd),
const struct snd_soc_ops *ops);
int asoc_sdw_count_sdw_endpoints(struct snd_soc_card *card, int *num_devs, int *num_ends);
diff --git a/sound/soc/amd/acp/acp-sdw-legacy-mach.c b/sound/soc/amd/acp/acp-sdw-legacy-mach.c
index 2020c5cfb3d5..0bb06cb7d0f9 100644
--- a/sound/soc/amd/acp/acp-sdw-legacy-mach.c
+++ b/sound/soc/amd/acp/acp-sdw-legacy-mach.c
@@ -321,7 +321,7 @@ static int create_dmic_dailinks(struct snd_soc_card *card,
*be_id = ACP_DMIC_BE_ID;
ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, "acp-dmic-codec",
0, 1, // DMIC only supports capture
- pdm_cpu->name, pdm_platform->name, 1,
+ pdm_cpu->name, pdm_platform->name,
"dmic-codec.0", "dmic-hifi", no_pcm,
asoc_sdw_dmic_init, NULL);
if (ret)
diff --git a/sound/soc/amd/acp/acp-sdw-sof-mach.c b/sound/soc/amd/acp/acp-sdw-sof-mach.c
index c09b1f118a6c..2141ccb8329d 100644
--- a/sound/soc/amd/acp/acp-sdw-sof-mach.c
+++ b/sound/soc/amd/acp/acp-sdw-sof-mach.c
@@ -245,7 +245,6 @@ static int create_dmic_dailinks(struct snd_soc_card *card,
ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, "acp-dmic-codec",
0, 1, // DMIC only supports capture
"acp-sof-dmic", platform_component->name,
- ARRAY_SIZE(platform_component),
"dmic-codec", "dmic-hifi", no_pcm,
asoc_sdw_dmic_init, NULL);
if (ret)
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
index 095d08b3fc82..29b813943ccc 100644
--- a/sound/soc/intel/boards/sof_sdw.c
+++ b/sound/soc/intel/boards/sof_sdw.c
@@ -994,8 +994,7 @@ static int create_ssp_dailinks(struct snd_soc_card *card,
ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, name,
playback, capture, cpu_dai_name,
- platform_component->name,
- ARRAY_SIZE(platform_component), codec_name,
+ platform_component->name, codec_name,
ssp_info->dais[0].dai_name, 1, NULL,
ssp_info->ops);
if (ret)
@@ -1020,7 +1019,6 @@ static int create_dmic_dailinks(struct snd_soc_card *card,
ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, "dmic01",
0, 1, // DMIC only supports capture
"DMIC01 Pin", platform_component->name,
- ARRAY_SIZE(platform_component),
"dmic-codec", "dmic-hifi", 1,
asoc_sdw_dmic_init, NULL);
if (ret)
@@ -1031,7 +1029,6 @@ static int create_dmic_dailinks(struct snd_soc_card *card,
ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, "dmic16k",
0, 1, // DMIC only supports capture
"DMIC16k Pin", platform_component->name,
- ARRAY_SIZE(platform_component),
"dmic-codec", "dmic-hifi", 1,
/* don't call asoc_sdw_dmic_init() twice */
NULL, NULL);
@@ -1075,7 +1072,6 @@ static int create_hdmi_dailinks(struct snd_soc_card *card,
ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, name,
1, 0, // HDMI only supports playback
cpu_dai_name, platform_component->name,
- ARRAY_SIZE(platform_component),
codec_name, codec_dai_name, 1,
i == 0 ? sof_sdw_hdmi_init : NULL, NULL);
if (ret)
@@ -1102,7 +1098,6 @@ static int create_bt_dailinks(struct snd_soc_card *card,
ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, name,
1, 1, cpu_dai_name, platform_component->name,
- ARRAY_SIZE(platform_component),
snd_soc_dummy_dlc.name, snd_soc_dummy_dlc.dai_name,
1, NULL, NULL);
if (ret)
diff --git a/sound/soc/sdw_utils/soc_sdw_utils.c b/sound/soc/sdw_utils/soc_sdw_utils.c
index 60b731673f3b..30f84f4e7637 100644
--- a/sound/soc/sdw_utils/soc_sdw_utils.c
+++ b/sound/soc/sdw_utils/soc_sdw_utils.c
@@ -1067,9 +1067,8 @@ EXPORT_SYMBOL_NS(asoc_sdw_init_dai_link, "SND_SOC_SDW_UTILS");
int asoc_sdw_init_simple_dai_link(struct device *dev, struct snd_soc_dai_link *dai_links,
int *be_id, char *name, int playback, int capture,
const char *cpu_dai_name, const char *platform_comp_name,
- int num_platforms, const char *codec_name,
- const char *codec_dai_name, int no_pcm,
- int (*init)(struct snd_soc_pcm_runtime *rtd),
+ const char *codec_name, const char *codec_dai_name,
+ int no_pcm, int (*init)(struct snd_soc_pcm_runtime *rtd),
const struct snd_soc_ops *ops)
{
struct snd_soc_dai_link_component *dlc;
@@ -1086,8 +1085,8 @@ int asoc_sdw_init_simple_dai_link(struct device *dev, struct snd_soc_dai_link *d
dlc[2].dai_name = codec_dai_name;
asoc_sdw_init_dai_link(dev, dai_links, be_id, name, playback, capture,
- &dlc[0], 1, &dlc[1], num_platforms,
- &dlc[2], 1, no_pcm, init, ops);
+ &dlc[0], 1, &dlc[1], 1, &dlc[2], 1,
+ no_pcm, init, ops);
return 0;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] ASoC: Intel: sof_sdw: Don't bother to set platform string
2025-04-29 9:39 [PATCH 1/2] ASoC: sdw_utils: Remove num_platforms from simple DAI helper Charles Keepax
@ 2025-04-29 9:39 ` Charles Keepax
2025-05-04 0:09 ` Mark Brown
2025-05-04 0:08 ` [PATCH 1/2] ASoC: sdw_utils: Remove num_platforms from simple DAI helper Mark Brown
1 sibling, 1 reply; 6+ messages in thread
From: Charles Keepax @ 2025-04-29 9:39 UTC (permalink / raw)
To: broonie
Cc: lgirdwood, yung-chuan.liao, peter.ujfalusi, ranjani.sridharan,
kai.vehmanen, pierre-louis.bossart, Vijendar.Mukunda, linux-sound,
patches
The static platform_component name string is overwritten on card tear
down which will cause a NULL check on it to fail when the driver is
reprobed. However, it turns out that the ASoC core sets this string for
all topology systems anyway (after the aforementioned NULL check). So
there is no need for the machine driver to set the platform at all,
replace all the platform_component stuff with some simple place holders.
Fixes: 52db12d193d4 ("ASoC: Intel: boards: add sof_sdw machine driver")
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
sound/soc/intel/boards/sof_sdw.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
index 29b813943ccc..51b29ebdf405 100644
--- a/sound/soc/intel/boards/sof_sdw.c
+++ b/sound/soc/intel/boards/sof_sdw.c
@@ -780,13 +780,6 @@ static void sof_sdw_check_ssid_quirk(const struct snd_soc_acpi_mach *mach)
sof_sdw_quirk = quirk_entry->value;
}
-static struct snd_soc_dai_link_component platform_component[] = {
- {
- /* name might be overridden during probe */
- .name = "0000:00:1f.3"
- }
-};
-
static const struct snd_soc_ops sdw_ops = {
.startup = asoc_sdw_startup,
.prepare = asoc_sdw_prepare,
@@ -836,6 +829,7 @@ static int create_sdw_dailink(struct snd_soc_card *card,
struct snd_soc_dai_link_ch_map *codec_maps;
struct snd_soc_dai_link_component *codecs;
struct snd_soc_dai_link_component *cpus;
+ struct snd_soc_dai_link_component *platform;
int num_cpus = hweight32(sof_dai->link_mask[stream]);
int num_codecs = sof_dai->num_devs[stream];
int playback, capture;
@@ -876,6 +870,10 @@ static int create_sdw_dailink(struct snd_soc_card *card,
if (!codecs)
return -ENOMEM;
+ platform = devm_kzalloc(dev, sizeof(*platform), GFP_KERNEL);
+ if (!platform)
+ return -ENOMEM;
+
codec_maps = devm_kcalloc(dev, num_codecs, sizeof(*codec_maps), GFP_KERNEL);
if (!codec_maps)
return -ENOMEM;
@@ -917,8 +915,7 @@ static int create_sdw_dailink(struct snd_soc_card *card,
capture = (stream == SNDRV_PCM_STREAM_CAPTURE);
asoc_sdw_init_dai_link(dev, *dai_links, be_id, name, playback, capture,
- cpus, num_cpus, platform_component,
- ARRAY_SIZE(platform_component), codecs, num_codecs,
+ cpus, num_cpus, platform, 1, codecs, num_codecs,
1, asoc_sdw_rtd_init, &sdw_ops);
/*
@@ -994,7 +991,7 @@ static int create_ssp_dailinks(struct snd_soc_card *card,
ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, name,
playback, capture, cpu_dai_name,
- platform_component->name, codec_name,
+ "dummy", codec_name,
ssp_info->dais[0].dai_name, 1, NULL,
ssp_info->ops);
if (ret)
@@ -1018,7 +1015,7 @@ static int create_dmic_dailinks(struct snd_soc_card *card,
ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, "dmic01",
0, 1, // DMIC only supports capture
- "DMIC01 Pin", platform_component->name,
+ "DMIC01 Pin", "dummy",
"dmic-codec", "dmic-hifi", 1,
asoc_sdw_dmic_init, NULL);
if (ret)
@@ -1028,7 +1025,7 @@ static int create_dmic_dailinks(struct snd_soc_card *card,
ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, "dmic16k",
0, 1, // DMIC only supports capture
- "DMIC16k Pin", platform_component->name,
+ "DMIC16k Pin", "dummy",
"dmic-codec", "dmic-hifi", 1,
/* don't call asoc_sdw_dmic_init() twice */
NULL, NULL);
@@ -1071,7 +1068,7 @@ static int create_hdmi_dailinks(struct snd_soc_card *card,
ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, name,
1, 0, // HDMI only supports playback
- cpu_dai_name, platform_component->name,
+ cpu_dai_name, "dummy",
codec_name, codec_dai_name, 1,
i == 0 ? sof_sdw_hdmi_init : NULL, NULL);
if (ret)
@@ -1097,7 +1094,7 @@ static int create_bt_dailinks(struct snd_soc_card *card,
int ret;
ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, name,
- 1, 1, cpu_dai_name, platform_component->name,
+ 1, 1, cpu_dai_name, "dummy",
snd_soc_dummy_dlc.name, snd_soc_dummy_dlc.dai_name,
1, NULL, NULL);
if (ret)
--
2.39.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] ASoC: Intel: sof_sdw: Don't bother to set platform string
2025-04-29 9:39 ` [PATCH 2/2] ASoC: Intel: sof_sdw: Don't bother to set platform string Charles Keepax
@ 2025-05-04 0:09 ` Mark Brown
2025-05-05 8:24 ` Charles Keepax
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2025-05-04 0:09 UTC (permalink / raw)
To: Charles Keepax
Cc: lgirdwood, yung-chuan.liao, peter.ujfalusi, ranjani.sridharan,
kai.vehmanen, pierre-louis.bossart, Vijendar.Mukunda, linux-sound,
patches
[-- Attachment #1: Type: text/plain, Size: 637 bytes --]
On Tue, Apr 29, 2025 at 10:39:31AM +0100, Charles Keepax wrote:
> The static platform_component name string is overwritten on card tear
> down which will cause a NULL check on it to fail when the driver is
> reprobed. However, it turns out that the ASoC core sets this string for
> all topology systems anyway (after the aforementioned NULL check). So
> there is no need for the machine driver to set the platform at all,
> replace all the platform_component stuff with some simple place holders.
>
> Fixes: 52db12d193d4 ("ASoC: Intel: boards: add sof_sdw machine driver")
Similarly here, this seems like more of a cleanup?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] ASoC: Intel: sof_sdw: Don't bother to set platform string
2025-05-04 0:09 ` Mark Brown
@ 2025-05-05 8:24 ` Charles Keepax
0 siblings, 0 replies; 6+ messages in thread
From: Charles Keepax @ 2025-05-05 8:24 UTC (permalink / raw)
To: Mark Brown
Cc: lgirdwood, yung-chuan.liao, peter.ujfalusi, ranjani.sridharan,
kai.vehmanen, pierre-louis.bossart, Vijendar.Mukunda, linux-sound,
patches
On Sun, May 04, 2025 at 09:09:11AM +0900, Mark Brown wrote:
> On Tue, Apr 29, 2025 at 10:39:31AM +0100, Charles Keepax wrote:
> > The static platform_component name string is overwritten on card tear
> > down which will cause a NULL check on it to fail when the driver is
> > reprobed. However, it turns out that the ASoC core sets this string for
> > all topology systems anyway (after the aforementioned NULL check). So
> > there is no need for the machine driver to set the platform at all,
> > replace all the platform_component stuff with some simple place holders.
> >
> > Fixes: 52db12d193d4 ("ASoC: Intel: boards: add sof_sdw machine driver")
>
> Similarly here, this seems like more of a cleanup?
unbind then bind does fail here, so I think the fixes tag is good here, however
I will try to rephrase the commit message a little for v2.
Thanks,
Charles
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ASoC: sdw_utils: Remove num_platforms from simple DAI helper
2025-04-29 9:39 [PATCH 1/2] ASoC: sdw_utils: Remove num_platforms from simple DAI helper Charles Keepax
2025-04-29 9:39 ` [PATCH 2/2] ASoC: Intel: sof_sdw: Don't bother to set platform string Charles Keepax
@ 2025-05-04 0:08 ` Mark Brown
2025-05-05 8:23 ` Charles Keepax
1 sibling, 1 reply; 6+ messages in thread
From: Mark Brown @ 2025-05-04 0:08 UTC (permalink / raw)
To: Charles Keepax
Cc: lgirdwood, yung-chuan.liao, peter.ujfalusi, ranjani.sridharan,
kai.vehmanen, pierre-louis.bossart, Vijendar.Mukunda, linux-sound,
patches
[-- Attachment #1: Type: text/plain, Size: 529 bytes --]
On Tue, Apr 29, 2025 at 10:39:30AM +0100, Charles Keepax wrote:
> There is no point in passing num_platforms into
> asoc_sdw_init_simple_dai_link(). Firstly, as a single pointer for the
> component name is passed in only a single string can be passed and
> secondly if it is a complex DAI with multiple platforms it would make
> more sense to use asoc_sdw_init_dai_link().
> Fixes: 59f8b622d52e ("ASoC: intel/sdw_utils: refactor init_dai_link() and init_simple_dai_link()")
The changelog doesn't seem to describe a bug here...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] ASoC: sdw_utils: Remove num_platforms from simple DAI helper
2025-05-04 0:08 ` [PATCH 1/2] ASoC: sdw_utils: Remove num_platforms from simple DAI helper Mark Brown
@ 2025-05-05 8:23 ` Charles Keepax
0 siblings, 0 replies; 6+ messages in thread
From: Charles Keepax @ 2025-05-05 8:23 UTC (permalink / raw)
To: Mark Brown
Cc: lgirdwood, yung-chuan.liao, peter.ujfalusi, ranjani.sridharan,
kai.vehmanen, pierre-louis.bossart, Vijendar.Mukunda, linux-sound,
patches
On Sun, May 04, 2025 at 09:08:05AM +0900, Mark Brown wrote:
> On Tue, Apr 29, 2025 at 10:39:30AM +0100, Charles Keepax wrote:
> > There is no point in passing num_platforms into
> > asoc_sdw_init_simple_dai_link(). Firstly, as a single pointer for the
> > component name is passed in only a single string can be passed and
> > secondly if it is a complex DAI with multiple platforms it would make
> > more sense to use asoc_sdw_init_dai_link().
>
> > Fixes: 59f8b622d52e ("ASoC: intel/sdw_utils: refactor init_dai_link() and init_simple_dai_link()")
>
> The changelog doesn't seem to describe a bug here...
Yeah that is fair this probably is more of a clean up, I will
resend and drop the fixes.
Thanks,
Charles
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-05-05 8:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-29 9:39 [PATCH 1/2] ASoC: sdw_utils: Remove num_platforms from simple DAI helper Charles Keepax
2025-04-29 9:39 ` [PATCH 2/2] ASoC: Intel: sof_sdw: Don't bother to set platform string Charles Keepax
2025-05-04 0:09 ` Mark Brown
2025-05-05 8:24 ` Charles Keepax
2025-05-04 0:08 ` [PATCH 1/2] ASoC: sdw_utils: Remove num_platforms from simple DAI helper Mark Brown
2025-05-05 8:23 ` Charles Keepax
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox