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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <890b9e41-d5a2-4df3-bb21-71642b4e6410@oss.qualcomm.com>
Date: Tue, 20 May 2025 15:29:24 +0530
From: "Maulik Shah (mkshah)" <maulik.shah@....qualcomm.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>
Cc: linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        Doug Anderson <dianders@...omium.org>
Subject: Re: [PATCH 1/3] soc: qcom: qcom_stats: Add support to read DDR
 statistic


On 4/29/2025 4:04 PM, Konrad Dybcio wrote:
> On 4/29/25 5:52 AM, Maulik Shah wrote:
>> DDR statistic provide different DDR LPM and DDR frequency statistic.
>> +#define DDR_STATS_MAGIC_KEY		0xA1157A75
>> +#define DDR_STATS_MAX_NUM_MODES		0x14
> 
> Numbers (count) make more sense in decimal
> 

I will change to decimal.

>> +#define DDR_STATS_MAGIC_KEY_ADDR	0x0
>> +#define DDR_STATS_NUM_MODES_ADDR	0x4
>> +#define DDR_STATS_ENTRY_START_ADDR	0x8
> 
> [...]
> 

>> +	 */
>> +	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))

Okay. I will use switch-case.

> 

> 
>> +}
>> +
>> +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

I kept this return as success from details given in commit message of [1] 
which made the qcom_subsystem_sleep_stats_show() function return 0
in order to run command like below to collect the stats without interspersed errors
grep ^ /sys/kernel/debug/qcom_stats/*

The same may break if return error from ddr stats too.

[1] https://lore.kernel.org/r/20230119032329.2909383-1-swboyd@chromium.org 

Thanks,
Maulik