Bug 198953

I also see this bluetooth regression also on the Asus T100CHI starting with 4.16-rc1.  Reverting the "streamline runtime PM code" patch also fixes bluetooth on the T100CHI.

Thanks for finding that.

Another approach to the quirk is to change the T100TA entry to 
-			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "T100TA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "T100TA"),
as proposed by Hans de Goede.  This would cover the T100TA/TAS/TAM/TAF...

I added this to the list or regressions of Linux 4.16, but I wonder if the developers in question are aware of it, as many subsystems don't look much at it. Hence:

@Robert: Did you tell Lukas about the regression by mail? 

@jbMacAZ: Is Hans already working on a quirk?

(In reply to Thorsten Leemhuis from comment #2)
> I added this to the list or regressions of Linux 4.16, but I wonder if the
> developers in question are aware of it, as many subsystems don't look much
> at it. Hence:
> 
> @jbMacAZ: Is Hans already working on a quirk?

Hans proposed the edit on the T100/Ubuntu G+ group.  I've built several T100 custom kernels with the proposed edit, but I have not had explicit feedback.  Hans has not been given any feedback on his proposal.  It seems to work fine though.

Thorsten:  I did manage to contact Lukas by email after seeing your comment and I've been testing some debugging suggestions from him on my T100.  We've partially isolated the cause of the problem.  I'll post an update here once we do a few more tests.

@jbMacAZ:  Thanks for the suggestion about using DMI_MATCH.

Hi All,

So as discussed on the bluetooth list I've hit the same problem of the bcm bluetooth device getting stuck in suspend. My solution is a simple revert of the offending commit, as instead of being just a simple cleanup it has unintended and unwanted side-effects. Anyways lets discuss that further on the list.

My main reason for replying here is because of the adding of extra dmi matches for active-low interrupts. I've come up with an alternative approach which actually removes DMI matches, since I believe that we simply need to always treat the IRQ as active-low if it is declared with an "Interrupt" ACPI resource (the other way is with a "GpioInt" resource).

I'll attach my patch for this, if you can test this on the machines needing an extra DMI match that would be great. I would also appreciate it if you can run:

acpidump -o acpidump.vendorname-modelname

And attach the generated acpidump.vendorname-modelname file here.

Regards,

Hans

Hi Again,

I also see that you need to add a new BCM2E## ACPI HID for one machine. I'm also working on a way to make that easier. Specifically I've a patch removing the need to specify the GPIO order in the id table, replacing this with code to actually have the driver figure this out itself. My main reason for this is that I do not believe that there is a 1:1 relationship between ACPI HID and the resource order and so far we're just lucky that our 1:1 coupling of these work, also see:

https://fedorapeople.org/~jwrdegoede/bcm-bt-stuff

Which is an overview of info about bcm bt devices which I've extracted from the ACPI tables from over 50 different laptop models.

I'll attach this patch for testing too.

Regards,

Hans

Created attachment 274739 [details]
Patch to avoid the need for DMI quirks to force Interrupt resources as active-low

Created attachment 274741 [details]
Patch adding BCM2E84 HID, this is a pre-requisite for the Do-not-tie-GPIO-pin-order patch

Created attachment 274743 [details]
Patch: "hci_bcm: Do not tie GPIO pin order to a specific ACPI HID"

Hi Hans

I'm about to attach the acpidump files you requested from the three types of Baytrail machines I have available:  an ASUS-T100TA, an ASUS-T100TAM, and a Toshiba-Encore.  The Toshiba is their original 8 inch model Baytrail tablet, not the latter 10 inch version.

I'm in the process of testing your patches on the above machines and will post another comment with the results from that shortly.

Created attachment 274759 [details]
acpidump for ASUS T100TA

Created attachment 274761 [details]
acpidump for ASUS T100TAM

Created attachment 274763 [details]
acpidump for Toshiba Encore  (original 8 inch tablet model)

Hi Hans

I've tested your patches on all three machines mentioned above (The ASUS T100TA, ASUS T100TAM, and Toshiba Encore) and Bluetooth seems to be working well on all of them.

I started with the 4.16-rc5 kernel, reverted the "Streamline runtime PM code" patch, applied your three patches, then applied one patch of my own (which I'll attach in a minute) which simply adds the Encore BCM2E38 ID to those listed in your third patch.  (I also then add some other necessary Baytrail patches but those don't involve Bluetooth.)

From dmesg I do see the notice your code outputs about the "usually wrong" interrupt polarity for all three devices.  And as far as I can tell in about 1/2 hour of testing, Bluetooth is working well.  I don't see any unusual Bluetooth messages in the dmesg output either.

Thanks for your work on these Baytrail devices.  Let me know if there is anything else I can do.

Bob Howell

Created attachment 274765 [details]
Add Toshiba Encore BCM2E38 to latest version of acpi_match code

Hi Bob,

Thank you for testing this.

As patch of a patch series I've prepared I'm planning on submitting a patch adding a bunch of new ids to the driver. I'll attach my current version. Is it ok if I add:

Reported-and-tested-by: Robert R. Howell <redacted@redacted.dom>

To the commit message for this patch, to credit you for your work on this?

Normally adding a new id like this would be a good candidate for your first kernel patch submission, but since I'm currently doing a bunch of changes to the id table that is not really convenient in this case.

My plan is to submit the irq polarity patch + the id patch which I'm attaching (which still uses the old hardwired gpio ordering stuff) upstream first, with a Cc: stable to get these fixes added to older kernels and then send my other changes as a series on top.

Regards,

Hans

Created attachment 274767 [details]
Patch: Bluetooth: hci_bcm: Add 6 new ACPI HIDs

p.s.

Do you have a git repo or some such with the other Bay Trail patches you use somewhere? As part of my ongoing Bay and Cherry Trail support work I'm trying to get all patches from various places, such as:

https://github.com/burzumishi/linux-baytrail-flexx10/tree/master/kernel/patches/v4.8
https://github.com/muhviehstah/linux-baytrail411/commits/master
https://github.com/jfwells/linux-asus-t100ta

Upstream, so I'm always interested in what patches people use / need. Perhaps there is even a patch in there which would be a good candidate for your first upstream patch submission (you seem to know your way around the kernel, so I believe you would make a good kernel dev).

Anyways this is offtopic for this bug, so please drop me a mail at hdegoede@redhat.com about the the other Bay Trail patches you use (or file a new bug for them).

Hi Hans

Adding the "Reported-and-tested-by" tag would be fine.  

While I've been building kernels with a few special patches for my Baytrail devices for the last couple years I'm just starting to learn how git is used with the main repositories, and how to post patches and commits to the mailing lists.  So I'm more than happy to have you add the Encore information in your patch.

I don't yet have a public repository for the patches I use -- they are mostly from the files at the Ubuntu ASUS T100 group at <https://plus.google.com/communities/117853703024346186936>, with a few extra of my own.  But I'm certainly happy to share those.  Let me think about the best way to do that and I'll get back to you with a direct email on that.

Created attachment 274769 [details]
acpidump for T100CHI

Attached is the acpidump for the T100CHI.  I've applied the three patches to 4.16-5c5 (the revert is still applied.)  Bluetooth still works on the T100CHI through at least one BT timeout cycle.

Also, if there are other helpful patches for the T100*, please share them at the T100/Ubuntu group!  It can take months for accepted patches to actually get to the mainstream.

Thanks,
John

p.s.

Thanks for the apcidumps too.

Question can you attach the nvram file (/lib/firmware/brcm/brcmfmac43####.txt)
you are using on the Toshiba encore ?  I'm wondering what the crystal frequency there is (the xtalfreq=37400 / xtalfreq=26000 line in the textfile), because the loaded patchram file needs to match the frequency.

Hi Hans

I'll attach the brcmfmac43241b4-sdio.txt file I'm using for the Toshiba Encore in just a minute.  There was some confusion in my few year old notes about where I obtained the version I was using, so to eliminate that confusion that I made a new copy from the contents of /sys/firmware/efi/efivars/nvram... on the Encore and tested it to be sure WiFi and Bluetooth were still working OK.  That is the version I'm attaching.  It does show that the Encore uses xtalfreq=37400, like the T100's.

Bob

Created attachment 274797 [details]
brcmfmac43241b4-sdio.txt (nvram) file from Toshiba Encore

Ah, the Encore uses the nvram efivar, good. I'm working together with upstream to directly get the data from the efivar for laptops like these, so that users no longer need to manually copy it.

And thanks for attaching it.

Created attachment 274803 [details]
[PATCH] brcmfmac: Add support for getting nvram contents from EFI variables

Not really relevant to this bug, but FYI here is the patch to make the kernel get the nvram settings for the wifi bits directly from the efi variable.

(In reply to Hans de Goede from comment #25)
> Created attachment 274803 [details]
> [PATCH] brcmfmac: Add support for getting nvram contents from EFI variables
> 
> Not really relevant to this bug, but FYI here is the patch to make the
> kernel get the nvram settings for the wifi bits directly from the efi
> variable.

The new patch works for T100CHI (4.16-rc6).  I renamed the brcmfmac43241b4-sdio.txt, rebooted and wifi still worked.

Hi Hans

I've tested your new NVRAM patch on my T100TA, T100TAM, and Toshiba Encore and it works on all three.  If the ...sdio.txt file is present it uses that, otherwise it successfully gets the information from NVRAM.

There was one side effect you may want to know about.  Originally I had my kernel built with the CONFIG_FW_LOADER_USER_HELPER_FALLBACK option set.  It turns out that when that option is set your NVRAM patch causes an additional 60 second delay in WiFi coming up, as the system waits for some timeout to occur during the "firmware" load.  That is in addition to another 60 second delay which was introduced by CLM related changes in kernel 4.15, as I mentioned in my other bug report at <https://bugzilla.kernel.org/show_bug.cgi?id=198089>.  But both 60 second delays disappear when I build a kernel without CONFIG_FW_LOADER_USER_HELPER_FALLBACK being set, and apparently in most distributions, that option is NOT set.

In testing that NVRAM patch I discovered another issue with the brcmfmac driver, in that after unloading it with "modprobe -r brcmfmac", reloading it with "modprobe brcmfmac" often fails.  However while your patch MAY affect the details of what happens, sort of problem seems to be present at least from 4.15 onward, and perhaps earlier.  The problem may also be intermittent, so sorting this out has become difficult.  With your patch in use, the failure usually occurs as follows.  But something like this (perhaps with slightly different messages) often goes wrong without the NVRAM patch too.  The only way to clear this error is to reboot, so that also makes debugging difficult.  I'll let you know if I can sort out the pattern.

[  135.464064] brcmfmac: brcmf_fw_map_chip_to_name: using brcm/brcmfmac43241b4-sdio.bin for chip 0x004324(17188) rev 0x000005
[  135.464167] usbcore: registered new interface driver brcmfmac
[  140.718849] brcmfmac: brcmf_sdio_bus_stop: Failed to force clock for F2: err -16

Since in normal use I typically don't need to unload and reload the driver, usually this bug doesn't cause a problem.

The original "Bluetooth: hci_bcm: Streamline runtime PM code" patch was reverted in commit b09c61522c81886c34966825f9e5afcbfafac446 which is now incorporated in 4.16-rc7.  I've tested rc7 on my ASUS T100TA and T100TAM machines, and this does fix the problem.  When I apply the other device-id specific patches which have always been needed for my Toshiba Encore, the revert also fixes Bluetooth on that machine.