linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
	Ezequiel Garcia <ezequiel@collabora.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>,
	kernel@collabora.com, linux-media@vger.kernel.org,
	Helen Koike <helen.koike@collabora.com>
Subject: Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
Date: Thu, 25 Jul 2019 18:01:26 +0200	[thread overview]
Message-ID: <4a60eb6e-d0b7-2305-9ea0-407a6b53bd9b@xs4all.nl> (raw)
In-Reply-To: <20190725125730.2218f0a8@coco.lan>

On 7/25/19 5:57 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 15 Jul 2019 18:23:16 -0300
> Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> 
>> Many users have been complaining about not being able to find
>> certain menu options. One such example are camera sensor drivers
>> (e.g IMX219, OV5645, etc) which are common on embedded platforms
>> and not always ancillary devices.
>>
>> The problem with MEDIA_SUBDRV_AUTOSELECT seems to be related
>> to the fact that it uses the "visible" kbuild syntax to hide
>> entire group of drivers.
>>
>> This is not obvious and, as explained above, not always desired.
>>
>> To fix the problem, drop the "visible" and stop hiding any menu
>> options. Users skilled enough to configure their kernel are expected
>> to be skilled enough to know what (not) to configure anyway.
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>> ---
>>  drivers/media/dvb-frontends/Kconfig | 1 -
>>  drivers/media/i2c/Kconfig           | 1 -
>>  drivers/media/spi/Kconfig           | 1 -
>>  drivers/media/tuners/Kconfig        | 1 -
>>  4 files changed, 4 deletions(-)
>>
>> diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
>> index dc43749177df..2d1fea3bf546 100644
>> --- a/drivers/media/dvb-frontends/Kconfig
>> +++ b/drivers/media/dvb-frontends/Kconfig
>> @@ -1,5 +1,4 @@
>>  menu "Customise DVB Frontends"
>> -	visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
>>  
>>  comment "Multistandard (satellite) frontends"
>>  	depends on DVB_CORE
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 79ce9ec6fc1b..475072bb67d6 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -23,7 +23,6 @@ config VIDEO_IR_I2C
>>  #
>>  
>>  menu "I2C Encoders, decoders, sensors and other helper chips"
>> -	visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
> 
> Hmm... Hans picked this patch, but IMO it doesn't make sense
> for PC consumer people to see the hundreds of extra options
> that making those menus visible will produce.
> 
> This was added because in the past we had lots of issues with
> people desktop/laptop settings with all those things enabled.
> 
> In any case, if the desktop/laptop user is smart enough to
> go though it, he can simply disable MEDIA_SUBDRV_AUTOSELECT and
> manually select what he wants, so I really miss the point of
> making those stuff always visible.
> 
> Now, from this patch's comments, it seems that you want this
> to be visible if CONFIG_EMBEDDED. So, I won't complain if you
> replace the changes on this patch to:
> 
> 	menu "foo"
> 	    visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERT
> 
> In other words, for the normal guy that just wants to build the
> latest media stuff for his PC camera or TV device to work, he won't
> need to dig into hundreds of things that won't make any difference
> if he enables, except for making the Kernel bigger.

Good points, I agree.

Regards,

	Hans

> 
> 
>>  
>>  comment "Audio decoders, processors and mixers"
>>  
>> diff --git a/drivers/media/spi/Kconfig b/drivers/media/spi/Kconfig
>> index 08386abb9bbc..d94921fe3db5 100644
>> --- a/drivers/media/spi/Kconfig
>> +++ b/drivers/media/spi/Kconfig
>> @@ -2,7 +2,6 @@
>>  if VIDEO_V4L2
>>  
>>  menu "SPI helper chips"
>> -	visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
>>  
>>  config VIDEO_GS1662
>>  	tristate "Gennum Serializers video"
>> diff --git a/drivers/media/tuners/Kconfig b/drivers/media/tuners/Kconfig
>> index a7108e575e9b..01212df505ae 100644
>> --- a/drivers/media/tuners/Kconfig
>> +++ b/drivers/media/tuners/Kconfig
>> @@ -16,7 +16,6 @@ config MEDIA_TUNER
>>  	select MEDIA_TUNER_MC44S803 if MEDIA_SUBDRV_AUTOSELECT
>>  
>>  menu "Customize TV tuners"
>> -	visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
>>  	depends on MEDIA_ANALOG_TV_SUPPORT || MEDIA_DIGITAL_TV_SUPPORT || MEDIA_RADIO_SUPPORT || MEDIA_SDR_SUPPORT
>>  
>>  config MEDIA_TUNER_SIMPLE
> 
> 
> 
> Thanks,
> Mauro
> 


  reply	other threads:[~2019-07-25 16:01 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-15 21:23 [PATCH 0/2] Some cleanups for ancillary drivers autoselect Ezequiel Garcia
2019-07-15 21:23 ` [PATCH 1/2] media: Don't default-enable "ancillary driver autoselect" if EMBEDDED Ezequiel Garcia
2019-07-15 21:23 ` [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled Ezequiel Garcia
2019-07-25 15:57   ` Mauro Carvalho Chehab
2019-07-25 16:01     ` Hans Verkuil [this message]
2019-07-25 17:05     ` Ezequiel Garcia
2019-07-25 17:29       ` Chen-Yu Tsai
2019-07-25 18:41         ` Mauro Carvalho Chehab
2019-07-25 19:00           ` Helen Koike
2019-07-25 19:16             ` Mauro Carvalho Chehab
2019-07-25 23:55           ` Ezequiel Garcia
2019-07-26  2:09             ` Mauro Carvalho Chehab
2019-07-26  2:54               ` Ezequiel Garcia
2019-07-27  0:17               ` Ezequiel Garcia
2019-07-27  1:39                 ` Mauro Carvalho Chehab
2019-07-25 17:45       ` Mauro Carvalho Chehab

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4a60eb6e-d0b7-2305-9ea0-407a6b53bd9b@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=ezequiel@collabora.com \
    --cc=hans.verkuil@cisco.com \
    --cc=helen.koike@collabora.com \
    --cc=kernel@collabora.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab+samsung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).