All of lore.kernel.org
 help / color / mirror / Atom feed
* mt76 tx status
@ 2016-01-31 22:24 Johannes Stezenbach
  2016-02-01 12:28 ` Felix Fietkau
  0 siblings, 1 reply; 2+ messages in thread
From: Johannes Stezenbach @ 2016-01-31 22:24 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless

Hi Felix,

I noticed in mt76x2_mac_poll_tx_status() you are reading
MT_TX_STAT_FIFO before reading MT_TX_STAT_FIFO_EXT,
however I have a hunch that reading MT_TX_STAT_FIFO clobbers
the corresponding MT_TX_STAT_FIFO_EXT since MT_TX_STAT_FIFO
has the valid bit, and both registers together seem to implement
reading a 64bit FIFO entry.  I.e. you need to read
MT_TX_STAT_FIFO_EXT first otherwise you get a mix from
two FIFO entries.

I played a bit with the vendor driver for the mt7610u
and confirmed that multiple reads from MT_TX_STAT_FIFO_EXT
return the same value until MT_TX_STAT_FIFO is read.
(Note the mt7610u vendor driver does not define FIFO_EXT_SUPPORT
but the FIFO seems to exist in hardware.  The mt7612u vendor driver
code reads the MT_TX_STAT_FIFO_EXT first.)


Johannes

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: mt76 tx status
  2016-01-31 22:24 mt76 tx status Johannes Stezenbach
@ 2016-02-01 12:28 ` Felix Fietkau
  0 siblings, 0 replies; 2+ messages in thread
From: Felix Fietkau @ 2016-02-01 12:28 UTC (permalink / raw)
  To: Johannes Stezenbach; +Cc: linux-wireless

On 2016-01-31 23:24, Johannes Stezenbach wrote:
> Hi Felix,
> 
> I noticed in mt76x2_mac_poll_tx_status() you are reading
> MT_TX_STAT_FIFO before reading MT_TX_STAT_FIFO_EXT,
> however I have a hunch that reading MT_TX_STAT_FIFO clobbers
> the corresponding MT_TX_STAT_FIFO_EXT since MT_TX_STAT_FIFO
> has the valid bit, and both registers together seem to implement
> reading a 64bit FIFO entry.  I.e. you need to read
> MT_TX_STAT_FIFO_EXT first otherwise you get a mix from
> two FIFO entries.
> 
> I played a bit with the vendor driver for the mt7610u
> and confirmed that multiple reads from MT_TX_STAT_FIFO_EXT
> return the same value until MT_TX_STAT_FIFO is read.
> (Note the mt7610u vendor driver does not define FIFO_EXT_SUPPORT
> but the FIFO seems to exist in hardware.  The mt7612u vendor driver
> code reads the MT_TX_STAT_FIFO_EXT first.)
Hi Johannes,

When I wrote that particular part of the code, I still had a lot of
other bugs regarding aggregation and rate probing. Back then, somehow I
got better results reading FIFO before FIFO_EXT.
I just did a few experiments to not only verify that reading FIFO
clobbers FIFO_EXT, but also to ensure that the data from reading
FIFO_EXT first really belongs to the value read from FIFO.

I tested this by checking if the MT_TXWI_PKTID_PROBE flag in the pktid
field (in FIFO_EXT) corresponds to a different rate being selected.

My test results pretty much confirm your findings, so I've committed and
pushed a fix.

Thanks,

- Felix

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-02-01 12:28 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-31 22:24 mt76 tx status Johannes Stezenbach
2016-02-01 12:28 ` Felix Fietkau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.