On Tue, Jun 07, 2016 at 01:30:19PM +0200, Pali Rohár wrote: > On Thursday 02 June 2016 23:03:36 Jean Delvare wrote: > > Hi Pali, > > > > On Thu, 2 Jun 2016 13:10:29 +0200, Pali Rohár wrote: > > > On Thursday 02 June 2016 13:03:33 Michał Kępień wrote: > > > > > Dell-smbios is a helper module, it serves no purpose on its own, so > > > > > do not present it as an option to the user. Instead, select it > > > > > automatically whenever a driver which needs it is selected. Hi Yann, We have a question regarding best practice of depends and selects instigated by the following patch from Jean. > > > > > > > > > > Signed-off-by: Jean Delvare <jdelvare@xxxxxxx> > > > > > Cc: Michał Kępień <kernel@xxxxxxxxxx> > > > > > Cc: Pali Rohár <pali.rohar@xxxxxxxxx> > > > > > Cc: Darren Hart <dvhart@xxxxxxxxxxxxx> > > > > > --- > > > > > drivers/platform/x86/Kconfig | 8 +++++--- > > > > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > > > > > > > --- linux-4.6.orig/drivers/platform/x86/Kconfig 2016-05-16 00:43:13.000000000 +0200 > > > > > +++ linux-4.6/drivers/platform/x86/Kconfig 2016-05-26 11:18:50.029103790 +0200 > > > > > @@ -92,7 +92,7 @@ config ASUS_LAPTOP > > > > > If you have an ACPI-compatible ASUS laptop, say Y or M here. > > > > > > > > > > config DELL_SMBIOS > > > > > - tristate "Dell SMBIOS Support" > > > > > + tristate > > > > > depends on DCDBAS > > > > > default n > > > > > ---help--- > > > > > @@ -104,12 +104,13 @@ config DELL_SMBIOS > > > > > config DELL_LAPTOP > > > > > tristate "Dell Laptop Extras" > > > > > depends on X86 > > > > > - depends on DELL_SMBIOS > > > > > depends on DMI > > > > > depends on BACKLIGHT_CLASS_DEVICE > > > > > depends on ACPI_VIDEO || ACPI_VIDEO = n > > > > > depends on RFKILL || RFKILL = n > > > > > depends on SERIO_I8042 > > > > > + depends on DCDBAS > > > > > + select DELL_SMBIOS > > > > > select POWER_SUPPLY > > > > > select LEDS_CLASS > > > > > select NEW_LEDS > > > > > @@ -124,7 +125,8 @@ config DELL_WMI > > > > > depends on DMI > > > > > depends on INPUT > > > > > depends on ACPI_VIDEO || ACPI_VIDEO = n > > > > > - depends on DELL_SMBIOS > > > > > + depends on DCDBAS > > > > > + select DELL_SMBIOS > > > > > select INPUT_SPARSEKMAP > > > > > ---help--- > > > > > Say Y here if you want to support WMI-based hotkeys on Dell laptops. > > > > > > > > While I'm not a maintainer, I feel obliged to respond as I introduced > > > > the changes which this patch applies to. > > > > > > Well, I'm on maintainer list of dell modules, but I do care about kbuild > > > configuration if it is working. So I let review of this change to other > > > people who understand kbuild better. > > > > > > What I see in this change is adding "duplicate" or "redundant" > > > transitive dependency from DELL_WMI to DCDBAS. But DELL_WMI does not use > > > or need DCDBAS. It just needs DELL_SMIBIOS and basically DELL_WMI does > > > not care what DELL_SMBIOS is using (if DCDBAS, ACPI or any other thing). > > > So from my graph dependency point of view it is not correct, but I do > > > not know how kbuild is working... > > > > Sadly, options which select other options do not inherit from their > > dependencies, so kconfig would complain about unmet dependencies if you > > let the user enable an option which selects something that depends on > > something which is missing or not selected. I wish kconfig was smarter > > and would inherit dependencies in this case, but this doesn't happen. I > > don't know if it is a design decision, a limitation, or just the way it > > is for no specific reason. > > > > The only way I know of to avoid the duplicate dependency would be to > > let DELL_SMBIOS select DCDBAS instead of depending on it. But this is a > > more intrusive change. If it were just me I'd use select a lot more, as > > I don't think it is user-friendly to have drivers in one subsystem > > silently depend on options from another subsystem. But not everybody > > agrees with that. > > > > > So I think kbuild developers should review this change. Let's ask them then. +Yann +linux-kbuild > > > > Sounds unrealistic to me. It's as if you would ask Kernighan and Ritchie > > to review your driver code because they invented the C language ;-) > > No, that is not same. It is about that restriction that kbuild does not > support (correctly) transitive dependences and this is probably good > proposal either for extending kbuild or fixing something else. And > kbuild maintainers/developers either confirm this is really problem in > kbuild (which should be fixed) or show us how to write such thing > correctly. Or we are trying to use kbuild for something for which kbuild > is not designed... > > -- > Pali Rohár > pali.rohar@xxxxxxxxx > -- Darren Hart Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
