commit 4697ce3e3b8a7fb381c8acc2a10c30f3b15a0757 Author: Simone Date: Sat Jun 27 15:01:27 2026 +0200 i2c: imc-skylake: add driver for Intel Skylake-X iMC SMBus engine Add a driver for the integrated memory controller (iMC) SMBus engine on Intel Skylake-X / Cascade Lake-X processors (socket LGA 2066, platform X299, PCU function 8086:2085). The engine provides two SMBus channels — one per pair of DIMM slots — over which SPD EEPROMs, DDR4 thermal sensors and third-party LED controllers are accessible. Exposing it as a pair of standard Linux I2C adapters lets existing tools (i2c-tools, lm-sensors) use it without bespoke sysfs hacks. Key design decisions: - ECAM MMIO access instead of CF8/CFC port I/O (SMM traps port writes) - Dynamic MCFG table parsing for mmcfg_base (no hardcoding) - Support for SMBus BYTE_DATA and WORD_DATA transfers - devm-managed resources with automatic cleanup - Global mutex to serialize transactions across both channels sharing the same ECAM mapping Signed-off-by: Simone Chifari diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..ad7f87a --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,43 @@ +name: CI + +on: + push: + branches: + - main + pull_request: + branches: + - main + workflow_dispatch: + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +jobs: + build: + runs-on: ubuntu-latest + timeout-minutes: 30 + + steps: + - uses: actions/checkout@v4 + + - name: Install build dependencies + run: | + sudo apt-get update + sudo apt-get install -y build-essential linux-headers-$(uname -r) + + - name: Build module + run: make + + - name: Run checkpatch + run: | + curl -sSLo checkpatch.pl https://raw.githubusercontent.com/torvalds/linux/master/scripts/checkpatch.pl + curl -sSLo spelling.txt https://raw.githubusercontent.com/torvalds/linux/master/scripts/spelling.txt + perl checkpatch.pl --no-tree --strict --no-summary --show-types -f i2c-imc-skylake.c + + - name: Upload artifact + uses: actions/upload-artifact@v4 + with: + name: i2c-imc-skylake-${{ github.ref_name }} + path: i2c-imc-skylake.ko + if-no-files-found: error diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..ec87b1f --- /dev/null +++ b/.gitignore @@ -0,0 +1,54 @@ +# Kernel build artifacts & temporary files +*.ko +*.ko.zst +*.mod +*.mod.c +*.mod.o +*.o +*.a +*.s +.*.cmd +.tmp_versions/ +modules.order +Module.symvers +/i2c-imc-skylake.mod + +# C/C++ Developer tools and symbol databases +*.d +*.o.d +compile_commands.json +.clangd/ +.cache/ +tags +TAGS +GPATH +GRTAGS +GSYMS +GTAGS + +# IDEs and Editors +.vscode/ +.idea/ +*.swp +*.swo +*~ +*.code-workspace + +# OS-specific files +.DS_Store +Thumbs.db + +# Generated patch files (git format-patch output) +patches/ + +# Internal working documents (not for public repo) +TODO.md +submission-analysis.md +README.local.md +AGENTS.md +docs/mainline-plan.md +docs/archive-i2c-imc-driver-plan.md +docs/submission/submission-instructions.md + +# Private Gitea CI (replaced by .github/workflows/ for public repo) +.gitea/ diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..66f850b --- /dev/null +++ b/Makefile @@ -0,0 +1,63 @@ +obj-m := i2c-imc-skylake.o + +KVER ?= $(shell uname -r) +KDIR ?= /lib/modules/$(KVER)/build +MODDIR = /lib/modules/$(KVER)/kernel/drivers/i2c/busses +PWD := $(shell pwd) + +all: + $(MAKE) -C $(KDIR) M=$(PWD) modules + +clean: + $(MAKE) -C $(KDIR) M=$(PWD) clean + +# Install into kernel tree + modprobe. +# Ubuntu/Debian kernel 6.x+ ships .ko.zst; depmod skips uncompressed .ko files, +# so compress with zstd when available, else fall back to plain .ko. +install: all + if command -v zstd >/dev/null 2>&1; then \ + zstd -q -f i2c-imc-skylake.ko -o i2c-imc-skylake.ko.zst; \ + sudo install -D -m 644 i2c-imc-skylake.ko.zst $(MODDIR)/i2c-imc-skylake.ko.zst; \ + sudo rm -f $(MODDIR)/i2c-imc-skylake.ko; \ + else \ + sudo install -D -m 644 i2c-imc-skylake.ko $(MODDIR)/i2c-imc-skylake.ko; \ + sudo rm -f $(MODDIR)/i2c-imc-skylake.ko.zst; \ + fi + sudo depmod -a $(KVER) + sudo modprobe i2c-imc-skylake + +# Remove module and .ko/.ko.zst from kernel tree +uninstall: + sudo rmmod i2c-imc-skylake 2>/dev/null || true + sudo rm -f $(MODDIR)/i2c-imc-skylake.ko $(MODDIR)/i2c-imc-skylake.ko.zst + sudo depmod -a $(KVER) + +# Quick rmmod + insmod cycle for leak/oops testing +reload: all + sudo rmmod i2c-imc-skylake 2>/dev/null || true + sudo insmod i2c-imc-skylake.ko + +# Load without build (use after install) +load: + sudo modprobe i2c-dev + sudo modprobe i2c-imc-skylake + +unload: + sudo rmmod i2c-imc-skylake 2>/dev/null || true + +# Checkpatch (must be clean before submission) +checkpatch: + perl $(KDIR)/scripts/checkpatch.pl --strict --no-tree -f i2c-imc-skylake.c + +# Sparse static analysis (requires sparse >= kernel version; may show header errors on old sparse) +sparse: + $(MAKE) -C $(KDIR) M=$(PWD) C=1 CF="-D__CHECK_ENDIAN__" + +# Smoke test — requires root + module loaded (run after make install or make reload) +test: + sudo bash test-smoke.sh + +log: + sudo dmesg | grep i2c-imc-skylake | tail -80 + +.PHONY: all clean install uninstall reload load unload checkpatch sparse test log diff --git a/README.md b/README.md new file mode 100644 index 0000000..f1f2c49 --- /dev/null +++ b/README.md @@ -0,0 +1,123 @@ +# Intel Skylake-X iMC SMBus I2C Driver + +[![License: GPL v2](https://img.shields.io/badge/License-GPL%20v2-blue.svg)](https://www.gnu.org/licenses/old-licenses/gpl-2.0.html) +[![Kernel Compatibility](https://img.shields.io/badge/Kernel-6.x%20%7C%207.x-green.svg)](#) +[![Code Style: checkpatch](https://img.shields.io/badge/checkpatch-clean-brightgreen.svg)](#) +[![Upstream Status](https://img.shields.io/badge/Upstream-Submitted%20(v2)-orange.svg)](https://lore.kernel.org/linux-i2c/) + +A modern Linux PCI bus driver for the integrated Memory Controller (iMC) SMBus controller found in Intel Skylake-X and Cascade Lake-X (HEDT X299, function ID `8086:2085`) processors. + +This driver maps the iMC SMBus engine and registers it as two standard Linux I2C adapters (one per physical channel), allowing native userspace tools like `i2c-tools` and `lm-sensors` to interact with target devices (such as DDR4 DIMM SPD EEPROMs and thermal sensors) without custom raw PCI port-IO writes. + +--- + +## Origin & Use Case + +This driver was born out of the need to control the RGB lighting effects on **Kingston FURY DDR4 RGB** memory modules under Linux on the Intel X299 (HEDT) platform. + +These memory modules carry an onboard **ENE KB9012** LED controller accessible via the SMBus channels of the CPU's integrated memory controller (iMC). However, standard Linux tools and OpenRGB could not communicate with them because: +1. The X299 System Management Mode (SMM) firmware traps standard port-based PCI configuration writes, breaking standard driver access. +2. The iMC SMBus channels were not exposed as standard Linux I2C buses. + +By resolving the MMCONFIG base and using ECAM MMIO, this driver bypasses SMM and registers the physical memory channels as standard `/dev/i2c-*` adapters. + +While the kernel driver itself is kept 100% generic and brand-agnostic (to comply with Linux upstream requirements), its primary practical use cases are: +* Driving userspace Linux daemons for hardware/software-rendered RGB lighting animations without screen or GUI lag. +* Enabling compatibility with OpenRGB for memory modules on Skylake-X / Cascade Lake-X systems. +* Reading standard DDR4 SPD data and DIMM thermal sensor metrics using standard utilities like `decode-dimms`. + +--- + +## Upstream Status + +This driver is currently submitted for review to the Linux kernel `linux-i2c` subsystem mailing list for mainlining (patch series v2). See the [lore.kernel.org linux-i2c archive](https://lore.kernel.org/linux-i2c/) for the discussion thread. + +--- + +## Technical Features & SMM Bypass + +* **ECAM MMIO Access**: On the X299 platform, System Management Mode (SMM) firmware traps and drops legacy port-based (`CF8/CFC`) configuration space writes to the PCU function. This driver bypasses the trap by resolving the MMCONFIG base from the ACPI `MCFG` table and accessing the registers directly via ECAM MMIO memory-mapped pages. +* **Dual Channel Support**: Registers two independent I2C buses (`iMC SMBus Skylake-X channel 0` and `channel 1`), mapping physically to DIMM slots 1-2 and 3-4 respectively. +* **Clean Lifecycle**: Fully compliant with the `devm` kernel framework. All mapped registers, adapters, and mutexes are cleaned up automatically upon driver unbinding. +* **Zero Out-of-Tree Brandings**: Code is completely generic and compliant with standard Linux kernel styling guidelines. + +--- + +## Installation + +### Prerequisites + +Ensure you have kernel headers and compiler utilities installed: + +```bash +# Debian / Ubuntu +sudo apt-get install build-essential linux-headers-$(uname -r) + +# Fedora / RHEL +sudo dnf install gcc make kernel-devel +``` + +### Manual Build & Load + +1. **Compile the driver**: + ```bash + make + ``` +2. **Test style compliance**: + ```bash + make checkpatch + ``` +3. **Load immediately (testing)**: + ```bash + make reload + ``` +4. **Install permanently into the kernel tree**: + ```bash + sudo make install + ``` + +--- + +## DKMS Support (Dynamic Kernel Module Support) + +To build and load the driver automatically on every kernel upgrade: + +1. **Install DKMS**: + ```bash + sudo apt-get install dkms + ``` +2. **Register the module**: + ```bash + sudo dkms add . + ``` +3. **Build and install**: + ```bash + sudo dkms install -m i2c-imc-skylake -v 1.0 + ``` + +--- + +## Verification & Testing + +Once loaded, verify that the two channels are registered successfully: + +```bash +i2cdetect -l | grep iMC +``` + +Expected output: +```text +i2c-6 smbus iMC SMBus Skylake-X channel 0 SMBus adapter +i2c-7 smbus iMC SMBus Skylake-X channel 1 SMBus adapter +``` + +You can scan for SPD EEPROMs (usually located at addresses `0x50-0x57`) on channel 0: +```bash +sudo i2cdetect -y 6 +``` + +--- + +## License + +This driver is licensed under the **GPL v2 only** (`GPL-2.0-only`). diff --git a/dkms.conf b/dkms.conf new file mode 100644 index 0000000..b92ec38 --- /dev/null +++ b/dkms.conf @@ -0,0 +1,10 @@ +PACKAGE_NAME="i2c-imc-skylake" +PACKAGE_VERSION="1.0" + +BUILT_MODULE_NAME[0]="i2c-imc-skylake" +DEST_MODULE_LOCATION[0]="/updates" + +MAKE[0]="make KVER=${kernelver}" +CLEAN="make clean KVER=${kernelver}" + +AUTOINSTALL="yes" diff --git a/docs/submission/Kconfig.kernel b/docs/submission/Kconfig.kernel new file mode 100644 index 0000000..a2f1b31 --- /dev/null +++ b/docs/submission/Kconfig.kernel @@ -0,0 +1,18 @@ +config I2C_IMC_SKYLAKE + tristate "Intel Skylake-X iMC SMBus adapter" + depends on PCI && ACPI && X86 + help + Say Y here if you want kernel support for the integrated memory + controller (iMC) SMBus engine found in Intel Skylake-X / Cascade + Lake-X processors (socket LGA 2066, platform X299, PCU function + 8086:2085). + + The engine is exposed through the PCI configuration space of the + PCU function and is driven via ECAM MMIO (not CF8/CFC port I/O, + which is trapped by System Management Mode on this platform). + Two I2C adapters are registered, one per hardware SMBus channel, + allowing access to DDR4 DIMM SPD EEPROMs (0x50-0x57) and thermal + sensors from userspace via standard i2c-tools and lm-sensors. + + If unsure, say N. This driver is only useful on Intel X299 desktop + / HEDT systems with the Skylake-X or Cascade Lake-X CPU. diff --git a/docs/submission/cover-letter.txt b/docs/submission/cover-letter.txt new file mode 100644 index 0000000..b16a076 --- /dev/null +++ b/docs/submission/cover-letter.txt @@ -0,0 +1,110 @@ +Subject: [PATCH 0/1] i2c: imc-skylake: add Intel Skylake-X iMC SMBus adapter + +This is v3 of the i2c-imc-skylake driver. v2 was submitted under the name +i2c-imc-x299; the driver has been renamed to reflect that the iMC SMBus +engine is in the Skylake-X / Cascade Lake-X CPU, not in the X299 chipset. + +This series adds a driver for the integrated memory controller (iMC) SMBus +engine on Intel Skylake-X / Cascade Lake-X processors (socket LGA 2066, +platform X299, PCU function 8086:2085). + +The engine provides two SMBus channels — one per pair of DIMM slots — over +which SPD EEPROMs, DDR4 thermal sensors and third-party LED controllers +(e.g. ENE KB9012 at 0x27) are accessible. Exposing it as a pair of standard +Linux I2C adapters lets existing tools (i2c-tools, lm-sensors) use +it without bespoke sysfs hacks. + +Why ECAM MMIO instead of pci_{read,write}_config_dword +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +On X299 the kernel selects "configuration type 1" (CF8/CFC port I/O) for PCI +config space access, as reported by boot-time dmesg: + + PCI: Using configuration type 1 for base access + +System Management Mode traps writes to those ports for this device, so a +standard pci_write_config_dword() targeting the SMBus DATA register never +reaches the hardware — the transaction hangs at status bit 0x08 indefinitely. +The Windows NTIOLib (used by Kingston FURY) reaches the same registers via the +ECAM (MMIO) window, which is not trapped. This driver follows the same path: +ioremap() of the ECAM page for the target function and driving the registers +by MMIO read/write. + +Note that pci_mmcfg_* helpers are arch-internal and not exported to modules, +so a manual walk of the ACPI MCFG table is used to locate mmcfg_base at probe +time (no module parameter, no hardcoding). + +Relation to prior iMC SMBus work +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Two previous attempts to upstream an iMC SMBus driver exist: + + - Lutomirski 2013–2016 (Sandy Bridge-EP 8086:3ca8): used CF8/CFC + pci_read/write_config_dword — correct on that platform since SMM does not + trap those ports on Sandy Bridge-EP. Not merged due to missing bus + arbitration (BMC / CLTT sharing) and a required allow_unsafe_access flag. + + - schaecsn 2020 (Broadwell-E 8086:6fa8): added TSOD arbitration using the + Broadwell-documented tsod_polling_interval quiesce procedure. Not merged + (no reviewer response); register layout differs from X299. + +This driver is for a different device ID (0x2085), different register layout +(DATA/STATUS/CTRL at 0x9C-0xB8 vs 0x180-0x188), and a fundamentally different +access method (ECAM ioremap). Combining it with the Lutomirski/schaecsn code +would require a large per-generation hw_data table with mutually incompatible +access methods; a separate file is cleaner. + +The X299 HEDT (High-End Desktop) platform has no BMC and no CLTT firmware +polling, so the arbitration safety concern of the Lutomirski patch does not +apply. No allow_unsafe_access flag is needed. + +Independent validation +~~~~~~~~~~~~~~~~~~~~~~ +The register layout (DATA/STATUS/CTRL at 0x9C-0xB8, stride 4, GO bit 19, +WORD bit 17) was independently reverse-engineered and confirmed by the +PawnIO.Modules project (SmbusIntelSkylakeIMC.p, Windows userspace, Feb 2026) +and by commercial monitoring tools (HWiNFO, SIV) that access the same PCI +config space via the Windows kernel API (ECAM by default on this platform). + +Changes since v2 +~~~~~~~~~~~~~~~~~ + - Rename driver from i2c-imc-x299 to i2c-imc-skylake + - Fix iMC SMBus attribution: engine is in the CPU (Skylake-X), not + the X299 chipset + - Add I2C_SMBUS_WORD_DATA support (WORD_BIT bit 17) for TSOD reads + - Update adapter name: "iMC SMBus Skylake-X channel N" + +Testing +~~~~~~~ +Tested on: Intel X299 platform (Skylake-X CPU), 4× DDR4 DIMMs, +kernel 7.0.0-14-generic (Linux Mint 22.3). + + $ i2cdetect -l | grep iMC + i2c-7 smbus iMC SMBus Skylake-X channel 0 SMBus adapter + i2c-8 smbus iMC SMBus Skylake-X channel 1 SMBus adapter + +The driver supports SMBus BYTE_DATA and WORD_DATA transfers. WORD_DATA +uses the engine's WORD_BIT (bit 17) and is needed for DDR4 thermal sensors +(TSOD) which expose 16-bit temperature registers. The engine stores word +data in little-endian order in the CTRL register, so byte-swapping is applied +on both read and write paths. + +Since the hardware requires a register offset for every transaction, +I2C_SMBUS_QUICK and I2C_SMBUS_BYTE (which have no offset) are not supported +(returning -EOPNOTSUPP). Use `i2cdetect -r` (read byte data probe) to scan +for devices. Standard kernel drivers such as ee1004 (for DDR4 SPD) load +and bind successfully via BYTE_DATA reads. + +udev autoloads the module on PCI add event (MODULE_DEVICE_TABLE present). +20 rmmod/modprobe cycles: no oops, no warnings, no resource leaks in dmesg. + +Signed-off-by: Simone Chifari + +--- + +Simone Chifari (1): + i2c: imc-skylake: add driver for Intel Skylake-X iMC SMBus engine + + MAINTAINERS | 6 + + drivers/i2c/busses/Kconfig | 19 + + drivers/i2c/busses/Makefile | 1 + + drivers/i2c/busses/i2c-imc-skylake.c | 512 +++++++++++++++++++++++++++++++++++ + 4 files changed, 538 insertions(+) diff --git a/i2c-imc-skylake.c b/i2c-imc-skylake.c new file mode 100644 index 0000000..e37443e --- /dev/null +++ b/i2c-imc-skylake.c @@ -0,0 +1,519 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Intel Skylake-X iMC SMBus I2C adapter. + * + * The integrated memory controller (iMC) on Intel Skylake-X / Cascade Lake-X + * processors exposes an SMBus engine used to reach the SPD EEPROMs and thermal + * sensors on DDR4 DIMMs. The engine is driven through the PCI configuration + * space of the Sky Lake-E PCU function (0000:16:1e.5, 8086:2085). This driver + * presents that engine as two standard Linux I2C adapters - one per hardware + * SMBus channel - so that i2c-tools and lm-sensors can use it without bespoke + * sysfs hacks. + * + * Why ECAM MMIO instead of the usual CF8/CFC config accessors: + * On this platform System Management Mode (SMM) traps and mangles port-based + * (CF8/CFC) writes to the first 256 config bytes of this function, so the SMBus + * command never completes. The boot log reports "PCI: Using configuration + * type 1 (probe)" confirming the default path is port-based; the MMCONFIG + * (ECAM) window is not trapped. Windows' NTIOLib reaches the registers via + * ECAM, which is how we confirmed the register layout on hardware. We map the + * ECAM page of the target function and drive the registers by MMIO, exactly as + * the firmware does. + * + * ECAM phys(off) = mmcfg_base + (bus<<20) + (dev<<15) + (fn<<12) + off + * mmcfg_base is read from the ACPI MCFG table at probe time (not hardcoded). + * + * Per-channel register triple within the config page: + * ch0 ch1 + * CTRL (data) 0xB4 0xB8 write: data byte in bits[23:16]; read: low byte + * DATA (cmd) 0x9C 0xA0 FRAME | (cmd << 8) | reg ; bit19 = GO + * STATUS 0xA8 0xAC busy while bit0 set; done when clear, + * bit1 (0x02) set on completion = device NACKed + * SMBus command byte (DATA bits[15:8]) = (rw << 7) | addr7: the 7-bit slave + * address with bit7 = direction (1 write / 0 read). The register/offset goes + * in DATA[7:0]. So 0x50 reads SPD EEPROM 0x50. This was decoded and + * confirmed on hardware. + * + * The two channels are independent SMBus buses (DIMMs 1,2 on ch0; DIMMs 3,4 on + * ch1), hence two i2c_adapter instances exposed as separate /dev/i2c-* nodes. + * Each channel carries the DIMM SPD EEPROMs (0x50-0x57) and thermal sensors - + * all reachable by 7-bit address. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* expected PCI id of the Sky Lake-E PCU SMBus function */ +#define PCU_DEVICE 0x2085 +#define PCU_ID ((PCU_DEVICE << 16) | PCI_VENDOR_ID_INTEL) + +#define CFG_SIZE 0x1000UL + +/* per-channel register offsets within the config page */ +#define CH0_CTRL 0xB4 +#define CH0_DATA 0x9C +#define CH0_STAT 0xA8 +#define CH1_CTRL 0xB8 +#define CH1_DATA 0xA0 +#define CH1_STAT 0xAC + +/* + * Command word written to the DATA register: + * bits[31:16] = FRAME (engine config + GO bit19), constant + * bits[15:8] = SMBus command byte = (rw << 7) | addr7 + * rw bit (0x80): 1 = write, 0 = read + * addr7 : 7-bit SMBus slave address + * bits[7:0] = register/offset within the addressed device + * For a write the data byte is latched into CTRL[23:16] beforehand. This + * encoding was confirmed on hardware: command 0x50 reads SPD EEPROM 0x50 + * (DDR4 signature). + */ +#define FRAME 0x20080000U /* engine config + GO bit19, constant */ +#define RW_WRITE 0x80 /* OR into the command byte for a write */ +#define WORD_BIT BIT(17) /* 16-bit word transfer (vs 8-bit byte) */ +#define GO_BIT BIT(19) +#define STAT_BUSY BIT(0) /* low bit set while transaction in flight */ +#define STAT_NACK BIT(1) /* set on completion if the device NACKed */ + +struct imc_chan { + u32 ctrl, data, stat; +}; + +static const struct imc_chan imc_chans[2] = { + { CH0_CTRL, CH0_DATA, CH0_STAT }, + { CH1_CTRL, CH1_DATA, CH1_STAT }, +}; + +/* one driver state object, shared by both per-channel adapters */ +struct imc_smbus { + struct device *dev; /* &pdev->dev, for dev_*() logging */ + void __iomem *cfg; /* ioremapped ECAM page of the function */ + struct mutex lock; /* serialises all SMBus transactions */ + struct i2c_adapter adap[2]; /* one per hardware channel */ +}; + +/* + * ECAM base discovery from ACPI MCFG. acpi_table_parse() is not exported to + * modules, so map the MCFG table with acpi_get_table() (exported) and walk the + * allocation entries by hand. Uses pdev's PCI segment and bus number so no + * module parameter override is needed. + */ +#ifdef CONFIG_ACPI +static u64 imc_detect_mmcfg_base(struct pci_dev *pdev) +{ + unsigned int seg = pci_domain_nr(pdev->bus); + unsigned int bus = pdev->bus->number; + struct acpi_table_header *hdr; + struct acpi_table_mcfg *mcfg; + struct acpi_mcfg_allocation *e; + unsigned long n, i; + u64 base = 0; + + if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MCFG, 0, &hdr))) + return 0; + + mcfg = (struct acpi_table_mcfg *)hdr; + if (hdr->length < sizeof(*mcfg)) { + acpi_put_table(hdr); + return 0; + } + e = (struct acpi_mcfg_allocation *)(mcfg + 1); + n = (hdr->length - sizeof(*mcfg)) / sizeof(*e); + for (i = 0; i < n; i++) { + if (e[i].pci_segment == seg && + bus >= e[i].start_bus_number && + bus <= e[i].end_bus_number) { + base = e[i].address; + break; + } + } + acpi_put_table(hdr); + return base; +} +#else +static u64 imc_detect_mmcfg_base(struct pci_dev *pdev) +{ + return 0; +} +#endif + +/* + * Wait until GO clears (transaction issued), then until the busy bit drops. + * Returns 0 on completion, -ETIMEDOUT if the engine never went idle. On + * success *status (if non-NULL) gets the final status word. Process context + * only - it sleeps between polls. + * + * Timeout values validated empirically: 200ms for GO clear and 50ms for BUSY + * clear cover worst-case DIMM response times observed across 20+ load/unload + * cycles on Skylake-X hardware with various DDR4 modules. + */ +static int imc_wait(struct imc_smbus *s, const struct imc_chan *c, u32 *status) +{ + u32 val; + int ret; + + ret = readl_poll_timeout(s->cfg + c->data, val, !(val & GO_BIT), 10, 200000); + if (ret) + return ret; + + ret = readl_poll_timeout(s->cfg + c->stat, val, !(val & STAT_BUSY), 10, 50000); + if (ret) + return ret; + + if (status) + *status = val; + + return 0; +} + +/* + * Poll the busy bit clear only (no GO check). The firmware polls STATUS after + * the CTRL (data-latch) write too, before issuing the DATA/GO word. + * Timeout: 50ms validated empirically across multiple DIMM configurations. + */ +static int imc_wait_status(struct imc_smbus *s, const struct imc_chan *c) +{ + u32 val; + int ret; + + ret = readl_poll_timeout(s->cfg + c->stat, val, !(val & STAT_BUSY), 10, 50000); + if (ret) { + dev_dbg(s->dev, "pre-command poll timed out, engine still busy\n"); + return ret; + } + + return 0; +} + +/* translate a completed transaction's status into an errno (0 = ACK) */ +static int imc_check_status(struct imc_smbus *s, u32 status, u8 addr, u8 reg) +{ + if (status & STAT_NACK) { + /* no device at this address, or it refused the access */ + dev_dbg(s->dev, "addr 0x%02x reg 0x%02x NACK (stat 0x%08x)\n", + addr, reg, status); + return -ENXIO; + } + return 0; +} + +/* SMBus write-byte to addr: latch the data byte, then issue the command. */ +static int imc_write_byte(struct imc_smbus *s, const struct imc_chan *c, + u8 addr, u8 reg, u8 val) +{ + u32 status = 0; + int ret; + + writel((u32)val << 16, s->cfg + c->ctrl); + ret = imc_wait_status(s, c); + if (ret) + return ret; + writel(FRAME | ((u32)(addr | RW_WRITE) << 8) | reg, s->cfg + c->data); + ret = imc_wait(s, c, &status); + if (ret) { + dev_warn_ratelimited(s->dev, + "write addr 0x%02x reg 0x%02x timed out\n", + addr, reg); + return ret; + } + ret = imc_check_status(s, status, addr, reg); + if (ret) + return ret; + + return 0; +} + +/* SMBus read-byte from addr: issue the command, return the low data byte. */ +static int imc_read_byte(struct imc_smbus *s, const struct imc_chan *c, + u8 addr, u8 reg, u8 *val) +{ + u32 status = 0; + int ret; + + ret = imc_wait_status(s, c); + if (ret) + return ret; + writel(FRAME | ((u32)addr << 8) | reg, s->cfg + c->data); + ret = imc_wait(s, c, &status); + if (ret) { + dev_warn_ratelimited(s->dev, + "read addr 0x%02x reg 0x%02x timed out\n", + addr, reg); + return ret; + } + ret = imc_check_status(s, status, addr, reg); + if (ret) + return ret; + *val = readl(s->cfg + c->ctrl) & 0xFF; + return 0; +} + +/* + * SMBus write-word to addr: latch the byte-swapped 16-bit value into CTRL, + * then issue the command with WORD_BIT set. The engine stores word data in + * little-endian order in the CTRL register (low byte in bits[23:16], high + * byte in bits[31:24]), so we swap before writing to match hardware layout. + */ +static int imc_write_word(struct imc_smbus *s, const struct imc_chan *c, + u8 addr, u8 reg, u16 val) +{ + u32 status = 0; + int ret; + + /* byte-swap for little-endian hardware: low byte → bits[23:16] */ + writel((u32)swab16(val) << 16, s->cfg + c->ctrl); + ret = imc_wait_status(s, c); + if (ret) + return ret; + writel(FRAME | WORD_BIT | ((u32)(addr | RW_WRITE) << 8) | reg, + s->cfg + c->data); + ret = imc_wait(s, c, &status); + if (ret) { + dev_warn_ratelimited(s->dev, + "write word addr 0x%02x reg 0x%02x timed out\n", + addr, reg); + return ret; + } + return imc_check_status(s, status, addr, reg); +} + +/* + * SMBus read-word from addr: issue the command with WORD_BIT set, return + * the byte-swapped 16-bit value from CTRL. The engine returns word data + * in little-endian order (low byte in bits[23:16], high byte in bits[31:24]), + * so we swap after reading to match CPU endianness. + */ +static int imc_read_word(struct imc_smbus *s, const struct imc_chan *c, + u8 addr, u8 reg, u16 *val) +{ + u32 status = 0, raw; + int ret; + + ret = imc_wait_status(s, c); + if (ret) + return ret; + writel(FRAME | WORD_BIT | ((u32)addr << 8) | reg, s->cfg + c->data); + ret = imc_wait(s, c, &status); + if (ret) { + dev_warn_ratelimited(s->dev, + "read word addr 0x%02x reg 0x%02x timed out\n", + addr, reg); + return ret; + } + ret = imc_check_status(s, status, addr, reg); + if (ret) + return ret; + raw = readl(s->cfg + c->ctrl) & 0xFFFF; + *val = swab16(raw); + return 0; +} + +/* + * Standard SMBus transfer callback. The per-channel imc_chan is stashed in the + * adapter's algo_data. The command word carries the 7-bit address, so + * SPD EEPROMs (0x50-0x57) are reachable on each channel. BYTE_DATA and + * WORD_DATA are supported; larger transfers would need the engine's block + * primitives, which are not used by the devices on this bus. + */ +static s32 imc_smbus_xfer(struct i2c_adapter *adap, u16 addr, + unsigned short flags, char read_write, u8 command, + int size, union i2c_smbus_data *data) +{ + struct imc_smbus *s = i2c_get_adapdata(adap); + const struct imc_chan *c = adap->algo_data; + u8 reg, val = 0; + int ret; + + if (addr > 0x7f) + return -EINVAL; + + if (size != I2C_SMBUS_BYTE_DATA && size != I2C_SMBUS_WORD_DATA) + return -EOPNOTSUPP; + + reg = command; + + mutex_lock(&s->lock); + if (read_write == I2C_SMBUS_WRITE) { + if (size == I2C_SMBUS_WORD_DATA) { + dev_dbg(s->dev, "ch%d W addr=%02x reg=%02x val=%04x\n", + (int)(c - imc_chans), addr, reg, data->word); + ret = imc_write_word(s, c, addr, reg, data->word); + } else { + val = data->byte; + dev_dbg(s->dev, "ch%d W addr=%02x reg=%02x val=%02x\n", + (int)(c - imc_chans), addr, reg, val); + ret = imc_write_byte(s, c, addr, reg, val); + } + } else { + if (size == I2C_SMBUS_WORD_DATA) { + u16 wval; + + ret = imc_read_word(s, c, addr, reg, &wval); + if (!ret) + data->word = wval; + dev_dbg(s->dev, "ch%d R addr=%02x reg=%02x -> %04x (ret %d)\n", + (int)(c - imc_chans), addr, reg, wval, ret); + } else { + ret = imc_read_byte(s, c, addr, reg, &val); + if (!ret) + data->byte = val; + dev_dbg(s->dev, "ch%d R addr=%02x reg=%02x -> %02x (ret %d)\n", + (int)(c - imc_chans), addr, reg, val, ret); + } + } + mutex_unlock(&s->lock); + + return ret; +} + +static u32 imc_func(struct i2c_adapter *adap) +{ + return I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA; +} + +static const struct i2c_algorithm imc_algo = { + .smbus_xfer = imc_smbus_xfer, + .functionality = imc_func, +}; + +static void imc_mutex_destroy(void *data) +{ + struct mutex *lock = data; + + mutex_destroy(lock); +} + +static int imc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) +{ + resource_size_t phys; + struct imc_smbus *s; + u8 imc_bus_hw; + u64 base; + u32 cfg0, cc; + int ret, i; + + s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL); + if (!s) + return -ENOMEM; + + ret = pcim_enable_device(pdev); + if (ret) { + dev_err(&pdev->dev, "cannot enable PCI device: %d\n", ret); + return ret; + } + + s->dev = &pdev->dev; + mutex_init(&s->lock); + ret = devm_add_action_or_reset(&pdev->dev, imc_mutex_destroy, &s->lock); + if (ret) + return ret; + + base = imc_detect_mmcfg_base(pdev); + if (!base) { + dev_err(&pdev->dev, "cannot resolve MMCONFIG base\n"); + return -ENODEV; + } + + phys = base + + ((resource_size_t)pdev->bus->number << 20) + + ((resource_size_t)PCI_SLOT(pdev->devfn) << 15) + + ((resource_size_t)PCI_FUNC(pdev->devfn) << 12); + + /* + * Deliberately no request_mem_region(): the MMCONFIG window is already + * claimed as a firmware/PCI resource, so a reservation would fail with + * -EBUSY. The pci_driver binding keeps the function alive; the registers + * we drive are side-band controls the kernel does not otherwise touch. + */ + s->cfg = devm_ioremap(&pdev->dev, phys, CFG_SIZE); + if (!s->cfg) { + dev_err(&pdev->dev, "ioremap(%pa) failed\n", &phys); + return -ENOMEM; + } + + cfg0 = readl(s->cfg + 0); + if (cfg0 != PCU_ID) { + dev_err(&pdev->dev, "wrong device at ECAM %pa (cfg[0]=0x%08x)\n", + &phys, cfg0); + return -ENODEV; + } + + /* + * Cross-check the iMC bus number against the configuration register + * value: cfg[0xCC] bits[15:8] = the iMC SMBus bus number as seen by + * the PCU. On all known Skylake-X / Cascade Lake-X boards this matches + * the probed bus number. + * A mismatch means the ECAM walk landed on the wrong slot — warn but + * continue; the binding is already locked to 8086:2085. + */ + cc = readl(s->cfg + 0xCC); + imc_bus_hw = (cc >> 8) & 0xFF; + + if (imc_bus_hw && imc_bus_hw != (u8)pdev->bus->number) + dev_warn(&pdev->dev, + "cfg[0xCC] reports iMC bus 0x%02x but probed bus=0x%02x\n", + imc_bus_hw, (u8)pdev->bus->number); + else + dev_dbg(&pdev->dev, + "cfg[0xCC]=0x%08x iMC bus 0x%02x confirmed\n", + cc, imc_bus_hw); + + dev_info(&pdev->dev, "ECAM %pa (mmcfg_base=0x%llx)\n", &phys, base); + + for (i = 0; i < 2; i++) { + struct i2c_adapter *a = &s->adap[i]; + + a->owner = THIS_MODULE; + a->algo = &imc_algo; + a->algo_data = (void *)&imc_chans[i]; + a->dev.parent = &pdev->dev; + i2c_set_adapdata(a, s); + snprintf(a->name, sizeof(a->name), + "iMC SMBus Skylake-X channel %d", i); + + ret = devm_i2c_add_adapter(&pdev->dev, a); + if (ret) { + dev_err(&pdev->dev, + "i2c_add_adapter ch%d failed: %d\n", i, ret); + return ret; + } + } + + dev_info(&pdev->dev, "registered 2 SMBus channels (use i2cdetect -l)\n"); + return 0; +} + +static const struct pci_device_id imc_pci_ids[] = { + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCU_DEVICE) }, + { } +}; +MODULE_DEVICE_TABLE(pci, imc_pci_ids); + +/* + * All resources (ioremap, i2c adapters, mutex) are devm-managed and are + * released automatically after probe returns or during device unbinding, so + * no .remove callback is needed. The register state is deliberately left + * as-is on unbind: the engine carries no driver-private latched state that + * needs teardown, and the SMBus controls we drive are side-band registers the + * rest of the kernel does not touch. + */ +static struct pci_driver imc_driver = { + .name = "i2c-imc-skylake", + .id_table = imc_pci_ids, + .probe = imc_pci_probe, +}; +module_pci_driver(imc_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Simone Chifari"); +MODULE_DESCRIPTION("Intel Skylake-X iMC SMBus I2C adapter (ECAM MMIO)"); +MODULE_VERSION("1.0"); diff --git a/test-smoke.sh b/test-smoke.sh new file mode 100755 index 0000000..c80dd06 --- /dev/null +++ b/test-smoke.sh @@ -0,0 +1,80 @@ +#!/usr/bin/env bash +# Smoke test for i2c-imc-skylake (pci_driver, no bus/dev/fn params). +# Run after: make install OR make reload +# Requires: root, i2c-tools (i2cdetect), module already loaded. +set -euo pipefail + +PASS=0 +FAIL=0 + +_ok() { echo "PASS: $*"; ((PASS++)) || true; } +_fail() { echo "FAIL: $*"; ((FAIL++)) || true; } + +echo "=== adapter list ===" +i2cdetect -l | grep -i imc || _fail "no imc adapters visible" + +_bus() { + i2cdetect -l 2>/dev/null | grep -i "imc.*channel $1" | awk '{print $1}' | tr -d 'i2c-' +} + +CH0=$(_bus 0) +CH1=$(_bus 1) + +if [ -n "$CH0" ]; then + _ok "ch0 adapter at i2c-$CH0" +else + _fail "ch0 adapter not found" +fi + +if [ -n "$CH1" ]; then + _ok "ch1 adapter at i2c-$CH1" +else + _fail "ch1 adapter not found" +fi + +if [ -n "$CH0" ]; then + echo "" + echo "=== SPD scan ch0 (expect 0x50-0x57) ===" + i2cdetect -r -y "$CH0" 2>/dev/null + # at least one SPD slot must respond + if i2cdetect -r -y "$CH0" 2>/dev/null | grep '50' >/dev/null; then + _ok "SPD device at 0x50 on ch0" + else + _fail "no SPD device on ch0" + fi +fi + +if [ -n "$CH1" ]; then + echo "" + echo "=== SPD scan ch1 ===" + i2cdetect -r -y "$CH1" 2>/dev/null +fi + +echo "" +echo "=== udev autoload ===" +sudo rmmod i2c-imc-skylake +sleep 1 +sudo udevadm trigger --subsystem-match=pci --action=add +sleep 2 +if lsmod | grep i2c_imc_skylake >/dev/null; then + _ok "udev autoload" +else + _fail "udev did not reload module — check /etc/modules-load.d/ or udev rules" +fi + +echo "" +echo "=== load/unload x5 (leak/oops) ===" +for i in $(seq 1 5); do + sudo rmmod i2c-imc-skylake + sudo modprobe i2c-imc-skylake + echo " cycle $i OK" +done +_ok "5 load/unload cycles completed" + +echo "" +echo "=== dmesg tail ===" +dmesg | tail -20 + +echo "" +echo "=== result: $PASS passed, $FAIL failed ===" +[ "$FAIL" -eq 0 ]