Re: [PATCH v10 2/2] media: i2c: isl7998x: Add driver for Intersil ISL7998x

[Hans Verkuil <hverkuil@xs4all.nl>]

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@xs4all.nl>

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

Regards,

	Hans