linux-kernel - Re: [PATCH v2 1/6] media: ar0521: Refactor ar0521_power

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <m3tteycyx0.fsf@t19.piap.pl>
Date: Mon, 02 Sep 2024 09:11:55 +0200
From: Krzysztof Hałasa <khalasa@...p.pl>
To: Ricardo Ribalda <ribalda@...omium.org>
Cc: Sakari Ailus <sakari.ailus@...ux.intel.com>,  Mauro Carvalho Chehab
 <mchehab@...nel.org>,  Sylwester Nawrocki <s.nawrocki@...sung.com>,
  Andrzej Hajda <andrzej.hajda@...el.com>,  Neil Armstrong
 <neil.armstrong@...aro.org>,  Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>,  Kevin Hilman <khilman@...libre.com>,
  Jerome Brunet <jbrunet@...libre.com>,  Martin Blumenstingl
 <martin.blumenstingl@...glemail.com>,  Hans Verkuil
 <hverkuil-cisco@...all.nl>,  linux-media@...r.kernel.org,
  linux-kernel@...r.kernel.org,  linux-amlogic@...ts.infradead.org,
  linux-staging@...ts.linux.dev,  linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 1/6] media: ar0521: Refactor ar0521_power_off()

Hi Ricardo,

Ricardo Ribalda <ribalda@...omium.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@...p.pl>

> Signed-off-by: Ricardo Ribalda <ribalda@...omium.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