Linux Sound subsystem development
 help / color / mirror / Atom feed
* [bug report] ASoC: amd: acp: refactor SoundWire machine driver code
@ 2024-09-27 10:35 Dan Carpenter
  2024-10-03  6:28 ` Mukunda,Vijendar
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2024-09-27 10:35 UTC (permalink / raw)
  To: Vijendar Mukunda; +Cc: linux-sound

Hello Vijendar Mukunda,

Commit 6d8348ddc56e ("ASoC: amd: acp: refactor SoundWire machine
driver code") from Sep 13, 2024 (linux-next), leads to the following
Smatch static checker warning:

	sound/soc/amd/acp/acp-sdw-sof-mach.c:157 create_sdw_dailink()
	warn: iterator 'i' not incremented

sound/soc/amd/acp/acp-sdw-sof-mach.c
    144         for_each_pcm_streams(stream) {
    145                 static const char * const sdw_stream_name[] = {
    146                         "SDW%d-PIN%d-PLAYBACK",
    147                         "SDW%d-PIN%d-CAPTURE",
    148                         "SDW%d-PIN%d-PLAYBACK-%s",
    149                         "SDW%d-PIN%d-CAPTURE-%s",
    150                 };
    151                 struct snd_soc_dai_link_ch_map *codec_maps;
    152                 struct snd_soc_dai_link_component *codecs;
    153                 struct snd_soc_dai_link_component *cpus;
    154                 int num_cpus = hweight32(sof_dai->link_mask[stream]);
    155                 int num_codecs = sof_dai->num_devs[stream];
    156                 int playback, capture;
--> 157                 int i = 0, j = 0;
    158                 char *name;
    159 
    160                 if (!sof_dai->num_devs[stream])
    161                         continue;
    162 
    163                 sof_end = list_first_entry(&sof_dai->endpoints,
    164                                            struct asoc_sdw_endpoint, list);
    165 
    166                 *be_id = sof_end->dai_info->dailink[stream];
    167                 if (*be_id < 0) {
    168                         dev_err(dev, "Invalid dailink id %d\n", *be_id);
    169                         return -EINVAL;
    170                 }
    171 
    172                 switch (amd_ctx->acp_rev) {
    173                 case ACP63_PCI_REV:
    174                         ret = get_acp63_cpu_pin_id(ffs(sof_end->link_mask - 1),
    175                                                    *be_id, &cpu_pin_id, dev);
    176                         if (ret)
    177                                 return ret;
    178                         break;
    179                 default:
    180                         return -EINVAL;
    181                 }
    182                 /* create stream name according to first link id */
    183                 if (ctx->append_dai_type) {
    184                         name = devm_kasprintf(dev, GFP_KERNEL,
    185                                               sdw_stream_name[stream + 2],
    186                                               ffs(sof_end->link_mask) - 1,
    187                                               cpu_pin_id,
    188                                               type_strings[sof_end->dai_info->dai_type]);
    189                 } else {
    190                         name = devm_kasprintf(dev, GFP_KERNEL,
    191                                               sdw_stream_name[stream],
    192                                               ffs(sof_end->link_mask) - 1,
    193                                               cpu_pin_id);
    194                 }
    195                 if (!name)
    196                         return -ENOMEM;
    197 
    198                 cpus = devm_kcalloc(dev, num_cpus, sizeof(*cpus), GFP_KERNEL);
    199                 if (!cpus)
    200                         return -ENOMEM;
    201 
    202                 codecs = devm_kcalloc(dev, num_codecs, sizeof(*codecs), GFP_KERNEL);
    203                 if (!codecs)
    204                         return -ENOMEM;
    205 
    206                 codec_maps = devm_kcalloc(dev, num_codecs, sizeof(*codec_maps), GFP_KERNEL);
    207                 if (!codec_maps)
    208                         return -ENOMEM;
    209 
    210                 list_for_each_entry(sof_end, &sof_dai->endpoints, list) {
    211                         if (!sof_end->dai_info->direction[stream])
    212                                 continue;
    213 
    214                         int link_num = ffs(sof_end->link_mask) - 1;
    215 
    216                         cpus[i].dai_name = devm_kasprintf(dev, GFP_KERNEL,

i is always zero so this just sets the first element over and over.

    217                                                           "SDW%d Pin%d",
    218                                                           link_num, cpu_pin_id);
    219                         dev_dbg(dev, "cpu[%d].dai_name:%s\n", i, cpus[i].dai_name);
    220                         if (!cpus[i].dai_name)
    221                                 return -ENOMEM;
    222 
    223                         codec_maps[j].cpu = i;
    224                         codec_maps[j].codec = j;
    225 
    226                         codecs[j].name = sof_end->codec_name;
    227                         codecs[j].dai_name = sof_end->dai_info->dai_name;
    228                         j++;
    229                 }
    230 
    231                 WARN_ON(j != num_codecs);
    232 
    233                 playback = (stream == SNDRV_PCM_STREAM_PLAYBACK);
    234                 capture = (stream == SNDRV_PCM_STREAM_CAPTURE);
    235 
    236                 asoc_sdw_init_dai_link(dev, *dai_links, be_id, name, playback, capture,
    237                                        cpus, num_cpus, platform_component,
    238                                        ARRAY_SIZE(platform_component), codecs, num_codecs,
    239                                        asoc_sdw_rtd_init, &sdw_ops);
    240 
    241                 /*
    242                  * SoundWire DAILINKs use 'stream' functions and Bank Switch operations
    243                  * based on wait_for_completion(), tag them as 'nonatomic'.
    244                  */
    245                 (*dai_links)->nonatomic = true;
    246                 (*dai_links)->ch_maps = codec_maps;
    247 
    248                 list_for_each_entry(sof_end, &sof_dai->endpoints, list) {
    249                         if (sof_end->dai_info->init)
    250                                 sof_end->dai_info->init(card, *dai_links,
    251                                                         sof_end->codec_info,
    252                                                         playback);
    253                 }
    254 
    255                 (*dai_links)++;
    256         }
    257 
    258         return 0;
    259 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [bug report] ASoC: amd: acp: refactor SoundWire machine driver code
@ 2024-09-27 10:39 Dan Carpenter
  2024-09-28  6:04 ` Mukunda,Vijendar
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2024-09-27 10:39 UTC (permalink / raw)
  To: Vijendar Mukunda; +Cc: linux-sound

