2008-11-27 16:34:17

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 1/2] dcdbas: Export functionality for use in other drivers

dcdbas: Export functionality for use in other drivers

The dcdbas code allows calls to be made into the firmware on Dell
systems. Exporting this to other drivers allows them to implement
Dell-specific functionality in a safe way.

Signed-off-by: Matthew Garrett <[email protected]>

diff --git a/drivers/firmware/dcdbas.c b/drivers/firmware/dcdbas.c
index 50a071f..45f109d 100644
--- a/drivers/firmware/dcdbas.c
+++ b/drivers/firmware/dcdbas.c
@@ -238,11 +238,11 @@ static ssize_t host_control_on_shutdown_store(struct device *dev,
}

/**
- * smi_request: generate SMI request
+ * dcdbas_smi_request: generate SMI request
*
* Called with smi_data_lock.
*/
-static int smi_request(struct smi_cmd *smi_cmd)
+int dcdbas_smi_request(struct smi_cmd *smi_cmd)
{
cpumask_t old_mask;
int ret = 0;
@@ -278,6 +278,7 @@ out:
set_cpus_allowed_ptr(current, &old_mask);
return ret;
}
+EXPORT_SYMBOL(dcdbas_smi_request);

/**
* smi_request_store:
@@ -309,14 +310,14 @@ static ssize_t smi_request_store(struct device *dev,
switch (val) {
case 2:
/* Raw SMI */
- ret = smi_request(smi_cmd);
+ ret = dcdbas_smi_request(smi_cmd);
if (!ret)
ret = count;
break;
case 1:
/* Calling Interface SMI */
smi_cmd->ebx = (u32) virt_to_phys(smi_cmd->command_buffer);
- ret = smi_request(smi_cmd);
+ ret = dcdbas_smi_request(smi_cmd);
if (!ret)
ret = count;
break;
diff --git a/drivers/firmware/dcdbas.h b/drivers/firmware/dcdbas.h
index 87bc341..ca3cb0a 100644
--- a/drivers/firmware/dcdbas.h
+++ b/drivers/firmware/dcdbas.h
@@ -101,5 +101,7 @@ struct apm_cmd {
} __attribute__ ((packed)) parameters;
} __attribute__ ((packed));

+int dcdbas_smi_request(struct smi_cmd *smi_cmd);
+
#endif /* _DCDBAS_H_ */

--
Matthew Garrett | [email protected]


2008-11-27 16:34:59

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 2/2] misc: Add dell-laptop driver

Add a driver for controling Dell-specific backlight and rfkill interfaces.
This driver makes use of the dcdbas interface to the Dell firmware to allow
the backlight and rfkill interfaces on Dell systems to be driven through the
standardised sysfs interfaces.

Signed-off-by: Matthew Garrett <[email protected]>

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index fee7304..3949a1c 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -498,6 +498,18 @@ 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
+ default n
+ ---help---
+ This driver adds support for rfkill and backlight control to Dell
+ laptops.
+
source "drivers/misc/c2port/Kconfig"

