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
