linux-kernel - Re: [PATCH v2 3/5] platform/x86:intel/pmc: Improve function to show substate header

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: <951682a6-66e7-cd1a-4798-c5b4f88c79ec@linux.intel.com>
Date: Mon, 30 Jun 2025 14:34:10 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Xi Pardee <xi.pardee@...ux.intel.com>
cc: irenic.rajneesh@...il.com, david.e.box@...ux.intel.com, 
    Hans de Goede <hdegoede@...hat.com>, platform-driver-x86@...r.kernel.org, 
    LKML <linux-kernel@...r.kernel.org>, linux-pm@...r.kernel.org
Subject: Re: [PATCH v2 3/5] platform/x86:intel/pmc: Improve function to show
 substate header

On Tue, 24 Jun 2025, Xi Pardee wrote:

> Refactor pmc_core_substate_req_header_show() to accept a new argument.
> This is a preparation patch to introduce a new way to show Low Power
> Mode substate requirement data for platforms starting from Panther
> Lake. Increased the size for the name column as the Low Power Mode
> requirement register name is longer in newer platforms.
> 
> Signed-off-by: Xi Pardee <xi.pardee@...ux.intel.com>
> ---
>  drivers/platform/x86/intel/pmc/core.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index a1dd80bdbd413..47cc5120e7dd6 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -828,17 +828,20 @@ static int pmc_core_substate_l_sts_regs_show(struct seq_file *s, void *unused)
>  }
>  DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_l_sts_regs);
>  
> -static void pmc_core_substate_req_header_show(struct seq_file *s, int pmc_index)
> +static void pmc_core_substate_req_header_show(struct seq_file *s, int pmc_index, char *name)
>  {
>  	struct pmc_dev *pmcdev = s->private;
>  	int mode;
>  
> -	seq_printf(s, "%30s |", "Element");
> +	seq_printf(s, "%40s |", "Element");
>  	pmc_for_each_mode(mode, pmcdev)
>  		seq_printf(s, " %9s |", pmc_lpm_modes[mode]);
>  
> -	seq_printf(s, " %9s |", "Status");
> -	seq_printf(s, " %11s |\n", "Live Status");
> +	if (!strcmp(name, "Status")) {

I'm not very happy passing this as string. Could we use an 
internal defines/enum instead for this differentiation?

> +		seq_printf(s, " %9s |", name);
> +		seq_printf(s, " %11s |\n", "Live Status");
> +	} else
> +		seq_printf(s, " %9s |\n", name);

Please always use braces symmetrically in both blocks.

>  }
>  
>  static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
> @@ -872,7 +875,7 @@ static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
>  			continue;
>  
>  		/* Display the header */
> -		pmc_core_substate_req_header_show(s, pmc_index);
> +		pmc_core_substate_req_header_show(s, pmc_index, "Status");
>  
>  		/* Loop over maps */
>  		for (mp = 0; mp < num_maps; mp++) {
> @@ -910,7 +913,7 @@ static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
>  				}
>  
>  				/* Display the element name in the first column */
> -				seq_printf(s, "pmc%d: %26s |", pmc_index, map[i].name);
> +				seq_printf(s, "pmc%d: %34s |", pmc_index, map[i].name);
>  
>  				/* Loop over the enabled states and display if required */
>  				pmc_for_each_mode(mode, pmcdev) {
> 

-- 
 i.