Re: [PATCH v5 4/4] Watchdog: sp5100_tco: Enable Family 17h+ CPUs

[Guenter Roeck]

On Wed, Feb 02, 2022 at 09:35:25AM -0600, Terry Bowman wrote:
> The driver currently uses a CPU family match of 17h to determine
> EFCH_PM_DECODEEN_WDT_TMREN register support. This family check will not
> support future AMD CPUs and instead will require driver updates to add
> support.
> 
> Remove the family 17h family check and add a check for SMBus PCI
> revision ID 0x51 or greater. The MMIO access method has been available
> since at least SMBus controllers using PCI revision 0x51. This revision
> check will support family 17h and future AMD processors including EFCH
> functionality without requiring driver changes.
> 
> Co-developed-by: Robert Richter <rrichter@xxxxxxx>
> Signed-off-by: Robert Richter <rrichter@xxxxxxx>
> Signed-off-by: Terry Bowman <terry.bowman@xxxxxxx>
> Tested-by: Jean Delvare <jdelvare@xxxxxxx>
> Reviewed-by: Jean Delvare <jdelvare@xxxxxxx>

Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>

> ---
>  drivers/watchdog/sp5100_tco.c | 16 ++++------------
>  drivers/watchdog/sp5100_tco.h |  1 +
>  2 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
> index e02399ea8730..86ffb58fbc85 100644
> --- a/drivers/watchdog/sp5100_tco.c
> +++ b/drivers/watchdog/sp5100_tco.c
> @@ -86,6 +86,10 @@ static enum tco_reg_layout tco_reg_layout(struct pci_dev *dev)
>  	    dev->device == PCI_DEVICE_ID_ATI_SBX00_SMBUS &&
>  	    dev->revision < 0x40) {
>  		return sp5100;
> +	} else if (dev->vendor == PCI_VENDOR_ID_AMD &&
> +	    sp5100_tco_pci->device == PCI_DEVICE_ID_AMD_KERNCZ_SMBUS &&
> +	    sp5100_tco_pci->revision >= AMD_ZEN_SMBUS_PCI_REV) {
> +		return efch_mmio;
>  	} else if (dev->vendor == PCI_VENDOR_ID_AMD &&
>  	    ((dev->device == PCI_DEVICE_ID_AMD_HUDSON2_SMBUS &&
>  	     dev->revision >= 0x41) ||
> @@ -459,18 +463,6 @@ 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;
> diff --git a/drivers/watchdog/sp5100_tco.h b/drivers/watchdog/sp5100_tco.h
> index 8ca1b215e3ce..6a0986d2c94b 100644
> --- a/drivers/watchdog/sp5100_tco.h
> +++ b/drivers/watchdog/sp5100_tco.h
> @@ -89,3 +89,4 @@
>  #define EFCH_PM_ACPI_MMIO_PM_ADDR	(EFCH_PM_ACPI_MMIO_ADDR +	\
>  					 EFCH_PM_ACPI_MMIO_PM_OFFSET)
>  #define EFCH_PM_ACPI_MMIO_PM_SIZE	8
> +#define AMD_ZEN_SMBUS_PCI_REV		0x51
> -- 
> 2.30.2
>