What is the motivation for conn->power_save

What is the motivation for conn->power_save

[Nick Pelly]

If I understand correctly, conn->power_save prevents the host stack
from requesting active mode if it was not the host stack that
requested sniff mode.

I don't understand the motivation for this. If we have ACL data to
send, then it seems like a good idea for the host stack to explicitly
request active mode, regardless of the reason that we entered sniff
mode.

We want to enter active mode more aggressively when setting up SCO
connections, to avoid a 5 second delay with certain sniff modes. But
the conn->power_save code is getting in the way and doesn't appear to
be useful in the first place.

Sending patches in another email...

Nick

Re: What is the motivation for conn->power_save

[Marcel Holtmann]

Hi Nick,

> If I understand correctly, conn->power_save prevents the host stack
> from requesting active mode if it was not the host stack that
> requested sniff mode.
> 
> I don't understand the motivation for this. If we have ACL data to
> send, then it seems like a good idea for the host stack to explicitly
> request active mode, regardless of the reason that we entered sniff
> mode.
> 
> We want to enter active mode more aggressively when setting up SCO
> connections, to avoid a 5 second delay with certain sniff modes. But
> the conn->power_save code is getting in the way and doesn't appear to
> be useful in the first place.

we have discussed this a few times. And if you lock through the code
history then you see that initially we just took devices out of sniff
mode if we had to send data. However with HID devices this falls flat on
its face. They need to stay in control of sniff mode if they initiated
it. Some of them crash and others just drain the battery. With sniff
mode you can send small amounts of data even while in sniff and for HID
that is sometimes used. So the remote side better not interfere.

What we really need is a socket option where we can control this on a
per socket basis if we take devices out of sniff mode. And one extra
case might be when we try to establish a SCO channel, because then it is
clearly not an HID device. However even A2DP has this sort of problems
sometimes where the stream setup takes time.

Not sure if we have to make SCO setup special. The only reason would be
if there is a case where we don't get an AT command before SCO needs to
be established.

Regards

Marcel

Re: What is the motivation for conn->power_save

[Nick Pelly]

Hi Marcel,

On Thu, Nov 12, 2009 at 7:14 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Nick,
>
>> If I understand correctly, conn->power_save prevents the host stack
>> from requesting active mode if it was not the host stack that
>> requested sniff mode.
>>
>> I don't understand the motivation for this. If we have ACL data to
>> send, then it seems like a good idea for the host stack to explicitly
>> request active mode, regardless of the reason that we entered sniff
>> mode.
>>
>> We want to enter active mode more aggressively when setting up SCO
>> connections, to avoid a 5 second delay with certain sniff modes. But
>> the conn->power_save code is getting in the way and doesn't appear to
>> be useful in the first place.
>
> we have discussed this a few times. And if you lock through the code
> history then you see that initially we just took devices out of sniff
> mode if we had to send data. However with HID devices this falls flat on
> its face. They need to stay in control of sniff mode if they initiated
> it. Some of them crash and others just drain the battery. With sniff
> mode you can send small amounts of data even while in sniff and for HID
> that is sometimes used. So the remote side better not interfere.
>
> What we really need is a socket option where we can control this on a
> per socket basis if we take devices out of sniff mode. And one extra
> case might be when we try to establish a SCO channel, because then it is
> clearly not an HID device. However even A2DP has this sort of problems
> sometimes where the stream setup takes time.

Makes sense. Thanks for the explanation.

> Not sure if we have to make SCO setup special. The only reason would be
> if there is a case where we don't get an AT command before SCO needs to
> be established.

If you are in-call, and transfer audio from handset to BT headset,
then there is SCO setup without any AT command.

I think for the SCO setup case we would always want to enter active
mode. I could modify enter_active_mode() to take a parameter like 'int
force' that would force us to enter active mode regardless of the
state of power_save, and use this when setting up SCO. What do you
think?

Nick

Re: What is the motivation for conn->power_save

[Marcel Holtmann]

Hi Nick,

> >> If I understand correctly, conn->power_save prevents the host stack
> >> from requesting active mode if it was not the host stack that
> >> requested sniff mode.
> >>
> >> I don't understand the motivation for this. If we have ACL data to
> >> send, then it seems like a good idea for the host stack to explicitly
> >> request active mode, regardless of the reason that we entered sniff
> >> mode.
> >>
> >> We want to enter active mode more aggressively when setting up SCO
> >> connections, to avoid a 5 second delay with certain sniff modes. But
> >> the conn->power_save code is getting in the way and doesn't appear to
> >> be useful in the first place.
> >
> > we have discussed this a few times. And if you lock through the code
> > history then you see that initially we just took devices out of sniff
> > mode if we had to send data. However with HID devices this falls flat on
> > its face. They need to stay in control of sniff mode if they initiated
> > it. Some of them crash and others just drain the battery. With sniff
> > mode you can send small amounts of data even while in sniff and for HID
> > that is sometimes used. So the remote side better not interfere.
> >
> > What we really need is a socket option where we can control this on a
> > per socket basis if we take devices out of sniff mode. And one extra
> > case might be when we try to establish a SCO channel, because then it is
> > clearly not an HID device. However even A2DP has this sort of problems
> > sometimes where the stream setup takes time.
> 
> Makes sense. Thanks for the explanation.

this means you will be working on a patch for this :)

> > Not sure if we have to make SCO setup special. The only reason would be
> > if there is a case where we don't get an AT command before SCO needs to
> > be established.
> 
> If you are in-call, and transfer audio from handset to BT headset,
> then there is SCO setup without any AT command.

Fair enough.

> I think for the SCO setup case we would always want to enter active
> mode. I could modify enter_active_mode() to take a parameter like 'int
> force' that would force us to enter active mode regardless of the
> state of power_save, and use this when setting up SCO. What do you
> think?

