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.
Regards,
Hans
