lists.zerezo.com
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [v4l-dvb-maintainer] [PATCH] [PATCH] v4l: Introduce "index" attribute for?persistent video4linux device nodes

On Thu, 17 Jul 2008, Hans Verkuil wrote:


On Thursday 17 July 2008 18:57:49 Mauro Carvalho Chehab wrote:

On Thu, 17 Jul 2008, Hans Verkuil wrote:

On Thursday 17 July 2008 18:44:23 Hans Verkuil wrote:

On Thursday 17 July 2008 18:40:47 Mauro Carvalho Chehab wrote:

On Thu, 17 Jul 2008, Hans Verkuil wrote:

On Wednesday 25 June 2008 00:59:51 Brandon Philips wrote:

On 00:34 Tue 24 Jun 2008, Trent Piepho wrote:

On Mon, 23 Jun 2008, Brandon Philips wrote:

+	for (i = 0; i < 32; i++) {
+		if (used & (1 << i))
+			continue;
+		return i;
+	}


	i = ffz(used);
	return i >= 32 ? -ENFILE : i;


Err. Right :D  Tested and pushed.

Mauro-

Updated http://ifup.org/hg/v4l-dvb to have Trent's improvement.

Cheers,

	Brandon


Hi Mauro,

I think you missed this pull request from Brandon. Can you merge
this?


Yes, I missed that one.

Yet, I didn't like the usage of "32" magic numbers on those
parts:

-       if (num >= VIDEO_NUM_DEVICES)
+
+       if (num >= 32) {
+               printk(KERN_ERR "videodev: %s num is too
large\n", __func__);

+       return i >= 32 ? -ENFILE : i;


It seems better to use VIDEO_NUM_DEVICES as the maximum limit on
both usages of "32".

Brandon,

Could you fix and re-send me a pull request?


Mauro, Brandon,

If you do not mind, then I'll do this. I'm working on videodev.c
anyway (making it compatible with kernels <2.6.19) so it's easy
for me to do merge this and make the necessary adjustment. And I
can test it with a 2.6.18 kernel at the same time.


For me, it is OK if you want to touch on it.


Correction, the 32 refers to the number of bits in an u32, not to
VIDEO_NUM_DEVICES. So I think you can just merge this patch as is.
It does not conflict with my videodev.c changes (amazingly), so it
is no problem if you merge this change.


Hmm... If I understood the patch, if you change VIDEO_NUM_DEVICES to
a higher number, you'll still be limited on 32 max devices, right?


No. The value of 32 refers to the maximum number of devices
(/dev/vbiX, /dev/videoX, etc) created for ONE driver instance. E.g. my
first ivtv card can create up to 32 devices, my second bttv card can
create up to 32 devices, etc. etc.


So, IMO, the better would be to do something like this:
	sizeof(u32) * 8

Instead of a magic number. I would also add some comments about this.



The maximum currently in use is probably ivtv with 9 devices for a
PVR-350 card. So we are safe with 32.

VIDEO_NUM_DEVICES refers to the TOTAL number of video4linux devices
created. I have some future plans to enlarge that and possibly also
getting rid of how the minor numbers are allocated. Currently this is
very unfair with a range of 64 for radio devices and an equal number of
video devices, although each ivtv card creates 3 video devices against
one radio device (and 5 for a PVR-350 card!). Much better to just pick
the first free minor number.

Regards,

	Hans


If I'm right, then, IMO,  we should use VIDEO_NUM_DEVICES and put a
notice that increasing this number will require changing some static
var from u32 to u64.





--
Cheers,
Mauro Carvalho Chehab
http://linuxtv.org
mchehab@xxxxxxxxxxxxx

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@xxxxxxxxxx?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list