Re: [PATCH 06/20] ASoC: jz4740: Propagate codec wake-up errors

[Paul Cercueil]

Hi Pengpeng Hou,

Le vendredi 28 août 2026 à 19:34 +0800, Pengpeng Hou a écrit :
> The JZ4740 codec can only leave its suspend state through
> jz4740_codec_wakeup().  The helper ignores failures while asserting
> reset, clearing suspend/reset and replaying the dirty register cache,
> then the bias callback continues with later register writes.
> 
> Return the first error from all three operations and stop the
> OFF-to-STANDBY transition when wake-up restoration fails.
> 
> The issue was identified via static analysis and manually reviewed.
> 
> Fixes: 3b097d64eafa ("ASoC: Add JZ4740 codec driver")
> 
> Assisted-by: LLM
> Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>

I'll leave it to Mark to decide, but IMHO the code is fine without the
patch. We're using regmap-mmio here, with settings so basic that these
calls will never fail, and that's why there was no error-checking.

Cheers,
-Paul

> ---
>  sound/soc/codecs/jz4740.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/soc/codecs/jz4740.c b/sound/soc/codecs/jz4740.c
> index d3d801d850a1..efb3f8295e98 100644
> --- a/sound/soc/codecs/jz4740.c
> +++ b/sound/soc/codecs/jz4740.c
> @@ -217,15 +217,22 @@ static struct snd_soc_dai_driver
> jz4740_codec_dai = {
>  	.symmetric_rate = 1,
>  };
>  
> -static void jz4740_codec_wakeup(struct regmap *regmap)
> +static int jz4740_codec_wakeup(struct regmap *regmap)
>  {
> -	regmap_set_bits(regmap, JZ4740_REG_CODEC_1,
> JZ4740_CODEC_1_RESET);
> +	int ret;
> +
> +	ret = regmap_set_bits(regmap, JZ4740_REG_CODEC_1,
> JZ4740_CODEC_1_RESET);
> +	if (ret)
> +		return ret;
> +
>  	udelay(2);
>  
> -	regmap_clear_bits(regmap, JZ4740_REG_CODEC_1,
> -			  JZ4740_CODEC_1_SUSPEND |
> JZ4740_CODEC_1_RESET);
> +	ret = regmap_clear_bits(regmap, JZ4740_REG_CODEC_1,
> +				JZ4740_CODEC_1_SUSPEND |
> JZ4740_CODEC_1_RESET);
> +	if (ret)
> +		return ret;
>  
> -	regcache_sync(regmap);
> +	return regcache_sync(regmap);
>  }
>  
>  static int jz4740_codec_set_bias_level(struct snd_soc_component
> *component,
> @@ -235,6 +242,7 @@ static int jz4740_codec_set_bias_level(struct
> snd_soc_component *component,
>  	struct jz4740_codec *jz4740_codec =
> snd_soc_component_get_drvdata(component);
>  	struct regmap *regmap = jz4740_codec->regmap;
>  	unsigned int mask;
> +	int ret;
>  
>  	switch (level) {
>  	case SND_SOC_BIAS_ON:
> @@ -248,8 +256,11 @@ static int jz4740_codec_set_bias_level(struct
> snd_soc_component *component,
>  		break;
>  	case SND_SOC_BIAS_STANDBY:
>  		/* The only way to clear the suspend flag is to
> reset the codec */
> -		if (snd_soc_dapm_get_bias_level(dapm) ==
> SND_SOC_BIAS_OFF)
> -			jz4740_codec_wakeup(regmap);
> +		if (snd_soc_dapm_get_bias_level(dapm) ==
> SND_SOC_BIAS_OFF) {
> +			ret = jz4740_codec_wakeup(regmap);
> +			if (ret)
> +				return ret;
> +		}
>  
>  		mask = JZ4740_CODEC_1_VREF_DISABLE |
>  			JZ4740_CODEC_1_VREF_AMP_DISABLE |