Re: [PATCH v2 4/4] firmware: arm_scmi: Create debugfs files for statistics

[Cristian Marussi]

On Wed, Jul 03, 2024 at 03:37:38PM +0100, Luke Parkin wrote:
> Create debugfs files for the statistics in the scmi_debug_stats struct
> 
> Signed-off-by: Luke Parkin <luke.parkin@xxxxxxx>

Missing ---

> v1->v2
> Only create stats pointer if stats are enabled
> Move stats debugfs creation into a seperate helper function
> ---
>  drivers/firmware/arm_scmi/driver.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 937546397cf2..10cd9a319ffb 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -2858,6 +2858,24 @@ static int scmi_device_request_notifier(struct notifier_block *nb,
>  	return NOTIFY_OK;
>  }
>  
> +static void scmi_debugfs_stats_setup(struct scmi_info *info,
> +				     struct dentry *trans)
> +{
> +	struct dentry *stats;
> +
> +	stats = debugfs_create_dir("stats", trans);
> +	debugfs_create_atomic_t("response_ok", 0400, stats,
> +				&info->stats.response_ok);
> +	debugfs_create_atomic_t("dlyd_response_ok", 0400, stats,
> +				&info->stats.dlyd_response_ok);
> +	debugfs_create_atomic_t("sent_ok", 0400, stats,
> +				&info->stats.sent_ok);
> +	debugfs_create_atomic_t("sent_fail", 0400, stats,
> +				&info->stats.sent_fail);
> +	debugfs_create_atomic_t("xfers_response_timeout", 0400, stats,
> +				&info->stats.xfers_response_timeout);
> +}
> +
>  static void scmi_debugfs_common_cleanup(void *d)
>  {
>  	struct scmi_debug_info *dbg = d;
> @@ -2924,6 +2942,9 @@ static struct scmi_debug_info *scmi_debugfs_common_setup(struct scmi_info *info)
>  	debugfs_create_u32("rx_max_msg", 0400, trans,
>  			   (u32 *)&info->rx_minfo.max_msg);
>  
> +	if (IS_ENABLED(CONFIG_ARM_SCMI_DEBUG_STATISTICS))
> +		scmi_debugfs_stats_setup(info, trans);
> +

Nothing to say here if not that more entries will need to be added as said.

Moreover, You could take the chance in V3 to add in this patch the
support to handle resetting each single counter (hint...this is a low hanging
fruit :D) and also to support something like:

	transports/stats/reset

which will be a WOnly booolean entry that will reset ALL the counters in
one go.

Thanks,
Cristian