* [PATCH 1/2] dell-wmi, dell-laptop: hide dell-smbios @ 2016-05-26 9:42 Jean Delvare 2016-06-02 11:03 ` Michał Kępień 0 siblings, 1 reply; 7+ messages in thread From: Jean Delvare @ 2016-05-26 9:42 UTC (permalink / raw) To: platform-driver-x86 Cc: Michał Kępień, Pali Rohár, Darren Hart 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. Signed-off-by: Jean Delvare <jdelvare@suse.de> Cc: Michał Kępień <kernel@kempniu.pl> Cc: Pali Rohár <pali.rohar@gmail.com> Cc: Darren Hart <dvhart@infradead.org> --- 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. -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dell-wmi, dell-laptop: hide dell-smbios 2016-05-26 9:42 [PATCH 1/2] dell-wmi, dell-laptop: hide dell-smbios Jean Delvare @ 2016-06-02 11:03 ` Michał Kępień 2016-06-02 11:10 ` Pali Rohár 0 siblings, 1 reply; 7+ messages in thread From: Michał Kępień @ 2016-06-02 11:03 UTC (permalink / raw) To: Jean Delvare; +Cc: platform-driver-x86, Pali Rohár, Darren Hart > 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. > > Signed-off-by: Jean Delvare <jdelvare@suse.de> > Cc: Michał Kępień <kernel@kempniu.pl> > Cc: Pali Rohár <pali.rohar@gmail.com> > Cc: Darren Hart <dvhart@infradead.org> > --- > 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. I like this approach and all the way I've been thinking whether it's reasonable to hide configuration options that are pretty self-explanatory ("Dell Laptop Extras", "Dell WMI extras") behind a rather cryptic one ("Dell SMBIOS Support"). The only reason I didn't go the way you're suggesting in this patch is my lack of experience with kbuild and the warning in Documentation/kbuild/kconfig-language.txt that "select should be used with care". -- Best regards, Michał Kępień ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dell-wmi, dell-laptop: hide dell-smbios 2016-06-02 11:03 ` Michał Kępień @ 2016-06-02 11:10 ` Pali Rohár 2016-06-02 21:03 ` Jean Delvare 0 siblings, 1 reply; 7+ messages in thread From: Pali Rohár @ 2016-06-02 11:10 UTC (permalink / raw) To: Michał Kępień Cc: Jean Delvare, platform-driver-x86, Darren Hart 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. > > > > Signed-off-by: Jean Delvare <jdelvare@suse.de> > > Cc: Michał Kępień <kernel@kempniu.pl> > > Cc: Pali Rohár <pali.rohar@gmail.com> > > Cc: Darren Hart <dvhart@infradead.org> > > --- > > 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... So I think kbuild developers should review this change. > I like this approach and all the way I've been thinking whether it's > reasonable to hide configuration options that are pretty > self-explanatory ("Dell Laptop Extras", "Dell WMI extras") behind a > rather cryptic one ("Dell SMBIOS Support"). The only reason I didn't go > the way you're suggesting in this patch is my lack of experience with > kbuild and the warning in Documentation/kbuild/kconfig-language.txt that > "select should be used with care". -- Pali Rohár pali.rohar@gmail.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dell-wmi, dell-laptop: hide dell-smbios 2016-06-02 11:10 ` Pali Rohár @ 2016-06-02 21:03 ` Jean Delvare 2016-06-07 11:30 ` Pali Rohár 0 siblings, 1 reply; 7+ messages in thread From: Jean Delvare @ 2016-06-02 21:03 UTC (permalink / raw) To: Pali Rohár Cc: Michał Kępień, platform-driver-x86, Darren Hart 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. > > > > > > Signed-off-by: Jean Delvare <jdelvare@suse.de> > > > Cc: Michał Kępień <kernel@kempniu.pl> > > > Cc: Pali Rohár <pali.rohar@gmail.com> > > > Cc: Darren Hart <dvhart@infradead.org> > > > --- > > > 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. 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 ;-) -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dell-wmi, dell-laptop: hide dell-smbios 2016-06-02 21:03 ` Jean Delvare @ 2016-06-07 11:30 ` Pali Rohár 2016-06-08 21:57 ` Darren Hart 0 siblings, 1 reply; 7+ messages in thread From: Pali Rohár @ 2016-06-07 11:30 UTC (permalink / raw) To: Jean Delvare Cc: Michał Kępień, platform-driver-x86, Darren Hart 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. > > > > > > > > Signed-off-by: Jean Delvare <jdelvare@suse.de> > > > > Cc: Michał Kępień <kernel@kempniu.pl> > > > > Cc: Pali Rohár <pali.rohar@gmail.com> > > > > Cc: Darren Hart <dvhart@infradead.org> > > > > --- > > > > 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. > > 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@gmail.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dell-wmi, dell-laptop: hide dell-smbios 2016-06-07 11:30 ` Pali Rohár @ 2016-06-08 21:57 ` Darren Hart 0 siblings, 0 replies; 7+ messages in thread From: Darren Hart @ 2016-06-08 21:57 UTC (permalink / raw) To: Pali Rohár Cc: Jean Delvare, Michał Kępień, platform-driver-x86, linux-kbuild, Yann E. MORIN 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@suse.de> > > > > > Cc: Michał Kępień <kernel@kempniu.pl> > > > > > Cc: Pali Rohár <pali.rohar@gmail.com> > > > > > Cc: Darren Hart <dvhart@infradead.org> > > > > > --- > > > > > 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@gmail.com > -- Darren Hart Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dell-wmi, dell-laptop: hide dell-smbios @ 2016-06-08 21:57 ` Darren Hart 0 siblings, 0 replies; 7+ messages in thread From: Darren Hart @ 2016-06-08 21:57 UTC (permalink / raw) To: Pali Rohár Cc: Jean Delvare, Michał Kępień, platform-driver-x86, linux-kbuild, Yann E. MORIN 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@suse.de> > > > > > Cc: Michał Kępień <kernel@kempniu.pl> > > > > > Cc: Pali Rohár <pali.rohar@gmail.com> > > > > > Cc: Darren Hart <dvhart@infradead.org> > > > > > --- > > > > > 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@gmail.com > -- Darren Hart Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-06-08 21:57 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-05-26 9:42 [PATCH 1/2] dell-wmi, dell-laptop: hide dell-smbios Jean Delvare 2016-06-02 11:03 ` Michał Kępień 2016-06-02 11:10 ` Pali Rohár 2016-06-02 21:03 ` Jean Delvare 2016-06-07 11:30 ` Pali Rohár 2016-06-08 21:57 ` Darren Hart 2016-06-08 21:57 ` Darren Hart
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.