* [PATCH 0/2] media: ov6650: Add V4L2 asynchronous subdevice support @ 2019-03-25 0:34 Janusz Krzysztofik 2019-03-25 0:35 ` [PATCH 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper Janusz Krzysztofik ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Janusz Krzysztofik @ 2019-03-25 0:34 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Hans Verkuil, Sakari Ailus, linux-media, linux-kernel, Janusz Krzysztofik Janusz Krzysztofik (2): media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper media: ov6650: Register with asynchronous subdevice framework ov6650.c | 45 +++++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 16 deletions(-) ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper 2019-03-25 0:34 [PATCH 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik @ 2019-03-25 0:35 ` Janusz Krzysztofik 2019-03-29 18:39 ` Sakari Ailus 2019-03-25 0:35 ` [PATCH 2/2] media: ov6650: Register with asynchronous subdevice framework Janusz Krzysztofik 2019-03-29 20:29 ` [PATCH v2 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik 2 siblings, 1 reply; 14+ messages in thread From: Janusz Krzysztofik @ 2019-03-25 0:35 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Hans Verkuil, Sakari Ailus, linux-media, linux-kernel, Janusz Krzysztofik In preparation for adding asynchronous subdevice support to the driver, don't acquire v4l2_clk from the driver .probe() callback as that may fail if the clock is provided by a bridge driver which may be not yet initialized. Move the v4l2_clk_get() to ov6650_video_probe() helper which is going to be converted to v4l2_subdev_internal_ops.registered() callback, executed only when the bridge driver is ready. Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> --- drivers/media/i2c/ov6650.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index c33fd584cb44..f10b8053ed73 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -810,9 +810,16 @@ static int ov6650_video_probe(struct i2c_client *client) u8 pidh, pidl, midh, midl; int ret; + priv->clk = v4l2_clk_get(&client->dev, NULL); + if (IS_ERR(priv->clk)) { + ret = PTR_ERR(priv->clk); + dev_err(&client->dev, "v4l2_clk request err: %d\n", ret); + return ret; + } + ret = ov6650_s_power(&priv->subdev, 1); if (ret < 0) - return ret; + goto eclkput; /* * check and show product ID and manufacturer ID @@ -847,6 +854,11 @@ static int ov6650_video_probe(struct i2c_client *client) done: ov6650_s_power(&priv->subdev, 0); + if (ret) { +eclkput: + v4l2_clk_put(priv->clk); + } + return ret; } @@ -989,18 +1001,9 @@ static int ov6650_probe(struct i2c_client *client, priv->code = MEDIA_BUS_FMT_YUYV8_2X8; priv->colorspace = V4L2_COLORSPACE_JPEG; - priv->clk = v4l2_clk_get(&client->dev, NULL); - if (IS_ERR(priv->clk)) { - ret = PTR_ERR(priv->clk); - goto eclkget; - } - ret = ov6650_video_probe(client); - if (ret) { - v4l2_clk_put(priv->clk); -eclkget: + if (ret) v4l2_ctrl_handler_free(&priv->hdl); - } return ret; } -- 2.19.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper 2019-03-25 0:35 ` [PATCH 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper Janusz Krzysztofik @ 2019-03-29 18:39 ` Sakari Ailus 2019-03-29 19:48 ` Janusz Krzysztofik 0 siblings, 1 reply; 14+ messages in thread From: Sakari Ailus @ 2019-03-29 18:39 UTC (permalink / raw) To: Janusz Krzysztofik Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel Hi Janus, Thanks for the patchset. On Mon, Mar 25, 2019 at 01:35:00AM +0100, Janusz Krzysztofik wrote: > In preparation for adding asynchronous subdevice support to the driver, > don't acquire v4l2_clk from the driver .probe() callback as that may > fail if the clock is provided by a bridge driver which may be not yet > initialized. Move the v4l2_clk_get() to ov6650_video_probe() helper > which is going to be converted to v4l2_subdev_internal_ops.registered() > callback, executed only when the bridge driver is ready. > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > --- > drivers/media/i2c/ov6650.c | 25 ++++++++++++++----------- > 1 file changed, 14 insertions(+), 11 deletions(-) > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c > index c33fd584cb44..f10b8053ed73 100644 > --- a/drivers/media/i2c/ov6650.c > +++ b/drivers/media/i2c/ov6650.c > @@ -810,9 +810,16 @@ static int ov6650_video_probe(struct i2c_client *client) > u8 pidh, pidl, midh, midl; > int ret; > > + priv->clk = v4l2_clk_get(&client->dev, NULL); > + if (IS_ERR(priv->clk)) { > + ret = PTR_ERR(priv->clk); > + dev_err(&client->dev, "v4l2_clk request err: %d\n", ret); > + return ret; > + } > + > ret = ov6650_s_power(&priv->subdev, 1); > if (ret < 0) > - return ret; > + goto eclkput; > > /* > * check and show product ID and manufacturer ID > @@ -847,6 +854,11 @@ static int ov6650_video_probe(struct i2c_client *client) > > done: > ov6650_s_power(&priv->subdev, 0); > + if (ret) { > +eclkput: > + v4l2_clk_put(priv->clk); Could you rework the error handling, please? Labels should be outside conditionals. See e.g. the smiapp driver (drivers/media/i2c/smiapp/smiapp-core.c). I've pushed my master branch here: <URL:https://git.linuxtv.org/sailus/media_tree.git/log/> The patch also does not apply on top of the media tree master + your earlier patch cleanly. > + } > + > return ret; > } > > @@ -989,18 +1001,9 @@ static int ov6650_probe(struct i2c_client *client, > priv->code = MEDIA_BUS_FMT_YUYV8_2X8; > priv->colorspace = V4L2_COLORSPACE_JPEG; > > - priv->clk = v4l2_clk_get(&client->dev, NULL); > - if (IS_ERR(priv->clk)) { > - ret = PTR_ERR(priv->clk); > - goto eclkget; > - } > - > ret = ov6650_video_probe(client); > - if (ret) { > - v4l2_clk_put(priv->clk); > -eclkget: > + if (ret) > v4l2_ctrl_handler_free(&priv->hdl); > - } > > return ret; > } -- Kind regards, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper 2019-03-29 18:39 ` Sakari Ailus @ 2019-03-29 19:48 ` Janusz Krzysztofik 2019-03-29 19:51 ` Sakari Ailus 0 siblings, 1 reply; 14+ messages in thread From: Janusz Krzysztofik @ 2019-03-29 19:48 UTC (permalink / raw) To: Sakari Ailus Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel Hi Sakari, On Friday, March 29, 2019 7:39:24 PM CET Sakari Ailus wrote: > Hi Janus, > > Thanks for the patchset. > > On Mon, Mar 25, 2019 at 01:35:00AM +0100, Janusz Krzysztofik wrote: > > ... > > > > done: > > ov6650_s_power(&priv->subdev, 0); > > + if (ret) { > > +eclkput: > > + v4l2_clk_put(priv->clk); > > Could you rework the error handling, please? Labels should be outside > conditionals. OK. > See e.g. the smiapp driver > (drivers/media/i2c/smiapp/smiapp-core.c). I've pushed my master branch > here: > > <URL:https://git.linuxtv.org/sailus/media_tree.git/log/> I'm currently syncing my linux-media remote tracking branches with git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media.git Should I switch to a different repository, e.g. yours? Thanks, Janusz ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper 2019-03-29 19:48 ` Janusz Krzysztofik @ 2019-03-29 19:51 ` Sakari Ailus 0 siblings, 0 replies; 14+ messages in thread From: Sakari Ailus @ 2019-03-29 19:51 UTC (permalink / raw) To: Janusz Krzysztofik Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel On Fri, Mar 29, 2019 at 08:48:19PM +0100, Janusz Krzysztofik wrote: > Hi Sakari, > > On Friday, March 29, 2019 7:39:24 PM CET Sakari Ailus wrote: > > Hi Janus, > > > > Thanks for the patchset. > > > > On Mon, Mar 25, 2019 at 01:35:00AM +0100, Janusz Krzysztofik wrote: > > > ... > > > > > > done: > > > ov6650_s_power(&priv->subdev, 0); > > > + if (ret) { > > > +eclkput: > > > + v4l2_clk_put(priv->clk); > > > > Could you rework the error handling, please? Labels should be outside > > conditionals. > > OK. > > > See e.g. the smiapp driver > > (drivers/media/i2c/smiapp/smiapp-core.c). I've pushed my master branch > > here: > > > > <URL:https://git.linuxtv.org/sailus/media_tree.git/log/> > > I'm currently syncing my linux-media remote tracking branches with > git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media.git > Should I switch to a different repository, e.g. yours? Usually Mauro's tree is the best choice. I sometimes forget to update mine. In this case just make sure these patches apply on top of the one merged previouly if there's a dependency. -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] media: ov6650: Register with asynchronous subdevice framework 2019-03-25 0:34 [PATCH 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik 2019-03-25 0:35 ` [PATCH 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper Janusz Krzysztofik @ 2019-03-25 0:35 ` Janusz Krzysztofik 2019-03-29 20:29 ` [PATCH v2 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik 2 siblings, 0 replies; 14+ messages in thread From: Janusz Krzysztofik @ 2019-03-25 0:35 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Hans Verkuil, Sakari Ailus, linux-media, linux-kernel, Janusz Krzysztofik Register V4L2 subdevice implemented by the driver to the V4L2 asynchronous subdevice framework. Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> --- drivers/media/i2c/ov6650.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index f10b8053ed73..cc9e5388d5ec 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -804,8 +804,9 @@ static int ov6650_prog_dflt(struct i2c_client *client) return ret; } -static int ov6650_video_probe(struct i2c_client *client) +static int ov6650_video_probe(struct v4l2_subdev *sd) { + struct i2c_client *client = v4l2_get_subdevdata(sd); struct ov6650 *priv = to_ov6650(client); u8 pidh, pidl, midh, midl; int ret; @@ -817,7 +818,7 @@ static int ov6650_video_probe(struct i2c_client *client) return ret; } - ret = ov6650_s_power(&priv->subdev, 1); + ret = ov6650_s_power(sd, 1); if (ret < 0) goto eclkput; @@ -853,7 +854,7 @@ static int ov6650_video_probe(struct i2c_client *client) ret = v4l2_ctrl_handler_setup(&priv->hdl); done: - ov6650_s_power(&priv->subdev, 0); + ov6650_s_power(sd, 0); if (ret) { eclkput: v4l2_clk_put(priv->clk); @@ -941,6 +942,10 @@ static const struct v4l2_subdev_ops ov6650_subdev_ops = { .pad = &ov6650_pad_ops, }; +static const struct v4l2_subdev_internal_ops ov6650_internal_ops = { + .registered = ov6650_video_probe, +}; + /* * i2c_driver function */ @@ -1001,7 +1006,12 @@ static int ov6650_probe(struct i2c_client *client, priv->code = MEDIA_BUS_FMT_YUYV8_2X8; priv->colorspace = V4L2_COLORSPACE_JPEG; - ret = ov6650_video_probe(client); + priv->subdev.internal_ops = &ov6650_internal_ops; + priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + snprintf(priv->subdev.name, sizeof(priv->subdev.name), "%s %d-%04x", + did->name, i2c_adapter_id(client->adapter), client->addr); + + ret = v4l2_async_register_subdev(&priv->subdev); if (ret) v4l2_ctrl_handler_free(&priv->hdl); @@ -1013,7 +1023,7 @@ static int ov6650_remove(struct i2c_client *client) struct ov6650 *priv = to_ov6650(client); v4l2_clk_put(priv->clk); - v4l2_device_unregister_subdev(&priv->subdev); + v4l2_async_unregister_subdev(&priv->subdev); v4l2_ctrl_handler_free(&priv->hdl); return 0; } -- 2.19.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 0/2] media: ov6650: Add V4L2 asynchronous subdevice support 2019-03-25 0:34 [PATCH 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik 2019-03-25 0:35 ` [PATCH 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper Janusz Krzysztofik 2019-03-25 0:35 ` [PATCH 2/2] media: ov6650: Register with asynchronous subdevice framework Janusz Krzysztofik @ 2019-03-29 20:29 ` Janusz Krzysztofik 2019-03-29 20:29 ` [PATCH v2 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper Janusz Krzysztofik ` (2 more replies) 2 siblings, 3 replies; 14+ messages in thread From: Janusz Krzysztofik @ 2019-03-29 20:29 UTC (permalink / raw) To: Sakari Ailus Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel, Janusz Krzysztofik Janusz Krzysztofik (2): media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper media: ov6650: Register with asynchronous subdevice framework ov6650.c | 45 +++++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 16 deletions(-) Changelog: v1->v2: - labels should be outside conditionals - thanks Sakari - rebased on my former fix so it applied cleanly ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper 2019-03-29 20:29 ` [PATCH v2 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik @ 2019-03-29 20:29 ` Janusz Krzysztofik 2019-03-29 20:29 ` [PATCH v2 2/2] media: ov6650: Register with asynchronous subdevice framework Janusz Krzysztofik 2019-03-30 1:06 ` [PATCH v3 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik 2 siblings, 0 replies; 14+ messages in thread From: Janusz Krzysztofik @ 2019-03-29 20:29 UTC (permalink / raw) To: Sakari Ailus Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel, Janusz Krzysztofik In preparation for adding asynchronous subdevice support to the driver, don't acquire v4l2_clk from the driver .probe() callback as that may fail if the clock is provided by a bridge driver which may be not yet initialized. Move the v4l2_clk_get() to ov6650_video_probe() helper which is going to be converted to v4l2_subdev_internal_ops.registered() callback, executed only when the bridge driver is ready. Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> --- drivers/media/i2c/ov6650.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index f9359b11fa5c..de7d9790f054 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -810,9 +810,16 @@ static int ov6650_video_probe(struct i2c_client *client) u8 pidh, pidl, midh, midl; int ret; + priv->clk = v4l2_clk_get(&client->dev, NULL); + if (IS_ERR(priv->clk)) { + ret = PTR_ERR(priv->clk); + dev_err(&client->dev, "v4l2_clk request err: %d\n", ret); + return ret; + } + ret = ov6650_s_power(&priv->subdev, 1); if (ret < 0) - return ret; + goto eclkput; msleep(20); @@ -849,6 +856,11 @@ static int ov6650_video_probe(struct i2c_client *client) done: ov6650_s_power(&priv->subdev, 0); + if (!ret) + return 0; +eclkput: + v4l2_clk_put(priv->clk); + return ret; } @@ -991,18 +1003,9 @@ static int ov6650_probe(struct i2c_client *client, priv->code = MEDIA_BUS_FMT_YUYV8_2X8; priv->colorspace = V4L2_COLORSPACE_JPEG; - priv->clk = v4l2_clk_get(&client->dev, NULL); - if (IS_ERR(priv->clk)) { - ret = PTR_ERR(priv->clk); - goto eclkget; - } - ret = ov6650_video_probe(client); - if (ret) { - v4l2_clk_put(priv->clk); -eclkget: + if (ret) v4l2_ctrl_handler_free(&priv->hdl); - } return ret; } -- 2.19.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/2] media: ov6650: Register with asynchronous subdevice framework 2019-03-29 20:29 ` [PATCH v2 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik 2019-03-29 20:29 ` [PATCH v2 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper Janusz Krzysztofik @ 2019-03-29 20:29 ` Janusz Krzysztofik 2019-03-29 22:35 ` Sakari Ailus 2019-03-30 1:06 ` [PATCH v3 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik 2 siblings, 1 reply; 14+ messages in thread From: Janusz Krzysztofik @ 2019-03-29 20:29 UTC (permalink / raw) To: Sakari Ailus Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel, Janusz Krzysztofik Register V4L2 subdevice implemented by the driver to the V4L2 asynchronous subdevice framework. Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> --- drivers/media/i2c/ov6650.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index de7d9790f054..50e60da73036 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -804,8 +804,9 @@ static int ov6650_prog_dflt(struct i2c_client *client) return ret; } -static int ov6650_video_probe(struct i2c_client *client) +static int ov6650_video_probe(struct v4l2_subdev *sd) { + struct i2c_client *client = v4l2_get_subdevdata(sd); struct ov6650 *priv = to_ov6650(client); u8 pidh, pidl, midh, midl; int ret; @@ -817,7 +818,7 @@ static int ov6650_video_probe(struct i2c_client *client) return ret; } - ret = ov6650_s_power(&priv->subdev, 1); + ret = ov6650_s_power(sd, 1); if (ret < 0) goto eclkput; @@ -855,7 +856,7 @@ static int ov6650_video_probe(struct i2c_client *client) ret = v4l2_ctrl_handler_setup(&priv->hdl); done: - ov6650_s_power(&priv->subdev, 0); + ov6650_s_power(sd, 0); if (!ret) return 0; eclkput: @@ -943,6 +944,10 @@ static const struct v4l2_subdev_ops ov6650_subdev_ops = { .pad = &ov6650_pad_ops, }; +static const struct v4l2_subdev_internal_ops ov6650_internal_ops = { + .registered = ov6650_video_probe, +}; + /* * i2c_driver function */ @@ -1003,7 +1008,12 @@ static int ov6650_probe(struct i2c_client *client, priv->code = MEDIA_BUS_FMT_YUYV8_2X8; priv->colorspace = V4L2_COLORSPACE_JPEG; - ret = ov6650_video_probe(client); + priv->subdev.internal_ops = &ov6650_internal_ops; + priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + snprintf(priv->subdev.name, sizeof(priv->subdev.name), "%s %d-%04x", + did->name, i2c_adapter_id(client->adapter), client->addr); + + ret = v4l2_async_register_subdev(&priv->subdev); if (ret) v4l2_ctrl_handler_free(&priv->hdl); @@ -1015,7 +1025,7 @@ static int ov6650_remove(struct i2c_client *client) struct ov6650 *priv = to_ov6650(client); v4l2_clk_put(priv->clk); - v4l2_device_unregister_subdev(&priv->subdev); + v4l2_async_unregister_subdev(&priv->subdev); v4l2_ctrl_handler_free(&priv->hdl); return 0; } -- 2.19.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] media: ov6650: Register with asynchronous subdevice framework 2019-03-29 20:29 ` [PATCH v2 2/2] media: ov6650: Register with asynchronous subdevice framework Janusz Krzysztofik @ 2019-03-29 22:35 ` Sakari Ailus 0 siblings, 0 replies; 14+ messages in thread From: Sakari Ailus @ 2019-03-29 22:35 UTC (permalink / raw) To: Janusz Krzysztofik Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel Hi Janusz, On Fri, Mar 29, 2019 at 09:29:03PM +0100, Janusz Krzysztofik wrote: > Register V4L2 subdevice implemented by the driver to the V4L2 > asynchronous subdevice framework. > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > --- > drivers/media/i2c/ov6650.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c > index de7d9790f054..50e60da73036 100644 > --- a/drivers/media/i2c/ov6650.c > +++ b/drivers/media/i2c/ov6650.c > @@ -804,8 +804,9 @@ static int ov6650_prog_dflt(struct i2c_client *client) > return ret; > } > > -static int ov6650_video_probe(struct i2c_client *client) > +static int ov6650_video_probe(struct v4l2_subdev *sd) > { > + struct i2c_client *client = v4l2_get_subdevdata(sd); > struct ov6650 *priv = to_ov6650(client); > u8 pidh, pidl, midh, midl; > int ret; > @@ -817,7 +818,7 @@ static int ov6650_video_probe(struct i2c_client *client) > return ret; > } > > - ret = ov6650_s_power(&priv->subdev, 1); > + ret = ov6650_s_power(sd, 1); > if (ret < 0) > goto eclkput; > > @@ -855,7 +856,7 @@ static int ov6650_video_probe(struct i2c_client *client) > ret = v4l2_ctrl_handler_setup(&priv->hdl); > > done: > - ov6650_s_power(&priv->subdev, 0); > + ov6650_s_power(sd, 0); > if (!ret) > return 0; > eclkput: > @@ -943,6 +944,10 @@ static const struct v4l2_subdev_ops ov6650_subdev_ops = { > .pad = &ov6650_pad_ops, > }; > > +static const struct v4l2_subdev_internal_ops ov6650_internal_ops = { > + .registered = ov6650_video_probe, > +}; > + > /* > * i2c_driver function > */ > @@ -1003,7 +1008,12 @@ static int ov6650_probe(struct i2c_client *client, > priv->code = MEDIA_BUS_FMT_YUYV8_2X8; > priv->colorspace = V4L2_COLORSPACE_JPEG; > > - ret = ov6650_video_probe(client); > + priv->subdev.internal_ops = &ov6650_internal_ops; > + priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > + snprintf(priv->subdev.name, sizeof(priv->subdev.name), "%s %d-%04x", > + did->name, i2c_adapter_id(client->adapter), client->addr); Could you use v4l2_i2c_subdev_set_name()? > + > + ret = v4l2_async_register_subdev(&priv->subdev); > if (ret) > v4l2_ctrl_handler_free(&priv->hdl); > > @@ -1015,7 +1025,7 @@ static int ov6650_remove(struct i2c_client *client) > struct ov6650 *priv = to_ov6650(client); > > v4l2_clk_put(priv->clk); > - v4l2_device_unregister_subdev(&priv->subdev); > + v4l2_async_unregister_subdev(&priv->subdev); > v4l2_ctrl_handler_free(&priv->hdl); > return 0; > } > -- > 2.19.2 > -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 0/2] media: ov6650: Add V4L2 asynchronous subdevice support 2019-03-29 20:29 ` [PATCH v2 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik 2019-03-29 20:29 ` [PATCH v2 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper Janusz Krzysztofik 2019-03-29 20:29 ` [PATCH v2 2/2] media: ov6650: Register with asynchronous subdevice framework Janusz Krzysztofik @ 2019-03-30 1:06 ` Janusz Krzysztofik 2019-03-30 1:06 ` [PATCH v3 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper Janusz Krzysztofik 2019-03-30 1:06 ` [PATCH v3 2/2] media: ov6650: Register with asynchronous subdevice framework Janusz Krzysztofik 2 siblings, 2 replies; 14+ messages in thread From: Janusz Krzysztofik @ 2019-03-30 1:06 UTC (permalink / raw) To: Sakari Ailus Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel, Janusz Krzysztofik Janusz Krzysztofik (2): media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper media: ov6650: Register with asynchronous subdevice framework ov6650.c | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) Changelog: v2->v3: - use v4l2_i2c_subdev_set_name() - thanks Sakari v1->v2: - labels should be outside conditionals - thanks Sakari - rebased on my former fix so it applied cleanly ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper 2019-03-30 1:06 ` [PATCH v3 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik @ 2019-03-30 1:06 ` Janusz Krzysztofik 2019-03-30 1:06 ` [PATCH v3 2/2] media: ov6650: Register with asynchronous subdevice framework Janusz Krzysztofik 1 sibling, 0 replies; 14+ messages in thread From: Janusz Krzysztofik @ 2019-03-30 1:06 UTC (permalink / raw) To: Sakari Ailus Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel, Janusz Krzysztofik In preparation for adding asynchronous subdevice support to the driver, don't acquire v4l2_clk from the driver .probe() callback as that may fail if the clock is provided by a bridge driver which may be not yet initialized. Move the v4l2_clk_get() to ov6650_video_probe() helper which is going to be converted to v4l2_subdev_internal_ops.registered() callback, executed only when the bridge driver is ready. Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> --- drivers/media/i2c/ov6650.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index f9359b11fa5c..de7d9790f054 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -810,9 +810,16 @@ static int ov6650_video_probe(struct i2c_client *client) u8 pidh, pidl, midh, midl; int ret; + priv->clk = v4l2_clk_get(&client->dev, NULL); + if (IS_ERR(priv->clk)) { + ret = PTR_ERR(priv->clk); + dev_err(&client->dev, "v4l2_clk request err: %d\n", ret); + return ret; + } + ret = ov6650_s_power(&priv->subdev, 1); if (ret < 0) - return ret; + goto eclkput; msleep(20); @@ -849,6 +856,11 @@ static int ov6650_video_probe(struct i2c_client *client) done: ov6650_s_power(&priv->subdev, 0); + if (!ret) + return 0; +eclkput: + v4l2_clk_put(priv->clk); + return ret; } @@ -991,18 +1003,9 @@ static int ov6650_probe(struct i2c_client *client, priv->code = MEDIA_BUS_FMT_YUYV8_2X8; priv->colorspace = V4L2_COLORSPACE_JPEG; - priv->clk = v4l2_clk_get(&client->dev, NULL); - if (IS_ERR(priv->clk)) { - ret = PTR_ERR(priv->clk); - goto eclkget; - } - ret = ov6650_video_probe(client); - if (ret) { - v4l2_clk_put(priv->clk); -eclkget: + if (ret) v4l2_ctrl_handler_free(&priv->hdl); - } return ret; } -- 2.19.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/2] media: ov6650: Register with asynchronous subdevice framework 2019-03-30 1:06 ` [PATCH v3 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik 2019-03-30 1:06 ` [PATCH v3 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper Janusz Krzysztofik @ 2019-03-30 1:06 ` Janusz Krzysztofik 2019-04-02 8:24 ` Sakari Ailus 1 sibling, 1 reply; 14+ messages in thread From: Janusz Krzysztofik @ 2019-03-30 1:06 UTC (permalink / raw) To: Sakari Ailus Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel, Janusz Krzysztofik Register V4L2 subdevice implemented by the driver to the V4L2 asynchronous subdevice framework. Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> --- drivers/media/i2c/ov6650.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index de7d9790f054..7145bb68a9e1 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -804,8 +804,9 @@ static int ov6650_prog_dflt(struct i2c_client *client) return ret; } -static int ov6650_video_probe(struct i2c_client *client) +static int ov6650_video_probe(struct v4l2_subdev *sd) { + struct i2c_client *client = v4l2_get_subdevdata(sd); struct ov6650 *priv = to_ov6650(client); u8 pidh, pidl, midh, midl; int ret; @@ -817,7 +818,7 @@ static int ov6650_video_probe(struct i2c_client *client) return ret; } - ret = ov6650_s_power(&priv->subdev, 1); + ret = ov6650_s_power(sd, 1); if (ret < 0) goto eclkput; @@ -855,7 +856,7 @@ static int ov6650_video_probe(struct i2c_client *client) ret = v4l2_ctrl_handler_setup(&priv->hdl); done: - ov6650_s_power(&priv->subdev, 0); + ov6650_s_power(sd, 0); if (!ret) return 0; eclkput: @@ -943,6 +944,10 @@ static const struct v4l2_subdev_ops ov6650_subdev_ops = { .pad = &ov6650_pad_ops, }; +static const struct v4l2_subdev_internal_ops ov6650_internal_ops = { + .registered = ov6650_video_probe, +}; + /* * i2c_driver function */ @@ -1003,7 +1008,11 @@ static int ov6650_probe(struct i2c_client *client, priv->code = MEDIA_BUS_FMT_YUYV8_2X8; priv->colorspace = V4L2_COLORSPACE_JPEG; - ret = ov6650_video_probe(client); + priv->subdev.internal_ops = &ov6650_internal_ops; + priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + v4l2_i2c_subdev_set_name(&priv->subdev, client, NULL, NULL); + + ret = v4l2_async_register_subdev(&priv->subdev); if (ret) v4l2_ctrl_handler_free(&priv->hdl); @@ -1015,7 +1024,7 @@ static int ov6650_remove(struct i2c_client *client) struct ov6650 *priv = to_ov6650(client); v4l2_clk_put(priv->clk); - v4l2_device_unregister_subdev(&priv->subdev); + v4l2_async_unregister_subdev(&priv->subdev); v4l2_ctrl_handler_free(&priv->hdl); return 0; } -- 2.19.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] media: ov6650: Register with asynchronous subdevice framework 2019-03-30 1:06 ` [PATCH v3 2/2] media: ov6650: Register with asynchronous subdevice framework Janusz Krzysztofik @ 2019-04-02 8:24 ` Sakari Ailus 0 siblings, 0 replies; 14+ messages in thread From: Sakari Ailus @ 2019-04-02 8:24 UTC (permalink / raw) To: Janusz Krzysztofik Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel Hi Janusz, On Sat, Mar 30, 2019 at 02:06:10AM +0100, Janusz Krzysztofik wrote: > Register V4L2 subdevice implemented by the driver to the V4L2 > asynchronous subdevice framework. > > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > --- > drivers/media/i2c/ov6650.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c > index de7d9790f054..7145bb68a9e1 100644 > --- a/drivers/media/i2c/ov6650.c > +++ b/drivers/media/i2c/ov6650.c > @@ -804,8 +804,9 @@ static int ov6650_prog_dflt(struct i2c_client *client) > return ret; > } > > -static int ov6650_video_probe(struct i2c_client *client) > +static int ov6650_video_probe(struct v4l2_subdev *sd) > { > + struct i2c_client *client = v4l2_get_subdevdata(sd); > struct ov6650 *priv = to_ov6650(client); > u8 pidh, pidl, midh, midl; > int ret; > @@ -817,7 +818,7 @@ static int ov6650_video_probe(struct i2c_client *client) > return ret; > } > > - ret = ov6650_s_power(&priv->subdev, 1); > + ret = ov6650_s_power(sd, 1); > if (ret < 0) > goto eclkput; > > @@ -855,7 +856,7 @@ static int ov6650_video_probe(struct i2c_client *client) > ret = v4l2_ctrl_handler_setup(&priv->hdl); > > done: > - ov6650_s_power(&priv->subdev, 0); > + ov6650_s_power(sd, 0); > if (!ret) > return 0; > eclkput: > @@ -943,6 +944,10 @@ static const struct v4l2_subdev_ops ov6650_subdev_ops = { > .pad = &ov6650_pad_ops, > }; > > +static const struct v4l2_subdev_internal_ops ov6650_internal_ops = { > + .registered = ov6650_video_probe, > +}; > + > /* > * i2c_driver function > */ > @@ -1003,7 +1008,11 @@ static int ov6650_probe(struct i2c_client *client, > priv->code = MEDIA_BUS_FMT_YUYV8_2X8; > priv->colorspace = V4L2_COLORSPACE_JPEG; > > - ret = ov6650_video_probe(client); > + priv->subdev.internal_ops = &ov6650_internal_ops; > + priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > + v4l2_i2c_subdev_set_name(&priv->subdev, client, NULL, NULL); I missed that v4l2_i2c_subdev_init() already calls v4l2_i2c_subdev_set_name(), so it can be removed from the driver. I've made the change to the applied patch: diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index 7145bb68a9e1..1b972e591b48 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -1010,7 +1010,6 @@ static int ov6650_probe(struct i2c_client *client, priv->subdev.internal_ops = &ov6650_internal_ops; priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; - v4l2_i2c_subdev_set_name(&priv->subdev, client, NULL, NULL); ret = v4l2_async_register_subdev(&priv->subdev); if (ret) > + > + ret = v4l2_async_register_subdev(&priv->subdev); > if (ret) > v4l2_ctrl_handler_free(&priv->hdl); > > @@ -1015,7 +1024,7 @@ static int ov6650_remove(struct i2c_client *client) > struct ov6650 *priv = to_ov6650(client); > > v4l2_clk_put(priv->clk); > - v4l2_device_unregister_subdev(&priv->subdev); > + v4l2_async_unregister_subdev(&priv->subdev); > v4l2_ctrl_handler_free(&priv->hdl); > return 0; > } -- Regards, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-04-02 8:24 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-25 0:34 [PATCH 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik 2019-03-25 0:35 ` [PATCH 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper Janusz Krzysztofik 2019-03-29 18:39 ` Sakari Ailus 2019-03-29 19:48 ` Janusz Krzysztofik 2019-03-29 19:51 ` Sakari Ailus 2019-03-25 0:35 ` [PATCH 2/2] media: ov6650: Register with asynchronous subdevice framework Janusz Krzysztofik 2019-03-29 20:29 ` [PATCH v2 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik 2019-03-29 20:29 ` [PATCH v2 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper Janusz Krzysztofik 2019-03-29 20:29 ` [PATCH v2 2/2] media: ov6650: Register with asynchronous subdevice framework Janusz Krzysztofik 2019-03-29 22:35 ` Sakari Ailus 2019-03-30 1:06 ` [PATCH v3 0/2] media: ov6650: Add V4L2 asynchronous subdevice support Janusz Krzysztofik 2019-03-30 1:06 ` [PATCH v3 1/2] media: ov6650: Move v4l2_clk_get() to ov6650_video_probe() helper Janusz Krzysztofik 2019-03-30 1:06 ` [PATCH v3 2/2] media: ov6650: Register with asynchronous subdevice framework Janusz Krzysztofik 2019-04-02 8:24 ` Sakari Ailus
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.