endif # MISC_DEVICES
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 817f7f5..39043f8 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -32,4 +32,5 @@ obj-$(CONFIG_KGDB_TESTS) += kgdbts.o
obj-$(CONFIG_SGI_XP) += sgi-xp/
obj-$(CONFIG_SGI_GRU) += sgi-gru/
obj-$(CONFIG_HP_ILO) += hpilo.o
+obj-$(CONFIG_DELL_LAPTOP) += dell-laptop.o
obj-$(CONFIG_C2PORT) += c2port/
diff --git a/drivers/misc/dell-laptop.c b/drivers/misc/dell-laptop.c
new file mode 100644
index 0000000..49d202f
--- /dev/null
+++ b/drivers/misc/dell-laptop.c
@@ -0,0 +1,430 @@
+/*
+ * Driver for Dell laptop extras
+ *
+ * Copyright (c) Red Hat <[email protected]>
+ *
+ * Based on documentation in the libsmbios package, Copyright (C) 2005 Dell
+ * Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/backlight.h>
+#include <linux/err.h>
+#include <linux/dmi.h>
+#include <linux/io.h>
+#include <linux/rfkill.h>
+#include <linux/power_supply.h>
+#include <linux/acpi.h>
+#include "../firmware/dcdbas.h"
+
+#define BRIGHTNESS_TOKEN 0x7d
+
+/* This structure will be modified by the firmware when we enter
+ * system management mode, hence the volatiles */
+
+struct calling_interface_buffer {
+ u16 class;
+ u16 select;
+ volatile u32 input[4];
+ volatile u32 output[4];
+} __attribute__ ((packed));
+
+struct calling_interface_token {
+ u16 tokenID;
+ u16 location;
+ union {
+ u16 value;
+ u16 stringlength;
+ };
+};
+
+struct calling_interface_structure {
+ struct dmi_header header;
+ u16 cmdIOAddress;
+ u8 cmdIOCode;
+ u32 supportedCmds;
+ struct calling_interface_token tokens[];
+} __attribute__ ((packed));
+
+static int da_command_address;
+static int da_command_code;
+static int da_num_tokens;
+static struct calling_interface_token *da_tokens;
+
+static struct backlight_device *dell_backlight_device;
+static struct rfkill *wifi_rfkill;
+static struct rfkill *bluetooth_rfkill;
+static struct rfkill *wwan_rfkill;
+
+static struct dmi_system_id __initdata dell_device_table[] = {
+ {
+ .ident = "Dell laptop",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_CHASSIS_TYPE, "8"),
+ },
+ },
+ { }
+};
+
+static void 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;
+ struct calling_interface_structure *table =
+ container_of(dm, struct calling_interface_structure, header);
+
+ /* 4 bytes of table header, plus 7 bytes of Dell header, plus at least
+ 6 bytes of entry */
+
+ if (dm->length < 17)
+ return;
+
+ da_command_address = table->cmdIOAddress;
+ da_command_code = table->cmdIOCode;
+
+ da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
+ sizeof(struct calling_interface_token),
+ GFP_KERNEL);
+
+ if (!da_tokens)
+ return;
+
+ memcpy(da_tokens+da_num_tokens, table->tokens,
+ sizeof(struct calling_interface_token) * tokens);
+
+ da_num_tokens += tokens;
+}
+
+static void find_tokens(const struct dmi_header *dm)
+{
+ switch (dm->type) {
+ case 0xd4: /* Indexed IO */
+ break;
+ case 0xd5: /* Protected Area Type 1 */
+ break;
+ case 0xd6: /* Protected Area Type 2 */
+ break;
+ case 0xda: /* Calling interface */
+ parse_da_table(dm);
+ break;
+ }
+}
+
+static int find_token_location(int tokenid)
+{
+ int i;
+ for (i = 0; i < da_num_tokens; i++) {
+ if (da_tokens[i].tokenID == tokenid)
+ return da_tokens[i].location;
+ }
+
+ return -1;
+}
+
+static struct calling_interface_buffer *
+dell_send_request(struct calling_interface_buffer *buffer, int class,
+ int select)
+{
+ struct smi_cmd command;
+
+ command.magic = SMI_CMD_MAGIC;
+ command.command_address = da_command_address;
+ command.command_code = da_command_code;
+ command.ebx = virt_to_phys(buffer);
+ command.ecx = 0x42534931;
+
+ buffer->class = class;
+ buffer->select = select;
+
+ dcdbas_smi_request(&command);
+
+ return buffer;
+}
+
+/* Derived from information in DellWirelessCtl.cpp:
+ Class 17, select 11 is radio control. It returns an array of 32-bit values.
+
+ 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
+*/
+
+static int dell_rfkill_set(int radio, enum rfkill_state state)
+{
+ struct calling_interface_buffer buffer;
+ int disable = (state == RFKILL_STATE_UNBLOCKED) ? 0 : 1;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ buffer.input[0] = (1 | (radio<<8) | (disable << 16));
+ dell_send_request(&buffer, 17, 11);
+
+ return 0;
+}
+
+static int dell_wifi_set(void *data, enum rfkill_state state)
+{
+ return dell_rfkill_set(1, state);
+}
+
+static int dell_bluetooth_set(void *data, enum rfkill_state state)
+{
+ return dell_rfkill_set(2, state);
+}
+
+static int dell_wwan_set(void *data, enum rfkill_state state)
+{
+ return dell_rfkill_set(3, state);
+}
+
+static int dell_rfkill_get(int bit, enum rfkill_state *state)
+{
+ struct calling_interface_buffer buffer;
+ int status;
+ int new_state = RFKILL_STATE_HARD_BLOCKED;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ dell_send_request(&buffer, 17, 11);
+ status = buffer.output[1];
+
+ if (status & (1<<16))
+ new_state = RFKILL_STATE_SOFT_BLOCKED;
+
+ if (status & (1<<bit))
+ *state = new_state;
+ else
+ *state = RFKILL_STATE_UNBLOCKED;
+
+ return 0;
+}
+
+static int dell_wifi_get(void *data, enum rfkill_state *state)
+{
+ return dell_rfkill_get(17, state);
+}
+
+static int dell_bluetooth_get(void *data, enum rfkill_state *state)
+{
+ return dell_rfkill_get(18, state);
+}
+
+static int dell_wwan_get(void *data, enum rfkill_state *state)
+{
+ return dell_rfkill_get(19, state);
+}
+
+static int dell_setup_rfkill(void)
+{
+ struct calling_interface_buffer buffer;
+ int status;
+ int ret;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ dell_send_request(&buffer, 17, 11);
+ status = buffer.output[1];
+
+ if ((status & (1<<2|1<<8)) == (1<<2|1<<8)) {
+ wifi_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_WLAN);
+ if (!wifi_rfkill)
+ goto err_wifi;
+ wifi_rfkill->name = "dell-wifi";
+ wifi_rfkill->toggle_radio = dell_wifi_set;
+ wifi_rfkill->get_state = dell_wifi_get;
+ ret = rfkill_register(wifi_rfkill);
+ if (ret)
+ goto err_wifi;
+ }
+
+ if ((status & (1<<3|1<<9)) == (1<<3|1<<9)) {
+ bluetooth_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_BLUETOOTH);
+ if (!bluetooth_rfkill)
+ goto err_bluetooth;
+ bluetooth_rfkill->name = "dell-bluetooth";
+ bluetooth_rfkill->toggle_radio = dell_bluetooth_set;
+ bluetooth_rfkill->get_state = dell_bluetooth_get;
+ ret = rfkill_register(bluetooth_rfkill);
+ if (ret)
+ goto err_bluetooth;
+ }
+
+ if ((status & (1<<4|1<<10)) == (1<<4|1<<10)) {
+ wwan_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_WWAN);
+ if (!wwan_rfkill)
+ goto err_wwan;
+ wwan_rfkill->name = "dell-wwan";
+ wwan_rfkill->toggle_radio = dell_wwan_set;
+ wwan_rfkill->get_state = dell_wwan_get;
+ ret = rfkill_register(wwan_rfkill);
+ if (ret)
+ goto err_wwan;
+ }
+
+ return 0;
+err_wwan:
+ if (wwan_rfkill)
+ rfkill_free(wwan_rfkill);
+ if (bluetooth_rfkill) {
+ rfkill_unregister(bluetooth_rfkill);
+ bluetooth_rfkill = NULL;
+ }
+err_bluetooth:
+ if (bluetooth_rfkill)
+ rfkill_free(bluetooth_rfkill);
+ if (wifi_rfkill) {
+ rfkill_unregister(wifi_rfkill);
+ wifi_rfkill = NULL;
+ }
+err_wifi:
+ if (wifi_rfkill)
+ rfkill_free(wifi_rfkill);
+
+ return ret;
+}
+
+static int dell_send_intensity(struct backlight_device *bd)
+{
+ struct calling_interface_buffer buffer;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
+ buffer.input[1] = bd->props.brightness;
+
+ if (buffer.input[0] == -1)
+ return -ENODEV;
+
+ if (power_supply_is_system_supplied() > 0)
+ dell_send_request(&buffer, 1, 2);
+ else
+ dell_send_request(&buffer, 1, 1);
+
+ return 0;
+}
+
+static int dell_get_intensity(struct backlight_device *bd)
+{
+ struct calling_interface_buffer buffer;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
+
+ if (buffer.input[0] == -1)
+ return -ENODEV;
+
+ if (power_supply_is_system_supplied() > 0)
+ dell_send_request(&buffer, 0, 2);
+ else
+ dell_send_request(&buffer, 0, 1);
+
+ return buffer.output[1];
+}
+
+static struct backlight_ops dell_ops = {
+ .get_brightness = dell_get_intensity,
+ .update_status = dell_send_intensity,
+};
+
+static int __init dell_init(void)
+{
+ struct calling_interface_buffer buffer;
+ int max_intensity = 0;
+ int ret;
+
+ if (!dmi_check_system(dell_device_table))
+ return -ENODEV;
+
+ dmi_walk(find_tokens);
+
+ if (!da_tokens)
+ return -ENODEV;
+
+ ret = dell_setup_rfkill();
+
+ if (ret) {
+ printk(KERN_WARNING "dell-laptop: Unable to setup rfkill\n");
+ return ret;
+ }
+
+#ifdef CONFIG_ACPI
+ if (acpi_video_backlight_support())
+ return 0;
+#endif
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ 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 (max_intensity) {
+ dell_backlight_device = backlight_device_register(
+ "dell_backlight",
+ NULL, NULL,
+ &dell_ops);
+
+ if (IS_ERR(dell_backlight_device)) {
+ dell_backlight_device = NULL;
+ ret = PTR_ERR(dell_backlight_device);
+ goto out;
+ }
+
+ dell_backlight_device->props.max_brightness = max_intensity;
+ dell_backlight_device->props.brightness =
+ dell_get_intensity(dell_backlight_device);
+ backlight_update_status(dell_backlight_device);
+ }
+
+ return 0;
+out:
+ if (wifi_rfkill)
+ rfkill_unregister(wifi_rfkill);
+ if (bluetooth_rfkill)
+ rfkill_unregister(bluetooth_rfkill);
+ if (wwan_rfkill)
+ rfkill_unregister(wwan_rfkill);
+ return ret;
+}
+
+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);
+}
+
+module_init(dell_init);
+module_exit(dell_exit);
+
+MODULE_AUTHOR("Matthew Garrett <[email protected]>");
+MODULE_DESCRIPTION("Dell laptop driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("dmi:*svnDellInc.:*:ct8:*");

--
Matthew Garrett | [email protected]

2008-12-02 03:56:20

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 1/2] dcdbas: Export functionality for use in other drivers

On Thu, Nov 27, 2008 at 04:33:57PM +0000, Matthew Garrett wrote:
> dcdbas: Export functionality for use in other drivers
>
> The dcdbas code allows calls to be made into the firmware on Dell
> systems. Exporting this to other drivers allows them to implement
> Dell-specific functionality in a safe way.
>
> Signed-off-by: Matthew Garrett <[email protected]>

If you change these to EXPORT_SYMBOL_GPL() then you can add my:
Acked-by: Greg Kroah-Hartman <[email protected]>
to it.

Do you want this to go through my driver-core tree, or somewhere else?

thanks,

greg k-h

2008-12-02 04:03:33

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 1/2] dcdbas: Export functionality for use in other drivers

dcdbas: Export functionality for use in other drivers

The dcdbas code allows calls to be made into the firmware on Dell systems.
Exporting this to other drivers allows them to implement Dell-specific
functionality in a safe way.

Signed-off-by: Matthew Garrett <[email protected]>

---

Only change is the s/EXPORT_SYMBOL/EXPORT_SYMBOL_GPL/. The firmware
subdir doesn't seem to have a separate maintainer, so I guess
driver-core is fine?

diff --git a/drivers/firmware/dcdbas.c b/drivers/firmware/dcdbas.c
index 50a071f..45f109d 100644
--- a/drivers/firmware/dcdbas.c
+++ b/drivers/firmware/dcdbas.c
@@ -238,11 +238,11 @@ static ssize_t host_control_on_shutdown_store(struct device *dev,
}

/**
- * smi_request: generate SMI request
+ * dcdbas_smi_request: generate SMI request
*
* Called with smi_data_lock.
*/
-static int smi_request(struct smi_cmd *smi_cmd)
+int dcdbas_smi_request(struct smi_cmd *smi_cmd)
{
cpumask_t old_mask;
int ret = 0;
@@ -278,6 +278,7 @@ out:
set_cpus_allowed_ptr(current, &old_mask);
return ret;
}
+EXPORT_SYMBOL_GPL(dcdbas_smi_request);

/**
* smi_request_store:
@@ -309,14 +310,14 @@ static ssize_t smi_request_store(struct device *dev,
switch (val) {
case 2:
/* Raw SMI */
- ret = smi_request(smi_cmd);
+ ret = dcdbas_smi_request(smi_cmd);
if (!ret)
ret = count;
break;
case 1:
/* Calling Interface SMI */
smi_cmd->ebx = (u32) virt_to_phys(smi_cmd->command_buffer);
- ret = smi_request(smi_cmd);
+ ret = dcdbas_smi_request(smi_cmd);
if (!ret)
ret = count;
break;
diff --git a/drivers/firmware/dcdbas.h b/drivers/firmware/dcdbas.h
index 87bc341..ca3cb0a 100644
--- a/drivers/firmware/dcdbas.h
+++ b/drivers/firmware/dcdbas.h
@@ -101,5 +101,7 @@ struct apm_cmd {
} __attribute__ ((packed)) parameters;
} __attribute__ ((packed));

+int dcdbas_smi_request(struct smi_cmd *smi_cmd);
+
#endif /* _DCDBAS_H_ */

--
Matthew Garrett | [email protected]

2008-12-02 17:02:47

by Matt Domsch

[permalink] [raw]
Subject: Re: [PATCH 2/2] misc: Add dell-laptop driver

On Thu, Nov 27, 2008 at 04:34:44PM +0000, Matthew Garrett wrote:
> Add a driver for controling Dell-specific backlight and rfkill interfaces.
> This driver makes use of the dcdbas interface to the Dell firmware to allow
> the backlight and rfkill interfaces on Dell systems to be driven through the
> standardised sysfs interfaces.

ack on the rfkill aspect.

For the backlight, right now, userspace HAL calls out to libsmbios to
modify the backlight settings. Will this conflict? Or is it HAL that
would decide to use either the standard sysfs interface if present,
falling back to libsmbios if not?

Thanks,
Matt

--
Matt Domsch
Linux Technology Strategist, Dell Office of the CTO
linux.dell.com & http://www.dell.com/linux

2008-12-02 17:03:34

by Matt Domsch

[permalink] [raw]
Subject: Re: [PATCH 1/2] dcdbas: Export functionality for use in other drivers

On Tue, Dec 02, 2008 at 04:03:19AM +0000, Matthew Garrett wrote:
> dcdbas: Export functionality for use in other drivers
>
> The dcdbas code allows calls to be made into the firmware on Dell systems.
> Exporting this to other drivers allows them to implement Dell-specific
> functionality in a safe way.
>
> Signed-off-by: Matthew Garrett <[email protected]>

Looks good, thanks for doing this.
-Matt
--
Matt Domsch
Linux Technology Strategist, Dell Office of the CTO
linux.dell.com & http://www.dell.com/linux

2008-12-02 17:04:05

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2/2] misc: Add dell-laptop driver

On Tue, Dec 02, 2008 at 11:02:28AM -0600, Matt Domsch wrote:

> For the backlight, right now, userspace HAL calls out to libsmbios to
> modify the backlight settings. Will this conflict? Or is it HAL that
> would decide to use either the standard sysfs interface if present,
> falling back to libsmbios if not?

hal will currently provide both interfaces and it'll then be up to
userspace to decide which to use. In future, the libsmbios code can be
deprecated.

--
Matthew Garrett | [email protected]

2008-12-02 19:51:55

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 2/2] misc: Add dell-laptop driver

On Thu, 27 Nov 2008 16:34:44 +0000
Matthew Garrett <[email protected]> wrote:

> Add a driver for controling Dell-specific backlight and rfkill interfaces.
> This driver makes use of the dcdbas interface to the Dell firmware to allow
> the backlight and rfkill interfaces on Dell systems to be driven through the
> standardised sysfs interfaces.
>
>
> ...
>
> +struct calling_interface_buffer {
> + u16 class;
> + u16 select;
> + volatile u32 input[4];
> + volatile u32 output[4];
> +} __attribute__ ((packed));

We have that little __packed helper for this.

I don't think it adds a ton of value personally, but the __attribute__
syntax is a bit of a mouthful, and consistency is nice. I've suggested
that Andy add a checkpatch rule for open-coded use of the various
helpers which are defined in include/linux/compiler-gcc.h.

> +struct calling_interface_token {
> + u16 tokenID;
> + u16 location;
> + union {
> + u16 value;
> + u16 stringlength;
> + };
> +};
> +
> +struct calling_interface_structure {
> + struct dmi_header header;
> + u16 cmdIOAddress;
> + u8 cmdIOCode;
> + u32 supportedCmds;
> + struct calling_interface_token tokens[];
> +} __attribute__ ((packed));

ditto.

> +static int da_command_address;
> +static int da_command_code;
> +static int da_num_tokens;
> +static struct calling_interface_token *da_tokens;
> +
> +static struct backlight_device *dell_backlight_device;
> +static struct rfkill *wifi_rfkill;
> +static struct rfkill *bluetooth_rfkill;
> +static struct rfkill *wwan_rfkill;
> +
> +static struct dmi_system_id __initdata dell_device_table[] = {

this can be made const.

> + {
> + .ident = "Dell laptop",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_CHASSIS_TYPE, "8"),
> + },
> + },
> + { }
> +};
> +
> +static void 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;

lol.

> + struct calling_interface_structure *table =
> + container_of(dm, struct calling_interface_structure, header);
> +
> + /* 4 bytes of table header, plus 7 bytes of Dell header, plus at least
> + 6 bytes of entry */
> +
> + if (dm->length < 17)
> + return;
> +
> + da_command_address = table->cmdIOAddress;
> + da_command_code = table->cmdIOCode;
> +
> + da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
> + sizeof(struct calling_interface_token),
> + GFP_KERNEL);
> +
> + if (!da_tokens)
> + return;
> +
> + memcpy(da_tokens+da_num_tokens, table->tokens,
> + sizeof(struct calling_interface_token) * tokens);
> +
> + da_num_tokens += tokens;
> +}

OK. No locking is needed for updates to the global state?

>
> ...
>
> +static int __init dell_init(void)
> +{
> + struct calling_interface_buffer buffer;
> + int max_intensity = 0;
> + int ret;
> +
> + if (!dmi_check_system(dell_device_table))
> + return -ENODEV;
> +
> + dmi_walk(find_tokens);
> +
> + if (!da_tokens)
> + return -ENODEV;
> +
> + ret = dell_setup_rfkill();
> +
> + if (ret) {
> + printk(KERN_WARNING "dell-laptop: Unable to setup rfkill\n");
> + return ret;
> + }
> +
> +#ifdef CONFIG_ACPI
> + if (acpi_video_backlight_support())
> + return 0;
> +#endif

Do we need the ifdefs here? It looks like include/linux/acpi.h tries
to provide a suitable 0-returning stub?

If the ifdefs are needed, then perhaps include/linux/acpi.h needs fixing?

> + memset(&buffer, 0, sizeof(struct calling_interface_buffer));
> + 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 (max_intensity) {
> + dell_backlight_device = backlight_device_register(
> + "dell_backlight",
> + NULL, NULL,
> + &dell_ops);
> +
> + if (IS_ERR(dell_backlight_device)) {
> + dell_backlight_device = NULL;
> + ret = PTR_ERR(dell_backlight_device);

The above two statements are in the wrong order.

> + goto out;
> + }
> +
> + dell_backlight_device->props.max_brightness = max_intensity;
> + dell_backlight_device->props.brightness =
> + dell_get_intensity(dell_backlight_device);
> + backlight_update_status(dell_backlight_device);
> + }
> +
> + return 0;
> +out:
> + if (wifi_rfkill)
> + rfkill_unregister(wifi_rfkill);
> + if (bluetooth_rfkill)
> + rfkill_unregister(bluetooth_rfkill);
> + if (wwan_rfkill)
> + rfkill_unregister(wwan_rfkill);
> + return ret;
> +}
> +

2008-12-02 20:06:25

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2/2] misc: Add dell-laptop driver

On Tue, Dec 02, 2008 at 11:50:29AM -0800, Andrew Morton wrote:
> > +struct calling_interface_buffer {
> > + u16 class;
> > + u16 select;
> > + volatile u32 input[4];
> > + volatile u32 output[4];
> > +} __attribute__ ((packed));
>
> We have that little __packed helper for this.

Ok. I've switched to that, although the __attribute__ notation seems to
be the dominant form in the kernel right now.

> > +static struct dmi_system_id __initdata dell_device_table[] = {
>
> this can be made const.

Done.

> > + da_num_tokens += tokens;
> > +}
>
> OK. No locking is needed for updates to the global state?

The parse function is called once during module init, before the driver
is registered. On the other hand, that makes me realise that we leak it
if something else fails, so I've fixed that up as well.

> > +#ifdef CONFIG_ACPI
> > + if (acpi_video_backlight_support())
> > + return 0;
> > +#endif
>
> Do we need the ifdefs here? It looks like include/linux/acpi.h tries
> to provide a suitable 0-returning stub?

They're protected by #ifdef CONFIG_ACPI_VIDEO, but it looks like acpi.h
is empty if CONFIG_ACPI isn't set?

> > + dell_backlight_device = NULL;
> > + ret = PTR_ERR(dell_backlight_device);
>
> The above two statements are in the wrong order.

Good catch, thanks.

--
Matthew Garrett | [email protected]

2008-12-02 20:17:01

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 2/2 update] misc: Add dell-laptop driver

misc: Add dell-laptop driver

Add a driver for controling Dell-specific backlight and rfkill interfaces.
This driver makes use of the dcdbas interface to the Dell firmware to allow
the backlight and rfkill interfaces on Dell systems to be driven through the
standardised sysfs interfaces.

Signed-off-by: Matthew Garrett <[email protected]>

---

Contains Andrew's suggested updates, plus a fix for a leak on the init
error path.

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index fee7304..3949a1c 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -498,6 +498,18 @@ 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
+ default n
+ ---help---
+ This driver adds support for rfkill and backlight control to Dell
+ laptops.
+
source "drivers/misc/c2port/Kconfig"

endif # MISC_DEVICES
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 817f7f5..39043f8 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -32,4 +32,5 @@ obj-$(CONFIG_KGDB_TESTS) += kgdbts.o
obj-$(CONFIG_SGI_XP) += sgi-xp/
obj-$(CONFIG_SGI_GRU) += sgi-gru/
obj-$(CONFIG_HP_ILO) += hpilo.o
+obj-$(CONFIG_DELL_LAPTOP) += dell-laptop.o
obj-$(CONFIG_C2PORT) += c2port/
diff --git a/drivers/misc/dell-laptop.c b/drivers/misc/dell-laptop.c
new file mode 100644
index 0000000..4d33a20
--- /dev/null
+++ b/drivers/misc/dell-laptop.c
@@ -0,0 +1,436 @@
+/*
+ * Driver for Dell laptop extras
+ *
+ * Copyright (c) Red Hat <[email protected]>
+ *
+ * Based on documentation in the libsmbios package, Copyright (C) 2005 Dell
+ * Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/backlight.h>
+#include <linux/err.h>
+#include <linux/dmi.h>
+#include <linux/io.h>
+#include <linux/rfkill.h>
+#include <linux/power_supply.h>
+#include <linux/acpi.h>
+#include "../firmware/dcdbas.h"
+
+#define BRIGHTNESS_TOKEN 0x7d
+
+/* This structure will be modified by the firmware when we enter
+ * system management mode, hence the volatiles */
+
+struct calling_interface_buffer {
+ u16 class;
+ u16 select;
+ volatile u32 input[4];
+ volatile u32 output[4];
+} __packed;
+
+struct calling_interface_token {
+ u16 tokenID;
+ u16 location;
+ union {
+ u16 value;
+ u16 stringlength;
+ };
+};
+
+struct calling_interface_structure {
+ struct dmi_header header;
+ u16 cmdIOAddress;
+ u8 cmdIOCode;
+ u32 supportedCmds;
+ struct calling_interface_token tokens[];
+} __packed;
+
+static int da_command_address;
+static int da_command_code;
+static int da_num_tokens;
+static struct calling_interface_token *da_tokens;
+
+static struct backlight_device *dell_backlight_device;
+static struct rfkill *wifi_rfkill;
+static struct rfkill *bluetooth_rfkill;
+static struct rfkill *wwan_rfkill;
+
+static const struct dmi_system_id __initdata dell_device_table[] = {
+ {
+ .ident = "Dell laptop",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_CHASSIS_TYPE, "8"),
+ },
+ },
+ { }
+};
+
+static void 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;
+ struct calling_interface_structure *table =
+ container_of(dm, struct calling_interface_structure, header);
+
+ /* 4 bytes of table header, plus 7 bytes of Dell header, plus at least
+ 6 bytes of entry */
+
+ if (dm->length < 17)
+ return;
+
+ da_command_address = table->cmdIOAddress;
+ da_command_code = table->cmdIOCode;
+
+ da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
+ sizeof(struct calling_interface_token),
+ GFP_KERNEL);
+
+ if (!da_tokens)
+ return;
+
+ memcpy(da_tokens+da_num_tokens, table->tokens,
+ sizeof(struct calling_interface_token) * tokens);
+
+ da_num_tokens += tokens;
+}
+
+static void find_tokens(const struct dmi_header *dm)
+{
+ switch (dm->type) {
+ case 0xd4: /* Indexed IO */
+ break;
+ case 0xd5: /* Protected Area Type 1 */
+ break;
+ case 0xd6: /* Protected Area Type 2 */
+ break;
+ case 0xda: /* Calling interface */
+ parse_da_table(dm);
+ break;
+ }
+}
+
+static int find_token_location(int tokenid)
+{
+ int i;
+ for (i = 0; i < da_num_tokens; i++) {
+ if (da_tokens[i].tokenID == tokenid)
+ return da_tokens[i].location;
+ }
+
+ return -1;
+}
+
+static struct calling_interface_buffer *
+dell_send_request(struct calling_interface_buffer *buffer, int class,
+ int select)
+{
+ struct smi_cmd command;
+
+ command.magic = SMI_CMD_MAGIC;
+ command.command_address = da_command_address;
+ command.command_code = da_command_code;
+ command.ebx = virt_to_phys(buffer);
+ command.ecx = 0x42534931;
+
+ buffer->class = class;
+ buffer->select = select;
+
+ dcdbas_smi_request(&command);
+
+ return buffer;
+}
+
+/* Derived from information in DellWirelessCtl.cpp:
+ Class 17, select 11 is radio control. It returns an array of 32-bit values.
+
+ 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
+*/
+
+static int dell_rfkill_set(int radio, enum rfkill_state state)
+{
+ struct calling_interface_buffer buffer;
+ int disable = (state == RFKILL_STATE_UNBLOCKED) ? 0 : 1;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ buffer.input[0] = (1 | (radio<<8) | (disable << 16));
+ dell_send_request(&buffer, 17, 11);
+
+ return 0;
+}
+
+static int dell_wifi_set(void *data, enum rfkill_state state)
+{
+ return dell_rfkill_set(1, state);
+}
+
+static int dell_bluetooth_set(void *data, enum rfkill_state state)
+{
+ return dell_rfkill_set(2, state);
+}
+
+static int dell_wwan_set(void *data, enum rfkill_state state)
+{
+ return dell_rfkill_set(3, state);
+}
+
+static int dell_rfkill_get(int bit, enum rfkill_state *state)
+{
+ struct calling_interface_buffer buffer;
+ int status;
+ int new_state = RFKILL_STATE_HARD_BLOCKED;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ dell_send_request(&buffer, 17, 11);
+ status = buffer.output[1];
+
+ if (status & (1<<16))
+ new_state = RFKILL_STATE_SOFT_BLOCKED;
+
+ if (status & (1<<bit))
+ *state = new_state;
+ else
+ *state = RFKILL_STATE_UNBLOCKED;
+
+ return 0;
+}
+
+static int dell_wifi_get(void *data, enum rfkill_state *state)
+{
+ return dell_rfkill_get(17, state);
+}
+
+static int dell_bluetooth_get(void *data, enum rfkill_state *state)
+{
+ return dell_rfkill_get(18, state);
+}
+
+static int dell_wwan_get(void *data, enum rfkill_state *state)
+{
+ return dell_rfkill_get(19, state);
+}
+
+static int dell_setup_rfkill(void)
+{
+ struct calling_interface_buffer buffer;
+ int status;
+ int ret;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ dell_send_request(&buffer, 17, 11);
+ status = buffer.output[1];
+
+ if ((status & (1<<2|1<<8)) == (1<<2|1<<8)) {
+ wifi_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_WLAN);
+ if (!wifi_rfkill)
+ goto err_wifi;
+ wifi_rfkill->name = "dell-wifi";
+ wifi_rfkill->toggle_radio = dell_wifi_set;
+ wifi_rfkill->get_state = dell_wifi_get;
+ ret = rfkill_register(wifi_rfkill);
+ if (ret)
+ goto err_wifi;
+ }
+
+ if ((status & (1<<3|1<<9)) == (1<<3|1<<9)) {
+ bluetooth_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_BLUETOOTH);
+ if (!bluetooth_rfkill)
+ goto err_bluetooth;
+ bluetooth_rfkill->name = "dell-bluetooth";
+ bluetooth_rfkill->toggle_radio = dell_bluetooth_set;
+ bluetooth_rfkill->get_state = dell_bluetooth_get;
+ ret = rfkill_register(bluetooth_rfkill);
+ if (ret)
+ goto err_bluetooth;
+ }
+
+ if ((status & (1<<4|1<<10)) == (1<<4|1<<10)) {
+ wwan_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_WWAN);
+ if (!wwan_rfkill)
+ goto err_wwan;
+ wwan_rfkill->name = "dell-wwan";
+ wwan_rfkill->toggle_radio = dell_wwan_set;
+ wwan_rfkill->get_state = dell_wwan_get;
+ ret = rfkill_register(wwan_rfkill);
+ if (ret)
+ goto err_wwan;
+ }
+
+ return 0;
+err_wwan:
+ if (wwan_rfkill)
+ rfkill_free(wwan_rfkill);
+ if (bluetooth_rfkill) {
+ rfkill_unregister(bluetooth_rfkill);
+ bluetooth_rfkill = NULL;
+ }
+err_bluetooth:
+ if (bluetooth_rfkill)
+ rfkill_free(bluetooth_rfkill);
+ if (wifi_rfkill) {
+ rfkill_unregister(wifi_rfkill);
+ wifi_rfkill = NULL;
+ }
+err_wifi:
+ if (wifi_rfkill)
+ rfkill_free(wifi_rfkill);
+
+ return ret;
+}
+
+static int dell_send_intensity(struct backlight_device *bd)
+{
+ struct calling_interface_buffer buffer;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
+ buffer.input[1] = bd->props.brightness;
+
+ if (buffer.input[0] == -1)
+ return -ENODEV;
+
+ if (power_supply_is_system_supplied() > 0)
+ dell_send_request(&buffer, 1, 2);
+ else
+ dell_send_request(&buffer, 1, 1);
+
+ return 0;
+}
+
+static int dell_get_intensity(struct backlight_device *bd)
+{
+ struct calling_interface_buffer buffer;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
+
+ if (buffer.input[0] == -1)
+ return -ENODEV;
+
+ if (power_supply_is_system_supplied() > 0)
+ dell_send_request(&buffer, 0, 2);
+ else
+ dell_send_request(&buffer, 0, 1);
+
+ return buffer.output[1];
+}
+
+static struct backlight_ops dell_ops = {
+ .get_brightness = dell_get_intensity,
+ .update_status = dell_send_intensity,
+};
+
+static int __init dell_init(void)
+{
+ struct calling_interface_buffer buffer;
+ int max_intensity = 0;
+ int ret;
+
+ if (!dmi_check_system(dell_device_table))
+ return -ENODEV;
+
+ dmi_walk(find_tokens);
+
+ if (!da_tokens) {
+ printk(KERN_INFO "dell-laptop: Unable to find dmi tokens\n");
+ return -ENODEV;
+ }
+
+ ret = dell_setup_rfkill();
+
+ if (ret) {
+ printk(KERN_WARNING "dell-laptop: Unable to setup rfkill\n");
+ goto out;
+ }
+
+#ifdef CONFIG_ACPI
+ /* In the event of an ACPI backlight being available, don't
+ * register the platform controller.
+ */
+ if (acpi_video_backlight_support())
+ return 0;
+#endif
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ 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 (max_intensity) {
+ dell_backlight_device = backlight_device_register(
+ "dell_backlight",
+ NULL, NULL,
+ &dell_ops);
+
+ if (IS_ERR(dell_backlight_device)) {
+ ret = PTR_ERR(dell_backlight_device);
+ dell_backlight_device = NULL;
+ goto out;
+ }
+
+ dell_backlight_device->props.max_brightness = max_intensity;
+ dell_backlight_device->props.brightness =
+ dell_get_intensity(dell_backlight_device);
+ backlight_update_status(dell_backlight_device);
+ }
+
+ return 0;
+out:
+ if (wifi_rfkill)
+ rfkill_unregister(wifi_rfkill);
+ if (bluetooth_rfkill)
+ rfkill_unregister(bluetooth_rfkill);
+ if (wwan_rfkill)
+ rfkill_unregister(wwan_rfkill);
+ kfree(da_tokens);
+ return ret;
+}
+
+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);
+}
+
+module_init(dell_init);
+module_exit(dell_exit);
+
+MODULE_AUTHOR("Matthew Garrett <[email protected]>");
+MODULE_DESCRIPTION("Dell laptop driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("dmi:*svnDellInc.:*:ct8:*");

--
Matthew Garrett | [email protected]

2008-12-02 20:23:30

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 2/2] misc: Add dell-laptop driver

On Tue, 2 Dec 2008 20:05:45 +0000
Matthew Garrett <[email protected]> wrote:

> > > +#ifdef CONFIG_ACPI
> > > + if (acpi_video_backlight_support())
> > > + return 0;
> > > +#endif
> >
> > Do we need the ifdefs here? It looks like include/linux/acpi.h tries
> > to provide a suitable 0-returning stub?
>
> They're protected by #ifdef CONFIG_ACPI_VIDEO, but it looks like acpi.h
> is empty if CONFIG_ACPI isn't set?

No, acpi.h has a _few_ ACPI=n stubs:

: static inline int early_acpi_boot_init(void)
: {
: return 0;
: }
: static inline int acpi_boot_init(void)
: {
: return 0;
: }
:
: static inline int acpi_boot_table_init(void)
: {
: return 0;
: }
:
: static inline int acpi_mps_check(void)
: {
: return 0;
: }
:
: static inline int acpi_check_resource_conflict(struct resource *res)
: {
: return 0;
: }
:
: static inline int acpi_check_region(resource_size_t start, resource_size_t n,
: const char *name)
: {
: return 0;
: }
:
: static inline int acpi_check_mem_region(resource_size_t start,
: resource_size_t n, const char *name)
: {
: return 0;
: }

So it seems that acpi_video_backlight_support() should be added to this
list. Except it already has a stub, in the wrong place. This whole block:

: #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
:
: extern long acpi_video_get_capabilities(acpi_handle graphics_dev_handle);
: extern long acpi_is_video_device(struct acpi_device *device);
: extern int acpi_video_backlight_support(void);
: extern int acpi_video_display_switch_support(void);
:
: #else
:
: static inline long acpi_video_get_capabilities(acpi_handle graphics_dev_handle)
: {
: return 0;
: }
:
: static inline long acpi_is_video_device(struct acpi_device *device)
: {
: return 0;
: }
:
: static inline int acpi_video_backlight_support(void)
: {
: return 0;
: }
:
: static inline int acpi_video_display_switch_support(void)
: {
: return 0;
: }
:
: #endif /* defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE) */
:

