| Message ID | 15eccfa508fd0f55230c4274e3e968f91a123b73.1387588711.git.luto@amacapital.net |
|---|---|
| State | Deferred |
| Headers | show |
On Fri, Dec 20, 2013 at 05:45:13PM -0800, Andy Lutomirski wrote: > 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. Hmm, after thinking about it for a while and a couple of times, I get the impression that the functionality of i2c_scan_dimm_bus() should better be in userspace, i.e. a udev helper. I could imagine introducing a new functionality macro I2C_FUNC_DIMM_BUS which can be detected in userspace via i2c-dev. Based on that, the helper can do whatever necessary. What do you think?
On Wed, Feb 19, 2014 at 7:16 AM, Wolfram Sang <wsa@the-dreams.de> wrote: > On Fri, Dec 20, 2013 at 05:45:13PM -0800, Andy Lutomirski wrote: >> 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. > > Hmm, after thinking about it for a while and a couple of times, I get > the impression that the functionality of i2c_scan_dimm_bus() should > better be in userspace, i.e. a udev helper. I could imagine introducing > a new functionality macro I2C_FUNC_DIMM_BUS which can be detected in > userspace via i2c-dev. Based on that, the helper can do whatever > necessary. What do you think? > Hmm. So there would be udev rules that detect an I2C_FUNC_DIMM_BUS driver and probe it appropriately. I'm not sure I like it. It would mean that any future kernel code that wants to use things hanging off the dimm bus would need to stay in sync with the udev rules, and it would make things like sticking platform_data into the probed devices complicated or impossible. This seems to be a somewhat common problem with i2c code. For example, lots of graphics drivers provide i2c busses, and those busses often contain eeproms, and, in theory, things should know that the eeprom is associated with a particular graphics port, for example. Unfortunately, the i2c core does not know that, things like decode-dimms will try to decode it, sensors-detect will scan graphics ports for motherboard sensors, etc. It would be great if there was a generic way to tag an i2c bus as serving a particular purpose. Code that knew the purpose could probe appropriately, and code that wants to find things like eeproms on a particular port could look for the i2c bus that's tagged as belonging to the port and read its eeprom. Is this something that could be done using something like kobject symlinks? For example, suppose there were a new class device "i2c_port". A driver could instantiate this class and tell the i2c core (perhaps as part of i2c_register_adapter or in struct i2c_adapter?) that a given i2c_adapter that a certain range of addresses on the i2c_adapter belong to the port. The i2c core could create symlinks between the i2c_port and the i2c_adapter. For extra fun, there could be drivers for different types of i2c_port. One of them could be the "DIMM bus" driver, which would know how to probe the i2c adapter associated with a DIMM port. Another could be the graphics port driver, which (maybe with some extra configuration hints from the graphics driver) could look for EDID-related things. This could result in (after a bunch of extra code gets written) an actual struct device for a DIMM slot, which would have a class device for its i2c_port that claims all the the DIMM addresses with the slot number bits set appropriately for that port. In the even more distant future, maybe the EDAC devices would also hang off of that DIMM slot. I wonder if this would fit in well with the device tree stuff, too -- DT has ways to say "this node links to that one", right? *sigh*. The driver model wasn't really designed for a world where the logical device topology is completely unrelated to the physical bus topology. OK, enough rambling for now. :) --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
(I'm c/c Tony here, as he also shared the same concern that I had on a previous feedback about using I2C to talk with the DIMM). Em Wed, 19 Feb 2014 09:30:46 -0800 Andy Lutomirski <luto@amacapital.net> escreveu: > On Wed, Feb 19, 2014 at 7:16 AM, Wolfram Sang <wsa@the-dreams.de> wrote: > > On Fri, Dec 20, 2013 at 05:45:13PM -0800, Andy Lutomirski wrote: > >> 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. > > > > Hmm, after thinking about it for a while and a couple of times, I get > > the impression that the functionality of i2c_scan_dimm_bus() should > > better be in userspace, i.e. a udev helper. I could imagine introducing > > a new functionality macro I2C_FUNC_DIMM_BUS which can be detected in > > userspace via i2c-dev. Based on that, the helper can do whatever > > necessary. What do you think? > > > > Hmm. So there would be udev rules that detect an I2C_FUNC_DIMM_BUS > driver and probe it appropriately. > > I'm not sure I like it. It would mean that any future kernel code > that wants to use things hanging off the dimm bus would need to stay > in sync with the udev rules, and it would make things like sticking > platform_data into the probed devices complicated or impossible. As I said already a few times, my main concern with such patches is that it could eventually cause really bad things, especially since we're using experimental data collected on a system (or maybe on more than one, but still a limited set of systems), to infere that the same behavior will be valid for all. Even assuming that you covered all existing systems, couldn't a BIOS or microcode upgrade do some changes that could increase the chances that the above-described problems to happen more likely? I'm afraid so. Let me also cut-and-paste a few relevant parts taken from patch 4/4: > >> + It is possibly, although unlikely, that the use of this driver will > >> + interfere with your platform's RAM thermal management. > > > > This sounds scary and thus needs some more detail :) How does that show? > > What can happen? Anything to take care of? > > > > Here's how this works, as far as I can figure it out: > > The Sandy Bridge (and presumably Ivy Bridge) platforms make some > effort to avoid overheating system DRAM, and they can dynamically > adjust the refresh interval depending on DIMM temperature. This > happens in one of a few modes: ... > OLTT (open loop thermal throttling): the memory controller will > estimate recent heat output based on access patterns and will adjust > accordingly. There are lots of registers related to this in the > public docs, but the overall algorithm and purpose is not described > anywhere that I've looked. In this mode, something (SMI? P-code?) can > still poll the TSOD, but it respects the POLL_EN bit. ... > The interesting case is OLTT. If we fail to twiddle the POLL_EN bit > correctly, then, in principle, we could cause a problem. I don't know > what that problem would be -- the memory controller's P-code could > malfunction with unknown results. I've abused a test system quite a > bit, and I've been completely unable to cause a problem, though. > > There may be other modes, too, but, if so, I can't find them. Maybe > some day there will be CLTT with i2c access. If so, presumably the > driver will need updating. ... > * This is not documented at all AFAIK. I figured it out by trial and error. ... > >> + udelay(70); > > > > usleep_range? > > No -- see the comment just above this excerpt. There's an erratum > (which I can trigger on occasion but not reliably) that causes the i2c > hardware to return bogus results if the system enters a sufficiently > deep sleep while a transaction is running. udelay prevents that. Well, I've seen already very bad things happening on I2C. For example, some I2C eeproms miss-understands 0-sized I2C bus read messages, even sent to other I2C chips. On such eeproms, sometimes, instead of doing an eeprom read, they actually do an I2C write. There are enough known reported cases of such troubles with TV boards that we even added a contrib perl code at v4l2-utils to allow one to try to restore the board's EEPROM content, in order to allow fixing an accidental card brick, due to an EEPROM content loss caused by this bug. Of course, the user should have made a copy first of the content of the eeprom, or to find, at the net, a valid content for his board. This is easier said than done. Knowing nothing about how the DIMMs are manufactured, I can think on an hypothetical scenario where a manufacturer would test the DIMM timings and other DIMM quality parameters on their production line, and fill an I2C EEPROM at the DIMMS after those tests, in order to determine the safe zone for a DIMM. So, if such scenario is possible (someone with more experience with DIMM production could help here?), then, in thesis, a badly timed read (or an I2C read that would be interrupted by a SMI call that would also touch at the same I2C bus) could damage the EEPROM content, bricking the DIMM. Well, what I'm trying to say is that, before implementing any solution that reads data directly from the DIMM I2C bus (doesn't matter if in userspace or Kernelspace), we should know for sure that this won't cause DIMM loses, either by overheating them or by causing a corruption on the information that stores the DIMM size and timings on each DIMM slot. So, I suggest to not commit any patches here, without a careful review from an Intel engineer that would then be sure that: - the BIOS will respect POLL_EN; - there will be no way to accidentally write data at DIMM control eeproms; - that current and newer microcode/firmware updates will keep such driver reliable in the long term. That's said, as there are critical timings at the driver (so udelay is needed), and a precise time can't be warranted in userspace - as the userspace process may be interrupted during an userspace usleep() call - I would very much prefer to have this implemented at Kernelspace level. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> (I'm c/c Tony here, as he also shared the same concern that I had on a > previous feedback about using I2C to talk with the DIMM). Correct - I've heard the same issues that reads on I2C can be misinterpreted as writes ... and oops, you have a brick. What is the larger context/ What problem are we trying to solve? -Tony -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> example, lots of graphics drivers provide i2c busses, and those busses > often contain eeproms, and, in theory, things should know that the > eeprom is associated with a particular graphics port, for example. > Unfortunately, the i2c core does not know that, things like > decode-dimms will try to decode it, sensors-detect will scan graphics > ports for motherboard sensors, etc. ACPI does now try to describe what is on an i²c bus. We perhaps don't use that information well but on a modern PC class box at least for onboard devices most of the info is there for the munching. > For extra fun, there could be drivers for different types of i2c_port. > One of them could be the "DIMM bus" driver, which would know how to > probe the i2c adapter associated with a DIMM port. Another could be > the graphics port driver, which (maybe with some extra configuration > hints from the graphics driver) could look for EDID-related things. Busses are not necessarily that tidily organised. There isn't anything saying you can't sneak multiple things on the same bus. In the graphics case its unlikely but I wouldn't rule even that out for a display panel. Once you get onto phones and tablets it seems anything goes 8) > I wonder if this would fit in well with the device tree stuff, too -- > DT has ways to say "this node links to that one", right? ACPI basically tries to describe the heirarchy of devices on the bus, but as you say the "real" device can be a rambling mix of GPIO, I²C, SPI, PCI (quite probably fake PCI at that) and other resources. If there is a legitimate use case for poking around with memory dimm i2c on these boards then really there needs to be a proper defined interface for doing so that covers firmware and OS users. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 19, 2014 at 11:26 AM, One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> wrote: >> example, lots of graphics drivers provide i2c busses, and those busses >> often contain eeproms, and, in theory, things should know that the >> eeprom is associated with a particular graphics port, for example. >> Unfortunately, the i2c core does not know that, things like >> decode-dimms will try to decode it, sensors-detect will scan graphics >> ports for motherboard sensors, etc. > > ACPI does now try to describe what is on an i²c bus. We perhaps don't use > that information well but on a modern PC class box at least for onboard > devices most of the info is there for the munching. I'm discussing the in-kernel representation, not where the information comes from. Presumably there should be something in the kernel that works for ACPI, DT, and directly-enumerated busses (see below). > >> For extra fun, there could be drivers for different types of i2c_port. >> One of them could be the "DIMM bus" driver, which would know how to >> probe the i2c adapter associated with a DIMM port. Another could be >> the graphics port driver, which (maybe with some extra configuration >> hints from the graphics driver) could look for EDID-related things. > > Busses are not necessarily that tidily organised. There isn't anything > saying you can't sneak multiple things on the same bus. In the graphics > case its unlikely but I wouldn't rule even that out for a display panel. > Once you get onto phones and tablets it seems anything goes 8) > I'm suggesting identifying a range of addresses on a bus with a "port" (or whatever it should be called). Multiple ports could claim non-overlapping ranges on the same bus. >> I wonder if this would fit in well with the device tree stuff, too -- >> DT has ways to say "this node links to that one", right? > > ACPI basically tries to describe the heirarchy of devices on the bus, but > as you say the "real" device can be a rambling mix of GPIO, I²C, SPI, > PCI (quite probably fake PCI at that) and other resources. > > If there is a legitimate use case for poking around with memory dimm i2c > on these boards then really there needs to be a proper defined interface > for doing so that covers firmware and OS users. The legitimate use case is NV-DIMMs. They have control registers that are accessed via smbus. BIOS is likely to poke at those registers early in boot, but the kernel and/or userspace will need them, too. It would be nice to know which DIMM slot they map to. In this particular case (Intel LGA2011 systems), there's only one sane way to wire up the busses, since the memory controller *is* the smbus master. According to the JEDEC spec, each DIMM slot is has three pins that indicate which slot it is on its channel, and the onboard smbus hardware uses those pins to select its addresses. (This maps directly to three of the seven address bits.) Each memory controller supports two RAM channels and two i2c channels, and, if you wire them backwards (or mix them up between controllers), then I would be amazed if the system works. So this particular driver really does know the topology a priori. On the other hand, nothing rules out having NV-DIMMs in non-LGA2011 systems where the smbus/i2c topology is less certain. But I think it would be silly for an eventual NVDIMM driver to specifically depend on ACPI, since ACPI isn't needed for existing LGA2011 NVDIMM systems. (Knowing which physical memory addresses map to which DIMM is a different story. I think that the sb-edac driver knows the mapping, though.) --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 19, 2014 at 11:03 AM, Luck, Tony <tony.luck@intel.com> wrote: >> (I'm c/c Tony here, as he also shared the same concern that I had on a >> previous feedback about using I2C to talk with the DIMM). > > Correct - I've heard the same issues that reads on I2C can be misinterpreted > as writes ... and oops, you have a brick. Is this true on DDR3 DIMMs, i.e. anything that's compatible with LGA2011? If you plug a DIMM into an LGA2011 board's memory slot, then, one way or another, it's very likely that there will be TSOD traffic, if for no other purpose than to determine that there is no TSOD present. TSOD traffic consists of reads and writes, both with and without register numbers. (Sorry, I can never remember the smbus terminology here -- the relevant transactions are two-byte reads and two-byte writes, both with a command specified and without one. One of the bits in the iMC SMBUS registers tells the controller which kind of read to use to probe the thermometer.) > > What is the larger context/ What problem are we trying to solve? NV-DIMM control registers are exposed via i2c, presumably because trying to access them through the memory pins would be a giant mess. So, one way or another, something needs to be able to initiate transactions to access those registers. BIOS will do some initial setup, but the OS will need to poke at these registers, too. (The actual docs are covered by NDA. I suspect that this will change if the manufacturers ever want these things to be widely used, though, since these things really want a full-featured kernel driver so that things like pmfs will work cleanly.) As a secondary benefit, having access to the TSOD and SPD is nice, albeit far from critical. AFAICT Intel actively working on NV-DIMM-related things, so maybe Intel will contribute an engineer who help :) --Andy > > -Tony
> NV-DIMM control registers are exposed via i2c, presumably because > trying to access them through the memory pins would be a giant mess. > So, one way or another, something needs to be able to initiate > transactions to access those registers. BIOS will do some initial > setup, but the OS will need to poke at these registers, too. (The > actual docs are covered by NDA. I suspect that this will change if > the manufacturers ever want these things to be widely used, though, > since these things really want a full-featured kernel driver so that > things like pmfs will work cleanly.) > > As a secondary benefit, having access to the TSOD and SPD is nice, > albeit far from critical. > > AFAICT Intel actively working on NV-DIMM-related things, so maybe > Intel will contribute an engineer who help :) Yes - we have people looking at pmfs and NV-DIMMs. I don't know the internal details ... to keep these accesses safe may require letting the platform BIOS code perform them (via some ACPI thingy) ... messy and slow - but probably workable if these registers are only required for some maintenance/configuration usage patterns. Not so good if they are in the high frequency read/write path (but I2C in the critical path sounds like a recipe for failure). -Tony -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 20, 2014 at 8:42 AM, Luck, Tony <tony.luck@intel.com> wrote: >> NV-DIMM control registers are exposed via i2c, presumably because >> trying to access them through the memory pins would be a giant mess. >> So, one way or another, something needs to be able to initiate >> transactions to access those registers. BIOS will do some initial >> setup, but the OS will need to poke at these registers, too. (The >> actual docs are covered by NDA. I suspect that this will change if >> the manufacturers ever want these things to be widely used, though, >> since these things really want a full-featured kernel driver so that >> things like pmfs will work cleanly.) >> >> As a secondary benefit, having access to the TSOD and SPD is nice, >> albeit far from critical. >> >> AFAICT Intel actively working on NV-DIMM-related things, so maybe >> Intel will contribute an engineer who help :) > > Yes - we have people looking at pmfs and NV-DIMMs. I don't know > the internal details ... to keep these accesses safe may require letting > the platform BIOS code perform them (via some ACPI thingy) ... messy > and slow - but probably workable if these registers are only required > for some maintenance/configuration usage patterns. Not so good if > they are in the high frequency read/write path (but I2C in the critical > path sounds like a recipe for failure). As far as I know, they're not involved in reads and writes, but they are useful periodically to keep everything happy. I suppose that *shudder* calling an ACPI method to initiate an i2c transaction aimed at a DIMM slot isn't *that* bad. On the other hand, if we're supposed to issue an ACPI call to ask "is it okay to start using this device now", then won't we need to reflash firmware every time we switch NV-DIMM vendors? Ugh! In any case, let's hold off on merging this until Intel and/or the ACPI people figure out what's going on. This driver is IMO not worth it just to access SPD and TSOD. (Actually, I think I could write a separate TSOD driver that just asks the memory controller to report cached temperature values. That should be almost entirely non-scary.) --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> I'm suggesting identifying a range of addresses on a bus with a "port" > (or whatever it should be called). Multiple ports could claim > non-overlapping ranges on the same bus. Which is fine until you meant a mux or a device that can be moved about by writing to it, or has a wide range of addresses determined by strapping. > In this particular case (Intel LGA2011 systems), there's only one sane > way to wire up the busses, since the memory controller *is* the smbus > master. According to the JEDEC spec, each DIMM slot is has three pins Ok that helps a lot for the specific case, and you have at least in theory got a flag between the OS and BIOS to avoid things like SMM throwing parties on the smbus while you are using it. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 21, 2014 at 7:32 AM, One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> wrote: >> I'm suggesting identifying a range of addresses on a bus with a "port" >> (or whatever it should be called). Multiple ports could claim >> non-overlapping ranges on the same bus. > > Which is fine until you meant a mux or a device that can be moved about > by writing to it, or has a wide range of addresses determined by > strapping. For muxes: isn't there already a system in place for dealing with them? For things with addresses determined by strapping: my vague proposal is explicitly intended to support them. (DIMMs are such a device.) Whoever instantiates the i2c "ports" is supposed to *know* the topology, including which straps are set where. In some cases, that data could come from ACPI or DT. In other cases, it's enumerated directly. The idea would be to allow the code that figures out the topology (both physical connectivity and straps) to be separated from the code that talks to the i2c endpoints. Just specifying the address ranges might be insufficient for things that are more flexible than DIMMs, though -- it might be necessary to say "this port has these addresses and this kind of device lives at this address". --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Em Wed, 19 Feb 2014 17:39:07 -0800 Andy Lutomirski <luto@amacapital.net> escreveu: > On Wed, Feb 19, 2014 at 11:03 AM, Luck, Tony <tony.luck@intel.com> wrote: > >> (I'm c/c Tony here, as he also shared the same concern that I had on a > >> previous feedback about using I2C to talk with the DIMM). > > > > Correct - I've heard the same issues that reads on I2C can be misinterpreted > > as writes ... and oops, you have a brick. > > Is this true on DDR3 DIMMs, i.e. anything that's compatible with > LGA2011? If you plug a DIMM into an LGA2011 board's memory slot, > then, one way or another, it's very likely that there will be TSOD > traffic, if for no other purpose than to determine that there is no > TSOD present. TSOD traffic consists of reads and writes, both with > and without register numbers. (Sorry, I can never remember the smbus > terminology here -- the relevant transactions are two-byte reads and > two-byte writes, both with a command specified and without one. One > of the bits in the iMC SMBUS registers tells the controller which kind > of read to use to probe the thermometer.) An update on that: I double-checked with a DIMM manufacturer. I was told that some DIMM models have write protect circuits but others don't. So, yeah, accessing it could eventually brick the DIMM, depending on the DIMM model, if such driver won't block I2C write ops or if the BIOS is also trying to access the bus at the same time. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Feb 28, 2014 11:15 AM, "Mauro Carvalho Chehab" <m.chehab@samsung.com> wrote: > > Em Wed, 19 Feb 2014 17:39:07 -0800 > Andy Lutomirski <luto@amacapital.net> escreveu: > > > On Wed, Feb 19, 2014 at 11:03 AM, Luck, Tony <tony.luck@intel.com> wrote: > > >> (I'm c/c Tony here, as he also shared the same concern that I had on a > > >> previous feedback about using I2C to talk with the DIMM). > > > > > > Correct - I've heard the same issues that reads on I2C can be misinterpreted > > > as writes ... and oops, you have a brick. > > > > Is this true on DDR3 DIMMs, i.e. anything that's compatible with > > LGA2011? If you plug a DIMM into an LGA2011 board's memory slot, > > then, one way or another, it's very likely that there will be TSOD > > traffic, if for no other purpose than to determine that there is no > > TSOD present. TSOD traffic consists of reads and writes, both with > > and without register numbers. (Sorry, I can never remember the smbus > > terminology here -- the relevant transactions are two-byte reads and > > two-byte writes, both with a command specified and without one. One > > of the bits in the iMC SMBUS registers tells the controller which kind > > of read to use to probe the thermometer.) > > An update on that: I double-checked with a DIMM manufacturer. > I was told that some DIMM models have write protect circuits but > others don't. > > So, yeah, accessing it could eventually brick the DIMM, depending > on the DIMM model, if such driver won't block I2C write ops or > if the BIOS is also trying to access the bus at the same time. I'm not sure I buy the argument about blocking writes -- as far as I know, i2c-i801 on desktop platforms has exactly the same issue. Or maybe I'm misunderstanding you. I'd be okay with adding code to block writes to SPD addresses, but I'm not sure I see the point. If root wants to brick a DIMM, I'm sure root can find a way. On the other hand, racing with BIOS is a real problem. I'd like to know how Intel plans to handle this. I suspect that POLL_EN is the right bit to use, but the docs are vague, and there could be strange BIOSes out there. (Grr. Everything would be saner if SMM didn't exist.) --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 0fe76cc..8b46ad8 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -135,6 +135,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 diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index ea2a6a5..91827cb 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -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 diff --git a/drivers/i2c/busses/dimm-bus.c b/drivers/i2c/busses/dimm-bus.c new file mode 100644 index 0000000..0968428 --- /dev/null +++ b/drivers/i2c/busses/dimm-bus.c @@ -0,0 +1,97 @@ +/* + * Copyright (c) 2013 Andrew Lutomirski <luto@amacapital.net> + * + * 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. + */ + +#include <linux/i2c.h> +#include <linux/bug.h> +#include <linux/module.h> +#include <linux/i2c/dimm-bus.h> + +static bool probe_addr(struct i2c_adapter *adapter, int addr) +{ + /* + * So far, all known devices that live on DIMMs can be safely + * and reliably detected by trying to read a byte at address + * zero. (The exception is the SPD write protection control, + * which can't be probed and requires special hardware and/or + * quick writes to access, and has no driver.) + */ + union i2c_smbus_data dummy; + return i2c_smbus_xfer(adapter, addr, 0, I2C_SMBUS_READ, 0, + I2C_SMBUS_BYTE_DATA, &dummy) >= 0; +} + +/** + * i2c_scan_dimm_bus() - Scans an SMBUS segment known to contain DIMMs + * @adapter: The SMBUS adapter to scan + * + * This function tells the DIMM-bus code that the adapter is known to + * contain DIMMs. i2c_scan_dimm_bus will probe for devices known to + * live on DIMMs. + * + * Do NOT call this function on general-purpose system SMBUS segments + * unless you know that the only things on the bus are DIMMs. + * Otherwise is it very likely to mis-identify other things on the + * bus. + * + * Callers are advised not to set adapter->class = I2C_CLASS_SPD. + */ +void i2c_scan_dimm_bus(struct i2c_adapter *adapter) +{ + struct i2c_board_info info = {}; + int slot; + + /* + * We probe with "read byte data". If any DIMM SMBUS driver can't + * support that access type, this function should be updated. + */ + if (WARN_ON(!i2c_check_functionality(adapter, + I2C_FUNC_SMBUS_READ_BYTE_DATA))) + return; + + /* + * Addresses on DIMMs use the three low bits to identify the slot + * and the four high bits to identify the device type. Known + * devices are: + * + * - 0x50 - 0x57: SPD (Serial Presence Detect) EEPROM + * - 0x30 - 0x37: SPD WP control -- not worth trying to probe + * - 0x18 - 0x1f: TSOD (Temperature Sensor on DIMM) + * + * There may be more some day. + */ + for (slot = 0; slot < 8; slot++) { + /* If there's no SPD, then assume there's no DIMM here. */ + if (!probe_addr(adapter, 0x50 | slot)) + continue; + + strcpy(info.type, "spd"); + info.addr = 0x50 | slot; + i2c_new_device(adapter, &info); + + if (probe_addr(adapter, 0x18 | slot)) { + /* This is a temperature sensor. */ + strcpy(info.type, "jc42"); + info.addr = 0x18 | slot; + i2c_new_device(adapter, &info); + } + } +} +EXPORT_SYMBOL(i2c_scan_dimm_bus); + +MODULE_AUTHOR("Andrew Lutomirski <luto@amacapital.net>"); +MODULE_DESCRIPTION("i2c DIMM bus support"); +MODULE_LICENSE("GPL"); diff --git a/drivers/i2c/busses/i2c-imc.c b/drivers/i2c/busses/i2c-imc.c index c846077..437d8d8 100644 --- a/drivers/i2c/busses/i2c-imc.c +++ b/drivers/i2c/busses/i2c-imc.c @@ -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; } diff --git a/include/linux/i2c/dimm-bus.h b/include/linux/i2c/dimm-bus.h new file mode 100644 index 0000000..3d44df4 --- /dev/null +++ b/include/linux/i2c/dimm-bus.h @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2013 Andrew Lutomirski <luto@amacapital.net> + * + * 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 */
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. Signed-off-by: Andy Lutomirski <luto@amacapital.net> --- 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