dcdbas: Export functionality for use in other drivers
Subsystems (2)
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.
drivers/firmware/dcdbas.c +5 -4
@@ -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 ;
drivers/firmware/dcdbas.h +2 -0
@@ -101,5 +101,7 @@ struct apm_cmd { } __attribute__ ((packed)) parameters; } __attribute__ ((packed)); +int dcdbas_smi_request (struct smi_cmd *smi_cmd); +#endif /* _DCDBAS_H_ */
Signature · 1 line
Matthew Garrett | [email protected]
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.
drivers/misc/Kconfig +12 -0
@@ -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
drivers/misc/Makefile +1 -0
@@ -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/
drivers/misc/dell-laptop.c new file +430 -0
430 lines not on this page · raw message
Signature · 1 line
Matthew Garrett | [email protected]
Re: [PATCH 2/2] misc: Add dell-laptop driver
Quoted 4 lines · Matthew Garrett
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
Signature · 3 lines
Matt Domsch Linux Technology Strategist, Dell Office of the CTO linux.dell.com & www.dell.com/linux
Re: [PATCH 2/2] misc: Add dell-laptop driver
Quoted 4 lines · Matt Domsch
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.
Signature · 1 line
Matthew Garrett | [email protected]
Re: [PATCH 2/2] misc: Add dell-laptop driver
In reply to Matthew GarrettQuoted 8 lines · 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.
...
drivers/misc/dell-laptop.c line 37
Quoted 6 lines
drivers/misc/dell-laptop.c new file +6
-0+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.
Quoted 6 lines
drivers/misc/dell-laptop.c new file +6
-0+struct calling_interface_token { +u16 tokenID; +u16 location; +union { +u16 value; +u16 stringlength;
drivers/misc/dell-laptop.c line 54
Quoted 10 lines
drivers/misc/dell-laptop.c new file +10
-0+ }; +}; + +struct calling_interface_structure { +struct dmi_header header; +u16 cmdIOAddress; +u8 cmdIOCode; +u32 supportedCmds; +struct calling_interface_token tokens[]; +} __attribute__ ((packed));
ditto.
drivers/misc/dell-laptop.c line 66
Quoted 11 lines
drivers/misc/dell-laptop.c new file +11
-0+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.
Quoted 4 lines
drivers/misc/dell-laptop.c new file +4
-0+ { + .ident = "Dell laptop", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
drivers/misc/dell-laptop.c line 80
Quoted 10 lines
drivers/misc/dell-laptop.c new file +10
-0+ 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.
Quoted 14 lines
drivers/misc/dell-laptop.c new file +14
-0+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),
drivers/misc/dell-laptop.c line 104
Quoted 10 lines
drivers/misc/dell-laptop.c new file +10
-0+ 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?
Quoted 18 lines
...
drivers/misc/dell-laptop.c new file +15
-0+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 ();
drivers/misc/dell-laptop.c line 374
Quoted 10 lines
drivers/misc/dell-laptop.c new file +10
-0+ +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?
Quoted 7 lines
drivers/misc/dell-laptop.c new file +7
-0+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 ]; + }
drivers/misc/dell-laptop.c line 392
Quoted 10 lines
drivers/misc/dell-laptop.c new file +10
-0+ +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.
Quoted 20 lines
drivers/misc/dell-laptop.c new file +20
-0+ 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; +} +
Re: [PATCH 2/2] misc: Add dell-laptop driver
Quoted 8 lines · Andrew Morton
drivers/misc/dell-laptop.c line 37
Quoted 6 lines
drivers/misc/dell-laptop.c new file +6
-0+struct calling_interface_buffer { +u16 class; +u16 select; +volatile u32 input[4 ]; +volatile u32 output[4 ]; +} __attribute__ ((packed));We have that little
__packedhelper for this.
Ok. I've switched to that, although the __attribute__ notation seems to be the dominant form in the kernel right now.
Quoted 3 lines
Quoted 1 line
+static struct
dmi_system_id__initdatadell_device_table[] = {this can be made const.
Done.
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ 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.
Quoted 7 lines
drivers/misc/dell-laptop.c line 374
Quoted 4 lines
drivers/misc/dell-laptop.c new file +4
-0+#ifdef CONFIG_ACPI +if (acpi_video_backlight_support ()) +return 0 ; +#endif Do we need the ifdefs here? It looks like
include/linux/acpi.htries 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?
Quoted 4 lines
drivers/misc/dell-laptop.c line 392
Quoted 2 lines
drivers/misc/dell-laptop.c new file +2
-0+ dell_backlight_device =NULL ; + ret =PTR_ERR (dell_backlight_device);The above two statements are in the wrong order.
Good catch, thanks.
Signature · 1 line
Matthew Garrett | [email protected]
Re: [PATCH 2/2] misc: Add dell-laptop driver
Quoted 10 lines · On Tue, 2 Dec 2008 20:05:45 +0000 Matthew Garrett <[email protected]> wrote:
Quoted 7 lines
drivers/misc/dell-laptop.c line 374
Quoted 4 lines
drivers/misc/dell-laptop.c new file +4
-0+#ifdef CONFIG_ACPI +if (acpi_video_backlight_support ()) +return 0 ; +#endif Do we need the ifdefs here? It looks like
include/linux/acpi.htries to provide a suitable 0-returning stub?They're protected by #ifdef
CONFIG_ACPI_VIDEO, but it looks likeacpi.his empty ifCONFIG_ACPIisn'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.
Re: [PATCH 2/2] misc: Add dell-laptop driver
Quoted 1 line · Andrew Morton
No,
acpi.hhas a _few_ ACPI=n stubs:
Ah, yes. I'll send a patch for that to Len.
Signature · 1 line
Matthew Garrett | [email protected]
PATCH 2/2 misc: Add dell-laptop driver
In reply to Andrew Mortonmisc: 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.
---
Contains Andrew's suggested updates, plus a fix for a leak on the init error path.
drivers/misc/Kconfig +12 -0
@@ -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
drivers/misc/Makefile +1 -0
@@ -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/
drivers/misc/dell-laptop.c new file +436 -0
436 lines not on this page · raw message
Signature · 1 line
Matthew Garrett | [email protected]
Re: [PATCH 2/2 update] misc: Add dell-laptop driver
Quoted 7 lines · Matthew Garrett
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.
Quoted 7 lines
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
Re: [PATCH 2/2 update] misc: Add dell-laptop driver
In reply to Matthew GarrettQuoted 7 lines · Matthew Garrett
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.
Quoted 7 lines
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:
drivers/misc/dell-laptop.c line 256
Quoted 8 lines
drivers/misc/dell-laptop.c new file +7
-0+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.
drivers/misc/dell-laptop.c line 261
Quoted 5 lines
drivers/misc/dell-laptop.c new file +5
-0+ ret =rfkill_register (wifi_rfkill); +if (ret) + goto err_wifi; + } +
Regards
Alan
Re: [PATCH 2/2 update] misc: Add dell-laptop driver
(I feel weird replying to "none" :-) )
Quoted 4 lines · none
drivers/misc/dell-laptop.c line 256
Quoted 1 line
drivers/misc/dell-laptop.c new file +1
-0+ wifi_rfkill->get_state = dell_wifi_get;The rfkill doc (and code) say that you also need to initialize
wifi_rfkill->stateto 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.
Signature · 4 lines
"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
Len Brown
reviewerRe: [PATCH 2/2 update] misc: Add dell-laptop driver
In reply to Matthew GarrettI didn't notice a MAINTAINER entry for this new driver.
Signature · 2 lines
-Len Brown Intel Open Source Technology Center
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.
---
Includes a MAINTAINERS entry.
MAINTAINERS +5 -0
@@ -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]
drivers/misc/Kconfig +12 -0
@@ -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
drivers/misc/Makefile +1 -0
@@ -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/
drivers/misc/dell-laptop.c new file +436 -0
436 lines not on this page · raw message
Signature · 1 line
Matthew Garrett | [email protected]
Re: [PATCH 2/2 update] misc: Add dell-laptop driver
Quoted 6 lines · Matthew Garrett
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
Quoted 1 line
+static void
find_tokens(const struct dmi_header *dm)
__init
drivers/misc/dell-laptop.c line 118
Quoted 12 lines
drivers/misc/dell-laptop.c new file +12
-0+{ +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.
Quoted 13 lines
drivers/misc/dell-laptop.c new file +13
-0+/* 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
drivers/misc/dell-laptop.c line 174
Quoted 10 lines
drivers/misc/dell-laptop.c new file +10
-0+ 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.
Review
Quoted 1 line
(quoted diff) +1
-0+ memset(&buffer, 0, sizeof(struct calling_interface_buffer));
sizeof(buffer)
Likewise for other sizeof() occurrences that follow.
Quoted 1 line
+static int
dell_setup_rfkill(void)
__init
Sven
Re: [PATCH 2/2 update] misc: Add dell-laptop driver
In reply to Matthew GarrettI 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)
Re: [PATCH 2/2 update] misc: Add dell-laptop driver
Quoted 4 lines · Alan Jenkins
I still think you need to initialise
wifi_rfkill->statebefore you callrfkill_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.
Signature · 4 lines
"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
Quoted 7 lines · Matthew Garrett
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:
to it.
Do you want this to go through my driver-core tree, or somewhere else?
thanks,
greg k-h
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.
---
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?
drivers/firmware/dcdbas.c +5 -4
@@ -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 ;
drivers/firmware/dcdbas.h +2 -0
@@ -101,5 +101,7 @@ struct apm_cmd { } __attribute__ ((packed)) parameters; } __attribute__ ((packed)); +int dcdbas_smi_request (struct smi_cmd *smi_cmd); +#endif /* _DCDBAS_H_ */
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 7 lines · Matthew Garrett
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
Signature · 3 lines
Matt Domsch Linux Technology Strategist, Dell Office of the CTO linux.dell.com & www.dell.com/linux