On 7/7/2025 9:11 PM, Muhammad Usama Anjum wrote: >>>>>>> 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. >>>>> >>>> Hmm... not quite understand your question. Can you elaborate? >>> I'm trying to understand the possibility of cleanup of hal in suspend handler. For example: >>> * The driver has been loaded and has been working fine. >>> * The user called suspend. So all devices would be suspended. >>> * In suspend handler of the ath11k, we should do the necessary cleanups of the states >>> like hal. >>> * When the device would resume after long time, the hal would have the correct state >>> already. So we'll not need to deinit and init again. >> The hal cleanup is not only needed by suspend/resume, but also a step of reset/recover >> process. So If we are moving the cleanup to suspend handler, similar stuff needs to be >> done for reset/recover as well. > It makes sense. > > So clearing the hal structure completely other than ab->hal.srn_config doesn't seem > right. I've also tested it and it crashes the whole system. > > On contrary, with only the current patch applied, there is no abnormality. > > num_shadow_reg_configured and avail_blk_resource are non-zero. If I make them 0, > driver still keeps on working. > > ab->hal.num_shadow_reg_configured = 0; > ab->hal.avail_blk_resource = 0; > ab->hal.current_blk_index = 0; > > As you have suggested setting these 3 to zero, is there any other variable in hal > structure which should be set to zero? IMO srng_config, rdp, wrp and srng_key may keep unchanged through suspend/reset, all other fields should be cleared/reinitialized.
