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@xxxxxxxxxxx> 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@xxxxxxxxxxx> 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@xxxxxxxxxxx> >>>> Tested-by: Zijun Hu <quic_zijuhu@xxxxxxxxxxx> >>>> --- >>>> 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 >>>> >>> >>> >> > >