[PATCH v5 0/4] Watchdog: sp5100_tco: Replace cd6h/cd7h port I/O accesses with MMIO accesses

[PATCH v5 0/4] Watchdog: sp5100_tco: Replace cd6h/cd7h port I/O accesses with MMIO accesses

[Terry Bowman]

This driver uses cd6h/cd7h port I/O to access the FCH::PM::DECODEEN
register during driver initialization. cd6h/cd7h port I/O is no longer
supported on later AMD processors and the recommended method to access
this register is using MMIO. This series will replace the cd6h/cd7h port
I/O with MMIO accesses during initialization.

The first patch refactors watchdog timer initialization into a separate
function. This is needed to add support for new device layouts without
adding complexity.

The second patch moves region request/release into new functions. The
request/release functions provide a location for adding MMIO region
support.

The third patch introduces EFCH initialization using MMIO. This is
required because the registers are no longer accessible using cd6h/cd7h
port I/O.

The fourth patch adds SMBus controller PCI ID check to enable EFCH MMIO
initialization. This eliminates the need for driver updates to support
future processors supporting the same EFCH functionality.

This series includes patches with MMIO accesses to register
FCH::PM::DECODEEN. The same register is also accessed by the piix4_smbus
driver. Both drivers use request_mem_region_muxed() to synchronize the
accesses. request_mem_region_muxed() definition is added in parallel
piix4_smbus patchset review with review URL provided below as a dependency.

Dependency:
Link: https://lore.kernel.org/linux-i2c/20220130184130.176646-2-terry.bowman@amd.com/

Based on v5.17-rc2

Testing:
Tested on AMD family 17h and family 19h processors using:

cat  >> /dev/watchdog

Hi Jean,
Please confirm to leave your reviewed-by and tested-by.

Changes in v5:
  - Updated dev_info() in sp5100_tco_prepare_base() to use physical
    address. Patch 2. (Guenter Roeck)
  - Rebased to v5.17-rc2 (Guenter Roeck)
  
Changes in V4:
  - Change to only call devm_ioremap() once. (Guenter Roeck, Jean Delvare)
  - Remove trailing dot for consistency with the other messages.
    (Jean Delvare)
  - Update print formatting in sp5100_tco_prepare_base(). Change period to
    a comma, use '0x%x', and change return code to decimal display.
    (Jean Delvare)
  - Move dev_err() linebreak to 'dev,' in sp5100_tco_prepare_base().
    (Jean Delvare)
  - Remove unused variable. (Andy Shevchenko)
  - Remove unnecessary assignment in sp5100_tco_prepare_base().
    (Andy Shevchenko)
  - Unify comment in sp5100_tco_prepare_base().  (Andy Shevchenko)
  - Fix line break for readability in 'if' in sp5100_tco_prepare_base().
    (Andy Shevchenko)
  - Fix logic issue in 'if' in sp5100_tco_setupdevice(). Added temp
    variable val. (Terry Bowman, Jean Delvare)    
  - Change capitalized letters to lowercase in sp5100_tco_prepare_base().
    (Andy Shevchenko)
  - Add dependency note for piix4_smbus driver. (Andy Shevchenko)
  - Change "SMB" -> "SMBus". (Jean Delvare)
  - Add comment for logic in sp5100_tco_setupdevice_mmio(). (Jean Delvare)
  - Fix 2 locations of line breaks in sp5100_tco_setupdevice_mmio().
    (Jean Delvare)
  
Changes in V3:
  - Remove 'addr' and 'res' variables from struct sp5100_tco.
    (Guenter Roeck)
  - Pass address directly to efch_read_pm_reg8() and
    efch_update_pm_reg8(). (Guenter Roeck)
  - Reword patch descriptions. (Terry Bowman)
  - Change #define AMD_ZEN_SMBUS_PCI_REV value from 0x59 to 0x51. This was
    determined after investigating programmers manual and testing.
    (Robert Richter)
  - Refactor efch_* functions() (Robert Richter)
  - Remove trailing whitespace in patch. (Guenter Roeck)

Changes in V2:
   - Refactor into 4 patch series
   - Move MMIO reservation and mapping into helper functions
   - Combine mmio_addr and alternate mmio_addr base address discovery
   - Replace efch_use_mmio() with efch_mmio layout type
   
Terry Bowman (4):
  Watchdog: sp5100_tco: Move timer initialization into function
  Watchdog: sp5100_tco: Refactor MMIO base address initialization
  Watchdog: sp5100_tco: Add initialization using EFCH MMIO
  Watchdog: sp5100_tco: Enable Family 17h+ CPUs

 drivers/watchdog/sp5100_tco.c | 334 ++++++++++++++++++++++------------
 drivers/watchdog/sp5100_tco.h |   7 +
 2 files changed, 226 insertions(+), 115 deletions(-)

-- 
2.30.2

[PATCH v5 1/4] Watchdog: sp5100_tco: Move timer initialization into function

[Terry Bowman]

Refactor driver's timer initialization into new function. This is needed
inorder to support adding new device layouts while using common timer
initialization.

Co-developed-by: Robert Richter <rrichter@amd.com>
Signed-off-by: Robert Richter <rrichter@amd.com>
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Tested-by: Jean Delvare <jdelvare@suse.de>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/watchdog/sp5100_tco.c | 65 +++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 29 deletions(-)

diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
index dd9a744f82f8..b365bbc9ac36 100644
--- a/drivers/watchdog/sp5100_tco.c
+++ b/drivers/watchdog/sp5100_tco.c
@@ -223,6 +223,41 @@ static u32 sp5100_tco_read_pm_reg32(u8 index)
 	return val;
 }
 
+static int sp5100_tco_timer_init(struct sp5100_tco *tco)
+{
+	struct watchdog_device *wdd = &tco->wdd;
+	struct device *dev = wdd->parent;
+	u32 val;
+
+	val = readl(SP5100_WDT_CONTROL(tco->tcobase));
+	if (val & SP5100_WDT_DISABLED) {
+		dev_err(dev, "Watchdog hardware is disabled\n");
+		return -ENODEV;
+	}
+
+	/*
+	 * Save WatchDogFired status, because WatchDogFired flag is
+	 * cleared here.
+	 */
+	if (val & SP5100_WDT_FIRED)
+		wdd->bootstatus = WDIOF_CARDRESET;
+
+	/* Set watchdog action to reset the system */
+	val &= ~SP5100_WDT_ACTION_RESET;
+	writel(val, SP5100_WDT_CONTROL(tco->tcobase));
+
+	/* Set a reasonable heartbeat before we stop the timer */
+	tco_timer_set_timeout(wdd, wdd->timeout);
+
+	/*
+	 * Stop the TCO before we change anything so we don't race with
+	 * a zeroed timer.
+	 */
+	tco_timer_stop(wdd);
+
+	return 0;
+}
+
 static int sp5100_tco_setupdevice(struct device *dev,
 				  struct watchdog_device *wdd)
 {
@@ -348,35 +383,7 @@ static int sp5100_tco_setupdevice(struct device *dev,
 	/* Setup the watchdog timer */
 	tco_timer_enable(tco);
 
-	val = readl(SP5100_WDT_CONTROL(tco->tcobase));
-	if (val & SP5100_WDT_DISABLED) {
-		dev_err(dev, "Watchdog hardware is disabled\n");
-		ret = -ENODEV;
-		goto unreg_region;
-	}
-
-	/*
-	 * Save WatchDogFired status, because WatchDogFired flag is
-	 * cleared here.
-	 */
-	if (val & SP5100_WDT_FIRED)
-		wdd->bootstatus = WDIOF_CARDRESET;
-	/* Set watchdog action to reset the system */
-	val &= ~SP5100_WDT_ACTION_RESET;
-	writel(val, SP5100_WDT_CONTROL(tco->tcobase));
-
-	/* Set a reasonable heartbeat before we stop the timer */
-	tco_timer_set_timeout(wdd, wdd->timeout);
-
-	/*
-	 * Stop the TCO before we change anything so we don't race with
-	 * a zeroed timer.
-	 */
-	tco_timer_stop(wdd);
-
-	release_region(SP5100_IO_PM_INDEX_REG, SP5100_PM_IOPORTS_SIZE);
-
-	return 0;
+	ret = sp5100_tco_timer_init(tco);
 
 unreg_region:
 	release_region(SP5100_IO_PM_INDEX_REG, SP5100_PM_IOPORTS_SIZE);
-- 
2.30.2

[PATCH v5 2/4] Watchdog: sp5100_tco: Refactor MMIO base address initialization

[Terry Bowman]

Combine MMIO base address and alternate base address detection. Combine
based on layout type. This will simplify the function by eliminating
a switch case.

Move existing request/release code into functions. This currently only
supports port I/O request/release. The move into a separate function
will make it ready for adding MMIO region support.

Co-developed-by: Robert Richter <rrichter@amd.com>
Signed-off-by: Robert Richter <rrichter@amd.com>
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Tested-by: Jean Delvare <jdelvare@suse.de>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/watchdog/sp5100_tco.c | 155 ++++++++++++++++++----------------
 drivers/watchdog/sp5100_tco.h |   1 +
 2 files changed, 82 insertions(+), 74 deletions(-)

diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
index b365bbc9ac36..8db7504f0aa4 100644
--- a/drivers/watchdog/sp5100_tco.c
+++ b/drivers/watchdog/sp5100_tco.c
@@ -223,6 +223,55 @@ static u32 sp5100_tco_read_pm_reg32(u8 index)
 	return val;
 }
 
+static u32 sp5100_tco_request_region(struct device *dev,
+				     u32 mmio_addr,
+				     const char *dev_name)
+{
+	if (!devm_request_mem_region(dev, mmio_addr, SP5100_WDT_MEM_MAP_SIZE,
+				     dev_name)) {
+		dev_dbg(dev, "MMIO address 0x%08x already in use\n", mmio_addr);
+		return 0;
+	}
+
+	return mmio_addr;
+}
+
+static u32 sp5100_tco_prepare_base(struct sp5100_tco *tco,
+				   u32 mmio_addr,
+				   u32 alt_mmio_addr,
+				   const char *dev_name)
+{
+	struct device *dev = tco->wdd.parent;
+
+	dev_dbg(dev, "Got 0x%08x from SBResource_MMIO register\n", mmio_addr);
+
+	if (!mmio_addr && !alt_mmio_addr)
+		return -ENODEV;
+
+	/* Check for MMIO address and alternate MMIO address conflicts */
+	if (mmio_addr)
+		mmio_addr = sp5100_tco_request_region(dev, mmio_addr, dev_name);
+
+	if (!mmio_addr && alt_mmio_addr)
+		mmio_addr = sp5100_tco_request_region(dev, alt_mmio_addr, dev_name);
+
+	if (!mmio_addr) {
+		dev_err(dev, "Failed to reserve MMIO or alternate MMIO region\n");
+		return -EBUSY;
+	}
+
+	tco->tcobase = devm_ioremap(dev, mmio_addr, SP5100_WDT_MEM_MAP_SIZE);
+	if (!tco->tcobase) {
+		dev_err(dev, "MMIO address 0x%08x failed mapping\n", mmio_addr);
+		devm_release_mem_region(dev, mmio_addr, SP5100_WDT_MEM_MAP_SIZE);
+		return -ENOMEM;
+	}
+
+	dev_info(dev, "Using 0x%08x for watchdog MMIO address\n", mmio_addr);
+
+	return 0;
+}
+
 static int sp5100_tco_timer_init(struct sp5100_tco *tco)
 {
 	struct watchdog_device *wdd = &tco->wdd;
@@ -264,6 +313,7 @@ static int sp5100_tco_setupdevice(struct device *dev,
 	struct sp5100_tco *tco = watchdog_get_drvdata(wdd);
 	const char *dev_name;
 	u32 mmio_addr = 0, val;
+	u32 alt_mmio_addr = 0;
 	int ret;
 
 	/* Request the IO ports used by this driver */
@@ -282,11 +332,32 @@ static int sp5100_tco_setupdevice(struct device *dev,
 		dev_name = SP5100_DEVNAME;
 		mmio_addr = sp5100_tco_read_pm_reg32(SP5100_PM_WATCHDOG_BASE) &
 								0xfffffff8;
+
+		/*
+		 * Secondly, find the watchdog timer MMIO address
+		 * from SBResource_MMIO register.
+		 */
+
+		/* Read SBResource_MMIO from PCI config(PCI_Reg: 9Ch) */
+		pci_read_config_dword(sp5100_tco_pci,
+				      SP5100_SB_RESOURCE_MMIO_BASE,
+				      &val);
+
+		/* Verify MMIO is enabled and using bar0 */
+		if ((val & SB800_ACPI_MMIO_MASK) == SB800_ACPI_MMIO_DECODE_EN)
+			alt_mmio_addr = (val & ~0xfff) + SB800_PM_WDT_MMIO_OFFSET;
 		break;
 	case sb800:
 		dev_name = SB800_DEVNAME;
 		mmio_addr = sp5100_tco_read_pm_reg32(SB800_PM_WATCHDOG_BASE) &
 								0xfffffff8;
+
+		/* Read SBResource_MMIO from AcpiMmioEn(PM_Reg: 24h) */
+		val = sp5100_tco_read_pm_reg32(SB800_PM_ACPI_MMIO_EN);
+
+		/* Verify MMIO is enabled and using bar0 */
+		if ((val & SB800_ACPI_MMIO_MASK) == SB800_ACPI_MMIO_DECODE_EN)
+			alt_mmio_addr = (val & ~0xfff) + SB800_PM_WDT_MMIO_OFFSET;
 		break;
 	case efch:
 		dev_name = SB800_DEVNAME;
@@ -305,87 +376,23 @@ static int sp5100_tco_setupdevice(struct device *dev,
 		val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN);
 		if (val & EFCH_PM_DECODEEN_WDT_TMREN)
 			mmio_addr = EFCH_PM_WDT_ADDR;
+
+		val = sp5100_tco_read_pm_reg8(EFCH_PM_ISACONTROL);
+		if (val & EFCH_PM_ISACONTROL_MMIOEN)
+			alt_mmio_addr = EFCH_PM_ACPI_MMIO_ADDR +
+				EFCH_PM_ACPI_MMIO_WDT_OFFSET;
 		break;
 	default:
 		return -ENODEV;
 	}
 
-	/* Check MMIO address conflict */
-	if (!mmio_addr ||
-	    !devm_request_mem_region(dev, mmio_addr, SP5100_WDT_MEM_MAP_SIZE,
-				     dev_name)) {
-		if (mmio_addr)
-			dev_dbg(dev, "MMIO address 0x%08x already in use\n",
-				mmio_addr);
-		switch (tco->tco_reg_layout) {
-		case sp5100:
-			/*
-			 * Secondly, Find the watchdog timer MMIO address
-			 * from SBResource_MMIO register.
-			 */
-			/* Read SBResource_MMIO from PCI config(PCI_Reg: 9Ch) */
-			pci_read_config_dword(sp5100_tco_pci,
-					      SP5100_SB_RESOURCE_MMIO_BASE,
-					      &mmio_addr);
-			if ((mmio_addr & (SB800_ACPI_MMIO_DECODE_EN |
-					  SB800_ACPI_MMIO_SEL)) !=
-						  SB800_ACPI_MMIO_DECODE_EN) {
-				ret = -ENODEV;
-				goto unreg_region;
-			}
-			mmio_addr &= ~0xFFF;
-			mmio_addr += SB800_PM_WDT_MMIO_OFFSET;
-			break;
-		case sb800:
-			/* Read SBResource_MMIO from AcpiMmioEn(PM_Reg: 24h) */
-			mmio_addr =
-				sp5100_tco_read_pm_reg32(SB800_PM_ACPI_MMIO_EN);
-			if ((mmio_addr & (SB800_ACPI_MMIO_DECODE_EN |
-					  SB800_ACPI_MMIO_SEL)) !=
-						  SB800_ACPI_MMIO_DECODE_EN) {
-				ret = -ENODEV;
-				goto unreg_region;
-			}
-			mmio_addr &= ~0xFFF;
-			mmio_addr += SB800_PM_WDT_MMIO_OFFSET;
-			break;
-		case efch:
-			val = sp5100_tco_read_pm_reg8(EFCH_PM_ISACONTROL);
-			if (!(val & EFCH_PM_ISACONTROL_MMIOEN)) {
-				ret = -ENODEV;
-				goto unreg_region;
-			}
-			mmio_addr = EFCH_PM_ACPI_MMIO_ADDR +
-				    EFCH_PM_ACPI_MMIO_WDT_OFFSET;
-			break;
-		}
-		dev_dbg(dev, "Got 0x%08x from SBResource_MMIO register\n",
-			mmio_addr);
-		if (!devm_request_mem_region(dev, mmio_addr,
-					     SP5100_WDT_MEM_MAP_SIZE,
-					     dev_name)) {
-			dev_dbg(dev, "MMIO address 0x%08x already in use\n",
-				mmio_addr);
-			ret = -EBUSY;
-			goto unreg_region;
-		}
-	}
-
-	tco->tcobase = devm_ioremap(dev, mmio_addr, SP5100_WDT_MEM_MAP_SIZE);
-	if (!tco->tcobase) {
-		dev_err(dev, "failed to get tcobase address\n");
-		ret = -ENOMEM;
-		goto unreg_region;
+	ret = sp5100_tco_prepare_base(tco, mmio_addr, alt_mmio_addr, dev_name);
+	if (!ret) {
+		/* Setup the watchdog timer */
+		tco_timer_enable(tco);
+		ret = sp5100_tco_timer_init(tco);
 	}
 
-	dev_info(dev, "Using 0x%08x for watchdog MMIO address\n", mmio_addr);
-
-	/* Setup the watchdog timer */
-	tco_timer_enable(tco);
-
-	ret = sp5100_tco_timer_init(tco);
-
-unreg_region:
 	release_region(SP5100_IO_PM_INDEX_REG, SP5100_PM_IOPORTS_SIZE);
 	return ret;
 }
diff --git a/drivers/watchdog/sp5100_tco.h b/drivers/watchdog/sp5100_tco.h
index adf015aa4126..daee872f9b71 100644
--- a/drivers/watchdog/sp5100_tco.h
+++ b/drivers/watchdog/sp5100_tco.h
@@ -58,6 +58,7 @@
 #define SB800_PM_WATCHDOG_SECOND_RES	GENMASK(1, 0)
 #define SB800_ACPI_MMIO_DECODE_EN	BIT(0)
 #define SB800_ACPI_MMIO_SEL		BIT(1)
+#define SB800_ACPI_MMIO_MASK		GENMASK(1, 0)
 
 #define SB800_PM_WDT_MMIO_OFFSET	0xB00
 
-- 
2.30.2

[PATCH v5 3/4] Watchdog: sp5100_tco: Add initialization using EFCH MMIO

[Terry Bowman]

cd6h/cd7h port I/O can be disabled on recent AMD hardware. Read
accesses to disabled cd6h/cd7h port I/O will return F's and written
data is dropped. It is recommended to replace the cd6h/cd7h
port I/O with MMIO.

Co-developed-by: Robert Richter <rrichter@amd.com>
Signed-off-by: Robert Richter <rrichter@amd.com>
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Tested-by: Jean Delvare <jdelvare@suse.de>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/watchdog/sp5100_tco.c | 100 +++++++++++++++++++++++++++++++++-
 drivers/watchdog/sp5100_tco.h |   5 ++
 2 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
index 8db7504f0aa4..e02399ea8730 100644
--- a/drivers/watchdog/sp5100_tco.c
+++ b/drivers/watchdog/sp5100_tco.c
@@ -49,7 +49,7 @@
 /* internal variables */
 
 enum tco_reg_layout {
-	sp5100, sb800, efch
+	sp5100, sb800, efch, efch_mmio
 };
 
 struct sp5100_tco {
@@ -209,6 +209,8 @@ static void tco_timer_enable(struct sp5100_tco *tco)
 					  ~EFCH_PM_WATCHDOG_DISABLE,
 					  EFCH_PM_DECODEEN_SECOND_RES);
 		break;
+	default:
+		break;
 	}
 }
 
@@ -307,6 +309,99 @@ static int sp5100_tco_timer_init(struct sp5100_tco *tco)
 	return 0;
 }
 
