qcacld-3.0: Refine the link layer stats callback API

It is fine for an API to use a void pointer for a data structure that
is opaque or a binary blob, but it is not ok to do so when the type of
the data must be known and agreed upon by both the caller and the
callee. In the case of the link layer stats callback the API
definition uses a void pointer for both the context parameter and the
response parameter, but an HDD handle is always passed as the context
and a tSirLLStatsResults struct is always passed as the response, so
explicitly reference those types. This will allow the compiler to
verify that the correct types of parameters are being passed.

Change-Id: Iba181bbc97331f8fdde1cdf5c81a80efef014839
CRs-Fixed: 2276494

core/hdd/src/wlan_hdd_stats.c

@@ -1060,24 +1060,13 @@ static void hdd_ll_process_peer_stats(struct hdd_adapter *adapter,
 						  (tpSirWifiPeerStat) data);
 }
 
-/**
- * wlan_hdd_cfg80211_link_layer_stats_callback() - This function is called
- * @ctx: Pointer to hdd context
- * @indType: Indication type
- * @pRsp: Pointer to response
- *
- * After receiving Link Layer indications from FW.This callback converts the
- * firmware data to the NL data and send the same to the kernel/upper layers.
- *
- * Return: None
- */
-void wlan_hdd_cfg80211_link_layer_stats_callback(void *ctx,
-							int indType, void *pRsp)
+void wlan_hdd_cfg80211_link_layer_stats_callback(hdd_handle_t hdd_handle,
+						 int indication_type,
+						 tSirLLStatsResults *results)
 {
-	struct hdd_context *hdd_ctx = (struct hdd_context *) ctx;
+	struct hdd_context *hdd_ctx = hdd_handle_to_context(hdd_handle);
 	struct hdd_ll_stats_context *context;
 	struct hdd_adapter *adapter = NULL;
-	tpSirLLStatsResults linkLayerStatsResults = (tpSirLLStatsResults) pRsp;
 	int status;
 
 	status = wlan_hdd_validate_context(hdd_ctx);
@@ -1085,58 +1074,58 @@ void wlan_hdd_cfg80211_link_layer_stats_callback(void *ctx,
 		return;
 
 	adapter = hdd_get_adapter_by_vdev(hdd_ctx,
-					   linkLayerStatsResults->ifaceId);
+					   results->ifaceId);
 
 	if (NULL == adapter) {
 		hdd_err("vdev_id %d does not exist with host",
-			linkLayerStatsResults->ifaceId);
+			results->ifaceId);
 		return;
 	}
 
-	hdd_debug("Link Layer Indication indType: %d", indType);
+	hdd_debug("Link Layer Indication Type: %d", indication_type);
 
