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

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

[Yupeng Li]

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@zbhlos.com>
Reviewed-by: Caicai <caizp2008@163.com>
---
 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);
@@ -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");
 	}
-- 
2.34.1

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

[Hans de Goede]

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@zbhlos.com>
> Reviewed-by: Caicai <caizp2008@163.com>

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

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

[Mario Limonciello]

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@zbhlos.com>
>> Reviewed-by: Caicai <caizp2008@163.com>
> 
> 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?

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

[Borislav Petkov]

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@zbhlos.com>
> > > Reviewed-by: Caicai <caizp2008@163.com>
> > 
> > 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://bugzilla.kernel.org/show_bug.cgi?id=216679

Adding bug reporter to Cc too.

From a quick glance, I'd drop all that ifdeffery and add empty stubs.
IMNSVHO.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Borislav Petkov <bp@alien8.de>
> Sent: Friday, November 11, 2022 09:15
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Hans de Goede <hdegoede@redhat.com>; Yupeng Li
> <liyupeng@zbhlos.com>; S-k, Shyam-sundar <Shyam-sundar.S-
> k@amd.com>; markgross@kernel.org; platform-driver-x86@vger.kernel.org;
> linux-kernel@vger.kernel.org; caizp2008@163.com; smf-
> linux@virginmedia.com
> 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@zbhlos.com>
> > > > Reviewed-by: Caicai <caizp2008@163.com>
> > >
> > > 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@amd.com/

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

[Borislav Petkov]

On Fri, Nov 11, 2022 at 03:16:10PM +0000, Limonciello, Mario wrote:
> 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@amd.com/

Yap, that fixes it.

So yeah, Hans, make sure to Cc: stable too when applying.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Hans de Goede]

Hi,

On 11/11/22 16:39, Borislav Petkov wrote:
> On Fri, Nov 11, 2022 at 03:16:10PM +0000, Limonciello, Mario wrote:
>> 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@amd.com/
> 
> Yap, that fixes it.
> 
> So yeah, Hans, make sure to Cc: stable too when applying.

Done. I'll send this out in my next fixes pull-req to Linus
before the end of this week.

Regards,

Hans