[bug report] ASoC: amd: add vangogh machine driver

[bug report] ASoC: amd: add vangogh machine driver

[Dan Carpenter]

Hello Vijendar Mukunda,

The patch 34a0094b9ff7: "ASoC: amd: add vangogh machine driver" from
Oct 14, 2021, leads to the following Smatch static checker warning:

	sound/soc/amd/vangogh/acp5x-mach.c:190 acp5x_cs35l41_hw_params()
	error: uninitialized symbol 'ret'.

sound/soc/amd/vangogh/acp5x-mach.c
    158 static int acp5x_cs35l41_hw_params(struct snd_pcm_substream *substream,
    159                                    struct snd_pcm_hw_params *params)
    160 {
    161         struct snd_soc_pcm_runtime *rtd = substream->private_data;
    162         struct snd_soc_card *card = rtd->card;
    163         struct snd_soc_dai *codec_dai;
    164         int ret, i;
    165         unsigned int num_codecs = rtd->num_codecs;
    166         unsigned int bclk_val;
    167 
    168         for (i = 0; i < num_codecs; i++) {
    169                 codec_dai = asoc_rtd_to_codec(rtd, i);
    170                 if ((strcmp(codec_dai->name, "spi-VLV1776:00") == 0) ||
    171                     (strcmp(codec_dai->name, "spi-VLV1776:01") == 0)) {

How positive are we that we're always going to find one of these codecs?
Smatch is worried we might not find them.

    172                         switch (params_rate(params)) {
    173                         case 48000:
    174                                 bclk_val = 1536000;
    175                                 break;
    176                         default:
    177                                 dev_err(card->dev, "Invalid Samplerate:0x%x\n",
    178                                         params_rate(params));
    179                                 return -EINVAL;
    180                         }
    181                         ret = snd_soc_component_set_sysclk(codec_dai->component,
    182                                                            0, 0, bclk_val, SND_SOC_CLOCK_IN);
    183                         if (ret < 0) {
    184                                 dev_err(card->dev, "failed to set sysclk for CS35l41 dai\n");
    185                                 return ret;
    186                         }
    187                 }
    188         }
    189 
--> 190         return ret;
                ^^^^^^^^^^
Also it's a bit more readable to "return 0;" if we know this is a
success path.

    191 }

regards,
dan carpenter

Re: [bug report] ASoC: amd: add vangogh machine driver

[Mukunda,Vijendar]

On 11/25/21 8:29 PM, Dan Carpenter wrote:
> Hello Vijendar Mukunda,
> 
> The patch 34a0094b9ff7: "ASoC: amd: add vangogh machine driver" from
> Oct 14, 2021, leads to the following Smatch static checker warning:
> 
> 	sound/soc/amd/vangogh/acp5x-mach.c:190 acp5x_cs35l41_hw_params()
> 	error: uninitialized symbol 'ret'.

will fix it.
> 
> sound/soc/amd/vangogh/acp5x-mach.c
>     158 static int acp5x_cs35l41_hw_params(struct snd_pcm_substream *substream,
>     159                                    struct snd_pcm_hw_params *params)
>     160 {
>     161         struct snd_soc_pcm_runtime *rtd = substream->private_data;
>     162         struct snd_soc_card *card = rtd->card;
>     163         struct snd_soc_dai *codec_dai;
>     164         int ret, i;
>     165         unsigned int num_codecs = rtd->num_codecs;
>     166         unsigned int bclk_val;
>     167 
>     168         for (i = 0; i < num_codecs; i++) {
>     169                 codec_dai = asoc_rtd_to_codec(rtd, i);
>     170                 if ((strcmp(codec_dai->name, "spi-VLV1776:00") == 0) ||
>     171                     (strcmp(codec_dai->name, "spi-VLV1776:01") == 0)) {
> 
> How positive are we that we're always going to find one of these codecs?
> Smatch is worried we might not find them.
This code executed for CS35l41 codec DAI's only on VG platform. If both
the Codec dai's didn't found, sound card won't get registered. we have
added condition check so that for only these codec dai's set sysclk
change should be applied.

> 
>     172                         switch (params_rate(params)) {
>     173                         case 48000:
>     174                                 bclk_val = 1536000;
>     175                                 break;
>     176                         default:
>     177                                 dev_err(card->dev, "Invalid Samplerate:0x%x\n",
>     178                                         params_rate(params));
>     179                                 return -EINVAL;
>     180                         }
>     181                         ret = snd_soc_component_set_sysclk(codec_dai->component,
>     182                                                            0, 0, bclk_val, SND_SOC_CLOCK_IN);
>     183                         if (ret < 0) {
>     184                                 dev_err(card->dev, "failed to set sysclk for CS35l41 dai\n");
>     185                                 return ret;
>     186                         }
>     187                 }
>     188         }
>     189 
> --> 190         return ret;
>                 ^^^^^^^^^^
> Also it's a bit more readable to "return 0;" if we know this is a
> success path.
will fix it.
> 
>     191 }
> 
> regards,
> dan carpenter
>