[PATCH v5 0/9] i2c: piix4: Replace cd6h/cd7h port I/O accesses with MMIO accesses

[PATCH v5 0/9] i2c: piix4: Replace cd6h/cd7h port I/O accesses with MMIO accesses

[Terry Bowman]

This series changes the piix4_smbus driver's cd6h/cd7h port I/O accesses
to use MMIO instead. This is necessary because cd6h/cd7h port I/O may be
disabled on later AMD processors.

This series includes patches with MMIO accesses to register
FCH::PM::DECODEEN. The same register is also accessed by the sp5100_tco
driver.[1] Synchronization to the MMIO register is required in both
drivers.

The first patch creates a macro to request MMIO region using the 'muxed'
retry logic. This is used in patch 6 to synchronize accesses to EFCH MMIO.

The second patch replaces a hardcoded region size with a #define. This is
to improve maintainability and was requested from v2 review.

The third patch moves duplicated region request/release code into
functions. This locates related code into functions and reduces code line
count. This will also make adding MMIO support in patch 6 easier.

The fourth patch moves SMBus controller address detection into a function. 
This is in preparation for adding MMIO region support.

The fifth patch moves EFCH port selection into a function. This is in
preparation for adding MMIO region support.

The sixth patch adds MMIO support for region requesting/releasing and
mapping. This is necessary for using MMIO to detect SMBus controller
address, enable SMBbus controller region, and control the port select.

The seventh patch updates the SMBus controller address detection to support
using MMIO. This is necessary because the driver accesses register
FCH::PM::DECODEEN during initialization and only available using MMIO on
later AMD processors.

The eighth patch updates the SMBus port selection to support MMIO. This is
required because port selection control resides in the
FCH::PM::DECODEEN[smbus0sel] and is only accessible using MMIO on later AMD
processors.

The ninth patch enables the EFCH MMIO functionality added earlier in this
series. The SMBus controller's PCI revision ID is used to check if EFCH
MMIO is supported by HW and should be enabled in the driver.

Based on v5.17-rc2.

Testing:
  Tested on family 19h using:
    i2cdetect -y 0
    i2cdetect -y 2

  - Results using v5.16 and this pachset applied. Below
    shows the devices detected on the busses:
    
    # i2cdetect -y 0 
         0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
    00:                         -- -- -- -- -- -- -- -- 
    10: 10 11 -- -- -- -- -- -- 18 -- -- -- -- -- -- -- 
    20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    30: 30 -- -- -- -- 35 36 -- -- -- -- -- -- -- -- -- 
    40: -- -- -- -- -- -- -- -- -- -- 4a -- -- -- -- -- 
    50: 50 -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    70: -- -- -- 73 -- -- -- --                         
    # i2cdetect -y 2
         0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
    00:                         -- -- -- -- -- -- -- -- 
    10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    40: -- -- -- -- -- -- -- -- -- -- -- -- 4c -- -- -- 
    50: -- 51 -- -- 54 -- -- -- -- -- -- -- -- -- -- -- 
    60: 60 -- -- 63 -- -- 66 -- -- -- -- 6b -- -- 6e -- 
    70: 70 71 72 73 74 75 -- 77

  Also tested using sp5100_tco submitted series listed below.[1]
  I applied the sp5100_tco v4 series and ran:
    cat  >> /dev/watchdog

[1] sp5100_tco v4 patchset can be found here:
Link: https://lore.kernel.org/linux-watchdog/20220130191225.303115-1-terry.bowman@amd.com/

Changes in v5:
 - Use request/release helper function for sb800 device in
   piix4_setup_sb800(). Patch 3. (Jean Delvare)     
 - Revert 'piix4_smba' variable definition ordering back as it was in
   piix4_setup_sb800(). Patch 4. (Jean Delvare)
 - Add newline after piix4_sb800_port_sel(). Patch 5. (Jean Delvare)
 - Remove unnecessary initialization in piix4_add_adapter(). Patch 6.
   (Jean Delvare)
 - Remove unnecessary #define AMD_PCI_SMBUS_REVISION_MMIO. Patch 9.
   (Jean Delvare)
 - Add description for 0x51 constant moved in the above item. This is
   in piix4_sb800_use_mmio(). Patch 9. (Andy Shevchenko)
 - Rebase to v5.17-rc2. (Andy Shevchenko)
 - Update patch 9 description. (Terry Bowman)
 
Changes in v4:
 - Changed request_muxed_mem_region() macro to request_mem_region_muxed()
   in patch 1. (Andy Shevchenko)
 - Removed unnecessary newline where possible in calls to
   request_muxed_region() in patch 2. (Terry Bowman)
 - Changed piix4_sb800_region_setup() to piix4_sb800_region_request().
   Patch 3. (Jean Delvare)
 - Reordered piix4_setup_sb800() local variables from longest name length
   to shortest name length. Patch 4. (Terry Bowman)
 - Changed piix4_sb800_region_request() and piix4_sb800_region_release() by
   adding early return() to remove 'else' improving readability. Patch 6.
   (Terry Bowman)
 - Removed iowrite32(ioread32(...), ...). Unnecessary because MMIO is
   already enabled. (Terry Bowman)
 - Refactored piix4_sb800_port_sel() to simplify the 'if' statement using
   temp variable. Patch 8. (Terry Bowman)
 - Added mmio_cfg.use_mmio assignment in piix4_add_adapter(). This is
   needed for calls to piix4_sb800_port_sel() after initialization during
   normal operation. Patch 9. (Terry Bowman)
 
Changes in v3:
 - Added request_muxed_mem_region() patch (Wolfram, Guenter)
 - Reduced To/Cc list length. (Andy)
 
Changes in v2:
 - Split single patch. (Jean Delvare)
 - Replace constant 2 with SB800_PIIX4_SMB_MAP_SIZE where appropriate.
   (Jean Delvare)
 - Shorten SB800_PIIX4_FCH_PM_DECODEEN_MMIO_EN name length to
   SB800_PIIX4_FCH_PM_DECODEEN_MMIO. (Jean Delvare)
 - Change AMD_PCI_SMBUS_REVISION_MMIO from 0x59 to 0x51. (Terry Bowman)
 - Change piix4_sb800_region_setup() to piix4_sb800_region_request().
   (Jean Delvare)
 - Change 'SMB' text in  logging to 'SMBus' (Jean Delvare)
 - Remove unnecessary NULL assignment in piix4_sb800_region_release().
   (Jean Delvare)
 - Move 'u8' variable definitions to single line. (Jean Delvare)
 - Hardcode piix4_setup_sb800_smba() return value to 0 since it is always
   0. (Jean Delvare)

Terry Bowman (9):
  kernel/resource: Introduce request_mem_region_muxed()
  i2c: piix4: Replace hardcoded memory map size with a #define
  i2c: piix4: Move port I/O region request/release code into functions
  i2c: piix4: Move SMBus controller base address detect into function
  i2c: piix4: Move SMBus port selection into function
  i2c: piix4: Add EFCH MMIO support to region request and release
  i2c: piix4: Add EFCH MMIO support to SMBus base address detect
  i2c: piix4: Add EFCH MMIO support for SMBus port select
  i2c: piix4: Enable EFCH MMIO for Family 17h+

 drivers/i2c/busses/i2c-piix4.c | 213 ++++++++++++++++++++++++++-------
 include/linux/ioport.h         |   2 +
 2 files changed, 169 insertions(+), 46 deletions(-)

-- 
2.30.2

[PATCH v5 1/9] kernel/resource: Introduce request_mem_region_muxed()

[Terry Bowman]

Support for requesting muxed memory region is implemented but not
currently callable as a macro. Add the request muxed memory
region macro.

MMIO memory accesses can be synchronized using request_mem_region() which
is already available. This call will return failure if the resource is
busy. The 'muxed' version of this macro will handle a busy resource by
using a wait queue to retry until the resource is available.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 include/linux/ioport.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 8359c50f9988..ec5f71f7135b 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -262,6 +262,8 @@ resource_union(struct resource *r1, struct resource *r2, struct resource *r)
 #define request_muxed_region(start,n,name)	__request_region(&ioport_resource, (start), (n), (name), IORESOURCE_MUXED)
 #define __request_mem_region(start,n,name, excl) __request_region(&iomem_resource, (start), (n), (name), excl)
 #define request_mem_region(start,n,name) __request_region(&iomem_resource, (start), (n), (name), 0)
+#define request_mem_region_muxed(start, n, name) \
+	__request_region(&iomem_resource, (start), (n), (name), IORESOURCE_MUXED)
 #define request_mem_region_exclusive(start,n,name) \
 	__request_region(&iomem_resource, (start), (n), (name), IORESOURCE_EXCLUSIVE)
 #define rename_region(region, newname) do { (region)->name = (newname); } while (0)
-- 
2.30.2

[PATCH v5 2/9] i2c: piix4: Replace hardcoded memory map size with a #define

[Terry Bowman]

Replace number constant with #define to improve readability and
maintainability.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/i2c/busses/i2c-piix4.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 8c1b31ed0c42..3ff68967034e 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -77,6 +77,7 @@
 
 /* SB800 constants */
 #define SB800_PIIX4_SMB_IDX		0xcd6
+#define SB800_PIIX4_SMB_MAP_SIZE	2
 
 #define KERNCZ_IMC_IDX			0x3e
 #define KERNCZ_IMC_DATA			0x3f
@@ -290,7 +291,8 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 	else
 		smb_en = (aux) ? 0x28 : 0x2c;
 
