RE: BT large file transfer failed when do suspend/resume test

[Sherry Sun <sherry.sun@nxp.com>]

Hi Abhishek,

Gentle ping...
These issues really bother me, so looking forward to your reply!

> -----Original Message-----
> From: Sherry Sun
> Sent: 2021年1月12日 19:07
> To: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> Cc: linux-bluetooth@vger.kernel.org; marcel@holtmann.org;
> johan.hedberg@gmail.com
> Subject: RE: BT large file transfer failed when do suspend/resume test
> 
> Hi Abhishek,
> 
> > -----Original Message-----
> > From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > Sent: 2021年1月12日 7:55
> > To: Sherry Sun <sherry.sun@nxp.com>
> > Cc: linux-bluetooth@vger.kernel.org; marcel@holtmann.org;
> > johan.hedberg@gmail.com
> > 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@nxp.com> wrote:
> > >
> > > Hi Abhishek, thanks for you answer, please see my reply below.
> > >
> > > > -----Original Message-----
> > > > From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > > > Sent: 2021年1月7日 3:02
> > > > To: Sherry Sun <sherry.sun@nxp.com>
> > > > Cc: linux-bluetooth@vger.kernel.org; marcel@holtmann.org;
> > > > johan.hedberg@gmail.com
> > > > 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@nxp.com>
> > 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&amp;da
> > > > ta=0
> > > > >
> > > >
> >
> 4%7C01%7Csherry.sun%40nxp.com%7C9d41a6725ac1432355c408d8b275970
> > > > 0%7C686
> > > > >
> > > >
> >
> ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637455565422685536%7CU
> > > > nknown%7
> > > > >
> > > >
> >
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> > > > LCJXV
> > > > >
> > > >
> >
> CI6Mn0%3D%7C1000&amp;sdata=56h1%2BfVflqDZsx%2FcIbtUfa6kcPjV4p2n
> > > > cxPf1uA
> > > > > a7iY%3D&amp;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?
> 
> >
> > >
> > > > - 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&amp;sdata=EDn
> >
> MnLjPIN29nQpmH1%2Fk%2BBnhMq5IZ3xG7BXIEq0xs6A%3D&amp;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&amp;data=04%7C01%7Cshe
> >
> rry.sun%40nxp.com%7C1f3ec177fe974c90e1a508d8b68c4584%7C686ea1d3b
> >
> c2b4c6fa92cd99c5c301635%7C0%7C0%7C637460060872077285%7CUnknown
> > %7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> W
> >
> wiLCJXVCI6Mn0%3D%7C1000&amp;sdata=2jYeDwzoy87GA2V2SqilgdA502jeI
> > ZdGd4aeyXH125M%3D&amp;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%2Fpat
> > ch
> work.kernel.org%2Fproject%2Fbluetooth%2Fpatch%2F20201207154903.blue
> >
> z.1.I3e043a481273442748bcff0728b2f0e208017cd2%40changeid%2F&amp;d
> >
> ata=04%7C01%7Csherry.sun%40nxp.com%7C1f3ec177fe974c90e1a508d8b68
> >
> c4584%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637460060872
> >
> 077285%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
> > uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=aNXOfCTXs%
> > 2BISAbYBH2die47qfpqXRmqehSnW1mC8M7U%3D&amp;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.
> 
> > >
> > > > > 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%2F
> > > > pa
> > > > tch
> > > >
> >
> work.kernel.org%2Fproject%2Fbluetooth%2Fpatch%2F20201207154903.blue
> > > >
> >
> z.1.I3e043a481273442748bcff0728b2f0e208017cd2%40changeid%2F&amp;d
> > > >
> >
> ata=04%7C01%7Csherry.sun%40nxp.com%7C9d41a6725ac1432355c408d8b2
> > > >
> >
> 759700%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6374555654
> > > >
> >
> 22685536%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
> > > >
> >
> V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=6zZda8Yu
> > > > RRbAMkwOxiSwjbJIpvi%2ByMSDWCzVkFkkpzs%3D&amp;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?
> 
> 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