nxp uwb hal: Do not check the status code when PBF=1
HAL shouldn't check the status code for a non-header packet when the
packets are fragmented with PBF=1.
```
NxpUciR : len = 259 > 510400FF00...
NxpUciR : len = 73 > 4104004545...
...
NxpUwbHal: command failed! status = 0x45
NxpUwbHal: Response Status = 0xff
```
This patch simply skips checking status code when it's not from
phNxpUciHal_send_ext_cmd() path handling internal command packets,
assuming PBF is always zero when it's phNxpUciHal_send_ext_cmd() path.
Also remove unnecessary debug function
phNxpUciHal_print_response_status(), users can check the status code from
NxpUciR or upper layer's logs.
Bug: 307208367
Test: `cmd uwb start-fira-ranging-session`
Change-Id: Id5bacd97c5c3487e286c6db5b3b72f61d12d9e07
diff --git a/extns/inc/uci_defs.h b/extns/inc/uci_defs.h
index 4b2316c..a387bab 100644
--- a/extns/inc/uci_defs.h
+++ b/extns/inc/uci_defs.h
@@ -177,6 +177,7 @@
#define UCI_STATUS_INVALID_PARAM 0x04
#define UCI_STATUS_INVALID_MSG_SIZE 0x06
#define UCI_STATUS_COMMAND_RETRY 0x0A
+#define UCI_STATUS_UNKNOWN 0x0B
#define UCI_STATUS_THERMAL_RUNAWAY 0x54
#define UCI_STATUS_HW_RESET 0xFE
diff --git a/halimpl/hal/phNxpUciHal.cc b/halimpl/hal/phNxpUciHal.cc
index 438d893..b8f6390 100644
--- a/halimpl/hal/phNxpUciHal.cc
+++ b/halimpl/hal/phNxpUciHal.cc
@@ -71,8 +71,6 @@
phNxpUciHal_Control_t* p_nxpucihal_ctrl);
static void* phNxpUciHal_client_thread(void* arg);
extern int phNxpUciHal_fw_download();
-static void phNxpUciHal_print_response_status(uint8_t *p_rx_data,
- uint16_t p_len);
static void phNxpUciHal_getVersionInfo();
/******************************************************************************
@@ -886,9 +884,11 @@
tHAL_UWB_STATUS status;
uint8_t gid = 0, oid = 0, pbf = 0, mt = 0;
UNUSED(pContext);
+
if (nxpucihal_ctrl.read_retry_cnt == 1) {
nxpucihal_ctrl.read_retry_cnt = 0;
}
+
if (pInfo->wStatus == UWBSTATUS_SUCCESS) {
NXPLOG_UCIHAL_D("read successful status = 0x%x", pInfo->wStatus);
nxpucihal_ctrl.p_rx_data = pInfo->pBuff;
@@ -903,12 +903,18 @@
phNxpUciHal_parse_get_capsInfo(nxpucihal_ctrl.rx_data_len,
nxpucihal_ctrl.p_rx_data);
}
+
nxpucihal_ctrl.isSkipPacket = 0;
+
phNxpUciHal_parse(nxpucihal_ctrl.rx_data_len, nxpucihal_ctrl.p_rx_data);
phNxpUciHal_process_response();
if(!uwb_device_initialized) {
+ if (pbf) {
+ /* XXX: fix the whole logic if this really happens */
+ NXPLOG_UCIHAL_E("FIXME: Fragmented packets received during device-init!");
+ }
if((gid == UCI_GID_CORE) && (oid == UCI_MSG_CORE_DEVICE_STATUS_NTF)) {
nxpucihal_ctrl.uwbc_device_state = nxpucihal_ctrl.p_rx_data[UCI_RESPONSE_STATUS_OFFSET];
if(nxpucihal_ctrl.uwbc_device_state == UWB_DEVICE_INIT || nxpucihal_ctrl.uwbc_device_state == UWB_DEVICE_READY) {
@@ -947,86 +953,95 @@
}
}
- if (nxpucihal_ctrl.isSkipPacket == 0) {
- if((nxpucihal_ctrl.p_rx_data[0x00] & 0xF0) == 0x40){
- if (nxpucihal_ctrl.hal_ext_enabled) {
- nxpucihal_ctrl.isSkipPacket = 1;
- }
- if(nxpucihal_ctrl.p_rx_data[UCI_RESPONSE_STATUS_OFFSET] == UCI_STATUS_OK){
- nxpucihal_ctrl.ext_cb_data.status = UWBSTATUS_SUCCESS;
- } else if ((gid == UCI_GID_CORE) && (oid == UCI_MSG_CORE_SET_CONFIG)){
- NXPLOG_UCIHAL_E(" status = 0x%x",nxpucihal_ctrl.p_rx_data[UCI_RESPONSE_STATUS_OFFSET]);
- /* check if any configurations are not supported then ignore the
- * UWBSTATUS_FEATURE_NOT_SUPPORTED status code*/
- nxpucihal_ctrl.ext_cb_data.status = phNxpUciHal_process_ext_rsp(nxpucihal_ctrl.rx_data_len, nxpucihal_ctrl.p_rx_data);
- } else {
- if (gid == UCI_GID_SESSION_MANAGE &&
- oid == UCI_MSG_SESSION_QUERY_DATA_SIZE) {
- nxpucihal_ctrl.ext_cb_data.status =
- nxpucihal_ctrl
- .p_rx_data[UCI_MSG_SESSION_QUERY_DATA_SIZE_STATUS_OFFSET];
- } else {
- nxpucihal_ctrl.ext_cb_data.status = UWBSTATUS_FAILED;
- NXPLOG_UCIHAL_E(
- "command failed! status = 0x%x",
- nxpucihal_ctrl.p_rx_data[UCI_RESPONSE_STATUS_OFFSET]);
- }
- }
- usleep(1);
- SEM_POST(&(nxpucihal_ctrl.ext_cb_data));
- } else if (gid == UCI_GID_CORE && oid == UCI_MSG_CORE_GENERIC_ERROR_NTF &&
- nxpucihal_ctrl.p_rx_data[4] == UCI_STATUS_COMMAND_RETRY) {
- if (nxpucihal_ctrl.hal_ext_enabled) {
- nxpucihal_ctrl.ext_cb_data.status = UWBSTATUS_COMMAND_RETRANSMIT;
- }
- SEM_POST(&(nxpucihal_ctrl.ext_cb_data));
- } else if (gid == UCI_GID_CORE && oid == UCI_MSG_CORE_GENERIC_ERROR_NTF &&
- nxpucihal_ctrl.p_rx_data[4] == UCI_STATUS_INVALID_MSG_SIZE) {
- if (nxpucihal_ctrl.hal_ext_enabled) {
- nxpucihal_ctrl.ext_cb_data.status = UWBSTATUS_INVALID_COMMAND_LENGTH;
- nxpucihal_ctrl.isSkipPacket = 1;
- }
- SEM_POST(&(nxpucihal_ctrl.ext_cb_data));
+ // phNxpUciHal_process_ext_cmd_rsp() is waiting for the response packet
+ // set this true to wake it up for other reasons
+ bool bWakeupExtCmd = (mt == UCI_MT_RSP);
+
+ /* DBG packets not yet supported, just ignore them silently */
+ if (!nxpucihal_ctrl.isSkipPacket) {
+ if ((mt == UCI_MT_NTF) && (gid == UCI_GID_INTERNAL) &&
+ (oid == UCI_EXT_PARAM_DBG_RFRAME_LOG_NTF)) {
+ nxpucihal_ctrl.isSkipPacket = 1;
}
- } else if (nxpucihal_ctrl.hal_ext_enabled == 0) {
- SEM_POST(&(nxpucihal_ctrl.ext_cb_data));
- }
- if ((mt == UCI_MT_NTF) && (gid == UCI_GID_INTERNAL) &&
- (oid == UCI_EXT_PARAM_DBG_RFRAME_LOG_NTF)) {
- nxpucihal_ctrl.isSkipPacket = 1;
}
- /* if Debug Notification, then skip sending to application */
- if(nxpucihal_ctrl.isSkipPacket == 0) {
- phNxpUciHal_print_response_status(nxpucihal_ctrl.p_rx_data, nxpucihal_ctrl.rx_data_len);
- /* Read successful, send the event to higher layer */
- if ((nxpucihal_ctrl.p_uwb_stack_data_cback != NULL) && (nxpucihal_ctrl.rx_data_len <= UCI_MAX_PAYLOAD_LEN)) {
- (*nxpucihal_ctrl.p_uwb_stack_data_cback)(nxpucihal_ctrl.rx_data_len,
- nxpucihal_ctrl.p_rx_data);
- }
+ // Handle retransmissions
+ if (!nxpucihal_ctrl.isSkipPacket) {
+ if (!pbf && mt == UCI_MT_NTF && gid == UCI_GID_CORE && oid == UCI_MSG_CORE_GENERIC_ERROR_NTF) {
+ uint8_t status_code = nxpucihal_ctrl.p_rx_data[UCI_RESPONSE_STATUS_OFFSET];
+
+ if (status_code == UCI_STATUS_COMMAND_RETRY) {
+ nxpucihal_ctrl.ext_cb_data.status = UWBSTATUS_COMMAND_RETRANSMIT;
+ nxpucihal_ctrl.isSkipPacket = 1;
+ bWakeupExtCmd = true;
+ } else if (status_code == UCI_STATUS_INVALID_MSG_SIZE) {
+ nxpucihal_ctrl.ext_cb_data.status = UWBSTATUS_INVALID_COMMAND_LENGTH;
+ nxpucihal_ctrl.isSkipPacket = 1;
+ bWakeupExtCmd = true;
+ }
+ }
}
- } else {
+
+ // Check status code only for extension commands
+ if (!nxpucihal_ctrl.isSkipPacket) {
+ if (mt == UCI_MT_RSP) {
+ if (nxpucihal_ctrl.hal_ext_enabled) {
+ nxpucihal_ctrl.isSkipPacket = 1;
+
+ if (pbf) {
+ /* XXX: fix the whole logic if this really happens */
+ NXPLOG_UCIHAL_E("FIXME: Fragmented packets received while processing internal commands!");
+ }
+
+ uint8_t status_code = (nxpucihal_ctrl.rx_data_len > UCI_RESPONSE_STATUS_OFFSET) ?
+ nxpucihal_ctrl.p_rx_data[UCI_RESPONSE_STATUS_OFFSET] : UCI_STATUS_UNKNOWN;
+
+ if (status_code == UCI_STATUS_OK) {
+ nxpucihal_ctrl.ext_cb_data.status = UWBSTATUS_SUCCESS;
+ } else if ((gid == UCI_GID_CORE) && (oid == UCI_MSG_CORE_SET_CONFIG)){
+ /* check if any configurations are not supported then ignore the
+ * UWBSTATUS_FEATURE_NOT_SUPPORTED status code*/
+ nxpucihal_ctrl.ext_cb_data.status = phNxpUciHal_process_ext_rsp(nxpucihal_ctrl.rx_data_len, nxpucihal_ctrl.p_rx_data);
+ } else {
+ nxpucihal_ctrl.ext_cb_data.status = UWBSTATUS_FAILED;
+ NXPLOG_UCIHAL_E("Got error status code(0x%x) from internal command.", status_code);
+ usleep(1); // XXX: not sure if it's really needed
+ }
+ }
+ }
+ }
+
+ if (bWakeupExtCmd && nxpucihal_ctrl.ext_cb_waiting) {
+ SEM_POST(&(nxpucihal_ctrl.ext_cb_data));
+ }
+
+ if (!nxpucihal_ctrl.isSkipPacket) {
+ /* Read successful, send the event to higher layer */
+ if ((nxpucihal_ctrl.p_uwb_stack_data_cback != NULL) && (nxpucihal_ctrl.rx_data_len <= UCI_MAX_PAYLOAD_LEN)) {
+ (*nxpucihal_ctrl.p_uwb_stack_data_cback)(nxpucihal_ctrl.rx_data_len, nxpucihal_ctrl.p_rx_data);
+ }
+ }
+ } else { // pInfo->wStatus != UWBSTATUS_SUCCESS
NXPLOG_UCIHAL_E("read error status = 0x%x", pInfo->wStatus);
}
- if (nxpucihal_ctrl.halStatus == HAL_STATUS_CLOSE) {
- return;
- }
/* Disable junk data check for each UCI packet*/
if(nxpucihal_ctrl.fw_dwnld_mode) {
if((gid == UCI_GID_CORE) && (oid == UCI_MSG_CORE_DEVICE_STATUS_NTF)){
nxpucihal_ctrl.fw_dwnld_mode = false;
}
}
+
/* Read again because read must be pending always.*/
- status = phTmlUwb_Read(
- Rx_data, UCI_MAX_DATA_LEN,
- (pphTmlUwb_TransactCompletionCb_t)&phNxpUciHal_read_complete, NULL);
- if (status != UWBSTATUS_PENDING) {
- NXPLOG_UCIHAL_E("read status error status = %x", status);
- /* TODO: Not sure how to handle this ? */
+ if (nxpucihal_ctrl.halStatus != HAL_STATUS_CLOSE) {
+ status = phTmlUwb_Read(
+ Rx_data, UCI_MAX_DATA_LEN,
+ (pphTmlUwb_TransactCompletionCb_t)&phNxpUciHal_read_complete, NULL);
+ if (status != UWBSTATUS_PENDING) {
+ NXPLOG_UCIHAL_E("read status error status = %x", status);
+ /* TODO: Not sure how to handle this ? */
+ }
}
- return;
}
/******************************************************************************
@@ -1734,40 +1749,6 @@
}
/******************************************************************************
- * Function phNxpUciHal_print_response_status
- *
- * Description This function logs the response status
- *
- * Returns status
- *
- ******************************************************************************/
-static void phNxpUciHal_print_response_status(uint8_t* p_rx_data, uint16_t p_len) {
- uint8_t mt;
- int status_byte;
- const uint8_t response_buf[][30] = {"STATUS_OK",
- "STATUS_REJECTED",
- "STATUS_FAILED",
- "STATUS_SYNTAX_ERROR",
- "STATUS_INVALID_PARAM",
- "STATUS_INVALID_RANGE",
- "STATUS_INAVALID_MSG_SIZE",
- "STATUS_UNKNOWN_GID",
- "STATUS_UNKNOWN_OID",
- "STATUS_RFU",
- "STATUS_READ_ONLY",
- "STATUS_COMMAND_RETRY",
- "STATUS_UNKNOWN"};
- if(p_len > UCI_PKT_HDR_LEN) {
- mt = ((p_rx_data[0]) & UCI_MT_MASK) >> UCI_MT_SHIFT;
- status_byte = p_rx_data[UCI_RESPONSE_STATUS_OFFSET];
- if((mt == UCI_MT_RSP) && (status_byte <= MAX_RESPONSE_STATUS)) {
- NXPLOG_UCIHAL_D(" %s: Response Status = %s", __func__,
- response_buf[status_byte]);
- }
- }
-}
-
-/******************************************************************************
* Function phNxpUciHal_GetMwVersion
*
* Description This function gets the middleware version
diff --git a/halimpl/hal/phNxpUciHal.h b/halimpl/hal/phNxpUciHal.h
index 2ae77d0..10c2e1b 100644
--- a/halimpl/hal/phNxpUciHal.h
+++ b/halimpl/hal/phNxpUciHal.h
@@ -180,6 +180,10 @@
/* Waiting semaphore */
phNxpUciHal_Sem_t ext_cb_data;
+ // in case of fragmented response,
+ // ext_cb_data is flagged only from the 1st response packet
+ bool ext_cb_waiting;
+
phNxpUciHal_Sem_t dev_status_ntf_wait;
phNxpUciHal_Sem_t uwb_binding_status_ntf_wait;
phNxpUciHal_Sem_t uwb_get_binding_status_ntf_wait;
diff --git a/halimpl/hal/phNxpUciHal_ext.cc b/halimpl/hal/phNxpUciHal_ext.cc
index 54653ae..107a296 100644
--- a/halimpl/hal/phNxpUciHal_ext.cc
+++ b/halimpl/hal/phNxpUciHal_ext.cc
@@ -85,8 +85,12 @@
/* Send ext command */
do {
NXPLOG_UCIHAL_D("Entered do while loop");
+
nxpucihal_ctrl.ext_cb_data.status = UWBSTATUS_SUCCESS;
+ nxpucihal_ctrl.ext_cb_waiting = true;
+
*data_written = phNxpUciHal_write_unlocked(cmd_len, p_cmd);
+
if (*data_written != cmd_len) {
NXPLOG_UCIHAL_D("phNxpUciHal_write failed for hal ext");
goto clean_and_return;
@@ -120,6 +124,7 @@
NXPLOG_UCIHAL_E("p_hal_ext->ext_cb_data.sem semaphore error");
goto clean_and_return;
}
+ nxpucihal_ctrl.ext_cb_waiting = false;
switch (nxpucihal_ctrl.ext_cb_data.status) {
case UWBSTATUS_RESPONSE_TIMEOUT:
@@ -127,6 +132,7 @@
ext_cmd_retry_cnt++;
break;
case UWBSTATUS_INVALID_COMMAND_LENGTH:
+ // XXX: Why retrying here?
invalid_len_retry_cnt++;
break;
default:
