From: Sergey Shtylyov <s.shtylyov@omprussia.ru>
To: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>,
Bluetooth Kernel Mailing List <linux-bluetooth@vger.kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>, <netdev@vger.kernel.org>
Subject: Re: [PATCH RFC] bluetooth: add support for some old headsets
Date: Thu, 16 Jul 2020 23:33:40 +0300 [thread overview]
Message-ID: <e9f32310-2728-60a2-adc7-3a7418ce54e3@omprussia.ru> (raw)
In-Reply-To: <1834765D-52E6-45B8-9923-778C9182CFA9@holtmann.org>
Hello!
On 7/16/20 4:14 PM, Marcel Holtmann wrote:
>> The MediaTek Bluetooth platform (MT6630 etc.) has a peculiar implementation
>> for the eSCO/SCO connection via BT/EDR: the host controller returns error
>> code 0x20 (LMP feature not supported) for HCI_Setup_Synchronous_Connection
>> (0x0028) command without actually trying to setup connection with a remote
>> device in case such device (like Digma BT-14 headset) didn't advertise its
>> supported features. Even though this doesn't break compatibility with the
>> Bluetooth standard it breaks the compatibility with the Hands-Free Profile
>> (HFP).
>>
>> This patch returns the compatibility with the HFP profile and actually
>> tries to check all available connection parameters despite of the specific
>> MediaTek implementation. Without it one was unable to establish eSCO/SCO
>> connection with some headsets.
>
> please include the parts of btmon output that show this issue.
Funny, I had removed that part from the original patch. Here's that log:
< HCI Command: Setup Synchronous Connection (0x01|0x0028) plen 17 #1 [hci0] 6.705320
Handle: 50
Transmit bandwidth: 8000
Receive bandwidth: 8000
Max latency: 10
Setting: 0x0060
Input Coding: Linear
Input Data Format: 2's complement
Input Sample Size: 16-bit
of bits padding at MSB: 0
Air Coding Format: CVSD
Retransmission effort: Optimize for power consumption (0x01)
Packet type: 0x0380
3-EV3 may not be used
2-EV5 may not be used
3-EV5 may not be used
> HCI Event: Command Status (0x0f) plen 4 #2 [hci0] 6.719598
Setup Synchronous Connection (0x01|0x0028) ncmd 1
Status: Unsupported LMP Parameter Value / Unsupported LL Parameter Value (0x20)
>> Based on the patch by Ildar Kamaletdinov <i.kamaletdinov@omprussia.ru>.
>>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omprussia.ru>
>>
>> ---
>> This patch is against the 'bluetooth-next.git' repo.
>>
>> net/bluetooth/hci_event.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> Index: bluetooth-next/net/bluetooth/hci_event.c
>> ===================================================================
>> --- bluetooth-next.orig/net/bluetooth/hci_event.c
>> +++ bluetooth-next/net/bluetooth/hci_event.c
>> @@ -2187,6 +2187,13 @@ static void hci_cs_setup_sync_conn(struc
>> if (acl) {
>> sco = acl->link;
>> if (sco) {
>> + if (status == 0x20 && /* Unsupported LMP Parameter value */
>> + sco->out) {
>> + sco->pkt_type = (hdev->esco_type & SCO_ESCO_MASK) |
>> + (hdev->esco_type & EDR_ESCO_MASK);
>> + if (hci_setup_sync(sco, sco->link->handle))
>> + goto unlock;
>> + }
>> sco->state = BT_CLOSED;
>
> since this is the command status event, I doubt that sco->out check is needed.
Can't comment oin this, my BT fu is too weak...
> And I would start with a switch statement right away.
Funny, I had removed the *switch* statement from the original patch... :-)
> I also think that we need to re-structure this hci_cs_setup_sync_conn function a little to avoid the deep indentation.
> Make it look more like hci_sync_conn_complete_evt also use a switch statement even if right now we only have one
> entry.
Indeed, done now. :-)
> Regards
>
> Marcel
MBR, Sergey
next prev parent reply other threads:[~2020-07-16 20:41 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-16 11:08 [PATCH RFC] bluetooth: add support for some old headsets Sergey Shtylyov
2020-07-16 13:14 ` Marcel Holtmann
2020-07-16 20:33 ` Sergey Shtylyov [this message]
2020-07-17 6:59 ` Marcel Holtmann
2020-07-17 19:12 ` Sergey Shtylyov
2020-07-21 18:56 ` Sergey Shtylyov
2020-07-28 7:16 ` Marcel Holtmann
2020-07-28 13:27 ` Ildar Kamaletdinov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e9f32310-2728-60a2-adc7-3a7418ce54e3@omprussia.ru \
--to=s.shtylyov@omprussia.ru \
--cc=davem@davemloft.net \
--cc=johan.hedberg@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-bluetooth@vger.kernel.org \
--cc=marcel@holtmann.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).