Re: [GIT PULL for v7.4] media updates

[Mauro Carvalho Chehab]

On Mon, 26 Oct 2026 08:53:07 -1000
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Mon, 26 Oct 2026 at 08:25, Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> 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.

Thanks,
Mauro