In order to allow the dynamic creation of debugfs subsystem entries,
the notifier and the notifier_block need to be stored in a per-subsystem
fashion. For that we need some kind of state container, so add it and group
everything related to the probing device into it.
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/soc/qcom/qcom_stats.c | 39 +++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
index d6bfd1bbdc2a..fa30540b6583 100644
--- a/drivers/soc/qcom/qcom_stats.c
+++ b/drivers/soc/qcom/qcom_stats.c
@@ -68,6 +68,13 @@ struct appended_stats {
u32 reserved[3];
};
+struct qcom_stats_priv {
+ struct device dev;
+ struct stats_data *data;
+ struct dentry *root;
+ const struct stats_config *config;
+};
+
static void qcom_print_stats(struct seq_file *s, const struct sleep_stats *stat)
{
u64 accumulated = stat->accumulated;
@@ -121,10 +128,13 @@ static int qcom_soc_sleep_stats_show(struct seq_file *s, void *unused)
DEFINE_SHOW_ATTRIBUTE(qcom_soc_sleep_stats);
DEFINE_SHOW_ATTRIBUTE(qcom_subsystem_sleep_stats);
-static void qcom_create_soc_sleep_stat_files(struct dentry *root, void __iomem *reg,
- struct stats_data *d,
- const struct stats_config *config)
+static void qcom_create_soc_sleep_stat_files(struct qcom_stats_priv *stats,
+ void __iomem *reg)
{
+ struct dentry *root = stats->root;
+ struct stats_data *d = stats->data;
+ const struct stats_config *config = stats->config;
+
char stat_type[sizeof(u32) + 1] = {0};
size_t stats_offset = config->stats_offset;
u32 offset = 0, type;
@@ -167,10 +177,11 @@ static void qcom_create_soc_sleep_stat_files(struct dentry *root, void __iomem *
}
}
-static void qcom_create_subsystem_stat_files(struct dentry *root,
- const struct stats_config *config)
+static void qcom_create_subsystem_stat_files(struct qcom_stats_priv *stats)
{
const struct sleep_stats *stat;
+ const struct stats_config *config = stats->config;
+ struct dentry *root = stats->root;
int i;
if (!config->subsystem_stats_in_smem)
@@ -188,12 +199,17 @@ static void qcom_create_subsystem_stat_files(struct dentry *root,
static int qcom_stats_probe(struct platform_device *pdev)
{
+ struct qcom_stats_priv *stats = NULL;
void __iomem *reg;
struct dentry *root;
const struct stats_config *config;
struct stats_data *d;
int i;
+ stats = devm_kzalloc(&pdev->dev, sizeof(*stats), GFP_KERNEL);
+ if (!stats)
+ return -ENOMEM;
+
config = device_get_match_data(&pdev->dev);
if (!config)
return -ENODEV;
@@ -212,17 +228,22 @@ static int qcom_stats_probe(struct platform_device *pdev)
root = debugfs_create_dir("qcom_stats", NULL);
- qcom_create_subsystem_stat_files(root, config);
- qcom_create_soc_sleep_stat_files(root, reg, d, config);
+ stats->config = config;
+ stats->data = d;
+ stats->root = root;
+
+ qcom_create_subsystem_stat_files(stats);
+ qcom_create_soc_sleep_stat_files(stats, reg);
- platform_set_drvdata(pdev, root);
+ platform_set_drvdata(pdev, stats);
return 0;
}
static int qcom_stats_remove(struct platform_device *pdev)
{
- struct dentry *root = platform_get_drvdata(pdev);
+ struct qcom_stats_priv *stats = platform_get_drvdata(pdev);
+ struct dentry *root = stats->root;
debugfs_remove_recursive(root);
--
2.34.1
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 <[email protected]>
---
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,
+ (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
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 <[email protected]>
> ---
> 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
>
Hi,
Thanks for the patch.
> +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);
The subsystem->name passed should match the subsystem name already
registered with SSR.
The names in below table don't match as of now. for e.g. modem uses mpss
name.
static const struct subsystem_data subsystems[] = {
{ "modem", 605, 1 },
{ "wpss", 605, 13 },
{ "adsp", 606, 2 },
{ "cdsp", 607, 5 },
{ "slpi", 608, 3 },
{ "gpu", 609, 0 },
{ "display", 610, 0 },
{ "adsp_island", 613, 2 },
{ "slpi_island", 613, 3 },
};
struct subsystem_data {
const char *name;
u32 smem_item;
u32 pid;
+ const char *ssr_name;
};
Can you add one more entry in above struct with the 'ssr_name' for
subsystem.
For the adsp_island stat you can re-use the lpass subsystem notification
and create/destroy debugfs stats files for both adsp and adsp_island
stats, similarly for slpi_island stats comes from slpi.
The gpu and display don't have any pil/ssr and hence same notification
won't work for them, however today both of these are unused, so you may
want to remove them from above table.
Note that In downstream we have 'apps' stats as well in above for which
no registration with SSR is required. May be need to leave 'ssr_name'
uninitialized for apps/gpu/display and skip register for them.
Thanks,
Maulik