* [PATCH 1/3] ACPI: Support _OSI("Darwin") correctly
2014-03-11 22:19 Improve ACPI support for Apple hardware Matthew Garrett
@ 2014-03-11 22:20 ` Matthew Garrett
2014-03-11 22:20 ` [PATCH 2/3] ACPI: Don't call PCI OSC on Apple hardware when claiming to be Darwin Matthew Garrett
2014-03-11 22:20 ` [PATCH 3/3] ACPI: Don't re-select SBS battery if it's already selected Matthew Garrett
2 siblings, 0 replies; 6+ messages in thread
From: Matthew Garrett @ 2014-03-11 22:20 UTC (permalink / raw)
To: linux-acpi; +Cc: linux-kernel, lenb, rjw, Matthew Garrett
Apple hardware queries _OSI("Darwin") in order to determine whether the
system is running OS X, and changes firmware behaviour based on the answer.
The most obvious difference in behaviour is that Thunderbolt hardware is
forcibly powered down unless the system is running OS X. The obvious solution
would be to simply add Darwin to the list of supported _OSI strings, but this
causes problems.
Recent Apple hardware includes two separate methods for checking _OSI
strings. The first will check whether Darwin is supported, and if so will
exit. The second will check whether Darwin is supported, but will then
continue to check for further operating systems. If a further operating
system is found then later firmware code will assume that the OS is not OS X.
This results in the unfortunate situation where the Thunderbolt controller is
available at boot time but remains powered down after suspend.
The easiest way to handle this is to special-case it in the Linux-specific
OSI handling code. If we see Darwin, we should answer true and then disable
all other _OSI vendor strings.
Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
drivers/acpi/acpica/utosi.c | 1 +
drivers/acpi/osl.c | 10 ++++++++++
include/acpi/actypes.h | 25 +++++++++++++------------
3 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/drivers/acpi/acpica/utosi.c b/drivers/acpi/acpica/utosi.c
index 8856bd3..202b4da 100644
--- a/drivers/acpi/acpica/utosi.c
+++ b/drivers/acpi/acpica/utosi.c
@@ -62,6 +62,7 @@ ACPI_MODULE_NAME("utosi")
static struct acpi_interface_info acpi_default_supported_interfaces[] = {
/* Operating System Vendor Strings */
+ {"Darwin", NULL, 0, ACPI_OSI_DARWIN}, /* OS X */
{"Windows 2000", NULL, 0, ACPI_OSI_WIN_2000}, /* Windows 2000 */
{"Windows 2001", NULL, 0, ACPI_OSI_WIN_XP}, /* Windows XP */
{"Windows 2001 SP1", NULL, 0, ACPI_OSI_WIN_XP_SP1}, /* Windows XP SP1 */
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index fc1aa79..5bf45c06 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -152,6 +152,16 @@ static u32 acpi_osi_handler(acpi_string interface, u32 supported)
osi_linux.dmi ? " via DMI" : "");
}
+ if (!strcmp("Darwin", interface)) {
+ /*
+ * Apple firmware will behave poorly if it receives positive
+ * answers to "Darwin" and any other OS. Respond positively
+ * to Darwin and then disable all other vendor strings.
+ */
+ acpi_update_interfaces(ACPI_DISABLE_ALL_VENDOR_STRINGS);
+ supported = ACPI_UINT32_MAX;
+ }
+
return supported;
}
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index 68a3ada..4580c67 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -1210,17 +1210,18 @@ struct acpi_memory_list {
#define ACPI_ENABLE_ALL_FEATURE_STRINGS (ACPI_ENABLE_INTERFACES | ACPI_FEATURE_STRINGS)
#define ACPI_ENABLE_ALL_STRINGS (ACPI_ENABLE_INTERFACES | ACPI_VENDOR_STRINGS | ACPI_FEATURE_STRINGS)
-#define ACPI_OSI_WIN_2000 0x01
-#define ACPI_OSI_WIN_XP 0x02
-#define ACPI_OSI_WIN_XP_SP1 0x03
-#define ACPI_OSI_WINSRV_2003 0x04
-#define ACPI_OSI_WIN_XP_SP2 0x05
-#define ACPI_OSI_WINSRV_2003_SP1 0x06
-#define ACPI_OSI_WIN_VISTA 0x07
-#define ACPI_OSI_WINSRV_2008 0x08
-#define ACPI_OSI_WIN_VISTA_SP1 0x09
-#define ACPI_OSI_WIN_VISTA_SP2 0x0A
-#define ACPI_OSI_WIN_7 0x0B
-#define ACPI_OSI_WIN_8 0x0C
+#define ACPI_OSI_DARWIN 0x01
+#define ACPI_OSI_WIN_2000 0x02
+#define ACPI_OSI_WIN_XP 0x03
+#define ACPI_OSI_WIN_XP_SP1 0x04
+#define ACPI_OSI_WINSRV_2003 0x05
+#define ACPI_OSI_WIN_XP_SP2 0x06
+#define ACPI_OSI_WINSRV_2003_SP1 0x07
+#define ACPI_OSI_WIN_VISTA 0x08
+#define ACPI_OSI_WINSRV_2008 0x09
+#define ACPI_OSI_WIN_VISTA_SP1 0x0A
+#define ACPI_OSI_WIN_VISTA_SP2 0x0B
+#define ACPI_OSI_WIN_7 0x0C
+#define ACPI_OSI_WIN_8 0x0D
#endif /* __ACTYPES_H__ */
--
1.8.5.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] ACPI: Don't call PCI OSC on Apple hardware when claiming to be Darwin
2014-03-11 22:19 Improve ACPI support for Apple hardware Matthew Garrett
2014-03-11 22:20 ` [PATCH 1/3] ACPI: Support _OSI("Darwin") correctly Matthew Garrett
@ 2014-03-11 22:20 ` Matthew Garrett
2014-03-11 22:20 ` [PATCH 3/3] ACPI: Don't re-select SBS battery if it's already selected Matthew Garrett
2 siblings, 0 replies; 6+ messages in thread
From: Matthew Garrett @ 2014-03-11 22:20 UTC (permalink / raw)
To: linux-acpi; +Cc: linux-kernel, lenb, rjw, Matthew Garrett
The Apple PCI _OSC method has the following code:
if (LEqual (0x01, OSDW ()))
if (LAnd (LEqual (Arg0, GUID), NEXP)
(do stuff)
else
(fail)
NEXP is a value in high memory and is presumably under the control of the
firmware. No methods set it. The methods that are called in the "do stuff"
path are dummies. Unless there's some additional firmware call in early boot,
there's no way for this call to succeed - and even if it does, it doesn't do
anything.
The easiest way to handle this is simply to ignore it. We know which flags
would be set, so just set them by hand if the platform is running in Darwin
mode.
Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
drivers/acpi/pci_root.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index c1c4102..8d3178c 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -432,6 +432,17 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
acpi_handle handle = device->handle;
/*
+ * Apple always return failure on _OSC calls when _OSI("Darwin") has
+ * been called successfully. We know the feature set supported by the
+ * platform, so avoid calling _OSC at all
+ */
+
+ if (acpi_gbl_osi_data == ACPI_OSI_DARWIN) {
+ root->osc_control_set = ~OSC_PCI_EXPRESS_PME_CONTROL;
+ return;
+ }
+
+ /*
* All supported architectures that use ACPI have support for
* PCI domains, so we indicate this in _OSC support capabilities.
*/
--
1.8.5.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] ACPI: Don't re-select SBS battery if it's already selected
2014-03-11 22:19 Improve ACPI support for Apple hardware Matthew Garrett
2014-03-11 22:20 ` [PATCH 1/3] ACPI: Support _OSI("Darwin") correctly Matthew Garrett
2014-03-11 22:20 ` [PATCH 2/3] ACPI: Don't call PCI OSC on Apple hardware when claiming to be Darwin Matthew Garrett
@ 2014-03-11 22:20 ` Matthew Garrett
2014-03-14 5:39 ` Lan Tianyu
2 siblings, 1 reply; 6+ messages in thread
From: Matthew Garrett @ 2014-03-11 22:20 UTC (permalink / raw)
To: linux-acpi; +Cc: linux-kernel, lenb, rjw, Matthew Garrett
The existing SBS code explicitly sets the selected battery in the SBS
manager regardless of whether the battery in question is already selected.
This causes bus timeouts on Apple hardware. Check for this case and avoid
it.
Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
drivers/acpi/sbs.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
index dbd4849..c386505 100644
--- a/drivers/acpi/sbs.c
+++ b/drivers/acpi/sbs.c
@@ -470,17 +470,25 @@ static struct device_attribute alarm_attr = {
static int acpi_battery_read(struct acpi_battery *battery)
{
int result = 0, saved_present = battery->present;
- u16 state;
+ u16 state, selected, desired;
if (battery->sbs->manager_present) {
result = acpi_smbus_read(battery->sbs->hc, SMBUS_READ_WORD,
ACPI_SBS_MANAGER, 0x01, (u8 *)&state);
if (!result)
battery->present = state & (1 << battery->id);
- state &= 0x0fff;
- state |= 1 << (battery->id + 12);
- acpi_smbus_write(battery->sbs->hc, SMBUS_WRITE_WORD,
- ACPI_SBS_MANAGER, 0x01, (u8 *)&state, 2);
+ /*
+ * Don't switch battery if the correct one is already selected
+ */
+ selected = state & 0xf000;
+ desired = 1 << (battery->id + 12);
+ if (selected != desired) {
+ state &= 0x0fff;
+ state |= desired;
+ acpi_smbus_write(battery->sbs->hc, SMBUS_WRITE_WORD,
+ ACPI_SBS_MANAGER, 0x01,
+ (u8 *)&state, 2);
+ }
} else if (battery->id == 0)
battery->present = 1;
if (result || !battery->present)
--
1.8.5.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] ACPI: Don't re-select SBS battery if it's already selected
2014-03-11 22:20 ` [PATCH 3/3] ACPI: Don't re-select SBS battery if it's already selected Matthew Garrett
@ 2014-03-14 5:39 ` Lan Tianyu
2014-08-11 11:14 ` xavier.gnata
0 siblings, 1 reply; 6+ messages in thread
From: Lan Tianyu @ 2014-03-14 5:39 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-acpi@vger.kernel.org, linux-kernel@vger kernel org,
Len Brown, Rafael J. Wysocki
2014-03-12 6:20 GMT+08:00 Matthew Garrett <matthew.garrett@nebula.com>:
> The existing SBS code explicitly sets the selected battery in the SBS
> manager regardless of whether the battery in question is already selected.
> This causes bus timeouts on Apple hardware. Check for this case and avoid
> it.
>
Hi Matthew:
This patch is to avoid a redundant battery select operation when
the battery is selected. But the symptom "bus timeouts" is a bus transaction
issue, right? Will this happen during other SBS write/read operations? Do we
need to increase the wait time of SMBUS transaction?
> Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
> ---
> drivers/acpi/sbs.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
> index dbd4849..c386505 100644
> --- a/drivers/acpi/sbs.c
> +++ b/drivers/acpi/sbs.c
> @@ -470,17 +470,25 @@ static struct device_attribute alarm_attr = {
> static int acpi_battery_read(struct acpi_battery *battery)
> {
> int result = 0, saved_present = battery->present;
> - u16 state;
> + u16 state, selected, desired;
>
> if (battery->sbs->manager_present) {
> result = acpi_smbus_read(battery->sbs->hc, SMBUS_READ_WORD,
> ACPI_SBS_MANAGER, 0x01, (u8 *)&state);
> if (!result)
> battery->present = state & (1 << battery->id);
> - state &= 0x0fff;
> - state |= 1 << (battery->id + 12);
> - acpi_smbus_write(battery->sbs->hc, SMBUS_WRITE_WORD,
> - ACPI_SBS_MANAGER, 0x01, (u8 *)&state, 2);
> + /*
> + * Don't switch battery if the correct one is already selected
> + */
> + selected = state & 0xf000;
> + desired = 1 << (battery->id + 12);
> + if (selected != desired) {
> + state &= 0x0fff;
> + state |= desired;
> + acpi_smbus_write(battery->sbs->hc, SMBUS_WRITE_WORD,
> + ACPI_SBS_MANAGER, 0x01,
> + (u8 *)&state, 2);
> + }
> } else if (battery->id == 0)
> battery->present = 1;
> if (result || !battery->present)
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best regards
Tianyu Lan
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] ACPI: Don't re-select SBS battery if it's already selected
2014-03-14 5:39 ` Lan Tianyu
@ 2014-08-11 11:14 ` xavier.gnata
0 siblings, 0 replies; 6+ messages in thread
From: xavier.gnata @ 2014-08-11 11:14 UTC (permalink / raw)
To: Lan Tianyu, Matthew Garrett
Cc: linux-acpi@vger.kernel.org, linux-kernel@vger kernel org,
Len Brown, Rafael J. Wysocki
On 14/03/2014 06:39, Lan Tianyu wrote:
> 2014-03-12 6:20 GMT+08:00 Matthew Garrett <matthew.garrett@nebula.com>:
>> The existing SBS code explicitly sets the selected battery in the SBS
>> manager regardless of whether the battery in question is already selected.
>> This causes bus timeouts on Apple hardware. Check for this case and avoid
>> it.
>>
> Hi Matthew:
> This patch is to avoid a redundant battery select operation when
> the battery is selected. But the symptom "bus timeouts" is a bus transaction
> issue, right? Will this happen during other SBS write/read operations? Do we
> need to increase the wait time of SMBUS transaction?
Well, the patch does the job and is needed on a macbook air.
What should we test to make it potentially better?
>
>> Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
>> ---
>> drivers/acpi/sbs.c | 18 +++++++++++++-----
>> 1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
>> index dbd4849..c386505 100644
>> --- a/drivers/acpi/sbs.c
>> +++ b/drivers/acpi/sbs.c
>> @@ -470,17 +470,25 @@ static struct device_attribute alarm_attr = {
>> static int acpi_battery_read(struct acpi_battery *battery)
>> {
>> int result = 0, saved_present = battery->present;
>> - u16 state;
>> + u16 state, selected, desired;
>>
>> if (battery->sbs->manager_present) {
>> result = acpi_smbus_read(battery->sbs->hc, SMBUS_READ_WORD,
>> ACPI_SBS_MANAGER, 0x01, (u8 *)&state);
>> if (!result)
>> battery->present = state & (1 << battery->id);
>> - state &= 0x0fff;
>> - state |= 1 << (battery->id + 12);
>> - acpi_smbus_write(battery->sbs->hc, SMBUS_WRITE_WORD,
>> - ACPI_SBS_MANAGER, 0x01, (u8 *)&state, 2);
>> + /*
>> + * Don't switch battery if the correct one is already selected
>> + */
>> + selected = state & 0xf000;
>> + desired = 1 << (battery->id + 12);
>> + if (selected != desired) {
>> + state &= 0x0fff;
>> + state |= desired;
>> + acpi_smbus_write(battery->sbs->hc, SMBUS_WRITE_WORD,
>> + ACPI_SBS_MANAGER, 0x01,
>> + (u8 *)&state, 2);
>> + }
>> } else if (battery->id == 0)
>> battery->present = 1;
>> if (result || !battery->present)
>> --
>> 1.8.5.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread