On Thu, 2017-05-04 at 13:13 +0000, Kalle Valo wrote:

quoted

quoted

This code intentionally checked if TX status was requested, and
if not then it doesn't go to the effort of building it.

What I'm finding puzzling is the fact that the only caller of
ieee80211_led_tx() is ieee80211_tx_status() and it seems like
drivers, such as ath10k, call this for each packet handled - but
I'm likely missing something.

Yes, many drivers do call it for each packet, and as such, this
deficiency was never noted.

quoted

quoted

As it is with your patch, it'll go and report the TX status
without any
TX status information - which is handled in
wcn36xx_dxe_tx_ack_ind()
for those frames needing it.

Right, it doesn't sound desired. However, during normal operation
I'm not seeing IEEE80211_TX_CTL_REQ_TX_STATUS being set and as such
ieee80211_led_tx() is never called.

So what's the conclusion? How do we get leds working?

Well, frankly, I never thought the TX LED was a super good idea - but
it had been supported by the original code IIRC, so never removed. Some
people like frantic blinking I guess ;-)

But I think the problem also applies to the throughput trigger thing,
so perhaps we need to stick some LED feedback calls into other places,
like _noskb() or provide an extra way to do it?

johannes