From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 46554432316 for ; Tue, 28 Jul 2026 12:24:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785241470; cv=none; b=kNj4fxGZmTq+uJ/HWHxwtX2Wke5Y7fpWKPTDCGoNNK9If/Hw/lwO+rVcRTYDugC5EgosLhrOTasRUFA0hDFKo91vkFvIt4pzXE0gF1nuuanwtnGobD7SJxtcULHq25dsDe3ApJstbDD1D9RjvNPfWa90hZHuyluZql1/Rg0jbwk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785241470; c=relaxed/simple; bh=s98uX6obr3g8JsWCB6x0jmZaNfcpCwsSJ/Ug+Jg56lU=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=Omm/Wjwl6WRNiZVFYlCJyi/OfvmK8cvBv1cFVvuJ7Il9/5IRGlqrKdPKfiUfoUxBQggAtG3SW3cNluZVMf60MIegazWPfm7PTt+dkhz3kfsgGhiVNV0id0tNUzRsplBqaG85ds9qsfkSTGZFNUQ+3RiAk1rQT/8Zn4qqmYlnNXg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=gTfp2RvR; arc=none smtp.client-ip=209.85.128.48 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="gTfp2RvR" Received: by mail-wm1-f48.google.com with SMTP id 5b1f17b1804b1-495757ccbc1so34666205e9.2 for ; Tue, 28 Jul 2026 05:24:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1785241465; x=1785846265; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to:content-type; bh=bxJRceg/nCJ1Uzt6qzDcuq3q6rSwVGtr3AXAUYTTWPw=; b=gTfp2RvR2XyCytXy43OwASyOVgK6n+gn7eM9aRda4YxIKuMySZtK+aSXs/O5hkkvqi qgbobNdpS3WgVIxFtGe5IBWses0HlbUAJnOSy6c9EQTw/XB3Ay3h0VzdXSc2d6wCvJ0Q 2pMsfb3jQl4r6w44rX/SJDqrjvGz+emsxdsGNSTiq+cz5q3dxD7V6rVm6PNodoO95BqB RBicAO3ivyG8+UssozxcyGAK48uAh+UldEoSXt8XLNY8BKMPYUCgElAbeTMnxF6tSWFC hWcPNuA7iQINUOqXcNn51TEYce2po7m5O/QfGUYWMrO7b5S7CG2g90WY8Gl21HJQHw2+ vxNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785241465; x=1785846265; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=bxJRceg/nCJ1Uzt6qzDcuq3q6rSwVGtr3AXAUYTTWPw=; b=oGQh00EhC38CYw0q+2GSHNDECApc+4fQ5jGyFzp9xbwO5UpIqhyNCBOdKhjxCWek4D R99Q903guNnng2+uWfsSX55aLWN7PQEe+vVds6Rh10+3PqFVZ7a/rA0cVVVtejqu+GfT 5y2cPC0O4TvnzgXjGhtNIbcPdlagxycGfOLODs7e61JZF9a3/72pnKzg4mN4u4aXpiH3 02fu8yg4UDHRUfb/fSsXK6pRroEGDaOo2HFDjCG4hcOFD6WjAxTTmgLVCnPCzTRRKaYm dJSel2eKPaFlazXvW3Ez9JZyS8tTXvPREPZpg8l79606xc3IIq6iMig1tM/WUYhEBI9j p/mg== X-Gm-Message-State: AOJu0YxTmbL989Auv/kKvAnFSFCWeuNTuWPLMsbQSKcsglvgOwvmot79 uo8m/GAjCFR8/O9fTqig2Dihr78puwU78r6UgI53icyW1WOc3eB0QP+m X-Gm-Gg: AR+sD11VpYl+uh0aN3uwKQkHD23Zu+1ksPgCwMd1Enx8QodvtupRzL2LU4TYbHazsUV Mo+nlm11u05O0lzlF7r/Ui0evipMhOGj4XN5Fj9bU2rWYQgGtoLEdByYtwQHezhkSprr7mKd+Tg wo2Ns9hwrCuGIjKoUhlUqyKQSD3ONW/m7nDH4UUnQ2JvEgizGDEWvhoIZTvgtfD26zBopmpv7I3 8/NJRPzRICeMzudMFVoC85+RXxkKXG5cE5ZZhXc4zg78zJ1DAgQWyUbB+5yjkerP0KaT/TQeBVf uqi7GgMhidq5cggLYbnanBqydukG+AcDIg+Fm0Q+vMKk73t8Qf+6OzB1H0RaTnz+nzPnnRRVyVX KtS1BixsamUtNDaaDmBe4yt1DraAkO5zvy8iX/NXGxYvX/66n2+sTjKniYcFpxmPDFoisl6OTJX bwPr9pael/PG5cyKBBCkrwumkd7qiPveysSTapBVsCdj0ZaDb+3Y7Vqu9fZsCi/E3Dvi0g1cOLD XQCdoQ2WOYN X-Received: by 2002:a05:600c:4744:b0:495:4c74:6a14 with SMTP id 5b1f17b1804b1-496c6438e91mr24063305e9.14.1785241465205; Tue, 28 Jul 2026 05:24:25 -0700 (PDT) Received: from ws1-mint.. (host165-7-217-213.cgnat.broadband.ehiweb.it. [213.217.7.165]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-496c45e0424sm76427265e9.13.2026.07.28.05.24.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Jul 2026 05:24:24 -0700 (PDT) From: Simone Chifari To: Andi Shyti Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, Andy Lutomirski , Stefan Schaeckeler , Simone Chifari Subject: [PATCH v3 0/1] i2c: imc-skylake: add Intel Skylake-X iMC SMBus adapter Date: Tue, 28 Jul 2026 14:24:22 +0200 Message-ID: <20260728122423.358416-1-simone.chifari@gmail.com> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-i2c@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit The integrated memory controller of Intel Skylake-X and Cascade Lake-X processors contains an SMBus engine wired to the DDR4 DIMMs. This driver exposes its two channels as standard Linux I2C adapters, so that DDR4 SPD EEPROMs and DIMM thermal sensors can be read with i2c-tools, decode-dimms and lm-sensors. v2 was posted as i2c-imc-x299 and received no replies: https://lore.kernel.org/linux-i2c/20260620144131.415559-1-simone.chifari@gmail.com/ Correction to v2 ~~~~~~~~~~~~~~~~ v2 claimed that firmware filters CF8/CFC config writes to this function and that the registers can only be reached through the ECAM window. That claim was wrong and I am retracting it. It was never tested directly. The only evidence behind it was the boot log line "PCI: Using configuration type 1", which says which accessor the kernel uses and nothing about a write failing. Measured on the same board: # setpci -s 16:1e.5 9c.L=20085002 # read SPD 0x50, register 0x02 # setpci -s 16:1e.5 9c.L 20005002 # GO consumed # setpci -s 16:1e.5 a8.L 0000500c # READ_DONE, no error # setpci -s 16:1e.5 b4.L 0000000c # SPD byte 2: DDR4 type code The command written through CF8/CFC is executed and returns correct data, and MSR_SMI_COUNT does not move across it. This driver therefore uses pci_read_config_dword() and pci_write_config_dword() like any other driver. There is no ECAM mapping, no MCFG parsing and no PCI-core change in this series any more. Firmware arbitration ~~~~~~~~~~~~~~~~~~~~ This is the reason no iMC SMBus driver has ever been merged, and this patch does not solve it either. It does make the condition detectable. Andy Lutomirski's original i2c_imc put it plainly: a BMC, the memory controller's power management, and possibly SMM all like to touch this bus, and Intel defined no way to arbitrate between them. That driver already had a module parameter called allow_unsafe_access. Stefan Schaeckeler's 2020 Broadwell rewrite went further and stopped the PCU's TSOD polling for the duration of a transfer (write 0 to TSODCNTL, wait 10 ms for in-flight transactions to drain, restore afterwards), because on Broadwell the original driver's consistency check tripped about every 30 minutes. https://lwn.net/Articles/635898/ https://lore.kernel.org/linux-i2c/1582498270-50674-1-git-send-email-schaecsn@gmx.net/ So this driver: - keeps the default-off allow_unsafe_access gate, deliberately the same name as the original; - serializes both channels under one mutex and waits for the whole engine to be idle before touching any register; - saves the command state of both channels, clears TSOD_ACTIVE for the duration of the transfer, and restores both afterwards; - after every transfer compares the command registers against what it left in them and fails the transfer with -EAGAIN if anything else changed. This is the check the original driver had, and it is what makes an unarbitrated system detectable rather than silently wrong; - reads the command word back and requires it to be the one just written, because the done bits are latched. With the driver unloaded and nothing running, STATUS on the test system reads 0x0000300c: READ_DONE and WRITE_DONE both still set from an earlier transaction. Without the check, a command word that never reaches the register is indistinguishable from a completed transfer and the caller receives the previous transaction's data. The engine only ever clears GO, so that is the single permitted difference between the two words. - refuses to bind when the memory controller is polling the DIMM thermal sensors itself. Intel documents SMBCNTL on the iMC channel function (device 10 and 12, function 0, offsets 0xe88 and 0xe98) in the Xeon Processor Scalable Family datasheet volume 2, reference 614073, section 3.1.9. Its SMB_TSOD_POLL_EN bit is documented as mutually exclusive with SPD command access, so it answers directly whether the engine is in use. On the test system all four channels read 0xac000000: polling disabled, TSOD_PRESENT mask empty, which also explains why no jc42 device appears at 0x18-0x1f. This is deliberately weaker than what the Broadwell-E driver did. That one switched the polling off for the duration of a transfer. Reading the bit and declining is a smaller claim on hardware the driver does not own, and it needs no write to a firmware-configured register. It covers the memory controller only; SMM and a BMC remain out of reach, which is what the opt-in is for. Relation to prior work ~~~~~~~~~~~~~~~~~~~~~~ There is no iMC I2C driver in mainline. Lutomirski's i2c-imc was an RFC; the 2020 resend and github.com/Linux4/i2c-imc are out of tree. Neither can be extended to cover this part: the device ID (0x2085 vs 0x3ca8/0x6fa8) and the register layout (0x9C-0xB8 vs 0x180-0x188) differ. A separate file is cleaner than a hw_data table. The register encoding was cross-checked against an independent Windows implementation for the same engine, which predates this work: https://github.com/namazso/PawnIO.Modules/blob/c975ca1d1b8a9c7473a175fa1ca24d83b6cbc8f3/SmbusIntelSkylakeIMC.p It also implements SMBus BYTE through a pointer-select bit, without saying which bit. That bit is 18 here. The datasheet gave it away: SMBCMD on the iMC channel functions places PNTR_SEL immediately above WORD_ACCESS, and word access on the PCU function is bit 17. Bit 18 is the only part of this driver that rests on an undocumented encoding, so it is worth saying exactly what was measured rather than asserting it works. With the bit set on a read, eight consecutive transfers to an SPD EEPROM returned bytes 0x01 through 0x08 in order, matching an ordinary dump of the same range. That is the auto-increment of a Receive Byte; a transfer that still named a register would have returned the same byte eight times. With the bit set on a write, a Send Byte to the DDR4 page-select addresses moves the 256-byte window: byte 0x00 stops reading as the JEDEC 0x23 after selecting page 1 and returns to it after selecting page 0. ee1004 consequently binds, and decode-dimms reads all four DIMMs on this system with both SPD CRCs valid on each. jc42 binds where a sensor is populated. i2cdetect scans in read mode. With Receive Byte in place the driver can also do what the other SMBus host drivers do and call i2c_register_spd() per adapter, so the SPD EEPROMs are instantiated at probe rather than by hand. The helper counts the populated slots from DMI and probes upwards from 0x50, leaving alone any address that does not answer; each channel carries half the DIMMs and the scan is per adapter, so each finds its own. Nothing else is instantiated, jc42 included: there is no firmware description of these buses and this driver does not scan for anything but the SPDs. I used the write_disable variant. This part is DDR4 only, where the two are equivalent - the distinction gates spd5118 on DDR5 - and between two equivalent calls the one that does not read as "this driver enables SPD writes" seemed the better thing to leave in a memory-bus driver. Happy to switch to write_enable for consistency with i801 and piix4 if you prefer the convention. SMBus Quick is still not implemented, and not only because the engine has no obvious encoding for it. A write to the 0x30-0x37 range is how an EE1004 SPD is write-protected, permanently on many modules. An adapter whose bus is the memory bus should not advertise a probe that writes. Changes since v2 ~~~~~~~~~~~~~~~~ - Retract the CF8/CFC claim; use the ordinary config accessors. The driver no longer maps anything. - Rename i2c-imc-x299 to i2c-imc-skylake; the engine is in the CPU, not the chipset. - Add the default-off allow_unsafe_access gate. - Wait for both channels to be idle before touching the data latch. - Preserve and restore command and TSOD state for both channels. - Validate READ_DONE, WRITE_DONE and the hardware error state. - Detect interference from another master and report it as -EAGAIN. - Read the command word back and require it to be the one written. - Add bounded command-toggle recovery for a stuck BUSY state. - Add WORD_DATA support with the hardware byte ordering. - Reject I2C_M_TEN explicitly and validate transfer direction. - Use the i2c core's suspended-adapter mechanism instead of a private flag and a non-standard errno. - Use devm_mutex_init() and devm-managed adapters; drop the unnecessary pci_enable_device(); return -ENODEV rather than -EPERM from the default-off gate, which was logging a driver-core error on every boot. - Add Documentation/i2c/busses/i2c-imc-skylake.rst. - Drop the ACPI dependency. - Refuse to bind when the iMC reports TSOD polling enabled. - Correct the claim that BYTE_DATA and WORD_DATA cover ee1004: they do not, ee1004 also needs Receive Byte and Send Byte. - Implement both, on the pointer-select bit, so ee1004 binds and decode-dimms works. - Instantiate the SPD EEPROMs with i2c_register_spd() instead of leaving it to the user. Validation ~~~~~~~~~~ - In-tree build, W=1, x86_64 GCC and arm64 Clang COMPILE_TEST: no diagnostics. - checkpatch.pl --strict: clean. - Sparse with __CHECK_ENDIAN__: clean. - Hardware test on one Intel Core i9-10900X (Cascade Lake-X) system with four DDR4 DIMMs: both adapters, four SPD EEPROMs, BYTE_DATA, WORD_DATA byte order, Receive Byte tracking the EEPROM pointer, Send Byte moving the SPD page and putting it back, ENXIO for an absent device, 100 concurrent reads per channel, the default-off gate, and 20 unload/reload cycles: 19 checks, 0 failures, no kernel warning or oops, no interference reported, every command word confirmed in the register. - i2c_register_spd() instantiated ee1004 on all four populated SPD addresses at probe, two per channel, and claimed none of the empty ones. decode-dimms then decoded all four modules, each with the CRC of bytes 0-125 and of bytes 128-253 valid. The part number reads out of page 1, so the page select is exercised end to end. The four module serial numbers were identical before and after every experiment in this series. - Engine registers sampled every 2 s for one hour with the driver unloaded: 1791 samples, 0 changes. No firmware activity while the system is up. - Suspend and resume, s2idle and S3, with continuous traffic on both adapters: transfers recover and nothing complains. Across S3 the firmware demonstrably drives the engine. Both channels, sampled immediately before suspend and immediately after resume: DATA 0x2000b600 -> 0x20005000 STATUS 0x0000500c -> 0x00005004 The command word came back addressing SPD 0x50 for a read, and the status went from both done bits latched to READ_DONE alone. Nothing in Linux issued that: the adapter was marked suspended and userspace was frozen. This is consistent with firmware reading SPD during memory training on resume. It is the only direct observation of a second master on this engine I have, and it is the concrete reason the driver restores command state per transfer and compares it afterwards. - SMBCNTL on all four iMC channels reads 0xac000000: TSOD polling disabled, TSOD_PRESENT empty. - Writes validated end to end against a populated device: a 12-write frame to an LED controller at 0x27, across both channels, every write acknowledged, no NACK, and the device applied the result. A byte write to an unpopulated address returns -ENXIO with the engine reporting the address NACK. No SPD was written at any point, and the four SPD EEPROMs were verified present with valid CRCs before and after. Still missing: an independent Tested-by, ideally on Skylake-X rather than Cascade Lake-X. One note on the register interface, since a reader with the datasheet will ask. That document describes an SMBus interface on the iMC channel function at 0xe80-0xe9c, with a different encoding from the one this driver uses on PCU function 5 at 0x9c-0xb8; it does not describe function 5 at all, covering device 30 functions 0 and 2 only. I tried the documented path on this system: issuing SMBCMD with CMD_TRIGGER sets SMB_RDO and clears SMB_BUSY, but returns 0x00 for a populated DIMM at 0x50, for a second one at 0x52, and for an empty slot at 0x57 alike, with SMB_SBE never set. An interface that cannot tell a populated slot from an empty one is not reaching the bus here, while the path this driver uses returns the correct SPD contents and a NACK for the empty slot. I may well be missing a setup step; if someone knows what it is, I would rather use the documented interface. Tool assistance ~~~~~~~~~~~~~~~ Per Documentation/process/generated-content.rst: this work was developed with an AI coding assistant, run interactively rather than autonomously. - What: Claude Code, model claude-opus-5. - Where: it contributed to the transaction state machine, the .rst documentation, the commit message and this cover letter. The register decoding is mine. - How it was used: iteratively, one reviewed change at a time, in response to specific requests, not from a single generating prompt. A representative request is "the done bits are latched; show me how a command that is never accepted can be mistaken for a completed one, and make that case detectable", which produced imc_engine_responded(). It also designed the measurement that retracted the CF8/CFC claim above, after asking why that claim had never been tested directly. - Testing: every revision was built with W=1, checked with checkpatch --strict and sparse, and tested on the hardware by me. I have reviewed all of it, I understand it, and I take responsibility for it. Simone Chifari (1): i2c: imc-skylake: add Intel Skylake-X iMC SMBus adapter Documentation/i2c/busses/i2c-imc-skylake.rst | 178 +++++ Documentation/i2c/busses/index.rst | 1 + MAINTAINERS | 7 + drivers/i2c/busses/Kconfig | 27 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-imc-skylake.c | 734 +++++++++++++++++++ 6 files changed, 948 insertions(+) create mode 100644 Documentation/i2c/busses/i2c-imc-skylake.rst create mode 100644 drivers/i2c/busses/i2c-imc-skylake.c base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6 -- 2.43.0