On 7/25/19 3:41 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 26 Jul 2019 01:29:58 +0800
> Chen-Yu Tsai <w...@kernel.org> escreveu:
>
>> On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia <ezequ...@collabora.com>
>> 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 <ezequ...@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 <ezequ...@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.
>>>>
>>>
>>> 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
>
