Re: [RFC 2/2] soc: qcom_stats: Add dynamic debugfs entries for subsystems

From: Abel Vesa
Date: Thu Aug 18 2022 - 14:04:43 EST


On 22-08-18 17:22:15, Abel Vesa wrote:
> Register per-subsystem notifier using qcom_register_ssr_notifier().
> This will allow the order of between the remoteprocs actually starting and
> qcom_stats driver probing to become more relaxed.
>
> When the notifier callback gets called, depending on whether the remoteproc is
> starting up or shutting down, either create or remove the related debugfs
> entry. Also, in order to make sure we're not missing an already started
> remoteproc, after the notifier has been set up, we go though the subsystems
> list and try to create the entry for it, as it was doing before, but this time
> we store the dentry to use it later on for removal, if necessary.
>
> Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx>
> ---
> drivers/soc/qcom/qcom_stats.c | 77 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 75 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
> index fa30540b6583..baaa820c9a77 100644
> --- a/drivers/soc/qcom/qcom_stats.c
> +++ b/drivers/soc/qcom/qcom_stats.c
> @@ -7,8 +7,10 @@
> #include <linux/device.h>
> #include <linux/io.h>
> #include <linux/module.h>
> +#include <linux/notifier.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/remoteproc/qcom_rproc.h>
> #include <linux/seq_file.h>
>
> #include <linux/soc/qcom/smem.h>
> @@ -68,11 +70,20 @@ struct appended_stats {
> u32 reserved[3];
> };
>
> +struct subsystem_priv {
> + const struct subsystem_data *subsystem;
> + struct dentry *root;
> + struct dentry *dentry;
> + struct notifier_block nb;
> + void *notifier;
> +};
> +
> struct qcom_stats_priv {
> struct device dev;
> struct stats_data *data;
> struct dentry *root;
> const struct stats_config *config;
> + struct subsystem_priv ss_priv[ARRAY_SIZE(subsystems)];
> };
>
> static void qcom_print_stats(struct seq_file *s, const struct sleep_stats *stat)
> @@ -177,6 +188,57 @@ static void qcom_create_soc_sleep_stat_files(struct qcom_stats_priv *stats,
> }
> }
>
> +static int qcom_stats_subsys_ssr_notify(struct notifier_block *nb,
> + unsigned long action,
> + void *data)
> +{
> + struct subsystem_priv *ss_priv = container_of(nb, struct subsystem_priv, nb);
> +
> + switch (action) {
> + case QCOM_SSR_AFTER_POWERUP:
> + ss_priv->dentry = debugfs_create_file(ss_priv->subsystem->name, 0400, ss_priv->root,
> + (void *)ss_priv->subsystem,
> + &qcom_subsystem_sleep_stats_fops);
> + break;
> + case QCOM_SSR_BEFORE_SHUTDOWN:
> + debugfs_remove(ss_priv->dentry);
> + break;
> + default:
> + break;
> + }
> +
> + return NOTIFY_OK;
> +}
> +
> +static void qcom_register_subsystem_notifiers(struct qcom_stats_priv *stats)
> +{
> + struct device *dev = &stats->dev;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(subsystems); i++) {
> + const struct subsystem_data *subsystem = &subsystems[i];
> + struct subsystem_priv *ss_priv = &stats->ss_priv[i];
> +
> + ss_priv->subsystem = subsystem;
> + ss_priv->root = stats->root;
> + ss_priv->nb.notifier_call = qcom_stats_subsys_ssr_notify;
> + ss_priv->notifier = qcom_register_ssr_notifier(subsystem->name, &ss_priv->nb);
> + if (IS_ERR(ss_priv->notifier))
> + dev_err(dev, "failed to register ssr notifier for %s (%ld)",
> + subsystem->name, PTR_ERR(ss_priv->notifier));
> + }
> +}
> +
> +static void qcom_unregister_subsystem_notifiers(struct qcom_stats_priv *stats)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(subsystems); i++)
> + if (stats->ss_priv[i].notifier)
> + qcom_unregister_ssr_notifier(stats->ss_priv[i].notifier,
> + &stats->ss_priv[i].nb);
> +}
> +
> static void qcom_create_subsystem_stat_files(struct qcom_stats_priv *stats)
> {
> const struct sleep_stats *stat;
> @@ -188,12 +250,20 @@ static void qcom_create_subsystem_stat_files(struct qcom_stats_priv *stats)
> return;
>
> for (i = 0; i < ARRAY_SIZE(subsystems); i++) {
> + struct subsystem_priv *ss_priv = &stats->ss_priv[i];
> +
> stat = qcom_smem_get(subsystems[i].pid, subsystems[i].smem_item, NULL);
> if (IS_ERR(stat))
> continue;
>
> - debugfs_create_file(subsystems[i].name, 0400, root, (void *)&subsystems[i],
> - &qcom_subsystem_sleep_stats_fops);
> + /*
> + * At this point some subsystems have already started
> + * and so we already missed the startup notification,
> + * so let's create the entry post-startup.
> + */
> + ss_priv->dentry = debugfs_create_file(&subsystems[i]->name, 0400, root,

Please do not apply. There is a build issue here. Should be: subsystems[i].name

> + (void *)&subsystems[i],
> + &qcom_subsystem_sleep_stats_fops);
> }
> }
>
> @@ -232,6 +302,7 @@ static int qcom_stats_probe(struct platform_device *pdev)
> stats->data = d;
> stats->root = root;
>
> + qcom_register_subsystem_notifiers(stats);
> qcom_create_subsystem_stat_files(stats);
> qcom_create_soc_sleep_stat_files(stats, reg);
>
> @@ -245,6 +316,8 @@ static int qcom_stats_remove(struct platform_device *pdev)
> struct qcom_stats_priv *stats = platform_get_drvdata(pdev);
> struct dentry *root = stats->root;
>
> + qcom_unregister_subsystem_notifiers(stats);
> +
> debugfs_remove_recursive(root);
>
> return 0;
> --
> 2.34.1
>