Re: [PATCH v3] iio: trigger: stm32-timer-trigger: Add check for clk_enable()

[David Lechner <dlechner@baylibre.com>]

On 11/11/24 1:19 PM, Jiasheng Jiang wrote:
> Add check for the return value of clk_enable() in order to catch the
> potential exception.
> 
> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
> ---
> Changelog:
> 
> v2 -> v3:
> 
> 1. Simplify code with cleanup helpers.
> 
> v1 -> v2:
> 
> 1. Remove unsuitable dev_err_probe().
> ---

...

> @@ -492,21 +495,25 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
>  		return -EINVAL;
>  
>  	case IIO_CHAN_INFO_ENABLE:
> -		mutex_lock(&priv->lock);
> -		if (val) {
> -			if (!priv->enabled) {
> -				priv->enabled = true;
> -				clk_enable(priv->clk);
> -			}
> -			regmap_set_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN);
> -		} else {
> -			regmap_clear_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN);
> -			if (priv->enabled) {
> -				priv->enabled = false;
> -				clk_disable(priv->clk);
> +
> +		scoped_guard(mutex, &priv->lock) {
> +			if (val) {
> +				if (!priv->enabled) {
> +					priv->enabled = true;
> +					ret = clk_enable(priv->clk);
> +					if (ret)
> +						return ret;
> +				}
> +				regmap_set_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN);
> +			} else {
> +				regmap_clear_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN);
> +				if (priv->enabled) {
> +					priv->enabled = false;
> +					clk_disable(priv->clk);
> +				}
>  			}
>  		}
> -		mutex_unlock(&priv->lock);
> +		
>  		return 0;
>  	}


Another way to do this that avoids changing the indent
so much is placing braces around the case body like this.
This also avoids the compile error from using guard after
case directly.


 	case IIO_CHAN_INFO_ENABLE: {
		guard(mutex)(&priv->lock);

		if (val) {
			if (!priv->enabled) {
				priv->enabled = true;
				ret = clk_enable(priv->clk);
				if (ret)
					return ret;
			}
			regmap_set_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN);
		} else {
			regmap_clear_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN);
			if (priv->enabled) {
				priv->enabled = false;
				clk_disable(priv->clk);
			}
		}
		
 		return 0;
 	}