On Mon, Jul 30, 2018 at 8:39 PM Sakari Ailus
<sakari.ailus@xxxxxxxxxxxxxxx> wrote:
>
> Hi Tomasz,
>
> On Mon, Jul 30, 2018 at 07:19:56PM +0900, Tomasz Figa wrote:
> ...
> > > +static int imx208_set_ctrl(struct v4l2_ctrl *ctrl)
> > > +{
> > > + struct imx208 *imx208 =
> > > + container_of(ctrl->handler, struct imx208, ctrl_handler);
> > > + struct i2c_client *client = v4l2_get_subdevdata(&imx208->sd);
> > > + int ret;
> > > +
> > > + /*
> > > + * Applying V4L2 control value only happens
> > > + * when power is up for streaming
> > > + */
> > > + if (pm_runtime_get_if_in_use(&client->dev) <= 0)
> >
> > This is buggy, because it won't handle the case of runtime PM disabled
> > in kernel config. The check should be
> > (!pm_runtime_get_if_in_use(&client->dev)).
>
> Good find. This seems to be the case for most other sensor drivers that
> make use of the function. I can submit a fix for those as well.
>
> I suppose most people use these with runtime PM enabled as this hasn't been
> spotted previously.
Yeah, I spotted it first with imx258 and it took us few emails to get
to the right code. :)
These drivers probably don't have too many users yet in general, so I
guess this didn't manage to cause any problems yet, but it became a
good example of bug propagation via copy/paste. ;)
Fixing would be appreciated indeed!
Best regards,
Tomasz
