From 9baeb8fdd7e1e78973ad19a6d7e2bcf7dfeb2498 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 27 Jul 2012 07:11:32 -0700 Subject: hwmon: (acpi_power_meter) Fix build warning Commit c5dec0182256361a3f823316e8fb85263f76efe7 (acpi_power_meter: Use struct dev_pm_ops for power management) introduced the following build warning. It is seen if CONFIG_PM_SLEEP is not defined. acpi_power_meter.c:930:12: warning: acpi_power_meter_resume defined but not used Fix it. Cc: Rafael J. Wysocki Signed-off-by: Guenter Roeck Acked-by: Rafael J. Wysocki --- drivers/hwmon/acpi_power_meter.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers') diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c index 563c02904dd..23ab3c496b0 100644 --- a/drivers/hwmon/acpi_power_meter.c +++ b/drivers/hwmon/acpi_power_meter.c @@ -927,6 +927,8 @@ static int acpi_power_meter_remove(struct acpi_device *device, int type) return 0; } +#ifdef CONFIG_PM_SLEEP + static int acpi_power_meter_resume(struct device *dev) { struct acpi_power_meter_resource *resource; @@ -944,6 +946,8 @@ static int acpi_power_meter_resume(struct device *dev) return 0; } +#endif /* CONFIG_PM_SLEEP */ + static SIMPLE_DEV_PM_OPS(acpi_power_meter_pm, NULL, acpi_power_meter_resume); static struct acpi_driver acpi_power_meter_driver = { -- cgit v1.2.3-70-g09d2 From 2ccc8731e5054058d5fb990744b4f72a79018d34 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Thu, 26 Jul 2012 22:13:47 +0200 Subject: hwmon: (jc42) Simplify hysteresis mask Define JC42_CFG_HYST_MASK as the mask _before_ shifting instead of after shifting. This simplifies the current code slightly, and will simplify the code to come even more. Signed-off-by: Jean Delvare Cc: Guenter Roeck Signed-off-by: Guenter Roeck --- drivers/hwmon/jc42.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'drivers') diff --git a/drivers/hwmon/jc42.c b/drivers/hwmon/jc42.c index e72ba5d2a82..44f189ed414 100644 --- a/drivers/hwmon/jc42.c +++ b/drivers/hwmon/jc42.c @@ -57,7 +57,7 @@ static const unsigned short normal_i2c[] = { #define JC42_CFG_EVENT_LOCK (1 << 7) #define JC42_CFG_SHUTDOWN (1 << 8) #define JC42_CFG_HYST_SHIFT 9 -#define JC42_CFG_HYST_MASK 0x03 +#define JC42_CFG_HYST_MASK (0x03 << 9) /* Capabilities */ #define JC42_CAP_RANGE (1 << 2) @@ -287,8 +287,8 @@ static ssize_t show_temp_crit_hyst(struct device *dev, return PTR_ERR(data); temp = jc42_temp_from_reg(data->temp_crit); - hyst = jc42_hysteresis[(data->config >> JC42_CFG_HYST_SHIFT) - & JC42_CFG_HYST_MASK]; + hyst = jc42_hysteresis[(data->config & JC42_CFG_HYST_MASK) + >> JC42_CFG_HYST_SHIFT]; return sprintf(buf, "%d\n", temp - hyst); } @@ -302,8 +302,8 @@ static ssize_t show_temp_max_hyst(struct device *dev, return PTR_ERR(data); temp = jc42_temp_from_reg(data->temp_max); - hyst = jc42_hysteresis[(data->config >> JC42_CFG_HYST_SHIFT) - & JC42_CFG_HYST_MASK]; + hyst = jc42_hysteresis[(data->config & JC42_CFG_HYST_MASK) + >> JC42_CFG_HYST_SHIFT]; return sprintf(buf, "%d\n", temp - hyst); } @@ -362,8 +362,7 @@ static ssize_t set_temp_crit_hyst(struct device *dev, } mutex_lock(&data->update_lock); - data->config = (data->config - & ~(JC42_CFG_HYST_MASK << JC42_CFG_HYST_SHIFT)) + data->config = (data->config & ~JC42_CFG_HYST_MASK) | (hyst << JC42_CFG_HYST_SHIFT); err = i2c_smbus_write_word_swapped(client, JC42_REG_CONFIG, data->config); -- cgit v1.2.3-70-g09d2 From 5953e2761be088f66fd930dfbf6b36a5a41d82a3 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Thu, 26 Jul 2012 22:18:43 +0200 Subject: hwmon: (jc42) Don't reset hysteresis on device removal Restoring the configuration register on device removal has the side effect of also resetting the hysteresis value. This is inconsistent as the other limits are not reset, only hysteresis. So, following the principle of least surprise, preserve the hysteresis value when restoring the configuration register. Signed-off-by: Jean Delvare Cc: Guenter Roeck Signed-off-by: Guenter Roeck --- drivers/hwmon/jc42.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/hwmon/jc42.c b/drivers/hwmon/jc42.c index 44f189ed414..e21e43c1315 100644 --- a/drivers/hwmon/jc42.c +++ b/drivers/hwmon/jc42.c @@ -534,9 +534,16 @@ static int jc42_remove(struct i2c_client *client) struct jc42_data *data = i2c_get_clientdata(client); hwmon_device_unregister(data->hwmon_dev); sysfs_remove_group(&client->dev.kobj, &jc42_group); - if (data->config != data->orig_config) - i2c_smbus_write_word_swapped(client, JC42_REG_CONFIG, - data->orig_config); + + /* Restore original configuration except hysteresis */ + if ((data->config & ~JC42_CFG_HYST_MASK) != + (data->orig_config & ~JC42_CFG_HYST_MASK)) { + int config; + + config = (data->orig_config & ~JC42_CFG_HYST_MASK) + | (data->config & JC42_CFG_HYST_MASK); + i2c_smbus_write_word_swapped(client, JC42_REG_CONFIG, config); + } return 0; } -- cgit v1.2.3-70-g09d2 From 829917cd7246204d6c5f066c40b66d2b62d0930d Mon Sep 17 00:00:00 2001 From: Henrik Rydberg Date: Fri, 27 Jul 2012 20:12:46 +0200 Subject: hwmon: (applesmc) Decode and act on read/write status codes The behavior of the SMC has changed several times over the years, causing read failures in the driver. It seems the problem can be explained by a shift in SMC speed combined with improper action on status codes. We should first wait for the SMC to settle, which was the most frequent response on the old slow machines. Then, if the SMC is busy, we need to try again later by resending the command. This was the most likely response until 2012. Now, with a shorter wait time, we are again most likely to poll while the SMC is settling, and as a result we see high failure rates on many old and new models. With the distinction between busy and failure, we can also wait longer before retrying, without sacrificing speed. This seems to bring failures down to virtually zero on all models. Tested on: MBA1,1 MBA3,1 MBA5,1 MBA5,2 MBP9,2 Tested-by: Adam Somerville Tested-by: Hubert Eichner Signed-off-by: Henrik Rydberg Signed-off-by: Guenter Roeck --- drivers/hwmon/applesmc.c | 70 +++++++++++++++++++++++++++++++----------------- 1 file changed, 46 insertions(+), 24 deletions(-) (limited to 'drivers') diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c index 4d937a18fad..28270886051 100644 --- a/drivers/hwmon/applesmc.c +++ b/drivers/hwmon/applesmc.c @@ -55,9 +55,9 @@ /* wait up to 32 ms for a status change. */ #define APPLESMC_MIN_WAIT 0x0010 +#define APPLESMC_RETRY_WAIT 0x0100 #define APPLESMC_MAX_WAIT 0x8000 -#define APPLESMC_STATUS_MASK 0x0f #define APPLESMC_READ_CMD 0x10 #define APPLESMC_WRITE_CMD 0x11 #define APPLESMC_GET_KEY_BY_INDEX_CMD 0x12 @@ -162,51 +162,68 @@ static unsigned int key_at_index; static struct workqueue_struct *applesmc_led_wq; /* - * __wait_status - Wait up to 32ms for the status port to get a certain value - * (masked with 0x0f), returning zero if the value is obtained. Callers must + * wait_read - Wait for a byte to appear on SMC port. Callers must * hold applesmc_lock. */ -static int __wait_status(u8 val) +static int wait_read(void) { + u8 status; int us; - - val = val & APPLESMC_STATUS_MASK; - for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) { udelay(us); - if ((inb(APPLESMC_CMD_PORT) & APPLESMC_STATUS_MASK) == val) + status = inb(APPLESMC_CMD_PORT); + /* read: wait for smc to settle */ + if (status & 0x01) return 0; } + pr_warn("wait_read() fail: 0x%02x\n", status); return -EIO; } /* - * special treatment of command port - on newer macbooks, it seems necessary - * to resend the command byte before polling the status again. Callers must - * hold applesmc_lock. + * send_byte - Write to SMC port, retrying when necessary. Callers + * must hold applesmc_lock. */ -static int send_command(u8 cmd) +static int send_byte(u8 cmd, u16 port) { + u8 status; int us; + + outb(cmd, port); for (us = APPLESMC_MIN_WAIT; us < APPLESMC_MAX_WAIT; us <<= 1) { - outb(cmd, APPLESMC_CMD_PORT); udelay(us); - if ((inb(APPLESMC_CMD_PORT) & APPLESMC_STATUS_MASK) == 0x0c) + status = inb(APPLESMC_CMD_PORT); + /* write: wait for smc to settle */ + if (status & 0x02) + continue; + /* ready: cmd accepted, return */ + if (status & 0x04) return 0; + /* timeout: give up */ + if (us << 1 == APPLESMC_MAX_WAIT) + break; + /* busy: long wait and resend */ + udelay(APPLESMC_RETRY_WAIT); + outb(cmd, port); } + + pr_warn("send_byte(0x%02x, 0x%04x) fail: 0x%02x\n", cmd, port, status); return -EIO; } +static int send_command(u8 cmd) +{ + return send_byte(cmd, APPLESMC_CMD_PORT); +} + static int send_argument(const char *key) { int i; - for (i = 0; i < 4; i++) { - outb(key[i], APPLESMC_DATA_PORT); - if (__wait_status(0x04)) + for (i = 0; i < 4; i++) + if (send_byte(key[i], APPLESMC_DATA_PORT)) return -EIO; - } return 0; } @@ -219,11 +236,14 @@ static int read_smc(u8 cmd, const char *key, u8 *buffer, u8 len) return -EIO; } - outb(len, APPLESMC_DATA_PORT); + if (send_byte(len, APPLESMC_DATA_PORT)) { + pr_warn("%.4s: read len fail\n", key); + return -EIO; + } for (i = 0; i < len; i++) { - if (__wait_status(0x05)) { - pr_warn("%.4s: read data fail\n", key); + if (wait_read()) { + pr_warn("%.4s: read data[%d] fail\n", key, i); return -EIO; } buffer[i] = inb(APPLESMC_DATA_PORT); @@ -241,14 +261,16 @@ static int write_smc(u8 cmd, const char *key, const u8 *buffer, u8 len) return -EIO; } - outb(len, APPLESMC_DATA_PORT); + if (send_byte(len, APPLESMC_DATA_PORT)) { + pr_warn("%.4s: write len fail\n", key); + return -EIO; + } for (i = 0; i < len; i++) { - if (__wait_status(0x04)) { + if (send_byte(buffer[i], APPLESMC_DATA_PORT)) { pr_warn("%s: write data fail\n", key); return -EIO; } - outb(buffer[i], APPLESMC_DATA_PORT); } return 0; -- cgit v1.2.3-70-g09d2