[PATCH v2] SoC: mediatek: mt8188-mt6359: clean up a return in codec_init

[PATCH v2] SoC: mediatek: mt8188-mt6359: clean up a return in codec_init

[Dan Carpenter]

This code triggers a Smatch static checker warning and does sort of
look like an error path.

sound/soc/mediatek/mt8188/mt8188-mt6359.c:597 mt8188_max98390_codec_init() warn: missing error code? 'ret'

However, returning 0 is intentional.  Make that explicit.

Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
v2: At first I thought this was an error path but it is a not.

 sound/soc/mediatek/mt8188/mt8188-mt6359.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
index bc4b74970a46..5ad8d2a5bbc4 100644
--- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
+++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
@@ -594,7 +594,7 @@ static int mt8188_max98390_codec_init(struct snd_soc_pcm_runtime *rtd)
 	}
 
 	if (rtd->dai_link->num_codecs <= 2)
-		return ret;
+		return 0;
 
 	/* add widgets/controls/dapm for rear speakers */
 	ret = snd_soc_dapm_new_controls(&card->dapm, mt8188_rear_spk_widgets,
-- 
2.39.2

[PATCH v2] SoC: mediatek: mt8188-mt6359: clean up a return in codec_init

[Dan Carpenter]

This code triggers a Smatch static checker warning and does sort of
look like an error path.

sound/soc/mediatek/mt8188/mt8188-mt6359.c:597 mt8188_max98390_codec_init() warn: missing error code? 'ret'

However, returning 0 is intentional.  Make that explicit.

Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
v2: At first I thought this was an error path but it is a not.

 sound/soc/mediatek/mt8188/mt8188-mt6359.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
index bc4b74970a46..5ad8d2a5bbc4 100644
--- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
+++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
@@ -594,7 +594,7 @@ static int mt8188_max98390_codec_init(struct snd_soc_pcm_runtime *rtd)
 	}
 
 	if (rtd->dai_link->num_codecs <= 2)
-		return ret;
+		return 0;
 
 	/* add widgets/controls/dapm for rear speakers */
 	ret = snd_soc_dapm_new_controls(&card->dapm, mt8188_rear_spk_widgets,
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] SoC: mediatek: mt8188-mt6359: clean up a return in codec_init

[Trevor Wu (吳文良)]

On Wed, 2023-06-07 at 17:57 +0300, Dan Carpenter wrote:
>  This code triggers a Smatch static checker warning and does sort of
> look like an error path.
> 
> sound/soc/mediatek/mt8188/mt8188-mt6359.c:597
> mt8188_max98390_codec_init() warn: missing error code? 'ret'
> 
> However, returning 0 is intentional.  Make that explicit.

Hi Dan,

There are several similar instances in this file. If using 'return
ret;' is not recommended, should we update all of them simultaneously?

For instance, the value of 'ret' at the end of this function should
also be zero.

Thanks,
Trevor

> 
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> v2: At first I thought this was an error path but it is a not.
> 
>  sound/soc/mediatek/mt8188/mt8188-mt6359.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> index bc4b74970a46..5ad8d2a5bbc4 100644
> --- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> +++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> @@ -594,7 +594,7 @@ static int mt8188_max98390_codec_init(struct
> snd_soc_pcm_runtime *rtd)
>  }
>  
>  if (rtd->dai_link->num_codecs <= 2)
> -return ret;
> +return 0;
>  
>  /* add widgets/controls/dapm for rear speakers */
>  ret = snd_soc_dapm_new_controls(&card->dapm,
> mt8188_rear_spk_widgets,
> -- 
> 2.39.2
> 

Re: [PATCH v2] SoC: mediatek: mt8188-mt6359: clean up a return in codec_init

[Trevor Wu (吳文良)]

On Wed, 2023-06-07 at 17:57 +0300, Dan Carpenter wrote:
>  This code triggers a Smatch static checker warning and does sort of
> look like an error path.
> 
> sound/soc/mediatek/mt8188/mt8188-mt6359.c:597
> mt8188_max98390_codec_init() warn: missing error code? 'ret'
> 
> However, returning 0 is intentional.  Make that explicit.

Hi Dan,

There are several similar instances in this file. If using 'return
ret;' is not recommended, should we update all of them simultaneously?

For instance, the value of 'ret' at the end of this function should
also be zero.

Thanks,
Trevor

> 
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> v2: At first I thought this was an error path but it is a not.
> 
>  sound/soc/mediatek/mt8188/mt8188-mt6359.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> index bc4b74970a46..5ad8d2a5bbc4 100644
> --- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> +++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> @@ -594,7 +594,7 @@ static int mt8188_max98390_codec_init(struct
> snd_soc_pcm_runtime *rtd)
>  }
>  
>  if (rtd->dai_link->num_codecs <= 2)
> -return ret;
> +return 0;
>  
>  /* add widgets/controls/dapm for rear speakers */
>  ret = snd_soc_dapm_new_controls(&card->dapm,
> mt8188_rear_spk_widgets,
> -- 
> 2.39.2
> 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] SoC: mediatek: mt8188-mt6359: clean up a return in codec_init

[AngeloGioacchino Del Regno]

Il 08/06/23 06:02, Trevor Wu (吳文良) ha scritto:
> On Wed, 2023-06-07 at 17:57 +0300, Dan Carpenter wrote:
>>   This code triggers a Smatch static checker warning and does sort of
>> look like an error path.
>>
>> sound/soc/mediatek/mt8188/mt8188-mt6359.c:597
>> mt8188_max98390_codec_init() warn: missing error code? 'ret'
>>
>> However, returning 0 is intentional.  Make that explicit.
> 
> Hi Dan,
> 
> There are several similar instances in this file. If using 'return
> ret;' is not recommended, should we update all of them simultaneously?
> 
> For instance, the value of 'ret' at the end of this function should
> also be zero.
> 

I'm already in the process of performing a further cleanup to this file,
will include Dan's patch in my series.

Please hold on (sending today).

Regards,
Angelo

> Thanks,
> Trevor
> 
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
>> ---
>> v2: At first I thought this was an error path but it is a not.
>>
>>   sound/soc/mediatek/mt8188/mt8188-mt6359.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
>> b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
>> index bc4b74970a46..5ad8d2a5bbc4 100644
>> --- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
>> +++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
>> @@ -594,7 +594,7 @@ static int mt8188_max98390_codec_init(struct
>> snd_soc_pcm_runtime *rtd)
>>   }
>>   
>>   if (rtd->dai_link->num_codecs <= 2)
>> -return ret;
>> +return 0;
>>   
>>   /* add widgets/controls/dapm for rear speakers */
>>   ret = snd_soc_dapm_new_controls(&card->dapm,
>> mt8188_rear_spk_widgets,
>> -- 
>> 2.39.2
>>

Re: [PATCH v2] SoC: mediatek: mt8188-mt6359: clean up a return in codec_init

[AngeloGioacchino Del Regno]

Il 08/06/23 06:02, Trevor Wu (吳文良) ha scritto:
> On Wed, 2023-06-07 at 17:57 +0300, Dan Carpenter wrote:
>>   This code triggers a Smatch static checker warning and does sort of
>> look like an error path.
>>
>> sound/soc/mediatek/mt8188/mt8188-mt6359.c:597
>> mt8188_max98390_codec_init() warn: missing error code? 'ret'
>>
>> However, returning 0 is intentional.  Make that explicit.
> 
> Hi Dan,
> 
> There are several similar instances in this file. If using 'return
> ret;' is not recommended, should we update all of them simultaneously?
> 
> For instance, the value of 'ret' at the end of this function should
> also be zero.
> 

I'm already in the process of performing a further cleanup to this file,
will include Dan's patch in my series.

Please hold on (sending today).

Regards,
Angelo

> Thanks,
> Trevor
> 
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
>> ---
>> v2: At first I thought this was an error path but it is a not.
>>
>>   sound/soc/mediatek/mt8188/mt8188-mt6359.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
>> b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
>> index bc4b74970a46..5ad8d2a5bbc4 100644
>> --- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
>> +++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
>> @@ -594,7 +594,7 @@ static int mt8188_max98390_codec_init(struct
>> snd_soc_pcm_runtime *rtd)
>>   }
>>   
>>   if (rtd->dai_link->num_codecs <= 2)
>> -return ret;
>> +return 0;
>>   
>>   /* add widgets/controls/dapm for rear speakers */
>>   ret = snd_soc_dapm_new_controls(&card->dapm,
>> mt8188_rear_spk_widgets,
>> -- 
>> 2.39.2
>>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] SoC: mediatek: mt8188-mt6359: clean up a return in codec_init

[Dan Carpenter]

On Thu, Jun 08, 2023 at 04:02:07AM +0000, Trevor Wu (吳文良) wrote:
> On Wed, 2023-06-07 at 17:57 +0300, Dan Carpenter wrote:
> >  This code triggers a Smatch static checker warning and does sort of
> > look like an error path.
> > 
> > sound/soc/mediatek/mt8188/mt8188-mt6359.c:597
> > mt8188_max98390_codec_init() warn: missing error code? 'ret'
> > 
> > However, returning 0 is intentional.  Make that explicit.
> 
> Hi Dan,
> 
> There are several similar instances in this file. If using 'return
> ret;' is not recommended, should we update all of them simultaneously?
> 
> For instance, the value of 'ret' at the end of this function should
> also be zero.
> 

I would prefer that, yes.  "return 0;" is always more clear than
"return ret;"

I didn't change the "return ret;" at the end of the functions because
they were obviously intentional.  I'm doing static analysis and I have
an unpublished check which complains about the return ret at the end
of the function but I'm not going to publish it because that doesn't
find bugs and I'm only focused on bugs.

When we have code like:

	ret = frob();
	if (ret)
		return ret;

	if (x == 1)
		return ret;

More often than not, that is a bug.  But if it's code like:

	ret = frob();
	if (ret)
		return ret;

	frob();
	frob();
	frob();

	return ret;

Then that's probably not a bug.

regards,
dan carpenter

Re: [PATCH v2] SoC: mediatek: mt8188-mt6359: clean up a return in codec_init

[Dan Carpenter]

On Thu, Jun 08, 2023 at 04:02:07AM +0000, Trevor Wu (吳文良) wrote:
> On Wed, 2023-06-07 at 17:57 +0300, Dan Carpenter wrote:
> >  This code triggers a Smatch static checker warning and does sort of
> > look like an error path.
> > 
> > sound/soc/mediatek/mt8188/mt8188-mt6359.c:597
> > mt8188_max98390_codec_init() warn: missing error code? 'ret'
> > 
> > However, returning 0 is intentional.  Make that explicit.
> 
> Hi Dan,
> 
> There are several similar instances in this file. If using 'return
> ret;' is not recommended, should we update all of them simultaneously?
> 
> For instance, the value of 'ret' at the end of this function should
> also be zero.
> 

I would prefer that, yes.  "return 0;" is always more clear than
"return ret;"

I didn't change the "return ret;" at the end of the functions because
they were obviously intentional.  I'm doing static analysis and I have
an unpublished check which complains about the return ret at the end
of the function but I'm not going to publish it because that doesn't
find bugs and I'm only focused on bugs.

When we have code like:

	ret = frob();
	if (ret)
		return ret;

	if (x == 1)
		return ret;

More often than not, that is a bug.  But if it's code like:

	ret = frob();
	if (ret)
		return ret;

	frob();
	frob();
	frob();

	return ret;

Then that's probably not a bug.

regards,
dan carpenter


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel