Mario Limonciello's compal-laptop changes were partly based on a reading
of dell-laptop. Unfortunately dell-laptop set a few bad examples; let's
fix them.
I don't have the hardware to test this, but the first four patches
should be nice and low risk.
dell-laptop: fix a use-after-free error on the failure path
dell-laptop: fix rfkill memory leak on unload and failure paths
dell-laptop: create a platform device as a parent for the rfkill devices
etc.
dell-laptop: add __init to init functions
The last patch adds polling for the hardware switch which blocks all
radios. This exercises the hardware a little more than before; it would
benefit from testing. It should be possible to see events generated by
the hardware switch using "udevadm monitor --kernel --environment".
dell-laptop: poll the rfkill hard-block
Matthew Garrett wrote:
> On Wed, Aug 19, 2009 at 03:06:51PM +0100, Alan Jenkins wrote:
>
>> This is controlled by a hardware switch, so we should poll it in order
>> to pick up changes. (There does not appear to be an interrupt or any
>> other notification mechanism).
>>
>
> With the exception of this one they all look fine - I'm moving house
> next week and won't have access to any Dell hardware for over a month,
> so if anyone else could confirm that these work that would be excellent.
>
>
Ok, thanks!
Here's something else I just noticed. I was able to test this one myself.
----------------------------------------------------------------------
>From 7efa12d3ea8bb70897358b4a87f8dedce9b78cca Mon Sep 17 00:00:00 2001
From: Alan Jenkins <[email protected]>
Date: Wed, 19 Aug 2009 15:32:17 +0100
Subject: [PATCH] dell-laptop: remove duplicate Kconfig entry under drivers/misc
This showed up as an unselectable option when using xconfig and
searching for "dell". It must been overlooked when dell-laptop
was moved to drivers/platform/x86.
Signed-off-by: Alan Jenkins <[email protected]>
---
drivers/misc/Kconfig | 13 -------------
1 files changed, 0 insertions(+), 13 deletions(-)
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 68ab39d..22414c5 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -210,19 +210,6 @@ config SGI_GRU_DEBUG
This option enables addition debugging code for the SGI GRU driver. If
you are unsure, say N.
-config DELL_LAPTOP
- tristate "Dell Laptop Extras (EXPERIMENTAL)"
- depends on X86
- depends on DCDBAS
- depends on EXPERIMENTAL
- depends on BACKLIGHT_CLASS_DEVICE
- depends on RFKILL
- depends on POWER_SUPPLY
- default n
- ---help---
- This driver adds support for rfkill and backlight control to Dell
- laptops.
-
config ISL29003
tristate "Intersil ISL29003 ambient light sensor"
depends on I2C && SYSFS
--
1.6.3.2
Signed-off-by: Alan Jenkins <[email protected]>
---
drivers/platform/x86/dell-laptop.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index a13a9f7..250c4b1 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -82,7 +82,7 @@ static const struct dmi_system_id __initdata dell_device_table[] = {
{ }
};
-static void parse_da_table(const struct dmi_header *dm)
+static void __init parse_da_table(const struct dmi_header *dm)
{
/* Final token is a terminator, so we don't want to copy it */
int tokens = (dm->length-11)/sizeof(struct calling_interface_token)-1;
@@ -111,7 +111,7 @@ static void parse_da_table(const struct dmi_header *dm)
da_num_tokens += tokens;
}
-static void find_tokens(const struct dmi_header *dm, void *dummy)
+static void __init find_tokens(const struct dmi_header *dm, void *dummy)
{
switch (dm->type) {
case 0xd4: /* Indexed IO */
@@ -214,7 +214,7 @@ static const struct rfkill_ops dell_rfkill_ops = {
.query = dell_rfkill_query,
};
-static int dell_setup_rfkill(void)
+static int __init dell_setup_rfkill(void)
{
struct calling_interface_buffer buffer;
int status;
--
1.6.3.2
dell_setup_rfkill() already cleans up the rfkill devices on failure.
So if it returns an error, we should not try to unregister the rfkill
devices.
Signed-off-by: Alan Jenkins <[email protected]>
---
drivers/platform/x86/dell-laptop.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 74909c4..12b6f33 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -330,7 +330,7 @@ static int __init dell_init(void)
if (ret) {
printk(KERN_WARNING "dell-laptop: Unable to setup rfkill\n");
- goto out;
+ goto fail_rfkill;
}
#ifdef CONFIG_ACPI
@@ -358,7 +358,7 @@ static int __init dell_init(void)
if (IS_ERR(dell_backlight_device)) {
ret = PTR_ERR(dell_backlight_device);
dell_backlight_device = NULL;
- goto out;
+ goto fail_backlight;
}
dell_backlight_device->props.max_brightness = max_intensity;
@@ -368,13 +368,15 @@ static int __init dell_init(void)
}
return 0;
-out:
+
+fail_backlight:
if (wifi_rfkill)
rfkill_unregister(wifi_rfkill);
if (bluetooth_rfkill)
rfkill_unregister(bluetooth_rfkill);
if (wwan_rfkill)
rfkill_unregister(wwan_rfkill);
+fail_rfkill:
kfree(da_tokens);
return ret;
}
--
1.6.3.2
On Wed, Aug 19, 2009 at 03:06:51PM +0100, Alan Jenkins wrote:
> This is controlled by a hardware switch, so we should poll it in order
> to pick up changes. (There does not appear to be an interrupt or any
> other notification mechanism).
With the exception of this one they all look fine - I'm moving house
next week and won't have access to any Dell hardware for over a month,
so if anyone else could confirm that these work that would be excellent.
--
Matthew Garrett | [email protected]
On Wed, Aug 19, 2009 at 03:03:15PM +0100, Alan Jenkins wrote:
> The last patch adds polling for the hardware switch which blocks all
> radios. This exercises the hardware a little more than before; it would
> benefit from testing. It should be possible to see events generated by
> the hardware switch using "udevadm monitor --kernel --environment".
We get a hardware event (via the keyboard controller...) when the switch
is set. I need to fix up my i8042 filtering patch to match feedback from
the maintainer, but there's no real need to poll.
--
Matthew Garrett | [email protected]
dell-laptop may not need to export any sysfs files, but it should still
create a platform device as a parent for the rfkill and backlight
devices. Otherwise sysfs will display these as "virtual" devices,
with no connection to either physical hardware or the dell-laptop
module.
Apparently this is useful for hardware detection.
Signed-off-by: Alan Jenkins <[email protected]>
---
drivers/platform/x86/dell-laptop.c | 38 ++++++++++++++++++++++++++++++++---
1 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 8fbff38..a13a9f7 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -58,6 +58,14 @@ static int da_command_code;
static int da_num_tokens;
static struct calling_interface_token *da_tokens;
+static struct platform_driver platform_driver = {
+ .driver = {
+ .name = "dell-laptop",
+ .owner = THIS_MODULE,
+ }
+};
+
+static struct platform_device *platform_device;
static struct backlight_device *dell_backlight_device;
static struct rfkill *wifi_rfkill;
static struct rfkill *bluetooth_rfkill;
@@ -217,7 +225,8 @@ static int dell_setup_rfkill(void)
status = buffer.output[1];
if ((status & (1<<2|1<<8)) == (1<<2|1<<8)) {
- wifi_rfkill = rfkill_alloc("dell-wifi", NULL, RFKILL_TYPE_WLAN,
+ wifi_rfkill = rfkill_alloc("dell-wifi", &platform_device->dev,
+ RFKILL_TYPE_WLAN,
&dell_rfkill_ops, (void *) 1);
if (!wifi_rfkill) {
ret = -ENOMEM;
@@ -229,7 +238,8 @@ static int dell_setup_rfkill(void)
}
if ((status & (1<<3|1<<9)) == (1<<3|1<<9)) {
- bluetooth_rfkill = rfkill_alloc("dell-bluetooth", NULL,
+ bluetooth_rfkill = rfkill_alloc("dell-bluetooth",
+ &platform_device->dev,
RFKILL_TYPE_BLUETOOTH,
&dell_rfkill_ops, (void *) 2);
if (!bluetooth_rfkill) {
@@ -242,7 +252,9 @@ static int dell_setup_rfkill(void)
}
if ((status & (1<<4|1<<10)) == (1<<4|1<<10)) {
- wwan_rfkill = rfkill_alloc("dell-wwan", NULL, RFKILL_TYPE_WWAN,
+ wwan_rfkill = rfkill_alloc("dell-wwan",
+ &platform_device->dev,
+ RFKILL_TYPE_WWAN,
&dell_rfkill_ops, (void *) 3);
if (!wwan_rfkill) {
ret = -ENOMEM;
@@ -342,6 +354,18 @@ static int __init dell_init(void)
return -ENODEV;
}
+ ret = platform_driver_register(&platform_driver);
+ if (ret)
+ goto fail_platform_driver;
+ platform_device = platform_device_alloc("dell-laptop", -1);
+ if (!platform_device) {
+ ret = -ENOMEM;
+ goto fail_platform_device1;
+ }
+ ret = platform_device_add(platform_device);
+ if (ret)
+ goto fail_platform_device2;
+
ret = dell_setup_rfkill();
if (ret) {
@@ -368,7 +392,7 @@ static int __init dell_init(void)
if (max_intensity) {
dell_backlight_device = backlight_device_register(
"dell_backlight",
- NULL, NULL,
+ &platform_device->dev, NULL,
&dell_ops);
if (IS_ERR(dell_backlight_device)) {
@@ -388,6 +412,12 @@ static int __init dell_init(void)
fail_backlight:
dell_cleanup_rfkill();
fail_rfkill:
+ platform_device_del(platform_device);
+fail_platform_device2:
+ platform_device_put(platform_device);
+fail_platform_device1:
+ platform_driver_unregister(&platform_driver);
+fail_platform_driver:
kfree(da_tokens);
return ret;
}
--
1.6.3.2
rfkill_unregister() should always be followed by rfkill_destroy().
Signed-off-by: Alan Jenkins <[email protected]>
---
drivers/platform/x86/dell-laptop.c | 30 ++++++++++++++++++------------
1 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 12b6f33..8fbff38 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -268,6 +268,22 @@ err_wifi:
return ret;
}
+static void dell_cleanup_rfkill(void)
+{
+ if (wifi_rfkill) {
+ rfkill_unregister(wifi_rfkill);
+ rfkill_destroy(wifi_rfkill);
+ }
+ if (bluetooth_rfkill) {
+ rfkill_unregister(bluetooth_rfkill);
+ rfkill_destroy(bluetooth_rfkill);
+ }
+ if (wwan_rfkill) {
+ rfkill_unregister(wwan_rfkill);
+ rfkill_destroy(wwan_rfkill);
+ }
+}
+
static int dell_send_intensity(struct backlight_device *bd)
{
struct calling_interface_buffer buffer;
@@ -370,12 +386,7 @@ static int __init dell_init(void)
return 0;
fail_backlight:
- if (wifi_rfkill)
- rfkill_unregister(wifi_rfkill);
- if (bluetooth_rfkill)
- rfkill_unregister(bluetooth_rfkill);
- if (wwan_rfkill)
- rfkill_unregister(wwan_rfkill);
+ dell_cleanup_rfkill();
fail_rfkill:
kfree(da_tokens);
return ret;
@@ -384,12 +395,7 @@ fail_rfkill:
static void __exit dell_exit(void)
{
backlight_device_unregister(dell_backlight_device);
- if (wifi_rfkill)
- rfkill_unregister(wifi_rfkill);
- if (bluetooth_rfkill)
- rfkill_unregister(bluetooth_rfkill);
- if (wwan_rfkill)
- rfkill_unregister(wwan_rfkill);
+ dell_cleanup_rfkill();
}
module_init(dell_init);
--
1.6.3.2
This is controlled by a hardware switch, so we should poll it in order
to pick up changes. (There does not appear to be an interrupt or any
other notification mechanism).
Signed-off-by: Alan Jenkins <[email protected]>
---
drivers/platform/x86/dell-laptop.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 250c4b1..349cf12 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -195,7 +195,7 @@ static int dell_rfkill_set(void *data, bool blocked)
return 0;
}
-static void dell_rfkill_query(struct rfkill *rfkill, void *data)
+static void dell_rfkill_poll(struct rfkill *rfkill, void *data)
{
struct calling_interface_buffer buffer;
int status;
@@ -211,7 +211,7 @@ static void dell_rfkill_query(struct rfkill *rfkill, void *data)
static const struct rfkill_ops dell_rfkill_ops = {
.set_block = dell_rfkill_set,
- .query = dell_rfkill_query,
+ .poll = dell_rfkill_poll,
};
static int __init dell_setup_rfkill(void)
--
1.6.3.2
applied
thanks,
Len Brown, Intel Open Source Technology Center
applied
thanks,
Len Brown, Intel Open Source Technology Center
applied
thanks,
Len Brown, Intel Open Source Technology Center
applied
thanks,
Len Brown, Intel Open Source Technology Center