Hello Vijendar Mukunda,

Commit 6d8348ddc56e ("ASoC: amd: acp: refactor SoundWire machine
driver code") from Sep 13, 2024 (linux-next), leads to the following
Smatch static checker warning:

	sound/soc/amd/acp/acp-sdw-sof-mach.c:365 sof_card_dai_links_create()
	warn: inconsistent indenting

sound/soc/amd/acp/acp-sdw-sof-mach.c
    307 static int sof_card_dai_links_create(struct snd_soc_card *card)
    308 {
    309         struct device *dev = card->dev;
    310         struct snd_soc_acpi_mach *mach = dev_get_platdata(card->dev);
    311         int sdw_be_num = 0, dmic_num = 0;
    312         struct asoc_sdw_mc_private *ctx = snd_soc_card_get_drvdata(card);
    313         struct snd_soc_acpi_mach_params *mach_params = &mach->mach_params;
    314         struct snd_soc_codec_conf *codec_conf;
    315         struct asoc_sdw_endpoint *sof_ends;
    316         struct asoc_sdw_dailink *sof_dais;

Just declare these as:

	struct asoc_sdw_endpoint *sof_ends __free(kfree) = NULL;
	struct asoc_sdw_dailink *sof_dais __free(kfree) = NULL;

Then you can delete all the gotos.

    317         struct snd_soc_dai_link *dai_links;
    318         int num_devs = 0;
    319         int num_ends = 0;
    320         int num_links;
    321         int be_id = 0;
    322         int ret;
    323 
    324         ret = asoc_sdw_count_sdw_endpoints(card, &num_devs, &num_ends);
    325         if (ret < 0) {
    326                 dev_err(dev, "failed to count devices/endpoints: %d\n", ret);
    327                 return ret;
    328         }
    329 
    330         /* One per DAI link, worst case is a DAI link for every endpoint */
    331         sof_dais = kcalloc(num_ends, sizeof(*sof_dais), GFP_KERNEL);
    332         if (!sof_dais)
    333                 return -ENOMEM;
    334 
    335         /* One per endpoint, ie. each DAI on each codec/amp */
    336         sof_ends = kcalloc(num_ends, sizeof(*sof_ends), GFP_KERNEL);
    337         if (!sof_ends) {
    338                 ret = -ENOMEM;
    339                 goto err_dai;
    340         }
    341 
    342         ret = asoc_sdw_parse_sdw_endpoints(card, sof_dais, sof_ends, &num_devs);
    343         if (ret < 0)
    344                 goto err_end;
    345 
    346         sdw_be_num = ret;
    347 
    348         /* enable dmic */
    349         if (sof_sdw_quirk & ASOC_SDW_ACP_DMIC || mach_params->dmic_num)
    350                 dmic_num = 1;
    351 
    352         dev_dbg(dev, "sdw %d, dmic %d", sdw_be_num, dmic_num);
    353 
    354         codec_conf = devm_kcalloc(dev, num_devs, sizeof(*codec_conf), GFP_KERNEL);
    355         if (!codec_conf) {
    356                 ret = -ENOMEM;
    357                 goto err_end;
    358         }
    359 
    360         /* allocate BE dailinks */
    361         num_links = sdw_be_num + dmic_num;
    362         dai_links = devm_kcalloc(dev, num_links, sizeof(*dai_links), GFP_KERNEL);
    363         if (!dai_links) {
    364                 ret = -ENOMEM;
--> 365         goto err_end;

Missing tab.

    366         }
    367 
    368         card->codec_conf = codec_conf;
    369         card->num_configs = num_devs;
    370         card->dai_link = dai_links;
    371         card->num_links = num_links;
    372 
    373         /* SDW */
    374         if (sdw_be_num) {
    375                 ret = create_sdw_dailinks(card, &dai_links, &be_id,
    376                                           sof_dais, &codec_conf);
    377                 if (ret)
    378                         goto err_end;
    379         }
    380 
    381         /* dmic */
    382         if (dmic_num > 0) {
    383                 if (ctx->ignore_internal_dmic) {
    384                         dev_warn(dev, "Ignoring ACP DMIC\n");
    385                 } else {
    386                         ret = create_dmic_dailinks(card, &dai_links, &be_id);
    387                         if (ret)
    388                                 goto err_end;
    389                 }
    390         }
    391 
    392         WARN_ON(codec_conf != card->codec_conf + card->num_configs);
    393         WARN_ON(dai_links != card->dai_link + card->num_links);
    394 
    395 err_end:
    396         kfree(sof_ends);
    397 err_dai:
    398         kfree(sof_dais);
    399 
    400         return ret;
    401 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [bug report] ASoC: amd: acp: refactor SoundWire machine driver code
  2024-09-27 10:39 [bug report] ASoC: amd: acp: refactor SoundWire machine driver code Dan Carpenter
@ 2024-09-28  6:04 ` Mukunda,Vijendar
  0 siblings, 0 replies; 5+ messages in thread
From: Mukunda,Vijendar @ 2024-09-28  6:04 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-sound

On 27/09/24 16:09, Dan Carpenter wrote:
> Hello Vijendar Mukunda,
>
> Commit 6d8348ddc56e ("ASoC: amd: acp: refactor SoundWire machine
> driver code") from Sep 13, 2024 (linux-next), leads to the following
> Smatch static checker warning:
>
> 	sound/soc/amd/acp/acp-sdw-sof-mach.c:365 sof_card_dai_links_create()
> 	warn: inconsistent indenting
>
> sound/soc/amd/acp/acp-sdw-sof-mach.c
>     307 static int sof_card_dai_links_create(struct snd_soc_card *card)
>     308 {
>     309         struct device *dev = card->dev;
>     310         struct snd_soc_acpi_mach *mach = dev_get_platdata(card->dev);
>     311         int sdw_be_num = 0, dmic_num = 0;
>     312         struct asoc_sdw_mc_private *ctx = snd_soc_card_get_drvdata(card);
>     313         struct snd_soc_acpi_mach_params *mach_params = &mach->mach_params;
>     314         struct snd_soc_codec_conf *codec_conf;
>     315         struct asoc_sdw_endpoint *sof_ends;
>     316         struct asoc_sdw_dailink *sof_dais;
>
> Just declare these as:
>
> 	struct asoc_sdw_endpoint *sof_ends __free(kfree) = NULL;
> 	struct asoc_sdw_dailink *sof_dais __free(kfree) = NULL;
>
> Then you can delete all the gotos.
Will fix it.
>     317         struct snd_soc_dai_link *dai_links;
>     318         int num_devs = 0;
>     319         int num_ends = 0;
>     320         int num_links;
>     321         int be_id = 0;
>     322         int ret;
>     323 
>     324         ret = asoc_sdw_count_sdw_endpoints(card, &num_devs, &num_ends);
>     325         if (ret < 0) {
>     326                 dev_err(dev, "failed to count devices/endpoints: %d\n", ret);
>     327                 return ret;
>     328         }
>     329 
>     330         /* One per DAI link, worst case is a DAI link for every endpoint */
>     331         sof_dais = kcalloc(num_ends, sizeof(*sof_dais), GFP_KERNEL);
>     332         if (!sof_dais)
>     333                 return -ENOMEM;
>     334 
>     335         /* One per endpoint, ie. each DAI on each codec/amp */
>     336         sof_ends = kcalloc(num_ends, sizeof(*sof_ends), GFP_KERNEL);
>     337         if (!sof_ends) {
>     338                 ret = -ENOMEM;
>     339                 goto err_dai;
>     340         }
>     341 
>     342         ret = asoc_sdw_parse_sdw_endpoints(card, sof_dais, sof_ends, &num_devs);
>     343         if (ret < 0)
>     344                 goto err_end;
>     345 
>     346         sdw_be_num = ret;
>     347 
>     348         /* enable dmic */
>     349         if (sof_sdw_quirk & ASOC_SDW_ACP_DMIC || mach_params->dmic_num)
>     350                 dmic_num = 1;
>     351 
>     352         dev_dbg(dev, "sdw %d, dmic %d", sdw_be_num, dmic_num);
>     353 
>     354         codec_conf = devm_kcalloc(dev, num_devs, sizeof(*codec_conf), GFP_KERNEL);
>     355         if (!codec_conf) {
>     356                 ret = -ENOMEM;
>     357                 goto err_end;
>     358         }
>     359 
>     360         /* allocate BE dailinks */
>     361         num_links = sdw_be_num + dmic_num;
>     362         dai_links = devm_kcalloc(dev, num_links, sizeof(*dai_links), GFP_KERNEL);
>     363         if (!dai_links) {
>     364                 ret = -ENOMEM;
> --> 365         goto err_end;
>
> Missing tab.
Will fix it
>
>     366         }
>     367 
>     368         card->codec_conf = codec_conf;
>     369         card->num_configs = num_devs;
>     370         card->dai_link = dai_links;
>     371         card->num_links = num_links;
>     372 
>     373         /* SDW */
>     374         if (sdw_be_num) {
>     375                 ret = create_sdw_dailinks(card, &dai_links, &be_id,
>     376                                           sof_dais, &codec_conf);
>     377                 if (ret)
>     378                         goto err_end;
>     379         }
>     380 
>     381         /* dmic */
>     382         if (dmic_num > 0) {
>     383                 if (ctx->ignore_internal_dmic) {
>     384                         dev_warn(dev, "Ignoring ACP DMIC\n");
>     385                 } else {
>     386                         ret = create_dmic_dailinks(card, &dai_links, &be_id);
>     387                         if (ret)
>     388                                 goto err_end;
>     389                 }
>     390         }
>     391 
>     392         WARN_ON(codec_conf != card->codec_conf + card->num_configs);
>     393         WARN_ON(dai_links != card->dai_link + card->num_links);
>     394 
>     395 err_end:
>     396         kfree(sof_ends);
>     397 err_dai:
>     398         kfree(sof_dais);
>     399 
>     400         return ret;
>     401 }
>
> regards,
> dan carpenter


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [bug report] ASoC: amd: acp: refactor SoundWire machine driver code
  2024-09-27 10:35 Dan Carpenter
@ 2024-10-03  6:28 ` Mukunda,Vijendar
  2024-10-03  6:38   ` Mukunda,Vijendar
  0 siblings, 1 reply; 5+ messages in thread
From: Mukunda,Vijendar @ 2024-10-03  6:28 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-sound

On 27/09/24 16:05, Dan Carpenter wrote:
> Hello Vijendar Mukunda,
>
> Commit 6d8348ddc56e ("ASoC: amd: acp: refactor SoundWire machine
> driver code") from Sep 13, 2024 (linux-next), leads to the following
> Smatch static checker warning:
>
> 	sound/soc/amd/acp/acp-sdw-sof-mach.c:157 create_sdw_dailink()
> 	warn: iterator 'i' not incremented
>
> sound/soc/amd/acp/acp-sdw-sof-mach.c
>     144         for_each_pcm_streams(stream) {
>     145                 static const char * const sdw_stream_name[] = {
>     146                         "SDW%d-PIN%d-PLAYBACK",
>     147                         "SDW%d-PIN%d-CAPTURE",
>     148                         "SDW%d-PIN%d-PLAYBACK-%s",
>     149                         "SDW%d-PIN%d-CAPTURE-%s",
>     150                 };
>     151                 struct snd_soc_dai_link_ch_map *codec_maps;
>     152                 struct snd_soc_dai_link_component *codecs;
>     153                 struct snd_soc_dai_link_component *cpus;
>     154                 int num_cpus = hweight32(sof_dai->link_mask[stream]);
>     155                 int num_codecs = sof_dai->num_devs[stream];
>     156                 int playback, capture;
> --> 157                 int i = 0, j = 0;
>     158                 char *name;
>     159 
>     160                 if (!sof_dai->num_devs[stream])
>     161                         continue;
>     162 
>     163                 sof_end = list_first_entry(&sof_dai->endpoints,
>     164                                            struct asoc_sdw_endpoint, list);
>     165 
>     166                 *be_id = sof_end->dai_info->dailink[stream];
>     167                 if (*be_id < 0) {
>     168                         dev_err(dev, "Invalid dailink id %d\n", *be_id);
>     169                         return -EINVAL;
>     170                 }
>     171 
>     172                 switch (amd_ctx->acp_rev) {
>     173                 case ACP63_PCI_REV:
>     174                         ret = get_acp63_cpu_pin_id(ffs(sof_end->link_mask - 1),
>     175                                                    *be_id, &cpu_pin_id, dev);
>     176                         if (ret)
>     177                                 return ret;
>     178                         break;
>     179                 default:
>     180                         return -EINVAL;
>     181                 }
>     182                 /* create stream name according to first link id */
>     183                 if (ctx->append_dai_type) {
>     184                         name = devm_kasprintf(dev, GFP_KERNEL,
>     185                                               sdw_stream_name[stream + 2],
>     186                                               ffs(sof_end->link_mask) - 1,
>     187                                               cpu_pin_id,
>     188                                               type_strings[sof_end->dai_info->dai_type]);
>     189                 } else {
>     190                         name = devm_kasprintf(dev, GFP_KERNEL,
>     191                                               sdw_stream_name[stream],
>     192                                               ffs(sof_end->link_mask) - 1,
>     193                                               cpu_pin_id);
>     194                 }
>     195                 if (!name)
>     196                         return -ENOMEM;
>     197 
>     198                 cpus = devm_kcalloc(dev, num_cpus, sizeof(*cpus), GFP_KERNEL);
>     199                 if (!cpus)
>     200                         return -ENOMEM;
>     201 
>     202                 codecs = devm_kcalloc(dev, num_codecs, sizeof(*codecs), GFP_KERNEL);
>     203                 if (!codecs)
>     204                         return -ENOMEM;
>     205 
>     206                 codec_maps = devm_kcalloc(dev, num_codecs, sizeof(*codec_maps), GFP_KERNEL);
>     207                 if (!codec_maps)
>     208                         return -ENOMEM;
>     209 
>     210                 list_for_each_entry(sof_end, &sof_dai->endpoints, list) {
>     211                         if (!sof_end->dai_info->direction[stream])
>     212                                 continue;
>     213 
>     214                         int link_num = ffs(sof_end->link_mask) - 1;
>     215 
>     216                         cpus[i].dai_name = devm_kasprintf(dev, GFP_KERNEL,
>
> i is always zero so this just sets the first element over and over.
Currently, multi-link aggregation is not supported. i.e 'i' variable
will always be zero. 'i' variable can be dropped and index can be
hardcoded something like cpus[0].dai_name.


>
>     217                                                           "SDW%d Pin%d",
>     218                                                           link_num, cpu_pin_id);
>     219                         dev_dbg(dev, "cpu[%d].dai_name:%s\n", i, cpus[i].dai_name);
>     220                         if (!cpus[i].dai_name)
>     221                                 return -ENOMEM;
>     222 
>     223                         codec_maps[j].cpu = i;
>     224                         codec_maps[j].codec = j;
>     225 
>     226                         codecs[j].name = sof_end->codec_name;
>     227                         codecs[j].dai_name = sof_end->dai_info->dai_name;
>     228                         j++;
>     229                 }
>     230 
>     231                 WARN_ON(j != num_codecs);
>     232 
>     233                 playback = (stream == SNDRV_PCM_STREAM_PLAYBACK);
>     234                 capture = (stream == SNDRV_PCM_STREAM_CAPTURE);
>     235 
>     236                 asoc_sdw_init_dai_link(dev, *dai_links, be_id, name, playback, capture,
>     237                                        cpus, num_cpus, platform_component,
>     238                                        ARRAY_SIZE(platform_component), codecs, num_codecs,
>     239                                        asoc_sdw_rtd_init, &sdw_ops);
>     240 
>     241                 /*
>     242                  * SoundWire DAILINKs use 'stream' functions and Bank Switch operations
>     243                  * based on wait_for_completion(), tag them as 'nonatomic'.
>     244                  */
>     245                 (*dai_links)->nonatomic = true;
>     246                 (*dai_links)->ch_maps = codec_maps;
>     247 
>     248                 list_for_each_entry(sof_end, &sof_dai->endpoints, list) {
>     249                         if (sof_end->dai_info->init)
>     250                                 sof_end->dai_info->init(card, *dai_links,
>     251                                                         sof_end->codec_info,
>     252                                                         playback);
>     253                 }
>     254 
>     255                 (*dai_links)++;
>     256         }
>     257 
>     258         return 0;
>     259 }
>
> regards,
> dan carpenter


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [bug report] ASoC: amd: acp: refactor SoundWire machine driver code
  2024-10-03  6:28 ` Mukunda,Vijendar
@ 2024-10-03  6:38   ` Mukunda,Vijendar
  0 siblings, 0 replies; 5+ messages in thread
From: Mukunda,Vijendar @ 2024-10-03  6:38 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-sound

On 03/10/24 11:58, Mukunda,Vijendar wrote:
> On 27/09/24 16:05, Dan Carpenter wrote:
>> Hello Vijendar Mukunda,
>>
>> Commit 6d8348ddc56e ("ASoC: amd: acp: refactor SoundWire machine
>> driver code") from Sep 13, 2024 (linux-next), leads to the following
>> Smatch static checker warning:
>>
>> 	sound/soc/amd/acp/acp-sdw-sof-mach.c:157 create_sdw_dailink()
>> 	warn: iterator 'i' not incremented
>>
>> sound/soc/amd/acp/acp-sdw-sof-mach.c
>>     144         for_each_pcm_streams(stream) {
>>     145                 static const char * const sdw_stream_name[] = {
>>     146                         "SDW%d-PIN%d-PLAYBACK",
>>     147                         "SDW%d-PIN%d-CAPTURE",
>>     148                         "SDW%d-PIN%d-PLAYBACK-%s",
>>     149                         "SDW%d-PIN%d-CAPTURE-%s",
>>     150                 };
>>     151                 struct snd_soc_dai_link_ch_map *codec_maps;
>>     152                 struct snd_soc_dai_link_component *codecs;
>>     153                 struct snd_soc_dai_link_component *cpus;
>>     154                 int num_cpus = hweight32(sof_dai->link_mask[stream]);
>>     155                 int num_codecs = sof_dai->num_devs[stream];
>>     156                 int playback, capture;
>> --> 157                 int i = 0, j = 0;
>>     158                 char *name;
>>     159 
>>     160                 if (!sof_dai->num_devs[stream])
>>     161                         continue;
>>     162 
>>     163                 sof_end = list_first_entry(&sof_dai->endpoints,
>>     164                                            struct asoc_sdw_endpoint, list);
>>     165 
>>     166                 *be_id = sof_end->dai_info->dailink[stream];
>>     167                 if (*be_id < 0) {
>>     168                         dev_err(dev, "Invalid dailink id %d\n", *be_id);
>>     169                         return -EINVAL;
>>     170                 }
>>     171 
>>     172                 switch (amd_ctx->acp_rev) {
>>     173                 case ACP63_PCI_REV:
>>     174                         ret = get_acp63_cpu_pin_id(ffs(sof_end->link_mask - 1),
>>     175                                                    *be_id, &cpu_pin_id, dev);
>>     176                         if (ret)
>>     177                                 return ret;
>>     178                         break;
>>     179                 default:
>>     180                         return -EINVAL;
>>     181                 }
>>     182                 /* create stream name according to first link id */
>>     183                 if (ctx->append_dai_type) {
>>     184                         name = devm_kasprintf(dev, GFP_KERNEL,
>>     185                                               sdw_stream_name[stream + 2],
>>     186                                               ffs(sof_end->link_mask) - 1,
>>     187                                               cpu_pin_id,
>>     188                                               type_strings[sof_end->dai_info->dai_type]);
>>     189                 } else {
>>     190                         name = devm_kasprintf(dev, GFP_KERNEL,
>>     191                                               sdw_stream_name[stream],
>>     192                                               ffs(sof_end->link_mask) - 1,
>>     193                                               cpu_pin_id);
>>     194                 }
>>     195                 if (!name)
>>     196                         return -ENOMEM;
>>     197 
>>     198                 cpus = devm_kcalloc(dev, num_cpus, sizeof(*cpus), GFP_KERNEL);
>>     199                 if (!cpus)
>>     200                         return -ENOMEM;
>>     201 
>>     202                 codecs = devm_kcalloc(dev, num_codecs, sizeof(*codecs), GFP_KERNEL);
>>     203                 if (!codecs)
>>     204                         return -ENOMEM;
>>     205 
>>     206                 codec_maps = devm_kcalloc(dev, num_codecs, sizeof(*codec_maps), GFP_KERNEL);
>>     207                 if (!codec_maps)
>>     208                         return -ENOMEM;
>>     209 
>>     210                 list_for_each_entry(sof_end, &sof_dai->endpoints, list) {
>>     211                         if (!sof_end->dai_info->direction[stream])
>>     212                                 continue;
>>     213 
>>     214                         int link_num = ffs(sof_end->link_mask) - 1;
>>     215 
>>     216                         cpus[i].dai_name = devm_kasprintf(dev, GFP_KERNEL,
>>
>> i is always zero so this just sets the first element over and over.
> Currently, multi-link aggregation is not supported. i.e 'i' variable
> will always be zero. 'i' variable can be dropped and index can be
> hardcoded something like cpus[0].dai_name.

Small correction.  Array indexing is also not required here as single cpu dai
being created.
>
>
>>     217                                                           "SDW%d Pin%d",
>>     218                                                           link_num, cpu_pin_id);
>>     219                         dev_dbg(dev, "cpu[%d].dai_name:%s\n", i, cpus[i].dai_name);
>>     220                         if (!cpus[i].dai_name)
>>     221                                 return -ENOMEM;
>>     222 
>>     223                         codec_maps[j].cpu = i;
>>     224                         codec_maps[j].codec = j;
>>     225 
>>     226                         codecs[j].name = sof_end->codec_name;
>>     227                         codecs[j].dai_name = sof_end->dai_info->dai_name;
>>     228                         j++;
>>     229                 }
>>     230 
>>     231                 WARN_ON(j != num_codecs);
>>     232 
>>     233                 playback = (stream == SNDRV_PCM_STREAM_PLAYBACK);
>>     234                 capture = (stream == SNDRV_PCM_STREAM_CAPTURE);
>>     235 
>>     236                 asoc_sdw_init_dai_link(dev, *dai_links, be_id, name, playback, capture,
>>     237                                        cpus, num_cpus, platform_component,
>>     238                                        ARRAY_SIZE(platform_component), codecs, num_codecs,
>>     239                                        asoc_sdw_rtd_init, &sdw_ops);
>>     240 
>>     241                 /*
>>     242                  * SoundWire DAILINKs use 'stream' functions and Bank Switch operations
>>     243                  * based on wait_for_completion(), tag them as 'nonatomic'.
>>     244                  */
>>     245                 (*dai_links)->nonatomic = true;
>>     246                 (*dai_links)->ch_maps = codec_maps;
>>     247 
>>     248                 list_for_each_entry(sof_end, &sof_dai->endpoints, list) {
>>     249                         if (sof_end->dai_info->init)
>>     250                                 sof_end->dai_info->init(card, *dai_links,
>>     251                                                         sof_end->codec_info,
>>     252                                                         playback);
>>     253                 }
>>     254 
>>     255                 (*dai_links)++;
>>     256         }
>>     257 
>>     258         return 0;
>>     259 }
>>
>> regards,
>> dan carpenter


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-10-03  6:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-27 10:39 [bug report] ASoC: amd: acp: refactor SoundWire machine driver code Dan Carpenter
2024-09-28  6:04 ` Mukunda,Vijendar
  -- strict thread matches above, loose matches on Subject: below --
2024-09-27 10:35 Dan Carpenter
2024-10-03  6:28 ` Mukunda,Vijendar
2024-10-03  6:38   ` Mukunda,Vijendar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox