On Wed, Nov 8, 2023, at 19:33, Martin Tůma wrote:
> On 08. 11. 23 17:13, Arnd Bergmann wrote:
>> On Fri, Oct 27, 2023, at 16:17, Martin Tůma wrote:
>>> On 23. 10. 23 18:05, Arnd Bergmann wrote:
>>>> From: Arnd Bergmann <arnd@xxxxxxxx>
>
> On SoCs you probably get a kernel configuration that is missing some
> feature but still boots up when you do not select/depend on the exact
> controller, but in the case of the mgb4 PCIe card you get a driver that
> does not work at all (The SPI_XILINX dependency could theoretically be
> made configurable, but you would lose the ability to flash the correct
> FW for the current HW module and the access to the card's serial number.
> I2C and XDMA are crucial.).
My point was that we do this all the time for things that are
essential: if your clock controller or the irqchip have
no driver, then the camera device won't work, but neither
would anything else.
So in a SoC setting, you really just need to enable all
the drivers for devices on that chip through the .config.
>> Since this is a PCI device, it's a bit different, so maybe
>> something like this would work to correctly document which
>> dependencies are required at build time vs run time:
>>
>> --- a/drivers/media/pci/mgb4/Kconfig
>> +++ b/drivers/media/pci/mgb4/Kconfig
>> @@ -1,15 +1,13 @@
>> # SPDX-License-Identifier: GPL-2.0-only
>> config VIDEO_MGB4
>> tristate "Digiteq Automotive MGB4 support"
>> - depends on VIDEO_DEV && PCI && I2C && DMADEVICES && SPI && MTD && IIO
>> + depends on VIDEO_DEV && PCI && I2C && SPI && MTD && IIO
>> depends on COMMON_CLK
>> + depends on XILINX_XDMA
>> + depends on (I2C_XILINX && SPI_XILINX) || COMPILE_TEST
>> select VIDEOBUF2_DMA_SG
>> select IIO_BUFFER
>> select IIO_TRIGGERED_BUFFER
>> - select I2C_XILINX
>> - select SPI_XILINX
>> - select MTD_SPI_NOR
>> - select XILINX_XDMA
>> help
>> This is a video4linux driver for Digiteq Automotive MGB4 grabber
>> cards.
>>
>
> My motivation when using "select" was to help people using "make
> menuconfig" to get the module selected/configured as they will usually
> not know that there are some Xilinx IP cores used that need separate
> drivers and the menuconfig GUI simply hides the mgb4 option making it
> almost impossible just from the menus to find out what has to be selected.
>
> But when there are reasons, why to chose "depends on" (like various
> configurations, tests or the "readability" of the dependencies) than I'm
> ok with your patch proposal.
The main reason to use 'depends on' over 'select' here is that
mixing the two is a common source of dependency loops that end
up breaking the build. As a rule of thumb, I would use 'select'
only for symbols that others already select, or that are hidden
from visibility.
Arnd
