Hi Sherry, On Tue, Jan 12, 2021 at 3:07 AM Sherry Sun <sherry.sun@xxxxxxx> wrote: > > Hi Abhishek, > > > -----Original Message----- > > From: Abhishek Pandit-Subedi <abhishekpandit@xxxxxxxxxxxx> > > Sent: 2021年1月12日 7:55 > > To: Sherry Sun <sherry.sun@xxxxxxx> > > Cc: linux-bluetooth@xxxxxxxxxxxxxxx; marcel@xxxxxxxxxxxx; > > johan.hedberg@xxxxxxxxx > > Subject: Re: BT large file transfer failed when do suspend/resume test > > > > Hi Sherry, > > > > On Wed, Jan 6, 2021 at 7:12 PM Sherry Sun <sherry.sun@xxxxxxx> wrote: > > > > > > Hi Abhishek, thanks for you answer, please see my reply below. > > > > > > > -----Original Message----- > > > > From: Abhishek Pandit-Subedi <abhishekpandit@xxxxxxxxxxxx> > > > > Sent: 2021年1月7日 3:02 > > > > To: Sherry Sun <sherry.sun@xxxxxxx> > > > > Cc: linux-bluetooth@xxxxxxxxxxxxxxx; marcel@xxxxxxxxxxxx; > > > > johan.hedberg@xxxxxxxxx > > > > Subject: Re: BT large file transfer failed when do suspend/resume > > > > test > > > > > > > > Hi Sherry, > > > > > > > > On Wed, Jan 6, 2021 at 5:40 AM Sherry Sun <sherry.sun@xxxxxxx> > > wrote: > > > > > > > > > > Hi Abhishek, > > > > > > > > > > I want to ask you some questions about your patch: Bluetooth: > > > > > Handle BR/EDR devices during suspend. > > > > > (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2 > > > > > Fpat > > > > > > > > > > > chwork.kernel.org%2Fproject%2Fbluetooth%2Fpatch%2F20200311085359.RF > > > > C.v > > > > > > > > > > > 6.2.Icc7c35e1cabf10f8a383a009694987520f1d1b35%40changeid%2F&da > > > > ta=0 > > > > > > > > > > > 4%7C01%7Csherry.sun%40nxp.com%7C9d41a6725ac1432355c408d8b275970 > > > > 0%7C686 > > > > > > > > > > > ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637455565422685536%7CU > > > > nknown%7 > > > > > > > > > > > CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi > > > > LCJXV > > > > > > > > > > > CI6Mn0%3D%7C1000&sdata=56h1%2BfVflqDZsx%2FcIbtUfa6kcPjV4p2n > > > > cxPf1uA > > > > > a7iY%3D&reserved=0) > > > > > > > > > > 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%2Fgit.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%2Fgit.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 > > %7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW > > 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%2Fpatch > > 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've linked the patch series making it more configurable in previous emails (https://patchwork.kernel.org/project/bluetooth/patch/20201207154903.bluez.1.I3e043a481273442748bcff0728b2f0e208017cd2@changeid/). 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://github.com/torvalds/linux/blob/master/kernel/power/suspend.c#L562 > Best regards > Sherry > > > Thanks and regards > > > Sherry > > > > > > > > > > > > > > > > > I'm new to Bluetooth, there are many things I don't understand, > > > > > looking > > > > forward to your answer, and thanks for now! > > > > > > > > > > Best regards > > > > > Sherry sun > > > > > > > > > > > > > Thanks > > > > Abhishek
