On Wed, Feb 23, 2022 at 02:04:48PM +0100, Hans Verkuil wrote:

On 2/23/22 13:54, Sakari Ailus wrote:

quoted

Hi Hans, Michael,

On Wed, Feb 23, 2022 at 02:16:28PM +0200, Sakari Ailus wrote:

quoted

quoted

quoted

+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 <redacted>

to this patch. I'll delegate the series to you in patchwork.

Thanks!

-- 
Sakari Ailus