All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	linux-media@vger.kernel.org,
	Kyungmin Park <kyungmin.park@samsung.com>
Subject: Re: [PATCH] V4L: Drop meaningless video_is_registered() call in v4l2_open()
Date: Mon, 09 Sep 2013 12:48:34 +0200	[thread overview]
Message-ID: <522DA782.9050402@xs4all.nl> (raw)
In-Reply-To: <522DA4E7.4050600@xs4all.nl>

On 09/09/2013 12:37 PM, Hans Verkuil wrote:
> On 09/09/2013 12:24 PM, Laurent Pinchart wrote:
>> Hi Hans,
>>
>> On Monday 09 September 2013 12:17:34 Hans Verkuil wrote:
>>> On 09/09/2013 12:10 PM, Laurent Pinchart wrote:
>>>> On Monday 09 September 2013 12:07:18 Hans Verkuil wrote:
>>>>> On 09/09/2013 12:00 PM, Laurent Pinchart wrote:
>>>>>> On Monday 09 September 2013 11:07:43 Hans Verkuil wrote:
>>>>>>> On 09/06/2013 12:33 AM, Sylwester Nawrocki wrote:
>>>>>> [snip]
>>>>>>
>>>>>>>> The main issue as I see it is that we need to track both driver
>>>>>>>> remove() and struct device .release() calls and free resources only
>>>>>>>> when last of them executes. Data structures which are referenced in
>>>>>>>> fops must not be freed in remove() and we cannot use dev_get_drvdata()
>>>>>>>> in fops, e.g. not protected with device_lock().
>>>>>>>
>>>>>>> You can do all that by returning 0 if probe() was partially successful
>>>>>>> (i.e. one or more, but not all, nodes were created successfully) by
>>>>>>> doing what I described above. I don't see another way that doesn't
>>>>>>> introduce a race condition.
>>>>>>
>>>>>> But isn't this just plain wrong ? If probing fails, I don't see how
>>>>>> returning success could be a good idea.
>>>>>
>>>>> Well, the nodes that are created are working fine. So it's partially OK
>>>>> :-)
>>>>>
>>>>> That said, yes it would be better if it could safely clean up and return
>>>>> an error. But it is better than returning an error and introducing a race
>>>>> condition.
>>>>>
>>>>>>> That doesn't mean that there isn't one, it's just that I don't know of
>>>>>>> a better way of doing this.
>>>>>>
>>>>>> We might need support from the device core.
>>>>>
>>>>> I do come back to my main question: has anyone actually experienced this
>>>>> error in a realistic scenario? Other than in very low-memory situations I
>>>>> cannot imagine this happening.
>>>>
>>>> What about running out of minors, which could very well happen with subdev
>>>> nodes in complex SoCs ?
>>>
>>> Is that really realistic? What's the worst-case SoC we have in terms of
>>> device nodes? Frankly, if this might happen then we should allow for more
>>> minors or make the minor allocation completely dynamic.
>>
>> For the 4 VSP1 instances on the R-Car H2, I need 33 video nodes and 65 (if I'm 
>> not mistaken) subdev nodes. That doesn't include the camera interface.
> 
> So that leaves 158 free minors. That's a lot of webcams that can be attached :-)

Just for the record: I would have no objection whatsoever if we increase
VIDEO_NUM_DEVICES to 512.

Regards,

	Hans

  reply	other threads:[~2013-09-09 10:48 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-02 12:27 [PATCH] V4L: Drop meaningless video_is_registered() call in v4l2_open() Sylwester Nawrocki
2013-08-02 13:00 ` Hans Verkuil
2013-08-07 16:49   ` Sylwester Nawrocki
2013-08-07 17:49     ` Hans Verkuil
2013-08-07 22:16       ` Laurent Pinchart
2013-08-08 12:36       ` Andrzej Hajda
2013-09-05 22:33       ` Sylwester Nawrocki
2013-09-09  9:07         ` Hans Verkuil
2013-09-09 10:00           ` Laurent Pinchart
2013-09-09 10:07             ` Hans Verkuil
2013-09-09 10:10               ` Laurent Pinchart
2013-09-09 10:17                 ` Hans Verkuil
2013-09-09 10:24                   ` Laurent Pinchart
2013-09-09 10:37                     ` Hans Verkuil
2013-09-09 10:48                       ` Hans Verkuil [this message]
2013-09-11 13:07           ` Sylwester Nawrocki
2013-09-11 14:01             ` Hans Verkuil
2013-09-12 10:19               ` Sylwester Nawrocki

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=522DA782.9050402@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=kyungmin.park@samsung.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=s.nawrocki@samsung.com \
    --cc=sylvester.nawrocki@gmail.com \
    /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 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.