Re: [PATCH 1/3] soc: qcom: qcom_stats: Add support to read DDR statistic

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 4/29/25 5:52 AM, Maulik Shah wrote:
> DDR statistic provide different DDR LPM and DDR frequency statistic.
> Add support to read from MSGRAM and display via debugfs.
> 
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/qcom_stats.c | 99 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 99 insertions(+)
> 
> diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
> index 5de99cf59b9fbe32c0580e371c3cc362dfabb895..ee11fb0919742454d40442112787c087ba8f6598 100644
> --- a/drivers/soc/qcom/qcom_stats.c
> +++ b/drivers/soc/qcom/qcom_stats.c
> @@ -1,8 +1,10 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * Copyright (c) 2011-2021, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2022-2025, Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/debugfs.h>
>  #include <linux/device.h>
>  #include <linux/io.h>
> @@ -24,6 +26,17 @@
>  #define ACCUMULATED_OFFSET	0x18
>  #define CLIENT_VOTES_OFFSET	0x20
>  
> +#define DDR_STATS_MAGIC_KEY		0xA1157A75
> +#define DDR_STATS_MAX_NUM_MODES		0x14

Numbers (count) make more sense in decimal

> +#define DDR_STATS_MAGIC_KEY_ADDR	0x0
> +#define DDR_STATS_NUM_MODES_ADDR	0x4
> +#define DDR_STATS_ENTRY_START_ADDR	0x8

[...]

> +static void qcom_ddr_stats_print(struct seq_file *s, struct ddr_stats_entry *data)
> +{
> +	u32 cp_idx, name;
> +
> +	/*
> +	 * DDR statistic have two different types of details encoded.
> +	 * (1) DDR LPM Stats
> +	 * (2) DDR Frequency Stats
> +	 *
> +	 * The name field have details like which type of DDR stat (bits 8:15)
> +	 * along with other details as explained below
> +	 *
> +	 * In case of DDR LPM stat, name field will be encoded as,
> +	 * Bits	 -  Meaning
> +	 * 0:7	 -  DDR LPM name, can be of 0xd4, 0xd3, 0x11 and 0xd0.
> +	 * 8:15	 -  0x0 (indicates its a LPM stat)
> +	 * 16:31 -  Unused
> +	 *
> +	 * In case of DDR FREQ stats, name field will be encoded as,
> +	 * Bits  -  Meaning
> +	 * 0:4   -  DDR Clock plan index (CP IDX)
> +	 * 5:7   -  Unused
> +	 * 8:15  -  0x1 (indicates its Freq stat)
> +	 * 16:31 -  Frequency value in Mhz
> +	 */
> +	name = DDR_STATS_TYPE(data->name);
> +	if (name == 0x0) {
> +		name = DDR_STATS_LPM_NAME(data->name);

I'm not super keen on reusing the 'name' variable, maybe turn the
outer if-condition to switch(DDR_STATS_TYPE(data->name))

> +		seq_printf(s, "DDR LPM Stat Name:0x%x\tcount:%u\tDuration (ticks):%llu\n",
> +			   name, data->count, data->duration);
> +	} else if (name == 0x1) {
> +		name = DDR_STATS_FREQ(data->name);
> +		if (!name || !data->count)
> +			return;
> +
> +		cp_idx = DDR_STATS_CP_IDX(data->name);
> +		seq_printf(s, "DDR Freq %uMhz:\tCP IDX:%u\tcount:%u\tDuration (ticks):%llu\n",
> +			   name, cp_idx, data->count, data->duration);
> +	}

> +}
> +
> +static int qcom_ddr_stats_show(struct seq_file *s, void *d)
> +{
> +	struct ddr_stats_entry data[DDR_STATS_MAX_NUM_MODES];
> +	void __iomem *reg = (void __iomem *)s->private;
> +	u32 entry_count;
> +	int i;
> +
> +	entry_count = readl_relaxed(reg + DDR_STATS_NUM_MODES_ADDR);
> +	if (entry_count > DDR_STATS_MAX_NUM_MODES)
> +		return 0;

-EINVAL

Konrad