Re: [PATCH] ASoC: cht_bsw_rt5645: Use common error handling code in cht_codec_fixup()

[Pierre-Louis Bossart]

On 11/19/2017 03:00 AM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> Date: Sun, 19 Nov 2017 09:53:56 +0100
>
> * Add a jump target so that a specific error message is stored only once
>    at the end of this function implementation.
>
> * Replace two calls of the function "dev_err" by goto statements.
>
> This issue was detected by using the Coccinelle software.
Indeed this isn't very smart to provide the same test but no, this is 
the wrong way to fix this. The error message is in reply to two 
different configurations, one on the codec side and one on the cpu-dai 
side. We should change the error message to more clearly indicate the 
issue rather than fold both cases under the same handler.
Besides this is the same code as in other machine drivers so needs to be 
handled in multiple files as well.

> Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> ---
>   sound/soc/intel/boards/cht_bsw_rt5645.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
> index 18d129caa974..bf3ffc2b0d8f 100644
> --- a/sound/soc/intel/boards/cht_bsw_rt5645.c
> +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
> @@ -357,20 +357,16 @@ static int cht_codec_fixup(struct snd_soc_pcm_runtime *rtd,
>   					SND_SOC_DAIFMT_NB_NF   |
>   					SND_SOC_DAIFMT_CBS_CFS
>   			);
> -		if (ret < 0) {
> -			dev_err(rtd->dev, "can't set format to I2S, err %d\n", ret);
> -			return ret;
> -		}
> +		if (ret < 0)
> +			goto report_failure;
>   
>   		ret = snd_soc_dai_set_fmt(rtd->codec_dai,
>   					SND_SOC_DAIFMT_I2S     |
>   					SND_SOC_DAIFMT_NB_NF   |
>   					SND_SOC_DAIFMT_CBS_CFS
>   			);
> -		if (ret < 0) {
> -			dev_err(rtd->dev, "can't set format to I2S, err %d\n", ret);
> -			return ret;
> -		}
> +		if (ret < 0)
> +			goto report_failure;
>   
>   		ret = snd_soc_dai_set_tdm_slot(rtd->cpu_dai, 0x3, 0x3, 2, 16);
>   		if (ret < 0) {
> @@ -403,6 +399,10 @@ static int cht_codec_fixup(struct snd_soc_pcm_runtime *rtd,
>   		}
>   	}
>   	return 0;
> +
> +report_failure:
> +	dev_err(rtd->dev, "can't set format to I2S, err %d\n", ret);
> +	return ret;
>   }
>   
>   static int cht_aif1_startup(struct snd_pcm_substream *substream)