linux-kernel - Re: [PATCH v4 6/9] media: i2c: ov5645: Use runtime PM

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: <CA+V-a8tr9486HV4zvVG3bh4fWNQ-edUDPMZAUM-kKDN-PvRU+w@mail.gmail.com>
Date:   Tue, 29 Nov 2022 00:02:52 +0000
From:   "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To:     Hans Verkuil <hverkuil@...all.nl>
Cc:     Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Magnus Damm <magnus.damm@...il.com>,
        Shawn Tu <shawnx.tu@...el.com>,
        Jacopo Mondi <jacopo@...ndi.org>, linux-media@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-renesas-soc@...r.kernel.org,
        Biju Das <biju.das.jz@...renesas.com>,
        Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v4 6/9] media: i2c: ov5645: Use runtime PM

Hi Hans,

On Mon, Nov 28, 2022 at 1:49 PM Hans Verkuil <hverkuil@...all.nl> wrote:
>
> Hi Prabhakar,
>
> On 11/1/22 00:21, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> >
<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