* [PATCH 0/2] Some cleanups for ancillary drivers autoselect
@ 2019-07-15 21:23 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
0 siblings, 2 replies; 16+ messages in thread
From: Ezequiel Garcia @ 2019-07-15 21:23 UTC (permalink / raw)
To: Hans Verkuil; +Cc: kernel, linux-media, Helen Koike, Ezequiel Garcia
Hi Mauro, Hans:
While developing the Rockchip ISP1 driver Helen and I noticed
that the MEDIA_SUBDRV_AUTOSELECT option was a bit confusing,
hiding some menu options, preventing us from jumping to sensor
drivers via the numbered menuconfig shortcuts.
Then, I noticed a user on IRC falling on the same trap,
and so decided the problem was common enough it was worth
a try.
Thanks!
Eze
Ezequiel Garcia (2):
media: Don't default-enable "ancillary driver autoselect" if EMBEDDED
media: Don't hide any menu if "ancillary drivers autoselect" is
enabled
drivers/media/Kconfig | 2 +-
drivers/media/dvb-frontends/Kconfig | 1 -
drivers/media/i2c/Kconfig | 1 -
drivers/media/spi/Kconfig | 1 -
drivers/media/tuners/Kconfig | 1 -
5 files changed, 1 insertion(+), 5 deletions(-)
--
2.22.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] media: Don't default-enable "ancillary driver autoselect" if EMBEDDED
2019-07-15 21:23 [PATCH 0/2] Some cleanups for ancillary drivers autoselect Ezequiel Garcia
@ 2019-07-15 21:23 ` Ezequiel Garcia
2019-07-15 21:23 ` [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled Ezequiel Garcia
1 sibling, 0 replies; 16+ messages in thread
From: Ezequiel Garcia @ 2019-07-15 21:23 UTC (permalink / raw)
To: Hans Verkuil; +Cc: kernel, linux-media, Helen Koike, Ezequiel Garcia
Admittedly, it's not easy to say what CONFIG_EMBEDDED really should be
doing, just as it's not easy to say what is "embedded".
In any case, the very description of MEDIA_SUBDRV_AUTOSELECT
specifies that embedded systems usually don't want
to enable this autoselect option.
Therefore, drop the default-yes, when the CONFIG_EMBEDDED is chosen.
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
drivers/media/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/Kconfig b/drivers/media/Kconfig
index 21cd9c02960b..8404e80aa38e 100644
--- a/drivers/media/Kconfig
+++ b/drivers/media/Kconfig
@@ -190,7 +190,7 @@ config MEDIA_SUBDRV_AUTOSELECT
depends on HAS_IOMEM
select I2C
select I2C_MUX
- default y
+ default y if !EMBEDDED
help
By default, a media driver auto-selects all possible ancillary
devices such as tuners, sensors, video encoders/decoders and
--
2.22.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
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 ` Ezequiel Garcia
2019-07-25 15:57 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 16+ messages in thread
From: Ezequiel Garcia @ 2019-07-15 21:23 UTC (permalink / raw)
To: Hans Verkuil; +Cc: kernel, linux-media, Helen Koike, Ezequiel Garcia
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
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
--
2.22.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
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
2019-07-25 17:05 ` Ezequiel Garcia
0 siblings, 2 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2019-07-25 15:57 UTC (permalink / raw)
To: Ezequiel Garcia; +Cc: Hans Verkuil, kernel, linux-media, Helen Koike
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.
>
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
2019-07-25 15:57 ` Mauro Carvalho Chehab
@ 2019-07-25 16:01 ` Hans Verkuil
2019-07-25 17:05 ` Ezequiel Garcia
1 sibling, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2019-07-25 16:01 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Ezequiel Garcia
Cc: Hans Verkuil, kernel, linux-media, Helen Koike
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
2019-07-25 15:57 ` Mauro Carvalho Chehab
2019-07-25 16:01 ` Hans Verkuil
@ 2019-07-25 17:05 ` Ezequiel Garcia
2019-07-25 17:29 ` Chen-Yu Tsai
2019-07-25 17:45 ` Mauro Carvalho Chehab
1 sibling, 2 replies; 16+ messages in thread
From: Ezequiel Garcia @ 2019-07-25 17:05 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Hans Verkuil, kernel, linux-media, Helen Koike
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@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.
>
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.
Thanks,
Ezequiel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
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 17:45 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 16+ messages in thread
From: Chen-Yu Tsai @ 2019-07-25 17:29 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Mauro Carvalho Chehab, Hans Verkuil, kernel,
Linux Media Mailing List, Helen Koike
On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia <ezequiel@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 <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.
> >
>
> 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",
which does not equal hiding everything. You could still have drivers
auto-selected (or not) based on a Kconfig option without hiding things.
ChenYu
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
2019-07-25 17:05 ` Ezequiel Garcia
2019-07-25 17:29 ` Chen-Yu Tsai
@ 2019-07-25 17:45 ` Mauro Carvalho Chehab
1 sibling, 0 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2019-07-25 17:45 UTC (permalink / raw)
To: Ezequiel Garcia; +Cc: Hans Verkuil, kernel, linux-media, Helen Koike
Em Thu, 25 Jul 2019 14:05:58 -0300
Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> 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@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.
> >
>
> 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?
There are not zillion of media menu options. We want to make as easy as
possible for people that, for instance, rebuilds the media subsystem
from:
https://git.linuxtv.org/media_build.git/
For them to be able to build the driver they need without need to
be a technical person that knows what SAA711X means, and why
it shouldn't select TVP5150 if he has a board supported by the USBVision
driver.
> 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.
Well, so the problem is elsewhere, not on media.
>
> 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.
If the search tool is not capable of properly handling visible
and explain why a symbol can't be selected, it should be fixed.
Thanks,
Mauro
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
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 23:55 ` Ezequiel Garcia
0 siblings, 2 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2019-07-25 18:41 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Ezequiel Garcia, Hans Verkuil, kernel, Linux Media Mailing List,
Helen Koike
Em Fri, 26 Jul 2019 01:29:58 +0800
Chen-Yu Tsai <wens@kernel.org> escreveu:
> On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia <ezequiel@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 <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.
> > >
> >
> > 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",
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.
>
> ChenYu
Thanks,
Mauro
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
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
1 sibling, 1 reply; 16+ messages in thread
From: Helen Koike @ 2019-07-25 19:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Chen-Yu Tsai
Cc: Ezequiel Garcia, Hans Verkuil, kernel, Linux Media Mailing List
On 7/25/19 3:41 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 26 Jul 2019 01:29:58 +0800
> Chen-Yu Tsai <wens@kernel.org> escreveu:
>
>> On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia <ezequiel@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 <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.
>>>>
>>>
>>> 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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
2019-07-25 19:00 ` Helen Koike
@ 2019-07-25 19:16 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2019-07-25 19:16 UTC (permalink / raw)
To: Helen Koike
Cc: Chen-Yu Tsai, Ezequiel Garcia, Hans Verkuil, kernel,
Linux Media Mailing List
Em Thu, 25 Jul 2019 16:00:02 -0300
Helen Koike <helen.koike@collabora.com> escreveu:
> On 7/25/19 3:41 PM, Mauro Carvalho Chehab wrote:
> > Em Fri, 26 Jul 2019 01:29:58 +0800
> > Chen-Yu Tsai <wens@kernel.org> escreveu:
> >
> >> On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia <ezequiel@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 <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.
> >>>>
> >>>
> >>> 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.
Hmm... not sure if this would work as expected.
I mean, we could have something like:
config MEDIA_SUBDRV_ADVANCED
depends on MEDIA_SUBDRV_AUTOSELECT || !COMPILE_TEST || !EXPERT
help
Make customize buttons visible again
menu "Customise DVB Frontends"
visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT || MEDIA_SUBDRV_ADVANCED
But seems overkill to me. Btw, this would be identical to:
config MEDIA_SUBDRV_ADVANCED
depends on MEDIA_SUBDRV_AUTOSELECT || !COMPILE_TEST || !EXPERT
select EXPERT
help
Make customize buttons visible again
menu "Customise DVB Frontends"
visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
So, better to just ask any embedded developer to do:
./scripts/config -e EXPERT
before running make menuconfig/xconfig/...
Thanks,
Mauro
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
2019-07-25 18:41 ` Mauro Carvalho Chehab
2019-07-25 19:00 ` Helen Koike
@ 2019-07-25 23:55 ` Ezequiel Garcia
2019-07-26 2:09 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 16+ messages in thread
From: Ezequiel Garcia @ 2019-07-25 23:55 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Chen-Yu Tsai
Cc: Hans Verkuil, kernel, Linux Media Mailing List, Helen Koike
On Thu, 2019-07-25 at 15:41 -0300, Mauro Carvalho Chehab wrote:
> Em Fri, 26 Jul 2019 01:29:58 +0800
> Chen-Yu Tsai <wens@kernel.org> escreveu:
>
> > On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia <ezequiel@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 <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.
> > > >
> > >
> > > 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",
>
> 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.
>
I just recalled Buildroot made extensive use of comments,
so how about this instead:
From fdbb96242422823a6df59cf457ebd19f83e45ffe Mon Sep 17 00:00:00 2001
From: Ezequiel Garcia <ezequiel@collabora.com>
Date: Thu, 25 Jul 2019 20:45:07 -0300
Subject: [PATCH] media: Clarify how menus are hidden by SUBDRV_AUTOSELECT
Some users have been having a hard time finding certain menu
options. One such example are camera sensor drivers
(e.g IMX219, OV5645, etc) which are common on embedded
platforms and not really "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 it normally takes some time to
figure out.
To fix the problem, add a comment on each of hidden menus,
which should clarify what option is causing menus to be hidden.
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
drivers/media/dvb-frontends/Kconfig | 3 +++
drivers/media/i2c/Kconfig | 3 +++
drivers/media/spi/Kconfig | 3 +++
drivers/media/tuners/Kconfig | 4 ++++
4 files changed, 13 insertions(+)
diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
index dc43749177df..5e2ba9d03662 100644
--- a/drivers/media/dvb-frontends/Kconfig
+++ b/drivers/media/dvb-frontends/Kconfig
@@ -1,3 +1,6 @@
+comment "DVB Frontend drivers hidden by 'Autoselect ancillary drivers'"
+ depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
+
menu "Customise DVB Frontends"
visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 79ce9ec6fc1b..a110fa53233f 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -22,6 +22,9 @@ config VIDEO_IR_I2C
# Encoder / Decoder module configuration
#
+comment "I2C drivers hidden by 'Autoselect ancillary drivers'"
+ depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
+
menu "I2C Encoders, decoders, sensors and other helper chips"
visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
diff --git a/drivers/media/spi/Kconfig b/drivers/media/spi/Kconfig
index 08386abb9bbc..da1750f86bbc 100644
--- a/drivers/media/spi/Kconfig
+++ b/drivers/media/spi/Kconfig
@@ -1,6 +1,9 @@
# SPDX-License-Identifier: GPL-2.0-only
if VIDEO_V4L2
+comment "SPI drivers hidden by 'Autoselect ancillary drivers'"
+ depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
+
menu "SPI helper chips"
visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
diff --git a/drivers/media/tuners/Kconfig b/drivers/media/tuners/Kconfig
index a7108e575e9b..e017e09d5374 100644
--- a/drivers/media/tuners/Kconfig
+++ b/drivers/media/tuners/Kconfig
@@ -15,6 +15,10 @@ config MEDIA_TUNER
select MEDIA_TUNER_TDA9887 if MEDIA_SUBDRV_AUTOSELECT
select MEDIA_TUNER_MC44S803 if MEDIA_SUBDRV_AUTOSELECT
+comment "Tuner drivers hidden by 'Autoselect ancillary drivers'"
+ depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
+ depends on MEDIA_ANALOG_TV_SUPPORT || MEDIA_DIGITAL_TV_SUPPORT || MEDIA_RADIO_SUPPORT || MEDIA_SDR_SUPPORT
+
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
--
2.22.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
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
0 siblings, 2 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2019-07-26 2:09 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Chen-Yu Tsai, Hans Verkuil, kernel, Linux Media Mailing List,
Helen Koike
Em Thu, 25 Jul 2019 20:55:13 -0300
Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> On Thu, 2019-07-25 at 15:41 -0300, Mauro Carvalho Chehab wrote:
> > Em Fri, 26 Jul 2019 01:29:58 +0800
> > Chen-Yu Tsai <wens@kernel.org> escreveu:
> >
> > > On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia <ezequiel@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 <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.
> > > > >
> > > >
> > > > 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",
> >
> > 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.
> >
>
> I just recalled Buildroot made extensive use of comments,
> so how about this instead:
>
> From fdbb96242422823a6df59cf457ebd19f83e45ffe Mon Sep 17 00:00:00 2001
> From: Ezequiel Garcia <ezequiel@collabora.com>
> Date: Thu, 25 Jul 2019 20:45:07 -0300
> Subject: [PATCH] media: Clarify how menus are hidden by SUBDRV_AUTOSELECT
>
> Some users have been having a hard time finding certain menu
> options. One such example are camera sensor drivers
> (e.g IMX219, OV5645, etc) which are common on embedded
> platforms and not really "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 it normally takes some time to
> figure out.
>
> To fix the problem, add a comment on each of hidden menus,
> which should clarify what option is causing menus to be hidden.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> drivers/media/dvb-frontends/Kconfig | 3 +++
> drivers/media/i2c/Kconfig | 3 +++
> drivers/media/spi/Kconfig | 3 +++
> drivers/media/tuners/Kconfig | 4 ++++
> 4 files changed, 13 insertions(+)
>
> diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
> index dc43749177df..5e2ba9d03662 100644
> --- a/drivers/media/dvb-frontends/Kconfig
> +++ b/drivers/media/dvb-frontends/Kconfig
> @@ -1,3 +1,6 @@
> +comment "DVB Frontend drivers hidden by 'Autoselect ancillary drivers'"
> + depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
> +
> menu "Customise DVB Frontends"
> visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
Makes sense to me.
Yet, it will keep repeating the same dependency logic everywhere.
Maybe we could have something like:
config MEDIA_SIMPLIFY_SELECT
bool
depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
default y
(yeah, the name sucks - feel free to suggest a better name for
the symbol)
and use it instead of keeping repeating the same if over and over.
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 79ce9ec6fc1b..a110fa53233f 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -22,6 +22,9 @@ config VIDEO_IR_I2C
> # Encoder / Decoder module configuration
> #
>
> +comment "I2C drivers hidden by 'Autoselect ancillary drivers'"
> + depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
> +
> menu "I2C Encoders, decoders, sensors and other helper chips"
> visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
>
> diff --git a/drivers/media/spi/Kconfig b/drivers/media/spi/Kconfig
> index 08386abb9bbc..da1750f86bbc 100644
> --- a/drivers/media/spi/Kconfig
> +++ b/drivers/media/spi/Kconfig
> @@ -1,6 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0-only
> if VIDEO_V4L2
>
> +comment "SPI drivers hidden by 'Autoselect ancillary drivers'"
> + depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
> +
> menu "SPI helper chips"
> visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
>
> diff --git a/drivers/media/tuners/Kconfig b/drivers/media/tuners/Kconfig
> index a7108e575e9b..e017e09d5374 100644
> --- a/drivers/media/tuners/Kconfig
> +++ b/drivers/media/tuners/Kconfig
> @@ -15,6 +15,10 @@ config MEDIA_TUNER
> select MEDIA_TUNER_TDA9887 if MEDIA_SUBDRV_AUTOSELECT
> select MEDIA_TUNER_MC44S803 if MEDIA_SUBDRV_AUTOSELECT
>
> +comment "Tuner drivers hidden by 'Autoselect ancillary drivers'"
> + depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
> + depends on MEDIA_ANALOG_TV_SUPPORT || MEDIA_DIGITAL_TV_SUPPORT || MEDIA_RADIO_SUPPORT || MEDIA_SDR_SUPPORT
> +
> 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
Thanks,
Mauro
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
2019-07-26 2:09 ` Mauro Carvalho Chehab
@ 2019-07-26 2:54 ` Ezequiel Garcia
2019-07-27 0:17 ` Ezequiel Garcia
1 sibling, 0 replies; 16+ messages in thread
From: Ezequiel Garcia @ 2019-07-26 2:54 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Chen-Yu Tsai, Hans Verkuil, kernel, Linux Media Mailing List,
Helen Koike
On Thu, 2019-07-25 at 23:09 -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 25 Jul 2019 20:55:13 -0300
> Ezequiel Garcia <ezequiel@collabora.com> escreveu:
>
> > On Thu, 2019-07-25 at 15:41 -0300, Mauro Carvalho Chehab wrote:
> > > Em Fri, 26 Jul 2019 01:29:58 +0800
> > > Chen-Yu Tsai <wens@kernel.org> escreveu:
> > >
> > > > On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia <ezequiel@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 <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.
> > > > > >
> > > > >
> > > > > 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",
> > >
> > > 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.
> > >
> >
> > I just recalled Buildroot made extensive use of comments,
> > so how about this instead:
> >
> > From fdbb96242422823a6df59cf457ebd19f83e45ffe Mon Sep 17 00:00:00 2001
> > From: Ezequiel Garcia <ezequiel@collabora.com>
> > Date: Thu, 25 Jul 2019 20:45:07 -0300
> > Subject: [PATCH] media: Clarify how menus are hidden by SUBDRV_AUTOSELECT
> >
> > Some users have been having a hard time finding certain menu
> > options. One such example are camera sensor drivers
> > (e.g IMX219, OV5645, etc) which are common on embedded
> > platforms and not really "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 it normally takes some time to
> > figure out.
> >
> > To fix the problem, add a comment on each of hidden menus,
> > which should clarify what option is causing menus to be hidden.
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> > drivers/media/dvb-frontends/Kconfig | 3 +++
> > drivers/media/i2c/Kconfig | 3 +++
> > drivers/media/spi/Kconfig | 3 +++
> > drivers/media/tuners/Kconfig | 4 ++++
> > 4 files changed, 13 insertions(+)
> >
> > diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
> > index dc43749177df..5e2ba9d03662 100644
> > --- a/drivers/media/dvb-frontends/Kconfig
> > +++ b/drivers/media/dvb-frontends/Kconfig
> > @@ -1,3 +1,6 @@
> > +comment "DVB Frontend drivers hidden by 'Autoselect ancillary drivers'"
> > + depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
> > +
> > menu "Customise DVB Frontends"
> > visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
>
> Makes sense to me.
>
> Yet, it will keep repeating the same dependency logic everywhere.
>
> Maybe we could have something like:
>
> config MEDIA_SIMPLIFY_SELECT
> bool
> depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
> default y
>
> (yeah, the name sucks - feel free to suggest a better name for
> the symbol)
>
> and use it instead of keeping repeating the same if over and over.
>
I'll cook something up.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
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
1 sibling, 1 reply; 16+ messages in thread
From: Ezequiel Garcia @ 2019-07-27 0:17 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Chen-Yu Tsai, Hans Verkuil, kernel, Linux Media Mailing List,
Helen Koike
On Thu, 2019-07-25 at 23:09 -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 25 Jul 2019 20:55:13 -0300
> Ezequiel Garcia <ezequiel@collabora.com> escreveu:
>
> > On Thu, 2019-07-25 at 15:41 -0300, Mauro Carvalho Chehab wrote:
> > > Em Fri, 26 Jul 2019 01:29:58 +0800
> > > Chen-Yu Tsai <wens@kernel.org> escreveu:
> > >
> > > > On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia <ezequiel@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 <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.
> > > > > >
> > > > >
> > > > > 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",
> > >
> > > 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.
> > >
> >
> > I just recalled Buildroot made extensive use of comments,
> > so how about this instead:
> >
> > From fdbb96242422823a6df59cf457ebd19f83e45ffe Mon Sep 17 00:00:00 2001
> > From: Ezequiel Garcia <ezequiel@collabora.com>
> > Date: Thu, 25 Jul 2019 20:45:07 -0300
> > Subject: [PATCH] media: Clarify how menus are hidden by SUBDRV_AUTOSELECT
> >
> > Some users have been having a hard time finding certain menu
> > options. One such example are camera sensor drivers
> > (e.g IMX219, OV5645, etc) which are common on embedded
> > platforms and not really "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 it normally takes some time to
> > figure out.
> >
> > To fix the problem, add a comment on each of hidden menus,
> > which should clarify what option is causing menus to be hidden.
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> > drivers/media/dvb-frontends/Kconfig | 3 +++
> > drivers/media/i2c/Kconfig | 3 +++
> > drivers/media/spi/Kconfig | 3 +++
> > drivers/media/tuners/Kconfig | 4 ++++
> > 4 files changed, 13 insertions(+)
> >
> > diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
> > index dc43749177df..5e2ba9d03662 100644
> > --- a/drivers/media/dvb-frontends/Kconfig
> > +++ b/drivers/media/dvb-frontends/Kconfig
> > @@ -1,3 +1,6 @@
> > +comment "DVB Frontend drivers hidden by 'Autoselect ancillary drivers'"
> > + depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
> > +
> > menu "Customise DVB Frontends"
> > visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
>
> Makes sense to me.
>
> Yet, it will keep repeating the same dependency logic everywhere.
>
> Maybe we could have something like:
>
> config MEDIA_SIMPLIFY_SELECT
> bool
> depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
> default y
>
> (yeah, the name sucks - feel free to suggest a better name for
> the symbol)
>
> and use it instead of keeping repeating the same if over and over.
>
I've added an extra config, and used it also for the 'visible' syntax.
From e3d7207f7055bb7b69ce7fd0a5c9305c550b18ae Mon Sep 17 00:00:00 2001
From: Ezequiel Garcia <ezequiel@collabora.com>
Date: Thu, 25 Jul 2019 20:45:07 -0300
Subject: [PATCH] media: Clarify how menus are hidden by SUBDRV_AUTOSELECT
Some users have been having a hard time finding certain menu
options. One such example are camera sensor drivers
(e.g IMX219, OV5645, etc) which are common on embedded
platforms and not really "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 it normally takes some time to
figure out.
To fix the problem, add a comment on each of hidden menus,
which should clarify what option is causing menus to be hidden.
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
drivers/media/Kconfig | 5 +++++
drivers/media/dvb-frontends/Kconfig | 5 ++++-
drivers/media/i2c/Kconfig | 5 ++++-
drivers/media/spi/Kconfig | 5 ++++-
drivers/media/tuners/Kconfig | 6 +++++-
5 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/media/Kconfig b/drivers/media/Kconfig
index 8404e80aa38e..b36a41332867 100644
--- a/drivers/media/Kconfig
+++ b/drivers/media/Kconfig
@@ -207,6 +207,11 @@ config MEDIA_SUBDRV_AUTOSELECT
If unsure say Y.
+config MEDIA_HIDE_ANCILLARY_SUBDRV
+ bool
+ depends on MEDIA_SUBDRV_AUTOSELECT && !COMPILE_TEST && !EXPERT
+ default y
+
config MEDIA_ATTACH
bool
depends on MEDIA_ANALOG_TV_SUPPORT || MEDIA_DIGITAL_TV_SUPPORT || MEDIA_RADIO_SUPPORT
diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
index dc43749177df..a29e9ddf9c82 100644
--- a/drivers/media/dvb-frontends/Kconfig
+++ b/drivers/media/dvb-frontends/Kconfig
@@ -1,5 +1,8 @@
+comment "DVB Frontend drivers hidden by 'Autoselect ancillary drivers'"
+ depends on MEDIA_HIDE_ANCILLARY_SUBDRV
+
menu "Customise DVB Frontends"
- visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
+ visible if !MEDIA_HIDE_ANCILLARY_SUBDRV
comment "Multistandard (satellite) frontends"
depends on DVB_CORE
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 79ce9ec6fc1b..1f72eafa7d1a 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -22,8 +22,11 @@ config VIDEO_IR_I2C
# Encoder / Decoder module configuration
#
+comment "I2C drivers hidden by 'Autoselect ancillary drivers'"
+ depends on MEDIA_HIDE_ANCILLARY_SUBDRV
+
menu "I2C Encoders, decoders, sensors and other helper chips"
- visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
+ visible if !MEDIA_HIDE_ANCILLARY_SUBDRV
comment "Audio decoders, processors and mixers"
diff --git a/drivers/media/spi/Kconfig b/drivers/media/spi/Kconfig
index 08386abb9bbc..bcc49cb47de6 100644
--- a/drivers/media/spi/Kconfig
+++ b/drivers/media/spi/Kconfig
@@ -1,8 +1,11 @@
# SPDX-License-Identifier: GPL-2.0-only
if VIDEO_V4L2
+comment "SPI drivers hidden by 'Autoselect ancillary drivers'"
+ depends on MEDIA_HIDE_ANCILLARY_SUBDRV
+
menu "SPI helper chips"
- visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
+ visible if !MEDIA_HIDE_ANCILLARY_SUBDRV
config VIDEO_GS1662
tristate "Gennum Serializers video"
diff --git a/drivers/media/tuners/Kconfig b/drivers/media/tuners/Kconfig
index a7108e575e9b..e104bb7766e1 100644
--- a/drivers/media/tuners/Kconfig
+++ b/drivers/media/tuners/Kconfig
@@ -15,8 +15,12 @@ config MEDIA_TUNER
select MEDIA_TUNER_TDA9887 if MEDIA_SUBDRV_AUTOSELECT
select MEDIA_TUNER_MC44S803 if MEDIA_SUBDRV_AUTOSELECT
+comment "Tuner drivers hidden by 'Autoselect ancillary drivers'"
+ depends on MEDIA_HIDE_ANCILLARY_SUBDRV
+ depends on MEDIA_ANALOG_TV_SUPPORT || MEDIA_DIGITAL_TV_SUPPORT || MEDIA_RADIO_SUPPORT || MEDIA_SDR_SUPPORT
+
menu "Customize TV tuners"
- visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
+ visible if !MEDIA_HIDE_ANCILLARY_SUBDRV
depends on MEDIA_ANALOG_TV_SUPPORT || MEDIA_DIGITAL_TV_SUPPORT || MEDIA_RADIO_SUPPORT || MEDIA_SDR_SUPPORT
config MEDIA_TUNER_SIMPLE
--
2.22.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
2019-07-27 0:17 ` Ezequiel Garcia
@ 2019-07-27 1:39 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2019-07-27 1:39 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Chen-Yu Tsai, Hans Verkuil, kernel, Linux Media Mailing List,
Helen Koike
Em Fri, 26 Jul 2019 21:17:00 -0300
Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> On Thu, 2019-07-25 at 23:09 -0300, Mauro Carvalho Chehab wrote:
> > Em Thu, 25 Jul 2019 20:55:13 -0300
> > Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> >
> > > On Thu, 2019-07-25 at 15:41 -0300, Mauro Carvalho Chehab wrote:
> > > > Em Fri, 26 Jul 2019 01:29:58 +0800
> > > > Chen-Yu Tsai <wens@kernel.org> escreveu:
> > > >
> > > > > On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia <ezequiel@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 <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.
> > > > > > >
> > > > > >
> > > > > > 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",
> > > >
> > > > 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.
> > > >
> > >
> > > I just recalled Buildroot made extensive use of comments,
> > > so how about this instead:
> > >
> > > From fdbb96242422823a6df59cf457ebd19f83e45ffe Mon Sep 17 00:00:00 2001
> > > From: Ezequiel Garcia <ezequiel@collabora.com>
> > > Date: Thu, 25 Jul 2019 20:45:07 -0300
> > > Subject: [PATCH] media: Clarify how menus are hidden by SUBDRV_AUTOSELECT
> > >
> > > Some users have been having a hard time finding certain menu
> > > options. One such example are camera sensor drivers
> > > (e.g IMX219, OV5645, etc) which are common on embedded
> > > platforms and not really "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 it normally takes some time to
> > > figure out.
> > >
> > > To fix the problem, add a comment on each of hidden menus,
> > > which should clarify what option is causing menus to be hidden.
> > >
> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > ---
> > > drivers/media/dvb-frontends/Kconfig | 3 +++
> > > drivers/media/i2c/Kconfig | 3 +++
> > > drivers/media/spi/Kconfig | 3 +++
> > > drivers/media/tuners/Kconfig | 4 ++++
> > > 4 files changed, 13 insertions(+)
> > >
> > > diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
> > > index dc43749177df..5e2ba9d03662 100644
> > > --- a/drivers/media/dvb-frontends/Kconfig
> > > +++ b/drivers/media/dvb-frontends/Kconfig
> > > @@ -1,3 +1,6 @@
> > > +comment "DVB Frontend drivers hidden by 'Autoselect ancillary drivers'"
> > > + depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
> > > +
> > > menu "Customise DVB Frontends"
> > > visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
> >
> > Makes sense to me.
> >
> > Yet, it will keep repeating the same dependency logic everywhere.
> >
> > Maybe we could have something like:
> >
> > config MEDIA_SIMPLIFY_SELECT
> > bool
> > depends on !(!MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT)
> > default y
> >
> > (yeah, the name sucks - feel free to suggest a better name for
> > the symbol)
> >
> > and use it instead of keeping repeating the same if over and over.
> >
>
> I've added an extra config, and used it also for the 'visible' syntax.
>
> From e3d7207f7055bb7b69ce7fd0a5c9305c550b18ae Mon Sep 17 00:00:00 2001
> From: Ezequiel Garcia <ezequiel@collabora.com>
> Date: Thu, 25 Jul 2019 20:45:07 -0300
> Subject: [PATCH] media: Clarify how menus are hidden by SUBDRV_AUTOSELECT
>
> Some users have been having a hard time finding certain menu
> options. One such example are camera sensor drivers
> (e.g IMX219, OV5645, etc) which are common on embedded
> platforms and not really "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 it normally takes some time to
> figure out.
>
> To fix the problem, add a comment on each of hidden menus,
> which should clarify what option is causing menus to be hidden.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> drivers/media/Kconfig | 5 +++++
> drivers/media/dvb-frontends/Kconfig | 5 ++++-
> drivers/media/i2c/Kconfig | 5 ++++-
> drivers/media/spi/Kconfig | 5 ++++-
> drivers/media/tuners/Kconfig | 6 +++++-
> 5 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/Kconfig b/drivers/media/Kconfig
> index 8404e80aa38e..b36a41332867 100644
> --- a/drivers/media/Kconfig
> +++ b/drivers/media/Kconfig
> @@ -207,6 +207,11 @@ config MEDIA_SUBDRV_AUTOSELECT
>
> If unsure say Y.
>
> +config MEDIA_HIDE_ANCILLARY_SUBDRV
> + bool
> + depends on MEDIA_SUBDRV_AUTOSELECT && !COMPILE_TEST && !EXPERT
> + default y
> +
> config MEDIA_ATTACH
> bool
> depends on MEDIA_ANALOG_TV_SUPPORT || MEDIA_DIGITAL_TV_SUPPORT || MEDIA_RADIO_SUPPORT
> diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
> index dc43749177df..a29e9ddf9c82 100644
> --- a/drivers/media/dvb-frontends/Kconfig
> +++ b/drivers/media/dvb-frontends/Kconfig
> @@ -1,5 +1,8 @@
> +comment "DVB Frontend drivers hidden by 'Autoselect ancillary drivers'"
> + depends on MEDIA_HIDE_ANCILLARY_SUBDRV
> +
> menu "Customise DVB Frontends"
> - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
> + visible if !MEDIA_HIDE_ANCILLARY_SUBDRV
>
> comment "Multistandard (satellite) frontends"
> depends on DVB_CORE
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 79ce9ec6fc1b..1f72eafa7d1a 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -22,8 +22,11 @@ config VIDEO_IR_I2C
> # Encoder / Decoder module configuration
> #
>
> +comment "I2C drivers hidden by 'Autoselect ancillary drivers'"
> + depends on MEDIA_HIDE_ANCILLARY_SUBDRV
> +
> menu "I2C Encoders, decoders, sensors and other helper chips"
> - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
> + visible if !MEDIA_HIDE_ANCILLARY_SUBDRV
>
> comment "Audio decoders, processors and mixers"
>
> diff --git a/drivers/media/spi/Kconfig b/drivers/media/spi/Kconfig
> index 08386abb9bbc..bcc49cb47de6 100644
> --- a/drivers/media/spi/Kconfig
> +++ b/drivers/media/spi/Kconfig
> @@ -1,8 +1,11 @@
> # SPDX-License-Identifier: GPL-2.0-only
> if VIDEO_V4L2
>
> +comment "SPI drivers hidden by 'Autoselect ancillary drivers'"
> + depends on MEDIA_HIDE_ANCILLARY_SUBDRV
> +
> menu "SPI helper chips"
> - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
> + visible if !MEDIA_HIDE_ANCILLARY_SUBDRV
>
> config VIDEO_GS1662
> tristate "Gennum Serializers video"
> diff --git a/drivers/media/tuners/Kconfig b/drivers/media/tuners/Kconfig
> index a7108e575e9b..e104bb7766e1 100644
> --- a/drivers/media/tuners/Kconfig
> +++ b/drivers/media/tuners/Kconfig
> @@ -15,8 +15,12 @@ config MEDIA_TUNER
> select MEDIA_TUNER_TDA9887 if MEDIA_SUBDRV_AUTOSELECT
> select MEDIA_TUNER_MC44S803 if MEDIA_SUBDRV_AUTOSELECT
>
> +comment "Tuner drivers hidden by 'Autoselect ancillary drivers'"
> + depends on MEDIA_HIDE_ANCILLARY_SUBDRV
> + depends on MEDIA_ANALOG_TV_SUPPORT || MEDIA_DIGITAL_TV_SUPPORT || MEDIA_RADIO_SUPPORT || MEDIA_SDR_SUPPORT
> +
> menu "Customize TV tuners"
> - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
> + visible if !MEDIA_HIDE_ANCILLARY_SUBDRV
> depends on MEDIA_ANALOG_TV_SUPPORT || MEDIA_DIGITAL_TV_SUPPORT || MEDIA_RADIO_SUPPORT || MEDIA_SDR_SUPPORT
>
> config MEDIA_TUNER_SIMPLE
Good enough for me.
Regards,
Mauro
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-07-27 1:39 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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
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).