-	if (!request_muxed_region(SB800_PIIX4_SMB_IDX, 2, "sb800_piix4_smb")) {
+	if (!request_muxed_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE,
+				  "sb800_piix4_smb")) {
 		dev_err(&PIIX4_dev->dev,
 			"SMB base address index region 0x%x already in use.\n",
 			SB800_PIIX4_SMB_IDX);
@@ -302,7 +304,7 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 	outb_p(smb_en + 1, SB800_PIIX4_SMB_IDX);
 	smba_en_hi = inb_p(SB800_PIIX4_SMB_IDX + 1);
 
-	release_region(SB800_PIIX4_SMB_IDX, 2);
+	release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
 
 	if (!smb_en) {
 		smb_en_status = smba_en_lo & 0x10;
@@ -371,7 +373,8 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
 		}
 	} else {
-		if (!request_muxed_region(SB800_PIIX4_SMB_IDX, 2,
+		if (!request_muxed_region(SB800_PIIX4_SMB_IDX,
+					  SB800_PIIX4_SMB_MAP_SIZE,
 					  "sb800_piix4_smb")) {
 			release_region(piix4_smba, SMBIOSIZE);
 			return -EBUSY;
@@ -384,7 +387,7 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 				       SB800_PIIX4_PORT_IDX;
 		piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK;
 		piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
-		release_region(SB800_PIIX4_SMB_IDX, 2);
+		release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
 	}
 
 	dev_info(&PIIX4_dev->dev,
@@ -682,7 +685,8 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
 	u8 port;
 	int retval;
 
-	if (!request_muxed_region(SB800_PIIX4_SMB_IDX, 2, "sb800_piix4_smb"))
+	if (!request_muxed_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE,
+				  "sb800_piix4_smb"))
 		return -EBUSY;
 
 	/* Request the SMBUS semaphore, avoid conflicts with the IMC */
@@ -758,7 +762,7 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
 		piix4_imc_wakeup();
 
 release:
-	release_region(SB800_PIIX4_SMB_IDX, 2);
+	release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
 	return retval;
 }
 
-- 
2.30.2

[PATCH v5 3/9] i2c: piix4: Move port I/O region request/release code into functions

[Terry Bowman]

Move duplicated region request and release code into a function. Move is
in preparation for following MMIO changes.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/i2c/busses/i2c-piix4.c | 48 ++++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 3ff68967034e..cc488b1e92c3 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -165,6 +165,24 @@ struct i2c_piix4_adapdata {
 	u8 port;		/* Port number, shifted */
 };
 
+static int piix4_sb800_region_request(struct device *dev)
+{
+	if (!request_muxed_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE,
+				  "sb800_piix4_smb")) {
+		dev_err(dev,
+			"SMBus base address index region 0x%x already in use.\n",
+			SB800_PIIX4_SMB_IDX);
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
+static void piix4_sb800_region_release(struct device *dev)
+{
+	release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
+}
+
 static int piix4_setup(struct pci_dev *PIIX4_dev,
 		       const struct pci_device_id *id)
 {
@@ -270,6 +288,7 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 	unsigned short piix4_smba;
 	u8 smba_en_lo, smba_en_hi, smb_en, smb_en_status, port_sel;
 	u8 i2ccfg, i2ccfg_offset = 0x10;
+	int retval;
 
 	/* SB800 and later SMBus does not support forcing address */
 	if (force || force_addr) {
@@ -291,20 +310,16 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 	else
 		smb_en = (aux) ? 0x28 : 0x2c;
 
-	if (!request_muxed_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE,
-				  "sb800_piix4_smb")) {
-		dev_err(&PIIX4_dev->dev,
-			"SMB base address index region 0x%x already in use.\n",
-			SB800_PIIX4_SMB_IDX);
-		return -EBUSY;
-	}
+	retval = piix4_sb800_region_request(&PIIX4_dev->dev);
+	if (retval)
+		return retval;
 
 	outb_p(smb_en, SB800_PIIX4_SMB_IDX);
 	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
 	outb_p(smb_en + 1, SB800_PIIX4_SMB_IDX);
 	smba_en_hi = inb_p(SB800_PIIX4_SMB_IDX + 1);
 
-	release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
+	piix4_sb800_region_release(&PIIX4_dev->dev);
 
 	if (!smb_en) {
 		smb_en_status = smba_en_lo & 0x10;
@@ -373,11 +388,10 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
 		}
 	} else {
-		if (!request_muxed_region(SB800_PIIX4_SMB_IDX,
-					  SB800_PIIX4_SMB_MAP_SIZE,
-					  "sb800_piix4_smb")) {
+		retval = piix4_sb800_region_request(&PIIX4_dev->dev);
+		if (retval)
 			release_region(piix4_smba, SMBIOSIZE);
-			return -EBUSY;
+			return retval;
 		}
 
 		outb_p(SB800_PIIX4_PORT_IDX_SEL, SB800_PIIX4_SMB_IDX);
@@ -387,7 +401,7 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 				       SB800_PIIX4_PORT_IDX;
 		piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK;
 		piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
-		release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
+		piix4_sb800_region_release(&PIIX4_dev->dev);
 	}
 
 	dev_info(&PIIX4_dev->dev,
@@ -685,9 +699,9 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
 	u8 port;
 	int retval;
 
-	if (!request_muxed_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE,
-				  "sb800_piix4_smb"))
-		return -EBUSY;
+	retval = piix4_sb800_region_request(&adap->dev);
+	if (retval)
+		return retval;
 
 	/* Request the SMBUS semaphore, avoid conflicts with the IMC */
 	smbslvcnt  = inb_p(SMBSLVCNT);
@@ -762,7 +776,7 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
 		piix4_imc_wakeup();
 
 release:
-	release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
+	piix4_sb800_region_release(&adap->dev);
 	return retval;
 }
 
-- 
2.30.2

[PATCH v5 4/9] i2c: piix4: Move SMBus controller base address detect into function

[Terry Bowman]

Move SMBus controller base address detection into function. Refactor
is in preparation for following MMIO changes.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/i2c/busses/i2c-piix4.c | 69 ++++++++++++++++++++++------------
 1 file changed, 44 insertions(+), 25 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index cc488b1e92c3..b7998eb44027 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -282,11 +282,51 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
 	return piix4_smba;
 }
 
+static int piix4_setup_sb800_smba(struct pci_dev *PIIX4_dev,
+				  u8 smb_en,
+				  u8 aux,
+				  u8 *smb_en_status,
+				  unsigned short *piix4_smba)
+{
+	u8 smba_en_lo;
+	u8 smba_en_hi;
+	int retval;
+
+	retval = piix4_sb800_region_request(&PIIX4_dev->dev);
+	if (retval)
+		return retval;
+
+	outb_p(smb_en, SB800_PIIX4_SMB_IDX);
+	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
+	outb_p(smb_en + 1, SB800_PIIX4_SMB_IDX);
+	smba_en_hi = inb_p(SB800_PIIX4_SMB_IDX + 1);
+
+	piix4_sb800_region_release(&PIIX4_dev->dev);
+
+	if (!smb_en) {
+		*smb_en_status = smba_en_lo & 0x10;
+		*piix4_smba = smba_en_hi << 8;
+		if (aux)
+			*piix4_smba |= 0x20;
+	} else {
+		*smb_en_status = smba_en_lo & 0x01;
+		*piix4_smba = ((smba_en_hi << 8) | smba_en_lo) & 0xffe0;
+	}
+
+	if (!*smb_en_status) {
+		dev_err(&PIIX4_dev->dev,
+			"SMBus Host Controller not enabled!\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 			     const struct pci_device_id *id, u8 aux)
 {
 	unsigned short piix4_smba;
-	u8 smba_en_lo, smba_en_hi, smb_en, smb_en_status, port_sel;
+	u8 smb_en, smb_en_status, port_sel;
 	u8 i2ccfg, i2ccfg_offset = 0x10;
 	int retval;
 
@@ -310,33 +350,12 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 	else
 		smb_en = (aux) ? 0x28 : 0x2c;
 
-	retval = piix4_sb800_region_request(&PIIX4_dev->dev);
+	retval = piix4_setup_sb800_smba(PIIX4_dev, smb_en, aux, &smb_en_status,
+					&piix4_smba);
+
 	if (retval)
 		return retval;
 
-	outb_p(smb_en, SB800_PIIX4_SMB_IDX);
-	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
-	outb_p(smb_en + 1, SB800_PIIX4_SMB_IDX);
-	smba_en_hi = inb_p(SB800_PIIX4_SMB_IDX + 1);
-
-	piix4_sb800_region_release(&PIIX4_dev->dev);
-
-	if (!smb_en) {
-		smb_en_status = smba_en_lo & 0x10;
-		piix4_smba = smba_en_hi << 8;
-		if (aux)
-			piix4_smba |= 0x20;
-	} else {
-		smb_en_status = smba_en_lo & 0x01;
-		piix4_smba = ((smba_en_hi << 8) | smba_en_lo) & 0xffe0;
-	}
-
-	if (!smb_en_status) {
-		dev_err(&PIIX4_dev->dev,
-			"SMBus Host Controller not enabled!\n");
-		return -ENODEV;
-	}
-
 	if (acpi_check_region(piix4_smba, SMBIOSIZE, piix4_driver.name))
 		return -ENODEV;
 
-- 
2.30.2

[PATCH v5 5/9] i2c: piix4: Move SMBus port selection into function

[Terry Bowman]

Move port selection code into a separate function. Refactor is in
preparation for following MMIO changes.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/i2c/busses/i2c-piix4.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index b7998eb44027..f4a262a7d199 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -698,6 +698,20 @@ static void piix4_imc_wakeup(void)
 	release_region(KERNCZ_IMC_IDX, 2);
 }
 
+static int piix4_sb800_port_sel(u8 port)
+{
+	u8 smba_en_lo, val;
+
+	outb_p(piix4_port_sel_sb800, SB800_PIIX4_SMB_IDX);
+	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
+
+	val = (smba_en_lo & ~piix4_port_mask_sb800) | port;
+	if (smba_en_lo != val)
+		outb_p(val, SB800_PIIX4_SMB_IDX + 1);
+
+	return (smba_en_lo & piix4_port_mask_sb800);
+}
+
 /*
  * Handles access to multiple SMBus ports on the SB800.
  * The port is selected by bits 2:1 of the smb_en register (0x2c).
@@ -714,8 +728,7 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
 	unsigned short piix4_smba = adapdata->smba;
 	int retries = MAX_TIMEOUT;
 	int smbslvcnt;
-	u8 smba_en_lo;
-	u8 port;
+	u8 prev_port;
 	int retval;
 
 	retval = piix4_sb800_region_request(&adap->dev);
@@ -775,18 +788,12 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
 		}
 	}
 
-	outb_p(piix4_port_sel_sb800, SB800_PIIX4_SMB_IDX);
-	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
-
-	port = adapdata->port;
-	if ((smba_en_lo & piix4_port_mask_sb800) != port)
-		outb_p((smba_en_lo & ~piix4_port_mask_sb800) | port,
-		       SB800_PIIX4_SMB_IDX + 1);
+	prev_port = piix4_sb800_port_sel(adapdata->port);
 
 	retval = piix4_access(adap, addr, flags, read_write,
 			      command, size, data);
 
-	outb_p(smba_en_lo, SB800_PIIX4_SMB_IDX + 1);
+	piix4_sb800_port_sel(prev_port);
 
 	/* Release the semaphore */
 	outb_p(smbslvcnt | 0x20, SMBSLVCNT);
-- 
2.30.2

[PATCH v5 6/9] i2c: piix4: Add EFCH MMIO support to region request and release

[Terry Bowman]

EFCH cd6h/cd7h port I/O may no longer be available on later AMD
processors and it is recommended to use MMIO instead. Update the
request and release functions to support MMIO.

MMIO request/release and mmapping require details during cleanup.
Add a MMIO configuration structure containing resource and vaddress
details for mapping the region, accessing the region, and releasing
the region.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/i2c/busses/i2c-piix4.c | 68 +++++++++++++++++++++++++++++-----
 1 file changed, 59 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index f4a262a7d199..d06998f7b031 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -98,6 +98,9 @@
 #define SB800_PIIX4_PORT_IDX_MASK_KERNCZ	0x18
 #define SB800_PIIX4_PORT_IDX_SHIFT_KERNCZ	3
 
+#define SB800_PIIX4_FCH_PM_ADDR			0xFED80300
+#define SB800_PIIX4_FCH_PM_SIZE			8
+
 /* insmod parameters */
 
 /* If force is set to anything different from 0, we forcibly enable the
@@ -156,6 +159,12 @@ static const char *piix4_main_port_names_sb800[PIIX4_MAX_ADAPTERS] = {
 };
 static const char *piix4_aux_port_name_sb800 = " port 1";
 
+struct sb800_mmio_cfg {
+	void __iomem *addr;
+	struct resource *res;
+	bool use_mmio;
+};
+
 struct i2c_piix4_adapdata {
 	unsigned short smba;
 
@@ -163,10 +172,40 @@ struct i2c_piix4_adapdata {
 	bool sb800_main;
 	bool notify_imc;
 	u8 port;		/* Port number, shifted */
+	struct sb800_mmio_cfg mmio_cfg;
 };
 
-static int piix4_sb800_region_request(struct device *dev)
+static int piix4_sb800_region_request(struct device *dev,
+				      struct sb800_mmio_cfg *mmio_cfg)
 {
+	if (mmio_cfg->use_mmio) {
+		struct resource *res;
+		void __iomem *addr;
+
+		res = request_mem_region_muxed(SB800_PIIX4_FCH_PM_ADDR,
+					       SB800_PIIX4_FCH_PM_SIZE,
+					       "sb800_piix4_smb");
+		if (!res) {
+			dev_err(dev,
+				"SMBus base address memory region 0x%x already in use.\n",
+				SB800_PIIX4_FCH_PM_ADDR);
+			return -EBUSY;
+		}
+
+		addr = ioremap(SB800_PIIX4_FCH_PM_ADDR,
+			       SB800_PIIX4_FCH_PM_SIZE);
+		if (!addr) {
+			release_resource(res);
+			dev_err(dev, "SMBus base address mapping failed.\n");
+			return -ENOMEM;
+		}
+
+		mmio_cfg->res = res;
+		mmio_cfg->addr = addr;
+
+		return 0;
+	}
+
 	if (!request_muxed_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE,
 				  "sb800_piix4_smb")) {
 		dev_err(dev,
@@ -178,8 +217,15 @@ static int piix4_sb800_region_request(struct device *dev)
 	return 0;
 }
 
-static void piix4_sb800_region_release(struct device *dev)
+static void piix4_sb800_region_release(struct device *dev,
+				       struct sb800_mmio_cfg *mmio_cfg)
 {
+	if (mmio_cfg->use_mmio) {
+		iounmap(mmio_cfg->addr);
+		release_resource(mmio_cfg->res);
+		return;
+	}
+
 	release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
 }
 
@@ -288,11 +334,13 @@ static int piix4_setup_sb800_smba(struct pci_dev *PIIX4_dev,
 				  u8 *smb_en_status,
 				  unsigned short *piix4_smba)
 {
+	struct sb800_mmio_cfg mmio_cfg;
 	u8 smba_en_lo;
 	u8 smba_en_hi;
 	int retval;
 
-	retval = piix4_sb800_region_request(&PIIX4_dev->dev);
+	mmio_cfg.use_mmio = 0;
+	retval = piix4_sb800_region_request(&PIIX4_dev->dev, &mmio_cfg);
 	if (retval)
 		return retval;
 
@@ -301,7 +349,7 @@ static int piix4_setup_sb800_smba(struct pci_dev *PIIX4_dev,
 	outb_p(smb_en + 1, SB800_PIIX4_SMB_IDX);
 	smba_en_hi = inb_p(SB800_PIIX4_SMB_IDX + 1);
 
-	piix4_sb800_region_release(&PIIX4_dev->dev);
+	piix4_sb800_region_release(&PIIX4_dev->dev, &mmio_cfg);
 
 	if (!smb_en) {
 		*smb_en_status = smba_en_lo & 0x10;
@@ -328,6 +376,7 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 	unsigned short piix4_smba;
 	u8 smb_en, smb_en_status, port_sel;
 	u8 i2ccfg, i2ccfg_offset = 0x10;
+	struct sb800_mmio_cfg mmio_cfg;
 	int retval;
 
 	/* SB800 and later SMBus does not support forcing address */
@@ -407,8 +456,9 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
 		}
 	} else {
-		retval = piix4_sb800_region_request(&PIIX4_dev->dev);
-		if (retval)
+		mmio_cfg.use_mmio = 0;
+		retval = piix4_sb800_region_request(&PIIX4_dev->dev, &mmio_cfg);
+		if (retval) {
 			release_region(piix4_smba, SMBIOSIZE);
 			return retval;
 		}
@@ -420,7 +470,7 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 				       SB800_PIIX4_PORT_IDX;
 		piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK;
 		piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
-		piix4_sb800_region_release(&PIIX4_dev->dev);
+		piix4_sb800_region_release(&PIIX4_dev->dev, &mmio_cfg);
 	}
 
 	dev_info(&PIIX4_dev->dev,
@@ -731,7 +781,7 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
 	u8 prev_port;
 	int retval;
 
-	retval = piix4_sb800_region_request(&adap->dev);
+	retval = piix4_sb800_region_request(&adap->dev, &adapdata->mmio_cfg);
 	if (retval)
 		return retval;
 
@@ -802,7 +852,7 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
 		piix4_imc_wakeup();
 
 release:
-	piix4_sb800_region_release(&adap->dev);
+	piix4_sb800_region_release(&adap->dev, &adapdata->mmio_cfg);
 	return retval;
 }
 
-- 
2.30.2

[PATCH v5 7/9] i2c: piix4: Add EFCH MMIO support to SMBus base address detect

[Terry Bowman]

The EFCH SMBus controller's base address is determined using details in
FCH::PM::DECODEEN[smbusasfiobase] and FCH::PM::DECODEEN[smbusasfioen].These
register fields were accessed using cd6h/cd7h port I/O. cd6h/cd7h port I/O
is no longer available in later AMD processors. Change base address
detection to use MMIO instead of port I/O cd6h/cd7h.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/i2c/busses/i2c-piix4.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index d06998f7b031..7f312177eb27 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -344,10 +344,15 @@ static int piix4_setup_sb800_smba(struct pci_dev *PIIX4_dev,
 	if (retval)
 		return retval;
 
-	outb_p(smb_en, SB800_PIIX4_SMB_IDX);
-	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
-	outb_p(smb_en + 1, SB800_PIIX4_SMB_IDX);
-	smba_en_hi = inb_p(SB800_PIIX4_SMB_IDX + 1);
+	if (mmio_cfg.use_mmio) {
+		smba_en_lo = ioread8(mmio_cfg.addr);
+		smba_en_hi = ioread8(mmio_cfg.addr + 1);
+	} else {
+		outb_p(smb_en, SB800_PIIX4_SMB_IDX);
+		smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
+		outb_p(smb_en + 1, SB800_PIIX4_SMB_IDX);
+		smba_en_hi = inb_p(SB800_PIIX4_SMB_IDX + 1);
+	}
 
 	piix4_sb800_region_release(&PIIX4_dev->dev, &mmio_cfg);
 
-- 
2.30.2

[PATCH v5 8/9] i2c: piix4: Add EFCH MMIO support for SMBus port select

[Terry Bowman]

AMD processors include registers capable of selecting between 2 SMBus
ports. Port selection is made during each user access by writing to
FCH::PM::DECODEEN[smbus0sel]. Change the driver to use MMIO during
SMBus port selection because cd6h/cd7h port I/O is not available on
later AMD processors.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/i2c/busses/i2c-piix4.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 7f312177eb27..4789fc9ad270 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -753,10 +753,19 @@ static void piix4_imc_wakeup(void)
 	release_region(KERNCZ_IMC_IDX, 2);
 }
 
-static int piix4_sb800_port_sel(u8 port)
+static int piix4_sb800_port_sel(u8 port, struct sb800_mmio_cfg *mmio_cfg)
 {
 	u8 smba_en_lo, val;
 
+	if (mmio_cfg->use_mmio) {
+		smba_en_lo = ioread8(mmio_cfg->addr + piix4_port_sel_sb800);
+		val = (smba_en_lo & ~piix4_port_mask_sb800) | port;
+		if (smba_en_lo != val)
+			iowrite8(val, mmio_cfg->addr + piix4_port_sel_sb800);
+
+		return (smba_en_lo & piix4_port_mask_sb800);
+	}
+
 	outb_p(piix4_port_sel_sb800, SB800_PIIX4_SMB_IDX);
 	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
 
@@ -843,12 +852,12 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
 		}
 	}
 
-	prev_port = piix4_sb800_port_sel(adapdata->port);
+	prev_port = piix4_sb800_port_sel(adapdata->port, &adapdata->mmio_cfg);
 
 	retval = piix4_access(adap, addr, flags, read_write,
 			      command, size, data);
 
-	piix4_sb800_port_sel(prev_port);
+	piix4_sb800_port_sel(prev_port, &adapdata->mmio_cfg);
 
 	/* Release the semaphore */
 	outb_p(smbslvcnt | 0x20, SMBSLVCNT);
-- 
2.30.2

[PATCH v5 9/9] i2c: piix4: Enable EFCH MMIO for Family 17h+

[Terry Bowman]

Enable EFCH MMIO using check for SMBus PCI revision ID value 0x51 or
greater. This PCI revision ID check will enable family 17h and future
AMD processors with the same EFCH SMBus controller HW.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/i2c/busses/i2c-piix4.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 4789fc9ad270..ac8e7d60672a 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -229,6 +229,18 @@ static void piix4_sb800_region_release(struct device *dev,
 	release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
 }
 
+static bool piix4_sb800_use_mmio(struct pci_dev *PIIX4_dev)
+{
+	/*
+	 * cd6h/cd7h port I/O accesses can be disabled on AMD processors
+	 * w/ SMBus PCI revision ID 0x51 or greater. MMIO is supported on
+	 * the same processors and is the recommended access method.
+	 */
+	return (PIIX4_dev->vendor == PCI_VENDOR_ID_AMD &&
+		PIIX4_dev->device == PCI_DEVICE_ID_AMD_KERNCZ_SMBUS &&
+		PIIX4_dev->revision >= 0x51);
+}
+
 static int piix4_setup(struct pci_dev *PIIX4_dev,
 		       const struct pci_device_id *id)
 {
@@ -339,7 +351,7 @@ static int piix4_setup_sb800_smba(struct pci_dev *PIIX4_dev,
 	u8 smba_en_hi;
 	int retval;
 
-	mmio_cfg.use_mmio = 0;
+	mmio_cfg.use_mmio = piix4_sb800_use_mmio(PIIX4_dev);
 	retval = piix4_sb800_region_request(&PIIX4_dev->dev, &mmio_cfg);
 	if (retval)
 		return retval;
@@ -461,7 +473,7 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
 		}
 	} else {
-		mmio_cfg.use_mmio = 0;
+		mmio_cfg.use_mmio = piix4_sb800_use_mmio(PIIX4_dev);
 		retval = piix4_sb800_region_request(&PIIX4_dev->dev, &mmio_cfg);
 		if (retval) {
 			release_region(piix4_smba, SMBIOSIZE);
@@ -944,6 +956,7 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
 		return -ENOMEM;
 	}
 
+	adapdata->mmio_cfg.use_mmio = piix4_sb800_use_mmio(dev);
 	adapdata->smba = smba;
 	adapdata->sb800_main = sb800_main;
 	adapdata->port = port << piix4_port_shift_sb800;
-- 
2.30.2

Re: [PATCH v5 0/9] i2c: piix4: Replace cd6h/cd7h port I/O accesses with MMIO accesses

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 7435 bytes --]

On Wed, Feb 09, 2022 at 11:27:08AM -0600, Terry Bowman wrote:
> This series changes the piix4_smbus driver's cd6h/cd7h port I/O accesses
> to use MMIO instead. This is necessary because cd6h/cd7h port I/O may be
> disabled on later AMD processors.
> 
> This series includes patches with MMIO accesses to register
> FCH::PM::DECODEEN. The same register is also accessed by the sp5100_tco
> driver.[1] Synchronization to the MMIO register is required in both
> drivers.
> 
> The first patch creates a macro to request MMIO region using the 'muxed'
> retry logic. This is used in patch 6 to synchronize accesses to EFCH MMIO.
> 
> The second patch replaces a hardcoded region size with a #define. This is
> to improve maintainability and was requested from v2 review.
> 
> The third patch moves duplicated region request/release code into
> functions. This locates related code into functions and reduces code line
> count. This will also make adding MMIO support in patch 6 easier.
> 
> The fourth patch moves SMBus controller address detection into a function. 
> This is in preparation for adding MMIO region support.
> 
> The fifth patch moves EFCH port selection into a function. This is in
> preparation for adding MMIO region support.
> 
> The sixth patch adds MMIO support for region requesting/releasing and
> mapping. This is necessary for using MMIO to detect SMBus controller
> address, enable SMBbus controller region, and control the port select.
> 
> The seventh patch updates the SMBus controller address detection to support
> using MMIO. This is necessary because the driver accesses register
> FCH::PM::DECODEEN during initialization and only available using MMIO on
> later AMD processors.
> 
> The eighth patch updates the SMBus port selection to support MMIO. This is
> required because port selection control resides in the
> FCH::PM::DECODEEN[smbus0sel] and is only accessible using MMIO on later AMD
> processors.
> 
> The ninth patch enables the EFCH MMIO functionality added earlier in this
> series. The SMBus controller's PCI revision ID is used to check if EFCH
> MMIO is supported by HW and should be enabled in the driver.
> 
> Based on v5.17-rc2.
> 
> Testing:
>   Tested on family 19h using:
>     i2cdetect -y 0
>     i2cdetect -y 2
> 
>   - Results using v5.16 and this pachset applied. Below
>     shows the devices detected on the busses:
>     
>     # i2cdetect -y 0 
>          0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
>     00:                         -- -- -- -- -- -- -- -- 
>     10: 10 11 -- -- -- -- -- -- 18 -- -- -- -- -- -- -- 
>     20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
>     30: 30 -- -- -- -- 35 36 -- -- -- -- -- -- -- -- -- 
>     40: -- -- -- -- -- -- -- -- -- -- 4a -- -- -- -- -- 
>     50: 50 -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
>     60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
>     70: -- -- -- 73 -- -- -- --                         
>     # i2cdetect -y 2
>          0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
>     00:                         -- -- -- -- -- -- -- -- 
>     10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
>     20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
>     30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
>     40: -- -- -- -- -- -- -- -- -- -- -- -- 4c -- -- -- 
>     50: -- 51 -- -- 54 -- -- -- -- -- -- -- -- -- -- -- 
>     60: 60 -- -- 63 -- -- 66 -- -- -- -- 6b -- -- 6e -- 
>     70: 70 71 72 73 74 75 -- 77
> 
>   Also tested using sp5100_tco submitted series listed below.[1]
>   I applied the sp5100_tco v4 series and ran:
>     cat  >> /dev/watchdog
> 
> [1] sp5100_tco v4 patchset can be found here:
> Link: https://lore.kernel.org/linux-watchdog/20220130191225.303115-1-terry.bowman@amd.com/
> 
> Changes in v5:
>  - Use request/release helper function for sb800 device in
>    piix4_setup_sb800(). Patch 3. (Jean Delvare)     
>  - Revert 'piix4_smba' variable definition ordering back as it was in
>    piix4_setup_sb800(). Patch 4. (Jean Delvare)
>  - Add newline after piix4_sb800_port_sel(). Patch 5. (Jean Delvare)
>  - Remove unnecessary initialization in piix4_add_adapter(). Patch 6.
>    (Jean Delvare)
>  - Remove unnecessary #define AMD_PCI_SMBUS_REVISION_MMIO. Patch 9.
>    (Jean Delvare)
>  - Add description for 0x51 constant moved in the above item. This is
>    in piix4_sb800_use_mmio(). Patch 9. (Andy Shevchenko)
>  - Rebase to v5.17-rc2. (Andy Shevchenko)
>  - Update patch 9 description. (Terry Bowman)
>  
> Changes in v4:
>  - Changed request_muxed_mem_region() macro to request_mem_region_muxed()
>    in patch 1. (Andy Shevchenko)
>  - Removed unnecessary newline where possible in calls to
>    request_muxed_region() in patch 2. (Terry Bowman)
>  - Changed piix4_sb800_region_setup() to piix4_sb800_region_request().
>    Patch 3. (Jean Delvare)
>  - Reordered piix4_setup_sb800() local variables from longest name length
>    to shortest name length. Patch 4. (Terry Bowman)
>  - Changed piix4_sb800_region_request() and piix4_sb800_region_release() by
>    adding early return() to remove 'else' improving readability. Patch 6.
>    (Terry Bowman)
>  - Removed iowrite32(ioread32(...), ...). Unnecessary because MMIO is
>    already enabled. (Terry Bowman)
>  - Refactored piix4_sb800_port_sel() to simplify the 'if' statement using
>    temp variable. Patch 8. (Terry Bowman)
>  - Added mmio_cfg.use_mmio assignment in piix4_add_adapter(). This is
>    needed for calls to piix4_sb800_port_sel() after initialization during
>    normal operation. Patch 9. (Terry Bowman)
>  
> Changes in v3:
>  - Added request_muxed_mem_region() patch (Wolfram, Guenter)
>  - Reduced To/Cc list length. (Andy)
>  
> Changes in v2:
>  - Split single patch. (Jean Delvare)
>  - Replace constant 2 with SB800_PIIX4_SMB_MAP_SIZE where appropriate.
>    (Jean Delvare)
>  - Shorten SB800_PIIX4_FCH_PM_DECODEEN_MMIO_EN name length to
>    SB800_PIIX4_FCH_PM_DECODEEN_MMIO. (Jean Delvare)
>  - Change AMD_PCI_SMBUS_REVISION_MMIO from 0x59 to 0x51. (Terry Bowman)
>  - Change piix4_sb800_region_setup() to piix4_sb800_region_request().
>    (Jean Delvare)
>  - Change 'SMB' text in  logging to 'SMBus' (Jean Delvare)
>  - Remove unnecessary NULL assignment in piix4_sb800_region_release().
>    (Jean Delvare)
>  - Move 'u8' variable definitions to single line. (Jean Delvare)
>  - Hardcode piix4_setup_sb800_smba() return value to 0 since it is always
>    0. (Jean Delvare)
> 
> Terry Bowman (9):
>   kernel/resource: Introduce request_mem_region_muxed()
>   i2c: piix4: Replace hardcoded memory map size with a #define
>   i2c: piix4: Move port I/O region request/release code into functions
>   i2c: piix4: Move SMBus controller base address detect into function
>   i2c: piix4: Move SMBus port selection into function
>   i2c: piix4: Add EFCH MMIO support to region request and release
>   i2c: piix4: Add EFCH MMIO support to SMBus base address detect
>   i2c: piix4: Add EFCH MMIO support for SMBus port select
>   i2c: piix4: Enable EFCH MMIO for Family 17h+
> 

Applied the series to for-next, thank you Terry for keeping at it and
Jean and Andy for the review. I'll send the pull request containing the
ioport update to the WDT maintainers now. All further changes should be
based on top of this now.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 3/9] i2c: piix4: Move port I/O region request/release code into functions

[Jean Delvare]

On Wed, 09 Feb 2022 11:27:11 -0600, Terry Bowman wrote:
> Move duplicated region request and release code into a function. Move is
> in preparation for following MMIO changes.
> 
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Reviewed-by: Jean Delvare <jdelvare@suse.de>
> ---
>  drivers/i2c/busses/i2c-piix4.c | 48 ++++++++++++++++++++++------------
>  1 file changed, 31 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> index 3ff68967034e..cc488b1e92c3 100644
> --- a/drivers/i2c/busses/i2c-piix4.c
> +++ b/drivers/i2c/busses/i2c-piix4.c
> @@ -165,6 +165,24 @@ struct i2c_piix4_adapdata {
>  	u8 port;		/* Port number, shifted */
>  };
>  
> +static int piix4_sb800_region_request(struct device *dev)
> +{
> +	if (!request_muxed_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE,
> +				  "sb800_piix4_smb")) {
> +		dev_err(dev,
> +			"SMBus base address index region 0x%x already in use.\n",
> +			SB800_PIIX4_SMB_IDX);
> +		return -EBUSY;
> +	}
> +
> +	return 0;
> +}
> +
> +static void piix4_sb800_region_release(struct device *dev)
> +{
> +	release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
> +}
> +
>  static int piix4_setup(struct pci_dev *PIIX4_dev,
>  		       const struct pci_device_id *id)
>  {
> @@ -270,6 +288,7 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
>  	unsigned short piix4_smba;
>  	u8 smba_en_lo, smba_en_hi, smb_en, smb_en_status, port_sel;
>  	u8 i2ccfg, i2ccfg_offset = 0x10;
> +	int retval;
>  
>  	/* SB800 and later SMBus does not support forcing address */
>  	if (force || force_addr) {
> @@ -291,20 +310,16 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
>  	else
>  		smb_en = (aux) ? 0x28 : 0x2c;
>  
> -	if (!request_muxed_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE,
> -				  "sb800_piix4_smb")) {
> -		dev_err(&PIIX4_dev->dev,
> -			"SMB base address index region 0x%x already in use.\n",
> -			SB800_PIIX4_SMB_IDX);
> -		return -EBUSY;
> -	}
> +	retval = piix4_sb800_region_request(&PIIX4_dev->dev);
> +	if (retval)
> +		return retval;
>  
>  	outb_p(smb_en, SB800_PIIX4_SMB_IDX);
>  	smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
>  	outb_p(smb_en + 1, SB800_PIIX4_SMB_IDX);
>  	smba_en_hi = inb_p(SB800_PIIX4_SMB_IDX + 1);
>  
> -	release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
> +	piix4_sb800_region_release(&PIIX4_dev->dev);
>  
>  	if (!smb_en) {
>  		smb_en_status = smba_en_lo & 0x10;
> @@ -373,11 +388,10 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
>  			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
>  		}
>  	} else {
> -		if (!request_muxed_region(SB800_PIIX4_SMB_IDX,
> -					  SB800_PIIX4_SMB_MAP_SIZE,
> -					  "sb800_piix4_smb")) {
> +		retval = piix4_sb800_region_request(&PIIX4_dev->dev);
> +		if (retval)

Missing curly brace here, breaks the build.

>  			release_region(piix4_smba, SMBIOSIZE);
> -			return -EBUSY;
> +			return retval;
>  		}
>  
>  		outb_p(SB800_PIIX4_PORT_IDX_SEL, SB800_PIIX4_SMB_IDX);
> @@ -387,7 +401,7 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
>  				       SB800_PIIX4_PORT_IDX;
>  		piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK;
>  		piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
> -		release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
> +		piix4_sb800_region_release(&PIIX4_dev->dev);
>  	}
>  
>  	dev_info(&PIIX4_dev->dev,
> @@ -685,9 +699,9 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
>  	u8 port;
>  	int retval;
>  
> -	if (!request_muxed_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE,
> -				  "sb800_piix4_smb"))
> -		return -EBUSY;
> +	retval = piix4_sb800_region_request(&adap->dev);
> +	if (retval)
> +		return retval;
>  
>  	/* Request the SMBUS semaphore, avoid conflicts with the IMC */
>  	smbslvcnt  = inb_p(SMBSLVCNT);
> @@ -762,7 +776,7 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
>  		piix4_imc_wakeup();
>  
>  release:
> -	release_region(SB800_PIIX4_SMB_IDX, SB800_PIIX4_SMB_MAP_SIZE);
> +	piix4_sb800_region_release(&adap->dev);
>  	return retval;
>  }
>  


-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v5 0/9] i2c: piix4: Replace cd6h/cd7h port I/O accesses with MMIO accesses

[Jean Delvare]

On Wed, 09 Feb 2022 11:27:08 -0600, Terry Bowman wrote:
> This series changes the piix4_smbus driver's cd6h/cd7h port I/O accesses
> to use MMIO instead. This is necessary because cd6h/cd7h port I/O may be
> disabled on later AMD processors.
> 
> This series includes patches with MMIO accesses to register
> FCH::PM::DECODEEN. The same register is also accessed by the sp5100_tco
> driver.[1] Synchronization to the MMIO register is required in both
> drivers.
> (...)

Except for the curly brace issue in patch 3, all looks good, so I
confirm my

Reviewed-by: Jean Delvare <jdelvare@suse.de>
Tested-by: Jean Delvare <jdelvare@suse.de>

on all i2c-piix4 patches.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v5 3/9] i2c: piix4: Move port I/O region request/release code into functions

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 305 bytes --]

> > +		if (retval)
> 
> Missing curly brace here, breaks the build.

Bummer, need to check why this wasn't found by my build-testing.

I fixed it up and also rebased patch 6 to my change. Terry, please have
a look that I did everything correctly once I push out later today.

Thanks everyone!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 3/9] i2c: piix4: Move port I/O region request/release code into functions

[Terry Bowman]

Hi Wolfram and Jean,

On 2/11/22 08:42, Wolfram Sang wrote:
>>> +		if (retval)
>>
>> Missing curly brace here, breaks the build.
> 
> Bummer, need to check why this wasn't found by my build-testing.
> 
> I fixed it up and also rebased patch 6 to my change. Terry, please have
> a look that I did everything correctly once I push out later today.
> 
> Thanks everyone!
> 
I need to look at how I sent that out. I will look for the fix in 
i2c/for-next. Thanks for the help and review. 

