Re: [PATCH] media: Kconfig: cleanup VIDEO_DEV dependencies

[Mauro Carvalho Chehab <mchehab@kernel.org>]

Em Sun, 13 Mar 2022 13:59:04 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> Trimming the CC list to keep a few mailing lists only.
> 
> On Sun, Mar 13, 2022 at 08:12:05AM +0100, Mauro Carvalho Chehab wrote:
> > media Kconfig has two entries associated to V4L API:
> > VIDEO_DEV and VIDEO_V4L2.
> > 
> > On Kernel 2.6.x, there were two V4L APIs, each one with its own flag.
> > VIDEO_DEV were meant to:
> > 	1) enable Video4Linux and make its Kconfig options to appear;
> > 	2) it makes the Kernel build the V4L core.
> > 
> > while VIDEO_V4L2 where used to distinguish between drivers that
> > implement the newer API and drivers that implemented the former one.
> > 
> > With time, such meaning changed, specially after the removal of
> > all V4L version 1 drivers.
> > 
> > At the current implementation, VIDEO_DEV only does (1): it enables
> > the media options related to V4L, that now has:
> > 
> > 	menu "Video4Linux options"
> > 		visible if VIDEO_DEV
> > 
> > 	source "drivers/media/v4l2-core/Kconfig"
> > 	endmenu
> > 
> > but it doesn't affect anymore the V4L core drivers.
> > 
> > The rationale is that the V4L2 core has a "soft" dependency
> > at the I2C bus, and now requires to select a number of other
> > Kconfig options:
> > 
> > 	config VIDEO_V4L2
> > 		tristate
> > 		depends on (I2C || I2C=n) && VIDEO_DEV
> > 		select RATIONAL
> > 		select VIDEOBUF2_V4L2 if VIDEOBUF2_CORE
> > 		default (I2C || I2C=n) && VIDEO_DEV
> > 
> > In the past, merging them would be tricky, but it seems that it is now
> > possible to merge those symbols, in order to simplify V4L dependencies.
> > 
> > Let's keep VIDEO_DEV, as this one is used on some make *defconfig
> > configurations.  
> 
> I would have gone for VIDEO_V4L2, but if it makes configuration changes
> easier to handle, VIDEO_DEV is fine with me too.

That was actually my first option ;-) VIDEO_V4L2 (or VIDEO_V4L) is a much
better name. From my side, I'm fine either way.

As a plus, using VIDEO_V4L2 would produce a much smaller patch that won't be
rejected by some ML servers, but maybe it would require distros and people 
who have their own .config files stored somewhere to re-configure their
Kernels.

> [snip]
> 
> > diff --git a/drivers/media/pci/tw5864/Kconfig b/drivers/media/pci/tw5864/Kconfig
> > index d376d4ed65b9..0a0f3191f238 100644
> > --- a/drivers/media/pci/tw5864/Kconfig
> > +++ b/drivers/media/pci/tw5864/Kconfig
> > @@ -1,7 +1,7 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> >  config VIDEO_TW5864
> >  	tristate "Techwell TW5864 video/audio grabber and encoder"
> > -	depends on VIDEO_DEV && PCI && VIDEO_V4L2
> > +	depends on VIDEO_DEV && PCI && VIDEO_DEV  
> 
> You can drop the second VIDEO_DEV.
> 
> >  	select VIDEOBUF2_DMA_CONTIG
> >  	help
> >  	  Support for boards based on Techwell TW5864 chip which provides
> > diff --git a/drivers/media/pci/tw68/Kconfig b/drivers/media/pci/tw68/Kconfig
> > index af0cb60337bb..ef29be7db493 100644
> > --- a/drivers/media/pci/tw68/Kconfig
> > +++ b/drivers/media/pci/tw68/Kconfig
> > @@ -1,7 +1,7 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> >  config VIDEO_TW68
> >  	tristate "Techwell tw68x Video For Linux"
> > -	depends on VIDEO_DEV && PCI && VIDEO_V4L2
> > +	depends on VIDEO_DEV && PCI && VIDEO_DEV  
> 
> Here too.
> 
> Apart from that, the patch looks good to me.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

Mauro