Received-SPF: pass (google.com: domain of linux-wireless+bounces-7877-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Content-Type: text/plain; charset="utf-8"
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH] wifi: ath12k: fix Smatch warnings on
 ath12k_core_suspend()
From: Kalle Valo <kvalo@kernel.org>
In-Reply-To: <20240511095045.9623-1-quic_bqiang@quicinc.com>
References: <20240511095045.9623-1-quic_bqiang@quicinc.com>
To: Baochen Qiang <quic_bqiang@quicinc.com>
Cc: <ath12k@lists.infradead.org>, <linux-wireless@vger.kernel.org>,
 <quic_bqiang@quicinc.com>
User-Agent: pwcli/0.1.1-git (https://github.com/kvalo/pwcli/) Python/3.11.2
Message-ID: <171627874515.3970531.14849401766701912789.kvalo@kernel.org>
Date: Tue, 21 May 2024 08:05:46 +0000 (UTC)

Baochen Qiang <quic_bqiang@quicinc.com> wrote:

> Smatch is throwing below warning:
> 
> Commit 692921ead832 ("wifi: ath12k: flush all packets before
> suspend") leads to the following Smatch static checker warning:
> 
>         drivers/net/wireless/ath/ath12k/core.c:58 ath12k_core_suspend()
>         warn: sleeping in atomic context
> 
> and also gives the reason:
> 
> drivers/net/wireless/ath/ath12k/core.c
>     48         int ret, i;
>     49
>     50         if (!ab->hw_params->supports_suspend)
>     51                 return -EOPNOTSUPP;
>     52
>     53         rcu_read_lock();
>                ^^^^^^^^^^^^^^^
> Disables preemption.
> 
>     54         for (i = 0; i < ab->num_radios; i++) {
>     55                 ar = ath12k_mac_get_ar_by_pdev_id(ab, i);
>     56                 if (!ar)
>     57                         continue;
> --> 58                 ret = ath12k_mac_wait_tx_complete(ar);
>                                         ^^^^^^^
> Sleeping in atomic context.
> 
>     59                 if (ret) {
>     60                         ath12k_warn(ab, "failed to wait tx complete: %d\n", ret);
>     61                         rcu_read_unlock();
>     62                         return ret;
>     63                 }
>     64         }
>     65         rcu_read_unlock();
> 
> But it is weird that no warning on this in run time even with
> CONFIG_DEBUG_ATOMIC_SLEEP=y. With some debug it is found that this is
> because: when system goes to suspend, ath12k_mac_op_stop() gets called
> where then in ath12k_mac_stop() ab->pdevs_active[ar->pdev_idx] is cleared.
> This results in ath12k_mac_get_ar_by_pdev_id() always returning a NULL ar,
> and thereby ath12k_mac_wait_tx_complete() never gets a chance to run.
> 
> Fix it by retrieving ar directly from ab->pdevs[].ar instead of using
> ath12k_mac_get_ar_by_pdev_id(). Since ab->pdevs[].ar is set at boot time
> and won't get cleared when suspend, ath12k_mac_wait_tx_complete() won't
> be skipped. In addition, with ath12k_mac_get_ar_by_pdev_id() removed,
> rcu_read_lock()/unlock() are not needed any more, so remove them. This
> also fixes the warning above.
> 
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
> 
> Fixes: 692921ead832 ("wifi: ath12k: flush all packets before suspend")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/ath12k/7a96ca11-80b5-4751-8cfc-fa637f3aa63a@moroto.mountain/
> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

Patch applied to ath-next branch of ath.git, thanks.

33370412eced wifi: ath12k: fix Smatch warnings on ath12k_core_suspend()

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20240511095045.9623-1-quic_bqiang@quicinc.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches