[2/2] watchdog: sp5100_tco: Enable watchdog on Family 17h devices if disabled

Message ID 20200910163109.235136-2-linux@roeck-us.net
State New
Headers show
Series
  • Untitled series #52836
Related show

Commit Message

Guenter Roeck Sept. 10, 2020, 4:31 p.m.
On Family 17h (Ryzen) devices, the WatchdogTmrEn bit of PmDecodeEn not only
enables watchdog memory decoding at 0xfeb00000, it also enables the
watchdog hardware itself. Use this information to enable the watchdog if
it is not already enabled.

Cc: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/sp5100_tco.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Jan Kiszka Sept. 10, 2020, 4:34 p.m. | #1
On 10.09.20 18:31, Guenter Roeck wrote:
> On Family 17h (Ryzen) devices, the WatchdogTmrEn bit of PmDecodeEn not only
> enables watchdog memory decoding at 0xfeb00000, it also enables the
> watchdog hardware itself. Use this information to enable the watchdog if
> it is not already enabled.
> 
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/watchdog/sp5100_tco.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
> index 85e9664318c9..a730ecbf78cd 100644
> --- a/drivers/watchdog/sp5100_tco.c
> +++ b/drivers/watchdog/sp5100_tco.c
> @@ -17,6 +17,12 @@
>   *	    AMD Publication 51192 "AMD Bolton FCH Register Reference Guide"
>   *	    AMD Publication 52740 "BIOS and Kernel Developer’s Guide (BKDG)
>   *				for AMD Family 16h Models 30h-3Fh Processors"
> + *	    AMD Publication 55570-B1-PUB "Processor Programming Reference (PPR)
> + *				for AMD Family 17h Model 18h, Revision B1
> + *				Processors (PUB)
> + *	    AMD Publication 55772-A1-PUB "Processor Programming Reference (PPR)
> + *				for AMD Family 17h Model 20h, Revision A1
> + *				Processors (PUB)
>   */
>  
>  /*
> @@ -241,6 +247,18 @@ static int sp5100_tco_setupdevice(struct device *dev,
>  		break;
>  	case efch:
>  		dev_name = SB800_DEVNAME;
> +		/*
> +		 * On Family 17h devices, the EFCH_PM_DECODEEN_WDT_TMREN bit of
> +		 * EFCH_PM_DECODEEN not only enables the EFCH_PM_WDT_ADDR memory
> +		 * region, it also enables the watchdog itself.
> +		 */
> +		if (boot_cpu_data.x86 == 0x17) {
> +			val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN);
> +			if (!(val & EFCH_PM_DECODEEN_WDT_TMREN)) {
> +				sp5100_tco_update_pm_reg8(EFCH_PM_DECODEEN, 0xff,
> +							  EFCH_PM_DECODEEN_WDT_TMREN);
> +			}
> +		}
>  		val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN);
>  		if (val & EFCH_PM_DECODEEN_WDT_TMREN)
>  			mmio_addr = EFCH_PM_WDT_ADDR;
> 

Won't that bring us EFCH_PM_WDT_ADDR as address, rather than
EFCH_PM_ACPI_MMIO_ADDR which worked in my case? Or is one an alias of
the other.

Jan
Guenter Roeck Sept. 10, 2020, 4:53 p.m. | #2
Hi Jan,

On 9/10/20 9:34 AM, Jan Kiszka wrote:
> On 10.09.20 18:31, Guenter Roeck wrote:
>> On Family 17h (Ryzen) devices, the WatchdogTmrEn bit of PmDecodeEn not only
>> enables watchdog memory decoding at 0xfeb00000, it also enables the
>> watchdog hardware itself. Use this information to enable the watchdog if
>> it is not already enabled.
>>
>> Cc: Jan Kiszka <jan.kiszka@siemens.com>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>  drivers/watchdog/sp5100_tco.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
>> index 85e9664318c9..a730ecbf78cd 100644
>> --- a/drivers/watchdog/sp5100_tco.c
>> +++ b/drivers/watchdog/sp5100_tco.c
>> @@ -17,6 +17,12 @@
>>   *	    AMD Publication 51192 "AMD Bolton FCH Register Reference Guide"
>>   *	    AMD Publication 52740 "BIOS and Kernel Developer’s Guide (BKDG)
>>   *				for AMD Family 16h Models 30h-3Fh Processors"
>> + *	    AMD Publication 55570-B1-PUB "Processor Programming Reference (PPR)
>> + *				for AMD Family 17h Model 18h, Revision B1
>> + *				Processors (PUB)
>> + *	    AMD Publication 55772-A1-PUB "Processor Programming Reference (PPR)
>> + *				for AMD Family 17h Model 20h, Revision A1
>> + *				Processors (PUB)
>>   */
>>  
>>  /*
>> @@ -241,6 +247,18 @@ static int sp5100_tco_setupdevice(struct device *dev,
>>  		break;
>>  	case efch:
>>  		dev_name = SB800_DEVNAME;
>> +		/*
>> +		 * On Family 17h devices, the EFCH_PM_DECODEEN_WDT_TMREN bit of
>> +		 * EFCH_PM_DECODEEN not only enables the EFCH_PM_WDT_ADDR memory
>> +		 * region, it also enables the watchdog itself.
>> +		 */
>> +		if (boot_cpu_data.x86 == 0x17) {
>> +			val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN);
>> +			if (!(val & EFCH_PM_DECODEEN_WDT_TMREN)) {
>> +				sp5100_tco_update_pm_reg8(EFCH_PM_DECODEEN, 0xff,
>> +							  EFCH_PM_DECODEEN_WDT_TMREN);
>> +			}
>> +		}
>>  		val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN);
>>  		if (val & EFCH_PM_DECODEEN_WDT_TMREN)
>>  			mmio_addr = EFCH_PM_WDT_ADDR;
>>
> 
> Won't that bring us EFCH_PM_WDT_ADDR as address, rather than
> EFCH_PM_ACPI_MMIO_ADDR which worked in my case? Or is one an alias of
> the other.
> 

Yes, it does use EFCH_PM_WDT_ADDR. EFCH_PM_ACPI_MMIO_ADDR works as well,
but is meant to be a fallback. Both point to the watchdog memory space.

Thanks,
Guenter

Patch

diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
index 85e9664318c9..a730ecbf78cd 100644
--- a/drivers/watchdog/sp5100_tco.c
+++ b/drivers/watchdog/sp5100_tco.c
@@ -17,6 +17,12 @@ 
  *	    AMD Publication 51192 "AMD Bolton FCH Register Reference Guide"
  *	    AMD Publication 52740 "BIOS and Kernel Developer’s Guide (BKDG)
  *				for AMD Family 16h Models 30h-3Fh Processors"
+ *	    AMD Publication 55570-B1-PUB "Processor Programming Reference (PPR)
+ *				for AMD Family 17h Model 18h, Revision B1
+ *				Processors (PUB)
+ *	    AMD Publication 55772-A1-PUB "Processor Programming Reference (PPR)
+ *				for AMD Family 17h Model 20h, Revision A1
+ *				Processors (PUB)
  */
 
 /*
@@ -241,6 +247,18 @@  static int sp5100_tco_setupdevice(struct device *dev,
 		break;
 	case efch:
 		dev_name = SB800_DEVNAME;
+		/*
+		 * On Family 17h devices, the EFCH_PM_DECODEEN_WDT_TMREN bit of
+		 * EFCH_PM_DECODEEN not only enables the EFCH_PM_WDT_ADDR memory
+		 * region, it also enables the watchdog itself.
+		 */
+		if (boot_cpu_data.x86 == 0x17) {
+			val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN);
+			if (!(val & EFCH_PM_DECODEEN_WDT_TMREN)) {
+				sp5100_tco_update_pm_reg8(EFCH_PM_DECODEEN, 0xff,
+							  EFCH_PM_DECODEEN_WDT_TMREN);
+			}
+		}
 		val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN);
 		if (val & EFCH_PM_DECODEEN_WDT_TMREN)
 			mmio_addr = EFCH_PM_WDT_ADDR;