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.pinch...@ideasonboard.com>
> ---
>  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to