All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/16] Common Dell SMBIOS API
@ 2016-01-22 14:27 Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 01/16] dell-laptop: extract SMBIOS-related code to a separate module Michał Kępień
                   ` (16 more replies)
  0 siblings, 17 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

The Linux kernel tree currently contains two Dell laptop-related drivers
issuing SMBIOS requests in different ways (dell-laptop in
drivers/platform/x86 and dell-led in drivers/led).  As an upcoming patch
series for the dell-wmi driver (also in drivers/platform/x86) will
change it so that it also performs SMBIOS requests, I took the
opportunity to unify the API used for issuing Dell SMBIOS requests
throughout the kernel before any further code duplication happens.
Credit for suggesting this goes to Pali Rohár.

This patch series is primarily intended for the platform-x86 subsystem,
with only 2 final patches touching the LED subsystem.

The first patch generates a lot of checkpatch warnings, but these are
also raised for the original code and I decided that not changing the
code while moving around large quantities of it is critical for
reviewability.

Changes from v1:

  - the SMBIOS buffer is no longer exported from the new dell-smbios
    module (patch 03 from v1 has been dropped, patches 08-09 from v2
    implement this change),

  - dell_smbios_send_request() is changed so that it doesn't return an
    SMBIOS buffer (patch 07 from v2),

  - slight patch reordering.

Note:

    In this series (both v1 and v2) I tried to stick to the overall
    concept used in dell-laptop, but in the v1 thread me and Pali also
    briefly discussed his alternative ideas [1][2] as to what this API
    could look like, so feel free to suggest a different approach.

[1] http://www.spinics.net/lists/platform-driver-x86/msg08260.html
[2] http://www.spinics.net/lists/platform-driver-x86/msg08268.html

 drivers/leds/Kconfig               |    1 +
 drivers/leds/dell-led.c            |  126 ++-----------
 drivers/platform/x86/Kconfig       |   12 +-
 drivers/platform/x86/Makefile      |    1 +
 drivers/platform/x86/dell-laptop.c |  340 ++++++++++--------------------------
 drivers/platform/x86/dell-smbios.c |  176 +++++++++++++++++++
 drivers/platform/x86/dell-smbios.h |   44 +++++
 7 files changed, 344 insertions(+), 356 deletions(-)
 create mode 100644 drivers/platform/x86/dell-smbios.c
 create mode 100644 drivers/platform/x86/dell-smbios.h

-- 
1.7.10.4

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

* [PATCH v2 01/16] dell-laptop: extract SMBIOS-related code to a separate module
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-24  2:16   ` kbuild test robot
  2016-01-22 14:27 ` [PATCH v2 02/16] dell-smbios: rename get_buffer() to dell_smbios_get_buffer() Michał Kępień
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

Extract SMBIOS-related code from dell-laptop to a new kernel module,
dell-smbios.  The static specifier is removed from exported symbols,
otherwise code is just moved around.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/Kconfig       |   12 ++-
 drivers/platform/x86/Makefile      |    1 +
 drivers/platform/x86/dell-laptop.c |  163 +-----------------------------
 drivers/platform/x86/dell-smbios.c |  193 ++++++++++++++++++++++++++++++++++++
 drivers/platform/x86/dell-smbios.h |   50 ++++++++++
 5 files changed, 256 insertions(+), 163 deletions(-)
 create mode 100644 drivers/platform/x86/dell-smbios.c
 create mode 100644 drivers/platform/x86/dell-smbios.h

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 69f93a5..3e4d9c3 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -91,10 +91,20 @@ config ASUS_LAPTOP
 
 	  If you have an ACPI-compatible ASUS laptop, say Y or M here.
 
+config DELL_SMBIOS
+	tristate "Dell SMBIOS Support"
+	depends on DCDBAS
+	default n
+	---help---
+	This module provides common functions for kernel modules using
+	Dell SMBIOS.
+
+	If you have a Dell laptop, say Y or M here.
+
 config DELL_LAPTOP
 	tristate "Dell Laptop Extras"
 	depends on X86
-	depends on DCDBAS
+	depends on DELL_SMBIOS
 	depends on BACKLIGHT_CLASS_DEVICE
 	depends on ACPI_VIDEO || ACPI_VIDEO = n
 	depends on RFKILL || RFKILL = n
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 40574e7..448443c 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_EEEPC_WMI)		+= eeepc-wmi.o
 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
 obj-$(CONFIG_DELL_LAPTOP)	+= dell-laptop.o
 obj-$(CONFIG_DELL_WMI)		+= dell-wmi.o
 obj-$(CONFIG_DELL_WMI_AIO)	+= dell-wmi-aio.o
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index aaeeae8..d45d356 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -28,12 +28,11 @@
 #include <linux/acpi.h>
 #include <linux/mm.h>
 #include <linux/i8042.h>
-#include <linux/slab.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
 #include <acpi/video.h>
-#include "../../firmware/dcdbas.h"
 #include "dell-rbtn.h"
+#include "dell-smbios.h"
 
 #define BRIGHTNESS_TOKEN 0x7d
 #define KBD_LED_OFF_TOKEN 0x01E1
@@ -44,33 +43,6 @@
 #define KBD_LED_AUTO_75_TOKEN 0x02EC
 #define KBD_LED_AUTO_100_TOKEN 0x02F6
 
-/* This structure will be modified by the firmware when we enter
- * system management mode, hence the volatiles */
-
-struct calling_interface_buffer {
-	u16 class;
-	u16 select;
-	volatile u32 input[4];
-	volatile u32 output[4];
-} __packed;
-
-struct calling_interface_token {
-	u16 tokenID;
-	u16 location;
-	union {
-		u16 value;
-		u16 stringlength;
-	};
-};
-
-struct calling_interface_structure {
-	struct dmi_header header;
-	u16 cmdIOAddress;
-	u8 cmdIOCode;
-	u32 supportedCmds;
-	struct calling_interface_token tokens[];
-} __packed;
-
 struct quirk_entry {
 	u8 touchpad_led;
 
@@ -103,11 +75,6 @@ static struct quirk_entry quirk_dell_xps13_9333 = {
 	.kbd_timeouts = { 0, 5, 15, 60, 5 * 60, 15 * 60, -1 },
 };
 
-static int da_command_address;
-static int da_command_code;
-static int da_num_tokens;
-static struct calling_interface_token *da_tokens;
-
 static struct platform_driver platform_driver = {
 	.driver = {
 		.name = "dell-laptop",
@@ -306,112 +273,6 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
 	{ }
 };
 
-static struct calling_interface_buffer *buffer;
-static DEFINE_MUTEX(buffer_mutex);
-
-static void clear_buffer(void)
-{
-	memset(buffer, 0, sizeof(struct calling_interface_buffer));
-}
-
-static void get_buffer(void)
-{
-	mutex_lock(&buffer_mutex);
-	clear_buffer();
-}
-
-static void release_buffer(void)
-{
-	mutex_unlock(&buffer_mutex);
-}
-
-static void __init parse_da_table(const struct dmi_header *dm)
-{
-	/* Final token is a terminator, so we don't want to copy it */
-	int tokens = (dm->length-11)/sizeof(struct calling_interface_token)-1;
-	struct calling_interface_token *new_da_tokens;
-	struct calling_interface_structure *table =
-		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 */
-
-	if (dm->length < 17)
-		return;
-
-	da_command_address = table->cmdIOAddress;
-	da_command_code = table->cmdIOCode;
-
-	new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
-				 sizeof(struct calling_interface_token),
-				 GFP_KERNEL);
-
-	if (!new_da_tokens)
-		return;
-	da_tokens = new_da_tokens;
-
-	memcpy(da_tokens+da_num_tokens, table->tokens,
-	       sizeof(struct calling_interface_token) * tokens);
-
-	da_num_tokens += tokens;
-}
-
-static void __init find_tokens(const struct dmi_header *dm, void *dummy)
-{
-	switch (dm->type) {
-	case 0xd4: /* Indexed IO */
-	case 0xd5: /* Protected Area Type 1 */
-	case 0xd6: /* Protected Area Type 2 */
-		break;
-	case 0xda: /* Calling interface */
-		parse_da_table(dm);
-		break;
-	}
-}
-
-static int find_token_id(int tokenid)
-{
-	int i;
-
-	for (i = 0; i < da_num_tokens; i++) {
-		if (da_tokens[i].tokenID == tokenid)
-			return i;
-	}
-
-	return -1;
-}
-
-static int find_token_location(int tokenid)
-{
-	int id;
-
-	id = find_token_id(tokenid);
-	if (id == -1)
-		return -1;
-
-	return da_tokens[id].location;
-}
-
-static struct calling_interface_buffer *
-dell_send_request(struct calling_interface_buffer *buffer, int class,
-		  int select)
-{
-	struct smi_cmd command;
-
-	command.magic = SMI_CMD_MAGIC;
-	command.command_address = da_command_address;
-	command.command_code = da_command_code;
-	command.ebx = virt_to_phys(buffer);
-	command.ecx = 0x42534931;
-
-	buffer->class = class;
-	buffer->select = select;
-
-	dcdbas_smi_request(&command);
-
-	return buffer;
-}
-
 static inline int dell_smi_error(int value)
 {
 	switch (value) {
@@ -2122,13 +1983,6 @@ static int __init dell_init(void)
 	/* find if this machine support other functions */
 	dmi_check_system(dell_quirks);
 
-	dmi_walk(find_tokens, NULL);
-
-	if (!da_tokens)  {
-		pr_info("Unable to find dmi tokens\n");
-		return -ENODEV;
-	}
-
 	ret = platform_driver_register(&platform_driver);
 	if (ret)
 		goto fail_platform_driver;
@@ -2141,16 +1995,6 @@ static int __init dell_init(void)
 	if (ret)
 		goto fail_platform_device2;
 
-	/*
-	 * Allocate buffer below 4GB for SMI data--only 32-bit physical addr
-	 * is passed to SMI handler.
-	 */
-	buffer = (void *)__get_free_page(GFP_KERNEL | GFP_DMA32);
-	if (!buffer) {
-		ret = -ENOMEM;
-		goto fail_buffer;
-	}
-
 	ret = dell_setup_rfkill();
 
 	if (ret) {
@@ -2208,15 +2052,12 @@ static int __init dell_init(void)
 fail_backlight:
 	dell_cleanup_rfkill();
 fail_rfkill:
-	free_page((unsigned long)buffer);
-fail_buffer:
 	platform_device_del(platform_device);
 fail_platform_device2:
 	platform_device_put(platform_device);
 fail_platform_device1:
 	platform_driver_unregister(&platform_driver);
 fail_platform_driver:
-	kfree(da_tokens);
 	return ret;
 }
 
@@ -2232,8 +2073,6 @@ static void __exit dell_exit(void)
 		platform_device_unregister(platform_device);
 		platform_driver_unregister(&platform_driver);
 	}
-	kfree(da_tokens);
-	free_page((unsigned long)buffer);
 }
 
 /* dell-rbtn.c driver export functions which will not work correctly (and could
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
new file mode 100644
index 0000000..e08cf78
--- /dev/null
+++ b/drivers/platform/x86/dell-smbios.c
@@ -0,0 +1,193 @@
+/*
+ *  Common functions for kernel modules using Dell SMBIOS
+ *
+ *  Copyright (c) Red Hat <mjg@redhat.com>
+ *  Copyright (c) 2014 Gabriele Mazzotta <gabriele.mzt@gmail.com>
+ *  Copyright (c) 2014 Pali Rohár <pali.rohar@gmail.com>
+ *
+ *  Based on documentation in the libsmbios package:
+ *  Copyright (C) 2005-2014 Dell Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/dmi.h>
+#include <linux/gfp.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include "../../firmware/dcdbas.h"
+#include "dell-smbios.h"
+
+struct calling_interface_structure {
+	struct dmi_header header;
+	u16 cmdIOAddress;
+	u8 cmdIOCode;
+	u32 supportedCmds;
+	struct calling_interface_token tokens[];
+} __packed;
+
+struct calling_interface_buffer *buffer;
+EXPORT_SYMBOL_GPL(buffer);
+static DEFINE_MUTEX(buffer_mutex);
+
+static int da_command_address;
+static int da_command_code;
+static int da_num_tokens;
+struct calling_interface_token *da_tokens;
+EXPORT_SYMBOL_GPL(da_tokens);
+
+void get_buffer(void)
+{
+	mutex_lock(&buffer_mutex);
+	clear_buffer();
+}
+EXPORT_SYMBOL_GPL(get_buffer);
+
+void clear_buffer(void)
+{
+	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+}
+EXPORT_SYMBOL_GPL(clear_buffer);
+
+void release_buffer(void)
+{
+	mutex_unlock(&buffer_mutex);
+}
+EXPORT_SYMBOL_GPL(release_buffer);
+
+struct calling_interface_buffer *
+dell_send_request(struct calling_interface_buffer *buffer,
+		  int class, int select)
+{
+	struct smi_cmd command;
+
+	command.magic = SMI_CMD_MAGIC;
+	command.command_address = da_command_address;
+	command.command_code = da_command_code;
+	command.ebx = virt_to_phys(buffer);
+	command.ecx = 0x42534931;
+
+	buffer->class = class;
+	buffer->select = select;
+
+	dcdbas_smi_request(&command);
+
+	return buffer;
+}
+EXPORT_SYMBOL_GPL(dell_send_request);
+
+int find_token_id(int tokenid)
+{
+	int i;
+
+	for (i = 0; i < da_num_tokens; i++) {
+		if (da_tokens[i].tokenID == tokenid)
+			return i;
+	}
+
+	return -1;
+}
+EXPORT_SYMBOL_GPL(find_token_id);
+
+int find_token_location(int tokenid)
+{
+	int id;
+
+	id = find_token_id(tokenid);
+	if (id == -1)
+		return -1;
+
+	return da_tokens[id].location;
+}
+EXPORT_SYMBOL_GPL(find_token_location);
+
+static void __init parse_da_table(const struct dmi_header *dm)
+{
+	/* Final token is a terminator, so we don't want to copy it */
+	int tokens = (dm->length-11)/sizeof(struct calling_interface_token)-1;
+	struct calling_interface_token *new_da_tokens;
+	struct calling_interface_structure *table =
+		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 */
+
+	if (dm->length < 17)
+		return;
+
+	da_command_address = table->cmdIOAddress;
+	da_command_code = table->cmdIOCode;
+
+	new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
+				 sizeof(struct calling_interface_token),
+				 GFP_KERNEL);
+
+	if (!new_da_tokens)
+		return;
+	da_tokens = new_da_tokens;
+
+	memcpy(da_tokens+da_num_tokens, table->tokens,
+	       sizeof(struct calling_interface_token) * tokens);
+
+	da_num_tokens += tokens;
+}
+
+static void __init find_tokens(const struct dmi_header *dm, void *dummy)
+{
+	switch (dm->type) {
+	case 0xd4: /* Indexed IO */
+	case 0xd5: /* Protected Area Type 1 */
+	case 0xd6: /* Protected Area Type 2 */
+		break;
+	case 0xda: /* Calling interface */
+		parse_da_table(dm);
+		break;
+	}
+}
+
+static int __init dell_smbios_init(void)
+{
+	int ret;
+
+	dmi_walk(find_tokens, NULL);
+
+	if (!da_tokens)  {
+		pr_info("Unable to find dmi tokens\n");
+		return -ENODEV;
+	}
+
+	/*
+	 * Allocate buffer below 4GB for SMI data--only 32-bit physical addr
+	 * is passed to SMI handler.
+	 */
+	buffer = (void *)__get_free_page(GFP_KERNEL | GFP_DMA32);
+	if (!buffer) {
+		ret = -ENOMEM;
+		goto fail_buffer;
+	}
+
+	return 0;
+
+fail_buffer:
+	kfree(da_tokens);
+	return ret;
+}
+
+static void __exit dell_smbios_exit(void)
+{
+	kfree(da_tokens);
+	free_page((unsigned long)buffer);
+}
+
+subsys_initcall(dell_smbios_init);
+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_DESCRIPTION("Common functions for kernel modules using Dell SMBIOS");
+MODULE_LICENSE("GPL");
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
new file mode 100644
index 0000000..3bc9080
--- /dev/null
+++ b/drivers/platform/x86/dell-smbios.h
@@ -0,0 +1,50 @@
+/*
+ *  Common functions for kernel modules using Dell SMBIOS
+ *
+ *  Copyright (c) Red Hat <mjg@redhat.com>
+ *  Copyright (c) 2014 Gabriele Mazzotta <gabriele.mzt@gmail.com>
+ *  Copyright (c) 2014 Pali Rohár <pali.rohar@gmail.com>
+ *
+ *  Based on documentation in the libsmbios package:
+ *  Copyright (C) 2005-2014 Dell Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+
+#ifndef _DELL_SMBIOS_H_
+#define _DELL_SMBIOS_H_
+
+/* This structure will be modified by the firmware when we enter
+ * system management mode, hence the volatiles */
+
+struct calling_interface_buffer {
+	u16 class;
+	u16 select;
+	volatile u32 input[4];
+	volatile u32 output[4];
+} __packed;
+
+struct calling_interface_token {
+	u16 tokenID;
+	u16 location;
+	union {
+		u16 value;
+		u16 stringlength;
+	};
+};
+
+extern struct calling_interface_buffer *buffer;
+extern struct calling_interface_token *da_tokens;
+
+void get_buffer(void);
+void clear_buffer(void);
+void release_buffer(void);
+struct calling_interface_buffer *
+dell_send_request(struct calling_interface_buffer *buffer,
+		  int class, int select);
+
+int find_token_id(int tokenid);
+int find_token_location(int tokenid);
+#endif
-- 
1.7.10.4

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

* [PATCH v2 02/16] dell-smbios: rename get_buffer() to dell_smbios_get_buffer()
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 01/16] dell-laptop: extract SMBIOS-related code to a separate module Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 03/16] dell-smbios: rename clear_buffer() to dell_smbios_clear_buffer() Michał Kępień
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

As get_buffer() is exported from the module, it has to be renamed to
something less generic, so add a "dell_smbios_" prefix to the function
name.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-laptop.c |   26 +++++++++++++-------------
 drivers/platform/x86/dell-smbios.c |    4 ++--
 drivers/platform/x86/dell-smbios.h |    2 +-
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index d45d356..575b0df 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -416,7 +416,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 	int status;
 	int ret;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
@@ -479,7 +479,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 	int status;
 	int ret;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
@@ -519,7 +519,7 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 	int status;
 	int ret;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
@@ -617,7 +617,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 	int status;
 	int ret;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
@@ -709,7 +709,7 @@ static int __init dell_setup_rfkill(void)
 	if (!force_rfkill && !whitelisted)
 		return 0;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
@@ -873,7 +873,7 @@ static int dell_send_intensity(struct backlight_device *bd)
 	if (token == -1)
 		return -ENODEV;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 	buffer->input[0] = token;
 	buffer->input[1] = bd->props.brightness;
 
@@ -897,7 +897,7 @@ static int dell_get_intensity(struct backlight_device *bd)
 	if (token == -1)
 		return -ENODEV;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 	buffer->input[0] = token;
 
 	if (power_supply_is_system_supplied() > 0)
@@ -1157,7 +1157,7 @@ static int kbd_get_info(struct kbd_info *info)
 	u8 units;
 	int ret;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 
 	buffer->input[0] = 0x0;
 	dell_send_request(buffer, 4, 11);
@@ -1245,7 +1245,7 @@ static int kbd_get_state(struct kbd_state *state)
 {
 	int ret;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 
 	buffer->input[0] = 0x1;
 	dell_send_request(buffer, 4, 11);
@@ -1276,7 +1276,7 @@ static int kbd_set_state(struct kbd_state *state)
 {
 	int ret;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 	buffer->input[0] = 0x2;
 	buffer->input[1] = BIT(state->mode_bit) & 0xFFFF;
 	buffer->input[1] |= (state->triggers & 0xFF) << 16;
@@ -1323,7 +1323,7 @@ static int kbd_set_token_bit(u8 bit)
 	if (id == -1)
 		return -EINVAL;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 	buffer->input[0] = da_tokens[id].location;
 	buffer->input[1] = da_tokens[id].value;
 	dell_send_request(buffer, 1, 0);
@@ -1346,7 +1346,7 @@ static int kbd_get_token_bit(u8 bit)
 	if (id == -1)
 		return -EINVAL;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 	buffer->input[0] = da_tokens[id].location;
 	dell_send_request(buffer, 0, 0);
 	ret = buffer->output[0];
@@ -2017,7 +2017,7 @@ static int __init dell_init(void)
 
 	token = find_token_location(BRIGHTNESS_TOKEN);
 	if (token != -1) {
-		get_buffer();
+		dell_smbios_get_buffer();
 		buffer->input[0] = token;
 		dell_send_request(buffer, 0, 2);
 		if (buffer->output[0] == 0)
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index e08cf78..f28778b 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -40,12 +40,12 @@ static int da_num_tokens;
 struct calling_interface_token *da_tokens;
 EXPORT_SYMBOL_GPL(da_tokens);
 
-void get_buffer(void)
+void dell_smbios_get_buffer(void)
 {
 	mutex_lock(&buffer_mutex);
 	clear_buffer();
 }
-EXPORT_SYMBOL_GPL(get_buffer);
+EXPORT_SYMBOL_GPL(dell_smbios_get_buffer);
 
 void clear_buffer(void)
 {
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 3bc9080..ace16c1 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -38,7 +38,7 @@ struct calling_interface_token {
 extern struct calling_interface_buffer *buffer;
 extern struct calling_interface_token *da_tokens;
 
-void get_buffer(void);
+void dell_smbios_get_buffer(void);
 void clear_buffer(void);
 void release_buffer(void);
 struct calling_interface_buffer *
-- 
1.7.10.4

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

* [PATCH v2 03/16] dell-smbios: rename clear_buffer() to dell_smbios_clear_buffer()
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 01/16] dell-laptop: extract SMBIOS-related code to a separate module Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 02/16] dell-smbios: rename get_buffer() to dell_smbios_get_buffer() Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 04/16] dell-smbios: rename release_buffer() to dell_smbios_release_buffer() Michał Kępień
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

As clear_buffer() is exported from the module, it has to be renamed to
something less generic, so add a "dell_smbios_" prefix to the function
name.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-laptop.c |   12 ++++++------
 drivers/platform/x86/dell-smbios.c |    6 +++---
 drivers/platform/x86/dell-smbios.h |    2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 575b0df..90d75ce 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -425,7 +425,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 	if (ret != 0)
 		goto out;
 
-	clear_buffer();
+	dell_smbios_clear_buffer();
 
 	buffer->input[0] = 0x2;
 	dell_send_request(buffer, 17, 11);
@@ -438,7 +438,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 	    (status & BIT(0)) && !(status & BIT(16)))
 		disable = 1;
 
-	clear_buffer();
+	dell_smbios_clear_buffer();
 
 	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
 	dell_send_request(buffer, 17, 11);
@@ -456,7 +456,7 @@ static void dell_rfkill_update_sw_state(struct rfkill *rfkill, int radio,
 	if (status & BIT(0)) {
 		/* Has hw-switch, sync sw_state to BIOS */
 		int block = rfkill_blocked(rfkill);
-		clear_buffer();
+		dell_smbios_clear_buffer();
 		buffer->input[0] = (1 | (radio << 8) | (block << 16));
 		dell_send_request(buffer, 17, 11);
 	} else {
@@ -490,7 +490,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 		return;
 	}
 
-	clear_buffer();
+	dell_smbios_clear_buffer();
 
 	buffer->input[0] = 0x2;
 	dell_send_request(buffer, 17, 11);
@@ -525,7 +525,7 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 	ret = buffer->output[0];
 	status = buffer->output[1];
 
-	clear_buffer();
+	dell_smbios_clear_buffer();
 
 	buffer->input[0] = 0x2;
 	dell_send_request(buffer, 17, 11);
@@ -626,7 +626,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 	if (ret != 0)
 		goto out;
 
-	clear_buffer();
+	dell_smbios_clear_buffer();
 
 	buffer->input[0] = 0x2;
 	dell_send_request(buffer, 17, 11);
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index f28778b..8b1a0dd 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -43,15 +43,15 @@ EXPORT_SYMBOL_GPL(da_tokens);
 void dell_smbios_get_buffer(void)
 {
 	mutex_lock(&buffer_mutex);
-	clear_buffer();
+	dell_smbios_clear_buffer();
 }
 EXPORT_SYMBOL_GPL(dell_smbios_get_buffer);
 
-void clear_buffer(void)
+void dell_smbios_clear_buffer(void)
 {
 	memset(buffer, 0, sizeof(struct calling_interface_buffer));
 }
-EXPORT_SYMBOL_GPL(clear_buffer);
+EXPORT_SYMBOL_GPL(dell_smbios_clear_buffer);
 
 void release_buffer(void)
 {
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index ace16c1..c5e9474 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -39,7 +39,7 @@ extern struct calling_interface_buffer *buffer;
 extern struct calling_interface_token *da_tokens;
 
 void dell_smbios_get_buffer(void);
-void clear_buffer(void);
+void dell_smbios_clear_buffer(void);
 void release_buffer(void);
 struct calling_interface_buffer *
 dell_send_request(struct calling_interface_buffer *buffer,
-- 
1.7.10.4

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

* [PATCH v2 04/16] dell-smbios: rename release_buffer() to dell_smbios_release_buffer()
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (2 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 03/16] dell-smbios: rename clear_buffer() to dell_smbios_clear_buffer() Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 05/16] dell-smbios: rename dell_send_request() to dell_smbios_send_request() Michał Kępień
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

As release_buffer() is exported from the module, it has to be renamed to
something less generic, so add a "dell_smbios_" prefix to the function
name.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-laptop.c |   28 ++++++++++++++--------------
 drivers/platform/x86/dell-smbios.c |    4 ++--
 drivers/platform/x86/dell-smbios.h |    2 +-
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 90d75ce..b47dc5a 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -445,7 +445,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 	ret = buffer->output[0];
 
  out:
-	release_buffer();
+	dell_smbios_release_buffer();
 	return dell_smi_error(ret);
 }
 
@@ -486,7 +486,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 	status = buffer->output[1];
 
 	if (ret != 0 || !(status & BIT(0))) {
-		release_buffer();
+		dell_smbios_release_buffer();
 		return;
 	}
 
@@ -497,7 +497,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 	ret = buffer->output[0];
 	hwswitch = buffer->output[1];
 
-	release_buffer();
+	dell_smbios_release_buffer();
 
 	if (ret != 0)
 		return;
@@ -532,7 +532,7 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 	hwswitch_ret = buffer->output[0];
 	hwswitch_state = buffer->output[1];
 
-	release_buffer();
+	dell_smbios_release_buffer();
 
 	seq_printf(s, "return:\t%d\n", ret);
 	seq_printf(s, "status:\t0x%X\n", status);
@@ -650,7 +650,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 	}
 
  out:
-	release_buffer();
+	dell_smbios_release_buffer();
 }
 static DECLARE_DELAYED_WORK(dell_rfkill_work, dell_update_rfkill);
 
@@ -713,7 +713,7 @@ static int __init dell_setup_rfkill(void)
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
-	release_buffer();
+	dell_smbios_release_buffer();
 
 	/* dell wireless info smbios call is not supported */
 	if (ret != 0)
@@ -884,7 +884,7 @@ static int dell_send_intensity(struct backlight_device *bd)
 
 	ret = dell_smi_error(buffer->output[0]);
 
-	release_buffer();
+	dell_smbios_release_buffer();
 	return ret;
 }
 
@@ -910,7 +910,7 @@ static int dell_get_intensity(struct backlight_device *bd)
 	else
 		ret = buffer->output[1];
 
-	release_buffer();
+	dell_smbios_release_buffer();
 	return ret;
 }
 
@@ -1184,7 +1184,7 @@ static int kbd_get_info(struct kbd_info *info)
 		info->days = (buffer->output[3] >> 24) & 0xFF;
 
  out:
-	release_buffer();
+	dell_smbios_release_buffer();
 	return ret;
 }
 
@@ -1268,7 +1268,7 @@ static int kbd_get_state(struct kbd_state *state)
 	state->level = (buffer->output[2] >> 16) & 0xFF;
 
  out:
-	release_buffer();
+	dell_smbios_release_buffer();
 	return ret;
 }
 
@@ -1286,7 +1286,7 @@ static int kbd_set_state(struct kbd_state *state)
 	buffer->input[2] |= (state->level & 0xFF) << 16;
 	dell_send_request(buffer, 4, 11);
 	ret = buffer->output[0];
-	release_buffer();
+	dell_smbios_release_buffer();
 
 	return dell_smi_error(ret);
 }
@@ -1328,7 +1328,7 @@ static int kbd_set_token_bit(u8 bit)
 	buffer->input[1] = da_tokens[id].value;
 	dell_send_request(buffer, 1, 0);
 	ret = buffer->output[0];
-	release_buffer();
+	dell_smbios_release_buffer();
 
 	return dell_smi_error(ret);
 }
@@ -1351,7 +1351,7 @@ static int kbd_get_token_bit(u8 bit)
 	dell_send_request(buffer, 0, 0);
 	ret = buffer->output[0];
 	val = buffer->output[1];
-	release_buffer();
+	dell_smbios_release_buffer();
 
 	if (ret)
 		return dell_smi_error(ret);
@@ -2022,7 +2022,7 @@ static int __init dell_init(void)
 		dell_send_request(buffer, 0, 2);
 		if (buffer->output[0] == 0)
 			max_intensity = buffer->output[3];
-		release_buffer();
+		dell_smbios_release_buffer();
 	}
 
 	if (max_intensity) {
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 8b1a0dd..2607d1c 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -53,11 +53,11 @@ void dell_smbios_clear_buffer(void)
 }
 EXPORT_SYMBOL_GPL(dell_smbios_clear_buffer);
 
-void release_buffer(void)
+void dell_smbios_release_buffer(void)
 {
 	mutex_unlock(&buffer_mutex);
 }
-EXPORT_SYMBOL_GPL(release_buffer);
+EXPORT_SYMBOL_GPL(dell_smbios_release_buffer);
 
 struct calling_interface_buffer *
 dell_send_request(struct calling_interface_buffer *buffer,
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index c5e9474..8558103 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -40,7 +40,7 @@ extern struct calling_interface_token *da_tokens;
 
 void dell_smbios_get_buffer(void);
 void dell_smbios_clear_buffer(void);
-void release_buffer(void);
+void dell_smbios_release_buffer(void);
 struct calling_interface_buffer *
 dell_send_request(struct calling_interface_buffer *buffer,
 		  int class, int select);
-- 
1.7.10.4

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

* [PATCH v2 05/16] dell-smbios: rename dell_send_request() to dell_smbios_send_request()
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (3 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 04/16] dell-smbios: rename release_buffer() to dell_smbios_release_buffer() Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 06/16] dell-smbios: don't pass an SMBIOS buffer " Michał Kępień
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

As dell_send_request() is exported from the module, its prefix should be
consistent with other exported symbols, so change function name to
dell_smbios_send_request().

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-laptop.c |   42 ++++++++++++++++++------------------
 drivers/platform/x86/dell-smbios.c |    6 +++---
 drivers/platform/x86/dell-smbios.h |    4 ++--
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index b47dc5a..9210acf 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -418,7 +418,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 
 	dell_smbios_get_buffer();
 
-	dell_send_request(buffer, 17, 11);
+	dell_smbios_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
 
@@ -428,7 +428,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 	dell_smbios_clear_buffer();
 
 	buffer->input[0] = 0x2;
-	dell_send_request(buffer, 17, 11);
+	dell_smbios_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	hwswitch = buffer->output[1];
 
@@ -441,7 +441,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 	dell_smbios_clear_buffer();
 
 	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
-	dell_send_request(buffer, 17, 11);
+	dell_smbios_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 
  out:
@@ -458,7 +458,7 @@ static void dell_rfkill_update_sw_state(struct rfkill *rfkill, int radio,
 		int block = rfkill_blocked(rfkill);
 		dell_smbios_clear_buffer();
 		buffer->input[0] = (1 | (radio << 8) | (block << 16));
-		dell_send_request(buffer, 17, 11);
+		dell_smbios_send_request(buffer, 17, 11);
 	} else {
 		/* No hw-switch, sync BIOS state to sw_state */
 		rfkill_set_sw_state(rfkill, !!(status & BIT(radio + 16)));
@@ -481,7 +481,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 
 	dell_smbios_get_buffer();
 
-	dell_send_request(buffer, 17, 11);
+	dell_smbios_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
 
@@ -493,7 +493,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 	dell_smbios_clear_buffer();
 
 	buffer->input[0] = 0x2;
-	dell_send_request(buffer, 17, 11);
+	dell_smbios_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	hwswitch = buffer->output[1];
 
@@ -521,14 +521,14 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 
 	dell_smbios_get_buffer();
 
-	dell_send_request(buffer, 17, 11);
+	dell_smbios_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
 
 	dell_smbios_clear_buffer();
 
 	buffer->input[0] = 0x2;
-	dell_send_request(buffer, 17, 11);
+	dell_smbios_send_request(buffer, 17, 11);
 	hwswitch_ret = buffer->output[0];
 	hwswitch_state = buffer->output[1];
 
@@ -619,7 +619,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 
 	dell_smbios_get_buffer();
 
-	dell_send_request(buffer, 17, 11);
+	dell_smbios_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
 
@@ -629,7 +629,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 	dell_smbios_clear_buffer();
 
 	buffer->input[0] = 0x2;
-	dell_send_request(buffer, 17, 11);
+	dell_smbios_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 
 	if (ret == 0 && (status & BIT(0)))
@@ -710,7 +710,7 @@ static int __init dell_setup_rfkill(void)
 		return 0;
 
 	dell_smbios_get_buffer();
-	dell_send_request(buffer, 17, 11);
+	dell_smbios_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
 	dell_smbios_release_buffer();
@@ -878,9 +878,9 @@ static int dell_send_intensity(struct backlight_device *bd)
 	buffer->input[1] = bd->props.brightness;
 
 	if (power_supply_is_system_supplied() > 0)
-		dell_send_request(buffer, 1, 2);
+		dell_smbios_send_request(buffer, 1, 2);
 	else
-		dell_send_request(buffer, 1, 1);
+		dell_smbios_send_request(buffer, 1, 1);
 
 	ret = dell_smi_error(buffer->output[0]);
 
@@ -901,9 +901,9 @@ static int dell_get_intensity(struct backlight_device *bd)
 	buffer->input[0] = token;
 
 	if (power_supply_is_system_supplied() > 0)
-		dell_send_request(buffer, 0, 2);
+		dell_smbios_send_request(buffer, 0, 2);
 	else
-		dell_send_request(buffer, 0, 1);
+		dell_smbios_send_request(buffer, 0, 1);
 
 	if (buffer->output[0])
 		ret = dell_smi_error(buffer->output[0]);
@@ -1160,7 +1160,7 @@ static int kbd_get_info(struct kbd_info *info)
 	dell_smbios_get_buffer();
 
 	buffer->input[0] = 0x0;
-	dell_send_request(buffer, 4, 11);
+	dell_smbios_send_request(buffer, 4, 11);
 	ret = buffer->output[0];
 
 	if (ret) {
@@ -1248,7 +1248,7 @@ static int kbd_get_state(struct kbd_state *state)
 	dell_smbios_get_buffer();
 
 	buffer->input[0] = 0x1;
-	dell_send_request(buffer, 4, 11);
+	dell_smbios_send_request(buffer, 4, 11);
 	ret = buffer->output[0];
 
 	if (ret) {
@@ -1284,7 +1284,7 @@ static int kbd_set_state(struct kbd_state *state)
 	buffer->input[1] |= (state->timeout_unit & 0x3) << 30;
 	buffer->input[2] = state->als_setting & 0xFF;
 	buffer->input[2] |= (state->level & 0xFF) << 16;
-	dell_send_request(buffer, 4, 11);
+	dell_smbios_send_request(buffer, 4, 11);
 	ret = buffer->output[0];
 	dell_smbios_release_buffer();
 
@@ -1326,7 +1326,7 @@ static int kbd_set_token_bit(u8 bit)
 	dell_smbios_get_buffer();
 	buffer->input[0] = da_tokens[id].location;
 	buffer->input[1] = da_tokens[id].value;
-	dell_send_request(buffer, 1, 0);
+	dell_smbios_send_request(buffer, 1, 0);
 	ret = buffer->output[0];
 	dell_smbios_release_buffer();
 
@@ -1348,7 +1348,7 @@ static int kbd_get_token_bit(u8 bit)
 
 	dell_smbios_get_buffer();
 	buffer->input[0] = da_tokens[id].location;
-	dell_send_request(buffer, 0, 0);
+	dell_smbios_send_request(buffer, 0, 0);
 	ret = buffer->output[0];
 	val = buffer->output[1];
 	dell_smbios_release_buffer();
@@ -2019,7 +2019,7 @@ static int __init dell_init(void)
 	if (token != -1) {
 		dell_smbios_get_buffer();
 		buffer->input[0] = token;
-		dell_send_request(buffer, 0, 2);
+		dell_smbios_send_request(buffer, 0, 2);
 		if (buffer->output[0] == 0)
 			max_intensity = buffer->output[3];
 		dell_smbios_release_buffer();
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 2607d1c..a0c1bde 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -60,8 +60,8 @@ void dell_smbios_release_buffer(void)
 EXPORT_SYMBOL_GPL(dell_smbios_release_buffer);
 
 struct calling_interface_buffer *
-dell_send_request(struct calling_interface_buffer *buffer,
-		  int class, int select)
+dell_smbios_send_request(struct calling_interface_buffer *buffer,
+			 int class, int select)
 {
 	struct smi_cmd command;
 
@@ -78,7 +78,7 @@ dell_send_request(struct calling_interface_buffer *buffer,
 
 	return buffer;
 }
-EXPORT_SYMBOL_GPL(dell_send_request);
+EXPORT_SYMBOL_GPL(dell_smbios_send_request);
 
 int find_token_id(int tokenid)
 {
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 8558103..33ed971 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -42,8 +42,8 @@ void dell_smbios_get_buffer(void);
 void dell_smbios_clear_buffer(void);
 void dell_smbios_release_buffer(void);
 struct calling_interface_buffer *
-dell_send_request(struct calling_interface_buffer *buffer,
-		  int class, int select);
+dell_smbios_send_request(struct calling_interface_buffer *buffer,
+			 int class, int select);
 
 int find_token_id(int tokenid);
 int find_token_location(int tokenid);
-- 
1.7.10.4

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

* [PATCH v2 06/16] dell-smbios: don't pass an SMBIOS buffer to dell_smbios_send_request()
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (4 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 05/16] dell-smbios: rename dell_send_request() to dell_smbios_send_request() Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 07/16] dell-smbios: don't return an SMBIOS buffer from dell_smbios_send_request() Michał Kępień
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

Passing an SMBIOS buffer pointer to dell_smbios_send_request() is
redundant as it should always operate on the SMBIOS buffer exported from
the module.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-laptop.c |   42 ++++++++++++++++++------------------
 drivers/platform/x86/dell-smbios.c |    3 +--
 drivers/platform/x86/dell-smbios.h |    3 +--
 3 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 9210acf..0bb2211 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -418,7 +418,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 
 	dell_smbios_get_buffer();
 
-	dell_smbios_send_request(buffer, 17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
 
@@ -428,7 +428,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 	dell_smbios_clear_buffer();
 
 	buffer->input[0] = 0x2;
-	dell_smbios_send_request(buffer, 17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
 	hwswitch = buffer->output[1];
 
@@ -441,7 +441,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 	dell_smbios_clear_buffer();
 
 	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
-	dell_smbios_send_request(buffer, 17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
 
  out:
@@ -458,7 +458,7 @@ static void dell_rfkill_update_sw_state(struct rfkill *rfkill, int radio,
 		int block = rfkill_blocked(rfkill);
 		dell_smbios_clear_buffer();
 		buffer->input[0] = (1 | (radio << 8) | (block << 16));
-		dell_smbios_send_request(buffer, 17, 11);
+		dell_smbios_send_request(17, 11);
 	} else {
 		/* No hw-switch, sync BIOS state to sw_state */
 		rfkill_set_sw_state(rfkill, !!(status & BIT(radio + 16)));
@@ -481,7 +481,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 
 	dell_smbios_get_buffer();
 
-	dell_smbios_send_request(buffer, 17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
 
@@ -493,7 +493,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 	dell_smbios_clear_buffer();
 
 	buffer->input[0] = 0x2;
-	dell_smbios_send_request(buffer, 17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
 	hwswitch = buffer->output[1];
 
@@ -521,14 +521,14 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 
 	dell_smbios_get_buffer();
 
-	dell_smbios_send_request(buffer, 17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
 
 	dell_smbios_clear_buffer();
 
 	buffer->input[0] = 0x2;
-	dell_smbios_send_request(buffer, 17, 11);
+	dell_smbios_send_request(17, 11);
 	hwswitch_ret = buffer->output[0];
 	hwswitch_state = buffer->output[1];
 
@@ -619,7 +619,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 
 	dell_smbios_get_buffer();
 
-	dell_smbios_send_request(buffer, 17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
 
@@ -629,7 +629,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 	dell_smbios_clear_buffer();
 
 	buffer->input[0] = 0x2;
-	dell_smbios_send_request(buffer, 17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
 
 	if (ret == 0 && (status & BIT(0)))
@@ -710,7 +710,7 @@ static int __init dell_setup_rfkill(void)
 		return 0;
 
 	dell_smbios_get_buffer();
-	dell_smbios_send_request(buffer, 17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
 	dell_smbios_release_buffer();
@@ -878,9 +878,9 @@ static int dell_send_intensity(struct backlight_device *bd)
 	buffer->input[1] = bd->props.brightness;
 
 	if (power_supply_is_system_supplied() > 0)
-		dell_smbios_send_request(buffer, 1, 2);
+		dell_smbios_send_request(1, 2);
 	else
-		dell_smbios_send_request(buffer, 1, 1);
+		dell_smbios_send_request(1, 1);
 
 	ret = dell_smi_error(buffer->output[0]);
 
@@ -901,9 +901,9 @@ static int dell_get_intensity(struct backlight_device *bd)
 	buffer->input[0] = token;
 
 	if (power_supply_is_system_supplied() > 0)
-		dell_smbios_send_request(buffer, 0, 2);
+		dell_smbios_send_request(0, 2);
 	else
-		dell_smbios_send_request(buffer, 0, 1);
+		dell_smbios_send_request(0, 1);
 
 	if (buffer->output[0])
 		ret = dell_smi_error(buffer->output[0]);
@@ -1160,7 +1160,7 @@ static int kbd_get_info(struct kbd_info *info)
 	dell_smbios_get_buffer();
 
 	buffer->input[0] = 0x0;
-	dell_smbios_send_request(buffer, 4, 11);
+	dell_smbios_send_request(4, 11);
 	ret = buffer->output[0];
 
 	if (ret) {
@@ -1248,7 +1248,7 @@ static int kbd_get_state(struct kbd_state *state)
 	dell_smbios_get_buffer();
 
 	buffer->input[0] = 0x1;
-	dell_smbios_send_request(buffer, 4, 11);
+	dell_smbios_send_request(4, 11);
 	ret = buffer->output[0];
 
 	if (ret) {
@@ -1284,7 +1284,7 @@ static int kbd_set_state(struct kbd_state *state)
 	buffer->input[1] |= (state->timeout_unit & 0x3) << 30;
 	buffer->input[2] = state->als_setting & 0xFF;
 	buffer->input[2] |= (state->level & 0xFF) << 16;
-	dell_smbios_send_request(buffer, 4, 11);
+	dell_smbios_send_request(4, 11);
 	ret = buffer->output[0];
 	dell_smbios_release_buffer();
 
@@ -1326,7 +1326,7 @@ static int kbd_set_token_bit(u8 bit)
 	dell_smbios_get_buffer();
 	buffer->input[0] = da_tokens[id].location;
 	buffer->input[1] = da_tokens[id].value;
-	dell_smbios_send_request(buffer, 1, 0);
+	dell_smbios_send_request(1, 0);
 	ret = buffer->output[0];
 	dell_smbios_release_buffer();
 
@@ -1348,7 +1348,7 @@ static int kbd_get_token_bit(u8 bit)
 
 	dell_smbios_get_buffer();
 	buffer->input[0] = da_tokens[id].location;
-	dell_smbios_send_request(buffer, 0, 0);
+	dell_smbios_send_request(0, 0);
 	ret = buffer->output[0];
 	val = buffer->output[1];
 	dell_smbios_release_buffer();
@@ -2019,7 +2019,7 @@ static int __init dell_init(void)
 	if (token != -1) {
 		dell_smbios_get_buffer();
 		buffer->input[0] = token;
-		dell_smbios_send_request(buffer, 0, 2);
+		dell_smbios_send_request(0, 2);
 		if (buffer->output[0] == 0)
 			max_intensity = buffer->output[3];
 		dell_smbios_release_buffer();
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index a0c1bde..d573765 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -60,8 +60,7 @@ void dell_smbios_release_buffer(void)
 EXPORT_SYMBOL_GPL(dell_smbios_release_buffer);
 
 struct calling_interface_buffer *
-dell_smbios_send_request(struct calling_interface_buffer *buffer,
-			 int class, int select)
+dell_smbios_send_request(int class, int select)
 {
 	struct smi_cmd command;
 
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 33ed971..4220ac1 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -42,8 +42,7 @@ void dell_smbios_get_buffer(void);
 void dell_smbios_clear_buffer(void);
 void dell_smbios_release_buffer(void);
 struct calling_interface_buffer *
-dell_smbios_send_request(struct calling_interface_buffer *buffer,
-			 int class, int select);
+dell_smbios_send_request(int class, int select);
 
 int find_token_id(int tokenid);
 int find_token_location(int tokenid);
-- 
1.7.10.4

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

* [PATCH v2 07/16] dell-smbios: don't return an SMBIOS buffer from dell_smbios_send_request()
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (5 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 06/16] dell-smbios: don't pass an SMBIOS buffer " Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-02-08 18:44   ` Darren Hart
  2016-01-22 14:27 ` [PATCH v2 08/16] dell-smbios: return the SMBIOS buffer from dell_smbios_get_buffer() Michał Kępień
                   ` (9 subsequent siblings)
  16 siblings, 1 reply; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

An SMBIOS buffer pointer does not need to be returned by
dell_smbios_send_request(), because SMBIOS call results are stored in
the buffer passed as input.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-smbios.c |    5 +----
 drivers/platform/x86/dell-smbios.h |    3 +--
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index d573765..b10c613 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -59,8 +59,7 @@ void dell_smbios_release_buffer(void)
 }
 EXPORT_SYMBOL_GPL(dell_smbios_release_buffer);
 
-struct calling_interface_buffer *
-dell_smbios_send_request(int class, int select)
+void dell_smbios_send_request(int class, int select)
 {
 	struct smi_cmd command;
 
@@ -74,8 +73,6 @@ dell_smbios_send_request(int class, int select)
 	buffer->select = select;
 
 	dcdbas_smi_request(&command);
-
-	return buffer;
 }
 EXPORT_SYMBOL_GPL(dell_smbios_send_request);
 
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 4220ac1..80b5048 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -41,8 +41,7 @@ extern struct calling_interface_token *da_tokens;
 void dell_smbios_get_buffer(void);
 void dell_smbios_clear_buffer(void);
 void dell_smbios_release_buffer(void);
-struct calling_interface_buffer *
-dell_smbios_send_request(int class, int select);
+void dell_smbios_send_request(int class, int select);
 
 int find_token_id(int tokenid);
 int find_token_location(int tokenid);
-- 
1.7.10.4

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

* [PATCH v2 08/16] dell-smbios: return the SMBIOS buffer from dell_smbios_get_buffer()
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (6 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 07/16] dell-smbios: don't return an SMBIOS buffer from dell_smbios_send_request() Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 09/16] dell-smbios: make the SMBIOS buffer static Michał Kępień
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

Ultimately, the SMBIOS buffer should not be exported from dell-smbios.
Currently, dell-laptop accesses it directly using a global variable, so
make dell_smbios_get_buffer() return a pointer to the SMBIOS buffer and
replace all uses of the global variable with local variables.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-laptop.c |   49 +++++++++++++++++++++++-------------
 drivers/platform/x86/dell-smbios.c |    3 ++-
 drivers/platform/x86/dell-smbios.h |    2 +-
 3 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 0bb2211..5b200f2 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -409,6 +409,7 @@ static inline int dell_smi_error(int value)
 
 static int dell_rfkill_set(void *data, bool blocked)
 {
+	struct calling_interface_buffer *buffer;
 	int disable = blocked ? 1 : 0;
 	unsigned long radio = (unsigned long)data;
 	int hwswitch_bit = (unsigned long)data - 1;
@@ -416,7 +417,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 	int status;
 	int ret;
 
-	dell_smbios_get_buffer();
+	buffer = dell_smbios_get_buffer();
 
 	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
@@ -451,7 +452,8 @@ static int dell_rfkill_set(void *data, bool blocked)
 
 /* Must be called with the buffer held */
 static void dell_rfkill_update_sw_state(struct rfkill *rfkill, int radio,
-					int status)
+					int status,
+					struct calling_interface_buffer *buffer)
 {
 	if (status & BIT(0)) {
 		/* Has hw-switch, sync sw_state to BIOS */
@@ -474,12 +476,13 @@ static void dell_rfkill_update_hw_state(struct rfkill *rfkill, int radio,
 
 static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 {
+	struct calling_interface_buffer *buffer;
 	int radio = ((unsigned long)data & 0xF);
 	int hwswitch;
 	int status;
 	int ret;
 
-	dell_smbios_get_buffer();
+	buffer = dell_smbios_get_buffer();
 
 	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
@@ -514,12 +517,13 @@ static struct dentry *dell_laptop_dir;
 
 static int dell_debugfs_show(struct seq_file *s, void *data)
 {
+	struct calling_interface_buffer *buffer;
 	int hwswitch_state;
 	int hwswitch_ret;
 	int status;
 	int ret;
 
-	dell_smbios_get_buffer();
+	buffer = dell_smbios_get_buffer();
 
 	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
@@ -613,11 +617,12 @@ static const struct file_operations dell_debugfs_fops = {
 
 static void dell_update_rfkill(struct work_struct *ignored)
 {
+	struct calling_interface_buffer *buffer;
 	int hwswitch = 0;
 	int status;
 	int ret;
 
-	dell_smbios_get_buffer();
+	buffer = dell_smbios_get_buffer();
 
 	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
@@ -637,16 +642,17 @@ static void dell_update_rfkill(struct work_struct *ignored)
 
 	if (wifi_rfkill) {
 		dell_rfkill_update_hw_state(wifi_rfkill, 1, status, hwswitch);
-		dell_rfkill_update_sw_state(wifi_rfkill, 1, status);
+		dell_rfkill_update_sw_state(wifi_rfkill, 1, status, buffer);
 	}
 	if (bluetooth_rfkill) {
 		dell_rfkill_update_hw_state(bluetooth_rfkill, 2, status,
 					    hwswitch);
-		dell_rfkill_update_sw_state(bluetooth_rfkill, 2, status);
+		dell_rfkill_update_sw_state(bluetooth_rfkill, 2, status,
+					    buffer);
 	}
 	if (wwan_rfkill) {
 		dell_rfkill_update_hw_state(wwan_rfkill, 3, status, hwswitch);
-		dell_rfkill_update_sw_state(wwan_rfkill, 3, status);
+		dell_rfkill_update_sw_state(wwan_rfkill, 3, status, buffer);
 	}
 
  out:
@@ -694,6 +700,7 @@ static struct notifier_block dell_laptop_rbtn_notifier = {
 
 static int __init dell_setup_rfkill(void)
 {
+	struct calling_interface_buffer *buffer;
 	int status, ret, whitelisted;
 	const char *product;
 
@@ -709,7 +716,7 @@ static int __init dell_setup_rfkill(void)
 	if (!force_rfkill && !whitelisted)
 		return 0;
 
-	dell_smbios_get_buffer();
+	buffer = dell_smbios_get_buffer();
 	dell_smbios_send_request(17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
@@ -866,6 +873,7 @@ static void dell_cleanup_rfkill(void)
 
 static int dell_send_intensity(struct backlight_device *bd)
 {
+	struct calling_interface_buffer *buffer;
 	int token;
 	int ret;
 
@@ -873,7 +881,7 @@ static int dell_send_intensity(struct backlight_device *bd)
 	if (token == -1)
 		return -ENODEV;
 
-	dell_smbios_get_buffer();
+	buffer = dell_smbios_get_buffer();
 	buffer->input[0] = token;
 	buffer->input[1] = bd->props.brightness;
 
@@ -890,6 +898,7 @@ static int dell_send_intensity(struct backlight_device *bd)
 
 static int dell_get_intensity(struct backlight_device *bd)
 {
+	struct calling_interface_buffer *buffer;
 	int token;
 	int ret;
 
@@ -897,7 +906,7 @@ static int dell_get_intensity(struct backlight_device *bd)
 	if (token == -1)
 		return -ENODEV;
 
-	dell_smbios_get_buffer();
+	buffer = dell_smbios_get_buffer();
 	buffer->input[0] = token;
 
 	if (power_supply_is_system_supplied() > 0)
@@ -1154,10 +1163,11 @@ static bool kbd_led_present;
 
 static int kbd_get_info(struct kbd_info *info)
 {
+	struct calling_interface_buffer *buffer;
 	u8 units;
 	int ret;
 
-	dell_smbios_get_buffer();
+	buffer = dell_smbios_get_buffer();
 
 	buffer->input[0] = 0x0;
 	dell_smbios_send_request(4, 11);
@@ -1243,9 +1253,10 @@ static int kbd_set_level(struct kbd_state *state, u8 level)
 
 static int kbd_get_state(struct kbd_state *state)
 {
+	struct calling_interface_buffer *buffer;
 	int ret;
 
-	dell_smbios_get_buffer();
+	buffer = dell_smbios_get_buffer();
 
 	buffer->input[0] = 0x1;
 	dell_smbios_send_request(4, 11);
@@ -1274,9 +1285,10 @@ static int kbd_get_state(struct kbd_state *state)
 
 static int kbd_set_state(struct kbd_state *state)
 {
+	struct calling_interface_buffer *buffer;
 	int ret;
 
-	dell_smbios_get_buffer();
+	buffer = dell_smbios_get_buffer();
 	buffer->input[0] = 0x2;
 	buffer->input[1] = BIT(state->mode_bit) & 0xFFFF;
 	buffer->input[1] |= (state->triggers & 0xFF) << 16;
@@ -1313,6 +1325,7 @@ static int kbd_set_state_safe(struct kbd_state *state, struct kbd_state *old)
 
 static int kbd_set_token_bit(u8 bit)
 {
+	struct calling_interface_buffer *buffer;
 	int id;
 	int ret;
 
@@ -1323,7 +1336,7 @@ static int kbd_set_token_bit(u8 bit)
 	if (id == -1)
 		return -EINVAL;
 
-	dell_smbios_get_buffer();
+	buffer = dell_smbios_get_buffer();
 	buffer->input[0] = da_tokens[id].location;
 	buffer->input[1] = da_tokens[id].value;
 	dell_smbios_send_request(1, 0);
@@ -1335,6 +1348,7 @@ static int kbd_set_token_bit(u8 bit)
 
 static int kbd_get_token_bit(u8 bit)
 {
+	struct calling_interface_buffer *buffer;
 	int id;
 	int ret;
 	int val;
@@ -1346,7 +1360,7 @@ static int kbd_get_token_bit(u8 bit)
 	if (id == -1)
 		return -EINVAL;
 
-	dell_smbios_get_buffer();
+	buffer = dell_smbios_get_buffer();
 	buffer->input[0] = da_tokens[id].location;
 	dell_smbios_send_request(0, 0);
 	ret = buffer->output[0];
@@ -1972,6 +1986,7 @@ static void kbd_led_exit(void)
 
 static int __init dell_init(void)
 {
+	struct calling_interface_buffer *buffer;
 	int max_intensity = 0;
 	int token;
 	int ret;
@@ -2017,7 +2032,7 @@ static int __init dell_init(void)
 
 	token = find_token_location(BRIGHTNESS_TOKEN);
 	if (token != -1) {
-		dell_smbios_get_buffer();
+		buffer = dell_smbios_get_buffer();
 		buffer->input[0] = token;
 		dell_smbios_send_request(0, 2);
 		if (buffer->output[0] == 0)
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index b10c613..535d382 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -40,10 +40,11 @@ static int da_num_tokens;
 struct calling_interface_token *da_tokens;
 EXPORT_SYMBOL_GPL(da_tokens);
 
-void dell_smbios_get_buffer(void)
+struct calling_interface_buffer *dell_smbios_get_buffer(void)
 {
 	mutex_lock(&buffer_mutex);
 	dell_smbios_clear_buffer();
+	return buffer;
 }
 EXPORT_SYMBOL_GPL(dell_smbios_get_buffer);
 
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 80b5048..f7a51f5 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -38,7 +38,7 @@ struct calling_interface_token {
 extern struct calling_interface_buffer *buffer;
 extern struct calling_interface_token *da_tokens;
 
-void dell_smbios_get_buffer(void);
+struct calling_interface_buffer *dell_smbios_get_buffer(void);
 void dell_smbios_clear_buffer(void);
 void dell_smbios_release_buffer(void);
 void dell_smbios_send_request(int class, int select);
-- 
1.7.10.4

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

* [PATCH v2 09/16] dell-smbios: make the SMBIOS buffer static
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (7 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 08/16] dell-smbios: return the SMBIOS buffer from dell_smbios_get_buffer() Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 10/16] dell-smbios: implement new function for finding DMI table 0xDA tokens Michał Kępień
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

As dell-laptop has been changed to always retrieve a pointer to the
SMBIOS buffer using dell_smbios_get_buffer(), the SMBIOS buffer can be
marked static.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-smbios.c |    3 +--
 drivers/platform/x86/dell-smbios.h |    1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 535d382..b383d0d 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -30,8 +30,7 @@ struct calling_interface_structure {
 	struct calling_interface_token tokens[];
 } __packed;
 
-struct calling_interface_buffer *buffer;
-EXPORT_SYMBOL_GPL(buffer);
+static struct calling_interface_buffer *buffer;
 static DEFINE_MUTEX(buffer_mutex);
 
 static int da_command_address;
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index f7a51f5..af653ff 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -35,7 +35,6 @@ struct calling_interface_token {
 	};
 };
 
-extern struct calling_interface_buffer *buffer;
 extern struct calling_interface_token *da_tokens;
 
 struct calling_interface_buffer *dell_smbios_get_buffer(void);
-- 
1.7.10.4

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

* [PATCH v2 10/16] dell-smbios: implement new function for finding DMI table 0xDA tokens
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (8 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 09/16] dell-smbios: make the SMBIOS buffer static Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 11/16] dell-laptop: use dell_smbios_find_token() instead of find_token_id() Michał Kępień
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

Ultimately, the da_tokens table should not be exported from dell-smbios.
Currently, in some cases, dell-laptop accesses that table's members
directly, so implement a new function, dell_smbios_find_token(), which
returns a pointer to an entry inside the da_tokens table with the given
token ID (or NULL if it is not found).

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-smbios.c |   13 +++++++++++++
 drivers/platform/x86/dell-smbios.h |    2 ++
 2 files changed, 15 insertions(+)

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index b383d0d..43aafab 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -76,6 +76,19 @@ void dell_smbios_send_request(int class, int select)
 }
 EXPORT_SYMBOL_GPL(dell_smbios_send_request);
 
+struct calling_interface_token *dell_smbios_find_token(int tokenid)
+{
+	int i;
+
+	for (i = 0; i < da_num_tokens; i++) {
+		if (da_tokens[i].tokenID == tokenid)
+			return &da_tokens[i];
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(dell_smbios_find_token);
+
 int find_token_id(int tokenid)
 {
 	int i;
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index af653ff..6f6dbe8 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -42,6 +42,8 @@ void dell_smbios_clear_buffer(void);
 void dell_smbios_release_buffer(void);
 void dell_smbios_send_request(int class, int select);
 
+struct calling_interface_token *dell_smbios_find_token(int tokenid);
+
 int find_token_id(int tokenid);
 int find_token_location(int tokenid);
 #endif
-- 
1.7.10.4

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

* [PATCH v2 11/16] dell-laptop: use dell_smbios_find_token() instead of find_token_id()
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (9 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 10/16] dell-smbios: implement new function for finding DMI table 0xDA tokens Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 12/16] dell-laptop: use dell_smbios_find_token() instead of find_token_location() Michał Kępień
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

Replace all uses of find_token_id() with dell_smbios_find_token() to
avoid directly accessing the da_tokens table.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-laptop.c |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 5b200f2..4d1694d 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -1326,19 +1326,19 @@ static int kbd_set_state_safe(struct kbd_state *state, struct kbd_state *old)
 static int kbd_set_token_bit(u8 bit)
 {
 	struct calling_interface_buffer *buffer;
-	int id;
+	struct calling_interface_token *token;
 	int ret;
 
 	if (bit >= ARRAY_SIZE(kbd_tokens))
 		return -EINVAL;
 
-	id = find_token_id(kbd_tokens[bit]);
-	if (id == -1)
+	token = dell_smbios_find_token(kbd_tokens[bit]);
+	if (!token)
 		return -EINVAL;
 
 	buffer = dell_smbios_get_buffer();
-	buffer->input[0] = da_tokens[id].location;
-	buffer->input[1] = da_tokens[id].value;
+	buffer->input[0] = token->location;
+	buffer->input[1] = token->value;
 	dell_smbios_send_request(1, 0);
 	ret = buffer->output[0];
 	dell_smbios_release_buffer();
@@ -1349,19 +1349,19 @@ static int kbd_set_token_bit(u8 bit)
 static int kbd_get_token_bit(u8 bit)
 {
 	struct calling_interface_buffer *buffer;
-	int id;
+	struct calling_interface_token *token;
 	int ret;
 	int val;
 
 	if (bit >= ARRAY_SIZE(kbd_tokens))
 		return -EINVAL;
 
-	id = find_token_id(kbd_tokens[bit]);
-	if (id == -1)
+	token = dell_smbios_find_token(kbd_tokens[bit]);
+	if (!token)
 		return -EINVAL;
 
 	buffer = dell_smbios_get_buffer();
-	buffer->input[0] = da_tokens[id].location;
+	buffer->input[0] = token->location;
 	dell_smbios_send_request(0, 0);
 	ret = buffer->output[0];
 	val = buffer->output[1];
@@ -1370,7 +1370,7 @@ static int kbd_get_token_bit(u8 bit)
 	if (ret)
 		return dell_smi_error(ret);
 
-	return (val == da_tokens[id].value);
+	return (val == token->value);
 }
 
 static int kbd_get_first_active_token_bit(void)
@@ -1472,7 +1472,7 @@ static inline void kbd_init_tokens(void)
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(kbd_tokens); ++i)
-		if (find_token_id(kbd_tokens[i]) != -1)
+		if (dell_smbios_find_token(kbd_tokens[i]))
 			kbd_token_bits |= BIT(i);
 }
 
-- 
1.7.10.4

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

* [PATCH v2 12/16] dell-laptop: use dell_smbios_find_token() instead of find_token_location()
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (10 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 11/16] dell-laptop: use dell_smbios_find_token() instead of find_token_id() Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 13/16] dell-smbios: remove find_token_{id,location}() Michał Kępień
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

Replace all uses of find_token_location() with dell_smbios_find_token()
to avoid directly accessing the da_tokens table.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-laptop.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 4d1694d..76064c8 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -874,15 +874,15 @@ static void dell_cleanup_rfkill(void)
 static int dell_send_intensity(struct backlight_device *bd)
 {
 	struct calling_interface_buffer *buffer;
-	int token;
+	struct calling_interface_token *token;
 	int ret;
 
-	token = find_token_location(BRIGHTNESS_TOKEN);
-	if (token == -1)
+	token = dell_smbios_find_token(BRIGHTNESS_TOKEN);
+	if (!token)
 		return -ENODEV;
 
 	buffer = dell_smbios_get_buffer();
-	buffer->input[0] = token;
+	buffer->input[0] = token->location;
 	buffer->input[1] = bd->props.brightness;
 
 	if (power_supply_is_system_supplied() > 0)
@@ -899,15 +899,15 @@ static int dell_send_intensity(struct backlight_device *bd)
 static int dell_get_intensity(struct backlight_device *bd)
 {
 	struct calling_interface_buffer *buffer;
-	int token;
+	struct calling_interface_token *token;
 	int ret;
 
-	token = find_token_location(BRIGHTNESS_TOKEN);
-	if (token == -1)
+	token = dell_smbios_find_token(BRIGHTNESS_TOKEN);
+	if (!token)
 		return -ENODEV;
 
 	buffer = dell_smbios_get_buffer();
-	buffer->input[0] = token;
+	buffer->input[0] = token->location;
 
 	if (power_supply_is_system_supplied() > 0)
 		dell_smbios_send_request(0, 2);
@@ -1987,8 +1987,8 @@ static void kbd_led_exit(void)
 static int __init dell_init(void)
 {
 	struct calling_interface_buffer *buffer;
+	struct calling_interface_token *token;
 	int max_intensity = 0;
-	int token;
 	int ret;
 
 	if (!dmi_check_system(dell_device_table))
@@ -2030,10 +2030,10 @@ static int __init dell_init(void)
 	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
 		return 0;
 
-	token = find_token_location(BRIGHTNESS_TOKEN);
-	if (token != -1) {
+	token = dell_smbios_find_token(BRIGHTNESS_TOKEN);
+	if (token) {
 		buffer = dell_smbios_get_buffer();
-		buffer->input[0] = token;
+		buffer->input[0] = token->location;
 		dell_smbios_send_request(0, 2);
 		if (buffer->output[0] == 0)
 			max_intensity = buffer->output[3];
-- 
1.7.10.4

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

* [PATCH v2 13/16] dell-smbios: remove find_token_{id,location}()
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (11 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 12/16] dell-laptop: use dell_smbios_find_token() instead of find_token_location() Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 14/16] dell-smbios: make da_tokens static Michał Kępień
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

As dell-laptop has been changed to use dell_smbios_find_token() instead
of find_token_id() and find_token_location(), these functions can be
safely removed.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-smbios.c |   25 -------------------------
 drivers/platform/x86/dell-smbios.h |    3 ---
 2 files changed, 28 deletions(-)

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 43aafab..e313c16 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -89,31 +89,6 @@ struct calling_interface_token *dell_smbios_find_token(int tokenid)
 }
 EXPORT_SYMBOL_GPL(dell_smbios_find_token);
 
-int find_token_id(int tokenid)
-{
-	int i;
-
-	for (i = 0; i < da_num_tokens; i++) {
-		if (da_tokens[i].tokenID == tokenid)
-			return i;
-	}
-
-	return -1;
-}
-EXPORT_SYMBOL_GPL(find_token_id);
-
-int find_token_location(int tokenid)
-{
-	int id;
-
-	id = find_token_id(tokenid);
-	if (id == -1)
-		return -1;
-
-	return da_tokens[id].location;
-}
-EXPORT_SYMBOL_GPL(find_token_location);
-
 static void __init parse_da_table(const struct dmi_header *dm)
 {
 	/* Final token is a terminator, so we don't want to copy it */
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 6f6dbe8..f17cf7b 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -43,7 +43,4 @@ void dell_smbios_release_buffer(void);
 void dell_smbios_send_request(int class, int select);
 
 struct calling_interface_token *dell_smbios_find_token(int tokenid);
-
-int find_token_id(int tokenid);
-int find_token_location(int tokenid);
 #endif
-- 
1.7.10.4

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

* [PATCH v2 14/16] dell-smbios: make da_tokens static
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (12 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 13/16] dell-smbios: remove find_token_{id,location}() Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 15/16] dell-led: use dell_smbios_find_token() for finding mic DMI tokens Michał Kępień
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

As dell-laptop has been changed to use dell_smbios_find_token() instead
of directly accessing members of the da_tokens table, the latter can be
marked static.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-smbios.c |    3 +--
 drivers/platform/x86/dell-smbios.h |    2 --
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index e313c16..f68c434 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -36,8 +36,7 @@ static DEFINE_MUTEX(buffer_mutex);
 static int da_command_address;
 static int da_command_code;
 static int da_num_tokens;
-struct calling_interface_token *da_tokens;
-EXPORT_SYMBOL_GPL(da_tokens);
+static struct calling_interface_token *da_tokens;
 
 struct calling_interface_buffer *dell_smbios_get_buffer(void)
 {
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index f17cf7b..4f69b16 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -35,8 +35,6 @@ struct calling_interface_token {
 	};
 };
 
-extern struct calling_interface_token *da_tokens;
-
 struct calling_interface_buffer *dell_smbios_get_buffer(void);
 void dell_smbios_clear_buffer(void);
 void dell_smbios_release_buffer(void);
-- 
1.7.10.4

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

* [PATCH v2 15/16] dell-led: use dell_smbios_find_token() for finding mic DMI tokens
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (13 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 14/16] dell-smbios: make da_tokens static Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:27 ` [PATCH v2 16/16] dell-led: use dell_smbios_send_request() for performing SMBIOS calls Michał Kępień
  2016-01-22 14:48 ` [PATCH v2 00/16] Common Dell SMBIOS API Pali Rohár
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

With the advent of dell_smbios_find_token(), dell-led does not need to
perform any DMI walking on its own, but it can rather ask dell-smbios to
look up the DMI tokens it needs for changing the state of the microphone
LED.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 drivers/leds/Kconfig    |    1 +
 drivers/leds/dell-led.c |   63 +++++++----------------------------------------
 2 files changed, 10 insertions(+), 54 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 7f940c2..4351cbe 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -443,6 +443,7 @@ config LEDS_DELL_NETBOOKS
 	tristate "External LED on Dell Business Netbooks"
 	depends on LEDS_CLASS
 	depends on X86 && ACPI_WMI
+	depends on DELL_SMBIOS
 	help
 	  This adds support for the Latitude 2100 and similar
 	  notebooks that have an external LED.
diff --git a/drivers/leds/dell-led.c b/drivers/leds/dell-led.c
index c36acaf..bfa7511 100644
--- a/drivers/leds/dell-led.c
+++ b/drivers/leds/dell-led.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/dmi.h>
 #include <linux/dell-led.h>
+#include "../platform/x86/dell-smbios.h"
 
 MODULE_AUTHOR("Louis Davis/Jim Dailey");
 MODULE_DESCRIPTION("Dell LED Control Driver");
@@ -59,22 +60,6 @@ struct app_wmi_args {
 #define GLOBAL_MIC_MUTE_ENABLE	0x364
 #define GLOBAL_MIC_MUTE_DISABLE	0x365
 
-struct dell_bios_data_token {
-	u16 tokenid;
-	u16 location;
-	u16 value;
-};
-
-struct __attribute__ ((__packed__)) dell_bios_calling_interface {
-	struct	dmi_header header;
-	u16	cmd_io_addr;
-	u8	cmd_io_code;
-	u32	supported_cmds;
-	struct	dell_bios_data_token damap[];
-};
-
-static struct dell_bios_data_token dell_mic_tokens[2];
-
 static int dell_wmi_perform_query(struct app_wmi_args *args)
 {
 	struct app_wmi_args *bios_return;
@@ -112,43 +97,24 @@ static int dell_wmi_perform_query(struct app_wmi_args *args)
 	return rc;
 }
 
-static void __init find_micmute_tokens(const struct dmi_header *dm, void *dummy)
-{
-	struct dell_bios_calling_interface *calling_interface;
-	struct dell_bios_data_token *token;
-	int token_size = sizeof(struct dell_bios_data_token);
-	int i = 0;
-
-	if (dm->type == 0xda && dm->length > 17) {
-		calling_interface = container_of(dm,
-				struct dell_bios_calling_interface, header);
-
-		token = &calling_interface->damap[i];
-		while (token->tokenid != 0xffff) {
-			if (token->tokenid == GLOBAL_MIC_MUTE_DISABLE)
-				memcpy(&dell_mic_tokens[0], token, token_size);
-			else if (token->tokenid == GLOBAL_MIC_MUTE_ENABLE)
-				memcpy(&dell_mic_tokens[1], token, token_size);
-
-			i++;
-			token = &calling_interface->damap[i];
-		}
-	}
-}
-
 static int dell_micmute_led_set(int state)
 {
+	struct calling_interface_token *token;
 	struct app_wmi_args args;
-	struct dell_bios_data_token *token;
 
 	if (!wmi_has_guid(DELL_APP_GUID))
 		return -ENODEV;
 
-	if (state == 0 || state == 1)
-		token = &dell_mic_tokens[state];
+	if (state == 0)
+		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE);
+	else if (state == 1)
+		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE);
 	else
 		return -EINVAL;
 
+	if (!token)
+		return -ENODEV;
+
 	memset(&args, 0, sizeof(struct app_wmi_args));
 
 	args.class = 1;
@@ -177,14 +143,6 @@ int dell_app_wmi_led_set(int whichled, int on)
 }
 EXPORT_SYMBOL_GPL(dell_app_wmi_led_set);
 
-static int __init dell_micmute_led_init(void)
-{
-	memset(dell_mic_tokens, 0, sizeof(struct dell_bios_data_token) * 2);
-	dmi_walk(find_micmute_tokens, NULL);
-
-	return 0;
-}
-
 struct bios_args {
 	unsigned char length;
 	unsigned char result_code;
@@ -330,9 +288,6 @@ static int __init dell_led_init(void)
 	if (!wmi_has_guid(DELL_LED_BIOS_GUID) && !wmi_has_guid(DELL_APP_GUID))
 		return -ENODEV;
 
-	if (wmi_has_guid(DELL_APP_GUID))
-		error = dell_micmute_led_init();
-
 	if (wmi_has_guid(DELL_LED_BIOS_GUID)) {
 		error = led_off();
 		if (error != 0)
-- 
1.7.10.4

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

* [PATCH v2 16/16] dell-led: use dell_smbios_send_request() for performing SMBIOS calls
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (14 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 15/16] dell-led: use dell_smbios_find_token() for finding mic DMI tokens Michał Kępień
@ 2016-01-22 14:27 ` Michał Kępień
  2016-01-22 14:48 ` [PATCH v2 00/16] Common Dell SMBIOS API Pali Rohár
  16 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-22 14:27 UTC (permalink / raw)
  To: Darren Hart, Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski
  Cc: platform-driver-x86, linux-leds, linux-kernel

Instead of using the WMI wrapper, dell-led can take advantage of
dell_smbios_send_request() for performing the SMBIOS calls required to
change the state of the microphone LED.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 drivers/leds/dell-led.c |   65 +++++------------------------------------------
 1 file changed, 6 insertions(+), 59 deletions(-)

diff --git a/drivers/leds/dell-led.c b/drivers/leds/dell-led.c
index bfa7511..b3d6e9c 100644
--- a/drivers/leds/dell-led.c
+++ b/drivers/leds/dell-led.c
@@ -43,64 +43,13 @@ MODULE_ALIAS("wmi:" DELL_LED_BIOS_GUID);
 #define CMD_LED_OFF	17
 #define CMD_LED_BLINK	18
 
-struct app_wmi_args {
-	u16 class;
-	u16 selector;
-	u32 arg1;
-	u32 arg2;
-	u32 arg3;
-	u32 arg4;
-	u32 res1;
-	u32 res2;
-	u32 res3;
-	u32 res4;
-	char dummy[92];
-};
-
 #define GLOBAL_MIC_MUTE_ENABLE	0x364
 #define GLOBAL_MIC_MUTE_DISABLE	0x365
 
-static int dell_wmi_perform_query(struct app_wmi_args *args)
-{
-	struct app_wmi_args *bios_return;
-	union acpi_object *obj;
-	struct acpi_buffer input;
-	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
-	acpi_status status;
-	u32 rc = -EINVAL;
-
-	input.length = 128;
-	input.pointer = args;
-
-	status = wmi_evaluate_method(DELL_APP_GUID, 0, 1, &input, &output);
-	if (!ACPI_SUCCESS(status))
-		goto err_out0;
-
-	obj = output.pointer;
-	if (!obj)
-		goto err_out0;
-
-	if (obj->type != ACPI_TYPE_BUFFER)
-		goto err_out1;
-
-	bios_return = (struct app_wmi_args *)obj->buffer.pointer;
-	rc = bios_return->res1;
-	if (rc)
-		goto err_out1;
-
-	memcpy(args, bios_return, sizeof(struct app_wmi_args));
-	rc = 0;
-
- err_out1:
-	kfree(obj);
- err_out0:
-	return rc;
-}
-
 static int dell_micmute_led_set(int state)
 {
+	struct calling_interface_buffer *buffer;
 	struct calling_interface_token *token;
-	struct app_wmi_args args;
 
 	if (!wmi_has_guid(DELL_APP_GUID))
 		return -ENODEV;
@@ -115,13 +64,11 @@ static int dell_micmute_led_set(int state)
 	if (!token)
 		return -ENODEV;
 
-	memset(&args, 0, sizeof(struct app_wmi_args));
-
-	args.class = 1;
-	args.arg1 = token->location;
-	args.arg2 = token->value;
-
-	dell_wmi_perform_query(&args);
+	buffer = dell_smbios_get_buffer();
+	buffer->input[0] = token->location;
+	buffer->input[1] = token->value;
+	dell_smbios_send_request(1, 0);
+	dell_smbios_release_buffer();
 
 	return state;
 }
-- 
1.7.10.4

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

* Re: [PATCH v2 00/16] Common Dell SMBIOS API
  2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
                   ` (15 preceding siblings ...)
  2016-01-22 14:27 ` [PATCH v2 16/16] dell-led: use dell_smbios_send_request() for performing SMBIOS calls Michał Kępień
@ 2016-01-22 14:48 ` Pali Rohár
  2016-02-07 20:34   ` Darren Hart
  2016-02-08 19:20   ` Darren Hart
  16 siblings, 2 replies; 33+ messages in thread
From: Pali Rohár @ 2016-01-22 14:48 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Darren Hart, Matthew Garrett, Richard Purdie, Jacek Anaszewski,
	platform-driver-x86, linux-leds, linux-kernel

On Friday 22 January 2016 15:27:12 Michał Kępień wrote:
> Note:
> 
>     In this series (both v1 and v2) I tried to stick to the overall
>     concept used in dell-laptop, but in the v1 thread me and Pali also
>     briefly discussed his alternative ideas [1][2] as to what this API
>     could look like, so feel free to suggest a different approach.
> 
> [1] http://www.spinics.net/lists/platform-driver-x86/msg08260.html
> [2] http://www.spinics.net/lists/platform-driver-x86/msg08268.html

I would like to hear opinion about dell-smbios API also from other
people. Darren, can you look and comment it?

-- 
Pali Rohár
pali.rohar@gmail.com

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

* Re: [PATCH v2 01/16] dell-laptop: extract SMBIOS-related code to a separate module
  2016-01-22 14:27 ` [PATCH v2 01/16] dell-laptop: extract SMBIOS-related code to a separate module Michał Kępień
@ 2016-01-24  2:16   ` kbuild test robot
  2016-01-26 13:33     ` Michał Kępień
  0 siblings, 1 reply; 33+ messages in thread
From: kbuild test robot @ 2016-01-24  2:16 UTC (permalink / raw)
  To: Michał Kępień
  Cc: kbuild-all, Darren Hart, Matthew Garrett, Pali Rohár,
	Richard Purdie, Jacek Anaszewski, platform-driver-x86,
	linux-leds, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1444 bytes --]

Hi Michał,

[auto build test ERROR on platform-drivers-x86/for-next]
[also build test ERROR on v4.4 next-20160122]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Micha-K-pie/Common-Dell-SMBIOS-API/20160122-223550
base:   git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git for-next
config: x86_64-randconfig-n0-01240935 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/platform/x86/dell-smbios.c: In function 'dell_send_request':
>> drivers/platform/x86/dell-smbios.c:71:16: error: implicit declaration of function 'virt_to_phys' [-Werror=implicit-function-declaration]
     command.ebx = virt_to_phys(buffer);
                   ^
   cc1: some warnings being treated as errors

vim +/virt_to_phys +71 drivers/platform/x86/dell-smbios.c

    65	{
    66		struct smi_cmd command;
    67	
    68		command.magic = SMI_CMD_MAGIC;
    69		command.command_address = da_command_address;
    70		command.command_code = da_command_code;
  > 71		command.ebx = virt_to_phys(buffer);
    72		command.ecx = 0x42534931;
    73	
    74		buffer->class = class;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 26444 bytes --]

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

* Re: [PATCH v2 01/16] dell-laptop: extract SMBIOS-related code to a separate module
  2016-01-24  2:16   ` kbuild test robot
@ 2016-01-26 13:33     ` Michał Kępień
  0 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-01-26 13:33 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, Darren Hart, Matthew Garrett, Pali Rohár,
	Richard Purdie, Jacek Anaszewski, platform-driver-x86,
	linux-leds, linux-kernel

> Hi Michał,
> 
> [auto build test ERROR on platform-drivers-x86/for-next]
> [also build test ERROR on v4.4 next-20160122]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Micha-K-pie/Common-Dell-SMBIOS-API/20160122-223550
> base:   git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git for-next
> config: x86_64-randconfig-n0-01240935 (attached as .config)
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/platform/x86/dell-smbios.c: In function 'dell_send_request':
> >> drivers/platform/x86/dell-smbios.c:71:16: error: implicit declaration of function 'virt_to_phys' [-Werror=implicit-function-declaration]
>      command.ebx = virt_to_phys(buffer);
>                    ^
>    cc1: some warnings being treated as errors

Nice catch, turns out I missed the <linux/io.h> include in
dell-smbios.c, but that only causes errors for non-SMP builds.

I'll keep that in mind for v3 of this series, if it happens.  If not,
I'll post a v3 of just this patch once the rest of the series gets
acked.

-- 
Best regards,
Michał Kępień

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

* Re: [PATCH v2 00/16] Common Dell SMBIOS API
  2016-01-22 14:48 ` [PATCH v2 00/16] Common Dell SMBIOS API Pali Rohár
@ 2016-02-07 20:34   ` Darren Hart
  2016-02-08 19:20   ` Darren Hart
  1 sibling, 0 replies; 33+ messages in thread
From: Darren Hart @ 2016-02-07 20:34 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Michał Kępień,
	Matthew Garrett, Richard Purdie, Jacek Anaszewski,
	platform-driver-x86, linux-leds, linux-kernel

On Fri, Jan 22, 2016 at 03:48:51PM +0100, Pali Rohár wrote:
> On Friday 22 January 2016 15:27:12 Michał Kępień wrote:
> > Note:
> > 
> >     In this series (both v1 and v2) I tried to stick to the overall
> >     concept used in dell-laptop, but in the v1 thread me and Pali also
> >     briefly discussed his alternative ideas [1][2] as to what this API
> >     could look like, so feel free to suggest a different approach.
> > 
> > [1] http://www.spinics.net/lists/platform-driver-x86/msg08260.html
> > [2] http://www.spinics.net/lists/platform-driver-x86/msg08268.html
> 
> I would like to hear opinion about dell-smbios API also from other
> people. Darren, can you look and comment it?

Sorry for the long delay folks. I'm just about caught up. I'll get to this by
tomorrow at the latest.

-- 
Darren Hart
Intel Open Source Technology Center

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

* Re: [PATCH v2 07/16] dell-smbios: don't return an SMBIOS buffer from dell_smbios_send_request()
  2016-01-22 14:27 ` [PATCH v2 07/16] dell-smbios: don't return an SMBIOS buffer from dell_smbios_send_request() Michał Kępień
@ 2016-02-08 18:44   ` Darren Hart
  2016-02-09 13:27     ` Michał Kępień
  0 siblings, 1 reply; 33+ messages in thread
From: Darren Hart @ 2016-02-08 18:44 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski, platform-driver-x86, linux-leds, linux-kernel

On Fri, Jan 22, 2016 at 03:27:19PM +0100, Michał Kępień wrote:
> An SMBIOS buffer pointer does not need to be returned by
> dell_smbios_send_request(), because SMBIOS call results are stored in
> the buffer passed as input.

This should come before 6/16, update the commit message to reflect the module
exported buffer (not the one passed as input), or possibly just merge this patch
and 6/16 as correcting the use of SMBIOS buffer within the module.

-- 
Darren Hart
Intel Open Source Technology Center

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

* Re: [PATCH v2 00/16] Common Dell SMBIOS API
  2016-01-22 14:48 ` [PATCH v2 00/16] Common Dell SMBIOS API Pali Rohár
  2016-02-07 20:34   ` Darren Hart
@ 2016-02-08 19:20   ` Darren Hart
  2016-02-08 19:29     ` Greg Kroah-Hartman
  2016-02-08 19:30     ` Lukas Wunner
  1 sibling, 2 replies; 33+ messages in thread
From: Darren Hart @ 2016-02-08 19:20 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Michał Kępień,
	Greg Kroah-Hartman, Matthew Garrett, Richard Purdie,
	Jacek Anaszewski, platform-driver-x86, linux-leds, linux-kernel

On Fri, Jan 22, 2016 at 03:48:51PM +0100, Pali Rohár wrote:
> On Friday 22 January 2016 15:27:12 Michał Kępień wrote:
> > Note:
> > 
> >     In this series (both v1 and v2) I tried to stick to the overall
> >     concept used in dell-laptop, but in the v1 thread me and Pali also
> >     briefly discussed his alternative ideas [1][2] as to what this API
> >     could look like, so feel free to suggest a different approach.
> > 
> > [1] http://www.spinics.net/lists/platform-driver-x86/msg08260.html
> > [2] http://www.spinics.net/lists/platform-driver-x86/msg08268.html
> 
> I would like to hear opinion about dell-smbios API also from other
> people. Darren, can you look and comment it?

This is an excellently prepared series, nice work Michał.

Most of my concerns were addressed by later patches in the series. I have pushed
a version of this 1/7 fixed per lkp (linux/io.h) and 7/16 with a corrected body
as I sent in reply to that patch. This is on my tree as the dell-smbios branch.

My only major concern is module load order dependencies. Inter-module
dependencies are frowned upon with good reason, the kernel load ordering is
non-deterministic and it's possible, for example, for dell-laptop to fail to
find the symbols exported by dell-smbios under certain conditions.

I have worked around this in the past with things like the following:

#ifdef MODULE
#ifdef CONFIG_FOO_MODULE
	if (request_module("foo"))
		return -ENODEV;
#endif
#endif

Something like the above may be necessary for dell-smbios in dell-laptop,
dell-wmi, and dell-leds now that they depend on the dell-smbios exported
functions.

Cc Greg in case there is a better way to handle this that I'm not aware of.

-- 
Darren Hart
Intel Open Source Technology Center

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

* Re: [PATCH v2 00/16] Common Dell SMBIOS API
  2016-02-08 19:20   ` Darren Hart
@ 2016-02-08 19:29     ` Greg Kroah-Hartman
  2016-02-08 20:46       ` Darren Hart
  2016-02-08 19:30     ` Lukas Wunner
  1 sibling, 1 reply; 33+ messages in thread
From: Greg Kroah-Hartman @ 2016-02-08 19:29 UTC (permalink / raw)
  To: Darren Hart
  Cc: Pali Rohár, Michał Kępień,
	Matthew Garrett, Richard Purdie, Jacek Anaszewski,
	platform-driver-x86, linux-leds, linux-kernel

On Mon, Feb 08, 2016 at 11:20:14AM -0800, Darren Hart wrote:
> On Fri, Jan 22, 2016 at 03:48:51PM +0100, Pali Rohár wrote:
> > On Friday 22 January 2016 15:27:12 Michał Kępień wrote:
> > > Note:
> > > 
> > >     In this series (both v1 and v2) I tried to stick to the overall
> > >     concept used in dell-laptop, but in the v1 thread me and Pali also
> > >     briefly discussed his alternative ideas [1][2] as to what this API
> > >     could look like, so feel free to suggest a different approach.
> > > 
> > > [1] http://www.spinics.net/lists/platform-driver-x86/msg08260.html
> > > [2] http://www.spinics.net/lists/platform-driver-x86/msg08268.html
> > 
> > I would like to hear opinion about dell-smbios API also from other
> > people. Darren, can you look and comment it?
> 
> This is an excellently prepared series, nice work Michał.
> 
> Most of my concerns were addressed by later patches in the series. I have pushed
> a version of this 1/7 fixed per lkp (linux/io.h) and 7/16 with a corrected body
> as I sent in reply to that patch. This is on my tree as the dell-smbios branch.
> 
> My only major concern is module load order dependencies. Inter-module
> dependencies are frowned upon with good reason, the kernel load ordering is
> non-deterministic and it's possible, for example, for dell-laptop to fail to
> find the symbols exported by dell-smbios under certain conditions.
> 
> I have worked around this in the past with things like the following:
> 
> #ifdef MODULE
> #ifdef CONFIG_FOO_MODULE
> 	if (request_module("foo"))
> 		return -ENODEV;
> #endif
> #endif
> 
> Something like the above may be necessary for dell-smbios in dell-laptop,
> dell-wmi, and dell-leds now that they depend on the dell-smbios exported
> functions.
> 
> Cc Greg in case there is a better way to handle this that I'm not aware of.

No need to request_module anything, the symbol resolution should pull
the dependant module in automagically when you do a 'modprobe',
otherwise the symbols would never be found.

thanks,

greg k-h

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

* Re: [PATCH v2 00/16] Common Dell SMBIOS API
  2016-02-08 19:20   ` Darren Hart
  2016-02-08 19:29     ` Greg Kroah-Hartman
@ 2016-02-08 19:30     ` Lukas Wunner
  2016-02-08 20:43       ` Darren Hart
  1 sibling, 1 reply; 33+ messages in thread
From: Lukas Wunner @ 2016-02-08 19:30 UTC (permalink / raw)
  To: Darren Hart
  Cc: Pali Rohár, Micha?? K??pie??,
	Greg Kroah-Hartman, Matthew Garrett, Richard Purdie,
	Jacek Anaszewski, platform-driver-x86, linux-leds, linux-kernel

Hi,

On Mon, Feb 08, 2016 at 11:20:14AM -0800, Darren Hart wrote:
> My only major concern is module load order dependencies. Inter-module
> dependencies are frowned upon with good reason, the kernel load ordering is
> non-deterministic and it's possible, for example, for dell-laptop to fail to
> find the symbols exported by dell-smbios under certain conditions.
> 
> I have worked around this in the past with things like the following:
> 
> #ifdef MODULE
> #ifdef CONFIG_FOO_MODULE
> 	if (request_module("foo"))
> 		return -ENODEV;
> #endif
> #endif
> 
> Something like the above may be necessary for dell-smbios in dell-laptop,
> dell-wmi, and dell-leds now that they depend on the dell-smbios exported
> functions.
> 
> Cc Greg in case there is a better way to handle this that I'm not aware of.

Deferred probing seems to be the preferred way, see e.g.:
https://lists.freedesktop.org/archives/dri-devel/2016-January/098404.html

(In this example, apple_gmux_present() determines presence of the device
and !vga_switcheroo_handler_flags() determine non-presence of its driver.)

This will also work if the subsystem depended on is compiled in rather
than in a module.

Best regards,

Lukas

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

* Re: [PATCH v2 00/16] Common Dell SMBIOS API
  2016-02-08 19:30     ` Lukas Wunner
@ 2016-02-08 20:43       ` Darren Hart
  0 siblings, 0 replies; 33+ messages in thread
From: Darren Hart @ 2016-02-08 20:43 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Pali Rohár, Micha?? K??pie??,
	Greg Kroah-Hartman, Matthew Garrett, Richard Purdie,
	Jacek Anaszewski, platform-driver-x86, linux-leds, linux-kernel

On Mon, Feb 08, 2016 at 08:30:07PM +0100, Lukas Wunner wrote:
> Hi,
> 
> On Mon, Feb 08, 2016 at 11:20:14AM -0800, Darren Hart wrote:
> > My only major concern is module load order dependencies. Inter-module
> > dependencies are frowned upon with good reason, the kernel load ordering is
> > non-deterministic and it's possible, for example, for dell-laptop to fail to
> > find the symbols exported by dell-smbios under certain conditions.
> > 
> > I have worked around this in the past with things like the following:
> > 
> > #ifdef MODULE
> > #ifdef CONFIG_FOO_MODULE
> > 	if (request_module("foo"))
> > 		return -ENODEV;
> > #endif
> > #endif
> > 
> > Something like the above may be necessary for dell-smbios in dell-laptop,
> > dell-wmi, and dell-leds now that they depend on the dell-smbios exported
> > functions.
> > 
> > Cc Greg in case there is a better way to handle this that I'm not aware of.
> 
> Deferred probing seems to be the preferred way, see e.g.:
> https://lists.freedesktop.org/archives/dri-devel/2016-January/098404.html
> 
> (In this example, apple_gmux_present() determines presence of the device
> and !vga_switcheroo_handler_flags() determine non-presence of its driver.)
> 
> This will also work if the subsystem depended on is compiled in rather
> than in a module.

Ah, interesting. Thanks Lukas. Sounds like we're going to be fine as is with
this particular case.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center

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

* Re: [PATCH v2 00/16] Common Dell SMBIOS API
  2016-02-08 19:29     ` Greg Kroah-Hartman
@ 2016-02-08 20:46       ` Darren Hart
  2016-02-08 21:04         ` Pali Rohár
  2016-02-09 14:15         ` Michał Kępień
  0 siblings, 2 replies; 33+ messages in thread
From: Darren Hart @ 2016-02-08 20:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Pali Rohár, Michał Kępień,
	Matthew Garrett, Richard Purdie, Jacek Anaszewski,
	platform-driver-x86, linux-leds, linux-kernel

On Mon, Feb 08, 2016 at 11:29:20AM -0800, Greg Kroah-Hartman wrote:
> On Mon, Feb 08, 2016 at 11:20:14AM -0800, Darren Hart wrote:
> > On Fri, Jan 22, 2016 at 03:48:51PM +0100, Pali Rohár wrote:
> > > On Friday 22 January 2016 15:27:12 Michał Kępień wrote:
> > > > Note:
> > > > 
> > > >     In this series (both v1 and v2) I tried to stick to the overall
> > > >     concept used in dell-laptop, but in the v1 thread me and Pali also
> > > >     briefly discussed his alternative ideas [1][2] as to what this API
> > > >     could look like, so feel free to suggest a different approach.
> > > > 
> > > > [1] http://www.spinics.net/lists/platform-driver-x86/msg08260.html
> > > > [2] http://www.spinics.net/lists/platform-driver-x86/msg08268.html
> > > 
> > > I would like to hear opinion about dell-smbios API also from other
> > > people. Darren, can you look and comment it?
> > 
> > This is an excellently prepared series, nice work Michał.
> > 
> > Most of my concerns were addressed by later patches in the series. I have pushed
> > a version of this 1/7 fixed per lkp (linux/io.h) and 7/16 with a corrected body
> > as I sent in reply to that patch. This is on my tree as the dell-smbios branch.
> > 
> > My only major concern is module load order dependencies. Inter-module
> > dependencies are frowned upon with good reason, the kernel load ordering is
> > non-deterministic and it's possible, for example, for dell-laptop to fail to
> > find the symbols exported by dell-smbios under certain conditions.
> > 
> > I have worked around this in the past with things like the following:
> > 
> > #ifdef MODULE
> > #ifdef CONFIG_FOO_MODULE
> > 	if (request_module("foo"))
> > 		return -ENODEV;
> > #endif
> > #endif
> > 
> > Something like the above may be necessary for dell-smbios in dell-laptop,
> > dell-wmi, and dell-leds now that they depend on the dell-smbios exported
> > functions.
> > 
> > Cc Greg in case there is a better way to handle this that I'm not aware of.
> 
> No need to request_module anything, the symbol resolution should pull
> the dependant module in automagically when you do a 'modprobe',
> otherwise the symbols would never be found.

OK, Thank you Greg. Based on this, I have no concerns that haven't been addressed in the
pdx86/dell-smbios branch.

Michał, did you have any changes you wanted to make?

Pali, are you happy enough with this to add your reviewed-by?

I'm satisfied with the series and can push to next tomorrow if we're all in
agreement.

-- 
Darren Hart
Intel Open Source Technology Center

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

* Re: [PATCH v2 00/16] Common Dell SMBIOS API
  2016-02-08 20:46       ` Darren Hart
@ 2016-02-08 21:04         ` Pali Rohár
  2016-02-08 21:31           ` Darren Hart
  2016-02-09 14:15         ` Michał Kępień
  1 sibling, 1 reply; 33+ messages in thread
From: Pali Rohár @ 2016-02-08 21:04 UTC (permalink / raw)
  To: Darren Hart
  Cc: Greg Kroah-Hartman, Michał Kępień,
	Matthew Garrett, Richard Purdie, Jacek Anaszewski,
	platform-driver-x86, linux-leds, linux-kernel

[-- Attachment #1: Type: Text/Plain, Size: 617 bytes --]

On Monday 08 February 2016 21:46:46 Darren Hart wrote:
> Pali, are you happy enough with this to add your reviewed-by?

There was dicussion about dell-smbios API which you probably missed in
tons of other emails. It has subject:

"[PATCH 01/14] dell-laptop: extract SMBIOS-related code to a separate module"

And you can find it in archive at:
http://www.spinics.net/lists/linux-leds/msg05379.html

I wanted to hear your opinion about this API and I'm not 100% fine with
it but on other side it is not easy to design better... Maybe you could
have better idea.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

* Re: [PATCH v2 00/16] Common Dell SMBIOS API
  2016-02-08 21:04         ` Pali Rohár
@ 2016-02-08 21:31           ` Darren Hart
  0 siblings, 0 replies; 33+ messages in thread
From: Darren Hart @ 2016-02-08 21:31 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Greg Kroah-Hartman, Michał Kępień,
	Matthew Garrett, Richard Purdie, Jacek Anaszewski,
	platform-driver-x86, linux-leds, linux-kernel

On Mon, Feb 08, 2016 at 10:04:32PM +0100, Pali Rohár wrote:
> On Monday 08 February 2016 21:46:46 Darren Hart wrote:
> > Pali, are you happy enough with this to add your reviewed-by?
> 
> There was dicussion about dell-smbios API which you probably missed in
> tons of other emails. It has subject:
> 
> "[PATCH 01/14] dell-laptop: extract SMBIOS-related code to a separate module"
> 
> And you can find it in archive at:
> http://www.spinics.net/lists/linux-leds/msg05379.html
> 
> I wanted to hear your opinion about this API and I'm not 100% fine with
> it but on other side it is not easy to design better... Maybe you could
> have better idea.

I will review the above. However, a new API can be treated as separate from
the refactoring which is all this series really does. It doesn't do anything
that I saw beyond moving existing code into a separate module and wrapping the
use of the buffer and tokens. In that sense, it seems to me that this can be
considered a first step toward a redesigned API by first removing duplicate code
and reusing some of the existing code.

-- 
Darren Hart
Intel Open Source Technology Center

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

* Re: [PATCH v2 07/16] dell-smbios: don't return an SMBIOS buffer from dell_smbios_send_request()
  2016-02-08 18:44   ` Darren Hart
@ 2016-02-09 13:27     ` Michał Kępień
  2016-02-09 16:50       ` Darren Hart
  0 siblings, 1 reply; 33+ messages in thread
From: Michał Kępień @ 2016-02-09 13:27 UTC (permalink / raw)
  To: Darren Hart
  Cc: Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski, platform-driver-x86, linux-leds, linux-kernel

> > An SMBIOS buffer pointer does not need to be returned by
> > dell_smbios_send_request(), because SMBIOS call results are stored in
> > the buffer passed as input.
> 
> This should come before 6/16, update the commit message to reflect the module
> exported buffer (not the one passed as input), or possibly just merge this patch
> and 6/16 as correcting the use of SMBIOS buffer within the module.

I have only now noticed that I phrased the commit message for this patch
rather unfortunately as it inappropriately conveyed my reasoning.  What
I meant by "the buffer passed as input" was not "the buffer passed as an
argument to dell_smbios_send_request()", but rather "the buffer passed
to the SMI handler".  In other words, there is no reason to return a
buffer from dell_smbios_send_request(), because each caller will simply
find their output in the same buffer they used to provide input (no
matter whether the latter is passed as a function argument or accessed
using a module-wide variable).

Anyway, as even the above explanation is hardly a stellar demonstration
of clarity, I believe your idea of resolving this issue may simply be
the best one, thanks.

-- 
Best regards,
Michał Kępień

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

* Re: [PATCH v2 00/16] Common Dell SMBIOS API
  2016-02-08 20:46       ` Darren Hart
  2016-02-08 21:04         ` Pali Rohár
@ 2016-02-09 14:15         ` Michał Kępień
  1 sibling, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-02-09 14:15 UTC (permalink / raw)
  To: Darren Hart
  Cc: Greg Kroah-Hartman, Pali Rohár, Matthew Garrett,
	Richard Purdie, Jacek Anaszewski, platform-driver-x86,
	linux-leds, linux-kernel

> > > Most of my concerns were addressed by later patches in the series. I have pushed
> > > a version of this 1/7 fixed per lkp (linux/io.h) and 7/16 with a corrected body
> > > as I sent in reply to that patch. This is on my tree as the dell-smbios branch.

Thanks!

> > > 
> > > My only major concern is module load order dependencies. Inter-module
> > > dependencies are frowned upon with good reason, the kernel load ordering is
> > > non-deterministic and it's possible, for example, for dell-laptop to fail to
> > > find the symbols exported by dell-smbios under certain conditions.
> > > 
> > > I have worked around this in the past with things like the following:
> > > 
> > > #ifdef MODULE
> > > #ifdef CONFIG_FOO_MODULE
> > > 	if (request_module("foo"))
> > > 		return -ENODEV;
> > > #endif
> > > #endif
> > > 
> > > Something like the above may be necessary for dell-smbios in dell-laptop,
> > > dell-wmi, and dell-leds now that they depend on the dell-smbios exported
> > > functions.
> > > 
> > > Cc Greg in case there is a better way to handle this that I'm not aware of.
> > 
> > No need to request_module anything, the symbol resolution should pull
> > the dependant module in automagically when you do a 'modprobe',
> > otherwise the symbols would never be found.
> 
> OK, Thank you Greg. Based on this, I have no concerns that haven't been addressed in the
> pdx86/dell-smbios branch.
> 
> Michał, did you have any changes you wanted to make?

No, thanks, nothing new popped up.

-- 
Best regards,
Michał Kępień

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

* Re: [PATCH v2 07/16] dell-smbios: don't return an SMBIOS buffer from dell_smbios_send_request()
  2016-02-09 13:27     ` Michał Kępień
@ 2016-02-09 16:50       ` Darren Hart
  2016-02-09 17:34         ` Michał Kępień
  0 siblings, 1 reply; 33+ messages in thread
From: Darren Hart @ 2016-02-09 16:50 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski, platform-driver-x86, linux-leds, linux-kernel

On Tue, Feb 09, 2016 at 02:27:36PM +0100, Michał Kępień wrote:
> > > An SMBIOS buffer pointer does not need to be returned by
> > > dell_smbios_send_request(), because SMBIOS call results are stored in
> > > the buffer passed as input.
> > 
> > This should come before 6/16, update the commit message to reflect the module
> > exported buffer (not the one passed as input), or possibly just merge this patch
> > and 6/16 as correcting the use of SMBIOS buffer within the module.
> 
> I have only now noticed that I phrased the commit message for this patch
> rather unfortunately as it inappropriately conveyed my reasoning.  What
> I meant by "the buffer passed as input" was not "the buffer passed as an
> argument to dell_smbios_send_request()", but rather "the buffer passed
> to the SMI handler".  In other words, there is no reason to return a
> buffer from dell_smbios_send_request(), because each caller will simply
> find their output in the same buffer they used to provide input (no
> matter whether the latter is passed as a function argument or accessed
> using a module-wide variable).
> 
> Anyway, as even the above explanation is hardly a stellar demonstration
> of clarity, I believe your idea of resolving this issue may simply be
> the best one, thanks.

In my tree, I reworded this as:

    dell-smbios: don't return an SMBIOS buffer from dell_smbios_send_request()

    An SMBIOS buffer pointer does not need to be returned by
    dell_smbios_send_request(), because SMBIOS call results are stored in
    the buffer exported by the module.

Is that acceptable?

-- 
Darren Hart
Intel Open Source Technology Center

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

* Re: [PATCH v2 07/16] dell-smbios: don't return an SMBIOS buffer from dell_smbios_send_request()
  2016-02-09 16:50       ` Darren Hart
@ 2016-02-09 17:34         ` Michał Kępień
  0 siblings, 0 replies; 33+ messages in thread
From: Michał Kępień @ 2016-02-09 17:34 UTC (permalink / raw)
  To: Darren Hart
  Cc: Matthew Garrett, Pali Rohár, Richard Purdie,
	Jacek Anaszewski, platform-driver-x86, linux-leds, linux-kernel

> > > > An SMBIOS buffer pointer does not need to be returned by
> > > > dell_smbios_send_request(), because SMBIOS call results are stored in
> > > > the buffer passed as input.
> In my tree, I reworded this as:
> 
>     dell-smbios: don't return an SMBIOS buffer from dell_smbios_send_request()
> 
>     An SMBIOS buffer pointer does not need to be returned by
>     dell_smbios_send_request(), because SMBIOS call results are stored in
>     the buffer exported by the module.
> 
> Is that acceptable?

Sure, thanks.  I saw the edited commit message in your tree before
sending my previous message and I liked it.

-- 
Best regards,
Michał Kępień

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

end of thread, other threads:[~2016-02-09 17:34 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-22 14:27 [PATCH v2 00/16] Common Dell SMBIOS API Michał Kępień
2016-01-22 14:27 ` [PATCH v2 01/16] dell-laptop: extract SMBIOS-related code to a separate module Michał Kępień
2016-01-24  2:16   ` kbuild test robot
2016-01-26 13:33     ` Michał Kępień
2016-01-22 14:27 ` [PATCH v2 02/16] dell-smbios: rename get_buffer() to dell_smbios_get_buffer() Michał Kępień
2016-01-22 14:27 ` [PATCH v2 03/16] dell-smbios: rename clear_buffer() to dell_smbios_clear_buffer() Michał Kępień
2016-01-22 14:27 ` [PATCH v2 04/16] dell-smbios: rename release_buffer() to dell_smbios_release_buffer() Michał Kępień
2016-01-22 14:27 ` [PATCH v2 05/16] dell-smbios: rename dell_send_request() to dell_smbios_send_request() Michał Kępień
2016-01-22 14:27 ` [PATCH v2 06/16] dell-smbios: don't pass an SMBIOS buffer " Michał Kępień
2016-01-22 14:27 ` [PATCH v2 07/16] dell-smbios: don't return an SMBIOS buffer from dell_smbios_send_request() Michał Kępień
2016-02-08 18:44   ` Darren Hart
2016-02-09 13:27     ` Michał Kępień
2016-02-09 16:50       ` Darren Hart
2016-02-09 17:34         ` Michał Kępień
2016-01-22 14:27 ` [PATCH v2 08/16] dell-smbios: return the SMBIOS buffer from dell_smbios_get_buffer() Michał Kępień
2016-01-22 14:27 ` [PATCH v2 09/16] dell-smbios: make the SMBIOS buffer static Michał Kępień
2016-01-22 14:27 ` [PATCH v2 10/16] dell-smbios: implement new function for finding DMI table 0xDA tokens Michał Kępień
2016-01-22 14:27 ` [PATCH v2 11/16] dell-laptop: use dell_smbios_find_token() instead of find_token_id() Michał Kępień
2016-01-22 14:27 ` [PATCH v2 12/16] dell-laptop: use dell_smbios_find_token() instead of find_token_location() Michał Kępień
2016-01-22 14:27 ` [PATCH v2 13/16] dell-smbios: remove find_token_{id,location}() Michał Kępień
2016-01-22 14:27 ` [PATCH v2 14/16] dell-smbios: make da_tokens static Michał Kępień
2016-01-22 14:27 ` [PATCH v2 15/16] dell-led: use dell_smbios_find_token() for finding mic DMI tokens Michał Kępień
2016-01-22 14:27 ` [PATCH v2 16/16] dell-led: use dell_smbios_send_request() for performing SMBIOS calls Michał Kępień
2016-01-22 14:48 ` [PATCH v2 00/16] Common Dell SMBIOS API Pali Rohár
2016-02-07 20:34   ` Darren Hart
2016-02-08 19:20   ` Darren Hart
2016-02-08 19:29     ` Greg Kroah-Hartman
2016-02-08 20:46       ` Darren Hart
2016-02-08 21:04         ` Pali Rohár
2016-02-08 21:31           ` Darren Hart
2016-02-09 14:15         ` Michał Kępień
2016-02-08 19:30     ` Lukas Wunner
2016-02-08 20:43       ` 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.