-	switch (indType) {
+	switch (indication_type) {
 	case SIR_HAL_LL_STATS_RESULTS_RSP:
 	{
 		hdd_debug("LL_STATS RESP paramID = 0x%x, ifaceId = %u, respId= %u , moreResultToFollow = %u, num radio = %u result = %pK",
-			linkLayerStatsResults->paramId,
-			linkLayerStatsResults->ifaceId,
-			linkLayerStatsResults->rspId,
-			linkLayerStatsResults->moreResultToFollow,
-			linkLayerStatsResults->num_radio,
-			linkLayerStatsResults->results);
+			results->paramId,
+			results->ifaceId,
+			results->rspId,
+			results->moreResultToFollow,
+			results->num_radio,
+			results->results);
 
 		context = &ll_stats_context;
 		spin_lock(&context->context_lock);
 		/* validate response received from target */
-		if ((context->request_id != linkLayerStatsResults->rspId) ||
-		  !(context->request_bitmap & linkLayerStatsResults->paramId)) {
+		if ((context->request_id != results->rspId) ||
+		    !(context->request_bitmap & results->paramId)) {
 			spin_unlock(&context->context_lock);
 			hdd_err("Error : Request id %d response id %d request bitmap 0x%x response bitmap 0x%x",
-			context->request_id, linkLayerStatsResults->rspId,
-			context->request_bitmap, linkLayerStatsResults->paramId);
+			context->request_id, results->rspId,
+			context->request_bitmap, results->paramId);
 			return;
 		}
 		spin_unlock(&context->context_lock);
 
-		if (linkLayerStatsResults->paramId & WMI_LINK_STATS_RADIO) {
+		if (results->paramId & WMI_LINK_STATS_RADIO) {
 			hdd_ll_process_radio_stats(adapter,
-				linkLayerStatsResults->moreResultToFollow,
-				linkLayerStatsResults->results,
-				linkLayerStatsResults->num_radio,
-				linkLayerStatsResults->rspId);
+				results->moreResultToFollow,
+				results->results,
+				results->num_radio,
+				results->rspId);
 
 			spin_lock(&context->context_lock);
-			if (!linkLayerStatsResults->moreResultToFollow)
+			if (!results->moreResultToFollow)
 				context->request_bitmap &= ~(WMI_LINK_STATS_RADIO);
 			spin_unlock(&context->context_lock);
 
-		} else if (linkLayerStatsResults->paramId &
+		} else if (results->paramId &
 				WMI_LINK_STATS_IFACE) {
 			hdd_ll_process_iface_stats(adapter,
-				linkLayerStatsResults->results,
-				linkLayerStatsResults->num_peers,
-				linkLayerStatsResults->rspId);
+				results->results,
+				results->num_peers,
+				results->rspId);
 
 			spin_lock(&context->context_lock);
 			/* Firmware doesn't send peerstats event if no peers are
@@ -1144,21 +1133,21 @@ void wlan_hdd_cfg80211_link_layer_stats_callback(void *ctx,
 			 * this case and return the status to middleware after
 			 * receiving iface stats
 			 */
-			if (!linkLayerStatsResults->num_peers)
+			if (!results->num_peers)
 				context->request_bitmap &=
 					~(WMI_LINK_STATS_ALL_PEER);
 			context->request_bitmap &= ~(WMI_LINK_STATS_IFACE);
 			spin_unlock(&context->context_lock);
 
-		} else if (linkLayerStatsResults->
+		} else if (results->
 			   paramId & WMI_LINK_STATS_ALL_PEER) {
 			hdd_ll_process_peer_stats(adapter,
-				linkLayerStatsResults->moreResultToFollow,
-				linkLayerStatsResults->results,
-				linkLayerStatsResults->rspId);
+				results->moreResultToFollow,
+				results->results,
+				results->rspId);
 
 			spin_lock(&context->context_lock);
-			if (!linkLayerStatsResults->moreResultToFollow)
+			if (!results->moreResultToFollow)
 				context->request_bitmap &= ~(WMI_LINK_STATS_ALL_PEER);
 			spin_unlock(&context->context_lock);
 
@@ -1175,7 +1164,7 @@ void wlan_hdd_cfg80211_link_layer_stats_callback(void *ctx,
 		break;
 	}
 	default:
-		hdd_warn("invalid event type %d", indType);
+		hdd_warn("invalid event type %d", indication_type);
 		break;
 	}
 }

core/hdd/src/wlan_hdd_stats.h

