Re: [PATCH] [media] [kbuild] Add and use IS_REACHABLE macro

[Michal Marek]

Dne 19.2.2015 v 15:53 Arnd Bergmann napsal(a):
> On Thursday 19 February 2015 13:11:07 Michal Marek wrote:
>> On 2015-02-18 18:12, Arnd Bergmann wrote:
>>> In the media drivers, the v4l2 core knows about all submodules
>>> and calls into them from a common function. However this cannot
>>> work if the modules that get called are loadable and the
>>> core is built-in. In that case we get
>>>
>>> drivers/built-in.o: In function `set_type':
>>> drivers/media/v4l2-core/tuner-core.c:301: undefined reference to `tea5767_attach'
>>> drivers/media/v4l2-core/tuner-core.c:307: undefined reference to `tea5761_attach'
>>> drivers/media/v4l2-core/tuner-core.c:349: undefined reference to `tda9887_attach'
>>> drivers/media/v4l2-core/tuner-core.c:405: undefined reference to `xc4000_attach'
>>> [...]
>>> Ideally Kconfig would be used to avoid the case of a broken dependency,
>>> or the code restructured in a way to turn around the dependency, but either
>>> way would require much larger changes here.
>>
>> What can be done without extending kbuild is to accept
>> CONFIG_VIDEO_TUNER=y and CONFIG_MEDIA_TUNER_FOO=m, but build both into
>> the kernel, e.g.
> 
> Right, but
> 
>> diff --git a/drivers/media/tuners/Kconfig b/drivers/media/tuners/Kconfig
>> index 42e5a01..d2c7e89 100644
>> --- a/drivers/media/tuners/Kconfig
>> +++ b/drivers/media/tuners/Kconfig
>> @@ -71,6 +71,11 @@ config MEDIA_TUNER_TEA5767
>>         help
>>           Say Y here to include support for the Philips TEA5767 radio tuner.
>>  
>> +config MEDIA_TUNER_TEA5767_BUILD
>> +       tristate
>> +       default VIDEO_TUNER || MEDIA_TUNER_TEA5767
>> +       depends on MEDIA_TUNER_TEA5767!=n
>> +
>>  config MEDIA_TUNER_MSI001
>>         tristate "Mirics MSi001"
>>         depends on MEDIA_SUPPORT && SPI && VIDEO_V4L2
> 
> We'd then have to do the same for each tuner driver that we have in the
> kernel or that gets added later.

Yes :-(.


>> Actually, I have hard time coming up with a kconfig syntactic sugar to
>> express such dependency. If I understand it correctly, the valid
>> configurations in this case are
>>
>> MEDIA_TUNER_TEA5767     n       m       y
>> VIDEO_TUNER     n       x       x       x
>>                 m       x       x       x
>>                 y       x               x
>>
>> I.e. only VIDEO_TUNER=y and MEDIA_TUNER_TEA5767=m is incorrect, isn't
>> it?
> 
> Yes, I think that is correct.

OK. In this case, a kconfig extension would be really too specific: "if
set, set to at least X but only if X is set as well."


> We have similar problems in other areas
> of the kernel. In theory, we could enforce the VIDEO_TUNER driver to
> be modular here by adding lots of dependencies to it:
> 
> config VIDEO_TUNER
> 	tristate
> 	depends on MEDIA_TUNER_TEA5761 || !MEDIA_TUNER_TEA5761
> 	depends on MEDIA_TUNER_TEA5767 || !MEDIA_TUNER_TEA5767
> 	depends on MEDIA_TUNER_MSI001  || !MEDIA_TUNER_MSI001

Nah, that's even uglier. I suggest to merge your IS_REACHABLE patch.

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