Split original patch in to 3 patches
Fix patch subject lines
--
Ron Shaffer
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Certain headsets such as the Motorola H350 will reject SCO and eSCO
connection requests while the ACL is transitioning from sniff mode
to active mode. Add synchronization so that SCO and eSCO connection
requests will wait until the ACL has fully transitioned to active mode.
Signed-off-by: Ron Shaffer <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_conn.c | 18 ++++++++++++++++++
net/bluetooth/hci_event.c | 23 ++++++++++++++++++++++-
3 files changed, 41 insertions(+), 1 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index fd53323..c4a37fc 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -250,6 +250,7 @@ enum {
HCI_CONN_ENCRYPT_PEND,
HCI_CONN_RSWITCH_PEND,
HCI_CONN_MODE_CHANGE_PEND,
+ HCI_CONN_SCO_PEND,
};
static inline void hci_conn_hash_init(struct hci_dev *hdev)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 9bf4308..e900f85 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -117,9 +117,18 @@ void hci_add_sco(struct hci_conn *conn, __u16 handle)
{
struct hci_dev *hdev = conn->hdev;
struct hci_cp_add_sco cp;
+ struct hci_conn *acl = conn->link;
BT_DBG("%p", conn);
+ if (acl->mode == HCI_CM_SNIFF &&
+ test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
+ set_bit(HCI_CONN_SCO_PEND, &conn->pend);
+ return;
+ }
+
+ clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
+
conn->state = BT_CONNECT;
conn->out = 1;
@@ -135,9 +144,18 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
{
struct hci_dev *hdev = conn->hdev;
struct hci_cp_setup_sync_conn cp;
+ struct hci_conn *acl = conn->link;
BT_DBG("%p", conn);
+ if (acl->mode == HCI_CM_SNIFF &&
+ test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
+ set_bit(HCI_CONN_SCO_PEND, &conn->pend);
+ return;
+ }
+
+ clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
+
conn->state = BT_CONNECT;
conn->out = 1;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3af537a..7692db6 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -615,6 +615,7 @@ static void hci_cs_add_sco(struct hci_dev *hdev, __u8 status)
acl = hci_conn_hash_lookup_handle(hdev, handle);
if (acl && (sco = acl->link)) {
sco->state = BT_CLOSED;
+ clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
hci_proto_connect_cfm(sco, status);
hci_conn_del(sco);
@@ -760,6 +761,7 @@ static void hci_cs_setup_sync_conn(struct hci_dev *hdev, __u8 status)
acl = hci_conn_hash_lookup_handle(hdev, handle);
if (acl && (sco = acl->link)) {
sco->state = BT_CLOSED;
+ clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
hci_proto_connect_cfm(sco, status);
hci_conn_del(sco);
@@ -795,6 +797,7 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
{
struct hci_cp_exit_sniff_mode *cp;
struct hci_conn *conn;
+ struct hci_conn *sco;
BT_DBG("%s status 0x%x", hdev->name, status);
@@ -808,9 +811,17 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(cp->handle));
- if (conn)
+ if (conn) {
clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend);
+ sco = conn->link;
+ if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND, &sco->pend)) {
+ hci_proto_connect_cfm(sco, status);
+ hci_conn_del(sco);
+ }
+ }
+ }
+
hci_dev_unlock(hdev);
}
@@ -1463,6 +1474,7 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
{
struct hci_ev_mode_change *ev = (void *) skb->data;
struct hci_conn *conn;
+ struct hci_conn *sco;
BT_DBG("%s status %d", hdev->name, ev->status);
@@ -1478,6 +1490,15 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
conn->power_save = 1;
else
conn->power_save = 0;
+ } else {
+ sco = conn->link;
+ if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND,
+ &sco->pend)) {
+ if (lmp_esco_capable(hdev))
+ hci_setup_sync(sco, conn->handle);
+ else
+ hci_add_sco(sco, conn->handle);
+ }
}
}
--
1.7.0.2
--
Ron Shaffer
Qualcomm Innocation Center, Inc.
Qualcomm Innocation Center, Inc. is a member of Code Aurora Forum
Qualcomm, Inc. has reassigned rights to Code Aurora Forum. Accordingly,
as files are modified by Code Aurora Forum members, the copyright
statement will be updated.
Signed-off-by: Ron Shaffer <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 +-
net/bluetooth/hci_conn.c | 2 +-
net/bluetooth/hci_event.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4a664e6..fd53323 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1,6 +1,6 @@
/*
BlueZ - Bluetooth protocol stack for Linux
- Copyright (C) 2000-2001 Qualcomm Incorporated
+ Copyright (c) 2000-2001, 2010, Code Aurora Forum. All rights reserved.
Written 2000,2001 by Maxim Krasnyansky <[email protected]>
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index b10e3cd..9bf4308 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1,6 +1,6 @@
/*
BlueZ - Bluetooth protocol stack for Linux
- Copyright (C) 2000-2001 Qualcomm Incorporated
+ Copyright (c) 2000-2001, 2010, Code Aurora Forum. All rights reserved.
Written 2000,2001 by Maxim Krasnyansky <[email protected]>
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 6c57fc7..3af537a 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1,6 +1,6 @@
/*
BlueZ - Bluetooth protocol stack for Linux
- Copyright (C) 2000-2001 Qualcomm Incorporated
+ Copyright (c) 2000-2001, 2010, Code Aurora Forum. All rights reserved.
Written 2000,2001 by Maxim Krasnyansky <[email protected]>
--
1.7.0.2
--
Ron Shaffer
Qualcomm Innocation Center, Inc.
Qualcomm Innocation Center, Inc. is a member of Code Aurora Forum
Deleted extraneous white space from the end of several lines
Signed-off-by: Ron Shaffer <[email protected]>
---
include/net/bluetooth/hci_core.h | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e42f6ed..4a664e6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1,4 +1,4 @@
-/*
+/*
BlueZ - Bluetooth protocol stack for Linux
Copyright (C) 2000-2001 Qualcomm Incorporated
@@ -12,13 +12,13 @@
OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
- CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES
- WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
- ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES
+ WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
- ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
- COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
+ ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
+ COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
SOFTWARE IS DISCLAIMED.
*/
@@ -380,7 +380,7 @@ static inline void __hci_dev_put(struct hci_dev *d)
}
static inline void hci_dev_put(struct hci_dev *d)
-{
+{
__hci_dev_put(d);
module_put(d->owner);
}
--
1.7.0.2
--
Ron Shaffer
Qualcomm Innocation Center, Inc.
Qualcomm Innocation Center, Inc. is a member of Code Aurora Forum
Hi Marcel:
> Split original patch in to 3 patches
> Fix patch subject lines
>
Just a ping on this.
--
Ron Shaffer
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Hi Marcel,
> However what is up with the mode change. If that fails, we are still in
> sniff mode and can just proceed with the SCO setup attempt. With the
> broken hardware it will fail for sure, but with good behaving LM on the
> remote side it might just get the device out of sniff mode at that point
> and SCO setup can succeed.
>
> Why do you imply that a mode change event with a failure means that the
> ACL is no longer present?
I was thinking the Mode Change event would carry a LMP response timeout
or link supervision (LS) timeout status when the two devices lose
synchronization while in sniff just after sending exit sniff LMP PDU
(only baseband ACK). It is more clear when the LS timeout is greater
than the 30 second (immutable) LMP response timeout.
But reading the LMP spec (2.4.1) I conclude that the former cannot occur
in the exit sniff scenario and the Disconnection Complete event is the
only required event in the latter.
I agree to wait until we have a clear and real scenario and then discuss
further with those logs.
Best regards,
-Matt
Employee of the Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
Hi Ron,
> >> Certain headsets such as the Motorola H350 will reject SCO and eSCO
> >> connection requests while the ACL is transitioning from sniff mode
> >> to active mode. Add synchronization so that SCO and eSCO connection
> >> requests will wait until the ACL has fully transitioned to active mode.
> >
> > I find your patch actually highly complicated. So I tried to capture
> > your intend the in the attached patch (only compiled test) and it would
> > be good if you can try that.
> >
> > So in general it makes no difference which mode we are in. If a mode
> > change is pending, then we have to delay the SCO setup. And once we are
> > done we the mode change we just setup the SCO link. So in theory that
> > should be enough. Not sure if that is true. I could have overlooked
> > something since I don't really have tested the patch at all ;)
>
> Would have been back with you sooner on this but I'm having some
> hardware issues preventing me from testing your code. I'm fairly
> confident it works as you've consolidated the pending check into
> hci_conn which I like.
>
> However, the complicated pieces to which I think you are referring are
> the error handling pieces which we should want want to keep. These make
> sure that we clean up appropriately if:
>
> 1. The HW rejects the exit sniff or the SCO/eSCO request for some reason
> i.e. in the case the HW returns a command status with a failure
> 2. In the latter case, the user space process is also notified that its
> SCO/eSCO request has failed.
>
> Give the cleanup code some thought. I think you'll see we want this
> there to make the system more reliable.
>
> As soon as I've confirmed your patch I'll get back to you.
I think the general discussion here is what we expect a normal behaving
system to do and what would be a good idea.
So we already have the case where just always unsniff the ACL link
before establishing any SCO link. If that assumption still holds then I
think it is more than fair to wait for the mode change event as well.
My point is just that when we receive the mode change event, we don't
care if the unsniff (or unpark/unhold) for that matter was successful.
We proceed with the SCO setup.
The change is like this "if a mode change is in progress, then wait
until it finished before trying to setup SCO". I think that is a proper
way of doing this anyway. It really doesn't change anything else. If the
link is active anyway, then nothing has been changed here in the first
place.
And yes, in case of a HCI disconnect event we should clear out the
pending SCO setup request. However that is done anyway, since this flag
is stored inside the ACL connection handle. Right now I really don't see
an issue with this patch.
For the potential security concern, yes that can happen, but lets be
fair, we can't prevent this from the host anyway. Same as someone could
just block the 2.4 GHz spectrum and will disrupt calls.
Regards
Marcel
Marcel:
On 7/12/2010 5:07 PM, Marcel Holtmann wrote:
> Hi Ron,
>
>> Certain headsets such as the Motorola H350 will reject SCO and eSCO
>> connection requests while the ACL is transitioning from sniff mode
>> to active mode. Add synchronization so that SCO and eSCO connection
>> requests will wait until the ACL has fully transitioned to active mode.
>
> I find your patch actually highly complicated. So I tried to capture
> your intend the in the attached patch (only compiled test) and it would
> be good if you can try that.
>
> So in general it makes no difference which mode we are in. If a mode
> change is pending, then we have to delay the SCO setup. And once we are
> done we the mode change we just setup the SCO link. So in theory that
> should be enough. Not sure if that is true. I could have overlooked
> something since I don't really have tested the patch at all ;)
>
> Regards
>
> Marcel
>
Would have been back with you sooner on this but I'm having some
hardware issues preventing me from testing your code. I'm fairly
confident it works as you've consolidated the pending check into
hci_conn which I like.
However, the complicated pieces to which I think you are referring are
the error handling pieces which we should want want to keep. These make
sure that we clean up appropriately if:
1. The HW rejects the exit sniff or the SCO/eSCO request for some reason
i.e. in the case the HW returns a command status with a failure
2. In the latter case, the user space process is also notified that its
SCO/eSCO request has failed.
Give the cleanup code some thought. I think you'll see we want this
there to make the system more reliable.
As soon as I've confirmed your patch I'll get back to you.
In the meantime, I'll talk with Oleg and Matt regarding their responses.
--
Ron Shaffer
Employee of the Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
>Also until the SCO setup is completed the SCO socket should
>never be connected. So userspace can properly detect if SCO setup is in
>progress and route it to the speaker with a proper audio policy. So I
>think the emergency call argument is void.
Way it works (most of opensource phone systems) - when phone is "connected" to HS/CarKit with no active Ccall ACL + RFCOMM (for AT commnds) is up, but SCO is down for power savings because there's no active audio. Physically audio is routed to HS all the time while HS ACL is up.
When you enter call state phone will attempt to establish SCO over existing ACL with audio still routed to BT - this is when things get screwed up.
several seconds later unsniff will fail, ACL will get messed up and disconnected and phone will switch to speaker.
>So userspace can properly detect if SCO setup is in
>progress and route it to the speaker with a proper audio policy.
So during "normal" SCO setup (which can take up to 2.5 secs) you suggest to route audio back to speaker for 2.5 secs and then put it back to BT?:)
Oleg.
Hi Oleg,
> >> is there really a problem? The LMP will send an error via HCI.
> >
> >The attempt to start SCO after the related ACL is no longer present
> >makes no sense.
> >
> >Either after mode change event with no success or disconnection complete
> >event the result is the same; there is no valid ACL to attempt
> >SCO/eSCO.
> >
> >Error code 0x02 "unknown connection identifier" may apply for the
> >command status event for setup synchronous connection command but the
> >spec does not require it.
>
> Security classification that this case fits is partial DOS for voice services, which is
> considered to be severe for emergency calls even for few seconds.
>
> Depending on chip - It will take awhile (several seconds) before response will pop
> up and then no matter what we'll still go over sco/esco setup at top of non existing ACL as Matt says.
>
> Because whole thing is in kernel several seconds of audio may get lost before phone(userland)
> will realize that SCO failed and route audio to speaker, if ever:)
>
> I'm not big fan of having all of this complication in kernel because of couple of screwed up headsets.
> There's no problem doing similar logic in userland with tight control over particular HS and timing.
this might break for a broken headset, but that is than a problem of the
headset. Also until the SCO setup is completed the SCO socket should
never be connected. So userspace can properly detect if SCO setup is in
progress and route it to the speaker with a proper audio policy. So I
think the emergency call argument is void. And broken headset is broken
headset. If it wants to fail, it will fail eventually anyway. The patch
gives it a chance to et this working without breaking other headsets. So
disconnect event arriving on the ACL link and SCO setup pending is
something we need to fix. That is correct. However that is a different
issues actually. Since this can happen right now as well.
Regards
Marcel
Hi Matt,
> > is there really a problem? The LMP will send an error via HCI. So we do
> > get the mode changed event, but just with an error. And since we are not
> > checking the error at all, we just go ahead with the SCO setup attempt
> > in that case.
> >
>
> The attempt to start SCO after the related ACL is no longer present
> makes no sense.
>
> Either after mode change event with no success or disconnection complete
> event the result is the same; there is no valid ACL to attempt
> SCO/eSCO.
>
> Error code 0x02 "unknown connection identifier" may apply for the
> command status event for setup synchronous connection command but the
> spec does not require it.
I see the point the the disconnect event and in that case we need to
check if we have an pending SCO setup and remove that.
However what is up with the mode change. If that fails, we are still in
sniff mode and can just proceed with the SCO setup attempt. With the
broken hardware it will fail for sure, but with good behaving LM on the
remote side it might just get the device out of sniff mode at that point
and SCO setup can succeed.
Why do you imply that a mode change event with a failure means that the
ACL is no longer present?
Regards
Marcel
Marcel.
>> is there really a problem? The LMP will send an error via HCI.
>
>The attempt to start SCO after the related ACL is no longer present
>makes no sense.
>
>Either after mode change event with no success or disconnection complete
>event the result is the same; there is no valid ACL to attempt
>SCO/eSCO.
>
>Error code 0x02 "unknown connection identifier" may apply for the
>command status event for setup synchronous connection command but the
>spec does not require it.
Security classification that this case fits is partial DOS for voice services, which is
considered to be severe for emergency calls even for few seconds.
Depending on chip - It will take awhile (several seconds) before response will pop
up and then no matter what we'll still go over sco/esco setup at top of non existing ACL as Matt says.
Because whole thing is in kernel several seconds of audio may get lost before phone(userland)
will realize that SCO failed and route audio to speaker, if ever:)
I'm not big fan of having all of this complication in kernel because of couple of screwed up headsets.
There's no problem doing similar logic in userland with tight control over particular HS and timing.
Oleg.
Marcel,
On Wed, 2010-07-14 at 16:30 -0300, Marcel Holtmann wrote:
> is there really a problem? The LMP will send an error via HCI. So we do
> get the mode changed event, but just with an error. And since we are not
> checking the error at all, we just go ahead with the SCO setup attempt
> in that case.
>
The attempt to start SCO after the related ACL is no longer present
makes no sense.
Either after mode change event with no success or disconnection complete
event the result is the same; there is no valid ACL to attempt
SCO/eSCO.
Error code 0x02 "unknown connection identifier" may apply for the
command status event for setup synchronous connection command but the
spec does not require it.
-Matt
Marcel,
On Jul 14, 2010, at 2:30 PM, Marcel Holtmann <[email protected]> wrote:
>>
>>
>>
>
> is there really a problem? The LMP will send an error via HCI. So we do
> get the mode changed event, but just with an error. And since we are not
> checking the error at all, we just go ahead with the SCO setup attempt
> in that case.
The attempt to start SCO after the related ACL is no longer present makes no sense.
Either after mode change event with no success or disconnection complete event the result is the same; there is no valid ACL to attempt SCO/eSCO.
Error code 0x02 "unknown connection identifier" may apply for the command status event for setup synchronous connection command but the spec does not require it.
-Matt
Hi Oleg,
> >I find your patch actually highly complicated. So I tried to capture
> >your intend the in the attached patch (only compiled test) and it would
> >be good if you can try that.
>
> Marcel, your fix is mode delicate compared to original and has same functionality.
>
> There's small problem with both yours & Rons - there's no error handling for case when other device will never ACK's unsniff - you pretty much DOS session if there's no reply. No real HS will do that but it may present security flaw. I do not know how severe is that
>
> I attached original conversation when we 1st time seen the problem.
is there really a problem? The LMP will send an error via HCI. So we do
get the mode changed event, but just with an error. And since we are not
checking the error at all, we just go ahead with the SCO setup attempt
in that case.
Regards
Marcel
>I find your patch actually highly complicated. So I tried to capture
>your intend the in the attached patch (only compiled test) and it would
>be good if you can try that.
Marcel, your fix is mode delicate compared to original and has same functionality.
There's small problem with both yours & Rons - there's no error handling for case when other device will never ACK's unsniff - you pretty much DOS session if there's no reply. No real HS will do that but it may present security flaw. I do not know how severe is that
I attached original conversation when we 1st time seen the problem.
Oleg.
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Marcel Holtmann
Sent: Monday, July 12, 2010 5:08 PM
To: Ron Shaffer
Cc: [email protected]
Subject: Re: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state
Hi Ron,
> Certain headsets such as the Motorola H350 will reject SCO and eSCO
> connection requests while the ACL is transitioning from sniff mode
> to active mode. Add synchronization so that SCO and eSCO connection
> requests will wait until the ACL has fully transitioned to active mode.
I find your patch actually highly complicated. So I tried to capture
your intend the in the attached patch (only compiled test) and it would
be good if you can try that.
So in general it makes no difference which mode we are in. If a mode
change is pending, then we have to delay the SCO setup. And once we are
done we the mode change we just setup the SCO link. So in theory that
should be enough. Not sure if that is true. I could have overlooked
something since I don't really have tested the patch at all ;)
Regards
Marcel
Hi Ron,
> Certain headsets such as the Motorola H350 will reject SCO and eSCO
> connection requests while the ACL is transitioning from sniff mode
> to active mode. Add synchronization so that SCO and eSCO connection
> requests will wait until the ACL has fully transitioned to active mode.
I find your patch actually highly complicated. So I tried to capture
your intend the in the attached patch (only compiled test) and it would
be good if you can try that.
So in general it makes no difference which mode we are in. If a mode
change is pending, then we have to delay the SCO setup. And once we are
done we the mode change we just setup the SCO link. So in theory that
should be enough. Not sure if that is true. I could have overlooked
something since I don't really have tested the patch at all ;)
Regards
Marcel
Certain headsets such as the Motorola H350 will reject SCO and eSCO
connection requests while the ACL is transitioning from sniff mode
to active mode. Add synchronization so that SCO and eSCO connection
requests will wait until the ACL has fully transitioned to active mode.
Signed-off-by: Ron Shaffer <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_conn.c | 18 ++++++++++++++++++
net/bluetooth/hci_event.c | 22 +++++++++++++++++++++-
3 files changed, 40 insertions(+), 1 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index fd53323..c4a37fc 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -250,6 +250,7 @@ enum {
HCI_CONN_ENCRYPT_PEND,
HCI_CONN_RSWITCH_PEND,
HCI_CONN_MODE_CHANGE_PEND,
+ HCI_CONN_SCO_PEND,
};
static inline void hci_conn_hash_init(struct hci_dev *hdev)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 9bf4308..ffc67ac 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -117,9 +117,18 @@ void hci_add_sco(struct hci_conn *conn, __u16 handle)
{
struct hci_dev *hdev = conn->hdev;
struct hci_cp_add_sco cp;
+ struct hci_conn *acl = conn->link;
BT_DBG("%p", conn);
+ if (acl->mode == HCI_CM_SNIFF &&
+ test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
+ set_bit(HCI_CONN_SCO_PEND, &conn->pend);
+ return;
+ }
+
+ test_and_clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
+
conn->state = BT_CONNECT;
conn->out = 1;
@@ -135,9 +144,18 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
{
struct hci_dev *hdev = conn->hdev;
struct hci_cp_setup_sync_conn cp;
+ struct hci_conn *acl = conn->link;
BT_DBG("%p", conn);
+ if (acl->mode == HCI_CM_SNIFF &&
+ test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
+ set_bit(HCI_CONN_SCO_PEND, &conn->pend);
+ return;
+ }
+
+ test_and_clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
+
conn->state = BT_CONNECT;
conn->out = 1;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3af537a..c3a05e1 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -615,6 +615,7 @@ static void hci_cs_add_sco(struct hci_dev *hdev, __u8 status)
acl = hci_conn_hash_lookup_handle(hdev, handle);
if (acl && (sco = acl->link)) {
sco->state = BT_CLOSED;
+ clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
hci_proto_connect_cfm(sco, status);
hci_conn_del(sco);
@@ -760,6 +761,7 @@ static void hci_cs_setup_sync_conn(struct hci_dev *hdev, __u8 status)
acl = hci_conn_hash_lookup_handle(hdev, handle);
if (acl && (sco = acl->link)) {
sco->state = BT_CLOSED;
+ clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
hci_proto_connect_cfm(sco, status);
hci_conn_del(sco);
@@ -795,6 +797,7 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
{
struct hci_cp_exit_sniff_mode *cp;
struct hci_conn *conn;
+ struct hci_conn *sco;
BT_DBG("%s status 0x%x", hdev->name, status);
@@ -808,9 +811,16 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(cp->handle));
- if (conn)
+ if (conn) {
clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend);
+ sco = conn->link;
+ if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND, &sco->pend)) {
+ hci_proto_connect_cfm(sco, status);
+ hci_conn_del(sco);
+ }
+ }
+
hci_dev_unlock(hdev);
}
@@ -1463,6 +1473,7 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
{
struct hci_ev_mode_change *ev = (void *) skb->data;
struct hci_conn *conn;
+ struct hci_conn *sco;
BT_DBG("%s status %d", hdev->name, ev->status);
@@ -1478,6 +1489,15 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
conn->power_save = 1;
else
conn->power_save = 0;
+ } else {
+ sco = conn->link;
+ if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND,
+ &sco->pend)) {
+ if (lmp_esco_capable(hdev))
+ hci_setup_sync(sco, conn->handle);
+ else
+ hci_add_sco(sco, conn->handle);
+ }
}
}
--
1.7.0.2
--
Ron Shaffer
Employee of the Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
Hi Ron,
> Deleted extraneous white space from the end of several lines
>
> Signed-off-by: Ron Shaffer <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 14 +++++++-------
> 1 files changed, 7 insertions(+), 7 deletions(-)
patch has been applied. Thanks.
Regards
Marcel
Hi Ron,
> Qualcomm, Inc. has reassigned rights to Code Aurora Forum. Accordingly,
> as files are modified by Code Aurora Forum members, the copyright
> statement will be updated.
>
> Signed-off-by: Ron Shaffer <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 2 +-
> net/bluetooth/hci_conn.c | 2 +-
> net/bluetooth/hci_event.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
patch has been applied. Thanks.
Regards
Marcel
Hi Ron,
> Certain headsets such as the Motorola H350 will reject SCO and eSCO
> connection requests while the ACL is transitioning from sniff mode
> to active mode. Add synchronization so that SCO and eSCO connection
> requests will wait until the ACL has fully transitioned to active mode.
>
> Signed-off-by: Ron Shaffer <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_conn.c | 18 ++++++++++++++++++
> net/bluetooth/hci_event.c | 23 ++++++++++++++++++++++-
> 3 files changed, 41 insertions(+), 1 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index fd53323..c4a37fc 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -250,6 +250,7 @@ enum {
> HCI_CONN_ENCRYPT_PEND,
> HCI_CONN_RSWITCH_PEND,
> HCI_CONN_MODE_CHANGE_PEND,
> + HCI_CONN_SCO_PEND,
> };
>
> static inline void hci_conn_hash_init(struct hci_dev *hdev)
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 9bf4308..e900f85 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -117,9 +117,18 @@ void hci_add_sco(struct hci_conn *conn, __u16 handle)
> {
> struct hci_dev *hdev = conn->hdev;
> struct hci_cp_add_sco cp;
> + struct hci_conn *acl = conn->link;
>
> BT_DBG("%p", conn);
>
> + if (acl->mode == HCI_CM_SNIFF &&
> + test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
> + set_bit(HCI_CONN_SCO_PEND, &conn->pend);
> + return;
> + }
> +
> + clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
> +
> conn->state = BT_CONNECT;
> conn->out = 1;
>
> @@ -135,9 +144,18 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
> {
> struct hci_dev *hdev = conn->hdev;
> struct hci_cp_setup_sync_conn cp;
> + struct hci_conn *acl = conn->link;
>
> BT_DBG("%p", conn);
>
> + if (acl->mode == HCI_CM_SNIFF &&
> + test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
> + set_bit(HCI_CONN_SCO_PEND, &conn->pend);
> + return;
> + }
> +
> + clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
> +
I really would prefer test_and_clear_bit() here.
> conn->state = BT_CONNECT;
> conn->out = 1;
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 3af537a..7692db6 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -615,6 +615,7 @@ static void hci_cs_add_sco(struct hci_dev *hdev, __u8 status)
> acl = hci_conn_hash_lookup_handle(hdev, handle);
> if (acl && (sco = acl->link)) {
> sco->state = BT_CLOSED;
> + clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
>
> hci_proto_connect_cfm(sco, status);
> hci_conn_del(sco);
> @@ -760,6 +761,7 @@ static void hci_cs_setup_sync_conn(struct hci_dev *hdev, __u8 status)
> acl = hci_conn_hash_lookup_handle(hdev, handle);
> if (acl && (sco = acl->link)) {
> sco->state = BT_CLOSED;
> + clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
>
> hci_proto_connect_cfm(sco, status);
> hci_conn_del(sco);
> @@ -795,6 +797,7 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
> {
> struct hci_cp_exit_sniff_mode *cp;
> struct hci_conn *conn;
> + struct hci_conn *sco;
>
> BT_DBG("%s status 0x%x", hdev->name, status);
>
> @@ -808,9 +811,17 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
> hci_dev_lock(hdev);
>
> conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(cp->handle));
> - if (conn)
> + if (conn) {
> clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend);
>
> + sco = conn->link;
> + if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND, &sco->pend)) {
> + hci_proto_connect_cfm(sco, status);
> + hci_conn_del(sco);
> + }
> + }
> + }
> +
Something is wrong here. The } are not matching up.
> hci_dev_unlock(hdev);
> }
>
> @@ -1463,6 +1474,7 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
> {
> struct hci_ev_mode_change *ev = (void *) skb->data;
> struct hci_conn *conn;
> + struct hci_conn *sco;
>
> BT_DBG("%s status %d", hdev->name, ev->status);
>
> @@ -1478,6 +1490,15 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
> conn->power_save = 1;
> else
> conn->power_save = 0;
> + } else {
> + sco = conn->link;
> + if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND,
> + &sco->pend)) {
> + if (lmp_esco_capable(hdev))
> + hci_setup_sync(sco, conn->handle);
> + else
> + hci_add_sco(sco, conn->handle);
> + }
> }
> }
>
Regards
Marcel