Re: [PATCH v1] staging: rtl8723bs: remove GEN_CMD_CODE macro and callback array

[Bera Yüzlü]

On Tue, 31 Mar 2026 20:08:18 -0400, Xiyuan Guo wrote:
> The GEN_CMD_CODE macro and the rtw_cmd_callback function pointer array
> add unnecessary layers of indirection and make the code difficult to
> follow.
> 
> Remove the GEN_CMD_CODE macro and replace the function pointer array
> dispatcher in rtw_cmd_thread() with a switch statement.
> 
> Signed-off-by: Xiyuan Guo <tommyguo039@xxxxxxxxx>
> ---

Can't compile it :( Please compile driver with
'make M=drivers/staging/rtl8723bs' before sending your patch.

>  		/* call callback function for post-processed */
> -		if (pcmd->cmdcode < ARRAY_SIZE(rtw_cmd_callback)) {
> -			pcmd_callback = rtw_cmd_callback[pcmd->cmdcode].callback;
> -			if (!pcmd_callback) {
> -				rtw_free_cmd_obj(pcmd);
> -			} else {
> -				/* todo: !!! fill rsp_buf to pcmd->rsp if (pcmd->rsp!= NULL) */
> -				pcmd_callback(pcmd->padapter, pcmd);/* need consider that free cmd_obj in rtw_cmd_callback */
> -			}
> -		} else {
> +		switch (pcmd->cmdcode) {
> +		case _Read_BBREG:
> +		case _Read_RFREG:
> +			rtw_getbbrfreg_cmdrsp_callback(padapter, pcmd);
> +			break;
> +		case _JoinBss:
> +			rtw_joinbss_cmd_callback(padapter, pcmd);
> +			break;
> +		case _DisConnect:
> +			rtw_disassoc_cmd_callback(padapter, pcmd);
> +			break;
> +		case _CreateBss:
> +			rtw_createbss_cmd_callback(padapter, pcmd);
> +			break;
> +		case _SiteSurvey:
> +			rtw_survey_cmd_callback(padapter, pcmd);
> +			break;
> +		case _SetStaKey:
> +			rtw_setstaKey_cmdrsp_callback(padapter, pcmd);
> +			break;
> +		case _SetAssocSta:
> +			rtw_setassocsta_cmdrsp_callback(padapter, pcmd);
> +			break;
> +		default:
>  			rtw_free_cmd_obj(pcmd);
> +			break;
>  		}
>  		flush_signals_thread();
>  		goto _next;

You are deleting a TODO here.

> +	_Read_MACREG,	/*0*/
> +	_Write_MACREG,
> +	_Read_BBREG,
> +	_Write_BBREG,
> +	_Read_RFREG,
> +	_Write_RFREG, /*5*/
> +	_Read_EEPROM,
> +	_Write_EEPROM,
> +	_Read_EFUSE,
> +	_Write_EFUSE,
> +
> +	_Read_CAM,	/*10*/
> +	_Write_CAM,
> +	_setBCNITV,
> +	_setMBIDCFG,
> +	_JoinBss,   /*14*/
> +	_DisConnect, /*15*/
> +	_CreateBss,
> +	_SetOpMode,
> +	_SiteSurvey,  /*18*/
> +	_SetAuth,
> +
> +	_SetKey,	/*20*/
> +	_SetStaKey,
> +	_SetAssocSta,
> +	_DelAssocSta,
> +	_SetStaPwrState,
> +	_SetBasicRate, /*25*/
> +	_GetBasicRate,
> +	_SetDataRate,
> +	_GetDataRate,
> +	_SetPhyInfo,
> +
> +	_GetPhyInfo,	/*30*/
> +	_SetPhy,
> +	_GetPhy,
> +	_readRssi,
> +	_readGain,
> +	_SetAtim, /*35*/
> +	_SetPwrMode,
> +	_JoinbssRpt,
> +	_SetRaTable,
> +	_GetRaTable,
> +
> +	_GetCCXReport, /*40*/
> +	_GetDTMReport,
> +	_GetTXRateStatistics,
> +	_SetUsbSuspend,
> +	_SetH2cLbk,
> +	_AddBAReq, /*45*/
> +	_SetChannel, /*46*/
> +	_SetTxPower,
> +	_SwitchAntenna,
> +	_SetCrystalCap,
> +	_SetSingleCarrierTx, /*50*/
> +
> +	_SetSingleToneTx,/*51*/
> +	_SetCarrierSuppressionTx,
> +	_SetContinuousTx,
> +	_SwitchBandwidth, /*54*/
> +	_TX_Beacon, /*55*/
> +
> +	_Set_MLME_EVT, /*56*/
> +	_Set_Drv_Extra, /*57*/
> +	_Set_H2C_MSG, /*58*/
> +
> +	_SetChannelPlan, /*59*/
> +
> +	_SetChannelSwitch, /*60*/
> +	_TDLS, /*61*/
> +	_ChkBMCSleepq, /*62*/
> +
> +	_RunInThreadCMD, /*63*/ 

Why not use snake case instead?

Thanks,
Bera