Add rfkill support to compal-laptop
Subsystems (3)
PATCH 2/3 Add rfkill support to compal-laptop
In order to be useful in modern kernels and standard interfaces, compal-laptop should have rfkill support. This patch adds it.
Signature · 3 lines
Mario Limonciello *Dell | Linux Engineering* [email protected]
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +102 -0
@@ -52,6 +52,7 @@#include <linux/backlight.h> #include <linux/platform_device.h> #include <linux/autoconf.h> +#include <linux/rfkill.h> #define COMPAL_DRIVER_VERSION "0.2.6"
@@ -64,6 +65,9 @@#define WLAN_MASK 0x01#define BT_MASK 0x02 +static struct rfkill *wifi_rfkill; +static struct rfkill *bluetooth_rfkill; +static int force;module_param (force, bool,0 );MODULE_PARM_DESC (force,"Force driver load, ignore DMI data" );
@@ -89,6 +93,87 @@return (int ) result; } +static void compal_rfkill_query (struct rfkill *rfkill,void *data) +{ +unsigned long radio = (unsigned long ) data; +u8 result; +bool blocked; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + +if ((result &KILLSWITCH_MASK ) ==0 ) + blocked =1 ; +else if (radio ==WLAN_MASK ) + blocked = !(result &WLAN_MASK ); +else + blocked = !((result &BT_MASK ) >>1 ); + +rfkill_set_sw_state (rfkill,blocked); +rfkill_set_hw_state (rfkill,0 ); +} + +static int compal_rfkill_set (void *data,bool blocked) +{ +unsigned long radio = (unsigned long ) data; +u8 result, value; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + +if ((result &KILLSWITCH_MASK ) ==0 ) +return -EINVAL ; +else { +if (!blocked) + value = (u8) (result | radio); +else + value = (u8) (result & ~radio); +ec_write (COMPAL_EC_COMMAND_WIRELESS , value); + } + +return 0 ; +} + +static const struct rfkill_ops compal_rfkill_ops = { + .set_block = compal_rfkill_set, + .query = compal_rfkill_query, +}; + +static int setup_rfkill (void ) +{ +int ret; + + wifi_rfkill =rfkill_alloc ("compal-wifi" ,NULL ,RFKILL_TYPE_WLAN , + &compal_rfkill_ops, (void *)WLAN_MASK ); +if (!wifi_rfkill) { + ret = -ENOMEM ; + goto err_wifi; + } + ret =rfkill_register (wifi_rfkill); +if (ret) + goto err_wifi; + + bluetooth_rfkill =rfkill_alloc ("compal-bluetooth" ,NULL ,RFKILL_TYPE_BLUETOOTH , + &compal_rfkill_ops, (void *)BT_MASK ); +if (!bluetooth_rfkill) { + ret = -ENOMEM ; + goto err_bt; + } + ret =rfkill_register (bluetooth_rfkill); +if (ret) + goto err_bt; + +return 0 ; +err_bt: +rfkill_destroy (bluetooth_rfkill); +if (bluetooth_rfkill) +rfkill_unregister (bluetooth_rfkill); +err_wifi: +rfkill_destroy (wifi_rfkill); +if (wifi_rfkill) +rfkill_unregister (wifi_rfkill); + +return ret; +} +static int set_wlan_state (int state) {u8 result, value;
@@ -357,6 +442,12 @@if (!force && !dmi_check_system (compal_dmi_table))return -ENODEV ; + ret =setup_rfkill (); +if (ret) { +printk (KERN_WARNING "compal-laptop: Unable to setup rfkill\n" ); + goto fail_rfkill; + } +/* Register backlight stuff */ if (!acpi_video_backlight_support ()) {
@@ -410,6 +501,13 @@backlight_device_unregister (compalbl_device); +fail_rfkill: + +if (wifi_rfkill) +rfkill_unregister (wifi_rfkill); +if (bluetooth_rfkill) +rfkill_unregister (bluetooth_rfkill); +return ret; }
@@ -420,6 +518,10 @@platform_device_unregister (compal_device);platform_driver_unregister (&compal_driver);backlight_device_unregister (compalbl_device); +if (wifi_rfkill) +rfkill_unregister (wifi_rfkill); +if (bluetooth_rfkill) +rfkill_unregister (bluetooth_rfkill);printk (KERN_INFO "compal-laptop: driver unloaded.\n" ); }
Hi Mario,
the general rule for patches on LKML is that you send them inline and not as attachment. A successful workaround for the ones with Exchange only mail account is to use git send-email.
Quoted 28 lines
---
drivers/platform/x86/compal-laptop.c.old2009-08-17
07:01:36.244874430 -0500
+++drivers/platform/x86/compal-laptop.c2009-08-17
07:02:15.012836625 -0500platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +6
-0@@ -52,6 +52,7 @@#include <linux/backlight.h> #include <linux/platform_device.h> #include <linux/autoconf.h> +#include <linux/rfkill.h> #define COMPAL_DRIVER_VERSION "0.2.6"@@ -64,6 +65,9 @@#define WLAN_MASK 0x01#define BT_MASK 0x02 +static struct rfkill *wifi_rfkill; +static struct rfkill *bluetooth_rfkill; +static int force;module_param (force, bool,0 );MODULE_PARM_DESC (force,"Force driver load, ignore DMI data" );@@ -89,6 +93,87 @@return (int ) result; } +static void compal_rfkill_query (struct rfkill *rfkill,void *data) +{
platform/x86/compal-laptop.c line 107
Quoted 10 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +10
-0+unsigned long radio = (unsigned long ) data; +u8 result; +bool blocked; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + +if ((result &KILLSWITCH_MASK ) ==0 ) + blocked =1 ; +else if (radio ==WLAN_MASK ) + blocked = !(result &WLAN_MASK );
You are using spaces instead of tabs here.
Quoted 13 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +13
-0+else + blocked = !((result &BT_MASK ) >>1 ); + +rfkill_set_sw_state (rfkill,blocked); +rfkill_set_hw_state (rfkill,0 ); +} + +static int compal_rfkill_set (void *data,bool blocked) +{ +unsigned long radio = (unsigned long ) data; +u8 result, value; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result);
platform/x86/compal-laptop.c line 130
Quoted 10 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +10
-0+ +if ((result &KILLSWITCH_MASK ) ==0 ) +return -EINVAL ; +else { +if (!blocked) + value = (u8) (result | radio); +else + value = (u8) (result & ~radio); +ec_write (COMPAL_EC_COMMAND_WIRELESS , value); + }
This else part is utterly stupid. There is no need to have an else statement. You already left the function. So get rid of it. And at the same time this code becomes readable.
Quoted 35 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +24
-0+return 0 ; +} + +static const struct rfkill_ops compal_rfkill_ops = { + .set_block = compal_rfkill_set, + .query = compal_rfkill_query, +}; + +static int setup_rfkill (void ) +{ +int ret; + + wifi_rfkill =rfkill_alloc ("compal-wifi" ,NULL ,RFKILL_TYPE_WLAN , + &compal_rfkill_ops, (void *)WLAN_MASK ); +if (!wifi_rfkill) { + ret = -ENOMEM ; + goto err_wifi; + } + ret =rfkill_register (wifi_rfkill); +if (ret) + goto err_wifi; + + bluetooth_rfkill = rfkill_alloc("compal-bluetooth", NULL, RFKILL_TYPE_BLUETOOTH, + &compal_rfkill_ops, (void *)
BT_MASK);platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +7
-0+if (!bluetooth_rfkill) { + ret = -ENOMEM ; + goto err_bt; + } + ret =rfkill_register (bluetooth_rfkill); +if (ret) + goto err_bt;
platform/x86/compal-laptop.c line 172
Quoted 10 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +10
-0+ +return 0 ; +err_bt: +rfkill_destroy (bluetooth_rfkill); +if (bluetooth_rfkill) +rfkill_unregister (bluetooth_rfkill); +err_wifi: +rfkill_destroy (wifi_rfkill); +if (wifi_rfkill) +rfkill_unregister (wifi_rfkill);
I don't understand how this is not a potential NULL pointer dereference. There might some good luck that the pointer is still valid at that time, but I highly doubt it. So please unregister before destory.
Quoted 22 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +10
-0+ +return ret; +} +static int set_wlan_state (int state) {u8 result, value;@@ -357,6 +442,12 @@if (!force && !dmi_check_system (compal_dmi_table))return -ENODEV ; + ret =setup_rfkill (); +if (ret) { +printk (KERN_WARNING "compal-laptop: Unable to setup rfkill\n"); + goto fail_rfkill; + } +/* Register backlight stuff */ if (!acpi_video_backlight_support()) {
platform/x86/compal-laptop.c line 510
Quoted 10 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +7
-0@@ -410,6 +501,13 @@backlight_device_unregister (compalbl_device); +fail_rfkill: + +if (wifi_rfkill) +rfkill_unregister (wifi_rfkill); +if (bluetooth_rfkill) +rfkill_unregister (bluetooth_rfkill); +
What non-sense is this. If setup_rfkill() fails, then both RFKILL switches got unregistered if they ever have been registered.
platform/x86/compal-laptop.c line 524
Quoted 11 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +4
-0return ret; }@@ -420,6 +518,10 @@platform_device_unregister (compal_device);platform_driver_unregister (&compal_driver);backlight_device_unregister (compalbl_device); +if (wifi_rfkill) +rfkill_unregister (wifi_rfkill); +if (bluetooth_rfkill) +rfkill_unregister (bluetooth_rfkill);
Same here. It should never ever succeeded in the first place. You can call it conditionally.
platform/x86/compal-laptop.c
Quoted 4 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +0
-0printk (KERN_INFO "compal-laptop: driver unloaded.\n" ); }
Regards
Marcel
Quoted 1 line · Marcel Holtmann
Hi Mario,
...
Quoted 33 lines
Quoted 33 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +5
-0+static int setup_rfkill (void ) +{ +int ret; + + wifi_rfkill = rfkill_alloc("compal-wifi", NULL,
RFKILL_TYPE_WLAN,
+&compal_rfkill_ops, (void *)WLAN_MASK);platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +9
-0+if (!wifi_rfkill) { + ret = -ENOMEM ; + goto err_wifi; + } + ret =rfkill_register (wifi_rfkill); +if (ret) + goto err_wifi; + + bluetooth_rfkill = rfkill_alloc("compal-bluetooth", NULL,
RFKILL_TYPE_BLUETOOTH,
+&compal_rfkill_ops, (void *)BT_MASK);platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +13
-0+if (!bluetooth_rfkill) { + ret = -ENOMEM ; + goto err_bt; + } + ret =rfkill_register (bluetooth_rfkill); +if (ret) + goto err_bt; + +return 0 ; +err_bt: +rfkill_destroy (bluetooth_rfkill); +if (bluetooth_rfkill) +rfkill_unregister (bluetooth_rfkill);
Quoted 8 lines
platform/x86/compal-laptop.c line 172
Quoted 4 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +4
-0+err_wifi: +rfkill_destroy (wifi_rfkill); +if (wifi_rfkill) +rfkill_unregister (wifi_rfkill);I don't understand how this is not a potential NULL pointer dereference. There might some good luck that the pointer is still valid at that time, but I highly doubt it. So please unregister before destory.
Wrong as well :-).
If you fail to register wifi_rfkill, you should *only* call rfkill_destroy(). So I think it should look like this:
+ if (wifi_rfkill)
+ rfkill_unregister(wifi_rfkill);
+err_wifi:
+ rfkill_destroy(wifi_rfkill);
...
Quoted 11 lines
platform/x86/compal-laptop.c line 524
Quoted 8 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +4
-0@@ -420,6 +518,10 @@platform_device_unregister (compal_device);platform_driver_unregister (&compal_driver);backlight_device_unregister (compalbl_device); +if (wifi_rfkill) +rfkill_unregister (wifi_rfkill); +if (bluetooth_rfkill) +rfkill_unregister (bluetooth_rfkill);Same here. It should never ever succeeded in the first place. You can call it conditionally.
They're already called conditionally. I assume you mean unconditionally here.
I agree with all your other comments. Although I wouldn't call the return/else style issue stupid, I'd just say it was confused :-).
Regards
Alan
Quoted 7 lines · Mario Limonciello
In order to be useful in modern kernels and standard interfaces, compal-laptop should have rfkill support. This patch adds it. --
Mario Limonciello
*Dell | Linux Engineering*
[email protected]
I have some comments of my own as well, so here goes.
Quoted 5 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +5
-0+static void compal_rfkill_query (struct rfkill *rfkill,void *data) +{ +unsigned long radio = (unsigned long ) data; +u8 result; +bool blocked;
platform/x86/compal-laptop.c line 109
Quoted 10 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +9
-0+ +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + +if ((result &KILLSWITCH_MASK ) ==0 ) + blocked =1 ; +else if (radio ==WLAN_MASK ) + blocked = !(result &WLAN_MASK ); +else + blocked = !((result &BT_MASK ) >>1 );
Ick, this is overdone. You should just be able to do
+ else
+ blocked = !(result & radio);
platform/x86/compal-laptop.c line 112
Quoted 2 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +2
-0+rfkill_set_sw_state (rfkill,blocked); +rfkill_set_hw_state (rfkill,0 );
This last line is redundant. The HW state will default to "unblocked" if you never set it.
Quoted 9 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +9
-0+static int compal_rfkill_set (void *data,bool blocked) +{ +unsigned long radio = (unsigned long ) data; +u8 result, value; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + +if ((result &KILLSWITCH_MASK ) ==0 ) +return -EINVAL ;
platform/x86/compal-laptop.c line 133
Quoted 10 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +10
-0+else { +if (!blocked) + value = (u8) (result | radio); +else + value = (u8) (result & ~radio); +ec_write (COMPAL_EC_COMMAND_WIRELESS , value); + } + +return 0 ; +}
Note that the return value of rfkill_set is not propagated to userspace through the /dev/rfkill interface.
The way you use "KILLSWITCH_MASK", it looks like a hard rfkill bit that covers all radios. Why don't you report it as a hard block on each rfkill device?
Note that strictly speaking, you should only call one rfkill_set function within the query() method.
"@query: query the rfkill block state(s) and call exactly one of the rfkill_set{,_hw,_sw}_state family of functions."
(I'd recommend reading the comments in include/linux/rfkill.h if you haven't already). So If you do want to set both hw and sw states in query(), you should use rfkill_set_states().
platform/x86/compal-laptop.c line 145
Quoted 6 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +6
-0+static int setup_rfkill (void ) +{ +int ret; + + wifi_rfkill =rfkill_alloc ("compal-wifi" ,NULL ,RFKILL_TYPE_WLAN , + &compal_rfkill_ops, (void *)WLAN_MASK );
I think you should use "compal_device->dev" in place of NULL here. Otherwise the rfkill devices will appear in sysfs as "virtual devices", belonging to no "physical" device.
platform/x86/compal-laptop.c line 155
Quoted 2 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +2
-0+ bluetooth_rfkill =rfkill_alloc ("compal-bluetooth" ,NULL ,RFKILL_TYPE_BLUETOOTH , + &compal_rfkill_ops, (void *)BT_MASK );
Same here.
Regards
Alan
Quoted 1 line · Marcel Holtmann
Hi Mario,
...
Quoted 6 lines
Quoted 6 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +6
-0+static void compal_rfkill_query (struct rfkill *rfkill,void *data) +{ +unsigned long radio = (unsigned long ) data; +u8 result; +bool blocked; +
Quoted 8 lines
platform/x86/compal-laptop.c line 107
Quoted 6 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +6
-0+ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + +if ((result &KILLSWITCH_MASK ) ==0 ) + blocked =1 ; +else if (radio ==WLAN_MASK ) + blocked = !(result &WLAN_MASK );You are using spaces instead of tabs here.
Btw Mario, scripts/checkpatch.pl is a good way to check for whitespace errors and a bunch of other trivia. So human patch review can concentrate on the important stuff :-).
Thanks
Alan
Quoted 61 lines · Alan Jenkins
Quoted 7 lines · Mario Limonciello
In order to be useful in modern kernels and standard interfaces, compal-laptop should have rfkill support. This patch adds it. --
Mario Limonciello
*Dell | Linux Engineering*
[email protected]I have some comments of my own as well, so here goes.
Quoted 5 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +5
-0+static void compal_rfkill_query (struct rfkill *rfkill,void *data) +{ +unsigned long radio = (unsigned long ) data; +u8 result; +bool blocked;platform/x86/compal-laptop.c line 109
Quoted 10 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +9
-0+ +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + +if ((result &KILLSWITCH_MASK ) ==0 ) + blocked =1 ; +else if (radio ==WLAN_MASK ) + blocked = !(result &WLAN_MASK ); +else + blocked = !((result &BT_MASK ) >>1 );Ick, this is overdone. You should just be able to do
(quoted diff) +2
-0+ else + blocked = !(result & radio);platform/x86/compal-laptop.c line 112
Quoted 3 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +2
-0+rfkill_set_sw_state (rfkill,blocked); +rfkill_set_hw_state (rfkill,0 );This last line is redundant. The HW state will default to "unblocked" if you never set it.
Quoted 9 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +9
-0+static int compal_rfkill_set (void *data,bool blocked) +{ +unsigned long radio = (unsigned long ) data; +u8 result, value; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + +if ((result &KILLSWITCH_MASK ) ==0 ) +return -EINVAL ;platform/x86/compal-laptop.c line 133
Quoted 10 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +10
-0+else { +if (!blocked) + value = (u8) (result | radio); +else + value = (u8) (result & ~radio); +ec_write (COMPAL_EC_COMMAND_WIRELESS , value); + } + +return 0 ; +}Note that the return value of
rfkill_setis not propagated to userspace through the/dev/rfkillinterface.
Quoted 3 lines
The way you use "
KILLSWITCH_MASK", it looks like a hard rfkill bit that covers all radios. Why don't you report it as a hard block on each rfkill device?
P.S. If you do this, you probably also want to set up polling. I think you should be able to reuse the query() function for the poll() method (but you should check yourself whether that makes sense).
Thanks again
Alan
Quoted 71 lines · Alan Jenkins
Quoted 1 line · Marcel Holtmann
Hi Mario,
...
Quoted 33 lines
Quoted 33 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +5
-0+static int setup_rfkill (void ) +{ +int ret; + + wifi_rfkill = rfkill_alloc("compal-wifi", NULL,
RFKILL_TYPE_WLAN,
+&compal_rfkill_ops, (void *)WLAN_MASK);platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +9
-0+if (!wifi_rfkill) { + ret = -ENOMEM ; + goto err_wifi; + } + ret =rfkill_register (wifi_rfkill); +if (ret) + goto err_wifi; + + bluetooth_rfkill = rfkill_alloc("compal-bluetooth", NULL,
RFKILL_TYPE_BLUETOOTH,
+&compal_rfkill_ops, (void *)BT_MASK);platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +13
-0+if (!bluetooth_rfkill) { + ret = -ENOMEM ; + goto err_bt; + } + ret =rfkill_register (bluetooth_rfkill); +if (ret) + goto err_bt; + +return 0 ; +err_bt: +rfkill_destroy (bluetooth_rfkill); +if (bluetooth_rfkill) +rfkill_unregister (bluetooth_rfkill);Quoted 8 lines
platform/x86/compal-laptop.c line 172
Quoted 4 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +4
-0+err_wifi: +rfkill_destroy (wifi_rfkill); +if (wifi_rfkill) +rfkill_unregister (wifi_rfkill);I don't understand how this is not a potential NULL pointer dereference. There might some good luck that the pointer is still valid at that time, but I highly doubt it. So please unregister before destory.
Wrong as well :-).
If you fail to register
wifi_rfkill, you should *only* callrfkill_destroy(). So I think it should look like this:platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +4
-0+if (wifi_rfkill) +rfkill_unregister (wifi_rfkill); +err_wifi: +rfkill_destroy (wifi_rfkill);Quoted 12 lines
platform/x86/compal-laptop.c line 524
Quoted 9 lines
platform/x86/compal-laptop.c renamed from platform/x86/compal-laptop.c.old +4
-0@@ -420,6 +518,10 @@platform_device_unregister (compal_device);platform_driver_unregister (&compal_driver);backlight_device_unregister (compalbl_device); +if (wifi_rfkill) +rfkill_unregister (wifi_rfkill); +if (bluetooth_rfkill) +rfkill_unregister (bluetooth_rfkill);Same here. It should never ever succeeded in the first place. You can call it conditionally.
Quoted 2 lines
They're already called conditionally. I assume you mean unconditionally here.
Also, you're missing the calls to rfkill_destroy() here.
Whew, I think that's everything. I hope you find the feedback useful, despite it being a little fragmented.
Regards
Alan
Hi Alan & Marcel:
Quoted 6 lines · Alan Jenkins
Also, you're missing the calls to
rfkill_destroy()here.Whew, I think that's everything. I hope you find the feedback useful, despite it being a little fragmented.
Thanks for all the feedback. I think i've addressed all of the concerns that were pointed out. I appreciate the pointer to scripts/cleanpatch, that does significantly help in finding whitespace problems that the naked eye just browses over.
I'm attaching the updated patch (sorry, git send-email seems to still not be very graceful with line breaks when the SMTP implementation is exchange from what i've seen)
Signature · 3 lines
Mario Limonciello *Dell | Linux Engineering* [email protected]
compal-laptop.c renamed from compal-laptop.old +93 -2
@@ -52,6 +52,7 @@#include <linux/backlight.h> #include <linux/platform_device.h> #include <linux/autoconf.h> +#include <linux/rfkill.h> #define COMPAL_DRIVER_VERSION "0.2.6"
@@ -64,6 +65,10 @@#define WLAN_MASK 0x01#define BT_MASK 0x02 +static struct rfkill *wifi_rfkill; +static struct rfkill *bt_rfkill; +static struct platform_device *compal_device; +static int force;module_param (force, bool,0 );MODULE_PARM_DESC (force,"Force driver load, ignore DMI data" );
@@ -89,6 +94,86 @@return (int ) result; } +static void compal_rfkill_poll (struct rfkill *rfkill,void *data) +{ +unsigned long radio = (unsigned long ) data; +u8 result; +bool hw_blocked; +bool sw_blocked; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + + hw_blocked = !(result &KILLSWITCH_MASK ); + sw_blocked = (!hw_blocked && !(result & radio)); + +rfkill_set_states (rfkill, sw_blocked, hw_blocked); +} + +static int compal_rfkill_set (void *data,bool blocked) +{ +unsigned long radio = (unsigned long ) data; +u8 result, value; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + +if ((result &KILLSWITCH_MASK ) ==0 ) +return -EINVAL ; + +if (!blocked) + value = (u8) (result | radio); +else + value = (u8) (result & ~radio); +ec_write (COMPAL_EC_COMMAND_WIRELESS , value); + +return 0 ; +} + +static const struct rfkill_ops compal_rfkill_ops = { + .poll = compal_rfkill_poll, + .set_block = compal_rfkill_set, +}; + +static int setup_rfkill (void ) +{ +int ret; + + wifi_rfkill =rfkill_alloc ("compal-wifi" , &compal_device->dev, +RFKILL_TYPE_WLAN , &compal_rfkill_ops, + (void *)WLAN_MASK ); +if (!wifi_rfkill) { + ret = -ENOMEM ; + goto err_wifi; + } + ret =rfkill_register (wifi_rfkill); +if (ret) { +rfkill_unregister (wifi_rfkill); + goto err_wifi; + } + + bt_rfkill =rfkill_alloc ("compal-bluetooth" , &compal_device->dev, +RFKILL_TYPE_BLUETOOTH , &compal_rfkill_ops, + (void *)BT_MASK ); +if (!bt_rfkill) { + ret = -ENOMEM ; + goto err_bt; + } + ret =rfkill_register (bt_rfkill); +if (ret) { +rfkill_unregister (bt_rfkill); + goto err_bt; + } + +return 0 ; + +err_bt: +rfkill_destroy (bt_rfkill); + +err_wifi: +rfkill_destroy (wifi_rfkill); + +return ret; +} +static int set_wlan_state (int state) {u8 result, value;
@@ -258,8 +343,6 @@ } }; -static struct platform_device *compal_device; -/* Initialization */ static int dmi_check_cb (const struct dmi_system_id *id)
@@ -389,6 +472,10 @@if (ret) goto fail_platform_device2; + ret =setup_rfkill (); +if (ret) +printk (KERN_WARNING "compal-laptop: Unable to setup rfkill\n" ); +printk (KERN_INFO "compal-laptop: driver " COMPAL_DRIVER_VERSION " successfully loaded.\n" );
@@ -420,6 +507,10 @@platform_device_unregister (compal_device);platform_driver_unregister (&compal_driver);backlight_device_unregister (compalbl_device); +rfkill_unregister (wifi_rfkill); +rfkill_destroy (wifi_rfkill); +rfkill_unregister (bt_rfkill); +rfkill_destroy (bt_rfkill);printk (KERN_INFO "compal-laptop: driver unloaded.\n" ); }
Quoted 22 lines · Mario Limonciello
Hi Alan & Marcel:
Quoted 6 lines · Alan Jenkins
Also, you're missing the calls to
rfkill_destroy()here.Whew, I think that's everything. I hope you find the feedback useful, despite it being a little fragmented.
Thanks for all the feedback. I think i've addressed all of the concerns that were pointed out. I appreciate the pointer to
scripts/cleanpatch, that does significantly help in finding whitespace problems that the naked eye just browses over.I'm attaching the updated patch (sorry, git send-email seems to still not be very graceful with line breaks when the SMTP implementation is exchange from what i've seen)
compal-laptop.c renamed from compal-laptop.old +4
-0+static void compal_rfkill_poll (struct rfkill *rfkill,void *data) +{ +unsigned long radio = (unsigned long ) data; +u8 result;
compal-laptop.c line 110
Quoted 10 lines
compal-laptop.c renamed from compal-laptop.old +10
-0+bool hw_blocked; +bool sw_blocked; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + + hw_blocked = !(result &KILLSWITCH_MASK ); + sw_blocked = (!hw_blocked && !(result & radio)); + +rfkill_set_states (rfkill, sw_blocked, hw_blocked); +}
I assume you have good reason for having sw_block depend on hw_block. I.e. you can't read sw_blocked while hw_blocked is set, right?
If KILLSWITCH is toggled on and off, will the hardware "forget" any prior soft-blocks?
It would also be nice to know if hardware/firmware ever changes sw_blocked, e.g. in response to a button press.
Johannes, I think I'm confusing myself here. Can you have a look at this code? I remember the rfkill rewrite was designed to help with something like this, but I don't know how exactly.
Thanks
Alan
Quoted 5 lines · Mario Limonciello
Hi Alan & Marcel:
Quoted 2 lines · Alan Jenkins
Also, you're missing the calls to
rfkill_destroy()here.
Quoted 8 lines
Quoted 4 lines
Whew, I think that's everything. I hope you find the feedback useful, despite it being a little fragmented.
Thanks for all the feedback. I think i've addressed all of the concerns that were pointed out. I appreciate the pointer to
scripts/cleanpatch, that does significantly help in finding whitespace problems that the naked eye just browses over.
I think you've addressed most of the comments on your first patch. There are still problems with the error handling though.
+static int setup_rfkill(void)
+{
+ int ret;
+
+ wifi_rfkill = rfkill_alloc("compal-wifi", &compal_device->dev,
+ RFKILL_TYPE_WLAN, &compal_rfkill_ops,
+ (void *) WLAN_MASK);
+ if (!wifi_rfkill) {
+ ret = -ENOMEM;
+ goto err_wifi;
+ }
+ ret = rfkill_register(wifi_rfkill);
+ if (ret) {
+ rfkill_unregister(wifi_rfkill);
+ goto err_wifi;
+ }
If you fail to register an rfkill device, you don't need to unregister that rfkill device...
+
+ bt_rfkill = rfkill_alloc("compal-bluetooth", &compal_device->dev,
+ RFKILL_TYPE_BLUETOOTH, &compal_rfkill_ops,
+ (void *) BT_MASK);
+ if (!bt_rfkill) {
+ ret = -ENOMEM;
+ goto err_bt;
+ }
+ ret = rfkill_register(bt_rfkill);
+ if (ret) {
+ rfkill_unregister(bt_rfkill);
+ goto err_bt;
+ }
and the same applies here
+
+ return 0;
+
+err_bt:
+ rfkill_destroy(bt_rfkill);
+
... but you *do* need to unregister wifi_rfkill here, before you go on to destroy it.
+err_wifi:
+ rfkill_destroy(wifi_rfkill);
+
+ return ret;
+}
Regards
Alan
Hi everyone,
Quoted 22 lines
Quoted 8 lines
I'm attaching the updated patch (sorry, git send-email seems to still not be very graceful with line breaks when the SMTP implementation is exchange from what i've seen)
compal-laptop.c renamed from compal-laptop.old +4
-0+static void compal_rfkill_poll (struct rfkill *rfkill,void *data) +{ +unsigned long radio = (unsigned long ) data; +u8 result;compal-laptop.c line 110
Quoted 10 lines
compal-laptop.c renamed from compal-laptop.old +10
-0+bool hw_blocked; +bool sw_blocked; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + + hw_blocked = !(result &KILLSWITCH_MASK ); + sw_blocked = (!hw_blocked && !(result & radio)); + +rfkill_set_states (rfkill, sw_blocked, hw_blocked); +}I assume you have good reason for having
sw_blockdepend onhw_block. I.e. you can't readsw_blockedwhilehw_blockedis set, right?
Quoted 2 lines
If KILLSWITCH is toggled on and off, will the hardware "forget" any prior soft-blocks?
That's a bit strange indeed, but I haven't seen the rest of the code.
Does the 'soft block' bit change based on user input, like pressing a button?
If not, you shouldn't poll that bit at all, but just set it based on what rfkill gives you as the return value of set_hw_state().
hth,
johannes
Hi Guys:
Quoted 11 lines · Johannes Berg
Hi everyone,
That's a bit strange indeed, but I haven't seen the rest of the code.
Does the 'soft block' bit change based on user input, like pressing a button?
If not, you shouldn't poll that bit at all, but just set it based on what rfkill gives you as the return value of
set_hw_state().
No it doesn't, so i've followed your advice in an updated patch, Thanks.
Quoted 9 lines · Alan Jenkins
... but you *do* need to unregister
wifi_rfkillhere, before you go on to destroy it.(quoted diff) +5
-0+err_wifi: + rfkill_destroy(wifi_rfkill); + + return ret; +}
Quoted 3 lines
Regards
Alan
I think I've addressed this properly now and only go through each of the error handlers as necessary.
Signature · 3 lines
Mario Limonciello *Dell | Linux Engineering* [email protected]
compal-laptop.c renamed from compal-laptop.c.old +91 -2
@@ -52,6 +52,7 @@#include <linux/backlight.h> #include <linux/platform_device.h> #include <linux/autoconf.h> +#include <linux/rfkill.h> #define COMPAL_DRIVER_VERSION "0.2.6"
@@ -64,6 +65,10 @@#define WLAN_MASK 0x01#define BT_MASK 0x02 +static struct rfkill *wifi_rfkill; +static struct rfkill *bt_rfkill; +static struct platform_device *compal_device; +static int force;module_param (force, bool,0 );MODULE_PARM_DESC (force,"Force driver load, ignore DMI data" );
@@ -89,6 +94,84 @@return (int ) result; } +static void compal_rfkill_poll (struct rfkill *rfkill,void *data) +{ +unsigned long radio = (unsigned long ) data; +u8 result; +bool hw_blocked; +bool sw_blocked; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + + hw_blocked = !(result & (KILLSWITCH_MASK | radio)); + sw_blocked =rfkill_set_hw_state (rfkill, hw_blocked); + +rfkill_set_sw_state (rfkill, sw_blocked); +} + +static int compal_rfkill_set (void *data,bool blocked) +{ +unsigned long radio = (unsigned long ) data; +u8 result, value; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + +if ((result &KILLSWITCH_MASK ) ==0 ) +return -EINVAL ; + +if (!blocked) + value = (u8) (result | radio); +else + value = (u8) (result & ~radio); +ec_write (COMPAL_EC_COMMAND_WIRELESS , value); + +return 0 ; +} + +static const struct rfkill_ops compal_rfkill_ops = { + .poll = compal_rfkill_poll, + .set_block = compal_rfkill_set, +}; + +static int setup_rfkill (void ) +{ +int ret; + + wifi_rfkill =rfkill_alloc ("compal-wifi" , &compal_device->dev, +RFKILL_TYPE_WLAN , &compal_rfkill_ops, + (void *)WLAN_MASK ); +if (!wifi_rfkill) +return -ENOMEM ; + + ret =rfkill_register (wifi_rfkill); +if (ret) + goto err_wifi; + + bt_rfkill =rfkill_alloc ("compal-bluetooth" , &compal_device->dev, +RFKILL_TYPE_BLUETOOTH , &compal_rfkill_ops, + (void *)BT_MASK ); +if (!bt_rfkill) { + ret = -ENOMEM ; + goto err_allocate_bt; + } + ret =rfkill_register (bt_rfkill); +if (ret) + goto err_register_bt; + +return 0 ; + +err_register_bt: +rfkill_destroy (bt_rfkill); + +err_allocate_bt: +rfkill_unregister (wifi_rfkill); + +err_wifi: +rfkill_destroy (wifi_rfkill); + +return ret; +} +static int set_wlan_state (int state) {u8 result, value;
@@ -258,8 +341,6 @@ } }; -static struct platform_device *compal_device; -/* Initialization */ static int dmi_check_cb (const struct dmi_system_id *id)
@@ -397,6 +478,10 @@if (ret) goto fail_platform_device2; + ret =setup_rfkill (); +if (ret) +printk (KERN_WARNING "compal-laptop: Unable to setup rfkill\n" ); +printk (KERN_INFO "compal-laptop: driver " COMPAL_DRIVER_VERSION " successfully loaded.\n" );
@@ -428,6 +513,10 @@platform_device_unregister (compal_device);platform_driver_unregister (&compal_driver);backlight_device_unregister (compalbl_device); +rfkill_unregister (wifi_rfkill); +rfkill_destroy (wifi_rfkill); +rfkill_unregister (bt_rfkill); +rfkill_destroy (bt_rfkill);printk (KERN_INFO "compal-laptop: driver unloaded.\n" ); }
Alan Jenkins
Reviewed-byQuoted 1 line · Mario Limonciello
Hi Guys:
Hi again Mario
Quoted 11 lines
Quoted 9 lines · Alan Jenkins
... but you *do* need to unregister
wifi_rfkillhere, before you go on to destroy it.compal-laptop.c renamed from compal-laptop.c.old +5
-0+err_wifi: +rfkill_destroy (wifi_rfkill); + +return ret; +}
Quoted 5 lines
Quoted 3 lines
Regards
AlanI think I've addressed this properly now and only go through each of the error handlers as necessary.
Yes, that looks better. I'm still a bit confused about poll(), I'll have to leave that for Johannes to verify. Feel free to add
Regards
Alan
Ah, heh, thanks Alan for pointing out there was a patch here :)
compal-laptop.c line 106
Quoted 10 lines
compal-laptop.c renamed from compal-laptop.c.old +10
-0+static void compal_rfkill_poll (struct rfkill *rfkill,void *data) +{ +unsigned long radio = (unsigned long ) data; +u8 result; +bool hw_blocked; +bool sw_blocked; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + + hw_blocked = !(result & (KILLSWITCH_MASK | radio));
I don't quite understand the "| radio" bit since that seems to be the soft kill bit according to rfkill_set()?
compal-laptop.c line 109
Quoted 3 lines
compal-laptop.c renamed from compal-laptop.c.old +3
-0+ sw_blocked =rfkill_set_hw_state (rfkill, hw_blocked); + +rfkill_set_sw_state (rfkill, sw_blocked);
This is wrong. You can remove the entire part about sw_blocked, almost.
compal-laptop.c line 120
Quoted 9 lines
compal-laptop.c renamed from compal-laptop.c.old +9
-0+static int compal_rfkill_set (void *data,bool blocked) +{ +unsigned long radio = (unsigned long ) data; +u8 result, value; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + +if ((result &KILLSWITCH_MASK ) ==0 ) +return -EINVAL ;
Anyhow, here you reject the request to set the soft bit. I suspect you could let it go through but it would only change the soft bit in the BIOS, nothing else really.
Two options:
1) You can let it go though, in that case do that, and remove the sw block stuff from poll() completely.
2) You can't let it go through. In this case, you need to leave set as it is, but implement poll like this:
sw_block = rfkill_set_hw_state(rfkill, hw_blocked); compal_rfkill_set(data, sw_block);
so that when the user soft-blocks the device while hard-blocked, the soft block is still honoured after pushing the button on the laptop.
Also, I'm not entirely clear about the semantics -- you've called the bit KILLSWITCH_MASK, but does it really control all technologies as a hard block, i.e. it toggles both the bluetooth and wireless hard block?
johannes
Cezary Jackiewicz
maintainerQuoted 3 lines
Also, I'm not entirely clear about the semantics -- you've called the bit
KILLSWITCH_MASK, but does it really control all technologies as a hard block, i.e. it toggles both the bluetooth and wireless hard block?
In compal's laptop - yes. Switch disables all radios, bluetooth and wifi.
--
Cezary
Mario Limonciello
Reviewed-by Alan JenkinsHi Johannes:
Thanks for looking.
Quoted 7 lines · Johannes Berg
Ah, heh, thanks Alan for pointing out there was a patch here :)
I don't quite understand the "| radio" bit since that seems to be the soft kill bit according to
rfkill_set()?
Yeah you're right, this bit was unnecessary. I pulled it out.
Quoted 8 lines
Anyhow, here you reject the request to set the soft bit. I suspect you could let it go through but it would only change the soft bit in the BIOS, nothing else really.
Two options:
1) You can let it go though, in that case do that, and remove the sw block stuff frompoll()completely.
Quoted 10 lines
2) You can't let it go through. In this case, you need to leave set as it is, but implement poll like this:
sw_block = rfkill_set_hw_state(rfkill, hw_blocked); compal_rfkill_set(data, sw_block);so that when the user soft-blocks the device while hard-blocked, the soft block is still honoured after pushing the button on the laptop.
OK, the second option sounds more desirable, so I've implemented that.
Signature · 3 lines
Mario Limonciello *Dell | Linux Engineering* [email protected]
From 5f5dc9c1adf041418c6dd273cd4ee83d5ae96e74 Mon Sep 17 00:00:00 2001
From: Mario Limonciello <[email protected]>
Date: Wed, 19 Aug 2009 11:41:27 -0500
Subject: [PATCH 2/3] Add rfkill support to compal-laptop
--- drivers/platform/x86/compal-laptop.c | 91 +++++++++++++++++++++++++++++++++- 1 files changed, 89 insertions(+), 2 deletions(-)
drivers/platform/x86/compal-laptop.c +89 -2
@@ -52,6 +52,7 @@#include <linux/backlight.h> #include <linux/platform_device.h> #include <linux/autoconf.h> +#include <linux/rfkill.h> #define COMPAL_DRIVER_VERSION "0.2.6"
@@ -64,6 +65,10 @@#define WLAN_MASK 0x01#define BT_MASK 0x02 +static struct rfkill *wifi_rfkill; +static struct rfkill *bt_rfkill; +static struct platform_device *compal_device; +static int force;module_param (force, bool,0 );MODULE_PARM_DESC (force,"Force driver load, ignore DMI data" );
@@ -89,6 +94,82 @@ static int get_lcd_level(void)return (int ) result; } +static int compal_rfkill_set (void *data,bool blocked) +{ +unsigned long radio = (unsigned long ) data; +u8 result, value; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + +if ((result &KILLSWITCH_MASK ) ==0 ) +return -EINVAL ; + +if (!blocked) + value = (u8) (result | radio); +else + value = (u8) (result & ~radio); +ec_write (COMPAL_EC_COMMAND_WIRELESS , value); + +return 0 ; +} + +static void compal_rfkill_poll (struct rfkill *rfkill,void *data) +{ +u8 result; +bool hw_blocked; +bool sw_blocked; + +ec_read (COMPAL_EC_COMMAND_WIRELESS , &result); + + hw_blocked = !(result &KILLSWITCH_MASK ); + sw_blocked =rfkill_set_hw_state (rfkill, hw_blocked); +compal_rfkill_set (data,sw_blocked); +} + +static const struct rfkill_ops compal_rfkill_ops = { + .poll = compal_rfkill_poll, + .set_block = compal_rfkill_set, +}; + +static int setup_rfkill (void ) +{ +int ret; + + wifi_rfkill =rfkill_alloc ("compal-wifi" , &compal_device->dev, +RFKILL_TYPE_WLAN , &compal_rfkill_ops, + (void *)WLAN_MASK ); +if (!wifi_rfkill) +return -ENOMEM ; + + ret =rfkill_register (wifi_rfkill); +if (ret) + goto err_wifi; + + bt_rfkill =rfkill_alloc ("compal-bluetooth" , &compal_device->dev, +RFKILL_TYPE_BLUETOOTH , &compal_rfkill_ops, + (void *)BT_MASK ); +if (!bt_rfkill) { + ret = -ENOMEM ; + goto err_allocate_bt; + } + ret =rfkill_register (bt_rfkill); +if (ret) + goto err_register_bt; + +return 0 ; + +err_register_bt: +rfkill_destroy (bt_rfkill); + +err_allocate_bt: +rfkill_unregister (wifi_rfkill); + +err_wifi: +rfkill_destroy (wifi_rfkill); + +return ret; +} +static int set_wlan_state (int state) {u8 result, value;
@@ -258,8 +339,6 @@ static struct platform_driver compal_driver = { } }; -static struct platform_device *compal_device; -/* Initialization */ static int dmi_check_cb (const struct dmi_system_id *id)
@@ -356,6 +435,10 @@ static int __init compal_init(void)if (ret) goto fail_platform_device2; + ret =setup_rfkill (); +if (ret) +printk (KERN_WARNING "compal-laptop: Unable to setup rfkill\n" ); +printk (KERN_INFO "compal-laptop: driver " COMPAL_DRIVER_VERSION " successfully loaded.\n" );
@@ -387,6 +470,10 @@ static void __exit compal_cleanup(void)platform_device_unregister (compal_device);platform_driver_unregister (&compal_driver);backlight_device_unregister (compalbl_device); +rfkill_unregister (wifi_rfkill); +rfkill_destroy (wifi_rfkill); +rfkill_unregister (bt_rfkill); +rfkill_destroy (bt_rfkill);printk (KERN_INFO "compal-laptop: driver unloaded.\n" ); }
Signature · 1 line
1.6.3.3
Quoted 24 lines · Mario Limonciello
Hi Johannes:
Thanks for looking.
Quoted 7 lines · Johannes Berg
Ah, heh, thanks Alan for pointing out there was a patch here :)
I don't quite understand the "| radio" bit since that seems to be the soft kill bit according to
rfkill_set()?Yeah you're right, this bit was unnecessary. I pulled it out.
Quoted 11 lines
Anyhow, here you reject the request to set the soft bit. I suspect you could let it go through but it would only change the soft bit in the BIOS, nothing else really.
Two options:
1) You can let it go though, in that case do that, and remove the sw block stuff frompoll()completely.2) You can't let it go through. In this case, you need to leave set as it is, but implement poll like this:
Quoted 8 lines
Quoted 7 lines
sw_block = rfkill_set_hw_state(rfkill, hw_blocked); compal_rfkill_set(data, sw_block);so that when the user soft-blocks the device while hard-blocked, the soft block is still honoured after pushing the button on the laptop.
OK, the second option sounds more desirable, so I've implemented that.
I think the first option is more _desirable_, it's more a matter of whether it can work well on this hardware.
In case 2), the radio will be unblocked for a short period between the button-press, and the next poll() call. But 1) won't work if the hardware "forgets" the soft block when the hard block is toggled on and off.
Regards
Alan
Hi Mario,
First, let me say I agree with Alan, the option 1 is more desirable if possible to do with the hardware. But this looks ok from an rfkill POV now, except there's a small bug:
Quoted 4 lines
drivers/platform/x86/compal-laptop.c +3
-0+ ret =setup_rfkill (); +if (ret) + printk(KERN_WARNING "compal-laptop: Unable to setuprfkill\n");
That doesn't error out, so
drivers/platform/x86/compal-laptop.c static void __exit compal_cleanup(void) · line 476
Quoted 4 lines
drivers/platform/x86/compal-laptop.c +4
-0+rfkill_unregister (wifi_rfkill); +rfkill_destroy (wifi_rfkill); +rfkill_unregister (bt_rfkill); +rfkill_destroy (bt_rfkill);
this will crash without NULL checks.
(and you have to explicitly assign NULL in setup_rfkill() too, when bluetooth fails and wifi is freed)
johannes
Johannes:
Quoted 7 lines · Johannes Berg
Hi Mario,
First, let me say I agree with Alan, the option 1 is more desirable if possible to do with the hardware. But this looks ok from an rfkill POV now, except there's a small bug:
It looks like option 1 works properly on my hardware, so I've switched the other code around.
Quoted 11 lines
That doesn't error out, so
this will crash without NULL checks.
(and you have to explicitly assign NULL in
setup_rfkill()too, when bluetooth fails and wifi is freed)
OK, i've cleaned that up to just error out on the module if rfkill doesn't get initialized right.
Quoted 2 lines
johannes
I've resent separately, and think I have git send-email working, so hopefully won't have to attach in the future.
Signature · 3 lines
Mario Limonciello *Dell | Linux Engineering* [email protected]