Re: [PATCH 00/12] Thunderbolt hotplug support for Apple hardware (testers needed)
On Tue, Mar 11, 2014 at 02:08:55PM +0100, Andreas Noever wrote:
> That seems to do it. I was afraid that setting OSDW globally would
> affect other parts (USB?) But that does not seem to be the case.
I think it results in the USB ports coming up in XHCI mode by default,
but we can handle that. There's one other thing - the battery switches
from being a control method battery to being an SBS one, and Apple's
implementation seems to break our driver. I've got a trivial patch that
makes it work (attached).
> What is the reason for clearing the PME flag?
It looks like the PME flag gets cleared on the _OSC route in non-Darwin
mode? I'll go back and check that to make sure.
--
Matthew Garrett | mj...@srcf.ucam.org
>From 2a811f91aad146916ca14c768ebc407865aa Mon Sep 17 00:00:00 2001
From: Matthew Garrett
Date: Mon, 10 Mar 2014 14:44:55 -0400
Subject: [PATCH 21/28] ACPI: Don't re-select SBS battery if it's already
selected
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
---
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
Re: [PATCH 00/12] Thunderbolt hotplug support for Apple hardware (testers needed)
On Sat, Mar 8, 2014 at 3:40 AM, Matthew Garrett wrote: > Ok, can you try this one? > That seems to do it. I was afraid that setting OSDW globally would affect other parts (USB?) But that does not seem to be the case. What is the reason for clearing the PME flag? Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/12] Thunderbolt hotplug support for Apple hardware (testers needed)
Ok, can you try this one?
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/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.
*/
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_20030x04
-#define ACPI_OSI_WIN_XP_SP2 0x05
-#define ACPI_OSI_WINSRV_2003_SP10x06
-#define ACPI_OSI_WIN_VISTA 0x07
-#define ACPI_OSI_WINSRV_20080x08
-#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_20030x05
+#define ACPI_OSI_WIN_XP_SP2 0x06
+#define ACPI_OSI_WINSRV_2003_SP10x07
+#define ACPI_OSI_WIN_VISTA 0x08
+#define ACPI_OSI_WINSRV_20080x09
+#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__ */
--
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/12] Thunderbolt hotplug support for Apple hardware (testers needed)
On Wed, Mar 05, 2014 at 12:59:54AM +0100, Andreas Noever wrote: > > I belive that the patch has the same effect as passing > acpi_osi=! acpi_osi=Darwin > to the kernel. The problem with that approach is that it changes the > firmware behaviour quite a lot. In particular it prevents Linux from > taking over pci hotplug control: There's not really any way around this. The method to power up the chip will refuse to run unless the system claims Darwin and nothing else. > I would prefer to find a solution that boots without acpi_osi=Darwin > as seems to trigger quite a lot of ACPI code. My current approach is > to inject a custom OSDW method somewhere into the NHI namespace and to > replace _PTS and _WAK from my driver. I can then wake the controller > with the XRPE method. The last problem is that the PCI code does not > allocate enough (or any) bus numbers below the hotplug ports. I'm > trying to add some quirks to it but the code is not really made for > that... I don't think that's a workable approach - any change in the firmware implementation could break it. If we're going to insert quirks then I think it makes more sense to do it in the PCI layer rather than injecting things into ACPI. -- Matthew Garrett | mj...@srcf.ucam.org -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/12] Thunderbolt hotplug support for Apple hardware (testers needed)
On Tue, Mar 4, 2014 at 1:09 AM, Matthew Garrett wrote:
>
> Actually, turns out there's a much easier way. Can you try this patch?
> I see the Thunderbolt controller after resume, although it doesn't seem
> to be in a working state.
>
> commit 102547d63e2cbbda42a25f650df9a33cf929a385
> Author: Matthew Garrett
> Date: Mon Mar 3 18:49:28 2014 -0500
>
> ACPI: Support _OSI("Darwin") correctly
>
> 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
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 54a20ff..ef8656c 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -156,6 +156,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;
> }
>
>
> --
> Matthew Garrett | mj...@srcf.ucam.org
I belive that the patch has the same effect as passing
acpi_osi=! acpi_osi=Darwin
to the kernel. The problem with that approach is that it changes the
firmware behaviour quite a lot. In particular it prevents Linux from
taking over pci hotplug control:
acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments MSI]
\_SB_.PCI0:_OSC invalid UUID
_OSC request data:1 1f 0
acpi PNP0A08:00: _OSC failed (AE_ERROR); disabling ASPM
Booting with just acpi_osi=Darwin (or without acpi_osi) gives:
acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments MSI]
acpi PNP0A08:00: _OSC: platform does not support [PME]
acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug AER PCIeCapability]
The end result is that that pci hotplog events are not handled. If I
unplug the Thunderbolt Ethernet adapter the device is not removed from
lspci.
I would prefer to find a solution that boots without acpi_osi=Darwin
as seems to trigger quite a lot of ACPI code. My current approach is
to inject a custom OSDW method somewhere into the NHI namespace and to
replace _PTS and _WAK from my driver. I can then wake the controller
with the XRPE method. The last problem is that the PCI code does not
allocate enough (or any) bus numbers below the hotplug ports. I'm
trying to add some quirks to it but the code is not really made for
that...
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/12] Thunderbolt hotplug support for Apple hardware (testers needed)
Actually, turns out there's a much easier way. Can you try this patch?
I see the Thunderbolt controller after resume, although it doesn't seem
to be in a working state.
commit 102547d63e2cbbda42a25f650df9a33cf929a385
Author: Matthew Garrett
Date: Mon Mar 3 18:49:28 2014 -0500
ACPI: Support _OSI("Darwin") correctly
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
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 54a20ff..ef8656c 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -156,6 +156,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;
}
--
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/12] Thunderbolt hotplug support for Apple hardware (testers needed)
On Fri, Nov 29, 2013 at 02:35:37AM +0100, Andreas Noever wrote:> > There are still a number of limitations: > (1) The system must be booted with acpi_osi=Darwin. Otherwise ACPI will cut > power to the controller. > (2) After suspend the controller is gone. I think that ACPI thinks that we > are > Windows and cuts power, even with acpi_osi=Darwin. There's a few ACPI methods involved here. The first is DTLK, which is called by the firmware on resume. It'll cut power to the chip. So, you need to be able to power it back up. However, you only want to power it up if there's a device connected. The XRIL method will return 1 if there's a device connected and 0 otherwise. If there's a device connected, call XRPE with an argument of 1 to power it back up. That works fine if there's no hotplugging involved, but there is so things get a little more complicated. Apple provide an out of band mechanism for receiving notifications on the device. Call the _GPE method and install a gpe handler for the value that you get back. This will be called on every device plug or unplug (possibly just plug? It's been a while since I tested). Call XRIL to confirm that there's a device attached and then call XRPE to power it up. At least, that's my recollection. There's also the SXIO, SXIL, SXLV and SXFP methods that provide some kind of power management, but I'm not sure how those fit in. If you want to confirm that you're supposed to be using these methods, call _DSM on the device with a UUID of C6B7B5A0-1813-1C44-B0C9-FE695EAF949B (see DTGP in the DSDT), arg1 of 1, arg2 of 0. If it's an Apple you'll get back return value of 1 and a buffer containing the string "power-save". If it's not an Apple, or if the device doesn't support power saving, you'll get back a zero or an error that the method wasn't found. -- Matthew Garrett | mj...@srcf.ucam.org -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/12] Thunderbolt hotplug support for Apple hardware (testers needed)
On 2 December 2013 18:53, Andreas Noever wrote:
> On Mon, Dec 2, 2013 at 3:51 AM, Daniel J Blueman wrote:
>> On Friday, 29 November 2013 09:40:02 UTC+8, Andreas Noever wrote:
>>
>> Booting with 3.12.2, your patch series, with acpi_osi=Darwin and no
>> thunderbolt devices plugged, I run into this oops:
>
> Ahh, I should have put a warning (since I spend about 3 hours to
> figure that out when updating from 3.12 to 3.13):
> 3.13 contains
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=498d319bb512992ef0784c278fa03679f2f5649d
> which means that kfifo_put now takes a value instead of a pointer. To
> run the driver on 3.12 try changing the kfifo_put in
> tb_ctl.c:tb_ctl_rx_callback to:
>if (!kfifo_put(&cfg->response_fifo, &pkg)) {
Suspend issues aside, this works perfectly with 3.12 booted with
acpi_osi=Darwin in both thunderbolt ports with the Apple ethernet
adapter and my Buffalo MiniStation.
Great work!
Daniel
--
Daniel J Blueman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/12] Thunderbolt hotplug support for Apple hardware (testers needed)
On Friday, 29 November 2013 09:40:02 UTC+8, Andreas Noever wrote:
> Thunderbolt hotplug is supposed to be implemented by the firmware. But Apple's
> firmeware only initializes devices during boot and ignores hotplugged devices.
> This patch series adds a driver for the Intel Cactus Ridge Thunderbolt
> controller. The driver supports hotplug operations of simple (one PCI device,
> no chaining) thunderbolt devices.
[...]
> (1) and (2) should be solveable given some time. I also have a pretty good
> idea
> on how chaining should work. The main problem is that I have yet to see any
> Thunderbolt device besides the Apple ethernet adapter. If someone with a
> MacBook and access to more complicated TB hardware is willing to run some
> tests: Please contact me.
>
> I will try to debug (4) further and write a seperate bug report for the issue.
Booting with 3.12.2, your patch series, with acpi_osi=Darwin and no
thunderbolt devices plugged, I run into this oops:
dsl3510 :07:00.0: dma_pool_free dls3510 cfg,
8000/ee2429b8 (bad data)
kernel BUG at mm/slub.c:3338!
invalid opcode: [#1] SMP
tb_ctl_free_packet.isra.0+0x21/0x30
tb_cfg_rx_callback+0xbc/0x300
ring_drain_and_free+0x2c2/0x360
tb_cfg_free+0x1b/0x80
thunderbolt_shutdown_and_free
thunderbolt_alloc_and_start
dsl3510_probe
pci_device_probe
driver_probe_device
__driver_attach
? __device_attach
bus_for_each_dev
driver_attach
bus_add_driver
? extcon_class_init
driver_register
? extcon_class_init
__pci_register_driver
dsl3510_init
do_one_initcall
(gdb) list *(tb_ctl_free_packet+0x21)
0x815bb221 is in tb_ctl_free_packet (drivers/thunderbolt/tb_cfg.c:243).
238
239static void tb_ctl_free_packet(struct tb_cfg *cfg, struct
ring_packet *pkg)
240{
241dma_pool_free(cfg->packet_pool, pkg->buffer, pkg->buffer_phy);
242kfree(pkg);
243}
244
245/**
246 * tb_ctl_alloc_packets() - allocate multiple packets safely
247 *
Later, I'll add debug to see which path is being taken in
thunderbolt_alloc_and_start, as I guess that wasn't expected to fail,
though it looks like there's this teardown issue.
Thanks,
Daniel
--
Daniel J Blueman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/12] Thunderbolt hotplug support for Apple hardware (testers needed)
On Friday, 29 November 2013 09:40:02 UTC+8, Andreas Noever wrote: > Thunderbolt hotplug is supposed to be implemented by the firmware. But Apple's > firmeware only initializes devices during boot and ignores hotplugged devices. > This patch series adds a driver for the Intel Cactus Ridge Thunderbolt > controller. The driver supports hotplug operations of simple (one PCI device, > no chaining) thunderbolt devices. [...] > (1) and (2) should be solveable given some time. I also have a pretty good > idea > on how chaining should work. The main problem is that I have yet to see any > Thunderbolt device besides the Apple ethernet adapter. If someone with a > MacBook and access to more complicated TB hardware is willing to run some > tests: Please contact me. > > I will try to debug (4) further and write a seperate bug report for the issue. Superb work! I have a Buffalo MiniStation Thunderbolt [1] and the Apple ethernet adapter, so will give this a spin over the weekend on my mid-2012 MacBook Pro retina with the noted constraints. Thanks, Daniel [1] http://www.buffalo-technology.com/en/products/storage-devices/portable-storage/ministationtm/hd-patu3s-ministation-thunderbolt-portable-ssd/ -- Daniel J Blueman -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