Re: [PATCH v5 3/9] i2c: piix4: Move port I/O region request/release code into functions

[Terry Bowman]

Hi Wolfram an Jean,

On 2/11/22 09:00, Terry Bowman wrote:
> Hi Wolfram and Jean,
> 
> On 2/11/22 08:42, Wolfram Sang wrote:
>>>> +		if (retval)
>>>
>>> Missing curly brace here, breaks the build.
>>
>> Bummer, need to check why this wasn't found by my build-testing.
>>
>> I fixed it up and also rebased patch 6 to my change. Terry, please have
>> a look that I did everything correctly once I push out later today.
>>
>> Thanks everyone!
>>
> I need to look at how I sent that out. I will look for the fix in 
> i2c/for-next. Thanks for the help and review. 

I verified the fix looks good. From commit a3325d225b00 on i2c/for-next:

@@ -373,11 +388,10 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
 		}
 	} else {
-		if (!request_muxed_region(SB800_PIIX4_SMB_IDX,
-					  SB800_PIIX4_SMB_MAP_SIZE,
-					  "sb800_piix4_smb")) {
+		retval = piix4_sb800_region_request(&PIIX4_dev->dev);
+		if (retval) {
 			release_region(piix4_smba, SMBIOSIZE);
-			return -EBUSY;
+			return retval;
 		}

Re: [PATCH v5 3/9] i2c: piix4: Move port I/O region request/release code into functions

[Jean Delvare]

On Fri, 11 Feb 2022 15:42:34 +0100, Wolfram Sang wrote:
> > > +		if (retval)  
> > 
> > Missing curly brace here, breaks the build.  
> 
> Bummer, need to check why this wasn't found by my build-testing.

Maybe you build-tested the series as a whole but not individual
patches? The series did build fine, as the missing curly brace was
added back in a later patch.

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v5 3/9] i2c: piix4: Move port I/O region request/release code into functions

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]


> Maybe you build-tested the series as a whole but not individual
> patches? The series did build fine, as the missing curly brace was
> added back in a later patch.

It wasn't that, but another gory detail. Updating my build tests is in
the works. But thanks for the help!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]