RE: [PATCH 1/1] platform/x86/amd: Fix pmc compile dependency errors.

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Borislav Petkov <bp@xxxxxxxxx>
> Sent: Friday, November 11, 2022 09:15
> To: Limonciello, Mario <Mario.Limonciello@xxxxxxx>
> Cc: Hans de Goede <hdegoede@xxxxxxxxxx>; Yupeng Li
> <liyupeng@xxxxxxxxxx>; S-k, Shyam-sundar <Shyam-sundar.S-
> k@xxxxxxx>; markgross@xxxxxxxxxx; platform-driver-x86@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; caizp2008@xxxxxxx; smf-
> linux@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/1] platform/x86/amd: Fix pmc compile dependency
> errors.
> 
> On Mon, Nov 07, 2022 at 06:27:52AM -0600, Mario Limonciello wrote:
> > On 11/7/22 04:28, Hans de Goede wrote:
> > > Hi,
> > >
> > > On 10/26/22 09:25, Yupeng Li wrote:
> > > > When disabled CONFIG_SUSPEND and CONFIG_DEBUG_FS,
> get_metrics_table
> > > > and amd_pmc_idlemask_read is defined under two conditions of this,
> > > > pmc build with implicit declaration of function error.Some build error
> > > > messages are as follows:
> > > >
> > > >   CC [M]  drivers/platform/x86/amd/pmc.o
> > > > drivers/platform/x86/amd/pmc.c: In function 'smu_fw_info_show':
> > > > drivers/platform/x86/amd/pmc.c:436:6: error: implicit declaration of
> function 'get_metrics_table' [-Werror=implicit-function-declaration]
> > > >    436 |  if (get_metrics_table(dev, &table))
> > > >        |      ^~~~~~~~~~~~~~~~~
> > > > drivers/platform/x86/amd/pmc.c: In function
> 'amd_pmc_idlemask_show':
> > > > drivers/platform/x86/amd/pmc.c:508:8: error: implicit declaration of
> function 'amd_pmc_idlemask_read'; did you mean
> 'amd_pmc_idlemask_show'? [-Werror=implicit-function-declaration]
> > > >    508 |   rc = amd_pmc_idlemask_read(dev, NULL, s);
> > > >        |        ^~~~~~~~~~~~~~~~~~~~~
> > > >        |        amd_pmc_idlemask_show
> > > > cc1: some warnings being treated as errors
> > > >
> > > > Signed-off-by: Yupeng Li <liyupeng@xxxxxxxxxx>
> > > > Reviewed-by: Caicai <caizp2008@xxxxxxx>
> > >
> > > Thank you for your patch, however I do not believe that this is the correct
> fix:
> > >
> > > > ---
> > > >   drivers/platform/x86/amd/pmc.c | 5 ++++-
> > > >   1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/platform/x86/amd/pmc.c
> b/drivers/platform/x86/amd/pmc.c
> > > > index ce859b300712..2b08039921b8 100644
> > > > --- a/drivers/platform/x86/amd/pmc.c
> > > > +++ b/drivers/platform/x86/amd/pmc.c
> > > > @@ -433,8 +433,10 @@ static int smu_fw_info_show(struct seq_file
> *s, void *unused)
> > > >   	struct smu_metrics table;
> > > >   	int idx;
> > > > +#if defined(CONFIG_SUSPEND) || defined(CONFIG_DEBUG_FS)
> > > >   	if (get_metrics_table(dev, &table))
> > > >   		return -EINVAL;
> > > > +#endif
> > > >   	seq_puts(s, "\n=== SMU Statistics ===\n");
> > > >   	seq_printf(s, "Table Version: %d\n", table.table_version);
> > >
> > > The table variable gets used here, but now it no longer has been
> initialized by
> > > get_metrics_table() so just #ifdef-ing out the get_metrics_table() call is
> wrong.
> > >
> > > > @@ -503,11 +505,12 @@ static int amd_pmc_idlemask_show(struct
> seq_file *s, void *unused)
> > > >   		if (rc)
> > > >   			return rc;
> > > >   	}
> > > > -
> > > >   	if (dev->major > 56 || (dev->major >= 55 && dev->minor >= 37)) {
> > > > +#if defined(CONFIG_SUSPEND) || defined(CONFIG_DEBUG_FS)
> > > >   		rc = amd_pmc_idlemask_read(dev, NULL, s);
> > > >   		if (rc)
> > > >   			return rc;
> > > > +#endif
> > > >   	} else {
> > > >   		seq_puts(s, "Unsupported SMU version for Idlemask\n");
> > > >   	}
> > >
> > > Same thing except now nothing gets send to the struct seq_file *s,
> > > so this is wrong too.
> > >
> > > Shyam / Mario, can you take a look at the reported compile errors
> please?
> > >
> > > Regards,
> > >
> > > Hans
> > >
> >
> > What kernel is this?  Shyam removed CONFIG_DEBUGFS in b37fe34c83.  So
> is
> > this failure still happening in 6.1-rc4?
> 
> It managed to trickle into stable too:
> 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> illa.kernel.org%2Fshow_bug.cgi%3Fid%3D216679&amp;data=05%7C01%7Cm
> ario.limonciello%40amd.com%7C29820718c1334539d74e08dac3f782f2%7C3dd
> 8961fe4884e608e11a82d994e183d%7C0%7C0%7C638037765076195581%7CUn
> known%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
> Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=NcjgeDsIS%2
> BcW84%2FMcl9hKFNfL5Cw%2F2rYU8yXXsh2H5E%3D&amp;reserved=0
> 
> Adding bug reporter to Cc too.
> 
> From a quick glance, I'd drop all that ifdeffery and add empty stubs.
> IMNSVHO.
> 

I sent up a patch for it, but Hans hasn't applied it.
https://patchwork.kernel.org/project/platform-driver-x86/patch/20221108023323.19304-1-mario.limonciello@xxxxxxx/