Re-send to NXP email addresses for Chin-Ran Lo and Amitkumar Karwar (Marvell wireless IP acquired by NXP) On Tue, Nov 24, 2020 at 11:02 AM Abhishek Pandit-Subedi <abhishekpandit@xxxxxxxxxxxx> wrote: > > Hi Marcel, > > > On Mon, Nov 23, 2020 at 3:46 AM Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote: > > > > Hi Abhishek, > > > > > This patch series adds support for a quirk that will power down the > > > Bluetooth controller when suspending and power it back up when resuming. > > > > > > On Marvell SDIO Bluetooth controllers (SD8897 and SD8997), we are seeing > > > a large number of suspend failures with the following log messages: > > > > > > [ 4764.773873] Bluetooth: hci_cmd_timeout() hci0 command 0x0c14 tx timeout > > > [ 4767.777897] Bluetooth: btmrvl_enable_hs() Host sleep enable command failed > > > [ 4767.777920] Bluetooth: btmrvl_sdio_suspend() HS not actived, suspend failed! > > > [ 4767.777946] dpm_run_callback(): pm_generic_suspend+0x0/0x48 returns -16 > > > [ 4767.777963] call mmc2:0001:2+ returned -16 after 4882288 usecs > > > > > > The daily failure rate with this signature is quite significant and > > > users are likely facing this at least once a day (and some unlucky users > > > are likely facing it multiple times a day). > > > > > > Given the severity, we'd like to power off the controller during suspend > > > so the driver doesn't need to take any action (or block in any way) when > > > suspending and power on during resume. This will break wake-on-bt for > > > users but should improve the reliability of suspend. > > > > > > We don't want to force all users of MVL8897 and MVL8997 to encounter > > > this behavior if they're not affected (especially users that depend on > > > Bluetooth for keyboard/mouse input) so the new behavior is enabled via > > > module param. We are limiting this quirk to only Chromebooks (i.e. > > > laptop). Chromeboxes will continue to have the old behavior since users > > > may depend on BT HID to wake and use the system. > > > > I don’t have a super great feeling with this change. > > > > So historically only hciconfig hci0 up/down was doing a power cycle of the controller and when adding the mgmt interface we moved that to the mgmt interface. In addition we added a special case of power up via hdev->setup. We never had an intention that the kernel otherwise can power up/down the controller as it pleases. > > Aside from the powered setting, the stack is resilient to the > controller crashing (which would be akin to a power off and power on). > From the view of bluez, adapter lost and power down should be almost > equivalent right? ChromeOS has several platforms where Bluetooth has > been reset after suspend, usually due USB being powered off in S3, and > the stack is still well-behaving when that occurs. > > > > > Can we ask Marvell first to investigate why this is fundamentally broken with their hardware? > > +Chin-Ran Lo and +Amitkumar Karwar (added based on changes to > drivers/bluetooth/btmrvl_main.c) > > Could you please take a look at the original cover letter and comment > (or add others at Marvell who may be able to)? Is this a known issue > or a fix? > > >Since what you are proposing is a pretty heavy change that might has side affects. For example the state machine for the mgmt interface has no concept of a power down/up from the kernel. It is all triggered by bluetoothd. > > > > I am careful here since the whole power up/down path is already complicated enough. > > > > That sounds reasonable. I have landed this within ChromeOS so we can > test whether a) this improves stability enough and b) whether the > power off/on in the kernel has significant side effects. This will go > through our automated testing and dogfooding over the next few weeks > and hopefully identify those side-effects. I will re-raise this topic > with updates once we have more data. > > Also, in case it wasn't very clear, I put this behind a module param > that defaults to False because this is so heavy handed. We're only > using it on specific Chromebooks that are exhibiting the worst > behavior and not disabling it wholesale for all btmrvl controllers. > > Thanks > Abhishek > > > Regards > > > > Marcel > >
