Hi Hans,
On Mon, Nov 28, 2022 at 1:49 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> On 11/1/22 00:21, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
<snip>
> > -static void ov5645_set_power_off(struct ov5645 *ov5645)
> > -{
> > - gpiod_set_value_cansleep(ov5645->rst_gpio, 1);
> > - gpiod_set_value_cansleep(ov5645->enable_gpio, 0);
> > - clk_disable_unprepare(ov5645->xclk);
> > - regulator_bulk_disable(OV5645_NUM_SUPPLIES, ov5645->supplies);
> > -}
> > -
> > -static int ov5645_s_power(struct v4l2_subdev *sd, int on)
> > -{
> > - struct ov5645 *ov5645 = to_ov5645(sd);
> > - int ret = 0;
> > -
> > - mutex_lock(&ov5645->power_lock);
> > -
> > - /* If the power count is modified from 0 to != 0 or from != 0 to 0,
> > - * update the power state.
> > - */
> > - if (ov5645->power_count == !on) {
> > - if (on) {
> > - ret = ov5645_set_power_on(ov5645);
> > - if (ret < 0)
> > - goto exit;
> > -
> > - ret = ov5645_set_register_array(ov5645,
> > - ov5645_global_init_setting,
> > + ret = ov5645_set_register_array(ov5645, ov5645_global_init_setting,
> > ARRAY_SIZE(ov5645_global_init_setting));
> > - if (ret < 0) {
> > - dev_err(ov5645->dev,
> > - "could not set init registers\n");
> > - ov5645_set_power_off(ov5645);
> > - goto exit;
> > - }
> > -
> > - usleep_range(500, 1000);
> > - } else {
> > - ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x58);
> > - ov5645_set_power_off(ov5645);
> > - }
> > + if (ret < 0) {
> > + dev_err(ov5645->dev, "could not set init registers\n");
> > + goto exit;
> > }
> >
> > - /* Update the power count. */
> > - ov5645->power_count += on ? 1 : -1;
> > - WARN_ON(ov5645->power_count < 0);
> > + usleep_range(500, 1000);
> >
> > -exit:
> > - mutex_unlock(&ov5645->power_lock);
> > + return 0;
> >
> > +exit:
> > + ov5645_set_power_off(dev);
> > return ret;
>
> smatch gives this warning:
>
> drivers/media/i2c/ov5645.c:687 ov5645_set_power_on() warn: 'ov5645->xclk' from clk_prepare_enable() not released on lines: 687.
>
it's a false positive, as in case of error we jump to the exit label
which calls ov5645_set_power_off() which internally calls
clk_disable_unprepare() for xclk and on success we need to have xclk
ON during s_stream(ON) and eventually the
xclk will be unprepared in s_stream(OFF).
Cheers,
Prabhakar
