On 10/06/14 10:39, Oliver Hartkopp wrote: > > > On 10/06/2014 06:52 PM, Randy Dunlap wrote: >> On 10/06/14 01:06, Oliver Hartkopp wrote: >>> Hello all, >>> >>> just to get it right: >>> >>> So far it looks like this in linux/drivers/net/can/sja1000/Kconfig >>> >>> config CAN_PEAK_PCIEC >>> bool "PEAK PCAN-ExpressCard Cards" >>> depends on CAN_PEAK_PCI >>> select I2C >>> select I2C_ALGOBIT >>> >>> If one would change the >>> >>> select I2C >>> >>> into >>> >>> depends on I2C >>> >>> IMHO the CAN_PEAK_PCIEC hardware would *only* be visible and selectable when >>> I2C was selected before (from anyone else?). >> >> That is correct. >> >>> So what it wrong on the current Kconfig entry? >>> Is 'select' deprecated? >> >> No, it's not deprecated. It's just dangerous. and driver configs should not >> enable entire subsystems via 'select'. >> >>> Or did randconfig generate a configuration that would not be possible by >>> properly generating the config file with 'make menuconfig' ?? >> >> randconfig generated a config for another driver which causes a build error, >> not for a CAN driver. The CAN driver does not have a build error AFAIK. >> Its Kconfig is just doing something with a very big & ugly stick. > > But when it is not done like this, we might have an invisible config option in > the corner case that I2C is not enabled by anyone else. > > So what would you propose then? > > AFAICS there is 'just' a style problem as 'configs should not enable entire > subsystems'. But it finally is a correct and valid Kconfig, right? Yes, right. > When I2C is already enabled - fine. If (unlikely) I2C is not enabled, we need > to pull the ugly stick. So what is dangerous on this? Was there any misuse of > select statements before? No syntactic misuse, more of a style thing, like you say. The danger is in select being a big stick that does not check for symbol dependencies. In the unlikely case that I2C is not enabled, the user should have to enable it instead of a solitary driver enabling it. IOW, if a subsystem is disabled, the user probably wanted it that way and a single driver should not override that setting. -- ~Randy -- To unsubscribe from this list: send the line "unsubscribe linux-next" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
