Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v3] ath11k: cold boot calibration support
To:     Sowmiya Sree Elavalagan <ssreeela@codeaurora.org>,
        ath11k@lists.infradead.org
Cc:     linux-wireless@vger.kernel.org,
        Govindaraj Saminathan <gsamin@codeaurora.org>
References: <1600163991-2093-1-git-send-email-ssreeela@codeaurora.org>
From:   Peter Oh <peter.oh@eero.com>
Message-ID: <b0d42c5e-8aac-cea2-bc6d-3a6341689d05@eero.com>
Date:   Thu, 17 Sep 2020 12:54:51 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <1600163991-2093-1-git-send-email-ssreeela@codeaurora.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk


On 9/15/20 2:59 AM, Sowmiya Sree Elavalagan wrote:
> From: Govindaraj Saminathan <gsamin@codeaurora.org>
>
> cold boot calibration is the process to calibrate all the channels
> during the boot-up to avoid the calibration delay during the
> channel change.
> During the boot-up, firmware started with mode “cold_boot_calibration”
> Firmware calibrate all channels and generate CalDb(DDR).
> Subsequent WIFI ON will reuse the same CalDb.
Can you clarify the definition of "WIFI ON"?
> Firmware restarted with Mission mode to continue the normal operation.

Please change the mission mode to normal according to 
ATH11K_FIRMWARE_MODE_NORMAL.

Which code it sets back to normal mode from cold boot mode? I don't see it.

> +static int ath11k_qmi_process_coldboot_calibration(struct ath11k_base *ab)
> +{
> +	int timeout;
> +	int ret;
> +
> +	ret = ath11k_qmi_wlanfw_mode_send(ab, ATH11K_FIRMWARE_MODE_COLD_BOOT);
> +	if (ret < 0) {
> +		ath11k_warn(ab, "qmi failed to send wlan fw mode:%d\n", ret);
> +		return ret;
> +	}
> +
> +	ath11k_dbg(ab, ATH11K_DBG_QMI, "Coldboot calibration wait started\n");
> +
> +	timeout = wait_event_timeout(ab->qmi.cold_boot_waitq,
> +				     (ab->qmi.cal_done  == 1),
> +				     ATH11K_COLD_BOOT_FW_RESET_DELAY);
> +	if (timeout <= 0) {
> +		ath11k_warn(ab, "Coldboot Calibration timed out\n");
> +		return -ETIMEDOUT;

Does it make ath11k firmware stay in cold boot mode when timeout happens?

If so, is it worthy to have WLAN usuable due to cold boot calibration 
failure?

As I understand, initial calibration on current operation channel will 
be done regardless of cold boot calibration

> +	}
> +
> +	ath11k_dbg(ab, ATH11K_DBG_QMI, "Coldboot calibration wait ended\n");
> +
> +	return 0;
> +}
> +
>   static int
>   ath11k_qmi_driver_event_post(struct ath11k_qmi *qmi,
>   			     enum ath11k_qmi_event_type type,
> @@ -2597,9 +2643,16 @@ static void ath11k_qmi_driver_event_work(struct work_struct *work)
>   				break;
>   			}
>   
> -			ath11k_core_qmi_firmware_ready(ab);
> -			ab->qmi.cal_done = 1;
> -			set_bit(ATH11K_FLAG_REGISTERED, &ab->dev_flags);
> +			if (ath11k_cold_boot_cal && ab->qmi.cal_done == 0 &&
> +			    ab->hw_params.cold_boot_calib) {
> +				ath11k_qmi_process_coldboot_calibration(ab);
> +			} else {
> +				clear_bit(ATH11K_FLAG_CRASH_FLUSH,
> +					  &ab->dev_flags);
> +				clear_bit(ATH11K_FLAG_RECOVERY, &ab->dev_flags);
Please split this patch into a separate patch. It's not directly related 
to cold boot.
> +				ath11k_core_qmi_firmware_ready(ab);
> +				set_bit(ATH11K_FLAG_REGISTERED, &ab->dev_flags);
> +			}
>   
>   			break;
>   		case ATH11K_QMI_EVENT_COLD_BOOT_CAL_DONE:

Thanks,

Peter