Message-ID: <5f25455c-1801-66d6-578c-49e59cbb5d7b@quicinc.com>
Date:   Tue, 19 Jul 2022 11:37:12 +0800
From:   quic_zijuhu <quic_zijuhu@...cinc.com>
To:     Luiz Augusto von Dentz <luiz.dentz@...il.com>
CC:     Marcel Holtmann <marcel@...tmann.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        David Miller <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Luiz Augusto Von Dentz <luiz.von.dentz@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "linux-bluetooth@...r.kernel.org" <linux-bluetooth@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>,
        "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>
Subject: Re: [PATCH v1] Bluetooth: Fix cvsd sco setup failure

On 7/19/2022 7:59 AM, Luiz Augusto von Dentz wrote:
> Hi Quic_zijuhu,
> 
> On Thu, Jul 14, 2022 at 10:25 PM quic_zijuhu <quic_zijuhu@...cinc.com> wrote:
>>
>> On 7/15/2022 12:21 PM, Luiz Augusto von Dentz wrote:
>>> Hi,
>>>
>>> On Thu, Jul 14, 2022 at 8:31 PM quic_zijuhu <quic_zijuhu@...cinc.com> wrote:
>>>>
>>>> On 7/15/2022 5:24 AM, Luiz Augusto von Dentz wrote:
>>>>> Hi Zijun,
>>>>>
>>>>> On Thu, Jul 14, 2022 at 12:14 AM Zijun Hu <quic_zijuhu@...cinc.com> wrote:
>>>>>>
>>>>>> A cvsd sco setup failure issue is reported as shown by
>>>>>> below btmon log, it firstly tries to set up cvsd esco with
>>>>>> S3/S2/S1 configs sequentially, but these attempts are all
>>>>>> failed with error code "Unspecified Error (0x1f)", then it
>>>>>> tries to set up cvsd sco with D1 config, unfortunately, it
>>>>>> still fails to set up sco with error code
>>>>>> "Invalid HCI Command Parameters (0x12)", this error code
>>>>>> terminates attempt with remaining D0 config and marks overall
>>>>>> sco/esco setup failure.
>>>>>>
>>>>>> It is wrong D1/D0 @retrans_effort 0x01 within @esco_param_cvsd
>>>>>> that causes D1 config failure with error code
>>>>>> "Invalid HCI Command Parameters (0x12)", D1/D0 sco @retrans_effort
>>>>>> must not be 0x01 based on spec, so fix this issue by changing D1/D0
>>>>>> @retrans_effort from 0x01 to 0xff as present @sco_param_cvsd.
>>>>>
>>>>> Please quote the spec regarding the invalid parameters:
>>>>>
>>>> BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 1, Part F
>>>> page 375
>>>>
>>>> 2.18 INVALID HCI COMMAND PARAMETERS (0x12)
>>>> The Invalid HCI Command Parameters error code indicates that at least one of
>>>> the HCI command parameters is invalid.
>>>> This shall be used when:
>>>> • the parameter total length is invalid.
>>>> • a command parameter is an invalid type.
>>>> • a connection identifier does not match the corresponding event.
>>>> • a parameter is odd when it is required to be even.
>>>> • a parameter is outside of the specified range.
>>>> • two or more parameter values have inconsistent values.
>>>> Note: An invalid type can be, for example, when a SCO Connection_Handle is
>>>> used where an ACL Connection_Handle is required.
>>>>
>>>>> BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E
>>>>> page 1891
>>>>>
>>>>> 0x01 At least one retransmission, optimize for power consumption (eSCO con-
>>>>> nection required).
>>>>>
>>>>>> < HCI Command: Setup Synchrono.. (0x01|0x0028) plen 17  #3405 [hci0]
>>>>>>         Handle: 3
>>>>>>         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               #3406 [hci0]
>>>>>>       Setup Synchronous Connection (0x01|0x0028) ncmd 1
>>>>>>         Status: Success (0x00)
>>>>>>> HCI Event: Synchronous Connect Comp.. (0x2c) plen 17  #3408 [hci0]
>>>>>>         Status: Unspecified Error (0x1f)
>>>>>>         Handle: 4
>>>>>>         Address: 14:3F:A6:47:56:15 (OUI 14-3F-A6)
>>>>>>         Link type: eSCO (0x02)
>>>>>>         Transmission interval: 0x00
>>>>>>         Retransmission window: 0x00
>>>>>>         RX packet length: 0
>>>>>>         TX packet length: 0
>>>>>>         Air mode: CVSD (0x02)
>>>>>> < HCI Command: Setup Synchrono.. (0x01|0x0028) plen 17  #3409 [hci0]
>>>>>>         Handle: 3
>>>>>>         Transmit bandwidth: 8000
>>>>>>         Receive bandwidth: 8000
>>>>>>         Max latency: 7
>>>>>>         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               #3410 [hci0]
>>>>>>       Setup Synchronous Connection (0x01|0x0028) ncmd 1
>>>>>>         Status: Success (0x00)
>>>>>>> HCI Event: Synchronous Connect Comp.. (0x2c) plen 17  #3416 [hci0]
>>>>>>         Status: Unspecified Error (0x1f)
>>>>>>         Handle: 4
>>>>>>         Address: 14:3F:A6:47:56:15 (OUI 14-3F-A6)
>>>>>>         Link type: eSCO (0x02)
>>>>>>         Transmission interval: 0x00
>>>>>>         Retransmission window: 0x00
>>>>>>         RX packet length: 0
>>>>>>         TX packet length: 0
>>>>>>         Air mode: CVSD (0x02)
>>>>>> < HCI Command: Setup Synchrono.. (0x01|0x0028) plen 17  #3417 [hci0]
>>>>>>         Handle: 3
>>>>>>         Transmit bandwidth: 8000
>>>>>>         Receive bandwidth: 8000
>>>>>>         Max latency: 7
>>>>>>         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: 0x03c8
>>>>>>           EV3 may be used
>>>>>>           2-EV3 may not be used
>>>>>>           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               #3419 [hci0]
>>>>>>       Setup Synchronous Connection (0x01|0x0028) ncmd 1
>>>>>>         Status: Success (0x00)
>>>>>>> HCI Event: Synchronous Connect Comp.. (0x2c) plen 17  #3426 [hci0]
>>>>>>         Status: Unspecified Error (0x1f)
>>>>>>         Handle: 4
>>>>>>         Address: 14:3F:A6:47:56:15 (OUI 14-3F-A6)
>>>>>>         Link type: eSCO (0x02)
>>>>>>         Transmission interval: 0x00
>>>>>>         Retransmission window: 0x00
>>>>>>         RX packet length: 0
>>>>>>         TX packet length: 0
>>>>>>         Air mode: CVSD (0x02)
>>>>>> < HCI Command: Setup Synchrono.. (0x01|0x0028) plen 17  #3427 [hci0]
>>>>>>         Handle: 3
>>>>>>         Transmit bandwidth: 8000
>>>>>>         Receive bandwidth: 8000
>>>>>>         Max latency: 65535
>>>>>>         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: 0x03c4
>>>>>>           HV3 may be used
>>>>>>           2-EV3 may not be used
>>>>>>           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               #3428 [hci0]
>>>>>>       Setup Synchronous Connection (0x01|0x0028) ncmd 1
>>>>>>         Status: Success (0x00)
>>>>>>> HCI Event: Synchronous Connect Comp.. (0x2c) plen 17  #3429 [hci0]
>>>>>>         Status: Invalid HCI Command Parameters (0x12)
>>>>>>         Handle: 0
>>>>>>         Address: 14:3F:A6:47:56:15 (OUI 14-3F-A6)
>>>>>>         Link type: SCO (0x00)
>>>>>>         Transmission interval: 0x00
>>>>>>         Retransmission window: 0x00
>>>>>>         RX packet length: 0
>>>>>>         TX packet length: 0
>>>>>>         Air mode: u-law log (0x00)
>>>>>
>>>>> This really sounds like the controller fault, it seem to be picking up
>>>>> SCO based on packet type alone instead of checking if retransmission
>>>>> is suggesting to use eSCO instead, otherwise there is no use to define
>>>>> D1/D0 for both eSCO and SCO since the controller will always pick SCO
>>>>> instead.
>>>>>
>>>> i don't agree with you about above opinion:
>>>> S3/S2/S1 here is for eSCO but D1/D0 is for SCO, it should try to set up
>>>> SCO after all eSCO setup failures based HFP_v1.8 spec, so it is reasonable to
>>>> return "Invalid HCI Command Parameters" for SCO setup with retransmission parameter
>>>> 0x01 since SCO doesn't need retransmission.
>>>>
>>>> the spec doesn't say it is available for D1/D0 on eSCO.
>>>>
>>>> Hands-Free Profile V1.8 | page 133
>>>>
>>>> 5.7.1.1 Selection of Synchronous Transport
>>>> To select the type of synchronous transport (eSCO or SCO) to use, devices shall adhere to the following
>>>> logic:
>>>> • If eSCO is supported by the responder, the synchronous connection shall first be attempted on an
>>>> eSCO logical transport. See section 5.7.1.2
>>>> • If eSCO is unavailable for use (e.g., not supported by the Responder or link establishment fails),
>>>> and SCO is not currently forbidden because a BR/EDR secure connection is being used, the
>>>> Initiator shall open a SCO logical connection. See section 5.7.1.3.
>>>>
>>>> Hands-Free Profile V1.8 | page 115
>>>> 5.7.1.3 Negotiation of SCO Configuration Parameters
>>>> Requirements related to the use of SCO links, under the conditions when the use of a SCO logical
>>>> transport is permitted, are covered by the parameter sets D0-D1.
>>>>
>>>> Hands-Free Profile V1.8 | page 24
>>>> shows a summary of the mapping of codec requirements on link features for this profile.
>>>> Feature Support in HF Support in AG
>>>> 1. D0 – CVSD on SCO link (HV1) M M
>>>> 2. D1 – CVSD on SCO link (HV3) M M
>>>> 3. S1 – CVSD eSCO link (EV3) M M
>>>> 4. S2 – CVSD on EDR eSCO link (2-EV3) M M
>>>> 5. S3 – CVSD on EDR eSCO link (2-EV3) M M
>>>
>>> If D0-D1 are SCO only, then yes but then they should not even be part
>>> of the eSCO table, still I don't think the controller should be
>>> looking just to packet type or these types are not supported in eSCO?
>>>
>> Per BT 5.3 errata, BT controller will not retry SCO connection if eSCO connection failure. Instead, host shall retry sco connection with retransmission effort =0
>> you are right in theory, but it maybe be the simplest fix to correct D1/D0 retransmission parameter within the table in practice
>> the table esco_param_cvsd maybe be regarded as eSCO entry table.
>>
>> the failure reason is shown below based on F/W team explanation:
>> we set retransmission parameter with 0x01 to request controller to setup eSCO but the peer only accept SCO within
>> suggested retransmission parameter is 0x00.
> 
> I guess it must be some LL message then since I don't see anything in
> the traces that would indicate the peer only accept SCO within
> suggested retransmission.
> 
yes, your are right.
so i need to remove D1/D0 from @esco_param_cvsd based on discussion.
right?
>> so the failure reason is "choice between SCO and eSCO" not "packet type is not supported over eSCO"
>>
>>>>>> Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
>>>>>> Tested-by: Zijun Hu <quic_zijuhu@...cinc.com>
>>>>>> ---
>>>>>>  net/bluetooth/hci_conn.c | 4 ++--
>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>>>>>> index 7829433d54c1..2627d5ac15d6 100644
>>>>>> --- a/net/bluetooth/hci_conn.c
>>>>>> +++ b/net/bluetooth/hci_conn.c
>>>>>> @@ -45,8 +45,8 @@ static const struct sco_param esco_param_cvsd[] = {
>>>>>>         { EDR_ESCO_MASK & ~ESCO_2EV3, 0x000a,   0x01 }, /* S3 */
>>>>>>         { EDR_ESCO_MASK & ~ESCO_2EV3, 0x0007,   0x01 }, /* S2 */
>>>>>>         { EDR_ESCO_MASK | ESCO_EV3,   0x0007,   0x01 }, /* S1 */
>>>>>> -       { EDR_ESCO_MASK | ESCO_HV3,   0xffff,   0x01 }, /* D1 */
>>>>>> -       { EDR_ESCO_MASK | ESCO_HV1,   0xffff,   0x01 }, /* D0 */
>>>>>> +       { EDR_ESCO_MASK | ESCO_HV3,   0xffff,   0xff }, /* D1 */
>>>>>> +       { EDR_ESCO_MASK | ESCO_HV1,   0xffff,   0xff }, /* D0 */
>>>>>>  };
>>>>>
>>>>> This doesn't seem right, you are changing the parameters for eSCO
>>>>> table not SCO, which further reinforce this is probably the controller
>>>>> not really doing its job and checking if retransmission is actually
>>>>> meant for eSCO rather than SCO.
>>>>>
>>>> here it is D1/D0 SCO setup after S3/S2/S1 eSCO attempts failure as above my comments.
>>>
>>> Well then all we need to do is to remove the last 2 lines since they
>>> are already handled by the table below.
>>>
>>   if so, we must handle the fallback requirement with more complex logic.
> 
> Yeah it sounds like we didn't think about adding a fallback but
> perhaps it is as simple as changing find_next_esco_param to start
> returning the entry itself instead of an index, that would IMO have
> made this issue more visible.
> 
we maybe take the fallback requirements as another requirement to develop.
it is a good idea to have fallback you suggest.
>>>>>>  static const struct sco_param sco_param_cvsd[] = {
>>>>>> --
>>>>>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
> 
> 

Powered by blists - more mailing lists