Hi Laurent,
On Mon, 9 Mar 2015, Laurent Pinchart wrote:
> If the soc-camera host doesn't provide clock start and stop operations
> registering a v4l2 clock is pointless. Don't do it.
This can introduce breakage only for camera-host drivers, that don't
provide .clock_start() or .clock_stop(). After your other 3 patches from
this patch set there will be one such driver in the tree - rcar_vin.c. I
wouldn't mind this patch as long as we can have an ack from an rcar_vin.c
maintainer. Since I don't see one in MAINTAINERS, who can ack this? Simon?
Thanks
Guennadi
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/platform/soc_camera/soc_camera.c | 51 +++++++++++++++++---------
> 1 file changed, 33 insertions(+), 18 deletions(-)
>
> This requires proper review and testing, please don't apply it blindly.
>
> diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
> index 0943125..f3ea911 100644
> --- a/drivers/media/platform/soc_camera/soc_camera.c
> +++ b/drivers/media/platform/soc_camera/soc_camera.c
> @@ -1374,10 +1374,13 @@ static int soc_camera_i2c_init(struct soc_camera_device *icd,
> snprintf(clk_name, sizeof(clk_name), "%d-%04x",
> shd->i2c_adapter_id, shd->board_info->addr);
>
> - icd->clk = v4l2_clk_register(&soc_camera_clk_ops, clk_name, "mclk", icd);
> - if (IS_ERR(icd->clk)) {
> - ret = PTR_ERR(icd->clk);
> - goto eclkreg;
> + if (ici->ops->clock_start && ici->ops->clock_stop) {
> + icd->clk = v4l2_clk_register(&soc_camera_clk_ops, clk_name,
> + "mclk", icd);
> + if (IS_ERR(icd->clk)) {
> + ret = PTR_ERR(icd->clk);
> + goto eclkreg;
> + }
> }
>
> subdev = v4l2_i2c_new_subdev_board(&ici->v4l2_dev, adap,
> @@ -1394,8 +1397,10 @@ static int soc_camera_i2c_init(struct soc_camera_device *icd,
>
> return 0;
> ei2cnd:
> - v4l2_clk_unregister(icd->clk);
> - icd->clk = NULL;
> + if (icd->clk) {
> + v4l2_clk_unregister(icd->clk);
> + icd->clk = NULL;
> + }
> eclkreg:
> kfree(ssdd);
> ealloc:
> @@ -1420,8 +1425,10 @@ static void soc_camera_i2c_free(struct soc_camera_device *icd)
> i2c_unregister_device(client);
> i2c_put_adapter(adap);
> kfree(ssdd);
> - v4l2_clk_unregister(icd->clk);
> - icd->clk = NULL;
> + if (icd->clk) {
> + v4l2_clk_unregister(icd->clk);
> + icd->clk = NULL;
> + }
> }
>
> /*
> @@ -1555,17 +1562,21 @@ static int scan_async_group(struct soc_camera_host *ici,
> snprintf(clk_name, sizeof(clk_name), "%d-%04x",
> sasd->asd.match.i2c.adapter_id, sasd->asd.match.i2c.address);
>
> - icd->clk = v4l2_clk_register(&soc_camera_clk_ops, clk_name, "mclk", icd);
> - if (IS_ERR(icd->clk)) {
> - ret = PTR_ERR(icd->clk);
> - goto eclkreg;
> + if (ici->ops->clock_start && ici->ops->clock_stop) {
> + icd->clk = v4l2_clk_register(&soc_camera_clk_ops, clk_name,
> + "mclk", icd);
> + if (IS_ERR(icd->clk)) {
> + ret = PTR_ERR(icd->clk);
> + goto eclkreg;
> + }
> }
>
> ret = v4l2_async_notifier_register(&ici->v4l2_dev, &sasc->notifier);
> if (!ret)
> return 0;
>
> - v4l2_clk_unregister(icd->clk);
> + if (icd->clk)
> + v4l2_clk_unregister(icd->clk);
> eclkreg:
> icd->clk = NULL;
> platform_device_del(sasc->pdev);
> @@ -1660,17 +1671,21 @@ static int soc_of_bind(struct soc_camera_host *ici,
> snprintf(clk_name, sizeof(clk_name), "of-%s",
> of_node_full_name(remote));
>
> - icd->clk = v4l2_clk_register(&soc_camera_clk_ops, clk_name, "mclk", icd);
> - if (IS_ERR(icd->clk)) {
> - ret = PTR_ERR(icd->clk);
> - goto eclkreg;
> + if (ici->ops->clock_start && ici->ops->clock_stop) {
> + icd->clk = v4l2_clk_register(&soc_camera_clk_ops, clk_name,
> + "mclk", icd);
> + if (IS_ERR(icd->clk)) {
> + ret = PTR_ERR(icd->clk);
> + goto eclkreg;
> + }
> }
>
> ret = v4l2_async_notifier_register(&ici->v4l2_dev, &sasc->notifier);
> if (!ret)
> return 0;
>
> - v4l2_clk_unregister(icd->clk);
> + if (icd->clk)
> + v4l2_clk_unregister(icd->clk);
> eclkreg:
> icd->clk = NULL;
> platform_device_del(sasc->pdev);
> --
> 2.0.5
>
--
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
