linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] platform/x86: dell-smbios: Correct some style warnings
@ 2018-02-27 15:56 Mario Limonciello
  2018-02-27 15:56 ` [PATCH 2/3] platform/x86: dell-smbios: Rename dell-smbios source to dell-smbios-base Mario Limonciello
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Mario Limonciello @ 2018-02-27 15:56 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: pali.rohar, LKML, platform-driver-x86, Mario Limonciello

WARNING: function definition argument 'struct calling_interface_buffer *'
should also have an identifier name
+       int (*call_fn)(struct calling_interface_buffer *);

WARNING: Block comments use * on subsequent lines
+       /* 4 bytes of table header, plus 7 bytes of Dell header,
	plus at least
+          6 bytes of entry */

WARNING: Block comments use a trailing */ on a separate line
+          6 bytes of entry */

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/dell-smbios.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 8541cde..4850658 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -36,7 +36,7 @@ static DEFINE_MUTEX(smbios_mutex);
 struct smbios_device {
 	struct list_head list;
 	struct device *device;
-	int (*call_fn)(struct calling_interface_buffer *);
+	int (*call_fn)(struct calling_interface_buffer *arg);
 };
 
 struct smbios_call {
@@ -353,7 +353,8 @@ static void __init parse_da_table(const struct dmi_header *dm)
 		container_of(dm, struct calling_interface_structure, header);
 
 	/* 4 bytes of table header, plus 7 bytes of Dell header, plus at least
-	   6 bytes of entry */
+	 * 6 bytes of entry
+	 */
 
 	if (dm->length < 17)
 		return;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] platform/x86: dell-smbios: Rename dell-smbios source to dell-smbios-base
  2018-02-27 15:56 [PATCH 1/3] platform/x86: dell-smbios: Correct some style warnings Mario Limonciello
@ 2018-02-27 15:56 ` Mario Limonciello
  2018-02-27 15:56 ` [PATCH 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together Mario Limonciello
  2018-02-27 16:04 ` [PATCH 1/3] platform/x86: dell-smbios: Correct some style warnings Andy Shevchenko
  2 siblings, 0 replies; 8+ messages in thread
From: Mario Limonciello @ 2018-02-27 15:56 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: pali.rohar, LKML, platform-driver-x86, Mario Limonciello

This is being done to faciliate a later change to link all the dell-smbios
drivers together.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/Makefile                              | 1 +
 drivers/platform/x86/{dell-smbios.c => dell-smbios-base.c} | 0
 2 files changed, 1 insertion(+)
 rename drivers/platform/x86/{dell-smbios.c => dell-smbios-base.c} (100%)

diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index c388608..940b118 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_MSI_LAPTOP)	+= msi-laptop.o
 obj-$(CONFIG_ACPI_CMPC)		+= classmate-laptop.o
 obj-$(CONFIG_COMPAL_LAPTOP)	+= compal-laptop.o
 obj-$(CONFIG_DELL_SMBIOS)	+= dell-smbios.o
+dell-smbios-objs		:= dell-smbios-base.o
 obj-$(CONFIG_DELL_SMBIOS_WMI)	+= dell-smbios-wmi.o
 obj-$(CONFIG_DELL_SMBIOS_SMM)	+= dell-smbios-smm.o
 obj-$(CONFIG_DELL_LAPTOP)	+= dell-laptop.o
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios-base.c
similarity index 100%
rename from drivers/platform/x86/dell-smbios.c
rename to drivers/platform/x86/dell-smbios-base.c
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together
  2018-02-27 15:56 [PATCH 1/3] platform/x86: dell-smbios: Correct some style warnings Mario Limonciello
  2018-02-27 15:56 ` [PATCH 2/3] platform/x86: dell-smbios: Rename dell-smbios source to dell-smbios-base Mario Limonciello
@ 2018-02-27 15:56 ` Mario Limonciello
  2018-02-27 16:08   ` Andy Shevchenko
  2018-02-27 16:04 ` [PATCH 1/3] platform/x86: dell-smbios: Correct some style warnings Andy Shevchenko
  2 siblings, 1 reply; 8+ messages in thread
From: Mario Limonciello @ 2018-02-27 15:56 UTC (permalink / raw)
  To: dvhart, Andy Shevchenko
  Cc: pali.rohar, LKML, platform-driver-x86, Mario Limonciello

Some race conditions were raised due to dell-smbios and its backends
not being ready by the time that a consumer would call one of the
exported methods.

