Re: [PATCH 11/50] wifi: ath12k: add dp.c

Jeff Johnson
2022-10-13T20:17:27Z

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 :)