Actually when you leave sniff mode, then all bets for the power_save
value are off again. So you better set power_save and just call
enter_active_mode. Something like this:

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index a975098..e4591e0 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -376,6 +376,9 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type,
 
        if (acl->state == BT_CONNECTED &&
                        (sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
+               acl->power_save = 1;
+               hci_conn_enter_active_mode(acl);
+
                if (lmp_esco_capable(hdev))
                        hci_setup_sync(sco, acl->handle);
                else

Alternatively we could create hci_conn_force_active_mode() or just
implement a proper per socket sniff/active policy.

Regards

Marcel

Re: What is the motivation for conn->power_save

[Nick Pelly]

Hi Marcel,

On Thu, Nov 12, 2009 at 7:52 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Nick,
>
>> >> If I understand correctly, conn->power_save prevents the host stack
>> >> from requesting active mode if it was not the host stack that
>> >> requested sniff mode.
>> >>
>> >> I don't understand the motivation for this. If we have ACL data to
>> >> send, then it seems like a good idea for the host stack to explicitly
>> >> request active mode, regardless of the reason that we entered sniff
>> >> mode.
>> >>
>> >> We want to enter active mode more aggressively when setting up SCO
>> >> connections, to avoid a 5 second delay with certain sniff modes. But
>> >> the conn->power_save code is getting in the way and doesn't appear to
>> >> be useful in the first place.
>> >
>> > we have discussed this a few times. And if you lock through the code
>> > history then you see that initially we just took devices out of sniff
>> > mode if we had to send data. However with HID devices this falls flat =
on
>> > its face. They need to stay in control of sniff mode if they initiated
>> > it. Some of them crash and others just drain the battery. With sniff
>> > mode you can send small amounts of data even while in sniff and for HI=
D
>> > that is sometimes used. So the remote side better not interfere.
>> >
>> > What we really need is a socket option where we can control this on a
>> > per socket basis if we take devices out of sniff mode. And one extra
>> > case might be when we try to establish a SCO channel, because then it =
is
>> > clearly not an HID device. However even A2DP has this sort of problems
>> > sometimes where the stream setup takes time.
>>
>> Makes sense. Thanks for the explanation.
>
> this means you will be working on a patch for this :)
>
>> > Not sure if we have to make SCO setup special. The only reason would b=
e
>> > if there is a case where we don't get an AT command before SCO needs t=
o
>> > be established.
>>
>> If you are in-call, and transfer audio from handset to BT headset,
>> then there is SCO setup without any AT command.
>
> Fair enough.
>
>> I think for the SCO setup case we would always want to enter active
>> mode. I could modify enter_active_mode() to take a parameter like 'int
>> force' that would force us to enter active mode regardless of the
>> state of power_save, and use this when setting up SCO. What do you
>> think?
>
> Actually when you leave sniff mode, then all bets for the power_save
> value are off again. So you better set power_save and just call
> enter_active_mode. Something like this:
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index a975098..e4591e0 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -376,6 +376,9 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, in=
t type,
>
> =A0 =A0 =A0 =A0if (acl->state =3D=3D BT_CONNECTED &&
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(sco->state =3D=3D BT_OPEN=
 || sco->state =3D=3D BT_CLOSED)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 acl->power_save =3D 1;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 hci_conn_enter_active_mode(acl);
> +
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (lmp_esco_capable(hdev))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0hci_setup_sync(sco, acl->h=
andle);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else
>
> Alternatively we could create hci_conn_force_active_mode() or just
> implement a proper per socket sniff/active policy.
>
> Regards
>
> Marcel

Patch submitted to Android tree as:

http://android.git.kernel.org/?p=3Dkernel/common.git;a=3Dcommit;h=3D201ac2f=
225a31dffcb05f1db4d609c467c9c694c

Nick

Re: What is the motivation for conn->power_save

[Nick Pelly]

On Sat, Nov 14, 2009 at 9:45 AM, Nick Pelly <npelly@google.com> wrote:
> On Thu, Nov 12, 2009 at 7:52 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> Hi Nick,
>>
>>> >> If I understand correctly, conn->power_save prevents the host stack
>>> >> from requesting active mode if it was not the host stack that
>>> >> requested sniff mode.
>>> >>
>>> >> I don't understand the motivation for this. If we have ACL data to
>>> >> send, then it seems like a good idea for the host stack to explicitly
>>> >> request active mode, regardless of the reason that we entered sniff
>>> >> mode.
>>> >>
>>> >> We want to enter active mode more aggressively when setting up SCO
>>> >> connections, to avoid a 5 second delay with certain sniff modes. But
>>> >> the conn->power_save code is getting in the way and doesn't appear to
>>> >> be useful in the first place.
>>> >
>>> > we have discussed this a few times. And if you lock through the code
>>> > history then you see that initially we just took devices out of sniff
>>> > mode if we had to send data. However with HID devices this falls flat on
>>> > its face. They need to stay in control of sniff mode if they initiated
>>> > it. Some of them crash and others just drain the battery. With sniff
>>> > mode you can send small amounts of data even while in sniff and for HID
>>> > that is sometimes used. So the remote side better not interfere.
>>> >
>>> > What we really need is a socket option where we can control this on a
>>> > per socket basis if we take devices out of sniff mode. And one extra
>>> > case might be when we try to establish a SCO channel, because then it is
>>> > clearly not an HID device. However even A2DP has this sort of problems
>>> > sometimes where the stream setup takes time.
>>>
>>> Makes sense. Thanks for the explanation.
>>
>> this means you will be working on a patch for this :)

Actually, I want to put a patch together for a socket option to not
use power_save (so that we *always* exit sniff mode when sending ACL
data). We're seeing this problem with the Plantronics Voyager 855
which enters sniff mode during A2DP.

Given that power_save is just for HID devices, my preferred design is
to disable power_save by default, and have an L2CAP socket option to
turn on power_save that would be used by HID L2CAP sockets.
Unfortunately this would require userspace HID code to use the new
socket option to keep current behavior. But it seems preferable to the
alternative of having every single other L2CAP socket use a new socket
option just to disable power_save for the sake of HID.

Nick

Re: What is the motivation for conn->power_save

[Marcel Holtmann]

Hi Nick,

> >>> >> If I understand correctly, conn->power_save prevents the host stack
> >>> >> from requesting active mode if it was not the host stack that
> >>> >> requested sniff mode.
> >>> >>
> >>> >> I don't understand the motivation for this. If we have ACL data to
> >>> >> send, then it seems like a good idea for the host stack to explicitly
> >>> >> request active mode, regardless of the reason that we entered sniff
> >>> >> mode.
> >>> >>
> >>> >> We want to enter active mode more aggressively when setting up SCO
> >>> >> connections, to avoid a 5 second delay with certain sniff modes. But
> >>> >> the conn->power_save code is getting in the way and doesn't appear to
> >>> >> be useful in the first place.
> >>> >
> >>> > we have discussed this a few times. And if you lock through the code
> >>> > history then you see that initially we just took devices out of sniff
> >>> > mode if we had to send data. However with HID devices this falls flat on
> >>> > its face. They need to stay in control of sniff mode if they initiated
> >>> > it. Some of them crash and others just drain the battery. With sniff
> >>> > mode you can send small amounts of data even while in sniff and for HID
> >>> > that is sometimes used. So the remote side better not interfere.
> >>> >
> >>> > What we really need is a socket option where we can control this on a
> >>> > per socket basis if we take devices out of sniff mode. And one extra
> >>> > case might be when we try to establish a SCO channel, because then it is
> >>> > clearly not an HID device. However even A2DP has this sort of problems
> >>> > sometimes where the stream setup takes time.
> >>>
> >>> Makes sense. Thanks for the explanation.
> >>
> >> this means you will be working on a patch for this :)
> 
> Actually, I want to put a patch together for a socket option to not
> use power_save (so that we *always* exit sniff mode when sending ACL
> data). We're seeing this problem with the Plantronics Voyager 855
> which enters sniff mode during A2DP.
> 
> Given that power_save is just for HID devices, my preferred design is
> to disable power_save by default, and have an L2CAP socket option to
> turn on power_save that would be used by HID L2CAP sockets.
> Unfortunately this would require userspace HID code to use the new
> socket option to keep current behavior. But it seems preferable to the
> alternative of having every single other L2CAP socket use a new socket
> option just to disable power_save for the sake of HID.

actually you still mix up the meaning of the power_save option. From the
stack side, the automatic sniff mode is off by default. You have to
enable via sysfs first. The power_save option is just to control when
sniff mode is activated and the remote enters sniff mode, that we are
not getting out of it if we have ACL data.

In conclusion, I am fine with a socket option that allows to control
sniff mode (preferable with interval details) and sniff subrate details
so that we can use them for that ACL link.

So go ahead and work on this. We can fix userspace easily since a new
socket can be detected easily with newer kernels.

Regards

Marcel

Re: What is the motivation for conn->power_save

[Nick Pelly]

On Mon, Nov 16, 2009 at 10:55 AM, Nick Pelly <npelly@google.com> wrote:
> Hi Marcel,
>
> On Thu, Nov 12, 2009 at 7:52 PM, Marcel Holtmann <marcel@holtmann.org> wr=
ote:
>> Hi Nick,
>>
>>> >> If I understand correctly, conn->power_save prevents the host stack
>>> >> from requesting active mode if it was not the host stack that
>>> >> requested sniff mode.
>>> >>
>>> >> I don't understand the motivation for this. If we have ACL data to
>>> >> send, then it seems like a good idea for the host stack to explicitl=
y
>>> >> request active mode, regardless of the reason that we entered sniff
>>> >> mode.
>>> >>
>>> >> We want to enter active mode more aggressively when setting up SCO
>>> >> connections, to avoid a 5 second delay with certain sniff modes. But
>>> >> the conn->power_save code is getting in the way and doesn't appear t=
o
>>> >> be useful in the first place.
>>> >
>>> > we have discussed this a few times. And if you lock through the code
>>> > history then you see that initially we just took devices out of sniff
>>> > mode if we had to send data. However with HID devices this falls flat=
 on
>>> > its face. They need to stay in control of sniff mode if they initiate=
d
>>> > it. Some of them crash and others just drain the battery. With sniff
>>> > mode you can send small amounts of data even while in sniff and for H=
ID
>>> > that is sometimes used. So the remote side better not interfere.
>>> >
>>> > What we really need is a socket option where we can control this on a
>>> > per socket basis if we take devices out of sniff mode. And one extra
>>> > case might be when we try to establish a SCO channel, because then it=
 is
>>> > clearly not an HID device. However even A2DP has this sort of problem=
s
>>> > sometimes where the stream setup takes time.
>>>
>>> Makes sense. Thanks for the explanation.
>>
>> this means you will be working on a patch for this :)
>>
>>> > Not sure if we have to make SCO setup special. The only reason would =
be
>>> > if there is a case where we don't get an AT command before SCO needs =
to
>>> > be established.
>>>
>>> If you are in-call, and transfer audio from handset to BT headset,
>>> then there is SCO setup without any AT command.
>>
>> Fair enough.
>>
>>> I think for the SCO setup case we would always want to enter active
>>> mode. I could modify enter_active_mode() to take a parameter like 'int
>>> force' that would force us to enter active mode regardless of the
>>> state of power_save, and use this when setting up SCO. What do you
>>> think?
>>
>> Actually when you leave sniff mode, then all bets for the power_save
>> value are off again. So you better set power_save and just call
>> enter_active_mode. Something like this:
>>
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index a975098..e4591e0 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -376,6 +376,9 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, i=
nt type,
>>
>> =A0 =A0 =A0 =A0if (acl->state =3D=3D BT_CONNECTED &&
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(sco->state =3D=3D BT_OPE=
N || sco->state =3D=3D BT_CLOSED)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 acl->power_save =3D 1;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 hci_conn_enter_active_mode(acl);
>> +
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (lmp_esco_capable(hdev))
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0hci_setup_sync(sco, acl->=
handle);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else
>>
>> Alternatively we could create hci_conn_force_active_mode() or just
>> implement a proper per socket sniff/active policy.
>>
>> Regards
>>
>> Marcel
>
> Patch submitted to Android tree as:
>
> http://android.git.kernel.org/?p=3Dkernel/common.git;a=3Dcommit;h=3D201ac=
2f225a31dffcb05f1db4d609c467c9c694c

Ping.

Nick

Re: What is the motivation for conn->power_save

[Marcel Holtmann]

Hi Nick,

> >> >> If I understand correctly, conn->power_save prevents the host stack
> >> >> from requesting active mode if it was not the host stack that
> >> >> requested sniff mode.
> >> >>
> >> >> I don't understand the motivation for this. If we have ACL data to
> >> >> send, then it seems like a good idea for the host stack to explicitly
> >> >> request active mode, regardless of the reason that we entered sniff
> >> >> mode.
> >> >>
> >> >> We want to enter active mode more aggressively when setting up SCO
> >> >> connections, to avoid a 5 second delay with certain sniff modes. But
> >> >> the conn->power_save code is getting in the way and doesn't appear to
> >> >> be useful in the first place.
> >> >
> >> > we have discussed this a few times. And if you lock through the code
> >> > history then you see that initially we just took devices out of sniff
> >> > mode if we had to send data. However with HID devices this falls flat on
> >> > its face. They need to stay in control of sniff mode if they initiated
> >> > it. Some of them crash and others just drain the battery. With sniff
> >> > mode you can send small amounts of data even while in sniff and for HID
> >> > that is sometimes used. So the remote side better not interfere.
> >> >
> >> > What we really need is a socket option where we can control this on a
> >> > per socket basis if we take devices out of sniff mode. And one extra
> >> > case might be when we try to establish a SCO channel, because then it is
> >> > clearly not an HID device. However even A2DP has this sort of problems
> >> > sometimes where the stream setup takes time.
> >>
> >> Makes sense. Thanks for the explanation.
> >
> > this means you will be working on a patch for this :)
> >
> >> > Not sure if we have to make SCO setup special. The only reason would be
> >> > if there is a case where we don't get an AT command before SCO needs to
> >> > be established.
> >>
> >> If you are in-call, and transfer audio from handset to BT headset,
> >> then there is SCO setup without any AT command.
> >
> > Fair enough.
> >
> >> I think for the SCO setup case we would always want to enter active
> >> mode. I could modify enter_active_mode() to take a parameter like 'int
> >> force' that would force us to enter active mode regardless of the
> >> state of power_save, and use this when setting up SCO. What do you
> >> think?
> >
> > Actually when you leave sniff mode, then all bets for the power_save
> > value are off again. So you better set power_save and just call
> > enter_active_mode. Something like this:
> >
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index a975098..e4591e0 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -376,6 +376,9 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type,
> >
> >        if (acl->state == BT_CONNECTED &&
> >                        (sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
> > +               acl->power_save = 1;
> > +               hci_conn_enter_active_mode(acl);
> > +
> >                if (lmp_esco_capable(hdev))
> >                        hci_setup_sync(sco, acl->handle);
> >                else
> >
> > Alternatively we could create hci_conn_force_active_mode() or just
> > implement a proper per socket sniff/active policy.
> >
> 
> Patch submitted to Android tree as:
> 
> http://android.git.kernel.org/?p=kernel/common.git;a=commit;h=201ac2f225a31dffcb05f1db4d609c467c9c694c

can you please send a version to the mailing list so I can easily apply
it and also have it here for reference in the archives.

Regards

Marcel

Re: What is the motivation for conn->power_save

[Nick Pelly]

[-- Attachment #1: Type: text/plain, Size: 3873 bytes --]

On Wed, Feb 3, 2010 at 12:20 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Nick,
>
>> >> >> If I understand correctly, conn->power_save prevents the host stack
>> >> >> from requesting active mode if it was not the host stack that
>> >> >> requested sniff mode.
>> >> >>
>> >> >> I don't understand the motivation for this. If we have ACL data to
>> >> >> send, then it seems like a good idea for the host stack to explicitly
>> >> >> request active mode, regardless of the reason that we entered sniff
>> >> >> mode.
>> >> >>
>> >> >> We want to enter active mode more aggressively when setting up SCO
>> >> >> connections, to avoid a 5 second delay with certain sniff modes. But
>> >> >> the conn->power_save code is getting in the way and doesn't appear to
>> >> >> be useful in the first place.
>> >> >
>> >> > we have discussed this a few times. And if you lock through the code
>> >> > history then you see that initially we just took devices out of sniff
>> >> > mode if we had to send data. However with HID devices this falls flat on
>> >> > its face. They need to stay in control of sniff mode if they initiated
>> >> > it. Some of them crash and others just drain the battery. With sniff
>> >> > mode you can send small amounts of data even while in sniff and for HID
>> >> > that is sometimes used. So the remote side better not interfere.
>> >> >
>> >> > What we really need is a socket option where we can control this on a
>> >> > per socket basis if we take devices out of sniff mode. And one extra
>> >> > case might be when we try to establish a SCO channel, because then it is
>> >> > clearly not an HID device. However even A2DP has this sort of problems
>> >> > sometimes where the stream setup takes time.
>> >>
>> >> Makes sense. Thanks for the explanation.
>> >
>> > this means you will be working on a patch for this :)
>> >
>> >> > Not sure if we have to make SCO setup special. The only reason would be
>> >> > if there is a case where we don't get an AT command before SCO needs to
>> >> > be established.
>> >>
>> >> If you are in-call, and transfer audio from handset to BT headset,
>> >> then there is SCO setup without any AT command.
>> >
>> > Fair enough.
>> >
>> >> I think for the SCO setup case we would always want to enter active
>> >> mode. I could modify enter_active_mode() to take a parameter like 'int
>> >> force' that would force us to enter active mode regardless of the
>> >> state of power_save, and use this when setting up SCO. What do you
>> >> think?
>> >
>> > Actually when you leave sniff mode, then all bets for the power_save
>> > value are off again. So you better set power_save and just call
>> > enter_active_mode. Something like this:
>> >
>> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> > index a975098..e4591e0 100644
>> > --- a/net/bluetooth/hci_conn.c
>> > +++ b/net/bluetooth/hci_conn.c
>> > @@ -376,6 +376,9 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type,
>> >
>> >        if (acl->state == BT_CONNECTED &&
>> >                        (sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
>> > +               acl->power_save = 1;
>> > +               hci_conn_enter_active_mode(acl);
>> > +
>> >                if (lmp_esco_capable(hdev))
>> >                        hci_setup_sync(sco, acl->handle);
>> >                else
>> >
>> > Alternatively we could create hci_conn_force_active_mode() or just
>> > implement a proper per socket sniff/active policy.
>> >
>>
>> Patch submitted to Android tree as:
>>
>> http://android.git.kernel.org/?p=kernel/common.git;a=commit;h=201ac2f225a31dffcb05f1db4d609c467c9c694c
>
> can you please send a version to the mailing list so I can easily apply
> it and also have it here for reference in the archives.

Patch attached.

Nick

[-- Attachment #2: 0001-Bluetooth-Enter-active-mode-before-establishing-a-SC.patch --]
[-- Type: application/octet-stream, Size: 1888 bytes --]

From 786d9a4886338e80c11615da8091e6c57876aef3 Mon Sep 17 00:00:00 2001
From: Nick Pelly <npelly@google.com>
Date: Fri, 13 Nov 2009 14:16:32 -0800
Subject: [PATCH] Bluetooth: Enter active mode before establishing a SCO link.

When in sniff mode with a long interval time (1.28s) it can take 4+ seconds to
establish a SCO link. Fix by requesting active mode before requesting SCO
connection. This improves SCO setup time to ~500ms.

Bluetooth headsets that use a long interval time, and exhibit the long SCO
connection time include Motorola H790, HX1 and H17. They have a CSR 2.1 chipset

Verified this behavior and fix with host Bluetooth chipsets: BCM4329 and
TI1271.

2009-10-13 14:17:46.183722 > HCI Event: Mode Change (0x14) plen 6
    status 0x00 handle 1 mode 0x02 interval 2048
    Mode: Sniff
2009-10-13 14:17:53.436285 < HCI Command: Setup Synchronous Connection
        (0x01|0x0028) plen 17
    handle 1 voice setting 0x0060
2009-10-13 14:17:53.445593 > HCI Event: Command Status (0x0f) plen 4
    Setup Synchronous Connection (0x01|0x0028) status 0x00 ncmd 1
2009-10-13 14:17:57.788855 > HCI Event: Synchronous Connect Complete
        (0x2c) plen 17
    status 0x00 handle 257 bdaddr 00:1A:0E:F1:A4:7F type eSCO
    Air mode: CVSD

Signed-off-by: Nick Pelly <npelly@google.com>
---
 net/bluetooth/hci_conn.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index b7c4224..b10e3cd 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -377,6 +377,9 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
 
 	if (acl->state == BT_CONNECTED &&
 			(sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
+		acl->power_save = 1;
+		hci_conn_enter_active_mode(acl);
+
 		if (lmp_esco_capable(hdev))
 			hci_setup_sync(sco, acl->handle);
 		else
-- 
1.6.5.3

Re: What is the motivation for conn->power_save

[Marcel Holtmann]

Hi Nick,

> >> >> >> If I understand correctly, conn->power_save prevents the host stack
> >> >> >> from requesting active mode if it was not the host stack that
> >> >> >> requested sniff mode.
> >> >> >>
> >> >> >> I don't understand the motivation for this. If we have ACL data to
> >> >> >> send, then it seems like a good idea for the host stack to explicitly
> >> >> >> request active mode, regardless of the reason that we entered sniff
> >> >> >> mode.
> >> >> >>
> >> >> >> We want to enter active mode more aggressively when setting up SCO
> >> >> >> connections, to avoid a 5 second delay with certain sniff modes. But
> >> >> >> the conn->power_save code is getting in the way and doesn't appear to
> >> >> >> be useful in the first place.
> >> >> >
> >> >> > we have discussed this a few times. And if you lock through the code
> >> >> > history then you see that initially we just took devices out of sniff
> >> >> > mode if we had to send data. However with HID devices this falls flat on
> >> >> > its face. They need to stay in control of sniff mode if they initiated
> >> >> > it. Some of them crash and others just drain the battery. With sniff
> >> >> > mode you can send small amounts of data even while in sniff and for HID
> >> >> > that is sometimes used. So the remote side better not interfere.
> >> >> >
> >> >> > What we really need is a socket option where we can control this on a
> >> >> > per socket basis if we take devices out of sniff mode. And one extra
> >> >> > case might be when we try to establish a SCO channel, because then it is
> >> >> > clearly not an HID device. However even A2DP has this sort of problems
> >> >> > sometimes where the stream setup takes time.
> >> >>
> >> >> Makes sense. Thanks for the explanation.
> >> >
> >> > this means you will be working on a patch for this :)
> >> >
> >> >> > Not sure if we have to make SCO setup special. The only reason would be
> >> >> > if there is a case where we don't get an AT command before SCO needs to
> >> >> > be established.
> >> >>
> >> >> If you are in-call, and transfer audio from handset to BT headset,
> >> >> then there is SCO setup without any AT command.
> >> >
> >> > Fair enough.
> >> >
> >> >> I think for the SCO setup case we would always want to enter active
> >> >> mode. I could modify enter_active_mode() to take a parameter like 'int
> >> >> force' that would force us to enter active mode regardless of the
> >> >> state of power_save, and use this when setting up SCO. What do you
> >> >> think?
> >> >
> >> > Actually when you leave sniff mode, then all bets for the power_save
> >> > value are off again. So you better set power_save and just call
> >> > enter_active_mode. Something like this:
> >> >
> >> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> >> > index a975098..e4591e0 100644
> >> > --- a/net/bluetooth/hci_conn.c
> >> > +++ b/net/bluetooth/hci_conn.c
> >> > @@ -376,6 +376,9 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type,
> >> >
> >> >        if (acl->state == BT_CONNECTED &&
> >> >                        (sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
> >> > +               acl->power_save = 1;
> >> > +               hci_conn_enter_active_mode(acl);
> >> > +
> >> >                if (lmp_esco_capable(hdev))
> >> >                        hci_setup_sync(sco, acl->handle);
> >> >                else
> >> >
> >> > Alternatively we could create hci_conn_force_active_mode() or just
> >> > implement a proper per socket sniff/active policy.
> >> >
> >>
> >> Patch submitted to Android tree as:
> >>
> >> http://android.git.kernel.org/?p=kernel/common.git;a=commit;h=201ac2f225a31dffcb05f1db4d609c467c9c694c
> >
> > can you please send a version to the mailing list so I can easily apply
> > it and also have it here for reference in the archives.
> 
> Patch attached.

patch has been applied. Please keep the commit message to 72 chars so
that git log doesn't exceed on an 80 chars terminal. Also your hcidump
seems to line wrap. I had to fix that ;)

Regards

Marcel

Re: What is the motivation for conn->power_save

[Andrei Emeltchenko]

Hi,

On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org> wro=
te:
> Hi Nick,
>
>> >>> >> If I understand correctly, conn->power_save prevents the host sta=
ck
>> >>> >> from requesting active mode if it was not the host stack that
>> >>> >> requested sniff mode.
>> >>> >>
>> >>> >> I don't understand the motivation for this. If we have ACL data t=
o
>> >>> >> send, then it seems like a good idea for the host stack to explic=
itly
>> >>> >> request active mode, regardless of the reason that we entered sni=
ff
>> >>> >> mode.
>> >>> >>
>> >>> >> We want to enter active mode more aggressively when setting up SC=
O
>> >>> >> connections, to avoid a 5 second delay with certain sniff modes. =
But
>> >>> >> the conn->power_save code is getting in the way and doesn't appea=
r to
>> >>> >> be useful in the first place.
>> >>> >
>> >>> > we have discussed this a few times. And if you lock through the co=
de
>> >>> > history then you see that initially we just took devices out of sn=
iff
>> >>> > mode if we had to send data. However with HID devices this falls f=
lat on
>> >>> > its face. They need to stay in control of sniff mode if they initi=
ated
>> >>> > it. Some of them crash and others just drain the battery. With sni=
ff
>> >>> > mode you can send small amounts of data even while in sniff and fo=
r HID
>> >>> > that is sometimes used. So the remote side better not interfere.
>> >>> >
>> >>> > What we really need is a socket option where we can control this o=
n a
>> >>> > per socket basis if we take devices out of sniff mode. And one ext=
ra
>> >>> > case might be when we try to establish a SCO channel, because then=
 it is
>> >>> > clearly not an HID device. However even A2DP has this sort of prob=
lems
>> >>> > sometimes where the stream setup takes time.
>> >>>
>> >>> Makes sense. Thanks for the explanation.
>> >>
>> >> this means you will be working on a patch for this :)
>>
>> Actually, I want to put a patch together for a socket option to not
>> use power_save (so that we *always* exit sniff mode when sending ACL
>> data). We're seeing this problem with the Plantronics Voyager 855
>> which enters sniff mode during A2DP.
>>
>> Given that power_save is just for HID devices, my preferred design is
>> to disable power_save by default, and have an L2CAP socket option to
>> turn on power_save that would be used by HID L2CAP sockets.
>> Unfortunately this would require userspace HID code to use the new
>> socket option to keep current behavior. But it seems preferable to the
>> alternative of having every single other L2CAP socket use a new socket
>> option just to disable power_save for the sake of HID.
>
> actually you still mix up the meaning of the power_save option. From the
> stack side, the automatic sniff mode is off by default. You have to
> enable via sysfs first. The power_save option is just to control when
> sniff mode is activated and the remote enters sniff mode, that we are
> not getting out of it if we have ACL data.
>
> In conclusion, I am fine with a socket option that allows to control
> sniff mode (preferable with interval details) and sniff subrate details
> so that we can use them for that ACL link.
>
> So go ahead and work on this. We can fix userspace easily since a new
> socket can be detected easily with newer kernels.

We have found that some Nokia BT headsets stop working after idle
period when they enter
"power save" mode.

Do we have any solution for this problem? So far I see good enough
patch :-)) below:

http://android.git.kernel.org/?p=3Dkernel/common.git;a=3Dblobdiff;f=3Dnet/b=
luetooth/hci_conn.c;h=3Dfa8b412205cd89546b4a325c558c68040eeaf491;hp=3D0cf25=
114a3f576fc2788a549eb96d0087dd39b44;hb=3Dd8488237646920cd71ef43e5f3ae1001c6=
f4cf7b;hpb=3D3f68e5050c5ae559f56d5da9202cb88928d42b36

-       if (conn->mode !=3D HCI_CM_SNIFF || !conn->power_save)
+       if (conn->mode !=3D HCI_CM_SNIFF /* || !conn->power_save */)

Nick have you done socket option for "power_save"?

Regards,
Andrei

Re: What is the motivation for conn->power_save

[Nick Pelly]

On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
<andrei.emeltchenko.news@gmail.com> wrote:
> Hi,
>
> On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org> w=
rote:
>> Hi Nick,
>>
>>> >>> >> If I understand correctly, conn->power_save prevents the host st=
ack
>>> >>> >> from requesting active mode if it was not the host stack that
>>> >>> >> requested sniff mode.
>>> >>> >>
>>> >>> >> I don't understand the motivation for this. If we have ACL data =
to
>>> >>> >> send, then it seems like a good idea for the host stack to expli=
citly
>>> >>> >> request active mode, regardless of the reason that we entered sn=
iff
>>> >>> >> mode.
>>> >>> >>
>>> >>> >> We want to enter active mode more aggressively when setting up S=
CO
>>> >>> >> connections, to avoid a 5 second delay with certain sniff modes.=
 But
>>> >>> >> the conn->power_save code is getting in the way and doesn't appe=
ar to
>>> >>> >> be useful in the first place.
>>> >>> >
>>> >>> > we have discussed this a few times. And if you lock through the c=
ode
>>> >>> > history then you see that initially we just took devices out of s=
niff
>>> >>> > mode if we had to send data. However with HID devices this falls =
flat on
>>> >>> > its face. They need to stay in control of sniff mode if they init=
iated
>>> >>> > it. Some of them crash and others just drain the battery. With sn=
iff
>>> >>> > mode you can send small amounts of data even while in sniff and f=
or HID
>>> >>> > that is sometimes used. So the remote side better not interfere.
>>> >>> >
>>> >>> > What we really need is a socket option where we can control this =
on a
>>> >>> > per socket basis if we take devices out of sniff mode. And one ex=
tra
>>> >>> > case might be when we try to establish a SCO channel, because the=
n it is
>>> >>> > clearly not an HID device. However even A2DP has this sort of pro=
blems
>>> >>> > sometimes where the stream setup takes time.
>>> >>>
>>> >>> Makes sense. Thanks for the explanation.
>>> >>
>>> >> this means you will be working on a patch for this :)
>>>
>>> Actually, I want to put a patch together for a socket option to not
>>> use power_save (so that we *always* exit sniff mode when sending ACL
>>> data). We're seeing this problem with the Plantronics Voyager 855
>>> which enters sniff mode during A2DP.
>>>
>>> Given that power_save is just for HID devices, my preferred design is
>>> to disable power_save by default, and have an L2CAP socket option to
>>> turn on power_save that would be used by HID L2CAP sockets.
>>> Unfortunately this would require userspace HID code to use the new
>>> socket option to keep current behavior. But it seems preferable to the
>>> alternative of having every single other L2CAP socket use a new socket
>>> option just to disable power_save for the sake of HID.
>>
>> actually you still mix up the meaning of the power_save option. From the
>> stack side, the automatic sniff mode is off by default. You have to
>> enable via sysfs first. The power_save option is just to control when
>> sniff mode is activated and the remote enters sniff mode, that we are
>> not getting out of it if we have ACL data.
>>
>> In conclusion, I am fine with a socket option that allows to control
>> sniff mode (preferable with interval details) and sniff subrate details
>> so that we can use them for that ACL link.
>>
>> So go ahead and work on this. We can fix userspace easily since a new
>> socket can be detected easily with newer kernels.
>
> We have found that some Nokia BT headsets stop working after idle
> period when they enter
> "power save" mode.
>
> Do we have any solution for this problem? So far I see good enough
> patch :-)) below:
>
> http://android.git.kernel.org/?p=3Dkernel/common.git;a=3Dblobdiff;f=3Dnet=
/bluetooth/hci_conn.c;h=3Dfa8b412205cd89546b4a325c558c68040eeaf491;hp=3D0cf=
25114a3f576fc2788a549eb96d0087dd39b44;hb=3Dd8488237646920cd71ef43e5f3ae1001=
c6f4cf7b;hpb=3D3f68e5050c5ae559f56d5da9202cb88928d42b36
>
> - =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF || !conn->power_save)
> + =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF /* || !conn->power_save */=
)
>
> Nick have you done socket option for "power_save"?

No. We'll do it once we need it for HID support.

Nick

Re: What is the motivation for conn->power_save

[Liang Bao]

[-- Attachment #1: Type: text/plain, Size: 5173 bytes --]

2010/7/9 Nick Pelly <npelly@google.com>

> On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
> <andrei.emeltchenko.news@gmail.com> wrote:
> > Hi,
> >
> > On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org>
> wrote:
> >> Hi Nick,
> >>
> >>> >>> >> If I understand correctly, conn->power_save prevents the host
> stack
> >>> >>> >> from requesting active mode if it was not the host stack that
> >>> >>> >> requested sniff mode.
> >>> >>> >>
> >>> >>> >> I don't understand the motivation for this. If we have ACL data
> to
> >>> >>> >> send, then it seems like a good idea for the host stack to
> explicitly
> >>> >>> >> request active mode, regardless of the reason that we entered
> sniff
> >>> >>> >> mode.
> >>> >>> >>
> >>> >>> >> We want to enter active mode more aggressively when setting up
> SCO
> >>> >>> >> connections, to avoid a 5 second delay with certain sniff modes.
> But
> >>> >>> >> the conn->power_save code is getting in the way and doesn't
> appear to
> >>> >>> >> be useful in the first place.
> >>> >>> >
> >>> >>> > we have discussed this a few times. And if you lock through the
> code
> >>> >>> > history then you see that initially we just took devices out of
> sniff
> >>> >>> > mode if we had to send data. However with HID devices this falls
> flat on
> >>> >>> > its face. They need to stay in control of sniff mode if they
> initiated
> >>> >>> > it. Some of them crash and others just drain the battery. With
> sniff
> >>> >>> > mode you can send small amounts of data even while in sniff and
> for HID
> >>> >>> > that is sometimes used. So the remote side better not interfere.
> >>> >>> >
> >>> >>> > What we really need is a socket option where we can control this
> on a
> >>> >>> > per socket basis if we take devices out of sniff mode. And one
> extra
> >>> >>> > case might be when we try to establish a SCO channel, because
> then it is
> >>> >>> > clearly not an HID device. However even A2DP has this sort of
> problems
> >>> >>> > sometimes where the stream setup takes time.
> >>> >>>
> >>> >>> Makes sense. Thanks for the explanation.
> >>> >>
> >>> >> this means you will be working on a patch for this :)
> >>>
> >>> Actually, I want to put a patch together for a socket option to not
> >>> use power_save (so that we *always* exit sniff mode when sending ACL
> >>> data). We're seeing this problem with the Plantronics Voyager 855
> >>> which enters sniff mode during A2DP.
> >>>
> >>> Given that power_save is just for HID devices, my preferred design is
> >>> to disable power_save by default, and have an L2CAP socket option to
> >>> turn on power_save that would be used by HID L2CAP sockets.
> >>> Unfortunately this would require userspace HID code to use the new
> >>> socket option to keep current behavior. But it seems preferable to the
> >>> alternative of having every single other L2CAP socket use a new socket
> >>> option just to disable power_save for the sake of HID.
> >>
> >> actually you still mix up the meaning of the power_save option. From the
> >> stack side, the automatic sniff mode is off by default. You have to
> >> enable via sysfs first. The power_save option is just to control when
> >> sniff mode is activated and the remote enters sniff mode, that we are
> >> not getting out of it if we have ACL data.
> >>
> >> In conclusion, I am fine with a socket option that allows to control
> >> sniff mode (preferable with interval details) and sniff subrate details
> >> so that we can use them for that ACL link.
> >>
> >> So go ahead and work on this. We can fix userspace easily since a new
> >> socket can be detected easily with newer kernels.
> >
> > We have found that some Nokia BT headsets stop working after idle
> > period when they enter
> > "power save" mode.
> >
> > Do we have any solution for this problem? So far I see good enough
> > patch :-)) below:
> >
> >
> http://android.git.kernel.org/?p=kernel/common.git;a=blobdiff;f=net/bluetooth/hci_conn.c;h=fa8b412205cd89546b4a325c558c68040eeaf491;hp=0cf25114a3f576fc2788a549eb96d0087dd39b44;hb=d8488237646920cd71ef43e5f3ae1001c6f4cf7b;hpb=3f68e5050c5ae559f56d5da9202cb88928d42b36
> >
> > -       if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
> > +       if (conn->mode != HCI_CM_SNIFF /* || !conn->power_save */)
> >
> > Nick have you done socket option for "power_save"?
>
> No. We'll do it once we need it for HID support.
>
Can we do this: try to do check against some existing fields in structure
hci_conn, for example device_class and then determine if conn->power_save
shall be used here? By doing this, applications in user space can be saved
from having knowledge how sniff mode works for different devices.

Alternatively, is it possible to add another flag - I know the structure is
getting big so I agree adding something might not be a good idea. :)

>
> Nick
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[-- Attachment #2: Type: text/html, Size: 7334 bytes --]

Re: What is the motivation for conn->power_save

[Liang Bao]

Hi,

2010/7/9 Nick Pelly <npelly@google.com>
>
> On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
> <andrei.emeltchenko.news@gmail.com> wrote:
> > Hi,
> >
> > On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org>=
 wrote:
> >> Hi Nick,
> >>
> >>> >>> >> If I understand correctly, conn->power_save prevents the host =
stack
> >>> >>> >> from requesting active mode if it was not the host stack that
> >>> >>> >> requested sniff mode.
> >>> >>> >>
> >>> >>> >> I don't understand the motivation for this. If we have ACL dat=
a to
> >>> >>> >> send, then it seems like a good idea for the host stack to exp=
licitly
> >>> >>> >> request active mode, regardless of the reason that we entered =
sniff
> >>> >>> >> mode.
> >>> >>> >>
> >>> >>> >> We want to enter active mode more aggressively when setting up=
 SCO
> >>> >>> >> connections, to avoid a 5 second delay with certain sniff mode=
s. But
> >>> >>> >> the conn->power_save code is getting in the way and doesn't ap=
pear to
> >>> >>> >> be useful in the first place.
> >>> >>> >
> >>> >>> > we have discussed this a few times. And if you lock through the=
 code
> >>> >>> > history then you see that initially we just took devices out of=
 sniff
> >>> >>> > mode if we had to send data. However with HID devices this fall=
s flat on
> >>> >>> > its face. They need to stay in control of sniff mode if they in=
itiated
> >>> >>> > it. Some of them crash and others just drain the battery. With =
sniff
> >>> >>> > mode you can send small amounts of data even while in sniff and=
 for HID
> >>> >>> > that is sometimes used. So the remote side better not interfere=
.
> >>> >>> >
> >>> >>> > What we really need is a socket option where we can control thi=
s on a
> >>> >>> > per socket basis if we take devices out of sniff mode. And one =
extra
> >>> >>> > case might be when we try to establish a SCO channel, because t=
hen it is
> >>> >>> > clearly not an HID device. However even A2DP has this sort of p=
roblems
> >>> >>> > sometimes where the stream setup takes time.
> >>> >>>
> >>> >>> Makes sense. Thanks for the explanation.
> >>> >>
> >>> >> this means you will be working on a patch for this :)
> >>>
> >>> Actually, I want to put a patch together for a socket option to not
> >>> use power_save (so that we *always* exit sniff mode when sending ACL
> >>> data). We're seeing this problem with the Plantronics Voyager 855
> >>> which enters sniff mode during A2DP.
> >>>
> >>> Given that power_save is just for HID devices, my preferred design is
> >>> to disable power_save by default, and have an L2CAP socket option to
> >>> turn on power_save that would be used by HID L2CAP sockets.
> >>> Unfortunately this would require userspace HID code to use the new
> >>> socket option to keep current behavior. But it seems preferable to th=
e
> >>> alternative of having every single other L2CAP socket use a new socke=
t
> >>> option just to disable power_save for the sake of HID.
> >>
> >> actually you still mix up the meaning of the power_save option. From t=
he
> >> stack side, the automatic sniff mode is off by default. You have to
> >> enable via sysfs first. The power_save option is just to control when
> >> sniff mode is activated and the remote enters sniff mode, that we are
> >> not getting out of it if we have ACL data.
> >>
> >> In conclusion, I am fine with a socket option that allows to control
> >> sniff mode (preferable with interval details) and sniff subrate detail=
s
> >> so that we can use them for that ACL link.
> >>
> >> So go ahead and work on this. We can fix userspace easily since a new
> >> socket can be detected easily with newer kernels.
> >
> > We have found that some Nokia BT headsets stop working after idle
> > period when they enter
> > "power save" mode.
> >
> > Do we have any solution for this problem? So far I see good enough
> > patch :-)) below:
> >
> > http://android.git.kernel.org/?p=3Dkernel/common.git;a=3Dblobdiff;f=3Dn=
et/bluetooth/hci_conn.c;h=3Dfa8b412205cd89546b4a325c558c68040eeaf491;hp=3D0=
cf25114a3f576fc2788a549eb96d0087dd39b44;hb=3Dd8488237646920cd71ef43e5f3ae10=
01c6f4cf7b;hpb=3D3f68e5050c5ae559f56d5da9202cb88928d42b36
> >
> > - =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF || !conn->power_save)
> > + =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF /* || !conn->power_save =
*/)
> >
> > Nick have you done socket option for "power_save"?
>
> No. We'll do it once we need it for HID support.

