From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Lutomirski Subject: Re: [PATCH v4 2/2] i2c_imc: New driver for Intel's iMC, found on LGA2011 chips Date: Mon, 29 Jul 2013 13:32:00 -0700 Message-ID: References: <20130718213331.GA6237@roeck-us.net> <20130718224008.GA7314@roeck-us.net> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Return-path: In-Reply-To: <20130718224008.GA7314-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> Sender: linux-i2c-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Guenter Roeck Cc: lm-sensors-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jean Delvare , James Ralston List-Id: linux-i2c@vger.kernel.org On Thu, Jul 18, 2013 at 3:40 PM, Guenter Roeck wrote: > On Thu, Jul 18, 2013 at 02:37:40PM -0700, Andy Lutomirski wrote: >> On Thu, Jul 18, 2013 at 2:33 PM, Guenter Roeck wrote: >> > On Thu, Jul 18, 2013 at 11:36:54AM -0700, Andy Lutomirski wrote: >> >> Sandy Bridge Xeon and Extreme chips have integrated memory controllers >> >> with (rather limited) onboard SMBUS masters. This driver gives access >> >> to the bus. >> >> >> >> Signed-off-by: Andy Lutomirski >> >> --- >> >> drivers/i2c/busses/Kconfig | 15 ++ >> >> drivers/i2c/busses/Makefile | 1 + >> >> drivers/i2c/busses/i2c-imc.c | 548 +++++++++++++++++++++++++++++++++++++++++++ >> >> 3 files changed, 564 insertions(+) >> >> create mode 100644 drivers/i2c/busses/i2c-imc.c >> >> >> >> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig >> >> index e837f0e..7636782 100644 >> >> --- a/drivers/i2c/busses/Kconfig >> >> +++ b/drivers/i2c/busses/Kconfig >> >> @@ -137,6 +137,21 @@ config I2C_DIMM_BUS >> >> tristate >> >> default n >> >> >> >> +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 >> >> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile >> >> index 226bb2e..790b63d 100644 >> >> --- a/drivers/i2c/busses/Makefile >> >> +++ b/drivers/i2c/busses/Makefile >> >> @@ -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 >> >> diff --git a/drivers/i2c/busses/i2c-imc.c b/drivers/i2c/busses/i2c-imc.c >> >> new file mode 100644 >> >> index 0000000..9643aeb >> >> --- /dev/null >> >> +++ b/drivers/i2c/busses/i2c-imc.c >> > >> > [ ... ] >> > >> >> + >> >> + i2c_scan_dimm_bus(&ch->adapter); >> >> + >> > Wonder if this should (can) be part of the infrastructure, eg by introducing >> > I2C_CLASS_DIMM. >> >> If so, it'll have to work differently than the current class bits. >> All they do is determine which drivers get detected, and that >> detection happens by completely generic means that's not really >> applicable to DIMMs (and doesn't work on this bus). >> > In i2c_detect_address: > > if (adap->class & I2C_CLASS_DIMM) > return i2c_detect_dimm(); > > > or in i2c_default_probe, call a dimm specific probe function and then > depend on the device detect functions (if that is possible). > > Sure, the i2c core code would have to change a bit to accomodate the new > I2C_CLASS_DIMM, but I personally would prefer that to calling > the detect function from the adapter driver. > > Personal preference, though; this ultimately depends on the i2c maintainers. > > Thanks, > Guenter Jean? I have some driver changes pending for v5, but I'd like to nail down whether the current scanning approach is acceptable first. Thanks, Andy