All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] Bluetooth: Fix CVSD SCO setup failure
@ 2022-06-09 11:03 Zijun Hu
  2022-06-09 12:00 ` [v1] " bluez.test.bot
  0 siblings, 1 reply; 9+ messages in thread
From: Zijun Hu @ 2022-06-09 11:03 UTC (permalink / raw)
  To: marcel, johan.hedberg, luiz.dentz, davem, edumazet, kuba, pabeni
  Cc: linux-kernel, linux-bluetooth, linux-arm-msm, Zijun Hu

It will set up SCO after all CVSD eSCO attempts failure, but
still fails to setup SCO finally due to wrong D1/D0 @retrans_effort
within @esco_param_cvsd, so change it from 0x1 to 0xff to avoid
Invalid HCI Command Parameters error.

< HCI Command: Setup Synchrono.. (0x01|0x0028) plen 17  #3427
        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
      Setup Synchronous Connection (0x01|0x0028) ncmd 1
        Status: Success (0x00)
> HCI Event: Synchronous Connect Comp.. (0x2c) plen 17  #3429
        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)

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.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 */
 };
 
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* RE: [v1] Bluetooth: Fix CVSD SCO setup failure
  2022-06-09 11:03 [PATCH v1] Bluetooth: Fix CVSD SCO setup failure Zijun Hu
@ 2022-06-09 12:00 ` bluez.test.bot
  0 siblings, 0 replies; 9+ messages in thread
From: bluez.test.bot @ 2022-06-09 12:00 UTC (permalink / raw)
  To: linux-bluetooth, quic_zijuhu

[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=648803

---Test result---

Test Summary:
CheckPatch                    PASS      0.83 seconds
GitLint                       PASS      0.41 seconds
SubjectPrefix                 PASS      0.30 seconds
BuildKernel                   PASS      34.73 seconds
BuildKernel32                 PASS      33.51 seconds
Incremental Build with patchesPASS      43.15 seconds
TestRunner: Setup             PASS      543.61 seconds
TestRunner: l2cap-tester      PASS      19.00 seconds
TestRunner: bnep-tester       PASS      6.61 seconds
TestRunner: mgmt-tester       PASS      114.47 seconds
TestRunner: rfcomm-tester     PASS      10.30 seconds
TestRunner: sco-tester        PASS      10.04 seconds
TestRunner: smp-tester        PASS      10.00 seconds
TestRunner: userchan-tester   PASS      6.76 seconds



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1] Bluetooth: Fix cvsd sco setup failure
  2022-07-18 23:59         ` Luiz Augusto von Dentz
@ 2022-07-19  3:37           ` quic_zijuhu
  0 siblings, 0 replies; 9+ messages in thread
From: quic_zijuhu @ 2022-07-19  3:37 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, Johan Hedberg, David Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Luiz Augusto Von Dentz,
	Linux Kernel Mailing List, linux-bluetooth, linux-arm-msm,
	open list:NETWORKING [GENERAL]

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@quicinc.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@quicinc.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@quicinc.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@quicinc.com>
>>>>>> Tested-by: Zijun Hu <quic_zijuhu@quicinc.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
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1] Bluetooth: Fix cvsd sco setup failure
  2022-07-15  5:25       ` quic_zijuhu
@ 2022-07-18 23:59         ` Luiz Augusto von Dentz
  2022-07-19  3:37           ` quic_zijuhu
  0 siblings, 1 reply; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2022-07-18 23:59 UTC (permalink / raw)
  To: quic_zijuhu
  Cc: Marcel Holtmann, Johan Hedberg, David Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Luiz Augusto Von Dentz,
	Linux Kernel Mailing List, linux-bluetooth, linux-arm-msm,
	open list:NETWORKING [GENERAL]

Hi Quic_zijuhu,

On Thu, Jul 14, 2022 at 10:25 PM quic_zijuhu <quic_zijuhu@quicinc.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@quicinc.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@quicinc.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.

> 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@quicinc.com>
> >>>> Tested-by: Zijun Hu <quic_zijuhu@quicinc.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.

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


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1] Bluetooth: Fix cvsd sco setup failure
  2022-07-15  4:21     ` Luiz Augusto von Dentz
