mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Media Mailing List <linux-media@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Hans Verkuil <hans@jjverkuil.nl>
Subject: Re: [GIT PULL for v7.4] media updates
Date: Tue, 27 Oct 2026 09:01:34 +0100	[thread overview]
Message-ID: <20261027090134.09b112ba@foz.lan> (raw)
In-Reply-To: <20261027073720.GD2005298@killaraus.ideasonboard.com>

On Tue, 27 Oct 2026 09:37:20 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> On Tue, Oct 27, 2026 at 12:56:56AM +0100, Mauro Carvalho Chehab wrote:
> > On Mon, 26 Oct 2026 08:53:07 -1000 Linus Torvalds wrote:
> >   
> > > On Mon, 26 Oct 2026 at 08:25, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:  
> > > >
> > > > The patch in question puts camera sensors and related chips in line with
> > > > how the rest of the drivers are selected elsewhere.    
> > > 
> > > .. and other places could probably be improved too, I agree.
> > > 
> > > But this made things *worse*. Much worse.
> > > 
> > > The thing is, at least hide them behind some "embedded camera options"
> > > that in turn is hidden behind "EXPERT" or something like that.
> > > 
> > > Don't make *normal* people who build their own kernels have to see or know.
> > > 
> > > Let's say that you have a laptop or something that actually uses one
> > > of these sensors for a webcam, then it STILL does not help to ask a
> > > user
> > > 
> > >  "Do you want a driver for the Analog Devices ADV7170 video encoder"
> > > 
> > > Because (a) that chip legacy and likely not in any laptop anyway and
> > > (b) even if it was, no laptop user is going to *know* that.
> > > 
> > > See what I'm saying? You literally make things worse. Don't do that.
> > > You're making the kernel Kconfig script ask insane questions. They are
> > > insane because you cannot reasonably expect people to actually know
> > > the answer to the questions.
> > > 
> > > In other words: don't ask questions that you know the user cannot answer.
> > > 
> > > There was a REASON why those menus were hidden.  
> > 
> > I agree with Linus on that. You're basically rolling back a feature
> > that simplifies a lot the life for normal users: let Kconfig choose
> > options that only a media expert can answer.
> > 
> > Now, I understand why it is tempting to do that to help with IPU
> > support: camera sensors can now be easily interchangeable on new 
> > Intel hardware.
> > 
> > If you look on some drivers from the old days, like bttv, saa7134
> > and cx88, they had similar problems, but they still do support
> > autoselect mode.
> > 
> > For instance, drivers/media/tuners/Kconfig has this:
> > 
> >    config MEDIA_TUNER
> >         tristate
> >         depends on (MEDIA_ANALOG_TV_SUPPORT || MEDIA_DIGITAL_TV_SUPPORT || MEDIA_RADIO_SUPPORT || MEDIA_SDR_SUPPORT) && I2C
> >         default y
> >         select MEDIA_TUNER_XC2028 if MEDIA_SUBDRV_AUTOSELECT
> >         select MEDIA_TUNER_XC5000 if MEDIA_SUBDRV_AUTOSELECT
> >         select MEDIA_TUNER_XC4000 if MEDIA_SUBDRV_AUTOSELECT
> >         select MEDIA_TUNER_MT20XX if MEDIA_SUBDRV_AUTOSELECT
> >         select MEDIA_TUNER_TDA8290 if MEDIA_SUBDRV_AUTOSELECT
> >         select MEDIA_TUNER_TEA5761 if MEDIA_SUBDRV_AUTOSELECT && MEDIA_RADIO_SUPPORT
> >         select MEDIA_TUNER_TEA5767 if MEDIA_SUBDRV_AUTOSELECT && MEDIA_RADIO_SUPPORT
> >         select MEDIA_TUNER_SIMPLE if MEDIA_SUBDRV_AUTOSELECT
> >         select MEDIA_TUNER_TDA9887 if MEDIA_SUBDRV_AUTOSELECT
> >         select MEDIA_TUNER_MC44S803 if MEDIA_SUBDRV_AUTOSELECT
> > 
> > which is meant to support the most popular I2C tuners.
> > 
> > In the specific case of IPU (and atomisp, once moved out of staging),
> > the right solution would be to have something similar to it, e.g.
> > something like:
> > 
> > 	config VIDEO_INTEL_IPU6
> > 	select VIDEO_OV2680 if MEDIA_SUBDRV_AUTOSELECT
> > 	select VIDEO_OV2685 if MEDIA_SUBDRV_AUTOSELECT
> > 	...
> > 	
> > for each sensor that might be possible to be used there. Another
> > alternative would be to do it the reverse way, e.g.:
> > 	
> > 	config VIDEO_OV2680
> > 		default MEDIA_SUBDRV_AUTOSELECT && VIDEO_INTEL_IPU6
> > 
> > or, maybe even better:
> > 
> > 	# Make setup easier by identifying PC MC-based drivers on a single place
> > 	config MEDIA_SENSOR_AUTOSELECT
> > 	depends on VIDEO_INTEL_IPU6
> > 	default y
> > 
> > 	config VIDEO_OV2680
> > 	default MEDIA_SENSOR_AUTOSELECT
> > 
> > Placing MEDIA_SENSOR_AUTOSELECT on sensor drivers. We may need
> > similar code also for other I2C drivers that could be used on IPU.  
> 
> You're forgetting that this affects not only Intel-based machines with
> raw sensors, but a lot of Arm laptops too. We would end up selecting a
> vast majority of sensor drivers unconditionally. 

I never got any ARM laptop on my hands but the principle is the same:
For embedded systems, it is important to control each component to
be part of the kernel, but for general computing (PCs, laptops,
workstations), it should be simple enough to not require one to be an
expert on media to be able to build its own kernel.

So yes, the same principle applies also for ARM laptops: if
MEDIA_SUBDRV_AUTOSELECT is enabled, all sensors and other I2C
components that could be needed for them to work should be build
as well.

> If we want to go that
> route, a single option to select all camera sensor drivers would be
> simpler.

Works for me, although I'm pretty sure several sensors there won't
be used on either Intel or ARM machines. I'd say that most of
sensor drivers older than the atomisp driver are probably not
shipped either on Intel or ARM, because either they have very
low resolution for those days or because they were already EOL
by the time PC/laptop hardware started being shipped with MC-based
cameras.

So I would prefer having a MEDIA_CAMERA_SUBDRV_AUTOSELECT or
MEDIA_SENSOR_AUTOSELECT Kconfig option, dependent of 
MEDIA_SUBDRV_AUTOSELECT and use it for the drivers that makes
sense.

Thanks,
Mauro

  reply	other threads:[~2026-10-27  8:01 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-24 16:50 Mauro Carvalho Chehab
2026-10-25  5:51 ` pr-tracker-bot
2026-10-26 17:34 ` Linus Torvalds
2026-10-26 18:24   ` Sakari Ailus
2026-10-26 18:53     ` Linus Torvalds
2026-10-26 23:56       ` Mauro Carvalho Chehab
2026-10-27  0:04         ` Mauro Carvalho Chehab
2026-10-27  7:37         ` Laurent Pinchart
2026-10-27  8:01           ` Mauro Carvalho Chehab [this message]
2026-10-27 10:09       ` [PATCH 1/1] Revert "media: Don't hide I²C or SPI drivers by default" Sakari Ailus
2026-10-27 10:19         ` Laurent Pinchart
2026-10-27 11:03           ` Sakari Ailus
2026-10-26 19:13     ` [GIT PULL for v7.4] media updates Laurent Pinchart

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=20261027090134.09b112ba@foz.lan \
    --to=mchehab+huawei@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hans@jjverkuil.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=torvalds@linux-foundation.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

all inboxes | Powered by JetHome®