Skip to contentSkip to conversation

iMC SMBUS and DIMM bus probing

PATCH Superseded Andy Lutomirski 1 participant 6 messages Sep 28, 2013 – Nov 8, 2013

Subsystems (4)

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v5 cover iMC SMBUS and DIMM bus probing

Intel LGA2011 machines have dedicated SMBUS controllers for DIMM sockets. Because they're dedicated, they can be safely and accurately probed, since all devices on them are known to be attached to DIMMs. The devices found are:
- SPD EEPROMs
- TSODs (Temperature Sensor on DIMMs)
- Other interesting things, with drivers hopefully to follow...

This patch series adds a simple generic layer for probing for DIMMs over SMBUS and an i2c bus driver for the iMC controller found on Intel LGA2011 chips.

It now uses only modern infrastructure -- new-style I2C probing and the at24 (instead of eeprom) driver.

I've tested this on a Core i7 Extreme and on a Xeon E5 server.

Patches 1-3 are useful even without patch 4. I'm still hoping for feedback on patch 4.

Changes from v4:
- Added the sb_edac changes -- i2c_imc and sb_edac can now coexist
- Added some paranoid race detection.
- The driver now confirms its ability to claim software control of the SMBUS master. This prevents unpleasant problems on systems that enable CLTT (closed loop thermal throttling).
- Reordered the patches so that the DIMM bus code is last.

Changes from v3:
- Dropped redundant "tsod" driver
- Dropped eeprom modalias
- Switched to probing for the "eeprom" and "jc42"

Andy Lutomirski (4):
Move Intel SNB device ids from sb_edac to pci_ids.h
sb_edac: Claim a different PCI device
i2c_imc: New driver for Intel's iMC, found on LGA2011 chips i2c, i2c_imc: Add DIMM bus code

 drivers/edac/sb_edac.c        |  32 +--
 drivers/i2c/busses/Kconfig    |  19 ++
 drivers/i2c/busses/Makefile   |   5 +
 drivers/i2c/busses/dimm-bus.c |  97 ++++++++
 drivers/i2c/busses/i2c-imc.c  | 562 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c/dimm-bus.h  |  24 ++
 include/linux/pci_ids.h       |  15 ++
 7 files changed, 723 insertions(+), 31 deletions(-)
 create mode 100644 drivers/i2c/busses/dimm-bus.c
 create mode 100644 drivers/i2c/busses/i2c-imc.c
 create mode 100644 include/linux/i2c/dimm-bus.h
Signature · 1 line
1.8.3.1
#1 lore raw

PATCH v5 1/4 Move Intel SNB device ids from sb_edac to pci_ids.h

The i2c_imc driver will use two of them, and moving only part of the list seems messier.

Cc Mauro Carvalho Chehab <[email protected]>
Cc Rui Wang <[email protected]>
---
 drivers/edac/sb_edac.c  | 30 ------------------------------
 include/linux/pci_ids.h | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 30 deletions(-)
drivers/edac/sb_edac.c +0 -30
index e04462b..4fac6f5 100644
@@ -52,36 +52,6 @@ static int probed;
 #define GET_BITFIELD(v, lo, hi)	\
 	(((v) & ((1ULL << ((hi) - (lo) + 1)) - 1) << (lo)) >> (lo))
 
-/*
- * sbridge Memory Controller Registers
- */
-
-/*
- * FIXME: For now, let's order by device function, as it makes
- * easier for driver's development process. This table should be
- * moved to pci_id.h when submitted upstream
- */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD0	0x3cf4	/* 12.6 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1	0x3cf6	/* 12.7 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_BR		0x3cf5	/* 13.6 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0	0x3ca0	/* 14.0 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA	0x3ca8	/* 15.0 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_RAS	0x3c71	/* 15.1 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD0	0x3caa	/* 15.2 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD1	0x3cab	/* 15.3 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD2	0x3cac	/* 15.4 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD3	0x3cad	/* 15.5 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_DDRIO	0x3cb8	/* 17.0 */
-
-	/*
-	 * Currently, unused, but will be needed in the future
-	 * implementations, as they hold the error counters
-	 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR0	0x3c72	/* 16.2 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR1	0x3c73	/* 16.3 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR2	0x3c76	/* 16.6 */
-#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR3	0x3c77	/* 16.7 */
-
 /* Devices 12 Function 6, Offsets 0x80 to 0xcc */
 static const u32 dram_rule[] = {
 	0x80, 0x88, 0x90, 0x98, 0xa0,
include/linux/pci_ids.h +15 -0
index d1fe5d0..fb8932b 100644
@@ -2811,7 +2811,22 @@
 #define PCI_DEVICE_ID_INTEL_UNC_R2PCIE	0x3c43
 #define PCI_DEVICE_ID_INTEL_UNC_R3QPI0	0x3c44
 #define PCI_DEVICE_ID_INTEL_UNC_R3QPI1	0x3c45
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_RAS	0x3c71	/* 15.1 */
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR0	0x3c72	/* 16.2 */
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR1	0x3c73	/* 16.3 */
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR2	0x3c76	/* 16.6 */
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_ERR3	0x3c77	/* 16.7 */
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0	0x3ca0	/* 14.0 */
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA	0x3ca8	/* 15.0 */
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD0	0x3caa	/* 15.2 */
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD1	0x3cab	/* 15.3 */
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD2	0x3cac	/* 15.4 */
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TAD3	0x3cad	/* 15.5 */
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_DDRIO	0x3cb8	/* 17.0 */
 #define PCI_DEVICE_ID_INTEL_JAKETOWN_UBOX	0x3ce0
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD0	0x3cf4	/* 12.6 */
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_BR		0x3cf5	/* 13.6 */
+#define PCI_DEVICE_ID_INTEL_SBRIDGE_SAD1	0x3cf6	/* 12.7 */
 #define PCI_DEVICE_ID_INTEL_IOAT_SNB	0x402f
 #define PCI_DEVICE_ID_INTEL_5100_16	0x65f0
 #define PCI_DEVICE_ID_INTEL_5100_19	0x65f3
Signature · 1 line
1.8.3.1
#2 lore raw

PATCH v5 2/4 sb_edac: Claim a different PCI device

In reply to Andy Lutomirski

sb_edac controls a large number of different PCI functions. Rather than registering as a normal PCI driver for all of them, it registers for just one so that it gets probed and, at probe time, it looks for all the others.

Coincidentally, the device it registers for also contains the SMBUS registers, so the PCI core will refuse to probe both sb_edac and an iMC SMBUS driver. The drivers don't actually conflict, so just change sb_edac's device table to probe a different device.

An alternative fix would be to merge the two drivers, but sb_edac will also refuse to load on non-ECC systems, whereas the i2c_imc is still useful without ECC.

The only user-visible change should be that sb_edac appears to bind a different device.

Cc Mauro Carvalho Chehab <[email protected]>
Cc Rui Wang <[email protected]>
---
 drivers/edac/sb_edac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/edac/sb_edac.c +1 -1
index 4fac6f5..4f30aa7 100644
@@ -338,7 +338,7 @@ static const struct pci_id_table pci_dev_descr_sbridge_table[] = {
  *	pci_device_id	table for which devices we are looking for
  */
 static DEFINE_PCI_DEVICE_TABLE(sbridge_pci_tbl) = {
-	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_TA)},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0)},
 	{0,}			/* 0 terminated list. */
 };
 
Signature · 1 line
1.8.3.1
#3 lore raw

PATCH v5 3/4 i2c_imc: New driver for Intel's iMC, found on LGA2011 chips

In reply to Andy Lutomirski

Sandy Bridge Xeon and Extreme chips have integrated memory controllers with (rather limited) onboard SMBUS masters. This driver gives access to the bus.

---
 drivers/i2c/busses/Kconfig   |  15 ++
 drivers/i2c/busses/Makefile  |   1 +
 drivers/i2c/busses/i2c-imc.c | 559 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 575 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-imc.c
drivers/i2c/busses/Kconfig +15 -0
index dc6dea6..3709540 100644
@@ -134,6 +134,21 @@ config I2C_ISMT
 	  This driver can also be built as a module.  If so, the module will be
 	  called i2c-ismt.
 
+config I2C_IMC
+	tristate "Intel iMC (LGA 2011) SMBus Controller"
+	depends on PCI && X86
+	select I2C_DIMM_BUS
+	help
+	  If you say yes to this option, support will be included for the Intel
+	  Integrated Memory Controller SMBus host controller interface.  This
+	  controller is found on LGA 2011 Xeons and Core i7 Extremes.
+
+	  It is possibly, although unlikely, that the use of this driver will
+	  interfere with your platform's RAM thermal management.
+
+	  This driver can also be built as a module.  If so, the module will be
+	  called i2c-imc.
+
 config I2C_PIIX4
 	tristate "Intel PIIX4 and compatible (ATI/AMD/Serverworks/Broadcom/SMSC)"
 	depends on PCI
drivers/i2c/busses/Makefile +1 -0
index d00997f..d37340a 100644
@@ -15,6 +15,7 @@ obj-$(CONFIG_I2C_AMD8111)	+= i2c-amd8111.o
 obj-$(CONFIG_I2C_I801)		+= i2c-i801.o
 obj-$(CONFIG_I2C_ISCH)		+= i2c-isch.o
 obj-$(CONFIG_I2C_ISMT)		+= i2c-ismt.o
+obj-$(CONFIG_I2C_IMC)		+= i2c-imc.o
 obj-$(CONFIG_I2C_NFORCE2)	+= i2c-nforce2.o
 obj-$(CONFIG_I2C_NFORCE2_S4985)	+= i2c-nforce2-s4985.o
 obj-$(CONFIG_I2C_PIIX4)		+= i2c-piix4.o
drivers/i2c/busses/i2c-imc.c new file +559 -0

559 lines not on this page · raw message

Signature · 1 line
1.8.3.1
#4 lore raw

PATCH v5 4/4 i2c, i2c_imc: Add DIMM bus code

In reply to Andy Lutomirski

Add i2c_scan_dimm_bus to declare that a particular i2c_adapter contains DIMMs. This will probe (and autoload modules!) for useful SMBUS devices that live on DIMMs. i2c_imc calls it.

As more SMBUS-addressable DIMM components become supported, this code can be extended to probe for them.

---
 drivers/i2c/busses/Kconfig    |  4 ++
 drivers/i2c/busses/Makefile   |  4 ++
 drivers/i2c/busses/dimm-bus.c | 97 +++++++++++++++++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-imc.c  |  3 ++
 include/linux/i2c/dimm-bus.h  | 24 +++++++++++
 5 files changed, 132 insertions(+)
 create mode 100644 drivers/i2c/busses/dimm-bus.c
 create mode 100644 include/linux/i2c/dimm-bus.h
drivers/i2c/busses/Kconfig +4 -0
index 3709540..3eca819 100644
@@ -134,6 +134,10 @@ config I2C_ISMT
 	  This driver can also be built as a module.  If so, the module will be
 	  called i2c-ismt.
 
+config I2C_DIMM_BUS
+       tristate
+       default n
+
 config I2C_IMC
 	tristate "Intel iMC (LGA 2011) SMBus Controller"
 	depends on PCI && X86
drivers/i2c/busses/Makefile +4 -0
index d37340a..d4c0e02 100644
@@ -25,6 +25,10 @@ obj-$(CONFIG_I2C_SIS96X)	+= i2c-sis96x.o
 obj-$(CONFIG_I2C_VIA)		+= i2c-via.o
 obj-$(CONFIG_I2C_VIAPRO)	+= i2c-viapro.o
 
+# DIMM busses
+obj-$(CONFIG_I2C_DIMM_BUS)	+= dimm-bus.o
+obj-$(CONFIG_I2C_IMC)		+= i2c-imc.o
+
 # Mac SMBus host controller drivers
 obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
 obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o
drivers/i2c/busses/dimm-bus.c new file +97 -0

97 lines not on this page · raw message

drivers/i2c/busses/i2c-imc.c +3 -0
index c846077..437d8d8 100644
@@ -15,6 +15,7 @@
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#include <linux/i2c/dimm-bus.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
@@ -421,6 +422,8 @@ static int imc_init_channel(struct imc_priv *priv, int i, int socket)
 		return err;
 	}
 
+	i2c_scan_dimm_bus(&ch->adapter);
+
 	return 0;
 }
 
include/linux/i2c/dimm-bus.h new file +24 -0
new file mode 100644
index 0000000..3d44df4
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2013 Andrew Lutomirski <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef _I2C_DIMM_BUS
+#define _I2C_DIMM_BUS
+
+struct i2c_adapter;
+void i2c_scan_dimm_bus(struct i2c_adapter *adapter);
+
+#endif /* _I2C_DIMM_BUS */
Signature · 1 line
1.8.3.1
#5 lore raw
In reply to Andy Lutomirski
Quoted 39 lines · Andy Lutomirski

Intel LGA2011 machines have dedicated SMBUS controllers for DIMM sockets. Because they're dedicated, they can be safely and accurately probed, since all devices on them are known to be attached to DIMMs. The devices found are:
- SPD EEPROMs
- TSODs (Temperature Sensor on DIMMs)
- Other interesting things, with drivers hopefully to follow...

This patch series adds a simple generic layer for probing for DIMMs over SMBUS and an i2c bus driver for the iMC controller found on Intel LGA2011 chips.

It now uses only modern infrastructure -- new-style I2C probing and the at24 (instead of eeprom) driver.

I've tested this on a Core i7 Extreme and on a Xeon E5 server.

Patches 1-3 are useful even without patch 4. I'm still hoping for feedback on patch 4.

Changes from v4:
- Added the sb_edac changes -- i2c_imc and sb_edac can now coexist
- Added some paranoid race detection.
- The driver now confirms its ability to claim software control of the SMBUS master. This prevents unpleasant problems on systems that enable CLTT (closed loop thermal throttling).
- Reordered the patches so that the DIMM bus code is last.

Changes from v3:
- Dropped redundant "tsod" driver
- Dropped eeprom modalias
- Switched to probing for the "eeprom" and "jc42"

Andy Lutomirski (4):
Move Intel SNB device ids from sb_edac to pci_ids.h
sb_edac: Claim a different PCI device
i2c_imc: New driver for Intel's iMC, found on LGA2011 chips i2c, i2c_imc: Add DIMM bus code

Quoted 11 lines
 drivers/edac/sb_edac.c        |  32 +--
 drivers/i2c/busses/Kconfig    |  19 ++
 drivers/i2c/busses/Makefile   |   5 +
 drivers/i2c/busses/dimm-bus.c |  97 ++++++++
 drivers/i2c/busses/i2c-imc.c  | 562 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c/dimm-bus.h  |  24 ++
 include/linux/pci_ids.h       |  15 ++
 7 files changed, 723 insertions(+), 31 deletions(-)
 create mode 100644 drivers/i2c/busses/dimm-bus.c
 create mode 100644 drivers/i2c/busses/i2c-imc.c
 create mode 100644 include/linux/i2c/dimm-bus.h

Any update on these? It would be nice to get them reviewed and applied.

Thanks,
Andy