Hi Henrik, On Sun, Oct 31, 2010 at 03:50:28AM -0400, Henrik Rydberg wrote: > One main problem with the current driver is the inability to quickly > search for supported keys, resulting in detailed feature maps per > machine model which are cumbersome to maintain. > > This patch adds a register lookup table, which enables binary search > for supported keys. The lookup also reduces the io frequency, so the > original mutex is replaced by locks around the actual io. > > Signed-off-by: Henrik Rydberg <rydberg@xxxxxxxxxxx> > --- > drivers/hwmon/applesmc.c | 515 +++++++++++++++++++++++++--------------------- > 1 files changed, 281 insertions(+), 234 deletions(-) > > diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c > index 375e464..7f030f0 100644 > --- a/drivers/hwmon/applesmc.c > +++ b/drivers/hwmon/applesmc.c > @@ -32,6 +32,7 @@ > #include <linux/platform_device.h> > #include <linux/input-polldev.h> > #include <linux/kernel.h> > +#include <linux/slab.h> > #include <linux/module.h> > #include <linux/timer.h> > #include <linux/dmi.h> > @@ -51,6 +52,7 @@ > > #define APPLESMC_MAX_DATA_LENGTH 32 > > +/* wait up to 32 ms for a status change. */ > #define APPLESMC_MIN_WAIT 0x0040 > #define APPLESMC_MAX_WAIT 0x8000 > > @@ -196,6 +198,22 @@ struct dmi_match_data { > int temperature_set; > }; > > +/* AppleSMC entry - cached register information */ > +struct applesmc_entry { > + char key[5]; /* four-letter key code */ > + u8 valid; /* set when entry is successfully read once */ > + u8 len; /* bounded by APPLESMC_MAX_DATA_LENGTH */ > + char type[5]; /* four-letter type code (padded) */ > +}; > + > +/* Register lookup and registers common to all SMCs */ > +static struct applesmc_registers { > + struct mutex mutex; /* register read/write mutex */ > + unsigned int key_count; /* number of SMC registers */ > + bool init_complete; /* true when fully initialized */ > + struct applesmc_entry *entry; /* key entries */ > +} smcreg; > + > static const int debug; > static struct platform_device *pdev; > static s16 rest_x; > @@ -217,14 +235,13 @@ static unsigned int fans_handled; > /* Indicates which temperature sensors set to use. */ > static unsigned int applesmc_temperature_set; > > -static DEFINE_MUTEX(applesmc_lock); > - > /* > * Last index written to key_at_index sysfs file, and value to use for all other > * key_at_index_* sysfs files. > */ > static unsigned int key_at_index; > > + unnecessary blank line > static struct workqueue_struct *applesmc_led_wq; > > /* > @@ -241,16 +258,10 @@ static int __wait_status(u8 val) > for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) { > udelay(us); > if ((inb(APPLESMC_CMD_PORT) & APPLESMC_STATUS_MASK) == val) { > - if (debug) > - printk(KERN_DEBUG > - "Waited %d us for status %x\n", > - 2 * us - APPLESMC_MIN_WAIT, val); > return 0; > } > } > > - pr_warn("wait status failed: %x != %x\n", val, inb(APPLESMC_CMD_PORT)); > - > return -EIO; > } > > @@ -268,156 +279,228 @@ static int send_command(u8 cmd) > if ((inb(APPLESMC_CMD_PORT) & APPLESMC_STATUS_MASK) == 0x0c) > return 0; > } > - pr_warn("command failed: %x -> %x\n", cmd, inb(APPLESMC_CMD_PORT)); > return -EIO; > } > > -/* > - * applesmc_read_key - reads len bytes from a given key, and put them in buffer. > - * Returns zero on success or a negative error on failure. Callers must > - * hold applesmc_lock. > - */ > -static int applesmc_read_key(const char* key, u8* buffer, u8 len) > +static int send_argument(const char *key) > { > int i; > > - if (len > APPLESMC_MAX_DATA_LENGTH) { > - pr_err("%s(): cannot read more than %d bytes\n", > - __func__, APPLESMC_MAX_DATA_LENGTH); > - return -EINVAL; > - } > - > - if (send_command(APPLESMC_READ_CMD)) > - return -EIO; > - > for (i = 0; i < 4; i++) { > outb(key[i], APPLESMC_DATA_PORT); > if (__wait_status(0x04)) > return -EIO; > } > - if (debug) > - printk(KERN_DEBUG "<%s", key); > + return 0; > +} > + > +static int read_smc(u8 cmd, const char *key, u8 *buffer, u8 len) > +{ > + int i; > + > + if (send_command(cmd) || send_argument(key)) { > + pr_warn("%s: read arg fail\n", key); > + return -EIO; > + } > > outb(len, APPLESMC_DATA_PORT); > - if (debug) > - printk(KERN_DEBUG ">%x", len); > > for (i = 0; i < len; i++) { > - if (__wait_status(0x05)) > + if (__wait_status(0x05)) { > + pr_warn("%s: read data fail\n", key); > return -EIO; > + } > buffer[i] = inb(APPLESMC_DATA_PORT); > - if (debug) > - printk(KERN_DEBUG "<%x", buffer[i]); > } > - if (debug) > - printk(KERN_DEBUG "\n"); > > return 0; > } > > -/* > - * applesmc_write_key - writes len bytes from buffer to a given key. > - * Returns zero on success or a negative error on failure. Callers must > - * hold applesmc_lock. > - */ > -static int applesmc_write_key(const char* key, u8* buffer, u8 len) > +static int write_smc(u8 cmd, const char *key, const u8 *buffer, u8 len) > { > int i; > > - if (len > APPLESMC_MAX_DATA_LENGTH) { > - pr_err("%s(): cannot write more than %d bytes\n", > - __func__, APPLESMC_MAX_DATA_LENGTH); > - return -EINVAL; > - } > - > - if (send_command(APPLESMC_WRITE_CMD)) > + if (send_command(cmd) || send_argument(key)) { > + pr_warn("%s: write arg fail\n", key); > return -EIO; > - > - for (i = 0; i < 4; i++) { > - outb(key[i], APPLESMC_DATA_PORT); > - if (__wait_status(0x04)) > - return -EIO; > } > > outb(len, APPLESMC_DATA_PORT); > > for (i = 0; i < len; i++) { > - if (__wait_status(0x04)) > + if (__wait_status(0x04)) { > + pr_warn("%s: write data fail\n", key); > return -EIO; > + } > outb(buffer[i], APPLESMC_DATA_PORT); > } > > return 0; > } > > +static int read_register_count(unsigned int *count) > +{ > + __be32 be; > + int ret; > + > + ret = read_smc(APPLESMC_READ_CMD, KEY_COUNT_KEY, (u8 *)&be, 4); > + if (ret) > + return ret; > + > + *count = be32_to_cpu(be); > + return 0; > +} > + > /* > - * applesmc_get_key_at_index - get key at index, and put the result in key > - * (char[6]). Returns zero on success or a negative error on failure. Callers > - * must hold applesmc_lock. > + * Serialized I/O > + * > + * Returns zero on success or a negative error on failure. > + * All functions below are concurrency safe - callers should NOT hold lock. > */ > -static int applesmc_get_key_at_index(int index, char* key) > + > +static int applesmc_read_entry(const struct applesmc_entry *entry, > + u8 *buf, u8 len) > { > - int i; > - u8 readkey[4]; > - readkey[0] = index >> 24; > - readkey[1] = index >> 16; > - readkey[2] = index >> 8; > - readkey[3] = index; > + int ret; > > - if (send_command(APPLESMC_GET_KEY_BY_INDEX_CMD)) > - return -EIO; > + if (entry->len != len) > + return -EINVAL; > + mutex_lock(&smcreg.mutex); > + ret = read_smc(APPLESMC_READ_CMD, entry->key, buf, len); > + mutex_unlock(&smcreg.mutex); > > - for (i = 0; i < 4; i++) { > - outb(readkey[i], APPLESMC_DATA_PORT); > - if (__wait_status(0x04)) > - return -EIO; > - } > + return ret; > +} > + > +static int applesmc_write_entry(const struct applesmc_entry *entry, > + const u8 *buf, u8 len) > +{ > + int ret; > > - outb(4, APPLESMC_DATA_PORT); > + if (entry->len != len) > + return -EINVAL; > + mutex_lock(&smcreg.mutex); > + ret = write_smc(APPLESMC_WRITE_CMD, entry->key, buf, len); > + mutex_unlock(&smcreg.mutex); > + return ret; > +} > > - for (i = 0; i < 4; i++) { > - if (__wait_status(0x05)) > - return -EIO; > - key[i] = inb(APPLESMC_DATA_PORT); > +static int applesmc_get_entry_by_index(int index, struct applesmc_entry *entry) > +{ One thing I don't understand about the whole caching scheme - why don't you just return (and use) a pointer to the cached entry ? Seems to me that would be much simpler. If you want to return error types, you could use ERR_PTR, PTR_ERR, and IS_ERR. > + struct applesmc_entry *cache = &smcreg.entry[index]; > + __be32 be; > + int ret; > + > + if (cache->valid) { > + memcpy(entry, cache, sizeof(*entry)); > + return 0; > } > - key[4] = 0; > > - return 0; > + mutex_lock(&smcreg.mutex); > + > + be = cpu_to_be32(index); > + ret = read_smc(APPLESMC_GET_KEY_BY_INDEX_CMD, (u8 *)&be, cache->key, 4); > + if (ret) > + goto out; > + ret = read_smc(APPLESMC_GET_KEY_TYPE_CMD, cache->key, &cache->len, 6); This one is a bit odd. cache->len is an u8. You are reading 6 bytes into it. I assume this is supposed to fill both cache->len and cache->type in a single operation. Not sure if this is a good idea. Seems to depend a lot on the assumption that fields are consecutive. Might be safer to read the data into a temporary 6 byte buffer and copy it into ->len and ->type afterwards. If that is not acceptable, please add a comment describing what you are doing here and why. > + if (ret) > + goto out; > + > + cache->type[4] = 0; Why read 6 bytes above if you overwrite the last byte anyway ? > + cache->valid = 1; > + memcpy(entry, cache, sizeof(*entry)); > + > +out: > + mutex_unlock(&smcreg.mutex); > + return ret; > } > > -/* > - * applesmc_get_key_type - get key type, and put the result in type (char[6]). > - * Returns zero on success or a negative error on failure. Callers must > - * hold applesmc_lock. > - */ > -static int applesmc_get_key_type(char* key, char* type) > +static int applesmc_get_lower_bound(unsigned int *lo, const char *key) > { > - int i; > - > - if (send_command(APPLESMC_GET_KEY_TYPE_CMD)) > - return -EIO; > + int begin = 0, end = smcreg.key_count; > + struct applesmc_entry entry; > + int ret; > > - for (i = 0; i < 4; i++) { > - outb(key[i], APPLESMC_DATA_PORT); > - if (__wait_status(0x04)) > - return -EIO; > + while (begin != end) { > + int middle = begin + (end - begin) / 2; > + ret = applesmc_get_entry_by_index(middle, &entry); > + if (ret) > + return ret; > + if (strcmp(entry.key, key) < 0) > + begin = middle + 1; > + else > + end = middle; > } > > - outb(6, APPLESMC_DATA_PORT); > + *lo = begin; > + return 0; > +} > > - for (i = 0; i < 6; i++) { > - if (__wait_status(0x05)) > - return -EIO; > - type[i] = inb(APPLESMC_DATA_PORT); > +static int applesmc_get_upper_bound(unsigned int *hi, const char *key) > +{ > + int begin = 0, end = smcreg.key_count; > + struct applesmc_entry entry; > + int ret; > + > + while (begin != end) { > + int middle = begin + (end - begin) / 2; > + ret = applesmc_get_entry_by_index(middle, &entry); > + if (ret) > + return ret; > + if (strcmp(key, entry.key) < 0) > + end = middle; > + else > + begin = middle + 1; > } > - type[5] = 0; > > + *hi = begin; > return 0; > } > > +static int applesmc_get_entry_by_key(const char *key, > + struct applesmc_entry *entry) > +{ > + int begin, end; > + int ret; > + > + ret = applesmc_get_lower_bound(&begin, key); > + if (ret) > + return ret; > + ret = applesmc_get_upper_bound(&end, key); > + if (ret) > + return ret; > + if (end - begin != 1) > + return -EINVAL; > + > + return applesmc_get_entry_by_index(begin, entry); > +} > + > +static int applesmc_read_key(const char *key, u8 *buffer, u8 len) > +{ > + struct applesmc_entry entry; > + int ret; > + > + ret = applesmc_get_entry_by_key(key, &entry); > + if (ret) > + return ret; > + > + return applesmc_read_entry(&entry, buffer, len); > +} > + > +static int applesmc_write_key(const char *key, const u8 *buffer, u8 len) > +{ > + struct applesmc_entry entry; > + int ret; > + > + ret = applesmc_get_entry_by_key(key, &entry); > + if (ret) > + return ret; > + > + return applesmc_write_entry(&entry, buffer, len); > +} > + > /* > - * applesmc_read_motion_sensor - Read motion sensor (X, Y or Z). Callers must > - * hold applesmc_lock. > + * applesmc_read_motion_sensor - Read motion sensor (X, Y or Z). > */ > static int applesmc_read_motion_sensor(int index, s16* value) > { > @@ -455,8 +538,6 @@ static int applesmc_device_init(void) > if (!applesmc_accelerometer) > return 0; > > - mutex_lock(&applesmc_lock); > - > for (total = INIT_TIMEOUT_MSECS; total > 0; total -= INIT_WAIT_MSECS) { > if (!applesmc_read_key(MOTION_SENSOR_KEY, buffer, 2) && > (buffer[0] != 0x00 || buffer[1] != 0x00)) { > @@ -472,33 +553,90 @@ static int applesmc_device_init(void) > pr_warn("failed to init the device\n"); > > out: > - mutex_unlock(&applesmc_lock); > return ret; > } > > /* > - * applesmc_get_fan_count - get the number of fans. Callers must NOT hold > - * applesmc_lock. > + * applesmc_get_fan_count - get the number of fans. > */ > static int applesmc_get_fan_count(void) > { > int ret; > u8 buffer[1]; > > - mutex_lock(&applesmc_lock); > - > ret = applesmc_read_key(FANS_COUNT, buffer, 1); > > - mutex_unlock(&applesmc_lock); > if (ret) > return ret; > else > return buffer[0]; > } > > +/* > + * applesmc_init_smcreg_try - Try to initialize register cache. Idempotent. > + */ > +static int applesmc_init_smcreg_try(void) > +{ > + struct applesmc_registers *s = &smcreg; > + int ret; > + > + if (s->init_complete) > + return 0; > + > + mutex_init(&s->mutex); > + > + ret = read_register_count(&s->key_count); > + if (ret) > + return ret; > + > + if (!s->entry) > + s->entry = kcalloc(s->key_count, sizeof(*s->entry), GFP_KERNEL); > + if (!s->entry) > + return -ENOMEM; > + > + s->init_complete = true; > + > + pr_info("key=%d\n", s->key_count); > + > + return 0; > +} > + > +/* > + * applesmc_init_smcreg - Initialize register cache. > + * > + * Retries until initialization is successful, or the operation times out. > + * > + */ > +static int applesmc_init_smcreg(void) > +{ > + int ms, ret; > + > + for (ms = 0; ms < INIT_TIMEOUT_MSECS; ms += INIT_WAIT_MSECS) { > + ret = applesmc_init_smcreg_try(); > + if (!ret) > + return 0; > + pr_warn("slow init, retrying\n"); > + msleep(INIT_WAIT_MSECS); > + } > + > + return ret; > +} > + > +static void applesmc_destroy_smcreg(void) > +{ > + kfree(smcreg.entry); > + smcreg.entry = NULL; Do you also have to reset init_complete ? > +} > + > /* Device model stuff */ > static int applesmc_probe(struct platform_device *dev) > { > + int ret; > + > + ret = applesmc_init_smcreg(); > + if (ret) > + return ret; > + > applesmc_device_init(); > > return 0; > @@ -507,10 +645,8 @@ static int applesmc_probe(struct platform_device *dev) > /* Synchronize device with memorized backlight state */ > static int applesmc_pm_resume(struct device *dev) > { > - mutex_lock(&applesmc_lock); > if (applesmc_light) > applesmc_write_key(BACKLIGHT_KEY, backlight_state, 2); > - mutex_unlock(&applesmc_lock); > return 0; > } > > @@ -551,20 +687,15 @@ static void applesmc_idev_poll(struct input_polled_dev *dev) > struct input_dev *idev = dev->input; > s16 x, y; > > - mutex_lock(&applesmc_lock); > - > if (applesmc_read_motion_sensor(SENSOR_X, &x)) > - goto out; > + return; > if (applesmc_read_motion_sensor(SENSOR_Y, &y)) > - goto out; > + return; > > x = -x; > input_report_abs(idev, ABS_X, x - rest_x); > input_report_abs(idev, ABS_Y, y - rest_y); > input_sync(idev); > - > -out: > - mutex_unlock(&applesmc_lock); > } > > /* Sysfs Files */ > @@ -581,8 +712,6 @@ static ssize_t applesmc_position_show(struct device *dev, > int ret; > s16 x, y, z; > > - mutex_lock(&applesmc_lock); > - > ret = applesmc_read_motion_sensor(SENSOR_X, &x); > if (ret) > goto out; > @@ -594,7 +723,6 @@ static ssize_t applesmc_position_show(struct device *dev, > goto out; > > out: > - mutex_unlock(&applesmc_lock); > if (ret) > return ret; > else > @@ -604,18 +732,17 @@ out: > static ssize_t applesmc_light_show(struct device *dev, > struct device_attribute *attr, char *sysfsbuf) > { > + struct applesmc_entry entry; > static int data_length; > int ret; > u8 left = 0, right = 0; > - u8 buffer[10], query[6]; > - > - mutex_lock(&applesmc_lock); > + u8 buffer[10]; > > if (!data_length) { > - ret = applesmc_get_key_type(LIGHT_SENSOR_LEFT_KEY, query); > + ret = applesmc_get_entry_by_key(LIGHT_SENSOR_LEFT_KEY, &entry); > if (ret) > goto out; > - data_length = clamp_val(query[0], 0, 10); > + data_length = clamp_val(entry.len, 0, 10); > pr_info("light sensor data length set to %d\n", data_length); > } > > @@ -632,7 +759,6 @@ static ssize_t applesmc_light_show(struct device *dev, > right = buffer[2]; > > out: > - mutex_unlock(&applesmc_lock); > if (ret) > return ret; > else > @@ -661,14 +787,10 @@ static ssize_t applesmc_show_temperature(struct device *dev, > const char* key = > temperature_sensors_sets[applesmc_temperature_set][attr->index]; > > - mutex_lock(&applesmc_lock); > - > ret = applesmc_read_key(key, buffer, 2); > temp = buffer[0]*1000; > temp += (buffer[1] >> 6) * 250; > > - mutex_unlock(&applesmc_lock); > - > if (ret) > return ret; > else > @@ -691,12 +813,9 @@ static ssize_t applesmc_show_fan_speed(struct device *dev, > newkey[3] = fan_speed_keys[sensor_attr->nr][3]; > newkey[4] = 0; > > - mutex_lock(&applesmc_lock); > - > ret = applesmc_read_key(newkey, buffer, 2); > speed = ((buffer[0] << 8 | buffer[1]) >> 2); > > - mutex_unlock(&applesmc_lock); > if (ret) > return ret; > else > @@ -725,13 +844,10 @@ static ssize_t applesmc_store_fan_speed(struct device *dev, > newkey[3] = fan_speed_keys[sensor_attr->nr][3]; > newkey[4] = 0; > > - mutex_lock(&applesmc_lock); > - > buffer[0] = (speed >> 6) & 0xff; > buffer[1] = (speed << 2) & 0xff; > ret = applesmc_write_key(newkey, buffer, 2); > > - mutex_unlock(&applesmc_lock); > if (ret) > return ret; > else > @@ -746,12 +862,9 @@ static ssize_t applesmc_show_fan_manual(struct device *dev, > u8 buffer[2]; > struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > > - mutex_lock(&applesmc_lock); > - > ret = applesmc_read_key(FANS_MANUAL, buffer, 2); > manual = ((buffer[0] << 8 | buffer[1]) >> attr->index) & 0x01; > > - mutex_unlock(&applesmc_lock); > if (ret) > return ret; > else > @@ -770,8 +883,6 @@ static ssize_t applesmc_store_fan_manual(struct device *dev, > > input = simple_strtoul(sysfsbuf, NULL, 10); > > - mutex_lock(&applesmc_lock); > - > ret = applesmc_read_key(FANS_MANUAL, buffer, 2); > val = (buffer[0] << 8 | buffer[1]); > if (ret) > @@ -788,7 +899,6 @@ static ssize_t applesmc_store_fan_manual(struct device *dev, > ret = applesmc_write_key(FANS_MANUAL, buffer, 2); > > out: > - mutex_unlock(&applesmc_lock); > if (ret) > return ret; > else > @@ -810,12 +920,9 @@ static ssize_t applesmc_show_fan_position(struct device *dev, > newkey[3] = FAN_POSITION[3]; > newkey[4] = 0; > > - mutex_lock(&applesmc_lock); > - > ret = applesmc_read_key(newkey, buffer, 16); > buffer[16] = 0; > > - mutex_unlock(&applesmc_lock); > if (ret) > return ret; > else > @@ -831,18 +938,14 @@ static ssize_t applesmc_calibrate_show(struct device *dev, > static ssize_t applesmc_calibrate_store(struct device *dev, > struct device_attribute *attr, const char *sysfsbuf, size_t count) > { > - mutex_lock(&applesmc_lock); > applesmc_calibrate(); > - mutex_unlock(&applesmc_lock); > > return count; > } > > static void applesmc_backlight_set(struct work_struct *work) > { > - mutex_lock(&applesmc_lock); > applesmc_write_key(BACKLIGHT_KEY, backlight_state, 2); > - mutex_unlock(&applesmc_lock); > } > static DECLARE_WORK(backlight_work, &applesmc_backlight_set); > > @@ -865,13 +968,10 @@ static ssize_t applesmc_key_count_show(struct device *dev, > u8 buffer[4]; > u32 count; > > - mutex_lock(&applesmc_lock); > - > ret = applesmc_read_key(KEY_COUNT_KEY, buffer, 4); > count = ((u32)buffer[0]<<24) + ((u32)buffer[1]<<16) + > ((u32)buffer[2]<<8) + buffer[3]; > > - mutex_unlock(&applesmc_lock); > if (ret) > return ret; > else > @@ -881,113 +981,56 @@ static ssize_t applesmc_key_count_show(struct device *dev, > static ssize_t applesmc_key_at_index_read_show(struct device *dev, > struct device_attribute *attr, char *sysfsbuf) > { > - char key[5]; > - char info[6]; > + struct applesmc_entry entry; > int ret; > > - mutex_lock(&applesmc_lock); > - > - ret = applesmc_get_key_at_index(key_at_index, key); > - > - if (ret || !key[0]) { > - mutex_unlock(&applesmc_lock); > - > - return -EINVAL; > - } > - > - ret = applesmc_get_key_type(key, info); > - > - if (ret) { > - mutex_unlock(&applesmc_lock); > - > + ret = applesmc_get_entry_by_index(key_at_index, &entry); > + if (ret) > return ret; > - } > - > - /* > - * info[0] maximum value (APPLESMC_MAX_DATA_LENGTH) is much lower than > - * PAGE_SIZE, so we don't need any checks before writing to sysfsbuf. > - */ > - ret = applesmc_read_key(key, sysfsbuf, info[0]); > - > - mutex_unlock(&applesmc_lock); > - > - if (!ret) { > - return info[0]; > - } else { > + ret = applesmc_read_entry(&entry, sysfsbuf, entry.len); > + if (ret) > return ret; > - } > + > + return entry.len; > } > > static ssize_t applesmc_key_at_index_data_length_show(struct device *dev, > struct device_attribute *attr, char *sysfsbuf) > { > - char key[5]; > - char info[6]; > + struct applesmc_entry entry; > int ret; > > - mutex_lock(&applesmc_lock); > - > - ret = applesmc_get_key_at_index(key_at_index, key); > - > - if (ret || !key[0]) { > - mutex_unlock(&applesmc_lock); > - > - return -EINVAL; > - } > - > - ret = applesmc_get_key_type(key, info); > - > - mutex_unlock(&applesmc_lock); > - > - if (!ret) > - return snprintf(sysfsbuf, PAGE_SIZE, "%d\n", info[0]); > - else > + ret = applesmc_get_entry_by_index(key_at_index, &entry); > + if (ret) > return ret; > + > + return snprintf(sysfsbuf, PAGE_SIZE, "%d\n", entry.len); > } > > static ssize_t applesmc_key_at_index_type_show(struct device *dev, > struct device_attribute *attr, char *sysfsbuf) > { > - char key[5]; > - char info[6]; > + struct applesmc_entry entry; > int ret; > > - mutex_lock(&applesmc_lock); > - > - ret = applesmc_get_key_at_index(key_at_index, key); > - > - if (ret || !key[0]) { > - mutex_unlock(&applesmc_lock); > - > - return -EINVAL; > - } > - > - ret = applesmc_get_key_type(key, info); > - > - mutex_unlock(&applesmc_lock); > - > - if (!ret) > - return snprintf(sysfsbuf, PAGE_SIZE, "%s\n", info+1); > - else > + ret = applesmc_get_entry_by_index(key_at_index, &entry); > + if (ret) > return ret; > + > + return snprintf(sysfsbuf, PAGE_SIZE, "%s\n", entry.type); > } > > static ssize_t applesmc_key_at_index_name_show(struct device *dev, > struct device_attribute *attr, char *sysfsbuf) > { > - char key[5]; > + struct applesmc_entry entry; > int ret; > > - mutex_lock(&applesmc_lock); > - > - ret = applesmc_get_key_at_index(key_at_index, key); > - > - mutex_unlock(&applesmc_lock); > + ret = applesmc_get_entry_by_index(key_at_index, &entry); > + if (ret) > + return ret; > > - if (!ret && key[0]) > - return snprintf(sysfsbuf, PAGE_SIZE, "%s\n", key); > - else > - return -EINVAL; > + return snprintf(sysfsbuf, PAGE_SIZE, "%s\n", entry.key); > } > > static ssize_t applesmc_key_at_index_show(struct device *dev, > @@ -999,12 +1042,8 @@ static ssize_t applesmc_key_at_index_show(struct device *dev, > static ssize_t applesmc_key_at_index_store(struct device *dev, > struct device_attribute *attr, const char *sysfsbuf, size_t count) > { > - mutex_lock(&applesmc_lock); > - > key_at_index = simple_strtoul(sysfsbuf, NULL, 10); > > - mutex_unlock(&applesmc_lock); > - > return count; > } > > @@ -1640,10 +1679,15 @@ static int __init applesmc_init(void) > goto out_driver; > } > > - ret = sysfs_create_file(&pdev->dev.kobj, &dev_attr_name.attr); > + /* create register cache */ > + ret = applesmc_init_smcreg(); > if (ret) > goto out_device; > > + ret = sysfs_create_file(&pdev->dev.kobj, &dev_attr_name.attr); > + if (ret) > + goto out_smcreg; > + > /* Create key enumeration sysfs files */ > ret = sysfs_create_group(&pdev->dev.kobj, &key_enumeration_group); > if (ret) > @@ -1745,6 +1789,8 @@ out_fans: > sysfs_remove_group(&pdev->dev.kobj, &key_enumeration_group); > out_name: > sysfs_remove_file(&pdev->dev.kobj, &dev_attr_name.attr); > +out_smcreg: > + applesmc_destroy_smcreg(); > out_device: > platform_device_unregister(pdev); > out_driver: > @@ -1773,6 +1819,7 @@ static void __exit applesmc_exit(void) > &fan_attribute_groups[--fans_handled]); > sysfs_remove_group(&pdev->dev.kobj, &key_enumeration_group); > sysfs_remove_file(&pdev->dev.kobj, &dev_attr_name.attr); > + applesmc_destroy_smcreg(); > platform_device_unregister(pdev); > platform_driver_unregister(&applesmc_driver); > release_region(APPLESMC_DATA_PORT, APPLESMC_NR_PORTS); > -- > 1.7.1 > > > _______________________________________________ > lm-sensors mailing list > lm-sensors@xxxxxxxxxxxxxx > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors