Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled

2019-07-26 Thread Mauro Carvalho Chehab
Em Fri, 26 Jul 2019 21:17:00 -0300
Ezequiel Garcia  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  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  escreveu:
> > > > 
> > > > > On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia 
> > > > >  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  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 
> > > > > > > > ---
> > > > > > > >  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"
> > > > > >  

Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled

2019-07-26 Thread Ezequiel Garcia
On Thu, 2019-07-25 at 23:09 -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 25 Jul 2019 20:55:13 -0300
> Ezequiel Garcia  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  escreveu:
> > >   
> > > > On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia 
> > > >  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  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 
> > > > > > > ---
> > > > > > >  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, 

Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled

2019-07-25 Thread Ezequiel Garcia
On Thu, 2019-07-25 at 23:09 -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 25 Jul 2019 20:55:13 -0300
> Ezequiel Garcia  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  escreveu:
> > >   
> > > > On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia 
> > > >  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  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 
> > > > > > > ---
> > > > > > >  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, 

Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled

2019-07-25 Thread Mauro Carvalho Chehab
Em Thu, 25 Jul 2019 20:55:13 -0300
Ezequiel Garcia  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  escreveu:
> >   
> > > On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia  
> > > 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  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 
> > > > > > ---
> > > > > >  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.
> > 

Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled

2019-07-25 Thread Ezequiel Garcia
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  escreveu:
> 
> > On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia  
> > 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  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 
> > > > > ---
> > > > >  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

Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled

2019-07-25 Thread Mauro Carvalho Chehab
Em Thu, 25 Jul 2019 16:00:02 -0300
Helen Koike  escreveu:

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

Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled

2019-07-25 Thread Helen Koike



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

Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled

2019-07-25 Thread Mauro Carvalho Chehab
Em Fri, 26 Jul 2019 01:29:58 +0800
Chen-Yu Tsai  escreveu:

> On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia  
> 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  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 
> > > > ---
> > > >  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 i

Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled

2019-07-25 Thread Mauro Carvalho Chehab
Em Thu, 25 Jul 2019 14:05:58 -0300
Ezequiel Garcia  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  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 
> > > ---
> > >  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


Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled

2019-07-25 Thread Chen-Yu Tsai
On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia  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  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 
> > > ---
> > >  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


Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled

2019-07-25 Thread Ezequiel Garcia
On Thu, 2019-07-25 at 12:57 -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 15 Jul 2019 18:23:16 -0300
> Ezequiel Garcia  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 
> > ---
> >  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




Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled

2019-07-25 Thread Hans Verkuil
On 7/25/19 5:57 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 15 Jul 2019 18:23:16 -0300
> Ezequiel Garcia  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 
>> ---
>>  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
> 



Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled

2019-07-25 Thread Mauro Carvalho Chehab
Em Mon, 15 Jul 2019 18:23:16 -0300
Ezequiel Garcia  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 
> ---
>  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