Re: [PATCH v2 2/2] wcn36xx: Implement tx_rate reporting

Bryan O'Donoghue [off-list ref] writes:

On 20/03/2022 13:21, Bryan O'Donoghue wrote:

quoted

On 18/03/2022 19:58, Edmond Gagnon wrote:

quoted

+    INIT_DELAYED_WORK(&wcn->get_stats_work, wcn36xx_get_stats_work);

Instead of forking a worker and polling we could add the relevant
SMD command to

static int wcn36xx_smd_tx_compl_ind(struct wcn36xx *wcn, void *buf,
size_t len)
{
     wcn36xx_smd_get_stats(wcn, 0xSomeMask);
}

That way we only ever ask for and report a new TX data rate when we
know a TX event - and hence a potential TX data-rate update - has
taken place.

---
bod

Thinking a bit more

- Do the SMD get_stats in the tx completion
  This might be a problem initiating another SMD transaction inside
  of an SMD callback. But is the most straight forward way to
  get the data while avoiding alot of needless polling.

- Schedule your worker from the TX completion
  Again you should only care about gathering the data when you know
  something has happened which necessitates gathering that data
  like TX completion

- Schedule your worker from the RX indication routine
  Seems not as logical as the first two but it might be easier
  to schedule the worker in the RX data handler

Either way, I do think you should only gather this data on an event,
not as a continuous poll.

I agree, a continuous poll is not a good idea as it affects power
consumption. What about struct ieee80211_ops::sta_statistics? AFAIK
that's called only when user space is requestings stats so the overhead
should be minimal.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches