Kang Yang <quic_kangyang@xxxxxxxxxxx> writes: >>>> @@ -5623,6 +5625,9 @@ static int >>>> ath11k_dp_full_mon_process_rx(struct ath11k_base *ab, int mac_id, >>>> goto reap_status_ring; >>>> } >>>> + mon_dst_srng = &ar->ab->hal.srng_list[dp- >>>> >rxdma_mon_dst_ring.ring_id]; >>>> + spin_lock_bh(&mon_dst_srng->lock); >>> >>> Why initialise mon_dst_srng differently? The commit message mentions >>> nothing about this change. >> Because need to fetch spin lock inside 'struct hal_srng'. If still >> use 'void *mon_dst_srng', need to perform a variable type cast. >> Bur 'struct hal_srng' will make this line too long: >> 'struct hal_srng *mon_dst_srng = &ar->ab->hal.srng_list[dp- >> >rxdma_mon_dst_ring.ring_id];' >> So I separated the definition and initialization. >> ath11k_dp_rx_reap_mon_status_ring()/ath11k_dp_process_rx and others >> ring process function is the same. >> > > Do i need to send a new version for this? Yeah, please mention in the commit message why you mon_dst_srng initialisation. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches