Hello Peter Ujfalusi,
The patch 2a68ff846164: "ASoC: SOF: Intel: hda: Revisit IMR boot
sequence" from Apr 21, 2022, leads to the following Smatch static
checker warning:
sound/soc/sof/intel/hda-loader.c:397 hda_dsp_cl_boot_firmware()
info: return a literal instead of 'ret'
sound/soc/sof/intel/hda-loader.c
381 int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev)
382 {
383 struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
384 struct snd_sof_pdata *plat_data = sdev->pdata;
385 const struct sof_dev_desc *desc = plat_data->desc;
386 const struct sof_intel_dsp_desc *chip_info;
387 struct hdac_ext_stream *hext_stream;
388 struct firmware stripped_firmware;
389 struct snd_dma_buffer dmab;
390 int ret, ret1, i;
391
392 if (hda->imrboot_supported && !sdev->first_boot) {
393 dev_dbg(sdev->dev, "IMR restore supported, booting from IMR directly\n");
394 hda->boot_iteration = 0;
395 ret = hda_dsp_boot_imr(sdev);
396 if (ret >= 0)
--> 397 return ret;
The hda_dsp_boot_imr() has some similar stuff where it checks for
positive returns. As far as I can see, this code never returns positive
values. Normally kernel code returns zero on success and negative
error codes on failure. When code returns non-standard things then it
really should be documented what the positive returns mean. Nothing
complicated, just add a comment at the start of the function.
The TLDR back story of this Smatch check is that it's not published but
it regularly finds bugs. The issue is that it's more readable, plus it
looks more deliberate and intentional to write:
if (!ret)
return 0;
instead of:
if (!ret)
return ret;
With the latter format, the bug is that people intended to write:
if (ret)
return ret;
Obviously this kind of bug would get caught in testing, but testing is
often impossible in the kernel because it depends on hardware
availability.
398
399 dev_warn(sdev->dev, "IMR restore failed, trying to cold boot\n");
400 }
401
402 chip_info = desc->chip_info;
403
regards,
dan carpenter
