i2c_imc: New driver, at long last
v2 cover i2c_imc: New driver, at long last
This adds i2c_imc, a driver for the SMBUS lines on DIMM slots on modern Intel server chips. Conceptually, I like it a lot -- it's a driver for a bus for which we know the exact topology a priori. That means that we can actually enumerate the things on the bus reasonably cleanly.
This driver is weird, but I don't think that merging it will cause problems, and I believe that there are Real Users (tm) of this driver.
It has two caveats.
Big caveat: Lots of things like to touch this bus, such as a BMC, the memory controller power management stuff, and maybe even SMM code. Intel forgot to define a way to arbitrate between them, nor are the SMBUS master regs designed to be poked by multiple things at once. The upshot is that loading this driver is generally unsafe. The driver takes some measures to detect contention for the registers and shut itself down, but no one should rely on that.
My understanding is that there's work in ACPI land to define an arbitration mechanism. Once this happens, we can support it in this driver. In the mean time, this driver has actual users, and there are motherboards specifically designed to be used with a driver like this. (I have such a board, and you can even buy them on regular online shopping sutes.)
Therefore, I added a module parameter called allow_unsafe_access. If unset, the driver will refuse to load with an informative message. If set, the driver will pr_warn and load.
Little caveat: This submission only supposts Sandy Bridge. I'd rather get it merged without Ivy Bridge and Haswell support and add them later (should be easy) rather than trying to make the driver support all possible hardware before merging it.
Changes from v1:
- Fix checkpatch license nits.
- Fix MODULE_LICENSE.
- Add missing parentheses to macros.
- Tidy up imc_wait_not_busy.
- Clean up post-transaction-completion handling code.
- Downgrade a bunch of message severities.
- Misc checkpatch stuff.
- Move select I2C_DIMM_BUS where it belongs.
- Improve comments.
Changes from last year:
- This is an updated resubmission after about a year of thumb twiddling.
Andy Lutomirski (2):
i2c_imc: New driver for Intel's iMC, found on LGA2011 chips i2c, i2c_imc: Add DIMM bus code
drivers/i2c/busses/Kconfig | 22 ++ drivers/i2c/busses/Makefile | 5 + drivers/i2c/busses/dimm-bus.c | 107 ++++++++ drivers/i2c/busses/i2c-imc.c | 570 ++++++++++++++++++++++++++++++++++++++++++ include/linux/i2c/dimm-bus.h | 20 ++ 5 files changed, 724 insertions(+) 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
2.3.0
PATCH v2 1/2 i2c_imc: New driver for Intel's iMC, found on LGA2011 chips
Sandy Bridge Xeon and Extreme chips have integrated memory controllers with (rather limited) onboard SMBUS masters. This driver gives access to the bus.
There are various groups working on standardizing a way to arbitrate access to the bus between the OS, SMM firmware, a BMC, hardware thermal control, etc. In the mean time, running this driver is unsafe except under special circumstances. Nonetheless, this driver has real users.
As a compromise, the driver will refuse to load unless i2c_imc.allow_unsafe_access=Y. When safe access becomes available, we can leave this option as a way for legacy users to run the driver, and we'll allow the driver to load by default if safe bus access is available.
--- drivers/i2c/busses/Kconfig | 17 ++ drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-imc.c | 567 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 585 insertions(+) create mode 100644 drivers/i2c/busses/i2c-imc.c
drivers/i2c/busses/Kconfig +17 -0
@@ -149,6 +149,23 @@ 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 + 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. + + There are currently no systems on which the kernel knows that it can + safely enable this driver. For now, you need to pass this driver a + scary module parameter, and you should only pass that parameter if you + have a special motherboard and know exactly what you are doing. + Special motherboards include the Supermicro X9DRH-iF-NV. + + 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
@@ -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 +567 -0
567 lines not on this page · raw message
Signature · 1 line
2.3.0
Re: [PATCH v2 1/2] i2c_imc: New driver for Intel's iMC, found on LGA2011 chips
Quoted 36 lines · 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.
There are various groups working on standardizing a way to arbitrate access to the bus between the OS, SMM firmware, a BMC, hardware thermal control, etc. In the mean time, running this driver is unsafe except under special circumstances. Nonetheless, this driver has real users.
As a compromise, the driver will refuse to load unless i2c_imc.allow_unsafe_access=Y. When safe access becomes available, we can leave this option as a way for legacy users to run the driver, and we'll allow the driver to load by default if safe bus access is available.
Signed-off-by Andy Lutomirski <[email protected]>--- drivers/i2c/busses/Kconfig | 17 ++ drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-imc.c | 567 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 585 insertions(+)create mode 100644
drivers/i2c/busses/i2c-imc.cdrivers/i2c/busses/Kconfig +4
-0@@ -149,6 +149,23 @@ 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 + help
drivers/i2c/busses/Kconfig config I2C_ISMT · line 165
Quoted 10 lines
drivers/i2c/busses/Kconfig +10
-0+ 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. + + There are currently no systems on which the kernel knows that it can + safely enable this driver. For now, you need to pass this driver a + scary module parameter, and you should only pass that parameter if you + have a special motherboard and know exactly what you are doing. + Special motherboards include the Supermicro X9DRH-iF-NV. +
I think the current approach of issuing warnings here and in the driver on load is the wrong one. For the most part, users will just run with distributions. If a distribution enables the driver, it will end up being used, warning or not.
A much better approach, in my opinion, would be to only enable the driver for systems where it is known (or presumed) to be good, such as the Supermicro board mentioned above. This can be done with DMI data.
For other boards, a 'force' module parameter can be added. This would ensure that the user _has_ to do something manually to load the driver. The warning on load would then only be displayed if the force module parameter is set.
Having said that, I am still not convinced that the driver should be in the kernel to start with. Browsing through Intel's datasheets, the registers are supported in E5-2600 v1, v2, and v3. However, in v3 Intel added a note saying that the registers should not be accessed by the OS directly, but only through the bios. Given that, and if that is possible, it might make more sense to rely on ACPI. It would then be up to the board and/or BIOS vendor to decide if the information should be available to the OS or not.
Thanks,
Guenter
Re: [PATCH v2 1/2] i2c_imc: New driver for Intel's iMC, found on LGA2011 chips
Quoted 54 lines · Guenter Roeck
Quoted 37 lines · 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.
There are various groups working on standardizing a way to arbitrate access to the bus between the OS, SMM firmware, a BMC, hardware thermal control, etc. In the mean time, running this driver is unsafe except under special circumstances. Nonetheless, this driver has real users.
As a compromise, the driver will refuse to load unless i2c_imc.allow_unsafe_access=Y. When safe access becomes available, we can leave this option as a way for legacy users to run the driver, and we'll allow the driver to load by default if safe bus access is available.
Signed-off-by Andy Lutomirski <[email protected]>--- drivers/i2c/busses/Kconfig | 17 ++ drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-imc.c | 567 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 585 insertions(+)create mode 100644
drivers/i2c/busses/i2c-imc.cdrivers/i2c/busses/Kconfig +4
-0@@ -149,6 +149,23 @@ 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 + helpdrivers/i2c/busses/Kconfig config I2C_ISMT · line 165
Quoted 10 lines
drivers/i2c/busses/Kconfig +10
-0+ 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. + + There are currently no systems on which the kernel knows that it can + safely enable this driver. For now, you need to pass this driver a + scary module parameter, and you should only pass that parameter if you + have a special motherboard and know exactly what you are doing. + Special motherboards include the Supermicro X9DRH-iF-NV. +I think the current approach of issuing warnings here and in the driver on load is the wrong one. For the most part, users will just run with distributions. If a distribution enables the driver, it will end up being used, warning or not.
Quoted 8 lines
A much better approach, in my opinion, would be to only enable the driver for systems where it is known (or presumed) to be good, such as the Supermicro board mentioned above. This can be done with DMI data.
For other boards, a 'force' module parameter can be added. This would ensure that the user _has_ to do something manually to load the driver. The warning on load would then only be displayed if the force module parameter is set.
This driver already has that: that's what "allow_unsafe_access" does.
I wouldn't be opposed to adding a DMI whitelist, although I really don't want DMI entries to proliferate.
Quoted 7 lines
Having said that, I am still not convinced that the driver should be in the kernel to start with. Browsing through Intel's datasheets, the registers are supported in E5-2600 v1, v2, and v3. However, in v3 Intel added a note saying that the registers should not be accessed by the OS directly, but only through the bios. Given that, and if that is possible, it might make more sense to rely on ACPI. It would then be up to the board and/or BIOS vendor to decide if the information should be available to the OS or not.
I think the plan is to add something to ACPI to tell us when we can use these registers. Unfortunately I'm not privy to whatever the ACPI committee is doing.
--Andy
Re: [PATCH v2 1/2] i2c_imc: New driver for Intel's iMC, found on LGA2011 chips
Quoted 9 lines
Quoted 8 lines
Having said that, I am still not convinced that the driver should be in the kernel to start with. Browsing through Intel's datasheets, the registers are supported in E5-2600 v1, v2, and v3. However, in v3 Intel added a note saying that the registers should not be accessed by the OS directly, but only through the bios. Given that, and if that is possible, it might make more sense to rely on ACPI. It would then be up to the board and/or BIOS vendor to decide if the information should be available to the OS or not.
Quoted 3 lines
I think the plan is to add something to ACPI to tell us when we can use these registers. Unfortunately I'm not privy to whatever the ACPI committee is doing.
Coming from the embedded world and not knowing much about this, I tend to wait until the above issue is clear, meaning when and how this driver is safe to be used. I hope you don't mind. Keeping your code up to date and sharing it is much appreciated, though! As well as Guenter's comments.
Thanks!
Re: [PATCH v2 1/2] i2c_imc: New driver for Intel's iMC, found on LGA2011 chips
Quoted 9 lines · Wolfram Sang
Quoted 9 lines
Quoted 8 lines
Having said that, I am still not convinced that the driver should be in the kernel to start with. Browsing through Intel's datasheets, the registers are supported in E5-2600 v1, v2, and v3. However, in v3 Intel added a note saying that the registers should not be accessed by the OS directly, but only through the bios. Given that, and if that is possible, it might make more sense to rely on ACPI. It would then be up to the board and/or BIOS vendor to decide if the information should be available to the OS or not.
Quoted 9 lines
Quoted 3 lines
I think the plan is to add something to ACPI to tell us when we can use these registers. Unfortunately I'm not privy to whatever the ACPI committee is doing.
Coming from the embedded world and not knowing much about this, I tend to wait until the above issue is clear, meaning when and how this driver is safe to be used. I hope you don't mind. Keeping your code up to date and sharing it is much appreciated, though! As well as Guenter's comments.
When I suggested that we should rely on ACPI, I meant through ACPI SMBus CMI (i2c-scmi), not through some ACPI properties which would permit direct access.
Thanks,
Guenter
PATCH v2 2/2 i2c, i2c_imc: Add DIMM bus code
In reply to Andy LutomirskiAdd 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 | 5 ++ drivers/i2c/busses/Makefile | 4 ++ drivers/i2c/busses/dimm-bus.c | 107 ++++++++++++++++++++++++++++++++++++++++++ drivers/i2c/busses/i2c-imc.c | 3 ++ include/linux/i2c/dimm-bus.h | 20 ++++++++ 5 files changed, 139 insertions(+) create mode 100644 drivers/i2c/busses/dimm-bus.c create mode 100644 include/linux/i2c/dimm-bus.h
drivers/i2c/busses/Kconfig +5 -0
@@ -149,9 +149,14 @@ 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 + 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
drivers/i2c/busses/Makefile +4 -0
@@ -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 +107 -0
107 lines not on this page · raw message
drivers/i2c/busses/i2c-imc.c +3 -0
@@ -19,6 +19,7 @@#include <linux/pci.h> #include <linux/ratelimit.h> #include <linux/i2c.h> +#include <linux/i2c/dimm-bus.h> /* * The datasheet can be found here, for example:
@@ -416,6 +417,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 +20 -0
@@ -0,0 +1,20 @@ +/* +* 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. +*/ + +#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
2.3.0