All of lore.kernel.org
 help / color / mirror / Atom feed
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>,
	"Ildar Kamaletdinov" <i.kamaletdinov@omprussia.ru>
Subject: Re: [PATCH RFC] bluetooth: add support for some old headsets
Date: Fri, 17 Jul 2020 22:12:09 +0300	[thread overview]
Message-ID: <0c2a8da1-6071-6597-d0d1-32ce1490aba7@omprussia.ru> (raw)
In-Reply-To: <848144D3-85F9-47F8-8CDA-02457FA7530F@holtmann.org>

On 7/17/20 9:59 AM, 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)
 
> I double check with the specification and it is not precise that errors should be reported
> via sync conn complete events. My assumption would be that your headset only supports SCO and
> thus the controller realizes that eSCO request can not be completed anyway. So the controller
> opts for quickest path to get out of this.

>>>> Based on the patch by Ildar Kamaletdinov <i.kamaletdinov@omprussia.ru>.

   Adding him to CC...

>>>>
>>>> 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) {

    Actually, I was expecting that you'd tell me to create a HCI quirk for this situation.
I have a patch doing that but I haven't been able to locate the driver in which to set this
quirk flag...

>>>> +				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... 

> It is the case. Command status is only local to command we issued and thus in this case it
> is the connection creation attempt from our side. Meaning it is always outgoing.

   Ildar, what do you think?

> Regards
> 
> Marcel

MBR, Sergei

  reply	other threads:[~2020-07-17 19:12 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
2020-07-17  6:59     ` Marcel Holtmann
2020-07-17 19:12       ` Sergey Shtylyov [this message]
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=0c2a8da1-6071-6597-d0d1-32ce1490aba7@omprussia.ru \
    --to=s.shtylyov@omprussia.ru \
    --cc=davem@davemloft.net \
    --cc=i.kamaletdinov@omprussia.ru \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.