Received-SPF: pass (google.com: domain of linux-wireless+bounces-6269-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <8122e56b-1e57-4501-8180-24a65247465f@quicinc.com>
Date: Fri, 12 Apr 2024 11:24:40 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 01/10] wifi: ath12k: rearrange IRQ enable/disable in reset
 path
Content-Language: en-US
To: Baochen Qiang <quic_bqiang@quicinc.com>, <ath12k@lists.infradead.org>
CC: <linux-wireless@vger.kernel.org>
References: <20240412060620.27519-1-quic_bqiang@quicinc.com>
 <20240412060620.27519-2-quic_bqiang@quicinc.com>
From: Sidhanta Sahu <quic_sidhanta@quicinc.com>
In-Reply-To: <20240412060620.27519-2-quic_bqiang@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 4/11/2024 11:06 PM, Baochen Qiang wrote:
> For non-WoW suspend/resume, ath12k host powers down whole hardware
> when suspend and powers up it when resume, the code path it goes
> through is very like the ath12k reset logic.
> 
> In order to reuse that logic, rearrange IRQ handling in the reset
> path.
> 
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> 
> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
> ---
>   drivers/net/wireless/ath/ath12k/core.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
> index 3c522a4b3e9b..28663cf4db30 100644
> --- a/drivers/net/wireless/ath/ath12k/core.c
> +++ b/drivers/net/wireless/ath/ath12k/core.c
> @@ -879,9 +879,8 @@ static int ath12k_core_reconfigure_on_crash(struct ath12k_base *ab)
>   	int ret;
>   
>   	mutex_lock(&ab->core_lock);
> -	ath12k_hif_irq_disable(ab);
>   	ath12k_dp_pdev_free(ab);
> -	ath12k_hif_stop(ab);
> +	ath12k_ce_cleanup_pipes(ab);
>   	ath12k_wmi_detach(ab);
>   	ath12k_dp_rx_pdev_reo_cleanup(ab);
>   	mutex_unlock(&ab->core_lock);
> @@ -1136,6 +1135,9 @@ static void ath12k_core_reset(struct work_struct *work)
>   	time_left = wait_for_completion_timeout(&ab->recovery_start,
>   						ATH12K_RECOVER_START_TIMEOUT_HZ);
>   
> +	ath12k_hif_irq_disable(ab);
> +	ath12k_hif_ce_irq_disable(ab);
> +

This core reset is called due to a fatal error. In case of an unlikely 
scenario, when there are consecutive fatal errors notification before 
the first fatal error is handled, then we will be disabling same irqs 
again. Shall we have a check here to see if ab's already in reset using 
ab->is_reset.

>   	ath12k_hif_power_down(ab);
>   	ath12k_qmi_free_resource(ab);
>   	ath12k_hif_power_up(ab);