From: xinwu <xinwu.liu@intel.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: mchehab@kernel.org, hans.verkuil@cisco.com,
niklas.soderlund+renesas@ragnatech.se, ezequiel@collabora.com,
sque@chromium.org, linux-media@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] media: v4l2-core: expose the device after it was registered.
Date: Thu, 24 Jan 2019 15:11:53 +0800 [thread overview]
Message-ID: <85906cb2-5472-2c54-e854-136cba8e8d40@intel.com> (raw)
In-Reply-To: <20190122100311.isx57iktfpxawxjv@paasikivi.fi.intel.com>
Hi Sakari,
Thanks for your response.
On 2019年01月22日 18:03, Sakari Ailus wrote:
> Hi Xinwu,
>
> On Tue, Jan 22, 2019 at 04:34:44PM +0800, Liu, Xinwu wrote:
>> device_register exposes the device to userspace.
>>
>> Therefore, while the register process is ongoing, the userspace program
>> will fail to open the device (ENODEV will be set to errno currently).
>> The program in userspace must re-open the device to cover this case.
>>
>> It is more reasonable to expose the device after everythings ready.
>>
>> Signed-off-by: Liu, Xinwu <xinwu.liu@intel.com>
>> ---
>> drivers/media/v4l2-core/v4l2-dev.c | 14 ++++++++------
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
>> index d7528f8..01e5cc9 100644
>> --- a/drivers/media/v4l2-core/v4l2-dev.c
>> +++ b/drivers/media/v4l2-core/v4l2-dev.c
>> @@ -993,16 +993,11 @@ int __video_register_device(struct video_device *vdev,
>> goto cleanup;
>> }
>>
>> - /* Part 4: register the device with sysfs */
>> + /* Part 4: Prepare to register the device */
>> vdev->dev.class = &video_class;
>> vdev->dev.devt = MKDEV(VIDEO_MAJOR, vdev->minor);
>> vdev->dev.parent = vdev->dev_parent;
>> dev_set_name(&vdev->dev, "%s%d", name_base, vdev->num);
>> - ret = device_register(&vdev->dev);
>> - if (ret < 0) {
>> - pr_err("%s: device_register failed\n", __func__);
>> - goto cleanup;
>> - }
>> /* Register the release callback that will be called when the last
>> reference to the device goes away. */
>> vdev->dev.release = v4l2_device_release;
>> @@ -1020,6 +1015,13 @@ int __video_register_device(struct video_device *vdev,
>> /* Part 6: Activate this minor. The char device can now be used. */
>> set_bit(V4L2_FL_REGISTERED, &vdev->flags);
>>
>> + /* Part 7: Register the device with sysfs */
>> + ret = device_register(&vdev->dev);
>> + if (ret < 0) {
>> + pr_err("%s: device_register failed\n", __func__);
>> + goto cleanup;
> The error handling needs to reflect the change as well.
Yes, I think it need to clear the V4L2_FL_REGISTERED also.
>
> Speaking of which, the error handling appears to be somewhat broken here.
> It should be fixed first before making changes to the registration order.
I guess you mean to call "put_device()" in this error handling.
>
> The problem the patch addresses is related to another problem: how to tell
> the user space all devices in the media hardware complex have been
> registered successfully. Order generally has been the node first, and only
> then the entity. That suggests the order should probably be:
>
> 1. video_register_media_controller
> 2. set_bit(V4L2_FL_REGISTERED)
> 3. device_register
>
> I wonder what Hans thinks.
Yes, that's my suggestion, thanks for the highlight. I also want to know
if it's worth to make this change.
>
>> + }
>> +
>> return 0;
>>
>> cleanup:
next prev parent reply other threads:[~2019-01-24 6:59 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-22 8:34 [PATCH] media: v4l2-core: expose the device after it was registered Liu, Xinwu
2019-01-22 10:03 ` Sakari Ailus
2019-01-24 7:11 ` xinwu [this message]
2019-01-24 7:48 ` Sakari Ailus
2019-05-10 10:27 ` Hans Verkuil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=85906cb2-5472-2c54-e854-136cba8e8d40@intel.com \
--to=xinwu.liu@intel.com \
--cc=ezequiel@collabora.com \
--cc=hans.verkuil@cisco.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=niklas.soderlund+renesas@ragnatech.se \
--cc=sakari.ailus@linux.intel.com \
--cc=sque@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).