On Mon, Nov 11, 2024 at 2:45 PM David Lechner <dlechner@xxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxx>
> > ---
> > 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;
> }
>
Looks great.
But there is no indentation between "switch" and "case".
As a result, the closing braces of "switch" and "case" will
be placed in the same column.
Like this:
switch(mask) {
case IIO_CHAN_INFO_ENABLE: {
}
}
-Jiasheng
