On 7/25/19 3:41 PM, Mauro Carvalho Chehab wrote: > Em Fri, 26 Jul 2019 01:29:58 +0800 > Chen-Yu Tsai <wens@xxxxxxxxxx> escreveu: > >> On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx> wrote: >>> >>> On Thu, 2019-07-25 at 12:57 -0300, Mauro Carvalho Chehab wrote: >>>> Em Mon, 15 Jul 2019 18:23:16 -0300 >>>> Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> >>>>> --- >>>>> 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. >>>> >>> >>> Well, I think the real value of MEDIA_SUBDRV_AUTOSELECT is the autoselection, >>> not the hidden part. I'm really missing to see what hiding anything gives you. >>> >>> In other words, this option gets useful when driver authors select ancillary >>> drivers such as: >>> >>> config VIDEO_USBVISION >>> tristate "USB video devices based on Nogatech NT1003/1004/1005" >>> depends on I2C && VIDEO_V4L2 >>> select VIDEO_TUNER >>> select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT >>> >>> What's so confusing about having these drivers visible? Compared to the >>> rest of the zillion menu options, what's more confusing about seeing these? >>> >>> Now, while I would agree with EMBEDDED, the problem with that is that >>> many "embedded" platforms don't enable EMBEDDED. So, it's not that useful. >>> >>> Finally, let me give an example of why hiding the menus is so bad. >>> Normally, to enable a symbol, we use the search tool. >>> >>> Now, when MEDIA_SUBDRV_AUTOSELECT=y, the search tool will _not_ take you >>> there and there's no indication why. >> >> As someone who has done so in the past year, I agree it's confusing. >> I had to dig through the Kconfig files to figure out which knobs to >> turn to get the OV5640 option out. The description says "auto-selecting", I had this same problem. > > Well, the text and/or the help message can be changed, if it is not > clear enough, but this option was added because we had too many issues > with users trying to build drivers for their devices without being > able to do that, because selecting thousands of devices is something > that an average PC user has troubles. > > I'm all to improve it, provided that we don't make harder for non-devs > to build the Kernel. > >> which does not equal hiding everything. You could still have drivers >> auto-selected (or not) based on a Kconfig option without hiding things. >> Another idea is to separate in two options, MEDIA_SUBDRV_AUTOSELECT and MEDIA_SUBDRV_ADVANCED, where the first one autoselects things and the later enables more options. Helen >> ChenYu > > > > Thanks, > Mauro >
