On Mon, 11 Nov 2024 22:23:10 +0000
Jiasheng Jiang <jiashengjiangcool@xxxxxxxxx> wrote:
> Add check for the return value of clk_enable() in order to catch the
> potential exception.
>
> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@xxxxxxxxx>
Hi Jiasheng,
Should definitely mention the changes to use guard() to simplify
the resulting code.
One minor comment on the code inline. Otherwise looks good to me.
Thanks,
Jonathan
> ---
> Changelog:
>
> v3 -> v4:
>
> 1. Place braces around the case body.
>
> v2 -> v3:
>
> 1. Simplify code with cleanup helpers.
>
> v1 -> v2:
>
> 1. Remove unsuitable dev_err_probe().
> @@ -482,6 +484,7 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
> int val, int val2, long mask)
> {
> struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> + int ret;
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> @@ -491,12 +494,14 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
> /* fixed scale */
> return -EINVAL;
>
> - case IIO_CHAN_INFO_ENABLE:
> - mutex_lock(&priv->lock);
> + case IIO_CHAN_INFO_ENABLE: {
> + guard(mutex)(&priv->lock);
> if (val) {
> if (!priv->enabled) {
> priv->enabled = true;
> - clk_enable(priv->clk);
> + ret = clk_enable(priv->clk);
> + if (ret)
> + return ret;
> }
> regmap_set_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN);
> } else {
> @@ -506,9 +511,10 @@ static int stm32_counter_write_raw(struct iio_dev *indio_dev,
> clk_disable(priv->clk);
> }
> }
> - mutex_unlock(&priv->lock);
> +
> return 0;
> }
Add a default for reasons David mentioned and it also makes it visually clear
that we expect to get in here for other cases but they are all errors.
default:
return -EINVAL;
> + }
>
And drop this return as unreachable.
> return -EINVAL;
> }
