Hi Hans,
Thank you for the patch.
On Wednesday 17 Aug 2016 08:29:40 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
> Get the clock for this sensor.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---
> drivers/media/i2c/ov7670.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> index fe527b2..57adf3d 100644
> --- a/drivers/media/i2c/ov7670.c
> +++ b/drivers/media/i2c/ov7670.c
> @@ -10,6 +10,7 @@
> * This file may be distributed under the terms of the GNU General
> * Public License, version 2.
> */
> +#include <linux/clk.h>
> #include <linux/init.h>
> #include <linux/module.h>
> #include <linux/slab.h>
> @@ -18,6 +19,7 @@
> #include <linux/videodev2.h>
> #include <media/v4l2-device.h>
> #include <media/v4l2-ctrls.h>
> +#include <media/v4l2-clk.h>
> #include <media/v4l2-mediabus.h>
> #include <media/v4l2-image-sizes.h>
> #include <media/i2c/ov7670.h>
> @@ -228,6 +230,7 @@ struct ov7670_info {
> struct v4l2_ctrl *hue;
> };
> struct ov7670_format_struct *fmt; /* Current format */
> + struct v4l2_clk *clk;
> int min_width; /* Filter out smaller sizes */
> int min_height; /* Filter out smaller sizes */
> int clock_speed; /* External clock speed (MHz) */
> @@ -1588,8 +1591,19 @@ static int ov7670_probe(struct i2c_client *client,
> info->pclk_hb_disable = true;
> }
>
> + info->clk = v4l2_clk_get(&client->dev, "xvclk");
> + if (IS_ERR(info->clk))
> + return -EPROBE_DEFER;
> + v4l2_clk_enable(info->clk);
Do you really need the V4L2 clock API here, or could you use the CCF API
directly ?
> + info->clock_speed = v4l2_clk_get_rate(info->clk) / 1000000;
> + if (info->clock_speed < 12 ||
> + info->clock_speed > 48)
> + return -EINVAL;
> +
You need error handling here too. I recommend adding error handling code at
the end of the function and using goto's.
> /* Make sure it's an ov7670 */
> ret = ov7670_detect(sd);
> +
No need for a blank line here.
> if (ret) {
> v4l_dbg(1, debug, client,
> "chip found @ 0x%x (%s) is not an ov7670 chip.\n",
> @@ -1682,6 +1696,7 @@ static int ov7670_remove(struct i2c_client *client)
> #if defined(CONFIG_MEDIA_CONTROLLER)
> media_entity_cleanup(&sd->entity);
> #endif
> + v4l2_clk_put(info->clk);
Don't you need to call v4l2_clk_disable() before ?
> return 0;
> }
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
