| Message ID | 20241217095216.1543923-5-quic_rdevanat@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: ath12k: Support Sounding, Latency, Trigger, FSE stats | expand |
Roopni Devanathan <quic_rdevanat@quicinc.com> writes: > Currently, print_array_to_buf() is used to print arrays to HTT > stats buffer. This API supports printing only arrays with 32-bit > unsigned integers. Add API print_array_to_buf_s8(), which > implements print_array_to_buf()'s functionality to all the > arrays with 8-bit signed integers. > > Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-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: Roopni Devanathan <quic_rdevanat@quicinc.com> s/API/function/ And maybe not even use the term 'function' that, saying foo() should be clear anyone that it's a function. An example output would be nice. > --- > .../wireless/ath/ath12k/debugfs_htt_stats.c | 28 +++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c > index 4569ff40e9d8..621676daf971 100644 > --- a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c > +++ b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c > @@ -48,6 +48,34 @@ print_array_to_buf(u8 *buf, u32 offset, const char *header, > footer); > } > > +static u32 > +print_array_to_buf_s8(u8 *buf, u32 offset, const char *header, u32 stats_index, > + const s8 *array, u32 array_len, const char *footer) > +{ I would rather have a separate buf_len variable instead of silently assuming it's ATH12K_HTT_STATS_BUF_SIZE, much safer that way. > + int index = 0; > + u8 i; > + > + if (header) { > + index += scnprintf(buf + offset, > + ATH12K_HTT_STATS_BUF_SIZE - offset, > + "%s = ", header); > + } Empty line here. > + for (i = 0; i < array_len; i++) { > + index += scnprintf(buf + offset + index, > + (ATH12K_HTT_STATS_BUF_SIZE - offset) - index, > + " %u:%d,", stats_index++, array[i]); > + } Empty line. > + index--; > + *(buf + offset + index) = '\0'; Avoid pointer arithmetic: buf[offset + index] = '\0' But first check that 'offset + index < buf_len'. > + if (footer) { > + index += scnprintf(buf + offset + index, > + (ATH12K_HTT_STATS_BUF_SIZE - offset) - index, > + "%s", footer); > + } Empty line. > + return index; > +} > + > static const char *ath12k_htt_ax_tx_rx_ru_size_to_str(u8 ru_size) > { > switch (ru_size) {
On 12/19/2024 5:28 PM, Kalle Valo wrote: > Roopni Devanathan <quic_rdevanat@quicinc.com> writes: > >> Currently, print_array_to_buf() is used to print arrays to HTT >> stats buffer. This API supports printing only arrays with 32-bit >> unsigned integers. Add API print_array_to_buf_s8(), which >> implements print_array_to_buf()'s functionality to all the >> arrays with 8-bit signed integers. >> >> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-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: Roopni Devanathan <quic_rdevanat@quicinc.com> > > s/API/function/ > > And maybe not even use the term 'function' that, saying foo() should be > clear anyone that it's a function. > > An example output would be nice. Sure, Kalle. I'll modify the commit message in next version. > >> --- >> .../wireless/ath/ath12k/debugfs_htt_stats.c | 28 +++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> >> diff --git a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c >> index 4569ff40e9d8..621676daf971 100644 >> --- a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c >> +++ b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c >> @@ -48,6 +48,34 @@ print_array_to_buf(u8 *buf, u32 offset, const char *header, >> footer); >> } >> >> +static u32 >> +print_array_to_buf_s8(u8 *buf, u32 offset, const char *header, u32 stats_index, >> + const s8 *array, u32 array_len, const char *footer) >> +{ > > I would rather have a separate buf_len variable instead of silently > assuming it's ATH12K_HTT_STATS_BUF_SIZE, much safer that way. > Sure. I'll change this accordingly. >> + int index = 0; >> + u8 i; >> + >> + if (header) { >> + index += scnprintf(buf + offset, >> + ATH12K_HTT_STATS_BUF_SIZE - offset, >> + "%s = ", header); >> + } > > Empty line here. > >> + for (i = 0; i < array_len; i++) { >> + index += scnprintf(buf + offset + index, >> + (ATH12K_HTT_STATS_BUF_SIZE - offset) - index, >> + " %u:%d,", stats_index++, array[i]); >> + } > > Empty line. > >> + index--; >> + *(buf + offset + index) = '\0'; > > Avoid pointer arithmetic: > > buf[offset + index] = '\0' > > But first check that 'offset + index < buf_len'. Sure, I'll make changes in next version. > >> + if (footer) { >> + index += scnprintf(buf + offset + index, >> + (ATH12K_HTT_STATS_BUF_SIZE - offset) - index, >> + "%s", footer); >> + } > > Empty line. > I'll add empty line where it is necessary, in the next version. >> + return index; >> +} >> + >> static const char *ath12k_htt_ax_tx_rx_ru_size_to_str(u8 ru_size) >> { >> switch (ru_size) { >
diff --git a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c index 4569ff40e9d8..621676daf971 100644 --- a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c +++ b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c @@ -48,6 +48,34 @@ print_array_to_buf(u8 *buf, u32 offset, const char *header, footer); } +static u32 +print_array_to_buf_s8(u8 *buf, u32 offset, const char *header, u32 stats_index, + const s8 *array, u32 array_len, const char *footer) +{ + int index = 0; + u8 i; + + if (header) { + index += scnprintf(buf + offset, + ATH12K_HTT_STATS_BUF_SIZE - offset, + "%s = ", header); + } + for (i = 0; i < array_len; i++) { + index += scnprintf(buf + offset + index, + (ATH12K_HTT_STATS_BUF_SIZE - offset) - index, + " %u:%d,", stats_index++, array[i]); + } + index--; + *(buf + offset + index) = '\0'; + + if (footer) { + index += scnprintf(buf + offset + index, + (ATH12K_HTT_STATS_BUF_SIZE - offset) - index, + "%s", footer); + } + return index; +} + static const char *ath12k_htt_ax_tx_rx_ru_size_to_str(u8 ru_size) { switch (ru_size) {
Currently, print_array_to_buf() is used to print arrays to HTT stats buffer. This API supports printing only arrays with 32-bit unsigned integers. Add API print_array_to_buf_s8(), which implements print_array_to_buf()'s functionality to all the arrays with 8-bit signed integers. Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-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: Roopni Devanathan <quic_rdevanat@quicinc.com> --- .../wireless/ath/ath12k/debugfs_htt_stats.c | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+)