* [PATCH 01/11] hwmon: applesmc: Add MacBookAir3,1(3,2) support
2010-11-09 15:15 [PATCH 00/11] hwmon: applesmc: Dynamic configuration rewrite (rev2) Henrik Rydberg
@ 2010-11-09 15:15 ` Henrik Rydberg
2010-11-09 18:56 ` Guenter Roeck
2010-11-09 15:15 ` [PATCH 02/11] hwmon: applesmc: Relax the severity of device init failure (rev2) Henrik Rydberg
` (9 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-09 15:15 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: lm-sensors, linux-kernel, Edgar Hucek, stable, Henrik Rydberg
From: Edgar Hucek <gimli@dark-green.com>
This patch add support for the MacBookAir3,1 and MacBookAir3,2 to the
applesmc driver.
[rydberg@euromail.se: minor cleanup]
Cc: stable@kernel.org
Signed-off-by: Edgar Hucek <gimli@dark-green.com>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hwmon/applesmc.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index b6598aa..d616174 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -162,6 +162,10 @@ static const char *temperature_sensors_sets[][41] = {
/* Set 22: MacBook Pro 7,1 */
{ "TB0T", "TB1T", "TB2T", "TC0D", "TC0P", "TN0D", "TN0P", "TN0S",
"TN1D", "TN1F", "TN1G", "TN1S", "Th1H", "Ts0P", "Ts0S", NULL },
+/* Set 23: MacBook Air 3,1 */
+ { "TB0T", "TB1T", "TB2T", "TC0D", "TC0E", "TC0P", "TC1E", "TCZ3",
+ "TCZ4", "TCZ5", "TG0E", "TG1E", "TG2E", "TGZ3", "TGZ4", "TGZ5",
+ "TH0F", "TH0O", "TM0P" },
};
/* List of keys used to read/write fan speeds */
@@ -1524,11 +1528,17 @@ static __initdata struct dmi_match_data applesmc_dmi_data[] = {
{ .accelerometer = 1, .light = 1, .temperature_set = 21 },
/* MacBook Pro 7,1: accelerometer, backlight and temperature set 22 */
{ .accelerometer = 1, .light = 1, .temperature_set = 22 },
+/* MacBook Air 3,1: accelerometer, backlight and temperature set 23 */
+ { .accelerometer = 0, .light = 0, .temperature_set = 23 },
};
/* Note that DMI_MATCH(...,"MacBook") will match "MacBookPro1,1".
* So we need to put "Apple MacBook Pro" before "Apple MacBook". */
static __initdata struct dmi_system_id applesmc_whitelist[] = {
+ { applesmc_dmi_match, "Apple MacBook Air 3", {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookAir3") },
+ &applesmc_dmi_data[23]},
{ applesmc_dmi_match, "Apple MacBook Air 2", {
DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
DMI_MATCH(DMI_PRODUCT_NAME, "MacBookAir2") },
--
1.7.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 01/11] hwmon: applesmc: Add MacBookAir3,1(3,2) support
2010-11-09 15:15 ` [PATCH 01/11] hwmon: applesmc: Add MacBookAir3,1(3,2) support Henrik Rydberg
@ 2010-11-09 18:56 ` Guenter Roeck
0 siblings, 0 replies; 29+ messages in thread
From: Guenter Roeck @ 2010-11-09 18:56 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Jean Delvare, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org, Edgar Hucek, stable@kernel.org
On Tue, 2010-11-09 at 10:15 -0500, Henrik Rydberg wrote:
> From: Edgar Hucek <gimli@dark-green.com>
>
> This patch add support for the MacBookAir3,1 and MacBookAir3,2 to the
> applesmc driver.
>
> [rydberg@euromail.se: minor cleanup]
> Cc: stable@kernel.org
> Signed-off-by: Edgar Hucek <gimli@dark-green.com>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
Applied.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 02/11] hwmon: applesmc: Relax the severity of device init failure (rev2)
2010-11-09 15:15 [PATCH 00/11] hwmon: applesmc: Dynamic configuration rewrite (rev2) Henrik Rydberg
2010-11-09 15:15 ` [PATCH 01/11] hwmon: applesmc: Add MacBookAir3,1(3,2) support Henrik Rydberg
@ 2010-11-09 15:15 ` Henrik Rydberg
2010-11-09 18:57 ` Guenter Roeck
2010-11-09 15:15 ` [PATCH 03/11] drivers/hwmon/applesmc.c: Use pr_fmt and pr_<level> Henrik Rydberg
` (8 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-09 15:15 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: lm-sensors, linux-kernel, Henrik Rydberg, stable
The device init is used to reset the accelerometer. Failure to reset
is not severe enough to stop loading the module or to resume from
hibernation. This patch relaxes failure to a warning and drops
output in case of success.
Cc: stable@kernel.org
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hwmon/applesmc.c | 38 +++++++-------------------------------
1 files changed, 7 insertions(+), 31 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index d616174..87a5fd5 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -448,38 +448,22 @@ static int applesmc_read_motion_sensor(int index, s16* value)
}
/*
- * applesmc_device_init - initialize the accelerometer. Returns zero on success
- * and negative error code on failure. Can sleep.
+ * applesmc_device_init - initialize the accelerometer. Can sleep.
*/
-static int applesmc_device_init(void)
+static void applesmc_device_init(void)
{
- int total, ret = -ENXIO;
+ int total;
u8 buffer[2];
if (!applesmc_accelerometer)
- return 0;
+ return;
mutex_lock(&applesmc_lock);
for (total = INIT_TIMEOUT_MSECS; total > 0; total -= INIT_WAIT_MSECS) {
- if (debug)
- printk(KERN_DEBUG "applesmc try %d\n", total);
if (!applesmc_read_key(MOTION_SENSOR_KEY, buffer, 2) &&
- (buffer[0] != 0x00 || buffer[1] != 0x00)) {
- if (total == INIT_TIMEOUT_MSECS) {
- printk(KERN_DEBUG "applesmc: device has"
- " already been initialized"
- " (0x%02x, 0x%02x).\n",
- buffer[0], buffer[1]);
- } else {
- printk(KERN_DEBUG "applesmc: device"
- " successfully initialized"
- " (0x%02x, 0x%02x).\n",
- buffer[0], buffer[1]);
- }
- ret = 0;
+ (buffer[0] != 0x00 || buffer[1] != 0x00))
goto out;
- }
buffer[0] = 0xe0;
buffer[1] = 0x00;
applesmc_write_key(MOTION_SENSOR_KEY, buffer, 2);
@@ -490,7 +474,6 @@ static int applesmc_device_init(void)
out:
mutex_unlock(&applesmc_lock);
- return ret;
}
/*
@@ -516,13 +499,8 @@ static int applesmc_get_fan_count(void)
/* Device model stuff */
static int applesmc_probe(struct platform_device *dev)
{
- int ret;
+ applesmc_device_init();
- ret = applesmc_device_init();
- if (ret)
- return ret;
-
- printk(KERN_INFO "applesmc: device successfully initialized.\n");
return 0;
}
@@ -539,9 +517,7 @@ static int applesmc_pm_resume(struct device *dev)
/* Reinitialize device on resume from hibernation */
static int applesmc_pm_restore(struct device *dev)
{
- int ret = applesmc_device_init();
- if (ret)
- return ret;
+ applesmc_device_init();
return applesmc_pm_resume(dev);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 03/11] drivers/hwmon/applesmc.c: Use pr_fmt and pr_<level>
2010-11-09 15:15 [PATCH 00/11] hwmon: applesmc: Dynamic configuration rewrite (rev2) Henrik Rydberg
2010-11-09 15:15 ` [PATCH 01/11] hwmon: applesmc: Add MacBookAir3,1(3,2) support Henrik Rydberg
2010-11-09 15:15 ` [PATCH 02/11] hwmon: applesmc: Relax the severity of device init failure (rev2) Henrik Rydberg
@ 2010-11-09 15:15 ` Henrik Rydberg
2010-11-09 18:57 ` Guenter Roeck
2010-11-09 15:15 ` [PATCH 04/11] hwmon: applesmc: Introduce a register lookup table (rev2) Henrik Rydberg
` (7 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-09 15:15 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: lm-sensors, linux-kernel, Joe Perches, Henrik Rydberg
From: Joe Perches <joe@perches.com>
Added #define pr_fmt KBUILD_MODNAME ": " fmt
Converted printks to pr_<level>
Coalesced any long formats
Removed prefixes from formats
Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hwmon/applesmc.c | 58 ++++++++++++++++++++-------------------------
1 files changed, 26 insertions(+), 32 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index 87a5fd5..5f67e39 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -26,6 +26,8 @@
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/delay.h>
#include <linux/platform_device.h>
#include <linux/input-polldev.h>
@@ -251,8 +253,7 @@ static int __wait_status(u8 val)
}
}
- printk(KERN_WARNING "applesmc: wait status failed: %x != %x\n",
- val, inb(APPLESMC_CMD_PORT));
+ pr_warn("wait status failed: %x != %x\n", val, inb(APPLESMC_CMD_PORT));
return -EIO;
}
@@ -271,8 +272,7 @@ static int send_command(u8 cmd)
if ((inb(APPLESMC_CMD_PORT) & APPLESMC_STATUS_MASK) == 0x0c)
return 0;
}
- printk(KERN_WARNING "applesmc: command failed: %x -> %x\n",
- cmd, inb(APPLESMC_CMD_PORT));
+ pr_warn("command failed: %x -> %x\n", cmd, inb(APPLESMC_CMD_PORT));
return -EIO;
}
@@ -286,8 +286,8 @@ static int applesmc_read_key(const char* key, u8* buffer, u8 len)
int i;
if (len > APPLESMC_MAX_DATA_LENGTH) {
- printk(KERN_ERR "applesmc_read_key: cannot read more than "
- "%d bytes\n", APPLESMC_MAX_DATA_LENGTH);
+ pr_err("%s(): cannot read more than %d bytes\n",
+ __func__, APPLESMC_MAX_DATA_LENGTH);
return -EINVAL;
}
@@ -329,8 +329,8 @@ static int applesmc_write_key(const char* key, u8* buffer, u8 len)
int i;
if (len > APPLESMC_MAX_DATA_LENGTH) {
- printk(KERN_ERR "applesmc_write_key: cannot write more than "
- "%d bytes\n", APPLESMC_MAX_DATA_LENGTH);
+ pr_err("%s(): cannot write more than %d bytes\n",
+ __func__, APPLESMC_MAX_DATA_LENGTH);
return -EINVAL;
}
@@ -470,7 +470,7 @@ static void applesmc_device_init(void)
msleep(INIT_WAIT_MSECS);
}
- printk(KERN_WARNING "applesmc: failed to init the device\n");
+ pr_warn("failed to init the device\n");
out:
mutex_unlock(&applesmc_lock);
@@ -616,8 +616,7 @@ static ssize_t applesmc_light_show(struct device *dev,
if (ret)
goto out;
data_length = clamp_val(query[0], 0, 10);
- printk(KERN_INFO "applesmc: light sensor data length set to "
- "%d\n", data_length);
+ pr_info("light sensor data length set to %d\n", data_length);
}
ret = applesmc_read_key(LIGHT_SENSOR_LEFT_KEY, buffer, data_length);
@@ -1383,18 +1382,18 @@ static int applesmc_dmi_match(const struct dmi_system_id *id)
{
int i = 0;
struct dmi_match_data* dmi_data = id->driver_data;
- printk(KERN_INFO "applesmc: %s detected:\n", id->ident);
+ pr_info("%s detected:\n", id->ident);
applesmc_accelerometer = dmi_data->accelerometer;
- printk(KERN_INFO "applesmc: - Model %s accelerometer\n",
- applesmc_accelerometer ? "with" : "without");
+ pr_info(" - Model %s accelerometer\n",
+ applesmc_accelerometer ? "with" : "without");
applesmc_light = dmi_data->light;
- printk(KERN_INFO "applesmc: - Model %s light sensors and backlight\n",
- applesmc_light ? "with" : "without");
+ pr_info(" - Model %s light sensors and backlight\n",
+ applesmc_light ? "with" : "without");
applesmc_temperature_set = dmi_data->temperature_set;
while (temperature_sensors_sets[applesmc_temperature_set][i] != NULL)
i++;
- printk(KERN_INFO "applesmc: - Model with %d temperature sensors\n", i);
+ pr_info(" - Model with %d temperature sensors\n", i);
return 1;
}
@@ -1445,7 +1444,7 @@ out_sysfs:
sysfs_remove_group(&pdev->dev.kobj, &accelerometer_attributes_group);
out:
- printk(KERN_WARNING "applesmc: driver init failed (ret=%d)!\n", ret);
+ pr_warn("driver init failed (ret=%d)!\n", ret);
return ret;
}
@@ -1625,7 +1624,7 @@ static int __init applesmc_init(void)
int i;
if (!dmi_check_system(applesmc_whitelist)) {
- printk(KERN_WARNING "applesmc: supported laptop not found!\n");
+ pr_warn("supported laptop not found!\n");
ret = -ENODEV;
goto out;
}
@@ -1659,15 +1658,13 @@ static int __init applesmc_init(void)
/* create fan files */
count = applesmc_get_fan_count();
if (count < 0)
- printk(KERN_ERR "applesmc: Cannot get the number of fans.\n");
+ pr_err("Cannot get the number of fans\n");
else
- printk(KERN_INFO "applesmc: %d fans found.\n", count);
+ pr_info("%d fans found\n", count);
if (count > 4) {
count = 4;
- printk(KERN_WARNING "applesmc: More than 4 fans found,"
- " but at most 4 fans are supported"
- " by the driver.\n");
+ pr_warn("A maximum of 4 fans are supported by this driver\n");
}
while (fans_handled < count) {
@@ -1683,11 +1680,8 @@ static int __init applesmc_init(void)
i++) {
if (temperature_attributes[i] == NULL ||
label_attributes[i] == NULL) {
- printk(KERN_ERR "applesmc: More temperature sensors "
- "in temperature_sensors_sets (at least %i)"
- "than available sysfs files in "
- "temperature_attributes (%i), please report "
- "this bug.\n", i, i-1);
+ pr_err("More temperature sensors in temperature_sensors_sets (at least %i) than available sysfs files in temperature_attributes (%i), please report this bug\n",
+ i, i-1);
goto out_temperature;
}
ret = sysfs_create_file(&pdev->dev.kobj,
@@ -1731,7 +1725,7 @@ static int __init applesmc_init(void)
goto out_light_ledclass;
}
- printk(KERN_INFO "applesmc: driver successfully loaded.\n");
+ pr_info("driver successfully loaded\n");
return 0;
@@ -1764,7 +1758,7 @@ out_driver:
out_region:
release_region(APPLESMC_DATA_PORT, APPLESMC_NR_PORTS);
out:
- printk(KERN_WARNING "applesmc: driver init failed (ret=%d)!\n", ret);
+ pr_warn("driver init failed (ret=%d)!\n", ret);
return ret;
}
@@ -1789,7 +1783,7 @@ static void __exit applesmc_exit(void)
platform_driver_unregister(&applesmc_driver);
release_region(APPLESMC_DATA_PORT, APPLESMC_NR_PORTS);
- printk(KERN_INFO "applesmc: driver unloaded.\n");
+ pr_info("driver unloaded\n");
}
module_init(applesmc_init);
--
1.7.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 04/11] hwmon: applesmc: Introduce a register lookup table (rev2)
2010-11-09 15:15 [PATCH 00/11] hwmon: applesmc: Dynamic configuration rewrite (rev2) Henrik Rydberg
` (2 preceding siblings ...)
2010-11-09 15:15 ` [PATCH 03/11] drivers/hwmon/applesmc.c: Use pr_fmt and pr_<level> Henrik Rydberg
@ 2010-11-09 15:15 ` Henrik Rydberg
2010-11-09 19:04 ` Guenter Roeck
2010-11-09 15:15 ` [PATCH 05/11] hwmon: applesmc: Dynamic creation of temperature files (rev2) Henrik Rydberg
` (6 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-09 15:15 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: lm-sensors, linux-kernel, Henrik Rydberg
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@euromail.se>
---
drivers/hwmon/applesmc.c | 528 +++++++++++++++++++++++++---------------------
1 files changed, 286 insertions(+), 242 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index 5f67e39..623f71c 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
@@ -200,6 +202,23 @@ 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 */
+ u8 flags; /* 0x10: func; 0x40: write; 0x80: read */
+};
+
+/* 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 *cache; /* cached key entries */
+} smcreg;
+
static const int debug;
static struct platform_device *pdev;
static s16 rest_x;
@@ -221,8 +240,6 @@ 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.
@@ -245,16 +262,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;
}
@@ -272,156 +283,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;
+
+ 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;
+}
+
+static const struct applesmc_entry *applesmc_get_entry_by_index(int index)
+{
+ struct applesmc_entry *cache = &smcreg.cache[index];
+ u8 key[4], info[6];
+ __be32 be;
+ int ret = 0;
+
+ if (cache->valid)
+ return cache;
+
+ mutex_lock(&smcreg.mutex);
+
+ if (cache->valid)
+ goto out;
+ be = cpu_to_be32(index);
+ ret = read_smc(APPLESMC_GET_KEY_BY_INDEX_CMD, (u8 *)&be, key, 4);
+ if (ret)
+ goto out;
+ ret = read_smc(APPLESMC_GET_KEY_TYPE_CMD, key, info, 6);
+ if (ret)
+ goto out;
+
+ memcpy(cache->key, key, 4);
+ cache->len = info[0];
+ memcpy(cache->type, &info[1], 4);
+ cache->flags = info[5];
+ cache->valid = 1;
+
+out:
+ mutex_unlock(&smcreg.mutex);
+ if (ret)
+ return ERR_PTR(ret);
+ return cache;
+}
+
+static int applesmc_get_lower_bound(unsigned int *lo, const char *key)
+{
+ int begin = 0, end = smcreg.key_count;
+ const struct applesmc_entry *entry;
+
+ while (begin != end) {
+ int middle = begin + (end - begin) / 2;
+ entry = applesmc_get_entry_by_index(middle);
+ if (IS_ERR(entry))
+ return PTR_ERR(entry);
+ if (strcmp(entry->key, key) < 0)
+ begin = middle + 1;
+ else
+ end = middle;
}
- outb(4, APPLESMC_DATA_PORT);
+ *lo = begin;
+ return 0;
+}
- for (i = 0; i < 4; i++) {
- if (__wait_status(0x05))
- return -EIO;
- key[i] = inb(APPLESMC_DATA_PORT);
+static int applesmc_get_upper_bound(unsigned int *hi, const char *key)
+{
+ int begin = 0, end = smcreg.key_count;
+ const struct applesmc_entry *entry;
+
+ while (begin != end) {
+ int middle = begin + (end - begin) / 2;
+ entry = applesmc_get_entry_by_index(middle);
+ if (IS_ERR(entry))
+ return PTR_ERR(entry);
+ if (strcmp(key, entry->key) < 0)
+ end = middle;
+ else
+ begin = middle + 1;
}
- key[4] = 0;
+ *hi = begin;
return 0;
}
-/*
- * 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 const struct applesmc_entry *applesmc_get_entry_by_key(const char *key)
{
- int i;
+ int begin, end;
+ int ret;
- if (send_command(APPLESMC_GET_KEY_TYPE_CMD))
- return -EIO;
+ ret = applesmc_get_lower_bound(&begin, key);
+ if (ret)
+ return ERR_PTR(ret);
+ ret = applesmc_get_upper_bound(&end, key);
+ if (ret)
+ return ERR_PTR(ret);
+ if (end - begin != 1)
+ return ERR_PTR(-EINVAL);
- for (i = 0; i < 4; i++) {
- outb(key[i], APPLESMC_DATA_PORT);
- if (__wait_status(0x04))
- return -EIO;
- }
+ return applesmc_get_entry_by_index(begin);
+}
- outb(6, APPLESMC_DATA_PORT);
+static int applesmc_read_key(const char *key, u8 *buffer, u8 len)
+{
+ const struct applesmc_entry *entry;
- for (i = 0; i < 6; i++) {
- if (__wait_status(0x05))
- return -EIO;
- type[i] = inb(APPLESMC_DATA_PORT);
- }
- type[5] = 0;
+ entry = applesmc_get_entry_by_key(key);
+ if (IS_ERR(entry))
+ return PTR_ERR(entry);
- return 0;
+ return applesmc_read_entry(entry, buffer, len);
+}
+
+static int applesmc_write_key(const char *key, const u8 *buffer, u8 len)
+{
+ const struct applesmc_entry *entry;
+
+ entry = applesmc_get_entry_by_key(key);
+ if (IS_ERR(entry))
+ return PTR_ERR(entry);
+
+ 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)
{
@@ -458,12 +541,10 @@ static void applesmc_device_init(void)
if (!applesmc_accelerometer)
return;
- 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))
- goto out;
+ return;
buffer[0] = 0xe0;
buffer[1] = 0x00;
applesmc_write_key(MOTION_SENSOR_KEY, buffer, 2);
@@ -471,34 +552,89 @@ static void applesmc_device_init(void)
}
pr_warn("failed to init the device\n");
-
-out:
- mutex_unlock(&applesmc_lock);
}
/*
- * 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->cache)
+ s->cache = kcalloc(s->key_count, sizeof(*s->cache), GFP_KERNEL);
+ if (!s->cache)
+ 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.cache);
+ memset(&smcreg, 0, sizeof(smcreg));
+}
+
/* 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 +643,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 +685,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 +710,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 +721,6 @@ static ssize_t applesmc_position_show(struct device *dev,
goto out;
out:
- mutex_unlock(&applesmc_lock);
if (ret)
return ret;
else
@@ -604,18 +730,19 @@ out:
static ssize_t applesmc_light_show(struct device *dev,
struct device_attribute *attr, char *sysfsbuf)
{
+ const 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);
- if (ret)
- goto out;
- data_length = clamp_val(query[0], 0, 10);
+ entry = applesmc_get_entry_by_key(LIGHT_SENSOR_LEFT_KEY);
+ if (IS_ERR(entry))
+ return PTR_ERR(entry);
+ if (entry->len > 10)
+ return -ENXIO;
+ data_length = entry->len;
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,53 @@ 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];
+ const 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);
-
+ entry = applesmc_get_entry_by_index(key_at_index);
+ if (IS_ERR(entry))
+ return PTR_ERR(entry);
+ ret = applesmc_read_entry(entry, sysfsbuf, entry->len);
+ 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 {
- 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];
- int ret;
-
- mutex_lock(&applesmc_lock);
+ const struct applesmc_entry *entry;
- ret = applesmc_get_key_at_index(key_at_index, key);
+ entry = applesmc_get_entry_by_index(key_at_index);
+ if (IS_ERR(entry))
+ return PTR_ERR(entry);
- 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
- 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];
- 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);
+ const struct applesmc_entry *entry;
- mutex_unlock(&applesmc_lock);
+ entry = applesmc_get_entry_by_index(key_at_index);
+ if (IS_ERR(entry))
+ return PTR_ERR(entry);
- if (!ret)
- return snprintf(sysfsbuf, PAGE_SIZE, "%s\n", info+1);
- else
- 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];
- int ret;
-
- mutex_lock(&applesmc_lock);
+ const struct applesmc_entry *entry;
- ret = applesmc_get_key_at_index(key_at_index, key);
+ entry = applesmc_get_entry_by_index(key_at_index);
+ if (IS_ERR(entry))
+ return PTR_ERR(entry);
- mutex_unlock(&applesmc_lock);
-
- 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 +1039,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;
}
@@ -1646,10 +1682,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)
@@ -1751,6 +1792,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:
@@ -1779,6 +1822,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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 04/11] hwmon: applesmc: Introduce a register lookup table (rev2)
2010-11-09 15:15 ` [PATCH 04/11] hwmon: applesmc: Introduce a register lookup table (rev2) Henrik Rydberg
@ 2010-11-09 19:04 ` Guenter Roeck
2010-11-09 19:32 ` Henrik Rydberg
0 siblings, 1 reply; 29+ messages in thread
From: Guenter Roeck @ 2010-11-09 19:04 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Jean Delvare, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org
On Tue, 2010-11-09 at 10:15 -0500, 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@euromail.se>
Hi Henrik,
looks pretty good now. Couple of additional comments.
> ---
> drivers/hwmon/applesmc.c | 528 +++++++++++++++++++++++++---------------------
> 1 files changed, 286 insertions(+), 242 deletions(-)
>
> diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
> index 5f67e39..623f71c 100644
> --- a/drivers/hwmon/applesmc.c
> +++ b/drivers/hwmon/applesmc.c
[ ... ]
> +/*
> + * 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);
> +
I am a bit concerned that mutex_init() can be called multiple times. Are
you sure this is safe ?
> + ret = read_register_count(&s->key_count);
> + if (ret)
> + return ret;
> +
> + if (!s->cache)
> + s->cache = kcalloc(s->key_count, sizeof(*s->cache), GFP_KERNEL);
> + if (!s->cache)
> + return -ENOMEM;
> +
> + s->init_complete = true;
> +
> + pr_info("key=%d\n", s->key_count);
> +
Hope that means more to macbook users than it does to me ;).
> + 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");
INIT_WAIT_MS is 50ms, so you issue this warning every 50ms for up to
five seconds. Pretty noisy... sure that is what you want ? Also, does it
really make sense to retry if the error is ENOMEM ?
Thanks,
Guenter
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 04/11] hwmon: applesmc: Introduce a register lookup table (rev2)
2010-11-09 19:04 ` Guenter Roeck
@ 2010-11-09 19:32 ` Henrik Rydberg
2010-11-09 20:53 ` Guenter Roeck
2010-11-09 22:15 ` Guenter Roeck
0 siblings, 2 replies; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-09 19:32 UTC (permalink / raw)
To: guenter.roeck
Cc: Jean Delvare, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org
Hi Guenter,
>> +/*
>> + * 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);
>> +
> I am a bit concerned that mutex_init() can be called multiple times. Are
> you sure this is safe ?
mutex_destroy() is defined as a nop, so I guess the question is whether anything
could be holding the lock when entering a second init. There are no sysfs files
created at that point, so I would say no. The mutex could be put back with a
static initializer, if this is not satisfactory. The real reason to move it to
the smcreg struct was to force a rename of the mutex itself.
>
>> + ret = read_register_count(&s->key_count);
>> + if (ret)
>> + return ret;
>> +
>> + if (!s->cache)
>> + s->cache = kcalloc(s->key_count, sizeof(*s->cache), GFP_KERNEL);
>> + if (!s->cache)
>> + return -ENOMEM;
>> +
>> + s->init_complete = true;
>> +
>> + pr_info("key=%d\n", s->key_count);
>> +
> Hope that means more to macbook users than it does to me ;).
It means a lot from a diagnostic point of view - a normal user does not really
care about the dmesg output anyways. :-)
>> +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");
>
> INIT_WAIT_MS is 50ms, so you issue this warning every 50ms for up to
> five seconds. Pretty noisy... sure that is what you want ? Also, does it
> really make sense to retry if the error is ENOMEM ?
With the empirical failure rate, it is extremely unlikely to get more than a
couple of failures in a row - information which in itself could be very useful.
A direct escape on ENOMEM makes sense, though.
Changing the place of the mutex will ripple through all patches, so I will
resend from this one onwards. I suppose you have more comments on the following
patches?
Thanks,
Henrik
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 04/11] hwmon: applesmc: Introduce a register lookup table (rev2)
2010-11-09 19:32 ` Henrik Rydberg
@ 2010-11-09 20:53 ` Guenter Roeck
2010-11-10 10:57 ` Henrik Rydberg
2010-11-09 22:15 ` Guenter Roeck
1 sibling, 1 reply; 29+ messages in thread
From: Guenter Roeck @ 2010-11-09 20:53 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Jean Delvare, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org
Hi Henrik,
On Tue, 2010-11-09 at 14:32 -0500, Henrik Rydberg wrote:
> Hi Guenter,
>
> >> +/*
>
> >> + * 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);
> >> +
> > I am a bit concerned that mutex_init() can be called multiple times. Are
> > you sure this is safe ?
>
>
> mutex_destroy() is defined as a nop, so I guess the question is whether anything
> could be holding the lock when entering a second init. There are no sysfs files
> created at that point, so I would say no. The mutex could be put back with a
> static initializer, if this is not satisfactory. The real reason to move it to
> the smcreg struct was to force a rename of the mutex itself.
>
Alternatively, you could move the mutex initialization to the beginning
of applesmc_init_smcreg() and make it
mutex_init(&smcreg.mutex);
> >
> >> + ret = read_register_count(&s->key_count);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (!s->cache)
> >> + s->cache = kcalloc(s->key_count, sizeof(*s->cache), GFP_KERNEL);
> >> + if (!s->cache)
> >> + return -ENOMEM;
> >> +
> >> + s->init_complete = true;
> >> +
> >> + pr_info("key=%d\n", s->key_count);
> >> +
> > Hope that means more to macbook users than it does to me ;).
>
>
> It means a lot from a diagnostic point of view - a normal user does not really
> care about the dmesg output anyways. :-)
>
Ok, guess I am not a normal user ;).
> >> +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");
> >
> > INIT_WAIT_MS is 50ms, so you issue this warning every 50ms for up to
> > five seconds. Pretty noisy... sure that is what you want ? Also, does it
> > really make sense to retry if the error is ENOMEM ?
>
>
> With the empirical failure rate, it is extremely unlikely to get more than a
> couple of failures in a row - information which in itself could be very useful.
You would have alternative options, though, with less noise. For
example, something along the line of
for (...) {
...
if (!ret) {
if (ms)
pr_info("smcreg initialization took %d ms\n", ms);
return 0;
}
...
}
pr_err("smcreg initialization failed\n");
> A direct escape on ENOMEM makes sense, though.
>
> Changing the place of the mutex will ripple through all patches, so I will
> resend from this one onwards. I suppose you have more comments on the following
> patches?
Maybe it won't be that bad if you initialize it as I suggested above.
Regarding additional comments - I don't know yet. I didn't have time to
look into the other patches yet. I'll try to do that by tonight.
Guenter
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 04/11] hwmon: applesmc: Introduce a register lookup table (rev2)
2010-11-09 20:53 ` Guenter Roeck
@ 2010-11-10 10:57 ` Henrik Rydberg
2010-11-10 15:42 ` Guenter Roeck
0 siblings, 1 reply; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-10 10:57 UTC (permalink / raw)
To: guenter.roeck
Cc: Jean Delvare, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org
>>
>> mutex_destroy() is defined as a nop, so I guess the question is whether anything
>> could be holding the lock when entering a second init. There are no sysfs files
>> created at that point, so I would say no. The mutex could be put back with a
>> static initializer, if this is not satisfactory. The real reason to move it to
>> the smcreg struct was to force a rename of the mutex itself.
>>
>
> Alternatively, you could move the mutex initialization to the beginning
> of applesmc_init_smcreg() and make it
> mutex_init(&smcreg.mutex);
Looking at this again, it seems there are two other problems as well. Firstly,
the cache memory is not freed after probe failure, my apologies. Secondly,
execution continues after a probe failure, and the initialization is retried. I
would like to push the latter problem to some other occasion, since the whole
platform logic should be rewritten for the new interface, anyways.
>>
>> With the empirical failure rate, it is extremely unlikely to get more than a
>> couple of failures in a row - information which in itself could be very useful.
>
> You would have alternative options, though, with less noise. For
> example, something along the line of
>
> for (...) {
> ...
> if (!ret) {
> if (ms)
> pr_info("smcreg initialization took %d ms\n", ms);
> return 0;
> }
> ...
> }
> pr_err("smcreg initialization failed\n");
Looks nice, have applied, but without the last line; the probe failure report
should be enough to deduce this.
>>
>> Changing the place of the mutex will ripple through all patches, so I will
>> resend from this one onwards. I suppose you have more comments on the following
>> patches?
>
> Maybe it won't be that bad if you initialize it as I suggested above.
I tried several types of changes, and they all had some effect on later patches.
The patch below comprise the resulting changes to patch 4. Hope you like. In
addition, patch 5 and 7 needed one line of wiggling. I am resending all three.
@@ -217,7 +217,9 @@ static struct applesmc_registers {
unsigned int key_count; /* number of SMC registers */
bool init_complete; /* true when fully initialized */
struct applesmc_entry *cache; /* cached key entries */
-} smcreg;
+} smcreg = {
+ .mutex = __MUTEX_INITIALIZER(smcreg.mutex),
+};
static const int debug;
static struct platform_device *pdev;
@@ -581,8 +583,6 @@ static int applesmc_init_smcreg_try(void)
if (s->init_complete)
return 0;
- mutex_init(&s->mutex);
-
ret = read_register_count(&s->key_count);
if (ret)
return ret;
@@ -611,19 +611,25 @@ static int applesmc_init_smcreg(void)
for (ms = 0; ms < INIT_TIMEOUT_MSECS; ms += INIT_WAIT_MSECS) {
ret = applesmc_init_smcreg_try();
- if (!ret)
+ if (!ret) {
+ if (ms)
+ pr_info("smcreg initialization took %d ms\n", ms);
return 0;
- pr_warn("slow init, retrying\n");
+ }
msleep(INIT_WAIT_MSECS);
}
+ kfree(smcreg.cache);
+ smcreg.cache = NULL;
+
return ret;
}
static void applesmc_destroy_smcreg(void)
{
kfree(smcreg.cache);
- memset(&smcreg, 0, sizeof(smcreg));
+ smcreg.cache = NULL;
+ smcreg.init_complete = false;
}
/* Device model stuff */
Thanks,
Henrik
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 04/11] hwmon: applesmc: Introduce a register lookup table (rev2)
2010-11-10 10:57 ` Henrik Rydberg
@ 2010-11-10 15:42 ` Guenter Roeck
2010-11-10 17:14 ` Henrik Rydberg
0 siblings, 1 reply; 29+ messages in thread
From: Guenter Roeck @ 2010-11-10 15:42 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Jean Delvare, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org
On Wed, Nov 10, 2010 at 05:57:24AM -0500, Henrik Rydberg wrote:
>
> >>
> >> mutex_destroy() is defined as a nop, so I guess the question is whether anything
> >> could be holding the lock when entering a second init. There are no sysfs files
> >> created at that point, so I would say no. The mutex could be put back with a
> >> static initializer, if this is not satisfactory. The real reason to move it to
> >> the smcreg struct was to force a rename of the mutex itself.
> >>
> >
> > Alternatively, you could move the mutex initialization to the beginning
> > of applesmc_init_smcreg() and make it
> > mutex_init(&smcreg.mutex);
>
>
> Looking at this again, it seems there are two other problems as well. Firstly,
> the cache memory is not freed after probe failure, my apologies. Secondly,
> execution continues after a probe failure, and the initialization is retried. I
> would like to push the latter problem to some other occasion, since the whole
> platform logic should be rewritten for the new interface, anyways.
>
I see the cache problem; this is indeed a tricky one, since it is actually not yet
a problem after this patch, but will be one after patch 5.
I don't understand the second problem, though. Looking into the code,
the probe function will return an error if applesmc_init_smcreg() fails.
Am I missing something ? What execution continues ?
Thanks,
Guenter
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 04/11] hwmon: applesmc: Introduce a register lookup table (rev2)
2010-11-10 15:42 ` Guenter Roeck
@ 2010-11-10 17:14 ` Henrik Rydberg
2010-11-10 17:57 ` Guenter Roeck
0 siblings, 1 reply; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-10 17:14 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jean Delvare, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org
>>
>> Looking at this again, it seems there are two other problems as well. Firstly,
>> the cache memory is not freed after probe failure, my apologies. Secondly,
>> execution continues after a probe failure, and the initialization is retried. I
>> would like to push the latter problem to some other occasion, since the whole
>> platform logic should be rewritten for the new interface, anyways.
>>
> I see the cache problem; this is indeed a tricky one, since it is actually not yet
> a problem after this patch, but will be one after patch 5.
>
> I don't understand the second problem, though. Looking into the code,
> the probe function will return an error if applesmc_init_smcreg() fails.
> Am I missing something ? What execution continues ?
I think drivers/base/dd.c:142 shows the problem clearly. Basically, the probe
function is supposed to do the proper initialization, if successful. The driver
code has been rewritten heavily since the days of the applesmc, and the actual
probe error is now masked in a rather ugly fashion to allow the driver matching
to continue. It seems to me that the only clean way around this is to actually
implement the correct platform driver logic.
Thanks,
Henrik
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 04/11] hwmon: applesmc: Introduce a register lookup table (rev2)
2010-11-10 17:14 ` Henrik Rydberg
@ 2010-11-10 17:57 ` Guenter Roeck
0 siblings, 0 replies; 29+ messages in thread
From: Guenter Roeck @ 2010-11-10 17:57 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Jean Delvare, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org
On Wed, Nov 10, 2010 at 12:14:38PM -0500, Henrik Rydberg wrote:
> >>
>
> >> Looking at this again, it seems there are two other problems as well. Firstly,
> >> the cache memory is not freed after probe failure, my apologies. Secondly,
> >> execution continues after a probe failure, and the initialization is retried. I
> >> would like to push the latter problem to some other occasion, since the whole
> >> platform logic should be rewritten for the new interface, anyways.
> >>
> > I see the cache problem; this is indeed a tricky one, since it is actually not yet
> > a problem after this patch, but will be one after patch 5.
> >
> > I don't understand the second problem, though. Looking into the code,
> > the probe function will return an error if applesmc_init_smcreg() fails.
> > Am I missing something ? What execution continues ?
>
>
> I think drivers/base/dd.c:142 shows the problem clearly. Basically, the probe
> function is supposed to do the proper initialization, if successful. The driver
> code has been rewritten heavily since the days of the applesmc, and the actual
> probe error is now masked in a rather ugly fashion to allow the driver matching
> to continue. It seems to me that the only clean way around this is to actually
> implement the correct platform driver logic.
>
Ok, guess there is nothing we can do about that. I'll apply the rest of your series.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 04/11] hwmon: applesmc: Introduce a register lookup table (rev2)
2010-11-09 19:32 ` Henrik Rydberg
2010-11-09 20:53 ` Guenter Roeck
@ 2010-11-09 22:15 ` Guenter Roeck
1 sibling, 0 replies; 29+ messages in thread
From: Guenter Roeck @ 2010-11-09 22:15 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Jean Delvare, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org
Hi Henrik,
On Tue, 2010-11-09 at 14:32 -0500, Henrik Rydberg wrote:
[ ... ]
>
> Changing the place of the mutex will ripple through all patches, so I will
> resend from this one onwards. I suppose you have more comments on the following
> patches?
>
The other patches look ok to me.
Guenter
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 05/11] hwmon: applesmc: Dynamic creation of temperature files (rev2)
2010-11-09 15:15 [PATCH 00/11] hwmon: applesmc: Dynamic configuration rewrite (rev2) Henrik Rydberg
` (3 preceding siblings ...)
2010-11-09 15:15 ` [PATCH 04/11] hwmon: applesmc: Introduce a register lookup table (rev2) Henrik Rydberg
@ 2010-11-09 15:15 ` Henrik Rydberg
2010-11-09 15:15 ` [PATCH 06/11] hwmon: applesmc: Handle new temperature format (rev2) Henrik Rydberg
` (5 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-09 15:15 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: lm-sensors, linux-kernel, Henrik Rydberg
The current driver creates temperature files based on a list
of temperature keys given per device. Apart from slow adaption
to new machine models, the number of sensors also depends on
the number of processors. This patch looks up the temperature
keys dynamically, thereby supporting all models.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hwmon/applesmc.c | 500 ++++++++++-----------------------------------
1 files changed, 111 insertions(+), 389 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index 623f71c..55abb5f 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -84,94 +84,6 @@
#define FAN_TARGET_SPEED "F0Tg" /* r-w fpe2 (2 bytes) */
#define FAN_POSITION "F0ID" /* r-o char[16] */
-/*
- * Temperature sensors keys (sp78 - 2 bytes).
- */
-static const char *temperature_sensors_sets[][41] = {
-/* Set 0: Macbook Pro */
- { "TA0P", "TB0T", "TC0D", "TC0P", "TG0H", "TG0P", "TG0T", "Th0H",
- "Th1H", "Tm0P", "Ts0P", "Ts1P", NULL },
-/* Set 1: Macbook2 set */
- { "TB0T", "TC0D", "TC0P", "TM0P", "TN0P", "TN1P", "TTF0", "Th0H",
- "Th0S", "Th1H", NULL },
-/* Set 2: Macbook set */
- { "TB0T", "TC0D", "TC0P", "TM0P", "TN0P", "TN1P", "Th0H", "Th0S",
- "Th1H", "Ts0P", NULL },
-/* Set 3: Macmini set */
- { "TC0D", "TC0P", NULL },
-/* Set 4: Mac Pro (2 x Quad-Core) */
- { "TA0P", "TCAG", "TCAH", "TCBG", "TCBH", "TC0C", "TC0D", "TC0P",
- "TC1C", "TC1D", "TC2C", "TC2D", "TC3C", "TC3D", "THTG", "TH0P",
- "TH1P", "TH2P", "TH3P", "TMAP", "TMAS", "TMBS", "TM0P", "TM0S",
- "TM1P", "TM1S", "TM2P", "TM2S", "TM3S", "TM8P", "TM8S", "TM9P",
- "TM9S", "TN0H", "TS0C", NULL },
-/* Set 5: iMac */
- { "TC0D", "TA0P", "TG0P", "TG0D", "TG0H", "TH0P", "Tm0P", "TO0P",
- "Tp0C", NULL },
-/* Set 6: Macbook3 set */
- { "TB0T", "TC0D", "TC0P", "TM0P", "TN0P", "TTF0", "TW0P", "Th0H",
- "Th0S", "Th1H", NULL },
-/* Set 7: Macbook Air */
- { "TB0T", "TB1S", "TB1T", "TB2S", "TB2T", "TC0D", "TC0P", "TCFP",
- "TTF0", "TW0P", "Th0H", "Tp0P", "TpFP", "Ts0P", "Ts0S", NULL },
-/* Set 8: Macbook Pro 4,1 (Penryn) */
- { "TB0T", "TC0D", "TC0P", "TG0D", "TG0H", "TTF0", "TW0P", "Th0H",
- "Th1H", "Th2H", "Tm0P", "Ts0P", NULL },
-/* Set 9: Macbook Pro 3,1 (Santa Rosa) */
- { "TALP", "TB0T", "TC0D", "TC0P", "TG0D", "TG0H", "TTF0", "TW0P",
- "Th0H", "Th1H", "Th2H", "Tm0P", "Ts0P", NULL },
-/* Set 10: iMac 5,1 */
- { "TA0P", "TC0D", "TC0P", "TG0D", "TH0P", "TO0P", "Tm0P", NULL },
-/* Set 11: Macbook 5,1 */
- { "TB0T", "TB1T", "TB2T", "TB3T", "TC0D", "TC0P", "TN0D", "TN0P",
- "TTF0", "Th0H", "Th1H", "ThFH", "Ts0P", "Ts0S", NULL },
-/* Set 12: Macbook Pro 5,1 */
- { "TB0T", "TB1T", "TB2T", "TB3T", "TC0D", "TC0F", "TC0P", "TG0D",
- "TG0F", "TG0H", "TG0P", "TG0T", "TG1H", "TN0D", "TN0P", "TTF0",
- "Th2H", "Tm0P", "Ts0P", "Ts0S", NULL },
-/* Set 13: iMac 8,1 */
- { "TA0P", "TC0D", "TC0H", "TC0P", "TG0D", "TG0H", "TG0P", "TH0P",
- "TL0P", "TO0P", "TW0P", "Tm0P", "Tp0P", NULL },
-/* Set 14: iMac 6,1 */
- { "TA0P", "TC0D", "TC0H", "TC0P", "TG0D", "TG0H", "TG0P", "TH0P",
- "TO0P", "Tp0P", NULL },
-/* Set 15: MacBook Air 2,1 */
- { "TB0T", "TB1S", "TB1T", "TB2S", "TB2T", "TC0D", "TN0D", "TTF0",
- "TV0P", "TVFP", "TW0P", "Th0P", "Tp0P", "Tp1P", "TpFP", "Ts0P",
- "Ts0S", NULL },
-/* Set 16: Mac Pro 3,1 (2 x Quad-Core) */
- { "TA0P", "TCAG", "TCAH", "TCBG", "TCBH", "TC0C", "TC0D", "TC0P",
- "TC1C", "TC1D", "TC2C", "TC2D", "TC3C", "TC3D", "TH0P", "TH1P",
- "TH2P", "TH3P", "TMAP", "TMAS", "TMBS", "TM0P", "TM0S", "TM1P",
- "TM1S", "TM2P", "TM2S", "TM3S", "TM8P", "TM8S", "TM9P", "TM9S",
- "TN0C", "TN0D", "TN0H", "TS0C", "Tp0C", "Tp1C", "Tv0S", "Tv1S",
- NULL },
-/* Set 17: iMac 9,1 */
- { "TA0P", "TC0D", "TC0H", "TC0P", "TG0D", "TG0H", "TH0P", "TL0P",
- "TN0D", "TN0H", "TN0P", "TO0P", "Tm0P", "Tp0P", NULL },
-/* Set 18: MacBook Pro 2,2 */
- { "TB0T", "TC0D", "TC0P", "TG0H", "TG0P", "TG0T", "TM0P", "TTF0",
- "Th0H", "Th1H", "Tm0P", "Ts0P", NULL },
-/* Set 19: Macbook Pro 5,3 */
- { "TB0T", "TB1T", "TB2T", "TB3T", "TC0D", "TC0F", "TC0P", "TG0D",
- "TG0F", "TG0H", "TG0P", "TG0T", "TN0D", "TN0P", "TTF0", "Th2H",
- "Tm0P", "Ts0P", "Ts0S", NULL },
-/* Set 20: MacBook Pro 5,4 */
- { "TB0T", "TB1T", "TB2T", "TB3T", "TC0D", "TC0F", "TC0P", "TN0D",
- "TN0P", "TTF0", "Th2H", "Ts0P", "Ts0S", NULL },
-/* Set 21: MacBook Pro 6,2 */
- { "TB0T", "TB1T", "TB2T", "TC0C", "TC0D", "TC0P", "TC1C", "TG0D",
- "TG0P", "TG0T", "TMCD", "TP0P", "TPCD", "Th1H", "Th2H", "Tm0P",
- "Ts0P", "Ts0S", NULL },
-/* Set 22: MacBook Pro 7,1 */
- { "TB0T", "TB1T", "TB2T", "TC0D", "TC0P", "TN0D", "TN0P", "TN0S",
- "TN1D", "TN1F", "TN1G", "TN1S", "Th1H", "Ts0P", "Ts0S", NULL },
-/* Set 23: MacBook Air 3,1 */
- { "TB0T", "TB1T", "TB2T", "TC0D", "TC0E", "TC0P", "TC1E", "TCZ3",
- "TCZ4", "TCZ5", "TG0E", "TG1E", "TG2E", "TGZ3", "TGZ4", "TGZ5",
- "TH0F", "TH0O", "TM0P" },
-};
-
/* List of keys used to read/write fan speeds */
static const char* fan_speed_keys[] = {
FAN_ACTUAL_SPEED,
@@ -192,6 +104,8 @@ static const char* fan_speed_keys[] = {
#define SENSOR_Y 1
#define SENSOR_Z 2
+#define to_index(attr) (to_sensor_dev_attr(attr)->index)
+
/* Structure to be passed to DMI_MATCH function */
struct dmi_match_data {
/* Indicates whether this computer has an accelerometer. */
@@ -202,6 +116,20 @@ struct dmi_match_data {
int temperature_set;
};
+/* Dynamic device node attributes */
+struct applesmc_dev_attr {
+ struct sensor_device_attribute sda; /* hwmon attributes */
+ char name[32]; /* room for node file name */
+};
+
+/* Dynamic device node group */
+struct applesmc_node_group {
+ char *format; /* format string */
+ void *show; /* show function */
+ void *store; /* store function */
+ struct applesmc_dev_attr *nodes; /* dynamic node array */
+};
+
/* AppleSMC entry - cached register information */
struct applesmc_entry {
char key[5]; /* four-letter key code */
@@ -215,6 +143,9 @@ struct applesmc_entry {
static struct applesmc_registers {
struct mutex mutex; /* register read/write mutex */
unsigned int key_count; /* number of SMC registers */
+ unsigned int temp_count; /* number of temperature registers */
+ unsigned int temp_begin; /* temperature lower index bound */
+ unsigned int temp_end; /* temperature upper index bound */
bool init_complete; /* true when fully initialized */
struct applesmc_entry *cache; /* cached key entries */
} smcreg;
@@ -237,9 +168,6 @@ static unsigned int applesmc_light;
/* The number of fans handled by the driver */
static unsigned int fans_handled;
-/* Indicates which temperature sensors set to use. */
-static unsigned int applesmc_temperature_set;
-
/*
* Last index written to key_at_index sysfs file, and value to use for all other
* key_at_index_* sysfs files.
@@ -592,9 +520,17 @@ static int applesmc_init_smcreg_try(void)
if (!s->cache)
return -ENOMEM;
+ ret = applesmc_get_lower_bound(&s->temp_begin, "T");
+ if (ret)
+ return ret;
+ ret = applesmc_get_lower_bound(&s->temp_end, "U");
+ if (ret)
+ return ret;
+ s->temp_count = s->temp_end - s->temp_begin;
+
s->init_complete = true;
- pr_info("key=%d\n", s->key_count);
+ pr_info("key=%d temp=%d\n", s->key_count, s->temp_count);
return 0;
}
@@ -769,32 +705,38 @@ out:
static ssize_t applesmc_show_sensor_label(struct device *dev,
struct device_attribute *devattr, char *sysfsbuf)
{
- struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
- const char *key =
- temperature_sensors_sets[applesmc_temperature_set][attr->index];
+ int index = smcreg.temp_begin + to_index(devattr);
+ const struct applesmc_entry *entry;
- return snprintf(sysfsbuf, PAGE_SIZE, "%s\n", key);
+ entry = applesmc_get_entry_by_index(index);
+ if (IS_ERR(entry))
+ return PTR_ERR(entry);
+
+ return snprintf(sysfsbuf, PAGE_SIZE, "%s\n", entry->key);
}
/* Displays degree Celsius * 1000 */
static ssize_t applesmc_show_temperature(struct device *dev,
struct device_attribute *devattr, char *sysfsbuf)
{
+ int index = smcreg.temp_begin + to_index(devattr);
+ const struct applesmc_entry *entry;
int ret;
u8 buffer[2];
unsigned int temp;
- struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
- const char* key =
- temperature_sensors_sets[applesmc_temperature_set][attr->index];
- ret = applesmc_read_key(key, buffer, 2);
- temp = buffer[0]*1000;
- temp += (buffer[1] >> 6) * 250;
+ entry = applesmc_get_entry_by_index(index);
+ if (IS_ERR(entry))
+ return PTR_ERR(entry);
+ ret = applesmc_read_entry(entry, buffer, 2);
if (ret)
return ret;
- else
- return snprintf(sysfsbuf, PAGE_SIZE, "%u\n", temp);
+
+ temp = buffer[0]*1000;
+ temp += (buffer[1] >> 6) * 250;
+
+ return snprintf(sysfsbuf, PAGE_SIZE, "%u\n", temp);
}
static ssize_t applesmc_show_fan_speed(struct device *dev,
@@ -1150,263 +1092,10 @@ static const struct attribute_group fan_attribute_groups[] = {
{ .attrs = fan4_attributes },
};
-/*
- * Temperature sensors sysfs entries.
- */
-static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 0);
-static SENSOR_DEVICE_ATTR(temp2_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 1);
-static SENSOR_DEVICE_ATTR(temp3_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 2);
-static SENSOR_DEVICE_ATTR(temp4_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 3);
-static SENSOR_DEVICE_ATTR(temp5_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 4);
-static SENSOR_DEVICE_ATTR(temp6_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 5);
-static SENSOR_DEVICE_ATTR(temp7_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 6);
-static SENSOR_DEVICE_ATTR(temp8_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 7);
-static SENSOR_DEVICE_ATTR(temp9_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 8);
-static SENSOR_DEVICE_ATTR(temp10_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 9);
-static SENSOR_DEVICE_ATTR(temp11_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 10);
-static SENSOR_DEVICE_ATTR(temp12_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 11);
-static SENSOR_DEVICE_ATTR(temp13_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 12);
-static SENSOR_DEVICE_ATTR(temp14_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 13);
-static SENSOR_DEVICE_ATTR(temp15_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 14);
-static SENSOR_DEVICE_ATTR(temp16_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 15);
-static SENSOR_DEVICE_ATTR(temp17_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 16);
-static SENSOR_DEVICE_ATTR(temp18_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 17);
-static SENSOR_DEVICE_ATTR(temp19_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 18);
-static SENSOR_DEVICE_ATTR(temp20_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 19);
-static SENSOR_DEVICE_ATTR(temp21_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 20);
-static SENSOR_DEVICE_ATTR(temp22_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 21);
-static SENSOR_DEVICE_ATTR(temp23_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 22);
-static SENSOR_DEVICE_ATTR(temp24_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 23);
-static SENSOR_DEVICE_ATTR(temp25_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 24);
-static SENSOR_DEVICE_ATTR(temp26_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 25);
-static SENSOR_DEVICE_ATTR(temp27_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 26);
-static SENSOR_DEVICE_ATTR(temp28_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 27);
-static SENSOR_DEVICE_ATTR(temp29_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 28);
-static SENSOR_DEVICE_ATTR(temp30_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 29);
-static SENSOR_DEVICE_ATTR(temp31_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 30);
-static SENSOR_DEVICE_ATTR(temp32_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 31);
-static SENSOR_DEVICE_ATTR(temp33_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 32);
-static SENSOR_DEVICE_ATTR(temp34_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 33);
-static SENSOR_DEVICE_ATTR(temp35_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 34);
-static SENSOR_DEVICE_ATTR(temp36_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 35);
-static SENSOR_DEVICE_ATTR(temp37_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 36);
-static SENSOR_DEVICE_ATTR(temp38_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 37);
-static SENSOR_DEVICE_ATTR(temp39_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 38);
-static SENSOR_DEVICE_ATTR(temp40_label, S_IRUGO,
- applesmc_show_sensor_label, NULL, 39);
-static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
- applesmc_show_temperature, NULL, 0);
-static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO,
- applesmc_show_temperature, NULL, 1);
-static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO,
- applesmc_show_temperature, NULL, 2);
-static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO,
- applesmc_show_temperature, NULL, 3);
-static SENSOR_DEVICE_ATTR(temp5_input, S_IRUGO,
- applesmc_show_temperature, NULL, 4);
-static SENSOR_DEVICE_ATTR(temp6_input, S_IRUGO,
- applesmc_show_temperature, NULL, 5);
-static SENSOR_DEVICE_ATTR(temp7_input, S_IRUGO,
- applesmc_show_temperature, NULL, 6);
-static SENSOR_DEVICE_ATTR(temp8_input, S_IRUGO,
- applesmc_show_temperature, NULL, 7);
-static SENSOR_DEVICE_ATTR(temp9_input, S_IRUGO,
- applesmc_show_temperature, NULL, 8);
-static SENSOR_DEVICE_ATTR(temp10_input, S_IRUGO,
- applesmc_show_temperature, NULL, 9);
-static SENSOR_DEVICE_ATTR(temp11_input, S_IRUGO,
- applesmc_show_temperature, NULL, 10);
-static SENSOR_DEVICE_ATTR(temp12_input, S_IRUGO,
- applesmc_show_temperature, NULL, 11);
-static SENSOR_DEVICE_ATTR(temp13_input, S_IRUGO,
- applesmc_show_temperature, NULL, 12);
-static SENSOR_DEVICE_ATTR(temp14_input, S_IRUGO,
- applesmc_show_temperature, NULL, 13);
-static SENSOR_DEVICE_ATTR(temp15_input, S_IRUGO,
- applesmc_show_temperature, NULL, 14);
-static SENSOR_DEVICE_ATTR(temp16_input, S_IRUGO,
- applesmc_show_temperature, NULL, 15);
-static SENSOR_DEVICE_ATTR(temp17_input, S_IRUGO,
- applesmc_show_temperature, NULL, 16);
-static SENSOR_DEVICE_ATTR(temp18_input, S_IRUGO,
- applesmc_show_temperature, NULL, 17);
-static SENSOR_DEVICE_ATTR(temp19_input, S_IRUGO,
- applesmc_show_temperature, NULL, 18);
-static SENSOR_DEVICE_ATTR(temp20_input, S_IRUGO,
- applesmc_show_temperature, NULL, 19);
-static SENSOR_DEVICE_ATTR(temp21_input, S_IRUGO,
- applesmc_show_temperature, NULL, 20);
-static SENSOR_DEVICE_ATTR(temp22_input, S_IRUGO,
- applesmc_show_temperature, NULL, 21);
-static SENSOR_DEVICE_ATTR(temp23_input, S_IRUGO,
- applesmc_show_temperature, NULL, 22);
-static SENSOR_DEVICE_ATTR(temp24_input, S_IRUGO,
- applesmc_show_temperature, NULL, 23);
-static SENSOR_DEVICE_ATTR(temp25_input, S_IRUGO,
- applesmc_show_temperature, NULL, 24);
-static SENSOR_DEVICE_ATTR(temp26_input, S_IRUGO,
- applesmc_show_temperature, NULL, 25);
-static SENSOR_DEVICE_ATTR(temp27_input, S_IRUGO,
- applesmc_show_temperature, NULL, 26);
-static SENSOR_DEVICE_ATTR(temp28_input, S_IRUGO,
- applesmc_show_temperature, NULL, 27);
-static SENSOR_DEVICE_ATTR(temp29_input, S_IRUGO,
- applesmc_show_temperature, NULL, 28);
-static SENSOR_DEVICE_ATTR(temp30_input, S_IRUGO,
- applesmc_show_temperature, NULL, 29);
-static SENSOR_DEVICE_ATTR(temp31_input, S_IRUGO,
- applesmc_show_temperature, NULL, 30);
-static SENSOR_DEVICE_ATTR(temp32_input, S_IRUGO,
- applesmc_show_temperature, NULL, 31);
-static SENSOR_DEVICE_ATTR(temp33_input, S_IRUGO,
- applesmc_show_temperature, NULL, 32);
-static SENSOR_DEVICE_ATTR(temp34_input, S_IRUGO,
- applesmc_show_temperature, NULL, 33);
-static SENSOR_DEVICE_ATTR(temp35_input, S_IRUGO,
- applesmc_show_temperature, NULL, 34);
-static SENSOR_DEVICE_ATTR(temp36_input, S_IRUGO,
- applesmc_show_temperature, NULL, 35);
-static SENSOR_DEVICE_ATTR(temp37_input, S_IRUGO,
- applesmc_show_temperature, NULL, 36);
-static SENSOR_DEVICE_ATTR(temp38_input, S_IRUGO,
- applesmc_show_temperature, NULL, 37);
-static SENSOR_DEVICE_ATTR(temp39_input, S_IRUGO,
- applesmc_show_temperature, NULL, 38);
-static SENSOR_DEVICE_ATTR(temp40_input, S_IRUGO,
- applesmc_show_temperature, NULL, 39);
-
-static struct attribute *label_attributes[] = {
- &sensor_dev_attr_temp1_label.dev_attr.attr,
- &sensor_dev_attr_temp2_label.dev_attr.attr,
- &sensor_dev_attr_temp3_label.dev_attr.attr,
- &sensor_dev_attr_temp4_label.dev_attr.attr,
- &sensor_dev_attr_temp5_label.dev_attr.attr,
- &sensor_dev_attr_temp6_label.dev_attr.attr,
- &sensor_dev_attr_temp7_label.dev_attr.attr,
- &sensor_dev_attr_temp8_label.dev_attr.attr,
- &sensor_dev_attr_temp9_label.dev_attr.attr,
- &sensor_dev_attr_temp10_label.dev_attr.attr,
- &sensor_dev_attr_temp11_label.dev_attr.attr,
- &sensor_dev_attr_temp12_label.dev_attr.attr,
- &sensor_dev_attr_temp13_label.dev_attr.attr,
- &sensor_dev_attr_temp14_label.dev_attr.attr,
- &sensor_dev_attr_temp15_label.dev_attr.attr,
- &sensor_dev_attr_temp16_label.dev_attr.attr,
- &sensor_dev_attr_temp17_label.dev_attr.attr,
- &sensor_dev_attr_temp18_label.dev_attr.attr,
- &sensor_dev_attr_temp19_label.dev_attr.attr,
- &sensor_dev_attr_temp20_label.dev_attr.attr,
- &sensor_dev_attr_temp21_label.dev_attr.attr,
- &sensor_dev_attr_temp22_label.dev_attr.attr,
- &sensor_dev_attr_temp23_label.dev_attr.attr,
- &sensor_dev_attr_temp24_label.dev_attr.attr,
- &sensor_dev_attr_temp25_label.dev_attr.attr,
- &sensor_dev_attr_temp26_label.dev_attr.attr,
- &sensor_dev_attr_temp27_label.dev_attr.attr,
- &sensor_dev_attr_temp28_label.dev_attr.attr,
- &sensor_dev_attr_temp29_label.dev_attr.attr,
- &sensor_dev_attr_temp30_label.dev_attr.attr,
- &sensor_dev_attr_temp31_label.dev_attr.attr,
- &sensor_dev_attr_temp32_label.dev_attr.attr,
- &sensor_dev_attr_temp33_label.dev_attr.attr,
- &sensor_dev_attr_temp34_label.dev_attr.attr,
- &sensor_dev_attr_temp35_label.dev_attr.attr,
- &sensor_dev_attr_temp36_label.dev_attr.attr,
- &sensor_dev_attr_temp37_label.dev_attr.attr,
- &sensor_dev_attr_temp38_label.dev_attr.attr,
- &sensor_dev_attr_temp39_label.dev_attr.attr,
- &sensor_dev_attr_temp40_label.dev_attr.attr,
- NULL
-};
-
-static struct attribute *temperature_attributes[] = {
- &sensor_dev_attr_temp1_input.dev_attr.attr,
- &sensor_dev_attr_temp2_input.dev_attr.attr,
- &sensor_dev_attr_temp3_input.dev_attr.attr,
- &sensor_dev_attr_temp4_input.dev_attr.attr,
- &sensor_dev_attr_temp5_input.dev_attr.attr,
- &sensor_dev_attr_temp6_input.dev_attr.attr,
- &sensor_dev_attr_temp7_input.dev_attr.attr,
- &sensor_dev_attr_temp8_input.dev_attr.attr,
- &sensor_dev_attr_temp9_input.dev_attr.attr,
- &sensor_dev_attr_temp10_input.dev_attr.attr,
- &sensor_dev_attr_temp11_input.dev_attr.attr,
- &sensor_dev_attr_temp12_input.dev_attr.attr,
- &sensor_dev_attr_temp13_input.dev_attr.attr,
- &sensor_dev_attr_temp14_input.dev_attr.attr,
- &sensor_dev_attr_temp15_input.dev_attr.attr,
- &sensor_dev_attr_temp16_input.dev_attr.attr,
- &sensor_dev_attr_temp17_input.dev_attr.attr,
- &sensor_dev_attr_temp18_input.dev_attr.attr,
- &sensor_dev_attr_temp19_input.dev_attr.attr,
- &sensor_dev_attr_temp20_input.dev_attr.attr,
- &sensor_dev_attr_temp21_input.dev_attr.attr,
- &sensor_dev_attr_temp22_input.dev_attr.attr,
- &sensor_dev_attr_temp23_input.dev_attr.attr,
- &sensor_dev_attr_temp24_input.dev_attr.attr,
- &sensor_dev_attr_temp25_input.dev_attr.attr,
- &sensor_dev_attr_temp26_input.dev_attr.attr,
- &sensor_dev_attr_temp27_input.dev_attr.attr,
- &sensor_dev_attr_temp28_input.dev_attr.attr,
- &sensor_dev_attr_temp29_input.dev_attr.attr,
- &sensor_dev_attr_temp30_input.dev_attr.attr,
- &sensor_dev_attr_temp31_input.dev_attr.attr,
- &sensor_dev_attr_temp32_input.dev_attr.attr,
- &sensor_dev_attr_temp33_input.dev_attr.attr,
- &sensor_dev_attr_temp34_input.dev_attr.attr,
- &sensor_dev_attr_temp35_input.dev_attr.attr,
- &sensor_dev_attr_temp36_input.dev_attr.attr,
- &sensor_dev_attr_temp37_input.dev_attr.attr,
- &sensor_dev_attr_temp38_input.dev_attr.attr,
- &sensor_dev_attr_temp39_input.dev_attr.attr,
- &sensor_dev_attr_temp40_input.dev_attr.attr,
- NULL
-};
-
-static const struct attribute_group temperature_attributes_group =
- { .attrs = temperature_attributes };
-
-static const struct attribute_group label_attributes_group = {
- .attrs = label_attributes
+static struct applesmc_node_group temp_group[] = {
+ { "temp%d_label", applesmc_show_sensor_label },
+ { "temp%d_input", applesmc_show_temperature },
+ { }
};
/* Module stuff */
@@ -1416,7 +1105,6 @@ static const struct attribute_group label_attributes_group = {
*/
static int applesmc_dmi_match(const struct dmi_system_id *id)
{
- int i = 0;
struct dmi_match_data* dmi_data = id->driver_data;
pr_info("%s detected:\n", id->ident);
applesmc_accelerometer = dmi_data->accelerometer;
@@ -1426,13 +1114,65 @@ static int applesmc_dmi_match(const struct dmi_system_id *id)
pr_info(" - Model %s light sensors and backlight\n",
applesmc_light ? "with" : "without");
- applesmc_temperature_set = dmi_data->temperature_set;
- while (temperature_sensors_sets[applesmc_temperature_set][i] != NULL)
- i++;
- pr_info(" - Model with %d temperature sensors\n", i);
return 1;
}
+/*
+ * applesmc_destroy_nodes - remove files and free associated memory
+ */
+static void applesmc_destroy_nodes(struct applesmc_node_group *groups)
+{
+ struct applesmc_node_group *grp;
+ struct applesmc_dev_attr *node;
+
+ for (grp = groups; grp->nodes; grp++) {
+ for (node = grp->nodes; node->sda.dev_attr.attr.name; node++)
+ sysfs_remove_file(&pdev->dev.kobj,
+ &node->sda.dev_attr.attr);
+ kfree(grp->nodes);
+ grp->nodes = NULL;
+ }
+}
+
+/*
+ * applesmc_create_nodes - create a two-dimensional group of sysfs files
+ */
+static int applesmc_create_nodes(struct applesmc_node_group *groups, int num)
+{
+ struct applesmc_node_group *grp;
+ struct applesmc_dev_attr *node;
+ struct attribute *attr;
+ int ret, i;
+
+ for (grp = groups; grp->format; grp++) {
+ grp->nodes = kcalloc(num + 1, sizeof(*node), GFP_KERNEL);
+ if (!grp->nodes) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ for (i = 0; i < num; i++) {
+ node = &grp->nodes[i];
+ sprintf(node->name, grp->format, i + 1);
+ node->sda.index = i;
+ node->sda.dev_attr.show = grp->show;
+ node->sda.dev_attr.store = grp->store;
+ attr = &node->sda.dev_attr.attr;
+ attr->name = node->name;
+ attr->mode = S_IRUGO | (grp->store ? S_IWUSR : 0);
+ ret = sysfs_create_file(&pdev->dev.kobj, attr);
+ if (ret) {
+ attr->name = NULL;
+ goto out;
+ }
+ }
+ }
+
+ return 0;
+out:
+ applesmc_destroy_nodes(groups);
+ return ret;
+}
+
/* Create accelerometer ressources */
static int applesmc_create_accelerometer(void)
{
@@ -1657,7 +1397,6 @@ static int __init applesmc_init(void)
{
int ret;
int count;
- int i;
if (!dmi_check_system(applesmc_whitelist)) {
pr_warn("supported laptop not found!\n");
@@ -1716,24 +1455,9 @@ static int __init applesmc_init(void)
fans_handled++;
}
- for (i = 0;
- temperature_sensors_sets[applesmc_temperature_set][i] != NULL;
- i++) {
- if (temperature_attributes[i] == NULL ||
- label_attributes[i] == NULL) {
- pr_err("More temperature sensors in temperature_sensors_sets (at least %i) than available sysfs files in temperature_attributes (%i), please report this bug\n",
- i, i-1);
- goto out_temperature;
- }
- ret = sysfs_create_file(&pdev->dev.kobj,
- temperature_attributes[i]);
- if (ret)
- goto out_temperature;
- ret = sysfs_create_file(&pdev->dev.kobj,
- label_attributes[i]);
- if (ret)
- goto out_temperature;
- }
+ ret = applesmc_create_nodes(temp_group, smcreg.temp_count);
+ if (ret)
+ goto out_fans;
if (applesmc_accelerometer) {
ret = applesmc_create_accelerometer();
@@ -1783,8 +1507,7 @@ out_accelerometer:
if (applesmc_accelerometer)
applesmc_release_accelerometer();
out_temperature:
- sysfs_remove_group(&pdev->dev.kobj, &label_attributes_group);
- sysfs_remove_group(&pdev->dev.kobj, &temperature_attributes_group);
+ applesmc_destroy_nodes(temp_group);
out_fans:
while (fans_handled)
sysfs_remove_group(&pdev->dev.kobj,
@@ -1815,8 +1538,7 @@ static void __exit applesmc_exit(void)
}
if (applesmc_accelerometer)
applesmc_release_accelerometer();
- sysfs_remove_group(&pdev->dev.kobj, &label_attributes_group);
- sysfs_remove_group(&pdev->dev.kobj, &temperature_attributes_group);
+ applesmc_destroy_nodes(temp_group);
while (fans_handled)
sysfs_remove_group(&pdev->dev.kobj,
&fan_attribute_groups[--fans_handled]);
--
1.7.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 06/11] hwmon: applesmc: Handle new temperature format (rev2)
2010-11-09 15:15 [PATCH 00/11] hwmon: applesmc: Dynamic configuration rewrite (rev2) Henrik Rydberg
` (4 preceding siblings ...)
2010-11-09 15:15 ` [PATCH 05/11] hwmon: applesmc: Dynamic creation of temperature files (rev2) Henrik Rydberg
@ 2010-11-09 15:15 ` Henrik Rydberg
2010-11-10 18:07 ` Guenter Roeck
2010-11-09 15:15 ` [PATCH 07/11] hwmon: applesmc: Extract all features generically (rev2) Henrik Rydberg
` (4 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-09 15:15 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: lm-sensors, linux-kernel, Henrik Rydberg
The recent Macbooks have temperature registers of a new type.
This patch adds the logic to handle them.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hwmon/applesmc.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index 55abb5f..6663721 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -728,13 +728,19 @@ static ssize_t applesmc_show_temperature(struct device *dev,
entry = applesmc_get_entry_by_index(index);
if (IS_ERR(entry))
return PTR_ERR(entry);
+ if (entry->len > 2)
+ return -EINVAL;
- ret = applesmc_read_entry(entry, buffer, 2);
+ ret = applesmc_read_entry(entry, buffer, entry->len);
if (ret)
return ret;
- temp = buffer[0]*1000;
- temp += (buffer[1] >> 6) * 250;
+ if (entry->len == 2) {
+ temp = buffer[0] * 1000;
+ temp += (buffer[1] >> 6) * 250;
+ } else {
+ temp = buffer[0] * 4000;
+ }
return snprintf(sysfsbuf, PAGE_SIZE, "%u\n", temp);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 07/11] hwmon: applesmc: Extract all features generically (rev2)
2010-11-09 15:15 [PATCH 00/11] hwmon: applesmc: Dynamic configuration rewrite (rev2) Henrik Rydberg
` (5 preceding siblings ...)
2010-11-09 15:15 ` [PATCH 06/11] hwmon: applesmc: Handle new temperature format (rev2) Henrik Rydberg
@ 2010-11-09 15:15 ` Henrik Rydberg
2010-11-09 15:15 ` [PATCH 08/11] hwmon: applesmc: Dynamic creation of fan files (rev2) Henrik Rydberg
` (3 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-09 15:15 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: lm-sensors, linux-kernel, Henrik Rydberg
With temperature keys being determined automatically, the dmi match
data is only used to assign features that can easily be detected from
the smc. This patch removes the dmi match data altogether, and reduces
the match table to the main machine models.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hwmon/applesmc.c | 241 +++++++++++----------------------------------
1 files changed, 59 insertions(+), 182 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index 6663721..4e45aa7 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -106,16 +106,6 @@ static const char* fan_speed_keys[] = {
#define to_index(attr) (to_sensor_dev_attr(attr)->index)
-/* Structure to be passed to DMI_MATCH function */
-struct dmi_match_data {
-/* Indicates whether this computer has an accelerometer. */
- int accelerometer;
-/* Indicates whether this computer has light sensors and keyboard backlight. */
- int light;
-/* Indicates which temperature sensors set to use. */
- int temperature_set;
-};
-
/* Dynamic device node attributes */
struct applesmc_dev_attr {
struct sensor_device_attribute sda; /* hwmon attributes */
@@ -146,6 +136,9 @@ static struct applesmc_registers {
unsigned int temp_count; /* number of temperature registers */
unsigned int temp_begin; /* temperature lower index bound */
unsigned int temp_end; /* temperature upper index bound */
+ int num_light_sensors; /* number of light sensors */
+ bool has_accelerometer; /* has motion sensor */
+ bool has_key_backlight; /* has keyboard backlight */
bool init_complete; /* true when fully initialized */
struct applesmc_entry *cache; /* cached key entries */
} smcreg;
@@ -159,12 +152,6 @@ static u8 backlight_state[2];
static struct device *hwmon_dev;
static struct input_polled_dev *applesmc_idev;
-/* Indicates whether this computer has an accelerometer. */
-static unsigned int applesmc_accelerometer;
-
-/* Indicates whether this computer has light sensors and keyboard backlight. */
-static unsigned int applesmc_light;
-
/* The number of fans handled by the driver */
static unsigned int fans_handled;
@@ -431,6 +418,18 @@ static int applesmc_write_key(const char *key, const u8 *buffer, u8 len)
return applesmc_write_entry(entry, buffer, len);
}
+static int applesmc_has_key(const char *key, bool *value)
+{
+ const struct applesmc_entry *entry;
+
+ entry = applesmc_get_entry_by_key(key);
+ if (IS_ERR(entry) && PTR_ERR(entry) != -EINVAL)
+ return PTR_ERR(entry);
+
+ *value = !IS_ERR(entry);
+ return 0;
+}
+
/*
* applesmc_read_motion_sensor - Read motion sensor (X, Y or Z).
*/
@@ -466,7 +465,7 @@ static void applesmc_device_init(void)
int total;
u8 buffer[2];
- if (!applesmc_accelerometer)
+ if (!smcreg.has_accelerometer)
return;
for (total = INIT_TIMEOUT_MSECS; total > 0; total -= INIT_WAIT_MSECS) {
@@ -504,6 +503,7 @@ static int applesmc_get_fan_count(void)
static int applesmc_init_smcreg_try(void)
{
struct applesmc_registers *s = &smcreg;
+ bool left_light_sensor, right_light_sensor;
int ret;
if (s->init_complete)
@@ -528,9 +528,27 @@ static int applesmc_init_smcreg_try(void)
return ret;
s->temp_count = s->temp_end - s->temp_begin;
+ ret = applesmc_has_key(LIGHT_SENSOR_LEFT_KEY, &left_light_sensor);
+ if (ret)
+ return ret;
+ ret = applesmc_has_key(LIGHT_SENSOR_RIGHT_KEY, &right_light_sensor);
+ if (ret)
+ return ret;
+ ret = applesmc_has_key(MOTION_SENSOR_KEY, &s->has_accelerometer);
+ if (ret)
+ return ret;
+ ret = applesmc_has_key(BACKLIGHT_KEY, &s->has_key_backlight);
+ if (ret)
+ return ret;
+
+ s->num_light_sensors = left_light_sensor + right_light_sensor;
s->init_complete = true;
- pr_info("key=%d temp=%d\n", s->key_count, s->temp_count);
+ pr_info("key=%d temp=%d acc=%d lux=%d kbd=%d\n",
+ s->key_count, s->temp_count,
+ s->has_accelerometer,
+ s->num_light_sensors,
+ s->has_key_backlight);
return 0;
}
@@ -579,7 +597,7 @@ static int applesmc_probe(struct platform_device *dev)
/* Synchronize device with memorized backlight state */
static int applesmc_pm_resume(struct device *dev)
{
- if (applesmc_light)
+ if (smcreg.has_key_backlight)
applesmc_write_key(BACKLIGHT_KEY, backlight_state, 2);
return 0;
}
@@ -1107,23 +1125,6 @@ static struct applesmc_node_group temp_group[] = {
/* Module stuff */
/*
- * applesmc_dmi_match - found a match. return one, short-circuiting the hunt.
- */
-static int applesmc_dmi_match(const struct dmi_system_id *id)
-{
- struct dmi_match_data* dmi_data = id->driver_data;
- pr_info("%s detected:\n", id->ident);
- applesmc_accelerometer = dmi_data->accelerometer;
- pr_info(" - Model %s accelerometer\n",
- applesmc_accelerometer ? "with" : "without");
- applesmc_light = dmi_data->light;
- pr_info(" - Model %s light sensors and backlight\n",
- applesmc_light ? "with" : "without");
-
- return 1;
-}
-
-/*
* applesmc_destroy_nodes - remove files and free associated memory
*/
static void applesmc_destroy_nodes(struct applesmc_node_group *groups)
@@ -1237,165 +1238,38 @@ static void applesmc_release_accelerometer(void)
input_free_polled_device(applesmc_idev);
sysfs_remove_group(&pdev->dev.kobj, &accelerometer_attributes_group);
}
-
-static __initdata struct dmi_match_data applesmc_dmi_data[] = {
-/* MacBook Pro: accelerometer, backlight and temperature set 0 */
- { .accelerometer = 1, .light = 1, .temperature_set = 0 },
-/* MacBook2: accelerometer and temperature set 1 */
- { .accelerometer = 1, .light = 0, .temperature_set = 1 },
-/* MacBook: accelerometer and temperature set 2 */
- { .accelerometer = 1, .light = 0, .temperature_set = 2 },
-/* MacMini: temperature set 3 */
- { .accelerometer = 0, .light = 0, .temperature_set = 3 },
-/* MacPro: temperature set 4 */
- { .accelerometer = 0, .light = 0, .temperature_set = 4 },
-/* iMac: temperature set 5 */
- { .accelerometer = 0, .light = 0, .temperature_set = 5 },
-/* MacBook3, MacBook4: accelerometer and temperature set 6 */
- { .accelerometer = 1, .light = 0, .temperature_set = 6 },
-/* MacBook Air: accelerometer, backlight and temperature set 7 */
- { .accelerometer = 1, .light = 1, .temperature_set = 7 },
-/* MacBook Pro 4: accelerometer, backlight and temperature set 8 */
- { .accelerometer = 1, .light = 1, .temperature_set = 8 },
-/* MacBook Pro 3: accelerometer, backlight and temperature set 9 */
- { .accelerometer = 1, .light = 1, .temperature_set = 9 },
-/* iMac 5: light sensor only, temperature set 10 */
- { .accelerometer = 0, .light = 0, .temperature_set = 10 },
-/* MacBook 5: accelerometer, backlight and temperature set 11 */
- { .accelerometer = 1, .light = 1, .temperature_set = 11 },
-/* MacBook Pro 5: accelerometer, backlight and temperature set 12 */
- { .accelerometer = 1, .light = 1, .temperature_set = 12 },
-/* iMac 8: light sensor only, temperature set 13 */
- { .accelerometer = 0, .light = 0, .temperature_set = 13 },
-/* iMac 6: light sensor only, temperature set 14 */
- { .accelerometer = 0, .light = 0, .temperature_set = 14 },
-/* MacBook Air 2,1: accelerometer, backlight and temperature set 15 */
- { .accelerometer = 1, .light = 1, .temperature_set = 15 },
-/* MacPro3,1: temperature set 16 */
- { .accelerometer = 0, .light = 0, .temperature_set = 16 },
-/* iMac 9,1: light sensor only, temperature set 17 */
- { .accelerometer = 0, .light = 0, .temperature_set = 17 },
-/* MacBook Pro 2,2: accelerometer, backlight and temperature set 18 */
- { .accelerometer = 1, .light = 1, .temperature_set = 18 },
-/* MacBook Pro 5,3: accelerometer, backlight and temperature set 19 */
- { .accelerometer = 1, .light = 1, .temperature_set = 19 },
-/* MacBook Pro 5,4: accelerometer, backlight and temperature set 20 */
- { .accelerometer = 1, .light = 1, .temperature_set = 20 },
-/* MacBook Pro 6,2: accelerometer, backlight and temperature set 21 */
- { .accelerometer = 1, .light = 1, .temperature_set = 21 },
-/* MacBook Pro 7,1: accelerometer, backlight and temperature set 22 */
- { .accelerometer = 1, .light = 1, .temperature_set = 22 },
-/* MacBook Air 3,1: accelerometer, backlight and temperature set 23 */
- { .accelerometer = 0, .light = 0, .temperature_set = 23 },
-};
+static int applesmc_dmi_match(const struct dmi_system_id *id)
+{
+ return 1;
+}
/* Note that DMI_MATCH(...,"MacBook") will match "MacBookPro1,1".
* So we need to put "Apple MacBook Pro" before "Apple MacBook". */
static __initdata struct dmi_system_id applesmc_whitelist[] = {
- { applesmc_dmi_match, "Apple MacBook Air 3", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "MacBookAir3") },
- &applesmc_dmi_data[23]},
- { applesmc_dmi_match, "Apple MacBook Air 2", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "MacBookAir2") },
- &applesmc_dmi_data[15]},
{ applesmc_dmi_match, "Apple MacBook Air", {
DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
DMI_MATCH(DMI_PRODUCT_NAME, "MacBookAir") },
- &applesmc_dmi_data[7]},
- { applesmc_dmi_match, "Apple MacBook Pro 7", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro7") },
- &applesmc_dmi_data[22]},
- { applesmc_dmi_match, "Apple MacBook Pro 5,4", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro5,4") },
- &applesmc_dmi_data[20]},
- { applesmc_dmi_match, "Apple MacBook Pro 5,3", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro5,3") },
- &applesmc_dmi_data[19]},
- { applesmc_dmi_match, "Apple MacBook Pro 6", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro6") },
- &applesmc_dmi_data[21]},
- { applesmc_dmi_match, "Apple MacBook Pro 5", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro5") },
- &applesmc_dmi_data[12]},
- { applesmc_dmi_match, "Apple MacBook Pro 4", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro4") },
- &applesmc_dmi_data[8]},
- { applesmc_dmi_match, "Apple MacBook Pro 3", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro3") },
- &applesmc_dmi_data[9]},
- { applesmc_dmi_match, "Apple MacBook Pro 2,2", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple Computer, Inc."),
- DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro2,2") },
- &applesmc_dmi_data[18]},
+ },
{ applesmc_dmi_match, "Apple MacBook Pro", {
DMI_MATCH(DMI_BOARD_VENDOR,"Apple"),
DMI_MATCH(DMI_PRODUCT_NAME,"MacBookPro") },
- &applesmc_dmi_data[0]},
- { applesmc_dmi_match, "Apple MacBook (v2)", {
- DMI_MATCH(DMI_BOARD_VENDOR,"Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME,"MacBook2") },
- &applesmc_dmi_data[1]},
- { applesmc_dmi_match, "Apple MacBook (v3)", {
- DMI_MATCH(DMI_BOARD_VENDOR,"Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME,"MacBook3") },
- &applesmc_dmi_data[6]},
- { applesmc_dmi_match, "Apple MacBook 4", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "MacBook4") },
- &applesmc_dmi_data[6]},
- { applesmc_dmi_match, "Apple MacBook 5", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "MacBook5") },
- &applesmc_dmi_data[11]},
+ },
{ applesmc_dmi_match, "Apple MacBook", {
DMI_MATCH(DMI_BOARD_VENDOR,"Apple"),
DMI_MATCH(DMI_PRODUCT_NAME,"MacBook") },
- &applesmc_dmi_data[2]},
+ },
{ applesmc_dmi_match, "Apple Macmini", {
DMI_MATCH(DMI_BOARD_VENDOR,"Apple"),
DMI_MATCH(DMI_PRODUCT_NAME,"Macmini") },
- &applesmc_dmi_data[3]},
- { applesmc_dmi_match, "Apple MacPro2", {
- DMI_MATCH(DMI_BOARD_VENDOR,"Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME,"MacPro2") },
- &applesmc_dmi_data[4]},
- { applesmc_dmi_match, "Apple MacPro3", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "MacPro3") },
- &applesmc_dmi_data[16]},
+ },
{ applesmc_dmi_match, "Apple MacPro", {
DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
DMI_MATCH(DMI_PRODUCT_NAME, "MacPro") },
- &applesmc_dmi_data[4]},
- { applesmc_dmi_match, "Apple iMac 9,1", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
- DMI_MATCH(DMI_PRODUCT_NAME, "iMac9,1") },
- &applesmc_dmi_data[17]},
- { applesmc_dmi_match, "Apple iMac 8", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "iMac8") },
- &applesmc_dmi_data[13]},
- { applesmc_dmi_match, "Apple iMac 6", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "iMac6") },
- &applesmc_dmi_data[14]},
- { applesmc_dmi_match, "Apple iMac 5", {
- DMI_MATCH(DMI_BOARD_VENDOR, "Apple"),
- DMI_MATCH(DMI_PRODUCT_NAME, "iMac5") },
- &applesmc_dmi_data[10]},
+ },
{ applesmc_dmi_match, "Apple iMac", {
DMI_MATCH(DMI_BOARD_VENDOR,"Apple"),
DMI_MATCH(DMI_PRODUCT_NAME,"iMac") },
- &applesmc_dmi_data[5]},
+ },
{ .ident = NULL }
};
@@ -1465,18 +1339,20 @@ static int __init applesmc_init(void)
if (ret)
goto out_fans;
- if (applesmc_accelerometer) {
+ if (smcreg.has_accelerometer) {
ret = applesmc_create_accelerometer();
if (ret)
goto out_temperature;
}
- if (applesmc_light) {
+ if (smcreg.num_light_sensors) {
/* Add light sensor file */
ret = sysfs_create_file(&pdev->dev.kobj, &dev_attr_light.attr);
if (ret)
goto out_accelerometer;
+ }
+ if (smcreg.has_key_backlight) {
/* Create the workqueue */
applesmc_led_wq = create_singlethread_workqueue("applesmc-led");
if (!applesmc_led_wq) {
@@ -1501,16 +1377,16 @@ static int __init applesmc_init(void)
return 0;
out_light_ledclass:
- if (applesmc_light)
+ if (smcreg.has_key_backlight)
led_classdev_unregister(&applesmc_backlight);
out_light_wq:
- if (applesmc_light)
+ if (smcreg.has_key_backlight)
destroy_workqueue(applesmc_led_wq);
out_light_sysfs:
- if (applesmc_light)
+ if (smcreg.num_light_sensors)
sysfs_remove_file(&pdev->dev.kobj, &dev_attr_light.attr);
out_accelerometer:
- if (applesmc_accelerometer)
+ if (smcreg.has_accelerometer)
applesmc_release_accelerometer();
out_temperature:
applesmc_destroy_nodes(temp_group);
@@ -1537,12 +1413,13 @@ out:
static void __exit applesmc_exit(void)
{
hwmon_device_unregister(hwmon_dev);
- if (applesmc_light) {
+ if (smcreg.has_key_backlight) {
led_classdev_unregister(&applesmc_backlight);
destroy_workqueue(applesmc_led_wq);
- sysfs_remove_file(&pdev->dev.kobj, &dev_attr_light.attr);
}
- if (applesmc_accelerometer)
+ if (smcreg.num_light_sensors)
+ sysfs_remove_file(&pdev->dev.kobj, &dev_attr_light.attr);
+ if (smcreg.has_accelerometer)
applesmc_release_accelerometer();
applesmc_destroy_nodes(temp_group);
while (fans_handled)
--
1.7.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 08/11] hwmon: applesmc: Dynamic creation of fan files (rev2)
2010-11-09 15:15 [PATCH 00/11] hwmon: applesmc: Dynamic configuration rewrite (rev2) Henrik Rydberg
` (6 preceding siblings ...)
2010-11-09 15:15 ` [PATCH 07/11] hwmon: applesmc: Extract all features generically (rev2) Henrik Rydberg
@ 2010-11-09 15:15 ` Henrik Rydberg
2010-11-10 18:09 ` Guenter Roeck
2010-11-09 15:15 ` [PATCH 09/11] hwmon: applesmc: Simplify feature sysfs handling (rev2) Henrik Rydberg
` (2 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-09 15:15 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: lm-sensors, linux-kernel, Henrik Rydberg
With the dynamic temperature group in place, the setup of fans
can be simplified. This patch sets up the fans dynamically, removing
a hundred lines of code.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hwmon/applesmc.c | 188 +++++++++++-----------------------------------
1 files changed, 43 insertions(+), 145 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index 4e45aa7..733d4c9 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -77,20 +77,15 @@
#define FANS_COUNT "FNum" /* r-o ui8 */
#define FANS_MANUAL "FS! " /* r-w ui16 */
-#define FAN_ACTUAL_SPEED "F0Ac" /* r-o fpe2 (2 bytes) */
-#define FAN_MIN_SPEED "F0Mn" /* r-o fpe2 (2 bytes) */
-#define FAN_MAX_SPEED "F0Mx" /* r-o fpe2 (2 bytes) */
-#define FAN_SAFE_SPEED "F0Sf" /* r-o fpe2 (2 bytes) */
-#define FAN_TARGET_SPEED "F0Tg" /* r-w fpe2 (2 bytes) */
-#define FAN_POSITION "F0ID" /* r-o char[16] */
+#define FAN_ID_FMT "F%dID" /* r-o char[16] */
/* List of keys used to read/write fan speeds */
-static const char* fan_speed_keys[] = {
- FAN_ACTUAL_SPEED,
- FAN_MIN_SPEED,
- FAN_MAX_SPEED,
- FAN_SAFE_SPEED,
- FAN_TARGET_SPEED
+static const char *const fan_speed_fmt[] = {
+ "F%dAc", /* actual speed */
+ "F%dMn", /* minimum speed (rw) */
+ "F%dMx", /* maximum speed */
+ "F%dSf", /* safe speed - not all models */
+ "F%dTg", /* target speed (manual: rw) */
};
#define INIT_TIMEOUT_MSECS 5000 /* wait up to 5s for device init ... */
@@ -104,7 +99,8 @@ static const char* fan_speed_keys[] = {
#define SENSOR_Y 1
#define SENSOR_Z 2
-#define to_index(attr) (to_sensor_dev_attr(attr)->index)
+#define to_index(attr) (to_sensor_dev_attr(attr)->index & 0xffff)
+#define to_option(attr) (to_sensor_dev_attr(attr)->index >> 16)
/* Dynamic device node attributes */
struct applesmc_dev_attr {
@@ -117,6 +113,7 @@ struct applesmc_node_group {
char *format; /* format string */
void *show; /* show function */
void *store; /* store function */
+ int option; /* function argument */
struct applesmc_dev_attr *nodes; /* dynamic node array */
};
@@ -133,6 +130,7 @@ struct applesmc_entry {
static struct applesmc_registers {
struct mutex mutex; /* register read/write mutex */
unsigned int key_count; /* number of SMC registers */
+ unsigned int fan_count; /* number of fans */
unsigned int temp_count; /* number of temperature registers */
unsigned int temp_begin; /* temperature lower index bound */
unsigned int temp_end; /* temperature upper index bound */
@@ -152,9 +150,6 @@ static u8 backlight_state[2];
static struct device *hwmon_dev;
static struct input_polled_dev *applesmc_idev;
-/* The number of fans handled by the driver */
-static unsigned int fans_handled;
-
/*
* Last index written to key_at_index sysfs file, and value to use for all other
* key_at_index_* sysfs files.
@@ -482,28 +477,13 @@ static void applesmc_device_init(void)
}
/*
- * applesmc_get_fan_count - get the number of fans.
- */
-static int applesmc_get_fan_count(void)
-{
- int ret;
- u8 buffer[1];
-
- ret = applesmc_read_key(FANS_COUNT, buffer, 1);
-
- 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;
bool left_light_sensor, right_light_sensor;
+ u8 tmp[1];
int ret;
if (s->init_complete)
@@ -520,6 +500,11 @@ static int applesmc_init_smcreg_try(void)
if (!s->cache)
return -ENOMEM;
+ ret = applesmc_read_key(FANS_COUNT, tmp, 1);
+ if (ret)
+ return ret;
+ s->fan_count = tmp[0];
+
ret = applesmc_get_lower_bound(&s->temp_begin, "T");
if (ret)
return ret;
@@ -544,8 +529,8 @@ static int applesmc_init_smcreg_try(void)
s->num_light_sensors = left_light_sensor + right_light_sensor;
s->init_complete = true;
- pr_info("key=%d temp=%d acc=%d lux=%d kbd=%d\n",
- s->key_count, s->temp_count,
+ pr_info("key=%d fan=%d temp=%d acc=%d lux=%d kbd=%d\n",
+ s->key_count, s->fan_count, s->temp_count,
s->has_accelerometer,
s->num_light_sensors,
s->has_key_backlight);
@@ -770,14 +755,8 @@ static ssize_t applesmc_show_fan_speed(struct device *dev,
unsigned int speed = 0;
char newkey[5];
u8 buffer[2];
- struct sensor_device_attribute_2 *sensor_attr =
- to_sensor_dev_attr_2(attr);
- newkey[0] = fan_speed_keys[sensor_attr->nr][0];
- newkey[1] = '0' + sensor_attr->index;
- newkey[2] = fan_speed_keys[sensor_attr->nr][2];
- newkey[3] = fan_speed_keys[sensor_attr->nr][3];
- newkey[4] = 0;
+ sprintf(newkey, fan_speed_fmt[to_option(attr)], to_index(attr));
ret = applesmc_read_key(newkey, buffer, 2);
speed = ((buffer[0] << 8 | buffer[1]) >> 2);
@@ -796,19 +775,13 @@ static ssize_t applesmc_store_fan_speed(struct device *dev,
u32 speed;
char newkey[5];
u8 buffer[2];
- struct sensor_device_attribute_2 *sensor_attr =
- to_sensor_dev_attr_2(attr);
speed = simple_strtoul(sysfsbuf, NULL, 10);
if (speed > 0x4000) /* Bigger than a 14-bit value */
return -EINVAL;
- newkey[0] = fan_speed_keys[sensor_attr->nr][0];
- newkey[1] = '0' + sensor_attr->index;
- newkey[2] = fan_speed_keys[sensor_attr->nr][2];
- newkey[3] = fan_speed_keys[sensor_attr->nr][3];
- newkey[4] = 0;
+ sprintf(newkey, fan_speed_fmt[to_option(attr)], to_index(attr));
buffer[0] = (speed >> 6) & 0xff;
buffer[1] = (speed << 2) & 0xff;
@@ -821,15 +794,14 @@ static ssize_t applesmc_store_fan_speed(struct device *dev,
}
static ssize_t applesmc_show_fan_manual(struct device *dev,
- struct device_attribute *devattr, char *sysfsbuf)
+ struct device_attribute *attr, char *sysfsbuf)
{
int ret;
u16 manual = 0;
u8 buffer[2];
- struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
ret = applesmc_read_key(FANS_MANUAL, buffer, 2);
- manual = ((buffer[0] << 8 | buffer[1]) >> attr->index) & 0x01;
+ manual = ((buffer[0] << 8 | buffer[1]) >> to_index(attr)) & 0x01;
if (ret)
return ret;
@@ -838,14 +810,13 @@ static ssize_t applesmc_show_fan_manual(struct device *dev,
}
static ssize_t applesmc_store_fan_manual(struct device *dev,
- struct device_attribute *devattr,
+ struct device_attribute *attr,
const char *sysfsbuf, size_t count)
{
int ret;
u8 buffer[2];
u32 input;
u16 val;
- struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
input = simple_strtoul(sysfsbuf, NULL, 10);
@@ -855,9 +826,9 @@ static ssize_t applesmc_store_fan_manual(struct device *dev,
goto out;
if (input)
- val = val | (0x01 << attr->index);
+ val = val | (0x01 << to_index(attr));
else
- val = val & ~(0x01 << attr->index);
+ val = val & ~(0x01 << to_index(attr));
buffer[0] = (val >> 8) & 0xFF;
buffer[1] = val & 0xFF;
@@ -877,14 +848,8 @@ static ssize_t applesmc_show_fan_position(struct device *dev,
int ret;
char newkey[5];
u8 buffer[17];
- struct sensor_device_attribute_2 *sensor_attr =
- to_sensor_dev_attr_2(attr);
- newkey[0] = FAN_POSITION[0];
- newkey[1] = '0' + sensor_attr->index;
- newkey[2] = FAN_POSITION[2];
- newkey[3] = FAN_POSITION[3];
- newkey[4] = 0;
+ sprintf(newkey, FAN_ID_FMT, to_index(attr));
ret = applesmc_read_key(newkey, buffer, 16);
buffer[16] = 0;
@@ -1058,62 +1023,15 @@ static struct attribute *key_enumeration_attributes[] = {
static const struct attribute_group key_enumeration_group =
{ .attrs = key_enumeration_attributes };
-/*
- * Macro defining SENSOR_DEVICE_ATTR for a fan sysfs entries.
- * - show actual speed
- * - show/store minimum speed
- * - show maximum speed
- * - show safe speed
- * - show/store target speed
- * - show/store manual mode
- */
-#define sysfs_fan_speeds_offset(offset) \
-static SENSOR_DEVICE_ATTR_2(fan##offset##_input, S_IRUGO, \
- applesmc_show_fan_speed, NULL, 0, offset-1); \
-\
-static SENSOR_DEVICE_ATTR_2(fan##offset##_min, S_IRUGO | S_IWUSR, \
- applesmc_show_fan_speed, applesmc_store_fan_speed, 1, offset-1); \
-\
-static SENSOR_DEVICE_ATTR_2(fan##offset##_max, S_IRUGO, \
- applesmc_show_fan_speed, NULL, 2, offset-1); \
-\
-static SENSOR_DEVICE_ATTR_2(fan##offset##_safe, S_IRUGO, \
- applesmc_show_fan_speed, NULL, 3, offset-1); \
-\
-static SENSOR_DEVICE_ATTR_2(fan##offset##_output, S_IRUGO | S_IWUSR, \
- applesmc_show_fan_speed, applesmc_store_fan_speed, 4, offset-1); \
-\
-static SENSOR_DEVICE_ATTR(fan##offset##_manual, S_IRUGO | S_IWUSR, \
- applesmc_show_fan_manual, applesmc_store_fan_manual, offset-1); \
-\
-static SENSOR_DEVICE_ATTR(fan##offset##_label, S_IRUGO, \
- applesmc_show_fan_position, NULL, offset-1); \
-\
-static struct attribute *fan##offset##_attributes[] = { \
- &sensor_dev_attr_fan##offset##_input.dev_attr.attr, \
- &sensor_dev_attr_fan##offset##_min.dev_attr.attr, \
- &sensor_dev_attr_fan##offset##_max.dev_attr.attr, \
- &sensor_dev_attr_fan##offset##_safe.dev_attr.attr, \
- &sensor_dev_attr_fan##offset##_output.dev_attr.attr, \
- &sensor_dev_attr_fan##offset##_manual.dev_attr.attr, \
- &sensor_dev_attr_fan##offset##_label.dev_attr.attr, \
- NULL \
-};
-
-/*
- * Create the needed functions for each fan using the macro defined above
- * (4 fans are supported)
- */
-sysfs_fan_speeds_offset(1);
-sysfs_fan_speeds_offset(2);
-sysfs_fan_speeds_offset(3);
-sysfs_fan_speeds_offset(4);
-
-static const struct attribute_group fan_attribute_groups[] = {
- { .attrs = fan1_attributes },
- { .attrs = fan2_attributes },
- { .attrs = fan3_attributes },
- { .attrs = fan4_attributes },
+static struct applesmc_node_group fan_group[] = {
+ { "fan%d_label", applesmc_show_fan_position },
+ { "fan%d_input", applesmc_show_fan_speed, NULL, 0 },
+ { "fan%d_min", applesmc_show_fan_speed, applesmc_store_fan_speed, 1 },
+ { "fan%d_max", applesmc_show_fan_speed, NULL, 2 },
+ { "fan%d_safe", applesmc_show_fan_speed, NULL, 3 },
+ { "fan%d_output", applesmc_show_fan_speed, applesmc_store_fan_speed, 4 },
+ { "fan%d_manual", applesmc_show_fan_manual, applesmc_store_fan_manual },
+ { }
};
static struct applesmc_node_group temp_group[] = {
@@ -1160,7 +1078,7 @@ static int applesmc_create_nodes(struct applesmc_node_group *groups, int num)
for (i = 0; i < num; i++) {
node = &grp->nodes[i];
sprintf(node->name, grp->format, i + 1);
- node->sda.index = i;
+ node->sda.index = (grp->option << 16) | (i & 0xffff);
node->sda.dev_attr.show = grp->show;
node->sda.dev_attr.store = grp->store;
attr = &node->sda.dev_attr.attr;
@@ -1276,7 +1194,6 @@ static __initdata struct dmi_system_id applesmc_whitelist[] = {
static int __init applesmc_init(void)
{
int ret;
- int count;
if (!dmi_check_system(applesmc_whitelist)) {
pr_warn("supported laptop not found!\n");
@@ -1315,25 +1232,9 @@ static int __init applesmc_init(void)
if (ret)
goto out_name;
- /* create fan files */
- count = applesmc_get_fan_count();
- if (count < 0)
- pr_err("Cannot get the number of fans\n");
- else
- pr_info("%d fans found\n", count);
-
- if (count > 4) {
- count = 4;
- pr_warn("A maximum of 4 fans are supported by this driver\n");
- }
-
- while (fans_handled < count) {
- ret = sysfs_create_group(&pdev->dev.kobj,
- &fan_attribute_groups[fans_handled]);
- if (ret)
- goto out_fans;
- fans_handled++;
- }
+ ret = applesmc_create_nodes(fan_group, smcreg.fan_count);
+ if (ret)
+ goto out_info;
ret = applesmc_create_nodes(temp_group, smcreg.temp_count);
if (ret)
@@ -1391,9 +1292,8 @@ out_accelerometer:
out_temperature:
applesmc_destroy_nodes(temp_group);
out_fans:
- while (fans_handled)
- sysfs_remove_group(&pdev->dev.kobj,
- &fan_attribute_groups[--fans_handled]);
+ applesmc_destroy_nodes(fan_group);
+out_info:
sysfs_remove_group(&pdev->dev.kobj, &key_enumeration_group);
out_name:
sysfs_remove_file(&pdev->dev.kobj, &dev_attr_name.attr);
@@ -1422,9 +1322,7 @@ static void __exit applesmc_exit(void)
if (smcreg.has_accelerometer)
applesmc_release_accelerometer();
applesmc_destroy_nodes(temp_group);
- while (fans_handled)
- sysfs_remove_group(&pdev->dev.kobj,
- &fan_attribute_groups[--fans_handled]);
+ applesmc_destroy_nodes(fan_group);
sysfs_remove_group(&pdev->dev.kobj, &key_enumeration_group);
sysfs_remove_file(&pdev->dev.kobj, &dev_attr_name.attr);
applesmc_destroy_smcreg();
--
1.7.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 09/11] hwmon: applesmc: Simplify feature sysfs handling (rev2)
2010-11-09 15:15 [PATCH 00/11] hwmon: applesmc: Dynamic configuration rewrite (rev2) Henrik Rydberg
` (7 preceding siblings ...)
2010-11-09 15:15 ` [PATCH 08/11] hwmon: applesmc: Dynamic creation of fan files (rev2) Henrik Rydberg
@ 2010-11-09 15:15 ` Henrik Rydberg
2010-11-10 18:10 ` Guenter Roeck
2010-11-09 15:15 ` [PATCH 10/11] hwmon: applesmc: Silence driver Henrik Rydberg
2010-11-09 15:15 ` [PATCH 11/11] hwmon: applesmc: Update copyright information Henrik Rydberg
10 siblings, 1 reply; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-09 15:15 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: lm-sensors, linux-kernel, Henrik Rydberg
Given the dynamic node construction method, the setup of the
accelerometer, light sensor and keyboard backlight sysfs nodes
can be simplified. This patch does not contain any logic changes.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hwmon/applesmc.c | 169 +++++++++++++++++++++-------------------------
1 files changed, 76 insertions(+), 93 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index 733d4c9..b907c20 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -981,47 +981,27 @@ static struct led_classdev applesmc_backlight = {
.brightness_set = applesmc_brightness_set,
};
-static DEVICE_ATTR(name, 0444, applesmc_name_show, NULL);
-
-static DEVICE_ATTR(position, 0444, applesmc_position_show, NULL);
-static DEVICE_ATTR(calibrate, 0644,
- applesmc_calibrate_show, applesmc_calibrate_store);
-
-static struct attribute *accelerometer_attributes[] = {
- &dev_attr_position.attr,
- &dev_attr_calibrate.attr,
- NULL
+static struct applesmc_node_group info_group[] = {
+ { "name", applesmc_name_show },
+ { "key_count", applesmc_key_count_show },
+ { "key_at_index", applesmc_key_at_index_show, applesmc_key_at_index_store },
+ { "key_at_index_name", applesmc_key_at_index_name_show },
+ { "key_at_index_type", applesmc_key_at_index_type_show },
+ { "key_at_index_data_length", applesmc_key_at_index_data_length_show },
+ { "key_at_index_data", applesmc_key_at_index_read_show },
+ { }
};
-static const struct attribute_group accelerometer_attributes_group =
- { .attrs = accelerometer_attributes };
-
-static DEVICE_ATTR(light, 0444, applesmc_light_show, NULL);
-
-static DEVICE_ATTR(key_count, 0444, applesmc_key_count_show, NULL);
-static DEVICE_ATTR(key_at_index, 0644,
- applesmc_key_at_index_show, applesmc_key_at_index_store);
-static DEVICE_ATTR(key_at_index_name, 0444,
- applesmc_key_at_index_name_show, NULL);
-static DEVICE_ATTR(key_at_index_type, 0444,
- applesmc_key_at_index_type_show, NULL);
-static DEVICE_ATTR(key_at_index_data_length, 0444,
- applesmc_key_at_index_data_length_show, NULL);
-static DEVICE_ATTR(key_at_index_data, 0444,
- applesmc_key_at_index_read_show, NULL);
-
-static struct attribute *key_enumeration_attributes[] = {
- &dev_attr_key_count.attr,
- &dev_attr_key_at_index.attr,
- &dev_attr_key_at_index_name.attr,
- &dev_attr_key_at_index_type.attr,
- &dev_attr_key_at_index_data_length.attr,
- &dev_attr_key_at_index_data.attr,
- NULL
+static struct applesmc_node_group accelerometer_group[] = {
+ { "position", applesmc_position_show },
+ { "calibrate", applesmc_calibrate_show, applesmc_calibrate_store },
+ { }
};
-static const struct attribute_group key_enumeration_group =
- { .attrs = key_enumeration_attributes };
+static struct applesmc_node_group light_sensor_group[] = {
+ { "light", applesmc_light_show },
+ { }
+};
static struct applesmc_node_group fan_group[] = {
{ "fan%d_label", applesmc_show_fan_position },
@@ -1104,8 +1084,10 @@ static int applesmc_create_accelerometer(void)
struct input_dev *idev;
int ret;
- ret = sysfs_create_group(&pdev->dev.kobj,
- &accelerometer_attributes_group);
+ if (!smcreg.has_accelerometer)
+ return 0;
+
+ ret = applesmc_create_nodes(accelerometer_group, 1);
if (ret)
goto out;
@@ -1142,7 +1124,7 @@ out_idev:
input_free_polled_device(applesmc_idev);
out_sysfs:
- sysfs_remove_group(&pdev->dev.kobj, &accelerometer_attributes_group);
+ applesmc_destroy_nodes(accelerometer_group);
out:
pr_warn("driver init failed (ret=%d)!\n", ret);
@@ -1152,10 +1134,45 @@ out:
/* Release all ressources used by the accelerometer */
static void applesmc_release_accelerometer(void)
{
+ if (!smcreg.has_accelerometer)
+ return;
input_unregister_polled_device(applesmc_idev);
input_free_polled_device(applesmc_idev);
- sysfs_remove_group(&pdev->dev.kobj, &accelerometer_attributes_group);
+ applesmc_destroy_nodes(accelerometer_group);
}
+
+static int applesmc_create_light_sensor(void)
+{
+ if (!smcreg.num_light_sensors)
+ return 0;
+ return applesmc_create_nodes(light_sensor_group, 1);
+}
+
+static void applesmc_release_light_sensor(void)
+{
+ if (!smcreg.num_light_sensors)
+ return;
+ applesmc_destroy_nodes(light_sensor_group);
+}
+
+static int applesmc_create_key_backlight(void)
+{
+ if (!smcreg.has_key_backlight)
+ return 0;
+ applesmc_led_wq = create_singlethread_workqueue("applesmc-led");
+ if (!applesmc_led_wq)
+ return -ENOMEM;
+ return led_classdev_register(&pdev->dev, &applesmc_backlight);
+}
+
+static void applesmc_release_key_backlight(void)
+{
+ if (!smcreg.has_key_backlight)
+ return;
+ led_classdev_unregister(&applesmc_backlight);
+ destroy_workqueue(applesmc_led_wq);
+}
+
static int applesmc_dmi_match(const struct dmi_system_id *id)
{
return 1;
@@ -1223,15 +1240,10 @@ static int __init applesmc_init(void)
if (ret)
goto out_device;
- ret = sysfs_create_file(&pdev->dev.kobj, &dev_attr_name.attr);
+ ret = applesmc_create_nodes(info_group, 1);
if (ret)
goto out_smcreg;
- /* Create key enumeration sysfs files */
- ret = sysfs_create_group(&pdev->dev.kobj, &key_enumeration_group);
- if (ret)
- goto out_name;
-
ret = applesmc_create_nodes(fan_group, smcreg.fan_count);
if (ret)
goto out_info;
@@ -1240,32 +1252,17 @@ static int __init applesmc_init(void)
if (ret)
goto out_fans;
- if (smcreg.has_accelerometer) {
- ret = applesmc_create_accelerometer();
- if (ret)
- goto out_temperature;
- }
-
- if (smcreg.num_light_sensors) {
- /* Add light sensor file */
- ret = sysfs_create_file(&pdev->dev.kobj, &dev_attr_light.attr);
- if (ret)
- goto out_accelerometer;
- }
+ ret = applesmc_create_accelerometer();
+ if (ret)
+ goto out_temperature;
- if (smcreg.has_key_backlight) {
- /* Create the workqueue */
- applesmc_led_wq = create_singlethread_workqueue("applesmc-led");
- if (!applesmc_led_wq) {
- ret = -ENOMEM;
- goto out_light_sysfs;
- }
+ ret = applesmc_create_light_sensor();
+ if (ret)
+ goto out_accelerometer;
- /* register as a led device */
- ret = led_classdev_register(&pdev->dev, &applesmc_backlight);
- if (ret < 0)
- goto out_light_wq;
- }
+ ret = applesmc_create_key_backlight();
+ if (ret)
+ goto out_light_sysfs;
hwmon_dev = hwmon_device_register(&pdev->dev);
if (IS_ERR(hwmon_dev)) {
@@ -1278,25 +1275,17 @@ static int __init applesmc_init(void)
return 0;
out_light_ledclass:
- if (smcreg.has_key_backlight)
- led_classdev_unregister(&applesmc_backlight);
-out_light_wq:
- if (smcreg.has_key_backlight)
- destroy_workqueue(applesmc_led_wq);
+ applesmc_release_key_backlight();
out_light_sysfs:
- if (smcreg.num_light_sensors)
- sysfs_remove_file(&pdev->dev.kobj, &dev_attr_light.attr);
+ applesmc_release_light_sensor();
out_accelerometer:
- if (smcreg.has_accelerometer)
- applesmc_release_accelerometer();
+ applesmc_release_accelerometer();
out_temperature:
applesmc_destroy_nodes(temp_group);
out_fans:
applesmc_destroy_nodes(fan_group);
out_info:
- sysfs_remove_group(&pdev->dev.kobj, &key_enumeration_group);
-out_name:
- sysfs_remove_file(&pdev->dev.kobj, &dev_attr_name.attr);
+ applesmc_destroy_nodes(info_group);
out_smcreg:
applesmc_destroy_smcreg();
out_device:
@@ -1313,18 +1302,12 @@ out:
static void __exit applesmc_exit(void)
{
hwmon_device_unregister(hwmon_dev);
- if (smcreg.has_key_backlight) {
- led_classdev_unregister(&applesmc_backlight);
- destroy_workqueue(applesmc_led_wq);
- }
- if (smcreg.num_light_sensors)
- sysfs_remove_file(&pdev->dev.kobj, &dev_attr_light.attr);
- if (smcreg.has_accelerometer)
- applesmc_release_accelerometer();
+ applesmc_release_key_backlight();
+ applesmc_release_light_sensor();
+ applesmc_release_accelerometer();
applesmc_destroy_nodes(temp_group);
applesmc_destroy_nodes(fan_group);
- sysfs_remove_group(&pdev->dev.kobj, &key_enumeration_group);
- sysfs_remove_file(&pdev->dev.kobj, &dev_attr_name.attr);
+ applesmc_destroy_nodes(info_group);
applesmc_destroy_smcreg();
platform_device_unregister(pdev);
platform_driver_unregister(&applesmc_driver);
--
1.7.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 10/11] hwmon: applesmc: Silence driver
2010-11-09 15:15 [PATCH 00/11] hwmon: applesmc: Dynamic configuration rewrite (rev2) Henrik Rydberg
` (8 preceding siblings ...)
2010-11-09 15:15 ` [PATCH 09/11] hwmon: applesmc: Simplify feature sysfs handling (rev2) Henrik Rydberg
@ 2010-11-09 15:15 ` Henrik Rydberg
2010-11-10 18:10 ` Guenter Roeck
2010-11-09 15:15 ` [PATCH 11/11] hwmon: applesmc: Update copyright information Henrik Rydberg
10 siblings, 1 reply; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-09 15:15 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: lm-sensors, linux-kernel, Henrik Rydberg
Make the driver report a single line on success.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hwmon/applesmc.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index b907c20..576dcb4 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -1270,8 +1270,6 @@ static int __init applesmc_init(void)
goto out_light_ledclass;
}
- pr_info("driver successfully loaded\n");
-
return 0;
out_light_ledclass:
@@ -1312,8 +1310,6 @@ static void __exit applesmc_exit(void)
platform_device_unregister(pdev);
platform_driver_unregister(&applesmc_driver);
release_region(APPLESMC_DATA_PORT, APPLESMC_NR_PORTS);
-
- pr_info("driver unloaded\n");
}
module_init(applesmc_init);
--
1.7.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 11/11] hwmon: applesmc: Update copyright information
2010-11-09 15:15 [PATCH 00/11] hwmon: applesmc: Dynamic configuration rewrite (rev2) Henrik Rydberg
` (9 preceding siblings ...)
2010-11-09 15:15 ` [PATCH 10/11] hwmon: applesmc: Silence driver Henrik Rydberg
@ 2010-11-09 15:15 ` Henrik Rydberg
2010-11-10 18:10 ` Guenter Roeck
10 siblings, 1 reply; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-09 15:15 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: lm-sensors, linux-kernel, Henrik Rydberg
With the preceding patches, git blame assigns about half of
the file to the present author. Add a line to the copyright
to reflect this.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hwmon/applesmc.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index 576dcb4..ee91d69 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -4,6 +4,7 @@
* computers.
*
* Copyright (C) 2007 Nicolas Boichat <nicolas@boichat.ch>
+ * Copyright (C) 2010 Henrik Rydberg <rydberg@euromail.se>
*
* Based on hdaps.c driver:
* Copyright (C) 2005 Robert Love <rml@novell.com>
--
1.7.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 11/11] hwmon: applesmc: Update copyright information
2010-11-09 15:15 ` [PATCH 11/11] hwmon: applesmc: Update copyright information Henrik Rydberg
@ 2010-11-10 18:10 ` Guenter Roeck
2010-11-10 18:22 ` Henrik Rydberg
0 siblings, 1 reply; 29+ messages in thread
From: Guenter Roeck @ 2010-11-10 18:10 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Jean Delvare, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org
On Tue, Nov 09, 2010 at 10:15:11AM -0500, Henrik Rydberg wrote:
> With the preceding patches, git blame assigns about half of
> the file to the present author. Add a line to the copyright
> to reflect this.
>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
Applied.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 11/11] hwmon: applesmc: Update copyright information
2010-11-10 18:10 ` Guenter Roeck
@ 2010-11-10 18:22 ` Henrik Rydberg
0 siblings, 0 replies; 29+ messages in thread
From: Henrik Rydberg @ 2010-11-10 18:22 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jean Delvare, lm-sensors@lm-sensors.org,
linux-kernel@vger.kernel.org
On 11/10/2010 07:10 PM, Guenter Roeck wrote:
> On Tue, Nov 09, 2010 at 10:15:11AM -0500, Henrik Rydberg wrote:
>> With the preceding patches, git blame assigns about half of
>> the file to the present author. Add a line to the copyright
>> to reflect this.
>>
>> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
>
> Applied.
>
> Thanks,
> Guenter
>
Thanks to you too,
Henrik
^ permalink raw reply [flat|nested] 29+ messages in thread