Re: [bug report] ASoC: amd: acp: refactor SoundWire machine driver code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



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





[Index of Archives]     [Pulseaudio]     [Linux Audio Users]     [ALSA Devel]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux