Re: [PATCH] ov511.c: video_register_device() return zero on success

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> Em Thu, 11 Jun 2009 08:36:00 +0200
> Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
>
>
>> Since I made that change I'm willing to look at this. Some comments
>> definitely
>> need improving at the least.
>
> Thanks! Since the behavior changed, it is important to better document it.
>
>> Also ivtv and cx18 rely on the current behavior,
>> so any changes need to be done carefully.
>>
>> But one question first: isn't the current approach not better anyway
>> than the
>> old approach? In the past device creation would fail if you specified an
>> explicit device kernel number that was already in use. Now it will
>> attempt
>> to fulfill the request and skip to the next free number otherwise. Seems
>> a
>> pretty good approach to me.
>
> Well, the idea of not failing due to that is interesting. Yet, those force
> parameters are provided to avoid that a different modprobe order would
> change
> the device nodes. By doing something different than requested by the users
> without
> even warning them about that doesn't sound nice. At least a KERN_ERR log
> should be printed if the selected nr is different than the requested one.

KERN_WARNING would be better, I think. It is not an error after all.

> In the specific case of ov511, however, it caused a regression, since the
> used
> logic to get the next value of the array were based on the failure of
> video_register_device(). As it doesn't fail anymore, the current logic is
> broken.
>
>> There haven't been any complaints about this (probably also because
>> nobody is
>> using it).
>>
>> Let me know what you want and I can implement it. It's not that hard.
>
> IMO, what should be done:
>
> 1) better comment the function;
> 2) generate a KERN_ERR at v4l2-dev, if the requested nr is not available;
> 3) replace ov511 logic to restore the old behavior (or improve it a little
> bit);
> 4) double check if similar regressions are present on other drivers. Since
> ov511 is an old driver, I don't doubt that its logic is duplicated on
> other
> devices.

I've just double checked all video_register_device calls and ov511.c is
the only one with this behavior.

Regards,

        Hans

>
> Since ov511 is used for an usb device, extra care should be taken, since
> it
> should be considered the possibility of successive hot plug/unplug. I
> wrote an
> interesting logic for this at em28xx driver, that can be used as an
> alternative
> to the current logic



-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux