This driver adds backlight and rfkill support for Dell laptops. It uses
the DCDBAS driver to trigger the system management calls required for
this, and parses the DMI tables itself in order to find the appropriate
tokens. In future it should be possible to add LED control, but I don't
have any appropriate machines right now for testing.
--
Matthew Garrett | [email protected]
Rename the dcdbas SMI request call to avoid namespacing issues, and then
export it so other kernel modules can make use of the functionality.
Signed-off-by: Matthew Garrett <[email protected]>
---
commit be8f9ba0f636c9eec356976acc178623ec58651e
Author: Matthew Garrett <mjg59@m6300.(none)>
Date: Sat Aug 16 21:12:09 2008 +0100
Add support for using the dcdbas framework from inside the kernel
diff --git a/drivers/firmware/dcdbas.c b/drivers/firmware/dcdbas.c
index 50a071f..762131b 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;
@@ -279,6 +279,8 @@ out:
return ret;
}
+EXPORT_SYMBOL(dcdbas_smi_request);
+
/**
* smi_request_store:
*
@@ -309,14 +311,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]
This driver provides in-kernel control for the backlight and rfkill
functionality on Dell laptops. It's based on the publically available
information in the libsmbios package.
Signed-off-by: Matthew Garrett <[email protected]>
---
commit 472d8eed1891ba440f8e6ffde4787f6308390f7d
Author: Matthew Garrett <mjg59@m6300.(none)>
Date: Sat Aug 16 21:12:51 2008 +0100
Add Dell laptop driver
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a726f3b..c77eab3 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -475,4 +475,16 @@ 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.
+
endif # MISC_DEVICES
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c6c13f6..865343d 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -30,3 +30,4 @@ 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
\ No newline at end of file
diff --git a/drivers/misc/dell-laptop.c b/drivers/misc/dell-laptop.c
new file mode 100644
index 0000000..ab09a69
--- /dev/null
+++ b/drivers/misc/dell-laptop.c
@@ -0,0 +1,356 @@
+/*
+ * 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 "../firmware/dcdbas.h"
+
+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 {
+ u8 type;
+ u8 length;
+ u16 handle;
+ 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 =
+ (struct calling_interface_structure *)dm;
+
+ /* 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);
+
+ 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 = kzalloc(sizeof(struct smi_cmd), GFP_KERNEL);
+ 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);
+
+ kfree(command);
+
+ return buffer;
+}
+
+static int dell_rfkill_set(int radio, enum rfkill_state state)
+{
+ struct calling_interface_buffer *buffer =
+ kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
+ int disable = (state == RFKILL_STATE_UNBLOCKED) ? 0 : 1;
+ buffer->input[0] = (1 | (radio<<8) | (disable << 16));
+ dell_send_request(buffer, 17, 11);
+ kfree(buffer);
+ 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 =
+ kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
+ int status;
+ int new_state = RFKILL_STATE_HARD_BLOCKED;
+
+ 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;
+
+ kfree(buffer);
+
+ 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 void dell_setup_rfkill(void)
+{
+ struct calling_interface_buffer *buffer =
+ kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
+ int status;
+
+ 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);
+ wifi_rfkill->name = "dell-wifi";
+ wifi_rfkill->toggle_radio = dell_wifi_set;
+ wifi_rfkill->get_state = dell_wifi_get;
+ rfkill_register(wifi_rfkill);
+ }
+
+ if ((status & (1<<3|1<<9)) == (1<<3|1<<9)) {
+ bluetooth_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_BLUETOOTH);
+ bluetooth_rfkill->name = "dell-bluetooth";
+ bluetooth_rfkill->toggle_radio = dell_bluetooth_set;
+ bluetooth_rfkill->get_state = dell_bluetooth_get;
+ rfkill_register(bluetooth_rfkill);
+ }
+
+ if ((status & (1<<4|1<<10)) == (1<<4|1<<10)) {
+ wwan_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_WWAN);
+ wwan_rfkill->name = "dell-wwan";
+ wwan_rfkill->toggle_radio = dell_wwan_set;
+ wwan_rfkill->get_state = dell_wwan_get;
+ rfkill_register(wwan_rfkill);
+ }
+
+ kfree(buffer);
+}
+
+static int dell_send_intensity(struct backlight_device *bd)
+{
+ int intensity = bd->props.brightness;
+ struct calling_interface_buffer *buffer =
+ kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
+
+ buffer->input[0] = find_token_location(0x7d);
+ buffer->input[1] = intensity;
+
+ if (buffer->input[0] == -1) {
+ kfree(buffer);
+ return -ENODEV;
+ }
+
+ dell_send_request(buffer, 1, 1);
+ dell_send_request(buffer, 1, 2);
+
+ kfree(buffer);
+
+ return 0;
+}
+
+static int dell_get_intensity(struct backlight_device *bd)
+{
+ struct calling_interface_buffer *buffer =
+ kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
+ int brightness;
+
+ buffer->input[0] = find_token_location(0x7d);
+
+ if (buffer->input[0] == -1) {
+ kfree(buffer);
+ return -ENODEV;
+ }
+
+ dell_send_request(buffer, 0, 1);
+
+ brightness = buffer->output[1];
+
+ kfree(buffer);
+
+ return brightness;
+}
+
+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;
+
+ if (!dmi_check_system(dell_device_table))
+ return -ENODEV;
+
+ dmi_walk(find_tokens);
+
+ if (!da_tokens)
+ return -ENODEV;
+
+ buffer = kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
+ buffer->input[0] = find_token_location(0x7d);
+
+ if (buffer->input[0] != -1) {
+ dell_send_request(buffer, 2, 1);
+ max_intensity = buffer->output[3];
+ }
+
+ kfree(buffer);
+
+ 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;
+ 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);
+ }
+
+ dell_setup_rfkill();
+
+out:
+ return 0;
+}
+
+static void __exit dell_exit(void)
+{
+ if (dell_backlight_device)
+ 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]
On Sat, 16 Aug 2008 21:30:27 +0100
Matthew Garrett <[email protected]> wrote:
> This driver provides in-kernel control for the backlight and rfkill
> functionality on Dell laptops. It's based on the publically available
> information in the libsmbios package.
how does this interact with the backlight control done via ACPI ?
--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
On Sat, Aug 16, 2008 at 01:47:52PM -0700, Arjan van de Ven wrote:
> how does this interact with the backlight control done via ACPI ?
Dell's ACPI implementation appears to be implemented on top of this at a
firmware level, so both are consistent. However, in terms of
cleanliness, we should wait for Thomas's patches to be merged so this
driver can disable the backlight support if there's an ACPI backlight
present. The reason I implemented it is that there's no ACPI backlight
support on some currently shipping Dells, never mind the older hardware.
--
Matthew Garrett | [email protected]
Matthew Garrett <mjg59 <at> srcf.ucam.org> writes:
> diff --git a/drivers/misc/dell-laptop.c b/drivers/misc/dell-laptop.c
> new file mode 100644
> index 0000000..ab09a69
> --- /dev/null
> +++ b/drivers/misc/dell-laptop.c
[Snip]
> + 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);
Isn't a NULL check required here?
> +
> + memcpy(da_tokens+da_num_tokens, table->tokens,
> + sizeof(struct calling_interface_token) * tokens);
> +
> +static struct calling_interface_buffer *dell_send_request(
> + struct calling_interface_buffer *buffer, int class, int select)
> +{
> + struct smi_cmd *command = kzalloc(sizeof(struct smi_cmd), GFP_KERNEL);
NULL check - Here too.
> + 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;
> +
> +
> +static int dell_rfkill_set(int radio, enum rfkill_state state)
> +{
> + struct calling_interface_buffer *buffer =
> + kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
Ditto.
> + int disable = (state == RFKILL_STATE_UNBLOCKED) ? 0 : 1;
> + buffer->input[0] = (1 | (radio<<8) | (disable << 16));
> + dell_send_request(buffer, 17, 11);
> + kfree(buffer);
> + return 0;
> +}
> +static int dell_rfkill_get(int bit, enum rfkill_state *state)
> +{
> + struct calling_interface_buffer *buffer =
> + kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
This makes me confident there is code in the kernel that always
succeeds allocations by mjg59.
Just got to find where it's hidden :)
> +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 void dell_setup_rfkill(void)
> +{
> + struct calling_interface_buffer *buffer =
> + kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
Once more.
> +
> +static int dell_send_intensity(struct backlight_device *bd)
> +{
> + int intensity = bd->props.brightness;
> + struct calling_interface_buffer *buffer =
> + kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
Stopped counting!
> +
> + buffer->input[0] = find_token_location(0x7d);
> + buffer->input[1] = intensity;
> +
> + if (buffer->input[0] == -1) {
> + kfree(buffer);
> + return -ENODEV;
> + }
> +
> +static int dell_get_intensity(struct backlight_device *bd)
> +{
> + struct calling_interface_buffer *buffer =
> + kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
count_prev++;
> + if (!dmi_check_system(dell_device_table))
> + return -ENODEV;
> +
> + dmi_walk(find_tokens);
> +
> + if (!da_tokens)
> + return -ENODEV;
> +
> + buffer = kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
One last.
> + buffer->input[0] = find_token_location(0x7d);
> +
> + if (buffer->input[0] != -1) {
> + dell_send_request(buffer, 2, 1);
> + max_intensity = buffer->output[3];
> + }
> +
Parag
On Sat, Aug 16, 2008 at 07:17:05PM -0400, Parag Warudkar wrote:
> > + da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
> > + sizeof(struct calling_interface_token),
> > + GFP_KERNEL);
>
> Isn't a NULL check required here?
To the extent that it would result in this failing gracefully before
some other part of the kernel becomes hideously upset, sure. I have a
hard time caring, but I can easily fix it if other people do :)
--
Matthew Garrett | [email protected]
On Sat, Aug 16, 2008 at 7:23 PM, Matthew Garrett <[email protected]> wrote:
> On Sat, Aug 16, 2008 at 07:17:05PM -0400, Parag Warudkar wrote:
>> > + da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
>> > + sizeof(struct calling_interface_token),
>> > + GFP_KERNEL);
>>
>> Isn't a NULL check required here?
>
> To the extent that it would result in this failing gracefully before
> some other part of the kernel becomes hideously upset, sure. I have a
> hard time caring, but I can easily fix it if other people do :)
Yeah, it's a RFKill switch after all, not a LaptopKill switch :)
Seriously though a quick search through LXR results for kzalloc shows
that most everyone does care.
And if I find ones that don't a patch will follow soon.
Thanks
Parag
On Sat, 16 Aug 2008 21:30:27 +0100 Matthew Garrett <[email protected]> wrote:
> This driver provides in-kernel control for the backlight and rfkill
> functionality on Dell laptops. It's based on the publically available
> information in the libsmbios package.
>
> Signed-off-by: Matthew Garrett <[email protected]>
>
> ---
>
> commit 472d8eed1891ba440f8e6ffde4787f6308390f7d
> Author: Matthew Garrett <mjg59@m6300.(none)>
> Date: Sat Aug 16 21:12:51 2008 +0100
>
> Add Dell laptop driver
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index a726f3b..c77eab3 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -475,4 +475,16 @@ 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.
whitespace gone wild.
> endif # MISC_DEVICES
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index c6c13f6..865343d 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -30,3 +30,4 @@ 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
> \ No newline at end of file
> diff --git a/drivers/misc/dell-laptop.c b/drivers/misc/dell-laptop.c
> new file mode 100644
> index 0000000..ab09a69
> --- /dev/null
> +++ b/drivers/misc/dell-laptop.c
> @@ -0,0 +1,356 @@
> +/*
> + * 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 "../firmware/dcdbas.h"
> +
> +struct calling_interface_buffer {
> + u16 class;
> + u16 select;
> + volatile u32 input[4];
> + volatile u32 output[4];
> +} __attribute__ ((packed));
checkpatch will direct you to a fine document.
If the `volatiles' are unavoidable then I'd suggest that both the
changelog and code comments explain the reason(s) for their addition.
> +struct calling_interface_token {
> + u16 tokenID;
> + u16 location;
> + union {
> + u16 value;
> + u16 stringlength;
> + };
> +};
> +
> +struct calling_interface_structure {
> + u8 type;
> + u8 length;
> + u16 handle;
Make the above three members a `struct dmi_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 =
> + (struct calling_interface_structure *)dm;
... and switch this to container_of().
> + /* 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);
> +
> + memcpy(da_tokens+da_num_tokens, table->tokens,
> + sizeof(struct calling_interface_token) * tokens);
krealloc() can fail.
> + 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)
static struct calling_interface_buffer *
dell_send_request(struct calling_interface_buffer *buffer, int class,
int select)
would be more typical layout, although not vastly better.
> +{
> + struct smi_cmd *command = kzalloc(sizeof(struct smi_cmd), GFP_KERNEL);
> + 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;
Please put a blank line between end-of-locals and start-of-code.
Everywhere, all the time.
kzalloc() can fail and must be checked.
> + buffer->class = class;
> + buffer->select = select;
> +
> + dcdbas_smi_request(command);
> +
> + kfree(command);
Can we make `command' a local? It's pretty small. Perhaps
dcdbas_smi_request() (nee smi_request()) has some secret, undocumented
alignment restriction, similar to DMA?
> + return buffer;
> +}
> +
> +static int dell_rfkill_set(int radio, enum rfkill_state state)
> +{
> + struct calling_interface_buffer *buffer =
> + kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
> + int disable = (state == RFKILL_STATE_UNBLOCKED) ? 0 : 1;
> + buffer->input[0] = (1 | (radio<<8) | (disable << 16));
> + dell_send_request(buffer, 17, 11);
> + kfree(buffer);
> + return 0;
> +}
kzalloc() can fail.
calling_interface_buffer is small enough to be a local.
coding-style.
> +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 =
> + kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
> + int status;
> + int new_state = RFKILL_STATE_HARD_BLOCKED;
> +
> + dell_send_request(buffer, 17, 11);
> + status = buffer->output[1];
kzalloc() can fail.
> + if (status & (1<<16))
> + new_state = RFKILL_STATE_SOFT_BLOCKED;
> +
> + if (status & (1<<bit))
> + *state = new_state;
> + else
> + *state = RFKILL_STATE_UNBLOCKED;
> +
> + kfree(buffer);
`buffer' is small enough to be a local.
> + 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 void dell_setup_rfkill(void)
> +{
> + struct calling_interface_buffer *buffer =
> + kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
ditto, ditto
> + int status;
> +
> + dell_send_request(buffer, 17, 11);
> + status = buffer->output[1];
> +
> + if ((status & (1<<2|1<<8)) == (1<<2|1<<8)) {
Where does the reader go to find the meaning of these bits?
(code comment needed)
> + wifi_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_WLAN);
> + wifi_rfkill->name = "dell-wifi";
> + wifi_rfkill->toggle_radio = dell_wifi_set;
> + wifi_rfkill->get_state = dell_wifi_get;
> + rfkill_register(wifi_rfkill);
> + }
> +
> + if ((status & (1<<3|1<<9)) == (1<<3|1<<9)) {
> + bluetooth_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_BLUETOOTH);
> + bluetooth_rfkill->name = "dell-bluetooth";
> + bluetooth_rfkill->toggle_radio = dell_bluetooth_set;
> + bluetooth_rfkill->get_state = dell_bluetooth_get;
> + rfkill_register(bluetooth_rfkill);
> + }
> +
> + if ((status & (1<<4|1<<10)) == (1<<4|1<<10)) {
> + wwan_rfkill = rfkill_allocate(NULL, RFKILL_TYPE_WWAN);
> + wwan_rfkill->name = "dell-wwan";
> + wwan_rfkill->toggle_radio = dell_wwan_set;
> + wwan_rfkill->get_state = dell_wwan_get;
> + rfkill_register(wwan_rfkill);
> + }
dittoes
> + kfree(buffer);
> +}
> +
> +static int dell_send_intensity(struct backlight_device *bd)
> +{
> + int intensity = bd->props.brightness;
> + struct calling_interface_buffer *buffer =
> + kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
blah
> + buffer->input[0] = find_token_location(0x7d);
> + buffer->input[1] = intensity;
I don't think we really needed local variable `intensity'. I guess it
has some commentary value.
> + if (buffer->input[0] == -1) {
> + kfree(buffer);
> + return -ENODEV;
> + }
> +
> + dell_send_request(buffer, 1, 1);
> + dell_send_request(buffer, 1, 2);
> +
> + kfree(buffer);
> +
> + return 0;
> +}
> +
> +static int dell_get_intensity(struct backlight_device *bd)
> +{
> + struct calling_interface_buffer *buffer =
> + kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
ditto, ditto
> + int brightness;
> +
> + buffer->input[0] = find_token_location(0x7d);
> +
> + if (buffer->input[0] == -1) {
> + kfree(buffer);
> + return -ENODEV;
> + }
> +
> + dell_send_request(buffer, 0, 1);
> +
> + brightness = buffer->output[1];
> +
> + kfree(buffer);
> +
> + return brightness;
> +}
> +
> +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;
> +
> + if (!dmi_check_system(dell_device_table))
> + return -ENODEV;
> +
> + dmi_walk(find_tokens);
> +
> + if (!da_tokens)
> + return -ENODEV;
> +
> + buffer = kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
ditto
> + buffer->input[0] = find_token_location(0x7d);
> +
> + if (buffer->input[0] != -1) {
> + dell_send_request(buffer, 2, 1);
> + max_intensity = buffer->output[3];
> + }
> +
> + kfree(buffer);
> +
> + 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;
> + goto out;
Should propagate PTR_ERR(dell_backlight_device) back to caller.
> + }
> +
> + 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);
> + }
> +
> + dell_setup_rfkill();
> +
> +out:
> + return 0;
> +}
> +
> +static void __exit dell_exit(void)
> +{
> + if (dell_backlight_device)
> + backlight_device_unregister(dell_backlight_device);
backlight_device_unregister(NULL) is legal
> + 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:*");
On Tue, Aug 19, 2008 at 11:46:37PM -0700, Andrew Morton wrote:
> > +struct calling_interface_buffer {
> > + u16 class;
> > + u16 select;
> > + volatile u32 input[4];
> > + volatile u32 output[4];
> > +} __attribute__ ((packed));
>
> checkpatch will direct you to a fine document.
The address of the structure is passed to the BIOS, and it's then
modified in system management mode. I think volatile is the right thing
here? I'll fix up the rest.
--
Matthew Garrett | [email protected]