video_device -> v4l2_devnode rename

video_device -> v4l2_devnode rename

[Hans Verkuil]

Hi Mauro,

I saw that 2.6.38-rc1 was released. I also noticed that not all the patches
that are in the for_2.6.38-rc1 branch are in 2.6.38-rc1.

We want to rename video_device to v4l2_devnode. So let me know when I can
finalize my patches and, most importantly, against which branch.

My current tree:

http://git.linuxtv.org/hverkuil/media_tree.git?a=shortlog;h=refs/heads/devnode2

tracks for_2.6.38-rc1 and should apply cleanly at the moment.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: video_device -> v4l2_devnode rename

[Mauro Carvalho Chehab]

Em 19-01-2011 05:39, Hans Verkuil escreveu:
> Hi Mauro,
> 
> I saw that 2.6.38-rc1 was released. I also noticed that not all the patches
> that are in the for_2.6.38-rc1 branch are in 2.6.38-rc1.

Yes. Unfortunately, when I was sending the pull request yesterday, I noticed
an issue on my linux next tree, and I had to abort its send. After that, Linus
released -rc1, before I have time to fix it.

People should really send me patches for the next window before the start of the
merge window, as doing it during the merge window makes my work harder and may
cause troubles like that. 

The net result is that most patches were submitted in time and were applied upstream.
Of course, there are usual fix patches sent during the merge window, that will be sent
upstream anyway during the rc period.

There are two patch series with new stuff submitted in time and merged on my 
tree that didn't reach upstream:
	- vb2/s5p-fimc - they required me more time to review - I also spent 3 days testing it;
	- ngene - there were a pending API discussion - I waited for a while to see if
	  there were some solution, before deciding to merge and move the problematic
	  code to staging.

So, I'll need to dig into the pending patches, in order to send the ones that
are acceptable after the end of the merge window, and letting the other patches
for .39. I'll likely try to send the two above and the dib0700 patches on a separate
pull request, but this pull request might be rejected.

> We want to rename video_device to v4l2_devnode. So let me know when I can
> finalize my patches and, most importantly, against which branch.

It is too late for that. As I said you, the better time for doing that is during
the merge window. Linus said me that he don't want to make life easier for function
rename. So, he won't be accepting such patch after the merge window.

Cheers,
Mauro

Re: video_device -> v4l2_devnode rename

[Hans Verkuil]

> Em 19-01-2011 05:39, Hans Verkuil escreveu:
>> Hi Mauro,
>>
>> I saw that 2.6.38-rc1 was released. I also noticed that not all the
>> patches
>> that are in the for_2.6.38-rc1 branch are in 2.6.38-rc1.
>
> Yes. Unfortunately, when I was sending the pull request yesterday, I
> noticed
> an issue on my linux next tree, and I had to abort its send. After that,
> Linus
> released -rc1, before I have time to fix it.
>
> People should really send me patches for the next window before the start
> of the
> merge window, as doing it during the merge window makes my work harder and
> may
> cause troubles like that.
>
> The net result is that most patches were submitted in time and were
> applied upstream.
> Of course, there are usual fix patches sent during the merge window, that
> will be sent
> upstream anyway during the rc period.

Speaking of that, my prio patches and the dsbr100 patches (with the new
v4l2_device release callback) can be moved to 2.6.39. If they can be
merged fairly early on, then I can build on those.

> There are two patch series with new stuff submitted in time and merged on
> my
> tree that didn't reach upstream:
> 	- vb2/s5p-fimc - they required me more time to review - I also spent 3
> days testing it;
> 	- ngene - there were a pending API discussion - I waited for a while to
> see if
> 	  there were some solution, before deciding to merge and move the
> problematic
> 	  code to staging.
>
> So, I'll need to dig into the pending patches, in order to send the ones
> that
> are acceptable after the end of the merge window, and letting the other
> patches
> for .39. I'll likely try to send the two above and the dib0700 patches on
> a separate
> pull request, but this pull request might be rejected.
>
>> We want to rename video_device to v4l2_devnode. So let me know when I
>> can
>> finalize my patches and, most importantly, against which branch.
>
> It is too late for that. As I said you, the better time for doing that is
> during
> the merge window. Linus said me that he don't want to make life easier for
> function
> rename. So, he won't be accepting such patch after the merge window.

You were going to tell me when you had finished merging so that I wouldn't
aim at a moving target. This is very annoying.

Regards,

        Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: video_device -> v4l2_devnode rename

[Mauro Carvalho Chehab]

Em 19-01-2011 09:53, Hans Verkuil escreveu:
>> Em 19-01-2011 05:39, Hans Verkuil escreveu:
>>> Hi Mauro,
>>>
>>> I saw that 2.6.38-rc1 was released. I also noticed that not all the
>>> patches
>>> that are in the for_2.6.38-rc1 branch are in 2.6.38-rc1.
>>
>> Yes. Unfortunately, when I was sending the pull request yesterday, I
>> noticed
>> an issue on my linux next tree, and I had to abort its send. After that,
>> Linus
>> released -rc1, before I have time to fix it.
>>
>> People should really send me patches for the next window before the start
>> of the
>> merge window, as doing it during the merge window makes my work harder and
>> may
>> cause troubles like that.
>>
>> The net result is that most patches were submitted in time and were
>> applied upstream.
>> Of course, there are usual fix patches sent during the merge window, that
>> will be sent
>> upstream anyway during the rc period.
> 
> Speaking of that, my prio patches and the dsbr100 patches (with the new
> v4l2_device release callback) can be moved to 2.6.39. If they can be
> merged fairly early on, then I can build on those.
> 
>> There are two patch series with new stuff submitted in time and merged on
>> my
>> tree that didn't reach upstream:
>> 	- vb2/s5p-fimc - they required me more time to review - I also spent 3
>> days testing it;
>> 	- ngene - there were a pending API discussion - I waited for a while to
>> see if
>> 	  there were some solution, before deciding to merge and move the
>> problematic
>> 	  code to staging.
>>
>> So, I'll need to dig into the pending patches, in order to send the ones
>> that
>> are acceptable after the end of the merge window, and letting the other
>> patches
>> for .39. I'll likely try to send the two above and the dib0700 patches on
>> a separate
>> pull request, but this pull request might be rejected.
>>
>>> We want to rename video_device to v4l2_devnode. So let me know when I
>>> can
>>> finalize my patches and, most importantly, against which branch.
>>
>> It is too late for that. As I said you, the better time for doing that is
>> during
>> the merge window. Linus said me that he don't want to make life easier for
>> function
>> rename. So, he won't be accepting such patch after the merge window.
> 
> You were going to tell me when you had finished merging so that I wouldn't
> aim at a moving target. This is very annoying.

The vb2 merge took a longer time than I expected. Sorry for that.

Cheers,
Mauro

Re: video_device -> v4l2_devnode rename

[Mauro Carvalho Chehab]

Em 19-01-2011 05:39, Hans Verkuil escreveu:
> Hi Mauro,
> 
> We want to rename video_device to v4l2_devnode. So let me know when I can
> finalize my patches and, most importantly, against which branch.
> 
> My current tree:
> 
> http://git.linuxtv.org/hverkuil/media_tree.git?a=shortlog;h=refs/heads/devnode2
> 
> tracks for_2.6.38-rc1 and should apply cleanly at the moment.

Even not being able to handle it for .38, I did a look on the proposed
changes. I'm not convinced about those renaming stuff.

By looking on other subsystems, it seems to me that video_device_register()
is a better name than any other name. Btw, by far, the use of _node for the
device registration on Linux kernel is not usual at all:

$ git grep -e "_register"  --and -e "(" --and -e "node" include |grep -v "of_mdiobus_register("
include/linux/compaction.h:extern int compaction_register_node(struct node *node);
include/linux/compaction.h:static inline int compaction_register_node(struct node *node)
include/linux/swap.h:extern int scan_unevictable_register_node(struct node *node);
include/linux/swap.h:static inline int scan_unevictable_register_node(struct node *node)

There are only 2 functions using it. On those, the "node" at the function 
register name is due to "struct node", and they likely make sense.

A seek for *register*device or *device*register patterns show a lot:

$ git grep -e "_register_device"  --and -e "("  include|wc -l
28

$ git grep -e "_device_register"  --and -e "("  include|wc -l
32

Basically, what I'm trying to say is that, on all subsystems, the function that creates
the devices is called *register*device or *device*register.

Why should we adopt anything different than the kernel convention for V4L2?

Cheers,
Mauro

Re: video_device -> v4l2_devnode rename

[Hans Verkuil]

> Em 19-01-2011 05:39, Hans Verkuil escreveu:
>> Hi Mauro,
>>
>> We want to rename video_device to v4l2_devnode. So let me know when I
>> can
>> finalize my patches and, most importantly, against which branch.
>>
>> My current tree:
>>
>> http://git.linuxtv.org/hverkuil/media_tree.git?a=shortlog;h=refs/heads/devnode2
>>
>> tracks for_2.6.38-rc1 and should apply cleanly at the moment.
>
> Even not being able to handle it for .38, I did a look on the proposed
> changes. I'm not convinced about those renaming stuff.
>
> By looking on other subsystems, it seems to me that
> video_device_register()
> is a better name than any other name. Btw, by far, the use of _node for
> the
> device registration on Linux kernel is not usual at all:
>
> $ git grep -e "_register"  --and -e "(" --and -e "node" include |grep -v
> "of_mdiobus_register("
> include/linux/compaction.h:extern int compaction_register_node(struct node
> *node);
> include/linux/compaction.h:static inline int
> compaction_register_node(struct node *node)
> include/linux/swap.h:extern int scan_unevictable_register_node(struct node
> *node);
> include/linux/swap.h:static inline int
> scan_unevictable_register_node(struct node *node)
>
> There are only 2 functions using it. On those, the "node" at the function
> register name is due to "struct node", and they likely make sense.
>
> A seek for *register*device or *device*register patterns show a lot:
>
> $ git grep -e "_register_device"  --and -e "("  include|wc -l
> 28
>
> $ git grep -e "_device_register"  --and -e "("  include|wc -l
> 32
>
> Basically, what I'm trying to say is that, on all subsystems, the function
> that creates
> the devices is called *register*device or *device*register.
>
> Why should we adopt anything different than the kernel convention for
> V4L2?

I'm sure we went through this before.

1) the name originates from the time that drivers had only one video node.
It makes little sense anymore when drivers can create many video, radio,
vbi and later v4l-subdev nodes. The key thing is that this driver
registers a V4L2 node.

2) struct v4l2_device and struct video_device look too similar. While
v4l2_device represents the whole V4L2 hardware, the video_device
represents the video/radio/vbi/... device node only.

3) (less important) all types/functions within the v4l2 framework now have
the v4l2_ prefix, except this one. Aligning this will make everything more
consistent and recognizable.

We're not like most other subsystems where often just a single device node
is registered. We have much more complex hardware. So I think that
'v4l2_devnode' much more clearly identifies what it represents than
'video_device'.

Regards,

          Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: video_device -> v4l2_devnode rename

[Mauro Carvalho Chehab]

Em 19-01-2011 09:59, Hans Verkuil escreveu:
>> Em 19-01-2011 05:39, Hans Verkuil escreveu:
>>> Hi Mauro,
>>>
>>> We want to rename video_device to v4l2_devnode. So let me know when I
>>> can
>>> finalize my patches and, most importantly, against which branch.
>>>
>>> My current tree:
>>>
>>> http://git.linuxtv.org/hverkuil/media_tree.git?a=shortlog;h=refs/heads/devnode2
>>>
>>> tracks for_2.6.38-rc1 and should apply cleanly at the moment.
>>
>> Even not being able to handle it for .38, I did a look on the proposed
>> changes. I'm not convinced about those renaming stuff.
>>
>> By looking on other subsystems, it seems to me that
>> video_device_register()
>> is a better name than any other name. Btw, by far, the use of _node for
>> the
>> device registration on Linux kernel is not usual at all:
>>
>> $ git grep -e "_register"  --and -e "(" --and -e "node" include |grep -v
>> "of_mdiobus_register("
>> include/linux/compaction.h:extern int compaction_register_node(struct node
>> *node);
>> include/linux/compaction.h:static inline int
>> compaction_register_node(struct node *node)
>> include/linux/swap.h:extern int scan_unevictable_register_node(struct node
>> *node);
>> include/linux/swap.h:static inline int
>> scan_unevictable_register_node(struct node *node)
>>
>> There are only 2 functions using it. On those, the "node" at the function
>> register name is due to "struct node", and they likely make sense.
>>
>> A seek for *register*device or *device*register patterns show a lot:
>>
>> $ git grep -e "_register_device"  --and -e "("  include|wc -l
>> 28
>>
>> $ git grep -e "_device_register"  --and -e "("  include|wc -l
>> 32
>>
>> Basically, what I'm trying to say is that, on all subsystems, the function
>> that creates
>> the devices is called *register*device or *device*register.
>>
>> Why should we adopt anything different than the kernel convention for
>> V4L2?
> 
> I'm sure we went through this before.

Maybe.

> 1) the name originates from the time that drivers had only one video node.
> It makes little sense anymore when drivers can create many video, radio,
> vbi and later v4l-subdev nodes. The key thing is that this driver
> registers a V4L2 node.

Each of those has its own struct device, so each of those are different
devices. As we've discussed previously, "subdev" is a bad name, as, on Linux,
everything that ultimately creates a /dev on userspace is a device.

> 2) struct v4l2_device and struct video_device look too similar. While
> v4l2_device represents the whole V4L2 hardware, the video_device
> represents the video/radio/vbi/... device node only.

So, maybe v4l2_device is not a good name for it, as it is a set of video devices.

> 3) (less important) all types/functions within the v4l2 framework now have
> the v4l2_ prefix, except this one. Aligning this will make everything more
> consistent and recognizable.
> 
> We're not like most other subsystems where often just a single device node
> is registered. We have much more complex hardware. So I think that
> 'v4l2_devnode' much more clearly identifies what it represents than
> 'video_device'.

There are other subsystems where drivers register several devices. For example, 
I have one 3G modem here that registers 4 devices.

Cheers,
Mauro