Hans Verkuil wrote: >> Hans Verkuil wrote: >>> On Friday 20 November 2009 22:06:02 Devin Heitmueller wrote: >>>> On Fri, Nov 20, 2009 at 3:38 PM, Hans Verkuil <hverkuil@xxxxxxxxx> >>>> wrote: >>>>> Hi Mauro, >>>>> >>>>> Please pull from http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-staging >>>>> for the following: >>>>> >>>>> - Enable staging drivers by default when building v4l-dvb >>>>> - go7007: Add struct v4l2_device. >>>>> - s2250: Change module structure >>>>> - s2250: subdev conversion >>>>> - go7007: subdev conversion >>>> I have to admit that I am not sure that enabling staging drivers by >>>> default is a good idea. Staging drivers can be highly unstable, and >>>> can potentially damage hardware. I can totally imagine less >>>> experienced users with one of these devices building the current code >>>> and then being confused why their hardware is detected but is totally >>>> broken, or worse becomes damaged. >>> If there are drivers in the staging tree that are so unreliable that >>> they >>> can break the hardware, then those should be explicitly disabled, rather >>> than disabling all drivers in the staging tree. Or perhaps do not belong >>> there at all, or belong under the CONFIG_STAGING_BROKEN option. >>> >>> A driver like the go7007 is under active development, and it does work. >>> It >>> only needs more cleanup before it can be moved to drivers/media/video, >>> so >>> there was no reason that it should be disabled. >>> >>> Mauro, what are the risks of always compiling the tm6000 and cx25821 >>> drivers? Let me know if you think that either one or both should be >>> disabled for now and I'll make a patch for it. >> Even on upstream, drivers in staging are not compiled by default. To >> enable >> them, you need to answer yes to two questions. >> >> If the driver is OK, it shouldn't be in staging. The drivers that are in >> staging have issues that may be just coding style, can be the usage of >> wrong >> API's, or can be something serious. >> >> Also, since the criteria for a driver to be in staging are weak, they may >> not >> be prepared to be used widely, since they is likely doing some evil >> things, >> like duplicating existing code or creating non-accepted API's. >> >> In the go7007 case, among other things, the driver duplicates existing >> code at >> the saa7115 driver still uses the already deprecated DECODER ioctl's, and >> creates its own API (see go7007.h). >> >> So, I'm against of enabling any staging drivers to be compiled by default. >> >> Of course, in the case of auto-compilation tests tools like what you have, >> it is >> valuable if you add a patch there that enables their compilations inside >> the >> tool, but such patch shouldn't be committed at the development tree, >> simply >> because staging drivers aren't ready yet. >> >> The developers and testers of the staging drivers should manually enable >> it, >> after being warned about the risks of using them. >> >>> By not compiling you run the very high risk of bit rot: code getting >>> seriously out-of-sync with the rest of the tree. Possibly requiring a >>> lot >>> of work later. >> If the driver is being maintained, this risk is low, since the driver >> maintainer >> will take care of it. It helps if your automatic build scripts could point >> that >> the driver compilation broke for some kernels, but, as the driver is being >> prepared >> for upstream submission, the developer should already being using the >> latest -rc kernel >> for its development. >> >> If they aren't maintained, they'll be removed from staging tree on a few >> kernel >> cycles, so patches fixing the compilation for those unmaintained drivers >> just >> means that someone lost his time fixing a dead driver. > > I've reverted that patch and respun my v4l-dvb-staging tree. Thanks. I should be pulling from all pending requests today, after finishing the pending stuff at patchwork. Cheers, Mauro. -- 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
