Hi Abhishek, > > > > > > > > > > > > Platform: L5.10 + Bluez5.55 + Marvell BT chip > > > > > > > > > > > > Background: Our test team usually try suspend/resume test when > > > > > > transferring large file through BT, to see if the file > > > > > > transfer can be continued > > > > > after suspend/resume. It can works well before L5.10 But we > > > > > found on L5.10, the BT connection lost if we try to > > > > > suspend/resume, so the file transfer(through OBEX Object Push) > > > > > shows failed. Then we found your patches when debugging. > > > > > > > > > > > > Questions: > > > > > > 1. Before L5.10, kernel always keep BT connected during > > > suspend/resume. > > > > > So why we need to disconnect all the BT devices when system > > > > > suspend > > > now? > > > > > > > > > > Bluetooth has often been a source of spurious wakes in the past. > > > > > Using rfkill or masking the wake interrupt were used in the past > > > > > to make this more reliable but this was resulting in instability > > > > > on the controller (controller needs to drop traffic if host is > > > > > asleep and there's no > > > clean way to do that). > > > > > > > > > > The new suspend behavior is the following: > > > > > - All devices get disconnected during suspend. > > > > > - Only HID devices can wake the device from suspend (i.e. Remote > Wake). > > > > > The BT controller will be configured to scan (page scan and/or > > > > > LE passive > > > > > scan) based on currently paired devices. If the device is not > > > > > configured for wakeup (i.e. power/wakeup in sysfs is > > > > > "disabled"), we will not configure this scanning either (see the > > > > > hdev->prevent_wake implementation in btusb for an > > > > > example) > > > > > > > > So for HID devices, they also been disconnected when suspend and > > > > auto > > > reconnected when resume, right? > > > > > > HID will not be auto-reconnected because the peripheral is > > > responsible for reconnecting. Most HID devices do not page scan > > > unless they are pairing (or at least that's what I've found in practice). > > > > What's the peripheral mean here? > > > > The connected Bluetooth device (i.e. mouse, keyboard, etc). > > > > > > > > > > > > > - On resume, devices that support a2dp-sink will be > > > > > automatically reconnected > > > > > > > > > > > > > I just tried a2dp-sink devices(BT headphones), and it wasn't > > > > reconnected > > > after system resume. > > > > I checked your patches which to supported a2dp-sink auto > > > > reconnect, and > > > found them may didn't been included in bluez5.55, can you help > > > confirm that? > > > > > > It looks like bluez5.55 was released on Sep 06 > > > (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg > > > it.ke > > > > rnel.org%2Fpub%2Fscm%2Fbluetooth%2Fbluez.git%2Ftag%2F%3Fh%3D5.55& > > > > amp;data=04%7C01%7Csherry.sun%40nxp.com%7C1f3ec177fe974c90e1a508 > > > > d8b68c4584%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637460 > > > > 060872077285%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ > > > > QIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=EDn > > > > MnLjPIN29nQpmH1%2Fk%2BBnhMq5IZ3xG7BXIEq0xs6A%3D&reserved > > > =0) and my change was merged Sep 14 > > > (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg > > > it.ke > > > > rnel.org%2Fpub%2Fscm%2Fbluetooth%2Fbluez.git%2Fcommit%2F%3Fid%3D > > > > 6611b72600c370ec31795ab48a222594c4afb7ee&data=04%7C01%7Cshe > > > > rry.sun%40nxp.com%7C1f3ec177fe974c90e1a508d8b68c4584%7C686ea1d3b > > > > c2b4c6fa92cd99c5c301635%7C0%7C0%7C637460060872077285%7CUnknown > > > %7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1 > haW > > > > wiLCJXVCI6Mn0%3D%7C1000&sdata=2jYeDwzoy87GA2V2SqilgdA502jeI > > > ZdGd4aeyXH125M%3D&reserved=0). > > > > > > > Okay, got it, so you patches don't been included in bluez5.55. > > > > > > > > > > > This results in more reliable behavior from Bluetooth around > > > > > suspend while allowing Remote Wake to work properly. > > > > > > > > > > > > > Yes, it' reasonable, but I think it may need to reconnect all the > > > > devices > > > which are disconnected due to suspend, right? > > > > Otherwise for the user, when we connect the BT, which means we do > > > > want to use the BT device, but every time when system get into > > > > suspend, we will > > > need to reconnect the device manually, it's really inconvenient. > > > > > > Depending on the profile implemented, not all peer devices will page > > > scan once disconnected. HID devices at least will not page scan and > > > are expected to initiate the reconnect themselves (i.e. suspend, > > > resume and then click your mouse; the mouse will reconnect). > > > > > > If reconnecting to arbitrary profiles is desirable, you can merge > > > this patch > > > (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp > > > atch > > > > work.kernel.org%2Fproject%2Fbluetooth%2Fpatch%2F20201207154903.blue > > > > z.1.I3e043a481273442748bcff0728b2f0e208017cd2%40changeid%2F&d > > > > ata=04%7C01%7Csherry.sun%40nxp.com%7C1f3ec177fe974c90e1a508d8b68 > > > > c4584%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637460060872 > > > > 077285%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l > > > > uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=aNXOfCTXs% > > > 2BISAbYBH2die47qfpqXRmqehSnW1mC8M7U%3D&reserved=0). > > > > > > > But for the most Bluetooth profile, like A2DP-sink/A2DP- > source/OPP/HFP/HSP, I think it is necessary to re-connect it, right? > > So I think it's better to add the auto reconnect support for all those profiles > instead of the A2DP a2dp-sink only here. > > Please correct me if I'm wrong. > > > > I think it should be user configurable and I'm not sure what defaults make > sense for general Linux distributions. For ChromeOS, we only want a2dp-sink. I saw your patch series below didn't been accepted till now, right? And I believe the ChromeOS is just a use case, for the general Linux, I don't think it's a good choice to add these profiles one by one when users need. Since I'm not experienced in Bluetooth, maybe we need some more experienced man to provide some reasonable suggestions. Hi Marcel & Johan & Luiz, can you help to comment it, and the wakelock question below? @marcel@xxxxxxxxxxxx,@johan.hedberg@xxxxxxxxx,@luiz.dentz@xxxxxxxxx > > I've linked the patch series making it more configurable in previous emails > (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch > work.kernel.org%2Fproject%2Fbluetooth%2Fpatch%2F20201207154903.blue > z.1.I3e043a481273442748bcff0728b2f0e208017cd2%40changeid%2F&d > ata=04%7C01%7Csherry.sun%40nxp.com%7Cb06b00a1abc5495a3d2508d8bc > 374c07%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63746629297 > 2854206%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b5gZJ7bbD > e51F0YfGdFQSjUAjLNwhh95jYJuGhsvhjo%3D&reserved=0). > If you agree that behavior is necessary, please make your case to the bluez > maintainer (Luiz). > > > > > > > > > > > 2. I found that the device been disconnected due to suspend > > > > > > won't been > > > > > auto-connected after resume, shouldn't we get the BT device > > > > > auto-connected after resume like wifi devices done? > > > > > > > > > > We do this currently only for Bluetooth headphones (reconnect on > > > > > a2dp-sink service). I'm not familiar with Obex so I don't know > > > > > if this would work for that as well. I did send up a patch > > > > > making reconnect on resume configurable based on service uuid > > > > > that may be relevant to > > > > > this: > > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F% > > > > > 2Fpa > > > > > tch > > > > > > > > > work.kernel.org%2Fproject%2Fbluetooth%2Fpatch%2F20201207154903.blue > > > > > > > > > z.1.I3e043a481273442748bcff0728b2f0e208017cd2%40changeid%2F&d > > > > > > > > > ata=04%7C01%7Csherry.sun%40nxp.com%7C9d41a6725ac1432355c408d8b2 > > > > > > > > > 759700%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6374555654 > > > > > > > > > 22685536%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi > > > > > > > > > V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=6zZda8Yu > > > > > RRbAMkwOxiSwjbJIpvi%2ByMSDWCzVkFkkpzs%3D&reserved=0 > > > > > > > > > > > > > Why here only add a2dp-sink device auto reconnect support? > > > > Why not add all the devices auto reconnect support which are > > > disconnected due to suspend? > > > > > > > > > > 3. For the large file transfer, if the BT been disconnected > > > > > > during suspend, > > > > > the transfer will fail, do we have any methods to avoid this issue? > > > > > > > > > > If you have an active transfer, does it make sense to be suspending? > > > > > Perhaps you should hold a wakelock while a transfer is ongoing. > > > > > I am not sure how Obex and other services should behave here so > > > > > I will defer to Luiz and Marcel's opinion on this topic. > > > > > > > > I'm not sure whether it's reasonable, our drivers seems can be > > > > suspended > > > during transfer file and A2DP playing. > > > > Maybe we really need a wakelock here. > > > > > > > > I checked the wakelock related code in kernel suspend steps, but it seems > that the wakelock cannot prevent the Bluetooth disconnect behavior. > > Because when we run echo mem > /sys/power/state command to put the > > system get into suspend mode, it will call pm_notifier_call_chain_robust() > in suspend_prepare(), the Bluetooth will be disconnected here as you add > the pm_notifier callback. > > In the next few steps, pm_wakeup_pending will be called to determine > whether the suspend operation should be continued, if we registered the > wakelock, we will not enter the suspend state. > > But the Bluetooth has been disconnected already. Therefore, even if a > wakelock is added, we will also meet this BT large file transfer fail issue. Am I > right? > > > > From what you've described, that seems to be the case. I would expect > wakelocks to prevent suspend notifiers as well so this seems like a bug in the > suspend code. You should submit a patch to check that there isn't a pending > wakeup in enter_state since that's the case: > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub > .com%2Ftorvalds%2Flinux%2Fblob%2Fmaster%2Fkernel%2Fpower%2Fsuspe > nd.c%23L562&data=04%7C01%7Csherry.sun%40nxp.com%7Cb06b00a1 > abc5495a3d2508d8bc374c07%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 > %7C0%7C637466292972854206%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC > 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000& > amp;sdata=AhNB3i17HgSy9JoAqnD47oA5up19LY9fF8Zone90aqo%3D&r > eserved=0 > I don't think it is a bug in the suspend code, it may need to call suspend notifier before wakelock checking due to some limitation. I just wonder why we must do Bluetooth disconnect in suspend notifier, can we put the BT disconnect behavior later than wakelock checking? Hi Marcel & Johan & Luiz, can you give some advices about this? Best regards Sherry
