Re: [PATCH RFC] soc_camera: sensors: make v4l2_clk optional
Hi Laurent, Am 21.08.2013 23:20, schrieb Laurent Pinchart: Hi Frank, On Wednesday 21 August 2013 22:45:17 Frank Schäfer wrote: commit 9aea470b soc-camera: switch I2C subdevice drivers to use v4l2-clk made a v4l2_clk mandatory for each sensor. While this isn't necessary, it also broke the em28xx driver in connection with ov2640 subdevices and maybe other drivers outside soc_camera as well. While this probably fixes the issue, I don't think it's the way to go. Why do you want to force all users of these drivers to implement a v4l2_clk ? Please don't care about the soc_camera platform only. Things like this should be optional unless there isn't a good reason to force their usage. The em28xx driver should instead provide a clock. Maybe. If it makes sense / has sufficient benefits, I'll be glad to implement it. But at the moment, I don't see any benefits for em28xx, just the disadvantage of extra code and memory usage. I've asked some questions about v4l2-clk with regards to the em28xx+ov2640 scenario, but haven't got any answers yet. ;) Can you convice me ? If we can fix that in time reverting the patches until the next kernel version would have my preference. I agree. Regards, Frank Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- drivers/media/i2c/soc_camera/imx074.c | 12 ++-- drivers/media/i2c/soc_camera/mt9m001.c| 13 ++--- drivers/media/i2c/soc_camera/mt9m111.c|8 +--- drivers/media/i2c/soc_camera/mt9t031.c| 13 ++--- drivers/media/i2c/soc_camera/mt9t112.c|7 --- drivers/media/i2c/soc_camera/mt9v022.c| 13 ++--- drivers/media/i2c/soc_camera/ov2640.c | 13 ++--- drivers/media/i2c/soc_camera/ov5642.c |7 --- drivers/media/i2c/soc_camera/ov6650.c | 13 ++--- drivers/media/i2c/soc_camera/ov772x.c | 13 ++--- drivers/media/i2c/soc_camera/ov9640.c | 13 ++--- drivers/media/i2c/soc_camera/ov9740.c | 13 ++--- drivers/media/i2c/soc_camera/rj54n1cb0c.c | 13 ++--- drivers/media/i2c/soc_camera/tw9910.c |7 --- 14 Dateien geändert, 77 Zeilen hinzugefügt(+), 81 Zeilen entfernt(-) diff --git a/drivers/media/i2c/soc_camera/imx074.c b/drivers/media/i2c/soc_camera/imx074.c index 1d384a3..e7b6124 100644 --- a/drivers/media/i2c/soc_camera/imx074.c +++ b/drivers/media/i2c/soc_camera/imx074.c @@ -438,10 +438,8 @@ static int imx074_probe(struct i2c_client *client, priv-fmt = imx074_colour_fmts[0]; priv-clk = v4l2_clk_get(client-dev, mclk); -if (IS_ERR(priv-clk)) { -dev_info(client-dev, Error %ld getting clock\n, PTR_ERR(priv- clk)); -return -EPROBE_DEFER; -} +if (IS_ERR(priv-clk)) +priv-clk = NULL; ret = soc_camera_power_init(client-dev, ssdd); if (ret 0) @@ -455,7 +453,8 @@ static int imx074_probe(struct i2c_client *client, epwrinit: eprobe: -v4l2_clk_put(priv-clk); +if (priv-clk) +v4l2_clk_put(priv-clk); return ret; } @@ -465,7 +464,8 @@ static int imx074_remove(struct i2c_client *client) struct imx074 *priv = to_imx074(client); v4l2_async_unregister_subdev(priv-subdev); -v4l2_clk_put(priv-clk); +if (priv-clk) +v4l2_clk_put(priv-clk); if (ssdd-free_bus) ssdd-free_bus(ssdd); diff --git a/drivers/media/i2c/soc_camera/mt9m001.c b/drivers/media/i2c/soc_camera/mt9m001.c index df97033..07af1bc 100644 --- a/drivers/media/i2c/soc_camera/mt9m001.c +++ b/drivers/media/i2c/soc_camera/mt9m001.c @@ -685,15 +685,13 @@ static int mt9m001_probe(struct i2c_client *client, mt9m001-rect.height= MT9M001_MAX_HEIGHT; mt9m001-clk = v4l2_clk_get(client-dev, mclk); -if (IS_ERR(mt9m001-clk)) { -ret = PTR_ERR(mt9m001-clk); -goto eclkget; -} +if (IS_ERR(mt9m001-clk)) +mt9m001-clk = NULL; ret = mt9m001_video_probe(ssdd, client); if (ret) { -v4l2_clk_put(mt9m001-clk); -eclkget: +if (mt9m001-clk) +v4l2_clk_put(mt9m001-clk); v4l2_ctrl_handler_free(mt9m001-hdl); } @@ -705,7 +703,8 @@ static int mt9m001_remove(struct i2c_client *client) struct mt9m001 *mt9m001 = to_mt9m001(client); struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client); -v4l2_clk_put(mt9m001-clk); +if (mt9m001-clk) +v4l2_clk_put(mt9m001-clk); v4l2_device_unregister_subdev(mt9m001-subdev); v4l2_ctrl_handler_free(mt9m001-hdl); mt9m001_video_remove(ssdd); diff --git a/drivers/media/i2c/soc_camera/mt9m111.c b/drivers/media/i2c/soc_camera/mt9m111.c index 6f40566..498f22e 100644 --- a/drivers/media/i2c/soc_camera/mt9m111.c +++ b/drivers/media/i2c/soc_camera/mt9m111.c @@ -948,7 +948,7 @@ static int mt9m111_probe(struct i2c_client *client,
[PATCH RFC] soc_camera: sensors: make v4l2_clk optional
commit 9aea470b soc-camera: switch I2C subdevice drivers to use v4l2-clk made a v4l2_clk mandatory for each sensor. While this isn't necessary, it also broke the em28xx driver in connection with ov2640 subdevices and maybe other drivers outside soc_camera as well. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- drivers/media/i2c/soc_camera/imx074.c | 12 ++-- drivers/media/i2c/soc_camera/mt9m001.c| 13 ++--- drivers/media/i2c/soc_camera/mt9m111.c|8 +--- drivers/media/i2c/soc_camera/mt9t031.c| 13 ++--- drivers/media/i2c/soc_camera/mt9t112.c|7 --- drivers/media/i2c/soc_camera/mt9v022.c| 13 ++--- drivers/media/i2c/soc_camera/ov2640.c | 13 ++--- drivers/media/i2c/soc_camera/ov5642.c |7 --- drivers/media/i2c/soc_camera/ov6650.c | 13 ++--- drivers/media/i2c/soc_camera/ov772x.c | 13 ++--- drivers/media/i2c/soc_camera/ov9640.c | 13 ++--- drivers/media/i2c/soc_camera/ov9740.c | 13 ++--- drivers/media/i2c/soc_camera/rj54n1cb0c.c | 13 ++--- drivers/media/i2c/soc_camera/tw9910.c |7 --- 14 Dateien geändert, 77 Zeilen hinzugefügt(+), 81 Zeilen entfernt(-) diff --git a/drivers/media/i2c/soc_camera/imx074.c b/drivers/media/i2c/soc_camera/imx074.c index 1d384a3..e7b6124 100644 --- a/drivers/media/i2c/soc_camera/imx074.c +++ b/drivers/media/i2c/soc_camera/imx074.c @@ -438,10 +438,8 @@ static int imx074_probe(struct i2c_client *client, priv-fmt = imx074_colour_fmts[0]; priv-clk = v4l2_clk_get(client-dev, mclk); - if (IS_ERR(priv-clk)) { - dev_info(client-dev, Error %ld getting clock\n, PTR_ERR(priv-clk)); - return -EPROBE_DEFER; - } + if (IS_ERR(priv-clk)) + priv-clk = NULL; ret = soc_camera_power_init(client-dev, ssdd); if (ret 0) @@ -455,7 +453,8 @@ static int imx074_probe(struct i2c_client *client, epwrinit: eprobe: - v4l2_clk_put(priv-clk); + if (priv-clk) + v4l2_clk_put(priv-clk); return ret; } @@ -465,7 +464,8 @@ static int imx074_remove(struct i2c_client *client) struct imx074 *priv = to_imx074(client); v4l2_async_unregister_subdev(priv-subdev); - v4l2_clk_put(priv-clk); + if (priv-clk) + v4l2_clk_put(priv-clk); if (ssdd-free_bus) ssdd-free_bus(ssdd); diff --git a/drivers/media/i2c/soc_camera/mt9m001.c b/drivers/media/i2c/soc_camera/mt9m001.c index df97033..07af1bc 100644 --- a/drivers/media/i2c/soc_camera/mt9m001.c +++ b/drivers/media/i2c/soc_camera/mt9m001.c @@ -685,15 +685,13 @@ static int mt9m001_probe(struct i2c_client *client, mt9m001-rect.height= MT9M001_MAX_HEIGHT; mt9m001-clk = v4l2_clk_get(client-dev, mclk); - if (IS_ERR(mt9m001-clk)) { - ret = PTR_ERR(mt9m001-clk); - goto eclkget; - } + if (IS_ERR(mt9m001-clk)) + mt9m001-clk = NULL; ret = mt9m001_video_probe(ssdd, client); if (ret) { - v4l2_clk_put(mt9m001-clk); -eclkget: + if (mt9m001-clk) + v4l2_clk_put(mt9m001-clk); v4l2_ctrl_handler_free(mt9m001-hdl); } @@ -705,7 +703,8 @@ static int mt9m001_remove(struct i2c_client *client) struct mt9m001 *mt9m001 = to_mt9m001(client); struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client); - v4l2_clk_put(mt9m001-clk); + if (mt9m001-clk) + v4l2_clk_put(mt9m001-clk); v4l2_device_unregister_subdev(mt9m001-subdev); v4l2_ctrl_handler_free(mt9m001-hdl); mt9m001_video_remove(ssdd); diff --git a/drivers/media/i2c/soc_camera/mt9m111.c b/drivers/media/i2c/soc_camera/mt9m111.c index 6f40566..498f22e 100644 --- a/drivers/media/i2c/soc_camera/mt9m111.c +++ b/drivers/media/i2c/soc_camera/mt9m111.c @@ -948,7 +948,7 @@ static int mt9m111_probe(struct i2c_client *client, mt9m111-clk = v4l2_clk_get(client-dev, mclk); if (IS_ERR(mt9m111-clk)) - return -EPROBE_DEFER; + mt9m111-clk = NULL; /* Default HIGHPOWER context */ mt9m111-ctx = context_b; @@ -999,7 +999,8 @@ static int mt9m111_probe(struct i2c_client *client, out_hdlfree: v4l2_ctrl_handler_free(mt9m111-hdl); out_clkput: - v4l2_clk_put(mt9m111-clk); + if (mt9m111-clk) + v4l2_clk_put(mt9m111-clk); return ret; } @@ -1009,7 +1010,8 @@ static int mt9m111_remove(struct i2c_client *client) struct mt9m111 *mt9m111 = to_mt9m111(client); v4l2_async_unregister_subdev(mt9m111-subdev); - v4l2_clk_put(mt9m111-clk); + if (mt9m111-clk) + v4l2_clk_put(mt9m111-clk); v4l2_device_unregister_subdev(mt9m111-subdev); v4l2_ctrl_handler_free(mt9m111-hdl); diff
Re: [PATCH RFC] soc_camera: sensors: make v4l2_clk optional
Hi Frank, On Wednesday 21 August 2013 22:45:17 Frank Schäfer wrote: commit 9aea470b soc-camera: switch I2C subdevice drivers to use v4l2-clk made a v4l2_clk mandatory for each sensor. While this isn't necessary, it also broke the em28xx driver in connection with ov2640 subdevices and maybe other drivers outside soc_camera as well. While this probably fixes the issue, I don't think it's the way to go. The em28xx driver should instead provide a clock. If we can fix that in time reverting the patches until the next kernel version would have my preference. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- drivers/media/i2c/soc_camera/imx074.c | 12 ++-- drivers/media/i2c/soc_camera/mt9m001.c| 13 ++--- drivers/media/i2c/soc_camera/mt9m111.c|8 +--- drivers/media/i2c/soc_camera/mt9t031.c| 13 ++--- drivers/media/i2c/soc_camera/mt9t112.c|7 --- drivers/media/i2c/soc_camera/mt9v022.c| 13 ++--- drivers/media/i2c/soc_camera/ov2640.c | 13 ++--- drivers/media/i2c/soc_camera/ov5642.c |7 --- drivers/media/i2c/soc_camera/ov6650.c | 13 ++--- drivers/media/i2c/soc_camera/ov772x.c | 13 ++--- drivers/media/i2c/soc_camera/ov9640.c | 13 ++--- drivers/media/i2c/soc_camera/ov9740.c | 13 ++--- drivers/media/i2c/soc_camera/rj54n1cb0c.c | 13 ++--- drivers/media/i2c/soc_camera/tw9910.c |7 --- 14 Dateien geändert, 77 Zeilen hinzugefügt(+), 81 Zeilen entfernt(-) diff --git a/drivers/media/i2c/soc_camera/imx074.c b/drivers/media/i2c/soc_camera/imx074.c index 1d384a3..e7b6124 100644 --- a/drivers/media/i2c/soc_camera/imx074.c +++ b/drivers/media/i2c/soc_camera/imx074.c @@ -438,10 +438,8 @@ static int imx074_probe(struct i2c_client *client, priv-fmt = imx074_colour_fmts[0]; priv-clk = v4l2_clk_get(client-dev, mclk); - if (IS_ERR(priv-clk)) { - dev_info(client-dev, Error %ld getting clock\n, PTR_ERR(priv- clk)); - return -EPROBE_DEFER; - } + if (IS_ERR(priv-clk)) + priv-clk = NULL; ret = soc_camera_power_init(client-dev, ssdd); if (ret 0) @@ -455,7 +453,8 @@ static int imx074_probe(struct i2c_client *client, epwrinit: eprobe: - v4l2_clk_put(priv-clk); + if (priv-clk) + v4l2_clk_put(priv-clk); return ret; } @@ -465,7 +464,8 @@ static int imx074_remove(struct i2c_client *client) struct imx074 *priv = to_imx074(client); v4l2_async_unregister_subdev(priv-subdev); - v4l2_clk_put(priv-clk); + if (priv-clk) + v4l2_clk_put(priv-clk); if (ssdd-free_bus) ssdd-free_bus(ssdd); diff --git a/drivers/media/i2c/soc_camera/mt9m001.c b/drivers/media/i2c/soc_camera/mt9m001.c index df97033..07af1bc 100644 --- a/drivers/media/i2c/soc_camera/mt9m001.c +++ b/drivers/media/i2c/soc_camera/mt9m001.c @@ -685,15 +685,13 @@ static int mt9m001_probe(struct i2c_client *client, mt9m001-rect.height= MT9M001_MAX_HEIGHT; mt9m001-clk = v4l2_clk_get(client-dev, mclk); - if (IS_ERR(mt9m001-clk)) { - ret = PTR_ERR(mt9m001-clk); - goto eclkget; - } + if (IS_ERR(mt9m001-clk)) + mt9m001-clk = NULL; ret = mt9m001_video_probe(ssdd, client); if (ret) { - v4l2_clk_put(mt9m001-clk); -eclkget: + if (mt9m001-clk) + v4l2_clk_put(mt9m001-clk); v4l2_ctrl_handler_free(mt9m001-hdl); } @@ -705,7 +703,8 @@ static int mt9m001_remove(struct i2c_client *client) struct mt9m001 *mt9m001 = to_mt9m001(client); struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client); - v4l2_clk_put(mt9m001-clk); + if (mt9m001-clk) + v4l2_clk_put(mt9m001-clk); v4l2_device_unregister_subdev(mt9m001-subdev); v4l2_ctrl_handler_free(mt9m001-hdl); mt9m001_video_remove(ssdd); diff --git a/drivers/media/i2c/soc_camera/mt9m111.c b/drivers/media/i2c/soc_camera/mt9m111.c index 6f40566..498f22e 100644 --- a/drivers/media/i2c/soc_camera/mt9m111.c +++ b/drivers/media/i2c/soc_camera/mt9m111.c @@ -948,7 +948,7 @@ static int mt9m111_probe(struct i2c_client *client, mt9m111-clk = v4l2_clk_get(client-dev, mclk); if (IS_ERR(mt9m111-clk)) - return -EPROBE_DEFER; + mt9m111-clk = NULL; /* Default HIGHPOWER context */ mt9m111-ctx = context_b; @@ -999,7 +999,8 @@ static int mt9m111_probe(struct i2c_client *client, out_hdlfree: v4l2_ctrl_handler_free(mt9m111-hdl); out_clkput: - v4l2_clk_put(mt9m111-clk); + if (mt9m111-clk) + v4l2_clk_put(mt9m111-clk); return ret; } @@ -1009,7 +1010,8 @@ static int mt9m111_remove(struct i2c_client *client) struct mt9m111