Re: [PATCH v3 3/6] platform/x86:intel/pmc: Improve function to show substate header

[Xi Pardee]

Hi Ilpo,

Thanks for the view.

On 8/28/2025 6:09 AM, Ilpo Järvinen wrote:
> On Fri, 15 Aug 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@xxxxxxxxxxxxxxx>
> > ---
> >   drivers/platform/x86/intel/pmc/core.c | 22 ++++++++++++++++------
> >   1 file changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> > index a1dd80bdbd413..cbfdcdc50ad21 100644
> > --- a/drivers/platform/x86/intel/pmc/core.c
> > +++ b/drivers/platform/x86/intel/pmc/core.c
> > @@ -11,6 +11,11 @@
> >   
> >   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >   
> > +enum header_type {
> > +	HEADER_STATUS,
> > +	HEADER_VALUE
> Please have the comma in any non-terminating entry.

Will add comma in next version.

Xi

> > +};
> > +
> >   #include <linux/bitfield.h>
> >   #include <linux/debugfs.h>
> >   #include <linux/delay.h>
> > @@ -828,17 +833,22 @@ 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,
> > +					      enum header_type type)
> >   {
> >   	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 (type == HEADER_STATUS) {
> > +		seq_printf(s, " %9s |", "Status");
> > +		seq_printf(s, " %11s |\n", "Live Status");
> > +	} else {
> > +		seq_printf(s, " %9s |\n", "Value");
> > +	}
> >   }
> >   
> >   static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
> > @@ -872,7 +882,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, HEADER_STATUS);
> >   
> >   		/* Loop over maps */
> >   		for (mp = 0; mp < num_maps; mp++) {
> > @@ -910,7 +920,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) {