To avoid this problem, guarantee that all initialization has been
done by linking them all together and running init for them all.

As part of this change the Kconfig needs to be adjusted so that
CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
rather than modules.

CONFIG_DELL_SMBIOS is a visually selectable option again and both
CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/Kconfig            | 11 ++++++++---
 drivers/platform/x86/Makefile           |  6 +++---
 drivers/platform/x86/dell-smbios-base.c | 21 ++++++++++++++++++++-
 drivers/platform/x86/dell-smbios-smm.c  | 25 +++++++++++++------------
 drivers/platform/x86/dell-smbios-wmi.c  | 24 ++++++++++++++----------
 drivers/platform/x86/dell-smbios.h      |  6 ++++++
 6 files changed, 64 insertions(+), 29 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 9a8f964..0c0897e 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -106,10 +106,15 @@ config ASUS_LAPTOP
 	  If you have an ACPI-compatible ASUS laptop, say Y or M here.
 
 config DELL_SMBIOS
-	tristate
+	tristate "Dell SMBIOS driver"
+	---help---
+	This provides support for the Dell SMBIOS calling interface.
+	If you have a Dell computer you should enable this option.
+
+	Be sure to select at least one backend for it to work properly.
 
 config DELL_SMBIOS_WMI
-	tristate "Dell SMBIOS calling interface (WMI implementation)"
+	bool "Dell SMBIOS driver WMI support"
 	depends on ACPI_WMI
 	select DELL_WMI_DESCRIPTOR
 	select DELL_SMBIOS
@@ -122,7 +127,7 @@ config DELL_SMBIOS_WMI
 	it just won't load.
 
 config DELL_SMBIOS_SMM
-	tristate "Dell SMBIOS calling interface (SMM implementation)"
+	bool "Dell SMBIOS driver SMM support"
 	depends on DCDBAS
 	select DELL_SMBIOS
 	---help---
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 940b118..0e242bd 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -13,9 +13,9 @@ obj-$(CONFIG_MSI_LAPTOP)	+= msi-laptop.o
 obj-$(CONFIG_ACPI_CMPC)		+= classmate-laptop.o
 obj-$(CONFIG_COMPAL_LAPTOP)	+= compal-laptop.o
 obj-$(CONFIG_DELL_SMBIOS)	+= dell-smbios.o
-dell-smbios-objs		:= dell-smbios-base.o
-obj-$(CONFIG_DELL_SMBIOS_WMI)	+= dell-smbios-wmi.o
-obj-$(CONFIG_DELL_SMBIOS_SMM)	+= dell-smbios-smm.o
+dell-smbios-objs		:= dell-smbios-base.o	\
+				   dell-smbios-smm.o	\
+				   dell-smbios-wmi.o
 obj-$(CONFIG_DELL_LAPTOP)	+= dell-laptop.o
 obj-$(CONFIG_DELL_WMI)		+= dell-wmi.o
 obj-$(CONFIG_DELL_WMI_DESCRIPTOR)	+= dell-wmi-descriptor.o
diff --git a/drivers/platform/x86/dell-smbios-base.c b/drivers/platform/x86/dell-smbios-base.c
index 4850658..fdb1044 100644
--- a/drivers/platform/x86/dell-smbios-base.c
+++ b/drivers/platform/x86/dell-smbios-base.c
@@ -555,7 +555,7 @@ static void free_group(struct platform_device *pdev)
 static int __init dell_smbios_init(void)
 {
 	const struct dmi_device *valid;
-	int ret;
+	int ret, wmi, smm;
 
 	valid = dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System", NULL);
 	if (!valid) {
@@ -590,8 +590,24 @@ static int __init dell_smbios_init(void)
 	if (ret)
 		goto fail_create_group;
 
+	/* register backends */
+	wmi = init_dell_smbios_wmi();
+	if (wmi)
+		pr_debug("Failed to initialize WMI backend: %d\n", wmi);
+	smm = init_dell_smbios_smm();
+	if (smm)
+		pr_debug("Failed to initialize SMM backend: %d\n", smm);
+	if (wmi && smm) {
+		pr_err("No SMBIOS backends available (wmi: %d, smm: %d)\n",
+			wmi, smm);
+		goto fail_sysfs;
+	}
+
 	return 0;
 
+fail_sysfs:
+	free_group(platform_device);
+
 fail_create_group:
 	platform_device_del(platform_device);
 
@@ -608,6 +624,8 @@ static int __init dell_smbios_init(void)
 
 static void __exit dell_smbios_exit(void)
 {
+	exit_dell_smbios_wmi();
+	exit_dell_smbios_smm();
 	mutex_lock(&smbios_mutex);
 	if (platform_device) {
 		free_group(platform_device);
@@ -624,5 +642,6 @@ module_exit(dell_smbios_exit);
 MODULE_AUTHOR("Matthew Garrett <mjg@redhat.com>");
 MODULE_AUTHOR("Gabriele Mazzotta <gabriele.mzt@gmail.com>");
 MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>");
+MODULE_AUTHOR("Mario Limonciello <mario.limonciello@dell.com>");
 MODULE_DESCRIPTION("Common functions for kernel modules using Dell SMBIOS");
 MODULE_LICENSE("GPL");
diff --git a/drivers/platform/x86/dell-smbios-smm.c b/drivers/platform/x86/dell-smbios-smm.c
index 89f65c4..dc060f3 100644
--- a/drivers/platform/x86/dell-smbios-smm.c
+++ b/drivers/platform/x86/dell-smbios-smm.c
@@ -21,6 +21,7 @@
 #include "../../firmware/dcdbas.h"
 #include "dell-smbios.h"
 
+#ifdef CONFIG_DELL_SMBIOS_SMM
 static int da_command_address;
 static int da_command_code;
 static struct calling_interface_buffer *buffer;
@@ -58,7 +59,7 @@ static const struct dmi_system_id dell_device_table[] __initconst = {
 };
 MODULE_DEVICE_TABLE(dmi, dell_device_table);
 
-static void __init parse_da_table(const struct dmi_header *dm)
+static void parse_da_table(const struct dmi_header *dm)
 {
 	struct calling_interface_structure *table =
 		container_of(dm, struct calling_interface_structure, header);
@@ -73,7 +74,7 @@ static void __init parse_da_table(const struct dmi_header *dm)
 	da_command_code = table->cmdIOCode;
 }
 
-static void __init find_cmd_address(const struct dmi_header *dm, void *dummy)
+static void find_cmd_address(const struct dmi_header *dm, void *dummy)
 {
 	switch (dm->type) {
 	case 0xda: /* Calling interface */
@@ -128,7 +129,7 @@ static bool test_wsmt_enabled(void)
 	return false;
 }
 
-static int __init dell_smbios_smm_init(void)
+int init_dell_smbios_smm(void)
 {
 	int ret;
 	/*
@@ -176,7 +177,7 @@ static int __init dell_smbios_smm_init(void)
 	return ret;
 }
 
-static void __exit dell_smbios_smm_exit(void)
+void exit_dell_smbios_smm(void)
 {
 	if (platform_device) {
 		dell_smbios_unregister_device(&platform_device->dev);
@@ -185,12 +186,12 @@ static void __exit dell_smbios_smm_exit(void)
 	}
 }
 
-subsys_initcall(dell_smbios_smm_init);
-module_exit(dell_smbios_smm_exit);
+#else /* CONFIG_DELL_SMBIOS_SMM */
+int init_dell_smbios_smm(void)
+{
+	return -ENODEV;
+}
 
-MODULE_AUTHOR("Matthew Garrett <mjg@redhat.com>");
-MODULE_AUTHOR("Gabriele Mazzotta <gabriele.mzt@gmail.com>");
-MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>");
-MODULE_AUTHOR("Mario Limonciello <mario.limonciello@dell.com>");
-MODULE_DESCRIPTION("Dell SMBIOS communications over SMI");
-MODULE_LICENSE("GPL");
+void exit_dell_smbios_smm(void)
+{}
+#endif /* CONFIG_DELL_SMBIOS_SMM */
diff --git a/drivers/platform/x86/dell-smbios-wmi.c b/drivers/platform/x86/dell-smbios-wmi.c
index 609557a..c2264e4 100644
--- a/drivers/platform/x86/dell-smbios-wmi.c
+++ b/drivers/platform/x86/dell-smbios-wmi.c
@@ -18,6 +18,7 @@
 #include "dell-smbios.h"
 #include "dell-wmi-descriptor.h"
 
+#ifdef CONFIG_DELL_SMBIOS_WMI
 static DEFINE_MUTEX(call_mutex);
 static DEFINE_MUTEX(list_mutex);
 static int wmi_supported;
@@ -228,7 +229,7 @@ static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
 	{ },
 };
 
-static void __init parse_b1_table(const struct dmi_header *dm)
+static void parse_b1_table(const struct dmi_header *dm)
 {
 	struct misc_bios_flags_structure *flags =
 	container_of(dm, struct misc_bios_flags_structure, header);
@@ -242,7 +243,7 @@ static void __init parse_b1_table(const struct dmi_header *dm)
 		wmi_supported = 1;
 }
 
-static void __init find_b1(const struct dmi_header *dm, void *dummy)
+static void find_b1(const struct dmi_header *dm, void *dummy)
 {
 	switch (dm->type) {
 	case 0xb1: /* misc bios flags */
@@ -261,7 +262,7 @@ static struct wmi_driver dell_smbios_wmi_driver = {
 	.filter_callback = dell_smbios_wmi_filter,
 };
 
-static int __init init_dell_smbios_wmi(void)
+int init_dell_smbios_wmi(void)
 {
 	dmi_walk(find_b1, NULL);
 
@@ -271,15 +272,18 @@ static int __init init_dell_smbios_wmi(void)
 	return wmi_driver_register(&dell_smbios_wmi_driver);
 }
 
-static void __exit exit_dell_smbios_wmi(void)
+void exit_dell_smbios_wmi(void)
 {
 	wmi_driver_unregister(&dell_smbios_wmi_driver);
 }
 
-module_init(init_dell_smbios_wmi);
-module_exit(exit_dell_smbios_wmi);
-
 MODULE_ALIAS("wmi:" DELL_WMI_SMBIOS_GUID);
-MODULE_AUTHOR("Mario Limonciello <mario.limonciello@dell.com>");
-MODULE_DESCRIPTION("Dell SMBIOS communications over WMI");
-MODULE_LICENSE("GPL");
+#else /* CONFIG_DELL_SMBIOS_WMI */
+int init_dell_smbios_wmi(void)
+{
+	return -ENODEV;
+}
+
+void exit_dell_smbios_wmi(void)
+{}
+#endif /* CONFIG_DELL_SMBIOS_WMI */
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 138d478..386bfa2 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -75,4 +75,10 @@ int dell_laptop_register_notifier(struct notifier_block *nb);
 int dell_laptop_unregister_notifier(struct notifier_block *nb);
 void dell_laptop_call_notifier(unsigned long action, void *data);
 
+/* for the supported backends */
+int init_dell_smbios_wmi(void);
+void exit_dell_smbios_wmi(void);
+int init_dell_smbios_smm(void);
+void exit_dell_smbios_smm(void);
+
 #endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] platform/x86: dell-smbios: Correct some style warnings
  2018-02-27 15:56 [PATCH 1/3] platform/x86: dell-smbios: Correct some style warnings Mario Limonciello
  2018-02-27 15:56 ` [PATCH 2/3] platform/x86: dell-smbios: Rename dell-smbios source to dell-smbios-base Mario Limonciello
  2018-02-27 15:56 ` [PATCH 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together Mario Limonciello
@ 2018-02-27 16:04 ` Andy Shevchenko
  2018-02-27 16:15   ` Mario.Limonciello
  2 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2018-02-27 16:04 UTC (permalink / raw)
  To: Mario Limonciello; +Cc: Darren Hart, Pali Rohár, LKML, Platform Driver

On Tue, Feb 27, 2018 at 5:56 PM, Mario Limonciello
<mario.limonciello@dell.com> wrote:

>         /* 4 bytes of table header, plus 7 bytes of Dell header, plus at least
> -          6 bytes of entry */
> +        * 6 bytes of entry
> +        */

I would prefer not to mix with net style, since you are fixing this, perhaps

/*
 * 4 ...
 * 6 ...
 */

?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together
  2018-02-27 15:56 ` [PATCH 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together Mario Limonciello
@ 2018-02-27 16:08   ` Andy Shevchenko
  2018-02-27 16:13     ` Mario.Limonciello
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2018-02-27 16:08 UTC (permalink / raw)
  To: Mario Limonciello; +Cc: Darren Hart, Pali Rohár, LKML, Platform Driver

On Tue, Feb 27, 2018 at 5:56 PM, Mario Limonciello
<mario.limonciello@dell.com> wrote:
> Some race conditions were raised due to dell-smbios and its backends
> not being ready by the time that a consumer would call one of the
> exported methods.
>
> To avoid this problem, guarantee that all initialization has been
> done by linking them all together and running init for them all.
>
> As part of this change the Kconfig needs to be adjusted so that
> CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> rather than modules.
>
> CONFIG_DELL_SMBIOS is a visually selectable option again and both
> CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.


> --- a/drivers/platform/x86/dell-smbios-smm.c
> +++ b/drivers/platform/x86/dell-smbios-smm.c

> +#ifdef CONFIG_DELL_SMBIOS_SMM

> +#else /* CONFIG_DELL_SMBIOS_SMM */
> +int init_dell_smbios_smm(void)
> +{
> +       return -ENODEV;
> +}

> +void exit_dell_smbios_smm(void)
> +{}
> +#endif /* CONFIG_DELL_SMBIOS_SMM */

> --- a/drivers/platform/x86/dell-smbios-wmi.c
> +++ b/drivers/platform/x86/dell-smbios-wmi.c

> +#ifdef CONFIG_DELL_SMBIOS_WMI

> +#else /* CONFIG_DELL_SMBIOS_WMI */
> +int init_dell_smbios_wmi(void)
> +{
> +       return -ENODEV;
> +}
> +
> +void exit_dell_smbios_wmi(void)
> +{}
> +#endif /* CONFIG_DELL_SMBIOS_WMI */

It's rather strange to see above in *.c and not in *.h where it's
usually supposed to be...

> --- a/drivers/platform/x86/dell-smbios.h
> +++ b/drivers/platform/x86/dell-smbios.h
> @@ -75,4 +75,10 @@ int dell_laptop_register_notifier(struct notifier_block *nb);
>  int dell_laptop_unregister_notifier(struct notifier_block *nb);
>  void dell_laptop_call_notifier(unsigned long action, void *data);
>
> +/* for the supported backends */
> +int init_dell_smbios_wmi(void);
> +void exit_dell_smbios_wmi(void);
> +int init_dell_smbios_smm(void);
> +void exit_dell_smbios_smm(void);
> +

...here...

>  #endif

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together
  2018-02-27 16:08   ` Andy Shevchenko
@ 2018-02-27 16:13     ` Mario.Limonciello
  2018-02-27 16:16       ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Mario.Limonciello @ 2018-02-27 16:13 UTC (permalink / raw)
  To: andy.shevchenko; +Cc: dvhart, pali.rohar, linux-kernel, platform-driver-x86

> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> Sent: Tuesday, February 27, 2018 10:09 AM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Darren Hart <dvhart@infradead.org>; Pali Rohár <pali.rohar@gmail.com>;
> LKML <linux-kernel@vger.kernel.org>; Platform Driver <platform-driver-
> x86@vger.kernel.org>
> Subject: Re: [PATCH 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules
> together
> 
> On Tue, Feb 27, 2018 at 5:56 PM, Mario Limonciello
> <mario.limonciello@dell.com> wrote:
> > Some race conditions were raised due to dell-smbios and its backends
> > not being ready by the time that a consumer would call one of the
> > exported methods.
> >
> > To avoid this problem, guarantee that all initialization has been
> > done by linking them all together and running init for them all.
> >
> > As part of this change the Kconfig needs to be adjusted so that
> > CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> > rather than modules.
> >
> > CONFIG_DELL_SMBIOS is a visually selectable option again and both
> > CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> 
> 
> > --- a/drivers/platform/x86/dell-smbios-smm.c
> > +++ b/drivers/platform/x86/dell-smbios-smm.c
> 
> > +#ifdef CONFIG_DELL_SMBIOS_SMM
> 
> > +#else /* CONFIG_DELL_SMBIOS_SMM */
> > +int init_dell_smbios_smm(void)
> > +{
> > +       return -ENODEV;
> > +}
> 
> > +void exit_dell_smbios_smm(void)
> > +{}
> > +#endif /* CONFIG_DELL_SMBIOS_SMM */
> 
> > --- a/drivers/platform/x86/dell-smbios-wmi.c
> > +++ b/drivers/platform/x86/dell-smbios-wmi.c
> 
> > +#ifdef CONFIG_DELL_SMBIOS_WMI
> 
> > +#else /* CONFIG_DELL_SMBIOS_WMI */
> > +int init_dell_smbios_wmi(void)
> > +{
> > +       return -ENODEV;
> > +}
> > +
> > +void exit_dell_smbios_wmi(void)
> > +{}
> > +#endif /* CONFIG_DELL_SMBIOS_WMI */
> 
> It's rather strange to see above in *.c and not in *.h where it's
> usually supposed to be...

Thanks that was my original thought too, but I was following
the style that I had seen in drivers/input/mouse which is 
used for psmouse/synaptics.
Eg: https://github.com/torvalds/linux/blob/master/drivers/input/mouse/synaptics.c#L1692

> 
> > --- a/drivers/platform/x86/dell-smbios.h
> > +++ b/drivers/platform/x86/dell-smbios.h
> > @@ -75,4 +75,10 @@ int dell_laptop_register_notifier(struct notifier_block
> *nb);
> >  int dell_laptop_unregister_notifier(struct notifier_block *nb);
> >  void dell_laptop_call_notifier(unsigned long action, void *data);
> >
> > +/* for the supported backends */
> > +int init_dell_smbios_wmi(void);
> > +void exit_dell_smbios_wmi(void);
> > +int init_dell_smbios_smm(void);
> > +void exit_dell_smbios_smm(void);
> > +
> 
> ...here...

So do you mean you would rather see inline functions in header
file for the #else case then?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH 1/3] platform/x86: dell-smbios: Correct some style warnings
  2018-02-27 16:04 ` [PATCH 1/3] platform/x86: dell-smbios: Correct some style warnings Andy Shevchenko
@ 2018-02-27 16:15   ` Mario.Limonciello
  0 siblings, 0 replies; 8+ messages in thread
From: Mario.Limonciello @ 2018-02-27 16:15 UTC (permalink / raw)
  To: andy.shevchenko; +Cc: dvhart, pali.rohar, linux-kernel, platform-driver-x86

> -----Original Message-----
> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86-
> owner@vger.kernel.org] On Behalf Of Andy Shevchenko
> Sent: Tuesday, February 27, 2018 10:04 AM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Darren Hart <dvhart@infradead.org>; Pali Rohár <pali.rohar@gmail.com>;
> LKML <linux-kernel@vger.kernel.org>; Platform Driver <platform-driver-
> x86@vger.kernel.org>
> Subject: Re: [PATCH 1/3] platform/x86: dell-smbios: Correct some style warnings
> 
> On Tue, Feb 27, 2018 at 5:56 PM, Mario Limonciello
> <mario.limonciello@dell.com> wrote:
> 
> >         /* 4 bytes of table header, plus 7 bytes of Dell header, plus at least
> > -          6 bytes of entry */
> > +        * 6 bytes of entry
> > +        */
> 
> I would prefer not to mix with net style, since you are fixing this, perhaps
> 
> /*
>  * 4 ...
>  * 6 ...
>  */
> 
> ?
> 
Thanks, will adjust again when I resubmit with changes for 3/3.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together
  2018-02-27 16:13     ` Mario.Limonciello
@ 2018-02-27 16:16       ` Andy Shevchenko
  0 siblings, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2018-02-27 16:16 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Darren Hart, Pali Rohár, Linux Kernel Mailing List, Platform Driver

On Tue, Feb 27, 2018 at 6:13 PM,  <Mario.Limonciello@dell.com> wrote:

>> > --- a/drivers/platform/x86/dell-smbios.h
>> > +++ b/drivers/platform/x86/dell-smbios.h
>> > @@ -75,4 +75,10 @@ int dell_laptop_register_notifier(struct notifier_block
>> *nb);
>> >  int dell_laptop_unregister_notifier(struct notifier_block *nb);
>> >  void dell_laptop_call_notifier(unsigned long action, void *data);
>> >
>> > +/* for the supported backends */
>> > +int init_dell_smbios_wmi(void);
>> > +void exit_dell_smbios_wmi(void);
>> > +int init_dell_smbios_smm(void);
>> > +void exit_dell_smbios_smm(void);
>> > +
>>
>> ...here...
>
> So do you mean you would rather see inline functions in header
> file for the #else case then?

At least is sounds as a standard practice to me.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-02-27 16:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-27 15:56 [PATCH 1/3] platform/x86: dell-smbios: Correct some style warnings Mario Limonciello
2018-02-27 15:56 ` [PATCH 2/3] platform/x86: dell-smbios: Rename dell-smbios source to dell-smbios-base Mario Limonciello
2018-02-27 15:56 ` [PATCH 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together Mario Limonciello
2018-02-27 16:08   ` Andy Shevchenko
2018-02-27 16:13     ` Mario.Limonciello
2018-02-27 16:16       ` Andy Shevchenko
2018-02-27 16:04 ` [PATCH 1/3] platform/x86: dell-smbios: Correct some style warnings Andy Shevchenko
2018-02-27 16:15   ` Mario.Limonciello

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).