Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7d4eacd9-71be-e75f-fad4-ec07c00041d5@quicinc.com>
Date:   Thu, 20 Apr 2023 15:24:21 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH 1/2] wifi: ath11k: factory test mode support
To:     Kalle Valo <kvalo@kernel.org>
CC:     <ath11k@lists.infradead.org>, <linux-wireless@vger.kernel.org>,
        "Govindaraj Saminathan" <quic_gsaminat@quicinc.com>,
        Sowmiya Sree Elavalagan <quic_ssreeela@quicinc.com>
References: <20230213130854.2473-1-quic_rajkbhag@quicinc.com>
 <20230213130854.2473-2-quic_rajkbhag@quicinc.com> <87mt4hnfk2.fsf@kernel.org>
Content-Language: en-US
From:   Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
In-Reply-To: <87mt4hnfk2.fsf@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 3/13/2023 3:23 PM, Kalle Valo wrote:
> Raj Kumar Bhagat <quic_rajkbhag@quicinc.com> writes:
> 
>> From: Govindaraj Saminathan <quic_gsaminat@quicinc.com>
>>
>> Add support to process factory test mode commands(FTM) for calibration.
>> By default firmware start with NORMAL mode and to process the FTM commands
>> firmware needs to be restarted in FTM mode using module parameter ftm_mode.
>> The pre-request is all the radios should be down before starting the test.
>>
>> When start command ATH11K_TM_CMD_TESTMODE_START is received, ar state
>> is set to Test Mode. If the FTM command or event length is greater
>> than 256 bytes, it will be broken down into multiple segments and
>> encoded with TLV header if it is segmented commands, else it is sent
>> to firmware as it is.
>>
>> On receiving UTF event from firmware, if it is segmented event, the driver
>> will wait until it receives all the segments and notify the complete
>> data to user application. In case the segmented sequence are missed or
>> lost from the firmware, driver will skip the already received partial data.
>>
>> In case of unsegmented UTF event from firmware, driver notifies the
>> data to the user application as it comes. Applications handles
>> the data further.
>>
>> Command to boot in ftm mode
>> insmod ath11k ftm_mode=1
>>
>> Tested-on : IPQ8074 hw2.0 AHB WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
>>
>> Signed-off-by: Govindaraj Saminathan <quic_gsaminat@quicinc.com>
>> Co-developed-by: Sowmiya Sree Elavalagan <quic_ssreeela@quicinc.com>
>> Signed-off-by: Sowmiya Sree Elavalagan <quic_ssreeela@quicinc.com>
>> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
> 
> [...]
> 
>> --- a/drivers/net/wireless/ath/ath11k/core.c
>> +++ b/drivers/net/wireless/ath/ath11k/core.c
>> @@ -1,7 +1,7 @@
>>  // SPDX-License-Identifier: BSD-3-Clause-Clear
>>  /*
>>   * Copyright (c) 2018-2019 The Linux Foundation. All rights reserved.
>> - * Copyright (c) 2021-2022 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2021-2023 Qualcomm Innovation Center, Inc. All rights reserved.
>>   */
>>  
>>  #include <linux/module.h>
>> @@ -32,6 +32,10 @@ module_param_named(frame_mode, ath11k_frame_mode, uint, 0644);
>>  MODULE_PARM_DESC(frame_mode,
>>  		 "Datapath frame mode (0: raw, 1: native wifi (default), 2: ethernet)");
>>  
>> +unsigned int ath11k_ftm_mode;
>> +module_param_named(ftm_mode, ath11k_ftm_mode, uint, 0444);
>> +MODULE_PARM_DESC(ftm_mode, "Boots up in factory test mode");
> 
> I changed this to bool as there's only two values, true or false.
> 
>> @@ -1362,6 +1366,11 @@ static int ath11k_core_soc_create(struct ath11k_base *ab)
>>  {
>>  	int ret;
>>  
>> +	if (ath11k_ftm_mode) {
>> +		ab->fw_mode = ATH11K_FIRMWARE_MODE_FTM;
>> +		ath11k_info(ab, "Booting in ftm mode\n");
>> +	}
> 
> I changed this to:
> 
> "Booting in factory test mode\n"
> 
>> @@ -1822,6 +1832,10 @@ static void ath11k_core_post_reconfigure_recovery(struct ath11k_base *ab)
>>  			ath11k_warn(ab,
>>  				    "device is wedged, will not restart radio %d\n", i);
>>  			break;
>> +		case ATH11K_STATE_TM:
>> +			ath11k_warn(ab, "fw mode reset done radio %d\n", i);
>> +			break;
> 
> What is this warning supposed to tell the user? Should it be a debug
> message instead?

Will move this to debug message in version 2.


Thanks,
Raj