Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 13 Mar 2022 20:07:55 +0100
From:   Mauro Carvalho Chehab <mchehab@kernel.org>
To:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc:     alsa-devel@alsa-project.org, linux-arm-kernel@lists.infradead.org,
        linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-media@vger.kernel.org, linux-staging@lists.linux.dev,
        linux-usb@vger.kernel.org
Subject: Re: [PATCH] media: Kconfig: cleanup VIDEO_DEV dependencies
Message-ID: <20220313200755.0873537c@coco.lan>
In-Reply-To: <Yi3ciCTbHrxYUatX@pendragon.ideasonboard.com>
References: <42ae3d28d4d822f3e14db76b99f2f4c41688ae3e.1647155467.git.mchehab@kernel.org>
        <Yi3ciCTbHrxYUatX@pendragon.ideasonboard.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

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