Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c74d25e7-1947-6a4b-6e97-f1ab6f75cc89@quicinc.com>
Date:   Thu, 13 Oct 2022 22:07:40 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.1
Subject: Re: [PATCH 11/50] wifi: ath12k: add dp.c
Content-Language: en-US
From:   Pradeep Kumar Chitrapu <quic_pradeepc@quicinc.com>
To:     Jeff Johnson <quic_jjohnson@quicinc.com>,
        Kalle Valo <kvalo@kernel.org>, <linux-wireless@vger.kernel.org>
CC:     <ath12k@lists.infradead.org>
References: <20220812161003.27279-1-kvalo@kernel.org>
 <20220812161003.27279-12-kvalo@kernel.org>
 <65a4750f-1798-a624-0a2e-96930f8ec816@quicinc.com>
 <CH0PR02MB82123C176B0E05156B66C9F1F6229@CH0PR02MB8212.namprd02.prod.outlook.com>
 <94e894a8-a262-959e-a6ab-869dcba9e0fa@quicinc.com>
In-Reply-To: <94e894a8-a262-959e-a6ab-869dcba9e0fa@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 10/13/2022 10:00 PM, Pradeep Kumar Chitrapu wrote:
>
>>> +                                     work_done =
>>> +                                     ath12k_dp_mon_process_ring(ab,
>>> + id, napi, budget,
>> descendant is not indented from the first line, making it difficult to see the code structure
>>
>>> +                                                                flag);
>>> +                                     budget -= work_done;
>>> +                                     tot_work_done += work_done;
>>> +
>>> +                                     if (budget <= 0)
>>> +                                             goto done;
>>> +                             }
>>> +                     }
>>> +             }
>>> +     }
>>> +
>>> +     if (ab->hw_params->ring_mask->tx_mon_dest[grp_id]) {
>> this block of code has the same issues as the RX block. in fact it seems that this block is identical to the block above other than the flag and the tx_mon_desc vs rx_mon_desc, so I'm curious if it could be refactored into a single function that could be used by both tx and rx instead of duplicating code
>
Hi Jeff

Thanks for your review..

Although, rx and tx mon rings processing blocks can be moved to common 
function, this causes lot of variables to be sent to function using pass 
by reference, hence avoiding this gerrit for tradeoff. Please let me 
know if this is ok?

I will address rest of the comments in next revision of patch.

Thanks

Pradeep