[PATCH 00/14] Common Dell SMBIOS API

[PATCH 00/14] Common Dell SMBIOS API

[Michał Kępień]

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.  I decided to send
the whole series to everyone involved to provide context - my apologies
if this is frowned upon.

As for making dell-led dependent on a driver in drivers/platform/x86,
let me just hint that Pali and I think it could be possible to
eventually move all of dell-led's code to drivers/platform/x86.  But
first things first.

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.

Alex, as I don't have the hardware to test the changes in dell-led
(beyond compilation) and you contributed the parts of it which this
patch series changes, is there any way you might test it on relevant
hardware?

 drivers/leds/Kconfig               |    1 +
 drivers/leds/dell-led.c            |  125 ++--------
 drivers/platform/x86/Kconfig       |   12 +-
 drivers/platform/x86/Makefile      |    1 +
 drivers/platform/x86/dell-laptop.c |  444 ++++++++++++------------------------
 drivers/platform/x86/dell-smbios.c |  179 +++++++++++++++
 drivers/platform/x86/dell-smbios.h |   48 ++++
 7 files changed, 395 insertions(+), 415 deletions(-)
 create mode 100644 drivers/platform/x86/dell-smbios.c
 create mode 100644 drivers/platform/x86/dell-smbios.h

-- 
1.7.10.4

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

[Michał Kępień]

Extract SMBIOS-related code from dell-laptop to a new kernel module,
dell-smbios.  The static specifier was 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 |   51 ++++++++++
 5 files changed, 257 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 f37821f..177a794 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 8b8df29..1128595 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..260a32a
--- /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;
+
+static DEFINE_MUTEX(buffer_mutex);
+struct calling_interface_buffer *buffer;
+EXPORT_SYMBOL_GPL(buffer);
+
+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 clear_buffer(void)
+{
+	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+}
+EXPORT_SYMBOL_GPL(clear_buffer);
+
+void get_buffer(void)
+{
+	mutex_lock(&buffer_mutex);
+	clear_buffer();
+}
+EXPORT_SYMBOL_GPL(get_buffer);
+
+void release_buffer(void)
+{
+	mutex_unlock(&buffer_mutex);
+}
+EXPORT_SYMBOL_GPL(release_buffer);
+
+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);
+
+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);
+
+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..00e03b2
--- /dev/null
+++ b/drivers/platform/x86/dell-smbios.h
@@ -0,0 +1,51 @@
+/*
+ *  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 clear_buffer(void);
+void get_buffer(void);
+void release_buffer(void);
+
+int find_token_id(int tokenid);
+int find_token_location(int tokenid);
+
+struct calling_interface_buffer *
+dell_send_request(struct calling_interface_buffer *buffer, int class,
+		  int select);
+#endif
-- 
1.7.10.4

[PATCH 02/14] dell-smbios: don't pass a buffer to dell_send_request()

[Michał Kępień]

Passing a struct calling_interface_buffer pointer to dell_send_request()
is redundant as it should always operate on the 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 |    4 +---
 drivers/platform/x86/dell-smbios.h |    4 +---
 3 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index d45d356..572bdca 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)
 
 	get_buffer();
 
-	dell_send_request(buffer, 17, 11);
+	dell_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)
 	clear_buffer();
 
 	buffer->input[0] = 0x2;
-	dell_send_request(buffer, 17, 11);
+	dell_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)
 	clear_buffer();
 
 	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
-	dell_send_request(buffer, 17, 11);
+	dell_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);
 		clear_buffer();
 		buffer->input[0] = (1 | (radio << 8) | (block << 16));
-		dell_send_request(buffer, 17, 11);
+		dell_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)
 
 	get_buffer();
 
-	dell_send_request(buffer, 17, 11);
+	dell_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)
 	clear_buffer();
 
 	buffer->input[0] = 0x2;
-	dell_send_request(buffer, 17, 11);
+	dell_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)
 
 	get_buffer();
 
-	dell_send_request(buffer, 17, 11);
+	dell_send_request(17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
 
 	clear_buffer();
 
 	buffer->input[0] = 0x2;
-	dell_send_request(buffer, 17, 11);
+	dell_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)
 
 	get_buffer();
 
-	dell_send_request(buffer, 17, 11);
+	dell_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)
 	clear_buffer();
 
 	buffer->input[0] = 0x2;
-	dell_send_request(buffer, 17, 11);
+	dell_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;
 
 	get_buffer();
-	dell_send_request(buffer, 17, 11);
+	dell_send_request(17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
 	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_send_request(1, 2);
 	else
-		dell_send_request(buffer, 1, 1);
+		dell_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_send_request(buffer, 0, 2);
+		dell_send_request(0, 2);
 	else
-		dell_send_request(buffer, 0, 1);
+		dell_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)
 	get_buffer();
 
 	buffer->input[0] = 0x0;
-	dell_send_request(buffer, 4, 11);
+	dell_send_request(4, 11);
 	ret = buffer->output[0];
 
 	if (ret) {
@@ -1248,7 +1248,7 @@ static int kbd_get_state(struct kbd_state *state)
 	get_buffer();
 
 	buffer->input[0] = 0x1;
-	dell_send_request(buffer, 4, 11);
+	dell_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_send_request(buffer, 4, 11);
+	dell_send_request(4, 11);
 	ret = buffer->output[0];
 	release_buffer();
 
@@ -1326,7 +1326,7 @@ static int kbd_set_token_bit(u8 bit)
 	get_buffer();
 	buffer->input[0] = da_tokens[id].location;
 	buffer->input[1] = da_tokens[id].value;
-	dell_send_request(buffer, 1, 0);
+	dell_send_request(1, 0);
 	ret = buffer->output[0];
 	release_buffer();
 
@@ -1348,7 +1348,7 @@ static int kbd_get_token_bit(u8 bit)
 
 	get_buffer();
 	buffer->input[0] = da_tokens[id].location;
-	dell_send_request(buffer, 0, 0);
+	dell_send_request(0, 0);
 	ret = buffer->output[0];
 	val = buffer->output[1];
 	release_buffer();
@@ -2019,7 +2019,7 @@ static int __init dell_init(void)
 	if (token != -1) {
 		get_buffer();
 		buffer->input[0] = token;
-		dell_send_request(buffer, 0, 2);
+		dell_send_request(0, 2);
 		if (buffer->output[0] == 0)
 			max_intensity = buffer->output[3];
 		release_buffer();
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 260a32a..758680f 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -84,9 +84,7 @@ int find_token_location(int tokenid)
 }
 EXPORT_SYMBOL_GPL(find_token_location);
 
-struct calling_interface_buffer *
-dell_send_request(struct calling_interface_buffer *buffer, int class,
-		  int select)
+struct calling_interface_buffer *dell_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 00e03b2..0f58ce8 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -45,7 +45,5 @@ void release_buffer(void);
 int find_token_id(int tokenid);
 int find_token_location(int tokenid);
 
-struct calling_interface_buffer *
-dell_send_request(struct calling_interface_buffer *buffer, int class,
-		  int select);
+struct calling_interface_buffer *dell_send_request(int class, int select);
 #endif
-- 
1.7.10.4

[PATCH 03/14] dell-smbios: rename buffer to dell_smbios_buffer

[Michał Kępień]

As the SMBIOS buffer is exported from the module, it has to be renamed
to something less generic, so add a "dell_smbios_" prefix to the
variable name.

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

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 572bdca..aaef181 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -419,18 +419,18 @@ static int dell_rfkill_set(void *data, bool blocked)
 	get_buffer();
 
 	dell_send_request(17, 11);
-	ret = buffer->output[0];
-	status = buffer->output[1];
+	ret = dell_smbios_buffer->output[0];
+	status = dell_smbios_buffer->output[1];
 
 	if (ret != 0)
 		goto out;
 
 	clear_buffer();
 
-	buffer->input[0] = 0x2;
+	dell_smbios_buffer->input[0] = 0x2;
 	dell_send_request(17, 11);
-	ret = buffer->output[0];
-	hwswitch = buffer->output[1];
+	ret = dell_smbios_buffer->output[0];
+	hwswitch = dell_smbios_buffer->output[1];
 
 	/* If the hardware switch controls this radio, and the hardware
 	   switch is disabled, always disable the radio */
@@ -440,9 +440,9 @@ static int dell_rfkill_set(void *data, bool blocked)
 
 	clear_buffer();
 
-	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
+	dell_smbios_buffer->input[0] = (1 | (radio << 8) | (disable << 16));
 	dell_send_request(17, 11);
-	ret = buffer->output[0];
+	ret = dell_smbios_buffer->output[0];
 
  out:
 	release_buffer();
@@ -457,7 +457,8 @@ static void dell_rfkill_update_sw_state(struct rfkill *rfkill, int radio,
 		/* Has hw-switch, sync sw_state to BIOS */
 		int block = rfkill_blocked(rfkill);
 		clear_buffer();
-		buffer->input[0] = (1 | (radio << 8) | (block << 16));
+		dell_smbios_buffer->input[0] = (1 | (radio << 8)
+						  | (block << 16));
 		dell_send_request(17, 11);
 	} else {
 		/* No hw-switch, sync BIOS state to sw_state */
@@ -482,8 +483,8 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 	get_buffer();
 
 	dell_send_request(17, 11);
-	ret = buffer->output[0];
-	status = buffer->output[1];
+	ret = dell_smbios_buffer->output[0];
+	status = dell_smbios_buffer->output[1];
 
 	if (ret != 0 || !(status & BIT(0))) {
 		release_buffer();
@@ -492,10 +493,10 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 
 	clear_buffer();
 
-	buffer->input[0] = 0x2;
+	dell_smbios_buffer->input[0] = 0x2;
 	dell_send_request(17, 11);
-	ret = buffer->output[0];
-	hwswitch = buffer->output[1];
+	ret = dell_smbios_buffer->output[0];
+	hwswitch = dell_smbios_buffer->output[1];
 
 	release_buffer();
 
@@ -522,15 +523,15 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 	get_buffer();
 
 	dell_send_request(17, 11);
-	ret = buffer->output[0];
-	status = buffer->output[1];
+	ret = dell_smbios_buffer->output[0];
+	status = dell_smbios_buffer->output[1];
 
 	clear_buffer();
 
-	buffer->input[0] = 0x2;
+	dell_smbios_buffer->input[0] = 0x2;
 	dell_send_request(17, 11);
-	hwswitch_ret = buffer->output[0];
-	hwswitch_state = buffer->output[1];
+	hwswitch_ret = dell_smbios_buffer->output[0];
+	hwswitch_state = dell_smbios_buffer->output[1];
 
 	release_buffer();
 
@@ -620,20 +621,20 @@ static void dell_update_rfkill(struct work_struct *ignored)
 	get_buffer();
 
 	dell_send_request(17, 11);
-	ret = buffer->output[0];
-	status = buffer->output[1];
+	ret = dell_smbios_buffer->output[0];
+	status = dell_smbios_buffer->output[1];
 
 	if (ret != 0)
 		goto out;
 
 	clear_buffer();
 
-	buffer->input[0] = 0x2;
+	dell_smbios_buffer->input[0] = 0x2;
 	dell_send_request(17, 11);
-	ret = buffer->output[0];
+	ret = dell_smbios_buffer->output[0];
 
 	if (ret == 0 && (status & BIT(0)))
-		hwswitch = buffer->output[1];
+		hwswitch = dell_smbios_buffer->output[1];
 
 	if (wifi_rfkill) {
 		dell_rfkill_update_hw_state(wifi_rfkill, 1, status, hwswitch);
@@ -711,8 +712,8 @@ static int __init dell_setup_rfkill(void)
 
 	get_buffer();
 	dell_send_request(17, 11);
-	ret = buffer->output[0];
-	status = buffer->output[1];
+	ret = dell_smbios_buffer->output[0];
+	status = dell_smbios_buffer->output[1];
 	release_buffer();
 
 	/* dell wireless info smbios call is not supported */
@@ -874,15 +875,15 @@ static int dell_send_intensity(struct backlight_device *bd)
 		return -ENODEV;
 
 	get_buffer();
-	buffer->input[0] = token;
-	buffer->input[1] = bd->props.brightness;
+	dell_smbios_buffer->input[0] = token;
+	dell_smbios_buffer->input[1] = bd->props.brightness;
 
 	if (power_supply_is_system_supplied() > 0)
 		dell_send_request(1, 2);
 	else
 		dell_send_request(1, 1);
 
-	ret = dell_smi_error(buffer->output[0]);
+	ret = dell_smi_error(dell_smbios_buffer->output[0]);
 
 	release_buffer();
 	return ret;
@@ -898,17 +899,17 @@ static int dell_get_intensity(struct backlight_device *bd)
 		return -ENODEV;
 
 	get_buffer();
-	buffer->input[0] = token;
+	dell_smbios_buffer->input[0] = token;
 
 	if (power_supply_is_system_supplied() > 0)
 		dell_send_request(0, 2);
 	else
 		dell_send_request(0, 1);
 
-	if (buffer->output[0])
-		ret = dell_smi_error(buffer->output[0]);
+	if (dell_smbios_buffer->output[0])
+		ret = dell_smi_error(dell_smbios_buffer->output[0]);
 	else
-		ret = buffer->output[1];
+		ret = dell_smbios_buffer->output[1];
 
 	release_buffer();
 	return ret;
@@ -1159,29 +1160,29 @@ static int kbd_get_info(struct kbd_info *info)
 
 	get_buffer();
 
-	buffer->input[0] = 0x0;
+	dell_smbios_buffer->input[0] = 0x0;
 	dell_send_request(4, 11);
-	ret = buffer->output[0];
+	ret = dell_smbios_buffer->output[0];
 
 	if (ret) {
 		ret = dell_smi_error(ret);
 		goto out;
 	}
 
-	info->modes = buffer->output[1] & 0xFFFF;
-	info->type = (buffer->output[1] >> 24) & 0xFF;
-	info->triggers = buffer->output[2] & 0xFF;
-	units = (buffer->output[2] >> 8) & 0xFF;
-	info->levels = (buffer->output[2] >> 16) & 0xFF;
+	info->modes = dell_smbios_buffer->output[1] & 0xFFFF;
+	info->type = (dell_smbios_buffer->output[1] >> 24) & 0xFF;
+	info->triggers = dell_smbios_buffer->output[2] & 0xFF;
+	units = (dell_smbios_buffer->output[2] >> 8) & 0xFF;
+	info->levels = (dell_smbios_buffer->output[2] >> 16) & 0xFF;
 
 	if (units & BIT(0))
-		info->seconds = (buffer->output[3] >> 0) & 0xFF;
+		info->seconds = (dell_smbios_buffer->output[3] >> 0) & 0xFF;
 	if (units & BIT(1))
-		info->minutes = (buffer->output[3] >> 8) & 0xFF;
+		info->minutes = (dell_smbios_buffer->output[3] >> 8) & 0xFF;
 	if (units & BIT(2))
-		info->hours = (buffer->output[3] >> 16) & 0xFF;
+		info->hours = (dell_smbios_buffer->output[3] >> 16) & 0xFF;
 	if (units & BIT(3))
-		info->days = (buffer->output[3] >> 24) & 0xFF;
+		info->days = (dell_smbios_buffer->output[3] >> 24) & 0xFF;
 
  out:
 	release_buffer();
@@ -1247,25 +1248,25 @@ static int kbd_get_state(struct kbd_state *state)
 
 	get_buffer();
 
-	buffer->input[0] = 0x1;
+	dell_smbios_buffer->input[0] = 0x1;
 	dell_send_request(4, 11);
-	ret = buffer->output[0];
+	ret = dell_smbios_buffer->output[0];
 
 	if (ret) {
 		ret = dell_smi_error(ret);
 		goto out;
 	}
 
-	state->mode_bit = ffs(buffer->output[1] & 0xFFFF);
+	state->mode_bit = ffs(dell_smbios_buffer->output[1] & 0xFFFF);
 	if (state->mode_bit != 0)
 		state->mode_bit--;
 
-	state->triggers = (buffer->output[1] >> 16) & 0xFF;
-	state->timeout_value = (buffer->output[1] >> 24) & 0x3F;
-	state->timeout_unit = (buffer->output[1] >> 30) & 0x3;
-	state->als_setting = buffer->output[2] & 0xFF;
-	state->als_value = (buffer->output[2] >> 8) & 0xFF;
-	state->level = (buffer->output[2] >> 16) & 0xFF;
+	state->triggers = (dell_smbios_buffer->output[1] >> 16) & 0xFF;
+	state->timeout_value = (dell_smbios_buffer->output[1] >> 24) & 0x3F;
+	state->timeout_unit = (dell_smbios_buffer->output[1] >> 30) & 0x3;
+	state->als_setting = dell_smbios_buffer->output[2] & 0xFF;
+	state->als_value = (dell_smbios_buffer->output[2] >> 8) & 0xFF;
+	state->level = (dell_smbios_buffer->output[2] >> 16) & 0xFF;
 
  out:
 	release_buffer();
@@ -1277,15 +1278,15 @@ static int kbd_set_state(struct kbd_state *state)
 	int ret;
 
 	get_buffer();
-	buffer->input[0] = 0x2;
-	buffer->input[1] = BIT(state->mode_bit) & 0xFFFF;
-	buffer->input[1] |= (state->triggers & 0xFF) << 16;
-	buffer->input[1] |= (state->timeout_value & 0x3F) << 24;
-	buffer->input[1] |= (state->timeout_unit & 0x3) << 30;
-	buffer->input[2] = state->als_setting & 0xFF;
-	buffer->input[2] |= (state->level & 0xFF) << 16;
+	dell_smbios_buffer->input[0] = 0x2;
+	dell_smbios_buffer->input[1] = BIT(state->mode_bit) & 0xFFFF;
+	dell_smbios_buffer->input[1] |= (state->triggers & 0xFF) << 16;
+	dell_smbios_buffer->input[1] |= (state->timeout_value & 0x3F) << 24;
+	dell_smbios_buffer->input[1] |= (state->timeout_unit & 0x3) << 30;
+	dell_smbios_buffer->input[2] = state->als_setting & 0xFF;
+	dell_smbios_buffer->input[2] |= (state->level & 0xFF) << 16;
 	dell_send_request(4, 11);
-	ret = buffer->output[0];
+	ret = dell_smbios_buffer->output[0];
 	release_buffer();
 
 	return dell_smi_error(ret);
@@ -1324,10 +1325,10 @@ static int kbd_set_token_bit(u8 bit)
 		return -EINVAL;
 
 	get_buffer();
-	buffer->input[0] = da_tokens[id].location;
-	buffer->input[1] = da_tokens[id].value;
+	dell_smbios_buffer->input[0] = da_tokens[id].location;
+	dell_smbios_buffer->input[1] = da_tokens[id].value;
 	dell_send_request(1, 0);
-	ret = buffer->output[0];
+	ret = dell_smbios_buffer->output[0];
 	release_buffer();
 
 	return dell_smi_error(ret);
@@ -1347,10 +1348,10 @@ static int kbd_get_token_bit(u8 bit)
 		return -EINVAL;
 
 	get_buffer();
-	buffer->input[0] = da_tokens[id].location;
+	dell_smbios_buffer->input[0] = da_tokens[id].location;
 	dell_send_request(0, 0);
-	ret = buffer->output[0];
-	val = buffer->output[1];
+	ret = dell_smbios_buffer->output[0];
+	val = dell_smbios_buffer->output[1];
 	release_buffer();
 
 	if (ret)
@@ -2018,10 +2019,10 @@ static int __init dell_init(void)
 	token = find_token_location(BRIGHTNESS_TOKEN);
 	if (token != -1) {
 		get_buffer();
-		buffer->input[0] = token;
+		dell_smbios_buffer->input[0] = token;
 		dell_send_request(0, 2);
-		if (buffer->output[0] == 0)
-			max_intensity = buffer->output[3];
+		if (dell_smbios_buffer->output[0] == 0)
+			max_intensity = dell_smbios_buffer->output[3];
 		release_buffer();
 	}
 
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 758680f..a3898f9 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -31,8 +31,8 @@ struct calling_interface_structure {
 } __packed;
 
 static DEFINE_MUTEX(buffer_mutex);
-struct calling_interface_buffer *buffer;
-EXPORT_SYMBOL_GPL(buffer);
+struct calling_interface_buffer *dell_smbios_buffer;
+EXPORT_SYMBOL_GPL(dell_smbios_buffer);
 
 static int da_command_address;
 static int da_command_code;
@@ -42,7 +42,7 @@ EXPORT_SYMBOL_GPL(da_tokens);
 
 void clear_buffer(void)
 {
-	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+	memset(dell_smbios_buffer, 0, sizeof(struct calling_interface_buffer));
 }
 EXPORT_SYMBOL_GPL(clear_buffer);
 
@@ -91,15 +91,15 @@ struct calling_interface_buffer *dell_send_request(int class, int select)
 	command.magic = SMI_CMD_MAGIC;
 	command.command_address = da_command_address;
 	command.command_code = da_command_code;
-	command.ebx = virt_to_phys(buffer);
+	command.ebx = virt_to_phys(dell_smbios_buffer);
 	command.ecx = 0x42534931;
 
-	buffer->class = class;
-	buffer->select = select;
+	dell_smbios_buffer->class = class;
+	dell_smbios_buffer->select = select;
 
 	dcdbas_smi_request(&command);
 
-	return buffer;
+	return dell_smbios_buffer;
 }
 EXPORT_SYMBOL_GPL(dell_send_request);
 
@@ -162,8 +162,8 @@ static int __init dell_smbios_init(void)
 	 * 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) {
+	dell_smbios_buffer = (void *)__get_free_page(GFP_KERNEL | GFP_DMA32);
+	if (!dell_smbios_buffer) {
 		ret = -ENOMEM;
 		goto fail_buffer;
 	}
@@ -178,7 +178,7 @@ fail_buffer:
 static void __exit dell_smbios_exit(void)
 {
 	kfree(da_tokens);
-	free_page((unsigned long)buffer);
+	free_page((unsigned long)dell_smbios_buffer);
 }
 
 subsys_initcall(dell_smbios_init);
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 0f58ce8..89c787c 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -35,7 +35,7 @@ struct calling_interface_token {
 	};
 };
 
-extern struct calling_interface_buffer *buffer;
+extern struct calling_interface_buffer *dell_smbios_buffer;
 extern struct calling_interface_token *da_tokens;
 
 void clear_buffer(void);
-- 
1.7.10.4

[PATCH 04/14] dell-smbios: rename clear_buffer() to dell_smbios_clear_buffer()

[Michał Kępień]

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 aaef181..1bfc95e 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();
 
 	dell_smbios_buffer->input[0] = 0x2;
 	dell_send_request(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();
 
 	dell_smbios_buffer->input[0] = (1 | (radio << 8) | (disable << 16));
 	dell_send_request(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();
 		dell_smbios_buffer->input[0] = (1 | (radio << 8)
 						  | (block << 16));
 		dell_send_request(17, 11);
@@ -491,7 +491,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 		return;
 	}
 
-	clear_buffer();
+	dell_smbios_clear_buffer();
 
 	dell_smbios_buffer->input[0] = 0x2;
 	dell_send_request(17, 11);
@@ -526,7 +526,7 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 	ret = dell_smbios_buffer->output[0];
 	status = dell_smbios_buffer->output[1];
 
-	clear_buffer();
+	dell_smbios_clear_buffer();
 
 	dell_smbios_buffer->input[0] = 0x2;
 	dell_send_request(17, 11);
@@ -627,7 +627,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 	if (ret != 0)
 		goto out;
 
-	clear_buffer();
+	dell_smbios_clear_buffer();
 
 	dell_smbios_buffer->input[0] = 0x2;
 	dell_send_request(17, 11);
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index a3898f9..8fa740c 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -40,16 +40,16 @@ static int da_num_tokens;
 struct calling_interface_token *da_tokens;
 EXPORT_SYMBOL_GPL(da_tokens);
 
-void clear_buffer(void)
+void dell_smbios_clear_buffer(void)
 {
 	memset(dell_smbios_buffer, 0, sizeof(struct calling_interface_buffer));
 }
-EXPORT_SYMBOL_GPL(clear_buffer);
+EXPORT_SYMBOL_GPL(dell_smbios_clear_buffer);
 
 void get_buffer(void)
 {
 	mutex_lock(&buffer_mutex);
-	clear_buffer();
+	dell_smbios_clear_buffer();
 }
 EXPORT_SYMBOL_GPL(get_buffer);
 
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 89c787c..ba0bc22 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 *dell_smbios_buffer;
 extern struct calling_interface_token *da_tokens;
 
-void clear_buffer(void);
+void dell_smbios_clear_buffer(void);
 void get_buffer(void);
 void release_buffer(void);
 
-- 
1.7.10.4

[PATCH 05/14] dell-smbios: rename get_buffer() to dell_smbios_get_buffer()

[Michał Kępień]

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 1bfc95e..5df30a9 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(17, 11);
 	ret = dell_smbios_buffer->output[0];
@@ -480,7 +480,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 	int status;
 	int ret;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 
 	dell_send_request(17, 11);
 	ret = dell_smbios_buffer->output[0];
@@ -520,7 +520,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(17, 11);
 	ret = dell_smbios_buffer->output[0];
@@ -618,7 +618,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 	int status;
 	int ret;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 
 	dell_send_request(17, 11);
 	ret = dell_smbios_buffer->output[0];
@@ -710,7 +710,7 @@ static int __init dell_setup_rfkill(void)
 	if (!force_rfkill && !whitelisted)
 		return 0;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 	dell_send_request(17, 11);
 	ret = dell_smbios_buffer->output[0];
 	status = dell_smbios_buffer->output[1];
@@ -874,7 +874,7 @@ static int dell_send_intensity(struct backlight_device *bd)
 	if (token == -1)
 		return -ENODEV;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 	dell_smbios_buffer->input[0] = token;
 	dell_smbios_buffer->input[1] = bd->props.brightness;
 
@@ -898,7 +898,7 @@ static int dell_get_intensity(struct backlight_device *bd)
 	if (token == -1)
 		return -ENODEV;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 	dell_smbios_buffer->input[0] = token;
 
 	if (power_supply_is_system_supplied() > 0)
@@ -1158,7 +1158,7 @@ static int kbd_get_info(struct kbd_info *info)
 	u8 units;
 	int ret;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 
 	dell_smbios_buffer->input[0] = 0x0;
 	dell_send_request(4, 11);
@@ -1246,7 +1246,7 @@ static int kbd_get_state(struct kbd_state *state)
 {
 	int ret;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 
 	dell_smbios_buffer->input[0] = 0x1;
 	dell_send_request(4, 11);
@@ -1277,7 +1277,7 @@ static int kbd_set_state(struct kbd_state *state)
 {
 	int ret;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 	dell_smbios_buffer->input[0] = 0x2;
 	dell_smbios_buffer->input[1] = BIT(state->mode_bit) & 0xFFFF;
 	dell_smbios_buffer->input[1] |= (state->triggers & 0xFF) << 16;
@@ -1324,7 +1324,7 @@ static int kbd_set_token_bit(u8 bit)
 	if (id == -1)
 		return -EINVAL;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 	dell_smbios_buffer->input[0] = da_tokens[id].location;
 	dell_smbios_buffer->input[1] = da_tokens[id].value;
 	dell_send_request(1, 0);
@@ -1347,7 +1347,7 @@ static int kbd_get_token_bit(u8 bit)
 	if (id == -1)
 		return -EINVAL;
 
-	get_buffer();
+	dell_smbios_get_buffer();
 	dell_smbios_buffer->input[0] = da_tokens[id].location;
 	dell_send_request(0, 0);
 	ret = dell_smbios_buffer->output[0];
@@ -2018,7 +2018,7 @@ static int __init dell_init(void)
 
 	token = find_token_location(BRIGHTNESS_TOKEN);
 	if (token != -1) {
-		get_buffer();
+		dell_smbios_get_buffer();
 		dell_smbios_buffer->input[0] = token;
 		dell_send_request(0, 2);
 		if (dell_smbios_buffer->output[0] == 0)
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 8fa740c..27dbfb9 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -46,12 +46,12 @@ void dell_smbios_clear_buffer(void)
 }
 EXPORT_SYMBOL_GPL(dell_smbios_clear_buffer);
 
-void get_buffer(void)
+void dell_smbios_get_buffer(void)
 {
 	mutex_lock(&buffer_mutex);
 	dell_smbios_clear_buffer();
 }
-EXPORT_SYMBOL_GPL(get_buffer);
+EXPORT_SYMBOL_GPL(dell_smbios_get_buffer);
 
 void release_buffer(void)
 {
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index ba0bc22..e3c2014 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -39,7 +39,7 @@ extern struct calling_interface_buffer *dell_smbios_buffer;
 extern struct calling_interface_token *da_tokens;
 
 void dell_smbios_clear_buffer(void);
-void get_buffer(void);
+void dell_smbios_get_buffer(void);
 void release_buffer(void);
 
 int find_token_id(int tokenid);
-- 
1.7.10.4

[PATCH 06/14] dell-smbios: rename release_buffer() to dell_smbios_release_buffer()

[Michał Kępień]

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 5df30a9..ab256ae 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 = dell_smbios_buffer->output[0];
 
  out:
-	release_buffer();
+	dell_smbios_release_buffer();
 	return dell_smi_error(ret);
 }
 
@@ -487,7 +487,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 	status = dell_smbios_buffer->output[1];
 
 	if (ret != 0 || !(status & BIT(0))) {
-		release_buffer();
+		dell_smbios_release_buffer();
 		return;
 	}
 
@@ -498,7 +498,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 	ret = dell_smbios_buffer->output[0];
 	hwswitch = dell_smbios_buffer->output[1];
 
-	release_buffer();
+	dell_smbios_release_buffer();
 
 	if (ret != 0)
 		return;
@@ -533,7 +533,7 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 	hwswitch_ret = dell_smbios_buffer->output[0];
 	hwswitch_state = dell_smbios_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);
@@ -651,7 +651,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);
 
@@ -714,7 +714,7 @@ static int __init dell_setup_rfkill(void)
 	dell_send_request(17, 11);
 	ret = dell_smbios_buffer->output[0];
 	status = dell_smbios_buffer->output[1];
-	release_buffer();
+	dell_smbios_release_buffer();
 
 	/* dell wireless info smbios call is not supported */
 	if (ret != 0)
@@ -885,7 +885,7 @@ static int dell_send_intensity(struct backlight_device *bd)
 
 	ret = dell_smi_error(dell_smbios_buffer->output[0]);
 
-	release_buffer();
+	dell_smbios_release_buffer();
 	return ret;
 }
 
@@ -911,7 +911,7 @@ static int dell_get_intensity(struct backlight_device *bd)
 	else
 		ret = dell_smbios_buffer->output[1];
 
-	release_buffer();
+	dell_smbios_release_buffer();
 	return ret;
 }
 
@@ -1185,7 +1185,7 @@ static int kbd_get_info(struct kbd_info *info)
 		info->days = (dell_smbios_buffer->output[3] >> 24) & 0xFF;
 
  out:
-	release_buffer();
+	dell_smbios_release_buffer();
 	return ret;
 }
 
@@ -1269,7 +1269,7 @@ static int kbd_get_state(struct kbd_state *state)
 	state->level = (dell_smbios_buffer->output[2] >> 16) & 0xFF;
 
  out:
-	release_buffer();
+	dell_smbios_release_buffer();
 	return ret;
 }
 
@@ -1287,7 +1287,7 @@ static int kbd_set_state(struct kbd_state *state)
 	dell_smbios_buffer->input[2] |= (state->level & 0xFF) << 16;
 	dell_send_request(4, 11);
 	ret = dell_smbios_buffer->output[0];
-	release_buffer();
+	dell_smbios_release_buffer();
 
 	return dell_smi_error(ret);
 }
@@ -1329,7 +1329,7 @@ static int kbd_set_token_bit(u8 bit)
 	dell_smbios_buffer->input[1] = da_tokens[id].value;
 	dell_send_request(1, 0);
 	ret = dell_smbios_buffer->output[0];
-	release_buffer();
+	dell_smbios_release_buffer();
 
 	return dell_smi_error(ret);
 }
@@ -1352,7 +1352,7 @@ static int kbd_get_token_bit(u8 bit)
 	dell_send_request(0, 0);
 	ret = dell_smbios_buffer->output[0];
 	val = dell_smbios_buffer->output[1];
-	release_buffer();
+	dell_smbios_release_buffer();
 
 	if (ret)
 		return dell_smi_error(ret);
@@ -2023,7 +2023,7 @@ static int __init dell_init(void)
 		dell_send_request(0, 2);
 		if (dell_smbios_buffer->output[0] == 0)
 			max_intensity = dell_smbios_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 27dbfb9..20d7f21 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -53,11 +53,11 @@ void dell_smbios_get_buffer(void)
 }
 EXPORT_SYMBOL_GPL(dell_smbios_get_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);
 
 int find_token_id(int tokenid)
 {
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index e3c2014..1ffd680 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_clear_buffer(void);
 void dell_smbios_get_buffer(void);
-void release_buffer(void);
+void dell_smbios_release_buffer(void);
 
 int find_token_id(int tokenid);
 int find_token_location(int tokenid);
-- 
1.7.10.4

[PATCH 07/14] dell-smbios: rename dell_send_request() to dell_smbios_send_request()

[Michał Kępień]

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 |    5 +++--
 drivers/platform/x86/dell-smbios.h |    3 ++-
 3 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index ab256ae..65aa1ff 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(17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = dell_smbios_buffer->output[0];
 	status = dell_smbios_buffer->output[1];
 
@@ -428,7 +428,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 	dell_smbios_clear_buffer();
 
 	dell_smbios_buffer->input[0] = 0x2;
-	dell_send_request(17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = dell_smbios_buffer->output[0];
 	hwswitch = dell_smbios_buffer->output[1];
 
@@ -441,7 +441,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 	dell_smbios_clear_buffer();
 
 	dell_smbios_buffer->input[0] = (1 | (radio << 8) | (disable << 16));
-	dell_send_request(17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = dell_smbios_buffer->output[0];
 
  out:
@@ -459,7 +459,7 @@ static void dell_rfkill_update_sw_state(struct rfkill *rfkill, int radio,
 		dell_smbios_clear_buffer();
 		dell_smbios_buffer->input[0] = (1 | (radio << 8)
 						  | (block << 16));
-		dell_send_request(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)));
@@ -482,7 +482,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 
 	dell_smbios_get_buffer();
 
-	dell_send_request(17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = dell_smbios_buffer->output[0];
 	status = dell_smbios_buffer->output[1];
 
@@ -494,7 +494,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 	dell_smbios_clear_buffer();
 
 	dell_smbios_buffer->input[0] = 0x2;
-	dell_send_request(17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = dell_smbios_buffer->output[0];
 	hwswitch = dell_smbios_buffer->output[1];
 
@@ -522,14 +522,14 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 
 	dell_smbios_get_buffer();
 
-	dell_send_request(17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = dell_smbios_buffer->output[0];
 	status = dell_smbios_buffer->output[1];
 
 	dell_smbios_clear_buffer();
 
 	dell_smbios_buffer->input[0] = 0x2;
-	dell_send_request(17, 11);
+	dell_smbios_send_request(17, 11);
 	hwswitch_ret = dell_smbios_buffer->output[0];
 	hwswitch_state = dell_smbios_buffer->output[1];
 
@@ -620,7 +620,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 
 	dell_smbios_get_buffer();
 
-	dell_send_request(17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = dell_smbios_buffer->output[0];
 	status = dell_smbios_buffer->output[1];
 
@@ -630,7 +630,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 	dell_smbios_clear_buffer();
 
 	dell_smbios_buffer->input[0] = 0x2;
-	dell_send_request(17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = dell_smbios_buffer->output[0];
 
 	if (ret == 0 && (status & BIT(0)))
@@ -711,7 +711,7 @@ static int __init dell_setup_rfkill(void)
 		return 0;
 
 	dell_smbios_get_buffer();
-	dell_send_request(17, 11);
+	dell_smbios_send_request(17, 11);
 	ret = dell_smbios_buffer->output[0];
 	status = dell_smbios_buffer->output[1];
 	dell_smbios_release_buffer();
@@ -879,9 +879,9 @@ static int dell_send_intensity(struct backlight_device *bd)
 	dell_smbios_buffer->input[1] = bd->props.brightness;
 
 	if (power_supply_is_system_supplied() > 0)
-		dell_send_request(1, 2);
+		dell_smbios_send_request(1, 2);
 	else
-		dell_send_request(1, 1);
+		dell_smbios_send_request(1, 1);
 
 	ret = dell_smi_error(dell_smbios_buffer->output[0]);
 
@@ -902,9 +902,9 @@ static int dell_get_intensity(struct backlight_device *bd)
 	dell_smbios_buffer->input[0] = token;
 
 	if (power_supply_is_system_supplied() > 0)
-		dell_send_request(0, 2);
+		dell_smbios_send_request(0, 2);
 	else
-		dell_send_request(0, 1);
+		dell_smbios_send_request(0, 1);
 
 	if (dell_smbios_buffer->output[0])
 		ret = dell_smi_error(dell_smbios_buffer->output[0]);
@@ -1161,7 +1161,7 @@ static int kbd_get_info(struct kbd_info *info)
 	dell_smbios_get_buffer();
 
 	dell_smbios_buffer->input[0] = 0x0;
-	dell_send_request(4, 11);
+	dell_smbios_send_request(4, 11);
 	ret = dell_smbios_buffer->output[0];
 
 	if (ret) {
@@ -1249,7 +1249,7 @@ static int kbd_get_state(struct kbd_state *state)
 	dell_smbios_get_buffer();
 
 	dell_smbios_buffer->input[0] = 0x1;
-	dell_send_request(4, 11);
+	dell_smbios_send_request(4, 11);
 	ret = dell_smbios_buffer->output[0];
 
 	if (ret) {
@@ -1285,7 +1285,7 @@ static int kbd_set_state(struct kbd_state *state)
 	dell_smbios_buffer->input[1] |= (state->timeout_unit & 0x3) << 30;
 	dell_smbios_buffer->input[2] = state->als_setting & 0xFF;
 	dell_smbios_buffer->input[2] |= (state->level & 0xFF) << 16;
-	dell_send_request(4, 11);
+	dell_smbios_send_request(4, 11);
 	ret = dell_smbios_buffer->output[0];
 	dell_smbios_release_buffer();
 
@@ -1327,7 +1327,7 @@ static int kbd_set_token_bit(u8 bit)
 	dell_smbios_get_buffer();
 	dell_smbios_buffer->input[0] = da_tokens[id].location;
 	dell_smbios_buffer->input[1] = da_tokens[id].value;
-	dell_send_request(1, 0);
+	dell_smbios_send_request(1, 0);
 	ret = dell_smbios_buffer->output[0];
 	dell_smbios_release_buffer();
 
@@ -1349,7 +1349,7 @@ static int kbd_get_token_bit(u8 bit)
 
 	dell_smbios_get_buffer();
 	dell_smbios_buffer->input[0] = da_tokens[id].location;
-	dell_send_request(0, 0);
+	dell_smbios_send_request(0, 0);
 	ret = dell_smbios_buffer->output[0];
 	val = dell_smbios_buffer->output[1];
 	dell_smbios_release_buffer();
@@ -2020,7 +2020,7 @@ static int __init dell_init(void)
 	if (token != -1) {
 		dell_smbios_get_buffer();
 		dell_smbios_buffer->input[0] = token;
-		dell_send_request(0, 2);
+		dell_smbios_send_request(0, 2);
 		if (dell_smbios_buffer->output[0] == 0)
 			max_intensity = dell_smbios_buffer->output[3];
 		dell_smbios_release_buffer();
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 20d7f21..597a29f 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -84,7 +84,8 @@ int find_token_location(int tokenid)
 }
 EXPORT_SYMBOL_GPL(find_token_location);
 
-struct calling_interface_buffer *dell_send_request(int class, int select)
+struct calling_interface_buffer *dell_smbios_send_request(int class,
+							  int select)
 {
 	struct smi_cmd command;
 
@@ -101,7 +102,7 @@ struct calling_interface_buffer *dell_send_request(int class, int select)
 
 	return dell_smbios_buffer;
 }
-EXPORT_SYMBOL_GPL(dell_send_request);
+EXPORT_SYMBOL_GPL(dell_smbios_send_request);
 
 static void __init parse_da_table(const struct dmi_header *dm)
 {
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 1ffd680..5ccc0ea 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -45,5 +45,6 @@ void dell_smbios_release_buffer(void);
 int find_token_id(int tokenid);
 int find_token_location(int tokenid);
 
-struct calling_interface_buffer *dell_send_request(int class, int select);
+struct calling_interface_buffer *dell_smbios_send_request(int class,
+							  int select);
 #endif
-- 
1.7.10.4

[PATCH 08/14] dell-smbios: implement new function for finding DMI table 0xDA tokens

[Michał Kępień]

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 597a29f..83e35ed 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -59,6 +59,19 @@ void dell_smbios_release_buffer(void)
 }
 EXPORT_SYMBOL_GPL(dell_smbios_release_buffer);
 
+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 5ccc0ea..23f6e95 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_get_buffer(void);
 void dell_smbios_release_buffer(void);
 
+struct calling_interface_token *dell_smbios_find_token(int tokenid);
+
 int find_token_id(int tokenid);
 int find_token_location(int tokenid);
 
-- 
1.7.10.4

[PATCH 09/14] dell-laptop: use dell_smbios_find_token() instead of find_token_id()

[Michał Kępień]

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 65aa1ff..dfe2afd 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -1314,19 +1314,19 @@ static int kbd_set_state_safe(struct kbd_state *state, struct kbd_state *old)
 
 static int kbd_set_token_bit(u8 bit)
 {
-	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;
 
 	dell_smbios_get_buffer();
-	dell_smbios_buffer->input[0] = da_tokens[id].location;
-	dell_smbios_buffer->input[1] = da_tokens[id].value;
+	dell_smbios_buffer->input[0] = token->location;
+	dell_smbios_buffer->input[1] = token->value;
 	dell_smbios_send_request(1, 0);
 	ret = dell_smbios_buffer->output[0];
 	dell_smbios_release_buffer();
@@ -1336,19 +1336,19 @@ static int kbd_set_token_bit(u8 bit)
 
 static int kbd_get_token_bit(u8 bit)
 {
-	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;
 
 	dell_smbios_get_buffer();
-	dell_smbios_buffer->input[0] = da_tokens[id].location;
+	dell_smbios_buffer->input[0] = token->location;
 	dell_smbios_send_request(0, 0);
 	ret = dell_smbios_buffer->output[0];
 	val = dell_smbios_buffer->output[1];
@@ -1357,7 +1357,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)
@@ -1459,7 +1459,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

[PATCH 10/14] dell-laptop: use dell_smbios_find_token() instead of find_token_location()

[Michał Kępień]

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 dfe2afd..4de61b7 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -867,15 +867,15 @@ static void dell_cleanup_rfkill(void)
 
 static int dell_send_intensity(struct backlight_device *bd)
 {
-	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;
 
 	dell_smbios_get_buffer();
-	dell_smbios_buffer->input[0] = token;
+	dell_smbios_buffer->input[0] = token->location;
 	dell_smbios_buffer->input[1] = bd->props.brightness;
 
 	if (power_supply_is_system_supplied() > 0)
@@ -891,15 +891,15 @@ static int dell_send_intensity(struct backlight_device *bd)
 
 static int dell_get_intensity(struct backlight_device *bd)
 {
-	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;
 
 	dell_smbios_get_buffer();
-	dell_smbios_buffer->input[0] = token;
+	dell_smbios_buffer->input[0] = token->location;
 
 	if (power_supply_is_system_supplied() > 0)
 		dell_smbios_send_request(0, 2);
@@ -1973,8 +1973,8 @@ static void kbd_led_exit(void)
 
 static int __init dell_init(void)
 {
+	struct calling_interface_token *token;
 	int max_intensity = 0;
-	int token;
 	int ret;
 
 	if (!dmi_check_system(dell_device_table))
@@ -2016,10 +2016,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) {
 		dell_smbios_get_buffer();
-		dell_smbios_buffer->input[0] = token;
+		dell_smbios_buffer->input[0] = token->location;
 		dell_smbios_send_request(0, 2);
 		if (dell_smbios_buffer->output[0] == 0)
 			max_intensity = dell_smbios_buffer->output[3];
-- 
1.7.10.4

[PATCH 11/14] dell-smbios: remove find_token_{id,location}()

[Michał Kępień]

As find_token_id() and find_token_location() were only used in
dell-laptop, which has been changed to use dell_smbios_find_token()
instead, the old 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 83e35ed..f506a20 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -72,31 +72,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);
-
 struct calling_interface_buffer *dell_smbios_send_request(int class,
 							  int select)
 {
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 23f6e95..3e39e13 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -44,9 +44,6 @@ void dell_smbios_release_buffer(void);
 
 struct calling_interface_token *dell_smbios_find_token(int tokenid);
 
-int find_token_id(int tokenid);
-int find_token_location(int tokenid);
-
 struct calling_interface_buffer *dell_smbios_send_request(int class,
 							  int select);
 #endif
-- 
1.7.10.4

[PATCH 12/14] dell-smbios: make da_tokens static

[Michał Kępień]

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 |    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 f506a20..7da7363 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -37,8 +37,7 @@ EXPORT_SYMBOL_GPL(dell_smbios_buffer);
 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;
 
 void dell_smbios_clear_buffer(void)
 {
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 3e39e13..b0e1730 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -36,7 +36,6 @@ struct calling_interface_token {
 };
 
 extern struct calling_interface_buffer *dell_smbios_buffer;
-extern struct calling_interface_token *da_tokens;
 
 void dell_smbios_clear_buffer(void);
 void dell_smbios_get_buffer(void);
-- 
1.7.10.4

[PATCH 13/14] dell-led: use dell_smbios_find_token() for finding mic DMI tokens

[Michał Kępień]

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>
---
 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 b1ab8bd..171810c 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -441,6 +441,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

[PATCH 14/14] dell-led: use dell_smbios_send_request() for SMBIOS requests

[Michał Kępień]

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

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/leds/dell-led.c |   64 ++++-------------------------------------------
 1 file changed, 5 insertions(+), 59 deletions(-)

diff --git a/drivers/leds/dell-led.c b/drivers/leds/dell-led.c
index bfa7511..a9c04bf 100644
--- a/drivers/leds/dell-led.c
+++ b/drivers/leds/dell-led.c
@@ -43,64 +43,12 @@ 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_token *token;
-	struct app_wmi_args args;
 
 	if (!wmi_has_guid(DELL_APP_GUID))
 		return -ENODEV;
@@ -115,13 +63,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);
+	dell_smbios_get_buffer();
+	dell_smbios_buffer->input[0] = token->location;
+	dell_smbios_buffer->input[1] = token->value;
+	dell_smbios_send_request(1, 0);
+	dell_smbios_release_buffer();
 
 	return state;
 }
-- 
1.7.10.4

Re: [PATCH 00/14] Common Dell SMBIOS API

[Darren Hart]

On Tue, Jan 12, 2016 at 03:02:46PM +0100, Michał Kępień wrote:
> 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.  I decided to send
> the whole series to everyone involved to provide context - my apologies
> if this is frowned upon.

I much prefer it, thank you.

> 
> As for making dell-led dependent on a driver in drivers/platform/x86,
> let me just hint that Pali and I think it could be possible to
> eventually move all of dell-led's code to drivers/platform/x86.  But
> first things first.
> 
> 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.

Noted, thanks.

> 
> Alex, as I don't have the hardware to test the changes in dell-led
> (beyond compilation) and you contributed the parts of it which this
> patch series changes, is there any way you might test it on relevant
> hardware?

OK, before I dive into a review on this, I'm going to be looking for an ack from
Pali and some Tested-by from the usual suspects. We're already into the merge
window and this series needs to spend some time in next. We'll plan on getting
this into next after the merge window closes and have it land in 4.6. Hopefully
that will allow us to work through Andy's wmi rearchitecting at the same time
and catch any incompatibilities without introducing undue churn to mainline.

> 
>  drivers/leds/Kconfig               |    1 +
>  drivers/leds/dell-led.c            |  125 ++--------
>  drivers/platform/x86/Kconfig       |   12 +-
>  drivers/platform/x86/Makefile      |    1 +
>  drivers/platform/x86/dell-laptop.c |  444 ++++++++++++------------------------
>  drivers/platform/x86/dell-smbios.c |  179 +++++++++++++++
>  drivers/platform/x86/dell-smbios.h |   48 ++++
>  7 files changed, 395 insertions(+), 415 deletions(-)

My favorite kind of patch              ^ :-)

Thanks!

>  create mode 100644 drivers/platform/x86/dell-smbios.c
>  create mode 100644 drivers/platform/x86/dell-smbios.h
> 
> -- 
> 1.7.10.4
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

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

[Pali Rohár]

On Tuesday 12 January 2016 15:02:47 Michał Kępień wrote:
> Extract SMBIOS-related code from dell-laptop to a new kernel module,
> dell-smbios.  The static specifier was 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 |   51 ++++++++++
>  5 files changed, 257 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 f37821f..177a794 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 8b8df29..1128595 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..260a32a
> --- /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;
> +
> +static DEFINE_MUTEX(buffer_mutex);
> +struct calling_interface_buffer *buffer;
> +EXPORT_SYMBOL_GPL(buffer);
> +
> +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 clear_buffer(void)
> +{
> +	memset(buffer, 0, sizeof(struct calling_interface_buffer));
> +}
> +EXPORT_SYMBOL_GPL(clear_buffer);
> +
> +void get_buffer(void)
> +{
> +	mutex_lock(&buffer_mutex);
> +	clear_buffer();
> +}
> +EXPORT_SYMBOL_GPL(get_buffer);
> +
> +void release_buffer(void)
> +{
> +	mutex_unlock(&buffer_mutex);
> +}
> +EXPORT_SYMBOL_GPL(release_buffer);
> +
> +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);
> +
> +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);
> +
> +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..00e03b2
> --- /dev/null
> +++ b/drivers/platform/x86/dell-smbios.h
> @@ -0,0 +1,51 @@
> +/*
> + *  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;
> +	};
> +};

After patch 12/14 you do not need to define this struct in header file.

> +extern struct calling_interface_buffer *buffer;
> +extern struct calling_interface_token *da_tokens;

Better hide this variable in dell-smbios.c code ...

> +void clear_buffer(void);
> +void get_buffer(void);
> +void release_buffer(void);

... and let those functions to get parameter to buffer.

E.g. get_buffer will return buffer and other two functions will take
buffer parameter.

> +int find_token_id(int tokenid);
> +int find_token_location(int tokenid);
> +
> +struct calling_interface_buffer *
> +dell_send_request(struct calling_interface_buffer *buffer, int class,
> +		  int select);
> +#endif

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

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

[Michał Kępień]

Hi Pali,

Thanks for taking a look.

> > +struct calling_interface_token {
> > +	u16 tokenID;
> > +	u16 location;
> > +	union {
> > +		u16 value;
> > +		u16 stringlength;
> > +	};
> > +};
> 
> After patch 12/14 you do not need to define this struct in header file.

dell_smbios_find_token() returns a struct calling_interface_token *,
which can then be dereferenced by the caller.  See patch 13.

> > +extern struct calling_interface_buffer *buffer;
> > +extern struct calling_interface_token *da_tokens;
> 
> Better hide this variable in dell-smbios.c code ...

Patch 12 makes da_tokens static, but I believe you were referring to
buffer.

> > +void clear_buffer(void);
> > +void get_buffer(void);
> > +void release_buffer(void);
> 
> ... and let those functions to get parameter to buffer.
> 
> E.g. get_buffer will return buffer and other two functions will take
> buffer parameter.

You're right.  My original approach looked tempting because it reduces
the amount of changes required in dell-laptop, but on second thought, it
_assumes_ that users of this API would play nicely with the SMBIOS
buffer while your way _enforces_ it.

I will prepare a v2 including this suggestion within the next couple of
days.

-- 
Best regards,
Michał Kępień

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

[Michał Kępień]

> > +extern struct calling_interface_buffer *buffer;
> > +extern struct calling_interface_token *da_tokens;
> 
> Better hide this variable in dell-smbios.c code ...
> 
> > +void clear_buffer(void);
> > +void get_buffer(void);
> > +void release_buffer(void);
> 
> ... and let those functions to get parameter to buffer.
> 
> E.g. get_buffer will return buffer and other two functions will take
> buffer parameter.

Before I spam everyone with another set of 15 patches, I'd like to
discuss this a bit further.  There is no point in passing the buffer to
release_buffer(), because it only unlocks a mutex.  I also see no point
in passing the buffer to clear_buffer() and dell_send_request(), because
there is always just one buffer to operate on.

A total of four functions have something to do with the SMBIOS buffer:

  * get_buffer()
  * clear_buffer()
  * release_buffer()
  * dell_send_request()

This rework is a chance to make them all consistent, i.e. remove the
SMBIOS buffer from their argument lists.  This way we can "signal" this
API's users that there is only one SMBIOS buffer ever involved while
still removing the extern and EXPORT_SYMBOL_GPL for the buffer.  BTW, I
also see little point in returning the buffer from dell_send_request()
as none of its callers in dell-laptop assign its return value to
anything (i.e. there is no "buffer = dell_send_request(buffer, ...)" in
the code).

To sum up, I'd suggest that function prototypes could look like this:

    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);

What do you think?

-- 
Best regards,
Michał Kępień

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

[Pali Rohár]

On Wednesday 20 January 2016 10:21:07 Michał Kępień wrote:
> > > +extern struct calling_interface_buffer *buffer;
> > > +extern struct calling_interface_token *da_tokens;
> > 
> > Better hide this variable in dell-smbios.c code ...
> > 
> > > +void clear_buffer(void);
> > > +void get_buffer(void);
> > > +void release_buffer(void);
> > 
> > ... and let those functions to get parameter to buffer.
> > 
> > E.g. get_buffer will return buffer and other two functions will take
> > buffer parameter.
> 
> Before I spam everyone with another set of 15 patches, I'd like to
> discuss this a bit further.  There is no point in passing the buffer to
> release_buffer(), because it only unlocks a mutex.  I also see no point
> in passing the buffer to clear_buffer() and dell_send_request(), because
> there is always just one buffer to operate on.
> 
> A total of four functions have something to do with the SMBIOS buffer:
> 
>   * get_buffer()
>   * clear_buffer()
>   * release_buffer()
>   * dell_send_request()
> 
> This rework is a chance to make them all consistent, i.e. remove the
> SMBIOS buffer from their argument lists.  This way we can "signal" this
> API's users that there is only one SMBIOS buffer ever involved while
> still removing the extern and EXPORT_SYMBOL_GPL for the buffer.  BTW, I
> also see little point in returning the buffer from dell_send_request()
> as none of its callers in dell-laptop assign its return value to
> anything (i.e. there is no "buffer = dell_send_request(buffer, ...)" in
> the code).
> 
> To sum up, I'd suggest that function prototypes could look like this:
> 
>     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);
> 
> What do you think?
> 

In other scenario functions should do something like this:

struct buf *buf_alloc(void);
buf_clear(struct buf *buf);
buf_free(struct buf *buf);
buf_do_something(struct buf *buf, ...);

But here I do not know how hard is to create alloc/free functions and
what is cost for creating that buffer in first 4GB memory...

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

Re: [PATCH 13/14] dell-led: use dell_smbios_find_token() for finding mic DMI tokens

[Jacek Anaszewski]

Hi Michał,

Thanks for the patches.
They should probably be merged through linux-platform-drivers-x86.git.

Feel free to add my

Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>

to 13/14 and 14/14.

Best Regards,
Jacek Anaszewski

On 01/12/2016 03:02 PM, Michał Kępień wrote:
> 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>
> ---
>   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 b1ab8bd..171810c 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -441,6 +441,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)
>

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

[Michał Kępień]

> > A total of four functions have something to do with the SMBIOS buffer:
> > 
> >   * get_buffer()
> >   * clear_buffer()
> >   * release_buffer()
> >   * dell_send_request()
> > 
> > This rework is a chance to make them all consistent, i.e. remove the
> > SMBIOS buffer from their argument lists.  This way we can "signal" this
> > API's users that there is only one SMBIOS buffer ever involved while
> > still removing the extern and EXPORT_SYMBOL_GPL for the buffer.  BTW, I
> > also see little point in returning the buffer from dell_send_request()
> > as none of its callers in dell-laptop assign its return value to
> > anything (i.e. there is no "buffer = dell_send_request(buffer, ...)" in
> > the code).
> > 
> > To sum up, I'd suggest that function prototypes could look like this:
> > 
> >     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);
> > 
> > What do you think?
> > 
> 
> In other scenario functions should do something like this:
> 
> struct buf *buf_alloc(void);
> buf_clear(struct buf *buf);
> buf_free(struct buf *buf);
> buf_do_something(struct buf *buf, ...);
> 
> But here I do not know how hard is to create alloc/free functions and
> what is cost for creating that buffer in first 4GB memory...

I'm guessing the cost is negligible, given that SMBIOS calls are not
present in any hot path.  Writing these functions is also pretty
straightforward, but the inconvenience of this approach is that it
forces the callers to do the error-checking for each buf_alloc() call.
It also seems pretty inefficient - notice we only need 36 bytes for the
calling interface buffer, yet we would be allocating a whole page in
each buf_alloc() call.

On the other hand, I believe returning a separate buffer for each
buf_alloc() caller makes it possible to drop buffer_mutex altogether.
Yet, the approach I suggested is more similar to what the Dell-supplied
dcdbas driver does internally (it manages a single, resizable buffer,
which is protected by a mutex and controllable from userspace through
sysfs), which is why I think it's a good idea to stick to that concept
for consistency.

As this patch series already touches a lot of code, I would like to
avoid changing the underlying concepts as much as possible.  If that's
okay with you, I'll post a v2 which includes your suggestion to make the
buffer pointer static while keeping the interface similar to the
original one.  If you would really like me to take a different path,
please let me know and I'll comply.

-- 
Best regards,
Michał Kępień

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

[Pali Rohár]

On Thursday 21 January 2016 14:06:03 Michał Kępień wrote:
> > > A total of four functions have something to do with the SMBIOS buffer:
> > > 
> > >   * get_buffer()
> > >   * clear_buffer()
> > >   * release_buffer()
> > >   * dell_send_request()
> > > 
> > > This rework is a chance to make them all consistent, i.e. remove the
> > > SMBIOS buffer from their argument lists.  This way we can "signal" this
> > > API's users that there is only one SMBIOS buffer ever involved while
> > > still removing the extern and EXPORT_SYMBOL_GPL for the buffer.  BTW, I
> > > also see little point in returning the buffer from dell_send_request()
> > > as none of its callers in dell-laptop assign its return value to
> > > anything (i.e. there is no "buffer = dell_send_request(buffer, ...)" in
> > > the code).
> > > 
> > > To sum up, I'd suggest that function prototypes could look like this:
> > > 
> > >     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);
> > > 
> > > What do you think?
> > > 
> > 
> > In other scenario functions should do something like this:
> > 
> > struct buf *buf_alloc(void);
> > buf_clear(struct buf *buf);
> > buf_free(struct buf *buf);
> > buf_do_something(struct buf *buf, ...);
> > 
> > But here I do not know how hard is to create alloc/free functions and
> > what is cost for creating that buffer in first 4GB memory...
> 
> I'm guessing the cost is negligible, given that SMBIOS calls are not
> present in any hot path.  Writing these functions is also pretty
> straightforward, but the inconvenience of this approach is that it
> forces the callers to do the error-checking for each buf_alloc() call.
> It also seems pretty inefficient - notice we only need 36 bytes for the
> calling interface buffer, yet we would be allocating a whole page in
> each buf_alloc() call.
> 
> On the other hand, I believe returning a separate buffer for each
> buf_alloc() caller makes it possible to drop buffer_mutex altogether.
> Yet, the approach I suggested is more similar to what the Dell-supplied
> dcdbas driver does internally (it manages a single, resizable buffer,
> which is protected by a mutex and controllable from userspace through
> sysfs), which is why I think it's a good idea to stick to that concept
> for consistency.
> 
> As this patch series already touches a lot of code, I would like to
> avoid changing the underlying concepts as much as possible.  If that's
> okay with you, I'll post a v2 which includes your suggestion to make the
> buffer pointer static while keeping the interface similar to the
> original one.  If you would really like me to take a different path,
> please let me know and I'll comply.
> 

Another idea:

What about passing struct calling_interface_buffer from caller allocated
memory (either from stack or kernel alloc) to dell-smbios which will
copy it into own buffer under 4GB and then pass it to dcdbas?

This will avoid to use that get/release function and there will be only
one send_request.

But I will let decision for API to other people as I do not know what
the best API to use here...

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

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

[Michał Kępień]

> Another idea:
> 
> What about passing struct calling_interface_buffer from caller allocated
> memory (either from stack or kernel alloc) to dell-smbios which will
> copy it into own buffer under 4GB and then pass it to dcdbas?
> 
> This will avoid to use that get/release function and there will be only
> one send_request.

Well, yes, these two functions could then be ripped out, but the callers
would have to do the error checking on their own.  Of course that's not
a bad thing per se, but it changes the currently used concept.

> But I will let decision for API to other people as I do not know what
> the best API to use here...

In order to avoid delaying this any further, I'll post a v2 soon and
hopefully it'll be good enough for your Acked-by.  If it turns out more
people have misgivings about it, I'll adjust the code.

-- 
Best regards,
Michał Kępień

Re: [PATCH 13/14] dell-led: use dell_smbios_find_token() for finding mic DMI tokens

[Michał Kępień]

> Thanks for the patches.
> They should probably be merged through linux-platform-drivers-x86.git.
> 
> Feel free to add my
> 
> Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> 
> to 13/14 and 14/14.

Thanks for reviewing.  I will soon post a v2 of this series.  Given that
the only difference between v1 and v2 for dell-led will be a subtle
difference in API use, I'll allow myself to use your Acked-by for the
last two patches when I post v2.  Please let me know if that's
inappropriate.

-- 
Best regards,
Michał Kępień

Re: [PATCH 13/14] dell-led: use dell_smbios_find_token() for finding mic DMI tokens

[Jacek Anaszewski]

On 01/21/2016 04:00 PM, Michał Kępień wrote:
>> Thanks for the patches.
>> They should probably be merged through linux-platform-drivers-x86.git.
>>
>> Feel free to add my
>>
>> Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>>
>> to 13/14 and 14/14.
>
> Thanks for reviewing.  I will soon post a v2 of this series.  Given that
> the only difference between v1 and v2 for dell-led will be a subtle
> difference in API use, I'll allow myself to use your Acked-by for the
> last two patches when I post v2.  Please let me know if that's
> inappropriate.
>

Yes, feel free to carry my acks.

-- 
Best Regards,
Jacek Anaszewski

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

[Darren Hart]

On Thu, Jan 21, 2016 at 02:39:21PM +0100, Michał Kępień wrote:
> > Another idea:
> > 
> > What about passing struct calling_interface_buffer from caller allocated
> > memory (either from stack or kernel alloc) to dell-smbios which will
> > copy it into own buffer under 4GB and then pass it to dcdbas?
> > 
> > This will avoid to use that get/release function and there will be only
> > one send_request.
> 
> Well, yes, these two functions could then be ripped out, but the callers
> would have to do the error checking on their own.  Of course that's not
> a bad thing per se, but it changes the currently used concept.
> 
> > But I will let decision for API to other people as I do not know what
> > the best API to use here...
> 
> In order to avoid delaying this any further, I'll post a v2 soon and
> hopefully it'll be good enough for your Acked-by.  If it turns out more
> people have misgivings about it, I'll adjust the code.
> 

It seems to me the question is primarily over whether or not the interface
protects a shared resource (a common buffer) or if that buffer should be
independent for every caller.

I favor minimal and incremental change and avoiding complexity where it is not
needed. I don't believe there is anything performance critical in any of these
paths, e.g. nothing requires a better response time than about 100ms and nothing
is likely to occur at a frequency above 10Hz or so.

Assuming the above is an accurate view, I don't see any reason to go beyond the
minimal change to the existing SMBIOS code to make it a usable API. If the need
arises, we can always make such optimizations and performance improvements
later. This is an internal API and we can change it whenever we need to so long
as we update the call sites.

Does anyone have a compelling reason to look for changes to the v2 submitted by
Michał?

-- 
Darren Hart
Intel Open Source Technology Center

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

[Pali Rohár]

On Monday 08 February 2016 13:42:10 Darren Hart wrote:
> Assuming the above is an accurate view, I don't see any reason to go beyond the
> minimal change to the existing SMBIOS code to make it a usable API. If the need
> arises, we can always make such optimizations and performance improvements
> later. This is an internal API and we can change it whenever we need to so long
> as we update the call sites.

Problem is that now smbios code from dell-laptop.c is moved into
dell-smbios.c and dell-smbios.h and LED subsystem starts using
dell-smbios.h. In this case I'm thinking that we have something like API
usable by other modules/subsystem. And I'm thinking if it is not better
to create "correct" API now instead rewriting code in LED and platform
subsystem again later... As this API needs to provide just 1 function,
send command to Dell SMBIOS I think that API is still minimal. Currently
we have another two functions alloc/free buffer (needed for send).

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

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

[Michał Kępień]

> > Assuming the above is an accurate view, I don't see any reason to go beyond the
> > minimal change to the existing SMBIOS code to make it a usable API. If the need
> > arises, we can always make such optimizations and performance improvements
> > later. This is an internal API and we can change it whenever we need to so long
> > as we update the call sites.
> 
> Problem is that now smbios code from dell-laptop.c is moved into
> dell-smbios.c and dell-smbios.h and LED subsystem starts using
> dell-smbios.h. In this case I'm thinking that we have something like API
> usable by other modules/subsystem. And I'm thinking if it is not better
> to create "correct" API now instead rewriting code in LED and platform
> subsystem again later... As this API needs to provide just 1 function,
> send command to Dell SMBIOS I think that API is still minimal. Currently
> we have another two functions alloc/free buffer (needed for send).

Pali, forgive me, but I fail to understand your last two sentences.
Currently, we don't have functions for allocating/freeing the SMBIOS
buffer as it is allocated in dell_init() (or dell_smbios_init() after
applying this patch series) and freed in dell_(smbios_)exit().

In your previous messages [1][2], you suggested that we could allocate a
separate SMBIOS buffer for each caller or copy input parameters from
caller-allocated memory into a module-wide SMBIOS buffer before
performing the SMBIOS request.  Did you just mean to remind Darren of
these ideas or did you have something else on your mind?  If the latter,
could you please try rephrasing the last two sentences of your message?

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

-- 
Best regards,
Michał Kępień

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

[Darren Hart]

On Tue, Feb 09, 2016 at 09:33:03AM +0100, Pali Rohár wrote:
> On Monday 08 February 2016 13:42:10 Darren Hart wrote:
> > Assuming the above is an accurate view, I don't see any reason to go beyond the
> > minimal change to the existing SMBIOS code to make it a usable API. If the need
> > arises, we can always make such optimizations and performance improvements
> > later. This is an internal API and we can change it whenever we need to so long
> > as we update the call sites.
> 
> Problem is that now smbios code from dell-laptop.c is moved into
> dell-smbios.c and dell-smbios.h and LED subsystem starts using
> dell-smbios.h. In this case I'm thinking that we have something like API
> usable by other modules/subsystem. And I'm thinking if it is not better
> to create "correct" API now instead rewriting code in LED and platform
> subsystem again later... As this API needs to provide just 1 function,
> send command to Dell SMBIOS I think that API is still minimal. Currently
> we have another two functions alloc/free buffer (needed for send).

The internal kernel API changes all the time, we are not bound to it beyond
ensuring we update the internal users when we change it. I prefer not to
introduce complexity until we have to.

	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();

The get_buffer and release_buffer also include the locking which is necessary
for a shared buffer. If you eliminate the shared buffer, then you have to have a
local buffer, which adds back code to create the buffer, initializize
it, free it if it's dynamic, etc.

So from that sense, Michał's API seems at least as concise as the alternative,
and it introduces less change.

-- 
Darren Hart
Intel Open Source Technology Center

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

[Pali Rohár]

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

On Tuesday 09 February 2016 17:51:06 Darren Hart wrote:
> On Tue, Feb 09, 2016 at 09:33:03AM +0100, Pali Rohár wrote:
> > On Monday 08 February 2016 13:42:10 Darren Hart wrote:
> > > Assuming the above is an accurate view, I don't see any reason to
> > > go beyond the minimal change to the existing SMBIOS code to make
> > > it a usable API. If the need arises, we can always make such
> > > optimizations and performance improvements later. This is an
> > > internal API and we can change it whenever we need to so long as
> > > we update the call sites.
> > 
> > Problem is that now smbios code from dell-laptop.c is moved into
> > dell-smbios.c and dell-smbios.h and LED subsystem starts using
> > dell-smbios.h. In this case I'm thinking that we have something
> > like API usable by other modules/subsystem. And I'm thinking if it
> > is not better to create "correct" API now instead rewriting code
> > in LED and platform subsystem again later... As this API needs to
> > provide just 1 function, send command to Dell SMBIOS I think that
> > API is still minimal. Currently we have another two functions
> > alloc/free buffer (needed for send).
> 
> The internal kernel API changes all the time, we are not bound to it
> beyond ensuring we update the internal users when we change it. I
> prefer not to introduce complexity until we have to.
> 
> 	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();
> 
> The get_buffer and release_buffer also include the locking which is
> necessary for a shared buffer. If you eliminate the shared buffer,
> then you have to have a local buffer, which adds back code to create
> the buffer, initializize it, free it if it's dynamic, etc.
> 
> So from that sense, Michał's API seems at least as concise as the
> alternative, and it introduces less change.

Ok. Then let's stay with it. I have not tested patches yet, but do not 
see anything wrong. So go ahead you can add my Reviewed-by.

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

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

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

[Darren Hart]

On Tue, Feb 09, 2016 at 08:12:07PM +0100, Pali Rohár wrote:
> On Tuesday 09 February 2016 17:51:06 Darren Hart wrote:
> > On Tue, Feb 09, 2016 at 09:33:03AM +0100, Pali Rohár wrote:
> > > On Monday 08 February 2016 13:42:10 Darren Hart wrote:
> > > > Assuming the above is an accurate view, I don't see any reason to
> > > > go beyond the minimal change to the existing SMBIOS code to make
> > > > it a usable API. If the need arises, we can always make such
> > > > optimizations and performance improvements later. This is an
> > > > internal API and we can change it whenever we need to so long as
> > > > we update the call sites.
> > > 
> > > Problem is that now smbios code from dell-laptop.c is moved into
> > > dell-smbios.c and dell-smbios.h and LED subsystem starts using
> > > dell-smbios.h. In this case I'm thinking that we have something
> > > like API usable by other modules/subsystem. And I'm thinking if it
> > > is not better to create "correct" API now instead rewriting code
> > > in LED and platform subsystem again later... As this API needs to
> > > provide just 1 function, send command to Dell SMBIOS I think that
> > > API is still minimal. Currently we have another two functions
> > > alloc/free buffer (needed for send).
> > 
> > The internal kernel API changes all the time, we are not bound to it
> > beyond ensuring we update the internal users when we change it. I
> > prefer not to introduce complexity until we have to.
> > 
> > 	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();
> > 
> > The get_buffer and release_buffer also include the locking which is
> > necessary for a shared buffer. If you eliminate the shared buffer,
> > then you have to have a local buffer, which adds back code to create
> > the buffer, initializize it, free it if it's dynamic, etc.
> > 
> > So from that sense, Michał's API seems at least as concise as the
> > alternative, and it introduces less change.
> 
> Ok. Then let's stay with it. I have not tested patches yet, but do not 
> see anything wrong. So go ahead you can add my Reviewed-by.

Great. Thank you Pali.

This has been queued to testing.

Thank you all for your diligence in getting this series to this point.

-- 
Darren Hart
Intel Open Source Technology Center