Some cleanups for ancillary drivers autoselect
cover Some cleanups for ancillary drivers autoselect
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(-)
Signature · 1 line
2.22.0
PATCH 1/2 media: Don't default-enable "ancillary driver autoselect" if EMBEDDED
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.
--- drivers/media/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/Kconfig +1 -1
@@ -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
Signature · 1 line
2.22.0
PATCH 2/2 media: Don't hide any menu if "ancillary drivers autoselect" is enabled
In reply to Ezequiel GarciaMany 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.
--- 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(-)
drivers/media/dvb-frontends/Kconfig +0 -1
@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_CORE
drivers/media/i2c/Kconfig +0 -1
@@ -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"
drivers/media/spi/Kconfig +0 -1
@@ -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"
drivers/media/tuners/Kconfig +0 -1
@@ -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
Signature · 1 line
2.22.0
Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
Em Mon, 15 Jul 2019 18:23:16 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 38 lines
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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/dvb-frontends/Kconfig +0
-1@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_COREdrivers/media/i2c/Kconfig +0
-0
drivers/media/i2c/Kconfig config VIDEO_IR_I2C · line 26
Quoted 4 lines
drivers/media/i2c/Kconfig +0
-1@@ -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.
Quoted 20 lines
drivers/media/i2c/Kconfig +0
-0comment "Audio decoders, processors and mixers"drivers/media/spi/Kconfig +0
-1@@ -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"drivers/media/tuners/Kconfig +0
-0
drivers/media/tuners/Kconfig config MEDIA_TUNER · line 21
Quoted 7 lines
drivers/media/tuners/Kconfig +0
-1@@ -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
Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
Quoted 62 lines · Mauro Carvalho Chehab
Em Mon, 15 Jul 2019 18:23:16 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 38 lines
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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/dvb-frontends/Kconfig +0
-1@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_COREdrivers/media/i2c/Kconfig +0
-0drivers/media/i2c/Kconfig config VIDEO_IR_I2C · line 26
Quoted 4 lines
drivers/media/i2c/Kconfig +0
-1@@ -23,7 +23,6 @@ config VIDEO_IR_I2C # menu "I2C Encoders, decoders, sensors and other helper chips" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERTHmm... 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_AUTOSELECTand 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:
Quoted 7 lines
menu "foo" visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERTIn 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
Quoted 35 lines
Quoted 22 lines
drivers/media/i2c/Kconfig +0
-0comment "Audio decoders, processors and mixers"drivers/media/spi/Kconfig +0
-1@@ -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"drivers/media/tuners/Kconfig +0
-0drivers/media/tuners/Kconfig config MEDIA_TUNER · line 21
Quoted 7 lines
drivers/media/tuners/Kconfig +0
-1@@ -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_SIMPLEThanks,
Mauro
Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
In reply to Mauro Carvalho ChehabQuoted 62 lines · Mauro Carvalho Chehab
Em Mon, 15 Jul 2019 18:23:16 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 38 lines
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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/dvb-frontends/Kconfig +0
-1@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_COREdrivers/media/i2c/Kconfig +0
-0drivers/media/i2c/Kconfig config VIDEO_IR_I2C · line 26
Quoted 4 lines
drivers/media/i2c/Kconfig +0
-1@@ -23,7 +23,6 @@ config VIDEO_IR_I2C # menu "I2C Encoders, decoders, sensors and other helper chips" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERTHmm... 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_AUTOSELECTand 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:
Quoted 8 lines
menu "foo" visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERTIn 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_AUTOSELECTWhat'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
Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
Quoted 88 lines · Ezequiel Garcia
Quoted 62 lines · Mauro Carvalho Chehab
Em Mon, 15 Jul 2019 18:23:16 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 38 lines
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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/dvb-frontends/Kconfig +0
-1@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_COREdrivers/media/i2c/Kconfig +0
-0drivers/media/i2c/Kconfig config VIDEO_IR_I2C · line 26
Quoted 4 lines
drivers/media/i2c/Kconfig +0
-1@@ -23,7 +23,6 @@ config VIDEO_IR_I2C # menu "I2C Encoders, decoders, sensors and other helper chips" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERTHmm... 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_AUTOSELECTand 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:Quoted 8 lines
menu "foo" visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERTIn 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_AUTOSELECTis 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_USBVISIONtristate "USB video devices based on Nogatech NT1003/1004/1005" depends on I2C && VIDEO_V4L2 select VIDEO_TUNER select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECTWhat's so confusing about having these drivers visible? Compared to the rest of the zillion menu options, what's more confusing about seeing these?
Quoted 8 lines
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
Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
Em Fri, 26 Jul 2019 01:29:58 +0800
Chen-Yu Tsai <[email protected]> escreveu:
Quoted 95 lines · Ezequiel Garcia
Quoted 94 lines · Mauro Carvalho Chehab
Quoted 62 lines
Em Mon, 15 Jul 2019 18:23:16 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 38 lines
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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/dvb-frontends/Kconfig +0
-1@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_COREdrivers/media/i2c/Kconfig +0
-0drivers/media/i2c/Kconfig config VIDEO_IR_I2C · line 26
Quoted 4 lines
drivers/media/i2c/Kconfig +0
-1@@ -23,7 +23,6 @@ config VIDEO_IR_I2C # menu "I2C Encoders, decoders, sensors and other helper chips" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERTHmm... 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_AUTOSELECTand 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:Quoted 8 lines
menu "foo" visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERTIn 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_AUTOSELECTis 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_USBVISIONtristate "USB video devices based on Nogatech NT1003/1004/1005" depends on I2C && VIDEO_V4L2 select VIDEO_TUNER select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECTWhat'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.
Quoted 6 lines
Quoted 2 lines
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
Kconfigfiles 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.
Quoted 4 lines
which does not equal hiding everything. You could still have drivers auto-selected (or not) based on a
Kconfigoption without hiding things.ChenYu
Thanks,
Mauro
Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
Quoted 104 lines · Mauro Carvalho Chehab
Em Fri, 26 Jul 2019 01:29:58 +0800
Chen-Yu Tsai <[email protected]> escreveu:Quoted 101 lines · Ezequiel Garcia
Quoted 88 lines · Mauro Carvalho Chehab
Quoted 62 lines
Em Mon, 15 Jul 2019 18:23:16 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 38 lines
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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/dvb-frontends/Kconfig +0
-1@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_COREdrivers/media/i2c/Kconfig +0
-0drivers/media/i2c/Kconfig config VIDEO_IR_I2C · line 26
Quoted 4 lines
drivers/media/i2c/Kconfig +0
-1@@ -23,7 +23,6 @@ config VIDEO_IR_I2C # menu "I2C Encoders, decoders, sensors and other helper chips" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERTHmm... 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_AUTOSELECTand 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:Quoted 8 lines
menu "foo" visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERTIn 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_AUTOSELECTis 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_USBVISIONtristate "USB video devices based on Nogatech NT1003/1004/1005" depends on I2C && VIDEO_V4L2 select VIDEO_TUNER select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECTWhat's so confusing about having these drivers visible? Compared to the rest of the zillion menu options, what's more confusing about seeing these?
Quoted 8 lines
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
Kconfigfiles to figure out which knobs to turn to get the OV5640 option out. The description says "auto-selecting",
I had this same problem.
Quoted 13 lines
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.
Quoted 3 lines
which does not equal hiding everything. You could still have drivers auto-selected (or not) based on a
Kconfigoption 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
Quoted 7 lines
Quoted 1 line
ChenYu
Thanks,
Mauro
Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
Em Thu, 25 Jul 2019 16:00:02 -0300
Helen Koike <[email protected]> escreveu:
Quoted 118 lines · Mauro Carvalho Chehab
Quoted 104 lines
Em Fri, 26 Jul 2019 01:29:58 +0800
Chen-Yu Tsai <[email protected]> escreveu:Quoted 101 lines · Ezequiel Garcia
Quoted 88 lines · Mauro Carvalho Chehab
Quoted 62 lines
Em Mon, 15 Jul 2019 18:23:16 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 38 lines
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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/dvb-frontends/Kconfig +0
-1@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_COREdrivers/media/i2c/Kconfig +0
-0drivers/media/i2c/Kconfig config VIDEO_IR_I2C · line 26
Quoted 4 lines
drivers/media/i2c/Kconfig +0
-1@@ -23,7 +23,6 @@ config VIDEO_IR_I2C # menu "I2C Encoders, decoders, sensors and other helper chips" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERTHmm... 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_AUTOSELECTand 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:Quoted 8 lines
menu "foo" visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERTIn 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_AUTOSELECTis 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_USBVISIONtristate "USB video devices based on Nogatech NT1003/1004/1005" depends on I2C && VIDEO_V4L2 select VIDEO_TUNER select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECTWhat's so confusing about having these drivers visible? Compared to the rest of the zillion menu options, what's more confusing about seeing these?
Quoted 8 lines
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
Kconfigfiles to figure out which knobs to turn to get the OV5640 option out. The description says "auto-selecting",I had this same problem.
Quoted 10 lines
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.
Quoted 7 lines
Quoted 3 lines
Quoted 3 lines
which does not equal hiding everything. You could still have drivers auto-selected (or not) based on a
Kconfigoption without hiding things.Another idea is to separate in two options,
MEDIA_SUBDRV_AUTOSELECTandMEDIA_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
Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
In reply to Mauro Carvalho ChehabQuoted 104 lines · Mauro Carvalho Chehab
Em Fri, 26 Jul 2019 01:29:58 +0800
Chen-Yu Tsai <[email protected]> escreveu:Quoted 94 lines · Ezequiel Garcia
Quoted 93 lines · Mauro Carvalho Chehab
Quoted 62 lines
Em Mon, 15 Jul 2019 18:23:16 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 38 lines
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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/dvb-frontends/Kconfig +0
-1@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_COREdrivers/media/i2c/Kconfig +0
-0drivers/media/i2c/Kconfig config VIDEO_IR_I2C · line 26
Quoted 4 lines
drivers/media/i2c/Kconfig +0
-1@@ -23,7 +23,6 @@ config VIDEO_IR_I2C # menu "I2C Encoders, decoders, sensors and other helper chips" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERTHmm... 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_AUTOSELECTand 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:Quoted 8 lines
menu "foo" visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERTIn 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_AUTOSELECTis 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_USBVISIONtristate "USB video devices based on Nogatech NT1003/1004/1005" depends on I2C && VIDEO_V4L2 select VIDEO_TUNER select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECTWhat'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.
Quoted 6 lines
Quoted 2 lines
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
Kconfigfiles to figure out which knobs to turn to get the OV5640 option out. The description says "auto-selecting",
Quoted 9 lines
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 <[email protected]>
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.
--- 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(+)
drivers/media/dvb-frontends/Kconfig +3 -0
@@ -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
drivers/media/i2c/Kconfig +3 -0
@@ -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
drivers/media/spi/Kconfig +3 -0
@@ -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
drivers/media/tuners/Kconfig +4 -0
@@ -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
Signature · 1 line
2.22.0
Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
Em Thu, 25 Jul 2019 20:55:13 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 151 lines · Mauro Carvalho Chehab
Quoted 104 lines
Em Fri, 26 Jul 2019 01:29:58 +0800
Chen-Yu Tsai <[email protected]> escreveu:Quoted 94 lines · Ezequiel Garcia
Quoted 93 lines · Mauro Carvalho Chehab
Quoted 62 lines
Em Mon, 15 Jul 2019 18:23:16 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 38 lines
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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/dvb-frontends/Kconfig +0
-1@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_COREdrivers/media/i2c/Kconfig +0
-0drivers/media/i2c/Kconfig config VIDEO_IR_I2C · line 26
Quoted 4 lines
drivers/media/i2c/Kconfig +0
-1@@ -23,7 +23,6 @@ config VIDEO_IR_I2C # menu "I2C Encoders, decoders, sensors and other helper chips" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERTHmm... 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_AUTOSELECTand 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:Quoted 8 lines
menu "foo" visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERTIn 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_AUTOSELECTis 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_USBVISIONtristate "USB video devices based on Nogatech NT1003/1004/1005" depends on I2C && VIDEO_V4L2 select VIDEO_TUNER select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECTWhat'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.
Quoted 6 lines
Quoted 2 lines
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
Kconfigfiles to figure out which knobs to turn to get the OV5640 option out. The description says "auto-selecting",Quoted 9 lines
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
fdbb96242422823a6df59cf457ebd19f83e45ffeMon Sep 17 00:00:00 2001
From: Ezequiel Garcia <[email protected]>
Date: Thu, 25 Jul 2019 20:45:07 -0300
Subject: [PATCH] media: Clarify how menus are hidden bySUBDRV_AUTOSELECTSome 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_AUTOSELECTseems 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 <[email protected]>--- 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(+)drivers/media/dvb-frontends/Kconfig +0
-0
drivers/media/dvb-frontends/Kconfig line 5
Quoted 5 lines
drivers/media/dvb-frontends/Kconfig +3
-0@@ -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.
Quoted 34 lines
drivers/media/i2c/Kconfig +3
-0@@ -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 || EXPERTdrivers/media/spi/Kconfig +3
-0@@ -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 || EXPERTdrivers/media/tuners/Kconfig +0
-0
drivers/media/tuners/Kconfig config MEDIA_TUNER · line 24
Quoted 10 lines
drivers/media/tuners/Kconfig +4
-0@@ -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
Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
Quoted 167 lines · Mauro Carvalho Chehab
Em Thu, 25 Jul 2019 20:55:13 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 151 lines · Mauro Carvalho Chehab
Quoted 104 lines
Em Fri, 26 Jul 2019 01:29:58 +0800
Chen-Yu Tsai <[email protected]> escreveu:Quoted 94 lines · Ezequiel Garcia
Quoted 93 lines · Mauro Carvalho Chehab
Quoted 62 lines
Em Mon, 15 Jul 2019 18:23:16 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 38 lines
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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/dvb-frontends/Kconfig +0
-1@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_COREdrivers/media/i2c/Kconfig +0
-0drivers/media/i2c/Kconfig config VIDEO_IR_I2C · line 26
Quoted 4 lines
drivers/media/i2c/Kconfig +0
-1@@ -23,7 +23,6 @@ config VIDEO_IR_I2C # menu "I2C Encoders, decoders, sensors and other helper chips" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERTHmm... 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_AUTOSELECTand 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:Quoted 8 lines
menu "foo" visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERTIn 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_AUTOSELECTis 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_USBVISIONtristate "USB video devices based on Nogatech NT1003/1004/1005" depends on I2C && VIDEO_V4L2 select VIDEO_TUNER select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECTWhat'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.
Quoted 6 lines
Quoted 2 lines
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
Kconfigfiles to figure out which knobs to turn to get the OV5640 option out. The description says "auto-selecting",Quoted 9 lines
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
fdbb96242422823a6df59cf457ebd19f83e45ffeMon Sep 17 00:00:00 2001
From: Ezequiel Garcia <[email protected]>
Date: Thu, 25 Jul 2019 20:45:07 -0300
Subject: [PATCH] media: Clarify how menus are hidden bySUBDRV_AUTOSELECTSome 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_AUTOSELECTseems 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 <[email protected]>--- 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(+)drivers/media/dvb-frontends/Kconfig +0
-0drivers/media/dvb-frontends/Kconfig line 5
Quoted 5 lines
drivers/media/dvb-frontends/Kconfig +3
-0@@ -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 || EXPERTMakes sense to me.
Yet, it will keep repeating the same dependency logic everywhere.
Maybe we could have something like:
config
MEDIA_SIMPLIFY_SELECT
Quoted 9 lines
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.
Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
In reply to Mauro Carvalho ChehabQuoted 167 lines · Mauro Carvalho Chehab
Em Thu, 25 Jul 2019 20:55:13 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 151 lines · Mauro Carvalho Chehab
Quoted 104 lines
Em Fri, 26 Jul 2019 01:29:58 +0800
Chen-Yu Tsai <[email protected]> escreveu:Quoted 94 lines · Ezequiel Garcia
Quoted 93 lines · Mauro Carvalho Chehab
Quoted 62 lines
Em Mon, 15 Jul 2019 18:23:16 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 38 lines
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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/dvb-frontends/Kconfig +0
-1@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_COREdrivers/media/i2c/Kconfig +0
-0drivers/media/i2c/Kconfig config VIDEO_IR_I2C · line 26
Quoted 4 lines
drivers/media/i2c/Kconfig +0
-1@@ -23,7 +23,6 @@ config VIDEO_IR_I2C # menu "I2C Encoders, decoders, sensors and other helper chips" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERTHmm... 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_AUTOSELECTand 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:Quoted 8 lines
menu "foo" visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERTIn 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_AUTOSELECTis 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_USBVISIONtristate "USB video devices based on Nogatech NT1003/1004/1005" depends on I2C && VIDEO_V4L2 select VIDEO_TUNER select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECTWhat'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.
Quoted 6 lines
Quoted 2 lines
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
Kconfigfiles to figure out which knobs to turn to get the OV5640 option out. The description says "auto-selecting",Quoted 9 lines
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
fdbb96242422823a6df59cf457ebd19f83e45ffeMon Sep 17 00:00:00 2001
From: Ezequiel Garcia <[email protected]>
Date: Thu, 25 Jul 2019 20:45:07 -0300
Subject: [PATCH] media: Clarify how menus are hidden bySUBDRV_AUTOSELECTSome 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_AUTOSELECTseems 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 <[email protected]>--- 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(+)drivers/media/dvb-frontends/Kconfig +0
-0drivers/media/dvb-frontends/Kconfig line 5
Quoted 5 lines
drivers/media/dvb-frontends/Kconfig +3
-0@@ -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 || EXPERTMakes sense to me.
Yet, it will keep repeating the same dependency logic everywhere.
Maybe we could have something like:
config
MEDIA_SIMPLIFY_SELECT
Quoted 9 lines
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 <[email protected]>
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.
--- 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(-)
drivers/media/Kconfig +5 -0
@@ -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
drivers/media/dvb-frontends/Kconfig +4 -1
@@ -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
drivers/media/i2c/Kconfig +4 -1
@@ -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"
drivers/media/spi/Kconfig +4 -1
@@ -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"
drivers/media/tuners/Kconfig +5 -1
@@ -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
Signature · 1 line
2.22.0
Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
Em Fri, 26 Jul 2019 21:17:00 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 281 lines · Mauro Carvalho Chehab
Quoted 167 lines
Em Thu, 25 Jul 2019 20:55:13 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 151 lines · Mauro Carvalho Chehab
Quoted 104 lines
Em Fri, 26 Jul 2019 01:29:58 +0800
Chen-Yu Tsai <[email protected]> escreveu:Quoted 94 lines · Ezequiel Garcia
Quoted 93 lines · Mauro Carvalho Chehab
Quoted 62 lines
Em Mon, 15 Jul 2019 18:23:16 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 38 lines
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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/dvb-frontends/Kconfig +0
-1@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_COREdrivers/media/i2c/Kconfig +0
-0drivers/media/i2c/Kconfig config VIDEO_IR_I2C · line 26
Quoted 4 lines
drivers/media/i2c/Kconfig +0
-1@@ -23,7 +23,6 @@ config VIDEO_IR_I2C # menu "I2C Encoders, decoders, sensors and other helper chips" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERTHmm... 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_AUTOSELECTand 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:Quoted 8 lines
menu "foo" visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERTIn 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_AUTOSELECTis 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_USBVISIONtristate "USB video devices based on Nogatech NT1003/1004/1005" depends on I2C && VIDEO_V4L2 select VIDEO_TUNER select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECTWhat'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.
Quoted 6 lines
Quoted 2 lines
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
Kconfigfiles to figure out which knobs to turn to get the OV5640 option out. The description says "auto-selecting",Quoted 9 lines
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
fdbb96242422823a6df59cf457ebd19f83e45ffeMon Sep 17 00:00:00 2001
From: Ezequiel Garcia <[email protected]>
Date: Thu, 25 Jul 2019 20:45:07 -0300
Subject: [PATCH] media: Clarify how menus are hidden bySUBDRV_AUTOSELECTSome 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_AUTOSELECTseems 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 <[email protected]>--- 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(+)drivers/media/dvb-frontends/Kconfig +0
-0drivers/media/dvb-frontends/Kconfig line 5
Quoted 5 lines
drivers/media/dvb-frontends/Kconfig +3
-0@@ -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 || EXPERTMakes sense to me.
Yet, it will keep repeating the same dependency logic everywhere.
Maybe we could have something like:
config
MEDIA_SIMPLIFY_SELECTQuoted 9 lines
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
e3d7207f7055bb7b69ce7fd0a5c9305c550b18aeMon Sep 17 00:00:00 2001
From: Ezequiel Garcia <[email protected]>
Date: Thu, 25 Jul 2019 20:45:07 -0300
Subject: [PATCH] media: Clarify how menus are hidden bySUBDRV_AUTOSELECTSome 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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/Kconfig +5
-0@@ -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_SUPPORTdrivers/media/dvb-frontends/Kconfig +4
-1@@ -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_COREdrivers/media/i2c/Kconfig +4
-1@@ -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"drivers/media/spi/Kconfig +4
-1@@ -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"drivers/media/tuners/Kconfig +0
-0@@ -15,8 +15,12 @@ config MEDIA_TUNER select MEDIA_TUNER_TDA9887 if MEDIA_SUBDRV_AUTOSELECT select MEDIA_TUNER_MC44S803 if MEDIA_SUBDRV_AUTOSELECT
drivers/media/tuners/Kconfig config MEDIA_TUNER · line 26
Quoted 10 lines
drivers/media/tuners/Kconfig +5
-1+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
Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
In reply to Ezequiel GarciaEm Thu, 25 Jul 2019 14:05:58 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 78 lines · Mauro Carvalho Chehab
Quoted 62 lines
Em Mon, 15 Jul 2019 18:23:16 -0300 Ezequiel Garcia <[email protected]> escreveu:
Quoted 38 lines
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_AUTOSELECTseems 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 <[email protected]>--- 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(-)drivers/media/dvb-frontends/Kconfig +0
-1@@ -1,5 +1,4 @@ menu "Customise DVB Frontends" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT comment "Multistandard (satellite) frontends" depends on DVB_COREdrivers/media/i2c/Kconfig +0
-0drivers/media/i2c/Kconfig config VIDEO_IR_I2C · line 26
Quoted 4 lines
drivers/media/i2c/Kconfig +0
-1@@ -23,7 +23,6 @@ config VIDEO_IR_I2C # menu "I2C Encoders, decoders, sensors and other helper chips" - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERTHmm... 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_AUTOSELECTand 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:Quoted 8 lines
menu "foo" visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERTIn 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_AUTOSELECTis 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:
Quoted 8 lines
config
VIDEO_USBVISIONtristate "USB video devices based on Nogatech NT1003/1004/1005" depends on I2C && VIDEO_V4L2 select VIDEO_TUNER select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECTWhat'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.
Quoted 2 lines
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.
Quoted 6 lines
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