From: Chen-Yu Tsai <wens@kernel.org>
To: Ezequiel Garcia <ezequiel@collabora.com>
Cc: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
Hans Verkuil <hans.verkuil@cisco.com>,
kernel@collabora.com,
Linux Media Mailing List <linux-media@vger.kernel.org>,
Helen Koike <helen.koike@collabora.com>
Subject: Re: [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled
Date: Fri, 26 Jul 2019 01:29:58 +0800 [thread overview]
Message-ID: <CAGb2v65wOz+nUi=Leb3FudU7K5S_AHtuCarXHcO0kMvvqEw8rQ@mail.gmail.com> (raw)
In-Reply-To: <f87fb2e6bd740de8c44df1f8ff3b48b7b04af481.camel@collabora.com>
On Fri, Jul 26, 2019 at 1:06 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> On Thu, 2019-07-25 at 12:57 -0300, Mauro Carvalho Chehab wrote:
> > Em Mon, 15 Jul 2019 18:23:16 -0300
> > Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> >
> > > Many users have been complaining about not being able to find
> > > certain menu options. One such example are camera sensor drivers
> > > (e.g IMX219, OV5645, etc) which are common on embedded platforms
> > > and not always ancillary devices.
> > >
> > > The problem with MEDIA_SUBDRV_AUTOSELECT seems to be related
> > > to the fact that it uses the "visible" kbuild syntax to hide
> > > entire group of drivers.
> > >
> > > This is not obvious and, as explained above, not always desired.
> > >
> > > To fix the problem, drop the "visible" and stop hiding any menu
> > > options. Users skilled enough to configure their kernel are expected
> > > to be skilled enough to know what (not) to configure anyway.
> > >
> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > ---
> > > drivers/media/dvb-frontends/Kconfig | 1 -
> > > drivers/media/i2c/Kconfig | 1 -
> > > drivers/media/spi/Kconfig | 1 -
> > > drivers/media/tuners/Kconfig | 1 -
> > > 4 files changed, 4 deletions(-)
> > >
> > > diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
> > > index dc43749177df..2d1fea3bf546 100644
> > > --- a/drivers/media/dvb-frontends/Kconfig
> > > +++ b/drivers/media/dvb-frontends/Kconfig
> > > @@ -1,5 +1,4 @@
> > > menu "Customise DVB Frontends"
> > > - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
> > >
> > > comment "Multistandard (satellite) frontends"
> > > depends on DVB_CORE
> > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > > index 79ce9ec6fc1b..475072bb67d6 100644
> > > --- a/drivers/media/i2c/Kconfig
> > > +++ b/drivers/media/i2c/Kconfig
> > > @@ -23,7 +23,6 @@ config VIDEO_IR_I2C
> > > #
> > >
> > > menu "I2C Encoders, decoders, sensors and other helper chips"
> > > - visible if !MEDIA_SUBDRV_AUTOSELECT || COMPILE_TEST || EXPERT
> >
> > Hmm... Hans picked this patch, but IMO it doesn't make sense
> > for PC consumer people to see the hundreds of extra options
> > that making those menus visible will produce.
> >
> > This was added because in the past we had lots of issues with
> > people desktop/laptop settings with all those things enabled.
> >
> > In any case, if the desktop/laptop user is smart enough to
> > go though it, he can simply disable MEDIA_SUBDRV_AUTOSELECT and
> > manually select what he wants, so I really miss the point of
> > making those stuff always visible.
> >
> > Now, from this patch's comments, it seems that you want this
> > to be visible if CONFIG_EMBEDDED. So, I won't complain if you
> > replace the changes on this patch to:
> >
> > menu "foo"
> > visible if !MEDIA_SUBDRV_AUTOSELECT || !EMBEDDED || COMPILE_TEST || EXPERT
> >
> > In other words, for the normal guy that just wants to build the
> > latest media stuff for his PC camera or TV device to work, he won't
> > need to dig into hundreds of things that won't make any difference
> > if he enables, except for making the Kernel bigger.
> >
>
> Well, I think the real value of MEDIA_SUBDRV_AUTOSELECT is the autoselection,
> not the hidden part. I'm really missing to see what hiding anything gives you.
>
> In other words, this option gets useful when driver authors select ancillary
> drivers such as:
>
> config VIDEO_USBVISION
> tristate "USB video devices based on Nogatech NT1003/1004/1005"
> depends on I2C && VIDEO_V4L2
> select VIDEO_TUNER
> select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT
>
> What's so confusing about having these drivers visible? Compared to the
> rest of the zillion menu options, what's more confusing about seeing these?
>
> Now, while I would agree with EMBEDDED, the problem with that is that
> many "embedded" platforms don't enable EMBEDDED. So, it's not that useful.
>
> Finally, let me give an example of why hiding the menus is so bad.
> Normally, to enable a symbol, we use the search tool.
>
> Now, when MEDIA_SUBDRV_AUTOSELECT=y, the search tool will _not_ take you
> there and there's no indication why.
As someone who has done so in the past year, I agree it's confusing.
I had to dig through the Kconfig files to figure out which knobs to
turn to get the OV5640 option out. The description says "auto-selecting",
which does not equal hiding everything. You could still have drivers
auto-selected (or not) based on a Kconfig option without hiding things.
ChenYu
next prev parent reply other threads:[~2019-07-25 17:30 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-15 21:23 [PATCH 0/2] Some cleanups for ancillary drivers autoselect Ezequiel Garcia
2019-07-15 21:23 ` [PATCH 1/2] media: Don't default-enable "ancillary driver autoselect" if EMBEDDED Ezequiel Garcia
2019-07-15 21:23 ` [PATCH 2/2] media: Don't hide any menu if "ancillary drivers autoselect" is enabled Ezequiel Garcia
2019-07-25 15:57 ` Mauro Carvalho Chehab
2019-07-25 16:01 ` Hans Verkuil
2019-07-25 17:05 ` Ezequiel Garcia
2019-07-25 17:29 ` Chen-Yu Tsai [this message]
2019-07-25 18:41 ` Mauro Carvalho Chehab
2019-07-25 19:00 ` Helen Koike
2019-07-25 19:16 ` Mauro Carvalho Chehab
2019-07-25 23:55 ` Ezequiel Garcia
2019-07-26 2:09 ` Mauro Carvalho Chehab
2019-07-26 2:54 ` Ezequiel Garcia
2019-07-27 0:17 ` Ezequiel Garcia
2019-07-27 1:39 ` Mauro Carvalho Chehab
2019-07-25 17:45 ` Mauro Carvalho Chehab
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAGb2v65wOz+nUi=Leb3FudU7K5S_AHtuCarXHcO0kMvvqEw8rQ@mail.gmail.com' \
--to=wens@kernel.org \
--cc=ezequiel@collabora.com \
--cc=hans.verkuil@cisco.com \
--cc=helen.koike@collabora.com \
--cc=kernel@collabora.com \
--cc=linux-media@vger.kernel.org \
--cc=mchehab+samsung@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).