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

Em Sun, 13 Mar 2022 13:59:04 +0200
Laurent Pinchart [off-list ref] 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:

quoted

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]

quoted

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.

quoted

 	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