Can we do this: try to do check against some existing fields in
structure hci_conn, for example device_class and then determine if
conn->power_save shall be used here? By doing this, applications in
user space can be saved from having knowledge how sniff mode works for
different devices.

Alternatively, is it possible to add another flag - I know the
structure is getting big so I agree adding something might not be a
good idea. :)
>
> Nick
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html

Re: What is the motivation for conn->power_save

[Nick Pelly]

[-- Attachment #1: Type: text/plain, Size: 5217 bytes --]

On Tue, Aug 3, 2010 at 2:31 AM, Liang Bao <tim.bao@gmail.com> wrote:

>
>
> 2010/7/9 Nick Pelly <npelly@google.com>
>
> On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
>> <andrei.emeltchenko.news@gmail.com> wrote:
>> > Hi,
>> >
>> > On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org>
>> wrote:
>> >> Hi Nick,
>> >>
>> >>> >>> >> If I understand correctly, conn->power_save prevents the host
>> stack
>> >>> >>> >> from requesting active mode if it was not the host stack that
>> >>> >>> >> requested sniff mode.
>> >>> >>> >>
>> >>> >>> >> I don't understand the motivation for this. If we have ACL data
>> to
>> >>> >>> >> send, then it seems like a good idea for the host stack to
>> explicitly
>> >>> >>> >> request active mode, regardless of the reason that we entered
>> sniff
>> >>> >>> >> mode.
>> >>> >>> >>
>> >>> >>> >> We want to enter active mode more aggressively when setting up
>> SCO
>> >>> >>> >> connections, to avoid a 5 second delay with certain sniff
>> modes. But
>> >>> >>> >> the conn->power_save code is getting in the way and doesn't
>> appear to
>> >>> >>> >> be useful in the first place.
>> >>> >>> >
>> >>> >>> > we have discussed this a few times. And if you lock through the
>> code
>> >>> >>> > history then you see that initially we just took devices out of
>> sniff
>> >>> >>> > mode if we had to send data. However with HID devices this falls
>> flat on
>> >>> >>> > its face. They need to stay in control of sniff mode if they
>> initiated
>> >>> >>> > it. Some of them crash and others just drain the battery. With
>> sniff
>> >>> >>> > mode you can send small amounts of data even while in sniff and
>> for HID
>> >>> >>> > that is sometimes used. So the remote side better not interfere.
>> >>> >>> >
>> >>> >>> > What we really need is a socket option where we can control this
>> on a
>> >>> >>> > per socket basis if we take devices out of sniff mode. And one
>> extra
>> >>> >>> > case might be when we try to establish a SCO channel, because
>> then it is
>> >>> >>> > clearly not an HID device. However even A2DP has this sort of
>> problems
>> >>> >>> > sometimes where the stream setup takes time.
>> >>> >>>
>> >>> >>> Makes sense. Thanks for the explanation.
>> >>> >>
>> >>> >> this means you will be working on a patch for this :)
>> >>>
>> >>> Actually, I want to put a patch together for a socket option to not
>> >>> use power_save (so that we *always* exit sniff mode when sending ACL
>> >>> data). We're seeing this problem with the Plantronics Voyager 855
>> >>> which enters sniff mode during A2DP.
>> >>>
>> >>> Given that power_save is just for HID devices, my preferred design is
>> >>> to disable power_save by default, and have an L2CAP socket option to
>> >>> turn on power_save that would be used by HID L2CAP sockets.
>> >>> Unfortunately this would require userspace HID code to use the new
>> >>> socket option to keep current behavior. But it seems preferable to the
>> >>> alternative of having every single other L2CAP socket use a new socket
>> >>> option just to disable power_save for the sake of HID.
>> >>
>> >> actually you still mix up the meaning of the power_save option. From
>> the
>> >> stack side, the automatic sniff mode is off by default. You have to
>> >> enable via sysfs first. The power_save option is just to control when
>> >> sniff mode is activated and the remote enters sniff mode, that we are
>> >> not getting out of it if we have ACL data.
>> >>
>> >> In conclusion, I am fine with a socket option that allows to control
>> >> sniff mode (preferable with interval details) and sniff subrate details
>> >> so that we can use them for that ACL link.
>> >>
>> >> So go ahead and work on this. We can fix userspace easily since a new
>> >> socket can be detected easily with newer kernels.
>> >
>> > We have found that some Nokia BT headsets stop working after idle
>> > period when they enter
>> > "power save" mode.
>> >
>> > Do we have any solution for this problem? So far I see good enough
>> > patch :-)) below:
>> >
>> >
>> http://android.git.kernel.org/?p=kernel/common.git;a=blobdiff;f=net/bluetooth/hci_conn.c;h=fa8b412205cd89546b4a325c558c68040eeaf491;hp=0cf25114a3f576fc2788a549eb96d0087dd39b44;hb=d8488237646920cd71ef43e5f3ae1001c6f4cf7b;hpb=3f68e5050c5ae559f56d5da9202cb88928d42b36
>> >
>> > -       if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
>> > +       if (conn->mode != HCI_CM_SNIFF /* || !conn->power_save */)
>> >
>> > Nick have you done socket option for "power_save"?
>>
>> No. We'll do it once we need it for HID support.
>>
> Can we do this: try to do check against some existing fields in structure
> hci_conn, for example device_class and then determine if conn->power_save
> shall be used here? By doing this, applications in user space can be saved
> from having knowledge how sniff mode works for different devices.
>
>
The class bits cannot be trusted - many devices have the wrong class bits.
And even with the right class bits, what about a device that supports both
HID and A2DP? A socket option still seems like the best approach.
Nick

