From 8a583437e7fe7c8345f160c624bbca30968f7779 Mon Sep 17 00:00:00 2001 From: Krunal Soni Date: Sun, 5 Feb 2017 15:54:02 -0800 Subject: [PATCH] qcacmn: Fix bugs in serialization module reported by reviewer Few issues are observed and reported in serialization module, fix them. Change-Id: I11929f086a5f7ef227b8b8fa62d0cf8c1b5f0b9c CRs-Fixed: 2000032 --- qdf/inc/qdf_types.h | 128 +++++++++--------- .../inc/wlan_serialization_api.h | 64 ++++----- .../src/wlan_serialization_api.c | 8 ++ .../src/wlan_serialization_main.c | 23 ++-- .../src/wlan_serialization_utils.c | 6 +- 5 files changed, 114 insertions(+), 115 deletions(-) diff --git a/qdf/inc/qdf_types.h b/qdf/inc/qdf_types.h index ea1b0f4f64..2b3664de84 100644 --- a/qdf/inc/qdf_types.h +++ b/qdf/inc/qdf_types.h @@ -217,78 +217,78 @@ typedef void (*qdf_timer_func_t)(void *); /** * typedef enum QDF_MODULE_ID - Debug category level - * @QDF_MODULE_ID_TDLS : TDLS - * @QDF_MODULE_ID_ACS : auto channel selection - * @QDF_MODULE_ID_SCAN_SM : scan state machine - * @QDF_MODULE_ID_SCANENTRY : scan entry - * @QDF_MODULE_ID_WDS : WDS handling + * @QDF_MODULE_ID_TDLS: TDLS + * @QDF_MODULE_ID_ACS: auto channel selection + * @QDF_MODULE_ID_SCAN_SM: scan state machine + * @QDF_MODULE_ID_SCANENTRY: scan entry + * @QDF_MODULE_ID_WDS: WDS handling * @QDF_MODULE_ID_ACTION: action management frames - * @QDF_MODULE_ID_ROAM : sta mode roaming - * @QDF_MODULE_ID_INACT : inactivity handling - * @QDF_MODULE_ID_DOTH : 11.h - * @QDF_MODULE_ID_IQUE : IQUE features - * @QDF_MODULE_ID_WME : WME protocol - * @QDF_MODULE_ID_ACL : ACL handling - * @QDF_MODULE_ID_WPA : WPA/RSN protocol - * @QDF_MODULE_ID_RADKEYS : dump 802.1x keys - * @QDF_MODULE_ID_RADDUMP : dump 802.1x radius packets + * @QDF_MODULE_ID_ROAM: sta mode roaming + * @QDF_MODULE_ID_INACT: inactivity handling + * @QDF_MODULE_ID_DOTH: 11.h + * @QDF_MODULE_ID_IQUE: IQUE features + * @QDF_MODULE_ID_WME: WME protocol + * @QDF_MODULE_ID_ACL: ACL handling + * @QDF_MODULE_ID_WPA: WPA/RSN protocol + * @QDF_MODULE_ID_RADKEYS: dump 802.1x keys + * @QDF_MODULE_ID_RADDUMP: dump 802.1x radius packets * @QDF_MODULE_ID_RADIUS: 802.1x radius client - * @QDF_MODULE_ID_DOT1XSM : 802.1x state machine - * @QDF_MODULE_ID_DOT1X : 802.1x authenticator - * @QDF_MODULE_ID_POWER : power save handling - * @QDF_MODULE_ID_STATS : state machine + * @QDF_MODULE_ID_DOT1XSM: 802.1x state machine + * @QDF_MODULE_ID_DOT1X: 802.1x authenticator + * @QDF_MODULE_ID_POWER: power save handling + * @QDF_MODULE_ID_STATS: state machine * @QDF_MODULE_ID_OUTPUT: output handling - * @QDF_MODULE_ID_SCAN : scanning - * @QDF_MODULE_ID_AUTH : authentication handling - * @QDF_MODULE_ID_ASSOC : association handling - * @QDF_MODULE_ID_NODE : node handling + * @QDF_MODULE_ID_SCAN: scanning + * @QDF_MODULE_ID_AUTH: authentication handling + * @QDF_MODULE_ID_ASSOC: association handling + * @QDF_MODULE_ID_NODE: node handling * @QDF_MODULE_ID_ELEMID: element id parsing - * @QDF_MODULE_ID_XRATE : rate set handling - * @QDF_MODULE_ID_INPUT : input handling + * @QDF_MODULE_ID_XRATE: rate set handling + * @QDF_MODULE_ID_INPUT: input handling * @QDF_MODULE_ID_CRYPTO: crypto work - * @QDF_MODULE_ID_DUMPPKTS : IFF_LINK2 equivalant - * @QDF_MODULE_ID_DEBUG : IFF_DEBUG equivalent - * @QDF_MODULE_ID_MLME : MLME - * @QDF_MODULE_ID_RRM : Radio resource measurement - * @QDF_MODULE_ID_WNM : Wireless Network Management - * @QDF_MODULE_ID_P2P_PROT : P2P Protocol driver - * @QDF_MODULE_ID_PROXYARP : 11v Proxy ARP - * @QDF_MODULE_ID_L2TIF : Hotspot 2.0 L2 TIF - * @QDF_MODULE_ID_WIFIPOS : WifiPositioning Feature - * @QDF_MODULE_ID_WRAP : WRAP or Wireless ProxySTA - * @QDF_MODULE_ID_DFS : DFS debug mesg + * @QDF_MODULE_ID_DUMPPKTS: IFF_LINK2 equivalant + * @QDF_MODULE_ID_DEBUG: IFF_DEBUG equivalent + * @QDF_MODULE_ID_MLME: MLME + * @QDF_MODULE_ID_RRM: Radio resource measurement + * @QDF_MODULE_ID_WNM: Wireless Network Management + * @QDF_MODULE_ID_P2P_PROT: P2P Protocol driver + * @QDF_MODULE_ID_PROXYARP: 11v Proxy ARP + * @QDF_MODULE_ID_L2TIF: Hotspot 2.0 L2 TIF + * @QDF_MODULE_ID_WIFIPOS: WifiPositioning Feature + * @QDF_MODULE_ID_WRAP: WRAP or Wireless ProxySTA + * @QDF_MODULE_ID_DFS: DFS debug mesg * @QDF_MODULE_ID_TLSHIM: TLSHIM module ID - * @QDF_MODULE_ID_WMI : WMI module ID - * @QDF_MODULE_ID_HTT : HTT module ID - * @QDF_MODULE_ID_HDD : HDD module ID - * @QDF_MODULE_ID_SME : SME module ID - * @QDF_MODULE_ID_PE : PE module ID - * @QDF_MODULE_ID_WMA : WMA module ID - * @QDF_MODULE_ID_SYS : SYS module ID - * @QDF_MODULE_ID_QDF : QDF module ID - * @QDF_MODULE_ID_SAP : SAP module ID - * @QDF_MODULE_ID_HDD_SOFTAP : HDD SAP module ID - * @QDF_MODULE_ID_HDD_DATA : HDD DATA module ID - * @QDF_MODULE_ID_HDD_SAP_DATA : HDD SAP DATA module ID - * @QDF_MODULE_ID_HIF : HIF module ID - * @QDF_MODULE_ID_HTC : HTC module ID - * @QDF_MODULE_ID_TXRX : TXRX module ID - * @QDF_MODULE_ID_QDF_DEVICE : QDF DEVICE module ID - * @QDF_MODULE_ID_CFG : CFG module ID - * @QDF_MODULE_ID_BMI : BMI module ID + * @QDF_MODULE_ID_WMI: WMI module ID + * @QDF_MODULE_ID_HTT: HTT module ID + * @QDF_MODULE_ID_HDD: HDD module ID + * @QDF_MODULE_ID_SME: SME module ID + * @QDF_MODULE_ID_PE: PE module ID + * @QDF_MODULE_ID_WMA: WMA module ID + * @QDF_MODULE_ID_SYS: SYS module ID + * @QDF_MODULE_ID_QDF: QDF module ID + * @QDF_MODULE_ID_SAP: SAP module ID + * @QDF_MODULE_ID_HDD_SOFTAP: HDD SAP module ID + * @QDF_MODULE_ID_HDD_DATA: HDD DATA module ID + * @QDF_MODULE_ID_HDD_SAP_DATA: HDD SAP DATA module ID + * @QDF_MODULE_ID_HIF: HIF module ID + * @QDF_MODULE_ID_HTC: HTC module ID + * @QDF_MODULE_ID_TXRX: TXRX module ID + * @QDF_MODULE_ID_QDF_DEVICE: QDF DEVICE module ID + * @QDF_MODULE_ID_CFG: CFG module ID + * @QDF_MODULE_ID_BMI: BMI module ID * @QDF_MODULE_ID_EPPING: EPPING module ID - * @QDF_MODULE_ID_QVIT : QVIT module ID - * @QDF_MODULE_ID_DP : Data-path module ID - * @QDF_MODULE_ID_SOC : SOC module ID - * @QDF_MODULE_ID_OS_IF : OS-interface module ID - * @QDF_MODULE_ID_TARGET_IF : targer interface module ID - * @QDF_MODULE_ID_SCHEDULER : schduler module ID - * @QDF_MODULE_ID_MGMT_TXRX : management TX/RX module ID - * @QDF_MODULE_ID_SERIALIZATION : serialization module ID - * @QDF_MODULE_ID_PMO : PMO (power manager and offloads) Module ID + * @QDF_MODULE_ID_QVIT: QVIT module ID + * @QDF_MODULE_ID_DP: Data-path module ID + * @QDF_MODULE_ID_SOC: SOC module ID + * @QDF_MODULE_ID_OS_IF: OS-interface module ID + * @QDF_MODULE_ID_TARGET_IF: targer interface module ID + * @QDF_MODULE_ID_SCHEDULER: schduler module ID + * @QDF_MODULE_ID_MGMT_TXRX: management TX/RX module ID + * @QDF_MODULE_ID_SERIALIZATION: serialization module ID + * @QDF_MODULE_ID_PMO: PMO (power manager and offloads) Module ID * @QDF_MODULE_ID_P2P : P2P module ID - * @QDF_MODULE_ID_ANY : anything - * @QDF_MODULE_ID_MAX : Max place holder module ID + * @QDF_MODULE_ID_ANY: anything + * @QDF_MODULE_ID_MAX: Max place holder module ID */ typedef enum { QDF_MODULE_ID_TDLS = 0, diff --git a/umac/cmn_services/serialization/inc/wlan_serialization_api.h b/umac/cmn_services/serialization/inc/wlan_serialization_api.h index c7b25b53ac..8cdb84c028 100644 --- a/umac/cmn_services/serialization/inc/wlan_serialization_api.h +++ b/umac/cmn_services/serialization/inc/wlan_serialization_api.h @@ -42,16 +42,18 @@ /** * enum wlan_serialization_cb_reason - reason for calling the callback - * WLAN_SERIALIZATION_REASON_ACTIVATE_CMD: activate the cmd by sending it to FW - * WLAN_SERIALIZATION_REASON_CANCEL_CMD: Cancel the cmd in the pending list - * WLAN_SERIALIZATION_REASON_RELEASE_MEM_CMD:cmd execution complete. Release + * @WLAN_SERIALIZATION_REASON_ACTIVATE_CMD: activate the cmd by sending it to FW + * @WLAN_SERIALIZATION_REASON_CANCEL_CMD: Cancel the cmd in the pending list + * @WLAN_SERIALIZATION_REASON_RELEASE_MEM_CMD:cmd execution complete. Release * the memory allocated while * building the command + * @WLAN_SER_CB_ACTIVE_CMD_TIMEOUT: active cmd has been timeout. */ enum wlan_serialization_cb_reason { WLAN_SER_CB_ACTIVATE_CMD, WLAN_SER_CB_CANCEL_CMD, WLAN_SER_CB_RELEASE_MEM_CMD, + WLAN_SER_CB_ACTIVE_CMD_TIMEOUT, }; /** @@ -78,21 +80,15 @@ union wlan_serialization_rules_info { * @wlan_cmd: Command passed by the component for serialization * @reason: Reason code for which the callback is being called * - * Reason specifies the reason for which the callback is being called + * Reason specifies the reason for which the callback is being called. callback + * should return success or failure based up on overall success of callback. + * if callback returns failure then serialization will remove the command from + * active queue and proceed for next pending command. * - * Return: None + * Return: QDF_STATUS_SUCCESS or QDF_STATUS_E_FAILURE */ -typedef void (*wlan_serialization_cmd_callback)( - void *wlan_cmd, enum wlan_serialization_cb_reason reason); - -/** - * wlan_serialization_active_cmd_timeout_callback() - callback for cmd timeout - * @wlan_cmd: Command that has timed out - * - * Return: None - */ -typedef void (*wlan_serialization_active_cmd_timeout_callback)( - void *wlan_cmd); +typedef QDF_STATUS (*wlan_serialization_cmd_callback) (void *wlan_cmd, + enum wlan_serialization_cb_reason reason); /** * wlan_serialization_comp_info_cb() - callback to fill the rules information @@ -176,23 +172,6 @@ enum wlan_serialization_cmd_status { WLAN_SER_CMD_NOT_FOUND, }; -/** - * union wlan_serialization_obj_context - Object associated to the command - * @pdev: PDEV object associated to the command - * @vdev: VDEV object associated to the command - * - * Object passed for the command - * This is an agreement between the component and serialization as to which - * object is being passed. A copy of this command is maintained by - * serialization as part of queuing the command. So, the object ref count has - * to be maintained while queuing and released during cancellation/dequeuing/ - * flushing the command. - */ -union wlan_serialization_obj_context { - struct wlan_objmgr_pdev *pdev; - struct wlan_objmgr_vdev *vdev; -}; - /** * struct wlan_serialization_command - Command to be serialized * @wlan_serialization_cmd_type: Type of command @@ -202,18 +181,23 @@ union wlan_serialization_obj_context { * @is_high_priority: Normal/High Priority at which the cmd has to be queued * @cmd_timeout_cb: Command timeout callback * @cmd_timeout_duration: Timeout duration in milliseconds - * @obj_ctxt: Object passed for the command + * @vdev: VDEV object associated to the command * @umac_cmd: Actual command that needs to be sent to WMI/firmware + * + * Note: Unnamed union has been used in this structure, so that in future if + * somebody wants to add pdev or psoc structure then that person can add without + * modifying existing code. */ struct wlan_serialization_command { enum wlan_serialization_cmd_type cmd_type; uint16_t cmd_id; wlan_serialization_cmd_callback cmd_cb; uint8_t source; - uint8_t is_high_priority; - wlan_serialization_active_cmd_timeout_callback cmd_timeout_cb; + bool is_high_priority; uint16_t cmd_timeout_duration; - union wlan_serialization_obj_context obj_ctxt; + union { + struct wlan_objmgr_vdev *vdev; + }; void *umac_cmd; }; @@ -223,7 +207,7 @@ struct wlan_serialization_command { * @cmd_type: Command type * @cmd_id: Command ID * @req_type: Commands that need to be cancelled - * @obj_ctxt: PDEV/VDEV object on which the commands need to be cancelled + * @vdev: VDEV object associated to the command * @queue_type: Queues from which the command to be cancelled */ struct wlan_serialization_queued_cmd_info { @@ -231,7 +215,9 @@ struct wlan_serialization_queued_cmd_info { enum wlan_serialization_cmd_type cmd_type; uint16_t cmd_id; enum wlan_serialization_cancel_type req_type; - union wlan_serialization_obj_context obj_ctxt; + union { + struct wlan_objmgr_vdev *vdev; + }; uint8_t queue_type; }; diff --git a/umac/cmn_services/serialization/src/wlan_serialization_api.c b/umac/cmn_services/serialization/src/wlan_serialization_api.c index b5598230fc..f056c35cc2 100644 --- a/umac/cmn_services/serialization/src/wlan_serialization_api.c +++ b/umac/cmn_services/serialization/src/wlan_serialization_api.c @@ -40,6 +40,10 @@ wlan_serialization_register_comp_info_cb(struct wlan_objmgr_psoc *psoc, if (status != QDF_STATUS_SUCCESS) return status; ser_soc_obj = wlan_serialization_get_psoc_priv_obj(psoc); + if (!ser_soc_obj) { + serialization_err("invalid ser_soc_obj"); + return QDF_STATUS_E_FAILURE; + } ser_soc_obj->comp_info_cb[cmd_type][comp_id] = cb; return QDF_STATUS_SUCCESS; @@ -57,6 +61,10 @@ wlan_serialization_deregister_comp_info_cb(struct wlan_objmgr_psoc *psoc, if (status != QDF_STATUS_SUCCESS) return status; ser_soc_obj = wlan_serialization_get_psoc_priv_obj(psoc); + if (!ser_soc_obj) { + serialization_err("invalid ser_soc_obj"); + return QDF_STATUS_E_FAILURE; + } ser_soc_obj->comp_info_cb[cmd_type][comp_id] = NULL; return QDF_STATUS_SUCCESS; diff --git a/umac/cmn_services/serialization/src/wlan_serialization_main.c b/umac/cmn_services/serialization/src/wlan_serialization_main.c index e9ab1e55a1..aaf3c63f25 100644 --- a/umac/cmn_services/serialization/src/wlan_serialization_main.c +++ b/umac/cmn_services/serialization/src/wlan_serialization_main.c @@ -48,7 +48,7 @@ wlan_serialization_apply_rules_cb_init(struct wlan_objmgr_psoc *psoc) wlan_serialization_get_psoc_priv_obj(psoc); if (ser_soc_obj == NULL) { - serialization_alert("Serialization PSOC private obj is NULL"); + serialization_err("invalid ser_soc_obj"); return QDF_STATUS_E_PERM; } ser_soc_obj->apply_rules_cb[WLAN_SER_CMD_SCAN] = wlan_apply_scan_rules; @@ -76,7 +76,7 @@ wlan_serialization_apply_rules_cb_deinit(struct wlan_objmgr_psoc *psoc) uint8_t i; if (ser_soc_obj == NULL) { - serialization_alert("Serialization PSOC private obj is NULL"); + serialization_err("invalid ser_soc_obj"); return QDF_STATUS_E_PERM; } for (i = 0; i < WLAN_SER_CMD_MAX; i++) @@ -91,6 +91,10 @@ QDF_STATUS wlan_serialization_psoc_close(struct wlan_objmgr_psoc *psoc) struct wlan_serialization_psoc_priv_obj *ser_soc_obj = wlan_serialization_get_psoc_priv_obj(psoc); + if (!ser_soc_obj) { + serialization_err("invalid ser_soc_obj"); + return QDF_STATUS_E_FAILURE; + } qdf_mem_free(ser_soc_obj->timers); ser_soc_obj->timers = NULL; ser_soc_obj->max_active_cmds = 0; @@ -106,6 +110,10 @@ QDF_STATUS wlan_serialization_psoc_open(struct wlan_objmgr_psoc *psoc) struct wlan_serialization_psoc_priv_obj *ser_soc_obj = wlan_serialization_get_psoc_priv_obj(psoc); + if (!ser_soc_obj) { + serialization_err("invalid ser_soc_obj"); + return QDF_STATUS_E_FAILURE; + } /* TODO:Get WLAN_SERIALIZATION_MAX_ACTIVE_SCAN_CMDS frm service ready */ pdev_count = wlan_psoc_get_pdev_count(psoc); ser_soc_obj->max_active_cmds = WLAN_SERIALIZATION_MAX_ACTIVE_SCAN_CMDS + @@ -134,7 +142,7 @@ QDF_STATUS wlan_serialization_psoc_obj_create_notification( return QDF_STATUS_E_NOMEM; } wlan_objmgr_psoc_component_obj_attach(psoc, - WLAN_UMAC_COMP_SERIALIZATION, (void *)soc_ser_obj, + WLAN_UMAC_COMP_SERIALIZATION, soc_ser_obj, QDF_STATUS_SUCCESS); serialization_info("ser psoc obj created"); @@ -233,7 +241,7 @@ QDF_STATUS wlan_serialization_pdev_obj_create_notification( return status; } status = wlan_objmgr_pdev_component_obj_attach(pdev, - WLAN_UMAC_COMP_SERIALIZATION, (void *)ser_pdev_obj, + WLAN_UMAC_COMP_SERIALIZATION, ser_pdev_obj, QDF_STATUS_SUCCESS); if (status != QDF_STATUS_SUCCESS) { serialization_err("serialization pdev obj attach failed"); @@ -243,7 +251,7 @@ QDF_STATUS wlan_serialization_pdev_obj_create_notification( return status; } -QDF_STATUS wlan_serialization_psoc_obj_destroy_notification( +QDF_STATUS wlan_serialization_psoc_obj_destroy_notification( struct wlan_objmgr_psoc *psoc, void *arg_list) { QDF_STATUS status; @@ -251,7 +259,7 @@ QDF_STATUS wlan_serialization_psoc_obj_destroy_notification( wlan_serialization_get_psoc_priv_obj(psoc); if (NULL == ser_soc_obj) { - serialization_err("ser psoc private obj is NULL"); + serialization_err("invalid ser_soc_obj"); return QDF_STATUS_E_FAULT; } status = wlan_objmgr_psoc_component_obj_detach(psoc, @@ -273,8 +281,7 @@ QDF_STATUS wlan_serialization_pdev_obj_destroy_notification( wlan_serialization_get_pdev_priv_obj(pdev); status = wlan_objmgr_pdev_component_obj_detach(pdev, - WLAN_UMAC_COMP_SERIALIZATION, - (void *)ser_pdev_obj); + WLAN_UMAC_COMP_SERIALIZATION, ser_pdev_obj); wlan_serialization_destroy_list(ser_pdev_obj, &ser_pdev_obj->active_list); wlan_serialization_destroy_list(ser_pdev_obj, diff --git a/umac/cmn_services/serialization/src/wlan_serialization_utils.c b/umac/cmn_services/serialization/src/wlan_serialization_utils.c index 2dbec9591e..674789ae5f 100644 --- a/umac/cmn_services/serialization/src/wlan_serialization_utils.c +++ b/umac/cmn_services/serialization/src/wlan_serialization_utils.c @@ -65,8 +65,7 @@ struct wlan_serialization_psoc_priv_obj *wlan_serialization_get_psoc_priv_obj( { struct wlan_serialization_psoc_priv_obj *ser_soc_obj; wlan_psoc_obj_lock(psoc); - ser_soc_obj = (struct wlan_serialization_psoc_priv_obj *) - wlan_objmgr_psoc_get_comp_private_obj(psoc, + ser_soc_obj = wlan_objmgr_psoc_get_comp_private_obj(psoc, WLAN_UMAC_COMP_SERIALIZATION); wlan_psoc_obj_unlock(psoc); @@ -78,8 +77,7 @@ struct wlan_serialization_pdev_priv_obj *wlan_serialization_get_pdev_priv_obj( { struct wlan_serialization_pdev_priv_obj *obj; wlan_pdev_obj_lock(pdev); - obj = (struct wlan_serialization_pdev_priv_obj *) - wlan_objmgr_pdev_get_comp_private_obj(pdev, + obj = wlan_objmgr_pdev_get_comp_private_obj(pdev, WLAN_UMAC_COMP_SERIALIZATION); wlan_pdev_obj_unlock(pdev);