Hi Andy,

Thanks for the patch. Please see my comments inline.

On Fri, Jan 19, 2018 at 12:37 PM, Andy Yeh <andy....@intel.com> wrote:
> Add a V4L2 sub-device driver for the Sony IMX258 image sensor.
> This is a camera sensor using the I2C bus for control and the
> CSI-2 bus for data.
>
> Signed-off-by: Andy Yeh <andy....@intel.com>
> ---
> - v2->v3
> -- Update the streaming function to remove SW_STANDBY in the beginning.
> -- Adjust the delay time from 1ms to 12ms before set stream-on register.
> - v3->v4
> -- fix the sd.entity to make code be compiled on the mainline kernel.
> -- " - imx258->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;"
> -- " + imx258->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;"
>
>  MAINTAINERS                |    7 +
>  drivers/media/i2c/Kconfig  |   11 +
>  drivers/media/i2c/Makefile |    1 +
>  drivers/media/i2c/imx258.c | 1148 
> ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 1167 insertions(+)
>  create mode 100644 drivers/media/i2c/imx258.c
[snip]
> +static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +       struct imx258 *imx258 =
> +               container_of(ctrl->handler, struct imx258, ctrl_handler);
> +       struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
> +       s64 max;
> +       int ret = 0;
> +
> +       /* Propagate change of current control to all related controls */
> +       if (ctrl->id == V4L2_CID_VBLANK) {
> +               /* Update max exposure while meeting expected vblanking */
> +               max = imx258->cur_mode->height + ctrl->val - 8;
> +               __v4l2_ctrl_modify_range(imx258->exposure,
> +                                        imx258->exposure->minimum,
> +                                        max, imx258->exposure->step, max);
> +       }
> +
> +       /*
> +        * Applying V4L2 control value only happens
> +        * when power is up for streaming
> +        */
> +       if (pm_runtime_get_if_in_use(&client->dev) <= 0)
> +               return 0;

This won't work if runtime PM is not compiled in or is disabled at
runtime, i.e. pm_runtime_get_if_in_use() returns -EINVAL.

But actually, do we need to care about runtime PM here? Could we just
return early if we're not streaming? Then the controls would be
handled when streaming starts, since we call
__v4l2_ctrl_handler_setup() from _start_streaming().

Best regards,
Tomasz

Reply via email to