[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

Hans de Goede (jwrdegoede@fedoraproject.org) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jwrdegoede@fedoraproject.or
                   |                            |g

--- Comment #5 from Hans de Goede (jwrdegoede@fedoraproject.org) ---
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

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #6 from Hans de Goede (jwrdegoede@fedoraproject.org) ---
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

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #7 from Hans de Goede (jwrdegoede@fedoraproject.org) ---
Created attachment 274739
  --> https://bugzilla.kernel.org/attachment.cgi?id=274739&action=edit
Patch to avoid the need for DMI quirks to force Interrupt resources as
active-low

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #8 from Hans de Goede (jwrdegoede@fedoraproject.org) ---
Created attachment 274741
  --> https://bugzilla.kernel.org/attachment.cgi?id=274741&action=edit
Patch adding BCM2E84 HID, this is a pre-requisite for the
Do-not-tie-GPIO-pin-order patch

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #9 from Hans de Goede (jwrdegoede@fedoraproject.org) ---
Created attachment 274743
  --> https://bugzilla.kernel.org/attachment.cgi?id=274743&action=edit
Patch: "hci_bcm: Do not tie GPIO pin order to a specific ACPI HID"

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #10 from Robert R. Howell (rhowell@uwyo.edu) ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #11 from Robert R. Howell (rhowell@uwyo.edu) ---
Created attachment 274759
  --> https://bugzilla.kernel.org/attachment.cgi?id=274759&action=edit
acpidump for ASUS T100TA

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #12 from Robert R. Howell (rhowell@uwyo.edu) ---
Created attachment 274761
  --> https://bugzilla.kernel.org/attachment.cgi?id=274761&action=edit
acpidump for ASUS T100TAM

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #13 from Robert R. Howell (rhowell@uwyo.edu) ---
Created attachment 274763
  --> https://bugzilla.kernel.org/attachment.cgi?id=274763&action=edit
acpidump for Toshiba Encore  (original 8 inch tablet model)

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #14 from Robert R. Howell (rhowell@uwyo.edu) ---
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

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #15 from Robert R. Howell (rhowell@uwyo.edu) ---
Created attachment 274765
  --> https://bugzilla.kernel.org/attachment.cgi?id=274765&action=edit
Add Toshiba Encore BCM2E38 to latest version of acpi_match code

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #16 from Hans de Goede (jwrdegoede@fedoraproject.org) ---
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

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #17 from Hans de Goede (jwrdegoede@fedoraproject.org) ---
Created attachment 274767
  --> https://bugzilla.kernel.org/attachment.cgi?id=274767&action=edit
Patch: Bluetooth: hci_bcm: Add 6 new ACPI HIDs

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #18 from Hans de Goede (jwrdegoede@fedoraproject.org) ---
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).

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #19 from Robert R. Howell (rhowell@uwyo.edu) ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #20 from jbMacAZ (jbmacbrodie@yahoo.com) ---
Created attachment 274769
  --> https://bugzilla.kernel.org/attachment.cgi?id=274769&action=edit
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

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #21 from Hans de Goede (jwrdegoede@fedoraproject.org) ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #22 from Robert R. Howell (rhowell@uwyo.edu) ---
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

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #23 from Robert R. Howell (rhowell@uwyo.edu) ---
Created attachment 274797
  --> https://bugzilla.kernel.org/attachment.cgi?id=274797&action=edit
brcmfmac43241b4-sdio.txt (nvram) file from Toshiba Encore

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #24 from Hans de Goede (jwrdegoede@fedoraproject.org) ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #25 from Hans de Goede (jwrdegoede@fedoraproject.org) ---
Created attachment 274803
  --> https://bugzilla.kernel.org/attachment.cgi?id=274803&action=edit
[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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #26 from jbMacAZ (jbmacbrodie@yahoo.com) ---
(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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

--- Comment #27 from Robert R. Howell (rhowell@uwyo.edu) ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 198953] hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks bluetooth on ASUS T100TA

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=198953

Robert R. Howell (rhowell@uwyo.edu) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |CODE_FIX

--- Comment #28 from Robert R. Howell (rhowell@uwyo.edu) ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.