Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 14 Apr 2020 15:21:47 +0200
From:   Mauro Carvalho Chehab <mchehab@kernel.org>
To:     Ezequiel Garcia <ezequiel@collabora.com>
Cc:     linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
        kernel@collabora.com, Hans Verkuil <hverkuil@xs4all.nl>,
        Helen Koike <helen.koike@collabora.com>
Subject: Re: [PATCH] media: Kconfig: Don't expose API options
Message-ID: <20200414152147.053b4b29@coco.lan>
In-Reply-To: <20200414130210.18970-1-ezequiel@collabora.com>
References: <20200414130210.18970-1-ezequiel@collabora.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Em Tue, 14 Apr 2020 10:02:10 -0300
Ezequiel Garcia <ezequiel@collabora.com> escreveu:

> There is no need to expose API options; instead,
> drivers that support a given API are expected to just select it.
> 
> Happily, this is currently the case, so simply stop
> exposing the options.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/media/Kconfig           | 2 +-
>  drivers/media/mc/Kconfig        | 3 +--
>  drivers/media/v4l2-core/Kconfig | 2 +-
>  3 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/Kconfig b/drivers/media/Kconfig
> index a8def1591352..eef4c6761ffb 100644
> --- a/drivers/media/Kconfig
> +++ b/drivers/media/Kconfig
> @@ -173,7 +173,7 @@ config VIDEO_DEV
>  	  radio devices and by some input devices.
>  
>  config MEDIA_CONTROLLER
> -	bool "Media Controller API"
> +	bool

This is not true here... non-embedded drivers like DVB and UVC can work
with or without the media controller. For them, the API is experimental.

>  	default MEDIA_CAMERA_SUPPORT || MEDIA_ANALOG_TV_SUPPORT || MEDIA_DIGITAL_TV_SUPPORT || MEDIA_PLATFORM_SUPPORT
>  	help
>  	  Enable the media controller API used to query media devices internal
> diff --git a/drivers/media/mc/Kconfig b/drivers/media/mc/Kconfig
> index 002a918c4c75..0cdf885ce24d 100644
> --- a/drivers/media/mc/Kconfig
> +++ b/drivers/media/mc/Kconfig
> @@ -2,7 +2,6 @@
>  
>  #
>  # Media controller
> -#	Selectable only for webcam/grabbers, as other drivers don't use it
>  #
>  
>  config MEDIA_CONTROLLER_DVB
> @@ -14,7 +13,7 @@ config MEDIA_CONTROLLER_DVB
>  	  This is currently experimental.
>  
>  config MEDIA_CONTROLLER_REQUEST_API
> -	bool "Enable Media controller Request API (EXPERIMENTAL)"
> +	bool

This sounds OK.

The only reason why I didn't make this change is because of the 
"EXPERIMENTAL" warning. 

At least while this API is considered experimental, it sounds
wise to have a warning.

Perhaps we could apply this hunk, and add something like 
this to media/Kconfig:

comment "Please notice that the enabled Media controller Request API is EXPERIMENTAL"
	depends on MEDIA_CONTROLLER_REQUEST_API

Regards,
Mauro

>  	depends on MEDIA_CONTROLLER && STAGING_MEDIA
>  	help
>  	  DO NOT ENABLE THIS OPTION UNLESS YOU KNOW WHAT YOU'RE DOING.
> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
> index 3fa75352d04c..d034185856bb 100644
> --- a/drivers/media/v4l2-core/Kconfig
> +++ b/drivers/media/v4l2-core/Kconfig
> @@ -17,7 +17,7 @@ config VIDEO_V4L2_I2C
>  	default y
>  
>  config VIDEO_V4L2_SUBDEV_API
> -	bool "V4L2 sub-device userspace API"
> +	bool
>  	depends on VIDEO_DEV && MEDIA_CONTROLLER
>  	help
>  	  Enables the V4L2 sub-device pad-level userspace API used to configure


Thanks,
Mauro