@ 2022-07-15  5:25       ` quic_zijuhu
  2022-07-18 23:59         ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 9+ messages in thread
From: quic_zijuhu @ 2022-07-15  5:25 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, Johan Hedberg, David Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Luiz Augusto Von Dentz,
	Linux Kernel Mailing List, linux-bluetooth, linux-arm-msm,
	open list:NETWORKING [GENERAL]

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@quicinc.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@quicinc.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.

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@quicinc.com>
>>>> Tested-by: Zijun Hu <quic_zijuhu@quicinc.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.
>>>>  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
>>>>
>>>
>>>
>>
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1] Bluetooth: Fix cvsd sco setup failure
  2022-07-15  3:31   ` quic_zijuhu
@ 2022-07-15  4:21     ` Luiz Augusto von Dentz
  2022-07-15  5:25       ` quic_zijuhu
  0 siblings, 1 reply; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2022-07-15  4:21 UTC (permalink / raw)
  To: quic_zijuhu
  Cc: Marcel Holtmann, Johan Hedberg, David Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Luiz Augusto Von Dentz,
	Linux Kernel Mailing List, linux-bluetooth, linux-arm-msm,
	open list:NETWORKING [GENERAL]

Hi,

On Thu, Jul 14, 2022 at 8:31 PM quic_zijuhu <quic_zijuhu@quicinc.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@quicinc.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?

> >> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> >> Tested-by: Zijun Hu <quic_zijuhu@quicinc.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.

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


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1] Bluetooth: Fix cvsd sco setup failure
  2022-07-14 21:24 ` Luiz Augusto von Dentz
@ 2022-07-15  3:31   ` quic_zijuhu
  2022-07-15  4:21     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 9+ messages in thread
From: quic_zijuhu @ 2022-07-15  3:31 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, Johan Hedberg, David Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Luiz Augusto Von Dentz,
	Linux Kernel Mailing List, linux-bluetooth, linux-arm-msm,
	open list:NETWORKING [GENERAL]

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@quicinc.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

>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
>> Tested-by: Zijun Hu <quic_zijuhu@quicinc.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.
>>  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
>>
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1] Bluetooth: Fix cvsd sco setup failure
  2022-07-14  7:14 [PATCH v1] Bluetooth: Fix cvsd sco " Zijun Hu
@ 2022-07-14 21:24 ` Luiz Augusto von Dentz
  2022-07-15  3:31   ` quic_zijuhu
  0 siblings, 1 reply; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2022-07-14 21:24 UTC (permalink / raw)
  To: Zijun Hu
  Cc: Marcel Holtmann, Johan Hedberg, David Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Luiz Augusto Von Dentz,
	Linux Kernel Mailing List, linux-bluetooth, linux-arm-msm,
	open list:NETWORKING [GENERAL]

Hi Zijun,

On Thu, Jul 14, 2022 at 12:14 AM Zijun Hu <quic_zijuhu@quicinc.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 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.

> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> Tested-by: Zijun Hu <quic_zijuhu@quicinc.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.

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


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1] Bluetooth: Fix cvsd sco setup failure
@ 2022-07-14  7:14 Zijun Hu
  2022-07-14 21:24 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 9+ messages in thread
From: Zijun Hu @ 2022-07-14  7:14 UTC (permalink / raw)
  To: marcel, johan.hedberg, luiz.dentz, davem, edumazet, kuba, pabeni,
	luiz.von.dentz, quic_zijuhu
  Cc: linux-kernel, linux-bluetooth, linux-arm-msm, netdev

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.

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

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
Tested-by: Zijun Hu <quic_zijuhu@quicinc.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 */
 };
 
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-07-19  3:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-09 11:03 [PATCH v1] Bluetooth: Fix CVSD SCO setup failure Zijun Hu
2022-06-09 12:00 ` [v1] " bluez.test.bot
2022-07-14  7:14 [PATCH v1] Bluetooth: Fix cvsd sco " Zijun Hu
2022-07-14 21:24 ` Luiz Augusto von Dentz
2022-07-15  3:31   ` quic_zijuhu
2022-07-15  4:21     ` Luiz Augusto von Dentz
2022-07-15  5:25       ` quic_zijuhu
2022-07-18 23:59         ` Luiz Augusto von Dentz
2022-07-19  3:37           ` quic_zijuhu

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.