Bluetooth Problem

[peterychuang]

So I was poking around the Bluetooth, and I noticed that the DSDT contains this:

If (!OSDW ())
{
    Return (UBUF) /* \_SB_.PCI0.URT0.BLTH._CRS.UBUF */
}

Return (ABUF) /* \_SB_.PCI0.URT0.BLTH._CRS.ABUF */

This might be a stupid question, but isn't this bit the same as the SPI stuff that used to require patching to get the keyboard and touchpad working?

[roadrunner2]

@peterychuang The question is far from stupid: you're quite right, this looks very much like SPI stuff and it looks like it would benefit from the generic apple-properties work @l1k has done.

However, on closer examination it turns out the issue is not the missing acpi serial-bus resource, but the missing interrupt information: there's neither an interrupt-resource in the _CRS, nor is there a _GPE name like there is for the SPI device, and hence we get these hci_bcm errors in dmesg:

hci_bcm BCM2E7C:00: BCM irq: -22
hci_bcm BCM2E7C:00: invalid platform data
hci_bcm: probe of BCM2E7C:00 failed with error -22

In fact, I can't find any interrupt info whatsoever for the bluetooth device in the DSDT.

So the first thing to get bluetooth running is to figure out what interrupt it uses. After that, we'll need some small patches to set the oper_speed from the baud apple property, and probably something to call the power-down, power-up, etc acpi methods appropriately (however, I know next to nothing about bluetooth and their drivers in Linux, so take this with a large grain of salt).

[l1k]

Yes I did mention in the commit message of l1k/linux@9ae5c93 that it'll be needed for Bluetooth as well. :-)

Looking at the schematics of the MacBook8,1 I can't find a Bluetooth interrupt pin and apparently none is needed. A quick Google search turns up this:

To a host system, the UART appears as an 8-bit input and output port that it can read from and write to. Whenever the host has data to be sent, it just sends these data to the UART in byte format (8-bit wide), whenever the UART receives data from another serial device it will buffer these data in its FIFO (again 8-bit wide), then it will indicate the availability of these data to the host through an internal register bit, or through a hardware interrupt signal.

So the interrupt is coming from the UART on behalf of the Bluetooth device once data is received into the FIFO.

What's much more interesting are the power management methods which could be used to extend battery life:

• BTPU = BlueTooth Power Up
• BTPD = BlueTooth Power Down
• BTRS = BlueTooth Reset (power down followed by power up)
• BTLP = BlueTooth Low Power (takes argument 0 or 1, apparently toggles sleep mode)

The MBP13,3 has an additional BTRB method. No idea. The methods are called from IOBluetoothHostControllerUARTTransport.kext and various others.

[l1k]

Ah looking at hci_bcm.c I realize this is not about an interrupt for data reception but for host wakeup. On the MB8,1 that pin is called SMC_PME_S4_WAKE_L and goes into the SMC, the SMC then wakes the system. So this happens transparently to the OS on MacBooks and the driver need not care about it.

The invalid platform data error is emitted because neither the device_wakeup nor shutdown GPIO descriptors are set. On MacBooks the driver should instead detect presence of the BTLP and BTPD/BTPU methods. The BTLP method should be called for device wakeup and BTPD/BTPU for shutdown. The driver should cache the ACPI handles of the methods on probe and invoke them from bcm_gpio_set_power(), bcm_suspend_device() and bcm_resume_device().

Actually this looks fairly straightforward.

[roadrunner2]

Ah, silly me, didn't actually look at who was using the interrupt for what. Ok, that indeed simplifies things then.

Regarding sleep and wakeup, the DSDT seems to indicate that BTLP should be called for both, at least on Windows:

    Method (_PTS, 1, NotSerialized)  // _PTS: Prepare To Sleep
        ...
        If (!OSDW ())
        {
            If (Arg0 == 0x03)
            {
                \_SB.PCI0.URT0.BLTH.BTLP (One)
                Sleep (0x03E8)
                \_SB.PCI0.LPCB.EC.EWPM = One
                \_SB.PCI0.LPCB.EC.EWDK = One
            }
        }
    }

    Method (_WAK, 1, NotSerialized)  // _WAK: Wake
    {
        ...
        If (OSDW ()) {}
        ElseIf (Arg0 == 0x03)
        {
            \_SB.PCI0.URT0.BLTH.BTLP (Zero)
        }
    }

This seems a bit odd, though: I (probably naïvely) would expect the runtime PM (bcm_suspend_device and bcm_resume_device) should be calling BTLP, and the system sleep (bcm_suspend and bcm_resume) to call BTPD/BTPU.

And yes, I haven't been able to figure out what the BTRB method is supposed to be for either.

[l1k]

I guess BTLP puts the device in a low power state and enables wakeup. Based on the DSDT snippet you included above it seems Apple also wants this to work in sleep state S3, e.g. to wake the system upon key press on a Bluetooth keyboard. Makes sense to me. For some reason BTLP is not called in the DSDT of the MacBook8,1, this might be a bug.

In any case hci_bcm.c calls BTLP both for runtime suspend as well as system sleep, which is in line with what Apple does in the DSDT, so it seems fine to me. The hci_bcm.c driver only toggles the power enable pin (BTPU/BTPD) upon open() and close() of the device, so apparently it's powered down if Bluetooth isn't used at all.

[peterychuang]

In my machine (14,1), I actually can't find the \_SB.PCI0.URT0.BLTH.BTLP bits @roadrunner2 had.

In case this helps, this is (maybe) all the bluetooth related stuff from DSDT in my machine.

[roadrunner2]

The behaviours seem to be a bit all over the map, then: Looking at MacBook8,1 and MacBook9,1 DSDT's I see that _PTS and _WAK call BTPD and BTPU on the 8, respectively, but on the 9 there are no calls to bluetooth there at all, like on the 14,1.

@l1k I'm slightly confused by your 2nd paragraph about hci_bcm.c: are you saying there's already code there that calls these BTxx acpi methods?

[l1k]

Sorry for not being clear, let me try again. :-)

The combined WiFi/BT chip on these machines has 3 GPIO pins of interest for BT power management: power enable (toggled by BTPU/BTPD), device wake (toggled by BTLP, active low) and host wake (going out of the chip and into the SMC for system wake). I meant to say hci_bcm.c already contains all the necessary code to toggle the GPIO pins, however it assumes that the pins are directly toggled by the driver via the GPIO subsystem. That assumption is not correct on Macs where Apple invented their own abstraction (consisting of the three ACPI methods) to hide the specific pins used on the PCH (which may vary between models).

The driver seems to do the right thing by enabling device wake both for runtime suspend as well as system sleep, I wouldn't worry too much about presence or nonpresence of BTLP in _PTS and _WAK. It's possible that they're invoking it from their Bootcamp driver on newer models, or they just forgot to enable BT power saving on Windows.

So all that's needed is to search for the BTPU/BTPD/BTLP methods in the ->probe hook if x86_apple_machine is true, cache them in struct bcm_device and amend all the places where gpiod_set_value() is called to invoke the appropriate ACPI methods (again, if x86_apple_machine is true).

And perhaps it's also necessary to fetch device properties and adjust baud rate etc.

Let me know if my answer is still too cryptic. :-)

[roadrunner2]

Thanks for the elaboration. Ok, so that basically matches my thinking.

[l1k]

I don't know if anyone has started working on this, but in case noone has I've cooked up this branch. Top-most commit adds support for Apple's Bluetooth ACPI methods, the three preceding commits are from the spi_properties material queued for 4.14. Perhaps someone can test if this works, I don't have one of the affected machines. Thanks.

[Dunedan]

@l1k: I tried out your patch, but it still doesn't work. In fact it doesn't look different to the behavior before, printing the same error messages @roadrunner2 mentioned above. Any idea how to debug this?

[peterychuang]

@l1k I can confirm the patch doesn't work. Below is part of the dmesg:

[    7.708006] Bluetooth: Core ver 2.22
[    7.708051] Bluetooth: HCI device and connection manager initialized
[    7.708054] Bluetooth: HCI socket layer initialized
[    7.708056] Bluetooth: L2CAP socket layer initialized
[    7.708063] Bluetooth: SCO socket layer initialized
[    7.712788] Bluetooth: HCI UART driver ver 2.3
[    7.712789] Bluetooth: HCI UART protocol H4 registered
[    7.712790] Bluetooth: HCI UART protocol BCSP registered
[    7.712790] Bluetooth: HCI UART protocol ATH3K registered
[    7.712791] Bluetooth: HCI UART protocol Three-wire (H5) registered
[    7.712817] Bluetooth: HCI UART protocol Intel registered
[    7.717682] dw-apb-uart.2: ttyS0 at MMIO 0x9282b000 (irq = 20, base_baud = 3000000) is a 16550A
[    7.753856] hci_bcm BCM2E7C:00: BCM irq: -22
[    7.753858] hci_bcm BCM2E7C:00: invalid platform data
[    7.753871] hci_bcm: probe of BCM2E7C:00 failed with error -22
[    7.755161] Bluetooth: HCI UART protocol Broadcom registered
[    7.755161] Bluetooth: HCI UART protocol QCA registered
[    7.755162] Bluetooth: HCI UART protocol AG6XX registered
[   10.383828] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
[   10.383829] Bluetooth: BNEP filters: protocol multicast
[   10.383831] Bluetooth: BNEP socket layer initialized

Edit: some additional error messages while running btattach --bredr /dev/ttyS0 -P bcm

[ 3970.771762] Bluetooth: hci0 command 0xfc45 tx timeout
[ 3978.665055] Bluetooth: hci0: BCM: failed to write clock (-110)
[ 3980.798334] Bluetooth: hci0 command 0x0c03 tx timeout
[ 3988.691712] Bluetooth: hci0: BCM: Reset failed (-110)

[l1k]

@Dunedan @peterychuang: Thanks for testing, looking at the code with a fresh pair of eyeballs I noticed that I had botched the return code of bcm_apple_probe(), it would return 1 on success and the caller was expecting 0 on success. Should be fixed now and I've added a commit on top to emit some debug printks. Thanks!

[peterychuang]

@l1k unfortunately it's still not working. Here are the error messages, including the new debug messages from your last commit:

[    9.902071] Bluetooth: Core ver 2.22
[    9.902082] Bluetooth: HCI device and connection manager initialized
[    9.902084] Bluetooth: HCI socket layer initialized
[    9.902086] Bluetooth: L2CAP socket layer initialized
[    9.902090] Bluetooth: SCO socket layer initialized
[    9.908164] Bluetooth: HCI UART driver ver 2.3
[    9.908166] Bluetooth: HCI UART protocol H4 registered
[    9.908166] Bluetooth: HCI UART protocol BCSP registered
[    9.908167] Bluetooth: HCI UART protocol ATH3K registered
[    9.908167] Bluetooth: HCI UART protocol Three-wire (H5) registered
[    9.908189] Bluetooth: HCI UART protocol Intel registered
[    9.923770] hci_bcm BCM2E7C:00: BCM irq: -22
[    9.923773] hci_bcm BCM2E7C:00: oper_speed=3000000
[    9.923792] hci_bcm BCM2E7C:00: BCM2E7C:00 device registered.
[    9.944515] Bluetooth: HCI UART protocol Broadcom registered
[    9.944517] Bluetooth: HCI UART protocol QCA registered
[    9.944518] Bluetooth: HCI UART protocol AG6XX registered
[   10.146949] dw-apb-uart.2: ttyS0 at MMIO 0x9282b000 (irq = 20, base_baud = 3000000) is a 16550A
[   12.409042] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
[   12.409044] Bluetooth: BNEP filters: protocol multicast
[   12.409047] Bluetooth: BNEP socket layer initialized

The invalid platform data seems to have gone.

[l1k]

@peterychuang: Looks good! The BCM irq: -22 message is normal, we just don't need the IRQ on Macs. What exactly is not working?

BTW I notice that the baud rate (3000000) was already reported by dw-apb-uart without this patch. Perhaps it's not necessary to retrieve the baud device property...