Hi Maulik,
On Thu, Oct 07, 2021 at 03:27:26PM +0530, Maulik Shah wrote:
> From: Mahesh Sivasubramanian <msivasub@xxxxxxxxxxxxxx>
>
> Let's add a driver to read the stats from remote processor and
> export to debugfs.
>
> The driver creates "qcom_sleep_stats" directory in debugfs and
> adds files for various low power mode available. Below is sample
> output with command
>
> cat /sys/kernel/debug/qcom_sleep_stats/ddr
> count = 0
> Last Entered At = 0
> Last Exited At = 0
> Accumulated Duration = 0
>
> Signed-off-by: Mahesh Sivasubramanian <msivasub@xxxxxxxxxxxxxx>
> Signed-off-by: Lina Iyer <ilina@xxxxxxxxxxxxxx>
> [mkshah: add subsystem sleep stats, create one file for each stat]
> Signed-off-by: Maulik Shah <mkshah@xxxxxxxxxxxxxx>
> ---
> drivers/soc/qcom/Kconfig | 10 ++
> drivers/soc/qcom/Makefile | 1 +
> drivers/soc/qcom/qcom_sleep_stats.c | 259 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 270 insertions(+)
> create mode 100644 drivers/soc/qcom/qcom_sleep_stats.c
>
> [...]
> +
> +static int qcom_subsystem_sleep_stats_show(struct seq_file *s, void *unused)
> +{
> + struct subsystem_data *subsystem = s->private;
> + struct sleep_stats *stat;
> +
> + /* Items are allocated lazily, so lookup pointer each time */
> + stat = qcom_smem_get(subsystem->pid, subsystem->smem_item, NULL);
> + if (IS_ERR(stat))
> + return -EIO;
> +
> [...]
> +
> +static void qcom_create_subsystem_stat_files(struct dentry *root)
> +{
> + const struct sleep_stats *stat;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(subsystems); 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);
This causes WARNINGs on MSM8996 and MSM8916:
[ 0.503054] ------------[ cut here ]------------
[ 0.503100] WARNING: CPU: 1 PID: 1 at drivers/soc/qcom/smem.c:587 qcom_smem_get+0x184/0x1b0
[ 0.503184] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.15.0-rc4+ #378
[ 0.503218] Hardware name: Xiaomi Mi Note 2 (DT)
[ 0.503278] pc : qcom_smem_get+0x184/0x1b0
[ 0.503307] lr : qcom_sleep_stats_probe+0xfc/0x20
[ 0.503875] Call trace:
[ 0.503896] qcom_smem_get+0x184/0x1b0
[ 0.503925] qcom_sleep_stats_probe+0xfc/0x270
AFAICT from downstream the smem subsystem information is only read in
the rpmh_master_stat.c driver, should this be specific to RPMh?
There is a rpm_master_stat.c too but that looks quite different,
so I guess the approach is different with RPM?
Two more (unrelated) issues here:
1. This will silently not register anything if SMEM probes after the
qcom-sleep-stats driver (qcom_smem_get() will return -EPROBE_DEFER)
and you will just skip registering the debugfs files.
2. In qcom_subsystem_sleep_stats_show() you say
/* Items are allocated lazily, so lookup pointer each time */
But, if the lookup fails here you don't register the debugfs file
at all. Does this work if the subsystem is started after this driver?
Thanks,
Stephan
