Re: [PATCH v1] Bluetooth: Fix cvsd sco setup failure

[Luiz Augusto von Dentz]

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

>  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