| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 27 Jul 2012 20:12:46 +0200
From: "Henrik Rydberg" <rydberg@...omail.se>
To: Guenter Roeck <guenter.roeck@...csson.com>
Cc: Jean Delvare <khali@...ux-fr.org>, lm-sensors@...sensors.org,
linux-kernel@...r.kernel.org, Henrik Rydberg <rydberg@...omail.se>
Subject: [PATCH] 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 <adamsomerville@...il.com>
Tested-by: Hubert Eichner <hubert.georg.eichner@...il.com>
Signed-off-by: Henrik Rydberg <rydberg@...omail.se>
---
Hi Guenter,
It turns out the mid-2012 Macbooks need additional changes in order to
work reliably. Since the needed change is a great improvement also on
other problematic machines, it would make a lot of sense if this patch
could be squeezed into the merge window.
As I mentioned in a previous mail, backporting e70acc100 by itself is
not a good idea, but together with this patch it should be ok.
The patch has been wetted in the forums for a bit.
Thanks,
Henrik
drivers/hwmon/applesmc.c | 70 +++++++++++++++++++++++++++++++-----------------
1 file changed, 46 insertions(+), 24 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index 4d937a1..2827088 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;
--
1.7.11.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists