From: "Mukunda,Vijendar" <vijendar.mukunda@amd.com>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: linux-sound@vger.kernel.org
Subject: Re: [bug report] ASoC: amd: acp: refactor SoundWire machine driver code
Date: Thu, 3 Oct 2024 11:58:33 +0530 [thread overview]
Message-ID: <4c1d9af5-6bab-45a3-b588-c0e41f5ed84a@amd.com> (raw)
In-Reply-To: <d245bb1f-27ee-4a7f-a5be-6b2573bc2c8a@stanley.mountain>
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
next prev parent reply other threads:[~2024-10-03 6:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-27 10:35 [bug report] ASoC: amd: acp: refactor SoundWire machine driver code Dan Carpenter
2024-10-03 6:28 ` Mukunda,Vijendar [this message]
2024-10-03 6:38 ` Mukunda,Vijendar
-- strict thread matches above, loose matches on Subject: below --
2024-09-27 10:39 Dan Carpenter
2024-09-28 6:04 ` Mukunda,Vijendar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4c1d9af5-6bab-45a3-b588-c0e41f5ed84a@amd.com \
--to=vijendar.mukunda@amd.com \
--cc=dan.carpenter@linaro.org \
--cc=linux-sound@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox