Commit 7bc5a2b exposed the SBS on Apple hardware, resulting in intermittent hangs of several minutes on boot, and failure to detect or report the battery. We fix this two ways: - SMBUS hang should not hang the whole system. Respect the specified timeout by busy waiting instead of sleeping. This fix is already used on MSI hardware. - Stop the SBS from hanging in the first place by introducing a 5us delay before each SMBUS transaction. This fix is the result of experimentation. This particular delay was found to completely fix the problem on an Ivybridge Macbook Pro 13. Hangs were observed with 3us delay but never with 5us. Also, pr_warn if SBS communication fails instead of silently ignoring. Link: https://bugzilla.kernel.org/show_bug.cgi?id=94651 Signed-off-by: Chris Bainbridge <chris.bainbridge@xxxxxxxxx> --- drivers/acpi/ec.c | 14 +++++++++++++- drivers/acpi/sbs.c | 10 +++++++--- drivers/acpi/sbshc.c | 24 ++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 220d640..7c9dacc 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -131,6 +131,7 @@ struct acpi_ec *boot_ec, *first_ec; EXPORT_SYMBOL(first_ec); static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */ +static int EC_FLAGS_APPLE; /* Out-of-spec Apple controller */ static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to be validated */ static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan */ static int EC_FLAGS_CLEAR_ON_RESUME; /* Needs acpi_ec_clear() on boot/resume */ @@ -505,7 +506,7 @@ static int ec_poll(struct acpi_ec *ec) unsigned long usecs = ACPI_EC_UDELAY_POLL; do { /* don't sleep with disabled interrupts */ - if (EC_FLAGS_MSI || irqs_disabled()) { + if (EC_FLAGS_APPLE || EC_FLAGS_MSI || irqs_disabled()) { usecs = ACPI_EC_MSI_UDELAY; udelay(usecs); if (ec_transaction_completed(ec)) @@ -1246,6 +1247,14 @@ static int ec_flag_msi(const struct dmi_system_id *id) return 0; } +/* Apple Macbook needs special treatment, enable it */ +static int ec_flag_apple(const struct dmi_system_id *id) +{ + pr_debug("Detected Apple hardware, enabling workarounds.\n"); + EC_FLAGS_APPLE = 1; + return 0; +} + /* * Clevo M720 notebook actually works ok with IRQ mode, if we lifted * the GPE storm threshold back to 20 @@ -1323,6 +1332,9 @@ static struct dmi_system_id ec_dmi_table[] __initdata = { DMI_MATCH(DMI_SYS_VENDOR, "CLEVO CO."), DMI_MATCH(DMI_PRODUCT_NAME, "W35_37ET"),}, NULL}, { + ec_flag_apple, "Apple", { + DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc.")}, NULL}, + { ec_validate_ecdt, "ASUS hardware", { DMI_MATCH(DMI_BIOS_VENDOR, "ASUS") }, NULL}, { diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c index cd82762..165519e 100644 --- a/drivers/acpi/sbs.c +++ b/drivers/acpi/sbs.c @@ -369,8 +369,11 @@ static int acpi_battery_get_info(struct acpi_battery *battery) info_readers[i].command, (u8 *) battery + info_readers[i].offset); - if (result) + if (result) { + pr_warn("sbs read battery failed (cmd=%x)\n", + info_readers[i].command); break; + } } return result; } @@ -412,7 +415,6 @@ static int acpi_battery_set_alarm(struct acpi_battery *battery) int ret; - if (sbs->manager_present) { ret = acpi_smbus_read(sbs->hc, SMBUS_READ_WORD, ACPI_SBS_MANAGER, 0x01, (u8 *)&value); @@ -442,8 +444,10 @@ static int acpi_ac_get_present(struct acpi_sbs *sbs) result = acpi_smbus_read(sbs->hc, SMBUS_READ_WORD, ACPI_SBS_CHARGER, 0x13, (u8 *) & status); - if (result) + if (result) { + pr_warn("sbs read charger failed\n"); return result; + } /* * The spec requires that bit 4 always be 1. If it's not set, assume diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c index 26e5b50..2d40c24 100644 --- a/drivers/acpi/sbshc.c +++ b/drivers/acpi/sbshc.c @@ -14,6 +14,7 @@ #include <linux/delay.h> #include <linux/module.h> #include <linux/interrupt.h> +#include <linux/dmi.h> #include "sbshc.h" #define PREFIX "ACPI: " @@ -87,6 +88,8 @@ enum acpi_smb_offset { ACPI_SMB_ALARM_DATA = 0x26, /* 2 bytes alarm data */ }; +static int SMBUS_FLAGS_APPLE; + static inline int smb_hc_read(struct acpi_smb_hc *hc, u8 address, u8 *data) { return ec_read(hc->offset + address, data); @@ -132,6 +135,8 @@ static int acpi_smbus_transaction(struct acpi_smb_hc *hc, u8 protocol, } mutex_lock(&hc->lock); + if (SMBUS_FLAGS_APPLE) + udelay(5); if (smb_hc_read(hc, ACPI_SMB_PROTOCOL, &temp)) goto end; if (temp) { @@ -257,12 +262,31 @@ extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle, acpi_ec_query_func func, void *data); +static int smbus_flag_apple(const struct dmi_system_id *d) +{ + SMBUS_FLAGS_APPLE = 1; + return 0; +} + +static struct dmi_system_id acpi_smbus_dmi_table[] = { + { + .callback = smbus_flag_apple, + .ident = "Apple", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc.") + }, + }, + { }, +}; + static int acpi_smbus_hc_add(struct acpi_device *device) { int status; unsigned long long val; struct acpi_smb_hc *hc; + dmi_check_system(acpi_smbus_dmi_table); + if (!device) return -EINVAL; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