needs to be moved outside the `#ifdef CONFIG_ACPI' altogether. Then you
can drop the ifdef.

2008-12-02 20:31:16

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2/2] misc: Add dell-laptop driver

On Tue, Dec 02, 2008 at 12:22:22PM -0800, Andrew Morton wrote:

> No, acpi.h has a _few_ ACPI=n stubs:

Ah, yes. I'll send a patch for that to Len.

--
Matthew Garrett | [email protected]

2008-12-03 12:37:39

by Alan Jenkins

[permalink] [raw]
Subject: Re: [PATCH 2/2 update] misc: Add dell-laptop driver

Matthew Garrett wrote:
> misc: Add dell-laptop driver
>
> Add a driver for controling Dell-specific backlight and rfkill interfaces.
> This driver makes use of the dcdbas interface to the Dell firmware to allow
> the backlight and rfkill interfaces on Dell systems to be driven through the
> standardised sysfs interfaces.
>
> Signed-off-by: Matthew Garrett <[email protected]>
>
> ---
>
> Contains Andrew's suggested updates, plus a fix for a leak on the init
> error path.
>

I see you don't call rfkill_set_default(). Do Dell firmwares support
persistent rfkill state?

My ill-informed guess is that it shouldn't hurt to call it. If the
firmware is sane, it will initialize the rfkill to a useful state.
Otherwise, the radio(s) would be unusable without the platform driver.
But do feel free to ignore me, if you suspect the firmware of insanity :).

Regards
Alan

2008-12-03 13:01:44

by Alan Jenkins

[permalink] [raw]
Subject: Re: [PATCH 2/2 update] misc: Add dell-laptop driver

Matthew Garrett wrote:
> misc: Add dell-laptop driver
>
> Add a driver for controling Dell-specific backlight and rfkill interfaces.
> This driver makes use of the dcdbas interface to the Dell firmware to allow
> the backlight and rfkill interfaces on Dell systems to be driven through the
> standardised sysfs interfaces.
>
> Signed-off-by: Matthew Garrett <[email protected]>
>
> ---
>
> Contains Andrew's suggested updates, plus a fix for a leak on the init
> error path.
>

Sorry, I should have mentioned this in my first message:

> + if ((status & (1<<2|1<<8)) == (1<<2|1<<8)) {
> + wifi_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_WLAN);
> + if (!wifi_rfkill)
> + goto err_wifi;
> + wifi_rfkill->name = "dell-wifi";
> + wifi_rfkill->toggle_radio = dell_wifi_set;
> + wifi_rfkill->get_state = dell_wifi_get;
>

The rfkill doc (and code) say that you also need to initialize
wifi_rfkill->state to the current state of the hardware.

> + ret = rfkill_register(wifi_rfkill);
> + if (ret)
> + goto err_wifi;
> + }
> +

Regards
Alan

2008-12-03 19:15:26

by Len Brown

[permalink] [raw]
Subject: Re: [PATCH 2/2 update] misc: Add dell-laptop driver

I didn't notice a MAINTAINER entry for this new driver.

--
-Len Brown
Intel Open Source Technology Center

2008-12-03 20:07:31

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2/2 update] misc: Add dell-laptop driver

Add a driver for controling Dell-specific backlight and rfkill
interfaces. This driver makes use of the dcdbas interface to the Dell
firmware to allow the backlight and rfkill interfaces on Dell systems to
be driven through the standardised sysfs interfaces.

Signed-off-by: Matthew Garrett <[email protected]>

---

Includes a MAINTAINERS entry.

diff --git a/MAINTAINERS b/MAINTAINERS
index 618c1ef..2de9dca 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1360,6 +1360,11 @@ P: Maciej W. Rozycki
M: [email protected]
S: Maintained

+DELL LAPTOP DRIVER
+P: Matthew Garrett
+M: [email protected]
+S: Maintained
+
DELL LAPTOP SMM DRIVER
P: Massimo Dal Zotto
M: [email protected]
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index fee7304..3949a1c 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -498,6 +498,18 @@ 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
+ default n
+ ---help---
+ This driver adds support for rfkill and backlight control to Dell
+ laptops.
+
source "drivers/misc/c2port/Kconfig"

endif # MISC_DEVICES
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 817f7f5..39043f8 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -32,4 +32,5 @@ obj-$(CONFIG_KGDB_TESTS) += kgdbts.o
obj-$(CONFIG_SGI_XP) += sgi-xp/
obj-$(CONFIG_SGI_GRU) += sgi-gru/
obj-$(CONFIG_HP_ILO) += hpilo.o
+obj-$(CONFIG_DELL_LAPTOP) += dell-laptop.o
obj-$(CONFIG_C2PORT) += c2port/
diff --git a/drivers/misc/dell-laptop.c b/drivers/misc/dell-laptop.c
new file mode 100644
index 0000000..4d33a20
--- /dev/null
+++ b/drivers/misc/dell-laptop.c
@@ -0,0 +1,436 @@
+/*
+ * Driver for Dell laptop extras
+ *
+ * Copyright (c) Red Hat <[email protected]>
+ *
+ * Based on documentation in the libsmbios package, Copyright (C) 2005 Dell
+ * Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/backlight.h>
+#include <linux/err.h>
+#include <linux/dmi.h>
+#include <linux/io.h>
+#include <linux/rfkill.h>
+#include <linux/power_supply.h>
+#include <linux/acpi.h>
+#include "../firmware/dcdbas.h"
+
+#define BRIGHTNESS_TOKEN 0x7d
+
+/* This structure will be modified by the firmware when we enter
+ * system management mode, hence the volatiles */
+
+struct calling_interface_buffer {
+ u16 class;
+ u16 select;
+ volatile u32 input[4];
+ volatile u32 output[4];
+} __packed;
+
+struct calling_interface_token {
+ u16 tokenID;
+ u16 location;
+ union {
+ u16 value;
+ u16 stringlength;
+ };
+};
+
+struct calling_interface_structure {
+ struct dmi_header header;
+ u16 cmdIOAddress;
+ u8 cmdIOCode;
+ u32 supportedCmds;
+ struct calling_interface_token tokens[];
+} __packed;
+
+static int da_command_address;
+static int da_command_code;
+static int da_num_tokens;
+static struct calling_interface_token *da_tokens;
+
+static struct backlight_device *dell_backlight_device;
+static struct rfkill *wifi_rfkill;
+static struct rfkill *bluetooth_rfkill;
+static struct rfkill *wwan_rfkill;
+
+static const struct dmi_system_id __initdata dell_device_table[] = {
+ {
+ .ident = "Dell laptop",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_CHASSIS_TYPE, "8"),
+ },
+ },
+ { }
+};
+
+static void 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;
+ struct calling_interface_structure *table =
+ container_of(dm, struct calling_interface_structure, header);
+
+ /* 4 bytes of table header, plus 7 bytes of Dell header, plus at least
+ 6 bytes of entry */
+
+ if (dm->length < 17)
+ return;
+
+ da_command_address = table->cmdIOAddress;
+ da_command_code = table->cmdIOCode;
+
+ da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
+ sizeof(struct calling_interface_token),
+ GFP_KERNEL);
+
+ if (!da_tokens)
+ return;
+
+ memcpy(da_tokens+da_num_tokens, table->tokens,
+ sizeof(struct calling_interface_token) * tokens);
+
+ da_num_tokens += tokens;
+}
+
+static void find_tokens(const struct dmi_header *dm)
+{
+ switch (dm->type) {
+ case 0xd4: /* Indexed IO */
+ break;
+ case 0xd5: /* Protected Area Type 1 */
+ break;
+ case 0xd6: /* Protected Area Type 2 */
+ break;
+ case 0xda: /* Calling interface */
+ parse_da_table(dm);
+ break;
+ }
+}
+
+static int find_token_location(int tokenid)
+{
+ int i;
+ for (i = 0; i < da_num_tokens; i++) {
+ if (da_tokens[i].tokenID == tokenid)
+ return da_tokens[i].location;
+ }
+
+ return -1;
+}
+
+static struct calling_interface_buffer *
+dell_send_request(struct calling_interface_buffer *buffer, int class,
+ int select)
+{
+ struct smi_cmd command;
+
+ command.magic = SMI_CMD_MAGIC;
+ command.command_address = da_command_address;
+ command.command_code = da_command_code;
+ command.ebx = virt_to_phys(buffer);
+ command.ecx = 0x42534931;
+
+ buffer->class = class;
+ buffer->select = select;
+
+ dcdbas_smi_request(&command);
+
+ return buffer;
+}
+
+/* Derived from information in DellWirelessCtl.cpp:
+ Class 17, select 11 is radio control. It returns an array of 32-bit values.
+
+ 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
+*/
+
+static int dell_rfkill_set(int radio, enum rfkill_state state)
+{
+ struct calling_interface_buffer buffer;
+ int disable = (state == RFKILL_STATE_UNBLOCKED) ? 0 : 1;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ buffer.input[0] = (1 | (radio<<8) | (disable << 16));
+ dell_send_request(&buffer, 17, 11);
+
+ return 0;
+}
+
+static int dell_wifi_set(void *data, enum rfkill_state state)
+{
+ return dell_rfkill_set(1, state);
+}
+
+static int dell_bluetooth_set(void *data, enum rfkill_state state)
+{
+ return dell_rfkill_set(2, state);
+}
+
+static int dell_wwan_set(void *data, enum rfkill_state state)
+{
+ return dell_rfkill_set(3, state);
+}
+
+static int dell_rfkill_get(int bit, enum rfkill_state *state)
+{
+ struct calling_interface_buffer buffer;
+ int status;
+ int new_state = RFKILL_STATE_HARD_BLOCKED;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ dell_send_request(&buffer, 17, 11);
+ status = buffer.output[1];
+
+ if (status & (1<<16))
+ new_state = RFKILL_STATE_SOFT_BLOCKED;
+
+ if (status & (1<<bit))
+ *state = new_state;
+ else
+ *state = RFKILL_STATE_UNBLOCKED;
+
+ return 0;
+}
+
+static int dell_wifi_get(void *data, enum rfkill_state *state)
+{
+ return dell_rfkill_get(17, state);
+}
+
+static int dell_bluetooth_get(void *data, enum rfkill_state *state)
+{
+ return dell_rfkill_get(18, state);
+}
+
+static int dell_wwan_get(void *data, enum rfkill_state *state)
+{
+ return dell_rfkill_get(19, state);
+}
+
+static int dell_setup_rfkill(void)
+{
+ struct calling_interface_buffer buffer;
+ int status;
+ int ret;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ dell_send_request(&buffer, 17, 11);
+ status = buffer.output[1];
+
+ if ((status & (1<<2|1<<8)) == (1<<2|1<<8)) {
+ wifi_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_WLAN);
+ if (!wifi_rfkill)
+ goto err_wifi;
+ wifi_rfkill->name = "dell-wifi";
+ wifi_rfkill->toggle_radio = dell_wifi_set;
+ wifi_rfkill->get_state = dell_wifi_get;
+ ret = rfkill_register(wifi_rfkill);
+ if (ret)
+ goto err_wifi;
+ }
+
+ if ((status & (1<<3|1<<9)) == (1<<3|1<<9)) {
+ bluetooth_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_BLUETOOTH);
+ if (!bluetooth_rfkill)
+ goto err_bluetooth;
+ bluetooth_rfkill->name = "dell-bluetooth";
+ bluetooth_rfkill->toggle_radio = dell_bluetooth_set;
+ bluetooth_rfkill->get_state = dell_bluetooth_get;
+ ret = rfkill_register(bluetooth_rfkill);
+ if (ret)
+ goto err_bluetooth;
+ }
+
+ if ((status & (1<<4|1<<10)) == (1<<4|1<<10)) {
+ wwan_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_WWAN);
+ if (!wwan_rfkill)
+ goto err_wwan;
+ wwan_rfkill->name = "dell-wwan";
+ wwan_rfkill->toggle_radio = dell_wwan_set;
+ wwan_rfkill->get_state = dell_wwan_get;
+ ret = rfkill_register(wwan_rfkill);
+ if (ret)
+ goto err_wwan;
+ }
+
+ return 0;
+err_wwan:
+ if (wwan_rfkill)
+ rfkill_free(wwan_rfkill);
+ if (bluetooth_rfkill) {
+ rfkill_unregister(bluetooth_rfkill);
+ bluetooth_rfkill = NULL;
+ }
+err_bluetooth:
+ if (bluetooth_rfkill)
+ rfkill_free(bluetooth_rfkill);
+ if (wifi_rfkill) {
+ rfkill_unregister(wifi_rfkill);
+ wifi_rfkill = NULL;
+ }
+err_wifi:
+ if (wifi_rfkill)
+ rfkill_free(wifi_rfkill);
+
+ return ret;
+}
+
+static int dell_send_intensity(struct backlight_device *bd)
+{
+ struct calling_interface_buffer buffer;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
+ buffer.input[1] = bd->props.brightness;
+
+ if (buffer.input[0] == -1)
+ return -ENODEV;
+
+ if (power_supply_is_system_supplied() > 0)
+ dell_send_request(&buffer, 1, 2);
+ else
+ dell_send_request(&buffer, 1, 1);
+
+ return 0;
+}
+
+static int dell_get_intensity(struct backlight_device *bd)
+{
+ struct calling_interface_buffer buffer;
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
+
+ if (buffer.input[0] == -1)
+ return -ENODEV;
+
+ if (power_supply_is_system_supplied() > 0)
+ dell_send_request(&buffer, 0, 2);
+ else
+ dell_send_request(&buffer, 0, 1);
+
+ return buffer.output[1];
+}
+
+static struct backlight_ops dell_ops = {
+ .get_brightness = dell_get_intensity,
+ .update_status = dell_send_intensity,
+};
+
+static int __init dell_init(void)
+{
+ struct calling_interface_buffer buffer;
+ int max_intensity = 0;
+ int ret;
+
+ if (!dmi_check_system(dell_device_table))
+ return -ENODEV;
+
+ dmi_walk(find_tokens);
+
+ if (!da_tokens) {
+ printk(KERN_INFO "dell-laptop: Unable to find dmi tokens\n");
+ return -ENODEV;
+ }
+
+ ret = dell_setup_rfkill();
+
+ if (ret) {
+ printk(KERN_WARNING "dell-laptop: Unable to setup rfkill\n");
+ goto out;
+ }
+
+#ifdef CONFIG_ACPI
+ /* In the event of an ACPI backlight being available, don't
+ * register the platform controller.
+ */
+ if (acpi_video_backlight_support())
+ return 0;
+#endif
+
+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
+ 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 (max_intensity) {
+ dell_backlight_device = backlight_device_register(
+ "dell_backlight",
+ NULL, NULL,
+ &dell_ops);
+
+ if (IS_ERR(dell_backlight_device)) {
+ ret = PTR_ERR(dell_backlight_device);
+ dell_backlight_device = NULL;
+ goto out;
+ }
+
+ dell_backlight_device->props.max_brightness = max_intensity;
+ dell_backlight_device->props.brightness =
+ dell_get_intensity(dell_backlight_device);
+ backlight_update_status(dell_backlight_device);
+ }
+
+ return 0;
+out:
+ if (wifi_rfkill)
+ rfkill_unregister(wifi_rfkill);
+ if (bluetooth_rfkill)
+ rfkill_unregister(bluetooth_rfkill);
+ if (wwan_rfkill)
+ rfkill_unregister(wwan_rfkill);
+ kfree(da_tokens);
+ return ret;
+}
+
+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);
+}
+
+module_init(dell_init);
+module_exit(dell_exit);
+
+MODULE_AUTHOR("Matthew Garrett <[email protected]>");
+MODULE_DESCRIPTION("Dell laptop driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("dmi:*svnDellInc.:*:ct8:*");

--
Matthew Garrett | [email protected]

2008-12-04 09:12:56

by Sven Wegener

[permalink] [raw]
Subject: Re: [PATCH 2/2 update] misc: Add dell-laptop driver

On Wed, 3 Dec 2008, Matthew Garrett wrote:

> Add a driver for controling Dell-specific backlight and rfkill
> interfaces. This driver makes use of the dcdbas interface to the Dell
> firmware to allow the backlight and rfkill interfaces on Dell systems to
> be driven through the standardised sysfs interfaces.

> +static void parse_da_table(const struct dmi_header *dm)

__init

> +static void find_tokens(const struct dmi_header *dm)

__init

> +{
> + switch (dm->type) {
> + case 0xd4: /* Indexed IO */
> + break;
> + case 0xd5: /* Protected Area Type 1 */
> + break;
> + case 0xd6: /* Protected Area Type 2 */
> + break;
> + case 0xda: /* Calling interface */
> + parse_da_table(dm);
> + break;
> + }

This could be a simple if (dm->type == 0xda), it looks like the switch
is abused as documentation.

> +/* Derived from information in DellWirelessCtl.cpp:
> + Class 17, select 11 is radio control. It returns an array of 32-bit values.
> +
> + 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

Could these bit numbers be defines with declarative names, e.g define
DELL_WIFI_BLOCKED (1<<17)? It would make reading the code that just uses
plain numbers easier.

> + memset(&buffer, 0, sizeof(struct calling_interface_buffer));

sizeof(buffer)

Likewise for other sizeof() occurrences that follow.

> +static int dell_setup_rfkill(void)

__init

Sven

2008-12-04 11:01:09

by Alan Jenkins

[permalink] [raw]
Subject: Re: [PATCH 2/2 update] misc: Add dell-laptop driver

I still think you need to initialise wifi_rfkill->state before you call
rfkill_register().

It seems an easy mistake, probably the rfkill core should test for it.

Alan

(Sorry about the insane From: header in my previous mail)

Subject: Re: [PATCH 2/2 update] misc: Add dell-laptop driver

(I feel weird replying to "none" :-) )

On Wed, 03 Dec 2008, none wrote:
> > + wifi_rfkill->get_state = dell_wifi_get;
>
> The rfkill doc (and code) say that you also need to initialize
> wifi_rfkill->state to the current state of the hardware.

I may have screwed up and ommited from the docs the fact that it is not
strictly necessary to do so if you provide rfkill->get_state. So that is
not a serious problem with the patch at all.

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

Subject: Re: [PATCH 2/2 update] misc: Add dell-laptop driver

On Thu, 04 Dec 2008, Alan Jenkins wrote:
> I still think you need to initialise wifi_rfkill->state before you call
> rfkill_register().
>
> It seems an easy mistake, probably the rfkill core should test for it.

Well, yes, I could add that paranoia to the core. Added to the todo
list.

But the core will re-init rfkill->state from rfkill->get_state(), so
it is not a serious problem at all (since the drive provides
rfkill->get_state().

The reason it is recommended to always init the thing, is that if you
later move to a purely event-driven rfkill interface, you won't break
things.

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh