[PATCH v4 4/5] platform/x86/amd: pmc: use FCH_PM_BASE definition

Mario Limonciello posted 5 patches 8 months, 1 week ago
There is a newer version of this series
[PATCH v4 4/5] platform/x86/amd: pmc: use FCH_PM_BASE definition
Posted by Mario Limonciello 8 months, 1 week ago
From: Mario Limonciello <mario.limonciello@amd.com>

The s2idle mmio quirk uses a scratch register in the FCH.
Adjust the code to clarify that.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v4:
 * Use fch.h instead
---
 arch/x86/include/asm/amd/fch.h            | 1 +
 drivers/platform/x86/amd/pmc/pmc-quirks.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/amd/fch.h b/arch/x86/include/asm/amd/fch.h
index a5fd91ff92df3..9b32e8a03193e 100644
--- a/arch/x86/include/asm/amd/fch.h
+++ b/arch/x86/include/asm/amd/fch.h
@@ -8,5 +8,6 @@
 /* register offsets from PM base */
 #define FCH_PM_DECODEEN			0x00
 #define FCH_PM_DECODEEN_SMBUS0SEL	GENMASK(20, 19)
+#define FCH_PM_SCRATCH			0x80
 
 #endif
diff --git a/drivers/platform/x86/amd/pmc/pmc-quirks.c b/drivers/platform/x86/amd/pmc/pmc-quirks.c
index b4f49720c87f6..3c680d2029f62 100644
--- a/drivers/platform/x86/amd/pmc/pmc-quirks.c
+++ b/drivers/platform/x86/amd/pmc/pmc-quirks.c
@@ -8,6 +8,7 @@
  * Author: Mario Limonciello <mario.limonciello@amd.com>
  */
 
+#include <asm/amd/fch.h>
 #include <linux/dmi.h>
 #include <linux/io.h>
 #include <linux/ioport.h>
@@ -20,7 +21,7 @@ struct quirk_entry {
 };
 
 static struct quirk_entry quirk_s2idle_bug = {
-	.s2idle_bug_mmio = 0xfed80380,
+	.s2idle_bug_mmio = FCH_PM_BASE + FCH_PM_SCRATCH,
 };
 
 static struct quirk_entry quirk_spurious_8042 = {
-- 
2.43.0
Re: [PATCH v4 4/5] platform/x86/amd: pmc: use FCH_PM_BASE definition
Posted by Yazen Ghannam 8 months, 1 week ago
On Mon, Apr 14, 2025 at 07:26:57PM -0500, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> The s2idle mmio quirk uses a scratch register in the FCH.
> Adjust the code to clarify that.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v4:
>  * Use fch.h instead
> ---
>  arch/x86/include/asm/amd/fch.h            | 1 +
>  drivers/platform/x86/amd/pmc/pmc-quirks.c | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/amd/fch.h b/arch/x86/include/asm/amd/fch.h
> index a5fd91ff92df3..9b32e8a03193e 100644
> --- a/arch/x86/include/asm/amd/fch.h
> +++ b/arch/x86/include/asm/amd/fch.h
> @@ -8,5 +8,6 @@
>  /* register offsets from PM base */
>  #define FCH_PM_DECODEEN			0x00
>  #define FCH_PM_DECODEEN_SMBUS0SEL	GENMASK(20, 19)
> +#define FCH_PM_SCRATCH			0x80
>  
>  #endif
> diff --git a/drivers/platform/x86/amd/pmc/pmc-quirks.c b/drivers/platform/x86/amd/pmc/pmc-quirks.c
> index b4f49720c87f6..3c680d2029f62 100644
> --- a/drivers/platform/x86/amd/pmc/pmc-quirks.c
> +++ b/drivers/platform/x86/amd/pmc/pmc-quirks.c
> @@ -8,6 +8,7 @@
>   * Author: Mario Limonciello <mario.limonciello@amd.com>
>   */
>  
> +#include <asm/amd/fch.h>

Arch headers should go after linux headers, I think.

So that arch stuff can override generic stuff.

>  #include <linux/dmi.h>
>  #include <linux/io.h>
>  #include <linux/ioport.h>

Thanks,
Yazen
Re: [PATCH v4 4/5] platform/x86/amd: pmc: use FCH_PM_BASE definition
Posted by Ingo Molnar 8 months, 1 week ago
* Yazen Ghannam <yazen.ghannam@amd.com> wrote:

> On Mon, Apr 14, 2025 at 07:26:57PM -0500, Mario Limonciello wrote:
> > From: Mario Limonciello <mario.limonciello@amd.com>
> > 
> > The s2idle mmio quirk uses a scratch register in the FCH.
> > Adjust the code to clarify that.
> > 
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > ---
> > v4:
> >  * Use fch.h instead
> > ---
> >  arch/x86/include/asm/amd/fch.h            | 1 +
> >  drivers/platform/x86/amd/pmc/pmc-quirks.c | 3 ++-
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/amd/fch.h b/arch/x86/include/asm/amd/fch.h
> > index a5fd91ff92df3..9b32e8a03193e 100644
> > --- a/arch/x86/include/asm/amd/fch.h
> > +++ b/arch/x86/include/asm/amd/fch.h
> > @@ -8,5 +8,6 @@
> >  /* register offsets from PM base */
> >  #define FCH_PM_DECODEEN			0x00
> >  #define FCH_PM_DECODEEN_SMBUS0SEL	GENMASK(20, 19)
> > +#define FCH_PM_SCRATCH			0x80
> >  
> >  #endif
> > diff --git a/drivers/platform/x86/amd/pmc/pmc-quirks.c b/drivers/platform/x86/amd/pmc/pmc-quirks.c
> > index b4f49720c87f6..3c680d2029f62 100644
> > --- a/drivers/platform/x86/amd/pmc/pmc-quirks.c
> > +++ b/drivers/platform/x86/amd/pmc/pmc-quirks.c
> > @@ -8,6 +8,7 @@
> >   * Author: Mario Limonciello <mario.limonciello@amd.com>
> >   */
> >  
> > +#include <asm/amd/fch.h>
> 
> Arch headers should go after linux headers, I think.

That's true, but it's a mostly stylistic requirement these days.

> So that arch stuff can override generic stuff.

Arch headers that override generic stuff are very much supposed to be 
able to build stand-alone and in pretty much any order with other 
headers, with very few exceptions. Ordering dependencies are very much 
frowned upon, because if they don't trigger build failures they can 
result in subtle breakages.

Thanks,

	Ingo
Re: [PATCH v4 4/5] platform/x86/amd: pmc: use FCH_PM_BASE definition
Posted by Yazen Ghannam 8 months, 1 week ago
On Wed, Apr 16, 2025 at 09:58:20AM +0200, Ingo Molnar wrote:
> 
> * Yazen Ghannam <yazen.ghannam@amd.com> wrote:
> 
> > On Mon, Apr 14, 2025 at 07:26:57PM -0500, Mario Limonciello wrote:
> > > From: Mario Limonciello <mario.limonciello@amd.com>
> > > 
> > > The s2idle mmio quirk uses a scratch register in the FCH.
> > > Adjust the code to clarify that.
> > > 
> > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > ---
> > > v4:
> > >  * Use fch.h instead
> > > ---
> > >  arch/x86/include/asm/amd/fch.h            | 1 +
> > >  drivers/platform/x86/amd/pmc/pmc-quirks.c | 3 ++-
> > >  2 files changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/x86/include/asm/amd/fch.h b/arch/x86/include/asm/amd/fch.h
> > > index a5fd91ff92df3..9b32e8a03193e 100644
> > > --- a/arch/x86/include/asm/amd/fch.h
> > > +++ b/arch/x86/include/asm/amd/fch.h
> > > @@ -8,5 +8,6 @@
> > >  /* register offsets from PM base */
> > >  #define FCH_PM_DECODEEN			0x00
> > >  #define FCH_PM_DECODEEN_SMBUS0SEL	GENMASK(20, 19)
> > > +#define FCH_PM_SCRATCH			0x80
> > >  
> > >  #endif
> > > diff --git a/drivers/platform/x86/amd/pmc/pmc-quirks.c b/drivers/platform/x86/amd/pmc/pmc-quirks.c
> > > index b4f49720c87f6..3c680d2029f62 100644
> > > --- a/drivers/platform/x86/amd/pmc/pmc-quirks.c
> > > +++ b/drivers/platform/x86/amd/pmc/pmc-quirks.c
> > > @@ -8,6 +8,7 @@
> > >   * Author: Mario Limonciello <mario.limonciello@amd.com>
> > >   */
> > >  
> > > +#include <asm/amd/fch.h>
> > 
> > Arch headers should go after linux headers, I think.
> 
> That's true, but it's a mostly stylistic requirement these days.
> 
> > So that arch stuff can override generic stuff.
> 
> Arch headers that override generic stuff are very much supposed to be 
> able to build stand-alone and in pretty much any order with other 
> headers, with very few exceptions. Ordering dependencies are very much 
> frowned upon, because if they don't trigger build failures they can 
> result in subtle breakages.
> 

Ah okay, thanks for clarifying!

-Yazen
Re: [PATCH v4 4/5] platform/x86/amd: pmc: use FCH_PM_BASE definition
Posted by Mario Limonciello 8 months, 1 week ago
On 4/15/2025 10:13 AM, Yazen Ghannam wrote:
> On Mon, Apr 14, 2025 at 07:26:57PM -0500, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> The s2idle mmio quirk uses a scratch register in the FCH.
>> Adjust the code to clarify that.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> v4:
>>   * Use fch.h instead
>> ---
>>   arch/x86/include/asm/amd/fch.h            | 1 +
>>   drivers/platform/x86/amd/pmc/pmc-quirks.c | 3 ++-
>>   2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/amd/fch.h b/arch/x86/include/asm/amd/fch.h
>> index a5fd91ff92df3..9b32e8a03193e 100644
>> --- a/arch/x86/include/asm/amd/fch.h
>> +++ b/arch/x86/include/asm/amd/fch.h
>> @@ -8,5 +8,6 @@
>>   /* register offsets from PM base */
>>   #define FCH_PM_DECODEEN			0x00
>>   #define FCH_PM_DECODEEN_SMBUS0SEL	GENMASK(20, 19)
>> +#define FCH_PM_SCRATCH			0x80
>>   
>>   #endif
>> diff --git a/drivers/platform/x86/amd/pmc/pmc-quirks.c b/drivers/platform/x86/amd/pmc/pmc-quirks.c
>> index b4f49720c87f6..3c680d2029f62 100644
>> --- a/drivers/platform/x86/amd/pmc/pmc-quirks.c
>> +++ b/drivers/platform/x86/amd/pmc/pmc-quirks.c
>> @@ -8,6 +8,7 @@
>>    * Author: Mario Limonciello <mario.limonciello@amd.com>
>>    */
>>   
>> +#include <asm/amd/fch.h>
> 
> Arch headers should go after linux headers, I think.
> 
> So that arch stuff can override generic stuff.
> 

Thanks - that's the same problem for the previous patch (i2c: piix4: 
Move SB800_PIIX4_FCH_PM_ADDR definition to amd/fch.) too then.

I'll fix it in the next spin.

>>   #include <linux/dmi.h>
>>   #include <linux/io.h>
>>   #include <linux/ioport.h>
> 
> Thanks,
> Yazen