From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B816FC83F04 for ; Wed, 2 Jul 2025 16:47:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=kHonnTMMDQvh5LezYlicL8UTSbmvRPcd3Q0viWMvaiw=; b=mJkKM2M46R6jyE4WNa5EWYGygr YKwsXko1Sx8kazctHuJNRQIrjfQsw3Rtvr+rYBVV3j0+R+rKGbup0h8Oyw00/hSCpaV2jEF63p1WT 9DnRB+SGpc7yYc4YlCIEkLNqXV18Ur1IWw4x3oc889O+uXqDGjJ376GGdoVdPP5Q81d2Fu3ktb5B7 dU0U1AHO6brf8TNL5qjbHegK1HQ8TKx6Oa/ohgQ1LoVjWBxn28FwAVzEVwyAO/jGEfLgJAGT1KpKk JxHYsACxdx1TYi8HZtWYWObQoDeR5VrasZ70OLHfZjXcjl4xfS/vdjjObyvI9WLbFjz/SrWOy3dC7 h3ZXvmnw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uX0cA-000000090bF-49A0; Wed, 02 Jul 2025 16:47:18 +0000 Received: from sender4-op-o12.zoho.com ([136.143.188.12]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uX04i-00000008ucW-2q9H for ath11k@lists.infradead.org; Wed, 02 Jul 2025 16:12:45 +0000 ARC-Seal: i=1; a=rsa-sha256; t=1751472731; cv=none; d=zohomail.com; s=zohoarc; b=NZzFzK31qLQT6oSbwobh15Vji65dWAbwwFEkkMNl8TcJmLylR4y0jFb9m3r9DB9kfdcDwjt4smQPkKVF4Zv1rQ1QnAmuo/NqTNRbMwwYpSp3p0vrBoAbI64Zl2AvR/bGm0R6iyZ4D2cHoDvn0B0Uv6WLTCC64wmhHHZoVGq9tKM= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1751472731; h=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To; bh=kHonnTMMDQvh5LezYlicL8UTSbmvRPcd3Q0viWMvaiw=; b=H1wfFrn1OzG3+Woz0eV9r80LdVzL43Q6AeyvONmLFitLxeHh7CELPvQ+kefMppQ+HJEq+Dss5FJWkibjiO8n7YFneZYr2Z/yLuRsto0vtdFRvYzy2VKxnxC9yBXeXO1P/Eds9mGlqP0v1QivK5vagLoejHQmKx7xRl3RTyTHdDY= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=collabora.com; spf=pass smtp.mailfrom=usama.anjum@collabora.com; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1751472731; s=zohomail; d=collabora.com; i=usama.anjum@collabora.com; h=Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:Cc:Cc:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To; bh=kHonnTMMDQvh5LezYlicL8UTSbmvRPcd3Q0viWMvaiw=; b=UUJLIsIKL6CdgL+PpjU27TlDEtHpS4PgP0a44fvCO0O8SG+1WXv+HsXtdlfoy65p NiRGQ4QQtwAzHywtvNoD6Xar+G5wTAJO5XgRU43vkNvqN4P1hABmGY3/22iMVafyIf9 Fwe2NnKacPYKkonXzw5SH0EIdGlYvdWyhFNmDdaY= Received: by mx.zohomail.com with SMTPS id 1751472728803823.4525464952651; Wed, 2 Jul 2025 09:12:08 -0700 (PDT) Message-ID: <29ba0afa-9a1b-40f9-a174-d03902ea5d3f@collabora.com> Date: Wed, 2 Jul 2025 21:12:00 +0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/3] bus: mhi: don't deinitialize and re-initialize again To: Baochen Qiang , Manivannan Sadhasivam , Jeff Johnson , Jeff Hugo , Youssef Samir , Matthew Leung , Yan Zhen , Alexander Wilhelm , Alex Elder , Kunwu Chan , Greg Kroah-Hartman , Siddartha Mohanadoss , Sujeev Dias , Julia Lawall , John Crispin , Muna Sinada , Venkateswara Naralasetty , Maharaja Kennadyrajan , mhi@lists.linux.dev, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org, ath11k@lists.infradead.org Cc: kernel@collabora.com References: <20250630074330.253867-1-usama.anjum@collabora.com> <20250630074330.253867-3-usama.anjum@collabora.com> <5f2a900a-3c8e-4b16-bd91-500af7d0315e@oss.qualcomm.com> Content-Language: en-US From: Muhammad Usama Anjum In-Reply-To: <5f2a900a-3c8e-4b16-bd91-500af7d0315e@oss.qualcomm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ZohoMailClient: External X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250702_091244_787279_BB0965C0 X-CRM114-Status: GOOD ( 23.46 ) X-BeenThere: ath11k@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "ath11k" Errors-To: ath11k-bounces+ath11k=archiver.kernel.org@lists.infradead.org Thank you for reviewing. On 7/2/25 8:50 AM, Baochen Qiang wrote: > > > On 6/30/2025 3:43 PM, Muhammad Usama Anjum wrote: >> Don't deinitialize and reinitialize the HAL helpers. The dma memory is >> deallocated and there is high possibility that we'll not be able to get >> the same memory allocated from dma when there is high memory pressure. >> >> Tested-on: WCN6855 WLAN.HSP.1.1-03926.13-QCAHSPSWPL_V2_SILICONZ_CE-2.52297.6 >> >> Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices") >> Signed-off-by: Muhammad Usama Anjum >> --- >> drivers/net/wireless/ath/ath11k/core.c | 5 ----- >> 1 file changed, 5 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c >> index 4488e4cdc5e9e..bc4930fe6a367 100644 >> --- a/drivers/net/wireless/ath/ath11k/core.c >> +++ b/drivers/net/wireless/ath/ath11k/core.c >> @@ -2213,14 +2213,9 @@ static int ath11k_core_reconfigure_on_crash(struct ath11k_base *ab) >> mutex_unlock(&ab->core_lock); >> >> ath11k_dp_free(ab); >> - ath11k_hal_srng_deinit(ab); >> >> ab->free_vdev_map = (1LL << (ab->num_radios * TARGET_NUM_VDEVS(ab))) - 1; >> >> - ret = ath11k_hal_srng_init(ab); >> - if (ret) >> - return ret; >> - > > while I agree there is no need of a dealloc/realloc, we can not simply remove calling the > _deinit()/_init() pair. At least the memset() cleanup to hal parameters (e.g. Why do is it being done in the resume handler? Shouldn't those parameters be cleaned up in resume handler? So when device wakes up, its state is already correct. I'm not sure why it worked every time when I tested it on my device. > avail_blk_resource, current_blk_index and num_shadow_reg_configured etc.) inside the > _init() needs to be kept as the later operation needs a clean state of them. So should we just memset these 3? > >> clear_bit(ATH11K_FLAG_CRASH_FLUSH, &ab->dev_flags); >> >> ret = ath11k_core_qmi_firmware_ready(ab); > > the _deinit() is still getting called in case ath11k_core_qmi_firmware_ready() fails, > making it a little odd since there is no _init() anymore with this change, though this is > the way of current logic (I mean the hal is currently deinit in the error path). > > Thinking it more, if we hit the error path, seems the only way is to remove ath11k module. > In that case the _deinit() would be called again in ath11k_pci_remove(), leading to issues > (at least I see a double free of hal->srng_config). But this is another topic which can be > fixed in a separate patch. I don't think this is the problem as HAL is already initialized when before the system has suspended. So by removing deinit() and init() pairs, the HAL still remains initialized. Or maybe I've missed something?