| Message ID | d7f29306df3495e183304825cd3159bf1ccb1d87.1683930235.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Felix Fietkau |
| Headers | show |
| Series | wifi: mt76: move mib_stats structure in mt76.h | expand |
On Sat, May 13, 2023 at 12:29:53AM +0200, Lorenzo Bianconi wrote: > mib_stats structure is shared by mostly all chipsets. Move it to shared > code. > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> Reviewed-by: Simon Horman <simon.horman@corigine.com>
On 5/12/23 15:29, Lorenzo Bianconi wrote: > mib_stats structure is shared by mostly all chipsets. Move it to shared > code. In case this thing hasn't been pushed upstream yet, then a suggestion: Create a struct mt76_mib_stats_common class that has the common counters, and then per-driver can have that struct as first member of its mib stats. I have added a lot of per-driver stats that never made it upstream and are not fully shared across different drivers. Thanks, Ben > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > drivers/net/wireless/mediatek/mt76/mt76.h | 63 +++++++++++++++++++ > .../wireless/mediatek/mt76/mt7915/debugfs.c | 4 +- > .../net/wireless/mediatek/mt76/mt7915/mac.c | 2 +- > .../net/wireless/mediatek/mt76/mt7915/main.c | 4 +- > .../wireless/mediatek/mt76/mt7915/mt7915.h | 63 +------------------ > 5 files changed, 69 insertions(+), 67 deletions(-) > > diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h > index 75b5c2c56a92..45bf1022a051 100644 > --- a/drivers/net/wireless/mediatek/mt76/mt76.h > +++ b/drivers/net/wireless/mediatek/mt76/mt76.h > @@ -857,6 +857,69 @@ struct mt76_dev { > }; > }; > > +/* per-phy stats. */ > +struct mt76_mib_stats { > + u32 ack_fail_cnt;
> On 5/12/23 15:29, Lorenzo Bianconi wrote: > > mib_stats structure is shared by mostly all chipsets. Move it to shared > > code. > > In case this thing hasn't been pushed upstream yet, then a suggestion: > > Create a struct mt76_mib_stats_common class that has the common counters, > and then per-driver can have that struct as first member of its mib stats. > > I have added a lot of per-driver stats that never made it upstream > and are not fully shared across different drivers. I would say mt76_mib_stats is quite a general name. I think we can do somothing like (if it is really necessary): struct mt7915_mib_stats { struct mt76_mib_stats mt76; ... }; Regards, Lorenzo > > Thanks, > Ben > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > --- > > drivers/net/wireless/mediatek/mt76/mt76.h | 63 +++++++++++++++++++ > > .../wireless/mediatek/mt76/mt7915/debugfs.c | 4 +- > > .../net/wireless/mediatek/mt76/mt7915/mac.c | 2 +- > > .../net/wireless/mediatek/mt76/mt7915/main.c | 4 +- > > .../wireless/mediatek/mt76/mt7915/mt7915.h | 63 +------------------ > > 5 files changed, 69 insertions(+), 67 deletions(-) > > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h > > index 75b5c2c56a92..45bf1022a051 100644 > > --- a/drivers/net/wireless/mediatek/mt76/mt76.h > > +++ b/drivers/net/wireless/mediatek/mt76/mt76.h > > @@ -857,6 +857,69 @@ struct mt76_dev { > > }; > > }; > > +/* per-phy stats. */ > > +struct mt76_mib_stats { > > + u32 ack_fail_cnt; > > -- > Ben Greear <greearb@candelatech.com> > Candela Technologies Inc http://www.candelatech.com > >
On 7/15/23 4:54 AM, Lorenzo Bianconi wrote: >> On 5/12/23 15:29, Lorenzo Bianconi wrote: >>> mib_stats structure is shared by mostly all chipsets. Move it to shared >>> code. >> >> In case this thing hasn't been pushed upstream yet, then a suggestion: >> >> Create a struct mt76_mib_stats_common class that has the common counters, >> and then per-driver can have that struct as first member of its mib stats. >> >> I have added a lot of per-driver stats that never made it upstream >> and are not fully shared across different drivers. > > I would say mt76_mib_stats is quite a general name. I think we can do somothing > like (if it is really necessary): > > struct mt7915_mib_stats { > struct mt76_mib_stats mt76; > ... > }; Maybe: struct mt7915_mib_stats { struct mt76_mib_stats common; ... }; I also noticed that Ryder added a few of my patches that extended the stats, so I think there are probably a lot of stats in that struct that are not set on the 7921 driver and probably older drivers too. To me, unset stats are confusing, so I'd prefer to put stats that are mostly common in a common struct, and have per-driver stats just found in the driver itself. Thanks, Ben
On Sat, 2023-07-15 at 17:00 -0700, Ben Greear wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On 7/15/23 4:54 AM, Lorenzo Bianconi wrote: > >> On 5/12/23 15:29, Lorenzo Bianconi wrote: > >>> mib_stats structure is shared by mostly all chipsets. Move it to > shared > >>> code. > >> > >> In case this thing hasn't been pushed upstream yet, then a > suggestion: > >> > >> Create a struct mt76_mib_stats_common class that has the common > counters, > >> and then per-driver can have that struct as first member of its > mib stats. > >> > >> I have added a lot of per-driver stats that never made it upstream > >> and are not fully shared across different drivers. > > > > I would say mt76_mib_stats is quite a general name. I think we can > do somothing > > like (if it is really necessary): > > > > struct mt7915_mib_stats { > > struct mt76_mib_stats mt76; > > ... > > }; > > Maybe: > > struct mt7915_mib_stats { > struct mt76_mib_stats common; > ... > }; > > I also noticed that Ryder added a few of my patches that extended the > stats, so I think there are probably a lot of stats in that struct > that are not set on the 7921 driver and probably older drivers too. > > To me, unset stats are confusing, so I'd prefer to put stats that are > mostly > common in a common struct, and have per-driver stats just found in > the driver > itself. > Myabe you can add a patch for per-driver stuff. I just extended the stat for mt7915 and lore also sent this patch for common struct, but ended up merging together. Ryder
diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h index 75b5c2c56a92..45bf1022a051 100644 --- a/drivers/net/wireless/mediatek/mt76/mt76.h +++ b/drivers/net/wireless/mediatek/mt76/mt76.h @@ -857,6 +857,69 @@ struct mt76_dev { }; }; +/* per-phy stats. */ +struct mt76_mib_stats { + u32 ack_fail_cnt; + u32 fcs_err_cnt; + u32 rts_cnt; + u32 rts_retries_cnt; + u32 ba_miss_cnt; + u32 tx_bf_cnt; + u32 tx_mu_bf_cnt; + u32 tx_mu_mpdu_cnt; + u32 tx_mu_acked_mpdu_cnt; + u32 tx_su_acked_mpdu_cnt; + u32 tx_bf_ibf_ppdu_cnt; + u32 tx_bf_ebf_ppdu_cnt; + + u32 tx_bf_rx_fb_all_cnt; + u32 tx_bf_rx_fb_eht_cnt; + u32 tx_bf_rx_fb_he_cnt; + u32 tx_bf_rx_fb_vht_cnt; + u32 tx_bf_rx_fb_ht_cnt; + + u32 tx_bf_rx_fb_bw; /* value of last sample, not cumulative */ + u32 tx_bf_rx_fb_nc_cnt; + u32 tx_bf_rx_fb_nr_cnt; + u32 tx_bf_fb_cpl_cnt; + u32 tx_bf_fb_trig_cnt; + + u32 tx_ampdu_cnt; + u32 tx_stop_q_empty_cnt; + u32 tx_mpdu_attempts_cnt; + u32 tx_mpdu_success_cnt; + u32 tx_pkt_ebf_cnt; + u32 tx_pkt_ibf_cnt; + + u32 tx_rwp_fail_cnt; + u32 tx_rwp_need_cnt; + + /* rx stats */ + u32 rx_fifo_full_cnt; + u32 channel_idle_cnt; + u32 primary_cca_busy_time; + u32 secondary_cca_busy_time; + u32 primary_energy_detect_time; + u32 cck_mdrdy_time; + u32 ofdm_mdrdy_time; + u32 green_mdrdy_time; + u32 rx_vector_mismatch_cnt; + u32 rx_delimiter_fail_cnt; + u32 rx_mrdy_cnt; + u32 rx_len_mismatch_cnt; + u32 rx_mpdu_cnt; + u32 rx_ampdu_cnt; + u32 rx_ampdu_bytes_cnt; + u32 rx_ampdu_valid_subframe_cnt; + u32 rx_ampdu_valid_subframe_bytes_cnt; + u32 rx_pfdrop_cnt; + u32 rx_vec_queue_overflow_drop_cnt; + u32 rx_ba_cnt; + + u32 tx_amsdu[8]; + u32 tx_amsdu_cnt; +}; + struct mt76_power_limits { s8 cck[4]; s8 ofdm[8]; diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c b/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c index 879884ead660..dd5c494efa5f 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c +++ b/drivers/net/wireless/mediatek/mt76/mt7915/debugfs.c @@ -719,10 +719,10 @@ mt7915_ampdu_stat_read_phy(struct mt7915_phy *phy, static void mt7915_txbf_stat_read_phy(struct mt7915_phy *phy, struct seq_file *s) { + struct mt76_mib_stats *mib = &phy->mib; static const char * const bw[] = { "BW20", "BW40", "BW80", "BW160" }; - struct mib_stats *mib = &phy->mib; /* Tx Beamformer monitor */ seq_puts(s, "\nTx Beamformer applied PPDU counts: "); @@ -768,7 +768,7 @@ mt7915_tx_stats_show(struct seq_file *file, void *data) { struct mt7915_phy *phy = file->private; struct mt7915_dev *dev = phy->dev; - struct mib_stats *mib = &phy->mib; + struct mt76_mib_stats *mib = &phy->mib; int i; mutex_lock(&dev->mt76.mutex); diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c index 7df8d95fc3fb..9f5f15e219ac 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c @@ -1747,8 +1747,8 @@ void mt7915_reset(struct mt7915_dev *dev) void mt7915_mac_update_stats(struct mt7915_phy *phy) { + struct mt76_mib_stats *mib = &phy->mib; struct mt7915_dev *dev = phy->dev; - struct mib_stats *mib = &phy->mib; int i, aggr0 = 0, aggr1, cnt; u8 band = phy->mt76->band_idx; u32 val; diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c b/drivers/net/wireless/mediatek/mt76/mt7915/main.c index 2ada2806de66..19973e7c639f 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c +++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c @@ -843,7 +843,7 @@ mt7915_get_stats(struct ieee80211_hw *hw, { struct mt7915_phy *phy = mt7915_hw_phy(hw); struct mt7915_dev *dev = mt7915_hw_dev(hw); - struct mib_stats *mib = &phy->mib; + struct mt76_mib_stats *mib = &phy->mib; mutex_lock(&dev->mt76.mutex); @@ -1327,11 +1327,11 @@ void mt7915_get_et_stats(struct ieee80211_hw *hw, struct mt7915_dev *dev = mt7915_hw_dev(hw); struct mt7915_phy *phy = mt7915_hw_phy(hw); struct mt7915_vif *mvif = (struct mt7915_vif *)vif->drv_priv; + struct mt76_mib_stats *mib = &phy->mib; struct mt76_ethtool_worker_info wi = { .data = data, .idx = mvif->mt76.idx, }; - struct mib_stats *mib = &phy->mib; /* See mt7915_ampdu_stat_read_phy, etc */ int i, ei = 0, stats_size; diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h b/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h index b3ead3530740..50ddf6d3c60a 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mt7915.h @@ -157,67 +157,6 @@ struct mt7915_vif { struct cfg80211_bitrate_mask bitrate_mask; }; -/* per-phy stats. */ -struct mib_stats { - u32 ack_fail_cnt; - u32 fcs_err_cnt; - u32 rts_cnt; - u32 rts_retries_cnt; - u32 ba_miss_cnt; - u32 tx_bf_cnt; - u32 tx_mu_mpdu_cnt; - u32 tx_mu_acked_mpdu_cnt; - u32 tx_su_acked_mpdu_cnt; - u32 tx_bf_ibf_ppdu_cnt; - u32 tx_bf_ebf_ppdu_cnt; - - u32 tx_bf_rx_fb_all_cnt; - u32 tx_bf_rx_fb_he_cnt; - u32 tx_bf_rx_fb_vht_cnt; - u32 tx_bf_rx_fb_ht_cnt; - - u32 tx_bf_rx_fb_bw; /* value of last sample, not cumulative */ - u32 tx_bf_rx_fb_nc_cnt; - u32 tx_bf_rx_fb_nr_cnt; - u32 tx_bf_fb_cpl_cnt; - u32 tx_bf_fb_trig_cnt; - - u32 tx_ampdu_cnt; - u32 tx_stop_q_empty_cnt; - u32 tx_mpdu_attempts_cnt; - u32 tx_mpdu_success_cnt; - u32 tx_pkt_ebf_cnt; - u32 tx_pkt_ibf_cnt; - - u32 tx_rwp_fail_cnt; - u32 tx_rwp_need_cnt; - - /* rx stats */ - u32 rx_fifo_full_cnt; - u32 channel_idle_cnt; - u32 primary_cca_busy_time; - u32 secondary_cca_busy_time; - u32 primary_energy_detect_time; - u32 cck_mdrdy_time; - u32 ofdm_mdrdy_time; - u32 green_mdrdy_time; - u32 rx_vector_mismatch_cnt; - u32 rx_delimiter_fail_cnt; - u32 rx_mrdy_cnt; - u32 rx_len_mismatch_cnt; - u32 rx_mpdu_cnt; - u32 rx_ampdu_cnt; - u32 rx_ampdu_bytes_cnt; - u32 rx_ampdu_valid_subframe_cnt; - u32 rx_ampdu_valid_subframe_bytes_cnt; - u32 rx_pfdrop_cnt; - u32 rx_vec_queue_overflow_drop_cnt; - u32 rx_ba_cnt; - - u32 tx_amsdu[8]; - u32 tx_amsdu_cnt; -}; - /* crash-dump */ struct mt7915_crash_data { guid_t guid; @@ -263,7 +202,7 @@ struct mt7915_phy { u32 rx_ampdu_ts; u32 ampdu_ref; - struct mib_stats mib; + struct mt76_mib_stats mib; struct mt76_channel_state state_ts; #ifdef CONFIG_NL80211_TESTMODE
mib_stats structure is shared by mostly all chipsets. Move it to shared code. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- drivers/net/wireless/mediatek/mt76/mt76.h | 63 +++++++++++++++++++ .../wireless/mediatek/mt76/mt7915/debugfs.c | 4 +- .../net/wireless/mediatek/mt76/mt7915/mac.c | 2 +- .../net/wireless/mediatek/mt76/mt7915/main.c | 4 +- .../wireless/mediatek/mt76/mt7915/mt7915.h | 63 +------------------ 5 files changed, 69 insertions(+), 67 deletions(-)