On 19 September 2014 20:17, <greearb@xxxxxxxxxxxxxxx> wrote: > From: Ben Greear <greearb@xxxxxxxxxxxxxxx> > > Add support for reading firmware stats through the ethtool > API. This may be easier for applications to manipulate > compared to parsing a text based debugfs file. You also seem to be adding fw crash and reset countes. It's probably worth including this in the commit log. > +#ifdef CONFIG_ATH10K_DEBUGFS > +/* TODO: Would be nice to always support ethtool stats, would need to > + * move the stats storage out of ath10k_debug, or always have ath10k_debug > + * struct available.. > + */ I was actually wondering about moving ath10k_target_stats out of debug while doing some cleanups. Your patch can actually justify this kind of move. [...] > + data[i++] = fw_stats->hw_reaped; /* ppdu reaped */ > + data[i++] = 0; /* tx bytes */ > + data[i++] = fw_stats->htt_mpdus; > + data[i++] = 0; /* rx bytes */ > + data[i++] = fw_stats->ch_noise_floor; > + data[i++] = fw_stats->cycle_count; > + data[i++] = fw_stats->phy_err_count; > + data[i++] = fw_stats->rts_bad; > + data[i++] = fw_stats->rts_good; > + data[i++] = fw_stats->chan_tx_power; > + data[i++] = fw_stats->fcs_bad; > + data[i++] = fw_stats->no_beacons; > + data[i++] = fw_stats->mpdu_enqued; > + data[i++] = fw_stats->msdu_enqued; > + data[i++] = fw_stats->wmm_drop; > + data[i++] = fw_stats->local_enqued; > + data[i++] = fw_stats->local_freed; > + data[i++] = fw_stats->hw_queued; > + data[i++] = fw_stats->hw_reaped; > + data[i++] = fw_stats->underrun; > + data[i++] = fw_stats->tx_abort; > + data[i++] = fw_stats->mpdus_requed; > + data[i++] = fw_stats->tx_ko; > + data[i++] = fw_stats->data_rc; > + data[i++] = fw_stats->sw_retry_failure; > + data[i++] = fw_stats->illgl_rate_phy_err; > + data[i++] = fw_stats->pdev_cont_xretry; > + data[i++] = fw_stats->pdev_tx_timeout; > + data[i++] = fw_stats->txop_ovf; > + data[i++] = fw_stats->pdev_resets; > + data[i++] = fw_stats->mid_ppdu_route_change; > + data[i++] = fw_stats->status_rcvd; > + data[i++] = fw_stats->r0_frags; > + data[i++] = fw_stats->r1_frags; > + data[i++] = fw_stats->r2_frags; > + data[i++] = fw_stats->r3_frags; > + data[i++] = fw_stats->htt_msdus; > + data[i++] = fw_stats->htt_mpdus; > + data[i++] = fw_stats->loc_msdus; > + data[i++] = fw_stats->loc_mpdus; > + data[i++] = fw_stats->phy_errs; > + data[i++] = fw_stats->phy_err_drop; > + data[i++] = fw_stats->mpdu_errs; > + data[i++] = ar->fw_crash_counter; > + data[i++] = ar->fw_warm_reset_counter; > + data[i++] = ar->fw_cold_reset_counter; fw_stats should be protected by ar->data_lock. See ath10k_read_fw_stats(). You could in theory report inconsistent stats. > --- a/drivers/net/wireless/ath/ath10k/pci.c > +++ b/drivers/net/wireless/ath/ath10k/pci.c > @@ -986,6 +986,8 @@ static void ath10k_pci_fw_crashed_dump(struct ath10k *ar) > > spin_lock_bh(&ar->data_lock); > > + ar->fw_crash_counter++; > + > crash_data = ath10k_debug_get_new_fw_crash_data(ar); > > if (crash_data) > @@ -1671,6 +1673,7 @@ static int ath10k_pci_warm_reset(struct ath10k *ar) > u32 val; > > ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot warm reset\n"); > + ar->fw_warm_reset_counter++; > > /* debug */ > val = ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS + > @@ -2286,6 +2289,7 @@ static int ath10k_pci_cold_reset(struct ath10k *ar) > u32 val; > > ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot cold reset\n"); > + ar->fw_cold_reset_counter++; > > /* Put Target, including PCIe, into RESET. */ > val = ath10k_pci_reg_read32(ar, SOC_GLOBAL_RESET_ADDRESS); Unprotected? I guess it's not terribly wrong since this is the only place it updates the values. Michał -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
