[PATCH 0/4] Fixes for dell-laptop.c driver

[PATCH 0/4] Fixes for dell-laptop.c driver

[Pali Rohár]

This patch series contains contains two fixes (properly check return
values of all SMBIOS calls and fix calling correct page alloc/free
functions) and documentation update together with extending debugfs
code. Patch which fixes calling page alloc/free functions should be
backported to stable kernel series as it it fix kernel crash.

Pali Rohár (4):
  dell-laptop: Update information about wireless control
  dell-laptop: Check return value of all SMBIOS calls
  dell-laptop: Fix allocating & freeing SMI buffer page
  dell-laptop: Show info about WiGig and UWB in debugfs

 drivers/platform/x86/dell-laptop.c |  244 ++++++++++++++++++++++++++++--------
 1 file changed, 193 insertions(+), 51 deletions(-)

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] dell-laptop: Update information about wireless control

[Pali Rohár]

Make sure that all existing SMBIOS calls for wireless control are properly
documented. This commit also add new documentation released by Dell.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-laptop.c |  158 +++++++++++++++++++++++++++---------
 1 file changed, 119 insertions(+), 39 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 83e3d7f..ab89103 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -423,45 +423,125 @@ static inline int dell_smi_error(int value)
 	}
 }
 
-/* Derived from information in DellWirelessCtl.cpp:
-   Class 17, select 11 is radio control. It returns an array of 32-bit values.
-
-   Input byte 0 = 0: Wireless information
-
-   result[0]: return code
-   result[1]:
-     Bit 0:      Hardware switch supported
-     Bit 1:      Wifi locator supported
-     Bit 2:      Wifi is supported
-     Bit 3:      Bluetooth is supported
-     Bit 4:      WWAN is supported
-     Bit 5:      Wireless keyboard supported
-     Bits 6-7:   Reserved
-     Bit 8:      Wifi is installed
-     Bit 9:      Bluetooth is installed
-     Bit 10:     WWAN is installed
-     Bits 11-15: Reserved
-     Bit 16:     Hardware switch is on
-     Bit 17:     Wifi is blocked
-     Bit 18:     Bluetooth is blocked
-     Bit 19:     WWAN is blocked
-     Bits 20-31: Reserved
-   result[2]: NVRAM size in bytes
-   result[3]: NVRAM format version number
-
-   Input byte 0 = 2: Wireless switch configuration
-   result[0]: return code
-   result[1]:
-     Bit 0:      Wifi controlled by switch
-     Bit 1:      Bluetooth controlled by switch
-     Bit 2:      WWAN controlled by switch
-     Bits 3-6:   Reserved
-     Bit 7:      Wireless switch config locked
-     Bit 8:      Wifi locator enabled
-     Bits 9-14:  Reserved
-     Bit 15:     Wifi locator setting locked
-     Bits 16-31: Reserved
-*/
+/*
+ * Derived from information in smbios-wireless-ctl:
+ *
+ * cbSelect 17, Value 11
+ *
+ * Return Wireless Info
+ * cbArg1, byte0 = 0x00
+ *
+ *     cbRes1 Standard return codes (0, -1, -2)
+ *     cbRes2 Info bit flags:
+ *
+ *     0 Hardware switch supported (1)
+ *     1 WiFi locator supported (1)
+ *     2 WLAN supported (1)
+ *     3 Bluetooth (BT) supported (1)
+ *     4 WWAN supported (1)
+ *     5 Wireless KBD supported (1)
+ *     6 Uw b supported (1)
+ *     7 WiGig supported (1)
+ *     8 WLAN installed (1)
+ *     9 BT installed (1)
+ *     10 WWAN installed (1)
+ *     11 Uw b installed (1)
+ *     12 WiGig installed (1)
+ *     13-15 Reserved (0)
+ *     16 Hardware (HW) switch is On (1)
+ *     17 WLAN disabled (1)
+ *     18 BT disabled (1)
+ *     19 WWAN disabled (1)
+ *     20 Uw b disabled (1)
+ *     21 WiGig disabled (1)
+ *     20-31 Reserved (0)
+ *
+ *     cbRes3 NVRAM size in bytes
+ *     cbRes4, byte 0 NVRAM format version number
+ *
+ *
+ * Set QuickSet Radio Disable Flag
+ *     cbArg1, byte0 = 0x01
+ *     cbArg1, byte1
+ *     Radio ID     value:
+ *     0        Radio Status
+ *     1        WLAN ID
+ *     2        BT ID
+ *     3        WWAN ID
+ *     4        UWB ID
+ *     5        WIGIG ID
+ *     cbArg1, byte2    Flag bits:
+ *             0 QuickSet disables radio (1)
+ *             1-7 Reserved (0)
+ *
+ *     cbRes1    Standard return codes (0, -1, -2)
+ *     cbRes2    QuickSet (QS) radio disable bit map:
+ *     0 QS disables WLAN
+ *     1 QS disables BT
+ *     2 QS disables WWAN
+ *     3 QS disables UWB
+ *     4 QS disables WIGIG
+ *     5-31 Reserved (0)
+ *
+ * Wireless Switch Configuration
+ *     cbArg1, byte0 = 0x02
+ *
+ *     cbArg1, byte1
+ *     Subcommand:
+ *     0 Get config
+ *     1 Set config
+ *     2 Set WiFi locator enable/disable
+ *     cbArg1,byte2
+ *     Switch settings (if byte 1==1):
+ *     0 WLAN sw itch control (1)
+ *     1 BT sw itch control (1)
+ *     2 WWAN sw itch control (1)
+ *     3 UWB sw itch control (1)
+ *     4 WiGig sw itch control (1)
+ *     5-7 Reserved (0)
+ *    cbArg1, byte2 Enable bits (if byte 1==2):
+ *     0 Enable WiFi locator (1)
+ *
+ *    cbRes1     Standard return codes (0, -1, -2)
+ *    cbRes2 QuickSet radio disable bit map:
+ *     0 WLAN controlled by sw itch (1)
+ *     1 BT controlled by sw itch (1)
+ *     2 WWAN controlled by sw itch (1)
+ *     3 UWB controlled by sw itch (1)
+ *     4 WiGig controlled by sw itch (1)
+ *     5-6 Reserved (0)
+ *     7 Wireless sw itch config locked (1)
+ *     8 WiFi locator enabled (1)
+ *     9-14 Reserved (0)
+ *     15 WiFi locator setting locked (1)
+ *     16-31 Reserved (0)
+ *
+ * Read Local Config Data (LCD)
+ *     cbArg1, byte0 = 0x10
+ *     cbArg1, byte1 NVRAM index low byte
+ *     cbArg1, byte2 NVRAM index high byte
+ *     cbRes1 Standard return codes (0, -1, -2)
+ *     cbRes2 4 bytes read from LCD[index]
+ *     cbRes3 4 bytes read from LCD[index+4]
+ *     cbRes4 4 bytes read from LCD[index+8]
+ *
+ * Write Local Config Data (LCD)
+ *     cbArg1, byte0 = 0x11
+ *     cbArg1, byte1 NVRAM index low byte
+ *     cbArg1, byte2 NVRAM index high byte
+ *     cbArg2 4 bytes to w rite at LCD[index]
+ *     cbArg3 4 bytes to w rite at LCD[index+4]
+ *     cbArg4 4 bytes to w rite at LCD[index+8]
+ *     cbRes1 Standard return codes (0, -1, -2)
+ *
+ * Populate Local Config Data from NVRAM
+ *     cbArg1, byte0 = 0x12
+ *     cbRes1 Standard return codes (0, -1, -2)
+ *
+ * Commit Local Config Data to NVRAM
+ *     cbArg1, byte0 = 0x13
+ *     cbRes1 Standard return codes (0, -1, -2)
+ */
 
 static int dell_rfkill_set(void *data, bool blocked)
 {
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] dell-laptop: Update information about wireless control

[Darren Hart]

On Sun, Jun 21, 2015 at 10:39:26AM +0200, Pali Rohár wrote:
> Make sure that all existing SMBIOS calls for wireless control are properly
> documented. This commit also add new documentation released by Dell.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

Queued, thanks.

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/4] dell-laptop: Check return value of all SMBIOS calls

[Pali Rohár]

Make sure that return value of all SMBIOS calls are properly checked and
do not continue of processing (received) information if call failed.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-laptop.c |   61 +++++++++++++++++++++++++++++++-----
 1 file changed, 54 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index ab89103..aaef335 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -548,21 +548,27 @@ static int dell_rfkill_set(void *data, bool blocked)
 	int disable = blocked ? 1 : 0;
 	unsigned long radio = (unsigned long)data;
 	int hwswitch_bit = (unsigned long)data - 1;
+	int status;
+	int ret;
 
 	get_buffer();
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+	status = buffer->output[1];
 
 	/* If the hardware switch controls this radio, and the hardware
 	   switch is disabled, always disable the radio */
-	if ((hwswitch_state & BIT(hwswitch_bit)) &&
-	    !(buffer->output[1] & BIT(16)))
+	if (ret == 0 &&
+	    (hwswitch_state & BIT(hwswitch_bit)) &&
+	    !(status & BIT(16)))
 		disable = 1;
 
 	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 
 	release_buffer();
-	return 0;
+	return dell_smi_error(ret);
 }
 
 /* Must be called with the buffer held */
@@ -590,14 +596,18 @@ static void dell_rfkill_update_hw_state(struct rfkill *rfkill, int radio,
 static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 {
 	int status;
+	int ret;
 
 	get_buffer();
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
+	release_buffer();
 
-	dell_rfkill_update_hw_state(rfkill, (unsigned long)data, status);
+	if (ret != 0)
+		return;
 
-	release_buffer();
+	dell_rfkill_update_hw_state(rfkill, (unsigned long)data, status);
 }
 
 static const struct rfkill_ops dell_rfkill_ops = {
@@ -610,12 +620,15 @@ static struct dentry *dell_laptop_dir;
 static int dell_debugfs_show(struct seq_file *s, void *data)
 {
 	int status;
+	int ret;
 
 	get_buffer();
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
 	release_buffer();
 
+	seq_printf(s, "return:\t%d\n", ret);
 	seq_printf(s, "status:\t0x%X\n", status);
 	seq_printf(s, "Bit 0 : Hardware switch supported:   %lu\n",
 		   status & BIT(0));
@@ -677,11 +690,16 @@ static const struct file_operations dell_debugfs_fops = {
 static void dell_update_rfkill(struct work_struct *ignored)
 {
 	int status;
+	int ret;
 
 	get_buffer();
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
 
+	if (ret != 0)
+		goto out;
+
 	if (wifi_rfkill) {
 		dell_rfkill_update_hw_state(wifi_rfkill, 1, status);
 		dell_rfkill_update_sw_state(wifi_rfkill, 1, status);
@@ -695,6 +713,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 		dell_rfkill_update_sw_state(wwan_rfkill, 3, status);
 	}
 
+ out:
 	release_buffer();
 }
 static DECLARE_DELAYED_WORK(dell_rfkill_work, dell_update_rfkill);
@@ -755,13 +774,35 @@ static int __init dell_setup_rfkill(void)
 		return 0;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
+	if (ret != 0) {
+		/* dell wireless info smbios call is not working */
+		/* so there is no support for rfkill */
+		release_buffer();
+		return 0;
+	}
+
 	buffer->input[0] = 0x2;
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	hwswitch_state = buffer->output[1];
+
 	release_buffer();
 
+	if (ret != 0) {
+		/* dell wireless switch smbios call is not working */
+		if (force_rfkill) {
+			/* clear all hw-controlled bits */
+			hwswitch_state &= ~7;
+		} else {
+			/* rfkill is only tested on laptops with a hwswitch */
+			return 0;
+		}
+	}
+
 	if (!(status & BIT(0))) {
 		if (force_rfkill) {
 			/* No hwsitch, clear all hw-controlled bits */
@@ -931,6 +972,8 @@ static int dell_send_intensity(struct backlight_device *bd)
 	else
 		dell_send_request(buffer, 1, 1);
 
+	ret = dell_smi_error(buffer->output[0]);
+
  out:
 	release_buffer();
 	return ret;
@@ -953,7 +996,10 @@ static int dell_get_intensity(struct backlight_device *bd)
 	else
 		dell_send_request(buffer, 0, 1);
 
-	ret = buffer->output[1];
+	if (buffer->output[0])
+		ret = dell_smi_error(buffer->output[0]);
+	else
+		ret = buffer->output[1];
 
  out:
 	release_buffer();
@@ -2087,7 +2133,8 @@ static int __init dell_init(void)
 	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
 	if (buffer->input[0] != -1) {
 		dell_send_request(buffer, 0, 2);
-		max_intensity = buffer->output[3];
+		if (buffer->output[0] == 0)
+			max_intensity = buffer->output[3];
 	}
 	release_buffer();
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/4] dell-laptop: Check return value of all SMBIOS calls

[Darren Hart]

On Sun, Jun 21, 2015 at 10:39:27AM +0200, Pali Rohár wrote:
> Make sure that return value of all SMBIOS calls are properly checked and
> do not continue of processing (received) information if call failed.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/platform/x86/dell-laptop.c |   61 +++++++++++++++++++++++++++++++-----
>  1 file changed, 54 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c

...

> @@ -677,11 +690,16 @@ static const struct file_operations dell_debugfs_fops = {
>  static void dell_update_rfkill(struct work_struct *ignored)
>  {
>  	int status;
> +	int ret;
>  
>  	get_buffer();
>  	dell_send_request(buffer, 17, 11);
> +	ret = buffer->output[0];
>  	status = buffer->output[1];

Parallel to previous blocks, you can release_buffer() here...

>  
> +	if (ret != 0)
> +		goto out;

And just return here

> +
>  	if (wifi_rfkill) {
>  		dell_rfkill_update_hw_state(wifi_rfkill, 1, status);
>  		dell_rfkill_update_sw_state(wifi_rfkill, 1, status);
> @@ -695,6 +713,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
>  		dell_rfkill_update_sw_state(wwan_rfkill, 3, status);
>  	}
>  
> + out:

And drop this label.

>  	release_buffer();
>  }
>  static DECLARE_DELAYED_WORK(dell_rfkill_work, dell_update_rfkill);
> @@ -755,13 +774,35 @@ static int __init dell_setup_rfkill(void)
>  		return 0;
>  
>  	get_buffer();
> +
>  	dell_send_request(buffer, 17, 11);
> +	ret = buffer->output[0];
>  	status = buffer->output[1];
> +	if (ret != 0) {
> +		/* dell wireless info smbios call is not working */
> +		/* so there is no support for rfkill */

Please follow coding style for multi-line comments.

> +		release_buffer();
> +		return 0;
> +	}
> +
>  	buffer->input[0] = 0x2;
>  	dell_send_request(buffer, 17, 11);
> +	ret = buffer->output[0];
>  	hwswitch_state = buffer->output[1];
> +
>  	release_buffer();
>  
> +	if (ret != 0) {

Just "if (ret)" is more typical

> +		/* dell wireless switch smbios call is not working */
> +		if (force_rfkill) {
> +			/* clear all hw-controlled bits */
> +			hwswitch_state &= ~7;
> +		} else {
> +			/* rfkill is only tested on laptops with a hwswitch */
> +			return 0;
> +		}

Save an additional indent block and all the braces with:

		/* rfkill is only tested on laptops with a hwswitch */
		if (!force_rfkill)
			return 0

		/* clear all hw-controlled bits */
		hwswitch_state &= ~7;


> +	}
> +
>  	if (!(status & BIT(0))) {
>  		if (force_rfkill) {
>  			/* No hwsitch, clear all hw-controlled bits */
> @@ -931,6 +972,8 @@ static int dell_send_intensity(struct backlight_device *bd)
>  	else
>  		dell_send_request(buffer, 1, 1);
>  
> +	ret = dell_smi_error(buffer->output[0]);
> +
>   out:
>  	release_buffer();
>  	return ret;
> @@ -953,7 +996,10 @@ static int dell_get_intensity(struct backlight_device *bd)
>  	else
>  		dell_send_request(buffer, 0, 1);
>  
> -	ret = buffer->output[1];
> +	if (buffer->output[0])
> +		ret = dell_smi_error(buffer->output[0]);
> +	else
> +		ret = buffer->output[1];

This is OK, but this block reverses the ret/status terms applied to output[0]
and output[1] which is a little confusing.

>  
>   out:
>  	release_buffer();
> @@ -2087,7 +2133,8 @@ static int __init dell_init(void)
>  	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
>  	if (buffer->input[0] != -1) {
>  		dell_send_request(buffer, 0, 2);
> -		max_intensity = buffer->output[3];
> +		if (buffer->output[0] == 0)
> +			max_intensity = buffer->output[3];
>  	}
>  	release_buffer();
>  
> -- 
> 1.7.9.5
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2] dell-laptop: Check return value of all SMBIOS calls and do not cache hwswitch state

[Pali Rohár]

Make sure that return value of all SMBIOS calls are properly checked and
do not continue of processing (received) information if call failed.

Also do not chache hwswitch wireless state as it can be changed at runtime
(e.g from userspace smbios applications).

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
Changes since v1:
 * Call clear_buffer before each sequential SMBIOS call (we expect zero-filled buffer)
 * Do not cache hwswitch state as it can be modified at runtime by userspace
 * simplify some conditions
---
 drivers/platform/x86/dell-laptop.c |  173 ++++++++++++++++++++++++++----------
 1 file changed, 127 insertions(+), 46 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 35758cb..99f28d3 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -308,12 +308,15 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
 static struct calling_interface_buffer *buffer;
 static DEFINE_MUTEX(buffer_mutex);
 
-static int hwswitch_state;
+static void clear_buffer(void)
+{
+	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+}
 
 static void get_buffer(void)
 {
 	mutex_lock(&buffer_mutex);
-	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+	clear_buffer();
 }
 
 static void release_buffer(void)
@@ -547,21 +550,41 @@ static int dell_rfkill_set(void *data, bool blocked)
 	int disable = blocked ? 1 : 0;
 	unsigned long radio = (unsigned long)data;
 	int hwswitch_bit = (unsigned long)data - 1;
+	int hwswitch;
+	int status;
+	int ret;
 
 	get_buffer();
+
+	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+	status = buffer->output[1];
+
+	if (ret != 0)
+		goto out;
+
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+	hwswitch = buffer->output[1];
 
 	/* If the hardware switch controls this radio, and the hardware
 	   switch is disabled, always disable the radio */
-	if ((hwswitch_state & BIT(hwswitch_bit)) &&
-	    !(buffer->output[1] & BIT(16)))
+	if (ret == 0 && (hwswitch & BIT(hwswitch_bit)) &&
+	    (status & BIT(0)) && !(status & BIT(16)))
 		disable = 1;
 
+	clear_buffer();
+
 	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 
+ out:
 	release_buffer();
-	return 0;
+	return dell_smi_error(ret);
 }
 
 /* Must be called with the buffer held */
@@ -571,6 +594,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();
 		buffer->input[0] = (1 | (radio << 8) | (block << 16));
 		dell_send_request(buffer, 17, 11);
 	} else {
@@ -580,23 +604,43 @@ static void dell_rfkill_update_sw_state(struct rfkill *rfkill, int radio,
 }
 
 static void dell_rfkill_update_hw_state(struct rfkill *rfkill, int radio,
-					int status)
+					int status, int hwswitch)
 {
-	if (hwswitch_state & (BIT(radio - 1)))
+	if (hwswitch & (BIT(radio - 1)))
 		rfkill_set_hw_state(rfkill, !(status & BIT(16)));
 }
 
 static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 {
+	int radio = ((unsigned long)data & 0xF);
+	int hwswitch;
 	int status;
+	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
 
-	dell_rfkill_update_hw_state(rfkill, (unsigned long)data, status);
+	if (ret != 0 || !(status & BIT(0))) {
+		release_buffer();
+		return;
+	}
+
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+	hwswitch = buffer->output[1];
 
 	release_buffer();
+
+	if (ret != 0)
+		return;
+
+	dell_rfkill_update_hw_state(rfkill, radio, status, hwswitch);
 }
 
 static const struct rfkill_ops dell_rfkill_ops = {
@@ -608,13 +652,27 @@ static struct dentry *dell_laptop_dir;
 
 static int dell_debugfs_show(struct seq_file *s, void *data)
 {
+	int hwswitch_state;
+	int hwswitch_ret;
 	int status;
+	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
+
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	hwswitch_ret = buffer->output[0];
+	hwswitch_state = buffer->output[1];
+
 	release_buffer();
 
+	seq_printf(s, "return:\t%d\n", ret);
 	seq_printf(s, "status:\t0x%X\n", status);
 	seq_printf(s, "Bit 0 : Hardware switch supported:   %lu\n",
 		   status & BIT(0));
@@ -656,7 +714,9 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 	seq_printf(s, "Bit 21: WiGig is blocked:            %lu\n",
 		  (status & BIT(21)) >> 21);
 
-	seq_printf(s, "\nhwswitch_state:\t0x%X\n", hwswitch_state);
+	seq_printf(s, "\n");
+	seq_printf(s, "hwswitch_return:\t%d\n", hwswitch_ret);
+	seq_printf(s, "hwswitch_state:\t0x%X\n", hwswitch_state);
 	seq_printf(s, "Bit 0 : Wifi controlled by switch:      %lu\n",
 		   hwswitch_state & BIT(0));
 	seq_printf(s, "Bit 1 : Bluetooth controlled by switch: %lu\n",
@@ -692,25 +752,44 @@ static const struct file_operations dell_debugfs_fops = {
 
 static void dell_update_rfkill(struct work_struct *ignored)
 {
+	int hwswitch;
 	int status;
+	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
 
+	if (ret != 0)
+		goto out;
+
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+
+	if (ret == 0 && (status & BIT(0)))
+		hwswitch = buffer->output[1];
+	else
+		hwswitch = 0;
+
 	if (wifi_rfkill) {
-		dell_rfkill_update_hw_state(wifi_rfkill, 1, status);
+		dell_rfkill_update_hw_state(wifi_rfkill, 1, status, hwswitch);
 		dell_rfkill_update_sw_state(wifi_rfkill, 1, status);
 	}
 	if (bluetooth_rfkill) {
-		dell_rfkill_update_hw_state(bluetooth_rfkill, 2, status);
+		dell_rfkill_update_hw_state(bluetooth_rfkill, 2, status, hwswitch);
 		dell_rfkill_update_sw_state(bluetooth_rfkill, 2, status);
 	}
 	if (wwan_rfkill) {
-		dell_rfkill_update_hw_state(wwan_rfkill, 3, status);
+		dell_rfkill_update_hw_state(wwan_rfkill, 3, status, hwswitch);
 		dell_rfkill_update_sw_state(wwan_rfkill, 3, status);
 	}
 
+ out:
 	release_buffer();
 }
 static DECLARE_DELAYED_WORK(dell_rfkill_work, dell_update_rfkill);
@@ -772,21 +851,17 @@ static int __init dell_setup_rfkill(void)
 
 	get_buffer();
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
-	buffer->input[0] = 0x2;
-	dell_send_request(buffer, 17, 11);
-	hwswitch_state = buffer->output[1];
 	release_buffer();
 
-	if (!(status & BIT(0))) {
-		if (force_rfkill) {
-			/* No hwsitch, clear all hw-controlled bits */
-			hwswitch_state &= ~7;
-		} else {
-			/* rfkill is only tested on laptops with a hwswitch */
-			return 0;
-		}
-	}
+	/* dell wireless info smbios call is not supported */
+	if (ret != 0)
+		return 0;
+
+	/* rfkill is only tested on laptops with a hwswitch */
+	if (!(status & BIT(0)) && !force_rfkill)
+		return 0;
 
 	if ((status & (1<<2|1<<8)) == (1<<2|1<<8)) {
 		wifi_rfkill = rfkill_alloc("dell-wifi", &platform_device->dev,
@@ -931,47 +1006,50 @@ static void dell_cleanup_rfkill(void)
 
 static int dell_send_intensity(struct backlight_device *bd)
 {
-	int ret = 0;
+	int ret;
+	int token;
+
+	token = find_token_location(BRIGHTNESS_TOKEN);
+	if (token == -1)
+		return -ENODEV;
 
 	get_buffer();
-	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	buffer->input[0] = token;
 	buffer->input[1] = bd->props.brightness;
 
-	if (buffer->input[0] == -1) {
-		ret = -ENODEV;
-		goto out;
-	}
-
 	if (power_supply_is_system_supplied() > 0)
 		dell_send_request(buffer, 1, 2);
 	else
 		dell_send_request(buffer, 1, 1);
 
- out:
+	ret = dell_smi_error(buffer->output[0]);
+
 	release_buffer();
 	return ret;
 }
 
 static int dell_get_intensity(struct backlight_device *bd)
 {
-	int ret = 0;
+	int ret;
+	int token;
 
-	get_buffer();
-	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	token = find_token_location(BRIGHTNESS_TOKEN);
+	if (token == -1)
+		return -ENODEV;
 
-	if (buffer->input[0] == -1) {
-		ret = -ENODEV;
-		goto out;
-	}
+	get_buffer();
+	buffer->input[0] = token;
 
 	if (power_supply_is_system_supplied() > 0)
 		dell_send_request(buffer, 0, 2);
 	else
 		dell_send_request(buffer, 0, 1);
 
-	ret = buffer->output[1];
+	if (buffer->output[0])
+		ret = dell_smi_error(buffer->output[0]);
+	else
+		ret = buffer->output[1];
 
- out:
 	release_buffer();
 	return ret;
 }
@@ -2035,6 +2113,7 @@ static void kbd_led_exit(void)
 static int __init dell_init(void)
 {
 	int max_intensity = 0;
+	int token;
 	int ret;
 
 	if (!dmi_check_system(dell_device_table))
@@ -2098,13 +2177,15 @@ static int __init dell_init(void)
 		return 0;
 #endif
 
-	get_buffer();
-	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
-	if (buffer->input[0] != -1) {
+	token = find_token_location(BRIGHTNESS_TOKEN);
+	if (token != -1) {
+		get_buffer();
+		buffer->input[0] = token;
 		dell_send_request(buffer, 0, 2);
-		max_intensity = buffer->output[3];
+		if (buffer->output[0] == 0)
+			max_intensity = buffer->output[3];
+		release_buffer();
 	}
-	release_buffer();
 
 	if (max_intensity) {
 		struct backlight_properties props;
-- 
1.7.9.5

Re: [PATCH v2] dell-laptop: Check return value of all SMBIOS calls and do not cache hwswitch state

[Darren Hart]

On Sat, Jun 27, 2015 at 09:34:43AM +0200, Pali Rohár wrote:
> Make sure that return value of all SMBIOS calls are properly checked and
> do not continue of processing (received) information if call failed.
> 
> Also do not chache hwswitch wireless state as it can be changed at runtime
> (e.g from userspace smbios applications).

This "Also do.." tripled the size of the patch. This should really be two
patches.

> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> Changes since v1:
>  * Call clear_buffer before each sequential SMBIOS call (we expect zero-filled buffer)

Another good independent patch candidate

>  * Do not cache hwswitch state as it can be modified at runtime by userspace
>  * simplify some conditions
> ---
>  drivers/platform/x86/dell-laptop.c |  173 ++++++++++++++++++++++++++----------
>  1 file changed, 127 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> index 35758cb..99f28d3 100644
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c

...

>  static void dell_update_rfkill(struct work_struct *ignored)
>  {
> +	int hwswitch;
>  	int status;
> +	int ret;
>  
>  	get_buffer();
> +
>  	dell_send_request(buffer, 17, 11);
> +	ret = buffer->output[0];
>  	status = buffer->output[1];
>  
> +	if (ret != 0)
> +		goto out;
> +
> +	clear_buffer();
> +
> +	buffer->input[0] = 0x2;
> +	dell_send_request(buffer, 17, 11);
> +	ret = buffer->output[0];
> +
> +	if (ret == 0 && (status & BIT(0)))
> +		hwswitch = buffer->output[1];
> +	else
> +		hwswitch = 0;

Initializing hwswitch to 0 above saves the else and assignment line here.
Generally preferred.

...

>  
>  static int dell_get_intensity(struct backlight_device *bd)
>  {
> -	int ret = 0;
> +	int ret;
> +	int token;

Since we're talking respin, declare in order of descending line length please,
just as you did later when adding token to a function.


-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH v2] dell-laptop: Check return value of all SMBIOS calls and do not cache hwswitch state

[Pali Rohár]

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

On Tuesday 30 June 2015 01:02:40 Darren Hart wrote:
> On Sat, Jun 27, 2015 at 09:34:43AM +0200, Pali Rohár wrote:
> > Make sure that return value of all SMBIOS calls are properly
> > checked and do not continue of processing (received) information
> > if call failed.
> > 
> > Also do not chache hwswitch wireless state as it can be changed at
> > runtime (e.g from userspace smbios applications).
> 
> This "Also do.." tripled the size of the patch. This should really be
> two patches.
> 
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > ---
> > 
> > Changes since v1:
> >  * Call clear_buffer before each sequential SMBIOS call (we expect
> >  zero-filled buffer)
> 
> Another good independent patch candidate
> 

Ok, I will split it into 3 patches.

> >  * Do not cache hwswitch state as it can be modified at runtime by
> >  userspace * simplify some conditions
> > 
> > ---
> > 
> >  drivers/platform/x86/dell-laptop.c |  173
> >  ++++++++++++++++++++++++++---------- 1 file changed, 127
> >  insertions(+), 46 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/dell-laptop.c
> > b/drivers/platform/x86/dell-laptop.c index 35758cb..99f28d3 100644
> > --- a/drivers/platform/x86/dell-laptop.c
> > +++ b/drivers/platform/x86/dell-laptop.c
> 
> ...
> 
> >  static void dell_update_rfkill(struct work_struct *ignored)
> >  {
> > 
> > +	int hwswitch;
> > 
> >  	int status;
> > 
> > +	int ret;
> > 
> >  	get_buffer();
> > 
> > +
> > 
> >  	dell_send_request(buffer, 17, 11);
> > 
> > +	ret = buffer->output[0];
> > 
> >  	status = buffer->output[1];
> > 
> > +	if (ret != 0)
> > +		goto out;
> > +
> > +	clear_buffer();
> > +
> > +	buffer->input[0] = 0x2;
> > +	dell_send_request(buffer, 17, 11);
> > +	ret = buffer->output[0];
> > +
> > +	if (ret == 0 && (status & BIT(0)))
> > +		hwswitch = buffer->output[1];
> > +	else
> > +		hwswitch = 0;
> 
> Initializing hwswitch to 0 above saves the else and assignment line
> here. Generally preferred.
> 

Ok.

> ...
> 
> >  static int dell_get_intensity(struct backlight_device *bd)
> >  {
> > 
> > -	int ret = 0;
> > +	int ret;
> > +	int token;
> 
> Since we're talking respin, declare in order of descending line
> length please, just as you did later when adding token to a
> function.

Ok.

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

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

[PATCH 1/3] dell-laptop: Clear buffer before each SMBIOS call

[Pali Rohár]

Make sure that before initializing SMBIOS call input buffer does not contain
any garbage (e.g values from previous SMBIOS call). This fix problem with
passing undefined/random parameters to SMBIOS functions.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-laptop.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 35758cb..6728487 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -310,10 +310,15 @@ static DEFINE_MUTEX(buffer_mutex);
 
 static int hwswitch_state;
 
+static void clear_buffer(void)
+{
+	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+}
+
 static void get_buffer(void)
 {
 	mutex_lock(&buffer_mutex);
-	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+	clear_buffer();
 }
 
 static void release_buffer(void)
@@ -557,6 +562,8 @@ static int dell_rfkill_set(void *data, bool blocked)
 	    !(buffer->output[1] & BIT(16)))
 		disable = 1;
 
+	clear_buffer();
+
 	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
 	dell_send_request(buffer, 17, 11);
 
@@ -571,6 +578,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();
 		buffer->input[0] = (1 | (radio << 8) | (block << 16));
 		dell_send_request(buffer, 17, 11);
 	} else {
@@ -774,6 +782,7 @@ static int __init dell_setup_rfkill(void)
 	dell_send_request(buffer, 17, 11);
 	status = buffer->output[1];
 	buffer->input[0] = 0x2;
+	clear_buffer();
 	dell_send_request(buffer, 17, 11);
 	hwswitch_state = buffer->output[1];
 	release_buffer();
-- 
1.7.9.5

[PATCH 2/3] dell-laptop: Check return value of each SMBIOS call

[Pali Rohár]

Make sure that return value of each SMBIOS call is properly checked and
do not continue of processing (received) information if call failed.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-laptop.c |   83 +++++++++++++++++++++++++-----------
 1 file changed, 58 insertions(+), 25 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 6728487..b840ab1 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -552,9 +552,15 @@ static int dell_rfkill_set(void *data, bool blocked)
 	int disable = blocked ? 1 : 0;
 	unsigned long radio = (unsigned long)data;
 	int hwswitch_bit = (unsigned long)data - 1;
+	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+
+	if (ret != 0)
+		goto out;
 
 	/* If the hardware switch controls this radio, and the hardware
 	   switch is disabled, always disable the radio */
@@ -566,9 +572,11 @@ static int dell_rfkill_set(void *data, bool blocked)
 
 	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 
+ out:
 	release_buffer();
-	return 0;
+	return dell_smi_error(ret);
 }
 
 /* Must be called with the buffer held */
@@ -597,14 +605,18 @@ static void dell_rfkill_update_hw_state(struct rfkill *rfkill, int radio,
 static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 {
 	int status;
+	int ret;
 
 	get_buffer();
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
+	release_buffer();
 
-	dell_rfkill_update_hw_state(rfkill, (unsigned long)data, status);
+	if (ret != 0)
+		return;
 
-	release_buffer();
+	dell_rfkill_update_hw_state(rfkill, (unsigned long)data, status);
 }
 
 static const struct rfkill_ops dell_rfkill_ops = {
@@ -617,12 +629,15 @@ static struct dentry *dell_laptop_dir;
 static int dell_debugfs_show(struct seq_file *s, void *data)
 {
 	int status;
+	int ret;
 
 	get_buffer();
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
 	release_buffer();
 
+	seq_printf(s, "return:\t%d\n", ret);
 	seq_printf(s, "status:\t0x%X\n", status);
 	seq_printf(s, "Bit 0 : Hardware switch supported:   %lu\n",
 		   status & BIT(0));
@@ -701,11 +716,17 @@ static const struct file_operations dell_debugfs_fops = {
 static void dell_update_rfkill(struct work_struct *ignored)
 {
 	int status;
+	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
 
+	if (ret != 0)
+		goto out;
+
 	if (wifi_rfkill) {
 		dell_rfkill_update_hw_state(wifi_rfkill, 1, status);
 		dell_rfkill_update_sw_state(wifi_rfkill, 1, status);
@@ -719,6 +740,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 		dell_rfkill_update_sw_state(wwan_rfkill, 3, status);
 	}
 
+ out:
 	release_buffer();
 }
 static DECLARE_DELAYED_WORK(dell_rfkill_work, dell_update_rfkill);
@@ -780,6 +802,7 @@ static int __init dell_setup_rfkill(void)
 
 	get_buffer();
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
 	buffer->input[0] = 0x2;
 	clear_buffer();
@@ -787,6 +810,10 @@ static int __init dell_setup_rfkill(void)
 	hwswitch_state = buffer->output[1];
 	release_buffer();
 
+	/* dell wireless info smbios call is not supported */
+	if (ret != 0)
+		return 0;
+
 	if (!(status & BIT(0))) {
 		if (force_rfkill) {
 			/* No hwsitch, clear all hw-controlled bits */
@@ -940,47 +967,50 @@ static void dell_cleanup_rfkill(void)
 
 static int dell_send_intensity(struct backlight_device *bd)
 {
-	int ret = 0;
+	int token;
+	int ret;
+
+	token = find_token_location(BRIGHTNESS_TOKEN);
+	if (token == -1)
+		return -ENODEV;
 
 	get_buffer();
-	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	buffer->input[0] = token;
 	buffer->input[1] = bd->props.brightness;
 
-	if (buffer->input[0] == -1) {
-		ret = -ENODEV;
-		goto out;
-	}
-
 	if (power_supply_is_system_supplied() > 0)
 		dell_send_request(buffer, 1, 2);
 	else
 		dell_send_request(buffer, 1, 1);
 
- out:
+	ret = dell_smi_error(buffer->output[0]);
+
 	release_buffer();
 	return ret;
 }
 
 static int dell_get_intensity(struct backlight_device *bd)
 {
-	int ret = 0;
+	int token;
+	int ret;
 
-	get_buffer();
-	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	token = find_token_location(BRIGHTNESS_TOKEN);
+	if (token == -1)
+		return -ENODEV;
 
-	if (buffer->input[0] == -1) {
-		ret = -ENODEV;
-		goto out;
-	}
+	get_buffer();
+	buffer->input[0] = token;
 
 	if (power_supply_is_system_supplied() > 0)
 		dell_send_request(buffer, 0, 2);
 	else
 		dell_send_request(buffer, 0, 1);
 
-	ret = buffer->output[1];
+	if (buffer->output[0])
+		ret = dell_smi_error(buffer->output[0]);
+	else
+		ret = buffer->output[1];
 
- out:
 	release_buffer();
 	return ret;
 }
@@ -2044,6 +2074,7 @@ static void kbd_led_exit(void)
 static int __init dell_init(void)
 {
 	int max_intensity = 0;
+	int token;
 	int ret;
 
 	if (!dmi_check_system(dell_device_table))
@@ -2107,13 +2138,15 @@ static int __init dell_init(void)
 		return 0;
 #endif
 
-	get_buffer();
-	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
-	if (buffer->input[0] != -1) {
+	token = find_token_location(BRIGHTNESS_TOKEN);
+	if (token != -1) {
+		get_buffer();
+		buffer->input[0] = token;
 		dell_send_request(buffer, 0, 2);
-		max_intensity = buffer->output[3];
+		if (buffer->output[0] == 0)
+			max_intensity = buffer->output[3];
+		release_buffer();
 	}
-	release_buffer();
 
 	if (max_intensity) {
 		struct backlight_properties props;
-- 
1.7.9.5

[PATCH 3/3] dell-laptop: Do not cache hwswitch state

[Pali Rohár]

It can be changed at runtime so make sure that dell-laptop knows always
current state. It can be modified by userspace utility smbios-wireless-ctl.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-laptop.c |   85 ++++++++++++++++++++++++++----------
 1 file changed, 61 insertions(+), 24 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index b840ab1..d094153 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -308,8 +308,6 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
 static struct calling_interface_buffer *buffer;
 static DEFINE_MUTEX(buffer_mutex);
 
-static int hwswitch_state;
-
 static void clear_buffer(void)
 {
 	memset(buffer, 0, sizeof(struct calling_interface_buffer));
@@ -552,20 +550,30 @@ static int dell_rfkill_set(void *data, bool blocked)
 	int disable = blocked ? 1 : 0;
 	unsigned long radio = (unsigned long)data;
 	int hwswitch_bit = (unsigned long)data - 1;
+	int hwswitch;
+	int status;
 	int ret;
 
 	get_buffer();
 
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
+	status = buffer->output[1];
 
 	if (ret != 0)
 		goto out;
 
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+	hwswitch = buffer->output[1];
+
 	/* If the hardware switch controls this radio, and the hardware
 	   switch is disabled, always disable the radio */
-	if ((hwswitch_state & BIT(hwswitch_bit)) &&
-	    !(buffer->output[1] & BIT(16)))
+	if (ret == 0 && (hwswitch & BIT(hwswitch_bit)) &&
+	    (status & BIT(0)) && !(status & BIT(16)))
 		disable = 1;
 
 	clear_buffer();
@@ -596,27 +604,43 @@ static void dell_rfkill_update_sw_state(struct rfkill *rfkill, int radio,
 }
 
 static void dell_rfkill_update_hw_state(struct rfkill *rfkill, int radio,
-					int status)
+					int status, int hwswitch)
 {
-	if (hwswitch_state & (BIT(radio - 1)))
+	if (hwswitch & (BIT(radio - 1)))
 		rfkill_set_hw_state(rfkill, !(status & BIT(16)));
 }
 
 static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 {
+	int radio = ((unsigned long)data & 0xF);
+	int hwswitch;
 	int status;
 	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
+
+	if (ret != 0 || !(status & BIT(0))) {
+		release_buffer();
+		return;
+	}
+
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+	hwswitch = buffer->output[1];
+
 	release_buffer();
 
 	if (ret != 0)
 		return;
 
-	dell_rfkill_update_hw_state(rfkill, (unsigned long)data, status);
+	dell_rfkill_update_hw_state(rfkill, radio, status, hwswitch);
 }
 
 static const struct rfkill_ops dell_rfkill_ops = {
@@ -628,13 +652,24 @@ static struct dentry *dell_laptop_dir;
 
 static int dell_debugfs_show(struct seq_file *s, void *data)
 {
+	int hwswitch_state;
+	int hwswitch_ret;
 	int status;
 	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
+
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	hwswitch_ret = buffer->output[0];
+	hwswitch_state = buffer->output[1];
+
 	release_buffer();
 
 	seq_printf(s, "return:\t%d\n", ret);
@@ -679,7 +714,9 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 	seq_printf(s, "Bit 21: WiGig is blocked:            %lu\n",
 		  (status & BIT(21)) >> 21);
 
-	seq_printf(s, "\nhwswitch_state:\t0x%X\n", hwswitch_state);
+	seq_printf(s, "\n");
+	seq_printf(s, "hwswitch_return:\t%d\n", hwswitch_ret);
+	seq_printf(s, "hwswitch_state:\t0x%X\n", hwswitch_state);
 	seq_printf(s, "Bit 0 : Wifi controlled by switch:      %lu\n",
 		   hwswitch_state & BIT(0));
 	seq_printf(s, "Bit 1 : Bluetooth controlled by switch: %lu\n",
@@ -715,6 +752,7 @@ static const struct file_operations dell_debugfs_fops = {
 
 static void dell_update_rfkill(struct work_struct *ignored)
 {
+	int hwswitch = 0;
 	int status;
 	int ret;
 
@@ -727,16 +765,25 @@ static void dell_update_rfkill(struct work_struct *ignored)
 	if (ret != 0)
 		goto out;
 
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+
+	if (ret == 0 && (status & BIT(0)))
+		hwswitch = buffer->output[1];
+
 	if (wifi_rfkill) {
-		dell_rfkill_update_hw_state(wifi_rfkill, 1, status);
+		dell_rfkill_update_hw_state(wifi_rfkill, 1, status, hwswitch);
 		dell_rfkill_update_sw_state(wifi_rfkill, 1, status);
 	}
 	if (bluetooth_rfkill) {
-		dell_rfkill_update_hw_state(bluetooth_rfkill, 2, status);
+		dell_rfkill_update_hw_state(bluetooth_rfkill, 2, status, hwswitch);
 		dell_rfkill_update_sw_state(bluetooth_rfkill, 2, status);
 	}
 	if (wwan_rfkill) {
-		dell_rfkill_update_hw_state(wwan_rfkill, 3, status);
+		dell_rfkill_update_hw_state(wwan_rfkill, 3, status, hwswitch);
 		dell_rfkill_update_sw_state(wwan_rfkill, 3, status);
 	}
 
@@ -804,25 +851,15 @@ static int __init dell_setup_rfkill(void)
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
-	buffer->input[0] = 0x2;
-	clear_buffer();
-	dell_send_request(buffer, 17, 11);
-	hwswitch_state = buffer->output[1];
 	release_buffer();
 
 	/* dell wireless info smbios call is not supported */
 	if (ret != 0)
 		return 0;
 
-	if (!(status & BIT(0))) {
-		if (force_rfkill) {
-			/* No hwsitch, clear all hw-controlled bits */
-			hwswitch_state &= ~7;
-		} else {
-			/* rfkill is only tested on laptops with a hwswitch */
-			return 0;
-		}
-	}
+	/* rfkill is only tested on laptops with a hwswitch */
+	if (!(status & BIT(0)) && !force_rfkill)
+		return 0;
 
 	if ((status & (1<<2|1<<8)) == (1<<2|1<<8)) {
 		wifi_rfkill = rfkill_alloc("dell-wifi", &platform_device->dev,
-- 
1.7.9.5

Re: [PATCH 3/3] dell-laptop: Do not cache hwswitch state

[Darren Hart]

On Wed, Jul 01, 2015 at 08:08:21PM +0200, Pali Rohár wrote:
> It can be changed at runtime so make sure that dell-laptop knows always
> current state. It can be modified by userspace utility smbios-wireless-ctl.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

Couple of checkpatch warnings on this one.

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 3/3] dell-laptop: Do not cache hwswitch state

[Pali Rohár]

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

On Thursday 02 July 2015 02:42:04 Darren Hart wrote:
> On Wed, Jul 01, 2015 at 08:08:21PM +0200, Pali Rohár wrote:
> > It can be changed at runtime so make sure that dell-laptop knows
> > always current state. It can be modified by userspace utility
> > smbios-wireless-ctl.
> > 
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> 
> Couple of checkpatch warnings on this one.

Fixed in patches.

-- 
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 1/3] dell-laptop: Clear buffer before each SMBIOS call

[Darren Hart]

On Wed, Jul 01, 2015 at 08:08:19PM +0200, Pali Rohár wrote:
> Make sure that before initializing SMBIOS call input buffer does not contain
> any garbage (e.g values from previous SMBIOS call). This fix problem with
> passing undefined/random parameters to SMBIOS functions.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/platform/x86/dell-laptop.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> index 35758cb..6728487 100644
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -310,10 +310,15 @@ static DEFINE_MUTEX(buffer_mutex);
>  
>  static int hwswitch_state;
>  
> +static void clear_buffer(void)
> +{
> +	memset(buffer, 0, sizeof(struct calling_interface_buffer));
> +}
> +
>  static void get_buffer(void)
>  {
>  	mutex_lock(&buffer_mutex);
> -	memset(buffer, 0, sizeof(struct calling_interface_buffer));
> +	clear_buffer();
>  }
>  
>  static void release_buffer(void)
> @@ -557,6 +562,8 @@ static int dell_rfkill_set(void *data, bool blocked)
>  	    !(buffer->output[1] & BIT(16)))
>  		disable = 1;
>  
> +	clear_buffer();
> +
>  	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
>  	dell_send_request(buffer, 17, 11);
>  
> @@ -571,6 +578,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();
>  		buffer->input[0] = (1 | (radio << 8) | (block << 16));
>  		dell_send_request(buffer, 17, 11);
>  	} else {
> @@ -774,6 +782,7 @@ static int __init dell_setup_rfkill(void)
>  	dell_send_request(buffer, 17, 11);
>  	status = buffer->output[1];
>  	buffer->input[0] = 0x2;
> +	clear_buffer();
>  	dell_send_request(buffer, 17, 11);

This clears the buffer after modifying input[0] and right before
dell_send_request... so you're sending a completely empty buffer? Is that
intentional here? I guess I would have expected the clear_buffer to be one line
earlier.

>  	hwswitch_state = buffer->output[1];
>  	release_buffer();
> -- 
> 1.7.9.5
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 1/3] dell-laptop: Clear buffer before each SMBIOS call

[Pali Rohár]

On Wednesday 01 July 2015 17:45:44 Darren Hart wrote:
> On Wed, Jul 01, 2015 at 08:08:19PM +0200, Pali Rohár wrote:
> > Make sure that before initializing SMBIOS call input buffer does not contain
> > any garbage (e.g values from previous SMBIOS call). This fix problem with
> > passing undefined/random parameters to SMBIOS functions.
> > 
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > ---
> >  drivers/platform/x86/dell-laptop.c |   11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> > index 35758cb..6728487 100644
> > --- a/drivers/platform/x86/dell-laptop.c
> > +++ b/drivers/platform/x86/dell-laptop.c
> > @@ -310,10 +310,15 @@ static DEFINE_MUTEX(buffer_mutex);
> >  
> >  static int hwswitch_state;
> >  
> > +static void clear_buffer(void)
> > +{
> > +	memset(buffer, 0, sizeof(struct calling_interface_buffer));
> > +}
> > +
> >  static void get_buffer(void)
> >  {
> >  	mutex_lock(&buffer_mutex);
> > -	memset(buffer, 0, sizeof(struct calling_interface_buffer));
> > +	clear_buffer();
> >  }
> >  
> >  static void release_buffer(void)
> > @@ -557,6 +562,8 @@ static int dell_rfkill_set(void *data, bool blocked)
> >  	    !(buffer->output[1] & BIT(16)))
> >  		disable = 1;
> >  
> > +	clear_buffer();
> > +
> >  	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
> >  	dell_send_request(buffer, 17, 11);
> >  
> > @@ -571,6 +578,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();
> >  		buffer->input[0] = (1 | (radio << 8) | (block << 16));
> >  		dell_send_request(buffer, 17, 11);
> >  	} else {
> > @@ -774,6 +782,7 @@ static int __init dell_setup_rfkill(void)
> >  	dell_send_request(buffer, 17, 11);
> >  	status = buffer->output[1];
> >  	buffer->input[0] = 0x2;
> > +	clear_buffer();
> >  	dell_send_request(buffer, 17, 11);
> 
> This clears the buffer after modifying input[0] and right before
> dell_send_request... so you're sending a completely empty buffer? Is that
> intentional here? I guess I would have expected the clear_buffer to be one line
> earlier.
> 

Yes, now I see. I split that one patch into tree and I checked that
every one compiles fine and cumulative change of all patches is same...
Because that part (sending 0x2) is deleted in patch 3/3 it missed in my
eyes...

I will fix these problems and sent patch series again.

> >  	hwswitch_state = buffer->output[1];
> >  	release_buffer();
> > -- 
> > 1.7.9.5
> > 
> > 
> 

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

[PATCH v2 1/3] dell-laptop: Clear buffer before each SMBIOS call

[Pali Rohár]

Make sure that before initializing SMBIOS call input buffer does not
contain any garbage (e.g. values from previous SMBIOS call). This fix
problem with passing undefined/random parameters to SMBIOS functions.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-laptop.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 35758cb..2caefb2 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -310,10 +310,15 @@ static DEFINE_MUTEX(buffer_mutex);
 
 static int hwswitch_state;
 
+static void clear_buffer(void)
+{
+	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+}
+
 static void get_buffer(void)
 {
 	mutex_lock(&buffer_mutex);
-	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+	clear_buffer();
 }
 
 static void release_buffer(void)
@@ -557,6 +562,8 @@ static int dell_rfkill_set(void *data, bool blocked)
 	    !(buffer->output[1] & BIT(16)))
 		disable = 1;
 
+	clear_buffer();
+
 	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
 	dell_send_request(buffer, 17, 11);
 
@@ -571,6 +578,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();
 		buffer->input[0] = (1 | (radio << 8) | (block << 16));
 		dell_send_request(buffer, 17, 11);
 	} else {
@@ -773,6 +781,7 @@ static int __init dell_setup_rfkill(void)
 	get_buffer();
 	dell_send_request(buffer, 17, 11);
 	status = buffer->output[1];
+	clear_buffer();
 	buffer->input[0] = 0x2;
 	dell_send_request(buffer, 17, 11);
 	hwswitch_state = buffer->output[1];
-- 
1.7.9.5

[PATCH v2 2/3] dell-laptop: Check return value of each SMBIOS call

[Pali Rohár]

Make sure that return value of each SMBIOS call is properly checked and do
not continue of processing (received) information if call failed.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-laptop.c |   83 +++++++++++++++++++++++++-----------
 1 file changed, 58 insertions(+), 25 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 2caefb2..d9710c1 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -552,9 +552,15 @@ static int dell_rfkill_set(void *data, bool blocked)
 	int disable = blocked ? 1 : 0;
 	unsigned long radio = (unsigned long)data;
 	int hwswitch_bit = (unsigned long)data - 1;
+	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+
+	if (ret != 0)
+		goto out;
 
 	/* If the hardware switch controls this radio, and the hardware
 	   switch is disabled, always disable the radio */
@@ -566,9 +572,11 @@ static int dell_rfkill_set(void *data, bool blocked)
 
 	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 
+ out:
 	release_buffer();
-	return 0;
+	return dell_smi_error(ret);
 }
 
 /* Must be called with the buffer held */
@@ -597,14 +605,18 @@ static void dell_rfkill_update_hw_state(struct rfkill *rfkill, int radio,
 static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 {
 	int status;
+	int ret;
 
 	get_buffer();
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
+	release_buffer();
 
-	dell_rfkill_update_hw_state(rfkill, (unsigned long)data, status);
+	if (ret != 0)
+		return;
 
-	release_buffer();
+	dell_rfkill_update_hw_state(rfkill, (unsigned long)data, status);
 }
 
 static const struct rfkill_ops dell_rfkill_ops = {
@@ -617,12 +629,15 @@ static struct dentry *dell_laptop_dir;
 static int dell_debugfs_show(struct seq_file *s, void *data)
 {
 	int status;
+	int ret;
 
 	get_buffer();
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
 	release_buffer();
 
+	seq_printf(s, "return:\t%d\n", ret);
 	seq_printf(s, "status:\t0x%X\n", status);
 	seq_printf(s, "Bit 0 : Hardware switch supported:   %lu\n",
 		   status & BIT(0));
@@ -701,11 +716,17 @@ static const struct file_operations dell_debugfs_fops = {
 static void dell_update_rfkill(struct work_struct *ignored)
 {
 	int status;
+	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
 
+	if (ret != 0)
+		goto out;
+
 	if (wifi_rfkill) {
 		dell_rfkill_update_hw_state(wifi_rfkill, 1, status);
 		dell_rfkill_update_sw_state(wifi_rfkill, 1, status);
@@ -719,6 +740,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
 		dell_rfkill_update_sw_state(wwan_rfkill, 3, status);
 	}
 
+ out:
 	release_buffer();
 }
 static DECLARE_DELAYED_WORK(dell_rfkill_work, dell_update_rfkill);
@@ -780,6 +802,7 @@ static int __init dell_setup_rfkill(void)
 
 	get_buffer();
 	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
 	status = buffer->output[1];
 	clear_buffer();
 	buffer->input[0] = 0x2;
@@ -787,6 +810,10 @@ static int __init dell_setup_rfkill(void)
 	hwswitch_state = buffer->output[1];
 	release_buffer();
 
+	/* dell wireless info smbios call is not supported */
+	if (ret != 0)
+		return 0;
+
 	if (!(status & BIT(0))) {
 		if (force_rfkill) {
 			/* No hwsitch, clear all hw-controlled bits */
@@ -940,47 +967,50 @@ static void dell_cleanup_rfkill(void)
 
 static int dell_send_intensity(struct backlight_device *bd)
 {
-	int ret = 0;
+	int token;
+	int ret;
+
+	token = find_token_location(BRIGHTNESS_TOKEN);
+	if (token == -1)
+		return -ENODEV;
 
 	get_buffer();
-	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	buffer->input[0] = token;
 	buffer->input[1] = bd->props.brightness;
 
-	if (buffer->input[0] == -1) {
-		ret = -ENODEV;
-		goto out;
-	}
-
 	if (power_supply_is_system_supplied() > 0)
 		dell_send_request(buffer, 1, 2);
 	else
 		dell_send_request(buffer, 1, 1);
 
- out:
+	ret = dell_smi_error(buffer->output[0]);
+
 	release_buffer();
 	return ret;
 }
 
 static int dell_get_intensity(struct backlight_device *bd)
 {
-	int ret = 0;
+	int token;
+	int ret;
 
-	get_buffer();
-	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	token = find_token_location(BRIGHTNESS_TOKEN);
+	if (token == -1)
+		return -ENODEV;
 
-	if (buffer->input[0] == -1) {
-		ret = -ENODEV;
-		goto out;
-	}
+	get_buffer();
+	buffer->input[0] = token;
 
 	if (power_supply_is_system_supplied() > 0)
 		dell_send_request(buffer, 0, 2);
 	else
 		dell_send_request(buffer, 0, 1);
 
-	ret = buffer->output[1];
+	if (buffer->output[0])
+		ret = dell_smi_error(buffer->output[0]);
+	else
+		ret = buffer->output[1];
 
- out:
 	release_buffer();
 	return ret;
 }
@@ -2044,6 +2074,7 @@ static void kbd_led_exit(void)
 static int __init dell_init(void)
 {
 	int max_intensity = 0;
+	int token;
 	int ret;
 
 	if (!dmi_check_system(dell_device_table))
@@ -2107,13 +2138,15 @@ static int __init dell_init(void)
 		return 0;
 #endif
 
-	get_buffer();
-	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
-	if (buffer->input[0] != -1) {
+	token = find_token_location(BRIGHTNESS_TOKEN);
+	if (token != -1) {
+		get_buffer();
+		buffer->input[0] = token;
 		dell_send_request(buffer, 0, 2);
-		max_intensity = buffer->output[3];
+		if (buffer->output[0] == 0)
+			max_intensity = buffer->output[3];
+		release_buffer();
 	}
-	release_buffer();
 
 	if (max_intensity) {
 		struct backlight_properties props;
-- 
1.7.9.5

[PATCH v2 3/3] dell-laptop: Do not cache hwswitch state

[Pali Rohár]

It can be changed at runtime so make sure that dell-laptop knows always
current state. It can be modified by userspace utility smbios-wireless-ctl.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-laptop.c |   85 ++++++++++++++++++++++++++----------
 1 file changed, 61 insertions(+), 24 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index d9710c1..89d6004 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -308,8 +308,6 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
 static struct calling_interface_buffer *buffer;
 static DEFINE_MUTEX(buffer_mutex);
 
-static int hwswitch_state;
-
 static void clear_buffer(void)
 {
 	memset(buffer, 0, sizeof(struct calling_interface_buffer));
@@ -552,20 +550,30 @@ static int dell_rfkill_set(void *data, bool blocked)
 	int disable = blocked ? 1 : 0;
 	unsigned long radio = (unsigned long)data;
 	int hwswitch_bit = (unsigned long)data - 1;
+	int hwswitch;
+	int status;
 	int ret;
 
 	get_buffer();
 
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
+	status = buffer->output[1];
 
 	if (ret != 0)
 		goto out;
 
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+	hwswitch = buffer->output[1];
+
 	/* If the hardware switch controls this radio, and the hardware
 	   switch is disabled, always disable the radio */
-	if ((hwswitch_state & BIT(hwswitch_bit)) &&
-	    !(buffer->output[1] & BIT(16)))
+	if (ret == 0 && (hwswitch & BIT(hwswitch_bit)) &&
+	    (status & BIT(0)) && !(status & BIT(16)))
 		disable = 1;
 
 	clear_buffer();
@@ -596,27 +604,43 @@ static void dell_rfkill_update_sw_state(struct rfkill *rfkill, int radio,
 }
 
 static void dell_rfkill_update_hw_state(struct rfkill *rfkill, int radio,
-					int status)
+					int status, int hwswitch)
 {
-	if (hwswitch_state & (BIT(radio - 1)))
+	if (hwswitch & (BIT(radio - 1)))
 		rfkill_set_hw_state(rfkill, !(status & BIT(16)));
 }
 
 static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 {
+	int radio = ((unsigned long)data & 0xF);
+	int hwswitch;
 	int status;
 	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
+
+	if (ret != 0 || !(status & BIT(0))) {
+		release_buffer();
+		return;
+	}
+
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+	hwswitch = buffer->output[1];
+
 	release_buffer();
 
 	if (ret != 0)
 		return;
 
-	dell_rfkill_update_hw_state(rfkill, (unsigned long)data, status);
+	dell_rfkill_update_hw_state(rfkill, radio, status, hwswitch);
 }
 
 static const struct rfkill_ops dell_rfkill_ops = {
@@ -628,13 +652,24 @@ static struct dentry *dell_laptop_dir;
 
 static int dell_debugfs_show(struct seq_file *s, void *data)
 {
+	int hwswitch_state;
+	int hwswitch_ret;
 	int status;
 	int ret;
 
 	get_buffer();
+
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
+
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	hwswitch_ret = buffer->output[0];
+	hwswitch_state = buffer->output[1];
+
 	release_buffer();
 
 	seq_printf(s, "return:\t%d\n", ret);
@@ -679,7 +714,8 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 	seq_printf(s, "Bit 21: WiGig is blocked:            %lu\n",
 		  (status & BIT(21)) >> 21);
 
-	seq_printf(s, "\nhwswitch_state:\t0x%X\n", hwswitch_state);
+	seq_printf(s, "\nhwswitch_return:\t%d\n", hwswitch_ret);
+	seq_printf(s, "hwswitch_state:\t0x%X\n", hwswitch_state);
 	seq_printf(s, "Bit 0 : Wifi controlled by switch:      %lu\n",
 		   hwswitch_state & BIT(0));
 	seq_printf(s, "Bit 1 : Bluetooth controlled by switch: %lu\n",
@@ -715,6 +751,7 @@ static const struct file_operations dell_debugfs_fops = {
 
 static void dell_update_rfkill(struct work_struct *ignored)
 {
+	int hwswitch = 0;
 	int status;
 	int ret;
 
@@ -727,16 +764,26 @@ static void dell_update_rfkill(struct work_struct *ignored)
 	if (ret != 0)
 		goto out;
 
+	clear_buffer();
+
+	buffer->input[0] = 0x2;
+	dell_send_request(buffer, 17, 11);
+	ret = buffer->output[0];
+
+	if (ret == 0 && (status & BIT(0)))
+		hwswitch = buffer->output[1];
+
 	if (wifi_rfkill) {
-		dell_rfkill_update_hw_state(wifi_rfkill, 1, status);
+		dell_rfkill_update_hw_state(wifi_rfkill, 1, status, hwswitch);
 		dell_rfkill_update_sw_state(wifi_rfkill, 1, status);
 	}
 	if (bluetooth_rfkill) {
-		dell_rfkill_update_hw_state(bluetooth_rfkill, 2, status);
+		dell_rfkill_update_hw_state(bluetooth_rfkill, 2, status,
+					    hwswitch);
 		dell_rfkill_update_sw_state(bluetooth_rfkill, 2, status);
 	}
 	if (wwan_rfkill) {
-		dell_rfkill_update_hw_state(wwan_rfkill, 3, status);
+		dell_rfkill_update_hw_state(wwan_rfkill, 3, status, hwswitch);
 		dell_rfkill_update_sw_state(wwan_rfkill, 3, status);
 	}
 
@@ -804,25 +851,15 @@ static int __init dell_setup_rfkill(void)
 	dell_send_request(buffer, 17, 11);
 	ret = buffer->output[0];
 	status = buffer->output[1];
-	clear_buffer();
-	buffer->input[0] = 0x2;
-	dell_send_request(buffer, 17, 11);
-	hwswitch_state = buffer->output[1];
 	release_buffer();
 
 	/* dell wireless info smbios call is not supported */
 	if (ret != 0)
 		return 0;
 
-	if (!(status & BIT(0))) {
-		if (force_rfkill) {
-			/* No hwsitch, clear all hw-controlled bits */
-			hwswitch_state &= ~7;
-		} else {
-			/* rfkill is only tested on laptops with a hwswitch */
-			return 0;
-		}
-	}
+	/* rfkill is only tested on laptops with a hwswitch */
+	if (!(status & BIT(0)) && !force_rfkill)
+		return 0;
 
 	if ((status & (1<<2|1<<8)) == (1<<2|1<<8)) {
 		wifi_rfkill = rfkill_alloc("dell-wifi", &platform_device->dev,
-- 
1.7.9.5

Re: [PATCH v2 1/3] dell-laptop: Clear buffer before each SMBIOS call

[Darren Hart]

On Mon, Jul 06, 2015 at 12:08:55PM +0200, Pali Rohár wrote:
> Make sure that before initializing SMBIOS call input buffer does not
> contain any garbage (e.g. values from previous SMBIOS call). This fix
> problem with passing undefined/random parameters to SMBIOS functions.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

Queued all 3 to fixes. Thanks Pali.

(I made a few minor grammatical fixes tot he commit messages, review the changes
if you like in the fixes branch)

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH v2 1/3] dell-laptop: Clear buffer before each SMBIOS call

[Pali Rohár]

On Monday 06 July 2015 15:39:37 Darren Hart wrote:
> On Mon, Jul 06, 2015 at 12:08:55PM +0200, Pali Rohár wrote:
> > Make sure that before initializing SMBIOS call input buffer does not
> > contain any garbage (e.g. values from previous SMBIOS call). This fix
> > problem with passing undefined/random parameters to SMBIOS functions.
> > 
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> 
> Queued all 3 to fixes. Thanks Pali.
> 

Ok.

> (I made a few minor grammatical fixes tot he commit messages, review the changes
> if you like in the fixes branch)
> 

It is OK, I'm not native speaker and I have lot of troubles with English
language. So feel free to fix any grammatical mistakes...

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

Re: [PATCH v2 1/3] dell-laptop: Clear buffer before each SMBIOS call

[Darren Hart]

On Tue, Jul 07, 2015 at 04:04:57PM +0200, Pali Rohár wrote:
> On Monday 06 July 2015 15:39:37 Darren Hart wrote:
> > On Mon, Jul 06, 2015 at 12:08:55PM +0200, Pali Rohár wrote:
> > > Make sure that before initializing SMBIOS call input buffer does not
> > > contain any garbage (e.g. values from previous SMBIOS call). This fix
> > > problem with passing undefined/random parameters to SMBIOS functions.
> > > 
> > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > 
> > Queued all 3 to fixes. Thanks Pali.
> > 
> 
> Ok.
> 
> > (I made a few minor grammatical fixes tot he commit messages, review the changes
> > if you like in the fixes branch)
> > 
> 
> It is OK, I'm not native speaker and I have lot of troubles with English
> language. So feel free to fix any grammatical mistakes...

You do better than I do with any other language I attempt :-) I don't feel it's
reasonable for me to nag you about commit messages, but I do need to ensure they
are clear. I'll continue to update them going forward.

-- 
Darren Hart
Intel Open Source Technology Center

[PATCH 3/4] dell-laptop: Fix allocating & freeing SMI buffer page

[Pali Rohár]

This commit fix kernel crash when probing for rfkill devices in dell-laptop
driver failed. Function free_page() was incorrectly used on struct page *
instead of virtual address of SMI buffer.

This commit also simplify allocating page for SMI buffer by using
__get_free_page() function instead of sequential call of functions
alloc_page() and page_address().

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-laptop.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index aaef335..0a91599 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -306,7 +306,6 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
 };
 
 static struct calling_interface_buffer *buffer;
-static struct page *bufferpage;
 static DEFINE_MUTEX(buffer_mutex);
 
 static int hwswitch_state;
@@ -2097,12 +2096,11 @@ static int __init dell_init(void)
 	 * Allocate buffer below 4GB for SMI data--only 32-bit physical addr
 	 * is passed to SMI handler.
 	 */
-	bufferpage = alloc_page(GFP_KERNEL | GFP_DMA32);
-	if (!bufferpage) {
+	buffer = (void *)__get_free_page(GFP_KERNEL | GFP_DMA32);
+	if (!buffer) {
 		ret = -ENOMEM;
 		goto fail_buffer;
 	}
-	buffer = page_address(bufferpage);
 
 	ret = dell_setup_rfkill();
 
@@ -2165,7 +2163,7 @@ static int __init dell_init(void)
 fail_backlight:
 	dell_cleanup_rfkill();
 fail_rfkill:
-	free_page((unsigned long)bufferpage);
+	free_page((unsigned long)buffer);
 fail_buffer:
 	platform_device_del(platform_device);
 fail_platform_device2:
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/4] dell-laptop: Fix allocating & freeing SMI buffer page

[Michal Hocko]

On Sun 21-06-15 10:41:03, Pali Rohár wrote:
> This commit fix kernel crash when probing for rfkill devices in dell-laptop
> driver failed. Function free_page() was incorrectly used on struct page *
> instead of virtual address of SMI buffer.
> 
> This commit also simplify allocating page for SMI buffer by using
> __get_free_page() function instead of sequential call of functions
> alloc_page() and page_address().
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

Looks good to me.
FWIW
Acked-by: Michal Hocko <mhocko@suse.cz>

> ---
>  drivers/platform/x86/dell-laptop.c |    8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> index aaef335..0a91599 100644
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -306,7 +306,6 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
>  };
>  
>  static struct calling_interface_buffer *buffer;
> -static struct page *bufferpage;
>  static DEFINE_MUTEX(buffer_mutex);
>  
>  static int hwswitch_state;
> @@ -2097,12 +2096,11 @@ static int __init dell_init(void)
>  	 * Allocate buffer below 4GB for SMI data--only 32-bit physical addr
>  	 * is passed to SMI handler.
>  	 */
> -	bufferpage = alloc_page(GFP_KERNEL | GFP_DMA32);
> -	if (!bufferpage) {
> +	buffer = (void *)__get_free_page(GFP_KERNEL | GFP_DMA32);
> +	if (!buffer) {
>  		ret = -ENOMEM;
>  		goto fail_buffer;
>  	}
> -	buffer = page_address(bufferpage);
>  
>  	ret = dell_setup_rfkill();
>  
> @@ -2165,7 +2163,7 @@ static int __init dell_init(void)
>  fail_backlight:
>  	dell_cleanup_rfkill();
>  fail_rfkill:
> -	free_page((unsigned long)bufferpage);
> +	free_page((unsigned long)buffer);
>  fail_buffer:
>  	platform_device_del(platform_device);
>  fail_platform_device2:
> -- 
> 1.7.9.5
> 

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/4] dell-laptop: Fix allocating & freeing SMI buffer page

[Darren Hart]

On Sun, Jun 21, 2015 at 10:41:03AM +0200, Pali Rohár wrote:
> This commit fix kernel crash when probing for rfkill devices in dell-laptop
> driver failed. Function free_page() was incorrectly used on struct page *
> instead of virtual address of SMI buffer.
> 
> This commit also simplify allocating page for SMI buffer by using
> __get_free_page() function instead of sequential call of functions
> alloc_page() and page_address().
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

Looks good - please resend with Cc to stable - that's the simplest path to
inclusion in stable.

> ---
>  drivers/platform/x86/dell-laptop.c |    8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> index aaef335..0a91599 100644
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -306,7 +306,6 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
>  };
>  
>  static struct calling_interface_buffer *buffer;
> -static struct page *bufferpage;
>  static DEFINE_MUTEX(buffer_mutex);
>  
>  static int hwswitch_state;
> @@ -2097,12 +2096,11 @@ static int __init dell_init(void)
>  	 * Allocate buffer below 4GB for SMI data--only 32-bit physical addr
>  	 * is passed to SMI handler.
>  	 */
> -	bufferpage = alloc_page(GFP_KERNEL | GFP_DMA32);
> -	if (!bufferpage) {
> +	buffer = (void *)__get_free_page(GFP_KERNEL | GFP_DMA32);
> +	if (!buffer) {
>  		ret = -ENOMEM;
>  		goto fail_buffer;
>  	}
> -	buffer = page_address(bufferpage);
>  
>  	ret = dell_setup_rfkill();
>  
> @@ -2165,7 +2163,7 @@ static int __init dell_init(void)
>  fail_backlight:
>  	dell_cleanup_rfkill();
>  fail_rfkill:
> -	free_page((unsigned long)bufferpage);
> +	free_page((unsigned long)buffer);
>  fail_buffer:
>  	platform_device_del(platform_device);
>  fail_platform_device2:
> -- 
> 1.7.9.5
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] dell-laptop: Fix allocating & freeing SMI buffer page

[Pali Rohár]

This commit fix kernel crash when probing for rfkill devices in dell-laptop
driver failed. Function free_page() was incorrectly used on struct page *
instead of virtual address of SMI buffer.

This commit also simplify allocating page for SMI buffer by using
__get_free_page() function instead of sequential call of functions
alloc_page() and page_address().

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
Cc: stable@vger.kernel.org
---
 drivers/platform/x86/dell-laptop.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index d688d80..2c1d5f5 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -305,7 +305,6 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
 };
 
 static struct calling_interface_buffer *buffer;
-static struct page *bufferpage;
 static DEFINE_MUTEX(buffer_mutex);
 
 static int hwswitch_state;
@@ -1896,12 +1895,11 @@ static int __init dell_init(void)
 	 * Allocate buffer below 4GB for SMI data--only 32-bit physical addr
 	 * is passed to SMI handler.
 	 */
-	bufferpage = alloc_page(GFP_KERNEL | GFP_DMA32);
-	if (!bufferpage) {
+	buffer = (void *)__get_free_page(GFP_KERNEL | GFP_DMA32);
+	if (!buffer) {
 		ret = -ENOMEM;
 		goto fail_buffer;
 	}
-	buffer = page_address(bufferpage);
 
 	ret = dell_setup_rfkill();
 
@@ -1965,7 +1963,7 @@ fail_backlight:
 	cancel_delayed_work_sync(&dell_rfkill_work);
 	dell_cleanup_rfkill();
 fail_rfkill:
-	free_page((unsigned long)bufferpage);
+	free_page((unsigned long)buffer);
 fail_buffer:
 	platform_device_del(platform_device);
 fail_platform_device2:
-- 
1.7.10.4

Re: [PATCH] dell-laptop: Fix allocating & freeing SMI buffer page

[Darren Hart]

On Tue, Jun 23, 2015 at 10:11:19AM +0200, Pali Rohár wrote:
> This commit fix kernel crash when probing for rfkill devices in dell-laptop
> driver failed. Function free_page() was incorrectly used on struct page *
> instead of virtual address of SMI buffer.
> 
> This commit also simplify allocating page for SMI buffer by using
> __get_free_page() function instead of sequential call of functions
> alloc_page() and page_address().
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Acked-by: Michal Hocko <mhocko@suse.cz>
> Cc: stable@vger.kernel.org

Queued, thanks Pali.

-- 
Darren Hart
Intel Open Source Technology Center

[PATCH 4/4] dell-laptop: Show info about WiGig and UWB in debugfs

[Pali Rohár]

This commit show additional information about rfkill state in debugfs based
on newly released documentation by Dell.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-laptop.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 0a91599..9777195 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -641,12 +641,21 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 		  (status & BIT(4)) >> 4);
 	seq_printf(s, "Bit 5 : Wireless keyboard supported: %lu\n",
 		  (status & BIT(5)) >> 5);
+	seq_printf(s, "Bit 6 : UWB supported:               %lu\n",
+		  (status & BIT(6)) >> 6);
+	seq_printf(s, "Bit 7 : WiGig supported:             %lu\n",
+		  (status & BIT(7)) >> 7);
 	seq_printf(s, "Bit 8 : Wifi is installed:           %lu\n",
 		  (status & BIT(8)) >> 8);
 	seq_printf(s, "Bit 9 : Bluetooth is installed:      %lu\n",
 		  (status & BIT(9)) >> 9);
 	seq_printf(s, "Bit 10: WWAN is installed:           %lu\n",
 		  (status & BIT(10)) >> 10);
+	seq_printf(s, "Bit 11: UWB installed:               %lu\n",
+		  (status & BIT(11)) >> 11);
+	seq_printf(s, "Bit 12: WiGig installed:             %lu\n",
+		  (status & BIT(12)) >> 12);
+
 	seq_printf(s, "Bit 16: Hardware switch is on:       %lu\n",
 		  (status & BIT(16)) >> 16);
 	seq_printf(s, "Bit 17: Wifi is blocked:             %lu\n",
@@ -655,6 +664,10 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 		  (status & BIT(18)) >> 18);
 	seq_printf(s, "Bit 19: WWAN is blocked:             %lu\n",
 		  (status & BIT(19)) >> 19);
+	seq_printf(s, "Bit 20: UWB is blocked:              %lu\n",
+		  (status & BIT(20)) >> 20);
+	seq_printf(s, "Bit 21: WiGig is blocked:            %lu\n",
+		  (status & BIT(21)) >> 21);
 
 	seq_printf(s, "\nhwswitch_state:\t0x%X\n", hwswitch_state);
 	seq_printf(s, "Bit 0 : Wifi controlled by switch:      %lu\n",
@@ -663,6 +676,10 @@ static int dell_debugfs_show(struct seq_file *s, void *data)
 		   (hwswitch_state & BIT(1)) >> 1);
 	seq_printf(s, "Bit 2 : WWAN controlled by switch:      %lu\n",
 		   (hwswitch_state & BIT(2)) >> 2);
+	seq_printf(s, "Bit 3 : UWB controlled by switch:       %lu\n",
+		   (hwswitch_state & BIT(3)) >> 3);
+	seq_printf(s, "Bit 4 : WiGig controlled by switch:     %lu\n",
+		   (hwswitch_state & BIT(4)) >> 4);
 	seq_printf(s, "Bit 7 : Wireless switch config locked:  %lu\n",
 		   (hwswitch_state & BIT(7)) >> 7);
 	seq_printf(s, "Bit 8 : Wifi locator enabled:           %lu\n",
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/4] dell-laptop: Show info about WiGig and UWB in debugfs

[Darren Hart]

On Sun, Jun 21, 2015 at 10:41:42AM +0200, Pali Rohár wrote:
> This commit show additional information about rfkill state in debugfs based
> on newly released documentation by Dell.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

Queued, thanks.

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/4] Fixes for dell-laptop.c driver

[Darren Hart]

On Sun, Jun 21, 2015 at 10:39:25AM +0200, Pali Rohár wrote:
> This patch series contains contains two fixes (properly check return
> values of all SMBIOS calls and fix calling correct page alloc/free
> functions) and documentation update together with extending debugfs
> code. Patch which fixes calling page alloc/free functions should be
> backported to stable kernel series as it it fix kernel crash.

See stable_kernel_rules and add the stable Cc to this one patch please.

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/