Hi Sakari,
Thanks for the review.
On Fri, Mar 13, 2015 at 12:04 AM, Sakari Ailus <sakari.ai...@iki.fi> wrote:
> Hi Prabhakar,
>
> On Thu, Mar 12, 2015 at 11:22:36PM +0000, Lad Prabhakar wrote:
> ...
>> +static int ov2659_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + const struct ov2659_platform_data *pdata = ov2659_get_pdata(client);
>> + struct v4l2_subdev *sd;
>> + struct ov2659 *ov2659;
>> + struct clk *clk;
>> + int ret;
>> +
>> + if (!pdata) {
>> + dev_err(&client->dev, "platform data not specified\n");
>> + return -EINVAL;
>> + }
>> +
>> + ov2659 = devm_kzalloc(&client->dev, sizeof(*ov2659), GFP_KERNEL);
>> + if (!ov2659)
>> + return -ENOMEM;
>> +
>> + ov2659->pdata = pdata;
>> + ov2659->client = client;
>> +
>> + clk = devm_clk_get(&client->dev, "xvclk");
>> + if (IS_ERR(clk))
>> + return PTR_ERR(clk);
>> +
>> + ov2659->xvclk_frequency = clk_get_rate(clk);
>> + if (ov2659->xvclk_frequency < 6000000 ||
>> + ov2659->xvclk_frequency > 27000000)
>> + return -EINVAL;
>> +
>> + v4l2_ctrl_handler_init(&ov2659->ctrls, 2);
>> + v4l2_ctrl_new_std(&ov2659->ctrls, &ov2659_ctrl_ops,
>> + V4L2_CID_PIXEL_RATE, ov2659->xvclk_frequency,
>> + ov2659->xvclk_frequency, 1, ov2659->xvclk_frequency);
>
> ov2659->xvclk_frequency is the frequency of the external clock, not the
> pixel rate. If I understand correctly, you should use the value of the
> link-frequency property instead (as long as it's one pixel per clock).
>
this is what happens when you work late night :)
> With this fixed,
>
will post a v6 today.
Cheers,
--Prabhakar Lad
> Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
>
>> + v4l2_ctrl_new_std_menu_items(&ov2659->ctrls, &ov2659_ctrl_ops,
>> + V4L2_CID_TEST_PATTERN,
>> + ARRAY_SIZE(ov2659_test_pattern_menu) - 1,
>> + 0, 0, ov2659_test_pattern_menu);
>> + ov2659->sd.ctrl_handler = &ov2659->ctrls;
>> +
>> + if (ov2659->ctrls.error) {
>> + dev_err(&client->dev, "%s: control initialization error %d\n",
>> + __func__, ov2659->ctrls.error);
>> + return ov2659->ctrls.error;
>> + }
>> +
>> + sd = &ov2659->sd;
>> + client->flags |= I2C_CLIENT_SCCB;
>> + v4l2_i2c_subdev_init(sd, client, &ov2659_subdev_ops);
>> +
>> + sd->internal_ops = &ov2659_subdev_internal_ops;
>> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
>> + V4L2_SUBDEV_FL_HAS_EVENTS;
>> +
>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>> + ov2659->pad.flags = MEDIA_PAD_FL_SOURCE;
>> + sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
>> + ret = media_entity_init(&sd->entity, 1, &ov2659->pad, 0);
>> + if (ret < 0) {
>> + v4l2_ctrl_handler_free(&ov2659->ctrls);
>> + return ret;
>> + }
>> +#endif
>> +
>> + mutex_init(&ov2659->lock);
>> +
>> + ov2659_get_default_format(&ov2659->format);
>> + ov2659->frame_size = &ov2659_framesizes[2];
>> + ov2659->format_ctrl_regs = ov2659_formats[0].format_ctrl_regs;
>> +
>> + ret = ov2659_detect(sd);
>> + if (ret < 0)
>> + goto error;
>> +
>> + /* Calculate the PLL register value needed */
>> + ov2659_pll_calc_params(ov2659);
>> +
>> + ret = v4l2_async_register_subdev(&ov2659->sd);
>> + if (ret)
>> + goto error;
>> +
>> + dev_info(&client->dev, "%s sensor driver registered !!\n", sd->name);
>> +
>> + return 0;
>> +
>> +error:
>> + v4l2_ctrl_handler_free(&ov2659->ctrls);
>> +#if defined(CONFIG_MEDIA_CONTROLLER)
>> + media_entity_cleanup(&sd->entity);
>> +#endif
>> + mutex_destroy(&ov2659->lock);
>> + return ret;
>> +}
>
> --
> Kind regards,
>
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
