On Tue, 2023-04-25 at 10:49 +0200, Lorenzo Bianconi wrote:
> > From: Peter Chiu <chui-hao.chiu@xxxxxxxxxxxx>
> >
> > 1. Mixed tx_byte calculation - need to handle both (non)binding
> > packets
> > when WED is enabled.
> > 2. PPDU based TxS can only reports MPDU counts whereas mac80211
> > requires
> > MSDU unit(NL80211_STA_INFO_TX_PACKETS).
> >
> > To solve above issues - switch to use TxS reporting for all tx_byte
> > when
> > WED is active and get MSDU Tx counts from WA statistic.
>
> It seems to me we are doining multiple logic tasks in this patch. Can
> you
> please split them on multiple patches?
>
OK.
> >
> > Note that mt7915 WA firmware only counts tx_packet for WED path, so
> > driver needs to take care of host path.
> >
> > Fixes: 43eaa3689507 ("wifi: mt76: add PPDU based TxS support for
> > WED device")
> > Co-developed-by: Ryder Lee <ryder.lee@xxxxxxxxxxxx>
> > Signed-off-by: Ryder Lee <ryder.lee@xxxxxxxxxxxx>
> > Signed-off-by: Peter Chiu <chui-hao.chiu@xxxxxxxxxxxx>
> > ---
> > drivers/net/wireless/mediatek/mt76/mt76.h | 2 +-
> > .../wireless/mediatek/mt76/mt76_connac_mac.c | 8 +-
> > .../wireless/mediatek/mt76/mt76_connac_mcu.h | 1 +
> > .../net/wireless/mediatek/mt76/mt7915/init.c | 6 ++
> > .../net/wireless/mediatek/mt76/mt7915/main.c | 6 +-
> > .../net/wireless/mediatek/mt76/mt7915/mcu.c | 74
> > +++++++++++++++++--
> > .../net/wireless/mediatek/mt76/mt7915/mmio.c | 30 +-------
> > .../wireless/mediatek/mt76/mt7915/mt7915.h | 1 +
> > drivers/net/wireless/mediatek/mt76/tx.c | 6 ++
> > 9 files changed, 91 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h
> > b/drivers/net/wireless/mediatek/mt76/mt76.h
> > index 6b07b8fafec2..0e9f4197213a 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> > @@ -277,7 +277,7 @@ struct mt76_sta_stats {
> > u64 tx_mcs[16]; /* mcs idx */
> > u64 tx_bytes;
> > /* WED TX */
> > - u32 tx_packets;
> > + u32 tx_packets; /* unit: MSDU */
> > u32 tx_retries;
> > u32 tx_failed;
> > /* WED RX */
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> > b/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> > index d39a3cc5e381..84985a989293 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
> > @@ -521,9 +521,9 @@ void mt76_connac2_mac_write_txwi(struct
> > mt76_dev *dev, __le32 *txwi,
> > q_idx = wmm_idx * MT76_CONNAC_MAX_WMM_SETS +
> > mt76_connac_lmac_mapping(skb_get_queue_mapping(
> > skb));
> >
> > - /* counting non-offloading skbs */
> > - wcid->stats.tx_bytes += skb->len;
> > - wcid->stats.tx_packets++;
> > + /* mt7915 WA only counts WED path */
> > + if (mtk_wed_device_active(&dev->mmio.wed) &&
> > is_mt7915(dev))
> > + wcid->stats.tx_packets++;
>
> I think this will crash mt7921 usb/sdio drivers. It is enough to swap
> the conditions since mt7915 supports mmio only mode.
>
OK.
> > }
> >
> > val = FIELD_PREP(MT_TXD0_TX_BYTES, skb->len + sz_txd) |
> > @@ -610,8 +610,6 @@ bool mt76_connac2_mac_fill_txs(struct mt76_dev
> > *dev, struct mt76_wcid *wcid,
> > stats->tx_bytes +=
> > le32_get_bits(txs_data[5],
> > MT_TXS5_MPDU_TX_BYTE) -
> > le32_get_bits(txs_data[7],
> > MT_TXS7_MPDU_RETRY_BYTE);
> > - stats->tx_packets +=
> > - le32_get_bits(txs_data[5],
> > MT_TXS5_MPDU_TX_CNT);
>
> I think this will break accounting for mt7921, right?
>
Why? I don't think mt7921 supports WED. It should not go into this
chunk.
> Regards,
> Lorenzo
>
