Re: [PATCH v2 1/6] media: ar0521: Refactor ar0521_power_off()

["Krzysztof Hałasa" <khalasa@piap.pl>]

Hi Ricardo,

Ricardo Ribalda <ribalda@chromium.org> writes:

> Factor out all the power off logic, except the clk_disable_unprepare(),
> to a new function __ar0521_power_off().
>
> This allows ar0521_power_on() to explicitly clean-out the clock during
> the error-path.
>
> The following smatch warning is fixed:
> drivers/media/i2c/ar0521.c:912 ar0521_power_on() warn: 'sensor->extclk' from clk_prepare_enable() not released on lines: 912.

TBH I don't know if the code with the patch applied is better or not.
It if silences the tool, well, sometimes things are not as clear as we
want them.

Acked-by: Krzysztof Hałasa <khalasa@piap.pl>

> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/i2c/ar0521.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ar0521.c b/drivers/media/i2c/ar0521.c
> index 09331cf95c62..56a724b4d47e 100644
> --- a/drivers/media/i2c/ar0521.c
> +++ b/drivers/media/i2c/ar0521.c
> @@ -835,14 +835,12 @@ static const struct initial_reg {
>              be(0x0707)), /* 3F44: couple k factor 2 */
>  };
>
> -static int ar0521_power_off(struct device *dev)
> +static void __ar0521_power_off(struct device *dev)
>  {
>         struct v4l2_subdev *sd = dev_get_drvdata(dev);
>         struct ar0521_dev *sensor = to_ar0521_dev(sd);
>         int i;
>
> -       clk_disable_unprepare(sensor->extclk);
> -
>         if (sensor->reset_gpio)
>                 gpiod_set_value(sensor->reset_gpio, 1); /* assert RESET signal */
>
> @@ -850,6 +848,16 @@ static int ar0521_power_off(struct device *dev)
>                 if (sensor->supplies[i])
>                         regulator_disable(sensor->supplies[i]);
>         }
> +}
> +
> +static int ar0521_power_off(struct device *dev)
> +{
> +       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> +       struct ar0521_dev *sensor = to_ar0521_dev(sd);
> +
> +       clk_disable_unprepare(sensor->extclk);
> +       __ar0521_power_off(dev);
> +
>         return 0;
>  }
>
> @@ -908,7 +916,8 @@ static int ar0521_power_on(struct device *dev)
>
>         return 0;
>  off:
> -       ar0521_power_off(dev);
> +       clk_disable_unprepare(sensor->extclk);
> +       __ar0521_power_off(dev);
>         return ret;
>  }
>

-- 
Krzysztof "Chris" Hałasa

Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa