diff mbox

[2/3] Add rfkill support to compal-laptop

Message ID 4A8B246D.7050004@dell.com (mailing list archive)
State RFC, archived
Headers show

Commit Message

Mario Limonciello Aug. 18, 2009, 10 p.m. UTC 
Hi Guys:

Johannes Berg wrote:
> 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.


Alan Jenkins wrote:

> ... 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
>   
I think I've addressed this properly now and only go through each of the error handlers as necessary.

Comments

Alan Jenkins Aug. 19, 2009, 8:51 a.m. UTC | #1 
On 8/18/09, Mario Limonciello <mario_limonciello@dell.com> wrote:
> Hi Guys:

Hi again Mario

> Alan Jenkins wrote:
>
>> ... 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
>>
> I 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

Reviewed-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>

Regards
Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Berg Aug. 19, 2009, 9:01 a.m. UTC | #2 
Ah, heh, thanks Alan for pointing out there was a patch here :)

> +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()?

> +       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.

> +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 Aug. 19, 2009, 11:43 a.m. UTC | #3 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- compal-laptop.c.old	2009-08-18 05:23:39.668669312 -0500
+++ compal-laptop.c	2009-08-18 05:49:00.098015155 -0500
@@ -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");
 }