+static u8 efch_read_pm_reg8(void __iomem *addr, u8 index)
+{
+	return readb(addr + index);
+}
+
+static void efch_update_pm_reg8(void __iomem *addr, u8 index, u8 reset, u8 set)
+{
+	u8 val;
+
+	val = readb(addr + index);
+	val &= reset;
+	val |= set;
+	writeb(val, addr + index);
+}
+
+static void tco_timer_enable_mmio(void __iomem *addr)
+{
+	efch_update_pm_reg8(addr, EFCH_PM_DECODEEN3,
+			    ~EFCH_PM_WATCHDOG_DISABLE,
+			    EFCH_PM_DECODEEN_SECOND_RES);
+}
+
+static int sp5100_tco_setupdevice_mmio(struct device *dev,
+				       struct watchdog_device *wdd)
+{
+	struct sp5100_tco *tco = watchdog_get_drvdata(wdd);
+	const char *dev_name = SB800_DEVNAME;
+	u32 mmio_addr = 0, alt_mmio_addr = 0;
+	struct resource *res;
+	void __iomem *addr;
+	int ret;
+	u32 val;
+
+	res = request_mem_region_muxed(EFCH_PM_ACPI_MMIO_PM_ADDR,
+				       EFCH_PM_ACPI_MMIO_PM_SIZE,
+				       "sp5100_tco");
+
+	if (!res) {
+		dev_err(dev,
+			"Memory region 0x%08x already in use\n",
+			EFCH_PM_ACPI_MMIO_PM_ADDR);
+		return -EBUSY;
+	}
+
+	addr = ioremap(EFCH_PM_ACPI_MMIO_PM_ADDR, EFCH_PM_ACPI_MMIO_PM_SIZE);
+	if (!addr) {
+		dev_err(dev, "Address mapping failed\n");
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/*
+	 * EFCH_PM_DECODEEN_WDT_TMREN is dual purpose. This bitfield
+	 * enables sp5100_tco register MMIO space decoding. The bitfield
+	 * also starts the timer operation. Enable if not already enabled.
+	 */
+	val = efch_read_pm_reg8(addr, EFCH_PM_DECODEEN);
+	if (!(val & EFCH_PM_DECODEEN_WDT_TMREN)) {
+		efch_update_pm_reg8(addr, EFCH_PM_DECODEEN, 0xff,
+				    EFCH_PM_DECODEEN_WDT_TMREN);
+	}
+
+	/* Error if the timer could not be enabled */
+	val = efch_read_pm_reg8(addr, EFCH_PM_DECODEEN);
+	if (!(val & EFCH_PM_DECODEEN_WDT_TMREN)) {
+		dev_err(dev, "Failed to enable the timer\n");
+		ret = -EFAULT;
+		goto out;
+	}
+
+	mmio_addr = EFCH_PM_WDT_ADDR;
+
+	/* Determine alternate MMIO base address */
+	val = efch_read_pm_reg8(addr, EFCH_PM_ISACONTROL);
+	if (val & EFCH_PM_ISACONTROL_MMIOEN)
+		alt_mmio_addr = EFCH_PM_ACPI_MMIO_ADDR +
+			EFCH_PM_ACPI_MMIO_WDT_OFFSET;
+
+	ret = sp5100_tco_prepare_base(tco, mmio_addr, alt_mmio_addr, dev_name);
+	if (!ret) {
+		tco_timer_enable_mmio(addr);
+		ret = sp5100_tco_timer_init(tco);
+	}
+
+out:
+	if (addr)
+		iounmap(addr);
+
+	release_resource(res);
+
+	return ret;
+}
+
 static int sp5100_tco_setupdevice(struct device *dev,
 				  struct watchdog_device *wdd)
 {
@@ -316,6 +411,9 @@ static int sp5100_tco_setupdevice(struct device *dev,
 	u32 alt_mmio_addr = 0;
 	int ret;
 
+	if (tco->tco_reg_layout == efch_mmio)
+		return sp5100_tco_setupdevice_mmio(dev, wdd);
+
 	/* Request the IO ports used by this driver */
 	if (!request_muxed_region(SP5100_IO_PM_INDEX_REG,
 				  SP5100_PM_IOPORTS_SIZE, "sp5100_tco")) {
diff --git a/drivers/watchdog/sp5100_tco.h b/drivers/watchdog/sp5100_tco.h
index daee872f9b71..8ca1b215e3ce 100644
--- a/drivers/watchdog/sp5100_tco.h
+++ b/drivers/watchdog/sp5100_tco.h
@@ -83,4 +83,9 @@
 #define EFCH_PM_ISACONTROL_MMIOEN	BIT(1)
 
 #define EFCH_PM_ACPI_MMIO_ADDR		0xfed80000
+#define EFCH_PM_ACPI_MMIO_PM_OFFSET	0x00000300
 #define EFCH_PM_ACPI_MMIO_WDT_OFFSET	0x00000b00
+
+#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
-- 
2.30.2

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

[Terry Bowman]

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@amd.com>
Signed-off-by: Robert Richter <rrichter@amd.com>
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Tested-by: Jean Delvare <jdelvare@suse.de>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 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

Re: [PATCH v5 1/4] Watchdog: sp5100_tco: Move timer initialization into function

[Guenter Roeck]

On Wed, Feb 02, 2022 at 09:35:22AM -0600, Terry Bowman wrote:
> Refactor driver's timer initialization into new function. This is needed
> inorder to support adding new device layouts while using common timer
> initialization.
> 
> Co-developed-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Tested-by: Jean Delvare <jdelvare@suse.de>
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/sp5100_tco.c | 65 +++++++++++++++++++----------------
>  1 file changed, 36 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
> index dd9a744f82f8..b365bbc9ac36 100644
> --- a/drivers/watchdog/sp5100_tco.c
> +++ b/drivers/watchdog/sp5100_tco.c
> @@ -223,6 +223,41 @@ static u32 sp5100_tco_read_pm_reg32(u8 index)
>  	return val;
>  }
>  
> +static int sp5100_tco_timer_init(struct sp5100_tco *tco)
> +{
> +	struct watchdog_device *wdd = &tco->wdd;
> +	struct device *dev = wdd->parent;
> +	u32 val;
> +
> +	val = readl(SP5100_WDT_CONTROL(tco->tcobase));
> +	if (val & SP5100_WDT_DISABLED) {
> +		dev_err(dev, "Watchdog hardware is disabled\n");
> +		return -ENODEV;
> +	}
> +
> +	/*
> +	 * Save WatchDogFired status, because WatchDogFired flag is
> +	 * cleared here.
> +	 */
> +	if (val & SP5100_WDT_FIRED)
> +		wdd->bootstatus = WDIOF_CARDRESET;
> +
> +	/* Set watchdog action to reset the system */
> +	val &= ~SP5100_WDT_ACTION_RESET;
> +	writel(val, SP5100_WDT_CONTROL(tco->tcobase));
> +
> +	/* Set a reasonable heartbeat before we stop the timer */
> +	tco_timer_set_timeout(wdd, wdd->timeout);
> +
> +	/*
> +	 * Stop the TCO before we change anything so we don't race with
> +	 * a zeroed timer.
> +	 */
> +	tco_timer_stop(wdd);
> +
> +	return 0;
> +}
> +
>  static int sp5100_tco_setupdevice(struct device *dev,
>  				  struct watchdog_device *wdd)
>  {
> @@ -348,35 +383,7 @@ static int sp5100_tco_setupdevice(struct device *dev,
>  	/* Setup the watchdog timer */
>  	tco_timer_enable(tco);
>  
> -	val = readl(SP5100_WDT_CONTROL(tco->tcobase));
> -	if (val & SP5100_WDT_DISABLED) {
> -		dev_err(dev, "Watchdog hardware is disabled\n");
> -		ret = -ENODEV;
> -		goto unreg_region;
> -	}
> -
> -	/*
> -	 * Save WatchDogFired status, because WatchDogFired flag is
> -	 * cleared here.
> -	 */
> -	if (val & SP5100_WDT_FIRED)
> -		wdd->bootstatus = WDIOF_CARDRESET;
> -	/* Set watchdog action to reset the system */
> -	val &= ~SP5100_WDT_ACTION_RESET;
> -	writel(val, SP5100_WDT_CONTROL(tco->tcobase));
> -
> -	/* Set a reasonable heartbeat before we stop the timer */
> -	tco_timer_set_timeout(wdd, wdd->timeout);
> -
> -	/*
> -	 * Stop the TCO before we change anything so we don't race with
> -	 * a zeroed timer.
> -	 */
> -	tco_timer_stop(wdd);
> -
> -	release_region(SP5100_IO_PM_INDEX_REG, SP5100_PM_IOPORTS_SIZE);
> -
> -	return 0;
> +	ret = sp5100_tco_timer_init(tco);
>  
>  unreg_region:
>  	release_region(SP5100_IO_PM_INDEX_REG, SP5100_PM_IOPORTS_SIZE);
> -- 
> 2.30.2
> 

Re: [PATCH v5 2/4] Watchdog: sp5100_tco: Refactor MMIO base address initialization

[Guenter Roeck]

On Wed, Feb 02, 2022 at 09:35:23AM -0600, Terry Bowman wrote:
> Combine MMIO base address and alternate base address detection. Combine
> based on layout type. This will simplify the function by eliminating
> a switch case.
> 
> Move existing request/release code into functions. This currently only
> supports port I/O request/release. The move into a separate function
> will make it ready for adding MMIO region support.
> 
> Co-developed-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Tested-by: Jean Delvare <jdelvare@suse.de>
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/sp5100_tco.c | 155 ++++++++++++++++++----------------
>  drivers/watchdog/sp5100_tco.h |   1 +
>  2 files changed, 82 insertions(+), 74 deletions(-)
> 
> diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
> index b365bbc9ac36..8db7504f0aa4 100644
> --- a/drivers/watchdog/sp5100_tco.c
> +++ b/drivers/watchdog/sp5100_tco.c
> @@ -223,6 +223,55 @@ static u32 sp5100_tco_read_pm_reg32(u8 index)
>  	return val;
>  }
>  
> +static u32 sp5100_tco_request_region(struct device *dev,
> +				     u32 mmio_addr,
> +				     const char *dev_name)
> +{
> +	if (!devm_request_mem_region(dev, mmio_addr, SP5100_WDT_MEM_MAP_SIZE,
> +				     dev_name)) {
> +		dev_dbg(dev, "MMIO address 0x%08x already in use\n", mmio_addr);
> +		return 0;
> +	}
> +
> +	return mmio_addr;
> +}
> +
> +static u32 sp5100_tco_prepare_base(struct sp5100_tco *tco,
> +				   u32 mmio_addr,
> +				   u32 alt_mmio_addr,
> +				   const char *dev_name)
> +{
> +	struct device *dev = tco->wdd.parent;
> +
> +	dev_dbg(dev, "Got 0x%08x from SBResource_MMIO register\n", mmio_addr);
> +
> +	if (!mmio_addr && !alt_mmio_addr)
> +		return -ENODEV;
> +
> +	/* Check for MMIO address and alternate MMIO address conflicts */
> +	if (mmio_addr)
> +		mmio_addr = sp5100_tco_request_region(dev, mmio_addr, dev_name);
> +
> +	if (!mmio_addr && alt_mmio_addr)
> +		mmio_addr = sp5100_tco_request_region(dev, alt_mmio_addr, dev_name);
> +
> +	if (!mmio_addr) {
> +		dev_err(dev, "Failed to reserve MMIO or alternate MMIO region\n");
> +		return -EBUSY;
> +	}
> +
> +	tco->tcobase = devm_ioremap(dev, mmio_addr, SP5100_WDT_MEM_MAP_SIZE);
> +	if (!tco->tcobase) {
> +		dev_err(dev, "MMIO address 0x%08x failed mapping\n", mmio_addr);
> +		devm_release_mem_region(dev, mmio_addr, SP5100_WDT_MEM_MAP_SIZE);
> +		return -ENOMEM;
> +	}
> +
> +	dev_info(dev, "Using 0x%08x for watchdog MMIO address\n", mmio_addr);
> +
> +	return 0;
> +}
> +
>  static int sp5100_tco_timer_init(struct sp5100_tco *tco)
>  {
>  	struct watchdog_device *wdd = &tco->wdd;
> @@ -264,6 +313,7 @@ static int sp5100_tco_setupdevice(struct device *dev,
>  	struct sp5100_tco *tco = watchdog_get_drvdata(wdd);
>  	const char *dev_name;
>  	u32 mmio_addr = 0, val;
> +	u32 alt_mmio_addr = 0;
>  	int ret;
>  
>  	/* Request the IO ports used by this driver */
> @@ -282,11 +332,32 @@ static int sp5100_tco_setupdevice(struct device *dev,
>  		dev_name = SP5100_DEVNAME;
>  		mmio_addr = sp5100_tco_read_pm_reg32(SP5100_PM_WATCHDOG_BASE) &
>  								0xfffffff8;
> +
> +		/*
> +		 * Secondly, find the watchdog timer MMIO address
> +		 * from SBResource_MMIO register.
> +		 */
> +
> +		/* Read SBResource_MMIO from PCI config(PCI_Reg: 9Ch) */
> +		pci_read_config_dword(sp5100_tco_pci,
> +				      SP5100_SB_RESOURCE_MMIO_BASE,
> +				      &val);
> +
> +		/* Verify MMIO is enabled and using bar0 */
> +		if ((val & SB800_ACPI_MMIO_MASK) == SB800_ACPI_MMIO_DECODE_EN)
> +			alt_mmio_addr = (val & ~0xfff) + SB800_PM_WDT_MMIO_OFFSET;
>  		break;
>  	case sb800:
>  		dev_name = SB800_DEVNAME;
>  		mmio_addr = sp5100_tco_read_pm_reg32(SB800_PM_WATCHDOG_BASE) &
>  								0xfffffff8;
> +
> +		/* Read SBResource_MMIO from AcpiMmioEn(PM_Reg: 24h) */
> +		val = sp5100_tco_read_pm_reg32(SB800_PM_ACPI_MMIO_EN);
> +
> +		/* Verify MMIO is enabled and using bar0 */
> +		if ((val & SB800_ACPI_MMIO_MASK) == SB800_ACPI_MMIO_DECODE_EN)
> +			alt_mmio_addr = (val & ~0xfff) + SB800_PM_WDT_MMIO_OFFSET;
>  		break;
>  	case efch:
>  		dev_name = SB800_DEVNAME;
> @@ -305,87 +376,23 @@ static int sp5100_tco_setupdevice(struct device *dev,
>  		val = sp5100_tco_read_pm_reg8(EFCH_PM_DECODEEN);
>  		if (val & EFCH_PM_DECODEEN_WDT_TMREN)
>  			mmio_addr = EFCH_PM_WDT_ADDR;
> +
> +		val = sp5100_tco_read_pm_reg8(EFCH_PM_ISACONTROL);
> +		if (val & EFCH_PM_ISACONTROL_MMIOEN)
> +			alt_mmio_addr = EFCH_PM_ACPI_MMIO_ADDR +
> +				EFCH_PM_ACPI_MMIO_WDT_OFFSET;
>  		break;
>  	default:
>  		return -ENODEV;
>  	}
>  
> -	/* Check MMIO address conflict */
> -	if (!mmio_addr ||
> -	    !devm_request_mem_region(dev, mmio_addr, SP5100_WDT_MEM_MAP_SIZE,
> -				     dev_name)) {
> -		if (mmio_addr)
> -			dev_dbg(dev, "MMIO address 0x%08x already in use\n",
> -				mmio_addr);
> -		switch (tco->tco_reg_layout) {
> -		case sp5100:
> -			/*
> -			 * Secondly, Find the watchdog timer MMIO address
> -			 * from SBResource_MMIO register.
> -			 */
> -			/* Read SBResource_MMIO from PCI config(PCI_Reg: 9Ch) */
> -			pci_read_config_dword(sp5100_tco_pci,
> -					      SP5100_SB_RESOURCE_MMIO_BASE,
> -					      &mmio_addr);
> -			if ((mmio_addr & (SB800_ACPI_MMIO_DECODE_EN |
> -					  SB800_ACPI_MMIO_SEL)) !=
> -						  SB800_ACPI_MMIO_DECODE_EN) {
> -				ret = -ENODEV;
> -				goto unreg_region;
> -			}
> -			mmio_addr &= ~0xFFF;
> -			mmio_addr += SB800_PM_WDT_MMIO_OFFSET;
> -			break;
> -		case sb800:
> -			/* Read SBResource_MMIO from AcpiMmioEn(PM_Reg: 24h) */
> -			mmio_addr =
> -				sp5100_tco_read_pm_reg32(SB800_PM_ACPI_MMIO_EN);
> -			if ((mmio_addr & (SB800_ACPI_MMIO_DECODE_EN |
> -					  SB800_ACPI_MMIO_SEL)) !=
> -						  SB800_ACPI_MMIO_DECODE_EN) {
> -				ret = -ENODEV;
> -				goto unreg_region;
> -			}
> -			mmio_addr &= ~0xFFF;
> -			mmio_addr += SB800_PM_WDT_MMIO_OFFSET;
> -			break;
> -		case efch:
> -			val = sp5100_tco_read_pm_reg8(EFCH_PM_ISACONTROL);
> -			if (!(val & EFCH_PM_ISACONTROL_MMIOEN)) {
> -				ret = -ENODEV;
> -				goto unreg_region;
> -			}
> -			mmio_addr = EFCH_PM_ACPI_MMIO_ADDR +
> -				    EFCH_PM_ACPI_MMIO_WDT_OFFSET;
> -			break;
> -		}
> -		dev_dbg(dev, "Got 0x%08x from SBResource_MMIO register\n",
> -			mmio_addr);
> -		if (!devm_request_mem_region(dev, mmio_addr,
> -					     SP5100_WDT_MEM_MAP_SIZE,
> -					     dev_name)) {
> -			dev_dbg(dev, "MMIO address 0x%08x already in use\n",
> -				mmio_addr);
> -			ret = -EBUSY;
> -			goto unreg_region;
> -		}
> -	}
> -
> -	tco->tcobase = devm_ioremap(dev, mmio_addr, SP5100_WDT_MEM_MAP_SIZE);
> -	if (!tco->tcobase) {
> -		dev_err(dev, "failed to get tcobase address\n");
> -		ret = -ENOMEM;
> -		goto unreg_region;
> +	ret = sp5100_tco_prepare_base(tco, mmio_addr, alt_mmio_addr, dev_name);
> +	if (!ret) {
> +		/* Setup the watchdog timer */
> +		tco_timer_enable(tco);
> +		ret = sp5100_tco_timer_init(tco);
>  	}
>  
> -	dev_info(dev, "Using 0x%08x for watchdog MMIO address\n", mmio_addr);
> -
> -	/* Setup the watchdog timer */
> -	tco_timer_enable(tco);
> -
> -	ret = sp5100_tco_timer_init(tco);
> -
> -unreg_region:
>  	release_region(SP5100_IO_PM_INDEX_REG, SP5100_PM_IOPORTS_SIZE);
>  	return ret;
>  }
> diff --git a/drivers/watchdog/sp5100_tco.h b/drivers/watchdog/sp5100_tco.h
> index adf015aa4126..daee872f9b71 100644
> --- a/drivers/watchdog/sp5100_tco.h
> +++ b/drivers/watchdog/sp5100_tco.h
> @@ -58,6 +58,7 @@
>  #define SB800_PM_WATCHDOG_SECOND_RES	GENMASK(1, 0)
>  #define SB800_ACPI_MMIO_DECODE_EN	BIT(0)
>  #define SB800_ACPI_MMIO_SEL		BIT(1)
> +#define SB800_ACPI_MMIO_MASK		GENMASK(1, 0)
>  
>  #define SB800_PM_WDT_MMIO_OFFSET	0xB00
>  
> -- 
> 2.30.2
> 

Re: [PATCH v5 3/4] Watchdog: sp5100_tco: Add initialization using EFCH MMIO

[Guenter Roeck]

On Wed, Feb 02, 2022 at 09:35:24AM -0600, Terry Bowman wrote:
> cd6h/cd7h port I/O can be disabled on recent AMD hardware. Read
> accesses to disabled cd6h/cd7h port I/O will return F's and written
> data is dropped. It is recommended to replace the cd6h/cd7h
> port I/O with MMIO.
> 
> Co-developed-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Tested-by: Jean Delvare <jdelvare@suse.de>
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/sp5100_tco.c | 100 +++++++++++++++++++++++++++++++++-
>  drivers/watchdog/sp5100_tco.h |   5 ++
>  2 files changed, 104 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
> index 8db7504f0aa4..e02399ea8730 100644
> --- a/drivers/watchdog/sp5100_tco.c
> +++ b/drivers/watchdog/sp5100_tco.c
> @@ -49,7 +49,7 @@
>  /* internal variables */
>  
>  enum tco_reg_layout {
> -	sp5100, sb800, efch
> +	sp5100, sb800, efch, efch_mmio
>  };
>  
>  struct sp5100_tco {
> @@ -209,6 +209,8 @@ static void tco_timer_enable(struct sp5100_tco *tco)
>  					  ~EFCH_PM_WATCHDOG_DISABLE,
>  					  EFCH_PM_DECODEEN_SECOND_RES);
>  		break;
> +	default:
> +		break;
>  	}
>  }
>  
> @@ -307,6 +309,99 @@ static int sp5100_tco_timer_init(struct sp5100_tco *tco)
>  	return 0;
>  }
>  
> +static u8 efch_read_pm_reg8(void __iomem *addr, u8 index)
> +{
> +	return readb(addr + index);
> +}
> +
> +static void efch_update_pm_reg8(void __iomem *addr, u8 index, u8 reset, u8 set)
> +{
> +	u8 val;
> +
> +	val = readb(addr + index);
> +	val &= reset;
> +	val |= set;
> +	writeb(val, addr + index);
> +}
> +
> +static void tco_timer_enable_mmio(void __iomem *addr)
> +{
> +	efch_update_pm_reg8(addr, EFCH_PM_DECODEEN3,
> +			    ~EFCH_PM_WATCHDOG_DISABLE,
> +			    EFCH_PM_DECODEEN_SECOND_RES);
> +}
> +
> +static int sp5100_tco_setupdevice_mmio(struct device *dev,
> +				       struct watchdog_device *wdd)
> +{
> +	struct sp5100_tco *tco = watchdog_get_drvdata(wdd);
> +	const char *dev_name = SB800_DEVNAME;
> +	u32 mmio_addr = 0, alt_mmio_addr = 0;
> +	struct resource *res;
> +	void __iomem *addr;
> +	int ret;
> +	u32 val;
> +
> +	res = request_mem_region_muxed(EFCH_PM_ACPI_MMIO_PM_ADDR,
> +				       EFCH_PM_ACPI_MMIO_PM_SIZE,
> +				       "sp5100_tco");
> +
> +	if (!res) {
> +		dev_err(dev,
> +			"Memory region 0x%08x already in use\n",
> +			EFCH_PM_ACPI_MMIO_PM_ADDR);
> +		return -EBUSY;
> +	}
> +
> +	addr = ioremap(EFCH_PM_ACPI_MMIO_PM_ADDR, EFCH_PM_ACPI_MMIO_PM_SIZE);
> +	if (!addr) {
> +		dev_err(dev, "Address mapping failed\n");
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	/*
> +	 * EFCH_PM_DECODEEN_WDT_TMREN is dual purpose. This bitfield
> +	 * enables sp5100_tco register MMIO space decoding. The bitfield
> +	 * also starts the timer operation. Enable if not already enabled.
> +	 */
> +	val = efch_read_pm_reg8(addr, EFCH_PM_DECODEEN);
> +	if (!(val & EFCH_PM_DECODEEN_WDT_TMREN)) {
> +		efch_update_pm_reg8(addr, EFCH_PM_DECODEEN, 0xff,
> +				    EFCH_PM_DECODEEN_WDT_TMREN);
> +	}
> +
> +	/* Error if the timer could not be enabled */
> +	val = efch_read_pm_reg8(addr, EFCH_PM_DECODEEN);
> +	if (!(val & EFCH_PM_DECODEEN_WDT_TMREN)) {
> +		dev_err(dev, "Failed to enable the timer\n");
> +		ret = -EFAULT;
> +		goto out;
> +	}
> +
> +	mmio_addr = EFCH_PM_WDT_ADDR;
> +
> +	/* Determine alternate MMIO base address */
> +	val = efch_read_pm_reg8(addr, EFCH_PM_ISACONTROL);
> +	if (val & EFCH_PM_ISACONTROL_MMIOEN)
> +		alt_mmio_addr = EFCH_PM_ACPI_MMIO_ADDR +
> +			EFCH_PM_ACPI_MMIO_WDT_OFFSET;
> +
> +	ret = sp5100_tco_prepare_base(tco, mmio_addr, alt_mmio_addr, dev_name);
> +	if (!ret) {
> +		tco_timer_enable_mmio(addr);
> +		ret = sp5100_tco_timer_init(tco);
> +	}
> +
> +out:
> +	if (addr)
> +		iounmap(addr);
> +
> +	release_resource(res);
> +
> +	return ret;
> +}
> +
>  static int sp5100_tco_setupdevice(struct device *dev,
>  				  struct watchdog_device *wdd)
>  {
> @@ -316,6 +411,9 @@ static int sp5100_tco_setupdevice(struct device *dev,
>  	u32 alt_mmio_addr = 0;
>  	int ret;
>  
> +	if (tco->tco_reg_layout == efch_mmio)
> +		return sp5100_tco_setupdevice_mmio(dev, wdd);
> +
>  	/* Request the IO ports used by this driver */
>  	if (!request_muxed_region(SP5100_IO_PM_INDEX_REG,
>  				  SP5100_PM_IOPORTS_SIZE, "sp5100_tco")) {
> diff --git a/drivers/watchdog/sp5100_tco.h b/drivers/watchdog/sp5100_tco.h
> index daee872f9b71..8ca1b215e3ce 100644
> --- a/drivers/watchdog/sp5100_tco.h
> +++ b/drivers/watchdog/sp5100_tco.h
> @@ -83,4 +83,9 @@
>  #define EFCH_PM_ISACONTROL_MMIOEN	BIT(1)
>  
>  #define EFCH_PM_ACPI_MMIO_ADDR		0xfed80000
> +#define EFCH_PM_ACPI_MMIO_PM_OFFSET	0x00000300
>  #define EFCH_PM_ACPI_MMIO_WDT_OFFSET	0x00000b00
> +
> +#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
> -- 
> 2.30.2
> 

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@amd.com>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Tested-by: Jean Delvare <jdelvare@suse.de>
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  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
>