Skip to content

Fix handling of platform_component in sof_sdw #5383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
charleskeepax wants to merge 2 commits into from
Closed

Fix handling of platform_component in sof_sdw #5383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce862b3
ASoC: sdw_utils: Remove num_platforms from simple DAI helper
charleskeepax Apr 9, 2025
a4854d1
ASoC: Intel: sof_sdw: Don't bother to set platform string
charleskeepax Apr 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions include/sound/soc_sdw_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion sound/soc/amd/acp/acp-sdw-legacy-mach.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion sound/soc/amd/acp/acp-sdw-sof-mach.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
30 changes: 11 additions & 19 deletions sound/soc/intel/boards/sof_sdw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,13 +799,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[] = {
Copy link
Collaborator

@ranj063 ranj063 Apr 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @charleskeepax except for the typo in the commit title "don't both to set platform string"

{
/* 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,
Expand Down Expand Up @@ -855,6 +848,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;
Expand Down Expand Up @@ -895,6 +889,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;
Expand Down Expand Up @@ -936,8 +934,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);

/*
Expand Down Expand Up @@ -1013,8 +1010,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,
"dummy", codec_name,
ssp_info->dais[0].dai_name, 1, NULL,
ssp_info->ops);
if (ret)
Expand All @@ -1038,8 +1034,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,
ARRAY_SIZE(platform_component),
"DMIC01 Pin", "dummy",
"dmic-codec", "dmic-hifi", 1,
asoc_sdw_dmic_init, NULL);
if (ret)
Expand All @@ -1049,8 +1044,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,
ARRAY_SIZE(platform_component),
"DMIC16k Pin", "dummy",
"dmic-codec", "dmic-hifi", 1,
/* don't call asoc_sdw_dmic_init() twice */
NULL, NULL);
Expand Down Expand Up @@ -1093,8 +1087,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,
ARRAY_SIZE(platform_component),
cpu_dai_name, "dummy",
codec_name, codec_dai_name, 1,
i == 0 ? sof_sdw_hdmi_init : NULL, NULL);
if (ret)
Expand All @@ -1120,8 +1113,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,
ARRAY_SIZE(platform_component),
1, 1, cpu_dai_name, "dummy",
snd_soc_dummy_dlc.name, snd_soc_dummy_dlc.dai_name,
1, NULL, NULL);
if (ret)
Expand Down
9 changes: 4 additions & 5 deletions sound/soc/sdw_utils/soc_sdw_utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Copy link
Collaborator

@ranj063 ranj063 Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@charleskeepax did you mean to remove num_platforms from this one too?

Copy link
Collaborator

@ujfalusi ujfalusi Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While sound/soc/amd/acp/acp-sdw-sof-mach.c appears to use ARRAY_SIZE(platform_component), the platform_component there also limited to a single entry.

Copy link
Author

@charleskeepax charleskeepax Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry struggling with the github, can't seem to figure out if these comments are showing in two places or if they are two different comments. But this is attempting to comment on the asoc_sdw_init_simple_dai_link part. Yeah doesn't make sense for asoc_sdw_init_simple_dai_link to set multiple platforms, the input string is "const char *platform_comp_name", which can only pass a single platform in to the function, and if you have multiple platforms its not really a simple dai you the full helper.

no_pcm, init, ops);

return 0;
}
Expand Down
Loading