[-- Attachment #2: Type: text/html, Size: 7242 bytes --]

Re: What is the motivation for conn->power_save

[Nick Pelly]

On Tue, Aug 3, 2010 at 2:31 AM, Liang Bao <tim.bao@gmail.com> wrote:
>
>
> 2010/7/9 Nick Pelly <npelly@google.com>
>>
>> On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
>> <andrei.emeltchenko.news@gmail.com> wrote:
>> > Hi,
>> >
>> > On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org=
> wrote:
>> >> Hi Nick,
>> >>
>> >>> >>> >> If I understand correctly, conn->power_save prevents the host=
 stack
>> >>> >>> >> from requesting active mode if it was not the host stack that
>> >>> >>> >> requested sniff mode.
>> >>> >>> >>
>> >>> >>> >> I don't understand the motivation for this. If we have ACL da=
ta to
>> >>> >>> >> send, then it seems like a good idea for the host stack to ex=
plicitly
>> >>> >>> >> request active mode, regardless of the reason that we entered=
 sniff
>> >>> >>> >> mode.
>> >>> >>> >>
>> >>> >>> >> We want to enter active mode more aggressively when setting u=
p SCO
>> >>> >>> >> connections, to avoid a 5 second delay with certain sniff mod=
es. But
>> >>> >>> >> the conn->power_save code is getting in the way and doesn't a=
ppear to
>> >>> >>> >> be useful in the first place.
>> >>> >>> >
>> >>> >>> > we have discussed this a few times. And if you lock through th=
e code
>> >>> >>> > history then you see that initially we just took devices out o=
f sniff
>> >>> >>> > mode if we had to send data. However with HID devices this fal=
ls flat on
>> >>> >>> > its face. They need to stay in control of sniff mode if they i=
nitiated
>> >>> >>> > it. Some of them crash and others just drain the battery. With=
 sniff
>> >>> >>> > mode you can send small amounts of data even while in sniff an=
d for HID
>> >>> >>> > that is sometimes used. So the remote side better not interfer=
e.
>> >>> >>> >
>> >>> >>> > What we really need is a socket option where we can control th=
is on a
>> >>> >>> > per socket basis if we take devices out of sniff mode. And one=
 extra
>> >>> >>> > case might be when we try to establish a SCO channel, because =
then it is
>> >>> >>> > clearly not an HID device. However even A2DP has this sort of =
problems
>> >>> >>> > sometimes where the stream setup takes time.
>> >>> >>>
>> >>> >>> Makes sense. Thanks for the explanation.
>> >>> >>
>> >>> >> this means you will be working on a patch for this :)
>> >>>
>> >>> Actually, I want to put a patch together for a socket option to not
>> >>> use power_save (so that we *always* exit sniff mode when sending ACL
>> >>> data). We're seeing this problem with the Plantronics Voyager 855
>> >>> which enters sniff mode during A2DP.
>> >>>
>> >>> Given that power_save is just for HID devices, my preferred design i=
s
>> >>> to disable power_save by default, and have an L2CAP socket option to
>> >>> turn on power_save that would be used by HID L2CAP sockets.
>> >>> Unfortunately this would require userspace HID code to use the new
>> >>> socket option to keep current behavior. But it seems preferable to t=
he
>> >>> alternative of having every single other L2CAP socket use a new sock=
et
>> >>> option just to disable power_save for the sake of HID.
>> >>
>> >> actually you still mix up the meaning of the power_save option. From =
the
>> >> stack side, the automatic sniff mode is off by default. You have to
>> >> enable via sysfs first. The power_save option is just to control when
>> >> sniff mode is activated and the remote enters sniff mode, that we are
>> >> not getting out of it if we have ACL data.
>> >>
>> >> In conclusion, I am fine with a socket option that allows to control
>> >> sniff mode (preferable with interval details) and sniff subrate detai=
ls
>> >> so that we can use them for that ACL link.
>> >>
>> >> So go ahead and work on this. We can fix userspace easily since a new
>> >> socket can be detected easily with newer kernels.
>> >
>> > We have found that some Nokia BT headsets stop working after idle
>> > period when they enter
>> > "power save" mode.
>> >
>> > Do we have any solution for this problem? So far I see good enough
>> > patch :-)) below:
>> >
>> > http://android.git.kernel.org/?p=3Dkernel/common.git;a=3Dblobdiff;f=3D=
net/bluetooth/hci_conn.c;h=3Dfa8b412205cd89546b4a325c558c68040eeaf491;hp=3D=
0cf25114a3f576fc2788a549eb96d0087dd39b44;hb=3Dd8488237646920cd71ef43e5f3ae1=
001c6f4cf7b;hpb=3D3f68e5050c5ae559f56d5da9202cb88928d42b36
>> >
>> > - =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF || !conn->power_save)
>> > + =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF /* || !conn->power_save=
 */)
>> >
>> > Nick have you done socket option for "power_save"?
>>
>> No. We'll do it once we need it for HID support.
>
> Can we do this: try to do check against some existing fields in structure=
 hci_conn, for example device_class and then determine if conn->power_save =
shall be used here? By doing this, applications in user space can be saved =
from having knowledge how sniff mode works for different devices.

The class bits cannot be trusted - many devices have the wrong class
bits. And even with the right class bits, what about a device that
supports both HID and A2DP? A socket option still seems like the best
approach.

Nick