On Wed, Feb 23, 2022 at 02:04:48PM +0100, Hans Verkuil wrote:
> On 2/23/22 13:54, Sakari Ailus wrote:
> > 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.
> >
>
> OK, with that change you can add my:
>
> Acked-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
>
> to this patch. I'll delegate the series to you in patchwork.
Thanks!
--
Sakari Ailus