@@ -188,8 +188,21 @@ bool hdd_get_interface_info(struct hdd_adapter *adapter,
 int wlan_hdd_ll_stats_get(struct hdd_adapter *adapter, uint32_t req_id,
 			  uint32_t req_mask);
 
-void wlan_hdd_cfg80211_link_layer_stats_callback(void *ctx,
-						 int indType, void *pRsp);
+/**
+ * wlan_hdd_cfg80211_link_layer_stats_callback() - This function is called
+ * @hdd_handle: Handle to HDD context
+ * @indication_type: Indication type
+ * @results: Pointer to results
+ *
+ * After receiving Link Layer indications from FW.This callback converts the
+ * firmware data to the NL data and send the same to the kernel/upper layers.
+ *
+ * Return: None
+ */
+void wlan_hdd_cfg80211_link_layer_stats_callback(hdd_handle_t hdd_handle,
+						 int indication_type,
+						 tSirLLStatsResults *results);
+
 /**
  * wlan_hdd_cfg80211_link_layer_stats_ext_callback() - Callback for LL ext
  * @ctx: HDD context
@@ -248,8 +261,9 @@ wlan_hdd_clear_link_layer_stats(struct hdd_adapter *adapter)
 }
 
 static inline void
-wlan_hdd_cfg80211_link_layer_stats_callback(void *ctx,
-					    int indType, void *pRsp)
+wlan_hdd_cfg80211_link_layer_stats_callback(hdd_handle_t hdd_handle,
+					    int indication_type,
+					    tSirLLStatsResults *results)
 {
 }
 

core/sme/inc/sme_api.h

@@ -976,9 +976,19 @@ QDF_STATUS sme_ll_stats_set_req(tHalHandle hHal,
 		tSirLLStatsSetReq *psetStatsReq);
 QDF_STATUS sme_ll_stats_get_req(tHalHandle hHal,
 		tSirLLStatsGetReq *pgetStatsReq);
-QDF_STATUS sme_set_link_layer_stats_ind_cb(tHalHandle hHal,
-		void (*callbackRoutine)(void *callbackCtx,
-				int indType, void *pRsp));
+
+/**
+ * sme_set_link_layer_stats_ind_cb() -
+ * SME API to trigger the stats are available after get request
+ * @mac_handle: MAC handle
+ * @callback_routine - HDD callback which needs to be invoked after
+ *    getting status notification from FW
+ *
+ * Return: QDF_STATUS
+ */
+QDF_STATUS sme_set_link_layer_stats_ind_cb(mac_handle_t mac_handle,
+					   link_layer_stats_cb callback);
+
 QDF_STATUS sme_set_link_layer_ext_cb(tHalHandle hal,
 		     void (*ll_stats_ext_cb)(hdd_handle_t callback_ctx,
 					     tSirLLStatsResults * rsp));
@@ -995,8 +1005,8 @@ sme_set_link_layer_ext_cb(tHalHandle hal, void (*ll_stats_ext_cb)
 }
 
 static inline QDF_STATUS
-sme_set_link_layer_stats_ind_cb(tHalHandle hHal, void (*callback_routine)
-				(void *callbackCtx, int indType, void *pRsp))
+sme_set_link_layer_stats_ind_cb(mac_handle_t mac_handle,
+				link_layer_stats_cb callback)
 {
 	return QDF_STATUS_SUCCESS;
 }

core/sme/inc/sme_internal.h

@@ -116,6 +116,10 @@ typedef struct sSelfRecoveryStats {
 	uint8_t cmdStatsIndx;
 } tSelfRecoveryStats;
 
+typedef void (*link_layer_stats_cb)(hdd_handle_t hdd_handle,
+				    int indication_type,
+				    tSirLLStatsResults *results);
+
 typedef void (*ocb_callback)(void *context, void *response);
 typedef void (*sme_set_thermal_level_callback)(void *context, u_int8_t level);
 typedef void (*p2p_lo_callback)(void *context, void *event);
@@ -197,8 +201,7 @@ typedef struct tagSmeStruct {
 #ifdef FEATURE_WLAN_DIAG_SUPPORT_CSR
 	host_event_wlan_status_payload_type eventPayload;
 #endif
-	void (*pLinkLayerStatsIndCallback)(void *callbackContext,
-			int indType, void *pRsp);
+	link_layer_stats_cb link_layer_stats_cb;
 	void (*link_layer_stats_ext_cb)(hdd_handle_t callback_ctx,
 					tSirLLStatsResults *rsp);
 #ifdef WLAN_POWER_DEBUGFS

core/sme/src/common/sme_api.c

@@ -11809,28 +11809,19 @@ QDF_STATUS sme_ll_stats_get_req(tHalHandle hHal, tSirLLStatsGetReq
 	return status;
 }
 
-/*
- * sme_set_link_layer_stats_ind_cb() -
- * SME API to trigger the stats are available  after get request
- *
- * hHal
- * callback_routine - HDD callback which needs to be invoked after
-	   getting status notification from FW
- * Return QDF_STATUS
- */
-QDF_STATUS sme_set_link_layer_stats_ind_cb(tHalHandle hHal,
-	void (*callback_routine)(void *callbackCtx, int indType, void *pRsp))
+QDF_STATUS sme_set_link_layer_stats_ind_cb(mac_handle_t mac_handle,
+					   link_layer_stats_cb callback)
 {
-	QDF_STATUS status = QDF_STATUS_SUCCESS;
-	tpAniSirGlobal pMac = PMAC_STRUCT(hHal);
+	QDF_STATUS status;
+	tpAniSirGlobal mac = MAC_CONTEXT(mac_handle);
 
-	status = sme_acquire_global_lock(&pMac->sme);
+	status = sme_acquire_global_lock(&mac->sme);
 	if (QDF_IS_STATUS_SUCCESS(status)) {
-		pMac->sme.pLinkLayerStatsIndCallback = callback_routine;
-		sme_release_global_lock(&pMac->sme);
-	} else
-		QDF_TRACE(QDF_MODULE_ID_SME, QDF_TRACE_LEVEL_ERROR,
-			"%s: sme_acquire_global_lock error", __func__);
+		mac->sme.link_layer_stats_cb = callback;
+		sme_release_global_lock(&mac->sme);
+	} else {
+		sme_err("sme_acquire_global_lock error");
+	}
 
 	return status;
 }
@@ -11884,7 +11875,7 @@ QDF_STATUS sme_reset_link_layer_stats_ind_cb(tHalHandle h_hal)
 
 	status = sme_acquire_global_lock(&pmac->sme);
 	if (QDF_IS_STATUS_SUCCESS(status)) {
-		pmac->sme.pLinkLayerStatsIndCallback = NULL;
+		pmac->sme.link_layer_stats_cb = NULL;
 		sme_release_global_lock(&pmac->sme);
 	} else
 		QDF_TRACE(QDF_MODULE_ID_SME, QDF_TRACE_LEVEL_ERROR,

core/wma/src/wma_utils.c

@@ -1253,7 +1253,7 @@ static int wma_unified_link_peer_stats_event_handler(void *handle,
 		return -EINVAL;
 	}
 
-	if (!pMac->sme.pLinkLayerStatsIndCallback) {
+	if (!pMac->sme.link_layer_stats_cb) {
 		WMA_LOGD("%s: HDD callback is null", __func__);
 		return -EINVAL;
 	}
@@ -1375,7 +1375,7 @@ static int wma_unified_link_peer_stats_event_handler(void *handle,
 	 * vdev_id/ifacId in link_stats_results will be
 	 * used to retrieve the correct HDD context
 	 */
-	pMac->sme.pLinkLayerStatsIndCallback(pMac->hdd_handle,
+	pMac->sme.link_layer_stats_cb(pMac->hdd_handle,
 					     WMA_LINK_LAYER_STATS_RESULTS_RSP,
 					     link_stats_results);
 	qdf_mem_free(link_stats_results);
@@ -1449,7 +1449,7 @@ static int wma_unified_radio_tx_power_level_stats_event_handler(void *handle,
 		return -EINVAL;
 	}
 
-	if (!mac->sme.pLinkLayerStatsIndCallback) {
+	if (!mac->sme.link_layer_stats_cb) {
 		WMA_LOGD("%s: HDD callback is null", __func__);
 		return -EINVAL;
 	}
@@ -1571,7 +1571,7 @@ post_stats:
 	 * vdev_id/ifacId in link_stats_results will be
 	 * used to retrieve the correct HDD context
 	 */
-	mac->sme.pLinkLayerStatsIndCallback(mac->hdd_handle,
+	mac->sme.link_layer_stats_cb(mac->hdd_handle,
 		WMA_LINK_LAYER_STATS_RESULTS_RSP,
 		link_stats_results);
 	wma_unified_radio_tx_mem_free(handle);
@@ -1611,7 +1611,7 @@ static int wma_unified_link_radio_stats_event_handler(void *handle,
 		return -EINVAL;
 	}
 
-	if (!pMac->sme.pLinkLayerStatsIndCallback) {
+	if (!pMac->sme.link_layer_stats_cb) {
 		WMA_LOGD("%s: HDD callback is null", __func__);
 		return -EINVAL;
 	}
@@ -1779,7 +1779,7 @@ static int wma_unified_link_radio_stats_event_handler(void *handle,
 		return 0;
 	}
 
-	pMac->sme.pLinkLayerStatsIndCallback(pMac->hdd_handle,
+	pMac->sme.link_layer_stats_cb(pMac->hdd_handle,
 					     WMA_LINK_LAYER_STATS_RESULTS_RSP,
 					     link_stats_results);
 	wma_unified_radio_tx_mem_free(handle);
@@ -2073,7 +2073,7 @@ int wma_unified_link_iface_stats_event_handler(void *handle,
 		return -EINVAL;
 	}
 
-	if (!pMac->sme.pLinkLayerStatsIndCallback) {
+	if (!pMac->sme.link_layer_stats_cb) {
 		WMA_LOGD("%s: HDD callback is null", __func__);
 		return -EINVAL;
 	}
@@ -2204,7 +2204,7 @@ int wma_unified_link_iface_stats_event_handler(void *handle,
 	 * vdev_id/ifacId in link_stats_results will be
 	 * used to retrieve the correct HDD context
 	 */
-	pMac->sme.pLinkLayerStatsIndCallback(pMac->hdd_handle,
+	pMac->sme.link_layer_stats_cb(pMac->hdd_handle,
 					     WMA_LINK_LAYER_STATS_RESULTS_RSP,
 					     link_stats_results);
 	qdf_mem_free(link_stats_results);