Hi Alan: Alan Jenkins wrote: > > Ok. I wasn't sure that userspace was allowed to rely on that, but I > re-read Documentation/ABI/testing/sysfs-devices and I see now that it > can be allowed. > > Some of these issues with the compal-laptop work may come from copying > dell-laptop. dell-laptop is the only driver which creates rfkill > devices without a parent device. In fact, it doesn't even create a > platform device :-). > > I'll try sending a patch or two to clean up dell-laptop. I don't have > the hardware but it should be pretty mechanical, almost a copy+paste > job from a "good" driver. > > Yeah, a lot of the source for the design issues here were from copying bits out of dell-laptop. I've made adjustments to this third patch to only drop the sysfs attributes but still create the platform device. -- Mario Limonciello *Dell | Linux Engineering* mario_limonciello@xxxxxxxx
--- a/drivers/platform/x86/compal-laptop.c.old 2009-08-18 01:39:12.864668371 -0500 +++ b/drivers/platform/x86/compal-laptop.c 2009-08-18 01:42:41.715047544 -0500 @@ -26,17 +26,8 @@ /* * comapl-laptop.c - Compal laptop support. * - * This driver exports a few files in /sys/devices/platform/compal-laptop/: - * - * wlan - wlan subsystem state: contains 0 or 1 (rw) - * - * bluetooth - Bluetooth subsystem state: contains 0 or 1 (rw) - * - * raw - raw value taken from embedded controller register (ro) - * - * In addition to these platform device attributes the driver - * registers itself in the Linux backlight control subsystem and is - * available to userspace under /sys/class/backlight/compal-laptop/. + * The driver registers itself with the rfkill subsystem and + * the Linux backlight control subsystem. * * This driver might work on other laptops produced by Compal. If you * want to try it you can pass force=1 as argument to the module which @@ -174,67 +165,6 @@ return ret; } -static int set_wlan_state(int state) -{ - u8 result, value; - - ec_read(COMPAL_EC_COMMAND_WIRELESS, &result); - - if ((result & KILLSWITCH_MASK) == 0) - return -EINVAL; - else { - if (state) - value = (u8) (result | WLAN_MASK); - else - value = (u8) (result & ~WLAN_MASK); - ec_write(COMPAL_EC_COMMAND_WIRELESS, value); - } - - return 0; -} - -static int set_bluetooth_state(int state) -{ - u8 result, value; - - ec_read(COMPAL_EC_COMMAND_WIRELESS, &result); - - if ((result & KILLSWITCH_MASK) == 0) - return -EINVAL; - else { - if (state) - value = (u8) (result | BT_MASK); - else - value = (u8) (result & ~BT_MASK); - ec_write(COMPAL_EC_COMMAND_WIRELESS, value); - } - - return 0; -} - -static int get_wireless_state(int *wlan, int *bluetooth) -{ - u8 result; - - ec_read(COMPAL_EC_COMMAND_WIRELESS, &result); - - if (wlan) { - if ((result & KILLSWITCH_MASK) == 0) - *wlan = 0; - else - *wlan = result & WLAN_MASK; - } - - if (bluetooth) { - if ((result & KILLSWITCH_MASK) == 0) - *bluetooth = 0; - else - *bluetooth = (result & BT_MASK) >> 1; - } - - return 0; -} - /* Backlight device stuff */ static int bl_get_brightness(struct backlight_device *b) @@ -255,86 +185,6 @@ static struct backlight_device *compalbl_device; -/* Platform device */ - -static ssize_t show_wlan(struct device *dev, - struct device_attribute *attr, char *buf) -{ - int ret, enabled; - - ret = get_wireless_state(&enabled, NULL); - if (ret < 0) - return ret; - - return sprintf(buf, "%i\n", enabled); -} - -static ssize_t show_raw(struct device *dev, - struct device_attribute *attr, char *buf) -{ - u8 result; - - ec_read(COMPAL_EC_COMMAND_WIRELESS, &result); - - return sprintf(buf, "%i\n", result); -} - -static ssize_t show_bluetooth(struct device *dev, - struct device_attribute *attr, char *buf) -{ - int ret, enabled; - - ret = get_wireless_state(NULL, &enabled); - if (ret < 0) - return ret; - - return sprintf(buf, "%i\n", enabled); -} - -static ssize_t store_wlan_state(struct device *dev, - struct device_attribute *attr, const char *buf, size_t count) -{ - int state, ret; - - if (sscanf(buf, "%i", &state) != 1 || (state < 0 || state > 1)) - return -EINVAL; - - ret = set_wlan_state(state); - if (ret < 0) - return ret; - - return count; -} - -static ssize_t store_bluetooth_state(struct device *dev, - struct device_attribute *attr, const char *buf, size_t count) -{ - int state, ret; - - if (sscanf(buf, "%i", &state) != 1 || (state < 0 || state > 1)) - return -EINVAL; - - ret = set_bluetooth_state(state); - if (ret < 0) - return ret; - - return count; -} - -static DEVICE_ATTR(bluetooth, 0644, show_bluetooth, store_bluetooth_state); -static DEVICE_ATTR(wlan, 0644, show_wlan, store_wlan_state); -static DEVICE_ATTR(raw, 0444, show_raw, NULL); - -static struct attribute *compal_attributes[] = { - &dev_attr_bluetooth.attr, - &dev_attr_wlan.attr, - &dev_attr_raw.attr, - NULL -}; - -static struct attribute_group compal_attribute_group = { - .attrs = compal_attributes -}; static struct platform_driver compal_driver = { .driver = { @@ -475,11 +325,6 @@ if (ret) goto fail_platform_device1; - ret = sysfs_create_group(&compal_device->dev.kobj, - &compal_attribute_group); - if (ret) - goto fail_platform_device2; - ret = setup_rfkill(); if (ret) printk(KERN_WARNING "compal-laptop: Unable to setup rfkill\n"); @@ -489,10 +334,6 @@ return 0; -fail_platform_device2: - - platform_device_del(compal_device); - fail_platform_device1: platform_device_put(compal_device); @@ -511,7 +352,6 @@ static void __exit compal_cleanup(void) { - sysfs_remove_group(&compal_device->dev.kobj, &compal_attribute_group); platform_device_unregister(compal_device); platform_driver_unregister(&compal_driver); backlight_device_unregister(compalbl_device);
Attachment:
signature.asc
Description: OpenPGP digital signature