Hi Hans, Michael,
On Wed, Feb 23, 2022 at 02:16:28PM +0200, Sakari Ailus wrote:
> > > +static int isl7998x_pre_streamon(struct v4l2_subdev *sd, u32 flags)
> > > +{
> > > + struct i2c_client *client = v4l2_get_subdevdata(sd);
> > > + struct device *dev = &client->dev;
> > > +
> > > + if (flags & V4L2_SUBDEV_PRE_STREAMON_FL_MANUAL_LP)
> > > + return pm_runtime_resume_and_get(dev);
> > > +
> > > + return 0;
> >
> > This feels a bit scary: if V4L2_SUBDEV_PRE_STREAMON_FL_MANUAL_LP is NOT
> > set, then pm_runtime_resume_and_get() isn't called, but this function
> > still returns success...
>
> Good find.
>
> pm_runtime_resume_and_get() need to be called unconditionally.
>
> Alternatively, store what was done here, and put the PM use count
> accordingly below. But I see no reason to do that.
But I think the driver is otherwise good to go.
Unless there are objections, I'll drop the check in the pre_streamon()
callback and apply it into my tree.
--
Kind regards,
Sakari Ailus
