From 88e7b15ce16c1e04b4c4f3214946bcfbc7984875 Mon Sep 17 00:00:00 2001 From: Mudumba Ananth Date: Wed, 25 Mar 2015 05:20:02 -0700 Subject: [PATCH] Resolved hardware error observed during SCO Connection setup Hardware error was caused due to a faulty HCI command formed in the process of vendor specific pre-SCO setup in the stack(set_audio_state) Fixed the above problem and also added back the vendor (interface) mapping for the set_audio_state functionality to facilitate the sending of the pre-SCO vendor specific commands. Made common vendor library audio setting by moving to hci subsystem. General cleanup around this functionality. Bug: 19923226 Change-Id: I4a743f6725459f360bd2a90e0a46f08fcca2292d --- bta/ag/bta_ag_sco.c | 21 ++++++++--------- bta/include/bta_ag_api.h | 2 ++ bta/include/bta_ag_co.h | 13 ++--------- btif/co/bta_ag_co.c | 44 ++++++++-------------------------- hci/Android.mk | 1 + hci/include/bt_hci_bdroid.h | 9 ------- hci/include/bt_hci_lib.h | 5 ---- hci/include/hci_audio.h | 40 +++++++++++++++++++++++++++++++ hci/include/vendor.h | 3 ++- hci/src/hci_audio.c | 39 +++++++++++++++++++++++++++++++ main/bte_main.c | 57 --------------------------------------------- stack/include/btm_api.h | 2 ++ 12 files changed, 108 insertions(+), 128 deletions(-) create mode 100644 hci/include/hci_audio.h create mode 100644 hci/src/hci_audio.c diff --git a/bta/ag/bta_ag_sco.c b/bta/ag/bta_ag_sco.c index ec00514d4..4261621f3 100644 --- a/bta/ag/bta_ag_sco.c +++ b/bta/ag/bta_ag_sco.c @@ -559,8 +559,8 @@ static void bta_ag_create_sco(tBTA_AG_SCB *p_scb, BOOLEAN is_orig) #if (BTM_WBS_INCLUDED == TRUE ) /* Allow any platform specific pre-SCO set up to take place */ - bta_ag_co_audio_state(bta_ag_scb_to_idx(p_scb), p_scb->app_id, BTA_AG_CO_AUD_STATE_SETUP,\ - esco_codec); + bta_ag_co_audio_state(bta_ag_scb_to_idx(p_scb), p_scb->app_id, SCO_STATE_SETUP, + esco_codec); /* This setting may not be necessary */ /* To be verified with stable 2049 boards */ @@ -572,7 +572,7 @@ static void bta_ag_create_sco(tBTA_AG_SCB *p_scb, BOOLEAN is_orig) p_scb->inuse_codec = esco_codec; #else /* Allow any platform specific pre-SCO set up to take place */ - bta_ag_co_audio_state(bta_ag_scb_to_idx(p_scb), p_scb->app_id, BTA_AG_CO_AUD_STATE_SETUP); + bta_ag_co_audio_state(bta_ag_scb_to_idx(p_scb), p_scb->app_id, SCO_STATE_SETUP); #endif #if (BTM_SCO_HCI_INCLUDED == TRUE ) @@ -1456,10 +1456,10 @@ void bta_ag_sco_conn_open(tBTA_AG_SCB *p_scb, tBTA_AG_DATA *p_data) bta_sys_sco_open(BTA_ID_AG, p_scb->app_id, p_scb->peer_addr); #if (BTM_WBS_INCLUDED == TRUE) - bta_ag_co_audio_state(bta_ag_scb_to_idx(p_scb), p_scb->app_id, BTA_AG_CO_AUD_STATE_ON, + bta_ag_co_audio_state(bta_ag_scb_to_idx(p_scb), p_scb->app_id, SCO_STATE_ON, p_scb->inuse_codec); #else - bta_ag_co_audio_state(bta_ag_scb_to_idx(p_scb), p_scb->app_id, BTA_AG_CO_AUD_STATE_ON); + bta_ag_co_audio_state(bta_ag_scb_to_idx(p_scb), p_scb->app_id, SCO_STATE_ON); #endif #if (BTM_SCO_HCI_INCLUDED == TRUE ) @@ -1519,14 +1519,13 @@ void bta_ag_sco_conn_close(tBTA_AG_SCB *p_scb, tBTA_AG_DATA *p_data) #endif else { + sco_state_t sco_state = bta_ag_cb.sco.p_xfer_scb ? SCO_STATE_OFF_TRANSFER : SCO_STATE_OFF; #if (BTM_WBS_INCLUDED == TRUE) /* Indicate if the closing of audio is because of transfer */ - bta_ag_co_audio_state(handle, p_scb->app_id,(bta_ag_cb.sco.p_xfer_scb)?\ - BTA_AG_CO_AUD_STATE_OFF_XFER:BTA_AG_CO_AUD_STATE_OFF,p_scb->inuse_codec); + bta_ag_co_audio_state(handle, p_scb->app_id, sco_state, p_scb->inuse_codec); #else /* Indicate if the closing of audio is because of transfer */ - bta_ag_co_audio_state(handle, p_scb->app_id,(bta_ag_cb.sco.p_xfer_scb)?\ - BTA_AG_CO_AUD_STATE_OFF_XFER:BTA_AG_CO_AUD_STATE_OFF); + bta_ag_co_audio_state(handle, p_scb->app_id, sco_state); #endif bta_ag_sco_event(p_scb, BTA_AG_SCO_CONN_CLOSE_E); @@ -1606,11 +1605,11 @@ void bta_ag_sco_conn_rsp(tBTA_AG_SCB *p_scb, tBTM_ESCO_CONN_REQ_EVT_DATA *p_data #if (BTM_WBS_INCLUDED == FALSE ) /* Allow any platform specific pre-SCO set up to take place */ - bta_ag_co_audio_state(bta_ag_scb_to_idx(p_scb), p_scb->app_id, BTA_AG_CO_AUD_STATE_SETUP); + bta_ag_co_audio_state(bta_ag_scb_to_idx(p_scb), p_scb->app_id, SCO_STATE_SETUP); #else /* When HS initiated SCO, it cannot be WBS. */ /* Allow any platform specific pre-SCO set up to take place */ - bta_ag_co_audio_state(bta_ag_scb_to_idx(p_scb), p_scb->app_id, BTA_AG_CO_AUD_STATE_SETUP, + bta_ag_co_audio_state(bta_ag_scb_to_idx(p_scb), p_scb->app_id, SCO_STATE_SETUP, BTA_AG_CODEC_CVSD); #endif diff --git a/bta/include/bta_ag_api.h b/bta/include/bta_ag_api.h index 5a8f575d8..24bbc8708 100644 --- a/bta/include/bta_ag_api.h +++ b/bta/include/bta_ag_api.h @@ -114,6 +114,8 @@ typedef UINT8 tBTA_AG_RES; typedef UINT16 tBTA_AG_PEER_FEAT; /* HFP peer supported codec masks */ +// TODO(google) This should use common definitions +// in hci/include/hci_audio.h #define BTA_AG_CODEC_NONE BTM_SCO_CODEC_NONE #define BTA_AG_CODEC_CVSD BTM_SCO_CODEC_CVSD /* CVSD */ #define BTA_AG_CODEC_MSBC BTM_SCO_CODEC_MSBC /* mSBC */ diff --git a/bta/include/bta_ag_co.h b/bta/include/bta_ag_co.h index f938f2a8b..2f52581f5 100644 --- a/bta/include/bta_ag_co.h +++ b/bta/include/bta_ag_co.h @@ -25,12 +25,7 @@ #define BTA_AG_CO_H #include "bta_ag_api.h" - -/* Definitions for audio state callout function "state" parameter */ -#define BTA_AG_CO_AUD_STATE_OFF 0 -#define BTA_AG_CO_AUD_STATE_OFF_XFER 1 /* Closed pending transfer of audio */ -#define BTA_AG_CO_AUD_STATE_ON 2 -#define BTA_AG_CO_AUD_STATE_SETUP 3 +#include "hci/include/hci_audio.h" /******************************************************************************* ** @@ -56,12 +51,8 @@ extern void bta_ag_co_init(void); ** ** Parameters handle - handle of the AG instance ** state - Audio state -** BTA_AG_CO_AUD_STATE_OFF - Audio has been turned off -** BTA_AG_CO_AUD_STATE_OFF_XFER - Audio is closed pending transfer -** BTA_AG_CO_AUD_STATE_ON - Audio has been turned on -** BTA_AG_CO_AUD_STATE_SETUP - Audio is about to be turned on ** codec - if WBS support is compiled in, codec to going to be used is provided -** and when in BTA_AG_CO_AUD_STATE_SETUP, BTM_I2SPCMConfig() must be called with +** and when in SCO_STATE_SETUP, BTM_I2SPCMConfig() must be called with ** the correct platform parameters. ** in the other states codec type should not be ignored ** diff --git a/btif/co/bta_ag_co.c b/btif/co/bta_ag_co.c index 502813fe9..68c1567d5 100755 --- a/btif/co/bta_ag_co.c +++ b/btif/co/bta_ag_co.c @@ -18,28 +18,10 @@ #define LOG_TAG "bt_btif_bta_ag" -#include "gki.h" -#include "bta_api.h" -#include "bta_sys.h" -#include "bta_ag_api.h" -#include "bta_ag_co.h" -#include "bt_utils.h" - -#ifndef LINUX_NATIVE #include -#else -#include -#define LOGI(format, ...) fprintf (stdout, LOG_TAG format"\n", ## __VA_ARGS__) -#define LOGD(format, ...) fprintf (stdout, LOG_TAG format"\n", ## __VA_ARGS__) -#define LOGV(format, ...) fprintf (stdout, LOG_TAG format"\n", ## __VA_ARGS__) -#define LOGE(format, ...) fprintf (stderr, LOG_TAG format"\n", ## __VA_ARGS__) -#endif - -/************************************************************************************ -** Externs -************************************************************************************/ -extern int set_audio_state(UINT16 handle, UINT16 codec, UINT8 state, void *param); +#include "hci/include/hci_audio.h" +#include "osi/include/osi.h" /******************************************************************************* ** @@ -69,12 +51,8 @@ void bta_ag_co_init(void) ** ** Parameters handle - handle of the AG instance ** state - Audio state -** BTA_AG_CO_AUD_STATE_OFF - Audio has been turned off -** BTA_AG_CO_AUD_STATE_OFF_XFER - Audio has been turned off (xfer) -** BTA_AG_CO_AUD_STATE_ON - Audio has been turned on -** BTA_AG_CO_AUD_STATE_SETUP - Audio is about to be turned on ** codec - if WBS support is compiled in, codec to going to be used is provided -** and when in BTA_AG_CO_AUD_STATE_SETUP, BTM_I2SPCMConfig() must be called with +** and when in SCO_STATE_SETUP, BTM_I2SPCMConfig() must be called with ** the correct platform parameters. ** in the other states codec type should not be ignored ** @@ -90,24 +68,24 @@ void bta_ag_co_audio_state(UINT16 handle, UINT8 app_id, UINT8 state) BTIF_TRACE_DEBUG("bta_ag_co_audio_state: handle %d, state %d", handle, state); switch (state) { - case BTA_AG_CO_AUD_STATE_OFF: + case SCO_STATE_OFF: #if (BTM_WBS_INCLUDED == TRUE ) BTIF_TRACE_DEBUG("bta_ag_co_audio_state(handle %d)::Closed (OFF), codec: 0x%x", handle, codec); - set_audio_state(handle, codec, state, NULL); + set_audio_state(handle, codec, state); #else BTIF_TRACE_DEBUG("bta_ag_co_audio_state(handle %d)::Closed (OFF)", handle); #endif break; - case BTA_AG_CO_AUD_STATE_OFF_XFER: + case SCO_STATE_OFF_TRANSFER: BTIF_TRACE_DEBUG("bta_ag_co_audio_state(handle %d)::Closed (XFERRING)", handle); break; - case BTA_AG_CO_AUD_STATE_SETUP: + case SCO_STATE_SETUP: #if (BTM_WBS_INCLUDED == TRUE ) - set_audio_state(handle, codec, state, NULL); + set_audio_state(handle, codec, state); #else - set_audio_state(handle, BTA_AG_CODEC_CVSD, state, NULL); + set_audio_state(handle, BTA_AG_CODEC_CVSD, state); #endif break; default: @@ -170,9 +148,7 @@ void bta_ag_co_data_close(UINT16 handle) ** Returns void ** *******************************************************************************/ -void bta_ag_co_tx_write(UINT16 handle, UINT8 * p_data, UINT16 len) +void bta_ag_co_tx_write(UINT16 handle, UNUSED_ATTR UINT8 * p_data, UINT16 len) { - UNUSED(p_data); BTIF_TRACE_DEBUG( "bta_ag_co_tx_write: handle: %d, len: %d", handle, len ); } - diff --git a/hci/Android.mk b/hci/Android.mk index c7714bb8d..7c69464e5 100644 --- a/hci/Android.mk +++ b/hci/Android.mk @@ -6,6 +6,7 @@ LOCAL_SRC_FILES := \ src/btsnoop.c \ src/btsnoop_net.c \ src/buffer_allocator.c \ + src/hci_audio.c \ src/hci_hal.c \ src/hci_hal_h4.c \ src/hci_hal_mct.c \ diff --git a/hci/include/bt_hci_bdroid.h b/hci/include/bt_hci_bdroid.h index 220946b60..f68bdd515 100644 --- a/hci/include/bt_hci_bdroid.h +++ b/hci/include/bt_hci_bdroid.h @@ -64,8 +64,6 @@ typedef enum { HC_EVENT_LPM_IDLE_TIMEOUT, } bthc_event_t; -#define MSG_CTRL_TO_HC_CMD 0x0100 /* evt mask used by HC_EVENT_TX_CMD */ - /* Message event mask across Host/Controller lib and stack */ #define MSG_EVT_MASK 0xFF00 /* eq. BT_EVT_MASK */ #define MSG_SUB_EVT_MASK 0x00FF /* eq. BT_SUB_EVT_MASK */ @@ -85,13 +83,6 @@ typedef enum { /* Local Bluetooth Controller ID for BR/EDR */ #define LOCAL_BR_EDR_CONTROLLER_ID 0 -/* Definitions of audio codec type - * inherited from AG callout function "codec" parameter - */ -#define SCO_CODEC_NONE 0x0000 /* BTA_AG_CODEC_NONE/BTM_SCO_CODEC_NONE */ -#define SCO_CODEC_CVSD 0x0001 /* BTA_AG_CODEC_CVSD/BTM_SCO_CODEC_CVSD */ -#define SCO_CODEC_MSBC 0x0002 /* BTA_AG_CODEC_MSBC/BTM_SCO_CODEC_MSBC */ - /****************************************************************************** ** Type definitions and return values ******************************************************************************/ diff --git a/hci/include/bt_hci_lib.h b/hci/include/bt_hci_lib.h index c9f8cd7c4..f7b67f09e 100644 --- a/hci/include/bt_hci_lib.h +++ b/hci/include/bt_hci_lib.h @@ -58,11 +58,6 @@ typedef enum { BT_HC_LOGGING_ON, } bt_hc_logging_state_t; -/* commands to be used in LSB with MSG_CTRL_TO_HC_CMD */ -typedef enum { - BT_HC_AUDIO_STATE = 0, - BT_HC_CMD_MAX -} bt_hc_tx_cmd_t; /** Result of write request */ typedef enum { BT_HC_TX_SUCCESS, /* a buffer is fully processed and can be released */ diff --git a/hci/include/hci_audio.h b/hci/include/hci_audio.h new file mode 100644 index 000000000..251bcabb6 --- /dev/null +++ b/hci/include/hci_audio.h @@ -0,0 +1,40 @@ +/****************************************************************************** + * + * Copyright (C) 2015 Google, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +#pragma once + +#include "bta/include/bta_ag_api.h" + +// Audio state definitions. +typedef enum { + SCO_STATE_OFF = 0, // Audio is off. + SCO_STATE_OFF_TRANSFER, // Closed pending final transfer of audio. + SCO_STATE_ON, // Audio is on. + SCO_STATE_SETUP, // Open pending completion of audio setup. +} sco_state_t; + +// Codec type definitions. +typedef enum { + SCO_CODEC_NONE = 0x0000, + SCO_CODEC_CVSD = 0x0001, + SCO_CODEC_MSBC = 0x0002, +} sco_codec_t; + +// Set the audio state on the controller for SCO (PCM, WBS, ...) using the +// vendor library. +void set_audio_state(uint16_t handle, sco_codec_t codec, sco_state_t state); diff --git a/hci/include/vendor.h b/hci/include/vendor.h index 6e4c070a4..3181e6221 100644 --- a/hci/include/vendor.h +++ b/hci/include/vendor.h @@ -31,7 +31,8 @@ typedef enum { VENDOR_OPEN_USERIAL = BT_VND_OP_USERIAL_OPEN, VENDOR_CLOSE_USERIAL = BT_VND_OP_USERIAL_CLOSE, VENDOR_GET_LPM_IDLE_TIMEOUT = BT_VND_OP_GET_LPM_IDLE_TIMEOUT, - VENDOR_SET_LPM_WAKE_STATE = BT_VND_OP_LPM_WAKE_SET_STATE + VENDOR_SET_LPM_WAKE_STATE = BT_VND_OP_LPM_WAKE_SET_STATE, + VENDOR_SET_AUDIO_STATE = BT_VND_OP_SET_AUDIO_STATE } vendor_opcode_t; typedef enum { diff --git a/hci/src/hci_audio.c b/hci/src/hci_audio.c new file mode 100644 index 000000000..58397a1c9 --- /dev/null +++ b/hci/src/hci_audio.c @@ -0,0 +1,39 @@ +/****************************************************************************** + * + * Copyright (C) 2015 Google, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +#define LOG_TAG "hci_audio" + +#include + +#include "hci/include/bt_vendor_lib.h" +#include "hci/include/hci_audio.h" +#include "hci/include/vendor.h" +#include "osi/include/log.h" + +void set_audio_state(uint16_t handle, sco_codec_t codec, sco_state_t state) +{ + LOG_INFO("%s handle:%d codec:0x%x state:%d", __func__, handle, codec, state); + + bt_vendor_op_audio_state_t audio_state; + + audio_state.handle = handle; + audio_state.peer_codec = codec; + audio_state.state = state; + + vendor_get_interface()->send_command(VENDOR_SET_AUDIO_STATE, &audio_state); +} diff --git a/main/bte_main.c b/main/bte_main.c index 9343bad5c..de62232df 100755 --- a/main/bte_main.c +++ b/main/bte_main.c @@ -238,63 +238,6 @@ void bte_main_lpm_wake_bt_device() } #endif // HCILP_INCLUDED - -/* NOTICE: - * Definitions for audio state structure, this type needs to match to - * the bt_vendor_op_audio_state_t type defined in bt_vendor_lib.h - */ -typedef struct { - UINT16 handle; - UINT16 peer_codec; - UINT16 state; -} bt_hc_audio_state_t; - -struct bt_audio_state_tag { - BT_HDR hdr; - bt_hc_audio_state_t audio; -}; - -/****************************************************************************** -** -** Function set_audio_state -** -** Description Sets audio state on controller state for SCO (PCM, WBS, FM) -** -** Parameters handle: codec related handle for SCO: sco cb idx, unused for -** codec: BTA_AG_CODEC_MSBC, BTA_AG_CODEC_CSVD or FM codec -** state: codec state, eg. BTA_AG_CO_AUD_STATE_SETUP -** param: future extensions, e.g. call-in structure/event. -** -** Returns None -** -******************************************************************************/ -int set_audio_state(UINT16 handle, UINT16 codec, UINT8 state, void *param) -{ - struct bt_audio_state_tag *p_msg; - int result = -1; - - APPL_TRACE_API("set_audio_state(handle: %d, codec: 0x%x, state: %d)", handle, - codec, state); - if (NULL != param) - APPL_TRACE_WARNING("set_audio_state() non-null param not supported"); - p_msg = (struct bt_audio_state_tag *)GKI_getbuf(sizeof(*p_msg)); - if (!p_msg) - return result; - p_msg->audio.handle = handle; - p_msg->audio.peer_codec = codec; - p_msg->audio.state = state; - - p_msg->hdr.event = MSG_CTRL_TO_HC_CMD | (MSG_SUB_EVT_MASK & BT_HC_AUDIO_STATE); - p_msg->hdr.len = sizeof(p_msg->audio); - p_msg->hdr.offset = 0; - /* layer_specific shall contain return path event! for BTA events! - * 0 means no return message is expected. */ - p_msg->hdr.layer_specific = 0; - hci->transmit_downward(MSG_STACK_TO_HC_HCI_CMD, p_msg); - return result; -} - - /****************************************************************************** ** ** Function bte_main_hci_send diff --git a/stack/include/btm_api.h b/stack/include/btm_api.h index 28ea32aff..731ac3462 100644 --- a/stack/include/btm_api.h +++ b/stack/include/btm_api.h @@ -941,6 +941,8 @@ typedef UINT8 tBTM_SCO_ROUTE_TYPE; /******************* ** SCO Codec Types ********************/ +// TODO(google) This should use common definitions +// in hci/include/hci_audio.h #define BTM_SCO_CODEC_NONE 0x0000 #define BTM_SCO_CODEC_CVSD 0x0001 #define BTM_SCO_CODEC_MSBC 0x0002 -- 2.11.0