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: <8c3127db-85f8-c5ce-7939-7151060887e2@quicinc.com>
Date:   Thu, 13 Oct 2022 13:17:14 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.3
Subject: Re: [PATCH 11/50] wifi: ath12k: add dp.c
Content-Language: en-US
To:     Pradeep Kumar Chitrapu <quic_pradeepc@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>
From:   Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <94e894a8-a262-959e-a6ab-869dcba9e0fa@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 10/13/2022 9:30 AM, 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?

yes, that is ok. it is always a tradeoff and different folks will weigh 
the tradeoffs differently, and nobody is "right". As log as the code 
works :)