Message-ID: <Pine.LNX.4.64.0805170624490.11757@bombadil.infradead.org>
Date:	Sat, 17 May 2008 06:58:47 -0400 (EDT)
From:	Mauro Carvalho Chehab <mchehab@...radead.org>
To:	Adrian Bunk <bunk@...nel.org>
cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-dvb-maintainer@...uxtv.org, video4linux-list@...hat.com,
	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>
Subject: Re: [GIT PATCHES] V4L/DVB fixes for 2.6.26

On Fri, 16 May 2008, Adrian Bunk wrote:

> I wanted to simply turn the existing dependencies on INPUT to select's
> (and add a dependency on S390 to the "Multimedia devices" menu).

I suspect you're maning depends on !S390. This is not needed, since 
Multimedia devices is already dependent on HAS_IOMEM.

>> Yet, I can't imagine any production kernel without INPUT. What happens if INPUT
>> is disabled? No keyboard, no tablet and no mouse at all?
>
> CONFIG_INPUT is always y unless you enable CONFIG_EMBEDDED.
>
> But on some kinds of embedded systems kernels without INPUT are actually
> not uncommon - and they don't have any keyboard or mouse.

There are several embedded V4L/DVB devices: Cellular phones, Set Top 
Boxes, surveillance systems, etc. I'm not sure if forcing the need for 
INPUT would be nice. Also, from time to time, people ask for a feature of 
allowing to disable the IR.

Maybe the better would be to allow the user to explicitly select/unselect 
IR (for advanced users, and if INPUT). If IR is disabled, we may disable 
the corresponding <board>-input.c compilation. It doesn't seem hard to do 
this way, but it will require more time to bake a patch.

> I'm not only thinking about today, that's an ongoing problem that could
> be fixed this way.
>
> Plus the fact that the dependencies on HOTPLUG don't help you when the
> option gets select'ed.

Yes, but the committed patch is adding "depends on HOTPLUG" to all devices 
that selects FW_LOADER.

If you want, feel free to change this to select, although I can't see any 
real gain.

> Do you have a list of open issues (preferably with .config's)?

The open issue I see is to check the "depends on" for all symbols that are 
selected.

>>> Should I fix the dependency or can I let VIDEO_IR select I2C and remove
>>> VIDEO_IR_I2C?

I would remove the select inside VIDEO_IR, adding a separate select for 
VIDEO_IR_I2C. There's a problem on saa7134-input: It uses some symbols 
defined on ir-kbd-i2c:

$ grep EXPORT ir-kbd-i2c.c
EXPORT_SYMBOL_GPL(get_key_pinnacle_grey);
EXPORT_SYMBOL_GPL(get_key_pinnacle_color);

This breaks saa7134 compilation, if IR-I2C is not selected (or if it is a 
module, and saa7134 is 'Y).

The proper fix here is to move those symbols to ir-keymaps.c, where the 
shared IR tables should be.

Except for this, it is safe to allow the user to not compile VIDEO_IR_I2C, 
even for devices that has i2c IR's. If the module is not compiled, 
request_module() will fail, but everything else will work properly.

>> I would add an entry to allow the user to select this explicitly, for power
>> users, and select it implicitly. Something like:
>>
>> select VIDEO_IR_I2C  if VIDEO_HELPER_CHIPS_AUTO
>>
>> at the drivers under media/video that selects IR. This need to be mandatory for
>> a few drivers like saa7134, where some exported symbols at kbd-ir-i2c are used
>> there.
>
> Are there any real use cases for this justifying adding yet another
> twist to the kconfig stuff?
>
> I want to make it simpler, not more complicated, and your last sentence
> just describes another new pitfall.
>
> I get the point that it makes sense that it's possible to build only the
> one tuner you actually have instead of a dozen automatically select'ed,
> but you must somewhere draw a "this twist is not worth the maintainance
> overhead" line.
>
> The overall picture is that we cannot add a kconfig option and an
> #ifdef around each line of code in the kernel only because someone might
> want to disable it.
>
> We simply cannot maintain that in the long term
> (drivers/media/ is already at the edge).

True.

>
> And as soon as you enter the 10kB object code size area there are enough
> lower hanging fruits for saving space that do not involve increased
> complexity in kconfig.

Yes, but in is, in fact, 10K * lots of modules.

The point is that the I2C modules behave very well if they aren't 
compiled.

I can, for example, compile bttv with just tuner-simple, and nothing else. 
This will work with all bttv functionalities for two devices I have here. 
A third analog-only device will require TVAUDIO, otherwise, the audio chip 
won't be loaded, and audio decoder won't happen. So, the device will work 
only with video.

I don't see much troubles with most of those I2C helper modules, since the 
vast majority depends only on I2C (only a few also depends on FW_LOADER).

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists