Hi Guennadi,
On Sunday 15 March 2015 18:56:44 Guennadi Liakhovetski wrote:
> 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?
I don't think we have an official maintainer. Maybe a Tested-by would be
enough in this case ?
> > 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);
--
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
