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

Commit Message

Mario Limonciello Aug. 19, 2009, 4:46 p.m. UTC

Hi Johannes:

Thanks for looking.

Johannes Berg wrote:
> 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.
> 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.
>
>   
OK, the second option sounds more desirable, so I've implemented that.

Comments

Alan Jenkins Aug. 19, 2009, 4:57 p.m. UTC | #1

On 8/19/09, Mario Limonciello <[e-mail]> wrote:
> Hi Johannes:
>
> Thanks for looking.
>
> Johannes Berg wrote:
>> 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.
>> 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.
>>
>>
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Johannes Berg Aug. 19, 2009, 5:13 p.m. UTC | #2

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:

> +       ret = setup_rfkill();
> +       if (ret)
> +               printk(KERN_WARNING "compal-laptop: Unable to setup
> rfkill\n");

That doesn't error out, so

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

Mario Limonciello Aug. 19, 2009, 6:39 p.m. UTC | #3

Johannes:

Johannes Berg wrote:
> 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.
>
> 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.
> johannes
>   
I've resent separately, and think I have git send-email working, so
hopefully won't have to attach in the future.

Patch

From 5f5dc9c1adf041418c6dd273cd4ee83d5ae96e74 Mon Sep 17 00:00:00 2001
From: Mario Limonciello <[e-mail]>
Date: Wed, 19 Aug 2009 11:41:27 -0500
Subject: [PATCH 2/3] Add rfkill support to compal-laptop

Signed-off-by: Mario Limonciello <[e-mail]>
Reviewed-by: Alan Jenkins <[e-mail]>
---
 drivers/platform/x86/compal-laptop.c |   91 +++++++++++++++++++++++++++++++++-
 1 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/compal-laptop.c b/drivers/platform/x86/compal-laptop.c
index 11003bb..d997de5 100644
--- a/drivers/platform/x86/compal-laptop.c
+++ b/drivers/platform/x86/compal-laptop.c
@@ -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");
 }
-- 
1.6.3.3