Hi Marcel,
On Mon, Dec 22, 2008 at 2:18 PM, Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote:
> Hi Nick,
>
>> >>> Looking at this a bit further, I see a discrepancy between the code in
>> >>> function hci_conn_complete_evt and hci_sync_conn_complete_evt.
>> >>> Is this intentional or a bug ? Like I said below, using
>> >>> hci_conn_complete_evt (when the AG doesn't support eSCO) results in
>> >>> the connect() never returning because the socket state is not updated.
>> >>
>> >> I still have no idea what's your problem. It doesn't matter if the
>> >> remote side supports eSCO or not. The sync setup commands can be used
>> >> even for setting up SCO channels.
>> >>
>> >
>> > So here is the problem:
>> > The AG doesn't support eSCO i.e disable_esco is Y and
>> > lmp_esco_capable(dev) evaluates to 0, so we try to establish a SCO
>> > channel irrespective of whether the remote support eSCO or not.
>> >
>> > So sco_connect is called, and after the hci_connect, hcon->state is
>> > BT_CONNECT and the sk->sk_state = BT_CONNECT.
>> > When we get a response from the remote device
>> > hci_connect_complete_evt is called which sets the conn->state to
>> > BT_CONNECTED. However, as
>> > hci_proto_connect_cfm is not called (this was being called in
>> > 2.6.25) , sk->sk_state remained in BT_CONNECT state. Hence, we get
>> > stuck in bt_sock_wait_state
>> > and the connect() times out.
>>
>>
>> Explained another way:
>>
>> JK found that in 2.6.27 with "echo Y >
>> /sys/module/sco/parameters/disable_esco" the socket state is never
>> updated to BT_CONNECTED. The userspace connect() call then times out,
>> even though the transport is connected. This appears to be because
>> hci_proto_connect_cfm() is not called when the connection complete
>> event arrives. Below is a patch that resolves this issue for us.
>>
>> This is a request for comment as to whether this patch is correct. It
>> contradicts Marcel's change 769be974 which explicitly moved
>> hci_proto_connect_cfm() into the ev->status conditional.
>
> now I get what you are talking about. You guys should have sent the
> patch from the beginning since that makes it clear what you are trying
> to achieve.
>
> So the patch is wrong, because it breaks Simple Pairing. What you have
> to do is check if it is an ACL link, then the code is doing the right
> thing and doing features requests first.
>
> if (ev->status) {
> hci_proto_connect_cfm(conn, ev->status);
> hci_conn_del(conn);
> } else if (ev->link_type != ACL_LINK)
> hci_proto_connect_cfm(conn, ev->status);
>
> Regards
>
> Marcel
>
>
>
Sorry for not sending the patch before, we did the patch only on Friday.
Will change it according to your suggestion and submit.
Thanks
Jaikumar
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
