[media] ov2640: add support for async device registration
Commit Message
Move the clock detection code to the beginning of the probe().
If we meet any error in the clock detecting, then defer the probe.
Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
drivers/media/i2c/soc_camera/ov2640.c | 43 +++++++++++++++++++++------------
1 file changed, 28 insertions(+), 15 deletions(-)
Comments
Hi Josh,
On 03/14/2014 11:12 AM, Josh Wu wrote:
> + clk = v4l2_clk_get(&client->dev, "mclk");
> + if (IS_ERR(clk))
> + return -EPROBE_DEFER;
You should instead make it:
return PTR_ERR(clk);
But you will need this patch for that to work:
http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/clk/clk.c?id=a34cd4666f3da84228a82f70c94b8d9b692034ea
With this patch there is no need to overwrite any returned error
value with EPROBE_DEFER.
> @@ -1097,23 +1106,26 @@ static int ov2640_probe(struct i2c_client *client,
> v4l2_ctrl_new_std(&priv->hdl,&ov2640_ctrl_ops,
> V4L2_CID_HFLIP, 0, 1, 1, 0);
> priv->subdev.ctrl_handler =&priv->hdl;
> - if (priv->hdl.error)
> - return priv->hdl.error;
> -
> - priv->clk = v4l2_clk_get(&client->dev, "mclk");
> - if (IS_ERR(priv->clk)) {
> - ret = PTR_ERR(priv->clk);
> - goto eclkget;
--
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Sylwester
Thanks for your review.
On 3/15/2014 5:17 AM, Sylwester Nawrocki wrote:
> Hi Josh,
>
> On 03/14/2014 11:12 AM, Josh Wu wrote:
>> + clk = v4l2_clk_get(&client->dev, "mclk");
>> + if (IS_ERR(clk))
>> + return -EPROBE_DEFER;
>
> You should instead make it:
>
> return PTR_ERR(clk);
>
> But you will need this patch for that to work:
> http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/clk/clk.c?id=a34cd4666f3da84228a82f70c94b8d9b692034ea
>
>
> With this patch there is no need to overwrite any returned error
> value with EPROBE_DEFER.
Thanks for the information. I will use this in v2 version.
Best Regards,
Josh Wu
>
>> @@ -1097,23 +1106,26 @@ static int ov2640_probe(struct i2c_client
>> *client,
>> v4l2_ctrl_new_std(&priv->hdl,&ov2640_ctrl_ops,
>> V4L2_CID_HFLIP, 0, 1, 1, 0);
>> priv->subdev.ctrl_handler =&priv->hdl;
>> - if (priv->hdl.error)
>> - return priv->hdl.error;
>> -
>> - priv->clk = v4l2_clk_get(&client->dev, "mclk");
>> - if (IS_ERR(priv->clk)) {
>> - ret = PTR_ERR(priv->clk);
>> - goto eclkget;
>
> --
> Regards,
> Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Josh,
On 19/03/14 10:17, Josh Wu wrote:
> On 3/15/2014 5:17 AM, Sylwester Nawrocki wrote:
>> > On 03/14/2014 11:12 AM, Josh Wu wrote:
>>> >> + clk = v4l2_clk_get(&client->dev, "mclk");
>>> >> + if (IS_ERR(clk))
>>> >> + return -EPROBE_DEFER;
>> >
>> > You should instead make it:
>> >
>> > return PTR_ERR(clk);
>> >
>> > But you will need this patch for that to work:
>> > http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/clk/clk.c?id=a34cd4666f3da84228a82f70c94b8d9b692034ea
>> >
>> >
>> > With this patch there is no need to overwrite any returned error
>> > value with EPROBE_DEFER.
>
> Thanks for the information. I will use this in v2 version.
Oops, I missed somehow that it's v4l2_clk_get(), rather than
clk_get(). So it seems it will not work when you return PTR_ERR(clk),
since v4l2_clk_get() returns -ENODEV when clock is not found.
I think we should modify v4l2_clk_get() so it returns EPROBE_DEFER
rather than ENODEV on error. I anticipate v4l2_clk_get() might be
using clk_get() internally in future, and the v4l2 clk look up
will be used as a fallback only. So sensor drivers should just
do something like:
clk = v4l2_clk_get(...);
if (IS_ERR(clk))
return PTR_ERR(clk);
--
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Sylwester
On 3/20/2014 10:44 PM, Sylwester Nawrocki wrote:
> Hi Josh,
>
> On 19/03/14 10:17, Josh Wu wrote:
>> On 3/15/2014 5:17 AM, Sylwester Nawrocki wrote:
>>>> On 03/14/2014 11:12 AM, Josh Wu wrote:
>>>>>> + clk = v4l2_clk_get(&client->dev, "mclk");
>>>>>> + if (IS_ERR(clk))
>>>>>> + return -EPROBE_DEFER;
>>>> You should instead make it:
>>>>
>>>> return PTR_ERR(clk);
>>>>
>>>> But you will need this patch for that to work:
>>>> http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/clk/clk.c?id=a34cd4666f3da84228a82f70c94b8d9b692034ea
>>>>
>>>>
>>>> With this patch there is no need to overwrite any returned error
>>>> value with EPROBE_DEFER.
>> Thanks for the information. I will use this in v2 version.
> Oops, I missed somehow that it's v4l2_clk_get(), rather than
> clk_get(). So it seems it will not work when you return PTR_ERR(clk),
> since v4l2_clk_get() returns -ENODEV when clock is not found.
right, I missed that. So this version is still valid one. The v2 that I
already sent should be dropped.
> I think we should modify v4l2_clk_get() so it returns EPROBE_DEFER
> rather than ENODEV on error. I anticipate v4l2_clk_get() might be
> using clk_get() internally in future, and the v4l2 clk look up
> will be used as a fallback only. So sensor drivers should just
> do something like:
>
> clk = v4l2_clk_get(...);
> if (IS_ERR(clk))
> return PTR_ERR(clk);
I noticed that there are some driver like ov772x, ov9640 and etc, are
not supported the async probe yet.
If we return the EPROBE_DEFER for all v4l2_clk_get(), that will cause
the no-async probe function work incorrectly.
So IMO we can do your above suggestion after all sensors support async
probe.
Thanks.
>
> --
> Regards,
> Sylwester
Best Regards,
Josh Wu
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -22,6 +22,7 @@
#include <linux/videodev2.h>
#include <media/soc_camera.h>
+#include <media/v4l2-async.h>
#include <media/v4l2-clk.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-ctrls.h>
@@ -1069,6 +1070,7 @@ static int ov2640_probe(struct i2c_client *client,
struct ov2640_priv *priv;
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+ struct v4l2_clk *clk;
int ret;
if (!ssdd) {
@@ -1083,13 +1085,20 @@ static int ov2640_probe(struct i2c_client *client,
return -EIO;
}
+ clk = v4l2_clk_get(&client->dev, "mclk");
+ if (IS_ERR(clk))
+ return -EPROBE_DEFER;
+
priv = devm_kzalloc(&client->dev, sizeof(struct ov2640_priv), GFP_KERNEL);
if (!priv) {
dev_err(&adapter->dev,
"Failed to allocate memory for private data!\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto err_kzalloc;
}
+ priv->clk = clk;
+
v4l2_i2c_subdev_init(&priv->subdev, client, &ov2640_subdev_ops);
v4l2_ctrl_handler_init(&priv->hdl, 2);
v4l2_ctrl_new_std(&priv->hdl, &ov2640_ctrl_ops,
@@ -1097,23 +1106,26 @@ static int ov2640_probe(struct i2c_client *client,
v4l2_ctrl_new_std(&priv->hdl, &ov2640_ctrl_ops,
V4L2_CID_HFLIP, 0, 1, 1, 0);
priv->subdev.ctrl_handler = &priv->hdl;
- if (priv->hdl.error)
- return priv->hdl.error;
-
- priv->clk = v4l2_clk_get(&client->dev, "mclk");
- if (IS_ERR(priv->clk)) {
- ret = PTR_ERR(priv->clk);
- goto eclkget;
+ if (priv->hdl.error) {
+ ret = priv->hdl.error;
+ goto err_kzalloc;
}
ret = ov2640_video_probe(client);
- if (ret) {
- v4l2_clk_put(priv->clk);
-eclkget:
- v4l2_ctrl_handler_free(&priv->hdl);
- } else {
- dev_info(&adapter->dev, "OV2640 Probed\n");
- }
+ if (ret)
+ goto err_probe;
+
+ ret = v4l2_async_register_subdev(&priv->subdev);
+ if (ret)
+ goto err_probe;
+
+ dev_info(&adapter->dev, "OV2640 Probed\n");
+ return 0;
+
+err_probe:
+ v4l2_ctrl_handler_free(&priv->hdl);
+err_kzalloc:
+ v4l2_clk_put(clk);
return ret;
}
@@ -1122,6 +1134,7 @@ static int ov2640_remove(struct i2c_client *client)
{
struct ov2640_priv *priv = to_ov2640(client);
+ v4l2_async_unregister_subdev(&priv->subdev);
v4l2_clk_put(priv->clk);
v4l2_device_unregister_subdev(&priv->subdev);
v4l2_ctrl_handler_free(&priv->hdl);