| Message ID | 20241005105207.3350790-1-quic_rdevanat@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Jeff Johnson |
| Headers | show |
| Series | [v2] wifi: ath12k: Support DMAC Reset Stats | expand |
On 10/5/2024 3:52 AM, Roopni Devanathan wrote: > From: Rajat Soni <quic_rajson@quicinc.com> > > Add support to request DMAC reset stats from firmware through HTT stats > type 45. These stats give debug SoC error stats such as reset count, reset > time, engage time and count, disengage time and count and destination > ring mask. > > Sample output: > ------------- > echo 45 > /sys/kernel/debug/ath12k/pci-0000\:06\:00.0/mac0/htt_stats_type > cat /sys/kernel/debug/ath12k/pci-0000\:06\:00.0/mac0/htt_stats > HTT_DMAC_RESET_STATS_TLV: > reset_count = 1 > reset_time_ms = 8036717648 > disengage_time_ms = 8036717648 > engage_time_ms = 8036717649 > disengage_count = 1 > engage_count = 1 > drain_dest_ring_mask = 0x0 > > Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00214-QCAHKSWPL_SILICONZ-1 > Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4 > > Signed-off-by: Rajat Soni <quic_rajson@quicinc.com> > Signed-off-by: Roopni Devanathan <quic_rdevanat@quicinc.com> > --- > v2: > - Updated dependencies. No change in code. > --- > Depends-on: > [PATCH v2] wifi: ath12k: Modify print_array_to_buf() to support arrays with 1-based semantics > Link: https://lore.kernel.org/all/20241004085915.1788951-1-quic_rdevanat@quicinc.com/ > > [PATCH v2 0/4] wifi: ath12k: Support Ring, SFM, Transmit MU, SelfGen stats, CCA stats > Link: https://lore.kernel.org/ath12k/20241005101816.3314728-1-quic_rdevanat@quicinc.com/ > > [PATCH v2] wifi: ath12k: Support Pdev OBSS Stats > Link: https://lore.kernel.org/ath12k/20241005104206.3327143-1-quic_rdevanat@quicinc.com/ > --- > --- > .../wireless/ath/ath12k/debugfs_htt_stats.c | 41 +++++++++++++++++++ > .../wireless/ath/ath12k/debugfs_htt_stats.h | 15 +++++++ > 2 files changed, 56 insertions(+) > > diff --git a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c > index d890679a3f16..2debb253185c 100644 > --- a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c > +++ b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c > @@ -2502,6 +2502,44 @@ ath12k_htt_print_pdev_obss_pd_stats_tlv(const void *tag_buf, u16 tag_len, > stats_req->buf_len = len; > } > > +static void > +ath12k_htt_print_dmac_reset_stats_tlv(const void *tag_buf, u16 tag_len, > + struct debug_htt_stats_req *stats_req) > +{ > + const struct ath12k_htt_dmac_reset_stats_tlv *htt_stats_buf = tag_buf; > + u8 *buf = stats_req->buf; > + u32 len = stats_req->buf_len; > + u32 buf_len = ATH12K_HTT_STATS_BUF_SIZE; > + unsigned long time; > + > + if (tag_len < sizeof(*htt_stats_buf)) > + return; > + > + len += scnprintf(buf + len, buf_len - len, "HTT_DMAC_RESET_STATS_TLV:\n"); > + len += scnprintf(buf + len, buf_len - len, "reset_count = %u\n", > + le32_to_cpu(htt_stats_buf->reset_count)); > + time = ((unsigned long)le32_to_cpu(htt_stats_buf->reset_time_hi_ms) << 32) | > + le32_to_cpu(htt_stats_buf->reset_time_lo_ms); The v1 version was flagged by the Intel 0-DAY CI Kernel Test Service: https://lore.kernel.org/all/202410040200.Mwb85JzQ-lkp@intel.com/ I believe you need to use u64 instead of unsigned long since unsigned long will only be 32 bits on a 32-bit system. Also suggest you refactor into a helper function to remove any ambiguity about the order of operations between (typecast) and <<, i.e.: /* untested */ static u64 ath12k_le32hilo_to_u64(__le32 hi, __le32 lo) { u64 hi64 = le32_to_cpu(hi); u64 lo64 = le32_to_cpu(lo); return (hi64 << 32) | lo64; }
On 10/7/2024 8:37 AM, Jeff Johnson wrote: > On 10/5/2024 3:52 AM, Roopni Devanathan wrote: >> From: Rajat Soni <quic_rajson@quicinc.com> >> >> Add support to request DMAC reset stats from firmware through HTT stats >> type 45. These stats give debug SoC error stats such as reset count, reset >> time, engage time and count, disengage time and count and destination >> ring mask. >> >> Sample output: >> ------------- >> echo 45 > /sys/kernel/debug/ath12k/pci-0000\:06\:00.0/mac0/htt_stats_type >> cat /sys/kernel/debug/ath12k/pci-0000\:06\:00.0/mac0/htt_stats >> HTT_DMAC_RESET_STATS_TLV: >> reset_count = 1 >> reset_time_ms = 8036717648 >> disengage_time_ms = 8036717648 >> engage_time_ms = 8036717649 >> disengage_count = 1 >> engage_count = 1 >> drain_dest_ring_mask = 0x0 >> >> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00214-QCAHKSWPL_SILICONZ-1 >> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4 >> >> Signed-off-by: Rajat Soni <quic_rajson@quicinc.com> >> Signed-off-by: Roopni Devanathan <quic_rdevanat@quicinc.com> >> --- >> v2: >> - Updated dependencies. No change in code. >> --- >> Depends-on: >> [PATCH v2] wifi: ath12k: Modify print_array_to_buf() to support arrays with 1-based semantics >> Link: https://lore.kernel.org/all/20241004085915.1788951-1-quic_rdevanat@quicinc.com/ >> >> [PATCH v2 0/4] wifi: ath12k: Support Ring, SFM, Transmit MU, SelfGen stats, CCA stats >> Link: https://lore.kernel.org/ath12k/20241005101816.3314728-1-quic_rdevanat@quicinc.com/ >> >> [PATCH v2] wifi: ath12k: Support Pdev OBSS Stats >> Link: https://lore.kernel.org/ath12k/20241005104206.3327143-1-quic_rdevanat@quicinc.com/ >> --- >> --- >> .../wireless/ath/ath12k/debugfs_htt_stats.c | 41 +++++++++++++++++++ >> .../wireless/ath/ath12k/debugfs_htt_stats.h | 15 +++++++ >> 2 files changed, 56 insertions(+) >> >> diff --git a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c >> index d890679a3f16..2debb253185c 100644 >> --- a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c >> +++ b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c >> @@ -2502,6 +2502,44 @@ ath12k_htt_print_pdev_obss_pd_stats_tlv(const void *tag_buf, u16 tag_len, >> stats_req->buf_len = len; >> } >> >> +static void >> +ath12k_htt_print_dmac_reset_stats_tlv(const void *tag_buf, u16 tag_len, >> + struct debug_htt_stats_req *stats_req) >> +{ >> + const struct ath12k_htt_dmac_reset_stats_tlv *htt_stats_buf = tag_buf; >> + u8 *buf = stats_req->buf; >> + u32 len = stats_req->buf_len; >> + u32 buf_len = ATH12K_HTT_STATS_BUF_SIZE; >> + unsigned long time; >> + >> + if (tag_len < sizeof(*htt_stats_buf)) >> + return; >> + >> + len += scnprintf(buf + len, buf_len - len, "HTT_DMAC_RESET_STATS_TLV:\n"); >> + len += scnprintf(buf + len, buf_len - len, "reset_count = %u\n", >> + le32_to_cpu(htt_stats_buf->reset_count)); >> + time = ((unsigned long)le32_to_cpu(htt_stats_buf->reset_time_hi_ms) << 32) | >> + le32_to_cpu(htt_stats_buf->reset_time_lo_ms); > > The v1 version was flagged by the Intel 0-DAY CI Kernel Test Service: > https://lore.kernel.org/all/202410040200.Mwb85JzQ-lkp@intel.com/ > > I believe you need to use u64 instead of unsigned long since unsigned long > will only be 32 bits on a 32-bit system. Also suggest you refactor into a > helper function to remove any ambiguity about the order of operations between > (typecast) and <<, i.e.: > > /* untested */ > static u64 ath12k_le32hilo_to_u64(__le32 hi, __le32 lo) > { > u64 hi64 = le32_to_cpu(hi); > u64 lo64 = le32_to_cpu(lo); > > return (hi64 << 32) | lo64; > } compile tested the following diff against your patch: diff --git a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c index 8dc158675e12..9c54c457af30 100644 --- a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c +++ b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c @@ -2542,6 +2542,13 @@ ath12k_htt_print_pdev_obss_pd_stats_tlv(const void *tag_buf, u16 tag_len, stats_req->buf_len = len; } +static u64 ath12k_le32hilo_to_u64(__le32 hi, __le32 lo) +{ + u64 hi64 = le32_to_cpu(hi); + u64 lo64 = le32_to_cpu(lo); + + return (hi64 << 32) | lo64; +} static void ath12k_htt_print_dmac_reset_stats_tlv(const void *tag_buf, u16 tag_len, struct debug_htt_stats_req *stats_req) @@ -2550,7 +2557,7 @@ ath12k_htt_print_dmac_reset_stats_tlv(const void *tag_buf, u16 tag_len, u8 *buf = stats_req->buf; u32 len = stats_req->buf_len; u32 buf_len = ATH12K_HTT_STATS_BUF_SIZE; - unsigned long time; + u64 time; if (tag_len < sizeof(*htt_stats_buf)) return; @@ -2558,17 +2565,17 @@ ath12k_htt_print_dmac_reset_stats_tlv(const void *tag_buf, u16 tag_len, len += scnprintf(buf + len, buf_len - len, "HTT_DMAC_RESET_STATS_TLV:\n"); len += scnprintf(buf + len, buf_len - len, "reset_count = %u\n", le32_to_cpu(htt_stats_buf->reset_count)); - time = ((unsigned long)le32_to_cpu(htt_stats_buf->reset_time_hi_ms) << 32) | - le32_to_cpu(htt_stats_buf->reset_time_lo_ms); - len += scnprintf(buf + len, buf_len - len, "reset_time_ms = %ld\n", time); + time = ath12k_le32hilo_to_u64(htt_stats_buf->reset_time_hi_ms, + htt_stats_buf->reset_time_lo_ms); + len += scnprintf(buf + len, buf_len - len, "reset_time_ms = %llu\n", time); - time = ((unsigned long)le32_to_cpu(htt_stats_buf->disengage_time_hi_ms) << 32) | - le32_to_cpu(htt_stats_buf->disengage_time_lo_ms); - len += scnprintf(buf + len, buf_len - len, "disengage_time_ms = %ld\n", time); + time = ath12k_le32hilo_to_u64(htt_stats_buf->disengage_time_hi_ms, + htt_stats_buf->disengage_time_lo_ms); + len += scnprintf(buf + len, buf_len - len, "disengage_time_ms = %llu\n", time); - time = ((unsigned long)le32_to_cpu(htt_stats_buf->engage_time_hi_ms) << 32) | - le32_to_cpu(htt_stats_buf->engage_time_lo_ms); - len += scnprintf(buf + len, buf_len - len, "engage_time_ms = %ld\n", time); + time = ath12k_le32hilo_to_u64(htt_stats_buf->engage_time_hi_ms, + htt_stats_buf->engage_time_lo_ms); + len += scnprintf(buf + len, buf_len - len, "engage_time_ms = %llu\n", time); len += scnprintf(buf + len, buf_len - len, "disengage_count = %u\n", le32_to_cpu(htt_stats_buf->disengage_count));
diff --git a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c index d890679a3f16..2debb253185c 100644 --- a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c +++ b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c @@ -2502,6 +2502,44 @@ ath12k_htt_print_pdev_obss_pd_stats_tlv(const void *tag_buf, u16 tag_len, stats_req->buf_len = len; } +static void +ath12k_htt_print_dmac_reset_stats_tlv(const void *tag_buf, u16 tag_len, + struct debug_htt_stats_req *stats_req) +{ + const struct ath12k_htt_dmac_reset_stats_tlv *htt_stats_buf = tag_buf; + u8 *buf = stats_req->buf; + u32 len = stats_req->buf_len; + u32 buf_len = ATH12K_HTT_STATS_BUF_SIZE; + unsigned long time; + + if (tag_len < sizeof(*htt_stats_buf)) + return; + + len += scnprintf(buf + len, buf_len - len, "HTT_DMAC_RESET_STATS_TLV:\n"); + len += scnprintf(buf + len, buf_len - len, "reset_count = %u\n", + le32_to_cpu(htt_stats_buf->reset_count)); + time = ((unsigned long)le32_to_cpu(htt_stats_buf->reset_time_hi_ms) << 32) | + le32_to_cpu(htt_stats_buf->reset_time_lo_ms); + len += scnprintf(buf + len, buf_len - len, "reset_time_ms = %ld\n", time); + + time = ((unsigned long)le32_to_cpu(htt_stats_buf->disengage_time_hi_ms) << 32) | + le32_to_cpu(htt_stats_buf->disengage_time_lo_ms); + len += scnprintf(buf + len, buf_len - len, "disengage_time_ms = %ld\n", time); + + time = ((unsigned long)le32_to_cpu(htt_stats_buf->engage_time_hi_ms) << 32) | + le32_to_cpu(htt_stats_buf->engage_time_lo_ms); + len += scnprintf(buf + len, buf_len - len, "engage_time_ms = %ld\n", time); + + len += scnprintf(buf + len, buf_len - len, "disengage_count = %u\n", + le32_to_cpu(htt_stats_buf->disengage_count)); + len += scnprintf(buf + len, buf_len - len, "engage_count = %u\n", + le32_to_cpu(htt_stats_buf->engage_count)); + len += scnprintf(buf + len, buf_len - len, "drain_dest_ring_mask = 0x%x\n\n", + le32_to_cpu(htt_stats_buf->drain_dest_ring_mask)); + + stats_req->buf_len = len; +} + static int ath12k_dbg_htt_ext_stats_parse(struct ath12k_base *ab, u16 tag, u16 len, const void *tag_buf, void *user_data) @@ -2675,6 +2713,9 @@ static int ath12k_dbg_htt_ext_stats_parse(struct ath12k_base *ab, case HTT_STATS_PDEV_OBSS_PD_TAG: ath12k_htt_print_pdev_obss_pd_stats_tlv(tag_buf, len, stats_req); break; + case HTT_STATS_DMAC_RESET_STATS_TAG: + ath12k_htt_print_dmac_reset_stats_tlv(tag_buf, len, stats_req); + break; default: break; } diff --git a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.h b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.h index 597334830d02..120615fbe853 100644 --- a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.h +++ b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.h @@ -135,6 +135,7 @@ enum ath12k_dbg_htt_ext_stats_type { ATH12K_DBG_HTT_EXT_STATS_PDEV_TX_MU = 17, ATH12K_DBG_HTT_EXT_STATS_PDEV_CCA_STATS = 19, ATH12K_DBG_HTT_EXT_STATS_PDEV_OBSS_PD_STATS = 23, + ATH12K_DBG_HTT_EXT_STATS_SOC_ERROR = 45, /* keep this last */ ATH12K_DBG_HTT_NUM_EXT_STATS, @@ -196,6 +197,7 @@ enum ath12k_dbg_htt_tlv_tag { HTT_STATS_TX_SELFGEN_BE_ERR_STATS_TAG = 137, HTT_STATS_TX_SELFGEN_BE_STATS_TAG = 138, HTT_STATS_TX_SELFGEN_BE_SCHED_STATUS_STATS_TAG = 139, + HTT_STATS_DMAC_RESET_STATS_TAG = 155, HTT_STATS_MAX_TAG, }; @@ -1048,4 +1050,17 @@ struct ath12k_htt_pdev_obss_pd_stats_tlv { __le32 num_sr_ppdu_abort_flush_cnt; } __packed; +struct ath12k_htt_dmac_reset_stats_tlv { + __le32 reset_count; + __le32 reset_time_lo_ms; + __le32 reset_time_hi_ms; + __le32 disengage_time_lo_ms; + __le32 disengage_time_hi_ms; + __le32 engage_time_lo_ms; + __le32 engage_time_hi_ms; + __le32 disengage_count; + __le32 engage_count; + __le32 drain_dest_ring_mask; +} __packed; + #endif