Hi Andy,
Some issues found when reviewing cherry pick of this patch to Chrome
OS kernel. Please see inline.
On Sat, Mar 17, 2018 at 1:38 AM, Andy Yeh <andy.yeh@xxxxxxxxx> wrote:
[snip]
> + case V4L2_CID_VBLANK:
> + /*
> + * Auto Frame Length Line Control is enabled by default.
> + * Not need control Vblank Register.
> + */
What is the meaning of this control then? Should it be read-only?
> + break;
> + default:
> + dev_info(&client->dev,
> + "ctrl(id:0x%x,val:0x%x) is not handled\n",
> + ctrl->id, ctrl->val);
> + ret = -EINVAL;
> + break;
> + }
> +
> + pm_runtime_put(&client->dev);
> +
> + return ret;
> +
[snip]
> + v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &imx258_ctrl_ops,
> + V4L2_CID_TEST_PATTERN,
> + ARRAY_SIZE(imx258_test_pattern_menu) - 1,
> + 0, 0, imx258_test_pattern_menu);
There is no code for handling this control in imx258_s_ctrl(). It's
not a correct behavior to register a control, which isn't handled by
the driver. Please either implement the control completely or remove
it.
> +
> + if (ctrl_hdlr->error) {
> + ret = ctrl_hdlr->error;
> + dev_err(&client->dev, "%s control init failed (%d)\n",
> + __func__, ret);
> + goto error;
Something strange happening here with indentation.
Best regards,
Tomasz
